[RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider
christoph.langer at sap.com
Fri Dec 8 14:39:31 UTC 2017
a little style nit: Is it really a good idea to use import java.util.*; in src/java.naming/share/classes/com/sun/jndi/ldap/LdapCtxFactory.java? I thought one is supposed to only use qualified exports nowadays (with all the IDE support).
From: core-libs-dev [mailto:core-libs-dev-bounces at openjdk.java.net] On Behalf Of Rob McKenna
Sent: Mittwoch, 6. Dezember 2017 19:25
To: vyom tewari <vyom.tewari at oracle.com>
Cc: Osipov, Michael <michael.osipov at siemens.com>; core-libs-dev at openjdk.java.net
Subject: Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider
Thanks Vyom, these should be fixed in:
On 06/12/17 22:14, vyom tewari wrote:
> Hi Rob,
> Please find below comments.
> line 35: convert it to for-each code will be more readable.
> line 21: constant name does not follow Java naming convention, there are
> other places as well please fix it.
> getInstance() is already synchronized why you need another "synchronized"
> block ?
Ah, neglected to remove the outer synchronized keyword, thanks.
> Line 70: Please use result.getEndpoints().isEmpty() in place of
> result.getEndpoints().size() == 0
> constructor LdapDnsProvider() calls LdapDnsProvider(Void unused) which does
> nothing, can you explain why LdapDnsProvider() calls LdapDnsProvider(Void
> unused) ?
I've copied this pattern from the System.LoggerFinder api and I had
forgotten what it was for too:
> Private field domainName & endpoints both can be final.
> Fix the tag Order it is not correct. correct order is as follows.
> * @test
> * @bug 8160768
> * @summary ctx provider tests for ldap
> * @modules java.naming/com.sun.jndi.ldap
> * @compile dnsprovider/TestDnsProvider.java
> * @run main/othervm LdapDnsProviderTest
> * @run main/othervm LdapDnsProviderTest nosm
> * @run main/othervm LdapDnsProviderTest smnodns
> * @run main/othervm LdapDnsProviderTest smdns
> Line 82,83 you don't need System.out.println(e); e.printStackTrace();
I'm going to leave these in as I've had a few failing tests in the past
where I've needed to push diagnostic changes like this to determine the
> Line 70: you don't need this try block
> Line 96 : constant name does not follow Java naming convention
> Line 105: you can use try-with-resources this will avoid some boilerplate.
> On Tuesday 05 December 2017 11:18 PM, Rob McKenna wrote:
> >As this enhancement is limited to JDK10 this frees us up to explore a
> >different approach.
> >In this webrev we're using the Service Provider Interface to load an
> >implementation of the new LdapDnsProvider class. Implementations of this
> >class are responsible for resolving DNS endpoints for a given ldap url
> >should the default implementation be insufficient in a particular
> >Note: if a security manager is installed the ldapDnsProvider
> >RuntimePermission must be granted.
> > -Rob
More information about the core-libs-dev