RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v7]

[Alan Bateman]

On Fri, 22 Oct 2021 14:27:41 GMT, Aleksei Efimov <aefimov at openjdk.org> wrote:

>> This change implements a new service provider interface for host name and address resolution, so that java.net.InetAddress API can make use of resolvers other than the platform's built-in resolver.
>> 
>> The following API classes are added to `java.net.spi` package to facilitate this:
>> - `InetAddressResolverProvider` -  abstract class defining a service, and is, essentially, a factory for `InetAddressResolver` resolvers.
>> - `InetAddressResolverProvider.Configuration ` - an interface describing the platform's built-in configuration for resolution operations that could be used to bootstrap a resolver construction, or to implement partial delegation of lookup operations.
>> - `InetAddressResolver` - an interface that defines methods for the fundamental forward and reverse lookup operations.
>> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the characteristics of one forward lookup operation.  
>> 
>> More details in [JEP-418](https://openjdk.java.net/jeps/418).
>> 
>> Testing: new and existing `tier1:tier3` tests
>
> Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   More javadoc updates to API classes

I've done another pass over the API. Overall it's looking very good and my comments are just to improve the javadoc in a few places to make it clearer.

src/java.base/share/classes/java/net/InetAddress.java line 169:

> 167:  * Access Protocol (LDAP).
> 168:  * The particular naming services that the built-in resolver uses by default
> 169:  * depend on the configuration of the local machine.

depend -> depends

src/java.base/share/classes/java/net/spi/InetAddressResolver.java line 38:

> 36:  * deployed as a <a href="InetAddressResolverProvider.html#system-wide-resolver">
> 37:  * system-wide resolver</a>. {@link InetAddress} delegates all lookup requests to
> 38:  * the deployed <i>system-wide resolver</i> instance.

I think the class description would be a bit clearer if we drop sentence 2 because it overlaps with paragraph 2. I think we can adjust sentence 3 to "InetAddress delegates all lookup operations to the system-wide resolver" and it will all flow nicely.

src/java.base/share/classes/java/net/spi/InetAddressResolver.java line 88:

> 86:      *                                  to a valid IP address length
> 87:      */
> 88:     String lookupByAddress(byte[] addr) throws UnknownHostException;

I assume this throws NPE if addr is null.

src/java.base/share/classes/java/net/spi/InetAddressResolver.java line 137:

> 135:         /**
> 136:          * This factory method creates {@link LookupPolicy LookupPolicy} instance with a provided
> 137:          * {@code characteristics} value.

This should be "This factory method creates a LookupPolicy ...".

src/java.base/share/classes/java/net/spi/InetAddressResolverProvider.java line 45:

> 43:  * system-wide resolver instance, which is used by
> 44:  * <a href="{@docRoot}/java.base/java/net/InetAddress.html#host-name-resolution">
> 45:  * InetAddress</a>.

I think we should prune some some of the text from the class description to avoid repetition.  Here's a suggestion:

1. Add the following immediately after the sentence "A given innovation ..."
"It is set after the VM is fully initialized and when an invocation of a method in InetAddress class triggers the first lookup operation. 

2. Replace the text in L47-65 with:
"A resolver provider is located and loaded by InetAddress to create the systwm-wide resolver as follows:

3. Replace the remaining three usages of "install" with "set".

-------------

PR: https://git.openjdk.java.net/jdk/pull/5822