Review of new Http client API

[Paul Sandoz]

On Aug 23, 2012, at 5:05 PM, Chris Hegarty <chris.hegarty at oracle.com> wrote:

> Paul,
> 
> All good feedback, and I will leave it to Michael to reply to the specifics. On thought I had on separation of modes is something list this:
> 
>  interface HeaderHandler {
>      onHeaders(HttpResponse);
>  }
>  interface ErrorHandler {
>      onError(HttpResponse, Throwable);
>  }
>  interface BodyHandler {
>      onBodyPart(HttpResponse, ByteBuffer, boolean);
>  }
> 
>  class HttpRequest {
>      ....
>      AsyncHttpRequest async(HttpRequest);
>      ....
>  }
> 
>  class AsyncHttpRequest extends HttpRequest {
>      // no public constructors
>      ....
>      AsyncHttpRequest onHeaders(HeaderHandler);
>      AsyncHttpRequest onError(ErrorHandler);
>      AsyncHttpRequest onBodyPart(BodyHandler);
>  }
> 
>  class HttpClient {
>      ....
>      OutputStream sendHeaders(HttpRequest request, long contentLength)
>      Future<HttpResponse> sendRequest(HttpRequest req)
>      void sendRequest(AsyncHttpRequest req)
>  }
> 

OK, i would be inclined to separate out the instance used for building from the instance passed around, so one cannot muck around with the state of the latter.


> Then user code may do:
> 
>  AsyncHttpRequest request.async()
>               .onHeaders(r -> dumpHeaders(r))
>               .onError((r,t) -> handleError(r,t));
>               .onBodyPart((r,bb,c) -> transformBody(r,bb,t));
>  client.sendRequest(request);
> 

If these calls are on* calls are optional and one is not interested in when the headers have been received one could omit the onHeaders call, infact all those methods could be optional. That certainly simplifies things.


>  ....
> 
>  void dumpHeaders(HttpResponse r) {
>      System.out.println(r);
>  }
>  void handleError(HttpResponse r, t) {
>      throw t;
>  }
>  void transformBody(HttpResponse r, bb, boolean complete) {
>      System.out.println("Hello there!");
>  }
> 

FWIW you could use method references:

   AsyncHttpRequest request.async()
              .onHeaders(whateverthatclassiscalled::dumpHeaders)
              ...

Paul.


> Some experimentation is necessary to find a good balance here.
> 
> -Chris.