Hi Markus,
that was part of our thinking as well, but we didn't want to "hard-code"
the broadcaster identifier into the application code.
Imagine a chat, where you can dynamically create / delete rooms and each
room is a single broadcaster instance.
Adding Sse.getBroadcaster(...) solves that and would allow more
elaborate usecases. Of course, as Sergey mentions, delegating
broadcaster storage to the application is always an option.
Regards,
Pavel
On 31/03/2017 19:49, Markus KARG wrote:
>
> I wonder what sense it actually makes to have broadcasters being
> scoped non-application? I mean, I cannot imaginate a scenario where a
> broadcaster shall be created later than an application starts, or die
> earlier than an application ends. Also I think that broadcasters are
> shared often between resource classes. So wouldn't it make sense to
> simply do this:
>
> class MyResource {
>
> @Context @Named("MyBroadcaster") SseBroadcaster b; //
> application-scoped, injected at resource instantiation
>
> }
>
> -Markus
>
> *From:*Pavel Bucek [mailto:pavel.bucek_at_oracle.com]
> *Sent:* Donnerstag, 30. März 2017 09:52
> *To:* jsr370-experts_at_jax-rs-spec.java.net
> *Subject:* Re: Minor SSE API changes
>
> Hi Sergey,
>
> please see inline.
>
> On 29/03/2017 16:34, Sergey Beryozkin wrote:
>
> 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.
>
>
> that might be our disconnect. Does it make sense to use
> "request-scoped" broadcaster? That would imply that it will contain
> only single SseEventSink and if not stored, it will be discarded after
> resource method finishes. Which is not that useful - you could use
> SseEventSink only and it would achieve the same functionality; opt1 is
> equivalent to opt2
>
> // consider standard, request-scoped JAX-RS resource
>
> @GET
> @Path(*"subscribe"*)
> @Produces(MediaType.*/SERVER_SENT_EVENTS/*)
> *public void *subscribe(@Context SseEventSink eventSink) {
> /// opt1/
> //eventSink.onNext(*sse*.newEvent(*"msg"*));
> /// opt2/
> //SseBroadcaster broadcaster = *sse*.newBroadcaster();
> broadcaster.subscribe(eventSink);
> /// broadcaster has only single subsrciber and will be discarded/
> //broadcaster.broadcast(*sse*.newEvent(*"msg"*));
> }
>
> If we have additional methods like Sse#getBroadcaster(String ID) which
> would get-or-create the broadcaster, it could be different:
>
> @GET
> @Path(*"subscribe"*)
> @Produces(MediaType.*/SERVER_SENT_EVENTS/*)
> *public void *subscribe(@Context SseEventSink eventSink) {
> /// broadcaster has "x" subscribers/
> //SseBroadcaster broadcaster = *sse*.getBroadcaster(*this*.getClass());
> /// broadcaster has "x+1" subscribers, the instance is preserved/
> //broadcaster.subscribe(eventSink);
> broadcaster.broadcast(*sse*.newEvent(*"msg"*));
> }
>
> 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 might not see something here, not sure whether there could be any
> unwanted side-effect. The "Sse" instance would only become "real
> context" instead of just a factory for creating broadcasters and
> outbound events. Everything else can stay the same - in our prototype
> implementation, Sse instance already is a singleton and there are
> generally no issues with injecting singletons into any-scoped beans,
> as long as the injected object is thread safe.
>
> 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 ?
>
> I don't see that as an ambiguity, maybe just as another usecase.
> JAX-RS resources are by default request scoped, so it actually makes
> more sense to have "get or create store", since SseBroadcaster
> instances are expected to be retained for more than just a single
> request. The "newBroadcaster" is still valid for some usecases, even
> when it could be always replaced by "getOrCreate" with random id, but
> we are not removing that now. Or at least I don't think about this
> like that. Thanks and regards, Pavel
>
> 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 *Sse *sse*;
>
> *private static *SseBroadcaster /broadcaster/; /// ... 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 *Sse *sse*;
>
> *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
>