ncover21 commented on code in PR #11581:
URL: https://github.com/apache/nifi/pull/11581#discussion_r3833054963
##########
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);
+ for (final ConfigurationStep configurationStep : configurationSteps) {
+ final Map<String, ConnectorValueReference> propertyValues = new
HashMap<>();
+ final StepConfiguration existingConfiguration =
propertiesWithDefaults.get(configurationStep.getName());
+ if (existingConfiguration != null &&
existingConfiguration.getPropertyValues() != null) {
+
propertyValues.putAll(existingConfiguration.getPropertyValues());
+ }
+
+ boolean appliedMissingDefault = false;
+ for (final ConnectorPropertyGroup propertyGroup :
configurationStep.getPropertyGroups()) {
+ for (final ConnectorPropertyDescriptor descriptor :
propertyGroup.getProperties()) {
+ if (propertyValues.containsKey(descriptor.getName()) ||
descriptor.getDefaultValue() == null) {
Review Comment:
One thing im not sure if im reading correctly here, the defaults get applied
without looking at property or step dependencies.
`AbstractConnector.isDependencySatisfied` reads the controlling value through
the name-based `getProperty(stepName, name)` overload, which never returns
null, so an unset controlling property reads as null and the dependent property
stays gated off.
So for a NAR that adds `SSL Mode` (default `REQUIRED`) plus a required
`Truststore Filename` that dependsOn(SSL Mode, "REQUIRED"): before this change
the connector stays valid because SSL Mode is unset, after it the default makes
the dependency satisfied and `Truststore Filename` reports as required. That
would be the opposite of the intent. Does that hold or am I misreading the
dependency check?
--
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]
