This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch UNOMI-999-scope-purge in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 4ae92b426e75b57fbb60f6eb0b070689257da54b Author: Serge Huber <[email protected]> AuthorDate: Tue Oct 6 09:25:41 2026 +0200 UNOMI-999: Keep scope purge limited to one scope --- .../apache/unomi/itests/PersistenceServiceIT.java | 85 ++++++++++++++++++- .../ElasticSearchPersistenceServiceImpl.java | 20 ++++- .../ElasticSearchScopePurgeQueryTest.java | 98 ++++++++++++++++++++++ .../OpenSearchPersistenceServiceImpl.java | 3 + .../impl/InMemoryPersistenceServiceImplTest.java | 9 +- 5 files changed, 208 insertions(+), 7 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 a2c9e581e..55d5dc9ed 100644 --- a/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java @@ -16,8 +16,10 @@ */ package org.apache.unomi.itests; +import org.apache.unomi.api.Event; import org.apache.unomi.api.PartialList; import org.apache.unomi.api.Profile; +import org.apache.unomi.api.tenants.Tenant; import org.junit.After; import org.junit.Assert; import org.junit.Test; @@ -27,13 +29,14 @@ import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy; import org.ops4j.pax.exam.spi.reactors.PerSuite; import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; 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). + * Integration tests for {@link org.apache.unomi.persistence.spi.PersistenceService} against the live + * search backend (Elasticsearch or OpenSearch). */ @RunWith(PaxExam.class) @ExamReactorStrategy(PerSuite.class) @@ -42,14 +45,19 @@ public class PersistenceServiceIT extends BaseIT { private static final String AGE_PROPERTY = "properties.age"; private final List<String> profileIds = new ArrayList<>(); + private final List<String> eventIds = new ArrayList<>(); @After public void tearDown() throws InterruptedException { + for (String eventId : eventIds) { + persistenceService.remove(eventId, Event.class); + } + eventIds.clear(); for (String profileId : profileIds) { persistenceService.remove(profileId, Profile.class); } profileIds.clear(); - refreshPersistence(Profile.class); + refreshPersistence(Event.class, Profile.class); } @Test @@ -105,6 +113,75 @@ public class PersistenceServiceIT extends BaseIT { Assert.assertEquals(5, lastPage.getList().get(0).getProperty("age")); } + @Test + public void testPurgeByScopeKeepsOtherScopesAndProfiles() throws InterruptedException { + String suffix = UUID.randomUUID().toString(); + String dropScope = "purge-drop-" + suffix; + String keepScope = "purge-keep-" + suffix; + Profile profile = saveProfile("purge-profile-" + suffix); + Event dropEvent = saveViewEvent("purge-drop-event-" + suffix, profile, dropScope); + Event keepEvent = saveViewEvent("purge-keep-event-" + suffix, profile, keepScope); + refreshPersistence(Event.class, Profile.class); + + persistenceService.purge(dropScope); + refreshPersistence(Event.class, Profile.class); + + Assert.assertNull("Events of the purged scope should be gone", persistenceService.load(dropEvent.getItemId(), Event.class)); + Assert.assertNotNull("Events of another scope should stay", persistenceService.load(keepEvent.getItemId(), Event.class)); + Assert.assertNotNull("Profiles have no scope and must stay", persistenceService.load(profile.getItemId(), Profile.class)); + } + + @Test + public void testPurgeByScopeDoesNotRemoveAnotherTenantsEvents() throws Exception { + String suffix = UUID.randomUUID().toString(); + String scope = "purge-shared-scope-" + suffix; + Tenant otherTenant = tenantService.createTenant("purge-other-" + suffix.substring(0, 8), Collections.emptyMap()); + Event localEvent = saveViewEvent("purge-local-event-" + suffix, saveProfile("purge-local-profile-" + suffix), scope); + Event otherEvent = executionContextManager.executeAsTenant(otherTenant.getItemId(), () -> { + Profile otherProfile = saveProfile("purge-other-profile-" + suffix); + return saveViewEvent("purge-other-event-" + suffix, otherProfile, scope); + }); + refreshPersistence(Event.class, Profile.class); + + try { + persistenceService.purge(scope); + refreshPersistence(Event.class, Profile.class); + + Assert.assertNull("Current tenant events in the scope should be gone", + persistenceService.load(localEvent.getItemId(), Event.class)); + executionContextManager.executeAsTenant(otherTenant.getItemId(), () -> { + Assert.assertNotNull("Other tenant events in a scope with the same name should stay", + persistenceService.load(otherEvent.getItemId(), Event.class)); + }); + } finally { + tenantService.deleteTenant(otherTenant.getItemId()); + } + } + + @Test + public void testPurgeByScopeRejectsBlankScope() { + try { + persistenceService.purge(" "); + Assert.fail("A blank scope should be refused"); + } catch (IllegalArgumentException expected) { + Assert.assertTrue(expected.getMessage().contains("required")); + } + } + + private Profile saveProfile(String itemId) { + Profile profile = new Profile(itemId); + persistenceService.save(profile); + profileIds.add(itemId); + return profile; + } + + private Event saveViewEvent(String itemId, Profile profile, String scope) { + Event event = new Event(itemId, "view", null, profile, scope, null, null, new Date()); + persistenceService.save(event); + eventIds.add(itemId); + return event; + } + private void saveProfileWithAge(String idSuffix, int age) { Profile profile = new Profile(); profile.setItemId(idSuffix + "-" + UUID.randomUUID()); 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 f6d66d37f..c4ada031a 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 @@ -2726,12 +2726,11 @@ public class ElasticSearchPersistenceServiceImpl implements PersistenceService, } @Override public void purge(final String scope) { + Query query = buildScopePurgeQuery(scope, validateTenantAndGetId(SecurityServiceConfiguration.PERMISSION_PURGE)); LOGGER.debug("Purge scope {}", scope); - String finalTenantId = validateTenantAndGetId(SecurityServiceConfiguration.PERMISSION_PURGE); new InClassLoaderExecute<Void>(metricsService, this.getClass().getName() + ".purgeWithScope", this.bundleContext, this.fatalIllegalStateErrors, throwExceptions) { @Override protected Void execute(Object... args) throws IOException { - Query query = TermQuery.of(builder -> builder.field("scope").value(scope).field("tenantId").value(ConditionContextHelper.foldToASCII(finalTenantId)))._toQuery(); List<BulkOperation> operations = new ArrayList<>(); @@ -2777,6 +2776,23 @@ public class ElasticSearchPersistenceServiceImpl implements PersistenceService, }.catchingExecuteInClassLoader(true); } + /** + * Builds the search used by {@link #purge(String)}. + * A {@code TermQuery} has a single field, so both filters must be bool must clauses. + */ + static Query buildScopePurgeQuery(String scope, String tenantId) { + if (scope == null || scope.isBlank()) { + throw new IllegalArgumentException("A scope is required to purge by scope"); + } + if (tenantId == null || tenantId.isBlank()) { + throw new IllegalArgumentException("A tenant is required to purge by scope"); + } + return Query.of(q -> q.bool(b -> b + .must(m -> m.term(t -> t.field("scope").value(scope))) + .must(m -> m.term(t -> t.field("tenantId") + .value(ConditionContextHelper.foldToASCII(tenantId)))))); + } + @Override public Map<String, Double> getSingleValuesMetrics(final Condition condition, final String[] metrics, final String field, final String itemType) { return new InClassLoaderExecute<Map<String, Double>>(metricsService, this.getClass().getName() + ".getSingleValuesMetrics", diff --git a/persistence-elasticsearch/core/src/test/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchScopePurgeQueryTest.java b/persistence-elasticsearch/core/src/test/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchScopePurgeQueryTest.java new file mode 100644 index 000000000..baf27bc8a --- /dev/null +++ b/persistence-elasticsearch/core/src/test/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchScopePurgeQueryTest.java @@ -0,0 +1,98 @@ +/* + * 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.persistence.elasticsearch; + +import co.elastic.clients.elasticsearch._types.query_dsl.Query; +import co.elastic.clients.json.jackson.JacksonJsonpMapper; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import jakarta.json.stream.JsonGenerator; +import org.junit.Test; + +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; +import java.util.HashSet; +import java.util.Set; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class ElasticSearchScopePurgeQueryTest { + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + @Test + public void buildScopePurgeQueryRequiresBothScopeAndTenantTerms() throws Exception { + Query query = ElasticSearchPersistenceServiceImpl.buildScopePurgeQuery("drop_scope", "tenant_a"); + JsonNode json = OBJECT_MAPPER.readTree(toJson(query)); + + JsonNode must = json.path("bool").path("must"); + assertTrue("Purge query must be a bool query with must clauses: " + json, must.isArray()); + assertEquals(2, must.size()); + + Set<String> fields = new HashSet<>(); + for (JsonNode clause : must) { + JsonNode term = clause.path("term"); + assertTrue("Each must clause should be a term query: " + json, term.isObject() && term.size() == 1); + fields.add(term.fieldNames().next()); + } + assertEquals(Set.of("scope", "tenantId"), fields); + assertEquals("drop_scope", termValue(must, "scope")); + assertEquals("tenant_a", termValue(must, "tenantId")); + } + + @Test + public void buildScopePurgeQueryRejectsBlankScope() { + assertIllegalArgument(() -> ElasticSearchPersistenceServiceImpl.buildScopePurgeQuery(" ", "tenant_a")); + assertIllegalArgument(() -> ElasticSearchPersistenceServiceImpl.buildScopePurgeQuery(null, "tenant_a")); + assertIllegalArgument(() -> ElasticSearchPersistenceServiceImpl.buildScopePurgeQuery("drop_scope", " ")); + assertIllegalArgument(() -> ElasticSearchPersistenceServiceImpl.buildScopePurgeQuery("drop_scope", null)); + } + + private static String termValue(JsonNode must, String field) { + for (JsonNode clause : must) { + JsonNode term = clause.path("term").path(field); + if (term.has("value")) { + return term.get("value").asText(); + } + if (term.isTextual()) { + return term.asText(); + } + } + fail("No term for field " + field); + return null; + } + + private static String toJson(Query query) { + JacksonJsonpMapper mapper = new JacksonJsonpMapper(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + JsonGenerator generator = mapper.jsonProvider().createGenerator(out); + query.serialize(generator, mapper); + generator.close(); + return out.toString(StandardCharsets.UTF_8); + } + + private static void assertIllegalArgument(Runnable action) { + try { + action.run(); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException expected) { + assertTrue(expected.getMessage().contains("required")); + } + } +} 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 4afc072ca..8832c05c7 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 @@ -2666,6 +2666,9 @@ public class OpenSearchPersistenceServiceImpl implements PersistenceService, Syn @Override public void purge(final String scope) { + if (scope == null || scope.isBlank()) { + throw new IllegalArgumentException("A scope is required to purge by scope"); + } String finalTenantId = validateTenantAndGetId(SecurityServiceConfiguration.PERMISSION_PURGE); LOGGER.debug("Purge scope {}", scope); 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 9d1fc916b..8546f47d4 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 @@ -3944,9 +3944,16 @@ public class InMemoryPersistenceServiceImplTest { assertNull(persistenceService.load("purge-null-test", TestMetadataItem.class), "purge(null) should purge all items (used for test isolation reset)"); - // Test purge with null scope — also a no-op assertDoesNotThrow(() -> persistenceService.purge((String) null)); + TestMetadataItem keepOnBlank = new TestMetadataItem(); + keepOnBlank.setItemId("purge-blank-scope"); + keepOnBlank.setScope("scope1"); + persistenceService.save(keepOnBlank); + persistenceService.purge(" "); + assertNotNull(persistenceService.load("purge-blank-scope", TestMetadataItem.class), + "A blank scope must not delete items"); + // Test refresh index with null class assertDoesNotThrow(() -> persistenceService.refreshIndex(null, null));
