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]

Reply via email to