Community
Participate
Working Groups
org.eclipse.ecf.discovery.IDiscoveryContainerAdapter exposes two disjunct functionalities. One is locating/finding a service, the other is advertising/annoucing them. They should be separated into two independent Intefaces since we've learned that consumer ususually only want to use either of those. E.g. a server impl advertises services and a client is limited to locating services. Also provider implementations for both functionalities might require different sets of resources/privileges/... E.g. SLP advertiser need to bind to port 427 whereas locators do not (see bug #238710). [1] http://wiki.eclipse.org/ECF/Plan_for_3.0#Service_Discovery_and_Remote_OSGi_Services
Created attachment 119777 [details] patch v1 addListener/removeListener is gone completely. My idea is to offer the same functionality via the service registry (SR) so consumers use the SR to register/unregister service (type) listeners. Technically it means though, that a pure ECF consumer (no exposer to OSGi) cannot register listeners anymore. I'm inclined to accept this, since ContainerFactory.getDefault() as an entry point should be discouraged anyway.
(In reply to comment #1) > Created an attachment (id=119777) [details] > patch v1 > > addListener/removeListener is gone completely. My idea is to offer the same > functionality via the service registry (SR) so consumers use the SR to > register/unregister service (type) listeners. > Technically it means though, that a pure ECF consumer (no exposer to OSGi) > cannot register listeners anymore. I'm inclined to accept this, since > ContainerFactory.getDefault() as an entry point should be discouraged anyway. > I'm not so sure it's a good idea to remove the addListener/removeListener. That is, you are right that ContainerFactory.getDefault() should generally be discouraged as time goes on (and will be), but I don't want to disable it completely...at least in 3.0.
(In reply to comment #2) > I'm not so sure it's a good idea to remove the addListener/removeListener. > That is, you are right that ContainerFactory.getDefault() should generally be > discouraged as time goes on (and will be), but I don't want to disable it > completely...at least in 3.0. We could leave add/remove listener in IDCA and mark it as deprecated. The impl may then simply register the listeners with the service registry. FYI I plan on adding methods to Locator that return a future as well as a purge call that cleans the cache (see bug #254832).
(In reply to comment #3) > (In reply to comment #2) > > I'm not so sure it's a good idea to remove the addListener/removeListener. > > That is, you are right that ContainerFactory.getDefault() should generally be > > discouraged as time goes on (and will be), but I don't want to disable it > > completely...at least in 3.0. > > We could leave add/remove listener in IDCA and mark it as deprecated. The impl > may then simply register the listeners with the service registry. > > FYI I plan on adding methods to Locator that return a future as well as a purge > call that cleans the cache (see bug #254832). > RE: the future...we should consult on what to use for the returned future in the API. I'm engaged in some discussion about a possible IFutureStatus for Equinox (sub interface of IStatus) on e4 bug #253277. Ideally, I would like to see IFutureStatus in Equinox core, but I'm not sure if that's going to be agreed to on bug #253277. If you (Markus) agree...or even if you don't...express your sentiments on that bug to stir the pot a little. Also, there's the matter of consistency with the remote services API use of future on bug #250716 (i.e. one future interface to rule them all :). But I think that at least within ECF we should have one future interface (e.g. IAsyncResult) for the API until/if a future interface appears in Equinox. I would rather *not* directly use the the jre 1.5 Future interface in the discovery or remote services API directly (as that would tie everything to use jre 1.5).
(In reply to comment #4) <stuff deleted> > Equinox (sub interface of IStatus) on e4 bug #253277. Ideally, I would like to Sorry about the typo...the e4 bug is actually bug #253777.
(In reply to comment #1) > addListener/removeListener is gone completely. My idea is to offer the same > functionality via the service registry (SR) so consumers use the SR to > register/unregister service (type) listeners. This offers one essential benefit over the current solution, since it moves the (only) discovery provider state (the registered listeners) out of the provider into the service registry.
(In reply to comment #6) > (In reply to comment #1) > > addListener/removeListener is gone completely. My idea is to offer the same > > functionality via the service registry (SR) so consumers use the SR to > > register/unregister service (type) listeners. > > This offers one essential benefit over the current solution, since it moves the > (only) discovery provider state (the registered listeners) out of the provider > into the service registry. > Although I appreciate the desire to encourage the service registry/whiteboard pattern on clients, I'm not sure that requiring such a pattern to register/unregister asynchronous listeners is a great idea...especially since it breaks probably all existing consumers of the discovery API. Couldn't both be provided (service registry-based listeners as well as addListener)? If addListener/removeListener is retained, then obviously we should make clear in the API the need to do necessary cleanup.
(In reply to comment #7) > Although I appreciate the desire to encourage the service registry/whiteboard > pattern on clients, I'm not sure that requiring such a pattern to > register/unregister asynchronous listeners is a great idea...especially since > it breaks probably all existing consumers of the discovery API. As much as I like the whiteboard pattern it isn't just meant to spread it, nor does it break clients if they stick to using IDCA. :-) It is supposed to make providers stateless (see #c6) and to get rid of error prone implementations like in org.eclipse.ecf.provider.discovery.CompositeDiscoveryContainer with the synchronized handling of listeners. > Couldn't both be provided (service registry-based listeners as well as > addListener)? If addListener/removeListener is retained, then obviously we > should make clear in the API the need to do necessary cleanup. org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.addServiceListener(IServiceListener) org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.addServiceListener(IServiceTypeID, IServiceListener) org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.addServiceTypeListener(IServiceTypeListener) org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.removeServiceListener(IServiceListener) org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.removeServiceListener(IServiceTypeID, IServiceListener) org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.removeServiceTypeListener(IServiceTypeListener) would be marked deprecated and removed with ECF 4.0.
Created attachment 120601 [details] patch v2 snapshot reflecting current state of discussion
Don't try this at home. patch v2 is missing an IFutureStatus implementation and will thus NPE in most scenarios.
(In reply to comment #8) > (In reply to comment #7) <stuff deleted> > > Couldn't both be provided (service registry-based listeners as well as > > addListener)? If addListener/removeListener is retained, then obviously we > > should make clear in the API the need to do necessary cleanup. > > org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.addServiceListener(IServiceListener) > org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.addServiceListener(IServiceTypeID, > IServiceListener) > org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.addServiceTypeListener(IServiceTypeListener) > org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.removeServiceListener(IServiceListener) > org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.removeServiceListener(IServiceTypeID, > IServiceListener) > org.eclipse.ecf.discovery.IDiscoveryContainerAdapter.removeServiceTypeListener(IServiceTypeListener) > > would be marked deprecated and removed with ECF 4.0. > I still don't think this is a good idea. My expectation is that these service/service type listeners will be the easiest way for a client to use the discovery API, and making it harder for clients to use (requiring them to do more/other things to use the API effectively does not seem worth it...simply to have the provider be stateless. We (necessarily) use the asynchronous listener approach all over the place anyway, so I don't really see the benefit of making the discovery API stateless...except to ease the burden of provider implementation...which seems less important to me than the client usability anyway.
(In reply to comment #11) > I still don't think this is a good idea. My expectation is that these > service/service type listeners will be the easiest way for a client to use the > discovery API, and making it harder for clients to use (requiring them to do > more/other things to use the API effectively does not seem worth it...simply to > have the provider be stateless. It isn't just to have a stateless provider, but to make providers dynamic enabled. But we can keep add/remove listener in IDCA and let the implementation simply delegate to the service registry. Acceptable compromise? :-) > We (necessarily) use the asynchronous listener approach all over the place > anyway, so I don't really see the benefit of making the discovery API > stateless...except to ease the burden of provider implementation...which seems > less important to me than the client usability anyway. I'm not just a lazy programmer ;-), but also want to improve CDC's performance by using the (highly optimized) service registry. The current implementation can definitely be improved in this regard.
Released to HEAD
