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

[sk...@apache.org]

Author: skitching
Date: Mon Nov 26 09:05:12 2007
New Revision: 598345

URL: http://svn.apache.org/viewvc?rev=598345&view=rev
Log:
Fix ORCHESTRA-9 (NullPointerException in registerDestructionCallback). The critical change here is this one:
  String convName = _SpringUtils.getRealBeanName(beanName);

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=598345&r1=598344&r2=598345&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 Mon Nov 26 09:05:12 2007
@@ -20,6 +20,8 @@
 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.ConversationAware;
 import org.apache.myfaces.orchestra.conversation.ConversationBindingEvent;
@@ -58,6 +60,8 @@
 public abstract class AbstractSpringOrchestraScope implements ConversationFactory, 
 	Scope, BeanFactoryAware, ApplicationContextAware
 {
+	private final Log log = LogFactory.getLog(AbstractSpringOrchestraScope.class);
+
 	private ConfigurableApplicationContext applicationContext;
 	private Advice[] advices;
 
@@ -234,11 +238,13 @@
 
 	/**
 	 * Get the conversation for the given beanName.
+	 * Returns null if the conversation does not exist.
 	 */
 	protected Conversation getConversationForBean(String beanName)
 	{
 		ConversationManager manager = ConversationManager.getInstance();
-		Conversation conversation = manager.getConversation(getConversationNameForBean(beanName));
+		String conversationName = getConversationNameForBean(beanName);
+		Conversation conversation = manager.getConversation(conversationName);
 		return conversation;
 	}
 
@@ -247,22 +253,49 @@
 	 */
 	protected String getConversationNameForBean(String beanName)
 	{
-		if (applicationContext != null)
+		if (applicationContext == null)
 		{
-			BeanDefinition beanDefinition = applicationContext.getBeanFactory().getBeanDefinition(beanName);
-			if (ScopedProxyFactoryBean.class.getName().equals(beanDefinition.getBeanClassName()))
-			{
-				// bad hack to get access to the real definition
-				// TODO: is there a better way to do this?
-				beanDefinition = applicationContext.getBeanFactory().getBeanDefinition(_SpringUtils.getAlternateBeanName(beanName)); // NON-NLS
-			}
-			if (beanDefinition.hasAttribute(BeanDefinitionConversationNameAttrDecorator.CONVERSATION_NAME_ATTRIBUTE))
-			{
-				return (String) beanDefinition.getAttribute(BeanDefinitionConversationNameAttrDecorator.CONVERSATION_NAME_ATTRIBUTE);
-			}
+			// TODO: when can this happen? A scope method is being invoked but no context exists??
+			return beanName;
+		}
+		
+		// First, look up the definition with the specified name. 
+		BeanDefinition beanDefinition = applicationContext.getBeanFactory().getBeanDefinition(beanName);
+
+		// When a definition is present in the config file like this:
+		//  <bean name="foo" class="example.Foo">
+		//    <....>
+		//    <aop:scopedProxy/>
+		//  </bean>
+		// then what Spring actually does is create two BeanDefinition objects, one
+		// with name "foo" that creates a proxy object, and one with name "scopedTarget.foo"
+		// that actually defines the bean of type example.Foo.
+		//
+		// So what we do here is look up the definition by the provided name, then if that
+		// def is creating a proxy then look up the "renamed" definition.
+		if (ScopedProxyFactoryBean.class.getName().equals(beanDefinition.getBeanClassName()))
+		{
+			// bad hack to get access to the real definition
+			// TODO: is there a better way to do this?
+			
+			// Note that we deliberately change the value that will be returned, so that if there is no
+			// conversation name attribute then we return the 
+			beanName = _SpringUtils.getAlternateBeanName(beanName);
+			beanDefinition = applicationContext.getBeanFactory().getBeanDefinition(beanName); // NON-NLS
 		}
 
-		return beanName;
+		if (beanDefinition.hasAttribute(BeanDefinitionConversationNameAttrDecorator.CONVERSATION_NAME_ATTRIBUTE))
+		{
+			// The bean def explicitly included a conversation-name, so return that.
+			return (String) beanDefinition.getAttribute(BeanDefinitionConversationNameAttrDecorator.CONVERSATION_NAME_ATTRIBUTE);
+		}
+		else
+		{
+			// The beanname might have been of form "scopedTarget.foo" (esp from registerDestructionCallback).
+			// But in this case, the conversation name will just be "foo", so strip the prefix off.
+			String convName = _SpringUtils.getRealBeanName(beanName);
+			return convName;
+		}
 	}
 
 	/**
@@ -297,10 +330,32 @@
 	 * the conversation map.
 	 * <p>
 	 * This ensures it will be called during conversation destroy.
+	 * <p>
+	 * Spring calls this method whenever a bean in this scope is created, if that bean
+	 * has a "destroy method". Note however that it appears that it can also call it even
+	 * for beans that do not have a destroy method when there is a "destruction aware"
+	 * BeanPostProcessor attached to the context (spring version 2.5 at least).
 	 */
 	public void registerDestructionCallback(String name, final Runnable runnable)
 	{
+		if (log.isDebugEnabled())
+		{
+			log.debug("registerDestructionCallback for [" + name + "]");
+		}
+
 		Conversation conversation = getConversationForBean(name);
+		if (conversation == null)
+		{
+			// This should never happen because this should only be called after the bean
+			// instance has been created via scope.getBean, which always creates the
+			// conversation for the bean.
+			throw new IllegalStateException("No conversation for bean [" + name + "]");
+		}
+		if (runnable == null)
+		{
+			throw new IllegalStateException("No runnable object for bean [" + name + "]");
+		}
+
 		conversation.setAttribute(
 			runnable.getClass().getName() + "@" + System.identityHashCode(runnable),
 			new ConversationBindingListener()