From 32251df335ad95d6518782335ca5278255bb6877 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Thu, 25 Jun 2026 16:29:57 +0200 Subject: [PATCH 1/4] UNOMI-948: Add PersistenceService.rangeQuery with ES/OS implementations Expose rangeQuery on the persistence SPI and implement it for Elasticsearch, OpenSearch, and the in-memory test harness. Add smoke integration tests and strengthen unit-test pagination assertions for totalSize. Related: UNOMI-956 (full PersistenceService IT coverage follow-up). --- .../java/org/apache/unomi/itests/AllITs.java | 1 + .../unomi/itests/PersistenceServiceIT.java | 115 ++++++++++++++++++ .../ElasticSearchPersistenceServiceImpl.java | 5 + .../OpenSearchPersistenceServiceImpl.java | 5 + .../persistence/spi/PersistenceService.java | 18 +++ .../impl/InMemoryPersistenceServiceImpl.java | 4 +- .../InMemoryPersistenceServiceImplTest.java | 3 + 7 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java diff --git a/itests/src/test/java/org/apache/unomi/itests/AllITs.java b/itests/src/test/java/org/apache/unomi/itests/AllITs.java index b450b3310..ace015dfb 100644 --- a/itests/src/test/java/org/apache/unomi/itests/AllITs.java +++ b/itests/src/test/java/org/apache/unomi/itests/AllITs.java @@ -33,6 +33,7 @@ @SuiteClasses({ Migrate16xToCurrentVersionIT.class, MigrationIT.class, + PersistenceServiceIT.class, BasicIT.class, ConditionEvaluatorIT.class, ConditionQueryBuilderIT.class, diff --git a/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java b/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java new file mode 100644 index 000000000..8ced3264a --- /dev/null +++ b/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java @@ -0,0 +1,115 @@ +/* + * 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 + */ +package org.apache.unomi.itests; + +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.Profile; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; +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 java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +/** + * Integration tests for {@link org.apache.unomi.persistence.spi.PersistenceService} query APIs against the live + * search backend (Elasticsearch or OpenSearch). Initial coverage focuses on {@code rangeQuery}; additional methods + * should be covered in follow-up work (UNOMI-956). + */ +@RunWith(PaxExam.class) +@ExamReactorStrategy(PerSuite.class) +public class PersistenceServiceIT extends BaseIT { + + private static final String AGE_PROPERTY = "properties.age"; + + private final List profileIds = new ArrayList<>(); + + @After + public void tearDown() throws InterruptedException { + for (String profileId : profileIds) { + persistenceService.remove(profileId, Profile.class); + } + profileIds.clear(); + refreshPersistence(Profile.class); + } + + @Test + public void testRangeQueryReturnsProfilesInNumericRange() throws InterruptedException { + saveProfileWithAge("range-query-it-low", 10); + saveProfileWithAge("range-query-it-match-a", 20); + saveProfileWithAge("range-query-it-match-b", 30); + saveProfileWithAge("range-query-it-match-c", 40); + saveProfileWithAge("range-query-it-high", 50); + + refreshPersistence(Profile.class); + + // Upper bound uses "41" so Elasticsearch (exclusive lt) still includes age 40. + PartialList results = keepTrying( + "Range query should return profiles with age between 20 and 40", + () -> persistenceService.rangeQuery(AGE_PROPERTY, "20", "41", AGE_PROPERTY + ":asc", Profile.class, 0, -1), + r -> r != null && r.getList().size() == 3, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + Assert.assertEquals(3, results.getList().size()); + Assert.assertEquals(20, results.getList().get(0).getProperty("age")); + Assert.assertEquals(30, results.getList().get(1).getProperty("age")); + Assert.assertEquals(40, results.getList().get(2).getProperty("age")); + } + + @Test + public void testRangeQuerySupportsPagination() throws InterruptedException { + for (int age = 1; age <= 5; age++) { + saveProfileWithAge("range-query-it-page-" + age, age); + } + + refreshPersistence(Profile.class); + + PartialList firstPage = keepTrying( + "First range query page should be available", + () -> persistenceService.rangeQuery(AGE_PROPERTY, "1", "6", AGE_PROPERTY + ":asc", Profile.class, 0, 2), + r -> r != null && r.getList().size() == 2 && r.getTotalSize() == 5, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + Assert.assertEquals(2, firstPage.getList().size()); + Assert.assertEquals(5, firstPage.getTotalSize()); + Assert.assertEquals(1, firstPage.getList().get(0).getProperty("age")); + Assert.assertEquals(2, firstPage.getList().get(1).getProperty("age")); + + PartialList lastPage = keepTrying( + "Last range query page should be available", + () -> persistenceService.rangeQuery(AGE_PROPERTY, "1", "6", AGE_PROPERTY + ":asc", Profile.class, 4, 2), + r -> r != null && r.getList().size() == 1 && r.getTotalSize() == 5, + DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + Assert.assertEquals(1, lastPage.getList().size()); + Assert.assertEquals(5, lastPage.getTotalSize()); + Assert.assertEquals(5, lastPage.getList().get(0).getProperty("age")); + } + + private void saveProfileWithAge(String idSuffix, int age) { + Profile profile = new Profile(); + profile.setItemId(idSuffix + "-" + UUID.randomUUID()); + profile.setProperty("age", age); + persistenceService.save(profile); + profileIds.add(profile.getItemId()); + } +} diff --git a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java index 386729d50..bc27f6781 100644 --- a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java +++ b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java @@ -2074,6 +2074,11 @@ private String getPropertyNameWithData(String name, String itemType) { return query(Query.of(q -> q.queryString(qs -> qs.query(fulltext))), sortBy, clazz, offset, size, null, null); } + @Override + public PartialList rangeQuery(String fieldName, String from, String to, String sortBy, Class clazz, int offset, int size) { + return query(Query.of(q -> q.range(r -> r.untyped(v -> v.field(fieldName).gte(JsonData.of(from)).lt(JsonData.of(to))))), sortBy, clazz, offset, size, null, null); + } + @Override public long queryCount(Condition query, String itemType) { try { return conditionESQueryBuilderDispatcher.count(query); diff --git a/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java b/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java index 9e2344ed0..b7a639879 100644 --- a/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java +++ b/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java @@ -1948,6 +1948,11 @@ public PartialList queryFullText(String fulltext, String sor return query(Query.of(q->q.queryString(s->s.query(fulltext))), sortBy, clazz, offset, size, null, null); } + @Override + public PartialList rangeQuery(String fieldName, String from, String to, String sortBy, Class clazz, int offset, int size) { + return query(Query.of(q -> q.range(r -> r.field(fieldName).from(JsonData.of(from)).to(JsonData.of(to)))), sortBy, clazz, offset, size, null, null); + } + @Override public long queryCount(Condition query, String itemType) { try { diff --git a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java index 9a99b3420..3b6acfccb 100644 --- a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java +++ b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java @@ -693,6 +693,24 @@ default void refreshIndex(Class clazz) { */ void purgeTimeBasedItems(int existsNumberOfDays, Class clazz); + /** + * Retrieves all items of the specified Item subclass which specified ranged property is within the specified bounds, ordered according to the specified {@code sortBy} String + * and and paged: only {@code size} of them are retrieved, starting with the {@code offset}-th one. + * + * @param the type of the Item subclass we want to retrieve + * @param s the name of the range property we want items to retrieve to be included between the specified start and end points + * @param from the beginning of the range we want to consider + * @param to the end of the range we want to consider + * @param sortBy an optional ({@code null} if no sorting is required) String of comma ({@code ,}) separated property names on which ordering should be performed, ordering + * elements according to the property order in the String, considering each in turn and moving on to the next one in case of equality of all preceding ones. + * Each property name is optionally followed by a column ({@code :}) and an order specifier: {@code asc} or {@code desc}. + * @param clazz the {@link Item} subclass of the items we want to retrieve + * @param offset zero or a positive integer specifying the position of the first item in the total ordered collection of matching items + * @param size a positive integer specifying how many matching items should be retrieved or {@code -1} if all of them should be retrieved + * @return a {@link PartialList} of items matching the specified criteria + */ + PartialList rangeQuery(String s, String from, String to, String sortBy, Class clazz, int offset, int size); + /** * Retrieves the specified metrics for the specified field of items of the specified type as defined by the Item subclass public field {@code ITEM_TYPE} and matching the * specified {@link Condition}. diff --git a/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImpl.java b/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImpl.java index a71c1b604..facd96002 100644 --- a/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImpl.java +++ b/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImpl.java @@ -968,9 +968,7 @@ public PartialList query(String fieldName, String fieldValue return createPartialList(items, offset, size); } - /** - * Test-harness helper for range queries. Not on PersistenceService on master yet. - */ + @Override public PartialList rangeQuery(String fieldName, String from, String to, String sortBy, Class clazz, int offset, int size) { List items = filterItemsByClass(clazz); items = items.stream() diff --git a/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImplTest.java b/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImplTest.java index efcedf92e..0f9e7857f 100644 --- a/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImplTest.java +++ b/services/src/test/java/org/apache/unomi/services/impl/InMemoryPersistenceServiceImplTest.java @@ -3140,6 +3140,7 @@ void shouldHandlePaginationInRangeQueries() { // then assertEquals(2, page1.getList().size()); + assertEquals(5, page1.getTotalSize()); assertEquals(1.0, page1.getList().get(0).getNumericValue()); assertEquals(2.0, page1.getList().get(1).getNumericValue()); @@ -3152,6 +3153,7 @@ void shouldHandlePaginationInRangeQueries() { // then assertEquals(2, page2.getList().size()); + assertEquals(5, page2.getTotalSize()); assertEquals(3.0, page2.getList().get(0).getNumericValue()); assertEquals(4.0, page2.getList().get(1).getNumericValue()); @@ -3164,6 +3166,7 @@ void shouldHandlePaginationInRangeQueries() { // then assertEquals(1, page3.getList().size()); + assertEquals(5, page3.getTotalSize()); assertEquals(5.0, page3.getList().get(0).getNumericValue()); } From 6067ca66e44fbdd488f74a2c7eeb14f5e38f9d81 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Thu, 25 Jun 2026 22:41:47 +0200 Subject: [PATCH 2/4] UNOMI-948: Make rangeQuery upper bound inclusive on both ES and OpenSearch Elasticsearch used an exclusive upper bound (lt) while OpenSearch and the in-memory test harness treated it as inclusive, so the same call could return different results depending on the backend. Align Elasticsearch on the inclusive semantics, guard both implementations against null from/to so an unbounded side isn't serialized as a literal null, rename the SPI's unused "s" parameter to "fieldName", and update the IT to assert the inclusive boundary directly instead of working around the old mismatch. --- .../unomi/itests/PersistenceServiceIT.java | 4 +-- .../ElasticSearchPersistenceServiceImpl.java | 11 ++++++- .../OpenSearchPersistenceServiceImpl.java | 11 ++++++- .../persistence/spi/PersistenceService.java | 29 ++++++++++--------- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java b/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java index 8ced3264a..a2c9e581e 100644 --- a/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java @@ -62,10 +62,10 @@ public void testRangeQueryReturnsProfilesInNumericRange() throws InterruptedExce refreshPersistence(Profile.class); - // Upper bound uses "41" so Elasticsearch (exclusive lt) still includes age 40. + // Both bounds are inclusive, so age 40 (the "to" value) must be included while age 50 is excluded. PartialList results = keepTrying( "Range query should return profiles with age between 20 and 40", - () -> persistenceService.rangeQuery(AGE_PROPERTY, "20", "41", AGE_PROPERTY + ":asc", Profile.class, 0, -1), + () -> persistenceService.rangeQuery(AGE_PROPERTY, "20", "40", AGE_PROPERTY + ":asc", Profile.class, 0, -1), r -> r != null && r.getList().size() == 3, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); diff --git a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java index bc27f6781..efed6c03c 100644 --- a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java +++ b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java @@ -2076,7 +2076,16 @@ private String getPropertyNameWithData(String name, String itemType) { @Override public PartialList rangeQuery(String fieldName, String from, String to, String sortBy, Class clazz, int offset, int size) { - return query(Query.of(q -> q.range(r -> r.untyped(v -> v.field(fieldName).gte(JsonData.of(from)).lt(JsonData.of(to))))), sortBy, clazz, offset, size, null, null); + return query(Query.of(q -> q.range(r -> r.untyped(v -> { + v.field(fieldName); + if (from != null) { + v.gte(JsonData.of(from)); + } + if (to != null) { + v.lte(JsonData.of(to)); + } + return v; + }))), sortBy, clazz, offset, size, null, null); } @Override public long queryCount(Condition query, String itemType) { diff --git a/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java b/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java index b7a639879..42e95e543 100644 --- a/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java +++ b/persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java @@ -1950,7 +1950,16 @@ public PartialList queryFullText(String fulltext, String sor @Override public PartialList rangeQuery(String fieldName, String from, String to, String sortBy, Class clazz, int offset, int size) { - return query(Query.of(q -> q.range(r -> r.field(fieldName).from(JsonData.of(from)).to(JsonData.of(to)))), sortBy, clazz, offset, size, null, null); + return query(Query.of(q -> q.range(r -> { + r.field(fieldName); + if (from != null) { + r.from(JsonData.of(from)); + } + if (to != null) { + r.to(JsonData.of(to)); + } + return r; + })), sortBy, clazz, offset, size, null, null); } @Override diff --git a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java index 3b6acfccb..8e8b22622 100644 --- a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java +++ b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java @@ -695,21 +695,24 @@ default void refreshIndex(Class clazz) { /** * Retrieves all items of the specified Item subclass which specified ranged property is within the specified bounds, ordered according to the specified {@code sortBy} String - * and and paged: only {@code size} of them are retrieved, starting with the {@code offset}-th one. - * - * @param the type of the Item subclass we want to retrieve - * @param s the name of the range property we want items to retrieve to be included between the specified start and end points - * @param from the beginning of the range we want to consider - * @param to the end of the range we want to consider - * @param sortBy an optional ({@code null} if no sorting is required) String of comma ({@code ,}) separated property names on which ordering should be performed, ordering - * elements according to the property order in the String, considering each in turn and moving on to the next one in case of equality of all preceding ones. - * Each property name is optionally followed by a column ({@code :}) and an order specifier: {@code asc} or {@code desc}. - * @param clazz the {@link Item} subclass of the items we want to retrieve - * @param offset zero or a positive integer specifying the position of the first item in the total ordered collection of matching items - * @param size a positive integer specifying how many matching items should be retrieved or {@code -1} if all of them should be retrieved + * and paged: only {@code size} of them are retrieved, starting with the {@code offset}-th one. + *

+ * Both bounds are inclusive: items whose property value equals {@code from} or {@code to} are included in the results. Either bound may be {@code null} to leave that side + * of the range unbounded. + * + * @param the type of the Item subclass we want to retrieve + * @param fieldName the name of the range property we want items to retrieve to be included between the specified start and end points + * @param from the beginning (inclusive) of the range we want to consider, or {@code null} for no lower bound + * @param to the end (inclusive) of the range we want to consider, or {@code null} for no upper bound + * @param sortBy an optional ({@code null} if no sorting is required) String of comma ({@code ,}) separated property names on which ordering should be performed, ordering + * elements according to the property order in the String, considering each in turn and moving on to the next one in case of equality of all preceding ones. + * Each property name is optionally followed by a column ({@code :}) and an order specifier: {@code asc} or {@code desc}. + * @param clazz the {@link Item} subclass of the items we want to retrieve + * @param offset zero or a positive integer specifying the position of the first item in the total ordered collection of matching items + * @param size a positive integer specifying how many matching items should be retrieved or {@code -1} if all of them should be retrieved * @return a {@link PartialList} of items matching the specified criteria */ - PartialList rangeQuery(String s, String from, String to, String sortBy, Class clazz, int offset, int size); + PartialList rangeQuery(String fieldName, String from, String to, String sortBy, Class clazz, int offset, int size); /** * Retrieves the specified metrics for the specified field of items of the specified type as defined by the Item subclass public field {@code ITEM_TYPE} and matching the From 2009389a6f791d5e4481cf8ed9cea187238910c9 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Sat, 27 Jun 2026 07:03:33 +0200 Subject: [PATCH 3/4] UNOMI-948: Fix flaky ClusterServiceImplTest purge tests testPurgeByDateDelegatesToPersistenceService and testPurgeByScopeDelegatesToPersistenceService build ClusterNode fixtures without setting lastHeartbeat, which defaults to 0. The real scheduler set up in @BeforeEach fires cleanupStaleNodes() with initialDelay=0, so it can race the test and delete these fixtures as "stale" before the assertions run, intermittently on CI. Apply the same clusterService.cancelScheduledTasks() fix already used in testCleanupStaleNodes to both tests. --- .../services/impl/cluster/ClusterServiceImplTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/services/src/test/java/org/apache/unomi/services/impl/cluster/ClusterServiceImplTest.java b/services/src/test/java/org/apache/unomi/services/impl/cluster/ClusterServiceImplTest.java index 432bf165f..aa7acedd8 100644 --- a/services/src/test/java/org/apache/unomi/services/impl/cluster/ClusterServiceImplTest.java +++ b/services/src/test/java/org/apache/unomi/services/impl/cluster/ClusterServiceImplTest.java @@ -362,6 +362,11 @@ public void testCleanupStaleNodes() { @Test public void testPurgeByDateDelegatesToPersistenceService() { + // Cancel the background cluster tasks to prevent a race: cleanupStaleNodes() has + // initialDelay=0 and would treat these fixtures (no lastHeartbeat set, defaults to 0) + // as stale, deleting them before/while this test's own purge(Date) assertions run. + clusterService.cancelScheduledTasks(); + // Setup: create items with different creation dates in real persistence executionContextManager.executeAsSystem(() -> { ClusterNode oldNode = new ClusterNode(); @@ -390,6 +395,11 @@ public void testPurgeByDateDelegatesToPersistenceService() { @Test public void testPurgeByScopeDelegatesToPersistenceService() { + // Cancel the background cluster tasks to prevent a race: cleanupStaleNodes() has + // initialDelay=0 and would treat these fixtures (no lastHeartbeat set, defaults to 0) + // as stale, deleting them before/while this test's own purge(String) assertions run. + clusterService.cancelScheduledTasks(); + // Setup: create two nodes with different scopes in real persistence executionContextManager.executeAsSystem(() -> { ClusterNode scopedNode = new ClusterNode(); From 89db931beed112443b95d944d31645e540352d45 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Sat, 27 Jun 2026 08:59:19 +0200 Subject: [PATCH 4/4] UNOMI-948: Stop scheduling cluster tasks eagerly in ClusterServiceImplTest setUp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous fix (clusterService.cancelScheduledTasks() inside the two purge tests) didn't actually close the race: cancelTask() only prevents future executions, so if the zero-initial-delay task had already been dispatched to the scheduler's thread pool before the test method's first line ran, cancelling did nothing. setUp() was the one starting these tasks unconditionally, before any test method body got a chance to run. None of the tests actually need the tasks pre-started by setUp() — every test that exercises scheduled-task behavior already calls clusterService.init() itself, which starts them at the right time. Removing the eager start from setUp() closes the race for all tests in the class, not just the two that were observed failing. --- .../impl/cluster/ClusterServiceImplTest.java | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/services/src/test/java/org/apache/unomi/services/impl/cluster/ClusterServiceImplTest.java b/services/src/test/java/org/apache/unomi/services/impl/cluster/ClusterServiceImplTest.java index aa7acedd8..435360e48 100644 --- a/services/src/test/java/org/apache/unomi/services/impl/cluster/ClusterServiceImplTest.java +++ b/services/src/test/java/org/apache/unomi/services/impl/cluster/ClusterServiceImplTest.java @@ -107,8 +107,12 @@ public void setUp() { // Set scheduler in cluster service - this would normally be done by OSGi but we need to do it manually in tests clusterService.setSchedulerService(schedulerService); - // Explicitly initialize scheduled tasks to handle the circular dependency properly - clusterService.initializeScheduledTasks(); + // Note: scheduled tasks are intentionally NOT started here. clusterStaleNodesCleanup and + // clusterNodeStatisticsUpdate both have initialDelay=0, so starting them eagerly in every + // test's setUp() raced the test body on a real background thread pool and could delete + // freshly-saved fixtures before assertions ran (flaky only under CI-like scheduling/timing). + // Tests that actually exercise scheduled-task behavior call clusterService.init() themselves, + // which starts them at the right time. } @Test @@ -362,11 +366,6 @@ public void testCleanupStaleNodes() { @Test public void testPurgeByDateDelegatesToPersistenceService() { - // Cancel the background cluster tasks to prevent a race: cleanupStaleNodes() has - // initialDelay=0 and would treat these fixtures (no lastHeartbeat set, defaults to 0) - // as stale, deleting them before/while this test's own purge(Date) assertions run. - clusterService.cancelScheduledTasks(); - // Setup: create items with different creation dates in real persistence executionContextManager.executeAsSystem(() -> { ClusterNode oldNode = new ClusterNode(); @@ -395,11 +394,6 @@ public void testPurgeByDateDelegatesToPersistenceService() { @Test public void testPurgeByScopeDelegatesToPersistenceService() { - // Cancel the background cluster tasks to prevent a race: cleanupStaleNodes() has - // initialDelay=0 and would treat these fixtures (no lastHeartbeat set, defaults to 0) - // as stale, deleting them before/while this test's own purge(String) assertions run. - clusterService.cancelScheduledTasks(); - // Setup: create two nodes with different scopes in real persistence executionContextManager.executeAsSystem(() -> { ClusterNode scopedNode = new ClusterNode();