You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by li...@apache.org on 2009/11/08 07:26:24 UTC
svn commit: r833836 - in /incubator/shindig/trunk/java/gadgets/src:
main/java/org/apache/shindig/gadgets/http/
test/java/org/apache/shindig/gadgets/http/
Author: lindner
Date: Sun Nov 8 06:26:24 2009
New Revision: 833836
URL: http://svn.apache.org/viewvc?rev=833836&view=rev
Log:
SHINDIG-1218 | Support HTTP Retry-After header for failed requests
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/HttpResponse.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=833836&r1=833835&r2=833836&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 Nov 8 06:26:24 2009
@@ -95,8 +95,8 @@
if (request.getIgnoreCache()) {
return false;
}
- return !(!"GET".equals(request.getMethod()) &&
- !"GET".equals(request.getHeader("X-Method-Override")));
+ return ("GET".equals(request.getMethod()) ||
+ "GET".equals(request.getHeader("X-Method-Override")));
}
protected boolean isCacheable(HttpRequest request, HttpResponse response) {
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=833836&r1=833835&r2=833836&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 Nov 8 06:26:24 2009
@@ -26,6 +26,7 @@
import com.google.common.collect.Multimaps;
import com.google.inject.Inject;
import com.google.inject.name.Named;
+import org.apache.commons.lang.StringUtils;
import org.apache.shindig.common.util.DateUtil;
import org.apache.shindig.gadgets.encoding.EncodingDetector;
@@ -284,18 +285,37 @@
}
/**
+ * Calculate the Cache Expiration for this response.
+ *
+ *
+ * For errors (rc >=400) we intentionally ignore cache-control headers for most HTTP error responses, because if
+ * we don't we end up hammering sites that have gone down with lots of requests. Certain classes
+ * of client errors (authentication) have more severe behavioral implications if we cache them.
+ *
+ * For errors if the server provides a Retry-After header we use that.
+ *
+ * We technically shouldn't be caching certain 300 class status codes either, such as 302, but
+ * in practice this is a better option for performance.
+ *
* @return consolidated cache expiration time or -1
*/
public long getCacheExpiration() {
- // We intentionally ignore cache-control headers for most HTTP error responses, because if
- // we don't we end up hammering sites that have gone down with lots of requests. Certain classes
- // of client errors (authentication) have more severe behavioral implications if we cache them.
if (isError() && !NEGATIVE_CACHING_EXEMPT_STATUS.contains(httpStatusCode)) {
+ // If the server provides a Retry-After header use that as the cacheTtl
+ String retryAfter = this.getHeader("Retry-After");
+ if (retryAfter != null) {
+ if (StringUtils.isNumeric(retryAfter)) {
+ return date + Integer.parseInt(retryAfter);
+ } else {
+ Date expiresDate = DateUtil.parseRfc1123Date(retryAfter);
+ if (expiresDate != null)
+ return expiresDate.getTime();
+ }
+ }
+ // default value
return date + negativeCacheTtl;
}
- // We technically shouldn't be caching certain 300 class status codes either, such as 302, but
- // in practice this is a better option for performance.
if (isStrictNoCache()) {
return -1;
}
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=833836&r1=833835&r2=833836&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 Nov 8 06:26:24 2009
@@ -31,6 +31,7 @@
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.util.Arrays;
+import java.util.Date;
import static junitx.framework.Assert.assertEquals;
import static junitx.framework.Assert.assertFalse;
@@ -383,6 +384,22 @@
}
@Test
+ public void testRetryAfter() {
+ HttpResponse response;
+ String nowPlus60 = DateUtil.formatRfc1123Date(System.currentTimeMillis() + 60 * 1000L);
+ for (String date : Arrays.asList("60", nowPlus60)) {
+ for (int rc : Arrays.asList(HttpResponse.SC_INTERNAL_SERVER_ERROR, HttpResponse.SC_GATEWAY_TIMEOUT, HttpResponse.SC_BAD_REQUEST)) {
+ response = new HttpResponseBuilder()
+ .setHttpStatusCode(rc)
+ .setHeader("Retry-After","60")
+ .create();
+ long ttl = response.getCacheTtl();
+ assertTrue(ttl <= 60 * 1000L && ttl > 0);
+ }
+ }
+ }
+
+ @Test
public void testSetNoCache() {
int time = roundToSeconds(System.currentTimeMillis());
HttpResponse response = new HttpResponseBuilder()