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)`
--
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]
