[unomi] 01/01: UNOMI-709: avoid updating property type mapping in case a mapping already exists

[jk...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jkevan pushed a commit to branch propertyTypeAutoMapping
in repository https://gitbox.apache.org/repos/asf/unomi.git

commit 8b47b108d9375fed3c5f8098c7b2616c49c38662
Author: Kevan <ke...@jahia.com>
AuthorDate: Fri Mar 24 16:09:14 2023 +0100

    UNOMI-709: avoid updating property type mapping in case a mapping already exists
---
 .../ElasticSearchPersistenceServiceImpl.java       | 38 +++++++++++++++++-----
 .../services/impl/profiles/ProfileServiceImpl.java |  4 ++-
 2 files changed, 32 insertions(+), 10 deletions(-)

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 4562dd5cc..2ad652387 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
@@ -1544,11 +1544,6 @@ public class ElasticSearchPersistenceServiceImpl implements PersistenceService,
 
     public void setPropertyMapping(final PropertyType property, final String itemType) {
         try {
-            Map<String, Object> propertyMapping = createPropertyMapping(property);
-            if (propertyMapping.isEmpty()) {
-                return;
-            }
-
             Map<String, Map<String, Object>> mappings = getPropertiesMapping(itemType);
             if (mappings == null) {
                 mappings = new HashMap<>();
@@ -1556,6 +1551,16 @@ public class ElasticSearchPersistenceServiceImpl implements PersistenceService,
             Map<String, Object> subMappings = mappings.computeIfAbsent("properties", k -> new HashMap<>());
             Map<String, Object> subSubMappings = (Map<String, Object>) subMappings.computeIfAbsent("properties", k -> new HashMap<>());
 
+            if (subSubMappings.containsKey(property.getItemId())) {
+                logger.warn("Mapping already exists for type " + itemType + " and property " + property.getItemId());
+                return;
+            }
+
+            Map<String, Object> propertyMapping = createPropertyMapping(property);
+            if (propertyMapping.isEmpty()) {
+                return;
+            }
+
             mergePropertiesMapping(subSubMappings, propertyMapping);
 
             Map<String, Object> mappingsWrapper = new HashMap<>();
@@ -1621,16 +1626,31 @@ public class ElasticSearchPersistenceServiceImpl implements PersistenceService,
                 return "date";
             case "string":
             case "id":
+            case "email": // TODO Consider supporting email mapping in ES, right now will be map to text to avoid warning in logs
                 return "text";
             default:
                 return null;
         }
     }
 
-    private void putMapping(final String source, final String indexName) throws IOException {
-        PutMappingRequest putMappingRequest = new PutMappingRequest(indexName);
-        putMappingRequest.source(source, XContentType.JSON);
-        client.indices().putMapping(putMappingRequest, RequestOptions.DEFAULT);
+    private boolean putMapping(final String source, final String indexName) throws IOException {
+        Boolean result = new InClassLoaderExecute<Boolean>(metricsService, this.getClass().getName() + ".putMapping", this.bundleContext, this.fatalIllegalStateErrors, throwExceptions) {
+            protected Boolean execute(Object... args) throws Exception {
+                try {
+                    PutMappingRequest putMappingRequest = new PutMappingRequest(indexName);
+                    putMappingRequest.source(source, XContentType.JSON);
+                    AcknowledgedResponse response = client.indices().putMapping(putMappingRequest, RequestOptions.DEFAULT);
+                    return response.isAcknowledged();
+                } catch (Exception e) {
+                    throw new Exception("Cannot create/update mapping", e);
+                }
+            }
+        }.catchingExecuteInClassLoader(true);
+        if (result == null) {
+            return false;
+        } else {
+            return result;
+        }
     }
 
     @Override
diff --git a/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java
index 211e3e3de..a595ba892 100644
--- a/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java
+++ b/services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java
@@ -475,13 +475,15 @@ public class ProfileServiceImpl implements ProfileService, SynchronousBundleList
         PropertyType previousProperty = persistenceService.load(property.getItemId(), PropertyType.class);
         boolean result = false;
         if (previousProperty == null) {
+            persistenceService.setPropertyMapping(property, Profile.ITEM_TYPE);
             result = persistenceService.save(property);
             propertyTypes = propertyTypes.with(property);
         } else if (merge(previousProperty, property)) {
+            persistenceService.setPropertyMapping(previousProperty, Profile.ITEM_TYPE);
             result = persistenceService.save(previousProperty);
             propertyTypes = propertyTypes.with(previousProperty);
         }
-        persistenceService.setPropertyMapping(property, Profile.ITEM_TYPE);
+
         return result;
     }