You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by jo...@apache.org on 2009/12/02 03:43:31 UTC
svn commit: r886038 - in /incubator/shindig/trunk/java/gadgets/src:
main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java
test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java
Author: johnh
Date: Wed Dec 2 02:43:30 2009
New Revision: 886038
URL: http://svn.apache.org/viewvc?rev=886038&view=rev
Log:
Make feature caching system much more robust.
* Sensitive to RenderingContext (bugfix)
* Sensitive to container code (bugfix)
* Ignored if GadgetContext.getIgnoreCache()
* Single key object type for all properties keying a given feature request.
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java
Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java?rev=886038&r1=886037&r2=886038&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java Wed Dec 2 02:43:30 2009
@@ -64,9 +64,7 @@
= Logger.getLogger("org.apache.shindig.gadgets");
// Map keyed by FeatureNode object created as a lookup for transitive feature deps.
- private final Map<Collection<String>, List<FeatureResource>> cache = new MapMaker().makeMap();
- private final Map<Collection<String>, List<FeatureResource>> cacheIgnoreUnsupported =
- new MapMaker().makeMap();
+ private final Map<FeatureCacheKey, List<FeatureResource>> cache = new MapMaker().makeMap();
private final FeatureParser parser;
private final FeatureResourceLoader resourceLoader;
@@ -154,6 +152,9 @@
// Connect the dependency graph made up of all features and validate there
// are no circular deps.
connectDependencyGraph();
+
+ // Clear caches.
+ cache.clear();
} catch (IOException e) {
throw new GadgetException(GadgetException.Code.INVALID_PATH, e);
}
@@ -179,15 +180,13 @@
*/
public List<FeatureResource> getFeatureResources(
GadgetContext ctx, Collection<String> needed, List<String> unsupported, boolean transitive) {
- Map<Collection<String>, List<FeatureResource>> useCache = null;
- if (transitive) {
- useCache = (unsupported != null) ? cache : cacheIgnoreUnsupported;
- }
+ boolean useCache = (transitive && !ctx.getIgnoreCache());
+ FeatureCacheKey cacheKey = new FeatureCacheKey(needed, ctx, unsupported != null);
List<FeatureResource> resources = Lists.newLinkedList();
- if (useCache != null && useCache.containsKey(needed)) {
- return useCache.get(needed);
+ if (useCache && cache.containsKey(cacheKey)) {
+ return cache.get(cacheKey);
}
List<FeatureNode> featureNodes = null;
@@ -224,8 +223,8 @@
}
}
- if (useCache != null && (unsupported == null || unsupported.isEmpty())) {
- useCache.put(needed, resources);
+ if (useCache && (unsupported == null || unsupported.isEmpty())) {
+ cache.put(cacheKey, resources);
}
return resources;
@@ -592,4 +591,36 @@
return this.transitiveDeps;
}
}
+
+ private static class FeatureCacheKey {
+ private final Collection<String> needed;
+ private final RenderingContext rCtx;
+ private final String container;
+ private final Boolean useUnsupported;
+
+ private FeatureCacheKey(Collection<String> needed, GadgetContext ctx, boolean useUnsupported) {
+ this.needed = needed;
+ this.rCtx = ctx.getRenderingContext();
+ this.container = ctx.getContainer();
+ this.useUnsupported = useUnsupported;
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof FeatureCacheKey)) {
+ return false;
+ }
+ FeatureCacheKey otherKey = (FeatureCacheKey)other;
+ return otherKey.needed.equals(this.needed) &&
+ otherKey.rCtx == this.rCtx &&
+ otherKey.container.equals(this.container) &&
+ otherKey.useUnsupported == this.useUnsupported;
+ }
+
+ @Override
+ public int hashCode() {
+ // Doesn't need to be good, just cheap and consistent.
+ return needed.hashCode() + rCtx.hashCode() + container.hashCode() + useUnsupported.hashCode();
+ }
+ }
}
Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java?rev=886038&r1=886037&r2=886038&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java Wed Dec 2 02:43:30 2009
@@ -18,6 +18,7 @@
package org.apache.shindig.gadgets.features;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.fail;
@@ -206,6 +207,9 @@
Uri feature2Uri = expectResource(xml(BOTTOM_TPL, "gadget", content2Uri.getPath(), null));
registry.register(feature1Uri.toString());
+ List<FeatureResource> resources1 = registry.getAllFeatures();
+ assertEquals(1, resources1.size());
+ assertEquals(content1, resources1.get(0).getContent());
// Register it again, different def.
registry.register(feature2Uri.toString());
@@ -219,6 +223,113 @@
}
@Test
+ public void cacheAccountsForUnsupportedState() throws Exception {
+ String content1 = "content1()";
+ Uri content1Uri = expectResource(content1);
+ Map<String, String> attribs = Maps.newHashMap();
+ String theContainer = "the-container";
+ attribs.put("container", theContainer);
+ Uri feature1Uri = expectResource(xml(BOTTOM_TPL, "gadget", content1Uri.getPath(), null, attribs));
+
+ // Register it.
+ registry.register(feature1Uri.toString());
+
+ // Retrieve content for matching context.
+ List<String> needed = Lists.newArrayList("bottom");
+ List<String> unsupported = Lists.newArrayList();
+ List<FeatureResource> resources = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, theContainer), needed, unsupported);
+
+ // Retrieve again w/ no unsupported list.
+ List<FeatureResource> resourcesUnsup = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, theContainer), needed, null);
+
+ assertNotSame(resources, resourcesUnsup);
+ assertEquals(resources, resourcesUnsup);
+ assertEquals(1, resources.size());
+ assertEquals(content1, resources.get(0).getContent());
+
+ // Now make sure the cache DOES work when needed.
+ List<FeatureResource> resources2 = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, theContainer), needed, unsupported);
+ assertSame(resources, resources2);
+
+ List<FeatureResource> resourcesUnsup2 = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, theContainer), needed, null);
+ assertSame(resourcesUnsup, resourcesUnsup2);
+
+ // Lastly, ensure that ignoreCache is properly accounted.
+ List<FeatureResource> resourcesIgnoreCache = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, theContainer, true), needed, unsupported);
+ assertNotSame(resources, resourcesIgnoreCache);
+ assertEquals(1, resourcesIgnoreCache.size());
+ assertEquals(content1, resourcesIgnoreCache.get(0).getContent());
+ }
+
+ @Test
+ public void cacheAccountsForContext() throws Exception {
+ String content1 = "content1()";
+ Uri content1Uri = expectResource(content1);
+ Map<String, String> attribs = Maps.newHashMap();
+ String theContainer = "the-container";
+ attribs.put("container", theContainer);
+ Uri feature1Uri = expectResource(xml(BOTTOM_TPL, "gadget", content1Uri.getPath(), null, attribs));
+
+ // Register it.
+ registry.register(feature1Uri.toString());
+
+ // Retrieve content for matching context.
+ List<String> needed = Lists.newArrayList("bottom");
+ List<String> unsupported = Lists.newArrayList();
+ List<FeatureResource> resources = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, theContainer), needed, unsupported);
+
+ // Retrieve again w/ mismatch container.
+ List<FeatureResource> resourcesNoMatch = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, "foo"), needed, unsupported);
+
+ // Retrieve again w/ mismatched context.
+ List<FeatureResource> ctxMismatch = registry.getFeatureResources(
+ getCtx(RenderingContext.CONTAINER, theContainer), needed, unsupported);
+
+ assertNotSame(resources, resourcesNoMatch);
+ assertNotSame(resources, ctxMismatch);
+ assertNotSame(resourcesNoMatch, ctxMismatch);
+
+ assertEquals(1, resources.size());
+ assertEquals(content1, resources.get(0).getContent());
+
+ assertEquals(0, resourcesNoMatch.size());
+ assertEquals(0, ctxMismatch.size());
+
+ // Make sure caches work with appropriate matching.
+ List<FeatureResource> resources2 = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, theContainer), needed, unsupported);
+ assertSame(resources, resources2);
+
+ List<FeatureResource> resourcesNoMatch2 = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, "foo"), needed, unsupported);
+ assertSame(resourcesNoMatch, resourcesNoMatch2);
+
+ List<FeatureResource> ctxMismatch2 = registry.getFeatureResources(
+ getCtx(RenderingContext.CONTAINER, theContainer), needed, unsupported);
+ assertSame(ctxMismatch, ctxMismatch2);
+
+ // Check ignoreCache
+ List<FeatureResource> resourcesIC = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, theContainer, true), needed, unsupported);
+ assertNotSame(resources, resourcesIC);
+
+ List<FeatureResource> resourcesNoMatchIC = registry.getFeatureResources(
+ getCtx(RenderingContext.GADGET, "foo", true), needed, unsupported);
+ assertNotSame(resourcesNoMatch, resourcesNoMatchIC);
+
+ List<FeatureResource> ctxMismatchIC = registry.getFeatureResources(
+ getCtx(RenderingContext.CONTAINER, theContainer, true), needed, unsupported);
+ assertNotSame(ctxMismatch, ctxMismatchIC);
+ }
+
+ @Test
public void missingIndexResultsInException() throws Exception {
try {
registry.register(makeResourceUri(".txt").toString());
@@ -478,6 +589,11 @@
}
private GadgetContext getCtx(final RenderingContext rctx, final String container) {
+ return getCtx(rctx, container, false);
+ }
+
+ private GadgetContext getCtx(final RenderingContext rctx, final String container,
+ final boolean ignoreCache) {
return new GadgetContext() {
@Override
public RenderingContext getRenderingContext() {
@@ -488,6 +604,11 @@
public String getContainer() {
return container != null ? container : ContainerConfig.DEFAULT_CONTAINER;
}
+
+ @Override
+ public boolean getIgnoreCache() {
+ return ignoreCache;
+ }
};
}