exceptionfactory commented on code in PR #11663:
URL: https://github.com/apache/nifi/pull/11663#discussion_r3979910160
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/components/connector/TestStandardConnectorNode.java:
##########
@@ -183,6 +184,33 @@ public void testStopFromRunningState() throws Exception {
assertFalse(stopFuture.isCancelled());
}
+ @Test
+ public void testRestartRefreshesSecretBeforeStartingConnector() throws
Exception {
+ final SecretReference secretReference = new
SecretReference("provider-id", "Provider", "password",
"Provider.group.password");
+ final Secret firstSecret = mock(Secret.class);
+ when(firstSecret.getValue()).thenReturn("first");
+ final Secret secondSecret = mock(Secret.class);
+ when(secondSecret.getValue()).thenReturn("second");
+ final AtomicReference<Secret> currentSecret = new
AtomicReference<>(firstSecret);
+
+
when(secretsManager.getSecrets(anySet())).thenReturn(Map.of(secretReference,
firstSecret));
+ when(secretsManager.getSecrets(anySet(),
eq(true))).thenReturn(Map.of(secretReference, firstSecret));
+ when(secretsManager.getSecrets(anySet(),
eq(false))).thenAnswer(invocation -> Map.of(secretReference,
currentSecret.get()));
+
+ final StartRecordingSecretConnector connector = new
StartRecordingSecretConnector();
+ final StandardConnectorNode connectorNode =
createConnectorNode(connector, secretsManager);
+ seedActiveConfiguration(connectorNode, "requiredStep",
Map.of("RequiredSecret", secretReference));
Review Comment:
The `requiredStep` and `RequiredSecret` string literals are repeated here
and in the Connector class, they should be moved to static final variables.
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/components/connector/TestStandardConnectorNode.java:
##########
@@ -183,6 +184,33 @@ public void testStopFromRunningState() throws Exception {
assertFalse(stopFuture.isCancelled());
}
+ @Test
+ public void testRestartRefreshesSecretBeforeStartingConnector() throws
Exception {
+ final SecretReference secretReference = new
SecretReference("provider-id", "Provider", "password",
"Provider.group.password");
+ final Secret firstSecret = mock(Secret.class);
+ when(firstSecret.getValue()).thenReturn("first");
+ final Secret secondSecret = mock(Secret.class);
+ when(secondSecret.getValue()).thenReturn("second");
+ final AtomicReference<Secret> currentSecret = new
AtomicReference<>(firstSecret);
+
+
when(secretsManager.getSecrets(anySet())).thenReturn(Map.of(secretReference,
firstSecret));
+ when(secretsManager.getSecrets(anySet(),
eq(true))).thenReturn(Map.of(secretReference, firstSecret));
+ when(secretsManager.getSecrets(anySet(),
eq(false))).thenAnswer(invocation -> Map.of(secretReference,
currentSecret.get()));
+
+ final StartRecordingSecretConnector connector = new
StartRecordingSecretConnector();
+ final StandardConnectorNode connectorNode =
createConnectorNode(connector, secretsManager);
+ seedActiveConfiguration(connectorNode, "requiredStep",
Map.of("RequiredSecret", secretReference));
+
+ connectorNode.start(scheduler).get(5, TimeUnit.SECONDS);
+ connectorNode.stop(scheduler).get(5, TimeUnit.SECONDS);
+ currentSecret.set(secondSecret);
+ connectorNode.start(scheduler).get(5, TimeUnit.SECONDS);
+ connectorNode.start(scheduler).get(5, TimeUnit.SECONDS);
+
+ assertEquals(List.of("first", "second"),
connector.getStartedSecrets());
Review Comment:
The `first` and `second` strings are repeated and should be declared once
and reused.
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/components/connector/MutableConnectorConfigurationContext.java:
##########
@@ -54,6 +54,16 @@ public interface MutableConnectorConfigurationContext
extends ConnectorConfigura
*/
void resolvePropertyValues();
+ /**
+ * Resolves all existing property values, optionally using cached Secret
values.
+ *
+ * @implSpec Implementations that cache Secret values must override this
method to honor {@code useCache}.
+ * @param useCache whether cached Secret values may be used
+ */
+ default void resolvePropertyValues(final boolean useCache) {
Review Comment:
Although it is more verbose, I recommend creating an enum named something
like `CacheResolution` with values of `ENABLED` and `DISABLED` to start with.
That should make the behavior more readable, and provide the option for future
extension.
--
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]
