RFR: 8276559: (httpclient) Consider adding an HttpRequest.Builder.HEAD method to build a HEAD request.

Daniel Fuchs dfuchs at openjdk.java.net
Fri Nov 12 10:31:35 UTC 2021


On Thu, 11 Nov 2021 12:05:49 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?

Maybe `test/jdk/java/net/httpclient/HeadTest.java` could acquire a new dedicated method where `HttpRequest.Builder.HEAD()` is used. The test currently tests `method("HEAD", BodyPublishers.noBody());`

src/java.net.http/share/classes/java/net/http/HttpRequest.java line 382:

> 380:                         case "GET" -> builder.GET();
> 381:                         case "DELETE" -> builder.DELETE();
> 382:                         case "HEAD" -> builder.HEAD();

The comment above (line 376)  mentions:

                // otherwise, the body is absent, special case for GET/DELETE,
                // or else use empty body

maybe HEAD should be added there as well...

test/jdk/java/net/httpclient/HttpRequestNewBuilderTest.java line 124:

> 122:                 // dedicated method
> 123:                 { HttpRequest.newBuilder(URI.create("https://method-1/")).GET().build() },
> 124:                 { HttpRequest.newBuilder(URI.create("https://method-1/")).HEAD().build() },

I'd suggest to change the URI path to `/method-0/` and move the HEAD case to just after line 122 (before GET)

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

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


More information about the net-dev mailing list