diff --git a/components/engine/engine-intent/CLAUDE.md b/components/engine/engine-intent/CLAUDE.md index d2fe2db324a..2e553aaa161 100644 --- a/components/engine/engine-intent/CLAUDE.md +++ b/components/engine/engine-intent/CLAUDE.md @@ -395,6 +395,7 @@ Semantics worth knowing: - **`checks:` on an entity = declarative cross-field / cross-line validations (the double-entry shape).** Three kinds (`CheckIntent`): row-level `exactlyOne` (`fields:` — exactly one non-null; emitted PascalCased into the `.model` `checks` list and enforced in the generated REST `validate()` with 400) and document-level `itemsSumEqual` (`over:` two item fields whose sums must match) / `itemsMin` (`count:`), both REQUIRING a `status:` gate (an EntityStatus seed id) — parser-enforced, because an ungated sum check would forbid drafting a document item by item. The EDM generator precomputes everything template-side (`buildChecks`: items entity + back-FK via the composition child, `statusProperty`, PascalCased fields); `ModelParameterProcessor` splits `rowChecks`/`documentChecks`; the **DAO repository** enforces document checks in `save`/`update`/**`updateWithoutEvent`** whenever the persisted entity carries the gate status — so the workflow setter flipping DRAFT→POSTED hits `enforceChecks` and an unbalanced document FAILS the write instead of silently posting: it throws the SDK `org.eclipse.dirigible.sdk.db.ValidationException`, which the client-controller dispatcher (`ControllerInvoker`) maps to **HTTP 400** with the authored message on a REST create/update, and which rolls back the task completion on the BPMN path (the capacity guard on roll-ups throws the same). `recalculate()` deliberately bypasses it (it persists the recomputed totals through the BASE targeted write, `super.updateProperties(id, totals)`, so a document still being assembled line by line never fails its own gate). No Harmonia-side mirror in v1 — the task-completion error surfaces the authored message. - **`resolves:` = the effective-dated register lookup (#6712).** The enterprise shape with no declarative form before it: a register says "X applied to Y from A to B" (a vehicle assignment, a price list, a contract in force, an org assignment), a record carries the match key(s) and a date, and a to-one must be filled from the row whose period covers that date. Nothing else in the DSL reaches it - `dependsOn` is a UI-time copy with equality matching only, a `decision` condition is a single comparison, and `setField` writes constants - so every application hand-wrote the same delegate. Authored as `{ event: { onCreate|onUpdate: , when? }, set: , from: , match: { : , ... }, between: { start?, end?, value }, outcome?: , found?/notFound?/ambiguous?: { setStatus } }`; `ResolveIntent` -> `GlueIntentGenerator.buildResolves` -> the `resolves` glue collection -> `Resolve.java.template`, a `@Component MessageHandler` on the record's event topic. **All three outcomes are first-class, and that is the point of the construct:** exactly one covering row fills the relation, NO covering row and MORE THAN ONE covering row both leave it unset (an automation that silently picks one of two candidates is worse than none - the ambiguous register goes back to a human). Each outcome may route the record by `setStatus` (seed id or seeded name, resolved by `StatusSymbolResolver` like every other status site), and the attempt is **observable**: `outcome:` stamps `found`/`notFound`/`ambiguous` into a string field of the record - queryable, filterable in a list view, and readable by a process `decision` - and the handler logs the keys and the date it checked. **Decisions worth keeping:** the value copied is derived, not authored - the register must carry exactly ONE to-one to the same target as `set:`, and zero or two is a validation error rather than a guess (the same refusal, one altitude up); a record that already carries the relation is skipped, so a manual correction is never overwritten and a re-delivered event is a no-op; the write is a single targeted `updateProperties` of the relation + the outcome + the status, so no `-updated` re-fires and no concurrent write to another column is reverted; period bounds are optional on either side (open-ended = still valid), the end is INCLUSIVE, and a date-only bound covers its whole day (the generated `millis`/`endExclusive` helpers put a `LocalDate` and an `Instant` on one epoch-milli axis, UTC). v1 is same-model (`from:` must be declared here) and binds to `onCreate`/`onUpdate` only - `onDelete` is refused, there is nothing left to fill. The parser refuses a `when` guard it cannot render rather than degrading it to an always-open guard. - **Every DERIVED write is targeted (document totals, `rollups:`, `aggregates:`) — the last member of the lost-update family.** A recompute reads a row, changes the one or two columns it computes, and persists. Persisting the WHOLE row silently reverts any concurrent write to another column of that row: the trigger `ProcessId` variant was fixed in #6226 and the workflow setter/writer variant in #6306, and the recompute variant was live-reproduced against a roll-up (REST-create a parent, PUT another column immediately after → 200, but a re-read shows the OLD value; the recompute had read the row before the PUT and wrote its stale snapshot after it). All three recompute sites now write only what they computed: `Repository.recalculate(Object)` collects the document totals into a map and calls the base `super.updateProperties` (no gate checks, no `-updated` — exactly the previous `super.update` semantics minus the merge); `Rollup.java.template` and `Aggregate.java.template` collect each recomputed column into a `derived` map and persist through the generated **`updateDerived(id, values)`**, which routes through `updateProperties` (so a `checks:` entity still runs its gate and a labelled entity still refreshes its `Name`) and then re-publishes `---updated` — the event contract the old full-row `update()` provided, which TRANSITIVE roll-ups above the row depend on. Two invariants when touching these: a column assigned in the recompute must also be put into `derived` (a capacity roll-up writes count + balance + status), and an EMPTY `derived` map means nothing is persisted, so the map is what the emission oracle asserts. Covered by the `IntentEmissionCoverageIT` derived-write assertions (Bill document totals, `ClaimLineClaimRollupOnCreate`, `LedgerTotalAggregateOnCreate`). **The reverse direction had the same hole (#6822):** the master's resum was wired only to the item's FULL write paths (`save`/`update`/`delete`), so a line written by a TARGETED primitive - a workflow `setField`, any glue `updateProperty`/`updateProperties`/`updateDerived`, or the event-suppressed `updateWithoutEvent` - moved the line and left the header displaying, printing and POSTING a total that did not equal the sum of its lines. Those paths now resum too, guarded on the columns actually written (an aggregated column, or the FK - which MOVES the line, so both the document it joined and the one it left are resummed), so a status hop still costs nothing extra. It cannot recurse: the master's `recalculate` persists through the BASE targeted write. +- **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. - **`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. @@ -555,7 +556,7 @@ Every action below has a real SDK surface to generate against, so none of this n rollups: - { name: memberLoanCount, entity: Loan, via: member, field: loanCount } # Member.loanCount = #Loans whose `member` FK = that Member ``` - → two `gen/events//RollupOn{Create,Delete}.java` `@Listener`s on the child's create/delete topics that recompute the affected parent's count via a typed `Criteria` (`findAll(Criteria.create().eq("", entity.)).size()`) and write it back. Recompute-on-event (self-healing); **eventually consistent, not transactionally exact** under heavy concurrency. **Gap:** no `where` filter (counts all children), and re-parenting on child update isn't tracked (only create/delete). **`op: sum`** keeps a decimal sum of the child `of` field (+ optional `capacity`/`balance`/`status` for payment-settlement); **`op: latest`** copies the `of` value of the child row with the greatest `by` date/timestamp onto the parent field (create/update/delete handlers; parent field must match `of`'s type; empty child set → null) — the "keep the parent's rate equal to the newest child rate" shape (currencies `Currency.rate` ← latest `CurrencyRate`). `RollupAggregates` in the pipeline tracks the max-`by` row type-agnostically (`var` + `Objects.equals`). + → `gen/events//RollupOn{Create,Delete,Rekey}.java` `@Listener`s on the child's create/delete/**rekey** topics that recompute the affected parent's count via a typed `Criteria` (`findAll(Criteria.create().eq("", entity.)).size()`) and write it back. Recompute-on-event (self-healing); **eventually consistent, not transactionally exact** under heavy concurrency. The rekey handler is what repairs a **re-parented** child - see the rekey bullet in the conventions above. **Gap:** no `where` filter (counts all children). **`op: sum`** keeps a decimal sum of the child `of` field (+ optional `capacity`/`balance`/`status` for payment-settlement); **`op: latest`** copies the `of` value of the child row with the greatest `by` date/timestamp onto the parent field (create/update/delete handlers; parent field must match `of`'s type; empty child set → null) — the "keep the parent's rate equal to the newest child rate" shape (currencies `Currency.rate` ← latest `CurrencyRate`). `RollupAggregates` in the pipeline tracks the max-`by` row type-agnostically (`var` + `Objects.equals`). 10. **Dynamic user-task assignment** — `assignee: { path: member.branch.manager, fallback: manager }`, resolver-driven (extends the existing user-task glue). **(v1 implemented — see the resolver-path bullet in the conventions above.)** ### Guardrails (so this doesn't become the MDE expressiveness trap) diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/GlueIntentGenerator.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/GlueIntentGenerator.java index 105a9d66a71..3211ade9c7e 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/GlueIntentGenerator.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/GlueIntentGenerator.java @@ -585,6 +585,13 @@ private static List> buildRollups(IntentModel model, Map> aggregateKeys = new ArrayList<>(); - Set seenAggregateKeys = new LinkedHashSet<>(); + // An entity whose rows are GROUPED by something maintained asynchronously - the keys of every + // aggregate over it, and the parent FK of every roll-up it feeds - carries those columns plus + // its pk, so the DAO can notice that a write MOVED the row between groups. A move has two + // sides and only one of them has an event of its own, so the DAO publishes the row on the + // dedicated "-rekeyed" topic for the aggregate / roll-up handlers to repair the other. + List> groupingKeys = new ArrayList<>(); + Set seenGroupingKeys = new LinkedHashSet<>(); if (model.getAggregates() != null) { for (AggregateIntent a : model.getAggregates()) { if (a.getOf() == null || !a.getOf() @@ -618,19 +619,23 @@ else if (!extension && !dependent && !setting && !compositionParents.containsVal continue; } for (String key : a.getBy()) { - String fk = IntentNaming.pascalCase(key); - if (seenAggregateKeys.add(fk)) { - Map pair = new LinkedHashMap<>(); - pair.put("key", fk); - aggregateKeys.add(pair); - } + addGroupingKey(groupingKeys, seenGroupingKeys, key); } } } - if (!aggregateKeys.isEmpty()) { - entityMap.put("aggregateKeys", aggregateKeys); - FieldIntent aggregatePk = primaryKeyOf(entity); - entityMap.put("aggregateSourcePk", aggregatePk == null ? "Id" : IntentNaming.pascalCase(aggregatePk.getName())); + // A roll-up groups its child rows by ONE column - the `via` relation's FK - and re-parenting a + // child is the ordinary way that column moves. Without it here, only aggregate sources were + // tracked and a re-parented roll-up child left its former parent's total stale forever (#6819). + for (RollupIntent rollup : model.getRollups()) { + if (rollup.getVia() != null && entity.getName() + .equals(rollup.getEntity())) { + addGroupingKey(groupingKeys, seenGroupingKeys, rollup.getVia()); + } + } + if (!groupingKeys.isEmpty()) { + entityMap.put("groupingKeys", groupingKeys); + FieldIntent groupingPk = primaryKeyOf(entity); + entityMap.put("groupingSourcePk", groupingPk == null ? "Id" : IntentNaming.pascalCase(groupingPk.getName())); } List> checkMaps = buildChecks(entity, byName, model.getAggregates()); if (!checkMaps.isEmpty()) { @@ -2340,6 +2345,23 @@ private static Map relationLink(String ownerEntity, RelationInte return link; } + /** + * Records one grouping column of an entity, de-duplicated: an aggregate key and a roll-up's + * {@code via} FK can name the same relation, and the DAO must compare it once. + * + * @param keys the collected grouping columns + * @param seen the property names already collected + * @param name the authored relation / field name + */ + private static void addGroupingKey(List> keys, Set seen, String name) { + String property = IntentNaming.pascalCase(name); + if (seen.add(property)) { + Map pair = new LinkedHashMap<>(); + pair.put("key", property); + keys.add(pair); + } + } + /** The target entity's primary-key field, or null when the target is unknown or has no PK. */ private static FieldIntent primaryKeyOf(EntityIntent entity) { if (entity == null) { 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 b7f4ec7febc..1b09fdf993d 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 @@ -2391,6 +2391,10 @@ rollups: `entity` is the child being counted, `via` is the child's to-one relation pointing at the parent, and `field` is the integer field on the **parent** that holds the count. +**Re-parenting is handled.** Moving a child to another parent - an edit of its `via` relation, by a user +or by a process step - leaves BOTH parents' totals right: the one that received the child and the one it +left. Nothing to declare. + **Sum + balance + status (payment settlement).** With `op: sum` the roll-up keeps `field` equal to the sum of the children's `of` field. Add `capacity` (a numeric parent field the sum is measured against) to also maintain a `balance` field (= `capacity − sum`) and set a `status` relation to `statusWhenFull` diff --git a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueRollupLatestTest.java b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueRollupLatestTest.java index 5b9cf1af777..dd21347b2a5 100644 --- a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueRollupLatestTest.java +++ b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueRollupLatestTest.java @@ -54,8 +54,9 @@ class GlueRollupLatestTest { void rendersTheLatestRollupHandlers() { IntentModel model = IntentParser.parse(YAML); List> rollups = GlueIntentGenerator.buildRollupsForTest(model); - // create + update + delete (a new/edited/removed rate can change which row is latest). - assertEquals(3, rollups.size(), "latest must recompute on create, update and delete"); + // create + update + delete (a new/edited/removed rate can change which row is latest), plus the + // rekey handler that repairs the currency a rate was moved AWAY from. + assertEquals(4, rollups.size(), "latest must recompute on create, update, delete and rekey"); Map create = rollups.get(0); assertEquals("latest", create.get("op")); assertEquals("Rate", create.get("countField")); // parent field @@ -73,5 +74,9 @@ void rendersTheLatestRollupHandlers() { .anyMatch(r -> String.valueOf(r.get("topicSuffix")) .equals("-updated")), "latest must have an update handler"); + assertTrue(rollups.stream() + .anyMatch(r -> String.valueOf(r.get("topicSuffix")) + .equals("-rekeyed")), + "latest must have a rekey handler"); } } diff --git a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueRollupRekeyTest.java b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueRollupRekeyTest.java new file mode 100644 index 00000000000..84b2df49db7 --- /dev/null +++ b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueRollupRekeyTest.java @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2010-2026 Eclipse Dirigible contributors + * + * All rights reserved. This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v2.0 which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v20.html + * + * SPDX-FileCopyrightText: Eclipse Dirigible contributors SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.dirigible.components.intent.generator; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Map; + +import org.eclipse.dirigible.components.intent.model.IntentModel; +import org.eclipse.dirigible.components.intent.parser.IntentParser; +import org.junit.jupiter.api.Test; + +/** + * Verifies that every roll-up also binds the child's {@code "-rekeyed"} event (#6819). + * + *

+ * A re-parented child is named by no create / update / delete event of the parent it moved AWAY + * from - those all carry the parent it belongs to NOW - so that parent's total kept the moved + * child's contribution forever. The rekey handler is the same recompute keyed on the payload's FK, + * fed the row whose grouping moved. + */ +class GlueRollupRekeyTest { + + private static final String YAML = """ + name: library + entities: + - name: Member + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: loanCount, type: integer } + relations: + - { name: loans, kind: oneToMany, to: Loan } + - name: Loan + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: Member, kind: manyToOne, to: Member } + rollups: + - { name: memberLoanCount, entity: Loan, via: Member, field: loanCount, op: count } + """; + + @Test + void everyRollupBindsTheRekeyEvent() { + IntentModel model = IntentParser.parse(YAML); + List> rollups = GlueIntentGenerator.buildRollupsForTest(model); + + Map rekey = rollups.stream() + .filter(r -> "-rekeyed".equals(r.get("topicSuffix"))) + .findFirst() + .orElseThrow(() -> new AssertionError("no rekey handler in " + rollups)); + assertEquals("LoanMemberRollupOnRekey", rekey.get("className")); + assertEquals("Loan", rekey.get("childEntity")); + assertEquals("Member", rekey.get("parentEntity")); + // The recompute is keyed on the FK carried by the payload, so the SAME handler repairs the parent + // the child left (fed the previous row) and the one it moved into (fed the written row). + assertEquals("Criteria.create().eq(\"Member\", entity.Member)", rekey.get("criteriaExpression")); + assertTrue(rollups.stream() + .map(r -> String.valueOf(r.get("topicSuffix"))) + .toList() + .containsAll(List.of("", "-deleted", "-rekeyed")), + "the rekey handler must be additional to the create/delete ones, not replace them: " + rollups); + } +} 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 90e21722c17..58aa1fea1ae 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 @@ -777,15 +777,48 @@ void guardCheckEmitsKeyedAggregateGuard() { // The aggregate's SOURCE entity carries its grouping keys + pk, so the DAO can detect that a // key moved and let the aggregate repair the tuple the row left (there is no event for it). - List> sourceKeys = (List>) movement.get("aggregateKeys"); + List> sourceKeys = (List>) movement.get("groupingKeys"); assertEquals(2, sourceKeys.size(), "the aggregate source must carry every grouping key"); assertEquals("Product", sourceKeys.get(0) .get("key")); assertEquals("Store", sourceKeys.get(1) .get("key")); - assertEquals("Id", movement.get("aggregateSourcePk")); + assertEquals("Id", movement.get("groupingSourcePk")); // The TARGET is not a source, so it carries no rekey metadata. - assertNull(entityByName(entities(model), "ProductAvailability").get("aggregateKeys")); + assertNull(entityByName(entities(model), "ProductAvailability").get("groupingKeys")); + } + + @Test + @SuppressWarnings("unchecked") + void rollupChildCarriesItsParentFkAsAGroupingKey() { + String yaml = """ + name: orders + entities: + - name: Customer + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: orderCount, type: integer } + relations: + - { name: orders, kind: oneToMany, to: Order } + - name: Order + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: Customer, kind: manyToOne, to: Customer } + rollups: + - { name: orderCount, entity: Order, via: Customer, field: orderCount, op: count } + """; + Map model = EdmIntentGenerator.buildModelJsonForTest(IntentParser.parse(yaml), "orders"); + // Re-parenting a roll-up child moves its rows between parents exactly as an aggregate key does, + // so the child must carry the `via` FK too - the DAO compares it and publishes "-rekeyed", which + // is the only signal the parent the child moved AWAY from ever gets (#6819). + List> keys = (List>) entityByName(entities(model), "Order").get("groupingKeys"); + assertEquals(1, keys.size(), "a roll-up child must track its parent FK"); + assertEquals("Customer", keys.get(0) + .get("key")); + assertEquals("Id", entityByName(entities(model), "Order").get("groupingSourcePk")); + // The PARENT of a roll-up is not grouped by anything - only the child moves. + assertNull(entityByName(entities(model), "Customer").get("groupingKeys")); } /** diff --git a/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/GlueGenerator.java b/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/GlueGenerator.java index dcf01ae9212..d1d0751e3ec 100644 --- a/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/GlueGenerator.java +++ b/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/GlueGenerator.java @@ -856,10 +856,11 @@ private List rollups(GenerationTemplateMetadataSource source, Str * total from the store, which makes them idempotent and self-healing. * *

- * The fourth, re-keyed, variant is the same handler fed the previous row: the repository publishes - * that event only when a grouping key actually moved, and recomputing the former tuple - which no - * longer contains the row - drops the total it left behind. Without it, editing a grouping key - * leaves a stale aggregate on the tuple the row moved out of. + * The fourth, re-keyed, variant is the same handler fed a row whose grouping moved: the repository + * publishes that event only when a grouping key actually changed - the previous row, so recomputing + * the former tuple, which no longer contains it, drops the total it left behind, and on the + * targeted write path the written row as well, since that path publishes no "-updated" for the + * tuple the row moved into. Without it, editing a grouping key leaves a stale aggregate behind. * * @param source the template source * @param content the template content diff --git a/components/template/template-application-dao-java/src/main/resources/META-INF/dirigible/template-application-dao-java/data/Repository.java.template b/components/template/template-application-dao-java/src/main/resources/META-INF/dirigible/template-application-dao-java/data/Repository.java.template index 9acd2b8bfdf..72068e4afc0 100644 --- a/components/template/template-application-dao-java/src/main/resources/META-INF/dirigible/template-application-dao-java/data/Repository.java.template +++ b/components/template/template-application-dao-java/src/main/resources/META-INF/dirigible/template-application-dao-java/data/Repository.java.template @@ -299,15 +299,16 @@ public class ${name}Repository extends JavaRepository<${name}Entity> { #end } #end -#if($aggregateKeys && $aggregateKeys.size() > 0) - // Aggregate rekey: this entity groups an aggregate, so a change to one of its grouping keys - // MOVES the row between tuples. The tuple it moves into is recomputed off the "-updated" event - // like any other change; the tuple it LEAVES has no event of its own, so capture the previous - // row here and publish it on the dedicated "-rekeyed" topic after the write (below). Only the - // aggregate handlers listen there, so no other reaction sees a second event. - ${name}Entity aggregatePrevious = findById(entity.${aggregateSourcePk}); - boolean aggregateRekeyed = aggregatePrevious != null && (false#foreach($k in $aggregateKeys) - || !java.util.Objects.equals(aggregatePrevious.${k.key}, entity.${k.key})#end); +#if($groupingKeys && $groupingKeys.size() > 0) + // Rekey: this entity's rows are GROUPED by something maintained asynchronously (an aggregate's + // keys, a roll-up's parent FK), so a change to one of those columns MOVES the row between groups. + // The group it moves into is recomputed off the "-updated" event like any other change; the group + // it LEAVES has no event of its own, so capture the previous row here and publish it on the + // dedicated "-rekeyed" topic after the write (below). Only the aggregate / roll-up handlers listen + // there, so no other reaction sees a second event. + ${name}Entity groupingPrevious = findById(entity.${groupingSourcePk}); + boolean groupingMoved = groupingPrevious != null && (false#foreach($k in $groupingKeys) + || !java.util.Objects.equals(groupingPrevious.${k.key}, entity.${k.key})#end); #end #if($hasLabel) computeName(entity); @@ -343,12 +344,12 @@ public class ${name}Repository extends JavaRepository<${name}Entity> { #end // Publish the update event (suffixed topic) so intent reactions under gen/events can react. Producer.sendToTopic("${projectName}-${perspectiveName}-${name}-updated", Json.stringify(updated)); -#if($aggregateKeys && $aggregateKeys.size() > 0) - if (aggregateRekeyed) { - // Published AFTER the write, carrying the PREVIOUS row: the aggregate handler recomputes - // that row's old key-tuple from the store, which no longer contains this row, so the total - // it left behind drops instead of staying stale. - Producer.sendToTopic("${projectName}-${perspectiveName}-${name}-rekeyed", Json.stringify(aggregatePrevious)); +#if($groupingKeys && $groupingKeys.size() > 0) + if (groupingMoved) { + // Published AFTER the write, carrying the PREVIOUS row: the aggregate / roll-up handler + // recomputes that row's old group from the store, which no longer contains this row, so the + // total it left behind drops instead of staying stale. + Producer.sendToTopic("${projectName}-${perspectiveName}-${name}-rekeyed", Json.stringify(groupingPrevious)); } #end #if($documentItem) @@ -396,7 +397,7 @@ public class ${name}Repository extends JavaRepository<${name}Entity> { #end return updated; } -#if(($documentChecks && $documentChecks.size() > 0) || $hasLabel || $lifecycleEdges || $history == "true" || $documentItem) +#if(($documentChecks && $documentChecks.size() > 0) || $hasLabel || $lifecycleEdges || $history == "true" || $documentItem || ($groupingKeys && $groupingKeys.size() > 0)) /** * Targeted single-column write with this entity's semantics preserved: routed through @@ -417,13 +418,28 @@ public class ${name}Repository extends JavaRepository<${name}Entity> { * Name) are persisted in one statement. The workflow setters/writers come through here; a plain * pass-through to the base targeted write would silently skip the posting gate - and, on a document * line, leave the master summing to something other than its lines. +#if($groupingKeys && $groupingKeys.size() > 0) + * + *

+ * Still event-free for the reactions: the only event it can publish is {@code "-rekeyed"}, and only + * when the write MOVED the row between groups - a topic nothing but the generated aggregate / + * roll-up handlers subscribes to. +#end */ @Override public int updateProperties(Object id, java.util.Map values) { if (values == null || values.isEmpty()) { return 0; } -#if(($documentChecks && $documentChecks.size() > 0) || $hasLabel || $lifecycleEdges || $documentItem) +#if($groupingKeys && $groupingKeys.size() > 0) + // Rekey on the TARGETED path: a workflow setter, a resolver and a task-form writer move a + // grouping column exactly as a user edit does, but they publish no "-updated" - so without this + // neither side of the move would ever be recomputed (#6819). The row as it stands before the + // write is snapshotted as JSON here, so applying the incoming values below cannot mutate it. + String groupingPrevious = null; + boolean groupingMoved = false; +#end +#if(($documentChecks && $documentChecks.size() > 0) || $hasLabel || $lifecycleEdges || $documentItem || ($groupingKeys && $groupingKeys.size() > 0)) ${name}Entity entity = findOne(id).orElse(null); #if($documentItem) // The document this line belongs to as it stands BEFORE the write - a targeted write may also @@ -432,12 +448,18 @@ public class ${name}Repository extends JavaRepository<${name}Entity> { Object documentBefore = entity == null ? null : entity.${documentItem.fkProperty}; #end #end -#if(($documentChecks && $documentChecks.size() > 0) || $hasLabel || $lifecycleEdges) +#if(($documentChecks && $documentChecks.size() > 0) || $hasLabel || $lifecycleEdges || ($groupingKeys && $groupingKeys.size() > 0)) if (entity != null) { #if($lifecycleEdges) // The status as it stands BEFORE the targeted write - a transition button, a workflow // setter and a glue action all arrive here, and all of them are moves in the state machine. Object lifecyclePrevious = entity.${lifecycleStatusProperty}; +#end +#if($groupingKeys && $groupingKeys.size() > 0) + groupingPrevious = Json.stringify(entity); +#foreach($k in $groupingKeys) + Object groupingPrevious${k.key} = entity.${k.key}; +#end #end values = new java.util.LinkedHashMap<>(values); for (java.util.Map.Entry value : values.entrySet()) { @@ -448,6 +470,10 @@ public class ${name}Repository extends JavaRepository<${name}Entity> { throw new IllegalArgumentException("Unknown property [" + value.getKey() + "]", e); } } +#if($groupingKeys && $groupingKeys.size() > 0) + groupingMoved = false#foreach($k in $groupingKeys) + || !java.util.Objects.equals(groupingPrevious${k.key}, entity.${k.key})#end; +#end #if($hasLabel) // A targeted write may change a label input (a workflow status hop, a stamped number) - // recompute the display name from the post-write row and carry it in the same statement. @@ -499,6 +525,17 @@ public class ${name}Repository extends JavaRepository<${name}Entity> { new ${documentItem.parentEntity}Repository().recalculate(documentBefore); } } +#end +#if($groupingKeys && $groupingKeys.size() > 0) + if (updatedCount > 0 && groupingMoved) { + // BOTH sides of the move, on the one topic only the aggregate / roll-up handlers listen to - + // so no other reaction sees a second event, which is why this path can signal them without + // re-publishing "-updated". The previous row names the group the row LEFT (recomputed without + // it, its total drops instead of staying stale); the written row names the one it moved INTO, + // which on the targeted path gets no event of its own at all. + Producer.sendToTopic("${projectName}-${perspectiveName}-${name}-rekeyed", groupingPrevious); + Producer.sendToTopic("${projectName}-${perspectiveName}-${name}-rekeyed", Json.stringify(entity)); + } #end return updatedCount; } diff --git a/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Aggregate.java.template b/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Aggregate.java.template index 8cb8d2fb1b7..8343650a632 100644 --- a/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Aggregate.java.template +++ b/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Aggregate.java.template @@ -23,10 +23,13 @@ import gen.${javaGenFolderName}.data.${targetJavaPerspective}.${targetEntity}Rep * idempotent - re-delivery or replay converges to the same value). A source row with any grouping key * null is ignored (it belongs to no tuple). * - * A key change MOVES a row between tuples, and both sides are repaired: the tuple it moved into is - * recomputed off the "-updated" event like any other change, while the DAO publishes the PREVIOUS row - * on "-rekeyed" (only when a grouping key actually moved), which the OnRekey copy of this handler - * recomputes - the former tuple no longer contains the row, so its total drops instead of going stale. + * A key change MOVES a row between tuples, and both sides are repaired. The tuple it moved into is + * recomputed off the "-updated" event like any other change; the tuple it LEFT is named by no event of + * its own, so the DAO publishes the PREVIOUS row on "-rekeyed" (only when a grouping key actually moved) + * and the OnRekey copy of this handler recomputes it - the row is gone from it, so its total drops + * instead of going stale. A TARGETED write (a workflow setter, a resolver, a task-form writer) publishes + * no "-updated" at all, so it puts BOTH the previous and the written row on "-rekeyed" - the same + * handler, fed each side in turn. * A tuple whose last contributing row leaves keeps its target row with a zero total; the row is not * deleted. v1: same-model source + target, sum/count into a decimal target field. */ diff --git a/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Rollup.java.template b/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Rollup.java.template index 8b7b6103e78..c759a990cd2 100644 --- a/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Rollup.java.template +++ b/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Rollup.java.template @@ -25,6 +25,11 @@ import gen.${parentGenFolder}.data.${javaParentPerspective}.${parentEntity}Repos * recomputed from the store on each event (self-healing); the write + the "-updated" event fire only when * a value actually changes, which bounds the transitive cascade so it terminates at rest. * + * The parent recomputed is always the one the INCOMING payload names, which is what lets the same handler + * repair a re-parented child: the create/update/delete events carry the parent the child belongs to now, + * while "-rekeyed" carries the row whose group moved - the previous row for the parent it left, and (on + * the targeted writes, which publish no "-updated") the written row for the one it moved into. + * * The write itself is TARGETED (updateDerived persists only the recomputed columns), so a concurrent user * write to any other column of the parent cannot be reverted by this recompute - the same lost-update * shape fixed for the trigger write-back in #6226 and the workflow setters/writers in #6306. 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 8388555a8da..24fdec7d8d9 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 @@ -1603,19 +1603,45 @@ private void assertEmission() { // recomputes that former tuple - otherwise it keeps the moved row's contribution forever. String ledgerRepository = contentOf("gen/emission/data/ledger/LedgerRepository.java"); assertTrue( - ledgerRepository.contains("aggregatePrevious = findById(entity.Id)") - && ledgerRepository.contains("java.util.Objects.equals(aggregatePrevious.Person, entity.Person)") - && ledgerRepository.contains("java.util.Objects.equals(aggregatePrevious.Unit, entity.Unit)"), + ledgerRepository.contains("groupingPrevious = findById(entity.Id)") + && ledgerRepository.contains("java.util.Objects.equals(groupingPrevious.Person, entity.Person)") + && ledgerRepository.contains("java.util.Objects.equals(groupingPrevious.Unit, entity.Unit)"), "an aggregate source must compare EVERY grouping key against the previous row on update"); assertTrue( - ledgerRepository.contains("if (aggregateRekeyed)") - && ledgerRepository.contains("-rekeyed\", Json.stringify(aggregatePrevious)"), + ledgerRepository.contains("if (groupingMoved)") + && ledgerRepository.contains("-rekeyed\", Json.stringify(groupingPrevious)"), "a moved grouping key must publish the PREVIOUS row on the -rekeyed topic"); String ledgerRekey = contentOf("gen/events/emission/LedgerTotalAggregateOnRekey.java"); assertTrue(ledgerRekey.contains("-Ledger-rekeyed"), "the rekey handler must bind the source's -rekeyed topic"); assertTrue(ledgerRekey.contains("targets.updateDerived("), "the rekey handler must repair the former tuple through the targeted derived write"); + // The TARGETED writers (a workflow setter, a resolver, a task-form writer) move a grouping column + // exactly as a user edit does but publish no "-updated" at all - so both sides of the move are + // signalled here, on the topic only the aggregate / roll-up handlers subscribe to (#6819). + assertTrue( + ledgerRepository.contains("Object groupingPreviousPerson = entity.Person") + && ledgerRepository.contains("Object groupingPreviousUnit = entity.Unit") + && ledgerRepository.contains("groupingMoved = false") + && ledgerRepository.contains("!java.util.Objects.equals(groupingPreviousPerson, entity.Person)"), + "the targeted write must compare every grouping key before and after the write: " + ledgerRepository); + assertTrue( + ledgerRepository.contains("if (updatedCount > 0 && groupingMoved)") + && ledgerRepository.contains("-rekeyed\", groupingPrevious)") + && ledgerRepository.contains("-rekeyed\", Json.stringify(entity))"), + "a targeted write that moved a grouping key must publish BOTH the previous and the written row"); + + // Fix 2: the same move on a ROLL-UP child. Its parent FK is a grouping column too, so the child's + // DAO tracks it and a roll-up handler binds "-rekeyed" - without it the parent a child was moved + // AWAY from kept the child's contribution forever (no create/update/delete event names it). + String claimLineRepository = contentOf("gen/emission/data/claim/ClaimLineRepository.java"); + assertTrue(claimLineRepository.contains("java.util.Objects.equals(groupingPrevious.Claim, entity.Claim)"), + "a roll-up child must compare its parent FK against the previous row: " + claimLineRepository); + String claimRekey = contentOf("gen/events/emission/ClaimLineClaimRollupOnRekey.java"); + assertTrue(claimRekey.contains("-ClaimLine-rekeyed"), "the roll-up rekey handler must bind the child's -rekeyed topic"); + assertTrue(claimRekey.contains("Criteria.create().eq(\"Claim\", entity.Claim)") && claimRekey.contains("parents.updateDerived("), + "the roll-up rekey handler must recompute the parent the PAYLOAD names, through the targeted derived write"); + // checks: kind: guard - the aggregate precondition, one assertion per outcome. The guard // recomputes the keyed sum from the GUARDED entity's own store (race-free, not the async // aggregate target), then acts. block fails the write behind its config gate; task and reject