From 36a2bcd6623ed05a0b399b504601737c6fee3e47 Mon Sep 17 00:00:00 2001 From: Nedelcho Delchev Date: Mon, 5 Oct 2026 11:33:19 +0300 Subject: [PATCH] An agree check can compare against the record's own to-one `agree` resolved both of its sides as `.`, so it could only ever compare two hops. The rule an opening balance needs is the other shape: the record carries a fiscal `year` (which belongs to a company) and a `company` of its own, and what has to agree is `Year.Company == Company` - one hop on the left, none on the right. It could not be authored at all. A side whose hop does not resolve now falls back to the record's own to-one, but only when the OTHER side's hop did resolve: an `onProperty` that names nothing on either target must keep saying so rather than be read as a comparison of two bare foreign keys. Both sides must still end on the same entity - comparing a Company key with a Customer key is refused for the reason #7095 refuses it one construct over, the two nomenclatures making the comparison always false. A path that fails also leaves no hop behind in the walker it shares with the other side, so the own-to-one side emits no load for a record it never reads. Fixes #7631 Co-Authored-By: Claude Opus 5 (1M context) --- .claude/docs/intent-dsl-features.md | 2 + .../intent/generator/CheckSupport.java | 33 ++++++++++++ .../intent/generator/ResolvePathSupport.java | 13 +++++ .../generator/edm/EdmIntentGenerator.java | 8 ++- .../intent/parser/IntentParser.java | 46 +++++++++++++--- .../main/resources/intent-assistant-guide.md | 7 +++ .../generator/edm/EdmIntentGeneratorTest.java | 44 ++++++++++++++++ .../intent/parser/IntentParserTest.java | 52 +++++++++++++++++++ 8 files changed, 197 insertions(+), 8 deletions(-) diff --git a/.claude/docs/intent-dsl-features.md b/.claude/docs/intent-dsl-features.md index 319dd3dc776..131a51d1357 100644 --- a/.claude/docs/intent-dsl-features.md +++ b/.claude/docs/intent-dsl-features.md @@ -56,6 +56,8 @@ Loaded on demand, not on every session: read this before changing a DSL construc **Show it only from a status on (`visibleWhen:`, [#7502](https://github.com/eclipse-dirigible/dirigible/issues/7502)):** composition-child panels rendered unconditionally, so a DRAFT invoice showed empty payment-allocation, copy and reminder panels - `forbidWhen` and `locksWithMaster` hide only the Add/edit/delete affordances and `visibleTo:` gates by role, not status. `visibleWhen: ""` on a field, or on a composition child (written in the master's terms, `Status != DRAFT`), leaves the input or the whole panel out of the generated views until the condition holds against the record the page shows. Same grammar as `requiredWhen` (`==`/`!=`, a list = AND, statuses by seed name); UI-only - nothing is hidden from the API and nothing is deleted when a record regresses. +**An `agree` side may be the record's own to-one ([#7631](https://github.com/eclipse-dirigible/dirigible/issues/7631)):** `agree` resolved both of its sides as `.`, so it could only ever compare two hops - and the rule an opening balance needs is `Year.Company == Company`, where the record carries the agreed target directly. Each side now falls back to the bare relation when the hop does not resolve and the OTHER side's did, which keeps a typo'd `onProperty` saying it names nothing instead of being read as two foreign keys. Both sides must still end on the same entity: a Company key compared with a Customer key is refused for the reason #7095 refuses it a construct over, the comparison being always false. A path that fails now also leaves no hop behind in the shared walker, so the own-to-one side emits no load for a record it never reads. + **A picker can say a target row is not pickable (`pickable:`, [#7496](https://github.com/eclipse-dirigible/dirigible/issues/7496)):** a to-one's dropdown rendered every target row identically, so a clerk picked a customer with no registration number onto an invoice, typed twenty lines, and learned only at Issue that the invoice could not go out - `dependsOn.filterBy`/`where:` narrow by one value, not by a completeness rule. `pickable: { when: [registrationNumber != null, address != null], else: mark|hide, message: ... }` reads the check guard grammar over the TARGET's own properties, plus `!= null` / `== null` presence tests on any field type; `mark` (the default) lists a failing row disabled with the message as a second line (Harmonia's `aria-disabled` + `data-description`), `hide` leaves it out, and a value the record already holds always keeps its label. Emitted as one JSON attribute `widgetPickable`, evaluated by the shared runtime (`basePage.pickableOptions` / `visibleOptions`) on every picker that builds its own options - manage form, document header and line dialog, my/partner forms and documents. **It never gates the server** (a REST client bypasses any picker): the write-side rule stays a `checks:` entry. Details in the engine-intent guide's pickable bullet. **Two values of one row, related (`checks: compare`, [#7095](https://github.com/eclipse-dirigible/dirigible/issues/7095)):** `checks:` knew `exactlyOne`, `itemsSumEqual` and `itemsMin` - nothing compared two fields of the SAME record, so "a due date is never before the invoice date" was not expressible and a document was saved (200), issued and overdue the moment it existed; the module's workaround was a `calculatedActionOnCreate`/`OnUpdate` class per document type that silently CORRECTED the date instead of refusing it, which is a different thing and never tells the clerk. `- { kind: compare, field: due, op: ge, than: date, message: ... }` is row-level by default, like `exactlyOne`: enforced in every generated controller's `validate()` (the entity, personal and partner surfaces) as a 400 carrying the authored message - a rule about two values of one row holds from the first save, not from a transition (the optional gate that routes it to the transition instead arrived with [#7338](https://github.com/eclipse-dirigible/dirigible/issues/7338), below). `op:` is `ge`/`gt`/`le`/`lt`/`eq`/`ne`, spelled out because an omitted operator has no defensible default. Both operands are the entity's own **fields** - a comparison of two foreign keys means nothing - and must sit in ONE comparison family, which is what the generated code needs: two temporals compare through their own `compareTo` (a `LocalDate` does not compare to an `Instant`), two numbers by value through `BigDecimal` so a `decimal` against a `long` stays exact. Only dates, timestamps and numbers compare; a string / `month` / `week` is refused rather than silently ordered lexicographically, as is a field-with-itself. An **absent operand is not a violation** - a comparison is about two values that exist, and requiredness is its own declaration. diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/CheckSupport.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/CheckSupport.java index 4a982aefbb0..e45a2e05be6 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/CheckSupport.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/CheckSupport.java @@ -412,6 +412,39 @@ public static Map term(String owner, String property, Comparison return term; } + /** + * Both sides of an {@code agree} check, resolved (issue #7631). + * + *

+ * Normally a side is the path {@code .} - the shared value read off the + * record the relation points at. But a record may carry the shared target DIRECTLY as its own + * to-one: an {@code OpeningBalance} has {@code Year -> FiscalYear} (which has {@code Company}) and + * its own {@code Company}, and "the opening balance opens the books of the company its fiscal year + * belongs to" is {@code Year.Company == Company}. That side is then the record's own foreign key, + * which the same walker resolves from the bare relation name. + * + * @param walker the walker both sides share, so a prefix they have in common loads once + * @param entity the record the check is declared on + * @param left the authored relation naming the first side + * @param right the authored relation naming the second side + * @param onProperty the shared property + * @return the two resolved paths, in the authored order - each the hop, or the record's own to-one + */ + public static ResolvePathSupport.Path[] agreeSides(ResolvePathSupport.Walker walker, EntityIntent entity, String left, String right, + String onProperty) { + String[] names = {left, right}; + ResolvePathSupport.Path[] sides = {walker.resolve(left + "." + onProperty), walker.resolve(right + "." + onProperty)}; + for (int i = 0; i < 2; i++) { + // Only when the OTHER side found the shared property: a side that cannot reach it while + // nothing else can either is an onProperty that names nothing, and that must keep saying so + // rather than be read as two bare foreign keys. + if (!sides[i].resolved() && sides[1 - i].resolved() && toOne(entity, names[i]) != null) { + sides[i] = walker.resolve(names[i]); + } + } + return sides; + } + /** * The entity's field of that name, matched case-insensitively - the guard renders the property * through {@link IntentNaming#pascalCase}, so the case an author wrote it in never reaches the diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/ResolvePathSupport.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/ResolvePathSupport.java index 55925b938b8..88030ca2d5e 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/ResolvePathSupport.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/ResolvePathSupport.java @@ -166,6 +166,19 @@ public List hops() { * @return the resolved path, or a failure carrying the reason */ public Path resolve(String authored) { + // A path that does not resolve must leave no hop behind: a caller that tries one shape and + // falls back to another (an `agree` side that is the record's own to-one, #7631) would + // otherwise emit a load for the record it never reads. + List before = new ArrayList<>(steps.keySet()); + Path path = walk(authored); + if (!path.resolved()) { + steps.keySet() + .retainAll(before); + } + return path; + } + + private Path walk(String authored) { if (authored == null || authored.isBlank()) { return failed(authored, "is blank"); } diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java index 551c2227088..4a9e7999109 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java @@ -2570,8 +2570,12 @@ private static List> buildChecks(EntityIntent entity, List.}, or - for a side reading the record's own to-one - that + * relation's own target. {@code null} when the side ends on a scalar or leaves this model, where + * nothing here can say. + */ + private static String agreedTarget(EntityIntent entity, java.util.Map byName, String relation, + String onProperty) { + RelationIntent own = CheckSupport.toOne(entity, relation); + if (own == null) { + return null; + } + EntityIntent target = byName.get(own.getTo()); + RelationIntent shared = target == null ? null : CheckSupport.toOne(target, onProperty); + if (shared != null) { + return shared.getTo(); // the hop's own to-one - the usual junction shape + } + return target == null || CheckSupport.field(target, onProperty) != null ? null : own.getTo(); + } + private static void validateAgreeCheck(EntityIntent entity, CheckIntent check, java.util.Map byName, String subject, List issues) { List relations = check.getRelations(); @@ -6097,18 +6117,32 @@ private static void validateAgreeCheck(EntityIntent entity, CheckIntent check, j // reported by the walker in the vocabulary every other path failure uses. ResolvePathSupport.Walker walker = ResolvePathSupport.walker(entity, byName, java.util.Map.of(), null); String[] terminals = new String[2]; + String[] agreedOn = new String[2]; for (int i = 0; i < 2; i++) { - String relation = relations.get(i); - if (relation == null || relation.isBlank()) { + if (relations.get(i) == null || relations.get(i) + .isBlank()) { issues.add(subject + " relations[" + i + "] is blank"); return; } - ResolvePathSupport.Path path = walker.resolve(relation + "." + on); - if (!path.resolved()) { - issues.add(subject + " " + path.failure()); + } + ResolvePathSupport.Path[] sides = CheckSupport.agreeSides(walker, entity, relations.get(0), relations.get(1), on); + for (int i = 0; i < 2; i++) { + if (!sides[i].resolved()) { + issues.add(subject + " " + sides[i].failure()); return; } - terminals[i] = path.terminalType(); + terminals[i] = sides[i].terminalType(); + agreedOn[i] = agreedTarget(entity, byName, relations.get(i), on); + } + // Both sides must name the same third thing. A side reading the record's OWN to-one (#7631) + // compares a foreign key, and a key of the wrong nomenclature is the one mistake this widening + // makes reachable - comparing a Company id with a Customer id is the "two foreign keys mean + // nothing" refusal #7095 draws, one construct over. + if (agreedOn[0] != null && agreedOn[1] != null && !agreedOn[0].equals(agreedOn[1])) { + issues.add(subject + " compares a [" + agreedOn[0] + "] with a [" + agreedOn[1] + + "] - both sides must point at the same entity, or the keys are from different nomenclatures and the" + + " comparison is always false"); + return; } for (int i = 0; i < 2; i++) { if (terminals[i] == null || ResolvePathSupport.RELATION_TERMINAL.equals(terminals[i])) { diff --git a/components/engine/engine-intent/src/main/resources/intent-assistant-guide.md b/components/engine/engine-intent/src/main/resources/intent-assistant-guide.md index da39c04adcf..9e28c3ad4e0 100644 --- a/components/engine/engine-intent/src/main/resources/intent-assistant-guide.md +++ b/components/engine/engine-intent/src/main/resources/intent-assistant-guide.md @@ -620,6 +620,13 @@ field may declare: mandatory); `whenNull: refuse` rejects the write instead. Reach for this instead of writing the rule as a `calculatedActionOnCreate`/`OnUpdate` guard class - it is the shape every allocation, transfer, timesheet and assignment entity carries. + A side may also be the record's OWN to-one carrying that third thing directly (#7631): an + opening balance has a fiscal `year` (which belongs to a company) and a `company` of its own, and + the rule that matters is `Year.Company == Company` - one hop on the left, none on the right. + Author it the same way, naming the own relation as a side: + `{ kind: agree, relations: [year, company], onProperty: company }`. Both sides must still end on + the same entity: comparing a Company key with a Customer key is refused, the two nomenclatures + making the comparison always false. - `{ kind: requiredWhen, field: vatGround, whenAnyItem: "vatRate == 0", status: ISSUED, message: "..." }` (#7560): a header value required when **ANY LINE** satisfies the condition - the legal ground a zero-rated line calls for (ЗДДС чл. 114). `whenAnyItem` uses the `when` grammar over the ITEMS diff --git a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGeneratorTest.java b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGeneratorTest.java index b2ddf05d717..20397d729dc 100644 --- a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGeneratorTest.java +++ b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGeneratorTest.java @@ -1734,6 +1734,50 @@ void agreeChecksEmitBothSidesAndTheirLoads() { assertEquals("The Sales Invoice and the Customer Payment must have the same Customer", refusing.get("message")); } + /** + * One side of an {@code agree} may be the record's OWN to-one (#7631): a budget line holds the year + * and the company it books on, and the year belongs to a company too - the rule that matters is + * "the year's company is this company", which has one hop on the left and none on the right. + */ + @Test + @SuppressWarnings("unchecked") + void anAgreeSideMayBeTheRecordsOwnToOne() { + String yaml = """ + name: budgeting + entities: + - name: Company + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: name, type: string } + - name: Year + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: company, kind: manyToOne, to: Company } + - name: Budget + checks: + - { kind: agree, relations: [year, company], onProperty: company } + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: year, kind: manyToOne, to: Year } + - { name: company, kind: manyToOne, to: Company } + """; + Map model = EdmIntentGenerator.buildModelJsonForTest(IntentParser.parse(yaml), "budgeting"); + Map agree = ((List>) entityByName(entities(model), "Budget").get("checks")).get(0); + + assertEquals("(hop0 == null ? null : hop0.Company)", agree.get("leftExpression")); + assertEquals("entity.Company", agree.get("rightExpression"), "the own side is read off the record, with no hop at all"); + assertEquals("Year.Company", agree.get("leftLabel")); + assertEquals("Company", agree.get("rightLabel")); + List> loads = (List>) agree.get("pathLoads"); + assertEquals(1, loads.size(), "only the hopped side is loaded: " + loads); + assertEquals("entity.Year", loads.get(0) + .get("sourceExpression")); + assertEquals("The Year and the Company must have the same Company", agree.get("message"), + "the unauthored message names both sides and the property, as it does for two hops"); + } + /** * The parent side of an {@code agree} check (#7589): each record the junction links carries an * {@code agreeGuards} entry naming the junction, its foreign key and the agreed property, so the diff --git a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/IntentParserTest.java b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/IntentParserTest.java index 1db33f7c66c..7195408666b 100644 --- a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/IntentParserTest.java +++ b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/IntentParserTest.java @@ -1033,6 +1033,58 @@ void agreeChecksParseAndValidate() { assertCompareIssue(yaml.replace("onProperty: customer,", "onProperty: customer, status: 1,"), "cannot carry a `status` gate"); } + /** + * A side of an {@code agree} may be the record's OWN to-one (#7631): the fiscal year of an opening + * balance belongs to a company, and so does the balance itself, so what has to agree is + * {@code Year.Company == Company} - one hop on the left, none on the right. The widening makes one + * new mistake reachable, comparing two foreign keys of different nomenclatures, which is refused + * for the reason #7095 refuses it a construct over: the comparison is simply always false. + */ + @Test + void anAgreeSideMayBeTheRecordsOwnToOne() { + String yaml = """ + name: ledger + entities: + - name: Company + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - name: Customer + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - name: Year + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: company, kind: manyToOne, to: Company } + - name: OpeningBalance + checks: + - { kind: agree, relations: [year, company], onProperty: company, + message: "This balance opens the books of another company than its fiscal year" } + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: year, kind: manyToOne, to: Year } + - { name: company, kind: manyToOne, to: Company } + - { name: customer, kind: manyToOne, to: Customer } + """; + CheckIntent check = IntentParser.parse(yaml) + .getEntities() + .get(3) + .getChecks() + .get(0); + + assertEquals(List.of("year", "company"), check.getRelations()); + assertEquals("company", check.getOnProperty()); + + // The two keys must be drawn from the same nomenclature - a Company id and a Customer id are + // never equal, so comparing them is a rule that can only ever refuse. + assertCompareIssue(yaml.replace("relations: [year, company]", "relations: [year, customer]"), + "both sides must point at the same entity"); + // And a property neither side can reach still says exactly that, rather than being read as a + // comparison of the two bare foreign keys. + assertCompareIssue(yaml.replace("onProperty: company,", "onProperty: supplier,"), "has no field or to-one relation [supplier]"); + } + /** * The soft tier (#7466): the three cases the billing review asked for - a second customer with the * same name, a second product with the same name, a document line at price zero - are warnings the