pvillard31 commented on code in PR #11616:
URL: https://github.com/apache/nifi/pull/11616#discussion_r3923931012
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/serialization/VersionedFlowSynchronizer.java:
##########
@@ -954,7 +953,6 @@ private Map<String, Parameter> createParameterMap(
final Map<String, Parameter> parameters = new HashMap<>();
for (final VersionedParameter versioned :
versionedParameterContext.getParameters()) {
Review Comment:
Can we detect invalid legacy Parameter names, log a warning, and continue
loading them as NIFI-16285 requires?
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/serialization/VersionedFlowSynchronizerTest.java:
##########
@@ -394,10 +407,13 @@ void testSyncRejectsInvalidParameterName() {
versionedParameterContext.setParameters(Collections.singleton(versionedParameter));
when(versionedDataflow.getParameterContexts()).thenReturn(List.of(versionedParameterContext));
- final FlowSynchronizationException exception =
assertThrows(FlowSynchronizationException.class, () ->
+ assertDoesNotThrow(() ->
versionedFlowSynchronizer.sync(flowController, dataFlow,
flowService, BundleUpdateStrategy.USE_SPECIFIED_OR_GHOST));
- final IllegalArgumentException cause =
assertInstanceOf(IllegalArgumentException.class, exception.getCause());
- assertTrue(cause.getMessage().contains(invalidParameterName));
+
+ final ParameterContext loadedContext =
contextManager.getParameterContext("parameter-context-id");
Review Comment:
Should we add a case with an existing Parameter Context to cover
reconciliation during cluster flow inheritance or replacement?
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/serialization/VersionedFlowSynchronizerTest.java:
##########
@@ -394,10 +407,13 @@ void testSyncRejectsInvalidParameterName() {
versionedParameterContext.setParameters(Collections.singleton(versionedParameter));
when(versionedDataflow.getParameterContexts()).thenReturn(List.of(versionedParameterContext));
- final FlowSynchronizationException exception =
assertThrows(FlowSynchronizationException.class, () ->
+ assertDoesNotThrow(() ->
versionedFlowSynchronizer.sync(flowController, dataFlow,
flowService, BundleUpdateStrategy.USE_SPECIFIED_OR_GHOST));
- final IllegalArgumentException cause =
assertInstanceOf(IllegalArgumentException.class, exception.getCause());
- assertTrue(cause.getMessage().contains(invalidParameterName));
+
+ final ParameterContext loadedContext =
contextManager.getParameterContext("parameter-context-id");
+ final Optional<Parameter> loaded =
loadedContext.getParameter(invalidParameterName);
+ assertTrue(loaded.isPresent(), "Illegal Parameter already present in
the flow must be loaded so it can be removed");
Review Comment:
Can this test also verify that loading the invalid Parameter produces a
warning without exposing its value?
--
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]
