mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Add data handling and test isolation guidance
- add SCM guidance for deriving base quantities through line unit-of-measure validation`n- add security guidance for parameterizing SetFilter with external text`n- add test isolation guidance for resetting per-test state before initialization guards`n- add web-service guidance for JSON null handling and invariant standard format 9`n- route and cover all five rules with paired evaluation fixtures
This commit is contained in:
demiliani
parent
45ca57a23e
commit
570e776a37
20 changed files with 508 additions and 6 deletions
|
|
@ -0,0 +1,26 @@
|
|||
codeunit 50181 "Scanner Receipt Import Bad"
|
||||
{
|
||||
procedure PostScannedReceipt(ItemNo: Code[20]; LocationCode: Code[10]; UnitOfMeasureCode: Code[10]; ScannedQuantity: Decimal; DocumentNo: Code[20])
|
||||
var
|
||||
ItemJournalLine: Record "Item Journal Line";
|
||||
ItemJnlPostLine: Codeunit "Item Jnl.-Post Line";
|
||||
begin
|
||||
if ScannedQuantity <= 0 then
|
||||
Error(PositiveQuantityErr);
|
||||
|
||||
ItemJournalLine.Init();
|
||||
ItemJournalLine.Validate("Posting Date", WorkDate());
|
||||
ItemJournalLine.Validate("Entry Type", ItemJournalLine."Entry Type"::"Positive Adjmt.");
|
||||
ItemJournalLine.Validate("Document No.", DocumentNo);
|
||||
ItemJournalLine.Validate("Item No.", ItemNo);
|
||||
ItemJournalLine.Validate("Location Code", LocationCode);
|
||||
ItemJournalLine."Unit of Measure Code" := UnitOfMeasureCode;
|
||||
ItemJournalLine.Quantity := ScannedQuantity;
|
||||
ItemJournalLine."Quantity (Base)" := ScannedQuantity;
|
||||
|
||||
ItemJnlPostLine.RunWithCheck(ItemJournalLine);
|
||||
end;
|
||||
|
||||
var
|
||||
PositiveQuantityErr: Label 'The scanned quantity must be greater than zero.';
|
||||
}
|
||||
|
|
@ -0,0 +1,25 @@
|
|||
codeunit 50180 "Scanner Receipt Import Good"
|
||||
{
|
||||
procedure PostScannedReceipt(ItemNo: Code[20]; LocationCode: Code[10]; UnitOfMeasureCode: Code[10]; ScannedQuantity: Decimal; DocumentNo: Code[20])
|
||||
var
|
||||
ItemJournalLine: Record "Item Journal Line";
|
||||
ItemJnlPostLine: Codeunit "Item Jnl.-Post Line";
|
||||
begin
|
||||
if ScannedQuantity <= 0 then
|
||||
Error(PositiveQuantityErr);
|
||||
|
||||
ItemJournalLine.Init();
|
||||
ItemJournalLine.Validate("Posting Date", WorkDate());
|
||||
ItemJournalLine.Validate("Entry Type", ItemJournalLine."Entry Type"::"Positive Adjmt.");
|
||||
ItemJournalLine.Validate("Document No.", DocumentNo);
|
||||
ItemJournalLine.Validate("Item No.", ItemNo);
|
||||
ItemJournalLine.Validate("Location Code", LocationCode);
|
||||
ItemJournalLine.Validate("Unit of Measure Code", UnitOfMeasureCode);
|
||||
ItemJournalLine.Validate(Quantity, ScannedQuantity);
|
||||
|
||||
ItemJnlPostLine.RunWithCheck(ItemJournalLine);
|
||||
end;
|
||||
|
||||
var
|
||||
PositiveQuantityErr: Label 'The scanned quantity must be greater than zero.';
|
||||
}
|
||||
|
|
@ -0,0 +1,37 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: scm
|
||||
keywords: [quantity-base, qty-per-unit-of-measure, unit-of-measure-code, unit-of-measure-management, calcbaseqty, getqtyperunitofmeasure, qty-rounding-precision, item-journal-line]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Derive base quantities through the line's unit of measure
|
||||
|
||||
## Description
|
||||
|
||||
Inventory, item ledger entries, reservations, item tracking, and warehouse quantities are measured in the item's base unit of measure. Document and journal lines hold `Quantity` in the line's `"Unit of Measure Code"`, together with `"Qty. per Unit of Measure"` and base-unit fields such as `"Quantity (Base)"`. Ten boxes of twelve pieces are 120 base units, not 10. When a line's `Quantity` is validated, the table derives the base quantity through its `CalcBaseQty` procedure. That procedure calls `"Unit of Measure Management".CalcBaseQty` with the line's quantity rounding precision and raises an error when rounding would turn a non-zero quantity into a zero base quantity. Code that bypasses this conversion creates a line whose quantity and base quantity disagree, or makes a stock decision in the wrong unit.
|
||||
|
||||
## Best Practice
|
||||
|
||||
On a document or journal line, validate `"Unit of Measure Code"` before `Quantity`, and validate both. Validating the unit of measure sets `"Qty. per Unit of Measure"` from the item unit of measure; validating the quantity then fills the base fields with the correct rounding. Compare line quantities with inventory or availability in base units, for example `"Quantity (Base)"` or `"Outstanding Qty. (Base)"`.
|
||||
|
||||
Outside a line, get the factor with `"Unit of Measure Management".GetQtyPerUnitOfMeasure(Item, UnitOfMeasureCode)` and convert with its `CalcBaseQty` or `CalcQtyFromBase` procedures instead of multiplying by hand. Pass the item unit's quantity rounding precision where the available overload accepts it.
|
||||
|
||||
Reading these fields for display, reporting, or a temporary buffer that is never posted is not a conversion defect. Code that proves the line uses the base unit of measure (`"Qty. per Unit of Measure"` equal to 1) is also correct, but don't assume this from the item alone, because a line can use another unit.
|
||||
|
||||
See sample: [`derive-base-quantities-through-the-line-unit-of-measure.good.al`](derive-base-quantities-through-the-line-unit-of-measure.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Assigning `Quantity` directly on an item journal, sales, purchase, or transfer line and then inserting, modifying, or posting it. Also assigning a base field such as `"Quantity (Base)" := Quantity`, multiplying by a hard-coded or separately looked-up factor without the line's rounding, or comparing a line's `Quantity` with `Item.Inventory` or another base-unit value. Detection signal: a direct `:=` to `Quantity`, `"Qty. per Unit of Measure"`, or a `(Base)` quantity field on a persisted or posted line, or a comparison between a non-base line quantity and an inventory quantity.
|
||||
|
||||
See sample: [`derive-base-quantities-through-the-line-unit-of-measure.bad.al`](derive-base-quantities-through-the-line-unit-of-measure.bad.al).
|
||||
|
||||
## References
|
||||
|
||||
- [Set up units of measure, including quantity rounding precision](https://learn.microsoft.com/en-us/dynamics365/business-central/inventory-how-setup-units-of-measure)
|
||||
- [BCApps: Unit of Measure Management conversions](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/BaseApp/Foundation/UOM/UnitofMeasureManagement.Codeunit.al)
|
||||
- [BCApps: Item Journal Line quantity validation and CalcBaseQty](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/BaseApp/Inventory/Journal/ItemJournalLine.Table.al)
|
||||
- [BCApps: Sales Line quantity validation and CalcBaseQty](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/BaseApp/Sales/Document/SalesLine.Table.al)
|
||||
|
|
@ -0,0 +1,11 @@
|
|||
codeunit 50161 "Cancel External Quotes Bad"
|
||||
{
|
||||
procedure CancelQuote(ExternalDocumentNo: Text)
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
begin
|
||||
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Quote);
|
||||
SalesHeader.SetFilter("External Document No.", ExternalDocumentNo);
|
||||
SalesHeader.DeleteAll(true);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,25 @@
|
|||
codeunit 50160 "Cancel External Quotes Good"
|
||||
{
|
||||
procedure CancelQuote(ExternalDocumentNo: Code[35])
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
begin
|
||||
if ExternalDocumentNo = '' then
|
||||
Error(MissingExternalDocumentNoErr);
|
||||
|
||||
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Quote);
|
||||
SalesHeader.SetRange("External Document No.", ExternalDocumentNo);
|
||||
SalesHeader.DeleteAll(true);
|
||||
end;
|
||||
|
||||
procedure CancelQuotes(ExternalDocumentNos: List of [Code[35]])
|
||||
var
|
||||
ExternalDocumentNo: Code[35];
|
||||
begin
|
||||
foreach ExternalDocumentNo in ExternalDocumentNos do
|
||||
CancelQuote(ExternalDocumentNo);
|
||||
end;
|
||||
|
||||
var
|
||||
MissingExternalDocumentNoErr: Label 'The external document number is missing.';
|
||||
}
|
||||
|
|
@ -0,0 +1,36 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [setfilter, setrange, filter-expression, filter-injection, external-input, wildcard, deleteall, modifyall]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not concatenate external text into a SetFilter expression
|
||||
|
||||
## Description
|
||||
|
||||
The `String` argument of `SetFilter` is a filter expression, not a value. Characters such as `..`, `|`, `&`, `<`, `>`, `=`, `*`, `?`, `@`, parentheses, and single quotes are operators. Text from a user, request page, API payload, file, or another system that is concatenated into that expression can therefore change which records match: `*` matches every value, `A|B` widens an exact lookup to two values, `10000..` becomes a range, and `J & V` becomes a conjunction or an invalid filter. `SetFilter` with an empty expression applies no filter at all. When the filtered record is then modified, deleted, exported, or used for a permission-relevant decision, the procedure acts on records the caller never identified.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Treat an externally supplied identifier as a value. Use `SetRange(Field, Value)` for equality and `SetRange(Field, FromValue, ToValue)` for a typed range; neither parses operators. Reject an empty identifier explicitly when "no value" must not mean "every record". For several external values, apply `SetRange` once per value instead of joining them with `|`.
|
||||
|
||||
Use `SetFilter` when an operator is part of the procedure's own contract. Keep the operator in a constant expression and pass operands through replacement fields (`%1`, `%2`) of the field's data type, such as a `Date` or `Decimal` operand for `'>=%1'`. Do not rely on wrapping text in single quotes to neutralize it: according to the filter syntax, quotes protect `&`, `(`, `)`, `=`, and `|`, but `*` still acts as a wildcard inside `'J & V*'`.
|
||||
|
||||
Text that the user deliberately entered as a filter is supposed to be parsed. Do not report a request-page or `FilterPageBuilder` filter, a `GetFilters`/`GetView` round trip, a FlowFilter, or a field whose documented purpose is to hold a filter expression. Constant filter strings and operands produced by the extension's own code are also not external input.
|
||||
|
||||
See sample: [`do-not-concatenate-external-text-into-setfilter.good.al`](do-not-concatenate-external-text-into-setfilter.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`Rec.SetFilter(Field, ExternalText)` or `Rec.SetFilter(Field, Prefix + ExternalText + Suffix)` where the text is meant to identify one record or a known list of records, with no validation that restricts it to a literal value. The finding is strongest when the resulting set is written by `Modify`, `ModifyAll`, `Delete`, or `DeleteAll`, or is returned to an external caller. Detection signal: a non-constant expression, concatenation, or `StrSubstNo` result passed as the `String` argument of `SetFilter`, where the operand comes from a parameter, page field, JSON/XML value, file line, or HTTP request.
|
||||
|
||||
See sample: [`do-not-concatenate-external-text-into-setfilter.bad.al`](do-not-concatenate-external-text-into-setfilter.bad.al).
|
||||
|
||||
## References
|
||||
|
||||
- [Record.SetFilter method, including the empty-filter remark](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-setfilter-method)
|
||||
- [Filtering with SetRange and SetFilter](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-setcurrentkey-setrange-setfilter-getrangemin-and-getrangemax-methods)
|
||||
- [Filter criteria, operators, and values that contain symbols](https://learn.microsoft.com/en-us/dynamics365/business-central/ui-enter-criteria-filters#filter-criteria-and-operators)
|
||||
|
|
@ -0,0 +1,57 @@
|
|||
codeunit 50411 "Test Sales Setup Initialize Bad"
|
||||
{
|
||||
Subtype = Test;
|
||||
|
||||
[Test]
|
||||
[HandlerFunctions('CustomerCardHandler')]
|
||||
procedure CustomerCardOpensForSelectedCustomer()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Initialize();
|
||||
LibrarySales.CreateCustomer(Customer);
|
||||
LibraryVariableStorage.Enqueue(Customer."No.");
|
||||
|
||||
Page.RunModal(Page::"Customer Card", Customer);
|
||||
|
||||
LibraryVariableStorage.AssertEmpty();
|
||||
end;
|
||||
|
||||
[Test]
|
||||
procedure StockoutWarningCanBeDisabled()
|
||||
var
|
||||
SalesSetup: Record "Sales & Receivables Setup";
|
||||
begin
|
||||
Initialize();
|
||||
|
||||
LibrarySales.SetStockoutWarning(false);
|
||||
|
||||
SalesSetup.Get();
|
||||
Assert.IsFalse(SalesSetup."Stockout Warning", 'The stockout warning was not disabled.');
|
||||
end;
|
||||
|
||||
local procedure Initialize()
|
||||
begin
|
||||
if IsInitialized then
|
||||
exit;
|
||||
|
||||
LibraryVariableStorage.Clear();
|
||||
LibrarySetupStorage.Restore();
|
||||
LibrarySales.SetStockoutWarning(true);
|
||||
IsInitialized := true;
|
||||
LibrarySetupStorage.SaveSalesSetup();
|
||||
end;
|
||||
|
||||
[ModalPageHandler]
|
||||
procedure CustomerCardHandler(var CustomerCard: TestPage "Customer Card")
|
||||
begin
|
||||
Assert.AreEqual(LibraryVariableStorage.DequeueText(), CustomerCard."No.".Value(), 'The customer card opened for the wrong customer.');
|
||||
end;
|
||||
|
||||
var
|
||||
Assert: Codeunit Assert;
|
||||
LibrarySales: Codeunit "Library - Sales";
|
||||
LibrarySetupStorage: Codeunit "Library - Setup Storage";
|
||||
LibraryVariableStorage: Codeunit "Library - Variable Storage";
|
||||
IsInitialized: Boolean;
|
||||
}
|
||||
|
|
@ -0,0 +1,62 @@
|
|||
codeunit 50410 "Test Sales Setup Initialize Good"
|
||||
{
|
||||
Subtype = Test;
|
||||
|
||||
[Test]
|
||||
[HandlerFunctions('CustomerCardHandler')]
|
||||
procedure CustomerCardOpensForSelectedCustomer()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Initialize();
|
||||
LibrarySales.CreateCustomer(Customer);
|
||||
LibraryVariableStorage.Enqueue(Customer."No.");
|
||||
|
||||
Page.RunModal(Page::"Customer Card", Customer);
|
||||
|
||||
LibraryVariableStorage.AssertEmpty();
|
||||
end;
|
||||
|
||||
[Test]
|
||||
procedure StockoutWarningCanBeDisabled()
|
||||
var
|
||||
SalesSetup: Record "Sales & Receivables Setup";
|
||||
begin
|
||||
Initialize();
|
||||
|
||||
LibrarySales.SetStockoutWarning(false);
|
||||
|
||||
SalesSetup.Get();
|
||||
Assert.IsFalse(SalesSetup."Stockout Warning", 'The stockout warning was not disabled.');
|
||||
end;
|
||||
|
||||
local procedure Initialize()
|
||||
begin
|
||||
LibraryTestInitialize.OnTestInitialize(Codeunit::"Test Sales Setup Initialize Good");
|
||||
LibraryVariableStorage.Clear();
|
||||
LibrarySetupStorage.Restore();
|
||||
|
||||
if IsInitialized then
|
||||
exit;
|
||||
LibraryTestInitialize.OnBeforeTestSuiteInitialize(Codeunit::"Test Sales Setup Initialize Good");
|
||||
|
||||
LibrarySales.SetStockoutWarning(true);
|
||||
IsInitialized := true;
|
||||
LibrarySetupStorage.SaveSalesSetup();
|
||||
LibraryTestInitialize.OnAfterTestSuiteInitialize(Codeunit::"Test Sales Setup Initialize Good");
|
||||
end;
|
||||
|
||||
[ModalPageHandler]
|
||||
procedure CustomerCardHandler(var CustomerCard: TestPage "Customer Card")
|
||||
begin
|
||||
Assert.AreEqual(LibraryVariableStorage.DequeueText(), CustomerCard."No.".Value(), 'The customer card opened for the wrong customer.');
|
||||
end;
|
||||
|
||||
var
|
||||
Assert: Codeunit Assert;
|
||||
LibrarySales: Codeunit "Library - Sales";
|
||||
LibrarySetupStorage: Codeunit "Library - Setup Storage";
|
||||
LibraryTestInitialize: Codeunit "Library - Test Initialize";
|
||||
LibraryVariableStorage: Codeunit "Library - Variable Storage";
|
||||
IsInitialized: Boolean;
|
||||
}
|
||||
|
|
@ -0,0 +1,41 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: testing
|
||||
keywords: [initialize, isinitialized, library-test-initialize, ontestinitialize, library-variable-storage, library-setup-storage, test-fixture, test-codeunit]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Reset per-test state before the IsInitialized guard
|
||||
|
||||
## Description
|
||||
|
||||
Standard Business Central test codeunits call a local `Initialize` procedure at the start of every test method. Global variables in a test codeunit keep their values between the codeunit's test methods, so a Boolean such as `IsInitialized` lets `Initialize` run expensive shared setup only once. The procedure therefore has two parts with different lifetimes: work that must run before **every** test, and one-time setup behind the guard. If per-test reset is placed after the guard, it runs only for the first test. Values left in `Library - Variable Storage` by a failed test, or setup records a test changed, then leak into later tests, which pass or fail depending on execution order.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Call `Initialize()` as the first statement of every test method. Inside it, keep this order, which the Base Application tests follow:
|
||||
|
||||
1. Per-test work, before the guard: raise `"Library - Test Initialize".OnTestInitialize`, call `LibraryVariableStorage.Clear()`, and call `LibrarySetupStorage.Restore()` when setup tables were saved.
|
||||
2. `if IsInitialized then exit;`
|
||||
3. One-time work: raise `OnBeforeTestSuiteInitialize`, create the shared fixture and setup values, set `IsInitialized := true`, save the setup tables that tests may change (for example `LibrarySetupStorage.SaveSalesSetup()`), and raise `OnAfterTestSuiteInitialize`.
|
||||
|
||||
Create data that a single test changes inside that test, not in the shared fixture. Base Application suites also commit after the one-time setup so the shared fixture survives each test's transaction; whether that commit is valid depends on the test transaction model and runner isolation, see [`transactionmodel-attribute-governs-test-transactions.md`](transactionmodel-attribute-governs-test-transactions.md) and [`testisolation-belongs-on-the-test-runner.md`](testisolation-belongs-on-the-test-runner.md).
|
||||
|
||||
A test codeunit with no shared setup and no queued values doesn't need an `Initialize` procedure. Don't report its absence on its own.
|
||||
|
||||
See sample: [`reset-per-test-state-before-the-isinitialized-guard.good.al`](reset-per-test-state-before-the-isinitialized-guard.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`if IsInitialized then exit;` as the first statement of `Initialize`, followed by `LibraryVariableStorage.Clear()`, `LibrarySetupStorage.Restore()`, or other reset calls that are then skipped for every test after the first. A related defect is a test method in a codeunit that uses the pattern but doesn't call `Initialize()`, so it runs with whatever state the previous test left. Detection signal: in a `Subtype = Test` codeunit, a reset call placed after the `IsInitialized` exit, or a `[Test]` procedure that uses shared globals or queued values without first calling `Initialize()`.
|
||||
|
||||
See sample: [`reset-per-test-state-before-the-isinitialized-guard.bad.al`](reset-per-test-state-before-the-isinitialized-guard.bad.al).
|
||||
|
||||
## References
|
||||
|
||||
- [BCApps: `Initialize` in the ERM Sales Document tests](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/Tests/ERM-Sales/ERMSalesDocument.Codeunit.al)
|
||||
- [BCApps: Library - Test Initialize events](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/Tests/ApplicationTestLibrary/LibraryTestInitialize.Codeunit.al)
|
||||
- [BCApps: Library - Setup Storage](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/Tests/ApplicationTestLibrary/LibrarySetupStorage.Codeunit.al)
|
||||
- [Testing the application](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-testing-application)
|
||||
|
|
@ -0,0 +1,11 @@
|
|||
codeunit 50173 "Contact Payload Reader Bad"
|
||||
{
|
||||
procedure ApplyPayload(var Contact: Record Contact; Payload: JsonObject)
|
||||
var
|
||||
Token: JsonToken;
|
||||
begin
|
||||
if Payload.Get('email', Token) then
|
||||
Contact.Validate("E-Mail", CopyStr(Token.AsValue().AsText(), 1, MaxStrLen(Contact."E-Mail")));
|
||||
Contact.Modify(true);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
codeunit 50172 "Contact Payload Reader Good"
|
||||
{
|
||||
procedure ApplyPayload(var Contact: Record Contact; Payload: JsonObject)
|
||||
var
|
||||
EmailAddress: Text;
|
||||
begin
|
||||
if TryGetText(Payload, 'email', EmailAddress) then
|
||||
Contact.Validate("E-Mail", CopyStr(EmailAddress, 1, MaxStrLen(Contact."E-Mail")));
|
||||
Contact.Modify(true);
|
||||
end;
|
||||
|
||||
local procedure TryGetText(Payload: JsonObject; PropertyName: Text; var Value: Text): Boolean
|
||||
var
|
||||
Token: JsonToken;
|
||||
begin
|
||||
if not Payload.Get(PropertyName, Token) then
|
||||
exit(false);
|
||||
if not Token.IsValue() then
|
||||
Error(NotAValueErr, PropertyName);
|
||||
if Token.AsValue().IsNull() then
|
||||
exit(false);
|
||||
Value := Token.AsValue().AsText();
|
||||
exit(true);
|
||||
end;
|
||||
|
||||
var
|
||||
NotAValueErr: Label 'The property %1 must contain a single value.', Comment = '%1 = JSON property name';
|
||||
}
|
||||
|
|
@ -0,0 +1,35 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: web-services
|
||||
keywords: [jsonobject, jsontoken, jsonvalue, isnull, asvalue, astext, asdecimal, optional-property, json-null, payload-parsing]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Check for JSON null before converting a value
|
||||
|
||||
## Description
|
||||
|
||||
An optional property in a JSON payload can be missing or present with the value `null`, and the two cases behave differently in AL. When the Boolean result is captured, `JsonObject.Get` returns `false` for a missing key but `true` for `"email": null`, because the key exists. The conversion methods on `JsonValue` (`AsText`, `AsCode`, `AsDecimal`, `AsInteger`, `AsDate`, `AsBoolean`, and the others) fail with a runtime error when the value is `NULL` or `UNDEFINED`. Code that guards only with `Get` therefore passes its tests with the property omitted and fails in production when the sender serializes an empty field as `null`, which many services do by default.
|
||||
|
||||
## Best Practice
|
||||
|
||||
For every property that the contract allows to be optional or nullable, check three things before converting: that `Get` (or `SelectToken`) returned `true`, that the token `IsValue()` rather than an object or array, and that `AsValue().IsNull()` is `false`. Put this in one small helper per target type and decide explicitly what a missing or null property means: a default, leaving the field unchanged, or a validation error that names the property.
|
||||
|
||||
Required properties can still fail fast, but with an error that states the missing or null property instead of a generic conversion error. Keep a numeric or date conversion strict when the contract says the value must be a number or a date; `IsNull` covers only `null`, not a value of the wrong type.
|
||||
|
||||
See sample: [`check-json-null-before-converting-values.good.al`](check-json-null-before-converting-values.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`if Json.Get('email', Token) then Email := Token.AsValue().AsText();` or an unguarded `Token.AsValue().AsDecimal()` on a property that the external contract allows to be `null`. The `Get` check makes the code look defensive, but it doesn't handle a present `null`. Detection signal: an `As<Type>()` call on a `JsonValue` obtained from an external payload with no preceding `IsNull()` check on the same token, where the property isn't documented as always non-null.
|
||||
|
||||
See sample: [`check-json-null-before-converting-values.bad.al`](check-json-null-before-converting-values.bad.al).
|
||||
|
||||
## References
|
||||
|
||||
- [JsonObject.Get method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/jsonobject/jsonobject-get-method)
|
||||
- [JsonValue.AsText method: fails on NULL or UNDEFINED](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/jsonvalue/jsonvalue-astext-method)
|
||||
- [JsonValue.IsNull method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/jsonvalue/jsonvalue-isnull-method)
|
||||
- [JsonToken data type](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/jsontoken/jsontoken-data-type)
|
||||
|
|
@ -0,0 +1,27 @@
|
|||
codeunit 50171 "Exchange Rate Export Bad"
|
||||
{
|
||||
procedure SendRate(CurrencyCode: Code[10]; StartingDate: Date; ExchangeRate: Decimal)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
RequestUrl: Text;
|
||||
begin
|
||||
RequestUrl := StrSubstNo(RateUrlTok, CurrencyCode, Format(StartingDate), Format(ExchangeRate));
|
||||
if not Client.Get(RequestUrl, Response) then
|
||||
Error(RequestFailedErr);
|
||||
if not Response.IsSuccessStatusCode() then
|
||||
Error(RateRejectedErr, Response.HttpStatusCode());
|
||||
end;
|
||||
|
||||
procedure ReadRate(RateText: Text) ExchangeRate: Decimal
|
||||
begin
|
||||
if not Evaluate(ExchangeRate, RateText) then
|
||||
Error(InvalidRateErr, RateText);
|
||||
end;
|
||||
|
||||
var
|
||||
RateUrlTok: Label 'https://rates.example.com/rates?currency=%1&date=%2&rate=%3', Locked = true;
|
||||
RequestFailedErr: Label 'The exchange rate service could not be reached.';
|
||||
RateRejectedErr: Label 'The exchange rate service rejected the rate. Status code: %1.', Comment = '%1 = HTTP status code';
|
||||
InvalidRateErr: Label 'The exchange rate %1 is not a valid decimal number.', Comment = '%1 = received value';
|
||||
}
|
||||
|
|
@ -0,0 +1,27 @@
|
|||
codeunit 50170 "Exchange Rate Export Good"
|
||||
{
|
||||
procedure SendRate(CurrencyCode: Code[10]; StartingDate: Date; ExchangeRate: Decimal)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
RequestUrl: Text;
|
||||
begin
|
||||
RequestUrl := StrSubstNo(RateUrlTok, CurrencyCode, Format(StartingDate, 0, 9), Format(ExchangeRate, 0, 9));
|
||||
if not Client.Get(RequestUrl, Response) then
|
||||
Error(RequestFailedErr);
|
||||
if not Response.IsSuccessStatusCode() then
|
||||
Error(RateRejectedErr, Response.HttpStatusCode());
|
||||
end;
|
||||
|
||||
procedure ReadRate(RateText: Text) ExchangeRate: Decimal
|
||||
begin
|
||||
if not Evaluate(ExchangeRate, RateText, 9) then
|
||||
Error(InvalidRateErr, RateText);
|
||||
end;
|
||||
|
||||
var
|
||||
RateUrlTok: Label 'https://rates.example.com/rates?currency=%1&date=%2&rate=%3', Locked = true;
|
||||
RequestFailedErr: Label 'The exchange rate service could not be reached.';
|
||||
RateRejectedErr: Label 'The exchange rate service rejected the rate. Status code: %1.', Comment = '%1 = HTTP status code';
|
||||
InvalidRateErr: Label 'The exchange rate %1 is not a valid decimal number.', Comment = '%1 = received value';
|
||||
}
|
||||
|
|
@ -0,0 +1,39 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: web-services
|
||||
keywords: [format, evaluate, standard-format-9, xml-format, locale, regional-settings, decimal-separator, data-exchange, integration]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Format exchanged values with standard format 9
|
||||
|
||||
## Description
|
||||
|
||||
`Format(Value)` uses standard format 0, the display format, and follows the current user's regional settings. The same decimal renders as `-76.543,21` for a European region and `-76,543.21` for English (US); the same date renders as `05-04-21` or `04/05/21`. Text built this way and sent outside Business Central (an HTTP query string or body, an XML or CSV file, a signature or hash input, or an external key) changes with the user or job queue session that produces it. The receiver can reject it or, worse, misread it. `Evaluate` without a format number has the same dependency when it parses machine-generated text.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Use `Format(Value, 0, 9)` for machine-readable text. Standard format 9 is the XML format and doesn't depend on the region: `-76543.21` for a decimal, `2021-04-05` for a date, `04:35:55.553` for a time, `true`/`false` for a Boolean, and a UTC `DateTime` such as `2021-04-05T03:35:55.553Z`. Parse such text with `Evaluate(Variable, Text, 9)`.
|
||||
|
||||
Prefer typed APIs when they exist. `JsonObject.Add` and `JsonValue.SetValue` with a `Decimal`, `Date`, or `Boolean` argument write a JSON value without going through display text. An XMLport handles this with `FormatEvaluate = Xml`.
|
||||
|
||||
For `Enum` and `Option` values, format 9 produces the ordinal number, not the name. When the external contract exchanges names, map them explicitly; see [`api-enum-values-are-a-contract-by-name-not-ordinal.md`](api-enum-values-are-a-contract-by-name-not-ordinal.md).
|
||||
|
||||
Text shown to a person (messages, captions, report columns, notifications) should keep the regional display format. `Code`, `Text`, and `Guid` values don't need format 9 because their standard formats don't vary by region.
|
||||
|
||||
See sample: [`format-exchanged-values-with-standard-format-9.good.al`](format-exchanged-values-with-standard-format-9.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`Format(Amount)`, `Format(PostingDate)`, or `Format(SomeDateTime)` concatenated into a URL, request body, XML or CSV line, file name, or hash input. Passing the `Decimal` or `Date` itself to `StrSubstNo` for such text has the same effect, because `StrSubstNo` formats it with the display format. Also `Evaluate(DecimalOrDateVariable, ExternalText)` without format number 9 on text received from another system. The code usually works for the developer's own region and fails for users or job queue sessions in another one. Detection signal: `Format` with one argument, or with a format number other than 9, applied to a `Decimal`, `Date`, `Time`, `DateTime`, or `Boolean` on a path that writes to an `HttpContent`, `HttpRequestMessage`, `OutStream`, `XmlDocument`, or file.
|
||||
|
||||
See sample: [`format-exchanged-values-with-standard-format-9.bad.al`](format-exchanged-values-with-standard-format-9.bad.al).
|
||||
|
||||
## References
|
||||
|
||||
- [Formatting values, dates, and time: standard formats by region and format 9](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-format-property)
|
||||
- [System.Format method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/system/system-format-joker-integer-integer-method)
|
||||
- [System.Evaluate method and format number 9](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/system/system-evaluate-method)
|
||||
- [FormatEvaluate property for XMLports](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/properties/devenv-formatevaluate-property)
|
||||
Loading…
Add table
Add a link
Reference in a new issue