On Thu, 24 Mar 2022 06:46:01 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> This change proposes to implement the enhancement noted in >> https://bugs.openjdk.java.net/browse/JDK-8209137. >> >> The change introduces a new API to allow applications to build a >> `java.net.http.HTTPClient` configured with a specific local address that >> will be used while creating `Socket`(s) for connections. > > Jaikiran Pai has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 28 additional > commits since the last revision: > > - Merge latest from master branch > - merge latest from master branch > - Merge latest from master branch > - Merge latest changes from master branch > - fix javadoc to mention HttpClient instead of HTTPClient > - minor - rename variable in test > - Merge latest from master branch > - Merge latest from master branch > - Merge latest from master branch > - copyright year > - ... and 18 more: > https://git.openjdk.java.net/jdk/compare/6962a853...4c1361b2 I haven't rereviewed the test since it has not been updated yet. It would be good to have a unit test for the default implementation of the new method in HttpClient.Builder, as well as a test for its concrete implementation in HttpClientBuilderImpl. src/java.net.http/share/classes/java/net/http/HttpClient.java line 385: > 383: * @return this builder > 384: * @throws UnsupportedOperationException if this builder doesn't > support > 385: * configuring a local address Maybe there should be an `@implNote` to say that the default implementation of the HttpClient only supports `InetSocketAddress` - and that the port should be 0? Possibly also extend the `@throws UnsupportedOperationException` to cover the case where the given address is not supported? Something like: * @throws UnsupportedOperationException if this builder doesn't support * configuring a local address, or if the given local address is not one * supported by the concrete {@code HttpClient} implementation. src/java.net.http/share/classes/jdk/internal/net/http/HttpClientBuilderImpl.java line 140: > 138: return this; > 139: } > 140: Either this should check that localAddr is an `InetSocketAddress` and that port = 0, and throw an UnsupportedOperationException in that case, or the build() method should specify what exception is thrown and when, if the local address passed to the HttpClient implementation is not supported - because some exception will be thrown at some point... IMO it would be better to do the check early - although we would probably redo it in the HttpClientImpl (but there it wouldn't fail unless something bad happened). ------------- PR: https://git.openjdk.java.net/jdk/pull/6690
