Just an update here. I've done some scratch refactoring, and see how to do this now. I'm not quite done but am perhaps halfway there.
The main issue here is that there is a "hidden class" inside CachingHttpClient that handles resource management and storage of entries in an HttpCache; by extracting this we can simplify CachingHttpClient and reduce its responsibilities. Where I'm going: The current HttpCache interface will be renamed to something like CacheStorage; this is really just about storing cache entries. There will be a new interface for HttpCache that is a little higher level, incorporating some of the code (including the resource management code) that is currently in CachingHttpClient. There will be a primary implementation of this that will take as constructor dependencies a ResourceFactory and a CacheStorage object. This should simultaneously simplify CachingHttpClient and nicely encapsulate resource management and CacheEntry storage together behind the HttpCache interface. Jon -----Original Message----- From: Oleg Kalnichevski [mailto:ol...@apache.org] Sent: Friday, August 13, 2010 3:39 PM To: HttpComponents Project Subject: RE: File based HTTP cache implementation is feature complete On Fri, 2010-08-13 at 11:28 -0400, Moore, Jonathan wrote: > Hi Oleg, > > I took a quick look, and see where you are going with this. My only > suggestion here is that I'm not sure that an HttpCache and a > ResourceFactory should both be constructors to the CachingHttpClient; > this feels to me like the CachingHttpClient is getting a little too deep > into the details of how the cache is going about managing its storage, > which ought not to be the concern of the CachingHttpClient. > > I think this can be addressed pretty simply, though, by altering the > HttpCache interface appropriately, and pushing the details of Resources > and ResourceFactories down into the classes that implement HttpCache. > > (So, for example, the constructor for the BasicHttpCache might > instantiate a HeapResourceFactory itself, and the ManagedHttpCache could > take a constructor where you can pass in a ResourceFactory). > > This simplifies the code in CachingHttpClient (probably just by moving > it elsewhere). I suspect this means that the BasicHttpCache can go back > to not being concerned with Resources or ResourceFactories at all (which > I would suggest is simpler and clearer). > > This then gives you flexibility to add cache implementations along > several ways: > 1. implement HttpCache outright (e.g. the memcached and ehcache packages > we're working on contributing) > 2. subclass or write analogues to the ManagedHttpCache and inject > existing ResourceFactories > 3. re-use the ManagedHttpCache and write a new ResourceFactory > > I think this maximizes flexibility while not adding complexity where it > isn't needed. > > I can take a stab at the rewrite I'm suggesting here if you don't have > time, although probably not until next week. > > Jon > Hi Jon I am not sure it is feasible to completely decouple CachingHttpClient from the resource management details (ResourceFactory) unless you are willing to revert back to the state when CachingHttpClient always created an intermediate in-memory copy of the response content. Anyways, please do feel free to take a stab at it. cheers Oleg --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org
