Review of new Http client API

[Chris Hegarty]

On 22/08/12 21:05, Michael McMahon wrote:
> On 22/08/12 15:29, Chris Hegarty wrote:
>> Michael what you have looks good.
>>
>> But, I think what Sam is suggesting ( or maybe not, but I like it ;-)
>> ), is something like this. (I'd need to think more about what effect
>> this has on the different modes, async/blocking )
>>
>>  class HttpResponse {
>>    HttpResponse onHeaders(Block<HttpResponse>);
>>    HttpResponse onError(BiBlock<HttpResponse,Throwable>);
>>    HttpResponse onBodyPart(BiBlock<HttpResponse,ByteBuffer>);
>>  }
>>
> Right. I see what you mean in terms of making the setting of the callbacks
> fluent. I assume that Block and BiBlock are types associated with Lambda

Right, these are types defined in the lambda repo, that will most likely 
be part of JDK8. Worth a look, and we should also confirm with the 
lambda folks before build them into this API.

> somehow,
> but otherwise this is unfamiliar territory to me ....
>>  response.onHeaders(r -> headers(r))
>>          .onError((r,t) -> error(t))
>>          .onBodyPart((r,bb) -> body(r, bb));
>>
> So,  headers() and body() would be methods on HttpResponse ... Right ?
> What is error()?

No, sorry for the confusion. headers(), error(), and body() are just 
examples of what some user code could be ( rather than cluttering the 
example with too much code ).

-Chris.

>
> - Michael.
>> Alternatively, I believe something like this would also be compatible
>> with lambda (since there is a default implementation for on Error):
>>
>>   interface HTTPResponseHandler {
>>     public void onHeaders(HttpResponse resp);
>>
>>     public void onError(HttpRequest request, Throwable exception)
>>     default { throw exception; }
>>  }
>>
>> -Chris.
>>
>>
>> On 21/08/2012 14:57, Michael McMahon wrote:
>>> 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
>>>
>>>
>