From 54648ab6f766786e34d64e39fa4c391a2f1fb11c Mon Sep 17 00:00:00 2001 From: delchev Date: Thu, 27 Aug 2026 13:05:40 +0300 Subject: [PATCH] fix(intent): a generates: map key is validated against the target (#6953) A `map:` entry has two ends and only one was checked. The value side has always been resolved against the source; the KEY side - the property the target receives - was never checked against the target at all. The generator pascal-cases the key and emits `target. = ...`, so a key the target does not declare is not a mis-mapping that shows up at run time: it is Java that does not compile, and client Java compiles as one registry-wide batch, so one bad key takes every module's beans down with it. The model parses clean, generates clean, and fails in generated code - the authored-but-broken class this module refuses everywhere else. `postings:` never had the hole - it checks its `map` keys against its `creates` target. This closes the asymmetry with the same message shape and the same case-insensitive match (the key is authored PascalCase, the target's field camelCase), applied at the three sites that mint a record from a map: `generates[].map` (target = `to:`), `generates[].items.map` (target = the items `to:`) and `schedules[].generate.map`. A CROSS-MODEL target (`uses:`) is exempt, the convention every cross-model reference follows: its property names live in the owner's `.model` and are resolved at generation time. The items map inherits that exemption, since a cross-model header implies a cross-model item target. Two in-repo fixtures turned out to be exactly the bug being closed - both mapped onto a LOCAL target that declared no such property, and both would have generated Java that does not compile. They now declare the property they meant. Every other fixture and the whole production-intent corpus (117 .intent documents) parse unchanged. Related: the back-reference map entry doubles as the at-most-once guard (#6711), so a mistyped key there silently broke the guard derivation too - this catches it at parse time. Co-Authored-By: Claude Opus 5 --- .../intent/parser/IntentParser.java | 46 +++++ .../main/resources/intent-assistant-guide.md | 8 + .../intent/generator/GlueGeneratesTest.java | 1 + .../intent/parser/GeneratesIntentTest.java | 169 ++++++++++++++++++ .../intent/parser/IntentParserTest.java | 2 + 5 files changed, 226 insertions(+) diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/parser/IntentParser.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/parser/IntentParser.java index a9908b1bd12..c70b38cf020 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/parser/IntentParser.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/parser/IntentParser.java @@ -910,6 +910,8 @@ private static void validateScheduleGenerate(ScheduleIntent schedule, EntityInte + "] (add a uses: alias if the target lives in another model)"); } validateMapSource(source, byName, g.getMap(), "schedule [" + name + "]", "generate map", true, issues); + validateMapTarget(crossModel || g.getTo() == null ? null : byName.get(g.getTo()), g.getMap(), "schedule [" + name + "]", + "generate map", issues); if (g.getItems() != null || (g.getItemLines() != null && !g.getItemLines() .isEmpty())) { issues.add("schedule [" + name + "] generate declares items - item cloning is not supported for a scheduled generation;" @@ -6336,6 +6338,8 @@ private static void validateGenerates(IntentModel model, Set entityNames } validateGeneratesEvent(g, name, source, crossModelSource, model, issues); validateMapSource(source, byName, g.getMap(), "generates [" + name + "]", "map", true, issues); + validateMapTarget(crossModel || g.getTo() == null ? null : byName.get(g.getTo()), g.getMap(), "generates [" + name + "]", "map", + issues); if (g.getItems() != null) { GeneratesItemsIntent items = g.getItems(); EntityIntent itemSource = null; @@ -6355,6 +6359,10 @@ private static void validateGenerates(IntentModel model, Set entityNames issues.add("generates [" + name + "] items has no to entity"); } validateMapSource(itemSource, byName, items.getMap(), "generates [" + name + "]", "items map", false, issues); + // The item target lives in the SAME model as the header target, so a cross-model header + // implies a cross-model item - resolved in the owner's .model, not here. + validateMapTarget(crossModel || items.getTo() == null ? null : byName.get(items.getTo()), items.getMap(), + "generates [" + name + "]", "items map", issues); } validateGeneratesItemLines(g, name, source, byName, crossModel, issues); validateGeneratesPrompt(g, name, byName, crossModel, issues); @@ -7186,6 +7194,44 @@ private static void validateMapSource(EntityIntent source, Mapkey names a field or a to-one + * relation of the TARGET being created. The generator pascal-cases the key and emits + * {@code target. = ...}, so a key the target does not declare is not a mis-mapping that + * degrades at run time - it is Java that does not compile, and because client Java compiles as one + * registry-wide batch the failure takes every module's beans down with it. + * + *

+ * {@code postings:} has always checked its {@code map} keys against its {@code creates} target; a + * {@code generates:} (and a schedule's {@code generate:}) checked only the value side. This closes + * that asymmetry, with the same message shape and the same case-insensitive match - the key is + * authored PascalCase by convention, the target's field camelCase. + * + *

+ * Skipped when the target is unknown or CROSS-MODEL ({@code uses:}): a foreign target's property + * names live in the owner's {@code .model} and are resolved at generation time, the convention + * every cross-model reference follows. + * + * @param target the entity the map writes into, or {@code null} when it is not resolvable here + * @param map the authored {@code target property -> source property} map + * @param subject the message prefix naming the offending block + * @param role the map's role in that block ({@code map} / {@code items map} / {@code generate map}) + * @param issues the collected issues + */ + private static void validateMapTarget(EntityIntent target, Map map, String subject, String role, List issues) { + if (target == null || map == null) { + return; + } + for (String key : map.keySet()) { + if (key == null || key.isBlank()) { + continue; + } + if (!hasPropertyIgnoreCase(target, key)) { + issues.add(subject + " " + role + " [" + key + "] is not a field or to-one relation of [" + target.getName() + "]"); + } + } + } + /** * One {@code relation.field} map source: the head must be a to-one relation of the mapping source, * the tail a field of the entity that relation points at. Anything deeper, or a tail that is itself 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 25494543b7d..f8cdf32caef 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 @@ -1570,6 +1570,14 @@ through a **cross-model** relation is fine. The two ends are not type-checked, e A schedule's `generate.map` takes the same hop, off the row the cron query returned. +**The KEY side is checked too.** Each `map:` key must name a field or a to-one relation of the +target (`to:`) - the generator emits `target. = ...`, so a key the target does not declare is +not a mis-mapping that shows up at run time, it is Java that does not compile, and client Java +compiles as one registry-wide batch (one bad key takes every module's beans down). The same check +applies to an `items:` map (against the items `to:`) and to a schedule's `generate.map`. A +**cross-model** target (`uses:`) is exempt - its property names live in the owner's `.model` and are +resolved at generation time. + **Cross-model SOURCE (`fromUses:`) - author the create-from on the TARGET's module.** By default the `from` entity is local and the target may be foreign (`uses:`). `fromUses:` mirrors that: the SOURCE is owned by another model and the TARGET is the local one. Both directions describe the same button; they diff --git a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueGeneratesTest.java b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueGeneratesTest.java index 1aaa2f667bc..879ffdd0216 100644 --- a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueGeneratesTest.java +++ b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/GlueGeneratesTest.java @@ -739,6 +739,7 @@ void anEventDrivenGenerateWithoutABackReferenceFailsLoudly() { - name: Declaration fields: - { name: id, type: integer, primaryKey: true, generated: true } + - { name: note, type: string } relations: - { name: Fine, kind: manyToOne, to: Fine, model: fines } generates: diff --git a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/GeneratesIntentTest.java b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/GeneratesIntentTest.java index 62567df61d0..7987b03c791 100644 --- a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/GeneratesIntentTest.java +++ b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/GeneratesIntentTest.java @@ -316,6 +316,7 @@ void rejectsMapSourceThatIsNotASourceProperty() { - name: Order fields: - { name: id, type: integer, primaryKey: true, generated: true } + - { name: note, type: string } generates: - name: bad from: Quote @@ -1335,4 +1336,172 @@ void aReopenTheSourceLifecycleDeclaresParses() { .getSourceStatusOnRetire()); } + /** + * Issue #6953: a {@code map} KEY names a property of the target. An unknown one is not a + * mis-mapping that degrades at run time - the generator emits {@code target. = ...}, so it is + * Java that does not compile, and client Java compiles as one registry-wide batch. + */ + @Test + void rejectsMapKeyThatIsNotATargetProperty() { + String yaml = """ + name: fines + entities: + - name: Fine + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: violationAt, type: timestamp } + relations: + - { name: Vehicle, kind: manyToOne, to: Vehicle } + - name: Vehicle + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: plateNumber, type: string } + - name: FineLog + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: plate, type: string } + generates: + - name: identificationLog + from: Fine + to: FineLog + map: + Plate: Vehicle.plateNumber + Vehicle: Vehicle.plateNumber + violationAt: violationAt + """; + IntentValidationException ex = assertThrows(IntentValidationException.class, () -> IntentParser.parse(yaml)); + assertTrue(ex.getIssues() + .stream() + .anyMatch( + i -> i.contains("generates [identificationLog] map [Vehicle] is not a field or to-one relation of [FineLog]")), + "got: " + ex.getIssues()); + assertTrue(ex.getIssues() + .stream() + .anyMatch(i -> i.contains( + "generates [identificationLog] map [violationAt] is not a field or to-one relation of [FineLog]")), + "got: " + ex.getIssues()); + // The key that IS a target field is not reported, hop-valued or not. + assertFalse(ex.getIssues() + .stream() + .anyMatch(i -> i.contains("map [Plate]")), + "got: " + ex.getIssues()); + } + + /** + * The same check on the {@code items} map, whose target is the items {@code to:} entity. + */ + @Test + void rejectsItemsMapKeyThatIsNotATargetItemProperty() { + String yaml = """ + name: sales + entities: + - name: Quote + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - name: QuoteItem + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: amount, type: decimal } + relations: + - { name: Quote, kind: manyToOne, to: Quote, composition: true, required: true } + - name: Order + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - name: OrderItem + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: amount, type: decimal } + relations: + - { name: Order, kind: manyToOne, to: Order, composition: true, required: true } + generates: + - name: order-from-quote + from: Quote + to: Order + items: + from: QuoteItem + to: OrderItem + map: + Amount: amount + Discount: amount + """; + IntentValidationException ex = assertThrows(IntentValidationException.class, () -> IntentParser.parse(yaml)); + assertTrue(ex.getIssues() + .stream() + .anyMatch(i -> i.contains( + "generates [order-from-quote] items map [Discount] is not a field or to-one relation of [OrderItem]")), + "got: " + ex.getIssues()); + } + + /** + * And on a schedule's {@code generate} map, whose target is that generate's {@code to:}. + */ + @Test + void rejectsScheduleGenerateMapKeyThatIsNotATargetProperty() { + String yaml = """ + name: hr + entities: + - name: Person + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: name, type: string } + - name: Claim + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + relations: + - { name: Person, kind: manyToOne, to: Person } + schedules: + - name: monthly + cron: "0 0 4 1 * *" + entity: Person + generate: + to: Claim + map: { Person: id, Note: name } + """; + IntentValidationException ex = assertThrows(IntentValidationException.class, () -> IntentParser.parse(yaml)); + assertTrue(ex.getIssues() + .stream() + .anyMatch(i -> i.contains("schedule [monthly] generate map [Note] is not a field or to-one relation of [Claim]")), + "got: " + ex.getIssues()); + } + + /** + * A CROSS-MODEL target is skipped: its property names live in the owner's {@code .model} and are + * resolved at generation time, the convention every cross-model reference follows. Both maps - the + * header's and the items' - since a cross-model header implies a cross-model item target. + */ + @Test + void aCrossModelTargetSkipsTheMapKeyCheck() { + IntentModel model = IntentParser.parse(""" + name: timesheets + uses: + - { model: sales } + entities: + - name: ProjectTimesheet + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: note, type: string } + - name: ProjectTimesheetLine + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: hours, type: decimal } + relations: + - { name: ProjectTimesheet, kind: manyToOne, to: ProjectTimesheet, composition: true, required: true } + generates: + - name: invoice-from-timesheet + from: ProjectTimesheet + to: SalesInvoice + uses: sales + map: + NothingCheckableHere: note + items: + from: ProjectTimesheetLine + to: SalesInvoiceItem + map: + NorHere: hours + """); + assertEquals("SalesInvoice", model.getGenerates() + .get(0) + .getTo()); + } + } diff --git a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/IntentParserTest.java b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/IntentParserTest.java index d2a10618371..f3e125a9e68 100644 --- a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/IntentParserTest.java +++ b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/parser/IntentParserTest.java @@ -2000,6 +2000,8 @@ void scheduleGenerateChildrenValidate() { + " - name: Claim\n" // + " fields:\n" // + " - { name: id, type: integer, primaryKey: true, generated: true }\n" // + + " relations:\n" // + + " - { name: Person, kind: manyToOne, to: Person }\n" // + " - name: ClaimLine\n" // + " fields:\n" // + " - { name: id, type: integer, primaryKey: true, generated: true }\n" //