mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 17:36:53 +01:00
Add lifecycle error and privacy knowledge
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95c06ad8-377d-4faa-8d07-06300b1c81ec
This commit is contained in:
parent
9214f73819
commit
0c2a0ceb82
25 changed files with 457 additions and 37 deletions
|
|
@ -0,0 +1,32 @@
|
|||
codeunit 50303 "Upgrade Phases Bad"
|
||||
{
|
||||
Subtype = Upgrade;
|
||||
|
||||
trigger OnCheckPreconditionsPerCompany()
|
||||
begin
|
||||
// A precondition check must not repair the data it is checking.
|
||||
RenamePostingGroup();
|
||||
end;
|
||||
|
||||
trigger OnValidateUpgradePerCompany()
|
||||
begin
|
||||
// Validation must not perform a migration omitted from OnUpgrade.
|
||||
MigrateCustomerPostingGroups();
|
||||
end;
|
||||
|
||||
local procedure RenamePostingGroup()
|
||||
var
|
||||
CustomerPostingGroup: Record "Customer Posting Group";
|
||||
begin
|
||||
if CustomerPostingGroup.Get('OLD') then
|
||||
CustomerPostingGroup.Rename('NEW');
|
||||
end;
|
||||
|
||||
local procedure MigrateCustomerPostingGroups()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetRange("Customer Posting Group", 'OLD');
|
||||
Customer.ModifyAll("Customer Posting Group", 'NEW');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,63 @@
|
|||
codeunit 50302 "Upgrade Phases Good"
|
||||
{
|
||||
Subtype = Upgrade;
|
||||
|
||||
trigger OnCheckPreconditionsPerCompany()
|
||||
begin
|
||||
CheckTargetPostingGroup();
|
||||
end;
|
||||
|
||||
trigger OnUpgradePerCompany()
|
||||
var
|
||||
UpgradeTag: Codeunit "Upgrade Tag";
|
||||
begin
|
||||
if UpgradeTag.HasUpgradeTag(CustomerPostingGroupTag()) then
|
||||
exit;
|
||||
|
||||
MigrateCustomerPostingGroups();
|
||||
UpgradeTag.SetUpgradeTag(CustomerPostingGroupTag());
|
||||
end;
|
||||
|
||||
trigger OnValidateUpgradePerCompany()
|
||||
begin
|
||||
CheckLegacyPostingGroupsRemoved();
|
||||
end;
|
||||
|
||||
local procedure CheckTargetPostingGroup()
|
||||
var
|
||||
CustomerPostingGroup: Record "Customer Posting Group";
|
||||
begin
|
||||
if not CustomerPostingGroup.Get('NEW') then
|
||||
Error(TargetGroupMissingErr);
|
||||
end;
|
||||
|
||||
local procedure MigrateCustomerPostingGroups()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetRange("Customer Posting Group", 'OLD');
|
||||
if Customer.FindSet(true) then
|
||||
repeat
|
||||
Customer.Validate("Customer Posting Group", 'NEW');
|
||||
Customer.Modify(true);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
|
||||
local procedure CheckLegacyPostingGroupsRemoved()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetRange("Customer Posting Group", 'OLD');
|
||||
if not Customer.IsEmpty() then
|
||||
Error(MigrationIncompleteErr);
|
||||
end;
|
||||
|
||||
local procedure CustomerPostingGroupTag(): Code[250]
|
||||
begin
|
||||
exit('MS-50302-CustomerPostingGroup-20260714');
|
||||
end;
|
||||
|
||||
var
|
||||
MigrationIncompleteErr: Label 'The legacy customer posting group was not migrated.';
|
||||
TargetGroupMissingErr: Label 'Customer posting group NEW must exist before the upgrade.';
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: upgrade
|
||||
keywords: [on-check-preconditions, on-validate-upgrade, on-upgrade, read-only-check, data-migration]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Upgrade check triggers do not migrate data
|
||||
|
||||
## Description
|
||||
|
||||
`OnCheckPreconditionsPerCompany`/`PerDatabase` run before the upgrade to verify that it can start. `OnValidateUpgradePerCompany`/`PerDatabase` run after upgrade logic to verify that it succeeded. Treat both phases as read-only checks. The `OnUpgradePerCompany`/`PerDatabase` phase is where the platform expects actual data transformation.
|
||||
|
||||
## Best Practice
|
||||
|
||||
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`.
|
||||
|
||||
## 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`.
|
||||
|
|
@ -4,8 +4,7 @@ codeunit 50211 "Install My Extension"
|
|||
|
||||
trigger OnInstallAppPerCompany()
|
||||
begin
|
||||
// No DataVersion() guard — this runs on every reinstall and upgrade
|
||||
// path, duplicating seed rows.
|
||||
// No DataVersion() guard: a reinstall duplicates seed rows.
|
||||
SeedDefaultRows();
|
||||
end;
|
||||
|
||||
|
|
|
|||
|
|
@ -11,20 +11,21 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
On the first install of an extension on a tenant the platform records a zero data version: `AppInfo.DataVersion()` returns `Version.Create('0.0.0.0')`. Subsequent upgrades record the actual previous version. The `OnInstallAppPerCompany` trigger uses this distinction to detect a brand-new install — for example, to seed default rows that should not be re-inserted on a normal upgrade. This is the one place where reading `DataVersion()` is the right tool; for everything else, use an upgrade tag.
|
||||
On the first install of an extension on a tenant the platform records a zero data version: `AppInfo.DataVersion()` returns `Version.Create('0.0.0.0')`. During reinstall, `DataVersion()` identifies the previously installed data version. The `OnInstallAppPerCompany` trigger uses this distinction to separate a brand-new install from a reinstall. Ordinary version upgrades do not run install code.
|
||||
|
||||
## Best Practice
|
||||
|
||||
In `OnInstallAppPerCompany`, fetch the current `ModuleInfo` via `NavApp.GetCurrentModuleInfo`, compare `AppInfo.DataVersion()` to `Version.Create('0.0.0.0')`, and run install-only seed logic only when they match. On any non-zero data version, exit immediately — that path is an upgrade, not an install.
|
||||
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`.
|
||||
|
||||
## 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, install-only seed code re-runs on every upgrade and duplicates rows.
|
||||
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 also
|
||||
|
||||
- `use-upgrade-tags-not-version-checks.md` — for upgrade steps after first install, use upgrade tags rather than `DataVersion`.
|
||||
- `install-code-does-not-run-on-version-upgrade.md` — ordinary version upgrades invoke upgrade code, not install code.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,20 @@
|
|||
codeunit 50306 "My App Install Only"
|
||||
{
|
||||
Subtype = Install;
|
||||
|
||||
trigger OnInstallAppPerCompany()
|
||||
begin
|
||||
// A normal version upgrade never invokes this migration.
|
||||
MigrateLegacySetup();
|
||||
end;
|
||||
|
||||
local procedure MigrateLegacySetup()
|
||||
var
|
||||
MyAppSetup: Record "My App Setup";
|
||||
begin
|
||||
if MyAppSetup.Get() then begin
|
||||
MyAppSetup."Configuration Version" := 2;
|
||||
MyAppSetup.Modify(true);
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,48 @@
|
|||
codeunit 50304 "My App Install"
|
||||
{
|
||||
Subtype = Install;
|
||||
|
||||
trigger OnInstallAppPerCompany()
|
||||
begin
|
||||
InitializeSetup();
|
||||
end;
|
||||
|
||||
local procedure InitializeSetup()
|
||||
var
|
||||
MyAppSetup: Record "My App Setup";
|
||||
begin
|
||||
if MyAppSetup.IsEmpty() then
|
||||
MyAppSetup.Insert(true);
|
||||
end;
|
||||
}
|
||||
|
||||
codeunit 50305 "My App Upgrade"
|
||||
{
|
||||
Subtype = Upgrade;
|
||||
|
||||
trigger OnUpgradePerCompany()
|
||||
var
|
||||
UpgradeTag: Codeunit "Upgrade Tag";
|
||||
begin
|
||||
if UpgradeTag.HasUpgradeTag(ConfigurationVersionTag()) then
|
||||
exit;
|
||||
|
||||
MigrateLegacySetup();
|
||||
UpgradeTag.SetUpgradeTag(ConfigurationVersionTag());
|
||||
end;
|
||||
|
||||
local procedure MigrateLegacySetup()
|
||||
var
|
||||
MyAppSetup: Record "My App Setup";
|
||||
begin
|
||||
if MyAppSetup.Get() then begin
|
||||
MyAppSetup."Configuration Version" := 2;
|
||||
MyAppSetup.Modify(true);
|
||||
end;
|
||||
end;
|
||||
|
||||
local procedure ConfigurationVersionTag(): Code[250]
|
||||
begin
|
||||
exit('MS-50305-ConfigurationVersion-20260714');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: upgrade
|
||||
keywords: [install-codeunit, subtype-install, on-install-app, version-upgrade, upgrade-codeunit]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Install code does not run during a version upgrade
|
||||
|
||||
## Description
|
||||
|
||||
An install codeunit runs when an extension is installed for the first time or an uninstalled version is installed again. Installing a higher extension version through the data-upgrade operation does not invoke `OnInstallAppPerCompany` or `OnInstallAppPerDatabase`. Ordinary version-to-version migration is dispatched only through upgrade codeunits.
|
||||
|
||||
## Best Practice
|
||||
|
||||
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`.
|
||||
|
||||
## 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`.
|
||||
|
|
@ -4,10 +4,21 @@ codeunit 50235 "Upgrade With Validation"
|
|||
|
||||
trigger OnValidateUpgradePerCompany()
|
||||
begin
|
||||
// No skip logic and no written justification — full-table validation
|
||||
// runs on every single upgrade pass.
|
||||
// A full-table scan repeats on every upgrade.
|
||||
ValidateAllCustomers();
|
||||
end;
|
||||
|
||||
local procedure ValidateAllCustomers() begin end;
|
||||
local procedure ValidateAllCustomers()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
if Customer."Customer Posting Group" = 'OLD' then
|
||||
Error(MigrationIncompleteErr);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
|
||||
var
|
||||
MigrationIncompleteErr: Label 'The legacy customer posting group was not migrated.';
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,23 +3,19 @@ codeunit 50234 "Upgrade With Validation"
|
|||
Subtype = Upgrade;
|
||||
|
||||
trigger OnValidateUpgradePerCompany()
|
||||
begin
|
||||
CheckNoLegacyPostingGroups();
|
||||
end;
|
||||
|
||||
local procedure CheckNoLegacyPostingGroups()
|
||||
var
|
||||
UpgradeTag: Codeunit "Upgrade Tag";
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
// Justification: regulatory compliance requires a full-table scan once
|
||||
// per tenant after this release. Tag prevents re-runs.
|
||||
if UpgradeTag.HasUpgradeTag(MyValidationUpgradeTag()) then
|
||||
exit;
|
||||
|
||||
ValidateAllCustomers();
|
||||
|
||||
UpgradeTag.SetUpgradeTag(MyValidationUpgradeTag());
|
||||
Customer.SetRange("Customer Posting Group", 'OLD');
|
||||
if not Customer.IsEmpty() then
|
||||
Error(MigrationIncompleteErr);
|
||||
end;
|
||||
|
||||
local procedure ValidateAllCustomers() begin end;
|
||||
|
||||
local procedure MyValidationUpgradeTag(): Code[250]
|
||||
begin
|
||||
exit('MS-123456-CustomerValidation-20240101');
|
||||
end;
|
||||
var
|
||||
MigrationIncompleteErr: Label 'The legacy customer posting group was not migrated.';
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,26 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: upgrade
|
||||
keywords: [on-validate-upgrade-per-company, performance-impact, skip-logic, justification, upgrade-tag]
|
||||
keywords: [on-validate-upgrade-per-company, performance-impact, bounded-query, justification, read-only-check]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Performance-impacting upgrade triggers need justification and skip logic
|
||||
# Keep upgrade validation checks bounded
|
||||
|
||||
## Description
|
||||
|
||||
Triggers such as `OnValidateUpgradePerCompany` run on every upgrade pass. When their body performs non-trivial work — full-table scans, cross-table validations — the cost is paid on every upgrade of every tenant, even when there is nothing to validate. That cost is acceptable only when the validation is critical (regulatory compliance, data-integrity guarantees the platform depends on) AND the trigger short-circuits once it has done its work.
|
||||
Triggers such as `OnValidateUpgradePerCompany` run on every upgrade pass. A full-table scan or cross-table validation therefore adds cost to every upgrade of every tenant. Validation is a read-only lifecycle check, so it cannot make itself one-time by writing an upgrade tag.
|
||||
|
||||
## Best Practice
|
||||
|
||||
A performance-impacting upgrade trigger carries two things: a written comment that names the reason the work has to happen on every upgrade pass, and an early-exit guard backed by an upgrade tag so the work runs at most once per tenant. The `HasUpgradeTag` check at the top exits when the validation has already been recorded; the `SetUpgradeTag` call at the bottom records completion.
|
||||
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`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Doing real work in `OnValidateUpgradePerCompany` with no upgrade-tag guard. The same scan runs every upgrade, multiplying upgrade time by the number of releases the customer takes.
|
||||
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`.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue