[Fix_#1989] Considering '.' in variable name when merging variables (a…

…pache#1991) * [Fix_#1989] Considering '.' in variable name when merging variables * [Fix_#1989] Walters comments
fjtirado · Feb 17, 2024 · 6edc9b9 · 6edc9b9
1 parent b9fa5a9
commit 6edc9b9
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 34 deletions.
diff --git a/data-index/data-index-common/src/main/java/org/kie/kogito/index/json/JsonUtils.java b/data-index/data-index-common/src/main/java/org/kie/kogito/index/json/JsonUtils.java
@@ -20,10 +20,14 @@
 
 import org.kie.kogito.event.process.ProcessInstanceDataEvent;
 import org.kie.kogito.event.usertask.UserTaskInstanceDataEvent;
+import org.kie.kogito.jackson.utils.JsonObjectUtils;
+import org.kie.kogito.jackson.utils.MergeUtils;
+import org.kie.kogito.jackson.utils.ObjectMapperFactory;
 
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.module.SimpleModule;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
 
 import io.cloudevents.jackson.JsonFormat;
@@ -50,4 +54,20 @@ public static ObjectMapper configure(ObjectMapper objectMapper) {
         objectMapper.registerModule(module);
         return objectMapper;
     }
+
+    public static ObjectNode mergeVariable(String variableName, Object variableValue, ObjectNode variables) {
+        return (ObjectNode) MergeUtils.merge(createObjectNode(variableName, variableValue), variables);
+    }
+
+    private static ObjectNode createObjectNode(String variableName, Object variableValue) {
+        int indexOf = variableName.indexOf('.');
+        ObjectNode result = ObjectMapperFactory.get().createObjectNode();
+        if (indexOf == -1) {
+            result.set(variableName, JsonObjectUtils.fromValue(variableValue));
+        } else {
+            String name = variableName.substring(0, indexOf);
+            result.set(name, createObjectNode(variableName.substring(indexOf + 1), variableValue));
+        }
+        return result;
+    }
 }
diff --git a/data-index/data-index-common/src/test/java/org/kie/kogito/index/json/JsonUtilsTest.java b/data-index/data-index-common/src/test/java/org/kie/kogito/index/json/JsonUtilsTest.java
@@ -0,0 +1,48 @@
+/*
+ * 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.kie.kogito.index.json;
+
+import org.junit.jupiter.api.Test;
+import org.kie.kogito.jackson.utils.ObjectMapperFactory;
+
+import com.fasterxml.jackson.databind.node.ObjectNode;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class JsonUtilsTest {
+
+    @Test
+    void testMergeWithDot() {
+        ObjectNode expected = ObjectMapperFactory.get().createObjectNode().set("key", ObjectMapperFactory.get().createObjectNode().put("user1", "manolo"));
+        assertThat(JsonUtils.mergeVariable("key.user1", "manolo", null)).isEqualTo(expected);
+    }
+
+    @Test
+    void testSimpleMerge() {
+        ObjectNode expected = ObjectMapperFactory.get().createObjectNode().put("user1", "manolo");
+        assertThat(JsonUtils.mergeVariable("user1", "manolo", null)).isEqualTo(expected);
+    }
+
+    @Test
+    void testComplexMergeWithDot() {
+        ObjectNode expected = ObjectMapperFactory.get().createObjectNode().set("key", ObjectMapperFactory.get().createObjectNode().put("user1", "manolo").put("user2", "pepe"));
+        assertThat(JsonUtils.mergeVariable("key.user1", "manolo", ObjectMapperFactory.get().createObjectNode().set("key", ObjectMapperFactory.get().createObjectNode().put("user2", "pepe"))))
+                .isEqualTo(expected);
+    }
+}
diff --git a/...main/java/org/kie/kogito/index/storage/merger/ProcessInstanceVariableDataEventMerger.java b/...main/java/org/kie/kogito/index/storage/merger/ProcessInstanceVariableDataEventMerger.java
@@ -18,9 +18,6 @@
  */
 package org.kie.kogito.index.storage.merger;
 
-import java.util.HashMap;
-import java.util.Map;
-
 import org.kie.kogito.event.process.ProcessInstanceDataEvent;
 import org.kie.kogito.event.process.ProcessInstanceVariableDataEvent;
 import org.kie.kogito.event.process.ProcessInstanceVariableEventBody;
@@ -29,9 +26,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.ObjectMapper;
-
 import jakarta.enterprise.context.ApplicationScoped;
 
 @ApplicationScoped
@@ -44,23 +38,8 @@ public class ProcessInstanceVariableDataEventMerger extends ProcessInstanceEvent
     public ProcessInstance merge(ProcessInstance pi, ProcessInstanceDataEvent<?> data) {
         pi = getOrNew(pi, data);
         ProcessInstanceVariableDataEvent event = (ProcessInstanceVariableDataEvent) data;
-        try {
-            ProcessInstanceVariableEventBody body = event.getData();
-            ObjectMapper mapper = JsonUtils.getObjectMapper();
-
-            Map<String, Object> variables = null;
-            if (pi.getVariables() == null) {
-                variables = new HashMap<>();
-            } else {
-                variables = new HashMap<>(mapper.treeToValue(pi.getVariables(), HashMap.class));
-            }
-            variables.put(body.getVariableName(), body.getVariableValue());
-            pi.setVariables(mapper.valueToTree(variables));
-        } catch (JsonProcessingException e) {
-            LOGGER.error("error during unmarshalling variable instance", e);
-        } catch (IllegalArgumentException e) {
-            LOGGER.error("error during merging variable instance event", e);
-        }
+        ProcessInstanceVariableEventBody body = event.getData();
+        pi.setVariables(JsonUtils.mergeVariable(body.getVariableName(), body.getVariableValue(), pi.getVariables()));
         return pi;
     }
 }
diff --git a/...a-common/src/main/java/org/kie/kogito/index/jpa/storage/ProcessInstanceEntityStorage.java b/...a-common/src/main/java/org/kie/kogito/index/jpa/storage/ProcessInstanceEntityStorage.java
@@ -38,13 +38,10 @@
 import org.kie.kogito.index.jpa.model.ProcessInstanceEntity;
 import org.kie.kogito.index.jpa.model.ProcessInstanceEntityRepository;
 import org.kie.kogito.index.jpa.model.ProcessInstanceErrorEntity;
+import org.kie.kogito.index.json.JsonUtils;
 import org.kie.kogito.index.model.MilestoneStatus;
 import org.kie.kogito.index.model.ProcessInstance;
 import org.kie.kogito.index.storage.ProcessInstanceStorage;
-import org.kie.kogito.jackson.utils.JsonObjectUtils;
-import org.kie.kogito.jackson.utils.ObjectMapperFactory;
-
-import com.fasterxml.jackson.databind.node.ObjectNode;
 
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
@@ -203,13 +200,8 @@ private void indexState(ProcessInstanceStateEventBody data, Set<String> addons,
 
     private void indexVariable(ProcessInstanceVariableEventBody data) {
         ProcessInstanceEntity pi = findOrInit(data.getProcessId(), data.getProcessInstanceId());
-        ObjectNode node = pi.getVariables();
-        if (node == null) {
-            node = ObjectMapperFactory.get().createObjectNode();
-        }
-        node.set(data.getVariableName(), JsonObjectUtils.fromValue(data.getVariableValue()));
-        // Object node is not tracked, need explicit set
-        pi.setVariables(node);
+        pi.setVariables(JsonUtils.mergeVariable(data.getVariableName(), data.getVariableValue(), pi.getVariables()));
         repository.flush();
     }
+
 }