Author: etnu
Date: Sun Jun 1 23:05:09 2008
New Revision: 662333
URL: http://svn.apache.org/viewvc?rev=662333&view=rev
Log:
Implemented basic negative caching for SHINDIG-278. This can probably be
cleaned up quite a bit, but that will happen in different patch.
This change is critical for ensuring that remote sites that return errors
aren't bombarded. This requires eliminating static error objects, instead
replacing them with static metods to produce error objects.
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/BasicHttpFetcher.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/BasicHttpCacheTest.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.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=662333&r1=662332&r2=662333&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
Sun Jun 1 23:05:09 2008
@@ -17,19 +17,12 @@
*/
package org.apache.shindig.gadgets.http;
-import org.apache.shindig.common.util.DateUtil;
-
import java.net.URI;
-import java.util.Arrays;
-import java.util.Date;
-import java.util.List;
/**
* Base class for content caches. Defines cache expiration rules and
* and restrictions on allowed content. Also enforces rewriting
* on cacheable content.
- *
- * TODO: Move cache checking code into HttpUtil
*/
public abstract class AbstractHttpCache implements HttpCache {
@@ -102,72 +95,12 @@
* @return content or null
*/
protected HttpResponse checkResponse(HttpResponse response) {
- if (response == null) return null;
-
- if (response.getHttpStatusCode() != 200) return null;
-
- long now = System.currentTimeMillis();
-
- String expires = response.getHeader("Expires");
- if (expires != null) {
- Date expiresDate = DateUtil.parseDate(expires);
- if (expiresDate == null) {
- // parse problem
- return null;
- }
- long expiresMs = expiresDate.getTime();
- if (expiresMs > now) {
- return response;
- } else {
- return null;
- }
+ if (response == null) {
+ return null;
}
-
- // Cache-Control headers may be an explicit max-age, or no-cache, which
- // means we use a default expiration time.
- String cacheControl = response.getHeader("Cache-Control");
- if (cacheControl != null) {
- String[] directives = cacheControl.split(",");
- for (String directive : directives) {
- directive = directive.trim();
- // boolean params
- if (directive.equals("no-cache")) {
- return null;
- }
- if (directive.startsWith("max-age")) {
- String[] parts = directive.split("=");
- if (parts.length == 2) {
- try {
- // Record the max-age and store it in the content as an
- // absolute expiration
- long maxAgeMs = Long.parseLong(parts[1]) * 1000;
- Date newExpiry = new Date(now + maxAgeMs);
- response.getAllHeaders()
- .put("Expires",
Arrays.asList(DateUtil.formatDate(newExpiry)));
- return response;
- } catch (NumberFormatException e) {
- return null;
- }
- }
- }
- }
+ if (response.getCacheExpiration() < System.currentTimeMillis()) {
+ return null;
}
-
- // Look for Pragma: no-cache. If present, return null.
- List<String> pragmas = response.getHeaders("Pragma");
- if (pragmas != null) {
- for (String pragma : pragmas) {
- if ("no-cache".equals(pragma)) {
- return null;
- }
- }
- }
-
- // Assume the content is cacheable for the default TTL
- // if no other directives exist
- Date newExpiry = new Date(now + getDefaultTTL());
- response.getAllHeaders()
- .put("Expires", Arrays.asList(DateUtil.formatDate(newExpiry)));
return response;
}
@@ -213,14 +146,4 @@
}
return false;
}
-
- /**
- * Default TTL for an entry in the cache that does not have any
- * cache controlling headers
- * @return default TTL for cache entries
- */
- protected long getDefaultTTL() {
- // 5 mins
- return 5L * 60L * 1000L;
- }
}
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java?rev=662333&r1=662332&r2=662333&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java
Sun Jun 1 23:05:09 2008
@@ -156,12 +156,12 @@
return cache.addResponse(request, response);
} catch (IOException e) {
if (e instanceof FileNotFoundException) {
- return HttpResponse.NOT_FOUND;
+ return HttpResponse.notFound();
} else if (e instanceof java.net.SocketTimeoutException ||
e instanceof java.net.SocketException) {
- return HttpResponse.TIMEOUT;
+ return HttpResponse.timeout();
}
- return HttpResponse.ERROR;
+ return HttpResponse.error();
}
}
}
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java?rev=662333&r1=662332&r2=662333&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
Sun Jun 1 23:05:09 2008
@@ -24,6 +24,7 @@
import java.nio.ByteBuffer;
import java.nio.charset.Charset;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
@@ -45,10 +46,15 @@
private static final String DEFAULT_ENCODING = "UTF-8";
private final String encoding;
- public static final HttpResponse ERROR
- = new HttpResponse(SC_INTERNAL_SERVER_ERROR);
- public static final HttpResponse TIMEOUT = new HttpResponse(SC_TIMEOUT);
- public static final HttpResponse NOT_FOUND = new HttpResponse(SC_NOT_FOUND);
+ // TTL to use when an error response is fetched. This should be non-zero to
+ // avoid high rates of requests to bad urls in high-traffic situations.
+ private final static long NEGATIVE_CACHE_TTL = 30 * 1000;
+
+ /**
+ * Default TTL for an entry in the cache that does not have any
+ * cache controlling headers.
+ */
+ private static final long DEFAULT_TTL = 5L * 60L * 1000L;
// Used to lazily convert to a string representation of the input.
private String responseString = null;
@@ -59,17 +65,14 @@
private HttpResponse rewritten;
// Holds character sets for fast conversion
- private static ConcurrentHashMap<String,Charset> encodingToCharset= new
ConcurrentHashMap<String,Charset>();
+ private static ConcurrentHashMap<String,Charset> encodingToCharset
+ = new ConcurrentHashMap<String,Charset>();
/**
* Create a dummy empty map. Access via HttpResponse.ERROR
*/
- private HttpResponse(int statusCode) {
- this.httpStatusCode = statusCode;
- this.responseBytes = new byte[0];
- this.encoding = DEFAULT_ENCODING;
- this.headers = Collections.emptyMap();
- this.metadata = new HashMap<String, String>();
+ public HttpResponse(int statusCode) {
+ this(statusCode, new byte[0], null);
}
/**
@@ -88,17 +91,18 @@
responseBytes, 0, this.responseBytes, 0, responseBytes.length);
}
- if (headers == null) {
- this.headers = Collections.emptyMap();
- } else {
- Map<String, List<String>> tmpHeaders
- = new HashMap<String, List<String>>();
+ Map<String, List<String>> tmpHeaders = new HashMap<String, List<String>>();
+ if (headers != null) {
for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
List<String> newList = new ArrayList<String>(entry.getValue());
tmpHeaders.put(entry.getKey(), Collections.unmodifiableList(newList));
}
- this.headers = tmpHeaders;
}
+ // Force Date header.
+ tmpHeaders.put("Date",
+ Arrays.asList(DateUtil.formatDate(System.currentTimeMillis())));
+ this.headers = tmpHeaders;
+
this.metadata = new HashMap<String, String>();
this.encoding = detectEncoding();
}
@@ -113,6 +117,18 @@
this(SC_OK, body.getBytes(), null);
}
+ public static HttpResponse error() {
+ return new HttpResponse(SC_INTERNAL_SERVER_ERROR);
+ }
+
+ public static HttpResponse timeout() {
+ return new HttpResponse(SC_TIMEOUT);
+ }
+
+ public static HttpResponse notFound() {
+ return new HttpResponse(SC_NOT_FOUND);
+ }
+
/**
* Attempts to determine the encoding of the body. If it can't be determined,
* we use DEFAULT_ENCODING instead.
@@ -249,13 +265,32 @@
* @return consolidated cache expiration time or -1
*/
public long getCacheExpiration() {
- if (isStrictNoCache()) return -1;
- long maxAgeExpiration = getCacheControlMaxAge() +
System.currentTimeMillis();
+ if (httpStatusCode != SC_OK) {
+ return getDate() + NEGATIVE_CACHE_TTL;
+ }
+ if (isStrictNoCache()) {
+ return -1;
+ }
+ long maxAge = getCacheControlMaxAge();
+ if (maxAge != -1) {
+ return maxAge + System.currentTimeMillis();
+ }
long expiration = getExpiration();
- if (expiration == -1) {
- return maxAgeExpiration;
+ if (expiration != -1) {
+ return expiration;
+ }
+ return getDate() + DEFAULT_TTL;
+ }
+
+ /**
+ * @return The value of the HTTP Date header.
+ */
+ public long getDate() {
+ String date = getHeader("Date");
+ if (date == null) {
+ return -1;
}
- return expiration;
+ return DateUtil.parseDate(date).getTime();
}
/**
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/BasicHttpCacheTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/BasicHttpCacheTest.java?rev=662333&r1=662332&r2=662333&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/BasicHttpCacheTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/BasicHttpCacheTest.java
Sun Jun 1 23:05:09 2008
@@ -91,11 +91,11 @@
assertNull(cache.getResponse(req));
}
- public void testNotCacheableForErr() {
+ public void testCacheableForErr() {
HttpRequest req = createRequest("GET");
HttpResponse resp = createResponse(500, null, null);
cache.addResponse(req, resp);
- assertNull(cache.getResponse(req));
+ assertEquals(resp, cache.getResponse(req));
}
public void testCacheableForFutureExpires() {
@@ -114,14 +114,6 @@
assertNull(cache.getResponse(req));
}
- public void testNotCacheableForFutureExpiresWithError() {
- HttpRequest req = createRequest("GET");
- HttpResponse resp = createExpiresResponse(500,
- System.currentTimeMillis() - 10000L);
- cache.addResponse(req, resp);
- assertNull(cache.getResponse(req));
- }
-
public void testCacheableForFutureMaxAge() {
HttpRequest req = createRequest("GET");
HttpResponse resp = createMaxAgeResponse(200,
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java?rev=662333&r1=662332&r2=662333&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
Sun Jun 1 23:05:09 2008
@@ -17,6 +17,8 @@
*/
package org.apache.shindig.gadgets.http;
+import org.apache.shindig.common.util.DateUtil;
+
import org.apache.commons.io.IOUtils;
import junit.framework.TestCase;
@@ -109,13 +111,21 @@
assertFalse(response.isStrictNoCache());
}
- /*
- There seems to be some issue with the date parsing in joda?
public void testExpires() throws Exception {
- long time = System.currentTimeMillis() + 10000L;
- addHeader("Expires", HttpUtil.formatDate(new Date(time)));
+ // We use a timestamp here so that we can verify that the underlying parser
+ // is robust. The /1000 * 1000 bit is just rounding to seconds.
+ long time = ((System.currentTimeMillis() / 1000) * 1000) + 10000L;
+ addHeader("Expires", DateUtil.formatDate(time));
HttpResponse response = new HttpResponse(200, new byte[0], headers);
- assertEquals(response.getExpiration(), time);
+ assertEquals(time, response.getExpiration());
+ }
+
+ public void testNegativeCaching() {
+ assertTrue("Bad HTTP responses must be cacheable!",
+ HttpResponse.error().getCacheExpiration() >
System.currentTimeMillis());
+ assertTrue("Bad HTTP responses must be cacheable!",
+ HttpResponse.notFound().getCacheExpiration() >
System.currentTimeMillis());
+ assertTrue("Bad HTTP responses must be cacheable!",
+ HttpResponse.timeout().getCacheExpiration() >
System.currentTimeMillis());
}
- */
}
