mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Correct event compatibility guidance
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 645349fd-1892-48f3-8a84-db77d6abd1c3
This commit is contained in:
parent
23af51e02d
commit
6c5133fe7e
9 changed files with 54 additions and 47 deletions
|
|
@ -1,21 +1,14 @@
|
||||||
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
// Demonstration-only AL. Version 1 exposed PostDocument(SalesHeader).
|
||||||
codeunit 50251 "Param Append Bad Sample"
|
codeunit 50251 "Param Append Bad Sample"
|
||||||
{
|
{
|
||||||
procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
procedure PostDocument(var SalesHeader: Record "Sales Header")
|
||||||
var
|
|
||||||
IsHandled: Boolean;
|
|
||||||
begin
|
begin
|
||||||
IsHandled := false;
|
// Existing callers cannot supply the newly required argument.
|
||||||
// Anti-pattern: 'CalledFromBatch' was inserted before the existing
|
OnBeforePostDocument(SalesHeader);
|
||||||
// IsHandled parameter, shifting it and breaking the argument positions
|
|
||||||
// every existing subscriber relied on.
|
|
||||||
OnBeforePostDocument(SalesHeader, CalledFromBatch, IsHandled);
|
|
||||||
if IsHandled then
|
|
||||||
exit;
|
|
||||||
end;
|
end;
|
||||||
|
|
||||||
[IntegrationEvent(false, false)]
|
[IntegrationEvent(false, false)]
|
||||||
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean; var IsHandled: Boolean)
|
procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
||||||
begin
|
begin
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
// Demonstration-only AL. Version 1 had SalesHeader and IsHandled parameters.
|
||||||
codeunit 50250 "Param Append Good Sample"
|
codeunit 50250 "Param Append Good Sample"
|
||||||
{
|
{
|
||||||
procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
||||||
|
|
@ -6,15 +6,24 @@ codeunit 50250 "Param Append Good Sample"
|
||||||
IsHandled: Boolean;
|
IsHandled: Boolean;
|
||||||
begin
|
begin
|
||||||
IsHandled := false;
|
IsHandled := false;
|
||||||
// This local event can gain an optional trailing subscriber parameter.
|
// Subscribers bind by name, so the new parameter can sit between the
|
||||||
OnBeforePostDocument(SalesHeader, IsHandled, CalledFromBatch);
|
// existing parameters without breaking subscribers that omit it.
|
||||||
|
OnBeforePostDocument(SalesHeader, CalledFromBatch, IsHandled);
|
||||||
if IsHandled then
|
if IsHandled then
|
||||||
exit;
|
exit;
|
||||||
end;
|
end;
|
||||||
|
|
||||||
// Public events cannot use this evolution: dependent apps may raise them.
|
|
||||||
[IntegrationEvent(false, false)]
|
[IntegrationEvent(false, false)]
|
||||||
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean; CalledFromBatch: Boolean)
|
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean; var IsHandled: Boolean)
|
||||||
begin
|
begin
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
codeunit 50252 "Existing Param Subscriber"
|
||||||
|
{
|
||||||
|
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Param Append Good Sample", 'OnBeforePostDocument', '', false, false)]
|
||||||
|
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
IsHandled := SalesHeader."No." = '';
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,26 +1,26 @@
|
||||||
---
|
---
|
||||||
bc-version: [all]
|
bc-version: [all]
|
||||||
domain: events
|
domain: events
|
||||||
keywords: [event-parameters, signature, backward-compatibility, append, onbefore, integration-event, versioning]
|
keywords: [event-parameters, signature, backward-compatibility, public-event, local-event, internal-event, appsourcecop, as0024, as0025]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
countries: [w1]
|
countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
# Add new event parameters at the end
|
# Event parameter additions depend on publisher access, not position
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
Adding a parameter to an existing event publisher changes its signature. For a `local` or `internal` Business or Integration event, subscribers may omit parameters, so a new parameter can be compatible when appended at the end. A public event is also a public procedure that dependent extensions can raise; adding a parameter to it is breaking and requires a new event. Existing parameters must never be renamed, removed, reordered, or have their type changed.
|
Event subscribers bind publisher parameters by name and can omit parameters they do not use. A `local` or `internal` Business or Integration event can therefore gain a parameter at any position without breaking subscriber-only consumers; appending is not a compatibility requirement. A public event is also a public procedure that dependent extensions can raise, so adding a required parameter anywhere breaks callers under AppSourceCop AS0024.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
When extending an existing `local` or `internal` Business or Integration event, append the new parameter after all existing ones, including after a trailing `var IsHandled: Boolean` when present. Existing subscribers can continue omitting the new trailing parameter. Create a new event instead when the publisher procedure is public.
|
Add a parameter directly only when the shipped event publisher is `local` or `internal`. Place it where the signature is clearest; existing subscribers continue binding the parameters they name. For a public event, keep the original publisher unchanged and introduce a new event with the expanded contract.
|
||||||
|
|
||||||
See sample: `add-new-event-parameters-at-the-end.good.al`.
|
See sample: `add-new-event-parameters-at-the-end.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Inserting a new parameter before an existing parameter of a `local` or `internal` event, or adding any parameter to a public event. Detection: a changed event signature where a new parameter is not a compatible trailing addition.
|
Appending a parameter to a public event and assuming its position makes the change compatible. Existing external callers still lack the new required argument. Conversely, do not flag a parameter inserted among existing parameters on a `local` or `internal` Business or Integration event merely because it was not appended.
|
||||||
|
|
||||||
See sample: `add-new-event-parameters-at-the-end.bad.al`.
|
See sample: `add-new-event-parameters-at-the-end.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -8,7 +8,7 @@ codeunit 50530 "Shipment Events"
|
||||||
end;
|
end;
|
||||||
|
|
||||||
// Preserve the shipped attribute contract.
|
// Preserve the shipped attribute contract.
|
||||||
[IntegrationEvent(true, false, false)]
|
[IntegrationEvent(true, true, false)]
|
||||||
local procedure OnShipmentCreated(ShipmentNo: Code[20])
|
local procedure OnShipmentCreated(ShipmentNo: Code[20])
|
||||||
begin
|
begin
|
||||||
end;
|
end;
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
---
|
---
|
||||||
bc-version: [20..]
|
bc-version: [all]
|
||||||
domain: events
|
domain: events
|
||||||
keywords: [event-attribute, includesender, globalvaraccess, isolated-event, compatibility, integration-event, business-event, appsourcecop, as0021, as0101]
|
keywords: [event-attribute, includesender, globalvaraccess, isolated-event, compatibility, integration-event, business-event, appsourcecop, as0021, as0101]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
`IncludeSender`, `GlobalVarAccess`, and `Isolated` affect a subscriber contract, not just publisher implementation. Removing sender or global access breaks subscribers; changing `Isolated` changes transaction, error, and rollback behavior. AppSourceCop AS0021 prevents changing exposed sender or globals from `true` to `false`, while AS0101 prevents adding, removing, or changing `Isolated`. The three-argument event form with `Isolated` is available from runtime 9.0 (Business Central 2022 release wave 1, BC20).
|
`IncludeSender` and, on Integration events, `GlobalVarAccess` have been event-contract flags since runtime 1.0. Removing sender or global access breaks subscribers, so AppSourceCop AS0021 prevents changing those flags from `true` to `false`. On runtime 9.0 and later (Business Central 2022 release wave 1, BC20), `Isolated` also controls transaction, error, and rollback behavior; AS0101 prevents adding, removing, or changing that argument.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Keep every attribute argument exactly as shipped. If new subscribers need different sender/global exposure or isolation semantics, publish a new event with the desired flags and raise both events while the original contract is supported. Choose preferred flags only when designing a new event.
|
Keep every available attribute argument exactly as shipped. If new subscribers need different sender/global exposure, publish a new event with the desired flags. Apply the same rule to `Isolated` only on BC20 or later, where that argument exists. Raise both events while the original contract is supported, and choose preferred flags only when designing a new event.
|
||||||
|
|
||||||
See sample: `do-not-change-shipped-event-attribute-flags.good.al`.
|
See sample: `do-not-change-shipped-event-attribute-flags.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Changing a shipped event's attribute arguments to modernize its design, remove `GlobalVarAccess`, replace `IncludeSender` with an explicit parameter, or make the event isolated. Even a change that leaves old subscribers compiling can alter observable execution or exposure; version the event instead.
|
Changing a shipped event's `IncludeSender` or `GlobalVarAccess` to modernize its design, including replacing `IncludeSender` with an explicit parameter. On BC20 or later, adding, removing, or toggling `Isolated` is equally contract-significant. Even a change that leaves old subscribers compiling can alter observable execution or exposure; version the event instead.
|
||||||
|
|
||||||
See sample: `do-not-change-shipped-event-attribute-flags.bad.al`.
|
See sample: `do-not-change-shipped-event-attribute-flags.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
// Demonstration-only AL. Version 1 exposed Score as an Integer named Score.
|
// Demonstration-only AL. Version 1 exposed var Score as an Integer.
|
||||||
codeunit 50521 "Customer Scoring Events Bad"
|
codeunit 50521 "Customer Scoring Events Bad"
|
||||||
{
|
{
|
||||||
procedure ScoreCustomer(CustomerNo: Code[20]; ScoreText: Text)
|
procedure ScoreCustomer(CustomerNo: Code[20]; ScoreText: Text)
|
||||||
|
|
@ -6,8 +6,8 @@ codeunit 50521 "Customer Scoring Events Bad"
|
||||||
OnCustomerScored(CustomerNo, ScoreText);
|
OnCustomerScored(CustomerNo, ScoreText);
|
||||||
end;
|
end;
|
||||||
|
|
||||||
// 'local' limits raising, not subscription. This type/name change breaks
|
// 'local' limits raising, not subscription. Renaming Score to ScoreText,
|
||||||
// subscribers compiled against the shipped event.
|
// changing its type, and removing var all break existing subscribers.
|
||||||
[IntegrationEvent(false, false)]
|
[IntegrationEvent(false, false)]
|
||||||
local procedure OnCustomerScored(CustomerNo: Code[20]; ScoreText: Text)
|
local procedure OnCustomerScored(CustomerNo: Code[20]; ScoreText: Text)
|
||||||
begin
|
begin
|
||||||
|
|
|
||||||
|
|
@ -1,19 +1,24 @@
|
||||||
// Demonstration-only AL. Version 2 keeps the shipped local event unchanged.
|
// Demonstration-only AL. Version 1 had CustomerNo and var Score parameters.
|
||||||
codeunit 50520 "Customer Scoring Events"
|
codeunit 50520 "Customer Scoring Events"
|
||||||
{
|
{
|
||||||
procedure ScoreCustomer(CustomerNo: Code[20]; Score: Integer; Reason: Text)
|
procedure ScoreCustomer(CustomerNo: Code[20]; Reason: Text; var Score: Integer)
|
||||||
begin
|
begin
|
||||||
OnCustomerScored(CustomerNo, Score);
|
OnCustomerScored(CustomerNo, Reason, Score);
|
||||||
OnCustomerScoredV2(CustomerNo, Score, Reason);
|
|
||||||
end;
|
end;
|
||||||
|
|
||||||
|
// Adding Reason between existing parameters preserves subscriber bindings.
|
||||||
[IntegrationEvent(false, false)]
|
[IntegrationEvent(false, false)]
|
||||||
local procedure OnCustomerScored(CustomerNo: Code[20]; Score: Integer)
|
local procedure OnCustomerScored(CustomerNo: Code[20]; Reason: Text; var Score: Integer)
|
||||||
begin
|
|
||||||
end;
|
|
||||||
|
|
||||||
[IntegrationEvent(false, false)]
|
|
||||||
local procedure OnCustomerScoredV2(CustomerNo: Code[20]; Score: Integer; Reason: Text)
|
|
||||||
begin
|
begin
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
codeunit 50522 "Existing Scoring Subscriber"
|
||||||
|
{
|
||||||
|
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Customer Scoring Events", 'OnCustomerScored', '', false, false)]
|
||||||
|
local procedure OnCustomerScored(CustomerNo: Code[20]; var Score: Integer)
|
||||||
|
begin
|
||||||
|
if CustomerNo = '' then
|
||||||
|
Score := 0;
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
---
|
---
|
||||||
bc-version: [all]
|
bc-version: [all]
|
||||||
domain: events
|
domain: events
|
||||||
keywords: [local-event, internal-event, event-subscriber, compatibility, access-modifier, integration-event, business-event, appsourcecop, as0025]
|
keywords: [local-event, internal-event, event-subscriber, compatibility, access-modifier, integration-event, business-event, parameter-name, var-parameter, appsourcecop]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
countries: [w1]
|
countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
The `local` and `internal` access modifiers on Business and Integration event publishers restrict who can raise the procedure; they do not prevent dependent extensions from subscribing. Once shipped, the event name and existing parameter names, types, order, and passing modes are compatibility contracts even when the publisher is not public. This differs from `[InternalEvent]`, which is module-only except for modules named by `internalsVisibleTo`.
|
The `local` and `internal` access modifiers on Business and Integration event publishers restrict who can raise the procedure; they do not prevent dependent extensions from subscribing. Once shipped, the event name and each existing parameter's name, type/subtype, and value-versus-`var` passing mode are compatibility contracts even when the publisher is not public. Parameter order is not a subscriber contract because subscribers bind the parameters they use by name. This differs from `[InternalEvent]`, which is module-only except for modules named by `internalsVisibleTo`.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Preserve a shipped Business or Integration event's identity and existing parameters regardless of its procedure access modifier. A compatible trailing parameter may be added to a `local` or `internal` event as described by `add-new-event-parameters-at-the-end`; for an incompatible signature or a public publisher, add a new event and keep the original.
|
Preserve a shipped Business or Integration event's identity and every existing parameter's name, type/subtype, and passing mode regardless of the procedure access modifier. AS0025 protects names and types, while AS0063 and AS0077 protect removal and addition of `var`. New parameters may be added at any position on a `local` or `internal` event because subscribers can omit them; public event procedures follow the stricter caller contract described by `add-new-event-parameters-at-the-end`.
|
||||||
|
|
||||||
See sample: `treat-local-and-internal-events-as-subscriber-contracts.good.al`.
|
See sample: `treat-local-and-internal-events-as-subscriber-contracts.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Renaming, removing, reordering, or changing an existing parameter because the event publisher procedure is `local` or `internal`. AppSourceCop AS0025 checks these subscriber-breaking changes because dependent event subscribers can still bind to the event.
|
Renaming or removing an existing parameter, changing its type/subtype, or adding/removing its `var` modifier because the event publisher procedure is `local` or `internal`. AppSourceCop checks these subscriber-breaking changes because dependent event subscribers can still bind to the event. Reordering unchanged parameters, or inserting a new parameter among them, is not this anti-pattern.
|
||||||
|
|
||||||
See sample: `treat-local-and-internal-events-as-subscriber-contracts.bad.al`.
|
See sample: `treat-local-and-internal-events-as-subscriber-contracts.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -53,9 +53,9 @@ The following targeted checks map diff signals to specific `events` articles. Tr
|
||||||
|
|
||||||
- `IsHandled` raised without an immediately preceding `IsHandled := false;`, or one `IsHandled` variable reused across several raises with no reset between them — `initialize-ishandled-to-false-before-publishing`.
|
- `IsHandled` raised without an immediately preceding `IsHandled := false;`, or one `IsHandled` variable reused across several raises with no reset between them — `initialize-ishandled-to-false-before-publishing`.
|
||||||
- `if IsHandled then exit;` in a routine that also raises a paired `OnAfter…` event later, so the after-event is skipped whenever the call is handled — `preserve-onafter-execution-when-ishandled-skips-the-body`.
|
- `if IsHandled then exit;` in a routine that also raises a paired `OnAfter…` event later, so the after-event is skipped whenever the call is handled — `preserve-onafter-execution-when-ishandled-skips-the-body`.
|
||||||
- A parameter added before existing parameters on a changed `local` or `internal` Business/Integration event, or any parameter added to a public event — `add-new-event-parameters-at-the-end`.
|
- Any parameter added to a public Business/Integration event procedure, regardless of position; do not flag additions or reordering on `local`/`internal` publishers merely because a new parameter was not appended — `add-new-event-parameters-at-the-end`.
|
||||||
- A shipped Business/Integration event renamed or removed, or one of its existing parameters renamed, removed, reordered, or changed, based on the mistaken assumption that `local` or `internal` prevents dependent subscription — `treat-local-and-internal-events-as-subscriber-contracts`.
|
- A shipped Business/Integration event renamed or removed, or an existing parameter renamed, removed, retyped, or changed to/from `var`, based on the mistaken assumption that `local` or `internal` prevents dependent subscription; parameter order alone is not a subscriber-contract violation — `treat-local-and-internal-events-as-subscriber-contracts`.
|
||||||
- Any change to `IncludeSender`, `GlobalVarAccess`, or `Isolated` on a shipped event, including a change intended to modernize the publisher — `do-not-change-shipped-event-attribute-flags`.
|
- Any change to `IncludeSender` or `GlobalVarAccess` on a shipped event at any target version, or to `Isolated` on BC20/runtime 9.0 or later, including a change intended to modernize the publisher — `do-not-change-shipped-event-attribute-flags`.
|
||||||
- Publisher names that do not encode firing position (`OnBefore`/`OnAfter<Routine>` at the boundaries, `On<Routine>OnBefore`/`OnAfter<Context>` mid-routine) — `name-events-by-publisher-position`.
|
- Publisher names that do not encode firing position (`OnBefore`/`OnAfter<Routine>` at the boundaries, `On<Routine>OnBefore`/`OnAfter<Context>` mid-routine) — `name-events-by-publisher-position`.
|
||||||
- Two consecutive `OnBefore`/`OnAfter` raises with no logic between them, or a near-duplicate event differing only by an extra parameter — `prefer-reusing-or-extending-existing-events`.
|
- Two consecutive `OnBefore`/`OnAfter` raises with no logic between them, or a near-duplicate event differing only by an extra parameter — `prefer-reusing-or-extending-existing-events`.
|
||||||
- An event raised between `repeat` and `until` inside a record loop — `do-not-publish-events-inside-loops`.
|
- An event raised between `repeat` and `until` inside a record loop — `do-not-publish-events-inside-loops`.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue