Hi Michael
Please find my feedback on the current API docs below. It's divided in 3
parts: API (mostly questions), documentation (mostly suggestions for
clarifications) and spelling.
Thanks in advance for your consideration.
Kind regards, Anthony
=== API ===
* will there be support for HTTP 2.0 stream prioritization? For example
by providing a method requestHttp2(int priority) in
HttpClient.Builder/HttpRequest.Builder? This way one could create 2
clients with a different priority: a low-priority one for background
downloads of documents/large files, and a high-priority one for status
updates. Then the 2 clients would use the same TCP connection, but
different streams with different priorities.
* which convenience methods will be added to existing classes, if any?
I'm particularly thinking of URL, which already has
openConnection/openStream convenience methods for the old API.
* how is compression (the
Accept-Encoding/Content-Encoding/Transfer-Encoding headers) handled? Is
this handled transparently by the HttpClient? For example, if I request
a text document, will the client automatically send a header
"Accept-Encoding: gzip, deflate" (this is the current default in
Firefox)? And once I receive the response, transparently decompress it,
so I can just do "response.body(asString())"?
* how is caching handled? Is there anything analog to "useCaches" in
java.net.URLConnection?
* HttpClient.Builder: concerning "followRedirects", I feel there are 2
more options that ought to be possible to be set: "same-protocol" (i.e.
the current java.net.HttpURLConnection behavior) and "secure" (all
redirects are allowed, except for redirects from https to http). A
possible solution might be to introduce a "RedirectPolicy" enum with 4
constants: NEVER, ALWAYS, SAME_PROTOCOL, SECURE. Then the method would
become "followRedirects(RedirectPolicy policy)".
* HttpClient.Builder: the documentation for "requestHttp2" says: "If
that fails, then following requests will not attempt to upgrade again."
How is it determined that the upgrade to HTTP/2 fails? For example, is
the client able to distinguish "the upgrade to HTTP/2 failed" from "some
error occured which caused the request to fail" (e.g. the server is
down)? And how can I force to attempt to upgrade again, for a specific
origin and/or all origins? For example in the case of application
servers, having to restart the JVM just to have the HTTP/2 upgrade to be
attempted again, seems rather harsh.
* HttpHeaders: why is the method "firstValueAsInt" and not
"firstValueAsLong"? If I want to download e.g. the .iso file of Oracle
Linux, the value of Content-Length will be too big and a
NumberFormatException will be thrown.
* HttpHeaders: why is there only a special case for "firstValueAsInt"?
Personally, I would like some more methods to be added:
** java.time.OffsetDateTime firstValueAsDate(String name,
Supplier<OffsetDateTime> defaultValue) which parses headers such as
Expired & Last-Modified, using the datetime format recommended by RFC
7231 (i.e. the format defined in RFC 5322)
** javax.activation.MimeType firstValueAsMimeType(String name,
Supplier<MimeType> defaultValue) which parses headers such as
Content-Type & Accept-Patch
** List<java.net.HttpCookie> allCookies() which returns a list of all
cookies
* HttpHeaders: for the proposed first*() methods, please also consider
the signature: Optional<T> firstValueAsT(String name). This way, the
application developer can decide whether to use a default value or to
throw an exception (or anything else) when the header is absent.
* HttpRequest/HttpResponse: why weren't the static methods added to the
interfaces
(HttpRequestBodyProcessor/HttpResponseBodyProcessor/MultiResponseProcessor)
instead? In my opinion, this would be more logical, and would simplify
the API of HttpRequest/HttpResponse.
* HttpRequest.Builder: add validation to header/method/setHeader
(according to the rules in RFC 7230) & throw an IllegalArgumentException
if validation fails
* HttpRequest.Builder: for "method", how is the parameter handled? Since
method is case-sensitive ( cf.
https://tools.ietf.org/html/rfc7230#section-3.1.1 ), creating a request
with "delete" may fail, simply because it should be "DELETE". I think it
would be good to at least add a Javadoc note about the case-sensitivity,
and that the standardized methods (
http://tools.ietf.org/html/rfc7231#section-4.1 ) are defined as
all-uppercase.
* HttpRequest: I'd propose to rename fromByteArray(Iterator<byte[]>) to
fromByteArrays
* HttpRequest::fromString and HttpResponse::asString should take a
java.nio.charset.Charset as parameter
* HttpResponse: in my opinion, "asFile" should take a Path as parameter
& the parameter name should be "file", instead of "filename"
* HttpResponse: concerning the "asString" methods: they refer to "the
character set specified in the Content-encoding response header".
However, this should be "the character set specified in the charset
parameter of the Content-type response header".
* WebSocket: the documentation says "Once the WebSocket is available,
WebSocketMessages can be created and sent either blocking or
asynchronously.". What does it mean to create a message asynchronously?
And how can I send a message asynchronously (without manually creating a
CompletableFuture), as there isn't a "sendAsync" method in WebSocket?
* WebSocket: in "Asynchronous example", I believe "sockets" should be
defined as "new CopyOnWriteArrayList<>()", since LinkedList is not
thread-safe? Also, I personally would declare "futures" as "new
ArrayList<>()" (unless there's a compelling reason to use LinkedList of
which I'm unaware?).
* please consider adding a HttpResponseBodyProcessor implementation
"asDefined()", which uses the same mechanism as
java.net.URLConnection::getContent (i.e. using the Content-Type header &
ContentHandlers) to determine the object to return. This would allow for
easy migration from the old API to the new API. (the "defined" in the
method name refers to the fact that the value of the Content-Type header
is used)
* please consider adding a HttpResponseBodyProcessor implementation
"asFileDownload(Path downloadDirectory, OpenOption... openOptions)".
This would determine the filename automatically, exactly as browsers do
by inspecting e.g. the Content-Disposition header.
=== documentation ===
* as I'm sure you are aware, the package Javadoc should be updated to
document the new API. I also think it would be good to clarify its
relation to the old API, JAX-RS and Java API for WebSocket (the latter 2
of which will, I assume, in future versions use this API as the basis
for their Client API implementations).
* HttpClient: "Request builders are then created by calling request(URI)
if associated with an application created client."
rephrase as "Request builders that are associated with an
application-created client, are created by calling request(URI)."
* HttpClient.Builder: "If set, the first request to an origin server
using "http" URLs will attempt to upgrade to HTTP version 2. If that
fails, then following requests will not attempt to upgrade again."
** make "origin server" a hyperlink to
https://tools.ietf.org/html/rfc6454#section-4
** explicitly state "following requests to the same origin server"
* HttpRequest: "A request's uri, headers and body can be set."
change "uri" to "URI" & link it to java.net.URI
* HttpRequest: currently, all examples specify ".body(noBody())". This
gives the impression it's actually required. I propose to remove this
from all examples of GET requests (especially the one in the "Two
simple, example HTTP interactions" at the top).
* HttpRequest::fromString and HttpResponse::asString: replace
"ISO8859_1" with ISO-8859-1 and link to
java.nio.charset.StandardCharsets.ISO_8859_1
* HttpResponse: "such as String, byte arrays, Files."
change "Files" to "files", since "Files" suggests that it returns
java.io.File, when instead it's java.nio.file.Path
* HttpResponseBodyProcessor: "write responses to String, byte[], File,
Consumer<byte[]>"
change "File" to "file", since "File" suggests that it returns
java.io.File, when instead it's java.nio.file.Path
* HttpResponseBodyProcessor: "If an exact content length was provided in
onRequestStart(), then that number of bytes must be provided."
explicitly add "before returning null" at the end, so: "If an exact
content length was provided in onRequestStart(), then that number of
bytes must be provided before returning null."
* WebSocket: add a note that using a WebSocket in a try-with-resources
statement will cause the close to be done by closing the underlying TCP
connection & what the possible implications are.
=== spelling ===
* HttpHeaders: "read only"
should be "read-only"
* HttpRequest: "any type as body,"
either has missing text after the comma, or the comma should be a point
* HttpRequest: "client initiated"
should be "client-initiated"
* HttpRequest: "The response body can then be received (either
synchronous or asynchronously)"
should be "The response body can then be received (either synchronously
or asynchronously)."
* HttpRequest: "Path body = r2.body(asFile("/tmp/response.txt));"
should be "Path body = r2.body(asFile("/tmp/response.txt"));"
* HttpRequest: "All of above examples"
should be "All of the above examples"
* HttpRequest: "CompletableFuture<String>; last ="
should be "CompletableFuture<String> last ="
* HttpRequest: "Returns the HttpClient.requestHttp2() setting for this
request."
should note that this setting may have been overridden using
HttpRequest.Builder.requestHttp2(), and therefore is not always equal to
HttpClient.requestHttp2()
* HttpRequest.Builder: "A builder of HttpRequests"
should be "A builder of HttpRequests." (if you look at the package
overview, all classes except this one have a point at the end)
* HttpResponseBodyProcessor: "return null and the body"
has missing text after body
* MultiResponseProcessor: "provides the"
should be "Provides the"
* WebSocketMessage: "A WebSocketMessages (Binary or Text)"
should be "A WebSocketMessage (Binary or Text)"
On 9/03/2015 17:27, Michael McMahon wrote:
Hi,
JEP 110 HTTP 2 client
in JDK 9, is defining and implementing a new API for HTTP which also
supports
the new HTTP version 2 that has recently been working its way through
the IETF.
The work also includes support for websockets (RFC 6455).
In fact, the majority of the API is agnostic about the HTTP protocol
version, with only minor
configuration settings, and support for multiple responses (Http
server push) having any direct impact.
The HTTP API is defined around three main types (HttpClient, which is
the central
point for configuration of SSL, executor service cookie management
etc), HttpRequest
and HttpResponse (which should be self explanatory).
Requests are sent/received either synchronously (blocking) or in a
non-blocking (asynchronous)
mode using java.util.future.CompletableFuture which is a powerful new
framework for
asynchronous execution introduced in JDK 8.
The API docs can be seen at the link below:
http://cr.openjdk.java.net/~michaelm/httpclient/01/
All new classes and interfaces belong to the java.net package.
A prototype implementation of this API supporting HTTP/1.1 only, is
available and will
be uploaded into the JDK 9 sandbox forest in the coming day or two.
Comments welcome!
Thanks,
Michael.
