RFR JDK-8044627: Update JNDI to work with modules
Daniel Fuchs
daniel.fuchs at oracle.com
Tue Sep 16 13:11:18 UTC 2014
On 9/16/14 1:12 PM, Pavel Rappo wrote:
> Hi everyone,
>
> Could you please review my change for JDK-8044627?
>
> http://cr.openjdk.java.net/~prappo/8044627/webrev.00/
>
> -Pavel
>
Hi Pavel,
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 ?
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.
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).
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.
best regards,
-- daniel
More information about the core-libs-dev
mailing list