Fwd: Http Client API for JDK 8
Kurchi Hazra
kurchi.subhra.hazra at oracle.com
Thu May 3 13:28:46 PDT 2012
Hi Jim,
Thanks for all the input!
For (7), the request itself is also being sent asynchronously here (by
leveraging the async NIO API) (in addition to the response being
received asynchronously).
- Kurchi
On 5/3/2012 12:43 PM, Jim Gish wrote:
> -------- Original Message --------
>> Subject: Http Client API for JDK 8
>> Date: Thu, 03 May 2012 11:22:00 +0100
>> From: Michael McMahon <michael.x.mcmahon at oracle.com>
>> To: OpenJDK Network Dev list <net-dev at openjdk.java.net>
>>
>>
>>
>> Hi,
>>
>> Attached is a zipped bundle of a proposed new Http client API for JDK 8.
>> The Javadoc can also be inspected at the following URI
>>
>> http://cr.openjdk.java.net/~michaelm/httpclient/v0.2/javadoc/
>>
>> The documentation is still somewhat sparse, but I hope there is enough
>> to understand the intent behind all of the new classes/methods.
>> It is still in an early stage of definition. So, all comments are welcome.
>>
>> Thanks,
>> Michael
>>
> Hi Michael,
>
> I have little expertise in this area, so I have only a few comments on
> content, but overall it looks good. However, here are a few things
> that I noticed.
>
> 1. An Overview would be helpful
> 2. I think I understand the why you have capitalized Upgrade when
> referring to an "HTTP Upgrade" being done. However, it doesn't
> strike me as necessary or particularly correct. I would just
> use "upgrade".
> 3. When viewing the http javadoc via the URL above, and
> horizontally expanding and shrinking the window, the Method and
> Description content under Method Summary/Methods does not wrap
> and thus gets truncated - forcing the user to do horizontal
> scrolling to see the content. This is also true for some, but
> not all, Method Detail(s). For example, in
> httpclient.AsynchronousHttpClient, the method signatures for
> send and setAsynchronousChannelGroup do not wrap, although the
> descriptions for those methods do. (This may be a consequence
> of doclet behavior rather than anything you are doing -- I don't
> know).
> 4. HttpCompletionHandler
> 1. "The HttpCompletionHandler is the completion handler
> implemented by applications that make use of the
> asynchronous mode in order to obtain notification when the
> HttpResponse is ready to be read." -- I would strike "the
> completion handler", yielding "The HttpCompletionHandler
> is the completion handler implemented by applications that
> make use of the asynchronous mode in order to obtain
> notification when the HttpResponse is ready to be read.
> 2. "This method is always invoked once only for every
> request, regardless of whether an actual response was
> received from the serve." -> "This method is always
> invoked once only for every request, regardless of whether
> an actual response was received from the /*server*/. "
> (missing "r")
> 5. Similarly to above, AsynchronousUpgradeConnection -- "The
> AsynchronousUpgradeConnection represents a connection obtained
> as the successful result of an HTTP Upgrade using the client in
> asynchronous mode." -> "An AsynchronousUpgradeConnection is
> obtained as the successful result of an HTTP Upgrade using the
> client in asynchronous mode."
> 6. HttpConnectionCache - "The client calls before every request to
> either get an existing cached or newly opened connection. When
> finisthed with a connection it calls" would be better as "The
> methods on this interface are called by the client prior to
> using or creating a connection and also to inform the cache that
> a connection is no longer needed by the client." (The second
> sentence as it is incomplete.)
> 1. I think returnToCache should be called either
> releaseConnection or clear or freeConnection. Actually,
> is this method really needed at all? It seems like the
> close() on the connection should take care of this.
> Either that or the method should be on the connection itself.
> 2. "Represents a cache of idle TCP connections that can be
> managed according to a user (or application) defined
> policy" -> should be "user-defined" or
> "application-defined", hence "user- (or
> application-)defined" (Yeah I know. This is a /real
> /nit! :-) )
> 7. AsynchronousHttpClient -- "The type of client that sends an
> asynchronous request and receives the asynchronous response at
> some undetermined time later." -> "A client that sends a
> request and asynchronously receives a response." (unless the
> server is pushing responses before getting a request, you
> typically would receive the response /later/ than the request
> :-)). Also, I don't think you are /sending/ the request
> asynchronously, are you? Aren't you sending a request to which
> you are requiring an asynchronous /response. /You certainly can
> describe this as an asynchronous request, but it's not quite
> correct to say that the request is being made asynchronously
> (unless of course it really is). There is similar language in
> the send() methods' descriptions.
> 8. AsynchronousByteChannel - getBody() "Returns an asynchronous
> byte channel to read read the response body." -> 'Returns an
> asynchronous byte channel to read read the response body." (2
> occurrences - method summary and method detail)
> 9. DataSource - "It can also be implemented by Filter modules for
> the purpose filtering all request or response body data." -> "It
> can also be implemented by Filter modules for the purpose /*of*
> /filtering all request or response body data. Note."
> 10. HttpMethodBase - "MExtesible enum interface" /MExtesible/ ?
> 11. HttpUpgradeHandler
> 1. "This interface is implemented by the client side of an
> Http Upgrade protocol handshake..." -> "HTTP" or "http"
> instead of "Http" for consistency throughout the doc.
> (Although, given the usage of camel case in the
> interface/class/method names, maybe not. In any case, all
> three forms are used in various places in the docs) Also,
> as before Upgrade -> upgrade.
> 2. "If so, the Http Client library will request a reference
> to the protocol specific connection object by calling the
> getConnection method." -> /protocol-specific/
> 3. "The HttpResponse obtained from an HttpRequest with an
> HttpUpgradeHandler carries the active connection
> (UpgradeConnection) object object created as a result of
> the handshake (if successful)." -> "The HttpResponse
> obtained from an HttpRequest with an HttpUpgradeHandler
> carries the active connection (UpgradeConnection) object
> object created as a result of a successful handshake."
> 12. UpgradeConnection
> 1. "Interface to represent a connection managed in the
> HttpConnection Pool and which Encapsulates a
> SocketChannel" - no need to capitalize "encapsulates."
> 2. "The channel remains private and hidden. But all channels
> used by the client must be expose this API" -> "The
> channel remains private and hidden, but all channels used
> by the client must expose this API." ("But. The"-> "but,
> the" and adding missing period after "API").
> 13. BlockingHttpClient -"The type of client that sends a request,
> blocking the current thread of execution until the response is
> ready to read." -> "A client that ....." (less wordy)
> 14. ByteBufferWrapper -implRestart() "should be overridden if this
> class needs to do something when restart called Default is to do
> nothing." no need to capitalize "default"
> 15. Filter - "The new request will sent and the response to that
> request will be linked back to the original request. ie the
> calling code will see the response to the new request as the
> response to the original request." -> "The new request will
> */be/* sent and the response to that request will be linked back
> to the original request*/, i.e./* the calling code will see the
> response to the new request as the response to the original
> request."
> 16. HttpClient
> 1. "Each instance is responsible for a series of connections
> which all have the same characteristics (in particular
> execution mode), some of which can be overriden for a
> particular request, for example default request timeout."
> -> "Each instance is responsible for a series of
> connections. Each connection has the same
> characteristics, and in particular, the execution mode.
> Some of the characteristics, such as the default request
> timeout, can be overridden for a particular request."
> 2. "Notes on Thread Safety For the blocking client, the API
> will be threadsafe." -> "Notes on thread safety" (should
> this be paragraph header with "For the blocking client,
> the API will be threadsafe." the first sentence?
> 17. HttpConnectionCache.CachedConnection
> 1. isProxy() - "returns if this connection is to a proxy," ->
> "returns */true/* if this connection is to a proxy,"
> 2. getAge() - "returns age of TCP connection in milli-seconds
> at time of object creation." -> "returns /*the*/ age of
> TCP connection in milli-seconds at time of object creation."
> 3. getUseCount() - "returns number of times the connection
> has been used" -> "returns /*the */number of times the
> connection has been used"
> 4. isClosed() - "returns whether the connection is closed" ->
> "returns true iff the connection is closed"
> 5. close() -- as I said before, perhaps this is all that is
> needed to "return" the connection to the cache.
> 18. HttpRequest - setBody() - "Sets this request's request body
> statically, ie. it is provided up-front. in the given
> ByteBuffer[]." -> "Sets this request's request body statically,
> /*i.e.*/ it is provided up-front. in the given ByteBuffer[]."
> (extraneous period between "up-front" and "in the given").
> 19. SimpleHttpConnectionCache - "Connections are stored in a
> HashMap." Implementation details shouldn't be in the description.
> 20. HttpHeaderNames, HttpMethod -- aren't this elsewhere already?
>
> Thanks,
> Jim
>
>
>
--
-Kurchi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/net-dev/attachments/20120503/8b7bfeb8/attachment.html
More information about the net-dev
mailing list