turcsanyip commented on code in PR #7246:
URL: https://github.com/apache/nifi/pull/7246#discussion_r1194007689
##########
nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-processors/src/main/java/org/apache/nifi/processors/websocket/AbstractWebSocketGatewayProcessor.java:
##########
@@ -77,6 +77,18 @@ public abstract class AbstractWebSocketGatewayProcessor
extends AbstractSessionF
.description("The WebSocket binary message output")
.build();
+ public static final Relationship REL_SUCCESS = new Relationship.Builder()
+ .name("success")
+ .description("Connection HTTP header attributes in case of
successful connection")
+ .autoTerminateDefault(true)
+ .build();
+
+ public static final Relationship REL_FAILURE = new Relationship.Builder()
+ .name("failure")
+ .description("Connection HTTP header attributes in case of
connection failure")
+ .autoTerminateDefault(true)
+ .build();
+
Review Comment:
"Connection HTTP header attributes" is a bit unclear in this context and
other config parameters can be passed also. I would suggest the following
descriptions:
```suggestion
public static final Relationship REL_SUCCESS = new Relationship.Builder()
.name("success")
.description("FlowFile holding connection configuration
attributes (like URL or HTTP headers) in case of successful connection")
.autoTerminateDefault(true)
.build();
public static final Relationship REL_FAILURE = new Relationship.Builder()
.name("failure")
.description("FlowFile holding connection configuration
attributes (like URL or HTTP headers) in case of connection failure")
.autoTerminateDefault(true)
.build();
```
##########
nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-processors/src/test/java/org/apache/nifi/processors/websocket/TestConnectWebSocket.java:
##########
@@ -162,10 +154,44 @@ void testDynamicUrlsParsedFromFlowFileAndAbleToConnect()
throws InitializationEx
final List<MockFlowFile> flowFilesForRelationship =
runner.getFlowFilesForRelationship(ConnectWebSocket.REL_CONNECTED);
assertEquals(1, flowFilesForRelationship.size());
+ final List<MockFlowFile> flowFilesForSuccess =
runner.getFlowFilesForRelationship(ConnectWebSocket.REL_SUCCESS);
+ assertEquals(1, flowFilesForSuccess.size());
+
runner.stop();
webSocketListener.stop();
}
+ @Test
+ void testDynamicUrlsTransferredToFailure() throws InitializationException {
Review Comment:
The following test name would describe the test scenario more
straightforward:
```suggestion
void testDynamicUrlsParsedFromFlowFileButNotAbleToConnect() throws
InitializationException {
```
##########
nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-processors/src/test/java/org/apache/nifi/processors/websocket/TestConnectWebSocket.java:
##########
@@ -162,10 +154,44 @@ void testDynamicUrlsParsedFromFlowFileAndAbleToConnect()
throws InitializationEx
final List<MockFlowFile> flowFilesForRelationship =
runner.getFlowFilesForRelationship(ConnectWebSocket.REL_CONNECTED);
assertEquals(1, flowFilesForRelationship.size());
+ final List<MockFlowFile> flowFilesForSuccess =
runner.getFlowFilesForRelationship(ConnectWebSocket.REL_SUCCESS);
+ assertEquals(1, flowFilesForSuccess.size());
+
runner.stop();
webSocketListener.stop();
}
+ @Test
+ void testDynamicUrlsTransferredToFailure() throws InitializationException {
+ final TestRunner runner =
TestRunners.newTestRunner(ConnectWebSocket.class);
+
+ final String serviceId = "ws-service";
+ final String endpointId = "client-1";
+
+ MockFlowFile flowFile = getFlowFile();
+ runner.enqueue(flowFile);
+
+ JettyWebSocketClient service = new JettyWebSocketClient();
+
+
+ runner.addControllerService(serviceId, service);
+ runner.setProperty(service, JettyWebSocketClient.WS_URI,
"ws://localhost/12345");
Review Comment:
No need to set a fake URL because the failure comes due to ListenWebSocket
not started.
```suggestion
runner.setProperty(service, JettyWebSocketClient.WS_URI,
"ws://localhost/${dynamicUrlPart}");
```
--
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]