On Fri, 22 Oct 2021 14:27:41 GMT, Aleksei Efimov <aefi...@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
