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

[sk...@apache.org]

Author: skitching
Date: Tue Dec  4 06:16:26 2007
New Revision: 600960

URL: http://svn.apache.org/viewvc?rev=600960&view=rev
Log:
ORCHESTRA-11 fix multiple calls to the ConversationAware interface.
Also improve diagnostics when bean creation fails.

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=600960&r1=600959&r2=600960&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 Tue Dec  4 06:16:26 2007
@@ -62,6 +62,7 @@
 	Scope, BeanFactoryAware, ApplicationContextAware
 {
 	private final Log log = LogFactory.getLog(AbstractSpringOrchestraScope.class);
+	private static final String POST_PROCESSOR_BEAN_NAME = AbstractSpringOrchestraScope.class.getName() + "_BeanPostProcessor";
 
 	private BeanFactory beanFactory;
 	private ConfigurableApplicationContext applicationContext;
@@ -316,7 +317,7 @@
 			if (conversation == null)
 			{
 				// Start the conversation. This eventually results in a 
-				// callback to the createConversation method on t, beanFactoryhis class.
+				// callback to the createConversation method on this class.
 				conversation = manager.startConversation(conversationName, this);
 			}
 			else
@@ -348,7 +349,17 @@
 					}
 				}
 
-				value = factory.getProxy();
+				try
+				{
+					value = factory.getProxy();
+				}
+				catch(org.springframework.aop.framework.AopConfigException e)
+				{
+					throw new IllegalStateException(
+						"Unable to create Orchestra proxy"
+							+ " for bean " + beanName
+							+ " of type " + value.getClass().getName(), e);
+				}
 
 				conversation.setAttribute(beanName, value);
 			}
@@ -399,6 +410,9 @@
 	 * When this scope object's declaration is first processed by Spring, take the opportunity to
 	 * register a global BeanPostProcessor.
 	 * <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>
@@ -412,32 +426,47 @@
 	 * 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.
-	 * <p>
-	 * For Orchestra's "internal" scoped proxying, 
 	 */
 	public void setBeanFactory(BeanFactory beanFactory) throws BeansException
 	{
-		((ConfigurableBeanFactory) beanFactory).addBeanPostProcessor(
-			new BeanPostProcessor()
-			{
-				public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException
+		ConfigurableBeanFactory cbf = (ConfigurableBeanFactory) beanFactory;
+		if (!cbf.containsSingleton(POST_PROCESSOR_BEAN_NAME)) 
+		{
+			// TODO: this processor instance is implicitly bound to *this* scope instance, and
+			// it calls the non-static getConversationNameForBean method on *this* instance, even
+			// though the bean it is invoking may be handled by some other scope object. This
+			// doesn't actually matter, as the getConversationForBean method is actually stateless,
+			// but it is not elegant and should be fixed.
+			BeanPostProcessor processor = 
+				new BeanPostProcessor()
 				{
-					if (bean instanceof ConversationAware)
+					public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException
 					{
-						Conversation conversation = getConversationForBean(beanName);
-
-						((ConversationAware) bean).setConversation(conversation);
+						if (bean instanceof ConversationAware)
+						{
+							Conversation conversation = getConversationForBean(beanName);
+	
+							((ConversationAware) bean).setConversation(conversation);
+						}
+	
+						return bean;
 					}
-
-					return bean;
-				}
-
-				public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException
-				{
-					return bean;
-				}
-			}
-		);
+	
+					public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException
+					{
+						return bean;
+					}
+				};
+				
+			// 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..
+			cbf.registerSingleton(POST_PROCESSOR_BEAN_NAME, processor);
+		}
 		
 		this.beanFactory = beanFactory;
 	}