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/06/27 04:44:46 UTC

svn commit: r672095 - 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: Thu Jun 26 19:44:45 2008
New Revision: 672095

URL: http://svn.apache.org/viewvc?rev=672095&view=rev
Log:
Fixed incorrect handling of expiration time for HttpResponse objects. This was incorrectly returning the default TTL at all times.


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=672095&r1=672094&r2=672095&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 Thu Jun 26 19:44:45 2008
@@ -111,9 +111,11 @@
         }
       }
     }
-    // Force Date header.
-    tmpHeaders.put("Date",
-        Arrays.asList(DateUtil.formatDate(System.currentTimeMillis())));
+    // Force Last-Modified header -- caches should be sure to store this value.
+    if (tmpHeaders.get("Last-Modified") == null) {
+      tmpHeaders.put("Last-Modified",
+          Arrays.asList(DateUtil.formatDate(System.currentTimeMillis())));
+    }
     this.headers = tmpHeaders;
 
     this.metadata = new HashMap<String, String>();
@@ -287,7 +289,7 @@
    */
   public long getCacheExpiration() {
     if (httpStatusCode != SC_OK) {
-      return getDate() + NEGATIVE_CACHE_TTL;
+      return getLastModified() + NEGATIVE_CACHE_TTL;
     }
     if (isStrictNoCache()) {
       return -1;
@@ -300,42 +302,60 @@
     if (expiration != -1) {
       return expiration;
     }
-    return getDate() + DEFAULT_TTL;
+    return getLastModified() + DEFAULT_TTL;
   }
 
   /**
    * @return Consolidated ttl or -1.
    */
   public long getCacheTtl() {
-    if (httpStatusCode != SC_OK) {
-      return NEGATIVE_CACHE_TTL;
-    }
-    if (isStrictNoCache()) {
-      return -1;
-    }
-    long maxAge = getCacheControlMaxAge();
-    if (maxAge != -1) {
-      return maxAge;
-    }
-    long expiration = getExpiration();
+    long expiration = getCacheExpiration();
     if (expiration != -1) {
       return expiration - System.currentTimeMillis();
     }
-    return DEFAULT_TTL;
+    return -1;
   }
 
   /**
-   * @return The value of the HTTP Date header.
+   * @return true if a strict no-cache header is set in Cache-Control or Pragma
    */
-  public long getDate() {
-    String date = getHeader("Date");
+  public boolean isStrictNoCache() {
+    String cacheControl = getHeader("Cache-Control");
+    if (cacheControl != null) {
+      String[] directives = cacheControl.split(",");
+      for (String directive : directives) {
+        directive = directive.trim();
+        if (directive.equalsIgnoreCase("no-cache")
+            || directive.equalsIgnoreCase("no-store")
+            || directive.equalsIgnoreCase("private")) {
+          return true;
+        }
+      }
+    }
+
+    List<String> pragmas = getHeaders("Pragma");
+    if (pragmas != null) {
+      for (String pragma : pragmas) {
+        if ("no-cache".equalsIgnoreCase(pragma)) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
+  /**
+   * @return The value of the Last-Modified header.
+   */
+  protected long getLastModified() {
+    String date = getHeader("Last-Modified");
     return DateUtil.parseDate(date).getTime();
   }
 
   /**
    * @return the expiration time from the Expires header or -1 if not set
    */
-  public long getExpiration() {
+  protected long getExpiration() {
     String expires = getHeader("Expires");
     if (expires != null) {
       Date expiresDate = DateUtil.parseDate(expires);
@@ -349,7 +369,7 @@
   /**
    * @return max-age value or -1 if invalid or not set
    */
-  public long getCacheControlMaxAge() {
+  protected long getCacheControlMaxAge() {
     String cacheControl = getHeader("Cache-Control");
     if (cacheControl != null) {
       String[] directives = cacheControl.split(",");
@@ -369,32 +389,4 @@
     }
     return -1;
   }
-
-  /**
-   * @return true if a strict no-cache header is set in Cache-Control or Pragma
-   */
-  public boolean isStrictNoCache() {
-    String cacheControl = getHeader("Cache-Control");
-    if (cacheControl != null) {
-      String[] directives = cacheControl.split(",");
-      for (String directive : directives) {
-        directive = directive.trim();
-        if (directive.equalsIgnoreCase("no-cache")
-            || directive.equalsIgnoreCase("no-store")
-            || directive.equalsIgnoreCase("private")) {
-          return true;
-        }
-      }
-    }
-
-    List<String> pragmas = getHeaders("Pragma");
-    if (pragmas != null) {
-      for (String pragma : pragmas) {
-        if ("no-cache".equalsIgnoreCase(pragma)) {
-          return true;
-        }
-      }
-    }
-    return false;
-  }
 }

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=672095&r1=672094&r2=672095&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 Thu Jun 26 19:44:45 2008
@@ -172,12 +172,10 @@
     int expected = roundToSeconds(System.currentTimeMillis() + HttpResponse.DEFAULT_TTL);
     int expires = roundToSeconds(response.getCacheExpiration());
     assertEquals(expected, expires);
-    assertEquals(HttpResponse.DEFAULT_TTL, response.getCacheTtl());
+    assertTrue(response.getCacheTtl() <= HttpResponse.DEFAULT_TTL && response.getCacheTtl() > 0);
   }
 
   public void testExpires() throws Exception {
-    // We use a timestamp here so that we can verify that the underlying parser
-    // is robust. The /1000 * 1000 bit is just rounding to seconds.
     int ttl = 10;
     int time = roundToSeconds(System.currentTimeMillis()) + ttl;
     addHeader("Expires", DateUtil.formatDate(1000L * time));
@@ -197,6 +195,16 @@
     assertEquals(maxAge * 1000, response.getCacheTtl());
   }
 
+  public void testLastModified() throws Exception {
+    int time = roundToSeconds(System.currentTimeMillis());
+    addHeader("Last-Modified", DateUtil.formatDate(1000L * time));
+    HttpResponse response = new HttpResponse(200, null, headers);
+    assertEquals(time, roundToSeconds(response.getLastModified()));
+    // 9 because of rounding.
+    assertEquals(time + roundToSeconds(HttpResponse.DEFAULT_TTL),
+        roundToSeconds(response.getCacheExpiration()));
+  }
+
   public void testNegativeCaching() {
     assertTrue("Bad HTTP responses must be cacheable!",
         HttpResponse.error().getCacheExpiration() > System.currentTimeMillis());
@@ -204,7 +212,8 @@
         HttpResponse.notFound().getCacheExpiration() > System.currentTimeMillis());
     assertTrue("Bad HTTP responses must be cacheable!",
         HttpResponse.timeout().getCacheExpiration() > System.currentTimeMillis());
-    assertEquals(HttpResponse.NEGATIVE_CACHE_TTL, HttpResponse.error().getCacheTtl());
+    long ttl = HttpResponse.error().getCacheTtl();
+    assertTrue(ttl <= HttpResponse.DEFAULT_TTL && ttl > 0);
   }
 
   public void testNullHeaderNamesStripped() {