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 <[email protected]> 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) {
