[UNOMI-922] Inconsistency between value for nbOfVisits in Profile and number of Sessions

docker/src/main/docker/docker-compose-cluster.yml

@@ -17,7 +17,7 @@
 version: '2.4'
 services:
   elasticsearch:
-    image: docker.elastic.co/elasticsearch/elasticsearch:9.1.3
+    image: docker.elastic.co/elasticsearch/elasticsearch:9.2.1
     volumes:
       - unomi-3-elasticsearch-data:/usr/share/elasticsearch/data
     environment:

itests/README.md

@@ -153,12 +153,12 @@ public class Migrate16xTo200IT extends BaseIT {
             // Create snapshot repo
             HttpUtils.executePutRequest(httpClient, "http://localhost:9400/_snapshot/snapshots_repository/", resourceAsString("migration/create_snapshots_repository.json"), null);
             // Get snapshot, insure it exists
-            String snapshot = HttpUtils.executeGetRequest(httpClient, "http://localhost:9400/_snapshot/snapshots_repository/snapshot_2", null);
-            if (snapshot == null || !snapshot.contains("snapshot_2")) {
+            String snapshot = HttpUtils.executeGetRequest(httpClient, "http://localhost:9400/_snapshot/snapshots_repository/snapshot_3", null);
+            if (snapshot == null || !snapshot.contains("snapshot_3")) {
                 throw new RuntimeException("Unable to retrieve 1.6.x snapshot for ES restore");
             }
             // Restore the snapshot
-            HttpUtils.executePostRequest(httpClient, "http://localhost:9400/_snapshot/snapshots_repository/snapshot_2/_restore?wait_for_completion=true", "{}", null);
+            HttpUtils.executePostRequest(httpClient, "http://localhost:9400/_snapshot/snapshots_repository/snapshot_3/_restore?wait_for_completion=true", "{}", null);
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
@@ -183,7 +183,7 @@ public class Migrate16xTo200IT extends BaseIT {
 
 ### How to update a migration test ElasticSearch Snapshot ?
 
-In the following example we want to modify the snapshot: `snapshot_2`.
+In the following example we want to modify the snapshot: `snapshot_3`.
 This snapshot has been done on Unomi 1.6.x using ElasticSearch 7.11.0. 
 So we will set up locally those servers in the exact same versions.
 (For now just download them and do not start them yet.)
@@ -233,13 +233,13 @@ Now we have to add the snapshot repository, do the following request on your Ela
     }
 
 Now we need to restore the snapshot we want to modify, 
-but first let's try to see if the snapshot with the id `snapshot_2` correctly exists:
+but first let's try to see if the snapshot with the id `snapshot_3` correctly exists:
 
-    GET /_snapshot/snapshots_repository/snapshot_2
+    GET /_snapshot/snapshots_repository/snapshot_3
 
 If the snapshot exists we can restore it:
 
-    POST /_snapshot/snapshots_repository/snapshot_2/_restore?wait_for_completion=true
+    POST /_snapshot/snapshots_repository/snapshot_3/_restore?wait_for_completion=true
     {}
 
 At the end of the previous request ElasticSearch should be ready and our Unomi snapshot is restored to version `1.6.x`.
@@ -257,11 +257,11 @@ they are probably used by the actual migration tests already.)
 
 Once you data updated we need to recreate the snapshot, first we delete the old snapshot:
 
-    DELETE /_snapshot/snapshots_repository/snapshot_2
+    DELETE /_snapshot/snapshots_repository/snapshot_3
 
 Then we recreate it:
 
-    PUT /_snapshot/snapshots_repository/snapshot_2
+    PUT /_snapshot/snapshots_repository/snapshot_3
 
 Once the process finished (check the ElasticSearch logs to see that the snapshot is correctly created), 
 we need to remove the snapshot repository from our local ElasticSearch

itests/pom.xml

@@ -252,7 +252,7 @@
                         <!-- REPLACE THE FOLLOWING WITH THE PLUGIN VERSION YOU NEED -->
                         <version>6.29</version>
                         <configuration>
-                            <!-- <downloadUrl>https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-9.1.3-darwin-aarch64.tar
+                            <!-- <downloadUrl>https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-9.2.1-darwin-aarch64.tar
                             .gz</downloadUrl> -->
                             <clusterName>contextElasticSearchITests</clusterName>
                             <transportPort>9500</transportPort>

itests/src/test/java/org/apache/unomi/itests/ProfileServiceIT.java

@@ -19,7 +19,6 @@
 import org.apache.unomi.api.*;
 import org.apache.unomi.api.conditions.Condition;
 import org.apache.unomi.api.query.Query;
-import org.apache.unomi.persistence.spi.PersistenceService;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -151,8 +150,7 @@ public void testGetProfileWithScrolling() throws InterruptedException {
 
     // Relevant only when throwExceptions system property is true
     @Test
-    public void testGetProfileWithWrongScrollerIdThrowException()
-            throws InterruptedException, NoSuchFieldException, IllegalAccessException, IOException {
+    public void testGetProfileWithWrongScrollerIdThrowException() throws InterruptedException, IOException {
         boolean throwExceptionCurrent = false;
         Configuration searchEngineConfiguration = configurationAdmin.getConfiguration("org.apache.unomi.persistence." + searchEngine);
         if (searchEngineConfiguration != null && searchEngineConfiguration.getProperties().get("throwExceptions") != null) {
@@ -470,4 +468,71 @@ public void testBatchProfileUpdate() throws Exception {
         keepTrying("We should not be able to retrieve previous profile based on previous value", () -> persistenceService.queryCount(oldProfilesCondition, Profile.ITEM_TYPE),
                 (count) -> count == 0, 1000, 100);
     }
+
+    @Test
+    public void testPurgeSessions() throws Exception {
+        Date currentDate = new Date();
+        LocalDateTime minus6Months = LocalDateTime.ofInstant(currentDate.toInstant(), ZoneId.systemDefault()).minusMonths(6);
+        LocalDateTime minus18Months = LocalDateTime.ofInstant(currentDate.toInstant(), ZoneId.systemDefault()).minusMonths(18);
+        Date currentDateMinus6Months = Date.from(minus6Months.atZone(ZoneId.systemDefault()).toInstant());
+        Date currentDateMinus18Months = Date.from(minus18Months.atZone(ZoneId.systemDefault()).toInstant());
+
+        long originalSessionsCount  = persistenceService.getAllItemsCount(Session.ITEM_TYPE);
+
+        //Create 10 profiles with sessions
+        Profile[] profiles = new Profile[10];
+        for (int i=0; i < profiles.length; i++) {
+            profiles[i] = new Profile("dummy-profile-session-purge-test-" + i);
+            profiles[i].setProperty("nbOfVisits", 20);
+            profiles[i].setProperty("totalNbOfVisits", 20);
+            persistenceService.save(profiles[i]);
+        }
+
+        // create 6 months old sessions
+        for (int i = 0; i < profiles.length * 10; i++) {
+            Session session = new Session("6-months-old-session-" + i, profiles[i%10], currentDateMinus6Months, "dummy-scope");
+            persistenceService.save(session);
+        }
+
+        // create 18 months old sessions
+        for (int i = 0; i < profiles.length * 10; i++) {
+            Session session = new Session("18-months-old-session-" + i, profiles[i%10], currentDateMinus18Months, "dummy-scope");
+            persistenceService.save(session);
+        }
+
+        keepTrying("Sessions number should be 200", () -> persistenceService.getAllItemsCount(Session.ITEM_TYPE),
+                (count) -> count == (200 + originalSessionsCount), 1000, 100);
+        for (Profile value : profiles) {
+            String profileId = value.getItemId();
+            keepTrying("Profile should have nbOfVisits=20", () -> profileService.load(profileId),
+                    (profile) -> (Integer) profile.getProperty("nbOfVisits") == 20, 1000, 100);
+            keepTrying("Profile should have totalNbOfVisits=20", () -> profileService.load(profileId),
+                    (profile) -> (Integer) profile.getProperty("totalNbOfVisits") == 20, 1000, 100);
+        }
+
+        // Should have no effect
+        profileService.purgeSessionItems(0);
+        keepTrying("Sessions number should be 200", () -> persistenceService.getAllItemsCount(Session.ITEM_TYPE),
+                (count) -> count == (200 + originalSessionsCount), 1000, 100);
+        for (Profile value : profiles) {
+            String profileId = value.getItemId();
+            keepTrying("Profile should have nbOfVisits=20", () -> profileService.load(profileId),
+                    (profile) -> (Integer) profile.getProperty("nbOfVisits") == 20, 1000, 100);
+            keepTrying("Profile should have totalNbOfVisits=20", () -> profileService.load(profileId),
+                    (profile) -> (Integer) profile.getProperty("totalNbOfVisits") == 20, 1000, 100);
+        }
+
+        // Should purge sessions older than 365 days
+        profileService.purgeSessionItems(365);
+        keepTrying("Sessions number should be 100", () -> persistenceService.getAllItemsCount(Session.ITEM_TYPE),
+                (count) -> count == (100 + originalSessionsCount), 1000, 100);
+        for (Profile value : profiles) {
+            String profileId = value.getItemId();
+            keepTrying("Profile should have nbOfVisits=10", () -> profileService.load(profileId),
+                    (profile) -> (Integer) profile.getProperty("nbOfVisits") == 10, 1000, 100);
+            keepTrying("Profile should have totalNbOfVisits=20", () -> profileService.load(profileId),
+                    (profile) -> (Integer) profile.getProperty("totalNbOfVisits") == 20, 1000, 100);
+        }
+
+    }
 }

itests/src/test/java/org/apache/unomi/itests/migration/Migrate16xToCurrentVersionIT.java

@@ -40,7 +40,7 @@ public class Migrate16xToCurrentVersionIT extends BaseIT {
 
     private int eventCount = 0;
     private int sessionCount = 0;
-    private Set<String[]> initialScopes = new HashSet<>();
+    private final Set<String[]> initialScopes = new HashSet<>();
 
     private static final String SCOPE_NOT_EXIST = "SCOPE_NOT_EXIST";
     private static final List<String> oldSystemItemsIndices = Arrays.asList("context-actiontype", "context-campaign", "context-campaignevent", "context-goal",
@@ -71,12 +71,12 @@ public void waitForStartup() throws InterruptedException {
             // Create snapshot repo
             HttpUtils.executePutRequest(httpClient, "http://localhost:9400/_snapshot/snapshots_repository/", resourceAsString("migration/create_snapshots_repository.json"), null);
             // Get snapshot, insure it exists
-            String snapshot = HttpUtils.executeGetRequest(httpClient, "http://localhost:9400/_snapshot/snapshots_repository/snapshot_2", null);
-            if (snapshot == null || !snapshot.contains("snapshot_2")) {
+            String snapshot = HttpUtils.executeGetRequest(httpClient, "http://localhost:9400/_snapshot/snapshots_repository/snapshot_3", null);
+            if (snapshot == null || !snapshot.contains("snapshot_3")) {
                 throw new RuntimeException("Unable to retrieve 1.6.x snapshot for ES restore");
             }
             // Restore the snapshot
-            HttpUtils.executePostRequest(httpClient, "http://localhost:9400/_snapshot/snapshots_repository/snapshot_2/_restore?wait_for_completion=true", "{}", null);
+            HttpUtils.executePostRequest(httpClient, "http://localhost:9400/_snapshot/snapshots_repository/snapshot_3/_restore?wait_for_completion=true", "{}", null);
 
             String snapshotStatus = HttpUtils.executeGetRequest(httpClient, "http://localhost:9400/_snapshot/_status", null);
             System.out.println(snapshotStatus);
@@ -134,6 +134,7 @@ public void checkMigratedData() throws Exception {
         }
         checkMergedProfilesAliases();
         checkProfileInterests();
+        checkProfileTotalNbOfVisits();
         checkScopeHaveBeenCreated();
         checkLoginEventWithScope();
         checkFormEventRestructured();
@@ -224,7 +225,7 @@ private void checkFormEventRestructured() {
         for (Event formEvent : events) {
             Assert.assertEquals(0, formEvent.getProperties().size());
             Map<String, Object> fields = (Map<String, Object>) formEvent.getFlattenedProperties().get("fields");
-            Assert.assertTrue(fields.size() > 0);
+            Assert.assertFalse(fields.isEmpty());
 
             if (Objects.equals(formEvent.getItemId(), "7b55b4fd-5ff0-4a85-9dc4-ffde322a1de6")) {
                 // check singled valued
@@ -243,14 +244,14 @@ private void checkLoginEventWithScope() {
         List<String> digitallLoginEvent = Arrays.asList("4054a3e0-35ef-4256-999b-b9c05c1209f1", "f3f71ff8-2d6d-4b6c-8bdc-cb39905cddfe", "ff24ae6f-5a98-421e-aeb0-e86855b462ff");
         for (Event loginEvent : events) {
             if (loginEvent.getItemId().equals("5c4ac1df-f42b-4117-9432-12fdf9ecdf98")) {
-                Assert.assertEquals(loginEvent.getScope(), "systemsite");
-                Assert.assertEquals(loginEvent.getTarget().getScope(), "systemsite");
-                Assert.assertEquals(loginEvent.getSource().getScope(), "systemsite");
+                Assert.assertEquals("systemsite", loginEvent.getScope());
+                Assert.assertEquals("systemsite", loginEvent.getTarget().getScope());
+                Assert.assertEquals("systemsite", loginEvent.getSource().getScope());
             }
             if (digitallLoginEvent.contains(loginEvent.getItemId())) {
-                Assert.assertEquals(loginEvent.getScope(), "digitall");
-                Assert.assertEquals(loginEvent.getTarget().getScope(), "digitall");
-                Assert.assertEquals(loginEvent.getSource().getScope(), "digitall");
+                Assert.assertEquals("digitall", loginEvent.getScope());
+                Assert.assertEquals("digitall", loginEvent.getTarget().getScope());
+                Assert.assertEquals("digitall", loginEvent.getSource().getScope());
             }
         }
     }
@@ -346,6 +347,38 @@ private void checkProfileInterests() {
         }
     }
 
+    /**
+     * Data set contains a profile (id: 468ca2bf-7d24-41ea-9ef4-5b96f78207e4) with a property named totalNbOfVisits set to 3
+     * --> Because that profile has only one session, the nbOfVisits should be set to 1 after migration 3.1.0-00
+     * All other profiles that had an existing nbOfVisits should now have the totalNbOfVisits property set.
+     */
+    private void checkProfileTotalNbOfVisits() {
+        // check that totalNbOfVisits have been set for a specific profile
+        Profile profile = persistenceService.load("468ca2bf-7d24-41ea-9ef4-5b96f78207e4", Profile.class);
+        Assert.assertEquals("Bill", profile.getProperty("firstName"));
+        Assert.assertNotNull("Profile " + profile.getItemId() + " is missing totalNbOfVisits property", profile.getProperty("totalNbOfVisits"));
+        Assert.assertEquals("Profile " + profile.getItemId() + " has not the expected value for totalNbOfVisits", 3, profile.getProperty("totalNbOfVisits"));
+        Assert.assertNotNull("Profile " + profile.getItemId() + " is missing nbOfVisits property", profile.getProperty("nbOfVisits"));
+        Assert.assertEquals("Profile " + profile.getItemId() + " has not the expected value for nbOfVisits",3, profile.getProperty("nbOfVisits"));
+
+        // check that nbOfVisits have been corrected set for a specific profile
+        profile = persistenceService.load("ad6dc96a-964e-4f6a-b3dc-2395b6e8a069", Profile.class);
+        Assert.assertEquals("Leonard", profile.getProperty("firstName"));
+        Assert.assertNotNull("Profile " + profile.getItemId() + " is missing totalNbOfVisits property", profile.getProperty("totalNbOfVisits"));
+        Assert.assertEquals("Profile " + profile.getItemId() + " has not the expected value for totalNbOfVisits", 15, profile.getProperty("totalNbOfVisits"));
+        Assert.assertNotNull("Profile " + profile.getItemId() + " is missing nbOfVisits property", profile.getProperty("nbOfVisits"));
+        Assert.assertEquals("Profile " + profile.getItemId() + " has not the expected value for nbOfVisits",1, profile.getProperty("nbOfVisits"));
+
+        // check that the totalNbOfVisits property has been set for all profiles
+        List<Profile> allProfiles = persistenceService.getAllItems(Profile.class);
+        Assert.assertFalse("No profiles found in the data set", allProfiles.isEmpty());
+        for (Profile p : allProfiles) {
+            if (p.getProperties().containsKey("nbOfVisits")) {
+                Assert.assertNotNull("Profile " + p.getItemId() + " is missing totalNbOfVisits property", p.getProperty("totalNbOfVisits"));
+            }
+        }
+    }
+
     /**
      * Data set contains a master profile: 468ca2bf-7d24-41ea-9ef4-5b96f78207e4
      * And two profiles that have been merged with this master profile: c33dec90-ffc9-4484-9e61-e42c323f268f and ac5b6b0f-afce-4c4f-9391-4ff0b891b254
@@ -358,7 +391,7 @@ private void checkMergedProfilesAliases() {
             // control the created alias
             ProfileAlias alias = persistenceService.load(mergedProfile, ProfileAlias.class);
             Assert.assertNotNull(alias);
-            Assert.assertEquals(alias.getProfileID(), masterProfile);
+            Assert.assertEquals(masterProfile, alias.getProfileID());
 
             // control the merged profile do not exist anymore
             Assert.assertNull(persistenceService.load(mergedProfile, Profile.class));

manual/src/main/asciidoc/configuration.adoc

@@ -322,6 +322,14 @@ From https://github.com/apache/unomi/blob/unomi-1.5.x/plugins/baseplugin/src/mai
         "storeInSession": false
       },
       "type": "setPropertyAction"
+    },
+    {
+      "parameterValues": {
+        "setPropertyName": "properties.totalNbOfVisits",
+        "setPropertyValue": "script::profile.properties.?totalNbOfVisits != null ? (profile.properties.totalNbOfVisits + 1) : 1",
+        "storeInSession": false
+      },
+      "type": "setPropertyAction"
     }
   ]
 
@@ -344,6 +352,7 @@ Default allowed MVEL expressions (from https://github.com/apache/unomi/blob/unom
   "\\QminimumDuration*1000\\E",
   "\\QmaximumDuration*1000\\E",
   "\\Qprofile.properties.?nbOfVisits != null ? (profile.properties.nbOfVisits + 1) : 1\\E",
+  "\\Qprofile.properties.?totalNbOfVisits != null ? (profile.properties.totalNbOfVisits + 1) : 1\\E",
   "\\Qsession != null ? session.size + 1 : 0\\E",
   "\\Q'properties.optimizationTest_'+event.target.itemId\\E",
   "\\Qevent.target.properties.variantId\\E",

manual/src/main/asciidoc/datamodel.adoc

@@ -284,6 +284,7 @@ image::profile.png[]
         "lastName": "Galileo",
         "preferredLanguage": "en",
         "nbOfVisits": 2,
+        "totalNbOfVisits": 5,
         "gender": "male",
         "jobTitle": "Vice President",
         "lastVisit": "2020-01-31T08:41:22Z",
@@ -525,6 +526,7 @@ The visitor’s location is also resolve based on the IP address that was used t
         "properties": {
             "preferredLanguage": "en",
             "nbOfVisits": 2,
+            "totalNbOfVisits": 5,
             "gender": "male",
             "jobTitle": "Vice President",
             "lastVisit": "2020-01-31T08:41:22Z",

persistence-elasticsearch/core/src/main/resources/META-INF/cxs/mappings/profile.json

@@ -35,6 +35,9 @@
         "nbOfVisits": {
           "type": "long"
         },
+        "totalNbOfVisits": {
+          "type": "long"
+        },
         "interests": {
           "type": "nested"
         }

persistence-opensearch/core/src/main/resources/META-INF/cxs/mappings/profile.json

@@ -35,6 +35,9 @@
         "nbOfVisits": {
           "type": "long"
         },
+        "totalNbOfVisits": {
+          "type": "long"
+        },
         "interests": {
           "type": "nested"
         }

plugins/baseplugin/src/main/resources/META-INF/cxs/expressions/mvel.json

@@ -5,9 +5,10 @@
   "\\QminimumDuration*1000\\E",
   "\\QmaximumDuration*1000\\E",
   "\\Qprofile.properties.?nbOfVisits != null ? (profile.properties.nbOfVisits + 1) : 1\\E",
+  "\\Qprofile.properties.?totalNbOfVisits != null ? (profile.properties.totalNbOfVisits + 1) : 1\\E",
   "\\Qsession != null ? session.size + 1 : 0\\E",
   "\\Q'properties.optimizationTest_'+event.target.itemId\\E",
   "\\Qevent.target.properties.variantId\\E",
   "\\Qprofile.properties.?systemProperties.goals.\\E[\\w\\_]*\\QReached != null ? (profile.properties.systemProperties.goals.\\E[\\w\\_]*\\QReached) : 'now'\\E",
   "\\Qprofile.properties.?systemProperties.campaigns.\\E[\\w\\_]*\\QEngaged != null ? (profile.properties.systemProperties.campaigns.\\E[\\w\\_]*\\QEngaged) : 'now'\\E"
-]
+]

plugins/baseplugin/src/main/resources/META-INF/cxs/rules/sessionAssigned.json

@@ -40,6 +40,13 @@
         "storeInSession": false
       },
       "type": "incrementPropertyAction"
+    },
+    {
+      "parameterValues": {
+        "propertyName": "totalNbOfVisits",
+        "storeInSession": false
+      },
+      "type": "incrementPropertyAction"
     }
   ]
 }