From c8ec145d71946bb3acb6d36840ccf96814e28771 Mon Sep 17 00:00:00 2001 From: delchev Date: Thu, 17 Sep 2026 10:34:06 +0300 Subject: [PATCH] intent: a checks kind for two relations of a junction row that must agree on a shared target (#7409) A junction/allocation entity links two records that both point at the same third thing - the same Customer, the same Currency, the same Company - and no `checks:` kind could say so: `compare` relates two values of ONE row, and `requiredWhen`/`forbidWhen` relate a child to its OWN parent. So a EUR CustomerPayment of customer B was allocated against a USD SalesInvoice of customer A and both writes answered 200; a fleet probe found the same shape in 7 more places across 6 modules, every one closed by a hand-written calculatedActionOnCreate/OnUpdate guard class whose entire content was a rule the intent should be able to state. checks: - { kind: agree, relations: [SalesInvoice, CustomerPayment], onProperty: Customer, message: "..." } `relations:` names exactly two distinct to-one relations of the entity and `onProperty:` the property BOTH their targets declare - one of their to-ones (compared by its foreign key) or a scalar an equality is exact on. Each side is ONE PATH - `.` through the same ResolvePathSupport walker every other path in the DSL uses, sharing one walker so each related record is loaded exactly once and a cross-model target reads through its `uses:` owner like any other hop - and the comparison is emitted into the three controllers' validate() as a 400 with the authored message. `whenNull:` defaults to `skip` (a row not yet carrying both values has nothing to disagree about, and requiredness is the relation's own declaration); `refuse` rejects instead. The key is `onProperty`, not the proposed `on`: YAML 1.1 resolves a bare `on` key to the boolean true, so the declaration would arrive as the key `true`, bind to nothing and generate a check with no property to agree on. That spelling is refused by name on the raw tree, as `lifecycle`'s `on` already is. Refused at parse, each because the declaration could not mean anything: not exactly two relations, the same relation twice, a relation or an `onProperty` a target does not declare, a `status:` gate (two relations either agree or they do not, from the first save), an unknown `whenNull`, a terminal an equality is not exact on (a decimal, a double, a date), and two terminals of DIFFERENT types, where the boxed comparison is silently always false. Co-Authored-By: Claude Opus 5 --- components/engine/engine-intent/CLAUDE.md | 1 + .../generator/edm/EdmIntentGenerator.java | 40 ++++++ .../components/intent/model/CheckIntent.java | 49 +++++++ .../intent/parser/IntentParser.java | 132 +++++++++++++++++- .../main/resources/intent-assistant-guide.md | 14 ++ .../generator/edm/EdmIntentGeneratorTest.java | 60 ++++++++ .../intent/parser/IntentParserTest.java | 74 ++++++++++ .../model/ModelParameterProcessor.java | 4 +- .../api/EntityController.java.template | 20 +++ .../api/EntityMyController.java.template | 20 +++ .../api/EntityPartnerController.java.template | 20 +++ .../tests/api/IntentEmissionCoverageIT.java | 34 +++++ 12 files changed, 465 insertions(+), 3 deletions(-) diff --git a/components/engine/engine-intent/CLAUDE.md b/components/engine/engine-intent/CLAUDE.md index 3fe96a7f9f0..fc7e7ebd926 100644 --- a/components/engine/engine-intent/CLAUDE.md +++ b/components/engine/engine-intent/CLAUDE.md @@ -444,6 +444,7 @@ Semantics worth knowing: - **Re-parenting is a two-sided event, and `-rekeyed` is the whole mechanism (#6819).** A row whose grouping column moves - an `aggregates:` key, or a `rollups:` child's `via` FK - leaves one group and joins another, and the ordinary events name only the group it belongs to NOW: `-updated` carries the written row, so the group it LEFT is named by nothing and kept the row's contribution forever (a cost centre reassigned by a workflow step; a `sum` roll-up whose parent FK an ordinary edit re-points). The repair is one dedicated topic, `---rekeyed`, which **only** the generated aggregate / roll-up handlers subscribe to - so a write can signal them without re-publishing `-updated` and spuriously re-firing every reaction. Three parts, and all three are needed: (1) the entity's `.model` carries **`groupingKeys`** - the union of every aggregate key over it AND every roll-up `via` FK whose child it is (`EdmIntentGenerator`; it used to be `aggregateKeys`, aggregates-only, which is why re-parenting a roll-up child was invisible); (2) the DAO compares those columns before/after on **both** write paths - the full-row `update()` publishes the PREVIOUS row (the group it moved into is recomputed off `-updated` like any other change), and `updateProperties` - the targeted primitive every workflow setter, `resolves:` and task-form writer goes through, which publishes no `-updated` at all - publishes the previous row AND the written one, since on that path neither side has an event otherwise; (3) both handler families bind it, the aggregate as its `OnRekey` variant and the roll-up as `RollupOnRekey`. Each handler recomputes the group the PAYLOAD names, from the store, so one class repairs either side and re-delivery converges. The publish is gated on a key having actually moved, so a normal edit costs nothing extra and the cascade still terminates at rest. - **A roll-up's CHILD may be owned by another model (#6930), which is the n:m allocation direction.** `rollups: [{ entity: , model: , parent: , via: , field: ..., op: sum, of: ... }]` - declared by the module that owns the PARENT. The cross-model *parent* direction (a local child, `via`'s own `model:`) already existed, but the inverse was inexpressible, and it is the one an n:m pairing forces: the link entity lives with the document that owns ONE side (`SalesInvoiceCustomerPayment` belongs to `sales-invoices`, whose `invoicePaid` roll-up is local and works), while the OTHER side's total (`CustomerPayment.allocated`, and the `unapplied` figure derived from it) belongs to the module that owns the payment - so it had no declarative form at all and was answered by a register report instead of a stored, filterable number. **`parent:` is authored rather than derived** because a foreign child's relations are not in this document: nothing here can walk `via` to a target, which is also why `via` / `of` / `by` are resolved against the OWNER's `.model` at generation time (`firstUnresolvableChildProperty`, the schedules' cross-model-source rule) and a miss drops the roll-up loudly. The parent must be LOCAL - a total landing in a third model is that model's roll-up to declare, and writing it from here would invert the dependency edge. Emission-wise the child's coordinates simply come from the owner: `childProject` (the topic - this project publishes nothing about that entity, so a local topic would subscribe to silence) and `childGenFolder` (the imports), both defaulting to this project so **a local roll-up renders byte-identically**; the class name is prefixed with the owner alias and the pipeline's coalescing key gains `childModel`, because a local and a foreign child of the same name rolling up through the same relation are two handlers, and one class name for both would have the pipeline write one file over the other. Three deliberate limits: **`capacity`/`balance`/`status` are refused** (the capacity guard lives on the CHILD's DAO, which the owner model generates - a recomputed balance with no guard behind it would look like a limit and enforce nothing); **the vacated side of a re-parent is repaired only if the owner marks that relation as a grouping key**, since `-rekeyed` is published by the owner's DAO and `groupingKeys` is the union over the OWNER's own consumers (the handler is emitted regardless - it is the same store-driven recompute and converges whenever the notice does arrive; delete + re-create is always exact); and **`sensitive:`/`visibleTo:` do not propagate** from a foreign `of` field, so a restricted total must declare its own restriction. Both `EdmIntentGenerator` sites that walk `model.getRollups()` skip a cross-model child (`groupingKeys`, `buildRollupGuards`), as do the two parser propagation loops - otherwise a local entity that merely SHARES the foreign child's name would be treated as it. Covered by `GlueRollupCrossModelTest` (the emitted coordinates + class name, the local case unchanged, and every refusal). - **A roll-up's `status:` is relinquished, not only set (#7016).** The `statusWhenFull` / `statusWhenPartial` branch modelled "money arrives" and forgot "money leaves": the recompute had no `else`, so deleting the only allocation of a PAID invoice left it PAID with Paid 0 / Balance = Payable - and invisible to the settlement, whose payable statuses are ISSUED/SENT/PARTIAL. Now the FIRST move into a roll-up-owned status snapshots the status it displaces into a hidden, read-only INTEGER column on the parent, `Displaced` (`IntentNaming.displacedStatusProperty`, emitted by `EdmIntentGenerator.displacedStatusProperty` for every local parent of a capacity roll-up with a status, one per status relation; `GlueIntentGenerator.buildRollups` hands it to the emitter as `statusDisplacedField`), and a sum back at zero restores it when - and only when - the parent still holds one of the two roll-up-owned statuses, then clears the snapshot (`RollupAggregates.appendStatus`; every variant, create/update/delete/rekey, since an allocation amended to 0 or re-parented away is the same situation as a deleted one). Remembering beats a declared `statusWhenEmpty:` - that is wrong for every invoice paid straight from ISSUED and never CONFIRMED - so there is no such key. A roll-up-owned status with no recorded predecessor (a deployment upgraded mid-payment) is logged and left alone, never guessed. Both writes ride the same `derived` map into ONE `updateDerived`, so the parent's listeners see one `-updated`. The column is hidden through the **`isHiddenProperty`** flag, which is now the ONE thing the Harmonia templates consult to leave bookkeeping out of forms, lists and details blocks (`ModelParameterProcessor` sets it from the model and BY NAME for `ProcessIds`, so a `.model` written before the flag existed still hides the stamps; the modeler's serializer carries unknown attributes through the generic pass, so a hand round-trip keeps it); `isReadOnlyProperty` puts it in `preservedOnUpdate`, so a full-row form save cannot null it. With a `lifecycle:` on the parent the moves back must be declared edges like the moves in. +- **`checks: kind: agree` = two relations of a JUNCTION row must agree on a shared target (#7409).** The shape no kind reached: `compare` relates two values of ONE row and `requiredWhen`/`forbidWhen` relate a child to its own parent, but nothing compared two DIFFERENT relations' targets - so a EUR `CustomerPayment` of customer B was allocated against a USD `SalesInvoice` of customer A and both writes answered 200. A fleet probe found the same shape in 7 more places across 6 modules (`PurchaseInvoicePayment`, `StockTransfer`, `EmployeeTimesheet`, `EmployeeProjectAssignment`, `Payslip`, `VacationDay`), every one closed by a hand-written `calculatedActionOnCreate`/`OnUpdate` guard class whose entire content was a rule the intent should be able to state. Authored as `{ kind: agree, relations: [, ], onProperty: , whenNull?: skip | refuse, message }`. **Both sides are ONE PATH each** - `.` through the same `ResolvePathSupport` walker every other path in the DSL uses, sharing ONE walker so each related record is loaded exactly once and a cross-model target reads through its `uses:` owner like any other hop - and the comparison is emitted into the three controllers' `validate()` as a 400, next to `exactlyOne` and `compare`. **The key is `onProperty`, not `on`:** YAML 1.1 resolves a bare `on` key to the boolean `true`, so an `on:` declaration would arrive as the key `true`, bind to nothing and generate a check with no property to agree on - `rejectCheckOn` refuses that spelling by name on the raw tree (the issue proposed it, so authors and the assistant will write it), the same treatment `lifecycle`'s `on` gets. **Refused at parse**, each because the declaration could not mean anything: fewer or more than two relations, the same relation twice (it always agrees with itself), a relation or an `onProperty` a target does not declare (the walker's own message), a `status:` gate (two relations either agree or they do not, from the first save), an unknown `whenNull`, an `onProperty` whose terminal is not an exact equality - a to-one's foreign key, a string, an integer or a boolean, the same line a `when` condition draws, so a `decimal`/`date` is refused rather than compared by an equality nobody means - and two terminals of DIFFERENT types, where the boxed comparison is silently always false. `whenNull` defaults to `skip`: a row not yet carrying both values has nothing to disagree about, and requiredness is the relation's own declaration. Unit: `IntentParserTest.agreeChecksParseAndValidate`, `EdmIntentGeneratorTest.agreeChecksEmitBothSidesAndTheirLoads`; IT: `IntentEmissionCoverageIT` (both `whenNull` readings, compiled). - **`checks: kind: guard` = a precondition over a keyed `aggregates:` sum, with three outcomes.** The negative-stock / credit-limit / remaining-allowance shape: `aggregate:` names an `aggregates:` entry whose `of` is THIS entity (v1 self-referential), and the post-state is checked against `minimum:` (default 0). The sum is recomputed SYNCHRONOUSLY from the guarded entity's own store for the incoming row's key-tuple, excluding this row on update, then the incoming value is added - deliberately NOT read from the async-maintained aggregate target, so the decision cannot race the handler. Consequence worth remembering: the guard and the materialised aggregate are two independent computations of the same sum, and the guard is the authoritative one - do not "optimise" it into a target read. `enabledBy: ` wraps the whole guard in a `Configurations.get(key) == "true"` gate (a tenant-level business toggle). Emitted by `EdmIntentGenerator.buildChecks` (keys + `sumField` + `pk` + `minimum` + `enabledBy` + `outcome`) → `ModelParameterProcessor` splits `guardChecks` out → the DAO's `#aggregateGuardCheck` macro at both the save and update sites. **`outcome:` decides what a violation DOES**, and each non-default outcome carries its own companion key (parser-validated - a companion belonging to another outcome is an ERROR, since the write would look guarded and do nothing): - **`block`** (the default) - throws `ValidationException`, so the REST write fails with 4xx and nothing is persisted. - **`task`** + **`marker: `** - does NOT fail the write. It stamps the marker (`false` on violation, `true` when it holds) as the BRANCH INPUT a process `decision` reads to route the record to a hold/review step. The division of labour is deliberate and must stay documented as such: this keyword stamps a flag, the process decides what the flag means - the DSL neither creates nor routes to a task. 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 911a681ec5a..a30b5c199d6 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 @@ -2215,6 +2215,46 @@ private static List> buildChecks(EntityIntent entity, List.` - resolved through the + // shared walker, so the two loads are accumulated once per relation and a cross-model + // target reads like any other hop. No gate: two relations either agree or they do not, + // from the first save. + List relations = check.getRelations(); + if (relations == null || relations.size() != 2 || check.getOnProperty() == null) { + continue; // the parser already reported it + } + ResolvePathSupport.Walker walker = ResolvePathSupport.walker(entity, byName, compositionParents, crossModel); + ResolvePathSupport.Path left = walker.resolve(relations.get(0) + "." + check.getOnProperty()); + ResolvePathSupport.Path right = walker.resolve(relations.get(1) + "." + check.getOnProperty()); + if (!left.resolved() || !right.resolved()) { + continue; // the parser already reported it + } + checkMap.put("leftExpression", left.expression()); + checkMap.put("rightExpression", right.expression()); + checkMap.put("leftLabel", left.label()); + checkMap.put("rightLabel", right.label()); + checkMap.put("whenNull", check.getWhenNull() == null || check.getWhenNull() + .isBlank() ? "skip" + : check.getWhenNull() + .trim() + .toLowerCase(java.util.Locale.ROOT)); + List> pathLoads = pathLoadsOf(walker); + if (!pathLoads.isEmpty()) { + checkMap.put("pathLoads", pathLoads); + } + if (check.getMessage() == null || check.getMessage() + .isBlank()) { + // A check with no authored message still has to say something the person who pressed + // Save can act on, and only the declaration knows what disagreed. + checkMap.put("message", + "The " + IntentNaming.humanize(relations.get(0)) + " and the " + IntentNaming.humanize(relations.get(1)) + + " must have the same " + IntentNaming.humanize(check.getOnProperty())); + } + checkMaps.add(checkMap); + continue; + } if ("forbidWhen".equals(check.getKind())) { // The reject-twin of requiredWhen (#7275): the condition compiled to a Java boolean, but // now a term may read a value ONE HOP away (`SalesInvoice.Status == PAID`) so a child can diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/model/CheckIntent.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/model/CheckIntent.java index 1cb6336ffe5..ea239025a39 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/model/CheckIntent.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/model/CheckIntent.java @@ -24,6 +24,12 @@ * enforced on every user write; with a {@link #status} gate it is the repository's, and holds when * the record is persisted carrying that status - "days > 0 before SUBMITTED" rather than on the * first draft; + *
  • {@code agree}: the two to-one {@link #relations} of a junction row must AGREE on + * {@link #onProperty} - the property both their targets declare (a payment allocated against an + * invoice of another customer, or in another currency, is the rule a hand-written guard class used + * to carry). Row-level, so it is enforced on every user write; {@link #whenNull} decides what an + * unset side means, and defaults to skipping (the relation's own {@code required} is what makes it + * mandatory);
    • *
  • {@code requiredWhen}: {@link #field} - the record's own field, or a one-hop * {@code Relation.field} - must carry a value while {@link #when} holds (an e-mailed invoice needs * the customer's address). Enforced on every user write, or, with a {@link #status} gate, when the @@ -75,6 +81,25 @@ public class CheckIntent { private Object value; /** {@code itemsSumEqual}: the two numeric item fields whose sums must be equal. */ private List over; + /** + * {@code agree}: exactly two to-one relations of the entity - the two records the junction row + * links, which must point at the same {@link #onProperty}. + */ + private List relations; + /** + * {@code agree}: the property BOTH targets declare and must agree on - a to-one of theirs (compared + * by its foreign key: the same {@code Customer}, the same {@code Currency}) or a scalar field with + * an exact equality. Spelled {@code onProperty} and not {@code on}, because YAML 1.1 resolves a + * bare {@code on} key to the boolean {@code true} and the declaration would silently bind to + * nothing - the parser refuses that spelling by name rather than dropping it. + */ + private String onProperty; + /** + * {@code agree}: what an unset side means - {@code skip} (the default: a row that does not carry + * both values yet has nothing to disagree about, and requiredness is its own declaration) or + * {@code refuse}. + */ + private String whenNull; /** {@code itemsMin}: the minimum number of items. */ private Integer count; /** @@ -246,6 +271,30 @@ public void setOver(List over) { this.over = over; } + public List getRelations() { + return relations; + } + + public void setRelations(List relations) { + this.relations = relations; + } + + public String getOnProperty() { + return onProperty; + } + + public void setOnProperty(String onProperty) { + this.onProperty = onProperty; + } + + public String getWhenNull() { + return whenNull; + } + + public void setWhenNull(String whenNull) { + this.whenNull = whenNull; + } + public Integer getCount() { return count; } diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/parser/IntentParser.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/parser/IntentParser.java index 041e762be29..0e57ca4351e 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/parser/IntentParser.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/parser/IntentParser.java @@ -326,6 +326,7 @@ public static IntentModel parse(String yaml) { rejectRemovedNumberKeys(tree); rejectEmptyVisibleTo(tree); rejectLifecycleOn(tree); + rejectCheckOn(tree); moveGeneratesItemLines(tree); expandUniqueShorthand(tree); normalizeDuplicable(tree); @@ -5433,6 +5434,10 @@ private static void validateCheck(EntityIntent entity, CheckIntent check, java.u validateCompareCheck(entity, check, subject, issues); return; } + if ("agree".equals(kind)) { + validateAgreeCheck(entity, check, byName, subject, issues); + return; + } if ("itemsSumEqual".equals(kind) || "itemsMin".equals(kind)) { EntityIntent items = compositionChildOf(entity, entities); if (items == null) { @@ -5472,8 +5477,99 @@ private static void validateCheck(EntityIntent entity, CheckIntent check, java.u } return; } - issues.add( - subject + " has unknown kind - expected exactlyOne, compare, requiredWhen, forbidWhen, guard, itemsSumEqual or itemsMin"); + issues.add(subject + + " has unknown kind - expected exactlyOne, compare, agree, requiredWhen, forbidWhen, guard, itemsSumEqual or itemsMin"); + } + + /** + * An {@code agree} check relates the two records a JUNCTION row links: both must point at the same + * third thing (dirigible #7409). An allocation carries a {@code SalesInvoice} and a + * {@code CustomerPayment}; nothing in the DSL could say that the payment's customer must be the + * invoice's, so a EUR payment of customer B was allocated against a USD invoice of customer A and + * the write answered 200. {@code compare} relates two values of ONE row and the parent-child kinds + * relate a child to its own parent - neither reaches across two different relations, which is why + * every module carrying this shape had to write the rule as a Java guard class instead. + * + *

    + * {@code relations} names exactly two DISTINCT to-one relations of this entity, and + * {@code onProperty} the property BOTH their targets declare - resolved as the path + * {@code .} through the same walker every other path in the DSL uses, so a + * cross-model target reads too. The two terminals must be the same KIND of value: two foreign keys + * (the same customer, the same currency) or two fields of one exactly-comparable type. A decimal, a + * double or a date is deliberately not comparable here for the reason a condition does not compare + * them either - an equality on them is held by nobody who means it - and a terminal type that + * differs between the two sides is refused rather than compared across types, where the boxed + * comparison is silently always-false. + */ + private static void validateAgreeCheck(EntityIntent entity, CheckIntent check, java.util.Map byName, + String subject, List issues) { + List relations = check.getRelations(); + if (relations == null || relations.size() != 2) { + issues.add(subject + " requires `relations`: exactly two to-one relations of [" + entity.getName() + "]"); + return; + } + String on = check.getOnProperty(); + if (on == null || on.isBlank()) { + issues.add(subject + " requires `onProperty`: the property both targets declare and must agree on"); + return; + } + if (relations.get(0) != null && relations.get(0) + .equalsIgnoreCase(relations.get(1))) { + issues.add(subject + " names [" + relations.get(0) + "] twice - a relation always agrees with itself"); + return; + } + if (check.getStatus() != null) { + issues.add(subject + " is row-level and cannot carry a `status` gate - two relations either agree or they do not," + + " from the first save"); + } + String whenNull = whenNullOf(check); + if (!"skip".equals(whenNull) && !"refuse".equals(whenNull)) { + issues.add(subject + " has unknown `whenNull` [" + check.getWhenNull() + "] - expected skip or refuse"); + } + // Both operands are walked as one path each, so an `onProperty` a target does not declare is + // 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]; + for (int i = 0; i < 2; i++) { + String relation = relations.get(i); + if (relation == null || relation.isBlank()) { + issues.add(subject + " relations[" + i + "] is blank"); + return; + } + ResolvePathSupport.Path path = walker.resolve(relation + "." + on); + if (!path.resolved()) { + issues.add(subject + " " + path.failure()); + return; + } + terminals[i] = path.terminalType(); + } + for (int i = 0; i < 2; i++) { + if (terminals[i] == null || ResolvePathSupport.RELATION_TERMINAL.equals(terminals[i])) { + continue; // a foreign key, or a cross-model terminal whose type is not known here + } + String type = CheckSupport.guardType(terminals[i]); + if (!CheckSupport.GUARD_TYPES.contains(type)) { + issues.add(subject + " agrees on [" + relations.get(i) + "." + on + "], a [" + terminals[i] + + "] - two relations agree on a reference, a string, an integer or a boolean," + + " the values an equality is exact on"); + return; + } + } + if (terminals[0] != null && terminals[1] != null && !CheckSupport.guardType(terminals[0]) + .equals(CheckSupport.guardType(terminals[1]))) { + issues.add(subject + " compares [" + relations.get(0) + "." + on + "], a [" + terminals[0] + "], with [" + relations.get(1) + + "." + on + "], a [" + terminals[1] + + "] - both sides must be the same kind of value, or the comparison is always false"); + } + } + + /** An {@code agree} check's {@code whenNull}, normalised, defaulting to {@code skip}. */ + private static String whenNullOf(CheckIntent check) { + return check.getWhenNull() == null || check.getWhenNull() + .isBlank() ? "skip" + : check.getWhenNull() + .trim() + .toLowerCase(java.util.Locale.ROOT); } /** @@ -5837,6 +5933,38 @@ private static void rejectLifecycleOn(Object tree) { } } + /** + * An {@code agree} check names the shared property with {@code onProperty}, never {@code on}: YAML + * 1.1 resolves a bare {@code on} key to the boolean {@code true}, so the declaration would arrive + * as the key {@code true}, bind to nothing, and the check would generate with no property to agree + * on. The proposal that opened #7409 spelled it {@code on}, so authors and the assistant will write + * it - it is refused by name here, on the raw tree while the spelling still exists, rather than + * dropped silently. + * + * @param tree the raw parsed YAML + */ + private static void rejectCheckOn(Object tree) { + if (!(tree instanceof Map root) || !(root.get("entities") instanceof List entities)) { + return; + } + List issues = new ArrayList<>(); + for (Object entityNode : entities) { + if (!(entityNode instanceof Map entity) || !(entity.get("checks") instanceof List checks)) { + continue; + } + for (Object checkNode : checks) { + if (checkNode instanceof Map check && (check.containsKey("on") || check.containsKey(Boolean.TRUE))) { + issues.add("entity [" + entity.get("name") + "] check [" + check.get("kind") + + "] declares `on` - YAML reads a bare `on` as the boolean true, so the key never arrives;" + + " spell it `onProperty`"); + } + } + } + if (!issues.isEmpty()) { + throw new IntentValidationException(issues); + } + } + private static void rejectRemovedNumberKeys(Object tree) { if (!(tree instanceof Map root)) { return; 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 77e30dc5ed8..822292c2e9a 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 @@ -551,6 +551,20 @@ field may declare: "a submitted request covers at least one day" without forbidding the draft still being filled in - the rule to reach for instead of mis-authoring it as an `itemsMin` over a child the approval step has not created yet. A gated compare needs the `function: EntityStatus` relation. + - `{ kind: agree, relations: [SalesInvoice, CustomerPayment], onProperty: Customer, message: "..." }` + (#7409): the two records a JUNCTION row links must point at the same third thing - **this is how + "a payment may only be allocated against an invoice of the same customer, in the same currency" + is declared.** `relations:` names exactly two distinct to-one relations of the entity and + `onProperty:` the property BOTH their targets declare - one of their to-one relations (compared + by its foreign key: the same `Customer`, the same `Currency`, the same `Company`) or a scalar + field an equality is exact on (a string, an integer, a boolean). A cross-model target resolves + through its `uses:` owner like every other path. Enforced on every user write (400 with the + message), so it takes no `status:` gate. **The key is `onProperty`, never `on`** - YAML reads a + bare `on` as the boolean `true`, so that spelling never arrives and is refused by name. An unset + side is skipped by default (`whenNull: skip` - the relation's own `required:` is what makes it + 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. - `{ kind: requiredWhen, field: driver, when: "Status == IDENTIFIED", status: IDENTIFIED, message: "..." }` (#7094): a **conditionally required** value - `field` must be present whenever `when` holds. `field` is the entity's own field or a one-hop `Relation.field`; `when` is a guard (see *the event 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 ecc3d160755..0da75910938 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 @@ -1429,6 +1429,66 @@ void conditionallyRequiredValuesEmitTheirConditionAndTheHopsTheirValueIsReadThro assertNull(ownField.get("status")); } + /** + * An {@code agree} check emits BOTH sides as one hop each, sharing the walker - so the two records + * are loaded once, by foreign key, and the comparison is between two properties of records neither + * of which is the one being written (dirigible #7409). + */ + @Test + @SuppressWarnings("unchecked") + void agreeChecksEmitBothSidesAndTheirLoads() { + String yaml = """ + name: billing + entities: + - name: Customer + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: name, type: string } + - name: SalesInvoice + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: customer, kind: manyToOne, to: Customer } + - name: CustomerPayment + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: customer, kind: manyToOne, to: Customer } + - name: InvoicePayment + checks: + - { kind: agree, relations: [salesInvoice, customerPayment], onProperty: customer, + message: "This payment belongs to a different customer than the invoice" } + - { kind: agree, relations: [salesInvoice, customerPayment], onProperty: customer, whenNull: refuse } + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: salesInvoice, kind: manyToOne, to: SalesInvoice } + - { name: customerPayment, kind: manyToOne, to: CustomerPayment } + """; + Map model = EdmIntentGenerator.buildModelJsonForTest(IntentParser.parse(yaml), "billing"); + List> checks = (List>) entityByName(entities(model), "InvoicePayment").get("checks"); + assertEquals(2, checks.size()); + + Map agree = checks.get(0); + assertEquals("(hop0 == null ? null : hop0.Customer)", agree.get("leftExpression")); + assertEquals("(hop1 == null ? null : hop1.Customer)", agree.get("rightExpression")); + assertEquals("SalesInvoice.Customer", agree.get("leftLabel")); + assertEquals("CustomerPayment.Customer", agree.get("rightLabel")); + assertEquals("skip", agree.get("whenNull")); + assertEquals("This payment belongs to a different customer than the invoice", agree.get("message")); + List> loads = (List>) agree.get("pathLoads"); + assertEquals(2, loads.size(), "each side is loaded once: " + loads); + assertEquals("entity.SalesInvoice", loads.get(0) + .get("sourceExpression")); + assertEquals("entity.CustomerPayment", loads.get(1) + .get("sourceExpression")); + + // An unauthored message still says what disagreed - only the declaration knows. + Map refusing = checks.get(1); + assertEquals("refuse", refusing.get("whenNull")); + assertEquals("The Sales Invoice and the Customer Payment must have the same Customer", refusing.get("message")); + } + /** * A guard on a TO-ONE is compared numerically, not with a boxed equality (#7237). The foreign-key * column is typed from the target's key, and for a cross-model target that key is only readable 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 cc7d7eee6e1..c7ae74c1109 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 @@ -959,6 +959,80 @@ void compareChecksAgainstLiteralsParseAndValidate() { "requires the entity to declare a `function: EntityStatus` relation"); } + /** + * An {@code agree} check relates the two records a JUNCTION row links: both must point at the same + * third thing (dirigible #7409). Nothing in the DSL could say it, so a payment of one customer was + * allocated against another customer's invoice, in another currency, and the write answered 200 - + * every module carrying the shape closed it with a hand-written Java guard class instead. + */ + @Test + void agreeChecksParseAndValidate() { + String yaml = """ + name: billing + entities: + - name: Customer + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: name, type: string } + - name: Currency + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: code, type: string } + - name: SalesInvoice + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: reference, type: string } + - { name: note, type: string } + - { name: total, type: decimal } + relations: + - { name: customer, kind: manyToOne, to: Customer } + - { name: currency, kind: manyToOne, to: Currency } + - name: CustomerPayment + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: reference, type: integer } + - { name: note, type: string } + - { name: total, type: decimal } + relations: + - { name: customer, kind: manyToOne, to: Customer } + - { name: currency, kind: manyToOne, to: Currency } + - name: InvoicePayment + checks: + - { kind: agree, relations: [salesInvoice, customerPayment], onProperty: customer, + message: "This payment belongs to a different customer than the invoice" } + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: salesInvoice, kind: manyToOne, to: SalesInvoice } + - { name: customerPayment, kind: manyToOne, to: CustomerPayment } + """; + CheckIntent check = IntentParser.parse(yaml) + .getEntities() + .get(4) + .getChecks() + .get(0); + assertEquals(List.of("salesInvoice", "customerPayment"), check.getRelations()); + assertEquals("customer", check.getOnProperty()); + assertNull(check.getWhenNull()); + + // A scalar both targets declare agrees too, and so does an explicit whenNull. + IntentParser.parse(yaml.replace("onProperty: customer", "onProperty: note, whenNull: refuse")); + + assertCompareIssue(yaml.replace("relations: [salesInvoice, customerPayment]", "relations: [salesInvoice]"), + "requires `relations`: exactly two"); + assertCompareIssue(yaml.replace("relations: [salesInvoice, customerPayment]", "relations: [salesInvoice, salesInvoice]"), + "twice - a relation always agrees with itself"); + assertCompareIssue(yaml.replace(", onProperty: customer", ""), "requires `onProperty`"); + assertCompareIssue(yaml.replace("onProperty: customer", "on: customer"), "spell it `onProperty`"); + assertCompareIssue(yaml.replace("onProperty: customer", "onProperty: supplier"), "has no field or to-one relation [supplier]"); + // A decimal is not compared for equality by anybody who means it - the line a condition draws. + assertCompareIssue(yaml.replace("onProperty: customer", "onProperty: total"), "the values an equality is exact on"); + // ...nor may the two sides be different kinds of value: the boxed comparison is always false. + assertCompareIssue(yaml.replace("onProperty: customer", "onProperty: reference"), "both sides must be the same kind of value"); + assertCompareIssue(yaml.replace("onProperty: customer,", "onProperty: customer, whenNull: maybe,"), "unknown `whenNull`"); + assertCompareIssue(yaml.replace("onProperty: customer,", "onProperty: customer, status: 1,"), "cannot carry a `status` gate"); + } + private static void assertCompareIssue(String yaml, String expected) { IntentValidationException ex = assertThrows(IntentValidationException.class, () -> IntentParser.parse(yaml)); assertTrue(ex.getIssues() diff --git a/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessor.java b/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessor.java index d11032773f8..d2493c97ca3 100644 --- a/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessor.java +++ b/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessor.java @@ -225,7 +225,9 @@ private static void splitChecks(Map entity, Map resolveMessageLiteral(check); resolveCheckJavaExpressions(check); resolveCheckPathLoads(check, parameters); - if ("exactlyOne".equals(kind)) { + if ("exactlyOne".equals(kind) || "agree".equals(kind)) { + // Both hold from the first save and take no gate: one relates the row's own fields, the + // other the two records a junction row links (#7409). rowChecks.add(check); } else if ("compare".equals(kind)) { // A comparison is row-level unless it names the status it is enforced at - the same diff --git a/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityController.java.template b/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityController.java.template index daf490564f9..65bd897775a 100644 --- a/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityController.java.template +++ b/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityController.java.template @@ -949,6 +949,26 @@ public class ${name}Controller { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "${check.message}"); } } +#elseif($check.kind == "agree") +## The two records a junction row links must point at the same third thing (intent `checks: agree`, +## #7409) - the same customer, the same currency, the same company. Each side is one hop away, so both +## records are loaded by their foreign key first; an unset side is skipped unless the check says to +## refuse it, because requiredness is the relation's own declaration. + { +#foreach($load in $check.pathLoads) + Object ${load.local}Fk = ${load.sourceExpression}; + ${load.entityClass} ${load.local} = ${load.local}Fk == null ? null : new ${load.repositoryClass}().findById(${load.local}Fk); +#end + Object agreeLeft = ${check.leftExpression}; + Object agreeRight = ${check.rightExpression}; +#if($check.whenNull == "refuse") + if (agreeLeft == null || agreeRight == null || !agreeLeft.equals(agreeRight)) { +#else + if (agreeLeft != null && agreeRight != null && !agreeLeft.equals(agreeRight)) { +#end + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "${check.messageJavaLiteral}"); + } + } #elseif($check.kind == "compare") ## A value of the row compared with a second one (intent `checks: compare`) - another field of the ## SAME row, or a LITERAL (#7338: the positive quantity, the percentage at most 100, the date that is diff --git a/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityMyController.java.template b/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityMyController.java.template index 4e2c372b1b6..6275687b19c 100644 --- a/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityMyController.java.template +++ b/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityMyController.java.template @@ -686,6 +686,26 @@ public class ${name}MyController { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "${check.message}"); } } +#elseif($check.kind == "agree") +## The two records a junction row links must point at the same third thing (intent `checks: agree`, +## #7409) - the same customer, the same currency, the same company. Each side is one hop away, so both +## records are loaded by their foreign key first; an unset side is skipped unless the check says to +## refuse it, because requiredness is the relation's own declaration. + { +#foreach($load in $check.pathLoads) + Object ${load.local}Fk = ${load.sourceExpression}; + ${load.entityClass} ${load.local} = ${load.local}Fk == null ? null : new ${load.repositoryClass}().findById(${load.local}Fk); +#end + Object agreeLeft = ${check.leftExpression}; + Object agreeRight = ${check.rightExpression}; +#if($check.whenNull == "refuse") + if (agreeLeft == null || agreeRight == null || !agreeLeft.equals(agreeRight)) { +#else + if (agreeLeft != null && agreeRight != null && !agreeLeft.equals(agreeRight)) { +#end + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "${check.messageJavaLiteral}"); + } + } #elseif($check.kind == "compare") ## A value of the row compared with a second one (intent `checks: compare`) - another field of the ## SAME row, or a LITERAL (#7338: the positive quantity, the percentage at most 100, the date that is diff --git a/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityPartnerController.java.template b/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityPartnerController.java.template index c352575400b..e14fe6219fb 100644 --- a/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityPartnerController.java.template +++ b/components/template/template-application-rest-java/src/main/resources/META-INF/dirigible/template-application-rest-java/api/EntityPartnerController.java.template @@ -642,6 +642,26 @@ public class ${name}PartnerController { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "${check.message}"); } } +#elseif($check.kind == "agree") +## The two records a junction row links must point at the same third thing (intent `checks: agree`, +## #7409) - the same customer, the same currency, the same company. Each side is one hop away, so both +## records are loaded by their foreign key first; an unset side is skipped unless the check says to +## refuse it, because requiredness is the relation's own declaration. + { +#foreach($load in $check.pathLoads) + Object ${load.local}Fk = ${load.sourceExpression}; + ${load.entityClass} ${load.local} = ${load.local}Fk == null ? null : new ${load.repositoryClass}().findById(${load.local}Fk); +#end + Object agreeLeft = ${check.leftExpression}; + Object agreeRight = ${check.rightExpression}; +#if($check.whenNull == "refuse") + if (agreeLeft == null || agreeRight == null || !agreeLeft.equals(agreeRight)) { +#else + if (agreeLeft != null && agreeRight != null && !agreeLeft.equals(agreeRight)) { +#end + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "${check.messageJavaLiteral}"); + } + } #elseif($check.kind == "compare") ## A value of the row compared with a second one (intent `checks: compare`) - another field of the ## SAME row, or a LITERAL (#7338: the positive quantity, the percentage at most 100, the date that is diff --git a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java index 84b4a286b10..b5f8400617f 100644 --- a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java +++ b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java @@ -321,6 +321,25 @@ class IntentEmissionCoverageIT extends IntegrationTest { - { name: number, type: string, length: 100, number: { series: Emission Receipt, stampOn: create } } - { name: note, type: string, length: 200 } + # checks: agree (#7409) - the two records a JUNCTION row links must point at the same + # third thing. No check kind could say it (compare relates two values of ONE row, the + # parent-child kinds a child to its own parent), so every module carrying the shape - a + # payment allocated against an invoice of another customer, in another currency - closed + # it with a hand-written Java guard class. Both readings of an unset side are emitted: + # the default skips (requiredness is the relation's own declaration), refuse rejects. + - name: EntryLink + checks: + - { kind: agree, relations: [doc, entry], onProperty: Status, + message: "The document and the entry are not in the same state" } + - { kind: agree, relations: [entry, storno], onProperty: Status, whenNull: refuse, + message: "An entry and its storno must be in the same state" } + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: doc, kind: manyToOne, to: Doc } + - { name: entry, kind: manyToOne, to: Entry } + - { name: storno, kind: manyToOne, to: Entry } + - name: EntryLine checks: - { kind: exactlyOne, fields: [debit, credit], message: "Exactly one of debit/credit" } @@ -2285,6 +2304,21 @@ private void assertEmission() { && lineController.contains("java.util.Objects.equals((hop0 == null ? null : hop0.Status), 2)") && lineController.contains("Cannot add a line to a posted entry"), "an ungated forbidWhen must load the parent hop and refuse the write on the REST controller, got: " + lineController); + // checks: agree (#7409): both sides are ONE HOP away, so the generated controller loads each + // related record by its foreign key and compares the property they must share - the rule a + // hand-written guard class used to carry. The two readings of an unset side are both emitted: + // the default skips the comparison, `whenNull: refuse` rejects the write. + String linkController = contentOf("gen/emission/api/entrylink/EntryLinkController.java"); + assertTrue( + linkController.contains("DocRepository().findById(hop0Fk)") && linkController.contains("Object agreeLeft = (hop0 == null") + && linkController.contains("agreeLeft != null && agreeRight != null && !agreeLeft.equals(agreeRight)") + && linkController.contains("The document and the entry are not in the same state"), + "an agree check must load both sides and refuse a disagreement on the REST controller, got: " + linkController); + assertTrue( + linkController.contains("agreeLeft == null || agreeRight == null || !agreeLeft.equals(agreeRight)") + && linkController.contains("An entry and its storno must be in the same state"), + "whenNull: refuse must reject an unset side too, got: " + linkController); + // The document's own line items are the same story through a different layout - and it is the // one where the child literally resums the master (BillLineRepository -> BillRepository). assertTrue(contentOf("gen/emission/api/bill/BillLineController.java").contains("requireMasterMutable"),