Review of new Http client API

[Michael McMahon]

Sam,

Thanks for the comments. Some discussion below.


On 17/08/12 00:13, Sam Pullara wrote:
> I suggest that you make it a more fluent API rather than having
> multiple callback methods in your callback interface. As it stands it
> isn't compatible with lambdas. You might take some inspiration for the
> asynchronous callbacks from my work porting Twitter's Future/Promise
> to JDK8:

I agree with the above. In a previous version of the API the main 
callback was lambda compatible.
Originally we used HttpResponse to encapsulate everything related to a 
response
including errors. But, some preferred to keep HttpResponse aligned to an 
actual response
from a server in all cases. There might be other ways to get around that 
by combining
HttpResponseHeadersHandler.{onError(), onHeaders()} back into a single 
method.
Maybe, drop the onError() method and add the exception/throwable as a 
parameter to onHeaders()

But, we also wanted to provide notification of body data (through the 
sub-interface HttpResponseHandler).
Keeping the two interfaces distinct meant that applications could get 
asynchronous notification of
the response headers, but then possibly read the response body in a 
blocking manner.
Or alternatively, applications can use the handler to be notified of 
both headers and body.

So, if we revert HttpResponseHeadersHandler back to having a single 
method, the sub-interface
now would have two methods (instead of three).

One way around that could be to have two unrelated interfaces:

interface HttpResponseHeadersHandler  {
     public void onHeaders(HttpResponse response, Exception e);
}

interface HttpResponseBodyHandler {
     public void onBodyPart(HttpResponse resp, ByteBuffer buffer, boolean last);
}

// Then a HttpResponseBodyHandler would be added to HttpClient.sendRequest() as below:

public void sendRequest(HttpRequest, HttpResponseHeadersHandler, HttpResponseBodyHandler);


Both of the interfaces would be lambda compatible (again) though at the cost
of having to specify two separate handlers. So, the following might be how
it could be used (and using a builder for HttpClient)

HttpClient client = HttpClient.createBuilder()
			.setAsynchronousChannelGroup (..)
			.setCookieManager(..)
			.setDefaultTimeout(..)
			.setProxy(...)
			.addFilter(...)
			.buildClient();

HttpRequest request = client.createRequest(new URI("http://www.foo.com/"))
			.setBody("Hello world".getBytes())
			.setMethod(HttpMethod.POST);

client.sendRequest (
     request,

     // handle headers
     (HttpResponse response, Exception e) ->  {
	if (response.getResponseCode() != 200) {
	    // handle error response
	}
	// handle normal case
     },

     // handle body
     (HttpResponse response, ByteBuffer buf, boolean last) ->  {
	// handle data in buf
     }
);

It seems fairly readable still, I think.

Another thing that this usage points to, is the usefulness of being able 
to hang some user context
off of the HttpResponse or HttpRequest objects. That would be the only 
way to share some user state
between the two handlers above, in this Lambda style.
> https://github.com/spullara/java-future-jdk8
>
> Another consideration might be to make sure that it is compatible with
> an implementation that is using SPDY under the covers for connectivity
> as I suspect that HTTP as a wire protocol has peaked though the HTTP
> semantics will survive.
Right. This is important. One area where there will be changes is with 
pipe-lining.
We need to ensure that our pipe-lining API is not restricted to only 
Http 1.1 pipe-lining
Are you aware of other areas that could have an impact on the API?

Thanks
Michael.

> Sam
>
> On Tue, Aug 14, 2012 at 5:01 AM, Michael McMahon
> <michael.x.mcmahon at oracle.com>  wrote:
>> Hi,
>>
>> (apologies for sending this again)
>> We have just published a draft of a proposed new Http client API [1] for JDK
>> 8.
>>
>> This message has been cc'd to jdk8-dev so that as many people as possible
>> know about it, but the discussion will be on the net-dev list
>> (net-dev at openjdk.java.net).
>> So, folks will have to join that list [2], in order to take part.
>>
>> Thanks,
>> Michael.
>>
>> [1]http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/
>>
>> [2]http://mail.openjdk.java.net/mailman/listinfo/net-dev