1 2 Previous Next 27 Replies Latest reply on Sep 14, 2007 10:04 AM by dleerob Go to original post
      • 15. Re: required variables
        kukeltje

        ok.. duplicate analysis... nice....

        I'm almost 100000% sure this is an unnoticed regression from 3.1 since there are no unittests for this.....(anymore?)

        btw, hasVariableLocally should not contain the variable if it has no 'read' attribute, see the initializeVariables in the TaskController or the snippet in my post above. Strange....

        • 16. Re: required variables
          dleerob

           

          btw, hasVariableLocally should not contain the variable if it has no 'read' attribute, see the initializeVariables in the TaskController or the snippet in my post above. Strange....


          I don't think it's working like it should then. I tried the following:
          <variable name='IsReplacement' access='write,required'></variable>


          And retrieved all local variables, and printed them out:
          Map variables = startTaskInstance.getVariablesLocally();
           if (variables != null) {
           for (Iterator it = variables.keySet().iterator();it.hasNext();) {
           String variableName = (String)it.next();
           String variableValue = (String)variables.get(variableName);
           System.out.println(variableName+": "+variableValue);
           }
           }

          Is still found/printed the "IsReplacement" variable with a null value.

          IsReplacement: null


          • 17. Re: required variables
            dleerob

            If you take a look at the initializeVariables method, it actually does copy across the variables locally, but with a null value. Basically, the validation won't ever work because it doesn't take null values into account, and a null value is always copied acrossed to the local task instance, even if the variable is not readable.

            if (variableAccesses!=null) {
             Iterator iter = variableAccesses.iterator();
             while (iter.hasNext()) {
             VariableAccess variableAccess = (VariableAccess) iter.next();
             String mappedName = variableAccess.getMappedName();
             if (variableAccess.isReadable()) {
             String variableName = variableAccess.getVariableName();
             Object value = contextInstance.getVariable(variableName, token);
             log.debug("creating task instance variable '"+mappedName+"' from process variable '"+variableName+"', value '"+value+"'");
             taskInstance.setVariableLocally(mappedName, value);
             } else {
             log.debug("creating task instance local variable '"+mappedName+"'. initializing with null value.");
             taskInstance.setVariableLocally(mappedName, null);
             }
             }
             }

            Note the "else" block:
            else {
             log.debug("creating task instance local variable '"+mappedName+"'. initializing with null value.");
             taskInstance.setVariableLocally(mappedName, null);
             }
            


            Please correct me if I'm mistaken, as I just took a quick look.

            • 18. Re: required variables
              dleerob

              Well incase this will help anyone else, my quick fix will probably be running the following code before calling the taskInstance.end() method:

              Map variables = taskInstance.getVariablesLocally();
               if (variables != null) {
               for (Iterator it = variables.keySet().iterator();it.hasNext();) {
               String variableName = (String)it.next();
               if (variables.get(variableName) == null) {
               startTaskInstance.deleteVariable(variableName);
               }
               }
               }

              All local variables with a null value will be deleted, and if any of those variables are set to "required", the IllegalArgumentException will be thrown when the taskInstance.end() method is called, and therefore validation will work.

              • 19. Re: required variables
                olivier_debels

                Ah right,

                I didn't spot that else part, that explains why removing the 'read' access didn't help. Is this the behaviour we want? This kills the meaning of isRequired IMO. So I would opt to remove the else part.

                Anyway, I guess your code is indeed a valid way to fix this in your case.

                Or you could write a custom task controller...

                • 20. Re: required variables
                  dleerob

                  There may be another bug.
                  When calling taskInstance.end(), it now throws an exception (great!), but unfortunately, that exception does not stop the ending of the task instance. So the task instance is indeed considered "ended" anyway, even though you have required variables that are missing.

                  See the end method below, found in TaskInstance. The validation is only done when submitVariables() is called, but before that is called, a bunch of work has already taken place on the task instance.

                  /**
                   * marks this task as done and specifies a transition
                   * leaving the task-node for the case that the completion of this
                   * task instances triggers a signal on the token.
                   * If this task leads to a signal on the token, the given transition
                   * name will be used in the signal.
                   * If this task completion does not trigger execution to move on,
                   * the transition is ignored.
                   */
                   public void end(Transition transition) {
                   if (this.end!=null){
                   throw new IllegalStateException("task instance '"+id+"' is already ended");
                   }
                   if (this.isSuspended) {
                   throw new JbpmException("task instance '"+id+"' is suspended");
                   }
                  
                   // mark the end of this task instance
                   this.end = new Date();
                   this.isOpen = false;
                  
                   // fire the task instance end event
                   if ( (task!=null)
                   && (token!=null)
                   ) {
                   ExecutionContext executionContext = new ExecutionContext(token);
                   executionContext.setTask(task);
                   executionContext.setTaskInstance(this);
                   task.fireEvent(Event.EVENTTYPE_TASK_END, executionContext);
                   }
                  
                   // log this assignment
                   if (token!=null) {
                   token.addLog(new TaskEndLog(this));
                   }
                   
                   // submit the variables
                   submitVariables();
                  
                   // verify if the end of this task triggers continuation of execution
                   if (isSignalling) {
                   this.isSignalling = false;
                  
                  
                  
                   if ( this.isStartTaskInstance() // ending start tasks always leads to a signal
                   || ( (task!=null)
                   && (token!=null)
                   && (task.getTaskNode()!=null)
                   && (task.getTaskNode().completionTriggersSignal(this))
                   )
                   ) {
                  
                   if (transition==null) {
                   log.debug("completion of task '"+task.getName()+"' results in taking the default transition");
                   token.signal();
                   } else {
                   log.debug("completion of task '"+task.getName()+"' results in taking transition '"+transition+"'");
                   token.signal(transition);
                   }
                   }
                   }
                   }


                  Am I missing something? Should it work this way? Surely the task instance should not "end" if the required variables are not set.
                  I think I may need to do my own validation in my framework action class by calling startTaskInstance.getTask().getTaskController().getVariableAccesses(), then implementing my own (similair) validation function on that variable access list, and not even bother calling taskInstance.end() if my own validation fails. Would you recommend I do it this way?

                  • 21. Re: required variables
                    kukeltje

                    You are right it should not work this way..... I'll definately have a look at the 3.1 branche tomorrow (Friday ) (it's now 03:15AM CET) Many strange things here... :-(

                    I would not recoomend doing it this way, at least not for the long term. This should be fixed (imo it is a bug) I'll notify the developers and after some investigation file a (number of) jira issue(s)

                    • 22. Re: required variables
                      dleerob

                      You are right. For long term, I wont use my quick fix, but for now, until the bug is fixed, I will use it. Thanks Ronald and Olivier for all your input and help. Much appreciated.

                      • 23. Re: required variables
                        kukeltje

                        Could you try this patch for the validation?? I'm working on a patch for the ending as well. BUT... lots of tests have to be created before this is could be submitted...

                        ### Eclipse Workspace Patch 1.0
                        #P jbpm.3-clean
                        Index: jpdl/jar/src/main/java/org/jbpm/taskmgmt/def/TaskController.java
                        ===================================================================
                        RCS file: /cvsroot/jbpm/jbpm.3/jpdl/jar/src/main/java/org/jbpm/taskmgmt/def/TaskController.java,v
                        retrieving revision 1.2
                        diff -u -r1.2 TaskController.java
                        --- jpdl/jar/src/main/java/org/jbpm/taskmgmt/def/TaskController.java 11 Jun 2007 10:39:29 -0000 1.2
                        +++ jpdl/jar/src/main/java/org/jbpm/taskmgmt/def/TaskController.java 14 Sep 2007 11:55:00 -0000
                        @@ -136,7 +136,9 @@
                         String mappedName = variableAccess.getMappedName();
                         // first check if the required variableInstances are present
                         if ( (variableAccess.isRequired())
                        - && (! taskInstance.hasVariableLocally(mappedName))
                        + && (
                        + (! taskInstance.hasVariableLocally(mappedName))
                        + || ( taskInstance.getVariable(mappedName) == null))
                         ) {
                         if (missingTaskVariables==null) {
                         missingTaskVariables = mappedName;
                        


                        • 24. Re: required variables
                          dleerob

                          I'm sure that would work fine. I actually created a quick fix method which does just that (as you can see below), and it did validate correctly. The method returns true if validation passes, and I then only call taskInstance.end(); Doing it this way for now, I am then able to get around the "end" bug aswell. I've pasted the method below which may help people with a workaround for now.

                          /**
                           * A bug workaround method to validate that all required
                           * variables in a task instance have been set.
                           * @see Bug[2] at the bottom of this class.
                           * @param taskInstance
                           * @return boolean true if validation passes.
                           */
                           private boolean validateTaskInstanceVariables (TaskInstance taskInstance) {
                           boolean pass = true;
                           TaskController taskController = taskInstance.getTask().getTaskController();
                           if (taskController != null) {
                           List variableAccesses = taskController.getVariableAccesses();
                           if (variableAccesses != null) {
                           String missingTaskVariables = null;
                           Iterator it = variableAccesses.iterator();
                           while (it.hasNext()) {
                           VariableAccess variableAccess = (VariableAccess) it.next();
                           String mappedName = variableAccess.getMappedName();
                           //first check if the required variableInstances are present
                           if ((variableAccess.isRequired())
                           && (!taskInstance.hasVariableLocally(mappedName)
                           || taskInstance.getVariable(mappedName) == null)
                           ) {
                           if (missingTaskVariables==null) {
                           missingTaskVariables = mappedName;
                           }
                           else {
                           missingTaskVariables += ", "+mappedName;
                           }
                           }
                           }
                           // if there are missing, required parameters, set pass to false.
                           if (missingTaskVariables != null) {
                           log.debug("Missing task variables: "+missingTaskVariables);
                           pass = false;
                           }
                           }
                           }
                           return pass;
                           }


                          I can try and apply the patch on my system to test if validation works, but I am not building from source so would take me some time to setup my environment and get to it.

                          • 25. Re: required variables
                            olivier_debels


                            Just a little side remark:

                            I still don't see the point why the variables were added with null values in the first place. That's why I would opt to remove that peace of code.

                            Any reason why you choose a different solution (i.e. allowing the automatic initialization to null and afterwards checking if they were changed to not null).

                            • 26. Re: required variables
                              kukeltje

                              It is not the final solution and I am indeed looking into not setting them at all. This 'else' is probabbly added without looking into the consequences and (still strange, the lack of unittests). I'm investigating this in previous releases, but with the small patch his workaround probably does not have to be implemented and all other tests still work, while not setting them caused a test to fail (just checked).

                              Regarding the ending of the task without the variables being set, that is probably not an issue since all should take place in a transaction, but I'm not sure... I'll get back with more info in an hour.

                              • 27. Re: required variables
                                dleerob

                                 

                                Regarding the ending of the task without the variables being set, that is probably not an issue since all should take place in a transaction, but I'm not sure... I'll get back with more info in an hour.

                                Thanks Ronald, this works for me: if I call jbpmContext.setRollbackOnly(), the task instance is not saved. So all I need to do is call that in my catch block and my task instance won't end. Fixing my problem. So with your small patch, everything should work nicely for me.

                                1 2 Previous Next