Author: rbaxter85
Date: Thu Mar 7 14:26:01 2013
New Revision: 1453895
URL: http://svn.apache.org/r1453895
Log:
Do enhancement for FeatureRegistry to detect feature dependency loop problem
SHINDIG-1877
Comitted For Jiaging Guo
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java?rev=1453895&r1=1453894&r2=1453895&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java
Thu Mar 7 14:26:01 2013
@@ -46,7 +46,9 @@ import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.Iterator;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import java.util.Locale;
import java.util.Map;
import java.util.Queue;
@@ -693,6 +695,10 @@ public class FeatureRegistry {
return;
}
+ // Detect feature dependency loop
+ Set<String> tempList = new HashSet<String>();
+ this.checkDependencyLoop(this, tempList);
+
this.nodeDepth = 0;
this.transitiveDeps = Lists.newLinkedList();
this.transitiveDeps.add(this);
@@ -703,10 +709,6 @@ public class FeatureRegistry {
while (!toTraverse.isEmpty()) {
Pair<FeatureNode, Pair<Integer, String>> next = toTraverse.poll();
String debug = next.two.two + (next.two.one > 0 ? " -> " : "") +
next.one.name;
- if (next.one == this && next.two.one != 0) {
- throw new GadgetException(GadgetException.Code.INVALID_CONFIG,
- "Feature dep loop detected: " + debug);
- }
// Breadth-first list of dependencies.
this.transitiveDeps.add(next.one);
this.nodeDepth = Math.max(this.nodeDepth, next.two.one);
@@ -722,5 +724,30 @@ public class FeatureRegistry {
public List<FeatureNode> getTransitiveDeps() {
return this.transitiveDeps;
}
+
+ /**
+ * Check whether current feature node has dependency loop
+ * @param featureNode
+ * feature node which needs to check whether it has dependency loop.
+ * @param dependencyList
+ * used to check whether current branch of feature dependency tree has
dependency loop or not.
+ * @throws GadgetException
+ * a new GadgetException will be thrown if a loop is detected.
+ */
+ private void checkDependencyLoop(FeatureNode featureNode, Set<String>
dependencyList) throws GadgetException {
+ String featureNodeName = featureNode.name;
+ if (dependencyList.contains(featureNodeName)) {
+ // If dependencyList already contains this feature node, then
current branch has dependency loop.
+ throw new GadgetException(GadgetException.Code.INVALID_CONFIG,
"Feature " + featureNodeName + " has dependency loop problem.");
+ }
+ // Add the current dependency into branch.
+ dependencyList.add(featureNodeName);
+ List<FeatureNode> deps = featureNode.getDepList();
+ for (FeatureNode f : deps) {
+ checkDependencyLoop(f, dependencyList);
+ // Remove the part which already has been verified.
+ dependencyList.remove(f.name);
+ }
+ }
}
}
Modified:
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java?rev=1453895&r1=1453894&r2=1453895&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java
(original)
+++
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java
Thu Mar 7 14:26:01 2013
@@ -663,6 +663,106 @@ public class FeatureRegistryTest {
assertEquals("top();gadget();", resources.get(3).getContent());
}
+ @Test(expected = GadgetException.class)
+ public void testCheckDependencyLoopWithLoopDependencyError() throws
Exception {
+ TestFeatureRegistry.Builder builder = TestFeatureRegistry.newBuilder();
+ Uri featureA, featureB, featureC, featureD, txtFile;
+
+ // featureA, featureB, featureC and featureD have dependency loop
+ // problem.
+ featureA = builder.expectResource("<feature>" + "<name>featureA</name>"
+ + "<dependency>featureB</dependency>" + "<gadget>"
+ + " <script>top();gadget();</script>" + "</gadget>" + "<all>"
+ + " <script>top();all();</script>" + "</all>" + "</feature>");
+ featureB = builder.expectResource("<feature>" + "<name>featureB</name>"
+ + "<dependency>featureC</dependency>" + "<container>"
+ + " <script>mid_a();container();</script>" + "</container>"
+ + "<all>" + " <script>mid_a();all();</script>" + "</all>"
+ + "</feature>");
+ featureC = builder.expectResource("<feature>" + "<name>featureC</name>"
+ + "<dependency>featureD</dependency>" + "<container>"
+ + " <script>mid_b();container();</script>" + "</container>"
+ + "<gadget>" + " <script>mid_b();gadget();</script>"
+ + "</gadget>" + "</feature>");
+ featureD = builder.expectResource("<feature>" + "<name>featureD</name>"
+ + "<dependency>featureA</dependency>" + "<all>"
+ + " <script>bottom();all();</script>" + "</all>"
+ + "</feature>");
+ txtFile = builder.expectResource(
+ featureA.toString() + '\n' + featureB.toString() + '\n'
+ + featureC.toString() + '\n' + featureD.toString(),
+ ".txt");
+
+ registry = builder.build(txtFile.toString());
+ }
+
+ @Test(expected = GadgetException.class)
+ public void testCheckDependencyLoopWithPartialLoopDependencyError() throws
Exception {
+ TestFeatureRegistry.Builder builder = TestFeatureRegistry.newBuilder();
+ Uri featureA, featureB, featureC, featureD, txtFile;
+
+ // featureC and featureD have dependency loop problem.
+ featureA = builder.expectResource("<feature>" + "<name>featureA</name>"
+ + "<dependency>featureB</dependency>" + "<gadget>"
+ + " <script>top();gadget();</script>" + "</gadget>" + "<all>"
+ + " <script>top();all();</script>" + "</all>" + "</feature>");
+ featureB = builder.expectResource("<feature>" + "<name>featureB</name>"
+ + "<dependency>featureC</dependency>" + "<container>"
+ + " <script>mid_a();container();</script>" + "</container>"
+ + "<all>" + " <script>mid_a();all();</script>" + "</all>"
+ + "</feature>");
+ featureC = builder.expectResource("<feature>" + "<name>featureC</name>"
+ + "<dependency>featureD</dependency>" + "<container>"
+ + " <script>mid_b();container();</script>" + "</container>"
+ + "<gadget>" + " <script>mid_b();gadget();</script>"
+ + "</gadget>" + "</feature>");
+ featureD = builder.expectResource("<feature>" + "<name>featureD</name>"
+ + "<dependency>featureC</dependency>" + "<all>"
+ + " <script>bottom();all();</script>" + "</all>"
+ + "</feature>");
+ txtFile = builder.expectResource(
+ featureA.toString() + '\n' + featureB.toString() + '\n'
+ + featureC.toString() + '\n' + featureD.toString(),
+ ".txt");
+
+ registry = builder.build(txtFile.toString());
+ }
+
+ @Test
+ public void testCheckDependencyLoopWithNormalDependency() {
+ TestFeatureRegistry.Builder builder = TestFeatureRegistry.newBuilder();
+ Uri featureA, featureB, featureC, featureD, txtFile;
+
+ // There is no loop problem.
+ featureA = builder.expectResource("<feature>" + "<name>featureA</name>"
+ + "<dependency>featureB</dependency>" + "<gadget>"
+ + " <script>top();gadget();</script>" + "</gadget>" + "<all>"
+ + " <script>top();all();</script>" + "</all>" + "</feature>");
+ featureB = builder.expectResource("<feature>" + "<name>featureB</name>"
+ + "<dependency>featureC</dependency>" + "<container>"
+ + " <script>mid_a();container();</script>" + "</container>"
+ + "<all>" + " <script>mid_a();all();</script>" + "</all>"
+ + "</feature>");
+ featureC = builder.expectResource("<feature>" + "<name>featureC</name>"
+ + "<dependency>featureD</dependency>" + "<container>"
+ + " <script>mid_b();container();</script>" + "</container>"
+ + "<gadget>" + " <script>mid_b();gadget();</script>"
+ + "</gadget>" + "</feature>");
+ featureD = builder.expectResource("<feature>" + "<name>featureD</name>"
+ + "<all>" + " <script>bottom();all();</script>" + "</all>"
+ + "</feature>");
+ txtFile = builder.expectResource(
+ featureA.toString() + '\n' + featureB.toString() + '\n'
+ + featureC.toString() + '\n' + featureD.toString(),
+ ".txt");
+
+ try {
+ registry = builder.build(txtFile.toString());
+ } catch (GadgetException e) {
+ fail("Shouldn't throw a GadgetException.");
+ }
+ }
+
private GadgetContext getCtx(final RenderingContext rctx, final String
container) {
return getCtx(rctx, container, false);
}
