Hmm, thats a valid suggestion. I will try making some time this
weekend to work on it :). You can expect a patch from me then :).
Thank you,
Imran
On Wed, Oct 13, 2010 at 4:33 PM, Paul Sandoz <Paul.Sandoz_at_oracle.com> wrote:
>
> 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
>>
>
>
> ---------------------------------------------------------------------
> 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