jkevan commented on code in PR #659:
URL: https://github.com/apache/unomi/pull/659#discussion_r1613656537
##########
plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetEventOccurenceCountAction.java:
##########
@@ -123,14 +121,37 @@ public int execute(Action action, Event event) {
count++;
}
- String generatedPropertyKey = (String)
pastEventCondition.getParameter("generatedPropertyKey");
- if (PropertyHelper.setProperty(event.getProfile(),
"systemProperties.pastEvents." + generatedPropertyKey, count, "alwaysSet")) {
+ if (updatePastEvents(event, (String)
pastEventCondition.getParameter("generatedPropertyKey"), count)) {
return EventService.PROFILE_UPDATED;
}
return EventService.NO_CHANGE;
}
+ private boolean updatePastEvents(Event event, String generatedPropertyKey,
long count) {
+ List<Map<String, Object>> existingPastEvents = (List<Map<String,
Object>>) event.getProfile().getSystemProperties().get("pastEvents");
+ if (existingPastEvents == null) {
+ existingPastEvents = new ArrayList<>();
+ event.getProfile().getSystemProperties().put("pastEvents",
existingPastEvents);
+ }
+
+ for (Map<String, Object> pastEvent : existingPastEvents) {
+ if (generatedPropertyKey.equals(pastEvent.get("key"))) {
+ if (pastEvent.get("count").equals(count)) {
Review Comment:
`pastEvent.get("count")` does it need NPE check ?
##########
plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PastEventConditionESQueryBuilder.java:
##########
@@ -122,44 +124,39 @@ protected static boolean getStrategyFromOperator(String
operator) {
return operator == null || operator.equals("eventsOccurred");
}
- private Condition getProfileIdsCondition(Set<String> ids, boolean
shouldMatch) {
- Condition idsCondition = new Condition();
-
idsCondition.setConditionType(definitionsService.getConditionType("idsCondition"));
- idsCondition.setParameter("ids", ids);
- idsCondition.setParameter("match", shouldMatch);
- return idsCondition;
- }
-
private Condition getProfileConditionForCounter(String
generatedPropertyKey, Integer minimumEventCount, Integer maximumEventCount,
boolean eventsOccurred) {
- String generatedPropertyName = "systemProperties.pastEvents." +
generatedPropertyKey;
- ConditionType profilePropertyConditionType =
definitionsService.getConditionType("profilePropertyCondition");
if (eventsOccurred) {
- Condition counterIsBetweenBoundaries = new Condition();
-
counterIsBetweenBoundaries.setConditionType(profilePropertyConditionType);
- counterIsBetweenBoundaries.setParameter("propertyName",
generatedPropertyName);
- counterIsBetweenBoundaries.setParameter("comparisonOperator",
"between");
- counterIsBetweenBoundaries.setParameter("propertyValuesInteger",
Arrays.asList(minimumEventCount, maximumEventCount));
- return counterIsBetweenBoundaries;
+ return createEventOccurredCondition(generatedPropertyKey,
minimumEventCount, maximumEventCount);
} else {
- Condition counterMissing = new Condition();
- counterMissing.setConditionType(profilePropertyConditionType);
- counterMissing.setParameter("propertyName", generatedPropertyName);
- counterMissing.setParameter("comparisonOperator", "missing");
-
- Condition counterZero = new Condition();
- counterZero.setConditionType(profilePropertyConditionType);
- counterZero.setParameter("propertyName", generatedPropertyName);
- counterZero.setParameter("comparisonOperator", "equals");
- counterZero.setParameter("propertyValueInteger", 0);
-
- Condition counterCondition = new Condition();
-
counterCondition.setConditionType(definitionsService.getConditionType("booleanCondition"));
- counterCondition.setParameter("operator", "or");
- counterCondition.setParameter("subConditions",
Arrays.asList(counterMissing, counterZero));
- return counterCondition;
+ return createEventNotOccurredCondition(generatedPropertyKey);
}
}
+ private Condition createEventOccurredCondition(String
generatedPropertyKey, Integer minimumEventCount, Integer maximumEventCount) {
+ ConditionBuilder conditionBuilder =
definitionsService.getConditionBuilder();
+ ConditionBuilder.ConditionItem subConditionCount =
conditionBuilder.profileProperty("systemProperties.pastEvents.count").between(minimumEventCount,
maximumEventCount);
+ ConditionBuilder.ConditionItem subConditionKey =
conditionBuilder.profileProperty("systemProperties.pastEvents.key").equalTo(generatedPropertyKey);
+ ConditionBuilder.ConditionItem booleanCondition =
conditionBuilder.and(subConditionCount, subConditionKey);
+ return conditionBuilder.nested(booleanCondition,
"systemProperties.pastEvents").build();
+ }
+
+ private Condition createEventNotOccurredCondition(String
generatedPropertyKey) {
+ ConditionBuilder.ConditionItem counterMissing =
createPastEventMustNotExistCondition(generatedPropertyKey);
+ ConditionBuilder conditionBuilder =
definitionsService.getConditionBuilder();
+ ConditionBuilder.ConditionItem counterZero =
conditionBuilder.profileProperty("systemProperties.pastEvents.count").equalTo(0);
+ ConditionBuilder.ConditionItem keyEquals =
conditionBuilder.profileProperty("systemProperties.pastEvents.key").equalTo(generatedPropertyKey);
+ ConditionBuilder.ConditionItem keyExistsAndCounterZero =
conditionBuilder.and(counterZero, keyEquals);
+ ConditionBuilder.ConditionItem nestedKeyExistsAndCounterZero =
conditionBuilder.nested(keyExistsAndCounterZero, "systemProperties.pastEvents");
+ return conditionBuilder.or(counterMissing,
nestedKeyExistsAndCounterZero).build();
+ }
+
+ private ConditionBuilder.ConditionItem
createPastEventMustNotExistCondition(String generatedPropertyKey) {
+ ConditionBuilder conditionBuilder =
definitionsService.getConditionBuilder();
+ ConditionBuilder.ConditionItem keyEquals =
conditionBuilder.profileProperty("systemProperties.pastEvents.key").equalTo(generatedPropertyKey);
+ ConditionBuilder.ConditionItem keyNestedCondition =
conditionBuilder.nested(keyEquals, "systemProperties.pastEvents");
+ return conditionBuilder.not(keyNestedCondition);
Review Comment:
To be verified, but I'm not sure we required a nested level here.
Nested query is useful to do inner conditioning on a sub level of a nested
object using multiple clauses, those multiple clauses will then be evaluated on
per nested item basis.
Like for condition on both: `systemProperties.pastEvents.count` and
`systemProperties.pastEvents.key`
But here we only have one clause one `systemProperties.pastEvents.key`, so I
wonder if nested level is really useful or not.
To my current understanding of nested ES queries (a bit rusty) it's seem's
to me we could do a simple:
`conditionBuilder.not(conditionBuilder.profileProperty("systemProperties.pastEvents.key").equalTo(generatedPropertyKey))`
--
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: dev-unsubscr...@unomi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
