From: Markus KARG <markus_at_headcrashing.eu>
Date: Fri, 10 Feb 2017 18:24:26 +0100

I like the current proposal and would simply propose to use the upper-case classname SSE instead of Sse, to be aligned with other acronym classes like javax.xml.bind.JAXB which already serve as static factories for other technologies.

 

-Markus

 

From: Pavel Bucek [mailto:pavel.bucek_at_oracle.com]
Sent: Freitag, 10. Februar 2017 12:02
To: jsr370-experts_at_jax-rs-spec.java.net
Subject: Re: Server-Sent Events API proposal​ - update

 

Hi Sergey,

Sse might seem strage at the first look, but I kind of like it :-)

It can't be SseProvider, since providers in JAX-RS are something little different - you are usually registering them, this something else.

It still could be a factory, I like the idea about signaling the fact that this is only for container use. If that's not critical for you now, I think we can keep it for now. I'll discuss that with Santiago and Marek and we'll see whether we will be able to come up with a better name. Suggestions are always welcomed :)

Thanks!
Pavel

 

On 10/02/2017 11:48, Sergey Beryozkin wrote:

Hi Pavel

Sounds good, thanks.

The only possible but rather cosmetic improvement I can think of at this stage is to add a qualifier to
'Sse', it is a bit unusual to have only 3 chars in the name and we also have the SSE support on the client and on the server.

How about 'SseProvider' (or SseContainerProvider to 'note' it is relevant for the server-side SSE ...) ?
Your earlier idea, 'SseFactory' can also work, 'Provider' is probably a bit more JAX-RS-y, but 'Factory' is not bad either.

One of these options would be a bit better than just 'Sse' but I won't mind much if it stays for .m04, we can think of something else a bit later on...

Thanks, Sergey

On 10/02/17 10:41, Pavel Bucek wrote:

The last thing we want to do is to create some limitation which would forbid JAX-RS implementation running outside of CDI/other container like that :)

We really like lightweight runtimes like Jersey + Grizzly | Netty | Jetty | anything, I'm sure other JAX-RS implementations do have similar enhancements as well - don't take me wrong, I don't dislike CDI, but when the application doesn't need it, we definitely don't want to force it.

Having said that - I don't mind your reaction. It could happen that we introduce something like that by mistake and I'd rather get the feedback and have a chance to fix it compared to finding that much later and hoping it's not too late.

Anyway, since I have you on the line - I can release next JAX-RS milestone by the end of today, I believe it is already overdue. We obviously can change the code afterwards, but clearer is better - if you (or anyone else) has any prompt feedback, I'll happily incorporate it so it can be part of that milestone.

Best regards,
Pavel

 

On 10/02/2017 10:59, Sergey Beryozkin wrote:

Hi Pavel

Oh, that is great then, sounds good.
In fact what you did is really good, my reaction was really about, 'will that work with non-CDI (in Spring Boot for ex), and other scary thoughts',

Thanks for reacting to my over-reaction so nicely

Cheers, Sergey
On 10/02/17 09:59, Pavel Bucek wrote:

Hi Sergey,

@Inject is not a requirement. You still can inject with @Context, i.e.:

public ItemStoreResource(@Context Sse sse) {
    this.sse = sse;
    this.broadcaster = sse.newBroadcaster();
 
    broadcaster.onException((sseEventOutput, e) ->
            LOGGER.log(Level.WARNING, "An exception has been thrown while broadcasting to an event output.", e));
 
    broadcaster.onClose(sseEventOutput -> LOGGER.log(Level.INFO, "SSE event output has been closed."));
}

(I believe you are referring to this example code).

I think that @Inject was used because the Resource is a @Singleton, thus managed by the container.

Please don't regret anything. We can change anything we want and that's why we are asking for feedback.

Regards,
Pavel

On 10/02/2017 00:52, Sergey Beryozkin wrote:

So supporting @Inject is a must now for JAX-RS 2.1 which a minor maintenance release ?

What is it ? @Context for one thing, @Inject for another one, both parts being related to SSE ?

Can that Sse be rather mapped to RuntimeDelegate ?

I regret I raised that Broadcaster issue...

Sergey

On 09/02/17 21:22, Pavel Bucek wrote:

Dear experts,

thanks for all the feedback you've provided on presented Server Sent Events proposal.

Based on your feedback (and other inputs, most of them from Marek, the original author of SSE proposal), we'd like to present another iteration.

List of changes:

- SseEventInput is gone.
    - We don't need to be able to receive events in blocking fashion. If a user needs to do that, he still can - using a Deque and onEvent consumer (and some synchronization).
- SseClientSubscriber is gone.
    - replaced by EventSource#subscribe(Consumer<InboundSseEvent>) and other overloads.
- SseEventOuput is now SseEventSink (the opposite to SseEventSource).
- SseContext is now Sse (its not a context).
- Introduced SseSubscription (which will extend Flow.Subsription when we can use Java SE 9).
- SseEventSink (used to be SseEventOutput) is now injectable - there is no other way how to create it:

@GET
@Path("events")
@Produces(MediaType.SERVER_SENT_EVENTS)
public void itemEvents(@Context SseEventSink serverSink) {
    serverSink.onNext(sse.newEvent().data("welcome").build());
    broadcaster.subscribe(serverSink);
}

please note that the resource method now doesn't return injected SseEventSink - it's more similar to the pattern used in async API and users don't need to repeat "sseContext.newEventOutput();" and "return eventOutput;".

All these changes are done in a single commit: https://github.com/jax-rs/api/commit/459ddb615861959e4899b48d0b88498100308bcd

We also added couple of helper methods to create an outbound event: https://github.com/jax-rs/api/commit/8a61f14b2797cfa22da1c0fdb1b5a321ca435c8b

Please let us know what you think.

Thanks and regards,
Pavel & Santiago