Author: etnu
Date: Fri Jun 20 17:38:03 2008
New Revision: 670105
URL: http://svn.apache.org/viewvc?rev=670105&view=rev
Log:
Fixed a bug with cache control header output in ProxyBase. It was setting the
expiration time as the ttl.
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.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=670105&r1=670104&r2=670105&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
Fri Jun 20 17:38:03 2008
@@ -51,19 +51,20 @@
"image/jpeg", "image/png", "image/gif", "image/jpg",
"application/x-shockwave-flash"
));
- private final int httpStatusCode;
- private static final String DEFAULT_ENCODING = "UTF-8";
- private final String encoding;
-
// 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.
- private final static long NEGATIVE_CACHE_TTL = 30 * 1000;
+ public final static long NEGATIVE_CACHE_TTL = 30 * 1000;
/**
* Default TTL for an entry in the cache that does not have any
* cache controlling headers.
*/
- private static final long DEFAULT_TTL = 5L * 60L * 1000L;
+ public static final long DEFAULT_TTL = 5L * 60L * 1000L;
+
+ public static final String DEFAULT_ENCODING = "UTF-8";
+
+ private final int httpStatusCode;
+ private final String encoding;
// Used to lazily convert to a string representation of the input.
private String responseString = null;
@@ -303,13 +304,31 @@
}
/**
+ * @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();
+ if (expiration != -1) {
+ return expiration - System.currentTimeMillis();
+ }
+ return DEFAULT_TTL;
+ }
+
+ /**
* @return The value of the HTTP Date header.
*/
public long getDate() {
String date = getHeader("Date");
- if (date == null) {
- return -1;
- }
return DateUtil.parseDate(date).getTime();
}
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java?rev=670105&r1=670104&r2=670105&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
Fri Jun 20 17:38:03 2008
@@ -97,7 +97,7 @@
} else if (request.getParameter(REFRESH_PARAM) != null) {
refreshInterval = Integer.valueOf(request.getParameter(REFRESH_PARAM));
} else {
- refreshInterval = Math.max(60 * 60, (int)(results.getExpiration() /
1000));
+ refreshInterval = Math.max(60 * 60, (int)(results.getCacheTtl() /
1000L));
}
HttpUtil.setCachingHeaders(response, refreshInterval);
response.setHeader("Content-Disposition", "attachment;filename=p.txt");
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=670105&r1=670104&r2=670105&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
Fri Jun 20 17:38:03 2008
@@ -66,9 +66,13 @@
existing.add(value);
}
+ private int roundToSeconds(long ts) {
+ return (int)(ts / 1000);
+ }
+
public void testGetEncoding() throws Exception {
addHeader("Content-Type", "text/plain; charset=TEST-CHARACTER-SET");
- HttpResponse response = new HttpResponse(200, new byte[0], headers);
+ HttpResponse response = new HttpResponse(200, null, headers);
assertEquals("TEST-CHARACTER-SET", response.getEncoding());
}
@@ -136,47 +140,61 @@
};
addHeader("Content-Type", "application/octet-stream");
HttpResponse response = new HttpResponse(200, data, headers);
+
byte[] out = IOUtils.toByteArray(response.getResponse());
+ assertEquals(data.length, response.getContentLength());
+ assertTrue(Arrays.equals(data, out));
+
+ out = response.getResponseAsBytes();
assertTrue(Arrays.equals(data, out));
}
public void testStrictCacheControlNoCache() throws Exception {
addHeader("Cache-Control", "no-cache");
- HttpResponse response = new HttpResponse(200, new byte[0], headers);
+ HttpResponse response = new HttpResponse(200, null, headers);
assertTrue(response.isStrictNoCache());
assertEquals(-1, response.getCacheExpiration());
+ assertEquals(-1, response.getCacheTtl());
}
public void testStrictPragmaNoCache() throws Exception {
addHeader("Pragma", "no-cache");
- HttpResponse response = new HttpResponse(200, new byte[0], headers);
+ HttpResponse response = new HttpResponse(200, null, headers);
assertTrue(response.isStrictNoCache());
assertEquals(-1, response.getCacheExpiration());
+ assertEquals(-1, response.getCacheTtl());
}
public void testStrictPragmaJunk() throws Exception {
addHeader("Pragma", "junk");
- HttpResponse response = new HttpResponse(200, new byte[0], headers);
+ HttpResponse response = new HttpResponse(200, null, headers);
assertFalse(response.isStrictNoCache());
+ int expected = roundToSeconds(System.currentTimeMillis() +
HttpResponse.DEFAULT_TTL);
+ int expires = roundToSeconds(response.getCacheExpiration());
+ assertEquals(expected, expires);
+ assertEquals(HttpResponse.DEFAULT_TTL, response.getCacheTtl());
}
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.
- long time = ((System.currentTimeMillis() / 1000) * 1000) + 10000L;
- addHeader("Expires", DateUtil.formatDate(time));
- HttpResponse response = new HttpResponse(200, new byte[0], headers);
- assertEquals(time, response.getCacheExpiration());
+ int ttl = 10;
+ int time = roundToSeconds(System.currentTimeMillis()) + ttl;
+ addHeader("Expires", DateUtil.formatDate(1000L * time));
+ HttpResponse response = new HttpResponse(200, null, headers);
+ assertEquals(time, roundToSeconds(response.getCacheExpiration()));
+ // 9 because of rounding.
+ assertEquals(9, roundToSeconds(response.getCacheTtl()));
}
public void testMaxAge() throws Exception {
int maxAge = 10;
- long expected = ((System.currentTimeMillis() / 1000) * 1000) + (maxAge *
1000);
+ int expected = roundToSeconds(System.currentTimeMillis()) + maxAge;
addHeader("Cache-Control", "public, max-age=" + maxAge);
- HttpResponse response = new HttpResponse(200, new byte[0], headers);
- long expiration = response.getCacheExpiration();
- assertTrue("getExpiration is less than start time + maxAge", expiration >=
expected);
- assertTrue("getExpiration is too high.", expiration <= expected + 1000);
+ HttpResponse response = new HttpResponse(200, null, headers);
+ int expiration = roundToSeconds(response.getCacheExpiration());
+ assertEquals(expected, expiration);
+ assertEquals(maxAge * 1000, response.getCacheTtl());
}
public void testNegativeCaching() {
@@ -186,11 +204,12 @@
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());
}
public void testNullHeaderNamesStripped() {
addHeader(null, "dummy");
- HttpResponse response = new HttpResponse(200, new byte[0], headers);
+ HttpResponse response = new HttpResponse(200, null, headers);
for (String key : response.getAllHeaders().keySet()) {
assertNotNull("Null header not removed.", key);
}
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java?rev=670105&r1=670104&r2=670105&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
Fri Jun 20 17:38:03 2008
@@ -18,7 +18,6 @@
*/
package org.apache.shindig.gadgets.servlet;
-import static junitx.framework.StringAssert.assertContains;
import static org.easymock.EasyMock.expect;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
@@ -151,15 +150,14 @@
@Test
public void setResponseHeaders() {
HttpResponse results = new HttpResponse(HttpResponse.SC_OK);
- HttpServletResponseRecorder recorder = new
HttpServletResponseRecorder(fixture.response);
fixture.replay();
- proxy.setResponseHeaders(fixture.request, recorder, results);
+ proxy.setResponseHeaders(fixture.request, fixture.recorder, results);
// Just verify that they were set. Specific values are configurable.
- assertNotNull("Expires header not set", recorder.getHeader("Expires"));
- assertNotNull("Cache-Control header not set",
recorder.getHeader("Cache-Control"));
- assertEquals("attachment;filename=p.txt",
recorder.getHeader("Content-Disposition"));
+ assertNotNull("Expires header not set",
fixture.recorder.getHeader("Expires"));
+ assertNotNull("Cache-Control header not set",
fixture.recorder.getHeader("Cache-Control"));
+ assertEquals("attachment;filename=p.txt",
fixture.recorder.getHeader("Content-Disposition"));
}
@Test
@@ -167,31 +165,27 @@
Map<String, List<String>> headers =
Maps.newTreeMap(String.CASE_INSENSITIVE_ORDER);
headers.put("Pragma", Arrays.asList("no-cache"));
HttpResponse results = new HttpResponse(HttpResponse.SC_OK, new byte[0],
headers);
- HttpServletResponseRecorder recorder = new
HttpServletResponseRecorder(fixture.response);
fixture.replay();
- proxy.setResponseHeaders(fixture.request, recorder, results);
+ proxy.setResponseHeaders(fixture.request, fixture.recorder, results);
// Just verify that they were set. Specific values are configurable.
- assertNotNull("Expires header not set", recorder.getHeader("Expires"));
- assertEquals("no-cache", recorder.getHeader("Pragma"));
- assertEquals("no-cache", recorder.getHeader("Cache-Control"));
- assertEquals("attachment;filename=p.txt",
recorder.getHeader("Content-Disposition"));
+ assertNotNull("Expires header not set",
fixture.recorder.getHeader("Expires"));
+ assertEquals("no-cache", fixture.recorder.getHeader("Pragma"));
+ assertEquals("no-cache", fixture.recorder.getHeader("Cache-Control"));
+ assertEquals("attachment;filename=p.txt",
fixture.recorder.getHeader("Content-Disposition"));
}
@Test
public void setResponseHeadersForceParam() {
HttpResponse results = new HttpResponse(HttpResponse.SC_OK);
- HttpServletResponseRecorder recorder = new
HttpServletResponseRecorder(fixture.response);
expect(fixture.request.getParameter(ProxyBase.REFRESH_PARAM)).andReturn("30").anyTimes();
fixture.replay();
- proxy.setResponseHeaders(fixture.request, recorder, results);
+ proxy.setResponseHeaders(fixture.request, fixture.recorder, results);
- // Just verify that they were set. Specific values are configurable.
- assertNotNull("Expires header not set", recorder.getHeader("Expires"));
- assertContains("30", recorder.getHeader("Cache-Control"));
- assertEquals("attachment;filename=p.txt",
recorder.getHeader("Content-Disposition"));
+ fixture.checkCacheControlHeaders(30, false);
+ assertEquals("attachment;filename=p.txt",
fixture.recorder.getHeader("Content-Disposition"));
}
@Test
