You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by sh...@apache.org on 2021/12/22 16:45:47 UTC

[unomi] branch UNOMI-534-fix-rule-optimization created (now ad673f6)

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

shuber pushed a change to branch UNOMI-534-fix-rule-optimization
in repository https://gitbox.apache.org/repos/asf/unomi.git.


      at ad673f6  UNOMI-534 Fix bug in rule optimization - 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

This branch includes the following new commits:

     new ad673f6  UNOMI-534 Fix bug in rule optimization - 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

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[unomi] 01/01: UNOMI-534 Fix bug in rule optimization - 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

Posted by sh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

shuber pushed a commit to branch UNOMI-534-fix-rule-optimization
in repository https://gitbox.apache.org/repos/asf/unomi.git

commit ad673f69f5873b79ee7cb7e8a7acfaca1cd6a15c
Author: Serge Huber <sh...@jahia.com>
AuthorDate: Wed Dec 22 17:45:40 2021 +0100

    UNOMI-534 Fix bug in rule optimization
    - 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) {