Community
Participate
Working Groups
Build ID: I20080617-2000 Steps To Reproduce: In the classorg.eclipse.tm.tcf.core.AbstractChannel (abstract implementation of all channels) : 1. For local services : the instantiation is hard coded in the constructor (lines 152 and 153) through the private method addLocalService (line 572). 2. For remote services proxies : the instantiation is made in the method onLocatorHello (line 381) by searching classes that are present in a specific package. More information: In the current implementation local services cannot be added (their instantiation are hardcoded in the channel creation). Remote services proxies are instantiate by loading classes that are present in a particular package of the implementation. The addition of an API to add services will make easier the possibility to extend TCF (for instance in an Eclipse integration it could be coupled with the extension point mechanism).
Current implementation does have API for adding remote service proxies. It is in IChannel interface: /** * Install a service proxy object on this channel. * This method can be called only from channel open call-back. * It allows a client to extends TCF by adding proxy objects for non-standard services. * Client, wishing to become service proxy provider, should register itself * using either Protocol.addChannelOpenListener() or IChannel.addChannelListener(). * It is not allowed to register more then one proxy for a given service interface. */ <V extends IService> void setServiceProxy(Class<V> service_interface, IService service_proxy); Project org.eclipse.tm.tcf.examples.daytime shows usage of the method. Adding local services is not supported yet. How about fixing this by making addLocalService() public and adding a call-back from channel constructor? It would allow clients to add local services similar way as remote proxies. It seems there should be no problems using these call-backs to implement Eclipse extension points.
Ok, I am sorry I do not see this API. However, I don't find it easy to use with the extension point mechanism of eclipse. Indeed this API need to be used each time a new channel is created and after the channel has been initialized. Moreover, there is no verification that the target actually provides the corresponding services. What I though was more an API in order to configure the framework (for instance in a specific class of the framework that manages all the configurations) and enabling the developer to provide factories for local services and remote services proxies. So it is possible to provide the services either by instantiating the classes that are in a specific package or by using the Eclipse extension point. In fact that is the architecture I used in the implementation in the TCF implementation I made (available on the wiki page: http://wiki.eclipse.org/DSDP/TM/TCF_Meeting_with_Gaetan_from_Anyware-Tech_5-Aug-2008). I can try to see how to make this API available in the current implementation and propose a patch. However I think that a lot of re-factoring is needed, and so I want to know if this kind of API is needed by others.
(In reply to comment #2) > I don't find it easy to use with the extension point mechanism of eclipse. > Indeed this API need to be used each time a new channel is created and > after the channel has been initialized. Well, yes, proxies need to be created each time a new channel is created. This is true regardless of API. I'm not sure why you mention it. I'm not suggesting to use the API instead of extension point. My point is I see no problems to define an eclipse extension on top of the API. However, we need to keep such extension point definitions separate from core TCF code, because TCF is intended to be used not only in Eclipse, but also in other applications, including command line and embedded code. I think the best would be to have a separate plugin for that, something like org.eclipse.tm.tcf.extender, which would contain extension point schema, interfaces and a code that iterates extension point clients every time a channel is created. > Moreover, there is no verification that > the target actually provides the corresponding services. This line from the example code is exactly the verification: if (channel.getRemoteService(IDaytimeService.NAME) == null) return; > What I though was more > an API in order to configure the framework (for instance in a specific class of > the framework that manages all the configurations) and enabling the developer > to provide factories for local services and remote services proxies. So it is > possible to provide the services either by instantiating the classes that are > in a specific package or by using the Eclipse extension point. Sure.
I discussed with our experts in extension point declaration and they told me that the best is to use lazy and dynamic loading/unloading. So it seems best to use the existing API and to couple it with a Manager that use extension point (or other means) to dynamically instantiate service. Sorry, to have bothered you with my architecture, after reflexion it does not seem to be relevant. I completely agree with you to separate the extension point manager from the core TCF. I will try to make a patch for the "org.eclipse.tm.tcf.extender" and for the local service addition API on abstract channel and the channel interface.
Created attachment 110443 [details] Patch in order to add an API for local/remote service addition Here is a patch. It was a bit tricky to add local services to the channel. Indeed these must be created before locator message send (and so channel opening). So I decided To add a service manager that enable addition of service provider and I create a specific service provider for both local (locator and diagnostic) and remote (classes in the locator package) existing services. I modified the Abstract channel in order to use this manager and the plugin activator to configure it. However I wasn't able to build the C TCF client and I couldn't test my modifications.
I have committed the patch after making some changes: 1. Class InternalServiceProvider and most of ServiceManager are internal code and should not be part of public API, so I moved the classes into internal package. Methods addServiceProvider and removeServiceProvider are exposed through Protocol class. 2. Added copyright notices, comments, etc.
(In reply to comment #6) > I have committed the patch after making some changes: > 1. Class InternalServiceProvider and most of ServiceManager are internal code > 2. Added copyright notices, comments, etc. Since the patch went into the codebase, could you please add an "iplog+" flag on the Attachment. Ideally, the Copyright Notices should come from the Contributor already. In the past, I have often asked contributors to re-submit their patches after telling them what I find missing from the patch. That's a bit more work for the committer first, but pays off in the longer term when later contributions can be applied unchanged.
Gaetan, I just noticed that you never explicitly licensed your contributions under the EPL. To make it official, can you please add a comment here on bugzilla based on the template you find here: http://www.eclipse.org/dsdp/tm/development/committer_howto.php#external_contrib
I, Gaetan Morice, declare that I developed attached code from scratch, without referencing any 3rd party materials except material licensed under the EPL. I am authorized by my employer to make this contribution under the EPL.
Bulk update: Marking all TCF fixes from the Galileo period (Jun-2008 until Jun-2009) target 3.1 since they were done along with the TM 3.1 release.
Moving bugs to new home for IP log.
Bulk change: Marking all bugs from the TM era (until June 2011) target 0.3
