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
>>>>>
>>>>>
>>>>
>>>
>>
>