RFR: 8276559: (httpclient) Consider adding an HttpRequest.Builder.HEAD method to build a HEAD request. [v2]
Daniel Fuchs
dfuchs at openjdk.java.net
Tue Nov 16 12:10:34 UTC 2021
On Fri, 12 Nov 2021 11:24:56 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> Can I please get a review for this change which implements the enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8276559?
>>
>> The commit in this PR introduces a new `HEAD()` method on the `HttpRequest.Builder` interface. Given that this is a public interface on a public class of a public module, I decided to add this new method as a `default` method to prevent any potential breakages of any non-JDK implementations of this interface. The internal `jdk.internal.net.http.HttpRequestBuilderImpl` which implements this interface, overrides this default implementation to use an implementation of its own.
>>
>> Existing tests have been updated to include coverage for this new method.
>>
>> P.S: Slightly unrelated question - is it intentional that the `Builder` interface specifies the visibility modifiers on the interface methods. For example: `public abstract Builder timeout(Duration duration);`, `public Builder headers(String... headers);` and so on?
>
> Jaikiran Pai has updated the pull request incrementally with four additional commits since the last revision:
>
> - Implement Daniel's review suggestion - use a different URI for HEAD() request in HttpRequestNewBuilderTest
> - Remove unused imports and fields from HeadTest
> - - Update the HeadTest to use the convenience request builder methods GET() and HEAD()
> - Also add @bug tag to updated tests
> - fix comment about special casing certain HTTP methods
The changes look good. I'm going to give it a round of testing and I will approve if the results come clean.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6348
More information about the net-dev
mailing list