Revert "GRAILS-6790 - register the auto proxy creator in the “root” application context so that any configuration made by plugins/apps apply to our proxy creator."

Installing the APC in this way makes it not be used when generating transactional proxies, which means that Groovy classes don't automatically get proxied by class. Unfortunately, installing it in a way that does allow it to be used when generating transactional proxies runs up against the Spring bug SPR-7808 (which prevents usage of custom APC with 'aop' or 'tx' namespaces).

This exercise has revealed that any user or plugin code that looks up the APC via it's well defined name during application bootstrap will not receive our custom instance. When Spring is doing this internally it will create an instance based on the default implementation. Once the Spring bug is fixed, the solution for us will be to replace line 72 of this patch we are reverting to:

getSpringConfig().getUnrefreshedApplicationContext().beanFactory.registerBeanDefinition(AopConfigUtils.AUTO_PROXY_CREATOR_BEAN_NAME, proxyCreatorDefinition)

Author: ldaley

src/java/org/codehaus/groovy/grails/plugins/CoreGrailsPlugin.groovy

@@ -35,9 +35,7 @@ import org.codehaus.groovy.grails.support.proxy.DefaultProxyHandler
 import org.springframework.beans.factory.config.CustomEditorConfigurer
 import org.springframework.beans.factory.config.MethodInvokingFactoryBean
 import org.springframework.beans.factory.xml.XmlBeanFactory
-import org.springframework.aop.config.AopConfigUtils
 import org.springframework.core.io.Resource
