[UNOMI-922] Inconsistency between value for nbOfVisits in Profile and number of Sessions #744
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jayblanc
changed the title
jsinovassin
reviewed
Dec 29, 2025
| int profilesUpdated = 0 | ||
|
|
||
| // Scroll through all profiles | ||
| MigrationUtils.scrollQuery(context.getHttpClient(), esAddress, "/${profileIndex}/_search", scrollQuery, "5m", (hits) -> { |
Contributor
There was a problem hiding this comment.
In profile_scroll_query.json, profiles are currently retrieved in batches of 100, resulting in a bulk request for every 100 documents. Increasing the batch size to 1,000 would reduce the number of requests and improve throughput. Elasticsearch can handle this volume per request, so processing 1,000 documents at a time should significantly shorten the migration duration.
jsinovassin
approved these changes
Dec 30, 2025
…mber of Sessions diff --git c/itests/src/test/java/org/apache/unomi/itests/ProfileServiceIT.java i/itests/src/test/java/org/apache/unomi/itests/ProfileServiceIT.java index 2ff55fc..353bc49 100644 --- c/itests/src/test/java/org/apache/unomi/itests/ProfileServiceIT.java +++ i/itests/src/test/java/org/apache/unomi/itests/ProfileServiceIT.java @@ -19,7 +19,6 @@ package org.apache.unomi.itests; 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 class ProfileServiceIT extends BaseIT { // 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 class ProfileServiceIT extends BaseIT { 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); + } + + } } diff --git c/itests/src/test/java/org/apache/unomi/itests/migration/Migrate16xToCurrentVersionIT.java i/itests/src/test/java/org/apache/unomi/itests/migration/Migrate16xToCurrentVersionIT.java index 5656410..0752863 100644 --- c/itests/src/test/java/org/apache/unomi/itests/migration/Migrate16xToCurrentVersionIT.java +++ i/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", @@ -134,6 +134,7 @@ public class Migrate16xToCurrentVersionIT extends BaseIT { } checkMergedProfilesAliases(); checkProfileInterests(); + checkProfileTotalNbOfVisits(); checkScopeHaveBeenCreated(); checkLoginEventWithScope(); checkFormEventRestructured(); @@ -224,7 +225,7 @@ public class Migrate16xToCurrentVersionIT extends BaseIT { 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 @@ public class Migrate16xToCurrentVersionIT extends BaseIT { 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,30 @@ public class Migrate16xToCurrentVersionIT extends BaseIT { } } + /** + * Data set contains a profile (id: e67ecc69-a7b3-47f1-b91f-5d6e7b90276e) 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 the test_profile totalNbOfVisits have been set for a specific profile + Profile profile = persistenceService.load("e67ecc69-a7b3-47f1-b91f-5d6e7b90276e", Profile.class); + Assert.assertEquals("test_profile", 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",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 +383,7 @@ public class Migrate16xToCurrentVersionIT extends BaseIT { // 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)); diff --git c/manual/src/main/asciidoc/configuration.adoc i/manual/src/main/asciidoc/configuration.adoc index 4d923ad..4b191e5 100644 --- c/manual/src/main/asciidoc/configuration.adoc +++ i/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", diff --git c/manual/src/main/asciidoc/datamodel.adoc i/manual/src/main/asciidoc/datamodel.adoc index e45748f..9c4359b 100755 --- c/manual/src/main/asciidoc/datamodel.adoc +++ i/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", diff --git c/persistence-elasticsearch/core/src/main/resources/META-INF/cxs/mappings/profile.json i/persistence-elasticsearch/core/src/main/resources/META-INF/cxs/mappings/profile.json index 6e650a1..f54604e 100644 --- c/persistence-elasticsearch/core/src/main/resources/META-INF/cxs/mappings/profile.json +++ i/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" } diff --git c/persistence-opensearch/core/src/main/resources/META-INF/cxs/mappings/profile.json i/persistence-opensearch/core/src/main/resources/META-INF/cxs/mappings/profile.json index 6e650a1..f54604e 100644 --- c/persistence-opensearch/core/src/main/resources/META-INF/cxs/mappings/profile.json +++ i/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" } diff --git c/plugins/baseplugin/src/main/resources/META-INF/cxs/expressions/mvel.json i/plugins/baseplugin/src/main/resources/META-INF/cxs/expressions/mvel.json index 6c0a5a0..245deaf 100644 --- c/plugins/baseplugin/src/main/resources/META-INF/cxs/expressions/mvel.json +++ i/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" -] \ No newline at end of file +] diff --git c/plugins/baseplugin/src/main/resources/META-INF/cxs/rules/sessionAssigned.json i/plugins/baseplugin/src/main/resources/META-INF/cxs/rules/sessionAssigned.json index d453f21..a9714cd 100644 --- c/plugins/baseplugin/src/main/resources/META-INF/cxs/rules/sessionAssigned.json +++ i/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" } ] } diff --git c/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java i/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java index 7dd5db6..9673a70 100644 --- c/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java +++ i/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java @@ -29,17 +29,13 @@ import org.apache.unomi.api.services.DefinitionsService; import org.apache.unomi.api.services.ProfileService; import org.apache.unomi.api.services.SchedulerService; import org.apache.unomi.api.services.SegmentService; +import org.apache.unomi.api.utils.ParserHelper; 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.persistence.spi.aggregate.TermsAggregate; import org.apache.unomi.services.sorts.ControlGroupPersonalizationStrategy; -import org.osgi.framework.Bundle; -import org.osgi.framework.BundleContext; -import org.osgi.framework.BundleEvent; -import org.osgi.framework.InvalidSyntaxException; -import org.osgi.framework.ServiceReference; -import org.osgi.framework.SynchronousBundleListener; +import org.osgi.framework.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,13 +50,15 @@ import static org.apache.unomi.persistence.spi.CustomObjectMapper.getObjectMappe public class ProfileServiceImpl implements ProfileService, SynchronousBundleListener { + private static final String DECREMENT_NB_OF_VISITS_SCRIPT = "decNbOfVisits"; + /** * This class is responsible for storing property types and permits optimized access to them. * In order to assure data consistency, thread-safety and performance, this class is immutable and every operation on * property types requires creating a new instance (copy-on-write). */ private static class PropertyTypes { - private List<PropertyType> allPropertyTypes; + private final List<PropertyType> allPropertyTypes; private Map<String, PropertyType> propertyTypesById = new HashMap<>(); private Map<String, List<PropertyType>> propertyTypesByTags = new HashMap<>(); private Map<String, List<PropertyType>> propertyTypesBySystemTags = new HashMap<>(); @@ -161,6 +159,7 @@ public class ProfileServiceImpl implements ProfileService, SynchronousBundleList } private static final Logger LOGGER = LoggerFactory.getLogger(ProfileServiceImpl.class.getName()); + private static final int NB_OF_VISITS_DECREMENT_BATCH_SIZE = 500; private BundleContext bundleContext; @@ -373,8 +372,105 @@ public class ProfileServiceImpl implements ProfileService, SynchronousBundleList @OverRide public void purgeSessionItems(int existsNumberOfDays) { if (existsNumberOfDays > 0) { + ConditionType sessionPropertyConditionType = definitionsService.getConditionType("sessionPropertyCondition"); + if (sessionPropertyConditionType == null) { + LOGGER.error("Could not find sessionPropertyCondition type"); + return; + } + Condition timeCondition = new Condition(sessionPropertyConditionType); + timeCondition.setParameter("propertyName", "timeStamp"); + timeCondition.setParameter("comparisonOperator", "lessThanOrEqualTo"); + timeCondition.setParameter("propertyValueDateExpr", "now-" + existsNumberOfDays + "d"); + + TermsAggregate profileIdAggregate = new TermsAggregate("profileId"); + Map<String, Long> impactedProfiles = persistenceService.aggregateWithOptimizedQuery(timeCondition, profileIdAggregate, Session.ITEM_TYPE); + // Remove technical aggregation keys like "_filtered" that are not actual profile IDs + impactedProfiles.remove("_filtered"); + LOGGER.info("Purging: Sessions created since more than {} days", existsNumberOfDays); persistenceService.purgeTimeBasedItems(existsNumberOfDays, Session.class); + + LOGGER.info("Syncing profiles: decrementing nbOfVisits for session purge's {} impacted profiles", impactedProfiles.size()); + this.decrementProfilesNbOfVisits(impactedProfiles); + } + } + + /** + * Decrements the nbOfVisits property for profiles based on a map of ProfileId, nbVisits. + * Profiles are grouped by decrement value and processed in batches for optimal performance. + * + * @param profilesVisits Map of profile IDs to the number of visits to decrement + */ + private void decrementProfilesNbOfVisits(Map<String, Long> profilesVisits) { + if (profilesVisits == null || profilesVisits.isEmpty()) { + LOGGER.info("No profiles to update for nbOfVisits decrement"); + return; + } + LOGGER.info("Decrementing nbOfVisits for {} profiles", profilesVisits.size()); + + Map<Long, List<String>> profilesByDecrement = new TreeMap<>(); + profilesVisits.forEach((profileId, decrement) -> { + if (StringUtils.isNotBlank(profileId) && decrement != null && decrement > 0) { + profilesByDecrement.computeIfAbsent(decrement, k -> new ArrayList<>()).add(profileId); + } + }); + + int totalUpdated = 0; + + for (Map.Entry<Long, List<String>> entry : profilesByDecrement.entrySet()) { + Long decrementValue = entry.getKey(); + List<String> profileIds = entry.getValue(); + LOGGER.debug("Processing {} profiles with decrement value {}", profileIds.size(), decrementValue); + // Split into batches of NB_OF_VISITS_DECREMENT_BATCH_SIZE to avoid too large requests + for (int i = 0; i < profileIds.size(); i += NB_OF_VISITS_DECREMENT_BATCH_SIZE) { + int endIndex = Math.min(i + NB_OF_VISITS_DECREMENT_BATCH_SIZE, profileIds.size()); + List<String> batchProfileIds = profileIds.subList(i, endIndex); + if (applyNbOfVisitsDecrementForBatch(batchProfileIds, decrementValue)) { + totalUpdated += batchProfileIds.size(); + } + } + } + LOGGER.info("Successfully decremented nbOfVisits for {} profiles", totalUpdated); + } + + /** + * Applies the nbOfVisits decrement for a batch of profiles with the same decrement value. + * + * @param profileIds List of profile IDs to update + * @param decrementValue The value to decrement from nbOfVisits + * @return true if the update was successful + */ + private boolean applyNbOfVisitsDecrementForBatch(List<String> profileIds, Long decrementValue) { + if (profileIds == null || profileIds.isEmpty() || decrementValue == null || decrementValue <= 0) { + return false; + } + + try { + long startTime = System.currentTimeMillis(); + + String[] scripts = new String[1]; + Map<String, Object>[] scriptParams = new HashMap[1]; + Condition[] conditions = new Condition[1]; + + conditions[0] = new Condition(); + conditions[0].setConditionType(definitionsService.getConditionType("profilePropertyCondition")); + conditions[0].setParameter("propertyName", "itemId"); + conditions[0].setParameter("comparisonOperator", "in"); + conditions[0].setParameter("propertyValues", new ArrayList<>(profileIds)); + scriptParams[0] = new HashMap<>(); + scriptParams[0].put("decrementValue", decrementValue); + scripts[0] = DECREMENT_NB_OF_VISITS_SCRIPT; + + boolean updated = persistenceService.updateWithQueryAndStoredScript(Profile.class, scripts, scriptParams, conditions); + if (!updated) { + LOGGER.error("Failed to decrement nbOfVisits for {} profiles with decrement value {}", profileIds.size(), decrementValue); + } else { + LOGGER.info("Updated nbOfVisits for {} profiles in {}ms", profileIds.size(), System.currentTimeMillis() - startTime); + } + return updated; + } catch (Exception e) { + LOGGER.error("Error while decrementing nbOfVisits for batch of {} profiles", profileIds.size(), e); + return false; } } @@ -752,7 +848,7 @@ public class ProfileServiceImpl implements ProfileService, SynchronousBundleList profilesToMerge = filteredProfilesToMerge; - Set<String> allProfileProperties = new LinkedHashSet<>(); + Set<String> allProfileProperties = new LinkedHashSet(); for (Profile profile : profilesToMerge) { final Set<String> flatNestedPropertiesKeys = PropertyHelper.flatten(profile.getProperties()).keySet(); allProfileProperties.addAll(flatNestedPropertiesKeys); diff --git c/services/src/main/resources/META-INF/cxs/painless/decNbOfVisits.painless i/services/src/main/resources/META-INF/cxs/painless/decNbOfVisits.painless new file mode 100644 index 000000000..f0666ab --- /dev/null +++ i/services/src/main/resources/META-INF/cxs/painless/decNbOfVisits.painless @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + if (ctx._source.properties == null) { + ctx._source.properties = new HashMap(); +} +def current = ctx._source.properties.nbOfVisits; +if (current == null) { + current = 0; +} +long currentValue = (current instanceof Number) ? current.longValue() : Long.parseLong(current.toString()); +long newValue = currentValue - params.decrementValue; +if (newValue < 0) { + newValue = 0; +} +ctx._source.properties.nbOfVisits = newValue; diff --git c/services/src/main/resources/META-INF/cxs/properties/profiles/system/nbOfVisits.json i/services/src/main/resources/META-INF/cxs/properties/profiles/system/nbOfVisits.json index 25180f1..2cc093e 100644 --- c/services/src/main/resources/META-INF/cxs/properties/profiles/system/nbOfVisits.json +++ i/services/src/main/resources/META-INF/cxs/properties/profiles/system/nbOfVisits.json @@ -1,7 +1,7 @@ { "metadata": { "id": "nbOfVisits", - "name": "Number of visits", + "name": "Number of visits since oldest session", "systemTags": [ "properties", "profileProperties", @@ -22,4 +22,4 @@ "mergeStrategy": "addMergeStrategy", "rank": "101.0", "protected": true -} \ No newline at end of file +} diff --git c/services/src/main/resources/META-INF/cxs/properties/profiles/system/totalNbOfVisits.json i/services/src/main/resources/META-INF/cxs/properties/profiles/system/totalNbOfVisits.json new file mode 100644 index 000000000..1d1b42a --- /dev/null +++ i/services/src/main/resources/META-INF/cxs/properties/profiles/system/totalNbOfVisits.json @@ -0,0 +1,25 @@ +{ + "metadata": { + "id": "totalNbOfVisits", + "name": "Total number of visits", + "systemTags": [ + "properties", + "profileProperties", + "systemProfileProperties" + ] + }, + "type": "integer", + "defaultValue": "", + "automaticMappingsFrom": [ ], + "numericRanges": [ + {"key":"*_5", "to" : 5 }, + {"key":"5_10", "from" : 5, "to" : 10 }, + {"key":"10_20", "from" : 10, "to" : 20 }, + {"key":"20_40", "from" : 20, "to" : 40 }, + {"key":"40_80", "from" : 40, "to" : 80 }, + {"key":"100_*", "from" : 100 } + ], + "mergeStrategy": "addMergeStrategy", + "rank": "101.1", + "protected": true +} diff --git c/services/src/main/resources/messages_de.properties i/services/src/main/resources/messages_de.properties index 40f4807..3827d85 100644 --- c/services/src/main/resources/messages_de.properties +++ i/services/src/main/resources/messages_de.properties @@ -56,6 +56,7 @@ profilesProperty.linkedInId=LinkedIn ID profilesProperty.mergedWith=Zusammengef�hrt mit profilesProperty.name=Name profilesProperty.nbOfVisits=Anzahl Besuche +profilesProperty.totalNbOfVisits=Gesamtzahl der Besuche profilesProperty.phoneNumber=Telefonnummer profilesProperty.twitterId=Twitter ID profilesProperty.usageRate=Nutzungsrate diff --git c/services/src/main/resources/messages_en.properties i/services/src/main/resources/messages_en.properties index 62d6ba3..9334e91 100644 --- c/services/src/main/resources/messages_en.properties +++ i/services/src/main/resources/messages_en.properties @@ -60,6 +60,7 @@ profilesProperty.maritalStatus=Marital status profilesProperty.name=Name profilesProperty.nationality=Nationality profilesProperty.nbOfVisits=Number of visits +profilesProperty.totalNbOfVisits=Total number of visits profilesProperty.phoneNumber=Phone number profilesProperty.title=Title profilesProperty.twitterId=Twitter ID diff --git c/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-3.1.0-00-fixProfileNbOfVisits.groovy i/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-3.1.0-00-fixProfileNbOfVisits.groovy new file mode 100644 index 000000000..5aac80a --- /dev/null +++ i/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-3.1.0-00-fixProfileNbOfVisits.groovy @@ -0,0 +1,98 @@ +import groovy.json.JsonSlurper +import org.apache.unomi.shell.migration.service.MigrationContext +import org.apache.unomi.shell.migration.utils.HttpUtils +import org.apache.unomi.shell.migration.utils.MigrationUtils + +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +MigrationContext context = migrationContext +String esAddress = context.getConfigString("esAddress") +String indexPrefix = context.getConfigString("indexPrefix") +def jsonSlurper = new JsonSlurper() + +context.performMigrationStep("3.1.0-fix-profile-nbOfVisits", () -> { + String profileIndex = "${indexPrefix}-profile" + String sessionIndex = "${indexPrefix}-session-*" + + context.printMessage("Starting migration to fix Profile.nbOfVisits field") + + // First step: Copy nbOfVisits to totalNbOfVisits for all profiles + context.printMessage("Step 1: Copying nbOfVisits to totalNbOfVisits") + String copyScript = MigrationUtils.getFileWithoutComments(bundleContext, "requestBody/3.1.0/copy_nbOfVisits_to_totalNbOfVisits.painless") + String copyRequestBody = MigrationUtils.resourceAsString(bundleContext, "requestBody/3.1.0/profile_copy_nbOfVisits_request.json") + MigrationUtils.updateByQuery(context.getHttpClient(), esAddress, profileIndex, copyRequestBody.replace('#painless', copyScript)) + + context.printMessage("Step 1 completed: nbOfVisits copied to totalNbOfVisits") + + // Second step: Update nbOfVisits with actual session count for each profile + context.printMessage("Step 2: Updating nbOfVisits with actual session count") + + String scrollQuery = MigrationUtils.resourceAsString(bundleContext, "requestBody/3.1.0/profile_scroll_query.json") + int profilesProcessed = 0 + int profilesUpdated = 0 + + // Scroll through all profiles + MigrationUtils.scrollQuery(context.getHttpClient(), esAddress, "/${profileIndex}/_search", scrollQuery, "5m", (hits) -> { + def hitsArray = jsonSlurper.parseText(hits) + StringBuilder bulkUpdate = new StringBuilder() + + hitsArray.each { hit -> + String profileId = hit._id + profilesProcessed++ + + if (profilesProcessed % 100 == 0) { + context.printMessage("Processed ${profilesProcessed} profiles...") + } + + // Count sessions for this profile + String countQuery = MigrationUtils.resourceAsString(bundleContext, "requestBody/3.1.0/count_sessions_by_profile.json") + String countQueryWithProfileId = countQuery.replace('#profileId', profileId) + + try { + def countResponse = jsonSlurper.parseText( + HttpUtils.executePostRequest(context.getHttpClient(), esAddress + "/${sessionIndex}/_count", countQueryWithProfileId, null) + ) + + int sessionCount = countResponse.count + + // Prepare bulk update + bulkUpdate.append('{"update":{"_id":"').append(profileId).append('","_index":"').append(profileIndex).append('"}}\n') + bulkUpdate.append('{"doc":{"properties":{"nbOfVisits":').append(sessionCount).append('}}}\n') + + profilesUpdated++ + } catch (Exception e) { + context.printMessage("Error counting sessions for profile ${profileId}: ${e.message}") + } + } + + // Execute bulk update if we have updates + if (bulkUpdate.length() > 0) { + try { + MigrationUtils.bulkUpdate(context.getHttpClient(), esAddress + "/_bulk", bulkUpdate.toString()) + } catch (Exception e) { + context.printMessage("Error during bulk update: ${e.message}") + } + } + }) + + // Refresh the profile index + HttpUtils.executePostRequest(context.getHttpClient(), esAddress + "/${profileIndex}/_refresh", null, null) + + context.printMessage("Migration completed: Processed ${profilesProcessed} profiles, updated ${profilesUpdated} profiles") +}) + diff --git c/tools/shell-commands/src/main/resources/requestBody/3.1.0/copy_nbOfVisits_to_totalNbOfVisits.painless i/tools/shell-commands/src/main/resources/requestBody/3.1.0/copy_nbOfVisits_to_totalNbOfVisits.painless new file mode 100644 index 000000000..52aec85 --- /dev/null +++ i/tools/shell-commands/src/main/resources/requestBody/3.1.0/copy_nbOfVisits_to_totalNbOfVisits.painless @@ -0,0 +1,25 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* Copy nbOfVisits to totalNbOfVisits for all profiles */ + +if (ctx._source.properties != null && ctx._source.properties.containsKey('nbOfVisits')) { + if (ctx._source.properties.totalNbOfVisits == null) { + ctx._source.properties.put('totalNbOfVisits', ctx._source.properties.nbOfVisits); + } +} + diff --git c/tools/shell-commands/src/main/resources/requestBody/3.1.0/count_sessions_by_profile.json i/tools/shell-commands/src/main/resources/requestBody/3.1.0/count_sessions_by_profile.json new file mode 100644 index 000000000..4839ab5 --- /dev/null +++ i/tools/shell-commands/src/main/resources/requestBody/3.1.0/count_sessions_by_profile.json @@ -0,0 +1,19 @@ +{ + "query": { + "bool": { + "must": [ + { + "match": { + "itemType": "session" + } + }, + { + "term": { + "profileId.keyword": "#profileId" + } + } + ] + } + } +} + diff --git c/tools/shell-commands/src/main/resources/requestBody/3.1.0/profile_copy_nbOfVisits_request.json i/tools/shell-commands/src/main/resources/requestBody/3.1.0/profile_copy_nbOfVisits_request.json new file mode 100644 index 000000000..3b41798 --- /dev/null +++ i/tools/shell-commands/src/main/resources/requestBody/3.1.0/profile_copy_nbOfVisits_request.json @@ -0,0 +1,25 @@ +{ + "script": { + "source": "#painless", + "lang": "painless" + }, + "query": { + "bool": { + "must": [ + { + "match": { + "itemType": "profile" + } + } + ], + "filter": [ + { + "exists": { + "field": "properties.nbOfVisits" + } + } + ] + } + } +} + diff --git c/tools/shell-commands/src/main/resources/requestBody/3.1.0/profile_scroll_query.json i/tools/shell-commands/src/main/resources/requestBody/3.1.0/profile_scroll_query.json new file mode 100644 index 000000000..3280fc1 --- /dev/null +++ i/tools/shell-commands/src/main/resources/requestBody/3.1.0/profile_scroll_query.json @@ -0,0 +1,10 @@ +{ + "size": 100, + "query": { + "match": { + "itemType": "profile" + } + }, + "_source": ["itemId"] +} +
jayblanc
force-pushed
the
syncNbOfVisit
branch
from
7648016 to
87b4ff3
Compare
asf-gitbox-commits
pushed a commit
that referenced
this pull request
Jan 10, 2026
Merged latest changes from origin/master including: - [UNOMI-924] Add healthcheck activated by default (#748) - [UNOMI-922] Inconsistency between value for nbOfVisits in Profile and number of Sessions (#744) - chore: github settings to only enable squash commits (#747) Resolved conflicts in: - pom.xml - itests/pom.xml - itests/src/test/java/org/apache/unomi/itests/migration/Migrate16xToCurrentVersionIT.java - services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes an inconsistency where a profile’s
nbOfVisitscould diverge from the actual number of recorded sessions.Motivation & Context
In certain flows, the
Profile.nbOfVisitscounter was not updated in lockstep with session lifecycle, leading to mismatches between the counter on the profile and the number of existing sessions. This addresses [UNOMI-922] by ensuring the visit count remains authoritative and consistent.What’s Changed
Profile.nbOfVisitsupdates with session purge (decrement by the number of deleted sessions)Change stats: 1 commit, 19 files changed, +470 −25.
Breaking Changes
Relates to: UNOMI-922