You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Jonathan Moore (JIRA)" <ji...@apache.org> on 2010/09/11 23:19:33 UTC

[jira] Created: (HTTPCLIENT-994) cache does not allow client to override origin-specified freshness using max-stale

cache does not allow client to override origin-specified freshness using max-stale
----------------------------------------------------------------------------------

                 Key: HTTPCLIENT-994
                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-994
             Project: HttpComponents HttpClient
          Issue Type: Improvement
          Components: Cache
    Affects Versions: 4.1 Alpha2
            Reporter: Jonathan Moore


According to the RFC, the default freshness lifetime is supposed to be the LEAST restrictive of that specified by the origin, the client, and the cache. Right now, a client can't use 'max-stale' to relax the freshness constraints to get a cache hit without validation occuring first.


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (HTTPCLIENT-994) cache does not allow client to override origin-specified freshness using max-stale

Posted by "Jonathan Moore (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12908738#action_12908738 ] 

Jonathan Moore commented on HTTPCLIENT-994:
-------------------------------------------

I would definitely weigh your opinion and experience highly here.

Let me roll another patch that just introduces the max-stale fix without the refactoring, and then I'll propose that we start a dev list discussion about future evolution of the module in general. 

> cache does not allow client to override origin-specified freshness using max-stale
> ----------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-994
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-994
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: Cache
>    Affects Versions: 4.1 Alpha2
>            Reporter: Jonathan Moore
>         Attachments: max-stale.patch
>
>
> According to the RFC, the default freshness lifetime is supposed to be the LEAST restrictive of that specified by the origin, the client, and the cache. Right now, a client can't use 'max-stale' to relax the freshness constraints to get a cache hit without validation occuring first.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (HTTPCLIENT-994) cache does not allow client to override origin-specified freshness using max-stale

Posted by "Jonathan Moore (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCLIENT-994?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jonathan Moore updated HTTPCLIENT-994:
--------------------------------------

    Attachment: max-stale.patch

The attached patch addresses this issue, as well as a simplifying refactoring. I even fixed another bug while I was at it.

The refactoring includes:
* Removal of the CacheValidityPolicy and associated unit test; almost all of this functionality was moved to the HttpCacheEntry; created a new TestHttpCacheEntry unit test.
* Removal of the CacheEntry test class; instead added some convenience methods to HttpTestUtils to allow for easily manufacturing proper HttpCacheEntry instances instead.

The bug fix was a minor one related to the requirement that a cache cannot respond with a cache entry to a request with multiple conditional headers where not all of them match; it turned out the test case for this was incorrect (it was using If-Unmodified-Since instead of If-Modified-Since in the test) and hence the bug wasn't immediately obvious.

This patch is contributed to the ASF with the permission of my employer.

> cache does not allow client to override origin-specified freshness using max-stale
> ----------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-994
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-994
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: Cache
>    Affects Versions: 4.1 Alpha2
>            Reporter: Jonathan Moore
>         Attachments: max-stale.patch
>
>
> According to the RFC, the default freshness lifetime is supposed to be the LEAST restrictive of that specified by the origin, the client, and the cache. Right now, a client can't use 'max-stale' to relax the freshness constraints to get a cache hit without validation occuring first.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (HTTPCLIENT-994) cache does not allow client to override origin-specified freshness using max-stale

Posted by "Jonathan Moore (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12908446#action_12908446 ] 

Jonathan Moore commented on HTTPCLIENT-994:
-------------------------------------------

Hi Oleg,

Good question. Currently the CachedResponseSuitabilityChecker is where we decide whether a specific cache entry can be used to satisfy a given request. This is the class that could be replaced or extended to provide alternative (but allowed) caching policies, like heuristic caching, for example.  

Please take a look, but I am pretty certain that all the methods on HttpCacheEntry now are either calculations whose definitions are given in the RFC (apparent age, freshness lifetime, whether it can be revalidated, etc.) or related to MUST-level requirements (treating malformed Expires headers as 'already stale'). 

Also, CacheValidityPolicy, where all these methods used to live, had almost no interesting state, and all it's methods took an HttpCacheEntry as an argument; it seemed pretty clear these two classes "wanted" to be merged. :)

Jon

