Hi Anthony,
Comments/answers below
On 21/03/15 10:44, Anthony Vanelverdinghe wrote:
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.
I think setting a default priority could be a useful first step in that
direction, so long as we don't preclude full support of stream priority
in the future,
which includes per-request priority and inter-request dependencies. I
think these would be settable per-request only, but the default would be
on the client.
* 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.
I hadn't planned adding any convenience methods to other classes.
* 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())"?
It wouldn't be transparent as such. But, it should be possible to
implement it using HttpRequestBodyProcessor and
HttpResponseBodyProcessor, but I wasn't
planning to do it as part of this work initially. Someone could define a
class like:
class Deflator implements HttpResponseBodyProcessor {
public Deflator(HttpResponseBodyProcessor p) {..}
:
}
So Deflator takes another response processor and you might use it as:
String body = response.body(new Deflator(asString()));
or something similar.
* how is caching handled? Is there anything analog to "useCaches" in
java.net.URLConnection?
That is an omission in the current API. We need to add setting/getting
of a java.net.ResponseCache to HttpClient(builder)
* 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)".
That seems useful and exploits something the old API couldn;t support
given that HttpsURLConnection was a subtype of HttpURLConnection
* 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.
There is some vagueness there, which I hoped to clarify after
implementation starts. The upgrade "failing" just meant
either the ALPN negotiation failing or in the case of h2c, that the
server responds with a regualr HTTP/1.1 response (rather than
a 101 Switching Protocols response). These mechanisms seem to be quite
well defined in the spec and have nothing
to do with other server errors (such as transient network errors)
I think the upgrade should be attempted once per HttpClient instance. If
it fails once, it's not likely to succeed soon again.
But a new HttpClient instance can be created to retry if desired.
* 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.
Good point.
* 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 defaultValue) which parses headers such as
Expired & Last-Modified, using the datetime format recommended by RFC
7231 (i.e. the format defined in RFC
