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.

Sign up for GitHub

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

Jump to bottom

Update spring Jetty websocket usage #2

Merged
merged 12 commits into from
Apr 16, 2024
Merged

Update spring Jetty websocket usage #2

merged 12 commits into from
Apr 16, 2024

Conversation

lachlan-roberts
Copy link
Member

  • use Session.Listener class instead of @WebSocket annotations
  • use explicit onWebSocketPong method instead of the onFrame notification
  • JettyWebSocketSession now extends AbstractWebSocketSession<Session> directly instead of using AbstractListenerWebSocketSession<Session>, so it can implement the send and receive reactive APIs using Jetty WebSocket demand.
  • Implement a version of the org.springframework.web.reactive.socket.client.WebSocketClient interface for the Jetty WebSocket Client.

gregw and others added 6 commits February 12, 2024 17:17
@gregw @lachlan-roberts
Updated to jetty 12.0.6
b1c778d
@lachlan-roberts
Implement a JettyWebSocketSession based on demand.
5f1712d 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts
Add a Jetty implementation of the spring WebSocketClient interface.
700f8e6 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts
Prevent possible ReadPendingException from multiple demand.
1c7935e 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts
Update the JettyWebSocketHandlerAdapter as a Session.Listener
bbd4e68 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts
fixes for checkstyle
1f09a94 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
gregw
gregw requested changes Feb 12, 2024
View reviewed changes
Copy link
Member

@gregw gregw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a light review at this point.

framework-platform/framework-platform.gradle Outdated Show resolved Hide resolved
...-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpResponse.java Show resolved Hide resolved
...lux/src/main/java/org/springframework/web/reactive/socket/adapter/JettyWebSocketSession.java Show resolved Hide resolved
...lux/src/main/java/org/springframework/web/reactive/socket/adapter/JettyWebSocketSession.java Outdated Show resolved Hide resolved
@lachlan-roberts
Ensure DataBuffer is processed correctly for BINARY messages.
08e9177 
Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
@lachlan-roberts
Wait for close of Session before calling handlerCompletionSink
26cac28 
Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
@lachlan-roberts
fix checkstyle error
e2b452a 
Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
sbordet
sbordet requested changes Feb 16, 2024
View reviewed changes
.../main/java/org/springframework/web/reactive/socket/adapter/JettyWebSocketHandlerAdapter.java Show resolved Hide resolved
...lux/src/main/java/org/springframework/web/reactive/socket/adapter/JettyWebSocketSession.java Outdated Show resolved Hide resolved
...lux/src/main/java/org/springframework/web/reactive/socket/adapter/JettyWebSocketSession.java Outdated Show resolved Hide resolved
...lux/src/main/java/org/springframework/web/reactive/socket/adapter/JettyWebSocketSession.java Outdated Show resolved Hide resolved
...lux/src/main/java/org/springframework/web/reactive/socket/adapter/JettyWebSocketSession.java Outdated Show resolved Hide resolved
...lux/src/main/java/org/springframework/web/reactive/socket/adapter/JettyWebSocketSession.java Outdated Show resolved Hide resolved
...bflux/src/main/java/org/springframework/web/reactive/socket/client/JettyWebSocketClient.java Outdated Show resolved Hide resolved
...bflux/src/main/java/org/springframework/web/reactive/socket/client/JettyWebSocketClient.java Outdated Show resolved Hide resolved
...bflux/src/main/java/org/springframework/web/reactive/socket/client/JettyWebSocketClient.java Outdated Show resolved Hide resolved
...src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketHandlerAdapter.java Show resolved Hide resolved
@lachlan-roberts
Changes from review.
a0f4fe2 
Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
@gregw
Copy link
Member

gregw commented Apr 4, 2024

@lachlan-roberts what is the status of this?

@gregw gregw requested review from sbordet and olamy April 4, 2024 12:20
@lachlan-roberts
Implement LifeCycle for JettyWebSocketClient
85505fe 
Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
@lachlan-roberts
additional changes from review
cd9063b 
Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
@lachlan-roberts
Copy link
Member Author

@gregw I just updated it with some extra changes from outstanding reviews.

So it is ready to review or merge.

@lachlan-roberts
Copy link
Member Author

@gregw @sbordet nudge

@lachlan-roberts lachlan-roberts merged commit e9cb9a9 into JettyCoreHttpHandlerAdapter Apr 16, 2024
4 checks passed
@lachlan-roberts lachlan-roberts deleted the JettyCoreHttpHandlerAdapter-WebSocket branch April 16, 2024 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregw gregw gregw approved these changes

@sbordet sbordet Awaiting requested review from sbordet

@olamy olamy Awaiting requested review from olamy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@lachlan-roberts @gregw @olamy @sbordet