Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow easier wrapping of WebSocketHandlerDecorator around WebSocketHandler's [SPR-11533] #16158

Closed
spring-projects-issues opened this issue Mar 10, 2014 · 8 comments
Assignees
Labels
in: web status: declined type: enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Mar 10, 2014

Mark Galea opened SPR-11533 and commented

The SockJsHttpRequestHandler constructor wraps an ExceptionWebSocketHandlerDecorator and a LoggingWebSocketHandlerDecorator around the webSocketHandler (SubProtocolWebSocketHandler). Ideally, this wrapping of WebSocketHandlerDecorator s should be exposed.

One scenario where this would be useful is a WebSocketHandlerDecorator which keeps track of the relationship between the native httpSession and the webSocket session by listening to connection established and connection closed events. This would provide the necessary information to close off webSocket sessions which outlive their corresponding httpSession.


Affects: 4.0.1

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 10, 2014

Rossen Stoyanchev commented

Yes indeed we automatically wrap configured WebSocketHandler's to provide consistent logging and behavior with regards to exceptions that may escape WebSocketHandler method implementations. You can of course provide a WebSocketHandler instance wrapped with your own decorators. That should work just fine. Or you perhaps looking for a way to take full control over decoration, i.e. either not add the ones added by default or maybe decorate in a different order?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 11, 2014

Mark Galea commented

I have attached a sample xml which outlines your handler idea. I would appreciate if you could make sure that it is ok. I added some more comments below this xml snippet.

<websocket:message-broker application-destination-prefix="/app">
    <!-- The websocket:stomp-endpoint cannot be delete because of the xsd definition --> 
    <websocket:stomp-endpoint path="/comet2"/>
    <websocket:stomp-broker-relay client-login="guest" client-passcode="guest" prefix="/topic,/exchange"/>
</websocket:message-broker>


<bean id="userSessionRegistry" class="org.springframework.messaging.simp.user.DefaultUserSessionRegistry"/>
<!-- This would be the custom SubProtocolWebSocketHandler -->
<bean id="myHandler" class="org.springframework.web.socket.messaging.SubProtocolWebSocketHandler">
    <!-- It's not evident where the clientInboundChannel, clientOutboundChannel have been defined -->  
    <constructor-arg ref="clientInboundChannel"/>
    <constructor-arg ref="clientOutboundChannel"/>
    <property name="defaultProtocolHandler" ref="stompSubProtocolHandler"/>
</bean>

<bean id="stompSubProtocolHandler" class="org.springframework.web.socket.messaging.StompSubProtocolHandler">
    <property name="userSessionRegistry" ref="userSessionRegistry" />
</bean>

<websocket:handlers order="0">
    <websocket:mapping path="/comet" handler="myHandler"/>
    <websocket:handshake-handler ref="stompSubProtocolHandler"/>
    <websocket:sockjs websocket-enabled="true" session-cookie-needed="true"/>
</websocket:handlers>

Although the handler suggestion did work there are still some problems with this solution:

  1. Two stomp endpoints /comet, /comet2 are now defined. One defined in the websocket:stomp-endpoint and one defined in the handlers. Note One simply cannot delete the websocket:stomp-endpoint due to the xsd definition <xsd:element name="stomp-endpoint" minOccurs="1" maxOccurs="unbounded">.
  2. It is not evident that the clientInboundChannel and the clientOutboundChannel can be reference from the SubProtocolWebSocketHandler constructor. In fact is this the correct way to go about it? IntelliJ still marks this as an error since there is no way for it to know what the NamespaceHandlers and Parsers are conjuring!
  3. Although my small POC works, I'm getting weird errors in the console. From the bean name it seems that there are two clientOutboundChannel beans now.
[clientOutboundChannel-1] ERROR o.s.w.s.m.SubProtocolWebSocketHandler - Session not found for session with id ycr7885v

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 12, 2014

Rossen Stoyanchev commented

I see what you're trying to do now. Not just wrap any WebSocketHandler (trivial if it is yours) but the one created for <websocket:message-broker>>.

It is quite straight forward in the Java config by extending WebSocketMessageBrokerConfigurationSupport (from your own @Configuration class) and override subProtocolWebSocketHandler().

In the XML config you could use a BeanPostProcessor (for example) to find the SimpleUrlHandlerMapping with a handler for "/comet". The handler should be WebSocketHttpRequestHandler and could be re-created and re-registered with the handler mapping with a decorated WebSocketHandler. Not trivial but possible.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 12, 2014

Mark Galea commented

Ah I see! I'll give this a try soon and will post an update. Would an attribute on the websocket:message-broker to define which handler to use make sense? That way both configuration flavours would be equally simple.

On a different topic (perhaps I should open a Jira issue as a reference for our discussion) do you guys have any performance figures on the number of simultaneous clients you are able to serve on the following transports:

  • web-sockets
  • xhr-streaming
  • xhr-polling

Currently we cannot use web sockets due to our load balancers/firewall restrictions (which we do not have control over) so I was wondering how this would impact the server resource. Is xhr-streaming using the servlet async support?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 12, 2014

Rossen Stoyanchev commented

In the XML config you could use a BeanPostProcessor (for example) to find the SimpleUrlHandlerMapping...

It should even be way simpler. In the BeanPostProcessor look for the SubProtocolWebSocketHandler bean, wrap it, and return it from postProcessBeforeInitialization. That's it.

As for the other topic on performance figures, I would definitely ask that you post this in a forum like StackOverflow. JIRA is for specific issues and requests.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 17, 2014

Mark Galea commented

Works like a charm. Thanks for that Rossen

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 17, 2014

Rossen Stoyanchev commented

Alright, good to know.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 19, 2014

Rossen Stoyanchev commented

BTW regarding performance figures, there are now some load tests added here. These are all Java tests and therefore covers mainly the WebSocket transport but are still a very useful indicator.

@spring-projects-issues spring-projects-issues added status: declined type: enhancement in: web labels Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web status: declined type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants