Skip to content

Commit

Permalink
Simplify and demonstrate workaround for SPR-8639
Browse files Browse the repository at this point in the history
As originally submitted, SPR-8639 used ProxyFactoryBean to apply
MyInterceptor to beans A, B and C.

Typically this works fine, but because of a subtle circular dependency
being introduced due to A and B depending on C and C needing to autowire
D, the PFBs for A and B are prematurely queried for their type (to see
if they are autowire candidates for C) and this trigger a
BeanAlreadyInCreation exception that is subsequently suppressed.

This is a very specific use case and highly unlikely to be reproduced
by other users with any frequency.  However, we will address it even if
only by allowing the original exception to propagate fully.

In the meantime, I have refactored the test case to demonstrate use of
BeanNameAutoProxyCreator in lieu of the original ProxyFactoryBean, and
this approach is better in every way: it is more concise, more readable,
requires fewer bean definitions, and best of all does not suffer from
the above-mentioned bug.

Issue: SPR-8639
  • Loading branch information
cbeams committed Sep 13, 2011
1 parent 876f756 commit e2f5175
Show file tree
Hide file tree
Showing 17 changed files with 111 additions and 196 deletions.
16 changes: 7 additions & 9 deletions SPR-8639/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,20 @@
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-context</artifactId>
<!-- <version>3.1.0.BUILD-SNAPSHOT</version> -->
<version>3.0.5.RELEASE</version>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-test</artifactId>
<!-- <version>3.1.0.BUILD-SNAPSHOT</version> -->
<version>3.0.5.RELEASE</version>
<scope>test</scope>
<version>3.1.0.BUILD-SNAPSHOT</version>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.8</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>cglib</groupId>
<artifactId>cglib-nodep</artifactId>
<version>2.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
Expand Down
13 changes: 2 additions & 11 deletions SPR-8639/src/main/java/org/springframework/issues/A.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
package org.springframework.issues;


public class A implements IA {
public class A {

@SuppressWarnings("unused")
private IC c;
@SuppressWarnings("unused")
private ICWithAutoWired cWithAutoWired;

public void setC( IC c ) {
this.c = c;
public void setC(C c) {
}

public void setcWithAutoWired( ICWithAutoWired cWithAutoWired ) {
this.cWithAutoWired = cWithAutoWired;
}
}

This file was deleted.

11 changes: 1 addition & 10 deletions SPR-8639/src/main/java/org/springframework/issues/B.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,7 @@

public class B {

@SuppressWarnings("unused")
private IC c;
@SuppressWarnings("unused")
private ICWithAutoWired cWithAutoWired;

public void setC( IC c ) {
this.c = c;
public void setC(C c) {
}

public void setCWithAutoWired( ICWithAutoWired cWithAutoWired ) {
this.cWithAutoWired = cWithAutoWired;
}
}
12 changes: 3 additions & 9 deletions SPR-8639/src/main/java/org/springframework/issues/C.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
package org.springframework.issues;

import org.springframework.beans.factory.annotation.Autowired;

public class C implements IC {
public class C {

private D d;
@Autowired D d;

public void setD( D d ) {
this.d = d;
}

public D getD() {
return d;
}
}

This file was deleted.

5 changes: 0 additions & 5 deletions SPR-8639/src/main/java/org/springframework/issues/IA.java

This file was deleted.

5 changes: 0 additions & 5 deletions SPR-8639/src/main/java/org/springframework/issues/IB.java

This file was deleted.

7 changes: 0 additions & 7 deletions SPR-8639/src/main/java/org/springframework/issues/IC.java

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.springframework.issues;

import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;

public class MyInterceptor implements MethodInterceptor {

public Object invoke(MethodInvocation invocation) throws Throwable {
return invocation.proceed();
}

}

This file was deleted.

29 changes: 29 additions & 0 deletions SPR-8639/src/test/java/org/springframework/issues/ReproTests.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package org.springframework.issues;

import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;

import org.junit.Test;
import org.springframework.aop.Advisor;
import org.springframework.aop.framework.Advised;
import org.springframework.aop.support.AopUtils;
import org.springframework.context.support.GenericXmlApplicationContext;

public class ReproTests {

@Test
public void repro() throws Exception {
GenericXmlApplicationContext ctx = new GenericXmlApplicationContext();
ctx.load(getClass(), "ReproTests-context.xml");
ctx.refresh();

C c = ctx.getBean(C.class);
MyInterceptor interceptor = ctx.getBean(MyInterceptor.class);

assertThat("expected c to be a proxy", AopUtils.isAopProxy(c), is(true));
Advisor[] advisors = ((Advised)c).getAdvisors();
assertTrue("interceptor was not added to c's set of advisors",
advisors.length == 1 && advisors[0].getAdvice() == interceptor);
}
}
7 changes: 0 additions & 7 deletions SPR-8639/src/test/resources/log4j.properties

This file was deleted.

27 changes: 27 additions & 0 deletions SPR-8639/src/test/resources/log4j.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">

<log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">

<!-- Appenders -->
<appender name="console" class="org.apache.log4j.ConsoleAppender">
<param name="Target" value="System.out" />
<layout class="org.apache.log4j.PatternLayout">
<param name="ConversionPattern" value="%-5p: %c - %m%n" />
</layout>
</appender>

<logger name="org.springframework.beans.factory.support.DefaultListableBeanFactory">
<level value="debug" />
</logger>
<logger name="org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor">
<level value="debug" />
</logger>

<!-- Root Logger -->
<root>
<priority value="warn" />
<appender-ref ref="console" />
</root>

</log4j:configuration>
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:context="http://www.springframework.org/schema/context"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.1.xsd
http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.1.xsd">

<context:annotation-config/>

<bean class="org.springframework.aop.framework.autoproxy.BeanNameAutoProxyCreator">
<property name="interceptorNames" value="myInterceptor"/>
<property name="beanNames" value="a,b,c"/>
</bean>

<bean id="myInterceptor" class="org.springframework.issues.MyInterceptor"/>

<bean id="a" class="org.springframework.issues.A">
<property name="c" ref="c"/>
</bean>

<bean id="b" class="org.springframework.issues.B">
<property name="c" ref="c"/>
</bean>

<bean id="c" class="org.springframework.issues.C"/>

<!-- will be @Autowired into C -->
<bean id="d" class="org.springframework.issues.D"/>

</beans>

This file was deleted.

0 comments on commit e2f5175

Please sign in to comment.