Author: etnu
Date: Tue Jun 23 00:01:58 2009
New Revision: 787460
URL: http://svn.apache.org/viewvc?rev=787460&view=rev
Log:
Fixed a compliance issue with caching. REFRESH_INTERVAL should override *all*
HTTP response headers. Previously, it was not overriding responses that
completely disabled caching.
The order of caching precedence is now:
- Check nocache / ignore cache on request
- Check refresh interval on request
- Check response headers on response
- Check default expiration time
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseBuilderTest.java
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java?rev=787460&r1=787459&r2=787460&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
Tue Jun 23 00:01:58 2009
@@ -21,12 +21,12 @@
import static
org.apache.shindig.gadgets.rewrite.image.BasicImageRewriter.PARAM_RESIZE_QUALITY;
import static
org.apache.shindig.gadgets.rewrite.image.BasicImageRewriter.PARAM_RESIZE_WIDTH;
-import com.google.inject.Inject;
-
import org.apache.shindig.auth.SecurityToken;
import org.apache.shindig.common.util.TimeSource;
import org.apache.shindig.gadgets.AuthType;
+import com.google.inject.Inject;
+
/**
* Base class for content caches. Defines cache expiration rules and
* and restrictions on allowed content.
@@ -64,7 +64,7 @@
}
public boolean addResponse(HttpRequest request, HttpResponse response) {
- if (isCacheable(request) && isCacheable(response)) {
+ if (isCacheable(request, response)) {
// Both are cacheable. Check for forced cache TTL overrides.
HttpResponseBuilder responseBuilder = new HttpResponseBuilder(response);
int forcedTtl = request.getCacheTtl();
@@ -91,10 +91,6 @@
return null;
}
- protected boolean isCacheable(HttpResponse response) {
- return !response.isStrictNoCache();
- }
-
protected boolean isCacheable(HttpRequest request) {
if (request.getIgnoreCache()) {
return false;
@@ -103,6 +99,20 @@
!"GET".equals(request.getHeader("X-Method-Override")));
}
+ protected boolean isCacheable(HttpRequest request, HttpResponse response) {
+ if (!isCacheable(request)) {
+ return false;
+ }
+
+ if (request.getCacheTtl() != -1) {
+ // Caching was forced. Ignore what the response wants.
+ return true;
+ }
+
+ // If the HTTP response allows for it, we can cache.
+ return !response.isStrictNoCache();
+ }
+
/**
* Produce a key from the given request.
*
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java?rev=787460&r1=787459&r2=787460&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java
Tue Jun 23 00:01:58 2009
@@ -17,13 +17,15 @@
*/
package org.apache.shindig.gadgets.http;
+import org.apache.shindig.common.util.CharsetUtil;
+import org.apache.shindig.common.util.DateUtil;
+
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
+
import org.apache.commons.lang.ArrayUtils;
-import org.apache.shindig.common.util.CharsetUtil;
-import org.apache.shindig.common.util.DateUtil;
import java.util.Collection;
import java.util.List;
@@ -35,9 +37,9 @@
*/
public class HttpResponseBuilder {
private int httpStatusCode = HttpResponse.SC_OK;
- private Multimap<String, String> headers = HttpResponse.newHeaderMultimap();
+ private final Multimap<String, String> headers =
HttpResponse.newHeaderMultimap();
private byte[] responseBytes = ArrayUtils.EMPTY_BYTE_ARRAY;
- private Map<String, String> metadata = Maps.newHashMap();
+ private final Map<String, String> metadata = Maps.newHashMap();
public HttpResponseBuilder() {}
@@ -72,7 +74,7 @@
return this;
}
- /**
+ /**
* @param responseBytes The response body. Copied when set.
*/
public HttpResponseBuilder setResponse(byte[] responseBytes) {
@@ -149,7 +151,7 @@
public HttpResponseBuilder setCacheTtl(int cacheTtl) {
headers.removeAll("Pragma");
headers.removeAll("Expires");
- headers.put("Cache-Control", "public,max-age=" + cacheTtl);
+ headers.replaceValues("Cache-Control", ImmutableList.of("public,max-age="
+ cacheTtl));
return this;
}
@@ -167,7 +169,7 @@
/**
* Sets cache-control headers indicating the response is not cacheable.
*/
- private List<String> NO_CACHE_HEADER = ImmutableList.of("no-cache");
+ private final List<String> NO_CACHE_HEADER = ImmutableList.of("no-cache");
public HttpResponseBuilder setStrictNoCache() {
headers.replaceValues("Cache-Control", NO_CACHE_HEADER);
headers.replaceValues("Pragma", NO_CACHE_HEADER);
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java?rev=787460&r1=787459&r2=787460&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
Tue Jun 23 00:01:58 2009
@@ -24,9 +24,6 @@
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
-
import org.apache.shindig.auth.BasicSecurityToken;
import org.apache.shindig.auth.SecurityToken;
import org.apache.shindig.common.uri.Uri;
@@ -35,6 +32,10 @@
import org.apache.shindig.gadgets.AuthType;
import org.apache.shindig.gadgets.oauth.OAuthArguments;
import org.apache.shindig.gadgets.spec.RequestAuthenticationInfo;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+
import org.easymock.classextension.EasyMock;
import org.junit.Test;
@@ -300,6 +301,34 @@
}
@Test
+ public void addResponseWithForcedTtlAndStrictNoCache() {
+ HttpRequest request = new HttpRequest(DEFAULT_URI)
+ .setCacheTtl(10);
+
+ String key = cache.createKey(request);
+ HttpResponse response = new HttpResponseBuilder()
+ .setResponseString("result")
+ .setStrictNoCache()
+ .create();
+
+ assertTrue(cache.addResponse(request, response));
+
+ assertEquals("public,max-age=10",
cache.map.get(key).getHeader("Cache-Control"));
+ }
+
+ @Test
+ public void addResponseWithNoCachingHeaders() {
+ HttpRequest request = new HttpRequest(DEFAULT_URI);
+
+ String key = cache.createKey(request);
+ HttpResponse response = new HttpResponse("no headers");
+
+ assertTrue(cache.addResponse(request, response));
+
+ assertEquals("no headers", cache.map.get(key).getResponseAsString());
+ }
+
+ @Test
public void removeResponse() {
HttpRequest request = new HttpRequest(DEFAULT_URI);
String key = cache.createKey(request);
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseBuilderTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseBuilderTest.java?rev=787460&r1=787459&r2=787460&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseBuilderTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseBuilderTest.java
Tue Jun 23 00:01:58 2009
@@ -17,6 +17,10 @@
*/
package org.apache.shindig.gadgets.http;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -24,9 +28,6 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
import org.junit.Test;
import java.util.Arrays;
@@ -61,7 +62,7 @@
@Test
public void addHeadersMap() {
- Map<String, String> headers = ImmutableMap.of("foo", "bar", "blah",
"blah");
+ Map<String, String> headers = ImmutableMap.of("foo", "bar", "blah",
"blah");
HttpResponseBuilder builder = new HttpResponseBuilder()
.addHeaders(headers);
@@ -105,6 +106,7 @@
HttpResponseBuilder builder = new HttpResponseBuilder()
.addHeader("Pragma", "no-cache")
.addHeader("Expires", "some time stamp normally goes here")
+ .addHeader("Cache-Control", "no-cache")
.setCacheTtl(100);
Multimap<String, String> headers = builder.getHeaders();
