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 2020/11/19 20:12:19 UTC
[unomi] 01/01: UNOMI-400 Fix class hierarchy lookup for property
condition evaluator
This is an automated email from the ASF dual-hosted git repository.
shuber pushed a commit to branch UNOMI-400-class-hierarchy-fix
in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 5a0b71430b0627226570956dae45089e3af9eabf
Author: Serge Huber <sh...@jahia.com>
AuthorDate: Thu Nov 19 21:12:10 2020 +0100
UNOMI-400 Fix class hierarchy lookup for property condition evaluator
---
.../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 1ba3c72..a236d4f 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
@@ -203,9 +208,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);
@@ -213,7 +215,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);
@@ -221,8 +222,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() {