On Thu, May 14, 2009 at 3:15 PM, Simon Nash <[email protected]> wrote: > Thanks for this proposal. It helps to see something concrete. > See questions/comments inline below. > > Simon > > ant elder wrote: >> >> On Thu, May 14, 2009 at 1:30 PM, Simon Nash <[email protected]> wrote: >>> >>> ant elder wrote: >>>> >>>> On Wed, May 13, 2009 at 3:20 PM, Simon Laws <[email protected]> >>>> wrote: >>>>> >>>>> Hi Simon >>>>> >>>>> It's not clear to me from your answer whether you think that >>>>> SCAClientFactory is or isn't intended to return an SCAClient >>>>> implementation appropriate for the domain you which to retrieve >>>>> services from. >>>>> >>> It is able to do this, but the user needs to set things up >>> correctly to ensure that this happens. >>> >>>>>> The SCAClientFactory.newInstance() call uses various mechanisms >>>>>> to identify which vendor-specific SCAClient implementation to >>>>>> return. The returned SCAClient object will be capable of locating >>>>>> all domains that use the same vendor implementation that created >>>>>> the SCAClient object. >>>>> >>>>> Suggests to me that you think it should >>>>> >>> Yes, this is what the current proposal says. >>> >>>>>> The alternative form suggested above doesn't work because it would >>>>>> require the SCAClientFactory to maintain some global knowledge of >>>>>> which domains use which vendor implementations, and return the >>>>>> appropriate SCAClient vendor implementation for the domain specified. >>>>> >>>>> Suggests to me that you think it shouldn't. >>>>> >>> Sorry that I wan't clear. I was trying to explain that automating >>> the selection of the correct vendor implementation for a specific >>> domain can't be supported by the mechanisms in the current spec >>> proposal, because this would require SCAClientFactory to have global >>> knowledge of available domains and the vendor implementations that >>> those domains require. >>> >>>>> There are two thing going on here. >>>>> >>>>> 1/ establish what sort of client you need - depends on the technology >>>>> of the domain you wish to talk to >>> >>> This is what newInstance() does. It doesn't require any swapping >>> on the classpath, because the SCAClientFactory is able to use >>> properties and a class loader object to find the correct vendor >>> implementation. The newInstance() call invokes standard SCA code >>> that is not vendor specific. >>> >>>>> 2/ establish precise domain you need >>>>> >>> This is what getService() does. The getService() call invokes >>> vendor-specific code, which is presumed to be able to handle any >>> domains that were created by the same vendor-specific code. >>> >>>>> 1/ could certainly be achieved by swapping in and out the specific >>>>> factory implementation you have on the application classpath. Hence >>>>> you don't need a domain ID to choose as the choice is made for you >>>>> before the code runs. However I don't see why having the domain ID >>>>> does any harm here in that it would allow Tuscany to make step 2 >>>>> transparent. Unless the argument here is that the Assembly spec >>>>> doesn't define what a domain ID looks like so we can't type it in the >>>>> interface. >>> >>> No, that isn't the reason. A domain ID is just a URI. In order to >>> allow this to be specified in step 1, it would be necessary to extend >>> the role of SCAClientFactory to act as a global registry of domain IDs >>> and their corresponding vendor client implementations. This isn't >>> part of the current proposal, and I don't think it's a good idea >>> to add this extra complexity within the SCA standardized code. >>> >> >> I don't what to debate this too much as I know the proposal is still >> being worked on by the spec group, but IMHO this seems like a better >> approach so i will show that its not too hard to do at all: >> >> Add a new abstract method to org.oasisopen.sca.client.SCAClientFactory >> to test if the vendor factory supports a domain: >> >> protected abstract boolean supportsDomain(URI domainURI); >> > I understand this part. > >> Change the org.oasisopen.sca.client.SCAClientFactoryFinder method to >> take the domainURI and return an array of SCAClientFactorys: >> >> public static SCAClientFactory[] find(URI domainURI, Properties >> properties, ClassLoader classLoader) { >> > I don't understand this. How (and why) does this method return an > array of SCAClientFactory objects? > >> And then update the existing >> org.oasisopen.sca.client.SCAClientFactoryFinder find method to use >> that new supportsDomain method: >> > It seems the suggestion is to have two find() methods, one returning > a single factory object, and the other returning an array of factory > objects. Why are both of these needed? > >> final String[] factoryImplClassNames = >> discoverProviderFactoryImplClass(properties, classLoader); > >> > There would be additional changes needed to make this work. The > discoverProviderFactoryImplClass method currently returns a single > vendor-specific factory class name that is found by searching the > properties object and/or META-INF/services. Returning an array > of class names would require extending the property value and the > META-INF/services value to be a list of candidate class names > instead of a single class name. > >> for (String factoryImplClassName : factoryImplClassNames) { >> final Class<? extends SCAClientFactory> factoryImplClass = >> loadProviderFactoryClass(factoryImplClassName, classLoader); > >> > This assumes that a single class loader is used for all the factory > class names. I don't think this limitation is acceptable. The current > design allows each factory class name to have its own class loader. > >> final SCAClientFactory factory = >> instantiateSCAClientFactoryClass(factoryImplClass); >> if (factory.supportsDomain(domainURI)) { >> return factory; >> } >> } > >> > This could instantiate factories that are never returned because > they don't support the domain. This would be a problem if the > instantiation has any side effects or allocates memory. > >> throw new NoSuchDomainException("No such domain or no >> available SCAClientFactory"); >> >> Thats it. The SCAClientFactory doesn't need to act as a global >> registry of domain IDs because its down to the vendor specific >> SCAClientFactory implementation to decide how to determine if it >> supports a domain uri. They could see if that id exists in some local >> table or make a remote call to a vendor domain manager or whatever >> else appropriate for their domain implementation. >> > This part should work OK. > > Simon > >> ...ant >> >> >
I'd hacked this up in the tuscany sca-api module so the diff for those changes are at: http://people.apache.org/~antelder/tuscany/sca-api-diff.txt The main change and how it works is that checkMETAINFServicesForClassName is changed to use the Classloader getResources method instead of getResource so that it gets all the available meta-inf/service files instead of just the first one. I think the rest of your comments are probably right. It could also be updated to support multiple class names in the properties, but like you say it uses the one classloader and may be instantiating multiple factories which may not get used. ...ant
