You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by jo...@apache.org on 2010/02/20 04:53:55 UTC

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

Author: johnh
Date: Sat Feb 20 03:53:54 2010
New Revision: 912073

URL: http://svn.apache.org/viewvc?rev=912073&view=rev
Log:
"If we have data for a resource, lets serve it even if it stale in cases
that the external resource is not available.
Only serve non error pages, and only for server error (>=500)"

Patch provided by Ziv Horesh.


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/DefaultRequestPipeline.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/DefaultRequestPipelineTest.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=912073&r1=912072&r2=912073&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 Sat Feb 20 03:53:54 2010
@@ -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.
@@ -228,13 +228,12 @@
   }
 
   /**
-   * Utility function to verify that an entry is cacheable and not expired
+   * Utility function to verify that an entry is usable
+   * The cache still serve staled data, it is the responsible of the user
+   * to decide if to use it or not (use isStale).
    * @return true If the response can be used.
    */
   protected boolean responseStillUsable(HttpResponse response) {
-    if (response == null) {
-      return false;
-    }
-    return response.getCacheExpiration() > clock.currentTimeMillis();
+    return response != null;
   }
 }

Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java?rev=912073&r1=912072&r2=912073&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java Sat Feb 20 03:53:54 2010
@@ -17,15 +17,15 @@
  */
 package org.apache.shindig.gadgets.http;
 
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
 import org.apache.shindig.common.util.Utf8UrlCoder;
 import org.apache.shindig.gadgets.GadgetException;
 import org.apache.shindig.gadgets.oauth.OAuthRequest;
 import org.apache.shindig.gadgets.rewrite.image.ImageRewriter;
 
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import com.google.inject.Singleton;
-
 /**
  * A standard implementation of a request pipeline. Performs request caching and
  * signing on top of standard HTTP requests.
@@ -54,6 +54,7 @@
   public HttpResponse execute(HttpRequest request) throws GadgetException {
     normalizeProtocol(request);
     HttpResponse invalidatedResponse = null;
+    HttpResponse staleResponse = null;
 
     if (!request.getIgnoreCache()) {
       HttpResponse cachedResponse = httpCache.getResponse(request);
@@ -66,6 +67,11 @@
           } else {
             invalidatedResponse = cachedResponse;
           }
+        } else {
+          if (!cachedResponse.isError()) {
+            // Remember good but stale cached response, to be served if server unavailable
+            staleResponse = cachedResponse;
+          }
         }
       }
     }
@@ -89,6 +95,12 @@
       return invalidatedResponse;
     }
 
+    if (fetchedResponse.getHttpStatusCode() >= 500 && staleResponse != null) {
+      // If we have trouble accessing the remote server,
+      // Lets try the latest good but staled result 
+      return staleResponse;
+    }
+    
     if (!fetchedResponse.isError() && !request.getIgnoreCache() && request.getCacheTtl() != 0) {
       fetchedResponse = imageRewriter.rewrite(request, fetchedResponse);
     }

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=912073&r1=912072&r2=912073&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 Sat Feb 20 03:53:54 2010
@@ -24,6 +24,9 @@
 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;
@@ -32,10 +35,6 @@
 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;
 
@@ -228,7 +227,7 @@
     HttpRequest request = new HttpRequest(DEFAULT_URI);
     String key = cache.createKey(request);
     HttpResponse response = new HttpResponseBuilder().setStrictNoCache().create();
-    cache.map.put(key, response);
+    cache.addResponse(request, response);
 
     assertNull("Did not return null when response was uncacheable", cache.getResponse(request));
   }
@@ -340,7 +339,7 @@
   }
 
   @Test
-  public void removeResponseIsNoLongerUsable() {
+  public void removeResponseIsStaled() {
     long expiration = System.currentTimeMillis() + 1000L;
     HttpRequest request = new HttpRequest(DEFAULT_URI);
     String key = cache.createKey(request);
@@ -353,8 +352,9 @@
 
     cache.setClock(fakeClock);
 
-    assertNull("Returned an expired entry when removing from the cache.",
-               cache.removeResponse(request));
+    // The cache itself still hold and return staled value, 
+    // caller responsible to decide what to do about it 
+    assertEquals(response, cache.removeResponse(request));
     assertEquals(0, cache.map.size());
   }
 

Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java?rev=912073&r1=912072&r2=912073&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java Sat Feb 20 03:53:54 2010
@@ -17,15 +17,16 @@
  */
 package org.apache.shindig.gadgets.http;
 
+import static org.junit.Assert.assertEquals;
+
+import com.google.common.collect.Maps;
+import com.google.inject.Provider;
+
 import org.apache.shindig.common.uri.Uri;
 import org.apache.shindig.gadgets.AuthType;
 import org.apache.shindig.gadgets.GadgetException;
 import org.apache.shindig.gadgets.oauth.OAuthRequest;
 import org.apache.shindig.gadgets.rewrite.image.NoOpImageRewriter;
-
-import com.google.common.collect.Maps;
-import com.google.inject.Provider;
-import static org.junit.Assert.assertEquals;
 import org.junit.Test;
 
 import java.util.Map;
@@ -95,6 +96,49 @@
   }
 
   @Test
+  public void authTypeNoneStaleCachedServed() throws Exception {
+    HttpRequest request = new HttpRequest(DEFAULT_URI)
+        .setAuthType(AuthType.NONE);
+
+    HttpResponse cached = new HttpResponseBuilder().setCacheTtl(-1).create();
+    cache.data.put(DEFAULT_URI, cached);
+
+    fetcher.response = HttpResponse.error();
+
+    HttpResponse response = pipeline.execute(request);
+
+    assertEquals(cached, response); // cached item is served instead of 500
+    assertEquals(request, fetcher.request);
+    assertEquals(1, cache.readCount);
+    assertEquals(0, cache.writeCount);
+    assertEquals(1, fetcher.fetchCount);
+  }
+
+  @Test
+  public void authTypeNoneWasCachedErrorStale() throws Exception {
+    HttpRequest request = new HttpRequest(DEFAULT_URI)
+        .setAuthType(AuthType.NONE);
+
+    HttpResponse cached = new HttpResponseBuilder()
+        .setCacheTtl(-1)
+        .setHttpStatusCode(401)
+        .create();
+    cache.data.put(DEFAULT_URI, cached);
+
+    HttpResponse fetched = HttpResponse.error();
+    fetcher.response = fetched;
+
+    HttpResponse response = pipeline.execute(request);
+
+    assertEquals(fetched, response); // 500 served because cached is an error (401)
+    assertEquals(request, fetcher.request);
+    assertEquals(fetched, cache.data.get(DEFAULT_URI));
+    assertEquals(1, cache.readCount);
+    assertEquals(1, cache.writeCount);
+    assertEquals(1, fetcher.fetchCount);
+  }
+
+  @Test
   public void authTypeNoneIgnoreCache() throws Exception {
     HttpRequest request = new HttpRequest(DEFAULT_URI)
         .setAuthType(AuthType.NONE)