11 Replies Latest reply on Jul 11, 2007 8:15 PM by Wolfgang Schwendt

    Nested conversations - potential bug in Manager.beginNestedC

    Wolfgang Schwendt Newbie

      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");
       }
      
      


        • 1. Re: Nested conversations - potential bug in Manager.beginNes
          Wolfgang Schwendt Newbie

          Any comments about this, or should I add it to JIRA?

          • 2. Re: Nested conversations - potential bug in Manager.beginNes
            Przemyslaw Jaskierski Newbie

            Wolfgang, although you question is not directed to me, I would suggest filling JIRA issue for this, because this behavior looks like not intended, indeed.

            • 3. Re: Nested conversations - potential bug in Manager.beginNes
              Henri Karapuu Newbie

               

              IllegalStateException should be thrown if @Begin(nested=true) is encountered and there is no long-running conversation active.


              I think that might cause some difficulties with reuse and recursive use.

              I.e. In crud application i can start creation of a new object from a master list (=starting new conversation), OR from within the creation of another object (=starting nested conversation). In both cases the same @Begin method would be called, and the callee should not concern itself who called it or why.

              Disclaimer; i'm only evaluating Seam so i might have understood things wrong.

              • 4. Re: Nested conversations - potential bug in Manager.beginNes
                Wolfgang Schwendt Newbie

                 

                "hkarapuu" wrote:
                IllegalStateException should be thrown if @Begin(nested=true) is encountered and there is no long-running conversation active.



                I think that might cause some difficulties with reuse and recursive use.

                I.e. In crud application i can start creation of a new object from a master list (=starting new conversation), OR from within the creation of another object (=starting nested conversation). In both cases the same @Begin method would be called, and the callee should not concern itself who called it or why.


                hkarapuu, do I understand you correctly that you want the following behavior? (The following is the behavior I'd prefer to see if I could choose...)

                When @Begin(nested=true) is encountered and there is already a long-running conversation active, a new nested conversation (which is also long-running) gets created.

                When @Begin(nested=true) is encountered and there is NO long-running conversation active, a nested conversation does NOT get created. Instead, the temporary conversation being active gets promoted to a long-running conversation. So we get the same behavior in this case as would happen if @Begin (without nested=true) was encountered.


                The thing quoted above with the IllegalStateException is not my proposal. Rather, it is my interpretation based on the code of
                org.jboss.seam.core.Manager.beginNestedConversation() what the intented behavior probably is.

                If I could vote for a behavior, I wouldn't choose the one with the IllegalStateException either. I'd choose the one described in this post.

                But clearly, the present behavior where a nested (but temporary instead of long-running) conversation gets created when @Begin(nested=true) gets encountered and there is no long-running conversation active, looks not like intended to me.



                Disclaimer; i'm only evaluating Seam so i might have understood things wrong.


                Me too.

                • 5. Re: Nested conversations - potential bug in Manager.beginNes
                  Gavin King Master

                  The intended behavior is that a non-nested long-running conversation should begin in this case. thanks for reporting this.

                  • 6. Re: Nested conversations - potential bug in Manager.beginNes
                    Henri Karapuu Newbie

                     

                    hkarapuu, do I understand you correctly that you want the following behavior? (The following is the behavior I'd prefer to see if I could choose...)

                    When @Begin(nested=true) is encountered and there is already a long-running conversation active, a new nested conversation (which is also long-running) gets created.

                    When @Begin(nested=true) is encountered and there is NO long-running conversation active, a nested conversation does NOT get created. Instead, the temporary conversation being active gets promoted to a long-running conversation. So we get the same behavior in this case as would happen if @Begin (without nested=true) was encountered.



                    Yes, exactly like that.



                    Disclaimer; i'm only evaluating Seam so i might have understood things wrong.


                    Me too.


                    You might want to look at Spring Web Flow also, i was able to build my prototype in an hour (including conversation state rollback on back buttoning and recursive conversations) from the installation, while with seam i'v been seriously struggling between docs that don't define semantics clearly, and runtime behavior that does not match my expectations.

                    /Henri

                    • 7. Re: Nested conversations - potential bug in Manager.beginNes
                      Gavin King Master

                       

                      When @Begin(nested=true) is encountered and there is NO long-running conversation active, a nested conversation does NOT get created. Instead, the temporary conversation being active gets promoted to a long-running conversation. So we get the same behavior in this case as would happen if @Begin (without nested=true) was encountered.


                      Yes, this is what is intended. I'm sorry its broken in the beta release, I'll get it fixed. (That's why we do betas.)

                      • 8. Re: Nested conversations - potential bug in Manager.beginNes
                        Ning Zhao Apprentice

                        There are a lot of nested conversations in my application and I've been struggling with the conversation states for days (especially when a son conversation begins exactly inside a method which is annotated as @End in the mother conversation....). This fix can help me enormously, I'm really looking forward to the fix!!


                        Regards,
                        Ellen

                        • 9. Re: Nested conversations - potential bug in Manager.beginNes
                          Ning Zhao Apprentice

                          There is a method which ends a mother conversation like this:

                          
                          @End
                          public void doA(){
                           ...
                           // call a method of another action bean which is annotated with @Begin(nested=true)
                           ...
                           // call a method of another action bean which is annotated with @End
                           ...
                          
                          }
                          
                          


                          My expectation is, when this method is done, both nested conversation and the mother conversation will be ended. For now the problem is, the nested conversation gets ended, but the mother conversation was not ended. Is this a bug or I did something wrong? Thanks for any enlightenment!



                          Regards,
                          Ellen

                          • 10. Re: Nested conversations - potential bug in Manager.beginNes
                            Gavin King Master

                            You can't end two conversations in a single request. That's not how the model works. @End simply says "at the end of the request, end the current conversation".

                            • 11. Re: Nested conversations - potential bug in Manager.beginNes
                              Wolfgang Schwendt Newbie

                               

                              "gavin.king@jboss.com" wrote:
                              You can't end two conversations in a single request. That's not how the model works. @End simply says "at the end of the request, end the current conversation".


                              For error handling a typical idiom in pages.xml is:


                               <exception class="javax.persistence.OptimisticLockException">
                               <end-conversation/>
                               <redirect view-id="/error.xhtml">
                               <message>Another user changed the same data, please try again</message>
                               </redirect>
                               </exception>




                              Now imagine that the exeption was thrown when a nested conversation was in progress: Then above idiom would only end the nested conversation (???)

                              But wouldn't it make sense to end also the all parent conversations up to (inclusive) the top-level long-running conversation?

                              Assuming that nested conversations share the persistence context (EntityManager) with their parent conversation (is my understanding right or wrong here?), wouldn't an exception, thrown while the nested conversation is in progress, lead to the situation that also the persistence context of the parent conversation gets cleared?

                              I could be totally wrong here, but if also the persistence context of the parent conversation gets cleared, I think it would make sense to end also that parent conversation before redirecting to error.xhtml.

                              So something as follows would be nice (with a more meaningful attribute name than "transitive" however).

                              <exception class="javax.persistence.OptimisticLockException">
                               <end-conversation transitive="true"/> // end all conversations up to top-level long-running conversation (inclusive)
                               <redirect view-id="/error.xhtml">
                               <message>Another user changed the same data, please try again</message>
                               </redirect>
                               </exception>