users@jax-rs-spec.java.net

[jax-rs-spec users] Re: Server-Sent Events API proposal​ - update

From: Markus KARG <markus_at_headcrashing.eu>
Date: Sat, 11 Feb 2017 08:46:16 +0100

According to Oracle naming conventions (http://www.oracle.com/technetwork/java/codeconventions-135099.html) it has to be SSE (not Sse), as it is an acronym. Only the full-lenth variant ServerSentEvents is allowed to be camel case. Googling for "SSE" clearly shows lots of SSE but rather few Sse.

 

"Class names should be nouns, in mixed case with the first letter of each internal word capitalized. Try to keep your class names simple and descriptive. Use whole words-avoid acronyms and abbreviations (unless the abbreviation is much more widely used than the long form, such as URL or HTML)."

 

-Markus

 

From: Sergey Beryozkin [mailto:sberyozkin_at_talend.com]
Sent: Freitag, 10. Februar 2017 23:43
To: jsr370-experts_at_jax-rs-spec.java.net
Subject: Re: Server-Sent Events API proposal​ - update

 

Hi

Well, given it is an instance, I'd rather have 'Sse' left as is, might be with a qualifier if we can imagine a similar instance will be needed for the client side now or later...
Pavel, thanks for releasing the latest milestone

Cheers, Sergey
On 10/02/17 18:25, Pavel Bucek wrote:

Hi Markus,

there is the difference. Sse doesn't contain any static method.

But the proposal is noted, SSE might be more readable.

I'm in process of getting milestone build released, so I won't make any changes, but I will remember this when we have another naming discussion.

Thanks,
Pavel

 

On 10/02/2017 18:24, Markus KARG wrote:

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