Author: johnh
Date: Tue Sep 23 18:12:28 2008
New Revision: 698411
URL: http://svn.apache.org/viewvc?rev=698411&view=rev
Log:
CachingContentRewriterRegistry uses TTL values provided by rewriters. At
present, the implementation is simply that the TTL for a given entry is th e
minimum TTL of all rewriters in a given pass.
Also includes a fix to TtlCache accommodating the case where now + ttl overruns
Long type limits.
This CL completes SHINDIG-616.
Modified:
incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/TtlCache.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistry.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistry.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AppendingRewriter.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistryTest.java
Modified:
incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/TtlCache.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/TtlCache.java?rev=698411&r1=698410&r2=698411&view=diff
==============================================================================
---
incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/TtlCache.java
(original)
+++
incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/TtlCache.java
Tue Sep 23 18:12:28 2008
@@ -104,8 +104,8 @@
}
/**
- * Add an element to the cache, with the intended amount of time
- * it should live in the cache provided in milliseconds. If below
+ * Add an element to the cache, with the intended expiration time
+ * for its cache entry provided in milliseconds. If below
* minTtl, minTtl is used. If above maxTtl, maxTtl is used.
* @param key Element key.
* @param val Element value to cache.
@@ -113,7 +113,12 @@
*/
public void addElement(K key, V val, long expiration) {
long now = timeSource.currentTimeMillis();
- expiration = Math.max(now + minTtl, Math.min(now + maxTtl, expiration));
+ long minAllowed = Math.min(now + maxTtl, expiration);
+ if (now > 0 && maxTtl > 0 && minAllowed < 0) {
+ // Long-value overflow.
+ minAllowed = maxTtl;
+ }
+ expiration = Math.max(now + minTtl, minAllowed);
TimeoutPair<V> entry = new TimeoutPair<V>(val, expiration);
synchronized(baseCache) {
baseCache.addElement(key, entry);
@@ -121,6 +126,21 @@
}
/**
+ * Add an element to the cache with the intended number of milliseconds,
+ * from the time of add, it is expected to live in the cache. This method
+ * is less precise than standard [EMAIL PROTECTED] addElement}, to which is
specified
+ * the exact expiration time (also in ms) of the entry, due to calculation
+ * of the current time as relative to the add call. In many scenarios,
+ * however, this behavior is perfectly acceptable. It is provided as a
convenience.
+ * @param key Element key.
+ * @param val Element value to cache.
+ * @param ttl Amount of time, in millis, for the cache entry to be valid.
+ */
+ public void addElementWithTtl(K key, V val, long ttl) {
+ addElement(key, val, System.currentTimeMillis() + ttl);
+ }
+
+ /**
* Add an element to the cache, with lifetime set to the default configured
* for this cache object.
* @param key Element key.
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistry.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistry.java?rev=698411&r1=698410&r2=698411&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistry.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistry.java
Tue Sep 23 18:12:28 2008
@@ -17,11 +17,11 @@
*/
package org.apache.shindig.gadgets.rewrite;
-import org.apache.shindig.common.cache.Cache;
import org.apache.shindig.common.cache.CacheProvider;
+import org.apache.shindig.common.cache.TtlCache;
import org.apache.shindig.common.util.HashUtil;
import org.apache.shindig.gadgets.Gadget;
-import org.apache.shindig.gadgets.GadgetException;
+import org.apache.shindig.gadgets.MutableContent;
import org.apache.shindig.gadgets.http.HttpRequest;
import org.apache.shindig.gadgets.http.HttpResponse;
import org.apache.shindig.gadgets.http.HttpResponseBuilder;
@@ -41,17 +41,30 @@
*/
public class CachingContentRewriterRegistry extends
DefaultContentRewriterRegistry {
- private final Cache<String, String> rewrittenCache;
+ private final TtlCache<String, String> rewrittenCache;
+ private long minCacheTtl;
private String rewritersKey;
+ /**
+ * Creates a registry with underlying cache configured by the provided
params.
+ * @param htmlParser Parser used to generate parse tree versions of content.
+ * @param cacheProvider Used to generate a cache instance.
+ * @param capacity Maximum number of rewritten content entries to store in
the cache.
+ * @param minCacheTtl Minimum TTL value, in milliseconds, that it makes
sense to cache an entry.
+ */
@Inject
public CachingContentRewriterRegistry(List<ContentRewriter> rewriters,
GadgetHtmlParser htmlParser,
CacheProvider cacheProvider,
- @Named("shindig.rewritten-content.cache.capacity")int capacity) {
+ @Named("shindig.rewritten-content.cache.capacity")int capacity,
+ @Named("shindig.rewritten-content.cache.minTTL")long minCacheTtl) {
super(rewriters, htmlParser);
-
- rewrittenCache = cacheProvider.createCache(capacity);
+ // minTtl = 0 and maxTtl = MAX_VALUE because the underlying cache is
willing to store data
+ // with any TTL value specified. Entries are added with a given TTL value
per slightly
+ // different logic by this class: if a rewrite pass has a cacheTtl lower
than minCacheTtl,
+ // it's simply not added.
+ rewrittenCache = new TtlCache<String, String>(cacheProvider, capacity, 0,
Long.MAX_VALUE);
+ this.minCacheTtl = minCacheTtl;
}
protected String getGadgetCacheKey(Gadget gadget) {
@@ -69,7 +82,7 @@
StringBuilder keyBuilder = new StringBuilder();
for (ContentRewriter rewriter : rewriters) {
keyBuilder.append(rewriter.getClass().getCanonicalName())
- .append('-').append(rewriter.getClass().hashCode());
+ .append("-").append(rewriter.getClass().hashCode()).append(":");
}
rewritersKey = keyBuilder.toString();
}
@@ -78,7 +91,7 @@
/** [EMAIL PROTECTED] */
@Override
- public boolean rewriteGadget(Gadget gadget) throws GadgetException {
+ public boolean rewriteGadget(Gadget gadget) {
if (gadget.getContext().getIgnoreCache()) {
return super.rewriteGadget(gadget);
}
@@ -91,14 +104,25 @@
return true;
}
- // Do a fresh rewrite and cache the results.
- boolean rewritten = super.rewriteGadget(gadget);
- if (rewritten) {
- // Only cache if the rewriters did something.
- rewrittenCache.addElement(cacheKey, gadget.getContent());
+ long cacheTtl = Long.MAX_VALUE;
+ String original = gadget.getContent();
+ for (ContentRewriter rewriter : getRewriters()) {
+ RewriterResults rr = rewriter.rewrite(gadget);
+ if (rr == null) {
+ cacheTtl = 0;
+ } else {
+ cacheTtl = Math.min(cacheTtl, rr.getCacheTtl());
+ }
+ }
+
+ if (cacheTtl >= minCacheTtl) {
+ // Only cache if the cacheTtl is greater than the minimum time
configured for doing so.
+ // This prevents cache churn and may be more efficient when rewriting is
cheaper
+ // than a cache lookup.
+ rewrittenCache.addElementWithTtl(cacheKey, gadget.getContent(),
cacheTtl);
}
- return rewritten;
+ return !original.equals(gadget.getContent());
}
/** [EMAIL PROTECTED] */
@@ -115,12 +139,32 @@
return new HttpResponseBuilder(resp).setResponseString(cached).create();
}
- HttpResponse rewritten = super.rewriteHttpResponse(req, resp);
- if (rewritten != null) {
- // All these are bounded by the provided TTLs
- rewrittenCache.addElement(cacheKey, rewritten.getResponseAsString());
+ String original = resp.getResponseAsString();
+ MutableContent mc = getMutableContent(original);
+ long cacheTtl = Long.MAX_VALUE;
+ for (ContentRewriter rewriter : getRewriters()) {
+ RewriterResults rr = rewriter.rewrite(req, resp, mc);
+ if (rr == null) {
+ cacheTtl = 0;
+ } else {
+ cacheTtl = Math.min(cacheTtl, rr.getCacheTtl());
+ }
}
- return rewritten;
+ if (cacheTtl >= minCacheTtl) {
+ rewrittenCache.addElementWithTtl(cacheKey, mc.getContent(), cacheTtl);
+ }
+
+ if (!original.equals(mc.getContent())) {
+ return new
HttpResponseBuilder(resp).setResponseString(mc.getContent()).create();
+ }
+
+ // Not rewritten, just return original.
+ return resp;
+ }
+
+ // Methods for testing purposes
+ void setMinCacheTtl(long minCacheTtl) {
+ this.minCacheTtl = minCacheTtl;
}
}
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java?rev=698411&r1=698410&r2=698411&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java
Tue Sep 23 18:12:28 2008
@@ -29,11 +29,12 @@
public class CaptureRewriter implements ContentRewriter {
private boolean rewroteView = false;
private boolean rewroteResponse = false;
+ private long cacheTtl = -1;
public RewriterResults rewrite(HttpRequest request, HttpResponse original,
MutableContent content) {
rewroteResponse = true;
- return null;
+ return results();
}
public boolean responseWasRewritten() {
@@ -42,10 +43,25 @@
public RewriterResults rewrite(Gadget gadget) {
rewroteView = true;
- return null;
+ return results();
}
public boolean viewWasRewritten() {
return rewroteView;
}
+
+ private RewriterResults results() {
+ if (cacheTtl == -1) {
+ return RewriterResults.cacheableIndefinitely();
+ }
+ return RewriterResults.cacheable(cacheTtl);
+ }
+
+ /**
+ * Sets cache TTL. -1 means cacheable indefinitely.
+ * @param cacheTtl
+ */
+ public void setCacheTtl(long cacheTtl) {
+ this.cacheTtl = cacheTtl;
+ }
}
\ No newline at end of file
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistry.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistry.java?rev=698411&r1=698410&r2=698411&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistry.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistry.java
Tue Sep 23 18:12:28 2008
@@ -18,7 +18,6 @@
package org.apache.shindig.gadgets.rewrite;
import org.apache.shindig.gadgets.Gadget;
-import org.apache.shindig.gadgets.GadgetException;
import org.apache.shindig.gadgets.MutableContent;
import org.apache.shindig.gadgets.http.HttpRequest;
import org.apache.shindig.gadgets.http.HttpResponse;
@@ -51,7 +50,7 @@
}
/** [EMAIL PROTECTED] */
- public boolean rewriteGadget(Gadget gadget) throws GadgetException {
+ public boolean rewriteGadget(Gadget gadget) {
String originalContent = gadget.getContent();
if (originalContent == null) {
@@ -68,9 +67,8 @@
/** [EMAIL PROTECTED] */
public HttpResponse rewriteHttpResponse(HttpRequest req, HttpResponse resp) {
- MutableContent mc = new MutableContent(htmlParser);
String originalContent = resp.getResponseAsString();
- mc.setContent(originalContent);
+ MutableContent mc = getMutableContent(originalContent);
for (ContentRewriter rewriter : rewriters) {
rewriter.rewrite(req, resp, mc);
@@ -84,4 +82,13 @@
return new
HttpResponseBuilder(resp).setResponseString(rewrittenContent).create();
}
+ protected MutableContent getMutableContent(String content) {
+ MutableContent mc = new MutableContent(htmlParser);
+ mc.setContent(content);
+ return mc;
+ }
+
+ protected List<ContentRewriter> getRewriters() {
+ return rewriters;
+ }
}
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AppendingRewriter.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AppendingRewriter.java?rev=698411&r1=698410&r2=698411&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AppendingRewriter.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AppendingRewriter.java
Tue Sep 23 18:12:28 2008
@@ -28,22 +28,35 @@
* Used for testing.
*/
class AppendingRewriter implements ContentRewriter {
- private final String appender;
+ private String appender;
+ private final long cacheTtl;
AppendingRewriter(String appender) {
this.appender = appender;
+ this.cacheTtl = 0;
+ }
+
+ AppendingRewriter(String appender, long cacheTtl) {
+ this.appender = appender;
+ this.cacheTtl = cacheTtl;
}
public RewriterResults rewrite(HttpRequest request, HttpResponse original,
MutableContent c) {
// Appends appender to the end of the content string.
c.setContent(c.getContent() + appender);
- return RewriterResults.cacheableIndefinitely();
+ return RewriterResults.cacheable(cacheTtl);
}
public RewriterResults rewrite(Gadget gadget) {
// Appends appender to the end of the input string.
gadget.setContent(gadget.getContent() + appender);
- return RewriterResults.cacheableIndefinitely();
+ return RewriterResults.cacheable(cacheTtl);
+ }
+
+ void setAppender(String newAppender) {
+ // This can be used to simulate a rewriter that returns different
+ // results per run for the same input content.
+ this.appender = newAppender;
}
}
\ No newline at end of file
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistryTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistryTest.java?rev=698411&r1=698410&r2=698411&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistryTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistryTest.java
Tue Sep 23 18:12:28 2008
@@ -46,13 +46,14 @@
import java.util.Map;
public class CachingContentRewriterRegistryTest {
+ private final CaptureRewriter captureRewriter = new CaptureRewriter();
private final List<CaptureRewriter> rewriters
- = Lists.newArrayList(new CaptureRewriter(), new
ModifyingCaptureContentRewriter());
+ = Lists.newArrayList(captureRewriter, new
ModifyingCaptureContentRewriter());
private final List<ContentRewriter> contentRewriters
= Lists.<ContentRewriter>newArrayList(rewriters);
private final FakeCacheProvider provider = new FakeCacheProvider();
- private final ContentRewriterRegistry registry
- = new CachingContentRewriterRegistry(contentRewriters, null, provider,
100);
+ private final CachingContentRewriterRegistry registry
+ = new CachingContentRewriterRegistry(contentRewriters, null, provider,
100, 0);
private final IMocksControl control = EasyMock.createNiceControl();
private final ContainerConfig config =
control.createMock(ContainerConfig.class);
@@ -92,7 +93,6 @@
gadget = new Gadget(context, spec, new ArrayList<JsLibrary>(), config,
null);
registry.rewriteGadget(gadget);
- // TODO: We're not actually testing the TTL of the entries here.
assertEquals(3, provider.readCount);
assertEquals(1, provider.writeCount);
}
@@ -181,7 +181,7 @@
// The new registry is created using one additional rewriter, but the same
cache.
contentRewriters.add(new CaptureRewriter());
ContentRewriterRegistry newRegistry
- = new CachingContentRewriterRegistry(contentRewriters, null, provider,
100);
+ = new CachingContentRewriterRegistry(contentRewriters, null, provider,
100, 0);
newRegistry.rewriteHttpResponse(request, response);
@@ -190,6 +190,27 @@
assertFalse("Cache was written using identical keys.",
provider.keys.get(0).equals(provider.keys.get(1)));
}
+
+ @Test
+ public void rewriteBelowMinCacheDoesntWriteToCache() throws Exception {
+ registry.setMinCacheTtl(1000);
+ captureRewriter.setCacheTtl(500);
+
+ String body = "Hello, world";
+ String xml = "<Module><ModulePrefs title=''/><Content>" + body +
"</Content></Module>";
+ GadgetSpec spec = new GadgetSpec(URI.create("#"), xml);
+ GadgetContext context = new GadgetContext();
+
+ control.replay();
+
+ // We have to re-create Gadget objects because they get mutated directly,
which is really
+ // inconsistent with the behavior of rewriteHttpResponse.
+ Gadget gadget = new Gadget(context, spec, new ArrayList<JsLibrary>(),
config, null);
+ registry.rewriteGadget(gadget);
+
+ assertEquals(1, provider.readCount);
+ assertEquals(0, provider.writeCount);
+ }
private static class FakeCacheProvider implements CacheProvider {
private final Map<String, Object> entries = Maps.newHashMap();
@@ -239,5 +260,7 @@
gadget.setContent(gadget.getContent() + "-modified");
return RewriterResults.cacheableIndefinitely();
}
+
}
+
}
