Copilot commented on code in PR #7160:
URL: https://github.com/apache/incubator-kie/pull/7160#discussion_r4409610839
##########
drools-model/drools-canonical-model/src/main/java/org/drools/model/impl/ViewPatternBuilder.java:
##########
@@ -194,6 +219,235 @@ public static Condition ruleItem2Condition(RuleItem
ruleItem) {
throw new UnsupportedOperationException( "Unknown " + ruleItem );
}
+ private static boolean hasPositiveTrigger(SequenceStep step) {
+ if (step instanceof ExistentialExprViewItem) {
+ ExistentialExprViewItem ex = (ExistentialExprViewItem) step;
+ if (ex.getType() == Condition.Type.NOT) {
+ return false;
+ }
+ return true;
+ }
+ if (step instanceof CombinedExprViewItem) {
+ CombinedExprViewItem comb = (CombinedExprViewItem) step;
+ Condition.Type type = comb.getType();
+ if (type == Condition.Type.NOT || type == Condition.Type.NOR
+ || type == Condition.Type.XOR || type ==
Condition.Type.XNOR) {
+ return false;
+ }
+ if (type == Condition.Type.AND) {
+ // In an AND composite, at least one child must be a positive
trigger
+ for (org.drools.model.view.ViewItem expr :
comb.getExpressions()) {
+ if (expr instanceof SequenceStep) {
+ if (hasPositiveTrigger((SequenceStep) expr)) {
+ return true;
+ }
+ } else {
+ return true;
+ }
+ }
+ return false;
+ }
+ if (type == Condition.Type.OR) {
+ // In an OR composite, all branches must have a positive
trigger
+ for (org.drools.model.view.ViewItem expr :
comb.getExpressions()) {
+ if (expr instanceof SequenceStep) {
+ if (!hasPositiveTrigger((SequenceStep) expr)) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+ }
+ return true;
+ }
+
+ private static boolean isAbsenceStep(SequenceStep step) {
+ if (step instanceof ExistentialExprViewItem) {
+ return ((ExistentialExprViewItem) step).getType() ==
Condition.Type.NOT;
+ }
+ if (step instanceof CombinedExprViewItem) {
+ Condition.Type type = ((CombinedExprViewItem) step).getType();
+ return type == Condition.Type.NOT || type == Condition.Type.NOR;
+ }
+ throw new IllegalStateException("Unexpected SequenceStep type: " +
step.getClass());
+ }
+
+ private static void
validateActivationMatching(org.drools.model.view.ViewItem item) {
+ if (item instanceof CombinedExprViewItem) {
+ CombinedExprViewItem comb = (CombinedExprViewItem) item;
+ Condition.Type type = comb.getType();
+ for (org.drools.model.view.ViewItem expr : comb.getExpressions()) {
+ validateActivationMatching(expr);
+ }
+ if (type == Condition.Type.XOR) {
+ int matchedChildren = 0;
+ for (org.drools.model.view.ViewItem expr :
comb.getExpressions()) {
+ if (matchesOnActivation(expr)) {
+ matchedChildren++;
+ }
+ }
+ if (matchedChildren >= 2) {
+ throw new IllegalArgumentException(
+ "sequence(): oneOf() has " + matchedChildren + "
children that match on step activation (0 inputs), " +
+ "causing an immediate over-match veto upon activation.
oneOf() requires exactly one child to match.");
+ }
+ }
+ }
+ }
+
+ private static boolean matchesOnActivation(org.drools.model.view.ViewItem
item) {
+ if (item instanceof CombinedExprViewItem) {
+ CombinedExprViewItem comb = (CombinedExprViewItem) item;
+ Condition.Type type = comb.getType();
+ if (type == Condition.Type.XNOR) {
+ int matched = 0;
+ int total = comb.getExpressions().length;
+ for (org.drools.model.view.ViewItem expr :
comb.getExpressions()) {
+ if (matchesOnActivation(expr)) {
+ matched++;
+ }
+ }
+ return matched == 0 || matched == total;
+ }
+ if (type == Condition.Type.XOR) {
+ int matched = 0;
+ for (org.drools.model.view.ViewItem expr :
comb.getExpressions()) {
+ if (matchesOnActivation(expr)) {
+ matched++;
+ }
+ }
+ return matched == 1;
+ }
+ if (type == Condition.Type.AND) {
+ for (org.drools.model.view.ViewItem expr :
comb.getExpressions()) {
+ if (!matchesOnActivation(expr)) {
+ return false;
+ }
+ }
+ return comb.getExpressions().length > 0;
+ }
+ if (type == Condition.Type.OR) {
+ for (org.drools.model.view.ViewItem expr :
comb.getExpressions()) {
+ if (matchesOnActivation(expr)) {
+ return true;
+ }
+ }
+ return false;
+ }
+ if (type == Condition.Type.NOT || type == Condition.Type.NOR) {
+ return false;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Rewrites runs of revertible gate steps (such as {@code oneOf()} / XOR
and {@code noneOrAll()} / XNOR)
+ * by folding them together with their immediately following positive
trigger into an {@code and(...)}
+ * composite step.
+ * <p>
+ * Standalone XOR/XNOR gates are non-propagating on their own within the
sequence pipeline and require
+ * a following positive pattern step to act as the activation trigger.
This transformation:
+ * <ul>
+ * <li>Separates and emits any interleaved absence guards ({@code
NOT}/{@code NOR}/{@code absent()})
+ * before the folded composite so that compiler accumulation loops
build them as independent
+ * veto side-channels.</li>
+ * <li>Folds consecutive XOR/XNOR gate steps and the subsequent positive
trigger step into a single
+ * {@link CombinedExprViewItem} of type {@link
Condition.Type#AND}.</li>
+ * </ul>
+ *
+ * @param steps the original sequence steps
+ * @return the rewritten sequence steps with folded gate composites
+ */
+ private static SequenceStep[] rewriteRevertingGateSteps(SequenceStep[]
steps) {
+ boolean hasXor = false;
+ for (int i = 0; i < steps.length; i++) {
+ SequenceStep step = steps[i];
+ if (step instanceof CombinedExprViewItem) {
+ Condition.Type t = ((CombinedExprViewItem) step).getType();
+ if ((t == Condition.Type.XOR || t == Condition.Type.XNOR) && i
+ 1 < steps.length) {
Review Comment:
Only bare XOR/XNOR steps are normalized, so `sequence(start,
and(oneOf(a,b)), ack)` remains three runtime steps. Matching `a` completes the
middle step and unregisters `b`, so `start, a, b, ack` fires instead of vetoing
the attempt. Fold guard-only composites into their following positive trigger,
or reject them at intermediate positions as well as at the end.
##########
drools-base/src/main/java/org/drools/base/reteoo/sequencing/steps/LogicCircuitStep.java:
##########
@@ -38,7 +38,7 @@ public LogicCircuit getCircuit() {
public void activate(SequenceMemory sequenceMemory, ValueResolver
valueResolver) {
for (LogicGate gate : circuit.getGates()) {
- gate.activate(sequenceMemory);
+ gate.activate(sequenceMemory, valueResolver);
Review Comment:
`gate.activate()` can synchronously advance the sequence, but this loop then
activates remaining gates from the consumed step. For `sequence(start,
or(noneOrAll(a,b), other), ack)`, activation advances to `ack` and then
re-registers `other`, allowing an `other` event to complete the sequence
without an acknowledgement. Stop activating the old circuit when the current
step changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
