You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by GitBox <gi...@apache.org> on 2022/04/05 16:47:10 UTC
[GitHub] [wicket] reiern70 opened a new pull request, #509: {WICKET-6969} allow asynchronous pushing of messages.
reiern70 opened a new pull request, #509:
URL: https://github.com/apache/wicket/pull/509
WIP: do not review just yet.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r847826337
##########
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:
Done.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
solomax commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r843422043
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketRequestHandler.java:
##########
@@ -97,6 +128,36 @@ public void push(byte[] message, int offset, int length)
}
}
+ @Override
+ public Future<Void> pushAsync(byte[] message, int offset, int length)
Review Comment:
I would replace this method with `return pushAsync(message, offset, length, -1)`
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
solomax commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r843422352
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketResponse.java:
##########
@@ -47,9 +47,22 @@
private boolean isRedirect = false;
+ private final boolean asynchronous;
+
+ private final long timeout;
+
public WebSocketResponse(final IWebSocketConnection conn)
{
this.connection = conn;
Review Comment:
this can be replaced with `this(conn, false, -1)`
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r847824514
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketResponse.java:
##########
@@ -47,9 +47,22 @@
private boolean isRedirect = false;
+ private final boolean asynchronous;
+
+ private final long timeout;
+
public WebSocketResponse(final IWebSocketConnection conn)
{
this.connection = conn;
Review Comment:
Done
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
solomax commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r843420871
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketPushBroadcaster.java:
##########
@@ -189,7 +189,20 @@ private void process(final Application application, final Collection<IWebSocketC
@Override
public void run()
{
- wsConnection.sendMessage(message);
+ if (webSocketSettings.isAsynchronousPush())
+ {
+ if (webSocketSettings.getAsynchronousPushTimeout() > 0)
+ {
+ wsConnection.sendMessageAsync(message, webSocketSettings.getAsynchronousPushTimeout());
+ }
+ else {
Review Comment:
`else {` ->
```
else
{
```
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r847694761
##########
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:
Overlooked. Fixing, Thanks!
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r849884786
##########
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:
Done
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r849884537
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketPushBroadcaster.java:
##########
@@ -189,7 +189,20 @@ private void process(final Application application, final Collection<IWebSocketC
@Override
public void run()
{
- wsConnection.sendMessage(message);
+ if (webSocketSettings.isAsynchronousPush())
+ {
+ if (webSocketSettings.getAsynchronousPushTimeout() > 0)
Review Comment:
Done
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
solomax commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r843422972
##########
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)
Review Comment:
I would drop this `if` in favor of `connection.sendMessageAsync(text.toString(), timeout);`
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
martin-g commented on PR #509:
URL: https://github.com/apache/wicket/pull/509#issuecomment-1096089723
Please do not force-push between reviews! This way the reviewer(s) have to start from the beginning.
It is much easier to review just the new commits and to "Squash and merge" at the end.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r843936165
##########
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
+ * @return the response object that should be used to write the response back to the client
+ */
+ public WebResponse newWebSocketResponse(IWebSocketConnection connection, boolean asynchronousPush, long timeout)
Review Comment:
Thanks! Fixed.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r843954113
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketPushBroadcaster.java:
##########
@@ -189,7 +189,20 @@ private void process(final Application application, final Collection<IWebSocketC
@Override
public void run()
{
- wsConnection.sendMessage(message);
+ if (webSocketSettings.isAsynchronousPush())
+ {
+ if (webSocketSettings.getAsynchronousPushTimeout() > 0)
+ {
+ wsConnection.sendMessageAsync(message, webSocketSettings.getAsynchronousPushTimeout());
+ }
+ else {
Review Comment:
done
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
solomax commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r843419552
##########
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
+ * @return the response object that should be used to write the response back to the client
+ */
+ public WebResponse newWebSocketResponse(IWebSocketConnection connection, boolean asynchronousPush, long timeout)
Review Comment:
extra whitespace :)
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
solomax commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r843423501
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketPushBroadcaster.java:
##########
@@ -189,7 +189,20 @@ private void process(final Application application, final Collection<IWebSocketC
@Override
public void run()
{
- wsConnection.sendMessage(message);
+ if (webSocketSettings.isAsynchronousPush())
+ {
+ if (webSocketSettings.getAsynchronousPushTimeout() > 0)
Review Comment:
I would drop this `if` and call the function with `timeout`
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r843958807
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketRequestHandler.java:
##########
@@ -97,6 +128,36 @@ public void push(byte[] message, int offset, int length)
}
}
+ @Override
+ public Future<Void> pushAsync(byte[] message, int offset, int length)
Review Comment:
Done too.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] solomax commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
solomax commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r843421689
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketRequestHandler.java:
##########
@@ -77,6 +78,36 @@ 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;
+ }
+
+ @Override
+ public Future<Void> pushAsync(CharSequence message)
Review Comment:
I would replace this method with `return pushAsync(message, -1)`
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
martin-g commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r847987046
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/WebSocketResponse.java:
##########
@@ -87,13 +98,27 @@ public void close()
{
if (text != null)
{
- connection.sendMessage(text.toString());
+ if (asynchronous)
+ {
+ connection.sendMessageAsync(text.toString(), timeout);
+ }
+ 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, timeout);
Review Comment:
indentation issue
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/IWebSocketConnection.java:
##########
@@ -130,6 +130,31 @@
*/
void sendMessage(IWebSocketPushMessage message);
+ /**
+ * Broadcasts a push message to the wicket page (and it's components) associated with this
+ * connection. The components can then send messages or component updates to client by adding
+ * them to the target. Pushing to client is done asynchronously.
+ *
+ * @param message
+ * the push message to send
+ *
+ */
+ void sendMessageAsync(IWebSocketPushMessage message);
Review Comment:
Looking at the code it won't be easy ...
##########
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/IWebSocketConnection.java:
##########
@@ -130,6 +130,31 @@
*/
void sendMessage(IWebSocketPushMessage message);
+ /**
+ * Broadcasts a push message to the wicket page (and it's components) associated with this
+ * connection. The components can then send messages or component updates to client by adding
+ * them to the target. Pushing to client is done asynchronously.
+ *
+ * @param message
+ * the push message to send
+ *
+ */
+ void sendMessageAsync(IWebSocketPushMessage message);
Review Comment:
The earlier methods return `Future<Void>`. Can we do the same here too ?
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 commented on pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 commented on PR #509:
URL: https://github.com/apache/wicket/pull/509#issuecomment-1089318912
Fixed some API breaks.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 commented on a diff in pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 commented on code in PR #509:
URL: https://github.com/apache/wicket/pull/509#discussion_r849884394
##########
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)
Review Comment:
Done
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] reiern70 merged pull request #509: {WICKET-6969} allow asynchronous pushing of messages.
Posted by GitBox <gi...@apache.org>.
reiern70 merged PR #509:
URL: https://github.com/apache/wicket/pull/509
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org