Sangjin Lee wrote: > It looks great! I went through the APIs, and here are my early thoughts: > [HttpClient] > - Would a common usage be one HttpClient instance per host? It's not very > clear to me whether one instance can be used against multiple hosts or it > can be used only for a given host. Specifically, for the following method > > List<HttpRequestFuture> get(String... uris) > > must all uris be against the same host, or can they go to various hosts? > The javadoc "pipelining" suggests they must go all against the same host...
A single HttpClient instance may be used to connect to any number of hosts. With the List<HttpRequestFuture> get(String... uris), all the hosts and ports would have to be the same to pipeline the requests. > [HttpConnectorConfig] > - It would be good to support configuring socket parameters (tcpNoDelay, > soLinger, bufsize, ...). IMO these parameters should be configured through the underlying transport implementation. Because we're passing around buffers, it doesn't make a lot of sense to turn off tcpNoDelay and soLinger is something that should be managed by the transport. With MINA 2.0 (and perhaps 1.x, I don't remember) the buffer size is dynamic so I don't think we would want to expose this in the AsyncWeb API. > - One thing that we may want to see is supporting optional connection > retries. Agreed. > [HttpFuture] > - Should we support Future.cancel()? Whether we can truly cancel these > asynchronous operations here is bit tricky... If the operation gets queued up (because of maxConnections being exceeded, for example), it's trivial to cancel the request. Once the request gets sent, it really can't be cancelled in which case Future.cancel(boolean) would have to return false. But I agree, adding support for cancel is tricky. In ADBCJ I have support for cancel and it has been a real pain. In most cases I would be fine with just return false and not even trying to cancel the operation. One of the things I've been considering for ADBCJ is that if Future.cancel(true) is invoked, it closes the socket connection. This is similar to JSR 203 where calling IoFuture.cancel(true) on an asynchronous file request may close the file (this may be the same for asynchronous sockets too but I don't remember exactly.) We could use this same approach in AsyncWeb client. > [HttpListener] > - onComplete() would be invoked not only if there is a response ready but > also if there was an exception or a timeout on the response, correct? How > would one determine one case against another? Perhaps it might be a good > idea to provide a few convenience methods that let you determine exactly > which case (not unlike mina's IoFuture implementations). Some suggestions > include > > boolean isSuccessful(): terrible name for lack of a better alternative > boolean hasException() > boolean isTimedOut() > Throwable getException() > > We could simply say that one should call Future.get() and react according to > the outcome (whether it returns a result as opposed to throwing an executor > exception) but the above seems more direct and more convenient. In ADBCJ, you have to invoke Future.get() to determine if the request succeeded or not. In same cases it works well, in others it doesn't. I see two ways of solving the problem: - Create an abstract implementation of HttpListener that has callback methods such as "onSuccess, onCancel, onError, onTimeout". - Add methods as you suggested (isSuccessful, isTimedOut, etc.) Perhaps we should support both approaches. > [CookieManager] > - Would it also encapsulate the logic of accepting and validating cookies > coming from the server (i.e. handling the "Set-Cookie" header)? For > example, validation on the domain attribute needs to be done against the > request host name. Also, default values for the domain and the path > attributes need to be determined per spec before storing the cookies. I think the client should be responsible for validating the cookies in the HttpResponse so that neither the CM nor the code that invokes the AsyncWeb client have to worry about getting invalid cookies. We could have an event that is triggered when we receive an invalid cookie. > [Some things that exist in the old client but not here] > - Authentication support: we could migrate them with minimal modifications? We could put the username and password in the URI (i.e. http://USER_NAME:[EMAIL PROTECTED]/) for simple cases. I would like to add support for some type of AuthenticationManager that could be used for managing authentication credentials. The AuthenticationManager could also be used to provide a callback for when a request fails due to not authenticating (so, in the case of a GUI, for example, a dialog box could be displayed prompting the user to enter a username and password). > - Monitoring support based on key events: might be useful for > instrumentation and/or monitoring Yes, this needs to be added. > - Being able to insert an executor filter for isolating the codec and > post-operations > - Proxy configuration: support for exclusion lists Both good ideas. > > Thanks, > Sangjin Excellent suggestions. Thanks for the feedback. I've create a wiki page to track these ideas: http://cwiki.apache.org/confluence/display/AWEB/Client+Ideas -Mike > On Tue, Mar 18, 2008 at 1:56 PM, Mike Heath <[EMAIL PROTECTED]> wrote: > >> The proposed new AsyncWeb client API is available here for your viewing >> pleasure: >> http://svn.apache.org/repos/asf/mina/sandbox/mheath/asyncweb/client/ >> >> I've taken into account a lot of the feedback I've received so far. >> Some of the big changes include: >> >> - I've separated the HttpClientFactory and the HttpConnector. The >> HttpConnector provides HttpConnection objects which can be thought of as >> a step above a MINA IoSession. The HttpClientFactory produces >> HttpClient instances that provide a friendlier interface to issuing >> asynchronous HTTP requests. The configuration options have been >> separated as well >> - The HttpFuture object has get/setPayload methods that allow the user >> to arbitrarily associate an object with the future (this is an idea >> borrow from the JSR 203 IoFuture). >> - I added an HttpRequestFuture interface that extends HttpFuture and >> returns the HttpRequest object used to initiate the request >> - I added support for sending HttpRequest objects directly >> - I added PartialResponseListener for dealing with large responses. >> This has the same name as an interface proposed by Julien for the server >> API so we may need to change the name and/or discover overlap with the >> server side of the framework >> - I added some configuration options for HTTP proxy settings >> >> Some of the things that we need to work on still >> - We still need to add something analogous to the IoServiceListener in >> MINA or the Event mechanism in AHC >> - We need some way for configuring different encoding mechanisms (i.e. >> gzip encoding) >> - Caching - good caching and support for ETag would be a big plus >> >> I'm thinking that a HttpClientFactory would use a HttpConnector under >> the hood. This would allow us to implement HttpClientFactory and >> HttpClient once and be able to plug in any transport that implements >> HttpConnector. We can also implement connection pooling by creating a >> HttpConnector implementation that proxies another HttpConnector that >> does the actual network communication. >> >> It's still a work in progress so please tell me what you don't like and >> what could be improved. >> >> -Mike >> >
