You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by sk...@apache.org on 2007/12/05 10:18:41 UTC

svn commit: r601265 - in /myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring: AbstractSpringOrchestraScope.java OrchestraAdvisorBeanPostProcessor.java

Author: skitching
Date: Wed Dec  5 01:18:40 2007
New Revision: 601265

URL: http://svn.apache.org/viewvc?rev=601265&view=rev
Log:
ORCHESTRA-12 fix problems with "unable to subclass $Proxy0", caused by spring generating java.lang.reflect.Proxy proxies for orchestra-scoped beans:
(a) move to generating the CurrentConversationAdvice proxy using a BeanPostProcessor that subclasses AbstractAutoProxyCreator, rather than calling ProxyFactory directly inline. This allows multiple proxy objects for a bean to be merged into a single proxy holding the union of all advices.
(b) poke a magic value into the bean definition to ensure CGLIB is always used for proxies of orchestra-scoped beans.

Added:
    myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring/OrchestraAdvisorBeanPostProcessor.java
Modified:
    myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring/AbstractSpringOrchestraScope.java

Modified: myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring/AbstractSpringOrchestraScope.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring/AbstractSpringOrchestraScope.java?rev=601265&r1=601264&r2=601265&view=diff
==============================================================================
--- myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring/AbstractSpringOrchestraScope.java (original)
+++ myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring/AbstractSpringOrchestraScope.java Wed Dec  5 01:18:40 2007
@@ -28,15 +28,16 @@
 import org.apache.myfaces.orchestra.conversation.ConversationBindingListener;
 import org.apache.myfaces.orchestra.conversation.ConversationFactory;
 import org.apache.myfaces.orchestra.conversation.ConversationManager;
-import org.apache.myfaces.orchestra.conversation.CurrentConversationAdvice;
 import org.apache.myfaces.orchestra.frameworkAdapter.FrameworkAdapter;
-import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.framework.autoproxy.AutoProxyUtils;
 import org.springframework.aop.scope.ScopedProxyFactoryBean;
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.beans.factory.ObjectFactory;
 import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.beans.factory.config.BeanPostProcessor;
+import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.config.Scope;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
@@ -93,6 +94,11 @@
 		this.advices = advices;
 	}
 
+	protected Advice[] getAdvices()
+	{
+		return advices;
+	}
+
 	/**
 	 * Configuration for a scope object to control whether "scoped proxy" objects are
 	 * automatically wrapped around each conversation bean.
@@ -308,31 +314,26 @@
 		{
 			if (!conversation.hasAttribute(beanName))
 			{
-				// create the bean (if not already done)
-				Object value = objectFactory.getObject();
+				Object value;
 
-				ProxyFactory factory = new ProxyFactory(value);
-				factory.setProxyTargetClass(true);
-				factory.addAdvice(new CurrentConversationAdvice(conversation, beanName));
-
-				if (advices != null && advices.length > 0)
-				{
-					for (int i = 0; i < advices.length; i++)
-					{
-						factory.addAdvice(advices[i]);
-					}
-				}
+				// Set the magic property that forces all proxies of this bean to be CGLIB proxies.
+				// It doesn't matter if we do this multiple times..
+				BeanDefinition beanDefinition = applicationContext.getBeanFactory().getBeanDefinition(beanName);
+				beanDefinition.setAttribute(AutoProxyUtils.PRESERVE_TARGET_CLASS_ATTRIBUTE, Boolean.TRUE);
 
 				try
 				{
-					value = factory.getProxy();
+					// Create the new bean. Note that this will run the
+					// OrchestraAdvisorBeanPostProcessor processor, which
+					// will cause the returned object to actually be a proxy
+					// with the CurrentConversationAdvice (at least) attached to it.
+					value = objectFactory.getObject();
 				}
 				catch(org.springframework.aop.framework.AopConfigException e)
 				{
 					throw new IllegalStateException(
 						"Unable to create Orchestra proxy"
-							+ " for bean " + beanName
-							+ " of type " + value.getClass().getName(), e);
+							+ " for bean " + beanName, e);
 				}
 
 				conversation.setAttribute(beanName, value);
@@ -385,31 +386,45 @@
 	{
 	}
 
+    /**
+     * Invoked by Spring to notify this object of the BeanFactory it is associated with.
+     * <p>
+     * This method is redundant as we also have setApplicationContext. However as this
+     * method (and the BeanFactoryAware interface on this class) were present in release
+     * 1.0 this needs to be kept for backwards compatibility.
+     */
+	public void setBeanFactory(BeanFactory beanFactory) throws BeansException
+	{
+	}
+	
 	/**
-	 * When this scope object's declaration is first processed by Spring, take the opportunity to
-	 * register a global BeanPostProcessor.
+	 * Register any BeanPostProcessors that are needed by this scope.
 	 * <p>
 	 * This is an alternative to requiring users to also add an orchestra BeanPostProcessor element
 	 * to their xml configuration file manually.
 	 * <p>
 	 * When a bean <i>instance</i> is created by Spring, it always runs every single BeanPostProcessor
 	 * that has been registered with it.
-	 * <p>
-	 * The BeanPostProcessor implementation sets the {@link org.apache.myfaces.orchestra.conversation.Conversation}
-	 * object on the bean if it implements the {@link org.apache.myfaces.orchestra.conversation.ConversationAware}
-	 * interface.
-	 * <p>
-	 * Note: when an aop:scoped-proxy exists for the bean definition, then the scoped-proxy object is a
-	 * singleton. So the BeanPostProcessor will run once for it (but not do anything). Any time user code
-	 * references that (singleton) bean, an instance of the real bean will be fetched, which might trigger
-	 * an instance creation. If it does, then the BeanPostProcessor also runs for that instance; in this
-	 * case the beanName parameter passed to the methods will be the "scopedTarget" beanname as that is
-	 * the object that the ScopedProxyFactoryBean has asked Spring to fetch.
 	 */
