On Oct 13, 2010, at 12:05 PM, Imran M Yousuf wrote:
> Hi Paul,
>
> On Wed, Oct 13, 2010 at 3:57 PM, Paul Sandoz
> <Paul.Sandoz_at_oracle.com> wrote:
>> Hi Imran.
>> Is the problem with the constructors:
>>
>> public CacheableClientHandler(CacheStorage storage,
>> ResponseResolver
>> responseResolver)
>
> This one is required to be public, the other not decided yet, I might
> make it private later. The reason the above one needed to be public is
> so that users can use any HTTPCache4J resolver with this, i.e., if
> they are using Apache HttpClient in that case the configuration for
> jersey client will be same as Apache Jersey Client.
In that case it does sound like you require two separate handlers
(especially since the null value and the potential for disconnect
between the Entry key and value). The general and the Apache specific
one that delegates to the general one.
e.g. refactor CacheableClientHandler to be general and then define an
ApacheCacheableClientHandler that internally creates and instance of
CacheableClientHandler passing to it the CacheStorage and
ResponseResolver. The handler method of ApacheCacheableClientHandler
can do something like this:
public ClientResponse handle(ClientRequest request) {
try {
requestHolder.set(cr);
return ch.handle(request)
} finally {
requestHolder.remove();
}
}
Paul.
> And everything you
> mentioned below is the exact same reason why I last day thought it
> would be possible, but later reverted :(. the second constructor
> (below) is to public yet to allow users to hookup their custom
> resolver and method executor; but not sure how much value it will add,
> so still hanging on.
>
> Regards,
>
> Imran
>
>> public CacheableClientHandler(CacheStorage storage,
>> Entry<ResponseResolver, ApacheHttpMethodExecutor> entry) {
>> ? are those required to be public?
>> If the first is used the methodProcessor is set to null.
>> And presumably the latter only makes sense for the case when
>> ResponseResolver is somehow connected to ApacheHttpMethodExecutor
>> e.g. an
>> instance of CustomApacheHttpClientResponseResolver because the
>> value of the
>> Entry is never utilized (except to return the value from the getter
>> method).
>> If so this implies that you are merging two separate pieces of
>> functionality
>> in CacheableClientHandler that could be split into two separate
>> implementations where the Apache implementation could delegate to the
>> general implementation, which means you cleanly wrap the thread local
>> setting and getting (also the unsetting of the thread local value
>> should be
>> performed in a finally block).
>>
>> If those constructors are not required to be public it should be
>> possible to
>> support with the other constructors:
>> e.g.:
>> public class CacheableClientHandler
>>
>> extends ApacheHttpClientHandler {
>>
>>
>> private final HTTPCache cache;
>>
>>
>> private final boolean internalResolver;
>>
>>
>> private static final ThreadLocal<ClientRequest> REQUEST_HOLDER =
>> new
>> ThreadLocal<ClientRequest>();
>>
>>
>> public CacheableClientHandler(HttpClient httpClient, ClientConfig
>> clientConfig) {
>>
>> this(httpClient, clientConfig, new MemoryCacheStorage());
>>
>> }
>>
>>
>> public CacheableClientHandler(HttpClient httpClient, ClientConfig
>> clientConfig,
>>
>> CacheStorage storage) {
>>
>> super(httpClient, clientConfig);
>>
>>
>> ResponseResolver responseResolver = new
>> CustomApacheHttpClientResponseResolver(getMethodProcessor(),
>> REQUEST_HOLDER);
>>
>> this.cache = new HTTPCache(storage, responseResolver);
>>
>> this.internalResolver = true;
>>
>> }
>>
>> Paul.
>>
>> On Oct 13, 2010, at 6:38 AM, Imran M Yousuf wrote:
>>
>> On Tue, Oct 12, 2010 at 7:11 PM, Imran M Yousuf
>> <imyousuf_at_smartitengineering.com> wrote:
>>
>> On Tue, Oct 12, 2010 at 6:41 PM, Paul Sandoz
>> <Paul.Sandoz_at_oracle.com> wrote:
>>
>> <skip />
>>
>> Yes, we can do that. In fact in such cases the constructor could
>> just be:
>>
>> AbstractApacheHttpClientHandler(HttpClient client, ClientConfig
>> config)
>>
>> Do you want to send me another patch :-)
>>
>>
>> I would be honored to send out another patch, will do it first thing
>>
>> to tomorrow morning :).
>>
>>
>> Hi Paul,
>>
>> I went through the changes again and see that CacheableClientHandler
>> even can not extend a AbstractApacheHttpClientHandler as CCH depends
>> on HTTPCache4J resolver rather than HttpClient and creating CCH with
>> HttpClient means constructing CCH with our custom resolver for
>> HttpClient and nothing more, so I guess if the executor does not set
>> the configuration I will have live with having set it myself. So not
>> making further refactors to ApacheHttpClientHandler. Let me know what
>> you think.
>>
>> Regards,
>>
>> Imran
>>
>> Thank you,
>>
>> Imran
>>
>> Paul.
>>
>> ---------------------------------------------------------------------
>>
>> To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
>>
>> For additional commands, e-mail: users-help_at_jersey.dev.java.net
>>
>>
>>
>>
>>
>> --
>>
>> Imran M Yousuf
>>
>> Entrepreneur & CEO
>>
>> Smart IT Engineering Ltd.
>>
>> 25/5B, Block F, Haji Chinu Miah Road Bylane
>>
>> Joint Quarter, Mohammadpur
>>
>> Dhaka - 1207, Bangladesh
>>
>> Email: imran_at_smartitengineering.com
>>
>> Twitter: @imyousuf - http://twitter.com/imyousuf
>>
>> Blog: http://imyousuf-tech.blogs.smartitengineering.com/
>>
>> Mobile: +880-1711402557
>>
>>
>>
>>
>> --
>> Imran M Yousuf
>> Entrepreneur & CEO
>> Smart IT Engineering Ltd.
>> 25/5B, Block F, Haji Chinu Miah Road Bylane
>> Joint Quarter, Mohammadpur
>> Dhaka - 1207, Bangladesh
>> Email: imran_at_smartitengineering.com
>> Twitter: @imyousuf - http://twitter.com/imyousuf
>> Blog: http://imyousuf-tech.blogs.smartitengineering.com/
>> Mobile: +880-1711402557
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
>> For additional commands, e-mail: users-help_at_jersey.dev.java.net
>>
>>
>>
>
>
>
> --
> Imran M Yousuf
> Entrepreneur & CEO
> Smart IT Engineering Ltd.
> 25/5B, Block F, Haji Chinu Miah Road Bylane
> Joint Quarter, Mohammadpur
> Dhaka - 1207, Bangladesh
> Email: imran_at_smartitengineering.com
> Twitter: @imyousuf - http://twitter.com/imyousuf
> Blog: http://imyousuf-tech.blogs.smartitengineering.com/
> Mobile: +880-1711402557
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
> For additional commands, e-mail: users-help_at_jersey.dev.java.net
>