[
https://issues.apache.org/jira/browse/UNOMI-982?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jonathan Sinovassin-Naïk reassigned UNOMI-982:
----------------------------------------------
Assignee: Jonathan Sinovassin-Naïk
> A condition with an unset optional parameter never matches
> ----------------------------------------------------------
>
> Key: UNOMI-982
> URL: https://issues.apache.org/jira/browse/UNOMI-982
> Project: Apache Unomi
> Issue Type: Bug
> Affects Versions: unomi-3.1.0
> Reporter: Jonathan Sinovassin-Naïk
> Assignee: Jonathan Sinovassin-Naïk
> Priority: Major
>
> A condition type can declare an optional parameter. Its parent condition then
> compares a field to parameter::<name>. When the caller supplies no value for
> that name, the comparison states constraint, so the evaluator has to ignore
> the comparison. This is how pageViewEventCondition makes pagePath and
> language optional.
>
>
> Apache Unomi 3.1 evaluates the comparison instead of ignoring it. The
> comparison becomes field equals null, which matches nothing. A
> booleanCondition with the and operator holds that comparison, so the whole
> condition never matches, and every rule built on the condition type stops
> firing.
>
> The failure is silent. Unomi logs no warning and no error, the rule stays
> enabled and valid in persistence, and the REST API returns it unchanged.
>
> Where the behaviour changed
> UNOMI-883 changed three places, at
> https://github.com/apache/unomi/commit/901b5fcdc62a7732097f7db9305dd9404356f8a1
> and https://github.com/apache/unomi/pull/773.
>
>
>
>
> ConditionContextHelper.parseParameterWithValidationRecursive keeps the
> unresolved value in the parameter map. Before that commit, an unresolved
> parameter reference voided the whole condition:
>
>
>
>
> {code:java}
> Object parameter = parseParameter(context, entry.getValue(), scriptExecutor);
>
>
> if (parameter == null) {
>
>
> return null;
>
>
> }
> {code}
>
>
>
> ConditionEvaluatorDispatcherImpl.eval answers false for a null contextual
> condition. Before that commit it answered true, which means the caller drops
> the condition:
>
>
>
> {code:java}
> Condition contextualCondition =
> ConditionContextHelper.getContextualCondition(condition, context,
> scriptExecutor);
>
> if (contextualCondition != null) {
>
>
> return evaluator.eval(contextualCondition, item, context, dispatcher);
>
>
> } else {
>
>
> return true;
>
>
> }
> {code}
>
>
>
>
>
>
> ConditionESQueryBuilderDispatcher.buildFilter answers a match-none query
> for a null contextual condition. Before that commit it answered a match-all
> query. ConditionOSQueryBuilderDispatcher.buildFilter
> has the same shape.
> Steps to reproduce
>
>
>
>
>
> 1. Start Apache Unomi 3.1.0-SNAPSHOT.
>
>
> 2. Register a condition type whose parent condition holds an optional
> parameter. Post this document to /cxs/definitions/conditions:
>
>
>
>
> {code:java}
> {
>
>
> "metadata": { "id": "pageViewEventCondition", "name":
> "pageViewEventCondition",
>
> "systemTags": ["condition", "eventCondition", "event"] },
>
>
> "parentCondition": {
>
>
> "type": "booleanCondition",
>
>
> "parameterValues": {
>
>
> "operator": "and",
>
>
> "subConditions": [
>
>
> { "type": "eventTypeCondition", "parameterValues": { "eventTypeId":
> "view" } },
>
> { "type": "eventPropertyCondition", "parameterValues": {
>
>
> "propertyName": "target.properties.pageInfo.pagePath",
>
>
> "propertyValue": "parameter::pagePath",
>
>
> "comparisonOperator": "equals" } }
>
>
> ]
>
>
> }
>
>
> },
>
>
> "parameters": [ { "id": "pagePath", "type": "string", "multivalued":
> false } ]
>
> }
> {code}
> 3. Register a rule that uses the condition type and sets no pagePath. Post
> this document to /cxs/rules:
>
>
>
>
>
> {code:java}
> {
> "metadata": { "id": "copyURLParams", "name": "Copy URL params to
> profile", "scope": "systemscope" },
> "condition": { "type": "pageViewEventCondition", "parameterValues": {} },
> "actions": [ { "type": "copyPropertiesAction",
> "parameterValues": { "rootProperty":
> "flattenedProperties.URLParameters",
> "singleValueStrategy": "alwaysSet" }
> } ]
> }
> {code}
>
>
> 4. Send a view event to /cxs/context.json with a session id and no profile
> cookie:
>
>
> {code:java}
> { "sessionId": "<a new uuid>",
> "events": [ { "eventType": "view", "scope": "test",
> "target": { "scope": "test", "itemId": "page1", "itemType":
> "page" },
> "flattenedProperties": { "URLParameters": { "utm_content":
> "testParamValue" } } } ] }
> {code}
> 5. Read the profile named by the Set-Cookie header of the response.
> Expected result
>
>
>
>
>
> The rule matches the event, so the profile carries utm_content.
>
>
>
>
>
> Actual result
>
>
>
>
>
> The rule does not match, so the profile carries no utm_content. Replace the
> rule condition with eventTypeCondition and eventTypeId view, and the same
> event now copies the property. The event, the
> action and the rules engine therefore all work.
>
>
>
>
>
> The evaluation reaches the comparison and rejects it. Raise
> org.apache.unomi.persistence.spi.conditions to DEBUG to see the line:
>
>
>
>
> DEBUG ConditionContextHelper | Parameter reference 'pagePath' not found in
> context
>
>
>
>
> Scope
>
>
>
>
>
> Every condition type that declares an optional parameter is affected. In
> the Jahia jExperience module, four rules stop firing on Apache Unomi 3.1:
> copyURLParams, incrementPageViewCount,
> incrementInterests and the target event of a goal.
>
>
>
>
>
> The query builders carry the same inversion. A segment or a past event
> condition that reaches an optional parameter now selects nothing, where it
> used to select everything.
>
> Proposed fix
>
>
>
>
>
> getContextualCondition gives three answers instead of two:
>
>
>
>
>
> - the resolved condition,
>
>
> - null, when the context does not supply a parameter reference the
> condition carries,
>
> - a new UNRESOLVABLE sentinel, when resolution itself failed.
>
>
>
>
>
> A cyclic reference, a depth overrun and a script that did not run all
> produce UNRESOLVABLE. The callers ignore a null condition and refuse an
> UNRESOLVABLE one. UNOMI-883 therefore keeps its guard on
> the broken cases, and the optional parameter works again.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
