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.
