Hi Michael et al.
I've thought more about the API in general, and have some specific
feedback as well. I've also made the Javadoc available of an API which
incorporates all my ideas: http://jep110-anthonyvdotbe.rhcloud.com/api/
First of all, some practical questions:
Now that the implementation has been integrated into jdk9/dev, where
should I look for the latest changes: "dev"? or still the jep-110 branch
in "sandbox" for now?
What's the status of the API suggestions in my previous mail [1]? How
can I tell what 's been "considered and accepted/rejected" (& if
rejected, why) and what's "to be done"?
=== A bug
The simple test case below downloads a large .zip file 2 times, first
asynchronously, then synchronously. It has 2 issues:
- the asynchronous block prints "263" as the length of the response in
bytes (even though the .zip file is obviously much larger)
- the synchronous block immediately throws an "IOException: Connection
closed" when "response()" is invoked
Note that the order of the blocks doesn't matter: if the
asynchronous/synchronous blocks are swapped, the result is analog:
- the synchronous block prints "263"
- the asynchronous block immediately throws an "IOException: Connection
closed" when "get()" is invoked on responseAsync
HttpClient client = HttpClient.getDefault();
URI uri =
URI.create("http://www.java.net/download/java/jdk9/archive/110/binaries/jdk-9_doc-all.zip";);
// asynchronous
CompletableFuture responseAsync =
client.request(uri).GET().responseAsync();
Thread.sleep(120_000);
int lengthAsync =
responseAsync.get().body(HttpResponse.asByteArray()).length;
System.out.println(lengthAsync);
// synchronous
HttpResponse response = client.request(uri).GET().response();
Thread.sleep(120_000);
int length = response.body(HttpResponse.asByteArray()).length;
System.out.println(length);
=== BodyProcessors
In my previous message I proposed an alternative for these interfaces.
While I admit it had its flaws (e.g. the use of SubmissionPublisher), I
still believe the API should build upon the Flow API.
Related to this: in your response you said "extending Flow.Subscriber
means you would not be able to leverage existing Subscriber
implementations directly". What did you mean with this? How does the
current API allow to leverage existing Subscriber implementations in a
way that extending Flow.Subscriber wouldn't?
By building upon the Flow API, the API could simply state "give me a
correct Flow.Subscriber implementation". On the other hand, by not doing
so, a lot of specification has to be provided/duplicated, and many
questions are to be answered, e.g.:
- when is each method called, e.g. can onRequestError be called without
onRequestStart having been invoked first?
- what if any of the methods throws a RuntimeException?
- what if onRequestBodyChunk fills the ByteBuffer on another Thread
(e.g. by using an AsynchronousFileChannel) & returns false immediately?
- what if the flowController window remains at 0? Will the library set a
timeout, after which onXxxError will be invoked? Or will it wait
indefinitely?
Something else I'm not comfortable with, is that the responsibility for
providing the ByteBuffers is with the library, instead of the
BodyProcessor itself. Any buggy BodyProcessor implementation which
accidentally manipulates ByteBuffers after they've been returned to the
library, will likely cause totally unrelated requests to fail inexplicably.
Or, worst-case scenario: the library fills a ByteBuffer to be provided
to a HttpResponse.BodyProcessor X, then a buggy/malicious
HttpRequest.BodyProcessor Y overwrites the data in it (because the
Buffer had previously been provided to Y for writing part of its request
body to it), then X reads the ByteBuffer containing Y's data. Result: X
returns a corrupt result without being aware of it.
Finally, BodyProcessors don't currently have a way to cancel the
operation once it has started. For example, assume a large .zip file is
downloaded, but the BodyProcessor detects the file is corrupt as soon as
it has read the header (not sure if this is actually possible with .zip
files, but you get the idea). In this case the BodyProcessor should have
a way to say "I don't care about the remainder of the response, since
the file is corrupt anyway".
To address the above issues, I'd like to propose the following interfaces:
in HttpRequest:
interface BodyProcessor extends
Flow.Processor {}
interface BodyHandler {
BodyProcessor apply(URI uri, String method, Http.Version
version, HttpHeaders headers);
}
in HttpResponse:
interface BodyProcessor extends
Flow.Processor, Supplier> {}
interface BodyHandler {
BodyProcessor apply(int statusCode, Http.Version version,
HttpHeaders headers);
}
These BodyProcessor interfaces don't have any methods of their own &
merely extend the Flow interfaces. Any request/response-spec