> cache does not allow client to override origin-specified freshness using max-stale
> ----------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-994
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-994
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: Cache
>    Affects Versions: 4.1 Alpha2
>            Reporter: Jonathan Moore
>         Attachments: max-stale.patch
>
>
> According to the RFC, the default freshness lifetime is supposed to be the LEAST restrictive of that specified by the origin, the client, and the cache. Right now, a client can't use 'max-stale' to relax the freshness constraints to get a cache hit without validation occuring first.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (HTTPCLIENT-994) cache does not allow client to override origin-specified freshness using max-stale

Posted by "Jonathan Moore (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12908482#action_12908482 ] 

Jonathan Moore commented on HTTPCLIENT-994:
-------------------------------------------

I can understand keeping state and protocol processing decoupled, which is why we have a CachedResponseSuitabilityChecker instead of just saying HttpCacheEntry#canYouBeUsedToSatisfy(HttpRequest req). In this case, the methods that got moved to HttpCacheEntry are all essentially properties of a cache entry; just as a Header can find its HeaderElements for you, now an HttpCacheEntry can tell you its apparent age (a *defined* property in the RFC). Whether the caching module actually decides to use the cache entry to satisfy a request is outside of the scope of the HttpCacheEntry.

Generally, I think the main advantage here is a simplification of the implementation: one fewer class to maintain or understand. In particular, an HttpCacheEntry is a domain object, whereas a CacheValidityPolicy is a construct of the implementation. In this case, there is little future flexibility lost here, since all the calculations are explicitly defined by the RFC, and we still have a place to make implementation-specific behavioral changes (CachedResponseSuitabilityChecker); we don't need two such places.

Finally, I think the CacheValidityPolicy and HttpCacheEntry as they exist in trunk now exhibit a few OO "code smells", namely that they are two separate classes that are tightly coupled; there isn't anything you can do with a CacheValidityPolicy without also having an HttpCacheEntry instance. Also, the CacheValidityPolicy's state is pretty much just a single boolean; there's not much state there, just code, which suggests that the code probably wants to be moved towards the state it operates on (HttpCacheEntry).

But generally speaking, I think this is as simple as "why have two classes when one will do"?



> cache does not allow client to override origin-specified freshness using max-stale
> ----------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-994
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-994
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: Cache
>    Affects Versions: 4.1 Alpha2
>            Reporter: Jonathan Moore
>         Attachments: max-stale.patch
>
>
> According to the RFC, the default freshness lifetime is supposed to be the LEAST restrictive of that specified by the origin, the client, and the cache. Right now, a client can't use 'max-stale' to relax the freshness constraints to get a cache hit without validation occuring first.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (HTTPCLIENT-994) cache does not allow client to override origin-specified freshness using max-stale

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12908449#action_12908449 ] 

Oleg Kalnichevski commented on HTTPCLIENT-994:
----------------------------------------------

I am a strong believer in keeping state and behaviour / protocol processing decoupled, even if they seem to be closely related. I will not insist, though. The question still stands whether or not there is anything to be gained from merging those classes.

Oleg

> cache does not allow client to override origin-specified freshness using max-stale
> ----------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-994
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-994
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: Cache
>    Affects Versions: 4.1 Alpha2
>            Reporter: Jonathan Moore
>         Attachments: max-stale.patch
>
>
> According to the RFC, the default freshness lifetime is supposed to be the LEAST restrictive of that specified by the origin, the client, and the cache. Right now, a client can't use 'max-stale' to relax the freshness constraints to get a cache hit without validation occuring first.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (HTTPCLIENT-994) cache does not allow client to override origin-specified freshness using max-stale

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12908421#action_12908421 ] 

Oleg Kalnichevski commented on HTTPCLIENT-994:
----------------------------------------------

Jon

What is the benefit of moving protocol specific logic back to the HttpCacheEntry? Presently HttpCacheEntry is just a simple bean class that carries state information about a cache entry. One can apply different strategies to determine whether or not the entry is fresh. What do we have to gain by mixing state and protocol logic in one class?

Oleg

