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

WebSockets Next: client endpoints #40309

Merged
merged 1 commit into from
May 1, 2024

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Apr 26, 2024

  • includes refactoring of the server part so that we can reuse as much as possible

We introduce 2 new concepts for the client part:

  • client endpoints; using the same programming model as server endpoints, except that annotated with @WebSocketConnection and built around the WebSocketClientConnection (unlike @WebSocket and WebSocketConnection on the server)
  • WebSocket connector API - this API is used to configure and create new connections
@WebSocket(path = "/end/{name}")
public class ServerEndpoint {
    @OnTextMessage
    String echo(String message, WebSocketConnection connection, @PathParam String name) {
       // do something with connection and name
       return message;
    }
}

@WebSocketClient(path = "/end/{name}")
public class ClientEndpoint {
   @OnTextMessage
   String echo(String message, WebSocketClientConnection connection, @PathParam String name) {
      // do something with connection and name
      return message;
    }
}

The WebSocketConnector can be injected in any bean:

@Singleton
public class Service {

   @Inject
   WebSocketConnector<ClientEndpoint> connector;

   void connectToClientEndpoint() {
      connector.
         .baseUri(uri) // base URI is configurable per endpoint
         .pathParam("name", "Lu")
         .addHeader("X-Test", "foo")
         .connectAndAwait();
     }
}

TODO

  • add more tests
  • fail the build if @OnTextMessage.broadcast()=true/ @OnBinaryMessage.broadcast()=true on a client endpoint
  • Dev UI update (just make sure the server part works fine)

-@WebSocketClient#autoConnect() - connect the client automatically when the app starts removed from the list, not a high priority, maybe later...

NOTE: We decided to include the client part in the same extension as the server part. Therefore, we will get rid of extensions/websockets-next/server in a follow-up PR; i.e. the deployment module will be moved from extensions/websockets-next/server/deployment to extensions/websockets-next/deployment and the runtime module will be moved from extensions/websockets-next/server/runtime to extensions/websockets-next/runtime.

@geoand
Copy link
Contributor

geoand commented Apr 29, 2024

I started working with this and I am probably missing a lot in the way this is meant to be used, but I am really struggling to figure out what the declarative client gives me over being able to define message handlers on WebSocketClientConnection for example.

@mkouba
Copy link
Contributor Author

mkouba commented Apr 29, 2024

I started working with this and I am probably missing a lot in the way this is meant to be used, but I am really struggling to figure out what the declarative client gives me over being able to define message handlers on WebSocketClientConnection for example.

Well, for simle stuff a message handler represented as a callback works well. But for things like automatic serizalization/deserialization, CDI context activation, @PathParms, security, metrics, etc. calling a business method of a CDI bean is more convenient.

I can imagine that we support the simple callbacks as well. But TBH if you want to use callbacks then just use io.vertx.core.http.WebSocketClient 🤷 .

@geoand
Copy link
Contributor

geoand commented Apr 29, 2024

I can imagine that we support the simple callbacks as well. But TBH if you want to use callbacks then just use io.vertx.core.http.WebSocketClie

Not true really because that gets me into callback hell :)

@geoand
Copy link
Contributor

geoand commented Apr 29, 2024

CDI context activation

In the client? Is that really useful?

@mkouba
Copy link
Contributor Author

mkouba commented Apr 29, 2024

FYI I've added the BasicWebSocketConnectorSimpleWebSocketConnector API. This is a variant of WebSocketConnector which does not use a client endpoint class but callbacks.

@Singleton
public class Service {

   @Inject
   BasicWebSocketConnector connector;

   void connectToClientEndpoint() {
      connector.
         .baseUri(uri)
         .addHeader("X-Test", "foo")
         .onTextMessage((connection, message) -> {
             // you can access headers and other attrs of handshake req here
             // if you also specify path params using SimpleWebSocketConnector#path() you can access these params here as well
             connection.sendTextAndAwait(message);
         })
         .connectAndAwait();
     }
}

I will probably rename the SimpleWebSocketConnector#textMessageHandler() and other methods accepting callbacks to something like onTextMessage() to make it consistent with our annotations... Done.

@geoand
Copy link
Contributor

geoand commented Apr 29, 2024

Thanks a lot @mkouba!

@mkouba mkouba force-pushed the websockets-next-client branch 5 times, most recently from 781168e to b471879 Compare April 30, 2024 07:57
- add client endpoints and  WebSocketConnector API
- also includes refactoring of the server part so that we can reuse as much
  as possible
@mkouba mkouba force-pushed the websockets-next-client branch from b471879 to 6dff924 Compare April 30, 2024 08:04
@mkouba
Copy link
Contributor Author

mkouba commented Apr 30, 2024

I think that this pull request is now ready for the next round of review. Docs are still missing but I would like to add the client part in a follow-up pr.

@mkouba mkouba marked this pull request as ready for review April 30, 2024 08:05
Copy link

quarkus-bot bot commented Apr 30, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 6dff924.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@mkouba mkouba merged commit c02cafb into quarkusio:main May 1, 2024
32 of 33 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants