svn commit: r1490276 - in /shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/http/ test/java/org/apache/shindig/gadgets/http/

[ssievers]

Author: ssievers
Date: Thu Jun  6 13:28:30 2013
New Revision: 1490276

URL: http://svn.apache.org/r1490276
Log:
SHINDIG-1920 | Don't force cache time-to-lives on responses with an error status

Modified:
    
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
    
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
    
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
    
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java

Modified: 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
URL: 
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java?rev=1490276&r1=1490275&r2=1490276&view=diff
==============================================================================
--- 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
 (original)
+++ 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
 Thu Jun  6 13:28:30 2013
@@ -117,7 +117,7 @@ public abstract class AbstractHttpCache 
       return null;
     }
     int forcedTtl = request.getCacheTtl();
-    if (forcedTtl != -1) {
+    if (forcedTtl != -1 && !response.isError()) {
       responseBuilder.setCacheTtl(forcedTtl);
     }
     response = responseBuilder.create();

Modified: 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
URL: 
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java?rev=1490276&r1=1490275&r2=1490276&view=diff
==============================================================================
--- 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
 (original)
+++ 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
 Thu Jun  6 13:28:30 2013
@@ -18,6 +18,7 @@
  */
 package org.apache.shindig.gadgets.http;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Charsets;
 import com.google.common.base.Supplier;
 import com.google.common.cache.CacheBuilder;
@@ -379,6 +380,11 @@ public final class HttpResponse implemen
     if (expiration != -1) {
       return expiration;
     }
+
+    if (isError()) {
+      return date + negativeCacheTtl;
+    }
+
     return date + defaultTtl;
   }
 
@@ -519,6 +525,11 @@ public final class HttpResponse implemen
     return defaultTtl;
   }
 
+  @VisibleForTesting
+  long getNegativeTtl() {
+    return negativeCacheTtl;
+  }
+
   /**
    * Attempts to determine the encoding of the body. If it can't be 
determined, we use
    * DEFAULT_ENCODING instead.

Modified: 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
URL: 
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java?rev=1490276&r1=1490275&r2=1490276&view=diff
==============================================================================
--- 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
 (original)
+++ 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
 Thu Jun  6 13:28:30 2013
@@ -430,6 +430,24 @@ public class AbstractHttpCacheTest {
   }
 
   @Test
+  public void addResponseWithForcedTtlAndErrorResponse() {
+    HttpRequest request = new HttpRequest(DEFAULT_URI).setCacheTtl(10);
+
+    String key = cache.createKey(request);
+    HttpResponse response = new HttpResponseBuilder()
+        .setResponseString("result")
+        .setHttpStatusCode(500)
+        .create();
+
+    assertNotNull(cache.addResponse(request, response));
+
+    assertNull(cache.map.get(key).getHeader("Cache-Control"));
+
+    assertNotNull(extendedStrictNoCacheTtlCache.addResponse(request, 
response));
+    
assertNull(extendedStrictNoCacheTtlCache.map.get(key).getHeader("Cache-Control"));
+  }
+
+  @Test
   public void addResponseWithNoCachingHeaders() {
     HttpRequest request = new HttpRequest(DEFAULT_URI);
 

Modified: 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
URL: 
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java?rev=1490276&r1=1490275&r2=1490276&view=diff
==============================================================================
--- 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
 (original)
+++ 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
 Thu Jun  6 13:28:30 2013
@@ -615,4 +615,17 @@ public class HttpResponseTest extends As
   public void testCacheExpirationForStrictNoCacheResponseWithoutOverride() 
throws Exception {
     assertEquals(-1, new 
HttpResponseBuilder().setStrictNoCache().create().getCacheExpiration());
   }
+
+  @Test
+  public void testCacheExpirationForNegativeCacheExemptNoCacheControl() throws 
Exception {
+    // Response should return a 401 or 403 (which are negative cache exempt) 
that don't have cache
+    // control headers. They should still be cached for the negative ttl.
+    HttpResponse response = new HttpResponseBuilder()
+                                  .setHttpStatusCode(HttpResponse.SC_FORBIDDEN)
+                                  .create();
+    assertTrue(
+            "Response is cached for the negative TTL",
+            response.getCacheExpiration() <= timeSource.currentTimeMillis()
+                    + response.getNegativeTtl());
+  }
 }