[
https://issues.apache.org/jira/browse/WICKET-6950?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17484478#comment-17484478
]
ASF GitHub Bot commented on WICKET-6950:
----------------------------------------
martin-g commented on a change in pull request #496:
URL: https://github.com/apache/wicket/pull/496#discussion_r795267460
##########
File path:
wicket-native-websocket/wicket-native-websocket-javax/src/main/java/org/apache/wicket/protocol/ws/javax/WicketEndpoint.java
##########
@@ -43,6 +46,118 @@
*/
public class WicketEndpoint extends Endpoint
{
+ private static class JavaxWebSocketSessionWrapper implements
IWebSocketSession
Review comment:
Let's move this class as a non-inner class with package-private
visibility.
Also its name could be `JavaxWebSocketSession` to be more consistent with
the other classes in -javax module.
##########
File path:
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/api/IWebSocketProcessor.java
##########
@@ -16,13 +16,20 @@
*/
package org.apache.wicket.protocol.ws.api;
+import org.apache.wicket.protocol.http.WebApplication;
+import org.apache.wicket.protocol.ws.WebSocketSettings;
+
/**
* Processes web socket messages.
*
* @since 6.0
*/
public interface IWebSocketProcessor
{
+ default void onOpen(IWebSocketSession webSocketSession, final
WebApplication application) {
Review comment:
Please add javadoc
##########
File path:
wicket-examples/src/main/java/org/apache/wicket/examples/websocket/JSR356Application.java
##########
@@ -59,11 +65,22 @@ public void init()
getSharedResources().add(ChartWebSocketResource.NAME, new
ChartWebSocketResource());
+ final WebSocketSettings webSocketSettings = new
WebSocketSettings() {
+ @Override
+ public void configureSession(IWebSocketSession
webSocketSession) {
+ LOGGER.info("getMaxIdleTimeout = " +
webSocketSession.getMaxIdleTimeout());
+ // make sessions almost "immortal"
+
webSocketSession.setMaxIdleTimeout(Duration.ofHours(10).toMillis());
Review comment:
Is this a good idea ?
We hosts the examples at https://examples9x.wicket.apache.org/websockets/
##########
File path:
wicket-native-websocket/wicket-native-websocket-core/src/main/java/org/apache/wicket/protocol/ws/WebSocketSettings.java
##########
@@ -245,6 +246,16 @@ public Executor getSendPayloadExecutor()
return sendPayloadExecutor;
}
+ /**
+ * Allows to configure {@link
org.apache.wicket.protocol.ws.api.IWebSocketSession}
+ *
+ * @param webSocketSession The {@link
org.apache.wicket.protocol.ws.api.IWebSocketSession}
+ */
+ public void configureSession(IWebSocketSession webSocketSession)
Review comment:
I wonder whether we need more abstractions here.
It feels weird a Settings class to `do` something. Usually the Settings
classes have setters/getters for the do-ers.
E.g.
```java
public void setWebSocketSessionConfigurer(IWebSocketSessionConfigurer
configurer)
{
this.webSocketSessionConfigurer = Args.nonNull(configurer, "configurer");
}
```
The default configurer would do nothing in its `#configure()` method.
--
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]
> wicket web-sockets implementation does not allow to configure/access web
> socket session on onOpen
> -------------------------------------------------------------------------------------------------
>
> Key: WICKET-6950
> URL: https://issues.apache.org/jira/browse/WICKET-6950
> Project: Wicket
> Issue Type: Improvement
> Affects Versions: 10.0.0, 9.7.0
> Reporter: Ernesto Reinaldo Barreiro
> Assignee: Ernesto Reinaldo Barreiro
> Priority: Major
> Fix For: 9.8.0
>
>
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
