gnodet-bot commented on code in PR #27764:
URL: https://github.com/apache/camel/pull/27764#discussion_r4257749521
##########
components/camel-vertx/camel-vertx-websocket/src/main/java/org/apache/camel/component/vertx/websocket/VertxWebsocketEndpoint.java:
##########
@@ -163,6 +164,20 @@ Lock getClientConsumerConnectLock() {
return clientConsumerConnectLock;
}
+ /**
+ * Clears the message, close, and exception handlers on the endpoint's
WebSocket, but only if it is still open and
+ * {@code owner} is the last consumer that registered handlers on it. The
caller must hold
+ * {@link #clientConsumerConnectLock}.
+ */
+ void clearWebSocketHandlers(WebSocket owner) {
+ if (owner != null && webSocket == owner && !webSocket.isClosed()) {
+ webSocket.textMessageHandler(null);
+ webSocket.binaryMessageHandler(null);
+ webSocket.closeHandler(null);
+ webSocket.exceptionHandler(null);
Review Comment:
⚠️ **Indentation:** The body of the `if` block is at the same indentation
level as the `if` keyword itself — it should be indented one more level.
```suggestion
webSocket.textMessageHandler(null);
webSocket.binaryMessageHandler(null);
webSocket.closeHandler(null);
webSocket.exceptionHandler(null);
```
##########
components/camel-vertx/camel-vertx-websocket/src/main/java/org/apache/camel/component/vertx/websocket/VertxWebsocketEndpoint.java:
##########
@@ -224,6 +243,73 @@ protected WebSocket
getWebSocket(java.util.function.Consumer<WebSocket> setupCal
future.cancel(false);
throw e;
}
+ } else if (setupCallback != null) {
+ // WebSocket already open — register the caller's handlers on the
existing socket (CAMEL-25601).
+ setupCallback.accept(webSocket);
+ }
+ return webSocket;
+ }
+
+ /**
+ * Like {@link #getWebSocket(java.util.function.Consumer)}, but the setup
callback also receives a flag indicating
+ * whether the WebSocket was already open before this call ({@code true})
or was newly connected ({@code false}).
+ * This lets the caller distinguish the two cases without a racy external
check, since {@code getWebSocket} itself
+ * knows which branch it took.
+ */
+ WebSocket getWebSocket(BiConsumer<WebSocket, Boolean> setupCallback)
throws Exception {
+ if (client == null) {
+ resolvedClientOptions = configuration.getClientOptions();
+ if (resolvedClientOptions == null) {
+ resolvedClientOptions = new WebSocketClientOptions();
+ }
+
+ SSLContextParameters sslContextParameters =
configuration.getSslContextParameters();
+ if (sslContextParameters != null) {
+ VertxHelper.setupSSLOptions(getCamelContext(),
sslContextParameters, resolvedClientOptions);
+ }
+
+ client = getVertx().createWebSocketClient(resolvedClientOptions);
+ }
+
+ if (webSocket == null || webSocket.isClosed()) {
+ WebSocketConnectOptions connectOptions =
getWebSocketConnectOptions(resolvedClientOptions);
+ CompletableFuture<WebSocket> future = new CompletableFuture<>();
+ client.connect(connectOptions).onComplete(result -> {
+ if (result.succeeded()) {
+ WebSocket ws = result.result();
+ if (future.isDone()) {
+ ws.close();
+ return;
+ }
+ LOG.info("Connected to WebSocket on {}",
ws.remoteAddress());
+ if (setupCallback != null) {
+ try {
+ setupCallback.accept(ws, false);
+ } catch (Exception e) {
+ ws.close();
+ future.completeExceptionally(e);
+ return;
+ }
+ }
+ if (!future.complete(ws)) {
+ ws.close();
+ }
+ } else {
+ future.completeExceptionally(result.cause());
+ }
+ });
+ try {
+ webSocket =
future.get(resolvedClientOptions.getConnectTimeout(), TimeUnit.MILLISECONDS);
+ } catch (Exception e) {
+ webSocket = null;
+ future.cancel(false);
+ throw e;
+ }
+ } else {
+ if (setupCallback != null) {
+ // WebSocket already open — register the caller's handlers on
the existing socket (CAMEL-25601).
+ setupCallback.accept(webSocket, true);
+ }
Review Comment:
💡 **Code duplication:** This `BiConsumer` overload duplicates the entire
connection logic from `getWebSocket(Consumer<WebSocket>)` — client init, SSL
setup, `CompletableFuture` handling, timeout, and exception handling (~50
lines). Consider having the `Consumer` overload delegate to this one instead:
```java
protected WebSocket getWebSocket(java.util.function.Consumer<WebSocket>
setupCallback) throws Exception {
return getWebSocket(setupCallback != null ? (ws, alreadyOpen) ->
setupCallback.accept(ws) : null);
}
```
This eliminates the duplication and keeps the connection logic in a single
place.
--
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]
