Review of new Http client API

[Chris Hegarty]

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>);
  }

  response.onHeaders(r -> headers(r))
          .onError((r,t) -> error(t))
          .onBodyPart((r,bb) -> body(r, bb));

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
>
>