> cache does not allow client to override origin-specified freshness using max-stale
> ----------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-994
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-994
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: Cache
>    Affects Versions: 4.1 Alpha2
>            Reporter: Jonathan Moore
>         Attachments: max-stale.patch
>
>
> According to the RFC, the default freshness lifetime is supposed to be the LEAST restrictive of that specified by the origin, the client, and the cache. Right now, a client can't use 'max-stale' to relax the freshness constraints to get a cache hit without validation occuring first.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (HTTPCLIENT-994) cache does not allow client to override origin-specified freshness using max-stale

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12908671#action_12908671 ] 

Oleg Kalnichevski commented on HTTPCLIENT-994:
----------------------------------------------

> In this case, the methods that got moved to HttpCacheEntry are all essentially properties of a cache entry; 
> just as a Header can find its HeaderElements for you, now an HttpCacheEntry can tell you its apparent age 
> (a *defined* property in the RFC)

There would be too way much protocol logic beyond simple parsing operations in the HttpCacheEntry to my personal liking. RFCs do get superseded with updated versions. You never know what might happen in the future. If HttpCacheEntry API proves inflexible to meet new requirements, pretty much the entire caching API will have to be deprecated. 

> Finally, I think the CacheValidityPolicy and HttpCacheEntry as they exist in trunk now exhibit a few OO "code smells", 
> namely that they are two separate classes that are tightly coupled.

I am not sure I agree. HttpCacheEntry has no dependency on CacheValidityPolicy of what so ever.

> But generally speaking, I think this is as simple as "why have two classes when one will do"? 

Because you never know what might happen in the future. I have been maintaining HttpClient for looooong 7 years and I have had numerous moments when I wished some bits of code had never been made a part of public API.

If you remain unconvinced I'll commit the patch within a few days.

Oleg

> cache does not allow client to override origin-specified freshness using max-stale
> ----------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-994
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-994
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: Cache
>    Affects Versions: 4.1 Alpha2
>            Reporter: Jonathan Moore
>         Attachments: max-stale.patch
>
>
> According to the RFC, the default freshness lifetime is supposed to be the LEAST restrictive of that specified by the origin, the client, and the cache. Right now, a client can't use 'max-stale' to relax the freshness constraints to get a cache hit without validation occuring first.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (HTTPCLIENT-994) cache does not allow client to override origin-specified freshness using max-stale

Posted by "Jonathan Moore (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCLIENT-994?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jonathan Moore updated HTTPCLIENT-994:
--------------------------------------

    Attachment: max-stale-no-refactor.patch

I have attached another patch that addresses the issue without the major refactoring of HttpCacheEntry. I did, however, go ahead and remove the CacheEntry test class here, and several of the unit tests I touched while fixing the bug are now no longer mock tests, which makes them a lot less brittle.

This patch also fixes a bug in the processing of min-fresh (semantics were not being applied correctly).

This patch is contributed to the ASF with the permission of my employer.


> cache does not allow client to override origin-specified freshness using max-stale
> ----------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-994
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-994
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: Cache
>    Affects Versions: 4.1 Alpha2
>            Reporter: Jonathan Moore
>         Attachments: max-stale-no-refactor.patch, max-stale.patch
>
>
> According to the RFC, the default freshness lifetime is supposed to be the LEAST restrictive of that specified by the origin, the client, and the cache. Right now, a client can't use 'max-stale' to relax the freshness constraints to get a cache hit without validation occuring first.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Resolved: (HTTPCLIENT-994) cache does not allow client to override origin-specified freshness using max-stale

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCLIENT-994?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski resolved HTTPCLIENT-994.
------------------------------------------

    Fix Version/s: 4.1 Alpha3
       Resolution: Fixed

Patch checked in. Many thanks, Jon

Oleg

> cache does not allow client to override origin-specified freshness using max-stale
> ----------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-994
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-994
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: Cache
>    Affects Versions: 4.1 Alpha2
>            Reporter: Jonathan Moore
>             Fix For: 4.1 Alpha3
>
>         Attachments: max-stale-no-refactor.patch, max-stale.patch
>
>
> According to the RFC, the default freshness lifetime is supposed to be the LEAST restrictive of that specified by the origin, the client, and the cache. Right now, a client can't use 'max-stale' to relax the freshness constraints to get a cache hit without validation occuring first.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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