Author: matt.drees Date: 2008-09-01 01:09:03 -0400 (Mon, 01 Sep 2008) New Revision: 8863 Added: branches/Seam_2_0/src/main/org/jboss/seam/core/CyclicDependencyException.java branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/CyclicBar.java branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/CyclicFoo.java Modified: branches/Seam_2_0/src/main/org/jboss/seam/core/BijectionInterceptor.java branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/InterceptorTest.java Log: JBSEAM-3362 Detect cyclic dependencies Modified: branches/Seam_2_0/src/main/org/jboss/seam/core/BijectionInterceptor.java =================================================================== --- branches/Seam_2_0/src/main/org/jboss/seam/core/BijectionInterceptor.java 2008-09-01 05:07:32 UTC (rev 8862) +++ branches/Seam_2_0/src/main/org/jboss/seam/core/BijectionInterceptor.java 2008-09-01 05:09:03 UTC (rev 8863) @@ -23,7 +23,9 @@ private static final long serialVersionUID = 4686458105931528659L; private boolean injected; - + + private boolean injecting; + private int counter = 0; private ReentrantLock lock = new ReentrantLock(); @@ -41,7 +43,20 @@ { if (!injected) { - component.inject( invocation.getTarget(), enforceRequired ); + if (injecting == true) + { + throw new CyclicDependencyException(); + } + + injecting = true; + try + { + component.inject(invocation.getTarget(), enforceRequired); + } + finally + { + injecting = false; + } injected = true; } @@ -83,6 +98,11 @@ return result; } + catch (CyclicDependencyException cyclicDependencyException) + { + cyclicDependencyException.addInvocation(getComponent().getName(), invocation.getMethod()); + throw cyclicDependencyException; + } finally { if (injected) Added: branches/Seam_2_0/src/main/org/jboss/seam/core/CyclicDependencyException.java =================================================================== --- branches/Seam_2_0/src/main/org/jboss/seam/core/CyclicDependencyException.java (rev 0) +++ branches/Seam_2_0/src/main/org/jboss/seam/core/CyclicDependencyException.java 2008-09-01 05:09:03 UTC (rev 8863) @@ -0,0 +1,96 @@ +package org.jboss.seam.core; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.List; + +/** + * An exception that is thrown when {@link BijectionInterceptor} detects that a + * component's dependencies cannot be injected due to a cyclic dependency. As + * the exception is passed up the stack, the call sequence is recorded so that a + * useful exception message can be constructed. + * + * @author Matt Drees + * + */ +public class CyclicDependencyException extends IllegalStateException +{ + + /** + * stores the invocations in reverse call order + */ + private final List<String> invocations = new ArrayList<String>(); + private String tailComponentName; + private boolean cycleComplete; + + /** + * Records this invocation's component name and method to be displayed in + * {@link #getMessage()}, unless this invocation is not part of the detected + * cycle. This method will be successively called as the exception is + * propagated up the stack. + * + * @param componentName + * @param method + */ + public void addInvocation(String componentName, Method method) + { + if (cycleComplete) + { + return; + } + + if (invocations.isEmpty()) + { + tailComponentName = componentName; + } + else + { + if (tailComponentName.equals(componentName)) + { + cycleComplete = true; + } + } + invocations.add(createInvocationLabel(componentName, method)); + } + + /** + * returns e.g. "foo.doSomething()" + */ + private String createInvocationLabel(String componentName, Method method) + { + String invocationLabel = componentName + "." + method.getName() + "("; + int i = 1; + for (Class<?> parameterType : method.getParameterTypes()) + { + invocationLabel += parameterType.getSimpleName(); + if (i < method.getParameterTypes().length) + { + invocationLabel += ", "; + } + i++; + } + invocationLabel += ")"; + return invocationLabel; + } + + @Override + public String getMessage() + { + if (!cycleComplete) + { + return "Cyclic dependency found"; + } + else + { + String message = "Injection into " + tailComponentName + " resulted in a dependency cycle, requiring the invocation of " + invocations.get(0) + ". The complete cycle: "; + for (int i = invocations.size() - 1; i >= 0; i--) + { + message += invocations.get(i); + if (i != 0) + message += " -> "; + } + return message; + } + } + +} Added: branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/CyclicBar.java =================================================================== --- branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/CyclicBar.java (rev 0) +++ branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/CyclicBar.java 2008-09-01 05:09:03 UTC (rev 8863) @@ -0,0 +1,22 @@ +package org.jboss.seam.test.unit; + +import org.jboss.seam.ScopeType; +import org.jboss.seam.annotations.In; +import org.jboss.seam.annotations.Name; +import org.jboss.seam.annotations.Scope; +import org.jboss.seam.annotations.Factory; + +@Name("cyclicBar") +(a)Scope(ScopeType.APPLICATION) +public class CyclicBar +{ + + @In CyclicFoo cyclicFoo; + + @Factory(value = "cyclicFooBar", autoCreate = true) + public String provideCyclicFooBar() throws Exception + { + return cyclicFoo.getName() + "bar"; + } + +} Added: branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/CyclicFoo.java =================================================================== --- branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/CyclicFoo.java (rev 0) +++ branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/CyclicFoo.java 2008-09-01 05:09:03 UTC (rev 8863) @@ -0,0 +1,24 @@ +package org.jboss.seam.test.unit; + +import org.jboss.seam.ScopeType; +import org.jboss.seam.annotations.In; +import org.jboss.seam.annotations.Name; +import org.jboss.seam.annotations.Scope; + +@Name("cyclicFoo") +(a)Scope(ScopeType.APPLICATION) +public class CyclicFoo +{ + @In String cyclicFooBar; //from CyclicBar#provideCyclicFooBar + + public String getName() throws Exception + { + return "foo"; + } + + public String getFooBar() throws Exception + { + return cyclicFooBar; + } + +} Modified: branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/InterceptorTest.java =================================================================== --- branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/InterceptorTest.java 2008-09-01 05:07:32 UTC (rev 8862) +++ branches/Seam_2_0/src/test/unit/org/jboss/seam/test/unit/InterceptorTest.java 2008-09-01 05:09:03 UTC (rev 8863) @@ -19,6 +19,7 @@ import org.jboss.seam.core.ConversationEntries; import org.jboss.seam.core.ConversationInterceptor; import org.jboss.seam.core.ConversationalInterceptor; +import org.jboss.seam.core.CyclicDependencyException; import org.jboss.seam.core.Events; import org.jboss.seam.core.Init; import org.jboss.seam.core.Interpolator; @@ -235,6 +236,158 @@ } @Test + public void testCyclicDependencyThowsException() throws Exception + { + MockServletContext servletContext = new MockServletContext(); + ServletLifecycle.beginApplication(servletContext); + MockExternalContext externalContext = new MockExternalContext(servletContext); + Context appContext = new ApplicationContext( externalContext.getApplicationMap() ); + appContext.set( Seam.getComponentName(Init.class), new Init() ); + appContext.set( + Seam.getComponentName(ConversationEntries.class) + ".component", + new Component(ConversationEntries.class, appContext) + ); + appContext.set( + Seam.getComponentName(Manager.class) + ".component", + new Component(Manager.class, appContext) + ); + appContext.set( + Seam.getComponentName(CyclicFoo.class) + ".component", + new Component(CyclicFoo.class, appContext) + ); + appContext.set( + Seam.getComponentName(CyclicBar.class) + ".component", + new Component(CyclicBar.class, appContext) + ); + + FacesLifecycle.beginRequest(externalContext); + Manager.instance().setCurrentConversationId("1"); + FacesLifecycle.resumeConversation(externalContext); + FacesLifecycle.setPhaseId(PhaseId.RENDER_RESPONSE); + + final CyclicFoo cyclicFoo = new CyclicFoo(); + final CyclicBar cyclicBar = new CyclicBar(); + + final BijectionInterceptor cyclicFooBijectionInterceptor = new BijectionInterceptor(); + cyclicFooBijectionInterceptor.setComponent( new Component(CyclicFoo.class, appContext) ); + final Method cyclicFooGetName = CyclicFoo.class.getMethod("getName"); + final MockInvocationContext callGetName = new MockInvocationContext() { + @Override + public Object getTarget() + { + return cyclicFoo; + } + + @Override + public Object proceed() throws Exception + { + return cyclicFoo.getName(); + } + + @Override + public Method getMethod() + { + return cyclicFooGetName; + } + }; + + final Method cyclicFooGetFooBar = CyclicFoo.class.getMethod("getFooBar"); + final MockInvocationContext callGetCyclicFooBar = new MockInvocationContext() { + @Override + public Object getTarget() + { + return cyclicFoo; + } + + @Override + public Object proceed() throws Exception + { + return cyclicFoo.getFooBar(); + } + + @Override + public Method getMethod() + { + return cyclicFooGetFooBar; + } + }; + + CyclicFoo cyclicFooProxy = new CyclicFoo() + { + @Override + public String getName() throws Exception + { + return (String) cyclicFooBijectionInterceptor.aroundInvoke(callGetName); + } + + @Override + public String getFooBar() throws Exception + { + return (String) cyclicFooBijectionInterceptor.aroundInvoke(callGetCyclicFooBar); + } + }; + + + final BijectionInterceptor cyclicBarBijectionInterceptor = new BijectionInterceptor(); + cyclicBarBijectionInterceptor.setComponent( new Component(CyclicBar.class, appContext) ); + final Method cyclicBarProvideCyclicFooBar = CyclicBar.class.getMethod("provideCyclicFooBar"); + final MockInvocationContext callProvideCyclicFooBar = new MockInvocationContext() { + @Override + public Object getTarget() + { + return cyclicBar; + } + + @Override + public Object proceed() throws Exception + { + return cyclicBar.provideCyclicFooBar(); + } + + @Override + public Method getMethod() + { + return cyclicBarProvideCyclicFooBar; + } + }; + + final CyclicBar cyclicBarProxy = new CyclicBarProxy(callProvideCyclicFooBar, cyclicBarBijectionInterceptor); + + + appContext.set("cyclicFoo", cyclicFooProxy); + appContext.set("cyclicBar", cyclicBarProxy); + + try + { + cyclicFooProxy.getFooBar(); + assert false : "cyclic dependency not detected"; + } + catch (CyclicDependencyException e) {} + + } + + /* + * Needs to be non-anonymous, so that provideCyclicFooBar() can be accessed reflectively + */ + public class CyclicBarProxy extends CyclicBar + { + private final MockInvocationContext callProvideCyclicFooBar; + private final BijectionInterceptor cyclicBarBijectionInterceptor; + + private CyclicBarProxy(MockInvocationContext callProvideCyclicFooBar, BijectionInterceptor cyclicBarBijectionInterceptor) + { + this.callProvideCyclicFooBar = callProvideCyclicFooBar; + this.cyclicBarBijectionInterceptor = cyclicBarBijectionInterceptor; + } + + @Override + public String provideCyclicFooBar() throws Exception + { + return (String) cyclicBarBijectionInterceptor.aroundInvoke(callProvideCyclicFooBar); + } + } + + @Test public void testConversationInterceptor() throws Exception { MockServletContext servletContext = new MockServletContext();