[unomi] branch master updated: UNOMI-534 Fix bug in rule optimization (#369)

[jk...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jkevan 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 4e1341a  UNOMI-534 Fix bug in rule optimization (#369)
4e1341a is described below

commit 4e1341a9ed3adc8571893853ea5a3448c9b67a1f
Author: Serge Huber <sh...@jahia.com>
AuthorDate: Wed Jan 5 14:28:20 2022 +0100

    UNOMI-534 Fix bug in rule optimization (#369)
    
    - Make sure that if the rule parsing cannot determine an associated event type we put it in the all ("*") rule structure
    - Update integration tests to test that scenario
    - Fix a minor NPE in the ActionExecutorDispatcherImpl class
---
 itests/src/test/java/org/apache/unomi/itests/BaseIT.java    |  1 +
 .../test/java/org/apache/unomi/itests/RuleServiceIT.java    | 13 +++++++++++++
 .../services/actions/impl/ActionExecutorDispatcherImpl.java |  7 +++++--
 .../apache/unomi/services/impl/rules/RulesServiceImpl.java  |  4 ++++
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/itests/src/test/java/org/apache/unomi/itests/BaseIT.java b/itests/src/test/java/org/apache/unomi/itests/BaseIT.java
index 427b309..17fca68 100644
--- a/itests/src/test/java/org/apache/unomi/itests/BaseIT.java
+++ b/itests/src/test/java/org/apache/unomi/itests/BaseIT.java
@@ -285,6 +285,7 @@ public abstract class BaseIT {
     public void updateServices() throws InterruptedException {
         persistenceService = getService(PersistenceService.class);
         definitionsService = getService(DefinitionsService.class);
+        rulesService = getService(RulesService.class);
     }
 
     public void updateConfiguration(String serviceName, String configPid, String propName, Object propValue) throws InterruptedException, IOException {
diff --git a/itests/src/test/java/org/apache/unomi/itests/RuleServiceIT.java b/itests/src/test/java/org/apache/unomi/itests/RuleServiceIT.java
index 71eb9fd..3dc4241 100644
--- a/itests/src/test/java/org/apache/unomi/itests/RuleServiceIT.java
+++ b/itests/src/test/java/org/apache/unomi/itests/RuleServiceIT.java
@@ -103,6 +103,13 @@ public class RuleServiceIT extends BaseIT {
                 ).build()
         );
         createAndWaitForRule(complexEventTypeRule);
+        Rule noEventTypeRule = new Rule(new Metadata(TEST_SCOPE, "no-event-type-rule", "No event type rule", "A rule with a simple condition but no event type matching"));
+        noEventTypeRule.setCondition(builder.condition("eventPropertyCondition")
+                .parameter("propertyName", "target.properties.pageInfo.language")
+                .parameter("comparisonOperator", "equals")
+                .parameter("propertyValue", "en")
+                .build());
+        createAndWaitForRule(noEventTypeRule);
 
         Profile profile = new Profile(UUID.randomUUID().toString());
         Session session = new Session(UUID.randomUUID().toString(), profile, new Date(), TEST_SCOPE);
@@ -111,6 +118,7 @@ public class RuleServiceIT extends BaseIT {
 
         assertTrue("Simple rule should be matched", matchingRules.contains(simpleEventTypeRule));
         assertFalse("Complex rule should NOT be matched", matchingRules.contains(complexEventTypeRule));
+        assertTrue("No event type rule should be matched", matchingRules.contains(noEventTypeRule));
 
         Event loginEvent = new Event(UUID.randomUUID().toString(), "login", session, profile, TEST_SCOPE, null, null, new Date());
         matchingRules = rulesService.getMatchingRules(loginEvent);
@@ -119,6 +127,7 @@ public class RuleServiceIT extends BaseIT {
 
         rulesService.removeRule(simpleEventTypeRule.getItemId());
         rulesService.removeRule(complexEventTypeRule.getItemId());
+        rulesService.removeRule(noEventTypeRule.getItemId());
         refreshPersistence();
         rulesService.refreshRules();
     }
@@ -129,11 +138,15 @@ public class RuleServiceIT extends BaseIT {
         Session session = new Session(UUID.randomUUID().toString(), profile, new Date(), TEST_SCOPE);
 
         updateConfiguration(RulesService.class.getName(), "org.apache.unomi.services", "rules.optimizationActivated", "false");
+        rulesService = getService(RulesService.class);
+        eventService = getService(EventService.class);
 
         LOGGER.info("Running unoptimized rules performance test...");
         long unoptimizedRunTime = runEventTest(profile, session);
 
         updateConfiguration(RulesService.class.getName(), "org.apache.unomi.services", "rules.optimizationActivated", "true");
+        rulesService = getService(RulesService.class);
+        eventService = getService(EventService.class);
 
         LOGGER.info("Running optimized rules performance test...");
         long optimizedRunTime = runEventTest(profile, session);
diff --git a/services/src/main/java/org/apache/unomi/services/actions/impl/ActionExecutorDispatcherImpl.java b/services/src/main/java/org/apache/unomi/services/actions/impl/ActionExecutorDispatcherImpl.java
index b54fcdf..d5df384 100644
--- a/services/src/main/java/org/apache/unomi/services/actions/impl/ActionExecutorDispatcherImpl.java
+++ b/services/src/main/java/org/apache/unomi/services/actions/impl/ActionExecutorDispatcherImpl.java
@@ -84,9 +84,12 @@ public class ActionExecutorDispatcherImpl implements ActionExecutorDispatcher {
 
 
     public int execute(Action action, Event event) {
-        String actionKey = action.getActionType().getActionExecutor();
+        String actionKey = null;
+        if (action.getActionType() != null) {
+            actionKey = action.getActionType().getActionExecutor();
+        }
         if (actionKey == null) {
-            throw new UnsupportedOperationException("No service defined for : " + action.getActionType());
+            throw new UnsupportedOperationException("No service defined for : " + action.getActionTypeId());
         }
 
         int colonPos = actionKey.indexOf(":");
diff --git a/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java
index d87e036..60b0795 100644
--- a/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java
+++ b/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java
@@ -592,6 +592,10 @@ public class RulesServiceImpl implements RulesService, EventListenerService, Syn
 
     private void updateRulesByEventType(Map<String, Set<Rule>> rulesByEventType, Rule rule) {
         Set<String> eventTypeIds = ParserHelper.resolveConditionEventTypes(rule.getCondition());
+        if (eventTypeIds.isEmpty()) {
+            // if we couldn't resolve an event type, we always execute the conditions, these conditions might lead to performance issues though.
+            eventTypeIds.add("*");
+        }
         for (String eventTypeId : eventTypeIds) {
             Set<Rule> rules = rulesByEventType.get(eventTypeId);
             if (rules == null) {