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
The following commit(s) were added to refs/heads/UNOMI-999-scope-purge by this
push:
new fbb3295b0 UNOMI-999: Purge whole scopes and match folded scope names
fbb3295b0 is described below
commit fbb3295b069b616baa731aad5d12bad625df3ca5
Author: Serge Huber <[email protected]>
AuthorDate: Tue Oct 6 10:27:40 2026 +0200
UNOMI-999: Purge whole scopes and match folded scope names
* Elasticsearch read only the first 100 items of a scope on every scroll
page, so larger scopes were purged in part. Read each page instead.
* Fold the scope in the purge query on Elasticsearch and OpenSearch, as
is already done for the tenant. A scope with upper case or accented
letters matched nothing before.
* Build the OpenSearch purge query in a helper that mirrors the
Elasticsearch one, and check the permission before the scope on both.
* Refuse a null or blank scope in the in-memory test implementation too.
* Document the tenant limit and the refused blank scope on the SPI.
* Tests: OpenSearch query unit test, folding unit tests, integration
tests for a scope of 250 events and a mixed-case accented scope. The
other-tenant test now removes its own items.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---
.../apache/unomi/itests/PersistenceServiceIT.java | 63 +++++++++++++++---
.../ElasticSearchPersistenceServiceImpl.java | 10 +--
.../ElasticSearchScopePurgeQueryTest.java | 11 +++-
.../OpenSearchPersistenceServiceImpl.java | 40 ++++++------
.../opensearch/OpenSearchScopePurgeQueryTest.java | 74 ++++++++++++++++++++++
.../unomi/persistence/spi/PersistenceService.java | 5 +-
.../impl/InMemoryPersistenceServiceImpl.java | 4 +-
.../impl/InMemoryPersistenceServiceImplTest.java | 12 +---
8 files changed, 173 insertions(+), 46 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 55d5dc9ed..05d00aef4 100644
--- a/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java
+++ b/itests/src/test/java/org/apache/unomi/itests/PersistenceServiceIT.java
@@ -135,15 +135,19 @@ public class PersistenceServiceIT extends BaseIT {
public void testPurgeByScopeDoesNotRemoveAnotherTenantsEvents() throws
Exception {
String suffix = UUID.randomUUID().toString();
String scope = "purge-shared-scope-" + suffix;
+ // Items of the other tenant are not tracked for tearDown, which runs
as the current tenant and would not find them
+ String otherProfileId = "purge-other-profile-" + suffix;
+ String otherEventId = "purge-other-event-" + 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 {
+ Event localEvent = saveViewEvent("purge-local-event-" + suffix,
saveProfile("purge-local-profile-" + suffix), scope);
+ executionContextManager.executeAsTenant(otherTenant.getItemId(),
() -> {
+ Profile otherProfile = new Profile(otherProfileId);
+ persistenceService.save(otherProfile);
+ persistenceService.save(new Event(otherEventId, "view", null,
otherProfile, scope, null, null, new Date()));
+ });
+ refreshPersistence(Event.class, Profile.class);
+
persistenceService.purge(scope);
refreshPersistence(Event.class, Profile.class);
@@ -151,13 +155,54 @@ public class PersistenceServiceIT extends BaseIT {
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));
+ persistenceService.load(otherEventId, Event.class));
});
} finally {
- tenantService.deleteTenant(otherTenant.getItemId());
+ try {
+
executionContextManager.executeAsTenant(otherTenant.getItemId(), () -> {
+ persistenceService.remove(otherEventId, Event.class);
+ persistenceService.remove(otherProfileId, Profile.class);
+ });
+ } finally {
+ tenantService.deleteTenant(otherTenant.getItemId());
+ }
+ }
+ }
+
+ @Test
+ public void testPurgeByScopeRemovesMoreThanOnePageOfItems() throws
InterruptedException {
+ String suffix = UUID.randomUUID().toString();
+ String scope = "purge-many-" + suffix;
+ Profile profile = saveProfile("purge-many-profile-" + suffix);
+ // purge reads matching items 100 at a time
+ int eventCount = 250;
+ for (int i = 0; i < eventCount; i++) {
+ saveViewEvent("purge-many-event-" + i + "-" + suffix, profile,
scope);
+ }
+ refreshPersistence(Event.class, Profile.class);
+
+ persistenceService.purge(scope);
+ refreshPersistence(Event.class, Profile.class);
+
+ for (int i = 0; i < eventCount; i++) {
+ Assert.assertNull("Event " + i + " of the purged scope should be
gone",
+ persistenceService.load("purge-many-event-" + i + "-" +
suffix, Event.class));
}
}
+ @Test
+ public void testPurgeByScopeMatchesMixedCaseAndAccentedScope() throws
InterruptedException {
+ String suffix = UUID.randomUUID().toString();
+ String scope = "Purge-Mixed-Scopé-" + suffix;
+ Event event = saveViewEvent("purge-mixed-event-" + suffix,
saveProfile("purge-mixed-profile-" + suffix), scope);
+ refreshPersistence(Event.class, Profile.class);
+
+ persistenceService.purge(scope);
+ refreshPersistence(Event.class, Profile.class);
+
+ Assert.assertNull("Events of the purged scope should be gone",
persistenceService.load(event.getItemId(), Event.class));
+ }
+
@Test
public void testPurgeByScopeRejectsBlankScope() {
try {
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 c4ada031a..fea126b58 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,12 @@ public class ElasticSearchPersistenceServiceImpl
implements PersistenceService,
}
@Override public void purge(final String scope) {
- Query query = buildScopePurgeQuery(scope,
validateTenantAndGetId(SecurityServiceConfiguration.PERMISSION_PURGE));
+ String finalTenantId =
validateTenantAndGetId(SecurityServiceConfiguration.PERMISSION_PURGE);
+ Query query = buildScopePurgeQuery(scope, finalTenantId);
LOGGER.debug("Purge scope {}", scope);
new InClassLoaderExecute<Void>(metricsService,
this.getClass().getName() + ".purgeWithScope", this.bundleContext,
this.fatalIllegalStateErrors, throwExceptions) {
@Override protected Void execute(Object... args) throws
IOException {
-
List<BulkOperation> operations = new ArrayList<>();
Time keepAlive = Time.of(t -> t.time("1h"));
@@ -2744,7 +2744,7 @@ public class ElasticSearchPersistenceServiceImpl
implements PersistenceService,
String scrollId = searchResponse.scrollId();
// Scroll until no more hits are returned
while (!hits.isEmpty()) {
- for (Hit<JsonData> hit : searchResponse.hits().hits()) {
+ for (Hit<JsonData> hit : hits) {
// add hit to bulk delete
operations.add(BulkOperation.of(builder ->
builder.delete(d -> d.index(hit.index()).id(hit.id()))));
}
@@ -2779,6 +2779,8 @@ public class ElasticSearchPersistenceServiceImpl
implements PersistenceService,
/**
* Builds the search used by {@link #purge(String)}.
* A {@code TermQuery} has a single field, so both filters must be bool
must clauses.
+ * Both values are folded, as the fields are indexed with the folding
analyzer.
+ * Keep in step with the OpenSearch implementation.
*/
static Query buildScopePurgeQuery(String scope, String tenantId) {
if (scope == null || scope.isBlank()) {
@@ -2788,7 +2790,7 @@ public class ElasticSearchPersistenceServiceImpl
implements PersistenceService,
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("scope").value(ConditionContextHelper.foldToASCII(scope))))
.must(m -> m.term(t -> t.field("tenantId")
.value(ConditionContextHelper.foldToASCII(tenantId))))));
}
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
index baf27bc8a..83beef4c3 100644
---
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
@@ -57,7 +57,16 @@ public class ElasticSearchScopePurgeQueryTest {
}
@Test
- public void buildScopePurgeQueryRejectsBlankScope() {
+ public void buildScopePurgeQueryFoldsScopeAndTenant() throws Exception {
+ Query query =
ElasticSearchPersistenceServiceImpl.buildScopePurgeQuery("Drop_Scopé",
"Tenant_A");
+ JsonNode must =
OBJECT_MAPPER.readTree(toJson(query)).path("bool").path("must");
+
+ assertEquals("drop_scope", termValue(must, "scope"));
+ assertEquals("tenant_a", termValue(must, "tenantId"));
+ }
+
+ @Test
+ public void buildScopePurgeQueryRejectsBlankScopeOrTenant() {
assertIllegalArgument(() ->
ElasticSearchPersistenceServiceImpl.buildScopePurgeQuery(" ", "tenant_a"));
assertIllegalArgument(() ->
ElasticSearchPersistenceServiceImpl.buildScopePurgeQuery(null, "tenant_a"));
assertIllegalArgument(() ->
ElasticSearchPersistenceServiceImpl.buildScopePurgeQuery("drop_scope", " "));
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 8832c05c7..5b4270f57 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,10 +2666,8 @@ 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);
+ Query query = buildScopePurgeQuery(scope, finalTenantId);
LOGGER.debug("Purge scope {}", scope);
new InClassLoaderExecute<Void>(metricsService,
this.getClass().getName() + ".purgeWithScope", this.bundleContext,
this.fatalIllegalStateErrors, throwExceptions) {
@@ -2677,22 +2675,7 @@ public class OpenSearchPersistenceServiceImpl implements
PersistenceService, Syn
protected Void execute(Object... args) throws IOException {
SearchResponse<Item> response = client.search(s -> s
- .query(q -> q
- .bool(b -> b
- .must(m -> m
- .term(t -> t
- .field("scope")
- .value(v ->
v.stringValue(scope))
- )
- )
- .must(m -> m
- .term(t -> t
- .field("tenantId")
- .value(v ->
v.stringValue(ConditionContextHelper.foldToASCII(finalTenantId)))
- )
- )
- )
- )
+ .query(query)
.size(100)
.scroll(scr -> scr
.time("1h")
@@ -2733,6 +2716,25 @@ public class OpenSearchPersistenceServiceImpl implements
PersistenceService, Syn
}.catchingExecuteInClassLoader(true);
}
+ /**
+ * Builds the search used by {@link #purge(String)}.
+ * Both values are folded, as the fields are indexed with the folding
analyzer.
+ * Keep in step with the Elasticsearch implementation.
+ */
+ 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(v ->
v.stringValue(ConditionContextHelper.foldToASCII(scope)))))
+ .must(m -> m.term(t -> t.field("tenantId")
+ .value(v ->
v.stringValue(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", this.bundleContext,
this.fatalIllegalStateErrors, throwExceptions) {
diff --git
a/persistence-opensearch/core/src/test/java/org/apache/unomi/persistence/opensearch/OpenSearchScopePurgeQueryTest.java
b/persistence-opensearch/core/src/test/java/org/apache/unomi/persistence/opensearch/OpenSearchScopePurgeQueryTest.java
new file mode 100644
index 000000000..d7d5b18f6
--- /dev/null
+++
b/persistence-opensearch/core/src/test/java/org/apache/unomi/persistence/opensearch/OpenSearchScopePurgeQueryTest.java
@@ -0,0 +1,74 @@
+/*
+ * 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.opensearch;
+
+import org.junit.jupiter.api.Test;
+import org.opensearch.client.opensearch._types.query_dsl.Query;
+import org.opensearch.client.opensearch._types.query_dsl.TermQuery;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class OpenSearchScopePurgeQueryTest {
+
+ @Test
+ public void buildScopePurgeQueryRequiresBothScopeAndTenantTerms() {
+ Query query =
OpenSearchPersistenceServiceImpl.buildScopePurgeQuery("drop_scope", "tenant_a");
+
+ assertEquals(Map.of("scope", "drop_scope", "tenantId", "tenant_a"),
mustTerms(query));
+ }
+
+ @Test
+ public void buildScopePurgeQueryFoldsScopeAndTenant() {
+ Query query =
OpenSearchPersistenceServiceImpl.buildScopePurgeQuery("Drop_Scopé", "Tenant_A");
+
+ assertEquals(Map.of("scope", "drop_scope", "tenantId", "tenant_a"),
mustTerms(query));
+ }
+
+ @Test
+ public void buildScopePurgeQueryRejectsBlankScopeOrTenant() {
+ assertRequired("scope", () ->
OpenSearchPersistenceServiceImpl.buildScopePurgeQuery(" ", "tenant_a"));
+ assertRequired("scope", () ->
OpenSearchPersistenceServiceImpl.buildScopePurgeQuery(null, "tenant_a"));
+ assertRequired("tenant", () ->
OpenSearchPersistenceServiceImpl.buildScopePurgeQuery("drop_scope", " "));
+ assertRequired("tenant", () ->
OpenSearchPersistenceServiceImpl.buildScopePurgeQuery("drop_scope", null));
+ }
+
+ private static Map<String, String> mustTerms(Query query) {
+ assertTrue(query.isBool(), "Purge query must be a bool query: " +
query);
+ List<Query> must = query.bool().must();
+ assertEquals(2, must.size());
+ assertTrue(query.bool().should().isEmpty() &&
query.bool().mustNot().isEmpty());
+
+ Map<String, String> terms = new HashMap<>();
+ for (Query clause : must) {
+ assertTrue(clause.isTerm(), "Each must clause should be a term
query: " + query);
+ TermQuery term = clause.term();
+ terms.put(term.field(), term.value().stringValue());
+ }
+ return terms;
+ }
+
+ private static void assertRequired(String what, Runnable action) {
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class, action::run);
+ assertTrue(e.getMessage().contains(what) &&
e.getMessage().contains("required"), e.getMessage());
+ }
+}
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 b572ab129..5289917e0 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
@@ -790,9 +790,10 @@ public interface PersistenceService {
boolean removeIndex(final String itemType);
/**
- * Removes all data associated with the provided scope.
+ * Removes all data associated with the provided scope, for the current
tenant only.
*
- * @param scope the scope for which we want to remove data
+ * @param scope the scope for which we want to remove data, must not be
null or blank
+ * @throws IllegalArgumentException if the scope is null or blank
*/
void purge(final String scope);
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 38b7fb136..210f6913c 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
@@ -1193,8 +1193,8 @@ public class InMemoryPersistenceServiceImpl implements
PersistenceService {
@Override
public void purge(String scope) {
- if (scope == null) {
- return;
+ if (scope == null || scope.isBlank()) {
+ throw new IllegalArgumentException("A scope is required to purge
by scope");
}
String currentTenantId =
executionContextManager.getCurrentContext().getTenantId();
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 8546f47d4..3d730133d 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,15 +3944,9 @@ public class InMemoryPersistenceServiceImplTest {
assertNull(persistenceService.load("purge-null-test",
TestMetadataItem.class),
"purge(null) should purge all items (used for test isolation
reset)");
- 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");
+ // A null or blank scope is refused, as in the Elasticsearch and
OpenSearch implementations
+ assertThrows(IllegalArgumentException.class, () ->
persistenceService.purge((String) null));
+ assertThrows(IllegalArgumentException.class, () ->
persistenceService.purge(" "));
// Test refresh index with null class
assertDoesNotThrow(() -> persistenceService.refreshIndex(null,
null));
