hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: thoughts on httpclient decorators
Date Sat, 02 Jun 2012 13:15:05 GMT
On Fri, 2012-06-01 at 10:07 -0400, Jon Moore wrote:
> As we've seen, we've run into a couple of problems with some of our
> decorators for the DefaultHttpClient where underlying backends do
> non-transparent things (ranging from decompressing content bodies without
> updating response headers, to following redirects). I do think the
> decorators provide a nice way to layer the overall functionality from a
> codebase point of view, but I think they are brittle because they rely on
> unenforceable assumptions (e.g. that backend clients that get injected
> behave transparently).
> 
> I think the solution to this is to continue using the decorators, but hide
> them from most users--in other words, reimplementing the DefaultHttpClient
> as a stack of decorators that get enabled/disabled by configuration. In
> other words, a user should be able to "new DefaultHttpClient()" and get
> something that is capable of caching, following redirects, and handling
> compression (per configuration).
> 
> I think this looks like:
> (1) generate a decorator for redirect following
> (2) create a new class (BasicHttpClient?) that is essentially
> DefaultHttpClient minus the redirect logic
> (3) reimplement DefaultHttpClient so that it generates and configures a
> stack of RedirectFollowingHttpClient, DecompressingHttpClient,
> CachingHttpClient, BasicHttpClient (this might mean needing to pull the
> httpclient-cache module into the httpclient module)
> (4) add warning documentation on the DecompressingHttpClient and
> CachingHttpClient about needing to wire them up correctly (this should
> probably happen first, actually)
> 
> I suspect there are actually other decorators lurking in there too
> (authentication? cookie handling?).
> 
> I think this potentially gives us a clean way to layer in
> functionality--implement it as a decorator and then figure out where in the
> stack is the right/safe way to add it. Opting into the new functionality is
> a matter of turning it on in the config for DefaultHttpClient, which is far
> simpler for users to do.
> 
> Thoughts?
> 
> Jon

Hi Jon

Absolutely, having a number of smaller specialized components combined
together into a processing chain (based on the chain of responsibility
pattern0 would make for a much better design and clear API. There is,
unfortunately, a reason why redirect and authentication handling logic
is currently crammed into one class (DefaultRequestDirector). There can
be really complex scenarios where a redirect can cause re-authentication
with the site and successful authentication can trigger a new redirect,
which in its turn causes another re-authentication. It took _literally_
10 years to get all those scenarios working properly, especially those
involving NTLM. The last known issue related to HTTP authentication was
resolved just recently in the 4.2 release. Speaking from experience
troubleshooting such complex HTTP exchanges I suspect redirect and
authentication handling logic are simply inseparable and must be
executed in a single request execution loop. To make matters worse, many
of those scenarios cannot be easily simulated (mainly thanks to NTLM)
and therefore are not covered in our unit and integration tests.  

Having said all that please do not feel discouraged from pursuing the
idea of splitting DefaultRequestDirector into smaller chunks of code and
moving redirect handling code into a separate HttpClient decorator. We
probably can still settle for a middle ground approach: leave redirect /
auth logic in DefaultRequestDirector but disable it when using
HttpClient decorators. In any way I really like the idea of developing a
builder class to instantiate HttpClient for complex scenarios involving
caching / transparent decompression / authentication and transparent
redirect handing. Feel free to create a branch for 4.2.x releases and
start working on the trunk or on an experimental branch.

cheers

Oleg



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Mime
View raw message