From a457bb205c1a8d425727149f7fb06b760da00e04 Mon Sep 17 00:00:00 2001 From: ThuF Date: Thu, 10 Sep 2026 08:23:24 +0300 Subject: [PATCH] intent: a create-from's items where:/refuse: refusal is decided before the header save (#7224) The two refusals of a create-from's source-row rule ran after the target header was saved. The unit of work took the header back, but the document number allocation and the History create entry live outside it by design, so every refused click spent a number of a gap-free series and left a trail row for a document that never existed. The rule block now runs right after the at-most-once guard, before the target is built; IntentEmissionCoverageIT asserts the emitted order and, end to end, that a refused run leaves the Bill series counter and the Bill history trail unchanged. Co-Authored-By: Claude Fable 5.1 --- .claude/docs/client-java.md | 2 +- .claude/docs/intent-layer.md | 2 +- components/engine/engine-intent/CLAUDE.md | 2 +- .../main/resources/intent-assistant-guide.md | 5 +- .../events/Generate.java.template | 96 ++++++++++-------- .../tests/api/IntentEmissionCoverageIT.java | 99 ++++++++++++++++--- 6 files changed, 148 insertions(+), 58 deletions(-) diff --git a/.claude/docs/client-java.md b/.claude/docs/client-java.md index 1cb80a55f8c..466a5fd04db 100644 --- a/.claude/docs/client-java.md +++ b/.claude/docs/client-java.md @@ -9,7 +9,7 @@ Client `.java` under `/registry/public//...` is synchronized by `JavaSy - All client annotations/facades live in `org.eclipse.dirigible.sdk.*` (`api-modules-java`), not the old `engine.java.annotations.*`. Compile **and** bean-wiring errors surface in the IDE Problems view. - **Manage entities ONLY through their generated `Repository` — never the generic `Store`/`Database` for entity CRUD.** The generated `@Repository extends JavaRepository` is the sole sanctioned load/save/update/delete path; it carries validations, **event publishing** (create/`-updated`/`-deleted` topics that intent triggers/reactions/rollups/notifications consume — recorded in the tenant's `DIRIGIBLE_EVENT_OUTBOX` inside the write's own transaction, so the row and its event commit together and a broker outage neither loses the event nor fails the write; `EventOutboxRelayJob` drains what the in-process publish could not deliver), and — for `multilingual: true` entities — the **read-time translation overlay** (every find translates string properties from the sibling `_LANG` table for the caller's `Accept-Language`, via the SDK `org.eclipse.dirigible.sdk.db.Translator`). The name-keyed `org.eclipse.dirigible.sdk.db.Store` and raw `Database` SQL bypass all of that silently and must not touch a managed entity. (`updateWithoutEvent` is fine — a deliberate repository method that keeps validations/i18n and only omits the event, for workflow-driven system writes.) So a reusable delegate/service that must touch a *specific* entity lives **in that entity's project** (importing its repository); only entity-agnostic helpers belong in a shared project. See the engine-java guide. -- **Several writes that only make sense together are ONE transaction.** Every store call is otherwise its own transaction, so a multi-write operation that fails halfway leaves the earlier writes behind - an intent create-from committed the invoice header and the source's INVOICED flip and then failed on a line (#7069). `org.eclipse.dirigible.components.data.store.java.repository.UnitOfWork.call(() -> { ... })` runs the block on one session and one transaction (thread-bound, so every repository joins it; nested blocks defer to the outermost), reads see the block's own writes, and the outbox events dispatch only once the whole unit committed - which is why an announcement about the unit's outcome belongs INSIDE the block, recorded through the write it is about (the create-from's `-transitioned` rides its source's status flip): the unit's commit is what makes it true, and the outbox only hands out what committed, so a crash after the commit no longer loses the event (#7160). The `History` trail and document numbering deliberately stay outside. Alongside it, a generated repository now refuses a write that leaves a defaultless NOT NULL column empty with a `ValidationException` naming the property (a 400), instead of letting the statement come back as a driver-specific constraint violation; a column carrying a DEFAULT is exempt, the database supplying its value. +- **Several writes that only make sense together are ONE transaction.** Every store call is otherwise its own transaction, so a multi-write operation that fails halfway leaves the earlier writes behind - an intent create-from committed the invoice header and the source's INVOICED flip and then failed on a line (#7069). `org.eclipse.dirigible.components.data.store.java.repository.UnitOfWork.call(() -> { ... })` runs the block on one session and one transaction (thread-bound, so every repository joins it; nested blocks defer to the outermost), reads see the block's own writes, and the outbox events dispatch only once the whole unit committed - which is why an announcement about the unit's outcome belongs INSIDE the block, recorded through the write it is about (the create-from's `-transitioned` rides its source's status flip): the unit's commit is what makes it true, and the outbox only hands out what committed, so a crash after the commit no longer loses the event (#7160). The `History` trail and document numbering deliberately stay outside - which is why anything that can refuse a create (a required input, a from-status guard, a create-from's source-row rule) is decided BEFORE the target's header is saved: a refusal after the save takes the row back but not the number it spent or the trail row it wrote (#7224). Alongside it, a generated repository now refuses a write that leaves a defaultless NOT NULL column empty with a `ValidationException` naming the property (a 400), instead of letting the statement come back as a driver-specific constraint violation; a column carrying a DEFAULT is exempt, the database supplying its value. **Detailed guide:** [`components/engine/engine-java/CLAUDE.md`](components/engine/engine-java/CLAUDE.md). Read it before changing anything under `engine-java`, `data-store-java`, the `sdk.*` annotations, or the `*-java` templates — it covers the container, the consumers, the two handler styles + no-mixing rule, the `JavaHandler`-as-bean path, controller routing / OpenAPI / `@Roles`, `data-store-java` dynamic-map persistence, error surfacing, the **removed** internals (`RepositoryRegistry` / `RepositoryClassConsumer` / `DependencyResolver` / reflective fallback / `@Extension`), and the three-repo (platform + `dirigiblelabs/sample-java-*` + docs) sequencing. diff --git a/.claude/docs/intent-layer.md b/.claude/docs/intent-layer.md index 1d2fb1b5280..3785388e39c 100644 --- a/.claude/docs/intent-layer.md +++ b/.claude/docs/intent-layer.md @@ -32,7 +32,7 @@ A single `app.intent` YAML file at a project root is the source of truth one alt **The enrichment channel (`phases:` + `onPhase`, [#6929](https://github.com/eclipse-dirigible/dirigible/issues/6929)):** a value a listener computes AFTER the insert — a moving-average cost, a snapshot column, an external lookup — must be written back **event-silently** or it re-fires every onUpdate consumer of a change the user never made; so it published nothing at all, and a declarative consumer of that value had no moment to bind. Bound to `onCreate` it RACED the enrichment (two listeners on one topic have no order — each `MessageHandler` is its own durable subscriber, and there is no priority anywhere), and posted a balanced-looking journal entry for a null amount with parse, generation, compile and publish all green. The fix is a CHANNEL, not an ordering contract the broker cannot keep: an entity declares the moments it announces (`phases: [costed]`), the Java DAO template emits one **`announce(id, values)`** per phase — `updateProperties` with the phase's own topic, so the enrichment and its notice ride ONE write into the outbox and commit together — and any glue consumer binds `event: { onPhase: , phase: }`. The generated method is the point: a hand-typed topic string reproduces exactly the silence being removed, a mistyped `announceCosted` is a compile error. Accepted by `postings:` (the driver), `notifications:`, `integrations:`, `outbound:` and an event-driven `generates:`, with the `when:` guard optional there (the phase already IS one moment); deliberately not by a process `trigger:`, a `wait` or `resolves:`. Refused at parse, each because it is otherwise silent: a phase that is not a lower-camel identifier, one named after a platform channel (`updated`/`deleted`/`transitioned`/`rekeyed`), a duplicate, a `phase:` key on another axis, and a binding naming a phase the entity does not declare. Details in the engine-intent guide's phases bullet. -**Which source rows become lines (`items: where:` + `refuse:`, [#7091](https://github.com/eclipse-dirigible/dirigible/issues/7091)):** a create-from's mirror `items:` block cloned EVERY row of the source document into a target line and the DSL could not say which rows qualified, so base-timesheets billed every member timesheet of the project-month - a DRAFT / REJECTED one at the same footing as an APPROVED one, and an EMPTY one (whose mapped quantity the target refuses) stopped the whole Generate until someone deleted the row by hand. "Invoice the approved month" is the one flow a billing clerk runs, and the module could either bill unapproved hours or not bill at all; the gap is fleet-wide (proforma -> invoice, quotation -> order, order -> invoice). `where:` is the rule - the same `{ field, op, value }` triples a `schedules[].where` carries, incl. a moment value resolved against the clock of the run - pushed into the very `Criteria` that already selects the source's rows by their master foreign key, so an unqualified row is never loaded; a condition naming the source ITEM's own `function: EntityStatus` relation may use the seeded status name (on the item's nomenclature, never the header's). `refuse:` declares the other reading: an unqualified row stops the whole run with the authored message plus the KEYS of the offending rows, instead of being left out - dropping a rejected line silently and billing it silently are both wrong for different months, so skipping is the default and `refuse:` without a `where:` is refused at parse. **A rule that qualifies no row refuses too**, rather than committing a header with no lines - the harder failure to notice, the document existing and counting as the period's billing. Scoped to a `where`-declaring block, so a rule-less items block is byte-identical; the rule's `field` is checked against the item source's own properties at parse, unlike a schedule's query, whose source may be a cross-model row. +**Which source rows become lines (`items: where:` + `refuse:`, [#7091](https://github.com/eclipse-dirigible/dirigible/issues/7091)):** a create-from's mirror `items:` block cloned EVERY row of the source document into a target line and the DSL could not say which rows qualified, so base-timesheets billed every member timesheet of the project-month - a DRAFT / REJECTED one at the same footing as an APPROVED one, and an EMPTY one (whose mapped quantity the target refuses) stopped the whole Generate until someone deleted the row by hand. "Invoice the approved month" is the one flow a billing clerk runs, and the module could either bill unapproved hours or not bill at all; the gap is fleet-wide (proforma -> invoice, quotation -> order, order -> invoice). `where:` is the rule - the same `{ field, op, value }` triples a `schedules[].where` carries, incl. a moment value resolved against the clock of the run - pushed into the very `Criteria` that already selects the source's rows by their master foreign key, so an unqualified row is never loaded; a condition naming the source ITEM's own `function: EntityStatus` relation may use the seeded status name (on the item's nomenclature, never the header's). `refuse:` declares the other reading: an unqualified row stops the whole run with the authored message plus the KEYS of the offending rows, instead of being left out - dropping a rejected line silently and billing it silently are both wrong for different months, so skipping is the default and `refuse:` without a `where:` is refused at parse. **A rule that qualifies no row refuses too**, rather than committing a header with no lines - the harder failure to notice, the document existing and counting as the period's billing. Scoped to a `where`-declaring block, so a rule-less items block is byte-identical; the rule's `field` is checked against the item source's own properties at parse, unlike a schedule's query, whose source may be a cross-model row. Both refusals are decided BEFORE the target header is saved ([#7224](https://github.com/eclipse-dirigible/dirigible/issues/7224)): the generated repository allocates the document number and writes the `History` create entry outside the unit of work, so a refusal fired after the save spent a number of a gap-free series and left a trail row for a document that never existed - once per click. **A create-from is not offered twice (`fromStatus:`, [#7068](https://github.com/eclipse-dirigible/dirigible/issues/7068)):** a `generates:` with a `sourceStatus:` completion hook flipped its source once the target existed and then went on offering the same button on the flipped record - and answering the same endpoint 200 - so a second click minted a **second document**: a proforma already INVOICED produced a second invoice, in the customer's hands. The hook declared what "already done" looks like; nothing consulted it. A create-from now carries a from-status guard resolved ONCE and fed to both halves of the action: the generated `run()` refuses with **409** before anything is created, and the contributed action descriptor carries the same guard so the shared `customActions` store stops OFFERING the click on a record it would refuse (`getActions(view, type, record)` takes the record the view already has). Two shapes: `fromStatus: [...]` is the explicit allow-list - the `from:` of a `transitions:` entry, spelled differently only because `from:` on a create-from already names the source ENTITY - and absent it a declared `sourceStatus` IMPLIES the deny-list of exactly that status, so a model that already carries the defect is fixed with no authoring change. The guard is on the CLICK: an event-driven create-from keeps its own at-most-once back-reference guard and qualifies its moment with `event.when`, so `fromStatus` on an event-only rule is refused at parse rather than silently ignored - as are a `page` scope, a source with no `function: EntityStatus` relation, and an allow-list containing the `sourceStatus` the action itself writes. diff --git a/components/engine/engine-intent/CLAUDE.md b/components/engine/engine-intent/CLAUDE.md index d2f9bc884c3..5cf40f2e338 100644 --- a/components/engine/engine-intent/CLAUDE.md +++ b/components/engine/engine-intent/CLAUDE.md @@ -457,7 +457,7 @@ Semantics worth knowing: - **`fromStatus:` (and the guard `sourceStatus:` implies) = a create-from is not offered twice (#7068).** A `generates:` was unconditional. With a `sourceStatus:` completion hook it flipped the source once the target existed - and then went on offering the same button on the flipped record and answering the same endpoint 200, so a second click minted a **second document**: a ProformaInvoice already INVOICED produced SalesInvoice 10 and then SalesInvoice 11, both in the customer's hands. The hook DECLARED what "already done" looks like; nothing consulted it - the authored-but-unconsumed class, and the reason the gap was invisible (both halves of the model read correctly). The fix is one rule resolved once, `GeneratesGuardSupport`, feeding **both** halves of the action: the generated `run()` refuses with **409** before anything is created (the pre-check load is on the guarded path only), and the contributed action descriptor carries the same `guard: { property, allowed | blocked }` so the shared `customActions` store stops OFFERING the click on a record it would refuse - the store's `getActions(view, type, record)` gained an optional record and the four entity-action sites pass the one they already have (`selected`, and the document form). Two shapes, one guard: `fromStatus: [...]` is the explicit allow-list - the `from:` of a `transitions:` entry, spelled differently ONLY because `from:` on a create-from already names the source ENTITY, which is why the issue's suggested `from:` could not be taken literally; absent it, a declared `sourceStatus` IMPLIES the deny-list of exactly that status, which is the minimal refusal and needs no authoring change for the models that already carry the defect. The guard is on the **click**, deliberately: an event-driven create-from already carries the at-most-once back-reference guard (or asked for a row per event with `mode: append`) and qualifies its moment with `event.when`, so `fromStatus` on an event-only rule is **refused at parse** rather than silently ignored. Also refused: a `page` scope (no record to read a status from), a source with no `function: EntityStatus` relation (nothing to read), and an allow-list containing the `sourceStatus` the action itself writes (it re-opens exactly the duplicate the guard removes). The statuses are symbolic like every other status site (`StatusSymbolResolver.rewriteGenerates` now resolves `fromStatus` too). Emission is gated on a new `hasStatusGuard` boolean, so a `.glue` written before the key existed renders the unguarded `run()` it always did. Covered by `GeneratesIntentTest` + `GlueGeneratesTest` + `IntentEngineIT.generates_completion_hook_flips_the_source_via_targeted_update` (the 409 branch, its ordering before the create, and the descriptor's guard). - **A mutual cross-model `generates` cycle bootstraps through a declared pass, not a hand-strip ([#6539](https://github.com/eclipse-dirigible/dirigible/issues/6539)).** A cross-model create-from is resolved against the target's real `.model` (`CrossModelSupport.resolve`, workspace-or-registry, loud on absence), which has no first project when the pair is MUTUAL - the canonical opportunity -> quotation funnel, where A mints a document into B while B holds a foreign key back to A: A cannot generate because B's `.model` does not exist, and B cannot because A's does not. The workaround was to strip A's `generates` block, generate A, generate B, restore the block, regenerate A - five steps, four of them editing the intent to say something it does not mean. Now the pass itself takes `bootstrap=true` (`POST /services/ide/intent/generate?...&bootstrap=true` -> `IntentGenerationService.generate(..., bootstrap)` -> `IntentGenerationContext.isBootstrap()`), which skips exactly the create-from whose owner model is not there yet and names it in `warnings`: bootstrap here, generate the dependency, regenerate here normally. **Absence is the whole trigger, and it is asked as a separate question** - `CrossModelSupport.ownerModelExists` tests whether the owner's `.model` FILE is readable from either source, deliberately narrower than `resolve` succeeding, so an owner that IS there but declares no such entity keeps failing loudly in a bootstrap pass too ("the dependency is not generated yet" and "the reference is wrong" want opposite answers, and the second is the one a bootstrap flag could hide forever). **Nothing else is relaxed**: a cross-model RELATION never degrades to a guess - its table, key column and FK type would have to be invented and the emitted schema would be wrong rather than incomplete - and lazy resolution was rejected for the same reason (a `generates` glue entry needs the target's perspective + PK at glue-generation time, and the convention fallback is exactly the dead-dropdown guess `CrossModelSupport` exists to refuse). **The default pass teaches the escape**: `buildGenerates` asks the absence question in both modes and, outside a bootstrap, throws its own `BootstrapRequiredException extends IntentValidationException` naming the cycle and the three-step recipe - so the endpoint can answer the ordinary 422 plus `bootstrap: true`, the one fact a caller cannot read out of the text, and the Intent Editor offers "Generate anyway" as a retry instead of leaving the developer to edit the document. Covered by `GlueGeneratesBootstrapTest` (skip + warning, the loud default with the recipe, and the present-but-wrong reference staying fatal under bootstrap) and `IntentEngineIT.mutual_cross_model_generates_bootstraps`. - **`generates.event:` on the process-step axis + an opt-in `mode: append` (#6800).** Two narrow extensions that together close "on event E, append a derived row" - a `LogEntry` per process step, a protocol line per transition - which **no** event-driven construct could express: every candidate either writes into an existing row (`postings`/`rollups`/`aggregates`), or was at-most-once by construction (`generates` + `event:`), so the shape needed a hand-written listener under `custom/` or an `outbound` -> `inbound` loopback. (1) The `event:` map now also takes the **step axis** `onStepReached`/`onStepCompleted: { process, step }` that `notifications`/`integrations`/`outbound` already bind to (#6537) - so a create-from can hang off a moment in a flow rather than a status write, which is also the one route around a state write that publishes nothing. Its extra narrowing over the other consumers: the process's `trigger:` entity must EQUAL `from:` (the step event is delivered as a message about the process's trigger record, and that record is what the create-from reads by id), and the source must be local - a process and its steps belong to the model that declares them, so a `fromUses:` source is rejected. `when:` stays optional on this axis: the step IS the moment. (2) `mode: once` (**default** - unchanged behaviour, byte-identical output) vs `mode: append`, which drops the existing-target lookup in `Generate.java.template` (`#if($hasEvent && !$appendMode)`, the single guard site, inside the shared `create()`), so every delivery creates a row. **The back-reference stays REQUIRED in both modes** - the dedup key under `once`, the row's provenance under `append` (a log row nothing points back at cannot be read); the parser message names both roles. Emission: `putGeneratesEvent` gained `isStep`/`stepProcess`/`stepName`/`topicSuffix`/`appendMode`, and the listener's `destination()` now renders `${topicSuffix}` instead of branching on `isCreate` (`""` for a create, `-transitioned` for a transition, `-step---reached|completed` for a step - same strings as before). **`StepEventSupport.boundEvents` had to learn about `generates`**, not just `GlueIntentGenerator`: `emitters()` reads that list, so without it a moment whose ONLY consumer is a create-from got no `JavaDelegate` emitter and the listener bound a topic nothing published to. **What `append` is NOT:** a state-aware guard. It is the ABSENCE of one - a redelivery appends a duplicate (the step topic is published after commit, not transactionally with the step, the same at-least-once contract `outbound` states), and it is the wrong answer to "I voided the target and cannot regenerate it" (that is #6814's stage-aware predicate on `mode: once`). Two `append` rules sharing a target AND a back-reference are **legal by design** (each records a different moment) - which is why #6813's parse-time collision diagnostic must be scoped to `once` pairs only. Covered by `GeneratesIntentTest` (step binding accepted; unknown process/step, non-eventable kind, trigger-entity mismatch, cross-model source, a mode with no trigger, an unknown mode, a missing back-reference under append, a prompt on an appending create-from all rejected) + `GlueGeneratesTest` (the step topic, `appendMode`, the emitter for a generates-only moment, and both lifecycle axes unchanged) + `IntentEmissionCoverageIT.assertGeneratesStepAxisRuntime` - one shipment whose all-serviceTask flow appends TWO log rows from two moments sharing the same back-reference, a click appending a THIRD, and an at-most-once sibling on the same moment minting exactly one summary that a later click hands back. -- **A create-from's `items:` has a SOURCE-ROW RULE (`where:` + `refuse:`, [#7091](https://github.com/eclipse-dirigible/dirigible/issues/7091)).** The mirror `items:` block cloned EVERY row of the source document into a target line, and there was no way to say which rows qualified - `GeneratesItemsIntent` carried `from`/`to`/`map`/`defaults` and nothing else, and `checks:` lives on entities and cannot gate a generate's item selection. So base-timesheets `invoice-from-timesheet` billed every `EmployeeTimesheet` of the project-month: a DRAFT / SUBMITTED / REJECTED one at the same footing as an APPROVED one (hours nobody has approved on the customer's invoice), and an EMPTY one - `totalHours` null - refused the whole Generate since #7081, so the clerk could not invoice the month at all until someone deleted the row by hand. "Invoice the approved month" is the one flow a billing clerk runs, and the module could either bill unapproved hours or not bill at all; the gap is fleet-wide (proforma -> invoice, quotation -> order, order -> invoice). **`where:` is the rule**, the same field/op/value triples a `schedules[].where` carries (`ScheduleSupport.conditionChain` is the shared renderer, extracted from `criteriaExpression`), pushed into the very `Criteria` that already selects the source's rows by their master foreign key - so an unqualified row is never loaded, rather than filtered in Java after the fact. A condition naming the source item's own `function: EntityStatus` relation may use the seeded status NAME (`StatusSymbolResolver.rewriteGeneratesItemsWhere`, on the ITEM's nomenclature, not the header's - resolving against the document's lifecycle would take an id out of the wrong nomenclature and quietly filter on it); only that one condition is a candidate, exactly as a register lookup's static filter is, or a `like` pattern on a name would be reported as an unknown status. **`refuse:` declares the other reading**: an unqualified row stops the whole create-from with the authored message plus the KEYS of the offending rows, instead of being left out. Which of the two a document means is a property of the document, not of the platform - a rejected timesheet quietly dropped from an invoice and a rejected timesheet quietly billed are both wrong, for different months - so skipping is the default and `refuse:` is opt-in (and refused at parse without a `where`, there being nothing for a row to be unqualified against). **A rule that qualifies NO row refuses too**, rather than committing a header with no lines at all: that is the harder of the two failures to notice, since the document exists and counts as the period's billing. Scoped to a `where`-declaring block on purpose - a rule-less items block keeps exactly the behaviour it had, and the descriptor's two new keys default to the empty string in `GlueGenerator.bindGenerate`, so a `.glue` written before them renders the unfiltered clone loop it always did (the third-edit trap #7070 documents). Unlike a schedule's query, whose source may be a cross-model row or an `audit:` column this model cannot see, the rule reads a LOCAL row being cloned - so the `field` is checked against the item source's own fields and to-one relations at parse, a name it does not declare being a condition the database would reject on the first click. Covered by `GlueGeneratesItemsWhereTest` (the rendered chain incl. the resolved status name and a moment value; the rule-less descriptor unchanged; each refusal) and `IntentEmissionCoverageIT` (the emitted query and both refusals, then end to end: a mixed stay bills exactly its past nights, an all-unqualified one answers 400 with the authored message, and the same source under the skip rule answers 400 for the lineless document). +- **A create-from's `items:` has a SOURCE-ROW RULE (`where:` + `refuse:`, [#7091](https://github.com/eclipse-dirigible/dirigible/issues/7091)).** The mirror `items:` block cloned EVERY row of the source document into a target line, and there was no way to say which rows qualified - `GeneratesItemsIntent` carried `from`/`to`/`map`/`defaults` and nothing else, and `checks:` lives on entities and cannot gate a generate's item selection. So base-timesheets `invoice-from-timesheet` billed every `EmployeeTimesheet` of the project-month: a DRAFT / SUBMITTED / REJECTED one at the same footing as an APPROVED one (hours nobody has approved on the customer's invoice), and an EMPTY one - `totalHours` null - refused the whole Generate since #7081, so the clerk could not invoice the month at all until someone deleted the row by hand. "Invoice the approved month" is the one flow a billing clerk runs, and the module could either bill unapproved hours or not bill at all; the gap is fleet-wide (proforma -> invoice, quotation -> order, order -> invoice). **`where:` is the rule**, the same field/op/value triples a `schedules[].where` carries (`ScheduleSupport.conditionChain` is the shared renderer, extracted from `criteriaExpression`), pushed into the very `Criteria` that already selects the source's rows by their master foreign key - so an unqualified row is never loaded, rather than filtered in Java after the fact. A condition naming the source item's own `function: EntityStatus` relation may use the seeded status NAME (`StatusSymbolResolver.rewriteGeneratesItemsWhere`, on the ITEM's nomenclature, not the header's - resolving against the document's lifecycle would take an id out of the wrong nomenclature and quietly filter on it); only that one condition is a candidate, exactly as a register lookup's static filter is, or a `like` pattern on a name would be reported as an unknown status. **`refuse:` declares the other reading**: an unqualified row stops the whole create-from with the authored message plus the KEYS of the offending rows, instead of being left out. Which of the two a document means is a property of the document, not of the platform - a rejected timesheet quietly dropped from an invoice and a rejected timesheet quietly billed are both wrong, for different months - so skipping is the default and `refuse:` is opt-in (and refused at parse without a `where`, there being nothing for a row to be unqualified against). **A rule that qualifies NO row refuses too**, rather than committing a header with no lines at all: that is the harder of the two failures to notice, since the document exists and counts as the period's billing. Scoped to a `where`-declaring block on purpose - a rule-less items block keeps exactly the behaviour it had, and the descriptor's two new keys default to the empty string in `GlueGenerator.bindGenerate`, so a `.glue` written before them renders the unfiltered clone loop it always did (the third-edit trap #7070 documents). Unlike a schedule's query, whose source may be a cross-model row or an `audit:` column this model cannot see, the rule reads a LOCAL row being cloned - so the `field` is checked against the item source's own fields and to-one relations at parse, a name it does not declare being a condition the database would reject on the first click. Covered by `GlueGeneratesItemsWhereTest` (the rendered chain incl. the resolved status name and a moment value; the rule-less descriptor unchanged; each refusal) and `IntentEmissionCoverageIT` (the emitted query and both refusals, then end to end: a mixed stay bills exactly its past nights, an all-unqualified one answers 400 with the authored message, and the same source under the skip rule answers 400 for the lineless document). **Both refusals are decided BEFORE the header is saved** ([#7224](https://github.com/eclipse-dirigible/dirigible/issues/7224)): the generated repository's `save()` allocates the document number and records the `History` create entry OUTSIDE the unit of work (#7069, by design - the counter is a sequence in its own transaction), so a refusal fired after the save took the header back but left a spent number of a gap-free series and a trail row for a document that never existed, once per click - three presses of "Invoice this month" on a month with one rejected timesheet burned three invoice numbers. The rule block runs right after the at-most-once guard, before the target is even built; `IntentEmissionCoverageIT` asserts the emitted order and, end to end, that a refused run leaves the Bill series and the Bill history trail exactly as it found them. - **A create-from is ONE transaction, and a missing required value is refused by name (#7069).** The create-from writes three things that only make sense together - the target header, its lines, the source's `sourceStatus:` flip - and each repository call was its own transaction, so a line the target refused left the other two committed: a header-only invoice, its source already marked INVOICED, and an HTTP **500** carrying a PostgreSQL message about `SALES_INVOICE_ITEM_QUANTITY`. The record could never be generated from correctly again without an administrator, and the result is exactly the "empty document" class the quality gates exist to catch - reached through the platform's own generator. Two halves, both in the shared machinery rather than in this create-from: (1) `JavaEntityStore.inUnitOfWork` binds a session and a transaction to the thread, so every store call inside joins it, reads see the block's own writes, and the events ride the transaction and dispatch only after the whole unit commits; the client-facing entry is `org.eclipse.dirigible.components.data.store.java.repository.UnitOfWork.call(...)`, and `Generate.java.template` wraps `create(...)` in it. The `-transitioned` announcement rides the source's status flip INSIDE the block (#7160): the flip and its event are recorded by one write in the unit's own transaction, and a unit's events reach the broker only once the whole unit committed - so it still states a COMPLETED transition (a target the lines refuse takes the flip and its notice along), while a crash between commit and a separate publish no longer loses it. It needs no flag either: the flip runs only on the path that generates, past the early returns (no source; the at-most-once guard handing back an existing document). Deliberately outside the unit: the `History` trail, document numbering and the outbox DDL, each on its own connection - a rolled-back unit leaves a history row and consumes a number, both records of an attempt. (2) The generated repository refuses a write that leaves a **NOT NULL** column empty (`. is required`, a `ValidationException`, i.e. 400) instead of letting the statement reach the database. It is the NOT NULL columns the schema declares, MINUS those carrying a DEFAULT - the database supplies that value, so an empty one is not missing, which is exactly what a relation's `init:` opening status is (refusing it would reject every create that leaves the status to the model; `IntentEmissionCoverageIT` catches it at runtime). Nothing that used to be written is now refused - an insert of null into a defaultless NOT NULL column never had another ending; only the answer changed, from a driver message naming a physical column to the property the author knows. The check sits after everything the repository computes itself (numbering, uuid, calculated fields, a document's totals) and before the insert, in `save`, `update` and `updateWithoutEvent` - not on the targeted primitives, which name their own columns. The create-from adds the row the refused line was mapped FROM (`... (from EmployeeTimesheet [7])`), because which of a hundred lines is missing a value is the whole question the caller has - which needed one new glue key, `fromItemPk`. Covered by `JavaUnitOfWorkIT` (rollback, the control case without the block, read-your-own-writes) and `IntentEngineIT` (the unit wraps the body, the announcement is recorded inside it and never as a bare publish, the required refusal is generated by name). - **`prompt:` on a `generates` action = a declared input form before the create (#6685).** The gap it closes: `transitions:` writes but takes no input and `generates:` creates but declares every value up front, so an action that collects the two answers the source cannot derive (which payment, how much) had to be a hand-written page. It reaches a post-issue child on an IMMUTABLE document too, because per-record action buttons are deliberately NOT gated on mutability (that is why Void works) - the **action-shaped sibling of `locksWithMaster: false`** (#6700), which reopens the child's own panel: the panel is the affordance for ordinary data entry, a prompted action for a guided create over mostly-derived values. `prompt:` entries name fields / to-one relations of the TARGET; parser (`validateGeneratesPrompt`): local target only, target must declare a composition to-one relation to `forEntity` (that guarantees the generated detail registration the dialog renders from), scope `entity`, no `timestamp` fields, no overlap with `map`/`defaults` (one writer), no duplicates, and **no `event:`** (an event-driven create-from runs with nobody there to answer the form - which is also why the prompted values ride the ENDPOINT path only: `run()` checks the required ones and passes the map into `create(sourceId, values)`, while the event listener's `create(sourceId)` signature is untouched). Server half: `promptFields` in the glue (PascalCase prop + required + a pre-rendered `Object raw` -> field-type conversion), `Generate.java.template` takes `values` in the Request, 400s on a missing required input BEFORE anything is written, and sets prompted values after map/defaults - the save still goes through the target's repository so numbering/checks/events fire. Client half: the descriptor carries `prompt` + `promptEntity` (authored names ONLY - control types, lookup URLs and `dependsOn` metadata are resolved AT RUNTIME from `App.detailsFor(view)`'s edit-columns registration, so the intent layer never references template routes); the shared `customActions` store opens an input dialog instead of the plain confirm (`openPrompt`/`promptRun` + a mini dependsOn cascade seeded from the clicked master id - the invoice's Customer chain narrows the payment list, `valueFrom` defaults the amount), degrading to the confirm when the registration is absent (the shared shell). Dialog markup rides in all five shells wrapped in the `customActionPrompt` Alpine component so the Velocity shell stays `$store`-free. Covered by the `GeneratesIntentTest` prompt tests + `GlueGeneratesTest.promptFieldsRenderTypedConversions` + the `IntentEmissionCoverageIT` prompted-generates assertions (emission + 400 + value-reaches-the-row). 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 dd44ffd53fb..737df4a302b 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 @@ -1853,8 +1853,9 @@ keys of the offending rows - which of a hundred lines to go and fix is the calle **A rule that qualifies no row refuses either way.** An invoice with no lines is not the invoice that was asked for, and it is the harder failure to notice - it exists and counts as the period's -billing - so the run answers 400 rather than committing the header. An items block with no `where:` -keeps exactly the behaviour it had. +billing - so the run answers 400 rather than committing the header. Either refusal is decided before +the header is saved, so a refused run spends no document number and leaves no history entry. An +items block with no `where:` keeps exactly the behaviour it had. **A `map:` source may hop one relation - and that is how you SNAPSHOT a value.** A value is `map`ped rather than reached through a relation when the target must keep what was true at the moment it was diff --git a/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Generate.java.template b/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Generate.java.template index 9f1d40faf8b..61a18ef43af 100644 --- a/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Generate.java.template +++ b/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Generate.java.template @@ -30,6 +30,8 @@ import org.eclipse.dirigible.sdk.http.Response; #{else} * lines. An unqualified row is simply left out of the document (declare `refuse:` for the other reading). #end + * Either refusal is decided before the ${toEntity} header is saved, so a refused run spends no document + * number and leaves no history entry (issue #7224). #end * * Generated from the intent generates block - do not edit; it is re-generated with the application. @@ -172,6 +174,59 @@ public class ${className}Generate { } #end #end + #if($hasItems && $itemWhere != "") + // Source-row rule (intent `items: where:` - issue #7091): only the source rows the rule + // qualifies become lines. Pushed into the very query that selects them by their master + // foreign key, so an unqualified row is never loaded. Without a rule every row was cloned, + // which billed a draft or rejected line at the same footing as an approved one - and, since + // the target refuses a line missing a required value, let ONE empty row stop the whole month + // from being invoiced with nothing the intent could say about it. + // Decided HERE, before the ${toEntity} header is built and saved, and that ordering is as + // deliberate as the sourceStatus flip's below: both refusals under this rule read nothing + // but the source's rows, while the header's save allocates its document number and records + // its History create entry OUTSIDE this unit (the counter is a sequence in its own + // transaction so concurrent creates never serialize on it - dirigible #7069). A refusal + // fired after the save therefore rolled the header back and left a spent number in a + // series that must stay gap-free and a trail row for a document that never existed - one + // of each per click, three invoice numbers for three presses on a month with one rejected + // timesheet (dirigible #7224). Refused here, a rejected click writes nothing at all, which + // is what run() promises for a missing required `prompt:` input too. + java.util.List qualifying = + new gen.${fromGenFolder}.data.${fromItemJavaPerspective}.${fromItemEntity}Repository() + .findAll(Criteria.create() + .eq("${srcFkProperty}", sourceId)${itemWhere}); + #if($itemRefuse != "") + // `refuse:` - an unqualified row stops the whole create-from instead of being left out. + // Dropping a rejected line silently and billing it silently are both wrong, for different + // months; which one this document means is the author's call, and this is it. The keys of + // the offending rows travel with the message: which of them to go and fix is the whole + // question the caller has. + java.util.Set qualifyingKeys = new java.util.HashSet<>(); + for (gen.${fromGenFolder}.data.${fromItemJavaPerspective}.${fromItemEntity}Entity row : qualifying) { + qualifyingKeys.add(row.${fromItemPk}); + } + java.util.List unqualified = new java.util.ArrayList<>(); + for (gen.${fromGenFolder}.data.${fromItemJavaPerspective}.${fromItemEntity}Entity row : + new gen.${fromGenFolder}.data.${fromItemJavaPerspective}.${fromItemEntity}Repository() + .findAll(Criteria.create() + .eq("${srcFkProperty}", sourceId))) { + if (!qualifyingKeys.contains(row.${fromItemPk})) { + unqualified.add(row.${fromItemPk}); + } + } + if (!unqualified.isEmpty()) { + throw new org.eclipse.dirigible.sdk.db.ValidationException( + "${itemRefuseJavaLiteral} (${fromItemEntity} " + unqualified + ")"); + } + #end + if (qualifying.isEmpty()) { + // Every row failed the rule, so the ${toEntity} would carry no lines at all. A document + // of no lines is not the document that was asked for, and it is the harder of the two + // failures to notice - it exists, it counts as the period's billing, and it is empty. + throw new org.eclipse.dirigible.sdk.db.ValidationException( + "no ${fromItemEntity} row of the ${fromEntity} qualifies for ${name}, so the ${toEntity} would have no lines"); + } + #end #foreach($load in $relationLoads) // A one-hop `relation.field` map source: the related row is loaded by the source's foreign key, // once, and read below. Loaded AFTER the at-most-once guard so an already-generated source costs @@ -230,47 +285,6 @@ public class ${className}Generate { new gen.${toGenFolder}.data.${toJavaPerspective}.${toEntity}Repository().save(target); #if($hasItems) #if($itemWhere != "") - // Source-row rule (intent `items: where:` - issue #7091): only the source rows the rule - // qualifies become lines. Pushed into the very query that selects them by their master - // foreign key, so an unqualified row is never loaded. Without a rule every row was cloned, - // which billed a draft or rejected line at the same footing as an approved one - and, since - // the target refuses a line missing a required value, let ONE empty row stop the whole month - // from being invoiced with nothing the intent could say about it. - java.util.List qualifying = - new gen.${fromGenFolder}.data.${fromItemJavaPerspective}.${fromItemEntity}Repository() - .findAll(Criteria.create() - .eq("${srcFkProperty}", sourceId)${itemWhere}); - #if($itemRefuse != "") - // `refuse:` - an unqualified row stops the whole create-from instead of being left out. - // Dropping a rejected line silently and billing it silently are both wrong, for different - // months; which one this document means is the author's call, and this is it. The keys of - // the offending rows travel with the message: which of them to go and fix is the whole - // question the caller has. - java.util.Set qualifyingKeys = new java.util.HashSet<>(); - for (gen.${fromGenFolder}.data.${fromItemJavaPerspective}.${fromItemEntity}Entity row : qualifying) { - qualifyingKeys.add(row.${fromItemPk}); - } - java.util.List unqualified = new java.util.ArrayList<>(); - for (gen.${fromGenFolder}.data.${fromItemJavaPerspective}.${fromItemEntity}Entity row : - new gen.${fromGenFolder}.data.${fromItemJavaPerspective}.${fromItemEntity}Repository() - .findAll(Criteria.create() - .eq("${srcFkProperty}", sourceId))) { - if (!qualifyingKeys.contains(row.${fromItemPk})) { - unqualified.add(row.${fromItemPk}); - } - } - if (!unqualified.isEmpty()) { - throw new org.eclipse.dirigible.sdk.db.ValidationException( - "${itemRefuseJavaLiteral} (${fromItemEntity} " + unqualified + ")"); - } - #end - if (qualifying.isEmpty()) { - // Every row failed the rule, so the ${toEntity} would carry no lines at all. A document - // of no lines is not the document that was asked for, and it is the harder of the two - // failures to notice - it exists, it counts as the period's billing, and it is empty. - throw new org.eclipse.dirigible.sdk.db.ValidationException( - "no ${fromItemEntity} row of the ${fromEntity} qualifies for ${name}, so the ${toEntity} would have no lines"); - } for (gen.${fromGenFolder}.data.${fromItemJavaPerspective}.${fromItemEntity}Entity srcItem : qualifying) { #else for (gen.${fromGenFolder}.data.${fromItemJavaPerspective}.${fromItemEntity}Entity srcItem : 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 fc6380131da..7e7c065efab 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 @@ -33,6 +33,7 @@ import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.Statement; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -748,13 +749,21 @@ class IntentEmissionCoverageIT extends IntegrationTest { # DocumentItem child), it has a counterparty to mail one hop away (Person.email) and an # EntityStatus for the SendBill transition to flip - so a notify block with # `attach: print` is authored on a transition AND on a process step (below). + # + # number + history: what a REFUSED create-from must leave untouched (#7224). Bill is the + # target of both rule-carrying create-froms below, and its repository allocates the + # document number and records the History create entry outside the unit of work - so a + # rule that refused AFTER the header's save burned a number of the gap-free series and + # left a trail row for a document that never existed, once per click. - name: Bill function: Document + history: true # SENT (status 2) freezes the document: the assertions below add the line write that # would otherwise rewrite the totals the sent PDF was rendered from (#6695). immutableWhen: "Status == 2" fields: - { name: id, type: integer, primaryKey: true, generated: true } + - { name: number, type: string, length: 100, number: { series: Emission Bill, stampOn: create } } - { name: note, type: string, length: 200 } - { name: amount, type: decimal, aggregate: true } # expression-calculated from the aggregate: the document-totals recompute must @@ -1643,12 +1652,16 @@ class IntentEmissionCoverageIT extends IntegrationTest { @Autowired private DataSourcesManager dataSourcesManager; + /** The Bill series' prefix - the series a refused create-from must not advance (#7224). */ + private static final String BILL_NUMBER_PREFIX = "EB-"; + /** * The module's series declaration - AUTHORED next to app.intent (like .roles), never generated; the * .numbers synchronizer provisions it per tenant at publish. Prefix ER- in a total width of 8 → - * {@code ER-00001}. + * {@code ER-00001}; the Bill series is shaped the same way, {@code EB-00001}. */ - private static final String NUMBERS_JSON = "{\"series\": [{\"name\": \"Emission Receipt\", \"prefix\": \"ER-\", \"size\": 8}]}"; + private static final String NUMBERS_JSON = "{\"series\": [{\"name\": \"Emission Receipt\", \"prefix\": \"ER-\", \"size\": 8}," + + " {\"name\": \"Emission Bill\", \"prefix\": \"" + BILL_NUMBER_PREFIX + "\", \"size\": 8}]}"; /** * The hand-written half of a calculated action: the contract is that the developer authors the @@ -3381,6 +3394,16 @@ private void assertEmission() { String checkedBillFromStay = contentOf("gen/events/emission/CheckedBillFromStayGenerate.java"); assertTrue(checkedBillFromStay.contains("\"Stay night carries no \\\"amount\\\" (StayNight \" + unqualified + \")\""), "refuse: must throw the authored message carrying the keys of the offending rows"); + // ...and both refusals are decided BEFORE the header is saved (#7224). The target's save + // allocates the document number and records the History create entry outside the unit of + // work, so a refusal fired after it took the header back and left a spent number and a trail + // row for a document that never existed - once per click. + int billHeaderSave = billFromStay.indexOf("BillRepository().save(target)"); + assertTrue(billHeaderSave > 0 && billFromStay.indexOf("would have no lines") < billHeaderSave, + "the source-row rule must refuse before the header is saved, got: " + billFromStay); + int checkedBillHeaderSave = checkedBillFromStay.indexOf("BillRepository().save(target)"); + assertTrue(checkedBillHeaderSave > 0 && checkedBillFromStay.indexOf("Stay night carries no \\\"amount\\\"") < checkedBillHeaderSave, + "refuse: must fire before the header is saved, got: " + checkedBillFromStay); // generates on the step axis + mode: append (#6800): the listener binds the step-scoped topic // the generated emitter publishes the trigger entity on (NOT a lifecycle topic), and the @@ -4941,16 +4964,20 @@ private void assertGeneratesItemsRuleRuntime() { // The skip reading: the bill carries the three past nights and not the future one. AtomicInteger bill = new AtomicInteger(); - restAssuredExecutor.execute(() -> bill.set(io.restassured.path.json.JsonPath.from(given().contentType("application/json") - .body("{\"id\":" + mixed + "}") - .when() - .post("/services/java/" + PROJECT - + "/gen/events/emission/BillFromStayGenerate/run") - .then() - .statusCode(200) - .extract() - .asString()) - .getInt("Id"))); + AtomicReference billNumber = new AtomicReference<>(); + restAssuredExecutor.execute(() -> { + io.restassured.path.json.JsonPath created = io.restassured.path.json.JsonPath.from(given().contentType("application/json") + .body("{\"id\":" + mixed + "}") + .when() + .post("/services/java/" + PROJECT + + "/gen/events/emission/BillFromStayGenerate/run") + .then() + .statusCode(200) + .extract() + .asString()); + bill.set(created.getInt("Id")); + billNumber.set(created.getString("Number")); + }); restAssuredExecutor.execute(() -> given().when() .get(API + "/bill/BillLineController?Bill=" + bill.get()) .then() @@ -4958,6 +4985,14 @@ private void assertGeneratesItemsRuleRuntime() { .body("$", hasSize(3)), 30); + // A refused run must cost NOTHING (#7224). The Bill's save allocates its document number and + // records its History create entry outside the unit of work - by design, so concurrent creates + // never serialize on the counter - which the rollback of a refusal fired AFTER the save could + // not undo: every refused click spent a number of a gap-free series and left the trail of a + // document that never existed. The trail is read as CREATE rows only, because the BillFlow + // process the bill above started writes its ProcessId back on its own time. + long billsRecorded = billHistoryCreates(); + // The refusal reading: every night fails the amount rule, so the run stops with the authored // message rather than leaving them out - and it names the rows to go and fix. restAssuredExecutor.execute(() -> given().contentType("application/json") @@ -4976,6 +5011,46 @@ private void assertGeneratesItemsRuleRuntime() { .then() .statusCode(400) .body(containsString("would have no lines"))); + + assertEquals(billsRecorded, billHistoryCreates(), "a refused create-from must leave no history entry - its document never existed"); + // ...nor a spent number: the next Bill minted takes the number right after the one above. A + // click create-from keeps no at-most-once guard, so the same stay is simply billed again. + AtomicReference nextNumber = new AtomicReference<>(); + restAssuredExecutor.execute(() -> nextNumber.set(io.restassured.path.json.JsonPath.from(given().contentType("application/json") + .body("{\"id\":" + mixed + "}") + .when() + .post("/services/java/" + PROJECT + + "/gen/events/emission/BillFromStayGenerate/run") + .then() + .statusCode(200) + .extract() + .asString()) + .getString("Number"))); + assertEquals(nextBillNumber(billNumber.get()), nextNumber.get(), + "a refused create-from must not spend a document number - the next Bill minted must take the very next one"); + } + + /** The number the Bill series hands out right after the given one: same prefix, same width. */ + private static String nextBillNumber(String number) { + String digits = number.substring(BILL_NUMBER_PREFIX.length()); + return BILL_NUMBER_PREFIX + String.format("%0" + digits.length() + "d", Integer.parseInt(digits) + 1); + } + + /** + * How many Bill creates the history trail has recorded. A CREATE writes one row per tracked + * property, so the count moves by a whole record's worth at a time - what matters here is that a + * refused run moves it by nothing. + */ + private long billHistoryCreates() { + try (Connection connection = dataSourcesManager.getDefaultDataSource() + .getConnection(); + Statement statement = connection.createStatement(); + ResultSet count = statement.executeQuery("SELECT COUNT(*) FROM \"EMISSION_BILL_HISTORY\" WHERE \"Operation\" = 'CREATE'")) { + assertTrue(count.next(), "the Bill history table must be readable"); + return count.getLong(1); + } catch (SQLException ex) { + throw new IllegalStateException("Failed to count the Bill history's create rows", ex); + } } /** A stay whose nights the {@code nights} expansion spreads the given total across. */