[GitHub] [nifi] markap14 commented on a change in pull request #3931: NIFI-6872: support download flow

[GitBox <gi...@apache.org>]

markap14 commented on a change in pull request #3931: NIFI-6872: support download flow
URL: https://github.com/apache/nifi/pull/3931#discussion_r357692791
 
 

 ##########
 File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/registry/flow/mapping/NiFiRegistryFlowMapper.java
 ##########
 @@ -90,75 +92,97 @@
 
     // We need to keep a mapping of component id to versionedComponentId as we transform these objects. This way, when
     // we call #mapConnectable, instead of generating a new UUID for the ConnectableComponent, we can lookup the 'versioned'
-    // identifier based on the comopnent's actual id. We do connections last, so that all components will already have been
+    // identifier based on the component's actual id. We do connections last, so that all components will already have been
     // created before attempting to create the connection, where the ConnectableDTO is converted.
     private Map<String, String> versionedComponentIds = new HashMap<>();
 
     public NiFiRegistryFlowMapper(final ExtensionManager extensionManager) {
         this.extensionManager = extensionManager;
     }
 
-    public InstantiatedVersionedProcessGroup mapProcessGroup(final ProcessGroup group, final ControllerServiceProvider serviceProvider, final FlowRegistryClient registryClient,
-                                                             final boolean mapDescendantVersionedFlows) {
+    /**
+     * Map the given process group to a versioned process group without any use of an actual flow registry even if the
+     * group is currently versioned in a registry.
+     *
+     * @param group             the process group to map
+     * @param serviceProvider   the controller service provider to use for mapping
+     * @return a complete versioned process group without any registry related details
+     */
+    public InstantiatedVersionedProcessGroup mapNonVersionedProcessGroup(final ProcessGroup group, final ControllerServiceProvider serviceProvider) {
         versionedComponentIds.clear();
 
-        final InstantiatedVersionedProcessGroup mapped = mapGroup(group, serviceProvider, registryClient, true, mapDescendantVersionedFlows);
-        populateReferencedAncestorVariables(group, mapped);
-
-        return mapped;
+        // always include descendant flows and do not apply any registry versioning info that may be present in the group
+        return mapGroup(group, serviceProvider, (processGroup, versionedGroup) -> {return true;});
     }
 
-    private void populateReferencedAncestorVariables(final ProcessGroup group, final VersionedProcessGroup versionedGroup) {
-        final Set<String> ancestorVariableNames = new HashSet<>();
-        populateVariableNames(group.getParent(), ancestorVariableNames);
-
-        final Map<String, String> implicitlyDefinedVariables = new HashMap<>();
-        for (final String variableName : ancestorVariableNames) {
-            final boolean isReferenced = !group.getComponentsAffectedByVariable(variableName).isEmpty();
-            if (isReferenced) {
-                final String value = group.getVariableRegistry().getVariableValue(variableName);
-                implicitlyDefinedVariables.put(variableName, value);
-            }
-        }
-
-        if (!implicitlyDefinedVariables.isEmpty()) {
-            // Merge the implicit variables with the explicitly defined variables for the Process Group
-            // and set those as the Versioned Group's variables.
-            if (versionedGroup.getVariables() != null) {
-                implicitlyDefinedVariables.putAll(versionedGroup.getVariables());
-            }
+    /**
+     * Map the given process group to a versioned process group using the provided registry client.
+     *
+     * @param group             the process group to map
+     * @param serviceProvider   the controller service provider to use for mapping
+     * @param registryClient    the registry client to use when retrieving versioning details
+     * @param mapDescendantVersionedFlows  true in order to include descendant flows in the mapped result
+     * @return a complete versioned process group with applicable registry related details
+     */
+    public InstantiatedVersionedProcessGroup mapProcessGroup(final ProcessGroup group, final ControllerServiceProvider serviceProvider,
+                                                             final FlowRegistryClient registryClient,
+                                                             final boolean mapDescendantVersionedFlows) {
+        versionedComponentIds.clear();
 
-            versionedGroup.setVariables(implicitlyDefinedVariables);
-        }
-    }
+        // apply registry versioning according to the lambda below
+        // NOTE: lambda refers to registry client and map descendant boolean which will not change during recursion
 
 Review comment:
   This might read more clearly if this lambda were instead created as a method: `boolean isMapDescendants(ProcessGroup processGroup, VersionedProcessGroup versionedGroup)` and then referenced here as `return mapGroup(group, serviceProvider, this::isMapDescendants)`. I think this would avoid having a lengthy lambda and obviate the need for documentation explaining the point of the lambda.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services