This is an automated email from the ASF dual-hosted git repository.
jkevan pushed a commit to branch fixMergeControlGroup
in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 3992eb660b4eac47e01e18321b406f9d63d71970
Author: Kevan <ke...@jahia.com>
AuthorDate: Thu Nov 17 13:54:44 2022 +0100
UNOMI-691: fix personalization strategy status stored on profile during
merge
---
.../org/apache/unomi/itests/ProfileMergeIT.java | 91 +++++++++++++++++++---
.../services/impl/profiles/ProfileServiceImpl.java | 37 ++++++---
2 files changed, 107 insertions(+), 21 deletions(-)
diff --git a/itests/src/test/java/org/apache/unomi/itests/ProfileMergeIT.java
b/itests/src/test/java/org/apache/unomi/itests/ProfileMergeIT.java
index 206b21600..a5306d9f9 100644
--- a/itests/src/test/java/org/apache/unomi/itests/ProfileMergeIT.java
+++ b/itests/src/test/java/org/apache/unomi/itests/ProfileMergeIT.java
@@ -23,11 +23,6 @@ import org.apache.unomi.api.ProfileAlias;
import org.apache.unomi.api.actions.Action;
import org.apache.unomi.api.conditions.Condition;
import org.apache.unomi.api.rules.Rule;
-import org.apache.unomi.api.services.DefinitionsService;
-import org.apache.unomi.api.services.EventService;
-import org.apache.unomi.api.services.ProfileService;
-import org.apache.unomi.api.services.RulesService;
-import org.apache.unomi.persistence.spi.PersistenceService;
import org.junit.After;
import org.junit.Assert;
import org.junit.Test;
@@ -35,14 +30,8 @@ import org.junit.runner.RunWith;
import org.ops4j.pax.exam.junit.PaxExam;
import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
import org.ops4j.pax.exam.spi.reactors.PerSuite;
-import org.ops4j.pax.exam.util.Filter;
-import javax.inject.Inject;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Objects;
+import java.util.*;
/**
* Integration test for MergeProfilesOnPropertyAction
@@ -50,6 +39,10 @@ import java.util.Objects;
@RunWith(PaxExam.class)
@ExamReactorStrategy(PerSuite.class)
public class ProfileMergeIT extends BaseIT {
+ public static final String PERSONALIZATION_STRATEGY_STATUS =
"personalizationStrategyStatus";
+ public static final String PERSONALIZATION_STRATEGY_STATUS_ID =
"personalizationId";
+ public static final String PERSONALIZATION_STRATEGY_STATUS_IN_CTRL_GROUP =
"inControlGroup";
+ public static final String PERSONALIZATION_STRATEGY_STATUS_DATE =
"timeStamp";
private final static String TEST_EVENT_TYPE = "mergeProfileTestEventType";
private final static String TEST_RULE_ID = "mergeOnPropertyTest";
@@ -130,6 +123,80 @@ public class ProfileMergeIT extends BaseIT {
Assert.assertEquals("defaultClientId", aliases.get(0).getClientID());
}
+ @Test
+ public void testPersonalizationStrategyStatusMerge() {
+ // create some statuses for the tests:
+ Map<String, Object> perso1true =
buildPersonalizationStrategyStatus("perso-test-1", true);
+ Map<String, Object> perso1false =
buildPersonalizationStrategyStatus("perso-test-1", false);
+ Map<String, Object> perso2false =
buildPersonalizationStrategyStatus("perso-test-2", false);
+ Map<String, Object> perso3true =
buildPersonalizationStrategyStatus("perso-test-3", true);
+
+ // create a single master profile we will keep along all the tests:
+ Profile profileMaster = new Profile("master-profile");
+
+ // merge test 1: Master do not have statuses, but slave have statuses
+ // master have: NULL
+ // slave have: perso-test-1 -> true
+ // perso-test-2 -> false
+ // Expected: perso-test-1 -> true
+ // perso-test-2 -> false
+ Profile profileSlave = new Profile("slave-profile");
+ profileSlave.setSystemProperty(PERSONALIZATION_STRATEGY_STATUS, new
ArrayList<>(Arrays.asList(perso1true, perso2false)));
+ profileMaster = profileService.mergeProfiles(profileMaster,
Collections.singletonList(profileSlave));
+ assertPersonalizationStrategyStatus(profileMaster,
Arrays.asList(perso1true, perso2false));
+
+ // merge test 2: Master do not have statuses, but slave have statuses
+ // master have: perso-test-1 -> true
+ // perso-test-2 -> false
+ // slave have: NULL
+ // Expected: perso-test-1 -> true
+ // perso-test-2 -> false
+ profileSlave = new Profile("slave-profile");
+ profileMaster = profileService.mergeProfiles(profileMaster,
Collections.singletonList(profileSlave));
+ assertPersonalizationStrategyStatus(profileMaster,
Arrays.asList(perso1true, perso2false));
+
+ // merge test 3: both master and slave have strategy statuses and some
are conflicting
+ // master have: perso-test-1 -> true
+ // perso-test-2 -> false
+ // slave have: perso-test-1 -> false (conflicting)
+ // perso-test-3 -> true
+ // Expected: perso-test-1 -> true
+ // perso-test-2 -> false
+ // perso-test-3 -> true
+ profileSlave = new Profile("slave-profile");
+ profileSlave.setSystemProperty(PERSONALIZATION_STRATEGY_STATUS, new
ArrayList<>(Arrays.asList(perso1false, perso3true)));
+ profileMaster = profileService.mergeProfiles(profileMaster,
Collections.singletonList(profileSlave));
+ assertPersonalizationStrategyStatus(profileMaster,
Arrays.asList(perso1true, perso2false, perso3true));
+ }
+
+ private Map<String, Object> buildPersonalizationStrategyStatus(String
persoId, boolean inControlGroup) {
+ Map<String, Object> personalizationStrategyStatus = new HashMap<>();
+ personalizationStrategyStatus.put(PERSONALIZATION_STRATEGY_STATUS_ID,
persoId);
+
personalizationStrategyStatus.put(PERSONALIZATION_STRATEGY_STATUS_DATE, new
Date());
+
personalizationStrategyStatus.put(PERSONALIZATION_STRATEGY_STATUS_IN_CTRL_GROUP,
inControlGroup);
+ return personalizationStrategyStatus;
+ }
+
+ private void assertPersonalizationStrategyStatus(Profile profile,
List<Map<String, Object>> expectedStatuses) {
+ List<Map<String, Object>> strategyStatuses = (List<Map<String,
Object>>) profile.getSystemProperties().get(PERSONALIZATION_STRATEGY_STATUS);
+ Assert.assertEquals("We didn't get the good number of expected
personalization statuses on the given profile: " + profile.getItemId(),
+ expectedStatuses.size(), strategyStatuses.size());
+ for (Map<String, Object> expectedStatus : expectedStatuses) {
+ Optional<Map<String, Object>> foundStatusOp = strategyStatuses
+ .stream()
+ .filter(strategyStatus ->
strategyStatus.get(PERSONALIZATION_STRATEGY_STATUS_ID)
+
.equals(expectedStatus.get(PERSONALIZATION_STRATEGY_STATUS_ID)))
+ .findFirst();
+ if (foundStatusOp.isPresent()) {
+ Map<String, Object> foundStatus = foundStatusOp.get();
+
Assert.assertEquals(expectedStatus.get(PERSONALIZATION_STRATEGY_STATUS_DATE),
foundStatus.get(PERSONALIZATION_STRATEGY_STATUS_DATE));
+
Assert.assertEquals(expectedStatus.get(PERSONALIZATION_STRATEGY_STATUS_IN_CTRL_GROUP),
foundStatus.get(PERSONALIZATION_STRATEGY_STATUS_IN_CTRL_GROUP));
+ } else {
+ Assert.fail("We didn't found the expected personalization: " +
expectedStatus.get(PERSONALIZATION_STRATEGY_STATUS_ID) + " status on the given
profile: " + profile.getItemId());
+ }
+ }
+ }
+
private Event sendEvent() {
Profile profile = new Profile();
profile.setProperties(new HashMap<>());
diff --git
a/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java
b/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java
index a259c1491..bc93fd0c5 100644
---
a/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java
+++
b/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java
@@ -44,6 +44,7 @@ import org.apache.unomi.persistence.spi.CustomObjectMapper;
import org.apache.unomi.persistence.spi.PersistenceService;
import org.apache.unomi.persistence.spi.PropertyHelper;
import org.apache.unomi.api.utils.ParserHelper;
+import org.apache.unomi.services.sorts.ControlGroupPersonalizationStrategy;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.BundleEvent;
@@ -1210,16 +1211,34 @@ public class ProfileServiceImpl implements
ProfileService, SynchronousBundleList
}
} else if (targetProperty instanceof Collection &&
sourceProperty.getValue() instanceof Collection) {
// merge Collections like "lists"
- Collection sourceCollection = (Collection)
sourceProperty.getValue();
- Collection targetCollection = (Collection)
targetProperty;
-
- for (Object sourceItem : sourceCollection) {
- if (!targetCollection.contains(sourceItem)) {
- try {
- targetCollection.add(sourceItem);
+ if
(ControlGroupPersonalizationStrategy.PERSONALIZATION_STRATEGY_STATUS.equals(sourceProperty.getKey()))
{
+ // Special handling for personalization strategy
statuses
+ // TODO UNOMI-719: move this in a dedicated
extension point to handle this kind of merge strategy in a more generic way
+ List<Map<String, Object>> sourceStatuses =
(List<Map<String, Object>>) sourceProperty.getValue();
+ List<Map<String, Object>> targetStatuses =
(List<Map<String, Object>>) targetProperty;
+
+ for (Map<String, Object> sourceStatus :
sourceStatuses) {
+ if (targetStatuses
+ .stream()
+ .noneMatch(targetStatus ->
targetStatus.get(ControlGroupPersonalizationStrategy.PERSONALIZATION_STRATEGY_STATUS_ID)
+
.equals(sourceStatus.get(ControlGroupPersonalizationStrategy.PERSONALIZATION_STRATEGY_STATUS_ID))))
{
+ // there is no existing status for the
status ID, we can safely add it to master
+ targetStatuses.add(sourceStatus);
changed = true;
- } catch (Exception e) {
- // may be Collection type issue
+ }
+ }
+ } else {
+ Collection sourceCollection = (Collection)
sourceProperty.getValue();
+ Collection targetCollection = (Collection)
targetProperty;
+
+ for (Object sourceItem : sourceCollection) {
+ if (!targetCollection.contains(sourceItem)) {
+ try {
+ targetCollection.add(sourceItem);
+ changed = true;
+ } catch (Exception e) {
+ // may be Collection type issue
+ }
}
}
}
