You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by ga...@apache.org on 2011/03/29 19:33:55 UTC
svn commit: r1086648 - in /shindig/trunk/java/gadgets/src:
main/java/org/apache/shindig/gadgets/http/
main/java/org/apache/shindig/gadgets/servlet/
test/java/org/apache/shindig/gadgets/servlet/
Author: gagan
Date: Tue Mar 29 17:33:55 2011
New Revision: 1086648
URL: http://svn.apache.org/viewvc?rev=1086648&view=rev
Log:
Patch by pulkitgoyal2000 | Issue 3660043: Cache Ttl of concat resource should be minimum(cache ttl of included resources) when unversioned | http://codereview.appspot.com/3660043/
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java
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=1086648&r1=1086647&r2=1086648&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 Tue Mar 29 17:33:55 2011
@@ -136,7 +136,7 @@ public final class HttpResponse implemen
// Default TTL for resources that are public and has no explicit Cache-Control max-age
// and expires headers. Resources without cache-control are considered public by default.
@Inject(optional = true) @Named("shindig.cache.http.defaultTtl")
- private static long defaultTtl = DEFAULT_TTL;
+ public static long defaultTtl = DEFAULT_TTL;
@Inject(optional = true) @Named("shindig.http.fast-encoding-detection")
private static boolean fastEncodingDetection = true;
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java?rev=1086648&r1=1086647&r2=1086648&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java Tue Mar 29 17:33:55 2011
@@ -128,9 +128,7 @@ public class ConcatProxyServlet extends
throw new GadgetException(GadgetException.Code.MISSING_PARAMETER, "Missing type",
HttpResponse.SC_BAD_REQUEST);
}
- HttpUtil.setCachingHeaders(response,
- concatUri.translateStatusRefresh(LONG_LIVED_REFRESH, DEFAULT_REFRESH), false);
- } catch (GadgetException gex) {
+ } catch (GadgetException gex) {
response.sendError(HttpResponse.SC_BAD_REQUEST, formatError("doGet", gex, uri));
return;
}
@@ -140,7 +138,7 @@ public class ConcatProxyServlet extends
response.setHeader("Content-Type", concatType.getMimeType() + "; charset=UTF8");
response.setHeader("Content-Disposition", "attachment;filename=p.txt");
- if (doFetchConcatResources(response, concatUri)) {
+ if (doFetchConcatResources(response, concatUri, uri)) {
response.setStatus(HttpResponse.SC_OK);
} else {
response.setStatus(HttpResponse.SC_BAD_REQUEST);
@@ -154,9 +152,11 @@ public class ConcatProxyServlet extends
* @throws IOException
*/
private boolean doFetchConcatResources(HttpServletResponse response,
- ConcatUriManager.ConcatUri concatUri) throws IOException {
+ ConcatUriManager.ConcatUri concatUri, Uri uri) throws IOException {
// Check for json concat and set output stream.
ConcatOutputStream cos = null;
+ Long minCacheTtl = Long.MAX_VALUE;
+ boolean isMinCacheTtlSet = false;
String jsonVar = concatUri.getSplitParam();
if (jsonVar != null) {
@@ -216,6 +216,8 @@ public class ConcatProxyServlet extends
e.getHttpStatusCode());
}
}
+ minCacheTtl = Math.min(minCacheTtl, httpResp.getCacheTtl());
+ isMinCacheTtlSet = true;
cos.output(futureTask.one, httpResp);
} else {
return false;
@@ -226,6 +228,12 @@ public class ConcatProxyServlet extends
}
}
}
+ // TODO: Investigate Chunked Encoding
+ minCacheTtl = isMinCacheTtlSet ? (minCacheTtl / 1000) : DEFAULT_REFRESH;
+ HttpUtil.setCachingHeaders(response,
+ concatUri.translateStatusRefresh(LONG_LIVED_REFRESH, minCacheTtl.intValue()), false);
+ } catch (GadgetException gex) {
+ cos.outputError(uri, gex);
} finally {
if (cos != null) {
try {
@@ -276,9 +284,11 @@ public class ConcatProxyServlet extends
private static abstract class ConcatOutputStream extends ServletOutputStream {
private final ServletOutputStream wrapped;
+ private final StringBuilder stringBuilder;
protected ConcatOutputStream(ServletOutputStream wrapped) {
this.wrapped = wrapped;
+ stringBuilder = new StringBuilder();
}
protected abstract void outputJs(Uri uri, String data) throws IOException;
@@ -314,20 +324,20 @@ public class ConcatProxyServlet extends
@Override
public void close() throws IOException {
+ wrapped.write(CharsetUtil.getUtf8Bytes(stringBuilder.toString()));
wrapped.close();
}
@Override
public void print(String data) throws IOException {
- write(CharsetUtil.getUtf8Bytes(data));
+ stringBuilder.append(data);
}
- private byte[] CRLF_BYTES = CharsetUtil.getUtf8Bytes("\r\n");
+ private String CRLF = "\r\n";
@Override
public void println(String data) throws IOException {
- print(data);
- write(CRLF_BYTES);
+ print(data + CRLF);
}
}
Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java?rev=1086648&r1=1086647&r2=1086648&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java Tue Mar 29 17:33:55 2011
@@ -22,6 +22,7 @@ import static org.easymock.EasyMock.expe
import java.util.List;
+import org.apache.shindig.common.servlet.HttpServletResponseRecorder;
import org.apache.shindig.common.uri.Uri;
import org.apache.shindig.common.uri.UriBuilder;
import org.apache.shindig.gadgets.GadgetException;
@@ -58,7 +59,7 @@ public class ConcatProxyServletTest exte
private final ConcatProxyServlet servlet = new ConcatProxyServlet();
private TestConcatUriManager uriManager;
-
+
private final ExecutorService sequentialExecutor = Executors.newSingleThreadExecutor();
private final ExecutorService threadedExecutor = Executors.newCachedThreadPool();
@@ -280,7 +281,62 @@ public class ConcatProxyServletTest exte
assertEquals(results, recorder.getResponseAsString());
assertEquals(400, recorder.getHttpStatusCode());
}
-
+
+ @Test
+ public void testMinimumCacheTtl() throws Exception {
+ final Uri URL4 = Uri.parse("http://example.org/4.js");
+ final Uri URL5 = Uri.parse("http://example.org/5.js");
+ final Uri URL6 = Uri.parse("http://example.org/6.js");
+
+ final Integer cacheTtl4 = Integer.MAX_VALUE;
+ final Integer cacheTtl5 = 100000;
+ final Integer cacheTtl6 = Integer.MAX_VALUE;
+
+ expectGetAndSetCacheTtl(URL4, cacheTtl4);
+ expectGetAndSetCacheTtl(URL5, cacheTtl5);
+ expectGetAndSetCacheTtl(URL6, cacheTtl6);
+
+ expectRequestWithUris(Lists.newArrayList(URL4, URL5, URL6));
+
+ // Run the servlet
+ servlet.doGet(request, recorder);
+ verify();
+ int cacheValue = getCacheControlMaxAge(recorder);
+ assertEquals(cacheTtl5, cacheValue, 10);
+ }
+
+ @Test
+ public void testDefaultCacheTtlCacheHeaderMissing() throws Exception {
+ final Uri URL4 = Uri.parse("http://example.org/4.js");
+ final Uri URL5 = Uri.parse("http://example.org/5.js");
+
+ expectGetAndReturnData(URL4, "");
+ expectGetAndReturnData(URL5, "");
+ expectRequestWithUris(Lists.newArrayList(URL4, URL5));
+
+ servlet.doGet(request, recorder);
+ verify();
+ int cacheValue = getCacheControlMaxAge(recorder);
+ // HttpResponse.defaultTtl is in msec, division by 1000 is required to convert into sec.
+ assertEquals((int) (HttpResponse.defaultTtl / 1000), cacheValue, 10);
+ }
+
+ private void expectGetAndSetCacheTtl(Uri url, Integer cacheTtl) throws Exception {
+ HttpRequest req = new HttpRequest(url);
+ HttpResponse resp = new HttpResponseBuilder().setCacheTtl(cacheTtl).create();
+ expect(pipeline.execute(req)).andReturn(resp);
+ }
+
+ /**
+ * Returns cache control max age from HttpServletResponseRecorder
+ */
+ private int getCacheControlMaxAge(HttpServletResponseRecorder recorder) {
+ String cacheControl = recorder.getHeader("Cache-Control");
+ assertTrue(cacheControl != null);
+ String cacheValue = cacheControl.substring(cacheControl.indexOf('=') + 1);
+ return Integer.decode(cacheValue);
+ }
+
private void expectRequestWithUris(List<Uri> uris) {
expectRequestWithUris(uris, null);
}
@@ -292,7 +348,7 @@ public class ConcatProxyServletTest exte
expect(request.getRequestURI()).andReturn("/path").anyTimes();
expect(request.getQueryString()).andReturn("").anyTimes();
replay();
-
+
Uri uri = new UriBuilder(request).toUri();
uriManager.expect(uri, uris, tok);
}
@@ -304,8 +360,7 @@ public class ConcatProxyServletTest exte
this.uriMap = Maps.newHashMap();
}
- public List<ConcatData> make(
- List<ConcatUri> resourceUris, boolean isAdjacent) {
+ public List<ConcatData> make(List<ConcatUri> resourceUris, boolean isAdjacent) {
// Not used by ConcatProxyServlet
throw new UnsupportedOperationException();
}