[WIP] Add SSE Support

[aesteve]

From: https://github.com/aesteve/vertx-sse Original discussion: https://github.com/reactiverse/reactiverse/issues/5 Status: Still WIP. Misses documentation + implementation must be discussed further on.

Motivation:

Provide a way to deal with Server-Sent-Events with Vert.x SSE allows unidirectional communication between server and client in a pure HTTP way.

Status:

I migrated most of the code from my 3rd party implementation to make it officially supported by the Vert.x stack, as discussed in this reactiverse issue.

At this point, I'd like to receive feedback on:

• where to add documentation, and how to write it properly
• is the implementation correct, does it align with Vert.x 4 upcoming standards (Future/Promise, etc.)
• is the implementation codegen-compatible
• are you OK with names, variables, coding standards

Thank you.

[tsegismont]

where to add documentation, and how to write it properly

You should update the index.adoc file in the vertx-web module.

is the implementation correct, does it align with Vert.x 4 upcoming standards (Future/Promise, etc.)

The standard for Vert.x 4 is to have good old callback style as well as futurized versions of the async methods.

is the implementation codegen-compatible

The compiler will tell you for every object annotated with @VertxGen. Then you could also build the vertx-kotlin and vertx-rx projects locally with your modified version of vertx-web and see how it looks like.

are you OK with names, variables, coding standards

I haven't found anything odd except formatting and missing copyright headers.

Other than that, I wonder if this shouldn't be split into two parts: the Vert.x Web handler in the Vert.x Web module, and the client in a separate module. What do you think @pmlopes ?

[aesteve]

Ok so let me create a check list or there's a chance I'll be missing a lot of stuff since there's a lot of work to do:

