You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by jo...@apache.org on 2021/12/13 18:43:22 UTC

[nifi] 18/22: NIFI-9202 Improve Allowable Values merging to handle cases when different nodes have different set of Allowable Values.

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

joewitt pushed a commit to branch support/nifi-1.15
in repository https://gitbox.apache.org/repos/asf/nifi.git

commit 601ad648a02fc4eb7d012c4133fa959deb90007e
Author: Tamas Palfy <ta...@gmail.com>
AuthorDate: Fri Sep 10 14:27:21 2021 +0200

    NIFI-9202 Improve Allowable Values merging to handle cases when different nodes have different set of Allowable Values.
---
 .../manager/AllowableValueEntityMerger.java        |  10 ++
 .../manager/PropertyDescriptorDtoMerger.java       |  38 ++---
 .../manager/PropertyDescriptorDtoMergerTest.java   | 183 +++++++++++++++++++++
 3 files changed, 212 insertions(+), 19 deletions(-)

diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/AllowableValueEntityMerger.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/AllowableValueEntityMerger.java
index 64e83f3..6a70946 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/AllowableValueEntityMerger.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/AllowableValueEntityMerger.java
@@ -19,8 +19,18 @@ package org.apache.nifi.cluster.manager;
 import org.apache.nifi.web.api.entity.AllowableValueEntity;
 
 import java.util.Collection;
+import java.util.List;
 
 public class AllowableValueEntityMerger {
+    public static AllowableValueEntity merge(List<AllowableValueEntity> entities) {
+        AllowableValueEntity mergedEntity;
+
+        mergedEntity = entities.remove(0);
+        merge(mergedEntity, entities);
+
+        return mergedEntity;
+    }
+
     public static void merge(AllowableValueEntity clientAllowableValue, Collection<AllowableValueEntity> allowableValues) {
         for (AllowableValueEntity allowableValue : allowableValues) {
             if (clientAllowableValue.getCanRead() && !allowableValue.getCanRead()) {
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/PropertyDescriptorDtoMerger.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/PropertyDescriptorDtoMerger.java
index 3c18ced..7e74cc3 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/PropertyDescriptorDtoMerger.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/PropertyDescriptorDtoMerger.java
@@ -17,36 +17,36 @@
 package org.apache.nifi.cluster.manager;
 
 import org.apache.nifi.cluster.protocol.NodeIdentifier;
+import org.apache.nifi.web.api.dto.AllowableValueDTO;
 import org.apache.nifi.web.api.dto.PropertyDescriptorDTO;
 import org.apache.nifi.web.api.entity.AllowableValueEntity;
 
 import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 public class PropertyDescriptorDtoMerger {
     public static void merge(PropertyDescriptorDTO clientPropertyDescriptor, Map<NodeIdentifier, PropertyDescriptorDTO> dtoMap) {
-        final Map<Integer, List<AllowableValueEntity>> allowableValueMap = new HashMap<>();
+        if (clientPropertyDescriptor.getAllowableValues() != null) {
+            Map<AllowableValueDTO, List<AllowableValueEntity>> allowableValueDtoToEntities = new LinkedHashMap<>();
 
-        // values are guaranteed to be in order, so map each allowable value for each property descriptor across all node IDs to the index of the value in the descriptor's list of allowable values
-        for (final Map.Entry<NodeIdentifier, PropertyDescriptorDTO> nodeEntry : dtoMap.entrySet()) {
-            final PropertyDescriptorDTO nodePropertyDescriptor = nodeEntry.getValue();
-            final List<AllowableValueEntity> nodePropertyDescriptorAllowableValues = nodePropertyDescriptor.getAllowableValues();
-            if (nodePropertyDescriptorAllowableValues != null) {
-                nodePropertyDescriptorAllowableValues.stream().forEach(allowableValueEntity -> {
-                    allowableValueMap.computeIfAbsent(nodePropertyDescriptorAllowableValues.indexOf(allowableValueEntity), propertyDescriptorToAllowableValue -> new ArrayList<>())
-                            .add(allowableValueEntity);
-                });
-            }
-        }
+            addEntities(clientPropertyDescriptor, allowableValueDtoToEntities);
+            dtoMap.values().forEach(propertyDescriptorDTO -> addEntities(propertyDescriptorDTO, allowableValueDtoToEntities));
+
+            List<AllowableValueEntity> mergedAllowableValues = allowableValueDtoToEntities.values().stream()
+                .filter(entities -> Integer.valueOf(entities.size()).equals(dtoMap.size() + 1))
+                .map(AllowableValueEntityMerger::merge)
+                .collect(Collectors.toList());
 
-        // for each AllowableValueEntity in this PropertyDescriptorDTO, get the corresponding AVs previously aggregated and merge them.
-        final List<AllowableValueEntity> clientPropertyDescriptorAllowableValues = clientPropertyDescriptor.getAllowableValues();
-        if (clientPropertyDescriptorAllowableValues != null) {
-            for (AllowableValueEntity clientAllowableValueEntity : clientPropertyDescriptorAllowableValues) {
-                AllowableValueEntityMerger.merge(clientAllowableValueEntity, allowableValueMap.get(clientPropertyDescriptorAllowableValues.indexOf(clientAllowableValueEntity)));
-            }
+            clientPropertyDescriptor.setAllowableValues(mergedAllowableValues);
         }
     }
+
+    private static void addEntities(PropertyDescriptorDTO propertyDescriptorDTO, Map<AllowableValueDTO, List<AllowableValueEntity>> dtoToEntities) {
+        propertyDescriptorDTO.getAllowableValues().forEach(
+            allowableValueEntity -> dtoToEntities.computeIfAbsent(allowableValueEntity.getAllowableValue(), __ -> new ArrayList<>()).add(allowableValueEntity)
+        );
+    }
 }
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/java/org/apache/nifi/cluster/manager/PropertyDescriptorDtoMergerTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/java/org/apache/nifi/cluster/manager/PropertyDescriptorDtoMergerTest.java
new file mode 100644
index 0000000..120bc08
--- /dev/null
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/java/org/apache/nifi/cluster/manager/PropertyDescriptorDtoMergerTest.java
@@ -0,0 +1,183 @@
+/*
+ * 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.nifi.cluster.manager;
+
+import org.apache.nifi.cluster.protocol.NodeIdentifier;
+import org.apache.nifi.util.EqualsWrapper;
+import org.apache.nifi.web.api.dto.AllowableValueDTO;
+import org.apache.nifi.web.api.dto.PropertyDescriptorDTO;
+import org.apache.nifi.web.api.entity.AllowableValueEntity;
+import org.junit.jupiter.api.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+public class PropertyDescriptorDtoMergerTest {
+    @Test
+    void testMergeWithNoAllowableValues() {
+        // WHEN
+        PropertyDescriptorDTO clientPropertyDescriptor = new PropertyDescriptorDTO();
+
+        HashMap<NodeIdentifier, PropertyDescriptorDTO> dtoMap = new HashMap<NodeIdentifier, PropertyDescriptorDTO>() {{
+            put(createNodeIdentifier("node1"), new PropertyDescriptorDTO());
+            put(createNodeIdentifier("node2"), new PropertyDescriptorDTO());
+        }};
+
+        PropertyDescriptorDtoMerger.merge(clientPropertyDescriptor, dtoMap);
+
+        // THEN
+        assertNull(clientPropertyDescriptor.getAllowableValues());
+    }
+
+    @Test
+    void testMergeWithEmptyAllowableValuesList() {
+        testMerge(
+            createPropertyDescriptorDTO(),
+            new HashMap<NodeIdentifier, PropertyDescriptorDTO>() {{
+                put(createNodeIdentifier("node1"), createPropertyDescriptorDTO());
+                put(createNodeIdentifier("node2"), createPropertyDescriptorDTO());
+            }},
+            createPropertyDescriptorDTO()
+        );
+    }
+
+    @Test
+    void testMergeWithSingleNode() {
+        testMerge(
+            createPropertyDescriptorDTO(v("value1"), v("value2")),
+            Collections.emptyMap(),
+            createPropertyDescriptorDTO(v("value1"), v("value2"))
+        );
+    }
+
+    @Test
+    void testMergeWithNonOverlappingAllowableValues() {
+        testMerge(
+            createPropertyDescriptorDTO(v("value1"), v("value2")),
+            new HashMap<NodeIdentifier, PropertyDescriptorDTO>() {{
+                put(createNodeIdentifier("node1"), createPropertyDescriptorDTO(v("value3")));
+                put(createNodeIdentifier("node2"), createPropertyDescriptorDTO(v("value4"), v("value5"), v("value6")));
+            }},
+            createPropertyDescriptorDTO()
+        );
+    }
+
+    @Test
+    void testMergeWithOverlappingAllowableValues() {
+        testMerge(
+            createPropertyDescriptorDTO(v("value1"), v("value2"), v("value3")),
+            new HashMap<NodeIdentifier, PropertyDescriptorDTO>() {{
+                put(createNodeIdentifier("node1"), createPropertyDescriptorDTO(v("value1"), v("value2"), v("value3")));
+                put(createNodeIdentifier("node2"), createPropertyDescriptorDTO(v("value2"), v("value3", false)));
+            }},
+            createPropertyDescriptorDTO(v("value2"), v("value3", false))
+        );
+    }
+
+    @Test
+    void testMergeWithIdenticalAllowableValues() {
+        testMerge(
+            createPropertyDescriptorDTO(v("value1"), v("value2")),
+            new HashMap<NodeIdentifier, PropertyDescriptorDTO>() {{
+                put(createNodeIdentifier("node1"), createPropertyDescriptorDTO(v("value1"), v("value2")));
+                put(createNodeIdentifier("node2"), createPropertyDescriptorDTO(v("value1"), v("value2")));
+            }},
+            createPropertyDescriptorDTO(v("value1"), v("value2"))
+        );
+    }
+
+    private PropertyDescriptorDTO createPropertyDescriptorDTO(AllowableValueData... allowableValueData) {
+        PropertyDescriptorDTO clientPropertyDescriptor = new PropertyDescriptorDTO();
+
+        List<AllowableValueEntity> allowableValueEntities = Arrays.stream(allowableValueData)
+            .map(AllowableValueData::toEntity)
+            .collect(Collectors.toList());
+
+        clientPropertyDescriptor.setAllowableValues(allowableValueEntities);
+
+        return clientPropertyDescriptor;
+    }
+
+    private NodeIdentifier createNodeIdentifier(String id) {
+        NodeIdentifier nodeIdentifier = new NodeIdentifier(id, id, 1, id, 1, id, 1, null, false);
+
+        return nodeIdentifier;
+    }
+
+    private void testMerge(
+        PropertyDescriptorDTO clientPropertyDescriptor,
+        Map<NodeIdentifier, PropertyDescriptorDTO> dtoMap,
+        PropertyDescriptorDTO expected
+    ) {
+        // WHEN
+        PropertyDescriptorDtoMerger.merge(clientPropertyDescriptor, dtoMap);
+
+        // THEN
+        List<Function<AllowableValueEntity, Object>> equalsProperties = Arrays.asList(
+            AllowableValueEntity::getAllowableValue,
+            AllowableValueEntity::getCanRead
+        );
+
+        List<EqualsWrapper<AllowableValueEntity>> expectedWrappers = EqualsWrapper.wrapList(expected.getAllowableValues(), equalsProperties);
+        List<EqualsWrapper<AllowableValueEntity>> actualWrappers = EqualsWrapper.wrapList(clientPropertyDescriptor.getAllowableValues(), equalsProperties);
+
+        assertEquals(expectedWrappers, actualWrappers);
+
+    }
+
+    private static class AllowableValueData {
+        private final String value;
+        private final Boolean canRead;
+
+        private AllowableValueData(String value, Boolean canRead) {
+            this.value = value;
+            this.canRead = canRead;
+        }
+
+        private AllowableValueEntity toEntity() {
+            AllowableValueEntity entity = new AllowableValueEntity();
+
+            AllowableValueDTO allowableValueDTO = new AllowableValueDTO();
+            allowableValueDTO.setValue(value);
+
+            entity.setAllowableValue(allowableValueDTO);
+            entity.setCanRead(canRead);
+
+            return entity;
+        }
+    }
+
+    private static AllowableValueData v(String value) {
+        AllowableValueData allowableValueData = v(value, true);
+
+        return allowableValueData;
+    }
+
+    private static AllowableValueData v(String value, Boolean canRead) {
+        AllowableValueData allowableValueData = new AllowableValueData(value, canRead);
+
+        return allowableValueData;
+    }
+}