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:18 UTC
[unomi] branch UNOMI-400-class-hierarchy-fix created (now 5a0b714)
This is an automated email from the ASF dual-hosted git repository.
shuber pushed a change to branch UNOMI-400-class-hierarchy-fix
in repository https://gitbox.apache.org/repos/asf/unomi.git.
at 5a0b714 UNOMI-400 Fix class hierarchy lookup for property condition evaluator
This branch includes the following new commits:
new 5a0b714 UNOMI-400 Fix class hierarchy lookup for property condition evaluator
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-400 Fix class hierarchy lookup for property
condition evaluator
Posted by sh...@apache.org.
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() {