mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Merge main into AL development guidance
Reconcile the read-only plan-enrichment contracts with main's folder-review inputs and documentation structure. Record Windows alternate streams in runner evidence and clear the regression harness exit status after expected negative probes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 638b66d2-9f06-4f60-8781-808709e1485c
This commit is contained in:
commit
332947bcdb
298 changed files with 1818 additions and 780 deletions
|
|
@ -17,10 +17,10 @@ Primary-key changes and field-type changes (for example widening `Integer` to `B
|
|||
|
||||
Treat primary-key and field-type changes as restricted to tables introduced in the same change. For changes on tables with existing data, design and ship the corresponding upgrade procedure (typically backed by `DataTransfer` and an upgrade tag) that guarantees the new layout is achievable for every row, and verify with concrete evidence that the existing values fit the new constraint (no PK collisions, no value-range overflow).
|
||||
|
||||
See sample: `breaking-changes-only-on-tables-without-data.good.al`.
|
||||
See sample: [`breaking-changes-only-on-tables-without-data.good.al`](breaking-changes-only-on-tables-without-data.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Changing the primary key on a base-app table, or widening / narrowing a field type on a table that has been shipping for releases, with no accompanying upgrade plan. The change compiles cleanly and may even deploy on an empty-ish tenant, then fails on customers who actually have data.
|
||||
|
||||
See sample: `breaking-changes-only-on-tables-without-data.bad.al`.
|
||||
See sample: [`breaking-changes-only-on-tables-without-data.bad.al`](breaking-changes-only-on-tables-without-data.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ application-area: [all]
|
|||
|
||||
Have check triggers call query-only helpers that raise an error when an invariant fails. Put every `Insert`, `Modify`, `Delete`, `Rename`, `DataTransfer`, and other migration write behind helpers called from the matching `OnUpgrade...` trigger.
|
||||
|
||||
See sample: `check-only-triggers-do-not-migrate-data.good.al`.
|
||||
See sample: [`check-only-triggers-do-not-migrate-data.good.al`](check-only-triggers-do-not-migrate-data.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Repairing data in `OnCheckPreconditions...` or finishing migration in `OnValidateUpgrade...`. Those writes blur the phase contract and make a check alter the state it is supposed to assess.
|
||||
|
||||
See sample: `check-only-triggers-do-not-migrate-data.bad.al`.
|
||||
See sample: [`check-only-triggers-do-not-migrate-data.bad.al`](check-only-triggers-do-not-migrate-data.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,13 +17,13 @@ Tables that can contain more than 300,000 records, and any newly added field on
|
|||
|
||||
For a bulk update use a `DataTransfer` variable: call `SetTables(Database::"...", Database::"...")` (source and destination may be the same table), add filters with `AddSourceFilter`, set the target value with `AddConstantValue` (or copy a source field with `AddFieldValue`), and execute with `CopyFields()`. To express multiple distinct updates against the same table, `Clear` the `DataTransfer` between executions and configure the next one.
|
||||
|
||||
See sample: `datatransfer-for-bulk-init.good.al`.
|
||||
See sample: [`datatransfer-for-bulk-init.good.al`](datatransfer-for-bulk-init.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Iterating with `FindSet(true) ... repeat ... Modify() ... until Next() = 0` to set a single field across an entire large table. On 300k+ rows this is the canonical slow-upgrade footgun.
|
||||
|
||||
See sample: `datatransfer-for-bulk-init.bad.al`.
|
||||
See sample: [`datatransfer-for-bulk-init.bad.al`](datatransfer-for-bulk-init.bad.al).
|
||||
|
||||
## See also
|
||||
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ For *new fields and tables added in the same change* this is fine: nothing yet d
|
|||
|
||||
Use `DataTransfer` when set-based transfer is safe and row-level business logic is intentionally unnecessary — initial population of a new field is the canonical case. When an existing field's validation must run, loop through records and call `Validate(Field, Value)`; if the table's modify trigger must also run, follow with `Modify(true)`. If performance requires `DataTransfer`, document exactly which field-validation and row-modification triggers or subscribers are intentionally bypassed and verify that derived data remains correct.
|
||||
|
||||
See sample: `datatransfer-skips-triggers-and-subscribers.good.al`.
|
||||
See sample: [`datatransfer-skips-triggers-and-subscribers.good.al`](datatransfer-skips-triggers-and-subscribers.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Reaching for `DataTransfer` to update an existing field with non-trivial `OnValidate` or `OnModify` logic, without confirming that both validation and row-modification subscribers can be skipped. Replacing it with only `Modify(true)` is also incomplete when field validation is required; call `Validate` for that field first.
|
||||
|
||||
See sample: `datatransfer-skips-triggers-and-subscribers.bad.al`.
|
||||
See sample: [`datatransfer-skips-triggers-and-subscribers.bad.al`](datatransfer-skips-triggers-and-subscribers.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ When upgrade code encounters unexpected data — a record it expected to find, a
|
|||
|
||||
When an upgrade procedure detects something missing, call `Session.LogMessage` with a stable event ID, classify the message verbosity (typically `Warning`), and `exit` the procedure so the rest of the upgrade can proceed. The platform telemetry then surfaces the situation to the partner without breaking the customer.
|
||||
|
||||
See sample: `do-not-block-upgrade-on-data-errors.good.al`.
|
||||
See sample: [`do-not-block-upgrade-on-data-errors.good.al`](do-not-block-upgrade-on-data-errors.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `Record.Get(Key)` (or any other erroring API) and letting the error propagate out of the upgrade trigger. The first tenant with imperfect data fails to upgrade, and the failure surfaces as a hard upgrade error rather than as a telemetry signal.
|
||||
|
||||
See sample: `do-not-block-upgrade-on-data-errors.bad.al`.
|
||||
See sample: [`do-not-block-upgrade-on-data-errors.bad.al`](do-not-block-upgrade-on-data-errors.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,13 +17,13 @@ An AL `enum` is a fixed list of ordinal-named values. Persisted rows reference e
|
|||
|
||||
When adding an enum value, place it after the last existing `value(N; ...)` entry, with an ordinal strictly greater than every existing one. Never renumber existing entries. To retire a value, do not delete it: mark it `ObsoleteState = Pending` (and later `Removed`) with `ObsoleteReason` and `ObsoleteTag` so the ordinal remains taken.
|
||||
|
||||
See sample: `enum-values-additive-at-end.good.al`.
|
||||
See sample: [`enum-values-additive-at-end.good.al`](enum-values-additive-at-end.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Inserting a value between existing entries ("just put `NewMiddleValue` between `First` and `Second`"), or removing a value from the enum without first going through `ObsoleteState = Pending` → `Removed`. Every row whose persisted ordinal matched the removed or shifted value now reads as a different member.
|
||||
|
||||
See sample: `enum-values-additive-at-end.bad.al`.
|
||||
See sample: [`enum-values-additive-at-end.bad.al`](enum-values-additive-at-end.bad.al).
|
||||
|
||||
## See also
|
||||
|
||||
|
|
|
|||
|
|
@ -17,13 +17,13 @@ On the first install of an extension on a tenant the platform records a zero dat
|
|||
|
||||
In `OnInstallAppPerCompany`, fetch the current `ModuleInfo` via `NavApp.GetCurrentModuleInfo`, compare `AppInfo.DataVersion()` to `Version.Create('0.0.0.0')`, and run first-install seed logic only when they match. On a non-zero data version, follow the reinstall path or exit.
|
||||
|
||||
See sample: `first-install-dataversion-zero-check.good.al`.
|
||||
See sample: [`first-install-dataversion-zero-check.good.al`](first-install-dataversion-zero-check.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Treating `OnInstallAppPerCompany` as if it always implies "fresh tenant". The trigger also fires when reinstalling over an existing data set; without the `0.0.0.0` guard, first-install seed code can run again and duplicate rows.
|
||||
|
||||
See sample: `first-install-dataversion-zero-check.bad.al`.
|
||||
See sample: [`first-install-dataversion-zero-check.bad.al`](first-install-dataversion-zero-check.bad.al).
|
||||
|
||||
## See also
|
||||
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ Inside an upgrade codeunit (or any procedure transitively invoked from `OnUpgrad
|
|||
|
||||
Wrap every read in an `if`. `if Item.Get(No) then ...`, `if Customer.FindSet() then;`, `if not Vendor.FindLast() then exit;`. The empty-then form `if Customer.FindSet() then;` is the idiomatic way to attempt a read whose only purpose is to position a record, while swallowing the "not found" case.
|
||||
|
||||
See sample: `guard-database-reads.good.al`.
|
||||
See sample: [`guard-database-reads.good.al`](guard-database-reads.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `Item.Get()`, `Customer.FindSet()`, or `Vendor.FindLast()` bare in upgrade code. The first tenant whose data does not match the upgrade's assumptions will fail to upgrade.
|
||||
|
||||
See sample: `guard-database-reads.bad.al`.
|
||||
See sample: [`guard-database-reads.bad.al`](guard-database-reads.bad.al).
|
||||
|
|
|
|||
|
|
@ -23,10 +23,10 @@ Several legitimate cases do NOT need upgrade code:
|
|||
|
||||
When a new field on an existing table has an `InitValue` that matters, ship an upgrade procedure that walks the existing rows and sets the field to the same value — typically via `DataTransfer.AddConstantValue` for performance — guarded by an upgrade tag.
|
||||
|
||||
See sample: `initvalue-does-not-update-existing-rows.good.al`.
|
||||
See sample: [`initvalue-does-not-update-existing-rows.good.al`](initvalue-does-not-update-existing-rows.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Adding a field with `InitValue = true;` (or any non-default `InitValue`) and shipping no upgrade code. Existing rows silently carry the datatype default, leaving the table in two states: rows created before the upgrade with the wrong value, and rows created after with the right one.
|
||||
|
||||
See sample: `initvalue-does-not-update-existing-rows.bad.al`.
|
||||
See sample: [`initvalue-does-not-update-existing-rows.bad.al`](initvalue-does-not-update-existing-rows.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ An install codeunit runs when an extension is installed for the first time or an
|
|||
|
||||
Use `Subtype = Install` for first-install and reinstall initialization. Put version migration in a separate `Subtype = Upgrade` codeunit and enter it from `OnUpgradePerCompany` or `OnUpgradePerDatabase`.
|
||||
|
||||
See sample: `install-code-does-not-run-on-version-upgrade.good.al`.
|
||||
See sample: [`install-code-does-not-run-on-version-upgrade.good.al`](install-code-does-not-run-on-version-upgrade.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Putting a schema or data migration only in an install trigger and expecting it to run when a higher app version is upgraded. The migration is never invoked on that path.
|
||||
|
||||
See sample: `install-code-does-not-run-on-version-upgrade.bad.al`.
|
||||
See sample: [`install-code-does-not-run-on-version-upgrade.bad.al`](install-code-does-not-run-on-version-upgrade.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ Triggers such as `OnValidateUpgradePerCompany` run on every upgrade pass. A full
|
|||
|
||||
Filter directly to invalid rows and use `IsEmpty` or another bounded existence check where possible. If a broad validation is unavoidable, document the invariant that requires it and keep all data changes in `OnUpgrade...`.
|
||||
|
||||
See sample: `minimize-onvalidate-upgrade-triggers.good.al`.
|
||||
See sample: [`minimize-onvalidate-upgrade-triggers.good.al`](minimize-onvalidate-upgrade-triggers.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Reading every record in `OnValidateUpgradePerCompany` when a filtered existence check can prove the same invariant. The scan repeats on every upgrade.
|
||||
|
||||
See sample: `minimize-onvalidate-upgrade-triggers.bad.al`.
|
||||
See sample: [`minimize-onvalidate-upgrade-triggers.bad.al`](minimize-onvalidate-upgrade-triggers.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ The rule applies inside any codeunit with `Subtype = Upgrade` and to any procedu
|
|||
|
||||
Defer external calls to runtime code. If a piece of upgrade work conceptually needs data from an external service, set a flag or write a queue row during upgrade and have the runtime code make the call later (for example on first user sign-in or via job queue), where retries and degraded modes are tractable.
|
||||
|
||||
See sample: `no-external-calls-in-upgrade.good.al`.
|
||||
See sample: [`no-external-calls-in-upgrade.good.al`](no-external-calls-in-upgrade.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `HttpClient.Get`, `HttpClient.Post`, or DotNet interop methods from `OnUpgradePerCompany`, `OnUpgradePerDatabase`, or any procedure they invoke.
|
||||
|
||||
See sample: `no-external-calls-in-upgrade.bad.al`.
|
||||
See sample: [`no-external-calls-in-upgrade.bad.al`](no-external-calls-in-upgrade.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ application-area: [all]
|
|||
|
||||
Stage the deprecation across releases. Step 1: mark `Pending` with reason and tag; consumers are warned but data and code keep working. Step 2: in a later release, transition to `Removed` and (if persisted data references the element) ship an upgrade procedure that migrates that data — gated by an upgrade tag. The standard mechanic for retiring the actual implementation body is to remove the `#if not CLEAN<version>` block in the same release that flips the state to `Removed`.
|
||||
|
||||
See sample: `obsolete-pending-to-removed-staging.good.al`.
|
||||
See sample: [`obsolete-pending-to-removed-staging.good.al`](obsolete-pending-to-removed-staging.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Jumping straight to `ObsoleteState = Removed` without a prior `Pending` release. Consumers have no deprecation window to migrate and any data still referencing the element is stranded. Equally wrong: leaving an element `Pending` indefinitely and never staging its removal — the deprecation never completes.
|
||||
|
||||
See sample: `obsolete-pending-to-removed-staging.bad.al`.
|
||||
See sample: [`obsolete-pending-to-removed-staging.bad.al`](obsolete-pending-to-removed-staging.bad.al).
|
||||
|
|
|
|||
|
|
@ -22,13 +22,13 @@ In both forms, the reason should name the replacement and the tag should identif
|
|||
|
||||
For an object or field, set all three properties together. For a method, variable, or event, provide both `[Obsolete]` arguments. Keep the original tag stable through the lifecycle rather than changing it to a planned removal version.
|
||||
|
||||
See sample: `obsoletion-requires-reason-and-tag.good.al`.
|
||||
See sample: [`obsoletion-requires-reason-and-tag.good.al`](obsoletion-requires-reason-and-tag.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Setting only `ObsoleteState = Pending`/`Removed` on an object or field, or using `[Obsolete('', '')]` on a method, variable, or event. Both forms produce deprecation metadata without useful replacement guidance or traceability.
|
||||
|
||||
See sample: `obsoletion-requires-reason-and-tag.bad.al`.
|
||||
See sample: [`obsoletion-requires-reason-and-tag.bad.al`](obsoletion-requires-reason-and-tag.bad.al).
|
||||
|
||||
## See also
|
||||
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ Registration is not install-time seeding. When an extension is installed into an
|
|||
|
||||
In the upgrade codeunit, guard work with `HasUpgradeTag` and call `SetUpgradeTag` only after successful completion. Seed the same tag explicitly from `OnInstallAppPerCompany` when first-install logic should not run as a later upgrade. Also add historical per-company tags to `OnGetPerCompanyUpgradeTags` so `SetAllUpgradeTags` marks them complete for newly created companies. Keep the tag definition shared so all paths use the exact same value.
|
||||
|
||||
See sample: `register-upgrade-tags-with-subscribers.good.al`.
|
||||
See sample: [`register-upgrade-tags-with-subscribers.good.al`](register-upgrade-tags-with-subscribers.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Assuming an `OnGetPerCompanyUpgradeTags` subscriber sets tags during extension installation, or omitting the subscriber and allowing old upgrade steps to run when `SetAllUpgradeTags` initializes a new company. The subscriber supplies a list; only `SetAllUpgradeTags` or an explicit `SetUpgradeTag` call persists it.
|
||||
|
||||
See sample: `register-upgrade-tags-with-subscribers.bad.al`.
|
||||
See sample: [`register-upgrade-tags-with-subscribers.bad.al`](register-upgrade-tags-with-subscribers.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ This is the opposite of a load-bearing concern: code that MUST run during the up
|
|||
|
||||
In a runtime procedure that performs non-essential side effects, guard the side-effect block with `if GetExecutionContext() = ExecutionContext::Upgrade then exit;` and include a brief comment explaining what is being skipped and why.
|
||||
|
||||
See sample: `skip-nonessential-work-via-execution-context.good.al`.
|
||||
See sample: [`skip-nonessential-work-via-execution-context.good.al`](skip-nonessential-work-via-execution-context.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Using `GetExecutionContext()` to *enable* upgrade behaviour from outside an upgrade codeunit. Upgrade behaviour belongs in a codeunit with `Subtype = Upgrade`; runtime code should only use the check to *suppress* optional work.
|
||||
|
||||
See sample: `skip-nonessential-work-via-execution-context.bad.al`.
|
||||
See sample: [`skip-nonessential-work-via-execution-context.bad.al`](skip-nonessential-work-via-execution-context.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ Empty `OnUpgradePerCompany` / `OnUpgradePerDatabase` triggers are acceptable —
|
|||
|
||||
Each upgrade trigger contains an ordered list of procedure calls, one per feature: `UpgradeFeatureA();` `UpgradeFeatureB();`. Each procedure handles its own upgrade tag, its own data work, and can be added or removed independently.
|
||||
|
||||
See sample: `triggers-call-helpers-not-implementations.good.al`.
|
||||
See sample: [`triggers-call-helpers-not-implementations.good.al`](triggers-call-helpers-not-implementations.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Implementing record loops, `ModifyAll`, or other data work directly in the trigger body. The trigger then mixes orchestration with implementation, and adding a second feature requires editing the trigger rather than appending one line.
|
||||
|
||||
See sample: `triggers-call-helpers-not-implementations.bad.al`.
|
||||
See sample: [`triggers-call-helpers-not-implementations.bad.al`](triggers-call-helpers-not-implementations.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ A codeunit only participates in the upgrade pipeline when it sets `Subtype = Upg
|
|||
|
||||
Place every piece of upgrade logic in a codeunit declared with `Subtype = Upgrade;` and expose entry points via the two triggers `OnUpgradePerCompany` and `OnUpgradePerDatabase`. Helper procedures may live in normal codeunits, but they inherit the upgrade-context rules (guarded reads, no external calls, upgrade tags, etc.) when called from an upgrade trigger.
|
||||
|
||||
See sample: `upgrade-codeunit-subtype.good.al`.
|
||||
See sample: [`upgrade-codeunit-subtype.good.al`](upgrade-codeunit-subtype.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Putting upgrade-style logic in a regular codeunit that the platform never invokes during upgrade — for example a normal codeunit with a manually invented "RunUpgrade" procedure that nothing wires to the upgrade pipeline. The migration code will simply not run.
|
||||
|
||||
See sample: `upgrade-codeunit-subtype.bad.al`.
|
||||
See sample: [`upgrade-codeunit-subtype.bad.al`](upgrade-codeunit-subtype.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,13 +17,13 @@ Each piece of upgrade logic must run exactly once per company (or database) acro
|
|||
|
||||
Every upgrade procedure starts with a `HasUpgradeTag` guard and ends with `SetUpgradeTag` once the work is committed. Each feature gets its own tag string so features can be re-run independently if needed.
|
||||
|
||||
See sample: `use-upgrade-tags-not-version-checks.good.al`.
|
||||
See sample: [`use-upgrade-tags-not-version-checks.good.al`](use-upgrade-tags-not-version-checks.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Branching on `MyApp.DataVersion().Major > N`, or chains of `< N` / `< M` to decide which upgrade step to run. Such code becomes unmaintainable after a few releases and silently does the wrong thing on tenants that skip versions.
|
||||
|
||||
See sample: `use-upgrade-tags-not-version-checks.bad.al`.
|
||||
See sample: [`use-upgrade-tags-not-version-checks.bad.al`](use-upgrade-tags-not-version-checks.bad.al).
|
||||
|
||||
## See also
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue