[
https://issues.apache.org/jira/browse/UNOMI-982?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jonathan Sinovassin-Naïk updated UNOMI-982:
-------------------------------------------
Description:
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.
h3. Expected result
The rule matches the event, so the profile carries utm_content.
h3. 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
h3. Scope
Every condition type that declares an optional parameter is affected.
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.
h3. 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.
was:
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.
> 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.
> h3. Expected result
>
>
>
>
>
> The rule matches the event, so the profile carries utm_content.
>
>
>
>
>
> h3. 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
>
>
>
>
> h3. Scope
>
>
>
>
>
> Every condition type that declares an optional parameter is affected.
>
>
>
>
>
> 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.
>
> h3. 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)
