ncover21 commented on code in PR #11581:
URL: https://github.com/apache/nifi/pull/11581#discussion_r3833029097


##########
nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/components/connector/StandardConnectorNode.java:
##########
@@ -422,8 +422,48 @@ private Map<String, StepConfiguration> 
migrateProperties(final List<VersionedCon
         final StandardConnectorPropertyConfiguration propertyConfiguration = 
new StandardConnectorPropertyConfiguration(initial, this.toString());
         try (final NarCloseable ignored = 
NarCloseable.withComponentNarLoader(extensionManager, 
getConnector().getClass(), getIdentifier())) {
             getConnector().migrateProperties(propertyConfiguration);
+            return 
applyMissingPropertyDefaults(propertyConfiguration.getMutatedProperties(), 
getConnector().getConfigurationSteps());
         }
-        return propertyConfiguration.getMutatedProperties();
+    }
+
+    /**
+     * For each property declared on the Connector that has a default but no 
value in the given configuration,
+     * inserts that default. This is needed when a Connector NAR adds a 
property: the saved flow has no entry
+     * for it, so without filling in the default the Connector would be 
invalid. Properties that already have a
+     * value, and properties that have no default, are left unchanged.
+     */
+    private Map<String, StepConfiguration> applyMissingPropertyDefaults(final 
Map<String, StepConfiguration> migratedProperties, final 
List<ConfigurationStep> configurationSteps) {
+        if (configurationSteps == null || configurationSteps.isEmpty()) {
+            return migratedProperties;
+        }
+
+        final Map<String, StepConfiguration> propertiesWithDefaults = new 
HashMap<>(migratedProperties);

Review Comment:
   I think this needs to be a LinkedHashMap. `migrateProperties` builds initial 
as a LinkedHashMap with a comment about preserving persisted step order for the 
`notifyStepConfigured` loop at line 407, and copying into a `HashMap` reorders 
by hash. So a connector with steps Connection, Filter, Destination would get 
its onConfigurationStepConfigured callbacks in hash order on every flow load, 
but in declaration order when configured through the UI. If a later step reads 
values an earlier one established it would build a different managed flow after 
a restart.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to