[jboss-user] [JBoss Seam] - Nested conversations - potential bug in Manager.beginNestedC

[wschwendt]

As regards nested conversations, my interpretation based on the Seam docs
(2.0.0 beta, but also previous versions) is that a new nested conversation
can be started IF a long-running conversation is already in progress.
(See: Seam Docs, Chapter Seam Annotation, @Begin(nested=true).)

This raises of course the question what should happen if @Begin(nested=true)
is encountered and there is no long-running conversation active.  Unfortunately,
the Seam docs do NOT make any statement about this yet.

Therefore, I tried this with Seam 1.3.0Alpha and Seam2.0.0Beta. Interestingly,
if there is no long-running conversation active, Seam creates a new nested 
conversation which is not a long-running, but instead a temporary conversation.

However, looking at the method beginNestedConversation() of
org.jboss.seam.core.Manager (Seam 2.0.0 Beta), I'm not sure whether this is
really the intended behavior. In fact, it looks to me as if it is bug.


  | 
  |  // present seam code
  | 
  |  /**
  |     * Begin a new nested conversation.
  |     */
  |    public void beginNestedConversation()
  |    {
  |       log.debug("Beginning nested conversation");
  |       List<String> oldStack = getCurrentConversationIdStack();
  |      
  |       if (oldStack==null)
  |       {
  |          throw new IllegalStateException("No long-running conversation active");
  |       }
  |       String id = Id.nextId();
  |       setCurrentConversationId(id);
  |       createCurrentConversationIdStack(id).addAll(oldStack);
  |       createConversationEntry();
  |       storeConversationToViewRootIfNecessary();
  |       if ( Events.exists() ) Events.instance().raiseEvent("org.jboss.seam.beginConversation");
  |    }
  | 




Based on this code, my assumption about the intended behavior is that 
an IllegalStateException should be thrown if @Begin(nested=true) is encountered
and there is no long-running conversation active.  However, this guard
is too weak and therefore not sufficient.  Reason: Manager.initializeTemporaryConversation():
always creates a conversationIdStack, even for temporary conversations,
and places the id of the temporary conversation on the stack.


I've therefore fixed  beginNestedConversation() of org.jboss.seam.core.Manager 
as follows (see below).   But I'm not sure what the intended
behavior really is and therefore would highly appreciate some comments.

Regards,

Wolfgang Schwendt





  | 
  |  /**
  |     * Begin a new nested conversation.
  |     */
  |    public void beginNestedConversation()
  |    {
  |       log.debug("Beginning nested conversation");
  |       List<String> oldStack = getCurrentConversationIdStack();
  | 		// ws: original seam code is not a sufficient guard to guarantee that a 
  | 		// long-running conversation is active.  See Manager.initializeTemporaryConversation():
  | 		// it always creates a conversationIdStack and places the id of the temporary conversation
  | 		// on the stack, which implies that also temporary conversations have a conversationIdStack. 
  | 
  | 		// original insufficient guard:
  | 		// if (oldStack==null)
  | 		// {
  | 		// throw new IllegalStateException("No long-running conversation
  | 		// active");
  | 		// }
  | 
  | 		assure_that_long_running_conversation_exists: {
  | 			if (oldStack != null) {
  | 				if (oldStack.size() > 0) {
  | 					String idOnTop = oldStack.get(0);
  | 					ConversationEntries convEntries = ConversationEntries
  | 							.getInstance();
  | 					ConversationEntry ce = convEntries
  | 							.getConversationEntry(idOnTop);
  | 					if (ce != null) {
  | 						// wos: question: does the existence of a conversation
  | 						// entry
  | 						// always imply that the conversation is a long-running
  | 						// conversation?
  | 						// if not, we need to check the properties of the
  | 						// conversation entry ce.
  | 						break assure_that_long_running_conversation_exists;
  | 					}
  | 				}
  | 			}
  | 			throw new IllegalStateException("No long-running conversation active");
  |       }		
  | 
  |       String id = Id.nextId();
  |       setCurrentConversationId(id);
  |       createCurrentConversationIdStack(id).addAll(oldStack);
  |       createConversationEntry();
  |       storeConversationToViewRootIfNecessary();
  |       if ( Events.exists() ) Events.instance().raiseEvent("org.jboss.seam.beginConversation");
  |    }
  |    
  | 

View the original post : http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4059607#4059607

Reply to the post : http://www.jboss.com/index.html?module=bb&op=posting&mode=reply&p=4059607