You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by et...@apache.org on 2008/10/19 05:17:43 UTC

svn commit: r705954 - in /incubator/shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/http/HttpResponse.java test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java

Author: etnu
Date: Sat Oct 18 20:17:42 2008
New Revision: 705954

URL: http://svn.apache.org/viewvc?rev=705954&view=rev
Log:
Added missing HTTP status codes and improved negative caching behavior.


Modified:
    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/HttpResponse.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java?rev=705954&r1=705953&r2=705954&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 Sat Oct 18 20:17:42 2008
@@ -17,6 +17,8 @@
  */
 package org.apache.shindig.gadgets.http;
 
+import org.apache.shindig.common.util.DateUtil;
+
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
@@ -25,8 +27,6 @@
 import com.ibm.icu.text.CharsetDetector;
 import com.ibm.icu.text.CharsetMatch;
 
-import org.apache.shindig.common.util.DateUtil;
-
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 import java.nio.ByteBuffer;
@@ -45,14 +45,50 @@
  * caches and by multiple threads without worrying about concurrent modification.
  */
 public final class HttpResponse {
+  public static final int SC_CONTINUE = 100;
+  public static final int SC_SWITCHING_PROTOCOLS = 101;
+
   public static final int SC_OK = 200;
-  public static final int SC_MOVED_TEMPORARILY = 301;
-  public static final int SC_MOVED_PERMANENTLY = 302;
+  public static final int SC_CREATED = 201;
+  public static final int SC_ACCEPTED = 202;
+  public static final int SC_NON_AUTHORITATIVE_INFORMATION = 203;
+  public static final int SC_NO_CONTENT = 204;
+  public static final int SC_RESET_CONTENT = 205;
+  public static final int SC_PARTIAL_CONTENT = 206;
+
+  public static final int SC_MULTIPLE_CHOICES = 300;
+  public static final int SC_MOVED_PERMANENTLY = 301;
+  public static final int SC_FOUND = 302;
+  public static final int SC_SEE_OTHER = 303;
+  public static final int SC_NOT_MODIFIED = 304;
+  public static final int SC_USE_PROXY = 305;
+  public static final int SC_TEMPORARY_REDIRECT = 307;
+
+  public static final int SC_BAD_REQUEST = 400;
   public static final int SC_UNAUTHORIZED = 401;
+  public static final int SC_PAYMENT_REQUIRED = 402;
   public static final int SC_FORBIDDEN = 403;
   public static final int SC_NOT_FOUND = 404;
+  public static final int SC_METHOD_NOT_ALLOWED = 405;
+  public static final int SC_NOT_ACCEPTABLE = 406;
+  public static final int SC_PROXY_AUTHENTICATION_REQUIRED = 407;
+  public static final int SC_REQUEST_TIMEOUT = 408;
+  public static final int SC_CONFLICT = 409;
+  public static final int SC_GONE = 410;
+  public static final int SC_LENGTH_REQUIRED = 411;
+  public static final int SC_PRECONDITION_FAILED = 412;
+  public static final int SC_REQUEST_ENTITY_TOO_LARGE = 413;
+  public static final int SC_REQUEST_URI_TOO_LONG = 414;
+  public static final int SC_UNSUPPORTED_MEDIA_TYPE = 415;
+  public static final int SC_REQUESTED_RANGE_NOT_SATISFIABLE = 416;
+  public static final int SC_EXPECTATION_FAILED = 417;
+
   public static final int SC_INTERNAL_SERVER_ERROR = 500;
-  public static final int SC_TIMEOUT = 504;
+  public static final int SC_NOT_IMPLEMENTED = 501;
+  public static final int SC_BAD_GATEWAY = 502;
+  public static final int SC_SERVICE_UNAVAILABLE = 503;
+  public static final int SC_GATEWAY_TIMEOUT = 504;
+  public static final int SC_HTTP_VERSION_NOT_SUPPORTED = 505;
 
   // These content types can always skip encoding detection.
   private static final Collection<String> BINARY_CONTENT_TYPES = Sets.newHashSet(
@@ -64,9 +100,9 @@
   );
 
   // These HTTP status codes should always honor the HTTP status returned by the remote host. All
-  // other status codes are treated as errors and will use the negativeCacheTtl value.
-  private static final Collection<Integer> CACHE_CONTROL_OK_STATUS_CODES
-      = Sets.newHashSet(SC_OK, SC_UNAUTHORIZED, SC_FORBIDDEN);
+  // other error codes are treated as errors and will use the negativeCacheTtl value.
+  private static final Collection<Integer> NEGATIVE_CACHING_EXEMPT_STATUS
+      = Sets.newHashSet(SC_UNAUTHORIZED, SC_FORBIDDEN);
 
   // 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.
@@ -131,7 +167,7 @@
   }
 
   public static HttpResponse timeout() {
-    return new HttpResponse(SC_TIMEOUT, "");
+    return new HttpResponse(SC_GATEWAY_TIMEOUT, "");
   }
 
   public static HttpResponse notFound() {
@@ -143,6 +179,13 @@
   }
 
   /**
+   * @return 400
+   */
+  public boolean isError() {
+    return httpStatusCode > 400;
+  }
+
+  /**
    * @return The encoding of the response body, if we're able to determine it.
    */
   public String getEncoding() {
@@ -232,12 +275,14 @@
    */
   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.  Proper
-    // support for caching of OAuth responses is more complex, for that we have to respect
-    // cache-control headers for 401s and 403s.
-    if (!CACHE_CONTROL_OK_STATUS_CODES.contains(httpStatusCode)) {
+    // 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)) {
       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;
     }
@@ -287,7 +332,7 @@
     }
     return false;
   }
