RFR JDK-8044627: Update JNDI to work with modules

[Daniel Fuchs]

On 9/16/14 4:14 PM, Pavel Rappo wrote:
> Daniel,
>
>> Given that helper.loadClass uses the context class loader,
>> Should you also simply use
>>   ServiceLoader<InitialContextFactory> loader =
>>                      ServiceLoader.load(InitialContextFactory.class);
>> at lines 680-681 ?
>
> It needs to be the system class loader to allow for JNDI providers that might be on the class path or module path.

So is it expected that modules (e.g. java.corba) will register
their own service provider for the InitialContextFactory
(I mean - using META-INF/services/)?

>> Also it might be good to log an RFE against the ServiceLoader, so
>> that you could look for a service implementation of a specific
>> concrete class without having to instantiate all the other
>> service implementations encountered along the way.
>> Streams should provide a nice infrastructure for such an API.
>> It would certainly be more robust than looping over
>> ServiceLoader.iterator().next() - which is unfortunately the only
>> option available to you at the moment.
>
> IMO, it's not just an inconvenience, but rather a part of ServiceLoader's design. I mean, it's definitely designed to provide, so to say, a "one-to-many" mapping for the classes (providers) that implements some interface (a service) to a client. It merely delivers you implementations. You should than iterate though them and decide which one satisfies your needs. I'm not sure it's a good idea to get services based on their implementation classnames. It's more likely to be a little bit a flaw in design from the JNDI part -- when you have to specify the implementation class's FQN. But given the history of the JNDI (it's started as a project outside the JDK) -- it's totally understandable.

Right. It's not usual to use the service loader to look for a specific
concrete implementation class name. So on the one side what you're doing
may be better than modifying ServiceLoader to support an unusual usage.
I can buy this.

> (Just as a side-note, have a look at these examples of usage of ServiceLoader:
>
         [snip]
> 	javax.xml.validation.SchemaFactoryFinder:405
> 	javax.xml.xpath.XPathFactoryFinder:403
         [snip]

I know these two ;-) I updated JAXP to use ServiceLoader :-)

The difference however is that in  this two case we're looking for
a service implementing a specific feature, we're not looking for a
service whose concrete class matches a specific class name.

In the case of InitialContextFactory - as a client of the API, I
would be a bit surprised if - asking for a LDAP InitialContext,
I received a CORBA exception. This would look strange to me.
But maybe there's nothing you can do given how JNDI is
currently working.

A possibilty might be to use InitialContextFactoryBuilder as the
service interface instead, and loop over the implementations until
you find one that does not throw NamingException - but then it
would be difficult to make the distinction between
'This builder does not support the Context.INITIAL_CONTEXT_FACTORY
  that you ask for' and 'The InitialContextFactory could not be
  instantiated for various reasons'...

So I'm not sure it would be better.

> 	java.time.zone.ZoneRulesProvider:177)
>
>> Also - it would be good to clarify the specification of
>>   public static Context getInitialContext(Hashtable<?,?> env)
>> It was not clear to me that you would loop over all the
>> services found by the ServiceLoader until you'd find one
>> whose concrete class matched the name pointed to by
>> Context.INITIAL_CONTEXT_FACTORY.
>
> Don't you think it becomes than 'overspecified'? Why should we want to tie ourselves?

Maybe it's because I haven't used JNDI very often, but when
I read the spec I didn't realize that the value of 
Context.INITIAL_CONTEXT_FACTORY would have any effect if
ServiceLoader was used - because it's not usual to use
ServiceLoader in this way (even though the usage of
ServiceLoader is nested below 'the class specified in
the Context.INITIAL_CONTEXT_FACTORY</tt> environment property
is used').
I'm not sure it would be overspecifying to say that the ServiceLoader
is used to locate and load an implementation of the service whose
concrete class matches the named class - since that's what it's
doing anyway - but I understand your concern.


>> This seems a bit fragile to me - unless it's guaranteed that
>> the various InitialContextFactory have no static initializer
>> that might throw exceptions (such as SecurityException) - and
>> that their default constructor does nothing (so that instantiating
>> e.g. com.sun.jndi.cosnaming.CNCtxFactory when you're actually
>> looking for com.sun.jndi.ldap.LdapCtxFactory has no side effect
>> - which fortunately seems to be the case).
>
> Well, no one can guarantee us this. Even a constructor could do all the things you've mentioned :) It's just the nature of a factory. It should better be stateless and without any side effects.

So a broken InitialContextFactory for technology XXX can
prevent getting the InitialContext for technology YYY even if both
have nothing in common, and even if XXX is never actually
used in the application.
On the other hand I don't see that you have any other choice
but propagate the exception if one is emitted - as you can't know
whether it has been emitted by the class you're trying to load :-(

-- daniel

>
> -Pavel
>