-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11584/
-----------------------------------------------------------
(Updated June 3, 2013, 8:29 p.m.)
Review request for shindig.
Changes
-------
Updating the patch to include a use case I missed the first time around. We
run into the new use case when one of the response's status code is in the
negative cache exempt list, e.g., is 401 or 403 AND the response has not cache
control headers. In this case we would have fallen through to the end of the
getCacheExpiration method and would have returned a the default TTL and not the
default negative TTL. This would have resulted in 401 and 403s being cached
for much longer than we want.
The updated patch fixes this and adds a unit test for this case.
Description
-------
org.apache.shindig.gadgets.DefaultGadgetSpecFactory and
org.apache.shindig.gadgets.DefaultMessageBundleFactory utilize a property in
shindig.properties (shindig.cache.xml.refreshInterval) to force the
RequestPipeline/Fetcher layer to cache specs and message bundles for a set
amount of time, regardless of the cache headers on the response. This also
forces the cache ttl in the case that the response is an error instead of using
the "shindig.cache.http.negativeCacheTtl" value in shindig.properties, which
defaults to one minute.
I'd rather we always use the negativeCacheTtl in this case and ignore the
forced cache ttl.
This patch addresses that. Do others think this is a good idea?
This addresses bug SHINDIG-1920.
https://issues.apache.org/jira/browse/SHINDIG-1920
Diffs (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
1485016
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
1485016
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
1485016
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
1485016
Diff: https://reviews.apache.org/r/11584/diff/
Testing
-------
I functionally tested this and also wrote a unit test. All existing unit tests
continue to pass.
Thanks,
Stanton Sievers
