Hi Pavel
With the newBroadcaster() method I thought one would usually create it
at the injection time and save it. If it is a singleton - Sse will
allocate a single instance only. if it is per-request - Sse will
allocate an instance of SseBroadcaster per request, from the service
code's point of view it is still an instance of SseBroadcaster.
I'm just curious, what side-effects will be waiting to happen if we
introduce effectively a dedicate support for the static storage of the
broadcasters.
I can see how it can help to support per-request service instances reuse
the same SseBroadcaster instance, but what exactly does that achieve.
save the runtime to create an instance of SseBroadcaster per request. A
method like sse.getBroadcaster(Class) will not really make sense for the
singletons, so I can see some ambiguity being introduced into Sse API ?
Thanks, Sergey
On 29/03/17 13:07, Pavel Bucek wrote:
>
> Ah, I did not mean it like anything is going away - Sse#newBroadcaster
> will stay, only two method could be added to ease "storing" and
> retrieving the broadcasters.
>
> It's meant as an addition, existing Sse methods will stay as they are
> for now.
>
> Regards,
> Pavel
>
>
> On 29/03/2017 13:28, Sergey Beryozkin wrote:
>> Hi Pavel
>>
>> In your original example you had a non static SseBroadcaster created
>> from Sse.newBroadcaster. Yes, I recall it was also a singleton in
>> your example, but as I said, I don't understand what difference does
>> it make, @Context also works well for per-request resources.
>>
>> Is the actual problem is that SseBroadcasters are rather be 'static'
>> in nature ?
>> May be we can have sse.newBroadcaster() but add
>> sse.getBroadcaster(Class) ?
>>
>> Cheers, Sergey
>>
>>
>> On 29/03/17 12:14, Pavel Bucek wrote:
>>>
>>> Hi Sergey,
>>>
>>> that is still true, but with these new methods on Sse "context",
>>> we'd remove the need for having JAX-RS Resources as singleton, since
>>> the "store" on the Sse would be application-scoped.
>>>
>>> (let me add a code example to be extra sure we are on the same page)
>>>
>>> current state:
>>>
>>> (the synchronization could be done in a better way, please take it
>>> only as an example)
>>>
>>> @Path("server-sent-events")
>>> public class ServerSentEventsResource {
>>>
>>> private final Ssesse;
>>>
>>> private static SseBroadcasterbroadcaster;// ... per-class broadcaster storage public ServerSentEventsResource(@Context Sse sse) {
>>> this.sse = sse;
>>> }
>>>
>>> private static synchronized SseBroadcaster getBroadcaster(Sse sse) {
>>> if (broadcaster ==null) {
>>> broadcaster = sse.newBroadcaster();
>>> }
>>>
>>> return broadcaster;
>>> }
>>>
>>> @POST public void sendMessage(final String message)throws IOException {
>>> getBroadcaster(sse).broadcast(sse.newEvent(message));
>>> }
>>>
>>> // ... }
>>>
>>> future?:
>>>
>>> @Path("server-sent-events")
>>> public class ServerSentEventsResource {
>>>
>>> private final Ssesse;
>>>
>>> public ServerSentEventsResource(@Context Sse sse) {
>>> this.sse = sse;
>>> }
>>>
>>> @POST public void sendMessage(final String message)throws IOException {
>>> sse.getBroadcaster(this.getClass()).broadcast(sse.newEvent(message));
>>> }
>>>
>>> // ... }
>>>
>>> Thanks,
>>> Pavel
>>>
>>>
>>> On 29/03/2017 12:34, Sergey Beryozkin wrote:
>>>> HI Pavel
>>>>
>>>> That works for us. Recall that you said that 'Sse' object can be
>>>> injected with a JAX-RS @Context and
>>>> assuming the service resource is called DemoResource this works for
>>>> either per-request or singleton,
>>>> public class DemoResource {
>>>> public DemoResource(@Context Sse sse) {}
>>>> }
>>>>
>>>> For ex, if DemoService is per-request then the following works
>>>> right now:
>>>>
>>>> public DemoResource(@Context Application app) {}
>>>>
>>>> Both 'contexts' are very much similar in that both are effectively
>>>> the non-request specific factories
>>>>
>>>> Thanks, Sergey
>>>>
>>>> On 29/03/17 11:10, Pavel Bucek wrote:
>>>>> Dear experts,
>>>>>
>>>>> we did a couple of changes in SSE API:
>>>>>
>>>>> - removed SseEventSource.Builder#named(...)
>>>>>
>>>>> the intention was to name threads used by the event source, but in
>>>>> the light of future change, which should introduce "common"
>>>>> per-client thread pool(s) for async operation, this won't be
>>>>> feasible to have there (since it would require creating another
>>>>> threadpool per SseEventSource instance)
>>>>>
>>>>> - SseEventSource now implements Flow.Source<InboundSseEvent>
>>>>>
>>>>> seems like we'd need to have some form of Publisher and
>>>>> Subscriber, currently named as Source and Sink in the API. The
>>>>> main reason would be integration with other frameworks - without
>>>>> it, it would be harder to do without a good reason.
>>>>>
>>>>>
>>>>> Also, during the implementation and rewriting "old" Jersey
>>>>> examples, one difficulty was discovered. It used to be possible to
>>>>> create a broadcaster statically, which simulates "per resource
>>>>> class" scope. The problem is that currently the only way how to
>>>>> create a new SseBroadcaster is injecting Sse object. But that is
>>>>> not available during static initialization and since JAX-RS
>>>>> resources are by default request-scoped, storing the
>>>>> SseBroadcaster instances might be a challenge for some users.
>>>>>
>>>>> Does anyone see the same issue as we do?
>>>>>
>>>>> Would someone object if we add methods like
>>>>>
>>>>> Sse#getBroadcaster(String)
>>>>> Sse#getBroadcaster(Class<?>)
>>>>>
>>>>> which would serve as a storage for broadcasters? (semantically it
>>>>> could be "get-or-create", passed String or Class would serve as a
>>>>> key).
>>>>>
>>>>> Thanks and regards,
>>>>> Pavel
>>>>
>>>>
>>>
>>
>