-  
+
   /**
    * @return the expiration time from the Expires header or -1 if not set
    */

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=705954&r1=705953&r2=705954&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 Sat Oct 18 20:17:42 2008
@@ -17,9 +17,11 @@
  */
 package org.apache.shindig.gadgets.http;
 
+import org.apache.shindig.common.util.DateUtil;
+
 import junit.framework.TestCase;
+
 import org.apache.commons.io.IOUtils;
-import org.apache.shindig.common.util.DateUtil;
 
 import java.util.Arrays;
 
@@ -240,7 +242,7 @@
     int time = roundToSeconds(System.currentTimeMillis());
     HttpResponse response = new HttpResponseBuilder()
         .addHeader("Date", DateUtil.formatDate(1000L * time))
-        .create();    
+        .create();
     assertEquals(time + roundToSeconds(HttpResponse.DEFAULT_TTL),
         roundToSeconds(response.getCacheExpiration()));
     assertTtlOk(roundToSeconds(HttpResponse.DEFAULT_TTL), response);
@@ -275,14 +277,14 @@
     long ttl = response.getCacheTtl();
     assertTrue(ttl <= HttpResponse.DEFAULT_TTL && ttl > 0);
   }
-  
+
   public void testStrictNoCacheAndNegativeCaching() {
     assertDoesNotAllowNegativeCaching(HttpResponse.SC_UNAUTHORIZED);
     assertDoesNotAllowNegativeCaching(HttpResponse.SC_FORBIDDEN);
     assertDoesNotAllowNegativeCaching(HttpResponse.SC_OK);
     assertAllowsNegativeCaching(HttpResponse.SC_NOT_FOUND);
     assertAllowsNegativeCaching(HttpResponse.SC_INTERNAL_SERVER_ERROR);
-    assertAllowsNegativeCaching(HttpResponse.SC_TIMEOUT);
+    assertAllowsNegativeCaching(HttpResponse.SC_GATEWAY_TIMEOUT);
   }
 
   public void testSetNoCache() {



Re: svn commit: r705954 - in /incubator/shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/http/HttpResponse.java test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java

Posted by Kevin Brown <et...@google.com>.
Er, yeah, that should be >=

Re: svn commit: r705954 - in /incubator/shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/http/HttpResponse.java test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java

Posted by John Hjelmstad <fa...@google.com>.
On Sat, Oct 18, 2008 at 8:17 PM, <et...@apache.org> wrote:

> Author: etnu
> Date: Sat Oct 18 20:17:42 2008
> New Revision: 705954
>
> URL: http://svn.apache.org/viewvc?rev=705954&view=rev
> Log:
> Added missing HTTP status codes and improved negative caching behavior.
>
>
> Modified:
>
>  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/HttpResponse.java
> URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java?rev=705954&r1=705953&r2=705954&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
> Sat Oct 18 20:17:42 2008
> @@ -17,6 +17,8 @@
>  */
>  package org.apache.shindig.gadgets.http;
>
> +import org.apache.shindig.common.util.DateUtil;
> +
>  import com.google.common.collect.Lists;
>  import com.google.common.collect.Maps;
>  import com.google.common.collect.Sets;
> @@ -25,8 +27,6 @@
>  import com.ibm.icu.text.CharsetDetector;
>  import com.ibm.icu.text.CharsetMatch;
>
> -import org.apache.shindig.common.util.DateUtil;
> -
>  import java.io.ByteArrayInputStream;
>  import java.io.InputStream;
>  import java.nio.ByteBuffer;
> @@ -45,14 +45,50 @@
>  * caches and by multiple threads without worrying about concurrent
> modification.
>  */
>  public final class HttpResponse {
> +  public static final int SC_CONTINUE = 100;
> +  public static final int SC_SWITCHING_PROTOCOLS = 101;
> +
>   public static final int SC_OK = 200;
> -  public static final int SC_MOVED_TEMPORARILY = 301;
> -  public static final int SC_MOVED_PERMANENTLY = 302;
> +  public static final int SC_CREATED = 201;
> +  public static final int SC_ACCEPTED = 202;
> +  public static final int SC_NON_AUTHORITATIVE_INFORMATION = 203;
> +  public static final int SC_NO_CONTENT = 204;
> +  public static final int SC_RESET_CONTENT = 205;
> +  public static final int SC_PARTIAL_CONTENT = 206;
> +
> +  public static final int SC_MULTIPLE_CHOICES = 300;
> +  public static final int SC_MOVED_PERMANENTLY = 301;
> +  public static final int SC_FOUND = 302;
> +  public static final int SC_SEE_OTHER = 303;
> +  public static final int SC_NOT_MODIFIED = 304;
> +  public static final int SC_USE_PROXY = 305;
> +  public static final int SC_TEMPORARY_REDIRECT = 307;
> +
> +  public static final int SC_BAD_REQUEST = 400;
>   public static final int SC_UNAUTHORIZED = 401;
> +  public static final int SC_PAYMENT_REQUIRED = 402;
>   public static final int SC_FORBIDDEN = 403;
>   public static final int SC_NOT_FOUND = 404;
> +  public static final int SC_METHOD_NOT_ALLOWED = 405;
> +  public static final int SC_NOT_ACCEPTABLE = 406;
> +  public static final int SC_PROXY_AUTHENTICATION_REQUIRED = 407;
> +  public static final int SC_REQUEST_TIMEOUT = 408;
> +  public static final int SC_CONFLICT = 409;
> +  public static final int SC_GONE = 410;
> +  public static final int SC_LENGTH_REQUIRED = 411;
> +  public static final int SC_PRECONDITION_FAILED = 412;
> +  public static final int SC_REQUEST_ENTITY_TOO_LARGE = 413;
> +  public static final int SC_REQUEST_URI_TOO_LONG = 414;
> +  public static final int SC_UNSUPPORTED_MEDIA_TYPE = 415;
> +  public static final int SC_REQUESTED_RANGE_NOT_SATISFIABLE = 416;
> +  public static final int SC_EXPECTATION_FAILED = 417;
> +
>   public static final int SC_INTERNAL_SERVER_ERROR = 500;
> -  public static final int SC_TIMEOUT = 504;
> +  public static final int SC_NOT_IMPLEMENTED = 501;
> +  public static final int SC_BAD_GATEWAY = 502;
> +  public static final int SC_SERVICE_UNAVAILABLE = 503;
> +  public static final int SC_GATEWAY_TIMEOUT = 504;
> +  public static final int SC_HTTP_VERSION_NOT_SUPPORTED = 505;
>
>   // These content types can always skip encoding detection.
>   private static final Collection<String> BINARY_CONTENT_TYPES =
> Sets.newHashSet(
> @@ -64,9 +100,9 @@
>   );
>
>   // These HTTP status codes should always honor the HTTP status returned
> by the remote host. All
> -  // other status codes are treated as errors and will use the
> negativeCacheTtl value.
> -  private static final Collection<Integer> CACHE_CONTROL_OK_STATUS_CODES
> -      = Sets.newHashSet(SC_OK, SC_UNAUTHORIZED, SC_FORBIDDEN);
> +  // other error codes are treated as errors and will use the
> negativeCacheTtl value.
> +  private static final Collection<Integer> NEGATIVE_CACHING_EXEMPT_STATUS
> +      = Sets.newHashSet(SC_UNAUTHORIZED, SC_FORBIDDEN);
>
>   // 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.
> @@ -131,7 +167,7 @@
>   }
>
>   public static HttpResponse timeout() {
> -    return new HttpResponse(SC_TIMEOUT, "");
> +    return new HttpResponse(SC_GATEWAY_TIMEOUT, "");
>   }
>
>   public static HttpResponse notFound() {
> @@ -143,6 +179,13 @@
>   }
>
>   /**
> +   * @return 400
> +   */
> +  public boolean isError() {
> +    return httpStatusCode > 400;


SC_BAD_REQUEST (400) isn't an error?


>
> +  }
> +
> +  /**
>    * @return The encoding of the response body, if we're able to determine
> it.
>    */
>   public String getEncoding() {
> @@ -232,12 +275,14 @@
>    */
>   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.  Proper
> -    // support for caching of OAuth responses is more complex, for that we
> have to respect
> -    // cache-control headers for 401s and 403s.
> -    if (!CACHE_CONTROL_OK_STATUS_CODES.contains(httpStatusCode)) {
> +    // 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)) {
>       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;
>     }
> @@ -287,7 +332,7 @@
>     }
>     return false;
>   }
> -
> +
>   /**
>    * @return the expiration time from the Expires header or -1 if not set
>    */
>
> 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=705954&r1=705953&r2=705954&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
> Sat Oct 18 20:17:42 2008
> @@ -17,9 +17,11 @@
>  */
>  package org.apache.shindig.gadgets.http;
>
> +import org.apache.shindig.common.util.DateUtil;
> +
>  import junit.framework.TestCase;
> +
>  import org.apache.commons.io.IOUtils;
> -import org.apache.shindig.common.util.DateUtil;
>
>  import java.util.Arrays;
>
> @@ -240,7 +242,7 @@
>     int time = roundToSeconds(System.currentTimeMillis());
>     HttpResponse response = new HttpResponseBuilder()
>         .addHeader("Date", DateUtil.formatDate(1000L * time))
> -        .create();
> +        .create();
>     assertEquals(time + roundToSeconds(HttpResponse.DEFAULT_TTL),
>         roundToSeconds(response.getCacheExpiration()));
>     assertTtlOk(roundToSeconds(HttpResponse.DEFAULT_TTL), response);
> @@ -275,14 +277,14 @@
>     long ttl = response.getCacheTtl();
>     assertTrue(ttl <= HttpResponse.DEFAULT_TTL && ttl > 0);
>   }
> -
> +
>   public void testStrictNoCacheAndNegativeCaching() {
>     assertDoesNotAllowNegativeCaching(HttpResponse.SC_UNAUTHORIZED);
>     assertDoesNotAllowNegativeCaching(HttpResponse.SC_FORBIDDEN);
>     assertDoesNotAllowNegativeCaching(HttpResponse.SC_OK);
>     assertAllowsNegativeCaching(HttpResponse.SC_NOT_FOUND);
>     assertAllowsNegativeCaching(HttpResponse.SC_INTERNAL_SERVER_ERROR);
> -    assertAllowsNegativeCaching(HttpResponse.SC_TIMEOUT);
> +    assertAllowsNegativeCaching(HttpResponse.SC_GATEWAY_TIMEOUT);
>   }
>
>   public void testSetNoCache() {
>
>
>