Http client API

[Michael McMahon]

Chris,

Thanks for the comments. Some responses below:


On 09/08/12 19:15, Chris Hegarty wrote:
> Michael,
>
> Looking good, some comments.
>
> 1) Why the use of CookieManager, rather than CookieHandler?  I  would
>    expect that CookieHandler would be a cleaner API
>

CookieHandler is a very low level API, which doesn't offer much 
functionality
beyond managing the cookies yourself, which is what you would have to 
do, if there
were no cookie capabilities in the http api. So, I don't know what we 
would gain from using it.

While CookieManager isn't perfect (and we'd like to suggest a few 
improvements)
it is a higher level API that does take care of cookie management with 
some common
policies pre-defined.

> 2) What is the impact on the sendHeader, setBody for HEAD requests?
>

Ah yes, HEAD needs special treatment - though not with respect to the 
methods above
as HEAD is identical to GET except with respect to any returned response 
body.

So, we need to clarify that HttpResponse.getContentLength() returns the 
value of
the content-length header in the case of HEAD responses. Also, I think 
we should
clarify how empty response bodies are treated. Something like:-
- an InputStream that returns EOF on first read
- empty byte[] or ByteBuffer[] with one element that is empty
- HttpResponseHandler calls onBodyPart() once with last=true and an 
empty ByteBuffer
- Iterator<ByteBuffer> where hasNext() returns false on first call.

> 3) I think HttpClient could be an interface and move the create method
>    to a builder/factory, and make it as immutable as possible ( this
>    came up a few times now ).
>

Right. Am looking into that now.

> 4) The Filter API looks a little funny, in that filter instances are
>    added to the client, while the ByteBufferWrapper instances are
>    presumably created by the implementation after registering the
>    wrapper class with the filter instance. Probably best/cleaner
>    to use the same style. It could also be an interface.
>

ByteBufferWrapper implementations could be generally useful (eg content 
encoders/decoders)
but each Filter will be associated with zero or one specific 
ByteBufferWrapper implementation.

Though, perhaps Filter could return ByteBufferWrapper instances rather 
than the class ...

What do you think the advantage of Filter being an interface is? In that 
case, implementers
would have to implement both methods in all cases. I think there will be 
many cases
where filters are interested in either the request or the response, but 
not both.

> 5) ByteBufferWrapper seems a little cluttered with implementation detail
>    setSource() nor setWrapper()?? Maybe best to just provide an
>    interface.
>

That comment about setSource() is wrong and needs to be removed. I agree
setWrapper() is implementation detail as it only needs to be called by 
the implementation.
Maybe it could be package private...

> 6) Upgrade handler, similar comment to 4. Why not just register an
>    instance.
>

Danny?

> 7) Exposing the filter list seems a little wrong, given the getter/
>    setter style.
>

We'll look at that again with the builder changes

> 8) java.net.httpclient.HttpConnectionCache.CachedConnection should
>    probably be an interface.
>

Yes, we were thinking about doing that.

> 9) How does the cache handle tunnels? endpoint address/proxy/etc
>

good point. That needs to be clarified.

> 10) Missing fluent style return from HttpRequest.setRequestBodyLimit
>

ok

> 11) Should sendHeaders be specified to allow a null return value. I'm
>     thinking about when setSendExpectContinue is set.
>

Do you mean if a 417 "expectation failed" is received? Yes, in that case it
probably does make sense to return null. It's a slightly awkward special 
case,
but would have to be documented.

We're trying to handle the normal situation as transparently as possible.
The idea is that a blocking application will just block until
the 100 continue is received, before being allowed to send the body.
An asynchronous application will just not be "called back" to get the body
until the 100 is received.

Thanks
Michael
> -Chris.
>
>
> On 08/08/12 00:09, Michael McMahon wrote:
>> Hi,
>>
>> A new revision of the Http client API planned for jdk 8 can be viewed
>> at the following link
>>
>> http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/
>>
>> We would like to review the api on this mailing list.
>> So, all comments are welcome.
>>
>> Thanks
>> Michael McMahon.