This is an automated email from the ASF dual-hosted git repository.
shuber pushed a commit to branch unomi-1.4.x
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/unomi-1.4.x by this push:
new 92232f1 UNOMI-400 Fix class hierarchy lookup for property condition
evaluator (#220)
92232f1 is described below
commit 92232f10a9e3edb64ad8fb6bc203b3472983dfd9
Author: Serge Huber <[email protected]>
AuthorDate: Thu Nov 19 21:31:42 2020 +0100
UNOMI-400 Fix class hierarchy lookup for property condition evaluator (#220)
(cherry picked from commit 7b52f7efe2f9576ae4aac2a991a806c91a1902df)
---
.../HardcodedPropertyAccessorRegistry.java | 60 ++++++++++++++++------
.../conditions/accessors/MetadataItemAccessor.java | 2 +-
.../HardcodedPropertyAccessorRegistryTest.java | 20 ++++++++
.../conditions/PropertyConditionEvaluatorTest.java | 29 ++++++++---
4 files changed, 87 insertions(+), 24 deletions(-)
diff --git
a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/HardcodedPropertyAccessorRegistry.java
b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/HardcodedPropertyAccessorRegistry.java
index 59a70b5..56f8cd5 100644
---
a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/HardcodedPropertyAccessorRegistry.java
+++
b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/HardcodedPropertyAccessorRegistry.java
@@ -25,6 +25,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.*;
+import java.util.stream.Collectors;
/**
* This class contains the registry of all the hardcoded property accessors.
@@ -34,22 +35,23 @@ public class HardcodedPropertyAccessorRegistry {
private static final Logger logger =
LoggerFactory.getLogger(HardcodedPropertyAccessorRegistry.class.getName());
- Map<String, HardcodedPropertyAccessor> propertyAccessors = new HashMap<>();
+ protected Map<Class<?>, HardcodedPropertyAccessor> propertyAccessors = new
HashMap<>();
+ protected Map<Class<?>, List<Class<?>>> cachedClassAncestors = new
HashMap<>();
public HardcodedPropertyAccessorRegistry() {
- propertyAccessors.put(Item.class.getName(), new ItemAccessor(this));
- propertyAccessors.put(MetadataItem.class.getName(), new
MetadataItemAccessor(this));
- propertyAccessors.put(Metadata.class.getName(), new
MetadataAccessor(this));
- propertyAccessors.put(TimestampedItem.class.getName(), new
TimestampedItemAccessor(this));
- propertyAccessors.put(Event.class.getName(), new EventAccessor(this));
- propertyAccessors.put(Profile.class.getName(), new
ProfileAccessor(this));
- propertyAccessors.put(Consent.class.getName(), new
ConsentAccessor(this));
- propertyAccessors.put(Session.class.getName(), new
SessionAccessor(this));
- propertyAccessors.put(Rule.class.getName(), new RuleAccessor(this));
- propertyAccessors.put(Goal.class.getName(), new GoalAccessor(this));
- propertyAccessors.put(CustomItem.class.getName(), new
CustomItemAccessor(this));
- propertyAccessors.put(Campaign.class.getName(), new
CampaignAccessor(this));
- propertyAccessors.put(Map.class.getName(), new MapAccessor(this));
+ propertyAccessors.put(Item.class, new ItemAccessor(this));
+ propertyAccessors.put(MetadataItem.class, new
MetadataItemAccessor(this));
+ propertyAccessors.put(Metadata.class, new MetadataAccessor(this));
+ propertyAccessors.put(TimestampedItem.class, new
TimestampedItemAccessor(this));
+ propertyAccessors.put(Event.class, new EventAccessor(this));
+ propertyAccessors.put(Profile.class, new ProfileAccessor(this));
+ propertyAccessors.put(Consent.class, new ConsentAccessor(this));
+ propertyAccessors.put(Session.class, new SessionAccessor(this));
+ propertyAccessors.put(Rule.class, new RuleAccessor(this));
+ propertyAccessors.put(Goal.class, new GoalAccessor(this));
+ propertyAccessors.put(CustomItem.class, new CustomItemAccessor(this));
+ propertyAccessors.put(Campaign.class, new CampaignAccessor(this));
+ propertyAccessors.put(Map.class, new MapAccessor(this));
}
public static class NextTokens {
@@ -115,10 +117,16 @@ public class HardcodedPropertyAccessorRegistry {
NextTokens nextTokens = getNextTokens(expression);
List<Class<?>> lookupClasses = new ArrayList<>();
lookupClasses.add(object.getClass());
- lookupClasses.add(object.getClass().getSuperclass());
- lookupClasses.addAll(Arrays.asList(object.getClass().getInterfaces()));
+ List<Class<?>> objectClassAncestors =
cachedClassAncestors.get(object.getClass());
+ if (objectClassAncestors == null) {
+ objectClassAncestors = collectAncestors(object.getClass(),
propertyAccessors.keySet());
+ cachedClassAncestors.put(object.getClass(), objectClassAncestors);
+ }
+ if (objectClassAncestors != null) {
+ lookupClasses.addAll(objectClassAncestors);
+ }
for (Class<?> lookupClass : lookupClasses) {
- HardcodedPropertyAccessor propertyAccessor =
propertyAccessors.get(lookupClass.getName());
+ HardcodedPropertyAccessor propertyAccessor =
propertyAccessors.get(lookupClass);
if (propertyAccessor != null) {
Object result = propertyAccessor.getProperty(object,
nextTokens.propertyName, nextTokens.leftoverExpression);
if
(!HardcodedPropertyAccessor.PROPERTY_NOT_FOUND_MARKER.equals(result)) {
@@ -129,4 +137,22 @@ public class HardcodedPropertyAccessorRegistry {
logger.warn("Couldn't find any property access for class {} and
expression {}", object.getClass().getName(), expression);
return HardcodedPropertyAccessor.PROPERTY_NOT_FOUND_MARKER;
}
+
+ public List<Class<?>> collectAncestors(Class<?> targetClass, Set<Class<?>>
availableAccessors) {
+ Set<Class<?>> parentClasses = new LinkedHashSet<>();
+ if (targetClass.getSuperclass() != null) {
+ parentClasses.add(targetClass.getSuperclass());
+ }
+ if (targetClass.getInterfaces().length > 0) {
+ parentClasses.addAll(Arrays.asList(targetClass.getInterfaces()));
+ }
+ Set<Class<?>> ancestors = new LinkedHashSet<>();
+ for (Class<?> parentClass : parentClasses) {
+ ancestors.addAll(collectAncestors(parentClass,
availableAccessors));
+ }
+ Set<Class<?>> result = new LinkedHashSet<>();
+ result.addAll(parentClasses);
+ result.addAll(ancestors);
+ return
result.stream().filter(availableAccessors::contains).collect(Collectors.toList());
+ }
}
diff --git
a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/accessors/MetadataItemAccessor.java
b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/accessors/MetadataItemAccessor.java
index 1c787a3..25f946e 100644
---
a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/accessors/MetadataItemAccessor.java
+++
b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/accessors/MetadataItemAccessor.java
@@ -29,6 +29,6 @@ public class MetadataItemAccessor extends
HardcodedPropertyAccessor<MetadataItem
if ("metadata".equals(propertyName)) {
registry.getProperty(object.getMetadata(), leftoverExpression);
}
- return null;
+ return PROPERTY_NOT_FOUND_MARKER;
}
}
diff --git
a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/HardcodedPropertyAccessorRegistryTest.java
b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/HardcodedPropertyAccessorRegistryTest.java
index 03bdb30..c0dbd95 100644
---
a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/HardcodedPropertyAccessorRegistryTest.java
+++
b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/HardcodedPropertyAccessorRegistryTest.java
@@ -16,9 +16,17 @@
*/
package org.apache.unomi.plugins.baseplugin.conditions;
+import org.apache.unomi.api.Item;
+import org.apache.unomi.api.MetadataItem;
+import org.apache.unomi.api.rules.Rule;
import org.junit.Test;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
public class HardcodedPropertyAccessorRegistryTest {
@@ -53,4 +61,16 @@ public class HardcodedPropertyAccessorRegistryTest {
assertEquals("Property name value was wrong", expectedPropertyName,
nextTokens.propertyName);
assertEquals("Leftover expression value was wrong",
expectedLeftoverExpression, nextTokens.leftoverExpression);
}
+
+ @Test
+ public void testCollectAncestors() {
+ List<Class<?>> classAncestors =
registry.collectAncestors(HashMap.class, registry.propertyAccessors.keySet());
+ assertTrue("HashMap ancestors list size is not correct",
classAncestors.size() == 1);
+ assertTrue("Expected Map as ancestor of HashMap but wasn't found",
classAncestors.stream().anyMatch(ancestor -> ancestor.equals(Map.class)));
+
+ classAncestors = registry.collectAncestors(Rule.class,
registry.propertyAccessors.keySet());
+ assertTrue("Rule ancestors list size is not correct",
classAncestors.size() == 2);
+ assertTrue("Expected MetadataItem as ancestor of Rule but wasn't
found", classAncestors.stream().anyMatch(ancestor ->
ancestor.equals(MetadataItem.class)));
+ assertTrue("Expected Item as ancestor of Ruole but wasn't found",
classAncestors.stream().anyMatch(ancestor -> ancestor.equals(Item.class)));
+ }
}
diff --git
a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java
b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java
index d63c6be..8df17b6 100644
---
a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java
+++
b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java
@@ -18,6 +18,7 @@ package org.apache.unomi.plugins.baseplugin.conditions;
import ognl.MethodFailedException;
import org.apache.unomi.api.*;
+import org.apache.unomi.api.rules.Rule;
import
org.apache.unomi.plugins.baseplugin.conditions.accessors.HardcodedPropertyAccessor;
import org.apache.unomi.scripting.ExpressionFilter;
import org.apache.unomi.scripting.ExpressionFilterFactory;
@@ -51,6 +52,7 @@ public class PropertyConditionEvaluatorTest {
public static final Date PROFILE_PREVIOUS_VISIT = new Date();
public static final String NEWSLETTER_CONSENT_ID = "newsLetterConsentId";
public static final String TRACKING_CONSENT_ID = "trackingConsentId";
+ public static final String RULE_ITEM_ID = "mockRuleItemId";
private static PropertyConditionEvaluator propertyConditionEvaluator = new
PropertyConditionEvaluator();
private static Profile mockProfile = generateMockProfile();
private static Session mockSession = generateMockSession(mockProfile);
@@ -96,6 +98,9 @@ public class PropertyConditionEvaluatorTest {
// here let's make sure our reporting of non optimized expressions
works.
assertEquals("Should have received the non-optimized marker string",
HardcodedPropertyAccessor.PROPERTY_NOT_FOUND_MARKER,
propertyConditionEvaluator.getHardcodedPropertyValue(mockSession,
"profile.non-existing-field"));
+ Event mockRuleEvent = generateMockRuleFiredEvent(mockProfile,
mockSession);
+ assertEquals("Target itemId value is not correct", RULE_ITEM_ID,
propertyConditionEvaluator.getHardcodedPropertyValue(mockRuleEvent,
"target.itemId"));
+
}
@Test
@@ -201,9 +206,6 @@ public class PropertyConditionEvaluatorTest {
}
private static Event generateMockEvent(Profile mockProfile, Session
mockSession) {
- Event mockEvent = new Event();
- mockEvent.setProfile(mockProfile);
- mockEvent.setSession(mockSession);
CustomItem sourceItem = new CustomItem();
sourceItem.setItemId(MOCK_ITEM_ID);
sourceItem.setScope(DIGITALL_SCOPE);
@@ -211,7 +213,6 @@ public class PropertyConditionEvaluatorTest {
sourcePageInfoMap.put("pagePath", SOURCE_PAGE_PATH_VALUE);
sourcePageInfoMap.put("pageURL", SOURCE_PAGE_URL_VALUE);
sourceItem.getProperties().put("pageInfo", sourcePageInfoMap);
- mockEvent.setSource(sourceItem);
CustomItem targetItem = new CustomItem();
targetItem.setItemId(MOCK_ITEM_ID);
targetItem.setScope(DIGITALL_SCOPE);
@@ -219,8 +220,24 @@ public class PropertyConditionEvaluatorTest {
targetPageInfoMap.put("pagePath", TARGET_PAGE_PATH_VALUE);
targetPageInfoMap.put("pageURL", TARGET_PAGE_URL_VALUE);
targetItem.getProperties().put("pageInfo", targetPageInfoMap);
- mockEvent.setTarget(targetItem);
- return mockEvent;
+ return new Event("view", mockSession, mockProfile, DIGITALL_SCOPE,
sourceItem, targetItem, new HashMap<>(), new Date(), true);
+ }
+
+ private static Event generateMockRuleFiredEvent(Profile mockProfile,
Session mockSession) {
+ CustomItem sourceItem = new CustomItem();
+ sourceItem.setItemId(MOCK_ITEM_ID);
+ sourceItem.setScope(DIGITALL_SCOPE);
+ Map<String, Object> sourcePageInfoMap = new HashMap<>();
+ sourcePageInfoMap.put("pagePath", SOURCE_PAGE_PATH_VALUE);
+ sourcePageInfoMap.put("pageURL", SOURCE_PAGE_URL_VALUE);
+ sourceItem.getProperties().put("pageInfo", sourcePageInfoMap);
+ Metadata metadata = new Metadata();
+ metadata.setId(RULE_ITEM_ID);
+ metadata.setScope(DIGITALL_SCOPE);
+ metadata.setEnabled(true);
+ Rule rule = new Rule(metadata);
+ rule.setScope(DIGITALL_SCOPE);
+ return new Event("ruleFired", mockSession, mockProfile,
DIGITALL_SCOPE, sourceItem, rule, new HashMap<>(), new Date(), true);
}
public static Profile generateMockProfile() {
