This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch UNOMI-997-migration-tenants
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/UNOMI-997-migration-tenants by
this push:
new 026c6502a UNOMI-997: Address review findings on tenant assignment
026c6502a is described below
commit 026c6502a4ac31cf2fbb9ae9000f05fe92bef41f
Author: Serge Huber <[email protected]>
AuthorDate: Wed Oct 7 09:51:32 2026 +0200
UNOMI-997: Address review findings on tenant assignment
Keep the creation date and version, not only the creator, when a bundle
redeploys a migrated definition. Do not add the _itemtype suffix twice
to system item ids that already carry it from 3.0.
Make the Painless script the only tenant decision and drop the unused
Java copy. Group shipped ids by item type, reject an empty or malformed
list, add the shipped personas, and add a test that fails when the list
no longer matches the definitions in the source tree.
Fail the id-fix step when an item type fails, stop on conflicts, and
count only documents that changed. Cover the missing cases in the
tenant assignment integration test.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---
.../itests/migration/Migrate40IndexMappingIT.java | 2 +-
.../migration/Migrate40TenantAssignmentIT.java | 325 ++++++++++++---------
.../cache/AbstractMultiTypeCachingService.java | 19 +-
.../cache/AbstractMultiTypeCachingServiceTest.java | 27 +-
.../shell/migration/utils/MigrationUtils.java | 95 +++---
.../migrate-4.0.0-01-tenantDocumentIds.groovy | 6 +-
.../migrate-4.0.0-05-fixSystemItemIds.groovy | 29 +-
.../requestBody/4.0.0/fix_system_item_ids.painless | 37 +--
.../initialize_tenant_and_audit_fields.painless | 42 +--
.../4.0.0/shipped_system_definitions.json | 4 +
.../shell/migration/utils/MigrationUtilsTest.java | 144 ++++++---
11 files changed, 399 insertions(+), 331 deletions(-)
diff --git
a/itests/src/test/java/org/apache/unomi/itests/migration/Migrate40IndexMappingIT.java
b/itests/src/test/java/org/apache/unomi/itests/migration/Migrate40IndexMappingIT.java
index dc2f0ea70..04fba0729 100644
---
a/itests/src/test/java/org/apache/unomi/itests/migration/Migrate40IndexMappingIT.java
+++
b/itests/src/test/java/org/apache/unomi/itests/migration/Migrate40IndexMappingIT.java
@@ -155,7 +155,7 @@ public class Migrate40IndexMappingIT extends BaseIT {
params.put("itemType", itemType);
params.put("date", "2026-01-01T00:00:00Z");
params.put("systemItems", Collections.emptySet());
- params.put("shippedDefinitionIds", Collections.emptySet());
+ params.put("shippedDefinitionIds", Collections.emptyMap());
String painless = MigrationUtils.getFileWithoutComments(
migrationBundles,
"requestBody/4.0.0/initialize_tenant_and_audit_fields.painless");
diff --git
a/itests/src/test/java/org/apache/unomi/itests/migration/Migrate40TenantAssignmentIT.java
b/itests/src/test/java/org/apache/unomi/itests/migration/Migrate40TenantAssignmentIT.java
index 16d24c8f8..4c466756d 100644
---
a/itests/src/test/java/org/apache/unomi/itests/migration/Migrate40TenantAssignmentIT.java
+++
b/itests/src/test/java/org/apache/unomi/itests/migration/Migrate40TenantAssignmentIT.java
@@ -33,7 +33,6 @@ import org.osgi.framework.FrameworkUtil;
import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.Map;
import java.util.Set;
@@ -44,10 +43,24 @@ import java.util.Set;
@Category(SearchBackendIT.class)
public class Migrate40TenantAssignmentIT extends BaseIT {
- private static final String SOURCE_INDEX = "context-unomi997systemitems";
- private static final String DEST_INDEX =
"context-unomi997systemitems-dest";
- private static final String GEO_SOURCE = "context-unomi997geonameentry";
- private static final String GEO_DEST = "context-unomi997geonameentry-dest";
+ private static final String SOURCE_INDEX =
"context-unomi997source-systemitems";
+ private static final String DEST_INDEX =
"context-unomi997dest-systemitems";
+ private static final String GEO_SOURCE =
"context-unomi997source-geonameentry";
+ private static final String GEO_DEST = "context-unomi997dest-geonameentry";
+ private static final String DATA_SOURCE = "context-unomi997source-profile";
+ private static final String DATA_DEST = "context-unomi997dest-profile";
+ private static final String CAMEL_CASE_INDEX = "context-unomi997camelcase";
+
+ private static final String MAPPING = "{"
+ + "\"settings\":{\"number_of_shards\":1,\"number_of_replicas\":0},"
+ + "\"mappings\":{\"properties\":{"
+ + "\"itemId\":{\"type\":\"keyword\"},"
+ + "\"itemType\":{\"type\":\"keyword\"},"
+ + "\"scope\":{\"type\":\"keyword\"},"
+ + "\"tenantId\":{\"type\":\"keyword\"}"
+ + "}}}";
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
@After
public void deleteTestIndices() throws Exception {
@@ -56,187 +69,207 @@ public class Migrate40TenantAssignmentIT extends BaseIT {
}
try (CloseableHttpClient httpClient = createSearchEngineHttpClient()) {
String es = getSearchEngineBaseUrl();
- MigrationUtils.deleteIndex(httpClient, es, SOURCE_INDEX);
- MigrationUtils.deleteIndex(httpClient, es, DEST_INDEX);
- MigrationUtils.deleteIndex(httpClient, es, GEO_SOURCE);
- MigrationUtils.deleteIndex(httpClient, es, GEO_DEST);
+ for (String index : new String[]{SOURCE_INDEX, DEST_INDEX,
GEO_SOURCE, GEO_DEST, DATA_SOURCE, DATA_DEST, CAMEL_CASE_INDEX}) {
+ MigrationUtils.deleteIndex(httpClient, es, index);
+ }
}
}
@Test
- public void shippedDefinitionsListIsPresentInTheMigrationBundle() throws
Exception {
- Set<String> shipped = MigrationUtils.loadShippedDefinitionIds(
-
FrameworkUtil.getBundle(MigrationUtils.class).getBundleContext());
- Assert.assertTrue(shipped.contains("conditiontype:booleancondition"));
-
Assert.assertTrue(shipped.contains("conditiontype:pasteventcondition"));
-
Assert.assertTrue(shipped.contains("conditiontype:profilepropertycondition"));
+ public void shippedDefinitionsListIsPresentInTheMigrationBundle() {
+ Map<String, Set<String>> shipped =
MigrationUtils.loadShippedDefinitionIds(migrationBundle());
+
Assert.assertTrue(shipped.get("conditiontype").contains("booleancondition"));
+
Assert.assertTrue(shipped.get("conditiontype").contains("pasteventcondition"));
+
Assert.assertTrue(shipped.get("conditiontype").contains("profilepropertycondition"));
Assert.assertFalse("user content must not be in the shipped list by id
alone",
- shipped.contains("rule:my-user-rule"));
+ shipped.get("rule").contains("my-user-rule"));
}
@Test
public void migrationPutsEachKindOfItemInTheRightTenant() throws Exception
{
- Assume.assumeTrue(
- "HTTP admin API required (provider=" +
getPersistenceBackend().providerId() + ")",
- persistenceCapabilities().httpAdminApi());
+ assumeHttpAdminApi();
try (CloseableHttpClient httpClient = createSearchEngineHttpClient()) {
String es = getSearchEngineBaseUrl();
- BundleContext migrationBundles =
FrameworkUtil.getBundle(MigrationUtils.class).getBundleContext();
- Set<String> shipped =
MigrationUtils.loadShippedDefinitionIds(migrationBundles);
+ HttpUtils.executePutRequest(httpClient, es + "/" + SOURCE_INDEX,
MAPPING, null);
+ // Shipped definitions: without scope, with the 3.0 default scope,
and with the 3.0 _itemtype id suffix
+ indexDocument(httpClient, SOURCE_INDEX, "booleanCondition",
"booleanCondition", "conditionType", null);
+ indexDocument(httpClient, SOURCE_INDEX, "updateProperties",
"updateProperties", "rule", "systemscope");
+ indexDocument(httpClient, SOURCE_INDEX,
"pastEventCondition_conditiontype", "pastEventCondition_conditiontype",
"conditionType", null);
+ // User content with the 3.0 default scope
+ indexDocument(httpClient, SOURCE_INDEX, "my-user-rule",
"my-user-rule", "rule", "systemscope");
+ indexDocument(httpClient, SOURCE_INDEX, "my-segment",
"my-segment", "segment", "systemscope");
+ indexDocument(httpClient, SOURCE_INDEX, "my-scoring",
"my-scoring", "scoring", "systemscope");
+ indexDocument(httpClient, SOURCE_INDEX, "my-goal_goal", "my-goal",
"goal", "systemscope");
+ // Literal system scope on a type that is neither shipped nor a
system item type
+ indexDocument(httpClient, SOURCE_INDEX, "cfg1", "cfg1",
"sfdcConfiguration", "system");
- String mapping = "{"
- +
"\"settings\":{\"number_of_shards\":1,\"number_of_replicas\":0},"
- + "\"mappings\":{\"properties\":{"
- + "\"itemId\":{\"type\":\"keyword\"},"
- + "\"itemType\":{\"type\":\"keyword\"},"
- + "\"scope\":{\"type\":\"keyword\"},"
- + "\"tenantId\":{\"type\":\"keyword\"}"
- + "}}}";
- HttpUtils.executePutRequest(httpClient, es + "/" + SOURCE_INDEX,
mapping, null);
- HttpUtils.executePutRequest(httpClient, es + "/" + SOURCE_INDEX
- + "/_doc/booleanCondition?refresh=true",
-
"{\"itemId\":\"booleanCondition\",\"itemType\":\"conditionType\"}", null);
- HttpUtils.executePutRequest(httpClient, es + "/" + SOURCE_INDEX
- + "/_doc/my-user-rule?refresh=true",
-
"{\"itemId\":\"my-user-rule\",\"itemType\":\"rule\",\"scope\":\"systemscope\"}",
null);
- HttpUtils.executePutRequest(httpClient, es + "/" + SOURCE_INDEX
- + "/_doc/my-segment?refresh=true",
-
"{\"itemId\":\"my-segment\",\"itemType\":\"segment\",\"scope\":\"systemscope\"}",
null);
- HttpUtils.executePutRequest(httpClient, es + "/" + SOURCE_INDEX
- + "/_doc/my-scoring?refresh=true",
-
"{\"itemId\":\"my-scoring\",\"itemType\":\"scoring\",\"scope\":\"systemscope\"}",
null);
- HttpUtils.executePutRequest(httpClient, es + "/" + SOURCE_INDEX
- + "/_doc/my-goal?refresh=true",
-
"{\"itemId\":\"my-goal\",\"itemType\":\"goal\",\"scope\":\"systemscope\"}",
null);
-
- MigrationUtils.createIndex(httpClient, es, DEST_INDEX, mapping);
-
- Set<String> systemItems = new HashSet<>();
- Collections.addAll(systemItems, "conditiontype", "rule",
"segment", "scoring", "goal");
-
- Map<String, Object> params = new HashMap<>();
- params.put("tenantId", TEST_TENANT_ID);
- params.put("systemTenantId", "system");
- params.put("forceDefaultTenant", false);
- params.put("itemType", "systemitems");
- params.put("date", "2026-01-01T00:00:00Z");
- params.put("systemItems", systemItems);
- params.put("shippedDefinitionIds", shipped);
-
- String painless = MigrationUtils.getFileWithoutComments(
- migrationBundles,
"requestBody/4.0.0/initialize_tenant_and_audit_fields.painless");
- MigrationUtils.moveToIndex(httpClient, migrationBundles, es,
SOURCE_INDEX, DEST_INDEX, painless, params);
- HttpUtils.executePostRequest(httpClient, es + "/" + DEST_INDEX +
"/_refresh", null, null);
-
- JsonNode hits = new
ObjectMapper().readTree(HttpUtils.executeGetRequest(httpClient,
- es + "/" + DEST_INDEX + "/_search?size=20",
null)).path("hits").path("hits");
- Assert.assertEquals(5, hits.size());
-
- Map<String, JsonNode> byItemId = new HashMap<>();
- for (JsonNode hit : hits) {
- byItemId.put(hit.path("_source").path("itemId").asText(), hit);
- }
+ Map<String, JsonNode> byItemId = migrate(httpClient, SOURCE_INDEX,
DEST_INDEX, "systemitems", false,
+ Set.of("conditiontype", "rule", "segment", "scoring",
"goal"),
+
MigrationUtils.loadShippedDefinitionIds(migrationBundle()));
+ Assert.assertEquals(8, byItemId.size());
assertTenant(byItemId.get("booleanCondition"), "system");
+ assertTenant(byItemId.get("updateProperties"), "system");
+ assertTenant(byItemId.get("pastEventCondition_conditiontype"),
"system");
assertTenant(byItemId.get("my-user-rule"), TEST_TENANT_ID);
assertTenant(byItemId.get("my-segment"), TEST_TENANT_ID);
assertTenant(byItemId.get("my-scoring"), TEST_TENANT_ID);
assertTenant(byItemId.get("my-goal"), TEST_TENANT_ID);
- Assert.assertFalse("no user-created item may land in system",
-
byItemId.get("my-user-rule").path("_id").asText().startsWith("system_"));
+ assertTenant(byItemId.get("cfg1"), "system");
+
+ // System item types get the _itemtype suffix exactly once, other
types none
+ Assert.assertEquals("system_booleanCondition_conditiontype",
byItemId.get("booleanCondition").path("_id").asText());
+ Assert.assertEquals("system_pastEventCondition_conditiontype",
+
byItemId.get("pastEventCondition_conditiontype").path("_id").asText());
+ Assert.assertEquals(TEST_TENANT_ID + "_my-goal_goal",
byItemId.get("my-goal").path("_id").asText());
+ Assert.assertEquals("system_cfg1",
byItemId.get("cfg1").path("_id").asText());
+
+ // Migrated system items carry create audit, so the bundle
redeploy is an update
+ JsonNode source = byItemId.get("booleanCondition").path("_source");
+ Assert.assertEquals("system-migration-4.0.0",
source.path("createdBy").asText());
+ Assert.assertEquals("system-migration-4.0.0",
source.path("lastModifiedBy").asText());
+ Assert.assertEquals("2026-01-01T00:00:00Z",
source.path("creationDate").asText());
+ }
+ }
+
+ @Test
+ public void forcedDefaultTenantNeverUsesTheSystemTenant() throws Exception
{
+ assumeHttpAdminApi();
+
+ try (CloseableHttpClient httpClient = createSearchEngineHttpClient()) {
+ String es = getSearchEngineBaseUrl();
+ HttpUtils.executePutRequest(httpClient, es + "/" + SOURCE_INDEX,
MAPPING, null);
+ indexDocument(httpClient, SOURCE_INDEX, "node1", "node1",
"clusterNode", "system");
+ indexDocument(httpClient, SOURCE_INDEX, "booleanCondition",
"booleanCondition", "conditionType", null);
+
+ Map<String, JsonNode> byItemId = migrate(httpClient, SOURCE_INDEX,
DEST_INDEX, "clusterNode", true,
+ Collections.emptySet(),
MigrationUtils.loadShippedDefinitionIds(migrationBundle()));
+ Assert.assertEquals(2, byItemId.size());
+ assertTenant(byItemId.get("node1"), TEST_TENANT_ID);
+ assertTenant(byItemId.get("booleanCondition"), TEST_TENANT_ID);
+ }
+ }
+
+ @Test
+ public void profilesSessionsAndEventsStayInTheConfiguredTenant() throws
Exception {
+ assumeHttpAdminApi();
+
+ try (CloseableHttpClient httpClient = createSearchEngineHttpClient()) {
+ String es = getSearchEngineBaseUrl();
+ HttpUtils.executePutRequest(httpClient, es + "/" + DATA_SOURCE,
MAPPING, null);
+ indexDocument(httpClient, DATA_SOURCE, "p1", "p1", "profile",
null);
+ indexDocument(httpClient, DATA_SOURCE, "s1", "s1", "session",
"systemscope");
+ indexDocument(httpClient, DATA_SOURCE, "e1", "e1", "event",
"systemscope");
+
+ Map<String, JsonNode> byItemId = migrate(httpClient, DATA_SOURCE,
DATA_DEST, "profile", false,
+ Set.of("conditiontype", "rule"),
MigrationUtils.loadShippedDefinitionIds(migrationBundle()));
+ Assert.assertEquals(3, byItemId.size());
+ for (String itemId : new String[]{"p1", "s1", "e1"}) {
+ assertTenant(byItemId.get(itemId), TEST_TENANT_ID);
+ Assert.assertEquals(TEST_TENANT_ID + "_" + itemId,
byItemId.get(itemId).path("_id").asText());
+ }
}
}
@Test
public void geonamesGoToTheSystemTenant() throws Exception {
- Assume.assumeTrue(
- "HTTP admin API required (provider=" +
getPersistenceBackend().providerId() + ")",
- persistenceCapabilities().httpAdminApi());
+ assumeHttpAdminApi();
try (CloseableHttpClient httpClient = createSearchEngineHttpClient()) {
String es = getSearchEngineBaseUrl();
- BundleContext migrationBundles =
FrameworkUtil.getBundle(MigrationUtils.class).getBundleContext();
+ HttpUtils.executePutRequest(httpClient, es + "/" + GEO_SOURCE,
MAPPING, null);
+ indexDocument(httpClient, GEO_SOURCE, "paris", "paris",
"geonameEntry", null);
+
+ Map<String, JsonNode> byItemId = migrate(httpClient, GEO_SOURCE,
GEO_DEST, "geonameentry", false,
+ Collections.emptySet(), Collections.emptyMap());
+ assertTenant(byItemId.get("paris"), "system");
+ }
+ }
+
+ @Test
+ public void
itemIdFixMatchesCamelCaseItemTypesAndCountsOnlyChangedDocuments() throws
Exception {
+ assumeHttpAdminApi();
+ try (CloseableHttpClient httpClient = createSearchEngineHttpClient()) {
+ String es = getSearchEngineBaseUrl();
String mapping = "{"
- +
"\"settings\":{\"number_of_shards\":1,\"number_of_replicas\":0},"
+ + "\"settings\":{"
+ + "\"number_of_shards\":1,\"number_of_replicas\":0,"
+ +
"\"analysis\":{\"analyzer\":{\"folding\":{\"type\":\"custom\",\"tokenizer\":\"keyword\",\"filter\":[\"lowercase\",\"asciifolding\"]}}}"
+ + "},"
+ "\"mappings\":{\"properties\":{"
+ "\"itemId\":{\"type\":\"keyword\"},"
- + "\"itemType\":{\"type\":\"keyword\"},"
- + "\"tenantId\":{\"type\":\"keyword\"}"
+ +
"\"itemType\":{\"type\":\"text\",\"analyzer\":\"folding\",\"fields\":{\"keyword\":{\"type\":\"keyword\",\"ignore_above\":256}}}"
+ "}}}";
- HttpUtils.executePutRequest(httpClient, es + "/" + GEO_SOURCE,
mapping, null);
- HttpUtils.executePutRequest(httpClient, es + "/" + GEO_SOURCE
- + "/_doc/paris?refresh=true",
- "{\"itemId\":\"paris\",\"itemType\":\"geonameEntry\"}",
null);
- MigrationUtils.createIndex(httpClient, es, GEO_DEST, mapping);
-
- Map<String, Object> params = new HashMap<>();
- params.put("tenantId", TEST_TENANT_ID);
- params.put("systemTenantId", "system");
- params.put("forceDefaultTenant", false);
- params.put("itemType", "geonameentry");
- params.put("date", "2026-01-01T00:00:00Z");
- params.put("systemItems", Collections.emptySet());
- params.put("shippedDefinitionIds", Collections.emptySet());
-
- String painless = MigrationUtils.getFileWithoutComments(
- migrationBundles,
"requestBody/4.0.0/initialize_tenant_and_audit_fields.painless");
- MigrationUtils.moveToIndex(httpClient, migrationBundles, es,
GEO_SOURCE, GEO_DEST, painless, params);
- HttpUtils.executePostRequest(httpClient, es + "/" + GEO_DEST +
"/_refresh", null, null);
-
- JsonNode hit = new
ObjectMapper().readTree(HttpUtils.executeGetRequest(httpClient,
- es + "/" + GEO_DEST + "/_search?size=1",
null)).path("hits").path("hits").get(0);
- assertTenant(hit, "system");
+ HttpUtils.executePutRequest(httpClient, es + "/" +
CAMEL_CASE_INDEX, mapping, null);
+ indexDocument(httpClient, CAMEL_CASE_INDEX,
"system_booleanCondition_conditiontype", "wrong", "conditionType", null);
+ // Already consistent: must be left untouched and not counted
+ indexDocument(httpClient, CAMEL_CASE_INDEX,
"system_pastEventCondition_conditiontype",
+ "pastEventCondition_conditiontype", "conditionType", null);
+
+ // Same request as migrate-4.0.0-05-fixSystemItemIds.groovy
+ String fixScript = MigrationUtils.getFileWithoutComments(
+ migrationBundle(),
"requestBody/4.0.0/fix_system_item_ids.painless");
+ long updated = MigrationUtils.updateByQueryAndCount(httpClient,
es, CAMEL_CASE_INDEX,
+ MigrationUtils.buildItemTypeUpdateRequest(fixScript,
"conditionType"));
+ Assert.assertEquals(1L, updated);
+
+ JsonNode source =
MAPPER.readTree(HttpUtils.executeGetRequest(httpClient,
+ es + "/" + CAMEL_CASE_INDEX +
"/_doc/system_booleanCondition_conditiontype", null))
+ .path("_source");
+ Assert.assertEquals("booleanCondition_conditiontype",
source.path("itemId").asText());
}
}
- @Test
- public void camelCaseItemTypeKeywordQueryUpdatesDocuments() throws
Exception {
+ private void assumeHttpAdminApi() {
Assume.assumeTrue(
"HTTP admin API required (provider=" +
getPersistenceBackend().providerId() + ")",
persistenceCapabilities().httpAdminApi());
+ }
- try (CloseableHttpClient httpClient = createSearchEngineHttpClient()) {
- String es = getSearchEngineBaseUrl();
- BundleContext migrationBundles =
FrameworkUtil.getBundle(MigrationUtils.class).getBundleContext();
-
- String index = "context-unomi997camelcase";
- try {
- String mapping = "{"
- + "\"settings\":{"
- + "\"number_of_shards\":1,\"number_of_replicas\":0,"
- +
"\"analysis\":{\"analyzer\":{\"folding\":{\"type\":\"custom\",\"tokenizer\":\"keyword\",\"filter\":[\"lowercase\",\"asciifolding\"]}}}"
- + "},"
- + "\"mappings\":{\"properties\":{"
- + "\"itemId\":{\"type\":\"keyword\"},"
- +
"\"itemType\":{\"type\":\"text\",\"analyzer\":\"folding\",\"fields\":{\"keyword\":{\"type\":\"keyword\",\"ignore_above\":256}}}"
- + "}}}";
- HttpUtils.executePutRequest(httpClient, es + "/" + index,
mapping, null);
- HttpUtils.executePutRequest(httpClient, es + "/" + index
- +
"/_doc/system_booleanCondition_conditiontype?refresh=true",
-
"{\"itemId\":\"wrong\",\"itemType\":\"conditionType\"}", null);
-
- String fixScript = MigrationUtils.getFileWithoutComments(
- migrationBundles,
"requestBody/4.0.0/fix_system_item_ids.painless");
- String body = "{\"script\":{\"source\":"
- + new ObjectMapper().writeValueAsString(fixScript)
- + ",\"lang\":\"painless\"},"
- + "\"query\":{\"bool\":{\"should\":["
- +
"{\"term\":{\"itemType.keyword\":\"conditionType\"}},"
- + "{\"term\":{\"itemType\":\"conditiontype\"}}"
- + "],\"minimum_should_match\":1}}}";
- long updated =
MigrationUtils.updateByQueryAndCount(httpClient, es, index, body);
- Assert.assertEquals(1L, updated);
-
- JsonNode source = new
ObjectMapper().readTree(HttpUtils.executeGetRequest(httpClient,
- es + "/" + index +
"/_doc/system_booleanCondition_conditiontype", null))
- .path("_source");
- Assert.assertEquals("booleanCondition_conditiontype",
source.path("itemId").asText());
- } finally {
- MigrationUtils.deleteIndex(httpClient, es, index);
- }
+ private static BundleContext migrationBundle() {
+ return
FrameworkUtil.getBundle(MigrationUtils.class).getBundleContext();
+ }
+
+ private void indexDocument(CloseableHttpClient httpClient, String index,
String documentId, String itemId,
+ String itemType, String scope) throws Exception
{
+ String source = "{\"itemId\":\"" + itemId + "\",\"itemType\":\"" +
itemType + "\""
+ + (scope != null ? ",\"scope\":\"" + scope + "\"" : "") + "}";
+ HttpUtils.executePutRequest(httpClient,
+ getSearchEngineBaseUrl() + "/" + index + "/_doc/" + documentId
+ "?refresh=true", source, null);
+ }
+
+ /**
+ * Runs the tenant assignment script from {@code source} to {@code dest},
as migrate-4.0.0-01 does,
+ * and returns the migrated hits by source itemId.
+ */
+ private Map<String, JsonNode> migrate(CloseableHttpClient httpClient,
String source, String dest, String itemType,
+ boolean forceDefaultTenant,
Set<String> systemItems,
+ Map<String, Set<String>>
shippedDefinitionIds) throws Exception {
+ String es = getSearchEngineBaseUrl();
+ MigrationUtils.createIndex(httpClient, es, dest, MAPPING);
+
+ Map<String, Object> params = new HashMap<>();
+ params.put("tenantId", TEST_TENANT_ID);
+ params.put("systemTenantId", "system");
+ params.put("forceDefaultTenant", forceDefaultTenant);
+ params.put("itemType", itemType);
+ params.put("date", "2026-01-01T00:00:00Z");
+ params.put("systemItems", systemItems);
+ params.put("shippedDefinitionIds", shippedDefinitionIds);
+
+ String painless = MigrationUtils.getFileWithoutComments(
+ migrationBundle(),
"requestBody/4.0.0/initialize_tenant_and_audit_fields.painless");
+ MigrationUtils.moveToIndex(httpClient, migrationBundle(), es, source,
dest, painless, params);
+ HttpUtils.executePostRequest(httpClient, es + "/" + dest +
"/_refresh", null, null);
+
+ Map<String, JsonNode> byItemId = new HashMap<>();
+ for (JsonNode hit :
MAPPER.readTree(HttpUtils.executeGetRequest(httpClient,
+ es + "/" + dest + "/_search?size=20",
null)).path("hits").path("hits")) {
+ byItemId.put(hit.path("_source").path("itemId").asText(), hit);
}
+ return byItemId;
}
private static void assertTenant(JsonNode hit, String expectedTenant) {
diff --git
a/services-common/src/main/java/org/apache/unomi/services/common/cache/AbstractMultiTypeCachingService.java
b/services-common/src/main/java/org/apache/unomi/services/common/cache/AbstractMultiTypeCachingService.java
index 427298a62..8cf444157 100644
---
a/services-common/src/main/java/org/apache/unomi/services/common/cache/AbstractMultiTypeCachingService.java
+++
b/services-common/src/main/java/org/apache/unomi/services/common/cache/AbstractMultiTypeCachingService.java
@@ -932,17 +932,14 @@ public abstract class AbstractMultiTypeCachingService
extends AbstractContextAwa
Class<T> itemClass = (Class<T>) item.getClass();
T existingItem = persistenceService.load(itemId, itemClass);
- // Bundle JSON definitions have no create audit. If persistence
already has create
- // fields (e.g. from migration), copy them onto the incoming item
before save so a
- // later auditUpdate does not wipe them with nulls (UNOMI-997).
- if (existingItem != null) {
- if (item.getCreatedBy() == null && existingItem.getCreatedBy() !=
null) {
- item.setCreatedBy(existingItem.getCreatedBy());
- }
- if (item.getCreationDate() == null &&
existingItem.getCreationDate() != null) {
- item.setCreationDate(existingItem.getCreationDate());
- }
- if (item.getVersion() == null && existingItem.getVersion() !=
null) {
+ // Bundle JSON definitions have no creator, and Item() defaults their
creation date and
+ // version to "now" and 0. If persistence already has create audit
(e.g. from migration),
+ // carry it over to the incoming item so the save below does not
overwrite it (UNOMI-997).
+ if (existingItem != null && item.getCreatedBy() == null
+ && existingItem.getCreatedBy() != null &&
existingItem.getCreationDate() != null) {
+ item.setCreatedBy(existingItem.getCreatedBy());
+ item.setCreationDate(existingItem.getCreationDate());
+ if (existingItem.getVersion() != null) {
item.setVersion(existingItem.getVersion());
}
}
diff --git
a/services-common/src/test/java/org/apache/unomi/services/common/cache/AbstractMultiTypeCachingServiceTest.java
b/services-common/src/test/java/org/apache/unomi/services/common/cache/AbstractMultiTypeCachingServiceTest.java
index 174609ad4..ad962c909 100644
---
a/services-common/src/test/java/org/apache/unomi/services/common/cache/AbstractMultiTypeCachingServiceTest.java
+++
b/services-common/src/test/java/org/apache/unomi/services/common/cache/AbstractMultiTypeCachingServiceTest.java
@@ -474,8 +474,9 @@ public class AbstractMultiTypeCachingServiceTest {
}
/**
- * Bundle redeploy loads JSON without create audit. If persistence already
has create fields
- * (for example after migration), saveItem must keep them instead of
overwriting with nulls.
+ * Bundle redeploy loads JSON without a creator, with the creation date
and version defaulted by
+ * {@code Item()}. If persistence already has create audit (for example
after migration),
+ * saveItem must keep it instead of overwriting it with those defaults.
*/
@Test
public void testSaveItemPreservesMigratedCreateAuditOnBundleRedeploy() {
@@ -487,9 +488,8 @@ public class AbstractMultiTypeCachingServiceTest {
when(persistenceService.load("item1",
TestItem.class)).thenReturn(existing);
TestItem fromBundle = new TestItem("item1");
- // Simulate a bundle JSON definition that has no create audit (Item
may default dates).
- fromBundle.setCreatedBy(null);
- fromBundle.setCreationDate(null);
+ assertNull(fromBundle.getCreatedBy());
+ assertNotNull("Item() defaults the creation date",
fromBundle.getCreationDate());
testCachingService.callSaveItem(fromBundle);
@@ -497,7 +497,24 @@ public class AbstractMultiTypeCachingServiceTest {
verify(persistenceService).save(saved.capture());
assertEquals("system-migration-4.0.0",
saved.getValue().getCreatedBy());
assertEquals(creationDate, saved.getValue().getCreationDate());
+ assertEquals(Long.valueOf(3L), saved.getValue().getVersion());
verify(auditService).auditUpdate(same(fromBundle),
eq("system-bundle"));
verify(auditService, never()).auditCreate(any(), anyString());
}
+
+ /**
+ * A persisted item without complete create audit is still audited as a
creation.
+ */
+ @Test
+ public void testSaveItemAuditsCreateWhenExistingItemHasNoCreateAudit() {
+ TestItem existing = new TestItem("item1");
+ existing.setCreatedBy(null);
+ when(persistenceService.load("item1",
TestItem.class)).thenReturn(existing);
+
+ TestItem fromBundle = new TestItem("item1");
+ testCachingService.callSaveItem(fromBundle);
+
+ verify(auditService).auditCreate(same(fromBundle),
eq("system-bundle"));
+ verify(auditService, never()).auditUpdate(any(), anyString());
+ }
}
diff --git
a/tools/shell-commands/src/main/java/org/apache/unomi/shell/migration/utils/MigrationUtils.java
b/tools/shell-commands/src/main/java/org/apache/unomi/shell/migration/utils/MigrationUtils.java
index fbee87829..5ab1a8bc0 100644
---
a/tools/shell-commands/src/main/java/org/apache/unomi/shell/migration/utils/MigrationUtils.java
+++
b/tools/shell-commands/src/main/java/org/apache/unomi/shell/migration/utils/MigrationUtils.java
@@ -366,75 +366,62 @@ public class MigrationUtils {
}
/**
- * Loads the set of definitions Unomi ships and reloads at startup.
- * Keys are {@code itemType:itemId} in lower case (for example {@code
conditiontype:booleanCondition}).
+ * Loads the definitions Unomi ships and reloads at startup, as lower-case
item ids grouped by
+ * lower-case item type (for example {@code conditiontype ->
booleancondition}).
* The migrate command bundle ships a snapshot of that list so it is
available before plugins start.
+ * <p>
+ * Matching is by type and id only: a 3.0 item that shares both with a
shipped definition (such as a
+ * built-in rule edited in place) is migrated to the system tenant and
replaced by the 4.0 version
+ * at the next startup.
+ *
+ * @throws IllegalStateException if the list is empty or an entry is not
{@code itemType:itemId}
*/
- public static Set<String> loadShippedDefinitionIds(BundleContext
bundleContext) throws IOException {
- Set<String> ids = new LinkedHashSet<>();
- String resource = resourceAsString(bundleContext,
"requestBody/4.0.0/shipped_system_definitions.json");
- JSONArray array = new JSONArray(resource);
+ public static Map<String, Set<String>>
loadShippedDefinitionIds(BundleContext bundleContext) {
+ String resource = "requestBody/4.0.0/shipped_system_definitions.json";
+ JSONArray array = new JSONArray(resourceAsString(bundleContext,
resource));
+ Map<String, Set<String>> idsByType = new TreeMap<>();
for (int i = 0; i < array.length(); i++) {
- String key = array.optString(i, null);
- if (key != null && !key.isBlank()) {
- ids.add(key.toLowerCase(Locale.ROOT));
+ String key = String.valueOf(array.get(i)).toLowerCase(Locale.ROOT);
+ int separator = key.indexOf(':');
+ if (separator <= 0 || separator == key.length() - 1) {
+ throw new IllegalStateException("Invalid entry in " + resource
+ ": " + key);
}
+ idsByType.computeIfAbsent(key.substring(0, separator), type -> new
TreeSet<>()).add(key.substring(separator + 1));
}
- return ids;
- }
-
- /**
- * Strips a trailing {@code _itemType} suffix (any case) from a document
or item id.
- */
- public static String resolveBaseItemId(String documentOrItemId, String
itemType) {
- if (documentOrItemId == null || documentOrItemId.isBlank()) {
- return documentOrItemId;
- }
- if (itemType == null || itemType.isBlank()) {
- return documentOrItemId;
- }
- String suffix = "_" + itemType;
- if (documentOrItemId.regionMatches(true, documentOrItemId.length() -
suffix.length(), suffix, 0, suffix.length())) {
- return documentOrItemId.substring(0, documentOrItemId.length() -
suffix.length());
+ if (idsByType.isEmpty()) {
+ throw new IllegalStateException(resource + " is empty: shipped
definitions would be migrated to the wrong tenant");
}
- return documentOrItemId;
+ return idsByType;
}
/**
- * Decides whether a 3.0 item must land in the system tenant after
migration.
- * Shipped definitions and geonames go to system. User-created rules,
segments,
- * scorings and goals stay in the configured tenant even when their 3.0
scope is
- * {@code systemscope}. Unknown indices forced to the default tenant never
use system.
+ * Builds an update by query request running a Painless script on the
documents of an item type.
+ * The item type is matched on both the keyword sub-field and the folded
text field, so camelCase
+ * values such as {@code conditionType} are not missed.
*/
- public static boolean shouldAssignToSystemTenant(String itemType, String
itemId, String scope,
- String indexName, boolean
forceDefaultTenant,
- Collection<String>
shippedDefinitionIds) {
- if (forceDefaultTenant) {
- return false;
- }
- String typeLower = itemType == null ? "" :
itemType.toLowerCase(Locale.ROOT);
- if ("geonameentry".equals(typeLower)
- || (indexName != null &&
indexName.toLowerCase(Locale.ROOT).endsWith("-geonameentry"))) {
- return true;
- }
- String baseId = resolveBaseItemId(itemId, itemType);
- if (baseId != null && shippedDefinitionIds != null &&
!typeLower.isEmpty()) {
- String key = typeLower + ":" + baseId.toLowerCase(Locale.ROOT);
- if (shippedDefinitionIds.contains(key)) {
- return true;
- }
- }
- // Literal scope "system" only — not the 3.0 default "systemscope"
used by user content
- return "system".equals(scope);
+ public static String buildItemTypeUpdateRequest(String painlessScript,
String itemType) {
+ JSONArray should = new JSONArray()
+ .put(new JSONObject().put("term", new
JSONObject().put("itemType.keyword", itemType)))
+ .put(new JSONObject().put("term", new
JSONObject().put("itemType", itemType.toLowerCase(Locale.ROOT))));
+ return new JSONObject()
+ .put("script", new JSONObject().put("source",
painlessScript).put("lang", "painless"))
+ .put("query", new JSONObject().put("bool", new
JSONObject().put("should", should).put("minimum_should_match", 1)))
+ .toString();
}
/**
- * Same as {@link #updateByQuery(CloseableHttpClient, String, String,
String)} but returns how
- * many documents were updated, so callers can log the count.
+ * Runs an update by query synchronously, refreshes the index and returns
how many documents were
+ * actually changed (documents the script marks as noop are not counted).
+ *
+ * @throws IOException if the update timed out or reported failures;
version conflicts abort the request
*/
- public static long updateByQueryAndCount(CloseableHttpClient httpClient,
String esAddress, String indexName, String requestBody) throws Exception {
+ public static long updateByQueryAndCount(CloseableHttpClient httpClient,
String esAddress, String indexName, String requestBody) throws IOException {
JSONObject response = new JSONObject(HttpUtils.executePostRequest(
- httpClient, esAddress + "/" + indexName +
"/_update_by_query?refresh=true&conflicts=proceed", requestBody, null));
+ httpClient, esAddress + "/" + indexName +
"/_update_by_query?refresh=true", requestBody, null));
+ JSONArray failures = response.optJSONArray("failures");
+ if (response.optBoolean("timed_out") || (failures != null &&
!failures.isEmpty())) {
+ throw new IOException("Update by query on " + indexName + " did
not complete: " + response);
+ }
return response.optLong("updated", 0L);
}
diff --git
a/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-4.0.0-01-tenantDocumentIds.groovy
b/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-4.0.0-01-tenantDocumentIds.groovy
index 65e927682..6f3c755b4 100644
---
a/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-4.0.0-01-tenantDocumentIds.groovy
+++
b/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-4.0.0-01-tenantDocumentIds.groovy
@@ -26,7 +26,7 @@ MigrationContext context = migrationContext
String esAddress = context.getConfigString(CONFIG_ES_ADDRESS)
String indexPrefix = context.getConfigString(INDEX_PREFIX)
String tenantId = context.getConfigString(TENANT_ID)
-String systemTenantId = "system" // System tenant ID for system-level items
+String systemTenantId = "system"
String rolloverPolicyName = indexPrefix + "-unomi-rollover-policy"
String rolloverSessionAlias = indexPrefix + "-session"
String rolloverEventAlias = indexPrefix + "-event"
@@ -109,8 +109,8 @@ context.performMigrationStep("4.0.0-get-all-indices", () ->
{
.collect { it.toLowerCase(java.util.Locale.ROOT) } as Set
context.printMessage("Found " + systemItems.size() + " system items")
- Set<String> shippedDefinitionIds =
MigrationUtils.loadShippedDefinitionIds(bundleContext)
- context.printMessage("Loaded " + shippedDefinitionIds.size() + " shipped
definition ids")
+ Map<String, Set<String>> shippedDefinitionIds =
MigrationUtils.loadShippedDefinitionIds(bundleContext)
+ context.printMessage("Loaded " + shippedDefinitionIds.values().sum {
it.size() } + " shipped definition ids")
// Create base parameters
Map<String, Object> baseParams = new HashMap<>()
diff --git
a/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-4.0.0-05-fixSystemItemIds.groovy
b/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-4.0.0-05-fixSystemItemIds.groovy
index e5c8fbdd3..6a5065fa1 100644
---
a/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-4.0.0-05-fixSystemItemIds.groovy
+++
b/tools/shell-commands/src/main/resources/META-INF/cxs/migration/migrate-4.0.0-05-fixSystemItemIds.groovy
@@ -1,7 +1,6 @@
import org.apache.unomi.shell.migration.service.MigrationContext
import org.apache.unomi.shell.migration.utils.MigrationUtils
import org.apache.unomi.shell.migration.utils.HttpUtils
-import org.json.JSONObject
import static org.apache.unomi.shell.migration.service.MigrationConfig.*
/*
@@ -43,41 +42,33 @@ context.performMigrationStep("4.0.0-fix-system-item-ids",
() -> {
context.printMessage("Fixing itemIds in systemitems index that end
with itemType suffix")
long totalUpdated = 0L
+ List<String> failedItemTypes = []
- // Process each system item type. Match on itemType.keyword so
camelCase values
- // such as conditionType / actionType / propertyType are not missed
(UNOMI-997).
+ // Process each system item type. The query matches camelCase values
such as
+ // conditionType / actionType / propertyType (UNOMI-997).
systemItems.each { itemType ->
context.printMessage("Fixing items of type: ${itemType}")
String fixScript =
MigrationUtils.getFileWithoutComments(bundleContext,
"requestBody/4.0.0/fix_system_item_ids.painless")
- JSONObject scriptObj = new JSONObject()
- scriptObj.put("source", fixScript)
- scriptObj.put("lang", "painless")
-
- // Match camelCase item types on both the keyword sub-field and
the folded text field
- JSONObject keywordTerm = new JSONObject().put("term", new
JSONObject().put("itemType.keyword", itemType))
- JSONObject foldedTerm = new JSONObject().put("term", new
JSONObject().put("itemType", itemType.toLowerCase(java.util.Locale.ROOT)))
- JSONObject boolQuery = new JSONObject()
- boolQuery.put("should", [keywordTerm, foldedTerm])
- boolQuery.put("minimum_should_match", 1)
- JSONObject queryObj = new JSONObject().put("bool", boolQuery)
-
- JSONObject updateRequestObj = new JSONObject()
- updateRequestObj.put("script", scriptObj)
- updateRequestObj.put("query", queryObj)
+ String updateRequest =
MigrationUtils.buildItemTypeUpdateRequest(fixScript, itemType)
try {
- long updated =
MigrationUtils.updateByQueryAndCount(context.getHttpClient(), esAddress,
systemItemsIndex, updateRequestObj.toString())
+ long updated =
MigrationUtils.updateByQueryAndCount(context.getHttpClient(), esAddress,
systemItemsIndex, updateRequest)
totalUpdated += updated
context.printMessage("Fixed ${updated} itemId(s) for item
type: ${itemType}")
} catch (Exception e) {
context.printMessage("Warning: Could not fix itemIds for item
type ${itemType}: ${e.getMessage()}")
+ failedItemTypes.add(itemType)
}
}
HttpUtils.executePostRequest(context.getHttpClient(), esAddress +
"/${systemItemsIndex}/_refresh", null, null)
context.printMessage("Fixed ${totalUpdated} itemId(s) in systemitems
index")
+ // Fail the step so it is retried on the next run instead of being
recorded as completed
+ if (!failedItemTypes.isEmpty()) {
+ throw new IllegalStateException("Could not fix itemIds for item
types: " + failedItemTypes)
+ }
} else {
context.printMessage("Systemitems index does not exist, skipping
itemId fix")
}
diff --git
a/tools/shell-commands/src/main/resources/requestBody/4.0.0/fix_system_item_ids.painless
b/tools/shell-commands/src/main/resources/requestBody/4.0.0/fix_system_item_ids.painless
index 9d309d1cc..fdab21556 100644
---
a/tools/shell-commands/src/main/resources/requestBody/4.0.0/fix_system_item_ids.painless
+++
b/tools/shell-commands/src/main/resources/requestBody/4.0.0/fix_system_item_ids.painless
@@ -38,34 +38,15 @@ if (ctx._source.itemId != null && ctx._source.itemType !=
null && ctx._id != nul
documentIdWithoutTenant =
documentIdWithoutTenant.substring(firstUnderscoreIndex + 1);
}
- // For system items, the expected format after 2.2.0 migration is itemId =
documentId (with suffix)
- // However, new items created after migrations may have itemId = original
(without suffix)
- // Both work because setMetadata() extracts from document ID, but we
normalize to the migrated format
- // for consistency and to fix items affected by the 4.0.0-00 bug
- def itemTypeSuffix = '_' + ctx._source.itemType.toLowerCase();
- if (documentIdWithoutTenant.endsWith(itemTypeSuffix)) {
- // Document ID has the expected suffix format - itemId should match it
- if (ctx._source.itemId != documentIdWithoutTenant) {
- ctx._source.itemId = documentIdWithoutTenant;
- }
+ // Every case above ends the same way: itemId must equal the document ID
minus the tenant prefix.
+ // When the document ID lost its suffix (4.0.0-00 bug) it cannot be
repaired by update_by_query
+ // (that needs a reindex), but aligning itemId on it still lets
setMetadata() work.
+ // Documents that are already consistent are left untouched so they are
not counted as updated.
+ if (ctx._source.itemId != documentIdWithoutTenant) {
+ ctx._source.itemId = documentIdWithoutTenant;
} else {
- // Document ID doesn't have suffix (pre-2.2.0 format or incorrectly
processed by 4.0.0-00)
- // Check if source itemId has the suffix - this indicates the document
ID is wrong (from buggy 4.0.0-00)
- if (ctx._source.itemId != null &&
ctx._source.itemId.endsWith(itemTypeSuffix)) {
- // Source itemId has suffix but document ID doesn't - document ID
was incorrectly processed
- // We can't fix the document ID with update_by_query (would need
reindexing),
- // but we can at least ensure itemId matches the document ID so
setMetadata() can work
- // Note: This item will need to be reindexed to fix the document
ID properly
- if (ctx._source.itemId != documentIdWithoutTenant) {
- ctx._source.itemId = documentIdWithoutTenant;
- }
- } else {
- // Document ID doesn't have suffix and source itemId doesn't either
- // This is either pre-2.2.0 format or a legitimate case - ensure
they match
- if (ctx._source.itemId != documentIdWithoutTenant) {
- ctx._source.itemId = documentIdWithoutTenant;
- }
- }
+ ctx.op = 'noop';
}
+} else {
+ ctx.op = 'noop';
}
-
diff --git
a/tools/shell-commands/src/main/resources/requestBody/4.0.0/initialize_tenant_and_audit_fields.painless
b/tools/shell-commands/src/main/resources/requestBody/4.0.0/initialize_tenant_and_audit_fields.painless
index 5e4591fdc..97dcd9b38 100644
---
a/tools/shell-commands/src/main/resources/requestBody/4.0.0/initialize_tenant_and_audit_fields.painless
+++
b/tools/shell-commands/src/main/resources/requestBody/4.0.0/initialize_tenant_and_audit_fields.painless
@@ -18,21 +18,27 @@
// Decide the tenant for this document (UNOMI-997).
// Shipped definitions and geonames go to system. User-created
rules/segments/scorings/goals
// stay in the configured tenant even when their 3.0 scope is "systemscope".
+// A shipped definition is recognised by item type and id only, so a 3.0 item
sharing both
+// (such as a built-in rule edited in place) also goes to system.
boolean useSystemTenant = false;
if (params.forceDefaultTenant != true) {
- def itemTypeLower = ctx._source.itemType != null ?
ctx._source.itemType.toLowerCase() : '';
- def rawId = ctx._source.itemId != null ? ctx._source.itemId : ctx._id;
- def baseId = rawId;
- if (baseId != null && itemTypeLower != '' &&
baseId.toLowerCase().endsWith('_' + itemTypeLower)) {
- baseId = baseId.substring(0, baseId.length() - itemTypeLower.length()
- 1);
- }
- if (itemTypeLower == 'geonameentry' || (ctx._index != null &&
ctx._index.toLowerCase().endsWith('-geonameentry'))) {
- useSystemTenant = true;
- } else if (baseId != null && params.shippedDefinitionIds != null
- && params.shippedDefinitionIds.contains(itemTypeLower + ':' +
baseId.toLowerCase())) {
+ def itemTypeLower = ctx._source.itemType != null ?
ctx._source.itemType.toLowerCase(Locale.ROOT) : '';
+ // Shipped ids are grouped by item type, so other types (profiles,
events...) skip the lookup
+ def shippedIds = params.shippedDefinitionIds != null ?
params.shippedDefinitionIds.get(itemTypeLower) : null;
+ if (itemTypeLower == 'geonameentry' || (ctx._index != null &&
ctx._index.toLowerCase(Locale.ROOT).endsWith('-geonameentry'))) {
useSystemTenant = true;
- } else if (ctx._source.scope != null && ctx._source.scope == 'system') {
- // Literal "system" only — not the 3.0 default "systemscope"
+ } else if (shippedIds != null) {
+ def rawId = ctx._source.itemId != null ? ctx._source.itemId : ctx._id;
+ if (rawId != null) {
+ def shippedId = rawId.toLowerCase(Locale.ROOT);
+ if (shippedId.endsWith('_' + itemTypeLower)) {
+ shippedId = shippedId.substring(0, shippedId.length() -
itemTypeLower.length() - 1);
+ }
+ useSystemTenant = shippedIds.contains(shippedId);
+ }
+ }
+ // Literal "system" only — not the 3.0 default "systemscope"
+ if (!useSystemTenant && ctx._source.scope == 'system') {
useSystemTenant = true;
}
}
@@ -43,10 +49,12 @@ if (ctx._id != null) {
// Skip if already has tenant prefix
if (!ctx._id.startsWith(params.tenantId + '_') &&
!ctx._id.startsWith(params.systemTenantId + '_')) {
def baseId = ctx._id;
- if (ctx._index != null &&
ctx._index.toLowerCase().endsWith('-systemitems')) {
- // For system items, ensure itemType is lowercase and matches the
format in ElasticSearchPersistenceServiceImpl
- def itemType = ctx._source.itemType != null ?
ctx._source.itemType.toLowerCase() : null;
- if (itemType != null && params.systemItems != null &&
params.systemItems.contains(itemType)) {
+ if (ctx._index != null &&
ctx._index.toLowerCase(Locale.ROOT).endsWith('-systemitems')) {
+ // For system items the document ID is tenantId_itemId_itemtype
(see getDocumentIDForItemType).
+ // 3.0 document IDs already carry the lower-case _itemtype suffix,
so only add it when missing.
+ def itemType = ctx._source.itemType != null ?
ctx._source.itemType.toLowerCase(Locale.ROOT) : null;
+ if (itemType != null && params.systemItems != null &&
params.systemItems.contains(itemType)
+ && !baseId.toLowerCase(Locale.ROOT).endsWith('_' +
itemType)) {
ctx._id = assignedTenantId + '_' + baseId + '_' + itemType;
} else {
ctx._id = assignedTenantId + '_' + baseId;
@@ -61,7 +69,7 @@ if (ctx._id != null) {
// systemitems must get audit here: UNOMI-997 puts shipped defs under the
system
// tenant before Unomi starts, so bundle deploy treats them as updates and
would
// otherwise leave createdBy unset on docs that never had it.
-def indexLower = ctx._index != null ? ctx._index.toLowerCase() : '';
+def indexLower = ctx._index != null ? ctx._index.toLowerCase(Locale.ROOT) : '';
if (!indexLower.endsWith('-geonameentry') &&
!indexLower.endsWith('-geonames')) {
// Handle creation date based on item type
if (ctx._source.creationDate == null) {
diff --git
a/tools/shell-commands/src/main/resources/requestBody/4.0.0/shipped_system_definitions.json
b/tools/shell-commands/src/main/resources/requestBody/4.0.0/shipped_system_definitions.json
index 22f1bfda8..3d8cd07ad 100644
---
a/tools/shell-commands/src/main/resources/requestBody/4.0.0/shipped_system_definitions.json
+++
b/tools/shell-commands/src/main/resources/requestBody/4.0.0/shipped_system_definitions.json
@@ -60,6 +60,10 @@
"conditiontype:updatepropertieseventcondition",
"conditiontype:userlistpropertycondition",
"conditiontype:videovieweventcondition",
+ "persona:europeanvisitor",
+ "persona:usvisitor",
+ "personasession:51c8e9a8-3db6-41f8-9569-057c339e3017",
+ "personasession:aa3b04bd-8f4d-4a07-8e96-d33ffa04d3d9",
"propertytype:address",
"propertytype:age",
"propertytype:birthdate",
diff --git
a/tools/shell-commands/src/test/java/org/apache/unomi/shell/migration/utils/MigrationUtilsTest.java
b/tools/shell-commands/src/test/java/org/apache/unomi/shell/migration/utils/MigrationUtilsTest.java
index cb0322d1e..f2c1ea1a1 100644
---
a/tools/shell-commands/src/test/java/org/apache/unomi/shell/migration/utils/MigrationUtilsTest.java
+++
b/tools/shell-commands/src/test/java/org/apache/unomi/shell/migration/utils/MigrationUtilsTest.java
@@ -16,23 +16,34 @@
*/
package org.apache.unomi.shell.migration.utils;
+import org.json.JSONArray;
+import org.json.JSONObject;
import org.junit.Before;
import org.junit.Test;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.TreeSet;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.*;
@@ -305,58 +316,97 @@ public class MigrationUtilsTest {
assertEquals("generic",
MigrationUtils.resolveItemType("other-profile", "context", types));
}
- @Test
- public void resolveBaseItemIdStripsItemTypeSuffixIgnoringCase() {
- assertEquals("booleanCondition",
-
MigrationUtils.resolveBaseItemId("booleanCondition_conditionType",
"conditionType"));
- assertEquals("booleanCondition",
-
MigrationUtils.resolveBaseItemId("booleanCondition_conditiontype",
"conditionType"));
- assertEquals("booleanCondition",
- MigrationUtils.resolveBaseItemId("booleanCondition",
"conditionType"));
- }
-
- @Test
- public void shouldAssignToSystemTenantForShippedConditionWithoutScope() {
- Collection<String> shipped =
Collections.singleton("conditiontype:booleancondition");
- assertTrue(MigrationUtils.shouldAssignToSystemTenant(
- "conditionType", "booleanCondition", null,
"context-systemitems", false, shipped));
- }
+ private static final String SHIPPED_DEFINITIONS =
"requestBody/4.0.0/shipped_system_definitions.json";
@Test
- public void shouldAssignUserContentWithSystemScopeToDefaultTenant() {
- Collection<String> shipped =
Collections.singleton("conditiontype:booleancondition");
- assertFalse(MigrationUtils.shouldAssignToSystemTenant(
- "rule", "my-user-rule", "systemscope", "context-systemitems",
false, shipped));
- assertFalse(MigrationUtils.shouldAssignToSystemTenant(
- "segment", "my-segment", "systemscope", "context-systemitems",
false, shipped));
- assertFalse(MigrationUtils.shouldAssignToSystemTenant(
- "scoring", "my-scoring", "systemscope", "context-systemitems",
false, shipped));
- assertFalse(MigrationUtils.shouldAssignToSystemTenant(
- "goal", "my-goal", "systemscope", "context-systemitems",
false, shipped));
+ public void loadShippedDefinitionIdsGroupsLowerCaseIdsByType() throws
Exception {
+ stubShippedDefinitions("[\"conditionType:booleanCondition\",
\"rule:_abc_myRule.json\", \"rule:other\"]");
+ Map<String, Set<String>> shipped =
MigrationUtils.loadShippedDefinitionIds(bundleContext);
+ assertEquals(Set.of("conditiontype", "rule"), shipped.keySet());
+ assertEquals(Set.of("booleancondition"), shipped.get("conditiontype"));
+ assertEquals(Set.of("_abc_myrule.json", "other"), shipped.get("rule"));
}
@Test
- public void shouldAssignGeonamesToSystemTenant() {
- assertTrue(MigrationUtils.shouldAssignToSystemTenant(
- "geonameEntry", "paris", null, "context-geonameentry", false,
Collections.emptySet()));
- assertTrue(MigrationUtils.shouldAssignToSystemTenant(
- null, "paris", null, "context-geonameentry", false,
Collections.emptySet()));
- }
-
- @Test
- public void shouldAssignProfilesEventsAndForcedDefaultToConfiguredTenant()
{
- Collection<String> shipped =
Collections.singleton("conditiontype:booleancondition");
- assertFalse(MigrationUtils.shouldAssignToSystemTenant(
- "profile", "p1", null, "context-profile", false, shipped));
- assertFalse(MigrationUtils.shouldAssignToSystemTenant(
- "event", "e1", null, "context-event-000001", false, shipped));
- assertFalse(MigrationUtils.shouldAssignToSystemTenant(
- "clusterNode", "node1", "system", "context-clusternode", true,
shipped));
+ public void loadShippedDefinitionIdsRejectsEmptyOrMalformedLists() throws
Exception {
+ for (String content : Arrays.asList("[]", "[\"booleanCondition\"]",
"[\":booleanCondition\"]", "[\"rule:\"]", "[42]")) {
+ stubShippedDefinitions(content);
+ assertThrows(content, IllegalStateException.class, () ->
MigrationUtils.loadShippedDefinitionIds(bundleContext));
+ }
}
- @Test
- public void shouldAssignLiteralSystemScopeToSystemTenant() {
- assertTrue(MigrationUtils.shouldAssignToSystemTenant(
- "sfdcConfiguration", "cfg1", "system",
"context-sfdcconfiguration", false, Collections.emptySet()));
+ /**
+ * The shipped list is a snapshot: it must be updated whenever a
definition is added to or
+ * removed from the bundles, or migrated copies land in the wrong tenant.
+ */
+ @Test
+ public void shippedDefinitionsListMatchesTheDefinitionsInTheSourceTree()
throws Exception {
+ Path repositoryRoot =
Path.of("").toAbsolutePath().getParent().getParent();
+ assertTrue("Unexpected source layout: " + repositoryRoot,
Files.isDirectory(repositoryRoot.resolve("plugins")));
+
+ Map<String, String> typeByDirectory = Map.of("conditions",
"conditiontype", "actions", "actiontype",
+ "properties", "propertytype", "rules", "rule", "personas",
"persona");
+ Map<String, Set<String>> expected = new TreeMap<>();
+ Files.walkFileTree(repositoryRoot, new SimpleFileVisitor<Path>() {
+ @Override
+ public FileVisitResult preVisitDirectory(Path dir,
BasicFileAttributes attrs) {
+ String name = dir.getFileName().toString();
+ boolean skip = name.startsWith(".") || name.equals("target")
|| name.equals("node_modules") || name.equals("test");
+ return skip ? FileVisitResult.SKIP_SUBTREE :
FileVisitResult.CONTINUE;
+ }
+
+ @Override
+ public FileVisitResult visitFile(Path file, BasicFileAttributes
attrs) throws IOException {
+ String path =
repositoryRoot.relativize(file).toString().replace(File.separatorChar, '/');
+ int cxs = path.indexOf("/src/main/resources/META-INF/cxs/");
+ if (cxs < 0 || !path.endsWith(".json")) {
+ return FileVisitResult.CONTINUE;
+ }
+ String directory = path.substring(cxs +
"/src/main/resources/META-INF/cxs/".length()).split("/")[0];
+ String type = typeByDirectory.get(directory);
+ if (type == null) {
+ return FileVisitResult.CONTINUE;
+ }
+ JSONObject definition = new JSONObject(Files.readString(file));
+ if ("persona".equals(type)) {
+ addExpected(expected, type,
definition.getJSONObject("persona").getString("itemId"));
+ JSONArray sessions = definition.getJSONArray("sessions");
+ for (int i = 0; i < sessions.length(); i++) {
+ addExpected(expected, "personasession",
sessions.getJSONObject(i).getString("itemId"));
+ }
+ } else {
+ addExpected(expected, type,
definition.getJSONObject("metadata").getString("id"));
+ }
+ return FileVisitResult.CONTINUE;
+ }
+ });
+
+
when(bundle.getResource(SHIPPED_DEFINITIONS)).thenReturn(getClass().getResource("/"
+ SHIPPED_DEFINITIONS));
+ assertEquals("Update " + SHIPPED_DEFINITIONS + " to match the
definitions shipped under META-INF/cxs",
+ expected,
MigrationUtils.loadShippedDefinitionIds(bundleContext));
+ }
+
+ private static void addExpected(Map<String, Set<String>> expected, String
type, String id) {
+ expected.computeIfAbsent(type, key -> new
TreeSet<>()).add(id.toLowerCase(Locale.ROOT));
+ }
+
+ private void stubShippedDefinitions(String content) throws IOException {
+ Path file = Files.createTempFile("shipped-definitions", ".json");
+ file.toFile().deleteOnExit();
+ Files.writeString(file, content);
+
when(bundle.getResource(SHIPPED_DEFINITIONS)).thenReturn(file.toUri().toURL());
+ }
+
+ @Test
+ public void buildItemTypeUpdateRequestMatchesKeywordAndFoldedItemType() {
+ JSONObject request = new
JSONObject(MigrationUtils.buildItemTypeUpdateRequest("ctx.op = 'noop'",
"conditionType"));
+ assertEquals("ctx.op = 'noop'",
request.getJSONObject("script").getString("source"));
+ assertEquals("painless",
request.getJSONObject("script").getString("lang"));
+ JSONObject bool = request.getJSONObject("query").getJSONObject("bool");
+ assertEquals(1, bool.getInt("minimum_should_match"));
+ JSONArray should = bool.getJSONArray("should");
+ assertEquals(2, should.length());
+ assertEquals("conditionType",
should.getJSONObject(0).getJSONObject("term").getString("itemType.keyword"));
+ assertEquals("conditiontype",
should.getJSONObject(1).getJSONObject("term").getString("itemType"));
}
}
