This is an automated email from the ASF dual-hosted git repository.
sergehuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/master by this push:
new beea0b48a UNOMI-1004: Omit unset optional branches from boolean OR
(#919)
beea0b48a is described below
commit beea0b48aad19363170aadba323570a9a354c8fc
Author: Serge Huber <[email protected]>
AuthorDate: Fri Oct 16 14:08:28 2026 +0200
UNOMI-1004: Omit unset optional branches from boolean OR (#919)
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
.../apache/unomi/itests/ConditionEvaluatorIT.java | 19 +++
.../src/main/asciidoc/builtin-condition-types.adoc | 10 +-
manual/src/main/asciidoc/conditions-guide.adoc | 6 +
.../asciidoc/migrations/migrate-3.0-to-4.0.adoc | 31 ++++
manual/src/main/asciidoc/writing-plugins.adoc | 11 ++
persistence-elasticsearch/core/pom.xml | 5 +
.../core/BooleanConditionESQueryBuilder.java | 44 ++++--
.../resources/OSGI-INF/blueprint/blueprint.xml | 4 +-
.../core/BooleanConditionESQueryBuilderTest.java | 165 ++++++++++++++++++++
.../core/BooleanConditionOSQueryBuilder.java | 44 ++++--
.../resources/OSGI-INF/blueprint/blueprint.xml | 4 +-
.../core/BooleanConditionOSQueryBuilderTest.java | 168 +++++++++++++++++++++
.../spi/conditions/ConditionContextHelper.java | 78 ++++++++++
.../spi/conditions/ConditionContextHelperTest.java | 70 +++++++++
plugins/baseplugin/pom.xml | 5 +
.../conditions/BooleanConditionEvaluator.java | 27 +++-
.../resources/OSGI-INF/blueprint/blueprint.xml | 1 +
.../conditions/BooleanConditionEvaluatorTest.java | 103 +++++++++++++
.../BooleanConditionOptionalParameterTest.java | 121 +++++++++++++++
plugins/hover-event/pom.xml | 89 +++++++++++
.../hoverevent/HoverEventConditionTest.java | 134 ++++++++++++++++
.../services/impl/TestConditionEvaluators.java | 26 ++--
22 files changed, 1131 insertions(+), 34 deletions(-)
diff --git
a/itests/src/test/java/org/apache/unomi/itests/ConditionEvaluatorIT.java
b/itests/src/test/java/org/apache/unomi/itests/ConditionEvaluatorIT.java
index 75f770120..52cacc72c 100644
--- a/itests/src/test/java/org/apache/unomi/itests/ConditionEvaluatorIT.java
+++ b/itests/src/test/java/org/apache/unomi/itests/ConditionEvaluatorIT.java
@@ -109,6 +109,25 @@ public class ConditionEvaluatorIT extends BaseIT {
assertFalse(eval(builder.not(builder.profileProperty("properties.age").equalTo(30)).build()));
}
+ // UNOMI-1004: a branch whose parameter reference is unset is omitted.
Under OR it must not
+ // match everything; under AND it states no constraint (UNOMI-982).
+ @Test
+ public void testCompoundWithUnsetParameter() {
+ ConditionBuilder.ConditionItem unsetGender =
builder.profileProperty("properties.gender").equalTo("parameter::unsetGender");
+ ConditionBuilder.ConditionItem unsetAge =
builder.profileProperty("properties.age").equalTo("parameter::unsetAge");
+
+ assertFalse(eval(builder.or(unsetGender,
builder.profileProperty("properties.age").equalTo(40)).build()));
+ assertTrue(eval(builder.or(unsetGender,
builder.profileProperty("properties.age").equalTo(30)).build()));
+ assertFalse(eval(builder.or(unsetGender, unsetAge).build()));
+
+ assertTrue(eval(builder.and(unsetGender,
builder.profileProperty("properties.age").equalTo(30)).build()));
+ assertFalse(eval(builder.and(unsetGender,
builder.profileProperty("properties.age").equalTo(40)).build()));
+ assertTrue(eval(builder.and(unsetGender, unsetAge).build()));
+ // An AND with nothing left to test states no constraint, so an OR
holding it matches (as in 2.x and 3.x).
+ assertTrue(eval(builder.or(builder.and(unsetGender, unsetAge),
+
builder.profileProperty("properties.age").equalTo(40)).build()));
+ }
+
@Test
public void testDate() {
assertTrue(eval(builder.profileProperty("properties.lastVisit").equalTo(lastVisit).build()));
diff --git a/manual/src/main/asciidoc/builtin-condition-types.adoc
b/manual/src/main/asciidoc/builtin-condition-types.adoc
index b4e56a321..7f6e6f5cc 100644
--- a/manual/src/main/asciidoc/builtin-condition-types.adoc
+++ b/manual/src/main/asciidoc/builtin-condition-types.adoc
@@ -138,13 +138,21 @@ Use the same `queryBuilderId` in JSON, Elasticsearch, and
OpenSearch registratio
|===
| Condition type ID | Source | Key parameters / notes
| `cdpSessionEventCondition` | graphql | Params: —
-| `hoverEventCondition` | hover-event | Params: targetId, targetPath
+| `hoverEventCondition` | hover-event (not deployed by default) | Matches a
hover on the given target id or page path. Params: targetId, targetPath (set at
least one; with neither set, it matches nothing)
| `sourceEventPropertyCondition` | advanced-conditions | Params: id, path,
scope, type
| `updateConsentEventCondition` | graphql | Params: —
| `updateListsEventCondition` | graphql | Params: —
| `userListPropertyCondition` | graphql | Params: propertyName,
comparisonOperator, propertyValue, propertyValues
|===
+The hover-event plugin is built with Unomi but is not part of any Karaf
feature, so `hoverEventCondition` is not
+available on a default install. Deploy the bundle to use it, for example from
the Karaf console:
+
+[source]
+----
+bundle:install -s
mvn:org.apache.unomi/unomi-plugins-hover-event/${project.version}
+----
+
==== Source locations
Condition descriptors are JSON files under `META-INF/cxs/conditions/` in each
plugin, for example:
diff --git a/manual/src/main/asciidoc/conditions-guide.adoc
b/manual/src/main/asciidoc/conditions-guide.adoc
index f6e8b871c..06473960b 100644
--- a/manual/src/main/asciidoc/conditions-guide.adoc
+++ b/manual/src/main/asciidoc/conditions-guide.adoc
@@ -194,6 +194,12 @@ Combine conditions with `booleanCondition`. This is the
main way to build segmen
}
----
+An empty `subConditions` list matches every item in a search query. In-memory
evaluation, as in rules, matches
+every item for an empty AND and none for an empty OR. To select every item,
use `matchAllCondition` rather than
+an empty list. A sub-condition whose `parameter::` reference is unset is
+left out before the operator applies; see <<_writing_plugins,Writing plugins>>
for how condition types use
+these references.
+
Negate a sub-condition with `notCondition`:
[source,json]
diff --git a/manual/src/main/asciidoc/migrations/migrate-3.0-to-4.0.adoc
b/manual/src/main/asciidoc/migrations/migrate-3.0-to-4.0.adoc
index 4f7ae942a..31e3ba86b 100644
--- a/manual/src/main/asciidoc/migrations/migrate-3.0-to-4.0.adoc
+++ b/manual/src/main/asciidoc/migrations/migrate-3.0-to-4.0.adoc
@@ -266,6 +266,37 @@ This affects every endpoint that takes a `Query`, among
them `/cxs/profiles/sear
* A client that omits the `condition` field entirely still searches over
everything, and it needs no change.
* A client whose condition names a type that the server does not know now
reads an empty list where it used to read every item. Check the server log for
the condition type it names, and deploy the plugin that defines it.
+==== An unset optional parameter no longer makes an OR match (4.0)
+
+A condition parameter that reads `parameter::name` is optional: when nothing
supplies `name`, the branch that
+holds it states no constraint and is left out. 3.0 left it out of an AND, and
4.0 now also leaves it out of
+an OR instead of counting it as a match. A `hoverEventCondition` that sets
only `targetId`, for example, used
+to match every hover event, and now matches only hovers on that target. That
condition comes from the
+hover-event plugin, which is not deployed by default.
+
+.What changed
+[cols="1,2,2", options="header"]
+|===
+| Condition | 3.0 | 4.0
+
+| An OR with a branch whose parameter is unset
+| Matched every item.
+| The branch is left out, and the OR matches on its other branches.
+
+| An OR whose branches are all unset
+| Matched every item.
+| Matches no item.
+|===
+
+An OR whose `subConditions` list is empty is unchanged: it matches every item
when run as a search query, and no
+item when evaluated in memory, as a rule is.
+
+===== Migrating a client
+
+* An OR of optional parameters needs at least one of them set to match
anything.
+* An AND whose branches are all unset still states no constraint and matches
every item, as in 3.0, also
+ when it sits inside an OR.
+
[#_segment_and_scoring_lists]
==== Segment and scoring lists merge system and tenant items (4.0)
diff --git a/manual/src/main/asciidoc/writing-plugins.adoc
b/manual/src/main/asciidoc/writing-plugins.adoc
index 6cccd07f3..dea9d1b1a 100644
--- a/manual/src/main/asciidoc/writing-plugins.adoc
+++ b/manual/src/main/asciidoc/writing-plugins.adoc
@@ -289,6 +289,17 @@ Specialize an existing condition type with
`parentCondition` under `META-INF/cxs
}
----
+Inside `parentCondition`, a value written `parameter::name` takes the value
the condition instance gives its
+`name` parameter. A parameter the instance leaves unset is optional, and the
condition that reads it is left out:
+
+* Under an AND, it states no constraint. `formEventCondition` with no
`pagePath` matches a form on any page.
+* Under an OR, it is not a match. `hoverEventCondition` compares `targetId` OR
`targetPath`, and with only
+ `targetId` set it matches hovers on that target only.
+* An OR whose branches are all unset matches nothing, and an AND whose
branches are all unset matches everything.
+* A condition instance whose own parameter is unset is left out as a whole,
constraints of its type included.
+
+These rules apply in both rule evaluation and segment queries.
+
===== Predefined personas
Place JSON under `META-INF/cxs/personas/`. Each file can define a persona
profile and optional sessions for testing personalization.
diff --git a/persistence-elasticsearch/core/pom.xml
b/persistence-elasticsearch/core/pom.xml
index 62faf25f0..2df8c1490 100644
--- a/persistence-elasticsearch/core/pom.xml
+++ b/persistence-elasticsearch/core/pom.xml
@@ -130,6 +130,11 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.apache.lucene</groupId>
<artifactId>lucene-test-framework</artifactId>
diff --git
a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/querybuilders/core/BooleanConditionESQueryBuilder.java
b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/querybuilders/core/BooleanConditionESQueryBuilder.java
index 36ae5b650..0542b2952 100644
---
a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/querybuilders/core/BooleanConditionESQueryBuilder.java
+++
b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/querybuilders/core/BooleanConditionESQueryBuilder.java
@@ -22,6 +22,8 @@ import
co.elastic.clients.elasticsearch._types.query_dsl.Query;
import org.apache.unomi.api.conditions.Condition;
import org.apache.unomi.persistence.elasticsearch.ConditionESQueryBuilder;
import
org.apache.unomi.persistence.elasticsearch.ConditionESQueryBuilderDispatcher;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
+import org.apache.unomi.scripting.ScriptExecutor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -35,6 +37,12 @@ public class BooleanConditionESQueryBuilder implements
ConditionESQueryBuilder {
private static final Logger LOGGER =
LoggerFactory.getLogger(BooleanConditionESQueryBuilder.class.getName());
+ private ScriptExecutor scriptExecutor;
+
+ public void setScriptExecutor(ScriptExecutor scriptExecutor) {
+ this.scriptExecutor = scriptExecutor;
+ }
+
@Override
public Query buildQuery(Condition condition, Map<String, Object> context,
ConditionESQueryBuilderDispatcher dispatcher) {
@@ -42,16 +50,34 @@ public class BooleanConditionESQueryBuilder implements
ConditionESQueryBuilder {
@SuppressWarnings("unchecked")
List<Condition> conditions = (List<Condition>)
condition.getParameter("subConditions");
- int conditionCount = conditions.size();
+ // An empty list matches everything, for AND and OR alike, as in 2.x
and 3.x. Callers rely on it,
+ // for example EventService.searchEvents with no event types.
+ if (conditions == null || conditions.isEmpty()) {
+ return Query.of(q -> q.matchAll(m -> m));
+ }
+
+ // Omit branches whose optional parameter is unset (UNOMI-1004 for OR;
UNOMI-982 for AND).
+ ConditionContextHelper.ResolvedBooleanSubConditions resolved =
+ ConditionContextHelper.resolveBooleanSubConditions(conditions,
context, scriptExecutor);
+ if (resolved.hasUnresolvable() && isAndOperator) {
+ return Query.of(q -> q.matchNone(m -> m));
+ }
+ List<Condition> active = resolved.getActive();
+
+ if (active.isEmpty()) {
+ return isAndOperator
+ ? Query.of(q -> q.matchAll(m -> m))
+ : Query.of(q -> q.matchNone(m -> m));
+ }
- if (conditionCount == 1) {
- return dispatcher.buildFilter(conditions.get(0), context);
+ if (active.size() == 1) {
+ return dispatcher.buildFilter(active.get(0), context);
}
BoolQuery.Builder boolQueryBuilder = new BoolQuery.Builder();
- for (int i = 0; i < conditionCount; i++) {
+ for (Condition sub : active) {
if (isAndOperator) {
- Query andFilter = dispatcher.buildFilter(conditions.get(i),
context);
+ Query andFilter = dispatcher.buildFilter(sub, context);
if (andFilter != null) {
if (andFilter.isRange()) {
boolQueryBuilder.filter(andFilter);
@@ -61,22 +87,22 @@ public class BooleanConditionESQueryBuilder implements
ConditionESQueryBuilder {
} else {
LOGGER.warn("Null filter for boolean AND sub condition.
See debug log level for more information");
if (LOGGER.isDebugEnabled()) {
- LOGGER.debug("Null filter for boolean AND sub
condition {}", conditions.get(i));
+ LOGGER.debug("Null filter for boolean AND sub
condition {}", sub);
}
}
} else {
- Query orFilter = dispatcher.buildFilter(conditions.get(i),
context);
+ Query orFilter = dispatcher.buildFilter(sub, context);
if (orFilter != null) {
boolQueryBuilder.should(orFilter);
} else {
LOGGER.warn("Null filter for boolean OR sub condition. See
debug log level for more information");
if (LOGGER.isDebugEnabled()) {
- LOGGER.debug("Null filter for boolean OR sub condition
{}", conditions.get(i));
+ LOGGER.debug("Null filter for boolean OR sub condition
{}", sub);
}
}
}
}
- return Query.of(q->q.bool(boolQueryBuilder.build()));
+ return Query.of(q -> q.bool(boolQueryBuilder.build()));
}
}
diff --git
a/persistence-elasticsearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
b/persistence-elasticsearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
index 3df1b5b6e..59f015420 100644
---
a/persistence-elasticsearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
+++
b/persistence-elasticsearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
@@ -194,7 +194,9 @@
<service-properties>
<entry key="queryBuilderId" value="booleanConditionQueryBuilder"/>
</service-properties>
- <bean
class="org.apache.unomi.persistence.elasticsearch.querybuilders.core.BooleanConditionESQueryBuilder"/>
+ <bean
class="org.apache.unomi.persistence.elasticsearch.querybuilders.core.BooleanConditionESQueryBuilder">
+ <property name="scriptExecutor" ref="scriptExecutor"/>
+ </bean>
</service>
<service
diff --git
a/persistence-elasticsearch/core/src/test/java/org/apache/unomi/persistence/elasticsearch/querybuilders/core/BooleanConditionESQueryBuilderTest.java
b/persistence-elasticsearch/core/src/test/java/org/apache/unomi/persistence/elasticsearch/querybuilders/core/BooleanConditionESQueryBuilderTest.java
new file mode 100644
index 000000000..854ca915f
--- /dev/null
+++
b/persistence-elasticsearch/core/src/test/java/org/apache/unomi/persistence/elasticsearch/querybuilders/core/BooleanConditionESQueryBuilderTest.java
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.persistence.elasticsearch.querybuilders.core;
+
+import co.elastic.clients.elasticsearch._types.query_dsl.Query;
+import org.apache.unomi.api.conditions.Condition;
+import
org.apache.unomi.persistence.elasticsearch.ConditionESQueryBuilderDispatcher;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public class BooleanConditionESQueryBuilderTest {
+
+ private final BooleanConditionESQueryBuilder builder = new
BooleanConditionESQueryBuilder();
+
+ @Mock
+ private ConditionESQueryBuilderDispatcher dispatcher;
+
+ @Before
+ public void setUp() {
+ when(dispatcher.buildFilter(any(Condition.class),
any())).thenReturn(Query.of(q -> q.matchAll(m -> m)));
+ }
+
+ @Test
+ public void or_neitherOptionalSet_returnsMatchNone() {
+ Query query = builder.buildQuery(hoverLikeOr(), new HashMap<>(),
dispatcher);
+ assertTrue(query.isMatchNone());
+ verify(dispatcher, never()).buildFilter(any(Condition.class), any());
+ }
+
+ @Test
+ public void or_onlyTargetIdSet_buildsSingleBranch() {
+ Map<String, Object> context = new HashMap<>();
+ context.put("targetId", "button-a");
+
+ builder.buildQuery(hoverLikeOr(), context, dispatcher);
+ ArgumentCaptor<Condition> built =
ArgumentCaptor.forClass(Condition.class);
+ verify(dispatcher, times(1)).buildFilter(built.capture(), any());
+ assertEquals("target.itemId",
built.getValue().getParameter("propertyName"));
+ assertEquals("button-a",
built.getValue().getParameter("propertyValue"));
+ }
+
+ @Test
+ public void and_allOptionalsUnset_returnsMatchAll() {
+ Condition and = new Condition();
+ and.setParameter("operator", "and");
+ and.setParameter("subConditions", Arrays.asList(
+ optionalEquals("target.itemId", "targetId"),
+ optionalEquals("target.properties.pageInfo.pagePath",
"targetPath")));
+
+ assertTrue(builder.buildQuery(and, new HashMap<>(),
dispatcher).isMatchAll());
+ verify(dispatcher, never()).buildFilter(any(Condition.class), any());
+ }
+
+ @Test
+ public void or_emptySubConditions_returnsMatchAll() {
+ // 2.x/3.x behaviour that callers rely on, e.g.
EventService.searchEvents with no event types.
+ Condition or = new Condition();
+ or.setParameter("operator", "or");
+ or.setParameter("subConditions", Collections.emptyList());
+
+ assertTrue(builder.buildQuery(or, new HashMap<>(),
dispatcher).isMatchAll());
+ }
+
+ @Test
+ public void and_unresolvableBranch_returnsMatchNone() {
+ Condition and = new Condition();
+ and.setParameter("operator", "and");
+ and.setParameter("subConditions",
Arrays.asList(literalEquals("eventType", "hover"), unresolvable()));
+
+ assertTrue(builder.buildQuery(and, new HashMap<>(),
dispatcher).isMatchNone());
+ verify(dispatcher, never()).buildFilter(any(Condition.class), any());
+ }
+
+ @Test
+ public void or_unresolvableBranch_isSkipped() {
+ Condition literal = literalEquals("eventType", "hover");
+ Condition or = new Condition();
+ or.setParameter("operator", "or");
+ or.setParameter("subConditions", Arrays.asList(unresolvable(),
literal));
+
+ builder.buildQuery(or, new HashMap<>(), dispatcher);
+ ArgumentCaptor<Condition> built =
ArgumentCaptor.forClass(Condition.class);
+ verify(dispatcher, times(1)).buildFilter(built.capture(), any());
+ assertEquals(literal, built.getValue());
+ }
+
+ @Test
+ public void and_unsetOptional_omitsBranchAndKeepsRequired() {
+ Condition required = literalEquals("eventType", "hover");
+ Condition optional = optionalEquals("target.itemId", "targetId");
+
+ Condition and = new Condition();
+ and.setParameter("operator", "and");
+ and.setParameter("subConditions", Arrays.asList(required, optional));
+
+ builder.buildQuery(and, new HashMap<>(), dispatcher);
+ verify(dispatcher, times(1)).buildFilter(eq(required), any());
+ verify(dispatcher, times(1)).buildFilter(any(Condition.class), any());
+ }
+
+ private static Condition hoverLikeOr() {
+ Condition or = new Condition();
+ or.setParameter("operator", "or");
+ or.setParameter("subConditions", Arrays.asList(
+ optionalEquals("target.itemId", "targetId"),
+ optionalEquals("target.properties.pageInfo.pagePath",
"targetPath")));
+ return or;
+ }
+
+ private static Condition literalEquals(String propertyName, String value) {
+ Condition c = new Condition();
+ c.setParameter("propertyName", propertyName);
+ c.setParameter("propertyValue", value);
+ c.setParameter("comparisonOperator", "equals");
+ return c;
+ }
+
+ // A script that cannot run (no script executor is set) makes the
condition unresolvable.
+ private static Condition unresolvable() {
+ Condition c = new Condition();
+ c.setParameter("propertyValue", "script::anything");
+ return c;
+ }
+
+ private static Condition optionalEquals(String propertyName, String
parameterName) {
+ Condition c = new Condition();
+ c.setParameter("propertyName", propertyName);
+ c.setParameter("propertyValue", "parameter::" + parameterName);
+ c.setParameter("comparisonOperator", "equals");
+ return c;
+ }
+}
diff --git
a/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/querybuilders/core/BooleanConditionOSQueryBuilder.java
b/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/querybuilders/core/BooleanConditionOSQueryBuilder.java
index b201e4fb4..ff9e36e15 100644
---
a/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/querybuilders/core/BooleanConditionOSQueryBuilder.java
+++
b/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/querybuilders/core/BooleanConditionOSQueryBuilder.java
@@ -20,6 +20,8 @@ package
org.apache.unomi.persistence.opensearch.querybuilders.core;
import org.apache.unomi.api.conditions.Condition;
import org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilder;
import
org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilderDispatcher;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
+import org.apache.unomi.scripting.ScriptExecutor;
import org.opensearch.client.opensearch._types.query_dsl.BoolQuery;
import org.opensearch.client.opensearch._types.query_dsl.Query;
import org.slf4j.Logger;
@@ -35,6 +37,12 @@ public class BooleanConditionOSQueryBuilder implements
ConditionOSQueryBuilder {
private static final Logger LOGGER =
LoggerFactory.getLogger(BooleanConditionOSQueryBuilder.class.getName());
+ private ScriptExecutor scriptExecutor;
+
+ public void setScriptExecutor(ScriptExecutor scriptExecutor) {
+ this.scriptExecutor = scriptExecutor;
+ }
+
@Override
public Query buildQuery(Condition condition, Map<String, Object> context,
ConditionOSQueryBuilderDispatcher dispatcher) {
@@ -42,16 +50,34 @@ public class BooleanConditionOSQueryBuilder implements
ConditionOSQueryBuilder {
@SuppressWarnings("unchecked")
List<Condition> conditions = (List<Condition>)
condition.getParameter("subConditions");
- int conditionCount = conditions.size();
+ // An empty list matches everything, for AND and OR alike, as in 2.x
and 3.x. Callers rely on it,
+ // for example EventService.searchEvents with no event types.
+ if (conditions == null || conditions.isEmpty()) {
+ return Query.of(q -> q.matchAll(m -> m));
+ }
+
+ // Omit branches whose optional parameter is unset (UNOMI-1004 for OR;
UNOMI-982 for AND).
+ ConditionContextHelper.ResolvedBooleanSubConditions resolved =
+ ConditionContextHelper.resolveBooleanSubConditions(conditions,
context, scriptExecutor);
+ if (resolved.hasUnresolvable() && isAndOperator) {
+ return Query.of(q -> q.matchNone(m -> m));
+ }
+ List<Condition> active = resolved.getActive();
+
+ if (active.isEmpty()) {
+ return isAndOperator
+ ? Query.of(q -> q.matchAll(m -> m))
+ : Query.of(q -> q.matchNone(m -> m));
+ }
- if (conditionCount == 1) {
- return dispatcher.buildFilter(conditions.get(0), context);
+ if (active.size() == 1) {
+ return dispatcher.buildFilter(active.get(0), context);
}
BoolQuery.Builder boolQueryBuilder = new BoolQuery.Builder();
- for (int i = 0; i < conditionCount; i++) {
+ for (Condition sub : active) {
if (isAndOperator) {
- Query andFilter = dispatcher.buildFilter(conditions.get(i),
context);
+ Query andFilter = dispatcher.buildFilter(sub, context);
if (andFilter != null) {
if (andFilter.isRange()) {
boolQueryBuilder.filter(andFilter);
@@ -61,22 +87,22 @@ public class BooleanConditionOSQueryBuilder implements
ConditionOSQueryBuilder {
} else {
LOGGER.warn("Null filter for boolean AND sub condition.
See debug log level for more information");
if (LOGGER.isDebugEnabled()) {
- LOGGER.debug("Null filter for boolean AND sub
condition {}", conditions.get(i));
+ LOGGER.debug("Null filter for boolean AND sub
condition {}", sub);
}
}
} else {
- Query orFilter = dispatcher.buildFilter(conditions.get(i),
context);
+ Query orFilter = dispatcher.buildFilter(sub, context);
if (orFilter != null) {
boolQueryBuilder.should(orFilter);
} else {
LOGGER.warn("Null filter for boolean OR sub condition. See
debug log level for more information");
if (LOGGER.isDebugEnabled()) {
- LOGGER.debug("Null filter for boolean OR sub condition
{}", conditions.get(i));
+ LOGGER.debug("Null filter for boolean OR sub condition
{}", sub);
}
}
}
}
- return Query.of(q->q.bool(boolQueryBuilder.build()));
+ return Query.of(q -> q.bool(boolQueryBuilder.build()));
}
}
diff --git
a/persistence-opensearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
b/persistence-opensearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
index 3255eb984..62a383310 100644
---
a/persistence-opensearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
+++
b/persistence-opensearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
@@ -192,7 +192,9 @@
<service-properties>
<entry key="queryBuilderId" value="booleanConditionQueryBuilder"/>
</service-properties>
- <bean
class="org.apache.unomi.persistence.opensearch.querybuilders.core.BooleanConditionOSQueryBuilder"/>
+ <bean
class="org.apache.unomi.persistence.opensearch.querybuilders.core.BooleanConditionOSQueryBuilder">
+ <property name="scriptExecutor" ref="scriptExecutor" />
+ </bean>
</service>
<service
diff --git
a/persistence-opensearch/core/src/test/java/org/apache/unomi/persistence/opensearch/querybuilders/core/BooleanConditionOSQueryBuilderTest.java
b/persistence-opensearch/core/src/test/java/org/apache/unomi/persistence/opensearch/querybuilders/core/BooleanConditionOSQueryBuilderTest.java
new file mode 100644
index 000000000..07cf96c7d
--- /dev/null
+++
b/persistence-opensearch/core/src/test/java/org/apache/unomi/persistence/opensearch/querybuilders/core/BooleanConditionOSQueryBuilderTest.java
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.persistence.opensearch.querybuilders.core;
+
+import org.apache.unomi.api.conditions.Condition;
+import
org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilderDispatcher;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
+import org.opensearch.client.opensearch._types.query_dsl.Query;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+@MockitoSettings(strictness = Strictness.LENIENT)
+public class BooleanConditionOSQueryBuilderTest {
+
+ private final BooleanConditionOSQueryBuilder builder = new
BooleanConditionOSQueryBuilder();
+
+ @Mock
+ private ConditionOSQueryBuilderDispatcher dispatcher;
+
+ @BeforeEach
+ public void setUp() {
+ when(dispatcher.buildFilter(any(Condition.class),
any())).thenReturn(Query.of(q -> q.matchAll(m -> m)));
+ }
+
+ @Test
+ public void or_neitherOptionalSet_returnsMatchNone() {
+ Query query = builder.buildQuery(hoverLikeOr(), new HashMap<>(),
dispatcher);
+ assertTrue(query.isMatchNone());
+ verify(dispatcher, never()).buildFilter(any(Condition.class), any());
+ }
+
+ @Test
+ public void or_onlyTargetIdSet_buildsSingleBranch() {
+ Map<String, Object> context = new HashMap<>();
+ context.put("targetId", "button-a");
+
+ builder.buildQuery(hoverLikeOr(), context, dispatcher);
+ ArgumentCaptor<Condition> built =
ArgumentCaptor.forClass(Condition.class);
+ verify(dispatcher, times(1)).buildFilter(built.capture(), any());
+ assertEquals("target.itemId",
built.getValue().getParameter("propertyName"));
+ assertEquals("button-a",
built.getValue().getParameter("propertyValue"));
+ }
+
+ @Test
+ public void and_allOptionalsUnset_returnsMatchAll() {
+ Condition and = new Condition();
+ and.setParameter("operator", "and");
+ and.setParameter("subConditions", Arrays.asList(
+ optionalEquals("target.itemId", "targetId"),
+ optionalEquals("target.properties.pageInfo.pagePath",
"targetPath")));
+
+ assertTrue(builder.buildQuery(and, new HashMap<>(),
dispatcher).isMatchAll());
+ verify(dispatcher, never()).buildFilter(any(Condition.class), any());
+ }
+
+ @Test
+ public void or_emptySubConditions_returnsMatchAll() {
+ // 2.x/3.x behaviour that callers rely on, e.g.
EventService.searchEvents with no event types.
+ Condition or = new Condition();
+ or.setParameter("operator", "or");
+ or.setParameter("subConditions", Collections.emptyList());
+
+ assertTrue(builder.buildQuery(or, new HashMap<>(),
dispatcher).isMatchAll());
+ }
+
+ @Test
+ public void and_unresolvableBranch_returnsMatchNone() {
+ Condition and = new Condition();
+ and.setParameter("operator", "and");
+ and.setParameter("subConditions",
Arrays.asList(literalEquals("eventType", "hover"), unresolvable()));
+
+ assertTrue(builder.buildQuery(and, new HashMap<>(),
dispatcher).isMatchNone());
+ verify(dispatcher, never()).buildFilter(any(Condition.class), any());
+ }
+
+ @Test
+ public void or_unresolvableBranch_isSkipped() {
+ Condition literal = literalEquals("eventType", "hover");
+ Condition or = new Condition();
+ or.setParameter("operator", "or");
+ or.setParameter("subConditions", Arrays.asList(unresolvable(),
literal));
+
+ builder.buildQuery(or, new HashMap<>(), dispatcher);
+ ArgumentCaptor<Condition> built =
ArgumentCaptor.forClass(Condition.class);
+ verify(dispatcher, times(1)).buildFilter(built.capture(), any());
+ assertEquals(literal, built.getValue());
+ }
+
+ @Test
+ public void and_unsetOptional_omitsBranchAndKeepsRequired() {
+ Condition required = literalEquals("eventType", "hover");
+ Condition optional = optionalEquals("target.itemId", "targetId");
+
+ Condition and = new Condition();
+ and.setParameter("operator", "and");
+ and.setParameter("subConditions", Arrays.asList(required, optional));
+
+ builder.buildQuery(and, new HashMap<>(), dispatcher);
+ verify(dispatcher, times(1)).buildFilter(eq(required), any());
+ verify(dispatcher, times(1)).buildFilter(any(Condition.class), any());
+ }
+
+ private static Condition hoverLikeOr() {
+ Condition or = new Condition();
+ or.setParameter("operator", "or");
+ or.setParameter("subConditions", Arrays.asList(
+ optionalEquals("target.itemId", "targetId"),
+ optionalEquals("target.properties.pageInfo.pagePath",
"targetPath")));
+ return or;
+ }
+
+ private static Condition literalEquals(String propertyName, String value) {
+ Condition c = new Condition();
+ c.setParameter("propertyName", propertyName);
+ c.setParameter("propertyValue", value);
+ c.setParameter("comparisonOperator", "equals");
+ return c;
+ }
+
+ // A script that cannot run (no script executor is set) makes the
condition unresolvable.
+ private static Condition unresolvable() {
+ Condition c = new Condition();
+ c.setParameter("propertyValue", "script::anything");
+ return c;
+ }
+
+ private static Condition optionalEquals(String propertyName, String
parameterName) {
+ Condition c = new Condition();
+ c.setParameter("propertyName", propertyName);
+ c.setParameter("propertyValue", "parameter::" + parameterName);
+ c.setParameter("comparisonOperator", "equals");
+ return c;
+ }
+}
diff --git
a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java
b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java
index b98500606..ca27ba4cc 100644
---
a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java
+++
b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java
@@ -816,4 +816,82 @@ public class ConditionContextHelper {
return null;
}
+ /**
+ * Result of resolving the sub-conditions of a boolean condition against a
context.
+ * Unset optional parameters are omitted from {@link #getActive()};
unresolvable
+ * references are omitted too and reported via {@link #hasUnresolvable()},
so AND can fail
+ * closed while OR refuses only that branch's match.
+ */
+ public static final class ResolvedBooleanSubConditions {
+ private final List<Condition> active;
+ private final boolean unresolvable;
+
+ ResolvedBooleanSubConditions(List<Condition> active, boolean
unresolvable) {
+ this.active = active;
+ this.unresolvable = unresolvable;
+ }
+
+ /**
+ * Sub-conditions that still apply after omitting unset optional
parameters.
+ *
+ * @return the active sub-conditions; never {@code null}
+ */
+ public List<Condition> getActive() {
+ return active;
+ }
+
+ /**
+ * {@code true} when at least one sub-condition could not be resolved
+ * (cycle, depth overrun, or script failure).
+ *
+ * @return whether an unresolvable sub-condition was seen
+ */
+ public boolean hasUnresolvable() {
+ return unresolvable;
+ }
+ }
+
+ /**
+ * Resolves each boolean sub-condition against the context.
+ * A sub-condition whose optional parameter is unset ({@link
#getContextualCondition}
+ * returns {@code null}) is left out. That is required for OR: an unset
branch must not
+ * count as a match. For AND, leaving it out preserves the UNOMI-982
behaviour where an
+ * unset optional states no constraint.
+ *
+ * @param subConditions the boolean sub-conditions, may be {@code null}
+ * @param context parameter context
+ * @param scriptExecutor script executor, may be {@code null} when no
scripts are used
+ * @return the resolved active sub-conditions and whether any were
unresolvable
+ */
+ public static ResolvedBooleanSubConditions resolveBooleanSubConditions(
+ List<Condition> subConditions,
+ Map<String, Object> context,
+ ScriptExecutor scriptExecutor) {
+ List<Condition> active = new ArrayList<>();
+ boolean unresolvable = false;
+ if (subConditions == null) {
+ return new ResolvedBooleanSubConditions(active, false);
+ }
+ for (Condition sub : subConditions) {
+ if (sub == null) {
+ continue;
+ }
+ Condition resolved = getContextualCondition(sub, context,
scriptExecutor);
+ if (resolved == null) {
+ continue;
+ }
+ if (resolved == UNRESOLVABLE) {
+ unresolvable = true;
+ continue;
+ }
+ if (resolved.getConditionTypeId() == null) {
+ // A copy of a sub-condition whose type is not resolved yet
keeps its type id, so the
+ // dispatcher can still resolve it.
+ resolved.setConditionTypeId(sub.getConditionTypeId());
+ }
+ active.add(resolved);
+ }
+ return new ResolvedBooleanSubConditions(active, unresolvable);
+ }
+
}
diff --git
a/persistence-spi/src/test/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelperTest.java
b/persistence-spi/src/test/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelperTest.java
index 2e9a870d0..767651924 100644
---
a/persistence-spi/src/test/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelperTest.java
+++
b/persistence-spi/src/test/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelperTest.java
@@ -1155,6 +1155,76 @@ public class ConditionContextHelperTest {
resolved);
}
+ // ========== Boolean optional sub-conditions (UNOMI-1004) ==========
+
+ @Test
+ public void testResolveBooleanSubConditions_omitsUnsetOptionalUnderOr() {
+ Condition idBranch = new Condition();
+ idBranch.setParameter("propertyName", "target.itemId");
+ idBranch.setParameter("propertyValue",
ConditionContextHelper.PARAMETER_REFERENCE_PREFIX + "targetId");
+ idBranch.setParameter("comparisonOperator", "equals");
+
+ Condition pathBranch = new Condition();
+ pathBranch.setParameter("propertyName",
"target.properties.pageInfo.pagePath");
+ pathBranch.setParameter("propertyValue",
ConditionContextHelper.PARAMETER_REFERENCE_PREFIX + "targetPath");
+ pathBranch.setParameter("comparisonOperator", "equals");
+
+ Map<String, Object> context = new HashMap<>();
+ context.put("targetId", "button-a");
+
+ ConditionContextHelper.ResolvedBooleanSubConditions resolved =
+ ConditionContextHelper.resolveBooleanSubConditions(
+ Arrays.asList(idBranch, pathBranch), context,
scriptExecutor);
+
+ assertFalse(resolved.hasUnresolvable());
+ assertEquals(1, resolved.getActive().size());
+ assertEquals("button-a",
resolved.getActive().get(0).getParameter("propertyValue"));
+ }
+
+ @Test
+ public void testResolveBooleanSubConditions_neitherSet_yieldsEmptyActive()
{
+ Condition idBranch = new Condition();
+ idBranch.setParameter("propertyValue",
ConditionContextHelper.PARAMETER_REFERENCE_PREFIX + "targetId");
+ Condition pathBranch = new Condition();
+ pathBranch.setParameter("propertyValue",
ConditionContextHelper.PARAMETER_REFERENCE_PREFIX + "targetPath");
+
+ ConditionContextHelper.ResolvedBooleanSubConditions resolved =
+ ConditionContextHelper.resolveBooleanSubConditions(
+ Arrays.asList(idBranch, pathBranch), new HashMap<>(),
scriptExecutor);
+
+ assertTrue(resolved.getActive().isEmpty());
+ assertFalse(resolved.hasUnresolvable());
+ }
+
+ @Test
+ public void testResolveBooleanSubConditions_untypedSub_keepsTypeId() {
+ Condition sub = new Condition();
+ sub.setConditionTypeId("profilePropertyCondition");
+ sub.setParameter("propertyValue",
ConditionContextHelper.PARAMETER_REFERENCE_PREFIX + "gender");
+
+ Map<String, Object> context = new HashMap<>();
+ context.put("gender", "female");
+ ConditionContextHelper.ResolvedBooleanSubConditions resolved =
+
ConditionContextHelper.resolveBooleanSubConditions(Collections.singletonList(sub),
context, scriptExecutor);
+
+ assertEquals("profilePropertyCondition",
resolved.getActive().get(0).getConditionTypeId());
+ assertEquals("female",
resolved.getActive().get(0).getParameter("propertyValue"));
+ }
+
+ @Test
+ public void
testResolveBooleanSubConditions_unresolvableSub_isReportedAndOmitted() {
+ Condition broken = new Condition();
+ broken.setParameter("propertyValue",
ConditionContextHelper.SCRIPT_EXPRESSION_PREFIX + "anything");
+ Condition literal = new Condition();
+ literal.setParameter("propertyValue", "x");
+
+ ConditionContextHelper.ResolvedBooleanSubConditions resolved =
+
ConditionContextHelper.resolveBooleanSubConditions(Arrays.asList(broken,
literal), new HashMap<>(), null);
+
+ assertTrue(resolved.hasUnresolvable());
+ assertEquals(Collections.singletonList(literal), resolved.getActive());
+ }
+
// ========== Helper Methods ==========
private Condition createIntegerParameterCondition(Object testParamValue) {
diff --git a/plugins/baseplugin/pom.xml b/plugins/baseplugin/pom.xml
index 948ca2012..6d74cc33d 100644
--- a/plugins/baseplugin/pom.xml
+++ b/plugins/baseplugin/pom.xml
@@ -125,6 +125,11 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-metrics</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
diff --git
a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluator.java
b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluator.java
index df9fb677e..edf3fe9d6 100644
---
a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluator.java
+++
b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluator.java
@@ -19,8 +19,10 @@ package org.apache.unomi.plugins.baseplugin.conditions;
import org.apache.unomi.api.Item;
import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
import
org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluator;
import
org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher;
+import org.apache.unomi.scripting.ScriptExecutor;
import org.apache.unomi.tracing.api.RequestTracer;
import org.apache.unomi.tracing.api.TracerService;
@@ -30,11 +32,16 @@ import java.util.Map;
/** Evaluator for AND and OR conditions. */
public class BooleanConditionEvaluator implements ConditionEvaluator {
private TracerService tracerService;
+ private ScriptExecutor scriptExecutor;
public void setTracerService(TracerService tracerService) {
this.tracerService = tracerService;
}
+ public void setScriptExecutor(ScriptExecutor scriptExecutor) {
+ this.scriptExecutor = scriptExecutor;
+ }
+
@Override
public boolean eval(Condition condition, Item item, Map<String, Object>
context,
ConditionEvaluatorDispatcher dispatcher) {
@@ -55,18 +62,30 @@ public class BooleanConditionEvaluator implements
ConditionEvaluator {
@SuppressWarnings("unchecked")
List<Condition> conditions = (List<Condition>) subConditionsParam;
- if (conditions == null || conditions.isEmpty()) {
+ // Omit branches whose optional parameter is unset. For OR that
prevents an unset
+ // branch from matching everything (UNOMI-1004). For AND it keeps
UNOMI-982 behaviour.
+ ConditionContextHelper.ResolvedBooleanSubConditions resolved =
+
ConditionContextHelper.resolveBooleanSubConditions(conditions, context,
scriptExecutor);
+ if (resolved.hasUnresolvable() && isAnd) {
+ if (tracer != null) {
+ tracer.endOperation(false, "AND condition has an
unresolvable subcondition");
+ }
+ return false;
+ }
+ List<Condition> active = resolved.getActive();
+
+ if (active.isEmpty()) {
if (tracer != null) {
- tracer.endOperation(isAnd, "No subconditions found,
returning " + isAnd);
+ tracer.endOperation(isAnd, "No active subconditions after
omitting unset optionals, returning " + isAnd);
}
return isAnd;
}
if (tracer != null) {
- tracer.trace("Using " + (isAnd ? "AND" : "OR") + " operator
for " + conditions.size() + " subconditions", condition);
+ tracer.trace("Using " + (isAnd ? "AND" : "OR") + " operator
for " + active.size() + " subconditions", condition);
}
- for (Condition sub : conditions) {
+ for (Condition sub : active) {
boolean eval = dispatcher.eval(sub, item, context);
if (!eval && isAnd) {
if (tracer != null) {
diff --git
a/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml
b/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml
index 56fbadc84..fc495667c 100644
--- a/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml
+++ b/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml
@@ -67,6 +67,7 @@
</service-properties>
<bean
class="org.apache.unomi.plugins.baseplugin.conditions.BooleanConditionEvaluator">
<property name="tracerService" ref="tracerService" />
+ <property name="scriptExecutor" ref="scriptExecutor" />
</bean>
</service>
diff --git
a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluatorTest.java
b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluatorTest.java
index 67a027be6..0160464e1 100644
---
a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluatorTest.java
+++
b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluatorTest.java
@@ -28,6 +28,7 @@ import org.mockito.junit.MockitoJUnitRunner;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
+import java.util.Map;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -35,6 +36,7 @@ import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.same;
import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -182,6 +184,99 @@ public class BooleanConditionEvaluatorTest {
}
}
+ // --- optional parameters under OR (UNOMI-1004) ---
+
+ @Test
+ public void or_unsetOptionalBranch_isOmitted_andDoesNotMatchEverything() {
+ Condition targetIdBranch = optionalEquals("target.itemId", "targetId");
+ Condition targetPathBranch =
optionalEquals("target.properties.pageInfo.pagePath", "targetPath");
+
+ // Only targetId is set, and it does not match — the unset path branch
must not make OR true.
+ Map<String, Object> context = new HashMap<>();
+ context.put("targetId", "button-a");
+ when(dispatcher.eval(any(Condition.class), same(profile),
any())).thenReturn(false);
+
+ assertFalse(evaluator.eval(booleanCondition("or", targetIdBranch,
targetPathBranch),
+ profile, context, dispatcher));
+ // Only the resolved targetId branch is evaluated; the unset path
branch is omitted.
+ verify(dispatcher, times(1)).eval(any(Condition.class), same(profile),
any());
+ }
+
+ @Test
+ public void or_onlyTargetIdSet_matchesWhenIdMatches() {
+ Condition targetIdBranch = optionalEquals("target.itemId", "targetId");
+ Condition targetPathBranch =
optionalEquals("target.properties.pageInfo.pagePath", "targetPath");
+
+ Map<String, Object> context = new HashMap<>();
+ context.put("targetId", "button-a");
+ when(dispatcher.eval(any(Condition.class), same(profile),
any())).thenReturn(true);
+
+ assertTrue(evaluator.eval(booleanCondition("or", targetIdBranch,
targetPathBranch),
+ profile, context, dispatcher));
+ verify(dispatcher, times(1)).eval(any(Condition.class), same(profile),
any());
+ }
+
+ @Test
+ public void or_neitherOptionalSet_matchesNothing() {
+ Condition targetIdBranch = optionalEquals("target.itemId", "targetId");
+ Condition targetPathBranch =
optionalEquals("target.properties.pageInfo.pagePath", "targetPath");
+
+ assertFalse(evaluator.eval(booleanCondition("or", targetIdBranch,
targetPathBranch),
+ profile, new HashMap<>(), dispatcher));
+ verify(dispatcher, never()).eval(any(Condition.class), any(), any());
+ }
+
+ @Test
+ public void and_unsetOptionalBranch_isOmitted_stillMatches() {
+ // UNOMI-982: an unset optional under AND states no constraint.
+ Condition required = new Condition();
+ required.setParameter("propertyName", "eventType");
+ required.setParameter("propertyValue", "hover");
+ required.setParameter("comparisonOperator", "equals");
+ Condition optional = optionalEquals("target.itemId", "targetId");
+
+ when(dispatcher.eval(any(Condition.class), same(profile),
any())).thenReturn(true);
+
+ assertTrue(evaluator.eval(booleanCondition("and", required, optional),
+ profile, new HashMap<>(), dispatcher));
+ verify(dispatcher, times(1)).eval(any(Condition.class), same(profile),
any());
+ }
+
+ @Test
+ public void and_allOptionalsUnset_matches() {
+ Condition targetIdBranch = optionalEquals("target.itemId", "targetId");
+ Condition targetPathBranch =
optionalEquals("target.properties.pageInfo.pagePath", "targetPath");
+
+ assertTrue(evaluator.eval(booleanCondition("and", targetIdBranch,
targetPathBranch),
+ profile, new HashMap<>(), dispatcher));
+ verify(dispatcher, never()).eval(any(Condition.class), any(), any());
+ }
+
+ // A script that cannot run (no script executor here) makes the
sub-condition unresolvable.
+
+ @Test
+ public void and_unresolvableBranch_failsClosed() {
+ Condition broken = new Condition();
+ broken.setParameter("propertyValue", "script::anything");
+ Condition literal = new Condition();
+ literal.setParameter("propertyValue", "hover");
+
+ assertFalse(evaluator.eval(booleanCondition("and", literal, broken),
profile, new HashMap<>(), dispatcher));
+ verify(dispatcher, never()).eval(any(Condition.class), any(), any());
+ }
+
+ @Test
+ public void or_unresolvableBranch_isSkipped() {
+ Condition broken = new Condition();
+ broken.setParameter("propertyValue", "script::anything");
+ Condition literal = new Condition();
+ literal.setParameter("propertyValue", "hover");
+ when(dispatcher.eval(same(literal), same(profile),
any())).thenReturn(true);
+
+ assertTrue(evaluator.eval(booleanCondition("or", broken, literal),
profile, new HashMap<>(), dispatcher));
+ verify(dispatcher, times(1)).eval(any(Condition.class), same(profile),
any());
+ }
+
// --- helper ---
private Condition booleanCondition(String operator, Condition... subs) {
@@ -190,4 +285,12 @@ public class BooleanConditionEvaluatorTest {
c.setParameter("subConditions", subs.length == 0 ?
Collections.emptyList() : Arrays.asList(subs));
return c;
}
+
+ private Condition optionalEquals(String propertyName, String
parameterName) {
+ Condition c = new Condition();
+ c.setParameter("propertyName", propertyName);
+ c.setParameter("propertyValue", "parameter::" + parameterName);
+ c.setParameter("comparisonOperator", "equals");
+ return c;
+ }
}
diff --git
a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionOptionalParameterTest.java
b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionOptionalParameterTest.java
new file mode 100644
index 000000000..6aeb64f5f
--- /dev/null
+++
b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionOptionalParameterTest.java
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.conditions;
+
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.conditions.ConditionType;
+import org.apache.unomi.api.services.DefinitionsService;
+import
org.apache.unomi.persistence.spi.conditions.evaluator.impl.ConditionEvaluatorDispatcherImpl;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+
+/**
+ * UNOMI-1004 boolean semantics for unset optional parameters, through the
real dispatcher, which
+ * expands condition types into their parent condition. The shipped hover
condition is covered in
+ * the hover-event plugin.
+ */
+public class BooleanConditionOptionalParameterTest {
+
+ private final ConditionEvaluatorDispatcherImpl dispatcher = new
ConditionEvaluatorDispatcherImpl();
+ private final ConditionType booleanType = type("booleanCondition",
"booleanConditionEvaluator", null);
+ private final ConditionType eventPropertyType =
type("eventPropertyCondition", "propertyConditionEvaluator", null);
+
+ @Before
+ public void setUp() {
+ dispatcher.setDefinitionsService(mock(DefinitionsService.class));
+ dispatcher.addEvaluator("booleanConditionEvaluator", new
BooleanConditionEvaluator());
+ dispatcher.addEvaluator("propertyConditionEvaluator", new
PropertyConditionEvaluator());
+ }
+
+ @Test
+ public void typedSubConditionWithUnsetParameter_isOmittedUnderOr() {
+ // A type whose parent pins the event type and compares a target id.
The dispatcher merges a
+ // typed condition's own parameters into its expanded parent, so one
unset parameter voids
+ // the whole typed condition; under OR it must be omitted, not count
as a match.
+ Condition hoverTargetParent = booleanCondition("and",
+ eventProperty("eventType", "hover"),
+ eventProperty("target.itemId", "parameter::targetId"));
+ Condition sub = new Condition(type("hoverTargetCondition", null,
hoverTargetParent));
+ sub.setParameter("targetId", "parameter::unsetTargetId");
+ Condition or = booleanCondition("or", sub, eventProperty("eventType",
"view"));
+
+ assertFalse(dispatcher.eval(or, event("hover", "x", "/p"), new
HashMap<>()));
+ assertTrue(dispatcher.eval(or, event("view", "x", "/p"), new
HashMap<>()));
+ }
+
+ @Test
+ public void nestedAndWithAllOptionalsUnset_matchesUnderOr() {
+ // An AND with nothing left to test states no constraint (UNOMI-982),
so an OR holding it
+ // matches. This is the 2.x and 3.x behaviour, kept on purpose.
+ Condition unsetAnd = booleanCondition("and",
+ eventProperty("target.itemId", "parameter::unsetTargetId"),
+ eventProperty("target.properties.pageInfo.pagePath",
"parameter::unsetTargetPath"));
+ Condition or = booleanCondition("or", unsetAnd,
eventProperty("eventType", "never"));
+
+ assertTrue(dispatcher.eval(or, event("view", "x", "/p"), new
HashMap<>()));
+ }
+
+ private Condition booleanCondition(String operator, Condition...
subConditions) {
+ Condition condition = new Condition(booleanType);
+ condition.setParameter("operator", operator);
+ condition.setParameter("subConditions", Arrays.asList(subConditions));
+ return condition;
+ }
+
+ private Condition eventProperty(String propertyName, String propertyValue)
{
+ Condition condition = new Condition(eventPropertyType);
+ condition.setParameter("propertyName", propertyName);
+ condition.setParameter("propertyValue", propertyValue);
+ condition.setParameter("comparisonOperator", "equals");
+ return condition;
+ }
+
+ private static ConditionType type(String id, String evaluator, Condition
parentCondition) {
+ ConditionType type = new ConditionType();
+ type.setItemId(id);
+ type.setConditionEvaluator(evaluator);
+ type.setParentCondition(parentCondition);
+ return type;
+ }
+
+ private static Event event(String eventType, String targetId, String
pagePath) {
+ CustomItem target = new CustomItem();
+ target.setItemId(targetId);
+ target.setItemType("page");
+ Map<String, Object> pageInfo = new HashMap<>();
+ pageInfo.put("pagePath", pagePath);
+ Map<String, Object> properties = new HashMap<>();
+ properties.put("pageInfo", pageInfo);
+ target.setProperties(properties);
+
+ Event event = new Event(eventType, null, new Profile("profile-1"),
"systemscope", null, target, new Date());
+ event.setItemId("event-1");
+ return event;
+ }
+}
diff --git a/plugins/hover-event/pom.xml b/plugins/hover-event/pom.xml
index cbc4109bd..a533bce5d 100644
--- a/plugins/hover-event/pom.xml
+++ b/plugins/hover-event/pom.xml
@@ -40,6 +40,95 @@
</dependencies>
</dependencyManagement>
+ <dependencies>
+ <!-- Test only: evaluate the shipped hoverEventCondition through the
real dispatcher. -->
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-api</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-persistence-spi</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-plugins-base</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-metrics</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-scripting</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-tracing-api</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-lang3</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>commons-collections</groupId>
+ <artifactId>commons-collections</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>commons-beanutils</groupId>
+ <artifactId>commons-beanutils</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>joda-time</groupId>
+ <artifactId>joda-time</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>commons-io</groupId>
+ <artifactId>commons-io</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.datatype</groupId>
+ <artifactId>jackson-datatype-jsr310</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.module</groupId>
+ <artifactId>jackson-module-jaxb-annotations</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-simple</artifactId>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
<build>
<plugins>
<plugin>
diff --git
a/plugins/hover-event/src/test/java/org/apache/unomi/plugins/hoverevent/HoverEventConditionTest.java
b/plugins/hover-event/src/test/java/org/apache/unomi/plugins/hoverevent/HoverEventConditionTest.java
new file mode 100644
index 000000000..4633e132c
--- /dev/null
+++
b/plugins/hover-event/src/test/java/org/apache/unomi/plugins/hoverevent/HoverEventConditionTest.java
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.hoverevent;
+
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.conditions.ConditionType;
+import org.apache.unomi.api.services.DefinitionsService;
+import org.apache.unomi.api.utils.ParserHelper;
+import org.apache.unomi.persistence.spi.CustomObjectMapper;
+import
org.apache.unomi.persistence.spi.conditions.evaluator.impl.ConditionEvaluatorDispatcherImpl;
+import
org.apache.unomi.plugins.baseplugin.conditions.BooleanConditionEvaluator;
+import
org.apache.unomi.plugins.baseplugin.conditions.PropertyConditionEvaluator;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * UNOMI-1004: the shipped hoverEventCondition, evaluated through the real
dispatcher, must only
+ * match on the target id or path that is set.
+ */
+public class HoverEventConditionTest {
+
+ private final ConditionEvaluatorDispatcherImpl dispatcher = new
ConditionEvaluatorDispatcherImpl();
+ private ConditionType hoverType;
+
+ @Before
+ public void setUp() throws IOException {
+ DefinitionsService definitionsService = mock(DefinitionsService.class);
+ when(definitionsService.getConditionType("booleanCondition"))
+ .thenReturn(type("booleanCondition",
"booleanConditionEvaluator"));
+ when(definitionsService.getConditionType("eventPropertyCondition"))
+ .thenReturn(type("eventPropertyCondition",
"propertyConditionEvaluator"));
+
+ hoverType = CustomObjectMapper.getObjectMapper().readValue(
+
getClass().getResource("/META-INF/cxs/conditions/hoverEventCondition.json"),
ConditionType.class);
+ assertTrue(ParserHelper.resolveConditionType(definitionsService,
hoverType.getParentCondition(), "test"));
+
+ dispatcher.setDefinitionsService(definitionsService);
+ dispatcher.addEvaluator("booleanConditionEvaluator", new
BooleanConditionEvaluator());
+ dispatcher.addEvaluator("propertyConditionEvaluator", new
PropertyConditionEvaluator());
+ }
+
+ @Test
+ public void onlyTargetId_matchesMatchingHover_andRejectsOtherTarget() {
+ assertTrue(eval(hover("button-a", null), event("hover", "button-a",
"/other")));
+ assertFalse(eval(hover("button-a", null), event("hover", "button-b",
"/other")));
+ }
+
+ @Test
+ public void onlyTargetPath_matchesMatchingHover_andRejectsOtherPath() {
+ assertTrue(eval(hover(null, "/page-a"), event("hover", "x",
"/page-a")));
+ assertFalse(eval(hover(null, "/page-a"), event("hover", "x",
"/page-b")));
+ }
+
+ @Test
+ public void bothSet_matchesEither() {
+ assertTrue(eval(hover("button-a", "/page-a"), event("hover",
"button-a", "/other")));
+ assertTrue(eval(hover("button-a", "/page-a"), event("hover", "other",
"/page-a")));
+ assertFalse(eval(hover("button-a", "/page-a"), event("hover", "other",
"/other")));
+ }
+
+ @Test
+ public void neitherSet_matchesNothing() {
+ assertFalse(eval(hover(null, null), event("hover", "button-a",
"/page-a")));
+ }
+
+ @Test
+ public void otherEventType_neverMatches() {
+ assertFalse(eval(hover("button-a", null), event("view", "button-a",
"/other")));
+ }
+
+ private boolean eval(Condition condition, Event event) {
+ return dispatcher.eval(condition, event, new HashMap<>());
+ }
+
+ private Condition hover(String targetId, String targetPath) {
+ Condition hover = new Condition(hoverType);
+ if (targetId != null) {
+ hover.setParameter("targetId", targetId);
+ }
+ if (targetPath != null) {
+ hover.setParameter("targetPath", targetPath);
+ }
+ return hover;
+ }
+
+ private static ConditionType type(String id, String evaluator) {
+ ConditionType type = new ConditionType();
+ type.setItemId(id);
+ type.setConditionEvaluator(evaluator);
+ return type;
+ }
+
+ private static Event event(String eventType, String targetId, String
pagePath) {
+ CustomItem target = new CustomItem();
+ target.setItemId(targetId);
+ target.setItemType("page");
+ Map<String, Object> pageInfo = new HashMap<>();
+ pageInfo.put("pagePath", pagePath);
+ Map<String, Object> properties = new HashMap<>();
+ properties.put("pageInfo", pageInfo);
+ target.setProperties(properties);
+
+ Event event = new Event(eventType, null, new Profile("profile-1"),
"systemscope", null, target, new Date());
+ event.setItemId("event-1");
+ return event;
+ }
+}
diff --git
a/services/src/test/java/org/apache/unomi/services/impl/TestConditionEvaluators.java
b/services/src/test/java/org/apache/unomi/services/impl/TestConditionEvaluators.java
index 4c6d3d832..d3f53d895 100644
---
a/services/src/test/java/org/apache/unomi/services/impl/TestConditionEvaluators.java
+++
b/services/src/test/java/org/apache/unomi/services/impl/TestConditionEvaluators.java
@@ -87,23 +87,31 @@ public class TestConditionEvaluators {
tracer.startOperation("boolean", "Evaluating boolean condition
with operator: " + condition.getParameter("operator"), condition);
String operator = (String) condition.getParameter("operator");
Object subConditionsParam =
condition.getParameter("subConditions");
+ boolean isAnd = "and".equalsIgnoreCase(operator);
if (!(subConditionsParam instanceof List)) {
- tracer.endOperation(true, "No subconditions found, returning
true");
- return true;
+ tracer.endOperation(isAnd, "No subconditions found, returning
" + isAnd);
+ return isAnd;
}
@SuppressWarnings("unchecked")
List<Condition> subConditions = (List<Condition>)
subConditionsParam;
- if (subConditions.isEmpty()) {
- tracer.endOperation(true, "No subconditions found, returning
true");
- return true;
+ // Mirror BooleanConditionEvaluator: omit unset optional OR/AND
branches (UNOMI-1004 / UNOMI-982).
+ ConditionContextHelper.ResolvedBooleanSubConditions resolved =
+
ConditionContextHelper.resolveBooleanSubConditions(subConditions, context,
null);
+ if (resolved.hasUnresolvable() && isAnd) {
+ tracer.endOperation(false, "AND condition has an unresolvable
subcondition");
+ return false;
}
+ List<Condition> active = resolved.getActive();
- boolean isAnd = "and".equalsIgnoreCase(operator);
- tracer.trace("Using " + (isAnd ? "AND" : "OR") + " operator for "
+ subConditions.size() + " subconditions", condition);
+ if (active.isEmpty()) {
+ tracer.endOperation(isAnd, "No active subconditions after
omitting unset optionals, returning " + isAnd);
+ return isAnd;
+ }
+
+ tracer.trace("Using " + (isAnd ? "AND" : "OR") + " operator for "
+ active.size() + " subconditions", condition);
- for (int i = 0; i < subConditions.size(); i++) {
- Condition subCondition = subConditions.get(i);
+ for (Condition subCondition : active) {
boolean result = dispatcher.eval(subCondition, item, context);
if (isAnd && !result) {
tracer.endOperation(false, "AND condition failed on
subcondition");
