Author: johnh
Date: Wed Aug 27 16:13:59 2008
New Revision: 689654
URL: http://svn.apache.org/viewvc?rev=689654&view=rev
Log:
Fix for AbstractHttpCache behavior - caches rewritten content properly once
more.
The code is rather convoluted at this point, and is shortly due for cleanup.
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java?rev=689654&r1=689653&r2=689654&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
Wed Aug 27 16:13:59 2008
@@ -26,6 +26,7 @@
* Base class for content caches. Defines cache expiration rules and
* and restrictions on allowed content. Also enforces rewriting
* on cacheable content.
+ * TODO: separate this logic from rewriting - it's confusing
*/
public abstract class AbstractHttpCache implements HttpCache {
@@ -52,23 +53,26 @@
public HttpResponse addResponse(HttpCacheKey key, HttpRequest request,
HttpResponse response) {
if (key.isCacheable() && response != null) {
// !!! Note that we only rewrite cacheable content. Move this call above
the if
- // to rewrite all content that passes through the cache regardless of
cacheability
- HttpResponse rewritten = rewrite(request, response);
-
+ // to rewrite all content that passes through the cache regardless of
cacheability.
HttpResponseBuilder responseBuilder = new HttpResponseBuilder(response);
- responseBuilder.setRewritten(rewritten);
int forcedTtl = request.getCacheTtl();
if (forcedTtl != -1) {
- responseBuilder.setCacheTtl(request.getCacheTtl());
+ responseBuilder.setCacheTtl(forcedTtl);
}
response = responseBuilder.create();
-
- String keyString = key.toString();
- if (responseStillUsable(response)) {
- addResponseImpl(keyString, response);
+ HttpResponse rewritten = checkRewrite(key.toString(), request, response);
+ if (rewritten == response) {
+ // Nothing rewritten (and thus cached). Cache the entry.
+ addResponseImpl(key.toString(), response);
+ } else {
+ return rewritten;
+ }
+
+ if (!request.getIgnoreCache() &&
+ response.getRewritten() != null) {
+ return response.getRewritten();
}
- return checkRewrite(keyString, request, response);
}
return response;
}
@@ -100,22 +104,26 @@
/**
* Add rewritten content to the response if its not there and
- * we can add it. Re-cache if we created rewritten content.
- * Return the appropriately re-written version if requested
+ * we can add it. (Re-)cache if we created rewritten content.
+ * Return the appropriately re-written version if requested.
+ * @return Original response object if not rewritten; rewritten object if so.
*/
- protected HttpResponse checkRewrite(String key, HttpRequest request,
HttpResponse response) {
+ protected HttpResponse checkRewrite(String key,
+ HttpRequest request, HttpResponse response) {
if (response == null) {
return null;
}
// Perform a rewrite and store the content back to the cache if the
// content is actually rewritten
- HttpResponse rewritten = rewrite(request, response);
+ if (response.getRewritten() == null) {
+ HttpResponse rewritten = rewrite(request, response);
- if (rewritten != null) {
- // TODO: Remove this when new rewriting mechanism is ready.
- response = new
HttpResponseBuilder(response).setRewritten(rewritten).create();
- addResponseImpl(key, response);
+ if (rewritten != null) {
+ // TODO: Remove this and other rewriting logic from http cache when
ready.
+ response = new
HttpResponseBuilder(response).setRewritten(rewritten).create();
+ addResponseImpl(key, response);
+ }
}
// Return the rewritten version if requested
@@ -125,23 +133,22 @@
response.getRewritten().getContentLength() > 0) {
return response.getRewritten();
}
+
return response;
}
/**
- * Rewrite the content
- * @return true if rewritten content was generated
+ * Rewrite the content.
+ * @return rewritten HttpResponse object, if rewriting occurred.
*/
protected HttpResponse rewrite(HttpRequest request, HttpResponse response) {
- // TODO - Make this sensitive to custom rewriting rules
- if (response.getRewritten() == null &&
- rewriterRegistry != null) {
+ if (rewriterRegistry != null) {
HttpResponse rewritten = response;
for (ContentRewriter rewriter : rewriterRegistry.getRewriters()) {
rewritten = rewriter.rewrite(request, rewritten);
}
- if (response.getRewritten() != null) {
- return response;
+ if (response != rewritten) {
+ return rewritten;
}
}
return null;
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java?rev=689654&r1=689653&r2=689654&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
Wed Aug 27 16:13:59 2008
@@ -51,9 +51,10 @@
expect(key.isCacheable()).andReturn(true).anyTimes();
HttpRequest request = EasyMock.createNiceMock(HttpRequest.class);
expect(request.getIgnoreCache()).andReturn(false).anyTimes();
+ expect(request.getCacheTtl()).andReturn(Integer.MAX_VALUE).anyTimes();
replay(key, request);
HttpResponse response = new HttpResponseBuilder().setHttpStatusCode(200)
-
.setResponse("foo".getBytes()).setExpirationTime(Integer.MAX_VALUE).create();
+ .setResponse("foo".getBytes()).setCacheTtl(Integer.MAX_VALUE).create();
// Actual test.
AbstractHttpCache ahc = injector.getInstance(TestHttpCache.class);
@@ -61,7 +62,7 @@
assertNotSame(rewritten, response);
assertEquals(PFX_STR + "foo", rewritten.getResponseAsString());
assertSame(rewritten, ahc.getResponse(key, request));
- assertSame(response, ahc.removeResponse(key));
+ assertEquals(response, ahc.removeResponse(key));
}
private static class TestHttpCache extends AbstractHttpCache {
@@ -83,7 +84,6 @@
public HttpResponse removeResponseImpl(String key) {
return map.remove(key);
}
-
}
private static String PFX_STR = "--prefixtest--";
@@ -93,7 +93,7 @@
}
public HttpResponse rewrite(HttpRequest req, HttpResponse resp) {
- return new
HttpResponseBuilder().setHttpStatusCode(resp.getHttpStatusCode())
+ return new HttpResponseBuilder(resp)
.setResponse((PFX_STR +
resp.getResponseAsString()).getBytes()).create();
}
}
