mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 22:56:55 +01:00
knowledge(upgrade): upgrade code must not use ChangeCompany (#210)
* knowledge(upgrade): upgrade code must not use ChangeCompany Addresses ADO bug 651092. Upgrade and feature data update code must run in the context of the company being upgraded; ChangeCompany leaves triggers, events and upgrade tags in the calling company and races the target company's own upgrade. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Address review: self-contained samples, fixture registration, scoped feature routing - Declare the sample table in both companions so each compiles on its own. - Register no-changecompany-in-upgrade and changecompany-runs-triggers-in-the-calling-company in review-fixtures.json. - Limit the Feature Data Update rule to UpdateData/AfterUpdate and their reachable helpers; read-only IsDataUpdateRequired/ReviewData preflight is permitted and shown as a clean control. - Add feature data update execution to al-upgrade-review applicability and not-applicable scope. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Clarify cross-company upgrade sequencing without race claims Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Jesper Schulz-Wedde
GitHub
parent
206c5feff4
commit
488ce50775
6 changed files with 186 additions and 8 deletions
|
|
@ -0,0 +1,41 @@
|
|||
table 50263 "Sales Order Ext"
|
||||
{
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; "Shipping Agent Code"; Code[10]) { TableRelation = "Shipping Agent"; }
|
||||
field(3; "Legacy Carrier Code"; Code[10]) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50261 "Upgrade All Companies"
|
||||
{
|
||||
Subtype = Upgrade;
|
||||
|
||||
trigger OnUpgradePerDatabase()
|
||||
var
|
||||
Company: Record Company;
|
||||
SalesOrderExt: Record "Sales Order Ext";
|
||||
begin
|
||||
// Reaches into every company from one session. Each company also has its own
|
||||
// upgrade session, triggers and subscribers run in the calling context, and a
|
||||
// data error in any company aborts the whole upgrade.
|
||||
if Company.FindSet() then
|
||||
repeat
|
||||
SalesOrderExt.ChangeCompany(Company.Name);
|
||||
SalesOrderExt.SetRange("Shipping Agent Code", '');
|
||||
if SalesOrderExt.FindSet(true) then
|
||||
repeat
|
||||
SalesOrderExt.Validate("Shipping Agent Code", SalesOrderExt."Legacy Carrier Code");
|
||||
SalesOrderExt.Modify(true);
|
||||
until SalesOrderExt.Next() = 0;
|
||||
until Company.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,96 @@
|
|||
table 50262 "Sales Order Ext"
|
||||
{
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; "Shipping Agent Code"; Code[10]) { TableRelation = "Shipping Agent"; }
|
||||
field(3; "Legacy Carrier Code"; Code[10]) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50260 "Upgrade Current Company"
|
||||
{
|
||||
Subtype = Upgrade;
|
||||
|
||||
// The platform runs this trigger once per company, in that company's own session.
|
||||
trigger OnUpgradePerCompany()
|
||||
begin
|
||||
UpgradeShippingAgentCodes();
|
||||
end;
|
||||
|
||||
local procedure UpgradeShippingAgentCodes()
|
||||
var
|
||||
SalesOrderExt: Record "Sales Order Ext";
|
||||
UpgradeTag: Codeunit "Upgrade Tag";
|
||||
begin
|
||||
if UpgradeTag.HasUpgradeTag(ShippingAgentUpgradeTag()) then
|
||||
exit;
|
||||
|
||||
// Only the current company's rows; triggers, events, and the tag all apply here.
|
||||
SalesOrderExt.SetRange("Shipping Agent Code", '');
|
||||
if SalesOrderExt.FindSet(true) then
|
||||
repeat
|
||||
SalesOrderExt.Validate("Shipping Agent Code", SalesOrderExt."Legacy Carrier Code");
|
||||
SalesOrderExt.Modify(true);
|
||||
until SalesOrderExt.Next() = 0;
|
||||
|
||||
UpgradeTag.SetUpgradeTag(ShippingAgentUpgradeTag());
|
||||
end;
|
||||
|
||||
local procedure ShippingAgentUpgradeTag(): Code[250]
|
||||
begin
|
||||
exit('CONTOSO-1001-ShippingAgentCode-20260101');
|
||||
end;
|
||||
}
|
||||
|
||||
codeunit 50264 "Shipping Agent Feat. Data Upd." implements "Feature Data Update"
|
||||
{
|
||||
// Read-only preflight: counting rows across companies to report scope is allowed.
|
||||
procedure IsDataUpdateRequired(): Boolean
|
||||
var
|
||||
Company: Record Company;
|
||||
SalesOrderExt: Record "Sales Order Ext";
|
||||
begin
|
||||
if Company.FindSet() then
|
||||
repeat
|
||||
SalesOrderExt.ChangeCompany(Company.Name);
|
||||
SalesOrderExt.SetRange("Shipping Agent Code", '');
|
||||
if not SalesOrderExt.IsEmpty() then
|
||||
exit(true);
|
||||
until Company.Next() = 0;
|
||||
exit(false);
|
||||
end;
|
||||
|
||||
procedure ReviewData()
|
||||
begin
|
||||
end;
|
||||
|
||||
// Feature Management runs this once per company, in that company.
|
||||
procedure UpdateData(FeatureDataUpdateStatus: Record "Feature Data Update Status")
|
||||
var
|
||||
SalesOrderExt: Record "Sales Order Ext";
|
||||
begin
|
||||
SalesOrderExt.SetRange("Shipping Agent Code", '');
|
||||
if SalesOrderExt.FindSet(true) then
|
||||
repeat
|
||||
SalesOrderExt.Validate("Shipping Agent Code", SalesOrderExt."Legacy Carrier Code");
|
||||
SalesOrderExt.Modify(true);
|
||||
until SalesOrderExt.Next() = 0;
|
||||
end;
|
||||
|
||||
procedure AfterUpdate(FeatureDataUpdateStatus: Record "Feature Data Update Status")
|
||||
begin
|
||||
end;
|
||||
|
||||
procedure GetTaskDescription(): Text
|
||||
begin
|
||||
exit('Copies legacy carrier codes to the shipping agent code.');
|
||||
end;
|
||||
}
|
||||
36
microsoft/knowledge/upgrade/no-changecompany-in-upgrade.md
Normal file
36
microsoft/knowledge/upgrade/no-changecompany-in-upgrade.md
Normal file
|
|
@ -0,0 +1,36 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: upgrade
|
||||
keywords: [changecompany, cross-company, onupgradepercompany, onupgradeperdatabase, feature-data-update, taskscheduler, multi-company, company-context]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Upgrade code must not use ChangeCompany
|
||||
|
||||
## Description
|
||||
|
||||
The platform upgrades each company in its own context: `OnUpgradePerCompany` (like the other `PerCompany` upgrade and install triggers) runs once per company, in a separate system session opened for that company, while `PerDatabase` triggers run in a session that opens no company. Calling `ChangeCompany(<name>)` in upgrade code reaches from the company being upgraded into another company. Cross-company work bypasses the platform's per-company execution boundary, making execution order and migration ownership difficult to reason about and potentially repeating work. A cross-company read cannot assume that the other company's migration has already run, so it can observe pre-upgrade or post-upgrade data. Per-company upgrade tags are set for the current company only, so a tag set after writing to company B is recorded for company A, which can cause B's own session to repeat the migration. Execution context does not move with `ChangeCompany`: table triggers and trigger-event subscribers still run in the calling company (see `changecompany-runs-triggers-in-the-calling-company`). Data and side effects then land in different companies, and an error in another company's data aborts the current company's upgrade. The same applies to the migration path of a feature-switch data update: the `UpdateData` and `AfterUpdate` methods of a `Feature Data Update` implementation. Feature Management runs them for one company's status row, either as a task scheduled in that company or in the current session. The interface's preflight methods, `IsDataUpdateRequired` and `ReviewData`, are a separate phase that reports scope before any update is scheduled.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Upgrade code operates only on the company it is running in. Put per-company data migration in `OnUpgradePerCompany` (or a helper reachable only from it), guard it with a per-company upgrade tag, and let the platform invoke it for every company. Reserve `OnUpgradePerDatabase` for tables with `DataPerCompany = false` and other database-wide state; it needs no `ChangeCompany`. A feature data update implements `UpdateData` and `AfterUpdate` against the current company only. Its read-only preflight may use `ChangeCompany` to count or inspect rows in other companies, because it migrates nothing. When code outside the upgrade pipeline must start work in other companies, schedule it in each company with `TaskScheduler.CreateTask` and the company name, as Feature Management does, rather than writing there through `ChangeCompany`. The scheduled task runs in the target company, so triggers, events, permissions, and upgrade tags all use that company.
|
||||
|
||||
See sample: [`no-changecompany-in-upgrade.good.al`](no-changecompany-in-upgrade.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A loop over the `Company` table in `OnUpgradePerDatabase` or `OnUpgradePerCompany` that calls `ChangeCompany(Company.Name)` on a record or `RecordRef` and then reads, inserts, modifies, or deletes data. Another form is a `Feature Data Update` implementation whose `UpdateData` or `AfterUpdate` does the same to update every company from one task. On these paths reads are not exempt: they observe another company whose upgrade state is unknown.
|
||||
|
||||
Detection signal: `Record.ChangeCompany(<name>)` or `RecordRef.ChangeCompany(<name>)` with a company-name argument in a codeunit with `Subtype = Upgrade` or in any procedure transitively reachable from its upgrade triggers, or in a `Feature Data Update` implementation's `UpdateData` or `AfterUpdate` or any procedure transitively reachable from them. Do not flag `ChangeCompany` in `IsDataUpdateRequired`, `ReviewData`, or helpers reachable only from them; a helper shared with `UpdateData` or `AfterUpdate` is in scope. Do not flag the parameterless `ChangeCompany()`, which only points the variable back at the current company, or `ChangeCompany` in ordinary runtime code; `changecompany-runs-triggers-in-the-calling-company` governs that.
|
||||
|
||||
See sample: [`no-changecompany-in-upgrade.bad.al`](no-changecompany-in-upgrade.bad.al).
|
||||
|
||||
## References
|
||||
|
||||
- Upgrading extensions, Upgrade triggers: `PerCompany` triggers run once per company, each in its own system session for that company — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-upgrading-extensions
|
||||
- Record.ChangeCompany method, Remarks: triggers still run in the current company — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-changecompany-method
|
||||
- Interface "Feature Data Update" — https://learn.microsoft.com/en-us/dynamics365/business-central/application/system-application/interface/system.environment.configuration.feature-data-update
|
||||
- `FeatureManagementImpl.UpdateData` calls the interface's `UpdateData` then `AfterUpdate`; `ReviewData` calls `IsDataUpdateRequired` and `ReviewData` — https://github.com/microsoft/BCApps/blob/main/src/System%20Application/App/Feature%20Key/src/FeatureManagementImpl.Codeunit.al
|
||||
- `FeatureManagementImpl.CreateTask` schedules `Update Feature Data` with `TaskScheduler.CreateTask` for the status row's company — https://github.com/microsoft/BCApps/blob/main/src/System%20Application/App/Feature%20Key/src/FeatureManagementImpl.Codeunit.al
|
||||
Loading…
Add table
Add a link
Reference in a new issue