mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Add TableRelation field-length and RecordRef.Open Temp knowledge
- data-modeling: tablerelation-field-length-must-match-related-field (exact length for unconditional relations, at least the longest target when all relations are conditional; no compile-time diagnostic) - testing: recordref-open-temp-parameter-defeats-real-table-checks (a temp-opened RecordRef is empty, so existence/uniqueness checks on it never see persisted rows; cross-links use-generateguid article) - Wire both into the review leaves and register the fixture pairs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Michael Dieringer
parent
fd59919778
commit
260d064ece
9 changed files with 248 additions and 6 deletions
|
|
@ -0,0 +1,30 @@
|
|||
table 50100 "Sample Category"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[20]) { }
|
||||
}
|
||||
keys
|
||||
{
|
||||
key(PK; "Code") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
table 50102 "Sample Contract"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[20]) { }
|
||||
// Code[10] cannot hold every "Sample Category"."Code" value (Code[20]).
|
||||
// Compiles without a diagnostic; picking a category code longer than
|
||||
// 10 characters through the lookup fails at runtime.
|
||||
field(2; "Category Code"; Code[10])
|
||||
{
|
||||
TableRelation = "Sample Category"."Code";
|
||||
}
|
||||
}
|
||||
keys
|
||||
{
|
||||
key(PK; "Code") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,58 @@
|
|||
enum 50100 "Sample Source Type"
|
||||
{
|
||||
Extensible = true;
|
||||
|
||||
value(0; Category) { }
|
||||
value(1; Region) { }
|
||||
}
|
||||
|
||||
table 50100 "Sample Category"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[20]) { }
|
||||
}
|
||||
keys
|
||||
{
|
||||
key(PK; "Code") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
table 50101 "Sample Region"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[10]) { }
|
||||
}
|
||||
keys
|
||||
{
|
||||
key(PK; "Code") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
table 50102 "Sample Contract"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[20]) { }
|
||||
// Unconditional relation: same type and exactly the same length as
|
||||
// "Sample Category"."Code".
|
||||
field(2; "Category Code"; Code[20])
|
||||
{
|
||||
TableRelation = "Sample Category"."Code";
|
||||
}
|
||||
field(3; "Source Type"; Enum "Sample Source Type") { }
|
||||
// All branches conditional: at least as long as the longest related
|
||||
// field (Code[20] covers both Code[20] and Code[10]).
|
||||
field(4; "Source Code"; Code[20])
|
||||
{
|
||||
TableRelation = if ("Source Type" = const(Category)) "Sample Category"."Code"
|
||||
else
|
||||
if ("Source Type" = const(Region)) "Sample Region"."Code";
|
||||
}
|
||||
}
|
||||
keys
|
||||
{
|
||||
key(PK; "Code") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,51 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [tablerelation, field-length, code, text, conditional-relation, lookup]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# A TableRelation field's length must match the related field
|
||||
|
||||
## Description
|
||||
|
||||
`TableRelation` points a field at another table, or at a specific field of it with `Table.Field`. The relation is also used to validate entries and to drive the lookup. A referencing field that is shorter than the related field compiles without a length diagnostic: verified with AL compiler 30.0 and the CodeCop, UICop and PerTenantExtensionCop analyzers. Compare `AL0685`, which does warn about an analogous length mismatch for a FlowField's `CalcFormula` target.
|
||||
|
||||
A shorter field compiles cleanly and works for every value that happens to fit. It fails only when a real related value is longer than the field. For example, a `Code[10]` field related to a table with a `Code[20]` key fails when a user picks a 15-character code through the lookup. The runtime error reads "The length of the string is N, but it must be less than or equal to M characters."
|
||||
|
||||
The length Business Central's own relation check expects depends on whether the field has an unconditional relation:
|
||||
|
||||
- **At least one unconditional relation**, either a plain `TableRelation = X` or an unconditional branch: the field must have the **exact** length of the longest related field and the same type.
|
||||
- **Only conditional relations** (`if (...) X else if (...) Y`): the field must be **at least** as long as the longest related field. A longer field is accepted, and a `Code` relation may be stored in a `Text` field.
|
||||
|
||||
The check is codeunit 134926 "Table Relation Test" in the BC test app. Its validation test is `[Scope('OnPrem')]`, so it runs only on an on-premises test surface. It is not a compile-time guarantee. See [`table-relation-test-exclude-known-invalid-relations-via-event.md`](../testing/table-relation-test-exclude-known-invalid-relations-via-event.md) for how that check evaluates relations and how to exclude a known exception.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Before you add or change a `TableRelation`, read the declared type and length of every related field from its table definition. Lengths vary from table to table, so don't assume a typical `Code[10]` or `Code[20]`. Then size the field:
|
||||
|
||||
- For an unconditional relation, give it the same type and exact length as the related field.
|
||||
- For an all-conditional relation, make it at least as long as the longest branch target.
|
||||
|
||||
When a `tableextension` adds a branch with `modify(...)`, check the new target's length against the field's existing declaration too.
|
||||
|
||||
See sample: [`tablerelation-field-length-must-match-related-field.good.al`](tablerelation-field-length-must-match-related-field.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A field whose declared `Code`/`Text` length is shorter than the field it relates to. One example is a `Code[10]` field with `TableRelation` to a table keyed on `Code[20]`. Another is a conditional relation where one branch targets a longer field than the declaration. Both compile without a diagnostic and fail at runtime once a real, longer value is used.
|
||||
|
||||
A field that is longer than its related field under an unconditional relation is a lesser deviation. It holds every valid value, but it fails Business Central's relation test and accepts values that can never satisfy the relation.
|
||||
|
||||
See sample: [`tablerelation-field-length-must-match-related-field.bad.al`](tablerelation-field-length-must-match-related-field.bad.al).
|
||||
|
||||
Related: [`transferfields-mirrored-fields-must-match-type-and-length.md`](transferfields-mirrored-fields-must-match-type-and-length.md) covers the same length-mismatch failure between fields that `TransferFields` connects.
|
||||
|
||||
## Source
|
||||
|
||||
- [TableRelation property](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/properties/devenv-tablerelation-property): syntax `<TableName>[.<FieldName>]`, conditional `IF ... ELSE` relations, and use of the relation to validate entries.
|
||||
- [Compiler Warning (future error) AL0685](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/diagnostics/diagnostic-al685): the analogous FlowField length diagnostic, which warns that the mismatch "could result in a runtime error".
|
||||
- BCApps [`src/Layers/W1/Tests/Misc/TableRelationTest.Codeunit.al`](https://github.com/microsoft/BCApps/blob/main/src/Layers/W1/Tests/Misc/TableRelationTest.Codeunit.al): line 46 says "Fields must have the exact length of the largest field they relate to". Lines 68-84 apply `Field.Len < MaxRelatedFieldLength` when every relation is conditional and `Field.Len <> MaxRelatedFieldLength` otherwise. Line 15 is `[Scope('OnPrem')]`.
|
||||
- BCApps [`src/Apps/W1/Subcontracting/Test/Tests/SubcCommentsAttachmentTest.Codeunit.al`](https://github.com/microsoft/BCApps/blob/main/src/Apps/W1/Subcontracting/Test/Tests/SubcCommentsAttachmentTest.Codeunit.al) line 295 asserts the runtime text "The length of the string is 101".
|
||||
|
|
@ -0,0 +1,21 @@
|
|||
codeunit 50150 "Sample Payment Terms Codes"
|
||||
{
|
||||
procedure GenerateUnusedPaymentTermsCode(): Code[10]
|
||||
var
|
||||
PaymentTerms: Record "Payment Terms";
|
||||
LibraryUtility: Codeunit "Library - Utility";
|
||||
RecRef: RecordRef;
|
||||
FieldRef: FieldRef;
|
||||
NewCode: Code[10];
|
||||
begin
|
||||
// Temp = true: RecRef is an empty temporary instance, so IsEmpty()
|
||||
// is true on the first pass and existing Payment Terms are never seen.
|
||||
RecRef.Open(Database::"Payment Terms", true, CompanyName());
|
||||
FieldRef := RecRef.Field(PaymentTerms.FieldNo(Code));
|
||||
repeat
|
||||
NewCode := CopyStr(LibraryUtility.GenerateRandomXMLText(MaxStrLen(NewCode)), 1, MaxStrLen(NewCode));
|
||||
FieldRef.SetRange(NewCode);
|
||||
until RecRef.IsEmpty();
|
||||
exit(NewCode);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,33 @@
|
|||
codeunit 50150 "Sample Payment Terms Codes"
|
||||
{
|
||||
procedure GenerateUnusedPaymentTermsCode(): Code[10]
|
||||
var
|
||||
PaymentTerms: Record "Payment Terms";
|
||||
LibraryUtility: Codeunit "Library - Utility";
|
||||
RecRef: RecordRef;
|
||||
FieldRef: FieldRef;
|
||||
NewCode: Code[10];
|
||||
begin
|
||||
// Temp = false: the loop checks the persisted Payment Terms rows.
|
||||
RecRef.Open(Database::"Payment Terms", false, CompanyName());
|
||||
FieldRef := RecRef.Field(PaymentTerms.FieldNo(Code));
|
||||
repeat
|
||||
NewCode := CopyStr(LibraryUtility.GenerateRandomXMLText(MaxStrLen(NewCode)), 1, MaxStrLen(NewCode));
|
||||
FieldRef.SetRange(NewCode);
|
||||
until RecRef.IsEmpty();
|
||||
exit(NewCode);
|
||||
end;
|
||||
|
||||
procedure GetFieldFilterFromView(TableNo: Integer; TableView: Text; FieldNo: Integer): Text
|
||||
var
|
||||
RecRef: RecordRef;
|
||||
FieldRef: FieldRef;
|
||||
begin
|
||||
// Temp = true is correct here: the RecordRef only parses a view and
|
||||
// never reads rows.
|
||||
RecRef.Open(TableNo, true);
|
||||
RecRef.SetView(TableView);
|
||||
FieldRef := RecRef.Field(FieldNo);
|
||||
exit(FieldRef.GetFilter());
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,45 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: testing
|
||||
keywords: [recordref, open, temporary, isempty, existence-check, uniqueness]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# RecordRef.Open with Temp = true cannot check the real table
|
||||
|
||||
## Description
|
||||
|
||||
`RecordRef.Open(No: Integer [, Temp: Boolean] [, CompanyName: Text])` always takes a real table number. When `Temp` is `true`, though, the `RecordRef` refers to a temporary instance of that table. That instance starts empty and never contains the persisted rows. Microsoft Learn's example opens table 27 temporarily and notes that `Find('-')` "returns false" because "there are no records in a temporary table". So `IsEmpty`, `Find*`, `Get` or `Count` on a temp-opened `RecordRef` only sees rows the same code inserted into it. These calls can't answer "does this value already exist in the table?"
|
||||
|
||||
The defect is easy to miss at the call site. `Temp` is a positional, unnamed Boolean, the table number is genuine, and the code compiles without a diagnostic. The usual shape is a generate-until-unused loop: the code sets a range on a field and repeats `until RecRef.IsEmpty()`. That loop exits on its first pass and returns a value that may already exist. The defect then shows up later as a duplicate-key error or a wrong lookup, often only once the table holds data.
|
||||
|
||||
The shape occurs wherever `RecordRef` is used. This article sits in the testing domain because generic test-library helpers, which work on any table number, are where it typically appears. For how specific `LibraryUtility` helpers behave, including `GenerateRandomCode`'s temporary open, see [`use-generateguid-for-unique-test-fixture-values.md`](use-generateguid-for-unique-test-fixture-values.md). That article owns guidance on which helper to call.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When the code that follows has to see persisted rows, open with `Temp = false` or omit the parameter. Learn's first example omits it so the table "will not be open as temporary table".
|
||||
|
||||
`Temp = true` is correct when the `RecordRef` is deliberately a scratch instance that never answers existence questions about the real table, for example:
|
||||
|
||||
- A field-validation sandbox: insert a temporary row, then `Validate` a field on it.
|
||||
- Parsing a table view with `SetView` and reading `GetFilter`.
|
||||
- Reading table metadata such as `SystemIdNo`.
|
||||
|
||||
See sample: [`recordref-open-temp-parameter-defeats-real-table-checks.good.al`](recordref-open-temp-parameter-defeats-real-table-checks.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`RecRef.Open(<table>, true[, ...])` followed, on the same `RecordRef` and with no `Insert` into it, by `IsEmpty`, `Find`, `FindFirst`, `FindSet`, `Get` or `Count` whose result decides whether a value already exists in the table. The classic case is a uniqueness loop ending `until RecRef.IsEmpty()`.
|
||||
|
||||
See sample: [`recordref-open-temp-parameter-defeats-real-table-checks.bad.al`](recordref-open-temp-parameter-defeats-real-table-checks.bad.al).
|
||||
|
||||
## Source
|
||||
|
||||
- [RecordRef.Open method](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/methods-auto/recordref/recordref-open-method): the syntax, Example 1 (parameters omitted, not temporary) and Example 2 (temporary open is empty, `Find('-')` returns false).
|
||||
- BCApps [`src/Layers/W1/Tests/ApplicationTestLibrary/LibraryUtility.Codeunit.al`](https://github.com/microsoft/BCApps/blob/main/src/Layers/W1/Tests/ApplicationTestLibrary/LibraryUtility.Codeunit.al): `GenerateRandomCode` opens with `true` at line 288 and loops `until RecRef.IsEmpty()`. `GenerateRandomCodeWithLength` opens with `false` at line 309.
|
||||
- Legitimate `Temp = true` uses in BCApps W1:
|
||||
- [`src/Layers/W1/BaseApp/Inventory/Item/ItemTempl.Table.al`](https://github.com/microsoft/BCApps/blob/main/src/Layers/W1/BaseApp/Inventory/Item/ItemTempl.Table.al) line 1255: temporary `Item` insert plus `Validate`.
|
||||
- [`src/Apps/W1/Quality Management/app/src/Utilities/QltyFilterHelpers.Codeunit.al`](https://github.com/microsoft/BCApps/blob/main/src/Apps/W1/Quality%20Management/app/src/Utilities/QltyFilterHelpers.Codeunit.al) line 942: `SetView` plus `GetFilter`.
|
||||
- [`src/Layers/W1/BaseApp/Integration/SynchEngine/IntegrationRecordSynch.Codeunit.al`](https://github.com/microsoft/BCApps/blob/main/src/Layers/W1/BaseApp/Integration/SynchEngine/IntegrationRecordSynch.Codeunit.al) line 221: `SystemIdNo`.
|
||||
Loading…
Add table
Add a link
Reference in a new issue