-	public void setBeanFactory(BeanFactory beanFactory) throws BeansException
+	public void defineBeanPostProcessors(ConfigurableListableBeanFactory cbf) throws BeansException
 	{
+		if (!cbf.containsSingleton(POST_PROCESSOR_BEAN_NAME)) 
+		{
+			BeanPostProcessor processor = new OrchestraAdvisorBeanPostProcessor(applicationContext);
+				
+			// Adding the bean to the singletons set causes it to be picked up by the standard
+			// AbstractApplicationContext.RegisterBeanPostProcessors method; that calls
+			// getBeanNamesForType(BeanPostProcessor.class, ...) which finds stuff in the
+			// singleton map even when there is no actual BeanDefinition for it.
+			//
+			// We cannot call cbf.addBeanPostProcessor even if we want to, as the singleton
+			// registration will be added again, making the processor run twice on each bean.
+			// And we need the singleton registration in order to avoid registering this once
+			// for each scope object defined in spring.
+			cbf.registerSingleton(POST_PROCESSOR_BEAN_NAME, processor);
+		}
 	}
-	
+
 	/**
 	 * Get the conversation for the given beanName.
 	 * Returns null if the conversation does not exist.
@@ -578,5 +593,6 @@
 		}
 
 		this.applicationContext = (ConfigurableApplicationContext) applicationContext;
+		defineBeanPostProcessors(this.applicationContext.getBeanFactory());	
 	}
 }

Added: myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring/OrchestraAdvisorBeanPostProcessor.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring/OrchestraAdvisorBeanPostProcessor.java?rev=601265&view=auto
==============================================================================
--- myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring/OrchestraAdvisorBeanPostProcessor.java (added)
+++ myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/spring/OrchestraAdvisorBeanPostProcessor.java Wed Dec  5 01:18:40 2007
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.myfaces.orchestra.conversation.spring;
+
+import org.aopalliance.aop.Advice;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.myfaces.orchestra.conversation.Conversation;
+import org.apache.myfaces.orchestra.conversation.CurrentConversationAdvice;
+import org.springframework.aop.Advisor;
+import org.springframework.aop.TargetSource;
+import org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.context.ConfigurableApplicationContext;
+
+/**
+ * Define a BeanPostProcessor that is run when any bean is created by Spring.
+ * <p>
+ * This checks whether the scope of the bean is an Orchestra scope. If not, it has
+ * no effect.
+ * <p>
+ * For orchestra-scoped beans, this ensures that a CGLIB-generated proxy object is
+ * returned that wraps the real bean requested (ie holds an internal reference to
+ * an instance of the real bean). The proxy has the CurrentConversationAdvice
+ * attached to it always, plus any other advices that are configured for the scope
+ * that the bean is associated with.
+ * <p>
+ * These advices then run on each invocation of any method on the proxy.
+ * <p>
+ * Note that the proxy may have other advices attached to it to, as specified by
+ * any other BeanPostProcessor objects that happen to be registered and relevant
+ * to the created bean (eg an advice to handle declarative transactions).
+ */
+class OrchestraAdvisorBeanPostProcessor extends AbstractAutoProxyCreator
+{
+	private static final long serialVersionUID = 1;
+	private final Log log = LogFactory.getLog(OrchestraAdvisorBeanPostProcessor.class);
+	private ConfigurableApplicationContext appContext;
+
+	/**
+	 * Define a trivial Advisor object that always applies to the class passed to it.
+	 * <p>
+	 * Spring requires Advisor objects to be returned rather than Advices. Advisors
+	 * can implement various interfaces to control which classes or methods they
+	 * get applied to, but here the OrchestraAdvisorBeanPostProcessor only returns an
+	 * instance of this for classes that it really *should* apply to, and the advices
+	 * always apply to all methods, so there is really nothing for this Advisor to do.
+	 * <p>
+	 * TODO: maybe it would be nice to allow an orchestra scope object to hold Advisors
+	 * as well as just Advices, so that users can configure specific code to run only
+	 * for specific methods of orchestra beans. 
+	 */
+	private static class SimpleAdvisor implements Advisor
+	{
+		private Advice advice;
+
+		public SimpleAdvisor(Advice advice)
+		{
+			this.advice = advice;
+		}
+
+		public Advice getAdvice()
+		{
+			return advice;
+		}
+
+		public boolean isPerInstance()
+		{
+			return false;
+		}
+		
+	}
+
+	public OrchestraAdvisorBeanPostProcessor(ConfigurableApplicationContext appContext)
+	{
+		this.appContext = appContext;
+
+		// Always force CGLIB to be used to generate proxies, rather than java.lang.reflect.Proxy.
+		//
+		// Without this, the Orchestra scoped-proxy instance will not work; it requires
+		// the target to fully implement the same class it is proxying, not just the
+		// interfaces on the target class.
+		//
+		//
+		// Alas, this is not sufficient to solve all the problems. If a BeanPostProcessor runs
+		// before this processor, and it creates a CGLIB based proxy, then this class creates
+		// a new proxy that *replaces* that one by peeking into the preceding proxy to find
+		// its real target class/interfaces and its advices and merging that data with the
+		// settings here (see method Cglib2AopProxy.getProxy for details). However if an
+		// earlier BeanPostProcessor has created a java.lang.reflect.Proxy proxy instance
+		// then this merging does not occur; instead this class just tries to wrap that proxy
+		// in another cglib proxy, but that fails because java.lang.reflect.Proxy creates
+		// final (unsubclassable) classes. So in short either this BeanPostProcessor needs to
+		// be the *first* processor, or some trick is needed to force all BeanPostProcessors
+		// to use cglib. This can be done by setting a special attribute in the BeanDefinition
+		// for a bean, and AbstractSpringOrchestraScope does this.
+		//
+		// Note that forging cglib to be used for proxies is also necessary when creating a
+		// "scoped proxy" for an object. The aop:scoped-proxy class also has the same needs
+		// as the Orchestra scoped-proxy, and also forces CGLIB usage, using the same method
+		// (setting AutoProxyUtils.PRESERVE_TARGET_CLASS_ATTRIBUTE in the attributes of the
+		// BeanDefinition of the target bean).
+		setProxyTargetClass(true);
+	}
+
+	@Override
+	protected Object[] getAdvicesAndAdvisorsForBean(Class beanClass, String beanName,
+			TargetSource customTargetSource) throws BeansException
+	{
+		BeanDefinition bd = appContext.getBeanFactory().getBeanDefinition(beanName);
+		if (bd == null)
+		{
+			// odd
+			log.warn("Bean has no definition:" + beanName);
+			return null;
+		}
+
+		String scopeName = bd.getScope();
+		if (scopeName == null)
+		{
+			// does this ever happen?
+			if (log.isDebugEnabled())
+			{
+				log.debug("no scope associated with bean " + beanName);
+			}
+			return null;
+		}
+
+		if (log.isDebugEnabled())
+		{
+			log.debug("Processing scope [" + scopeName + "]");
+		}
+
+		Object scopeObj = appContext.getBeanFactory().getRegisteredScope(scopeName);
+		if (scopeObj == null)
+		{
+			// Ok, this is not an orchestra-scoped bean. This happens for standard scopes
+			// like Singleton.
+			if (log.isDebugEnabled())
+			{
+				log.debug("No scope object for scope [" + scopeName + "]");
+			}
+			return null;
+		}
+		else if (scopeObj instanceof AbstractSpringOrchestraScope == false)
+		{
+			// ok, this is not an orchestra-scoped bean
+			if (log.isDebugEnabled())
+			{
+				log.debug("scope associated with bean " + beanName + " is not orchestra:" + scopeObj.getClass().getName());
+			}
+			return null;
+		}
+
+		AbstractSpringOrchestraScope scopeForThisBean = (AbstractSpringOrchestraScope) scopeObj;
+		Conversation conversation = scopeForThisBean.getConversationForBean(beanName);
+			
+		if (conversation == null)
+		{
+			// In general, getConversationForBean is allowed to return null. However in this case
+			// that is really not expected. Calling getBean for a bean in a scope only ever calls
+			// the get method on the scope object. The only way an instance can *really* be
+			// created is by invoking the ObjectFactory that is passed to the scope object. And
+			// the AbstractSpringOrchestraScope type only ever does that after ensuring that the
+			// conversation object has been created.
+			//
+			// Therefore, this is theoretically impossible..
+			throw new IllegalStateException("Internal error: null conversation for bean " + beanName);
+		}
+
+		Advice[] advices = scopeForThisBean.getAdvices();
+		if ((advices == null) || (advices.length == 0))
+		{
+			advices = new Advice[0];
+		}
+
+		// wrap every Advice in an Advisor instance that returns it in all cases
+		int len = advices.length + 1;
+		Advisor[] advisors = new Advisor[len];
+		for(int i=0; i<advices.length; ++i) 
+		{
+			advisors[i] = new SimpleAdvisor(advices[i]);
+		}
+		// always add the standard CurrentConversationAdvice
+		advisors[len-1] = new SimpleAdvisor(new CurrentConversationAdvice(conversation, beanName));
+
+		return advisors;
+	}
+}