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()


Reply via email to