-import org.springframework.beans.factory.support.GenericBeanDefinition
 import org.codehaus.groovy.grails.aop.framework.autoproxy.GroovyAwareAspectJAwareAdvisorAutoProxyCreator
 
 /**
@@ -58,18 +56,15 @@ class CoreGrailsPlugin {
         addBeanFactoryPostProcessor(new MapBasedSmartPropertyOverrideConfigurer(application))
         addBeanFactoryPostProcessor(new GrailsPlaceholderConfigurer())
 
+        // replace AutoProxy advisor with Groovy aware one
         def grailsConfig = application.config.grails
         def springConfig = grailsConfig.spring
-
-        // configure a Groovy aware AutoProxy advisor
-        // we need to do this on the parent context to allow
-        // plugins/apps to affect it while creating/configing beans
-        // See GRAILS-6790
-        def aspectJDisabled = springConfig.disable.aspectj.autoweaving
-        def proxyCreatorType = aspectJDisabled ? GroovyAwareInfrastructureAdvisorAutoProxyCreator : GroovyAwareAspectJAwareAdvisorAutoProxyCreator
-        def proxyCreatorDefinition = new GenericBeanDefinition()
-        proxyCreatorDefinition.beanClass = proxyCreatorType
-        parentCtx.beanFactory.registerBeanDefinition(AopConfigUtils.AUTO_PROXY_CREATOR_BEAN_NAME, proxyCreatorDefinition)
+        if(springConfig.disable.aspectj.autoweaving) {
+            "org.springframework.aop.config.internalAutoProxyCreator"(GroovyAwareInfrastructureAdvisorAutoProxyCreator)
+        }
+        else {
+            "org.springframework.aop.config.internalAutoProxyCreator"(GroovyAwareAspectJAwareAdvisorAutoProxyCreator)
+        }
 
         // Allow the use of Spring annotated components
         context.'annotation-config'()

src/java/org/codehaus/groovy/grails/support/MockApplicationContext.java

@@ -50,7 +50,6 @@
 import org.springframework.util.AntPathMatcher;
 import org.springframework.util.PathMatcher;
 import org.springframework.web.context.WebApplicationContext;
-import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 
 public class MockApplicationContext extends GroovyObjectSupport implements WebApplicationContext {
 
@@ -60,7 +59,6 @@ public class MockApplicationContext extends GroovyObjectSupport implements WebAp
     List<String> ignoredClassLocations = new ArrayList<String>();
     PathMatcher pathMatcher = new AntPathMatcher();
     ServletContext servletContext = new MockServletContext();
-    DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
 
     public void registerMockBean(String name, Object instance) {
         beans.put(name, instance);
@@ -115,10 +113,6 @@ public ApplicationContext getParent() {
         throw new UnsupportedOperationException("Method not supported by implementation");
     }
 
-    BeanFactory getBeanFactory() {
-        return beanFactory;
-    }
-    
     public String getId() {
         return "MockApplicationContext";
     }
@@ -199,30 +193,26 @@ public Map<String, Object> getBeansWithAnnotation(Class<? extends Annotation> an
 
     public Object getBean(String name) throws BeansException {
         if (!beans.containsKey(name)) {
-            if (!beanFactory.containsBean(name)) {
-                throw new NoSuchBeanDefinitionException(name);
-            } else {
-                return beanFactory.getBean(name);
-            }
-        } else {
-            return beans.get(name);
+            throw new NoSuchBeanDefinitionException(name);
         }
+        return beans.get(name);
     }
 
     @SuppressWarnings("unchecked")
     public <T> T getBean(String name, Class<T> requiredType) throws BeansException {
-        Object bean = getBean(name);
+        if (!beans.containsKey(name)) {
+            throw new NoSuchBeanDefinitionException( name);
+        }
 
-        if (requiredType != null && !requiredType.isAssignableFrom(bean.getClass())) {
+        if (requiredType != null && !requiredType.isAssignableFrom(beans.get(name).getClass())) {
             throw new NoSuchBeanDefinitionException(name);
         }
 
-        return (T)bean;
+        return (T)beans.get(name);
     }
 
     public <T> T getBean(Class<T> tClass) throws BeansException {
         final Map<String, T> map = getBeansOfType(tClass);
-        map.putAll(beanFactory.getBeansOfType(tClass));
         if (map.isEmpty()) {
             throw new NoSuchBeanDefinitionException(tClass, "No bean found for type: "  + tClass.getName());
         }
@@ -339,7 +329,7 @@ public boolean containsLocalBean(String arg0) {
     }
 
     public AutowireCapableBeanFactory getAutowireCapableBeanFactory() throws IllegalStateException {
-        return beanFactory;
+        return new DefaultListableBeanFactory();
     }
     public ClassLoader getClassLoader() {
         return getClass().getClassLoader();

src/test/org/codehaus/groovy/grails/plugins/CoreGrailsPluginTests.groovy

@@ -6,8 +6,6 @@ import org.codehaus.groovy.grails.commons.test.AbstractGrailsMockTests
 import org.codehaus.groovy.grails.aop.framework.autoproxy.GroovyAwareAspectJAwareAdvisorAutoProxyCreator
 import org.codehaus.groovy.grails.aop.framework.autoproxy.GroovyAwareInfrastructureAdvisorAutoProxyCreator
 
-import grails.spring.BeanBuilder
-
 class CoreGrailsPluginTests extends AbstractGrailsMockTests {
 
     void testCorePlugin() {
@@ -117,37 +115,4 @@ class CoreGrailsPluginTests extends AbstractGrailsMockTests {
         assertEquals(1, appCtx.getBean('someTransactionalService').i)
         assertEquals(2, appCtx.getBean('nonTransactionalService').i)
     }
-    
-    // See GRAILS-6790
-    void testAopConfigurationIsEffective() {
-        def pluginClass = gcl.loadClass("org.codehaus.groovy.grails.plugins.CoreGrailsPlugin")
-        def plugin = new DefaultGrailsPlugin(pluginClass, ga)
-        def springConfig = new WebRuntimeSpringConfiguration(ctx)
-        
-        plugin.doWithRuntimeConfiguration(springConfig)
-        
-        def bb = new BeanBuilder(ctx, springConfig, null)
-        bb.beans {
-            xmlns aop: "http://www.springframework.org/schema/aop"
-
-            aop.config("proxy-target-class": true) {
-
-                aspect(id: "myPlainJavaAspect-id", ref: "myAspect") { 
-                    "before" method: "myBeforeAdvice", pointcut: "execution(* myMethod1(..))" 
-                }
-            }
-
-            myInterfaceImpl(CoreGrailsPluginTestsAopConfig.MyInterfaceImpl)
-            myAspect(CoreGrailsPluginTestsAopConfig.MyAspect)
-        }
-
-        def appCtx = springConfig.getApplicationContext()
-        def bean = appCtx.getBean("myInterfaceImpl")
-
-        // if we can call the following method, then the proxy was class based
-        // because 'myMethod2' is not on the only interface this implements
-        bean.myMethod2()
-    }
-    
-    
 }