• [x] Reformat the whole code using Vert.x standards (old projects PR-submitted code doesn't have the same indent)
• [x] Copyright headers
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/EventSource.java
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/SSEConnection.java
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/SSEHandler.java
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/SSEHeaders.java
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/impl/EventSourceImpl.java
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/impl/SSEConnectionImpl.java
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/impl/SSEHandlerImpl.java
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/impl/SSEPacket.java
  • [x] vertx-web/src/test/java/io/vertx/ext/web/handler/sse/SSETestBase.java
  • [x] vertx-web/src/test/java/io/vertx/ext/web/handler/sse/SSETestClose.java
  • [x] vertx-web/src/test/java/io/vertx/ext/web/handler/sse/SSETestEstablishConnection.java
  • [x] vertx-web/src/test/java/io/vertx/ext/web/handler/sse/SSETestReceiveData.java
  • [x] vertx-web/src/test/java/io/vertx/ext/web/handler/sse/SSETestRequestResponseHeaders.java
• [x] EventSource.close() should return void
• [x] SSEHeaders
  • [x] Turn it into an enum
  • [x] ~Change visibility to package protected~ => Actually NO, see below. It can be used for event bus messages
  • [x] ~Move to impl package~ => Actually NO, see below. It should be back to the root package
• [x] @VertxGen
  • [x] Add annotation to vertx-web/src/main/java/io/vertx/ext/web/handler/sse/EventSource.java
  • [x] Add annotation to vertx-web/src/main/java/io/vertx/ext/web/handler/sse/SSEConnection.java
  • [x] Add annotation to vertx-web/src/main/java/io/vertx/ext/web/handler/sse/SSEHandler.java
  • [x] Add annotation to vertx-web/src/main/java/io/vertx/ext/web/handler/sse/SSEHeaders.java
  • [ ] Check it generates the right stuff
  • [ ] Install locally + build vertx-kotlin, vertx-rx against this version
• [x] Perf improvements
  • [x] In SSEHandlerImpl.handle use request.headers().contains("Accept", "text/event-stream", true) + do not create the connection if not appropriate
• [x] Remove reject(int statusCode) on SSEConnection
• [x] Add EventSource.addEventListener as a shortcut for  EventSource.onEvent as per the docs
• [x] Move SSEHandler.closeHandler to SSEConnection.closeHandler
• [x] Do not use Long but long in SSEConnection.retry(...)
• [x] Remove EventSource.onclose since it's not supported by browsers
• [x] Do not mix events and data
  • [x] Remove 2nd parameter to SSEConnection.retry, data should be sent separately
  • [x] Remove 2nd parameter to SSEConnection.event, data should be sent separately
  • [x] Remove 2nd parameter to SSEConnection.id, data should be sent separately
• [x] Do not let the ability to send a list of data, it's useless
  • [x] io.vertx.ext.web.handler.sse.SSEConnection#forward(java.util.List<java.lang.String>)
  • [x] io.vertx.ext.web.handler.sse.SSEConnection#retry(java.lang.Long, java.util.List<java.lang.String>)
  • [x] io.vertx.ext.web.handler.sse.SSEConnection#event(java.lang.String, java.util.List<java.lang.String>)
  • [x] io.vertx.ext.web.handler.sse.SSEConnection#id(java.lang.String, java.util.List<java.lang.String>)
• [x] synchronized for every field and methods in
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/impl/EventSourceImpl.java
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/impl/SSEConnectionImpl.java
  • [x] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/impl/SSEHandlerImpl.java
• [x] Re-design SSEHandler.connectHandler and SSEHandler.closeHandler so that they take a single handler as parameter (therefore the fields tracking those handlers don't need to be lists anylonger)
• [x] EventSourceOptions instead of HttpClientOptions
  • [x] Create EventSourceOptions (+ copyright), including  retryPeriod
  • [x] Replace HttpClientOptions with EventSourceOptions
  • [x] Write a test for the retryPeriod option
• [x] EventSourceImpl
  • [x] Handle HTTP 204 + write a test for it
  • [x] Handle redirect + write a test for it
  • [x] Handle reconnect + write a test for it
• [x] BugFixes / Spec conformity
  • [x] eventName && id are both null => data is sent twice
    • [x] Reproduce the bug in an unit test
    • [x] Fix it in SSEConnectionImpl.ebMsgHandler and make the test pass
  • [x] Last "\n" on data should be removed in EventSource.onMessage, according to: https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/Using_server-sent_events#Event_stream_format
  • [x] Last id should never be replaced by null
• [x] Improvements
  • [x] SSEHeaders should actually be public, since it can be used as Event Bus Message<T> headers
• [x] Missing tests
  • [x] Forwarding messages from the event bus
  • [x] Data split on 2 different lines should be concatenated
  • [x] Reconnect with lastId => data received with id, lastId should be up-to-date in eventSource, disconnect / reconnect -> the "lastId" sent to server should be the last one received, not the same as the one used when connecting for the first time (for example)
• [ ] Javadoc for public interfaces / classes
  • [ ] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/EventSourceOptions.java
  • [ ] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/EventSource.java
  • [ ] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/SSEConnection.java
  • [ ] vertx-web/src/main/java/io/vertx/ext/web/handler/sse/SSEHandler.java

---

That's a ton of work to do, I'll give it a try by the end of february I think.

Thanks a lot for all the reviews.

[aesteve]

Just waiting for a second review @tsegismont before next phase (javadocs + official docs).

Plus, converting to draft until Doc is there, so that it doesn't get merged by accident;

[aesteve]

Note: about EventSource, would RecordParser make the implementation easier? Something I need to investigate.

[rgmz]

Hey @aesteve,

Have you had a chance to make any updates? I'd like to help out, but don't want to duplicate effort.

---

Assuming not, @vietj / @pmlopes / @tsegismont, what's the best way to make this compatible with Vert.x 4.x?

1. HttpClient.redirectHandler method used to expect Function<HttpClientResponse,Future<HttpClientRequest>>, but now expects Function<HttpClientResponse,Future<RequestOptions>>:

https://github.com/vert-x3/vertx-web/blob/2e2d52e422a6aa5f3e455d3a77eb519ebc59212c/vertx-web/src/main/java/io/vertx/ext/web/handler/sse/EventSourceImpl.java#L67-L70

2. HttpClient.request used to return HttpClientRequest, but now returns Future<HttpClientRequest>:

https://github.com/vert-x3/vertx-web/blob/2e2d52e422a6aa5f3e455d3a77eb519ebc59212c/vertx-web/src/main/java/io/vertx/ext/web/handler/sse/EventSourceImpl.java#L151-L159

Given those two changes, I'm not sure how I would implement equivalent logic in EventSourceImpl for 4.1.0. My best guess is to handle the redirectHandler and createRequest options separately, however, I'm not sure if that would be equivalent to the existing logic as the redirectHandler no longer has access to the request's onFailure/onSuccess conditions. (Edit: I suppose I'd dispatch the request and then set the redirectHandler -- opposite from the current logic.)

https://github.com/vert-x3/vertx-web/blob/2e2d52e422a6aa5f3e455d3a77eb519ebc59212c/vertx-web/src/main/java/io/vertx/ext/web/handler/sse/EventSourceImpl.java#L67-L72

I'll upload my attempt later today; any ideas/guidance would be appreciated.