[ https://issues.apache.org/jira/browse/WICKET-6969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17520764#comment-17520764 ]
ASF GitHub Bot commented on WICKET-6969: ---------------------------------------- martin-g commented on code in PR #509: URL: https://github.com/apache/wicket/pull/509#discussion_r847619483 ########## wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/WebSocketSettings.java: ########## @@ -158,11 +158,24 @@ public void configureSession(IWebSocketSession webSocketSession) { */ private Function<Integer, Boolean> notifyOnCloseEvent = (code) -> true; - public boolean shouldNotifyOnCloseEvent(int closeCode) { + /** + * Flag that allows to use asynchronous push. By default, it is set to false. + */ + private boolean asynchronousPush = false; + + /** + * The timeout to use for asynchronous push. By default, it is -1 which means use timeout configured by + * sever implementation. Review Comment: ```suggestion * server implementation. ``` ########## wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/WebSocketSettings.java: ########## @@ -158,11 +158,24 @@ public void configureSession(IWebSocketSession webSocketSession) { */ private Function<Integer, Boolean> notifyOnCloseEvent = (code) -> true; - public boolean shouldNotifyOnCloseEvent(int closeCode) { + /** + * Flag that allows to use asynchronous push. By default, it is set to false. Review Comment: ```suggestion * Flag that allows to use asynchronous push. By default, it is set to <code>false</code>. ``` ########## wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/WebSocketSettings.java: ########## @@ -305,7 +320,20 @@ public IWebSocketConnectionFilter getConnectionFilter() */ public WebResponse newWebSocketResponse(IWebSocketConnection connection) { - return new WebSocketResponse(connection); + return new WebSocketResponse(connection, isAsynchronousPush(), getAsynchronousPushTimeout()); Review Comment: ```suggestion return newWebSocketResponse(connection, isAsynchronousPush(), getAsynchronousPushTimeout()); ``` ########## wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/WebSocketSettings.java: ########## @@ -305,7 +320,20 @@ public IWebSocketConnectionFilter getConnectionFilter() */ public WebResponse newWebSocketResponse(IWebSocketConnection connection) { - return new WebSocketResponse(connection); + return new WebSocketResponse(connection, isAsynchronousPush(), getAsynchronousPushTimeout()); + } + + /** + * A factory method for the {@link org.apache.wicket.request.http.WebResponse} + * that should be used to write the response back to the client/browser + * + * @param connection + * The active web socket connection Review Comment: missing `@param`s for `asynchronousPush` and `timeout` ########## wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketRequestHandler.java: ########## @@ -97,6 +119,27 @@ public void push(byte[] message, int offset, int length) } } + @Override + public Future<Void> pushAsync(byte[] message, int offset, int length) + { + return pushAsync(message, offset, length, -1); + } + + @Override + public Future<Void> pushAsync(byte[] message, int offset, int length, long timeout) + { + if (connection.isOpen()) + { + Args.notNull(message, "message"); + return connection.sendMessageAsync(message, offset, length, timeout); + } + else + { + LOG.warn("The websocket connection is already closed. Cannot push the binary message '{}'", message); + } + return null; Review Comment: ```suggestion return java.util.concurrent.CompletableFuture.completedFuture(null); ``` ########## wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketResponse.java: ########## @@ -87,13 +100,34 @@ public void close() { if (text != null) { - connection.sendMessage(text.toString()); + if (asynchronous) + { + if (timeout > 0) + { + connection.sendMessageAsync(text.toString(), timeout); + } + else + { + connection.sendMessageAsync(text.toString()); + } + } + else + { + connection.sendMessage(text.toString()); + } text = null; } else if (binary != null) { byte[] bytes = binary.toByteArray(); - connection.sendMessage(bytes, 0, bytes.length); + if (asynchronous) + { + connection.sendMessageAsync(bytes, 0, bytes.length); Review Comment: Why this method does not use the `timeout` ? ########## wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketRequestHandler.java: ########## @@ -77,6 +78,27 @@ public void push(CharSequence message) } } + @Override + public Future<Void> pushAsync(CharSequence message, long timeout) + { + if (connection.isOpen()) + { + Args.notNull(message, "message"); + return connection.sendMessageAsync(message.toString(), timeout); + } + else + { + LOG.warn("The websocket connection is already closed. Cannot push the text message '{}'", message); + } + return null; Review Comment: Better return `java.util.concurrent.CompletableFuture#completedFuture(null)` > allow to process web socket push messages in an asynchronous way. > ----------------------------------------------------------------- > > Key: WICKET-6969 > URL: https://issues.apache.org/jira/browse/WICKET-6969 > Project: Wicket > Issue Type: Improvement > Reporter: Ernesto Reinaldo Barreiro > Priority: Major > > Currently web socket push messages are always processed in a synchronous > way. Allow to configure applications to use by default asynchronous messages. -- This message was sent by Atlassian Jira (v8.20.1#820001)