diff --git a/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.bad.al b/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.bad.al new file mode 100644 index 0000000..7b8e7d5 --- /dev/null +++ b/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.bad.al @@ -0,0 +1,91 @@ +codeunit 50100 "Transfer Request Bad" +{ + // Self-contained demonstration of the anti pattern. Not derived from base-app source. + procedure RequestFromCompany(TargetCompany: Text[30]; ItemNo: Code[20]; Quantity: Decimal) + var + TransferRequest: Record "Transfer Request Bad"; + TransferSetup: Record "Transfer Setup Bad"; + begin + TransferRequest.ChangeCompany(TargetCompany); + TransferSetup.ChangeCompany(TargetCompany); + TransferSetup.Get(); + + TransferRequest.Init(); + TransferRequest."Entry No." := NextEntryNo(TargetCompany); + TransferRequest."Item No." := ItemNo; + TransferRequest.Quantity := Quantity; + // OnInsert is skipped below, so the default is copied by hand from the target company's setup. + TransferRequest."Location Code" := TransferSetup."Default Location Code"; + // The OnAfterInsertEvent subscriber still fires, in the calling company, and grows the caller's counter. + TransferRequest.Insert(false); + + TransferSetup."Open Requests" += 1; + TransferSetup.Modify(); + end; + + local procedure NextEntryNo(TargetCompany: Text[30]): Integer + var + LastRequest: Record "Transfer Request Bad"; + begin + LastRequest.ChangeCompany(TargetCompany); + if LastRequest.FindLast() then + exit(LastRequest."Entry No." + 1); + exit(1); + end; +} + +table 50100 "Transfer Request Bad" +{ + DataClassification = CustomerContent; + + fields + { + field(1; "Entry No."; Integer) { } + field(2; "Item No."; Code[20]) { } + field(3; Quantity; Decimal) { } + field(4; "Location Code"; Code[10]) { } + } + + keys + { + key(PK; "Entry No.") { Clustered = true; } + } + + trigger OnInsert() + var + TransferSetup: Record "Transfer Setup Bad"; + begin + TransferSetup.Get(); + "Location Code" := TransferSetup."Default Location Code"; + end; +} + +table 50101 "Transfer Setup Bad" +{ + DataClassification = CustomerContent; + + fields + { + field(1; "Primary Key"; Code[10]) { } + field(2; "Default Location Code"; Code[10]) { } + field(3; "Open Requests"; Integer) { } + } + + keys + { + key(PK; "Primary Key") { Clustered = true; } + } +} + +codeunit 50101 "Transfer Request Count Bad" +{ + [EventSubscriber(ObjectType::Table, Database::"Transfer Request Bad", OnAfterInsertEvent, '', false, false)] + local procedure CountOpenRequest(var Rec: Record "Transfer Request Bad"; RunTrigger: Boolean) + var + TransferSetup: Record "Transfer Setup Bad"; + begin + TransferSetup.Get(); + TransferSetup."Open Requests" += 1; + TransferSetup.Modify(); + end; +} diff --git a/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.good.al b/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.good.al new file mode 100644 index 0000000..dd92d9d --- /dev/null +++ b/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.good.al @@ -0,0 +1,84 @@ +codeunit 50100 "Transfer Request Good" +{ + // Self-contained demonstration of the best practice. Not derived from base-app source. + procedure RequestFromCompany(TargetCompany: Text[30]; ItemNo: Code[20]; Quantity: Decimal) + var + TransferRequest: Record "Transfer Request Good"; + SessionId: Integer; + begin + TransferRequest.Init(); + TransferRequest."Item No." := ItemNo; + TransferRequest.Quantity := Quantity; + // The insert runs inside TargetCompany, so OnInsert and the subscriber read that company's setup. + StartSession(SessionId, Codeunit::"Transfer Request Create Good", TargetCompany, TransferRequest); + end; +} + +codeunit 50102 "Transfer Request Create Good" +{ + TableNo = "Transfer Request Good"; + + trigger OnRun() + begin + // "Entry No." is AutoIncrement, so concurrent background sessions in TargetCompany never race on the same value. + Rec.Insert(true); + end; +} + +table 50100 "Transfer Request Good" +{ + DataClassification = CustomerContent; + + fields + { + field(1; "Entry No."; Integer) { AutoIncrement = true; } + field(2; "Item No."; Code[20]) { } + field(3; Quantity; Decimal) { } + field(4; "Location Code"; Code[10]) { } + } + + keys + { + key(PK; "Entry No.") { Clustered = true; } + } + + trigger OnInsert() + var + TransferSetup: Record "Transfer Setup Good"; + begin + TransferSetup.Get(); + "Location Code" := TransferSetup."Default Location Code"; + end; +} + +table 50101 "Transfer Setup Good" +{ + DataClassification = CustomerContent; + + fields + { + field(1; "Primary Key"; Code[10]) { } + field(2; "Default Location Code"; Code[10]) { } + field(3; "Open Requests"; Integer) { } + } + + keys + { + key(PK; "Primary Key") { Clustered = true; } + } +} + +codeunit 50101 "Transfer Request Count Good" +{ + [EventSubscriber(ObjectType::Table, Database::"Transfer Request Good", OnAfterInsertEvent, '', false, false)] + local procedure CountOpenRequest(var Rec: Record "Transfer Request Good"; RunTrigger: Boolean) + var + TransferSetup: Record "Transfer Setup Good"; + begin + // Serializes the read-modify-write so concurrent background sessions don't lose an increment. + TransferSetup.LockTable(); + TransferSetup.Get(); + TransferSetup."Open Requests" += 1; + TransferSetup.Modify(); + end; +} diff --git a/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md b/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md new file mode 100644 index 0000000..4a524f4 --- /dev/null +++ b/microsoft/knowledge/events/changecompany-runs-triggers-in-the-calling-company.md @@ -0,0 +1,42 @@ +--- +bc-version: [all] +domain: events +keywords: [changecompany, cross-company, runtrigger, trigger-event, subscriber, onafterinsertevent, insert, startsession, multi-company] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# ChangeCompany leaves triggers and trigger-event subscribers running in the calling company + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +`ChangeCompany` redirects the data access of one record variable to another company's table. Execution context does not move with it: Microsoft Learn states that triggers still run in the current company, not in the company passed to `ChangeCompany`. Code that knows this usually reaches for `Insert(false)` and copies the trigger's work by hand from the target company's setup. That closes only half of the gap. The runtime raises the database trigger events (`OnBeforeInsertEvent`, `OnAfterInsertEvent`, and their modify, delete, and rename counterparts) on every database operation and only passes the `RunTrigger` flag to the subscriber, so every subscriber that does not exit on `RunTrigger = false` still runs, in the calling company, against the calling company's setup, number series, and companion tables. The row lands in the target company, the side effects land in the caller, and nothing reports an error. The per-row cost of the call is a separate concern, see `changecompany-in-loop-drops-caches`. + +## Best Practice + +Use `ChangeCompany` to read. Access rights in the target company are still enforced, so reads are safe. When the goal is business data in another company, run the code in that company: `StartSession` takes a company name and runs a codeunit there, so triggers, validation, and subscribers all execute with the target company as their context. `StartSession` is a background session, not a synchronous call: the `Ok` return value reports only whether the session started, not whether the codeunit's work inside it succeeded, the caller's transaction does not extend into it, and an error raised there does not come back to the caller — it has to be logged or telemetered from inside that session. Reach for `StartSession` only for work the caller does not need to confirm before it continues; a write whose success the caller must know synchronously needs a durable status or error channel (a field the caller polls, a job queue with retry) rather than a bare `StartSession` call. Learn notes that a background session costs as much as a user session to start, so batch the work rather than starting one session per row, or let the target company process a hand-off row on its own schedule. A direct cross-company write is acceptable only as such a hand-off into a table the writing extension owns, whose triggers do not read company data and whose trigger-event subscribers exit when `RunTrigger` is false, using `Insert(false)`, `Modify(false)`, or `Delete(false)`, and never `Validate`. + +See sample: [`changecompany-runs-triggers-in-the-calling-company.good.al`](changecompany-runs-triggers-in-the-calling-company.good.al). + +## Anti Pattern + +An `Insert`, `Modify`, `Delete`, or `Validate` on a record variable after `ChangeCompany()`, on a table whose triggers or trigger-event subscribers read setup, consume a number series, or write companion rows. With `RunTrigger = true` the trigger code fills the row from the caller's setup. With `RunTrigger = false` the trigger code is skipped, but the subscribers still fire in the caller, so a counter, log, or companion row maintained by a subscriber is written in the wrong company, and a caller that also updates the target by hand counts twice. + +Detection signal: a record variable that has had `ChangeCompany` called on it with a company name and is later used with `Insert`, `Modify`, `Delete`, or `Validate`, where the table is not owned by the extension, or has triggers that read company data, or has trigger-event subscribers that do not exit on `RunTrigger = false`. Do not flag reads after `ChangeCompany`; writes with `RunTrigger = false` into an owned table whose triggers do not read company data and whose subscribers exit on `RunTrigger = false`; or `ChangeCompany()` without an argument, which points the variable back at the current company. + +See sample: [`changecompany-runs-triggers-in-the-calling-company.bad.al`](changecompany-runs-triggers-in-the-calling-company.bad.al). + +## See also + +- Record.ChangeCompany method, Remarks — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-changecompany-method +- Record.Insert(Boolean) method, RunTrigger — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-insert-boolean-method +- Record.Delete method, RunTrigger defaults to false — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-delete-method +- OnInsert (Table) trigger, Remarks — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/triggers-auto/table/devenv-oninsert-table-trigger +- Event types, Database trigger events and order of event execution — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-event-types +- OnAfterInsertEvent trigger event, RunTrigger parameter — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/triggers-auto/events/table/devenv-onafterinsertevent-table-trigger +- Session.StartSession method, Company parameter, Remarks (background session, no UI), and Return Value (`Ok` reports whether the session started, not whether the codeunit's work succeeded) — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/session/session-startsession-integer-integer-string-table-method +- AL error handling, error handling strategies: an error inside a rolled-back transaction is logged from a background session or telemetry, it does not return to the caller — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-al-error-handling +- AutoIncrement property, Remarks: "if several transactions are performed at the same time, they will each be assigned a different number" — https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/properties/devenv-autoincrement-property diff --git a/microsoft/skills/review/al-events-review.md b/microsoft/skills/review/al-events-review.md index 68bcdb0..b257d3d 100644 --- a/microsoft/skills/review/al-events-review.md +++ b/microsoft/skills/review/al-events-review.md @@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed AL object names and types — especially codeunits that publish events or host event subscribers, posting/release/validation routines that should expose extension points, and test codeunits that bind subscribers. - The changed procedures and triggers, weighted toward event publisher methods, methods carrying the `[EventSubscriber(...)]` attribute, routines that raise `OnBefore`/`OnAfter` events, and any procedure that calls `BindSubscription`/`UnbindSubscription`. -- Tokens extracted from the diff that relate to events and the publish/subscribe model (`IntegrationEvent`, `BusinessEvent`, `InternalEvent`, `EventSubscriber`, `IsHandled`, `BindSubscription`, `UnbindSubscription`, `EventSubscriberInstance`, `OnBefore`, `OnAfter`, `Manual`, `IncludeSender`, `GlobalVarAccess`, `Isolated`, `local`, `internal`, `Sender`, `this`, `RecordRef`, `xRec`, `temporary`, `Temp`, `repeat`). +- Tokens extracted from the diff that relate to events and the publish/subscribe model (`IntegrationEvent`, `BusinessEvent`, `InternalEvent`, `EventSubscriber`, `IsHandled`, `BindSubscription`, `UnbindSubscription`, `EventSubscriberInstance`, `OnBefore`, `OnAfter`, `Manual`, `IncludeSender`, `GlobalVarAccess`, `Isolated`, `local`, `internal`, `Sender`, `this`, `RecordRef`, `xRec`, `temporary`, `Temp`, `repeat`, `ChangeCompany`, `StartSession`, `RunTrigger`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. @@ -65,6 +65,7 @@ The following targeted checks map diff signals to specific `events` articles. Tr - A `RecordRef` event parameter, or a passed-through `xRec`, where a concrete typed record fits — `avoid-loosely-typed-event-parameters`. - A `var IsHandled` added to a pre-existing event rather than introduced through a new `OnBefore` publisher — `do-not-add-ishandled-to-an-existing-event`. - An `if IsHandled then exit;` whose skipped body performs posting, ledger-entry creation, number-series consumption, or integrity/permission validation — `do-not-bypass-critical-operations-with-ishandled`. +- A record variable that had `ChangeCompany()` called on it and is later used with `Insert`, `Modify`, `Delete`, or `Validate`, where the table is not owned by the extension, has triggers that read company data, or has trigger-event subscribers that do not exit on `RunTrigger = false` — `changecompany-runs-triggers-in-the-calling-company`. Do not match a read-only use after `ChangeCompany`, a write with `RunTrigger = false` into an extension-owned table whose triggers do not read company data and whose trigger-event subscribers exit on `RunTrigger = false`, or the parameterless `ChangeCompany()` reset. ## Action