diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java index 70dc1572bd0..1f0ff64dd85 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/edm/EdmIntentGenerator.java @@ -2149,6 +2149,9 @@ private static List> buildChecks(EntityIntent entity, List + * The selector is rendered into the generated posting handler AS a Java literal, so a blank one + * emits {@code .eq("", "")} - a lookup that matches no rule row and therefore leaves every + * source document silently on the unposted worklist, with the intent, the generation and the + * publish all green. Refused here so the accident is named where it is authored (#7180). A value + * omitted outright ({@code documentType:} with nothing after it) never reaches this method: the + * typed mapping drops the null entry, so the empty selector is caught by the single-selector rule + * above. + * + * @param subject the message prefix naming the posting + * @param match the single-entry match selector + * @param issues collected validation issues + */ + private static void validateRuleMatchHasALiteral(String subject, java.util.Map match, List issues) { + Map.Entry selector = match.entrySet() + .iterator() + .next(); + Object value = selector.getValue(); + if (value == null || String.valueOf(value) + .isBlank()) { + issues.add(subject + " rule.match [" + selector.getKey() + + "] has no value - a determination rule selects on a literal, and an empty one matches no rule row"); + } + } + /** * The determination rule's {@code match} column must not be a translated one. The selector is a * literal authored in the model and compared against the rule row's own column, so the moment that @@ -7039,6 +7067,7 @@ private static void validatePostings(IntentModel model, Set usesAliases, issues.add(subject + " rule.match must be a single `column: literal` selector"); } else { validateRuleMatchIsNotTranslated(subject, ruleEntity, (java.util.Map) match, issues); + validateRuleMatchHasALiteral(subject, (java.util.Map) match, issues); } } // items 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 848c261d494..dd44ffd53fb 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 @@ -593,7 +593,10 @@ field may declare: references, arithmetic over the SOURCE's fields, or - for a to-one relation cell - a bare SOURCE relation name whose FK is copied onto the line; a row `when` is ` ==|!= `. A missing rule row or null referenced column SKIPS the posting (the unposted worklist = final-status - documents with no back-referencing target), never throws. + documents with no back-referencing target), never throws. `rule.match` is a single + `column: literal` selector and the literal must be there - an empty one is refused at parse, because + it is rendered into the handler as the authored literal and would select no rule row at all, leaving + every source document on the worklist with nothing failing anywhere. **Conditional rule column** - when the account must be chosen by a source value (a payment posts to the bank account for a transfer, the cash account for cash), a single row selects the rule column by a classifier instead of duplicating the row per case (the `by`/`cases`/`default` shape the 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 b81d2f5e1e4..ea91faf7576 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 @@ -874,6 +874,9 @@ void guardCheckEmitsKeyedAggregateGuard() { assertEquals("0", guard.get("minimum")); assertEquals("Insufficient stock", guard.get("message")); assertEquals("INVENTORY_BLOCK_NEGATIVE_STOCK", guard.get("enabledBy")); + // The guard names the aggregate it protects, so the skip it logs for a row that belongs to no + // key-tuple can be traced back to a declaration (#7180). + assertEquals("onHand", guard.get("aggregate")); List> keys = (List>) guard.get("keys"); assertEquals(2, keys.size()); assertEquals("Product", keys.get(0) 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 67d86509886..712b5951de3 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 @@ -1227,6 +1227,35 @@ void conditionalRuleUnknownClassifierIsRejected() { "expected an unknown-classifier issue, got: " + ex.getIssues()); } + /** + * A determination rule's {@code match} needs a literal that says something (#7180). + * + *

+ * The value is rendered into the generated posting handler AS an authored Java literal, so a blank + * one emits a lookup on the empty string - it matches no rule row, and every source document is + * left silently on the unposted worklist with the parse, the generation and the publish all green. + * A value omitted outright is already refused as an empty selector (the typed mapping drops a null + * entry, so the selector is not there at all); both readings now fail where they are authored. + */ + @Test + void postingRuleMatchWithNoLiteralIsRejected() { + String item = "{ Account: rule(BankAccount), debit: \"Amount\" }"; + String blank = conditionalRulePosting(item).replace("match: { documentType: \"Payment\" }", "match: { documentType: \"\" }"); + IntentValidationException blankEx = assertThrows(IntentValidationException.class, () -> IntentParser.parse(blank)); + assertTrue(blankEx.getIssues() + .stream() + .anyMatch(i -> i.contains("rule.match [documentType] has no value")), + "expected a blank rule.match issue, got: " + blankEx.getIssues()); + String omitted = conditionalRulePosting(item).replace("match: { documentType: \"Payment\" }", "match: { documentType: }"); + IntentValidationException omittedEx = assertThrows(IntentValidationException.class, () -> IntentParser.parse(omitted)); + assertTrue(omittedEx.getIssues() + .stream() + .anyMatch(i -> i.contains("rule.match must be a single `column: literal` selector")), + "expected an empty-selector issue, got: " + omittedEx.getIssues()); + // ...and the authored literal still parses, so nothing written before this changes. + IntentParser.parse(conditionalRulePosting(item)); + } + /** The event declares exactly one trigger - onTransition XOR onCreate. */ @Test void postingEventDeclaresExactlyOneTrigger() { 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 9bee867b124..511ae0f4e1f 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 @@ -178,7 +178,8 @@ package gen.${javaGenFolderName}.data.${javaPerspectiveName}; ## Aggregate guard (intent `checks: kind: guard`): reject a create/update whose post-state would drop ## a keyed aggregate sum below its minimum (negative stock, credit limit). Recomputed synchronously from ## THIS entity's store for the incoming row's key-tuple (race-free, not the async-maintained target), -## excluding this row on update, then adding the incoming value. Optionally gated by a config key. +## excluding this row on update, then adding the incoming value. Optionally gated by a config key. A row +## with any grouping key null belongs to no key-tuple and is not guarded at all (#7180). #macro(aggregateGuardCheck) #if($guardChecks && $guardChecks.size() > 0) #foreach($guard in $guardChecks) @@ -186,17 +187,32 @@ package gen.${javaGenFolderName}.data.${javaPerspectiveName}; if ("true".equalsIgnoreCase(org.eclipse.dirigible.sdk.core.Configurations.get("${guard.enabledBy}", ""))) { #end { - java.math.BigDecimal guardSum = java.math.BigDecimal.ZERO; - for (${name}Entity aggRow : findAll(Criteria.create()#foreach($k in $guard.keys).eq("${k.key}", entity.${k.key})#end)) { - if (entity.${guard.pk} != null && entity.${guard.pk}.equals(aggRow.${guard.pk})) { - continue; - } - if (aggRow.${guard.sumField} != null) { - guardSum = guardSum.add(aggRow.${guard.sumField}); + // A row with any grouping key null belongs to NO key-tuple, and the aggregate this guards + // ignores it for exactly that reason (the generated aggregate handler's own contract), so + // it contributes to no sum and can breach no minimum: there is nothing to guard and the + // row passes. Said explicitly, because the lookup below no longer says it by accident - + // Criteria.eq is NULL-SAFE (#7134), so a null key that used to match no row now matches + // the null group, weighing the write against a pool of tuple-less rows that no aggregate + // row materialises (#7180). The sibling roll-up capacity guard skips on a null FK the + // same way. + boolean guardKeyed = #foreach($k in $guard.keys)#if($foreach.first)entity.${k.key} != null#else && entity.${k.key} != null#end#end; + boolean guardWithin = true; + if (guardKeyed) { + java.math.BigDecimal guardSum = java.math.BigDecimal.ZERO; + for (${name}Entity aggRow : findAll(Criteria.create()#foreach($k in $guard.keys).eq("${k.key}", entity.${k.key})#end)) { + if (entity.${guard.pk} != null && entity.${guard.pk}.equals(aggRow.${guard.pk})) { + continue; + } + if (aggRow.${guard.sumField} != null) { + guardSum = guardSum.add(aggRow.${guard.sumField}); + } } + java.math.BigDecimal guardIncoming = entity.${guard.sumField} == null ? java.math.BigDecimal.ZERO : entity.${guard.sumField}; + guardWithin = guardSum.add(guardIncoming).compareTo(new java.math.BigDecimal("${guard.minimum}")) >= 0; + } else { + LOG.debug("Aggregate guard [${guard.aggregate}] skipped for a ${name} with a null grouping key" + + " - the row belongs to no key-tuple of the aggregate and contributes to no sum"); } - java.math.BigDecimal guardIncoming = entity.${guard.sumField} == null ? java.math.BigDecimal.ZERO : entity.${guard.sumField}; - boolean guardWithin = guardSum.add(guardIncoming).compareTo(new java.math.BigDecimal("${guard.minimum}")) >= 0; #if($guard.outcome == "task") // outcome: task - do NOT fail the write. Stamp the marker so this entity's process decision // can route the record to a hold/review step (the order is accepted, then parked). @@ -258,7 +274,7 @@ import org.eclipse.dirigible.sdk.utils.Calc; #if($haveCalculatedPropertyAction) import org.eclipse.dirigible.sdk.component.Beans; #end -#if($reportedOnUpdate.size() > 0) +#if($reportedOnUpdate.size() > 0 || ($guardChecks && $guardChecks.size() > 0)) import org.eclipse.dirigible.sdk.log.Logger; import org.eclipse.dirigible.sdk.log.Logging; #end @@ -305,8 +321,11 @@ ${importsCode} @Component("${javaGenFolderName}_${name}Repository") public class ${name}Repository extends JavaRepository<${name}Entity> { -#if($reportedOnUpdate.size() > 0) - /** Reports a system-owned column a full-row update() carried a different value for - see update(). */ +#if($reportedOnUpdate.size() > 0 || ($guardChecks && $guardChecks.size() > 0)) + /** + * Reports a system-owned column a full-row update() carried a different value for - see update() - + * and an aggregate guard skipped for a row that belongs to no key-tuple - see save(). + */ private static final Logger LOG = Logging.getLogger("gen.${javaGenFolderName}.data.${javaPerspectiveName}.${name}Repository"); #end diff --git a/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Posting.java.template b/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Posting.java.template index 9ecae8b7c18..23695ded056 100644 --- a/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Posting.java.template +++ b/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Posting.java.template @@ -76,6 +76,9 @@ public class ${className}Posting implements MessageHandler { #if($hasRule) // Determination rule first (the item derivation depends on it): a missing rule or a null // referenced column SKIPS - the document stays on the unposted worklist, never half-posted. + // The match value is the AUTHORED literal of rule.match, not a value read off the source, so it + // is never null here and the NULL-SAFE Criteria.eq (#7134) cannot turn this lookup into a match + // on the rule table's null group; an empty authored literal is refused at parse (#7180). java.util.List ruleRows = new gen.${javaGenFolderName}.data.${ruleJavaPerspective}.${ruleEntity}Repository().findAll( Criteria.create().eq("${ruleMatchProperty}", ${ruleMatchValueJava})); 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 13a0f2faf07..9043da2aac6 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 @@ -2320,6 +2320,13 @@ private void assertEmission() { // both PERSIST the row and mark it instead of throwing. assertTrue(ledgerRepository.contains("Criteria.create().eq(\"Person\", entity.Person).eq(\"Unit\", entity.Unit)"), "a guard must recompute its aggregate over the incoming row's full key-tuple"); + // ...and only for a row that HAS a full key-tuple. Criteria.eq is null-safe (#7134), so a null + // key no longer matches nothing - it matches the null group, a pool of tuple-less rows that the + // aggregate handler ignores by contract and materialises no target row for (#7180). + assertTrue(ledgerRepository.contains("boolean guardKeyed = entity.Person != null && entity.Unit != null;"), + "a guard must test every grouping key for null before it recomputes: " + ledgerRepository); + assertTrue(ledgerRepository.contains("boolean guardWithin = true;") && ledgerRepository.contains("if (guardKeyed) {"), + "a row belonging to no key-tuple must pass the guard untouched - no throw, no marker, no forced status"); assertTrue(ledgerRepository.contains("throw new ValidationException(\"Insufficient balance\")"), "outcome block must fail the write with the authored message"); assertTrue(ledgerRepository.contains("Configurations.get(\"EMISSION_BLOCK_NEGATIVE_LEDGER\""),