1 2 3 Previous Next 30 Replies Latest reply: May 20, 2010 6:07 PM by Alejandro Guizar RSS

JBPM-2537

Sebastian Schneider Master

Hello Maciej and Huisheng Xu.

 

Could you both have a look at https://jira.jboss.org/jira/browse/JBPM-2537 ?

 

Would be great if both of you could test this. Patch and test case are available in the issue. The test case is a bit weird and confusing. I advise you to take the one from the duplicated issues which is clearer. If everything works fine for you both as well I think the patch could be commited to the trunk. Maybe you could take care of this, Maciej?

 

Cheers

Sebastian

  • 1. Re: JBPM-2537
    HuiSheng Xu Master

    I Sebastian,

     

      Thank you for noticing that.  I had read this issue carefully.  Because there were already lots of comments for it.  And I just don't want to cost time to add new features in jBPM 4.4.  The simplest way to solve this problem is set a eventListener on the activity when activity end history event occure, we could delete related tasks by hand.

  • 2. Re: JBPM-2537
    Sebastian Schneider Master

    This does not solve the problem. As you can read in the issue tasks belonging to an execution cannot be deleted and should not be deleted like this because valueable information is lost in this case. The attached patch by Ronald takes care of all the problems. It does not add a new feature but corrects a bug and makes the engine's behaviour correct. Without this issue it is not possible to use a timer on a transition coming from a user task. So why start looking for workarounds if there is a complete patch available? If it works for you and Maciej as well it should be committed I think.

  • 3. Re: JBPM-2537
    HuiSheng Xu Master

    Hi Sebastian,

      Actually, I don't want to see modifying ExternalActivityBehaviour at this time.  Especially, the timeout() method is only used by TaskActivity.  If we want to implement other requirement, like rollback, withdraw, sub task complete, do we must create method for each of them?  I think we should make a good plan before change the API.

     

      I make a demo with your TaskTimeoutTest.java and TaskTimeout.java.  I use event-listener to delete the out of date Task and update the history task information.  The project is in the attach.  Please have a look at it.  Thank you very much.

     

      Or we could only modify the TaskActivity, let it check whether there is still uncompleted task, if so, it should handle it in the signal().

     

      All of above solution could be used, because in jBPM-4.4-SNAPSHOT, the TaskImpl had bean deleted before signal() invoked.

     

      In my opinion, I want to see PVM provide a more powerful feature for scope management.  We could register variable, timer, task on a scope.  When execution left a scope.  All of registered resources should be notified and do related operations.  It is most like the scope in BPEL.

  • 4. Re: JBPM-2537
    Maciej Swiderski Master

    Hi,

     

    I think that it is a serious issue that needs to be addressed as soon as possible. That is a very important functionality and most likely many people will be affected if that will not be resolved.

     

    I will definitely give it a try and post back results of my tests with the patch.

     

    If it comes to committing it to trunk I think Alejandro needs to join the discussion, he leads the project and in my opinion he shall take the decision if that should be included or not.

     

    Huisheng, of course we could do some workarounds but it is more like doing process engine job in your process definition and that sounds wrong to me. Especially that we already know that issue and it waits for quite some time now.

     

    Cheers,

    Maciej

  • 5. Re: JBPM-2537
    HuiSheng Xu Master

    Hi Maciej,

      I think I will vote yes, if we could fix this issue without modify ExternalActviityBehaviour.  We still have other way to implement this feature.  e.g. modify TaskActivity to notify if there are out of date tasks should be deleted.

     

      When you check the patch and testcase, please make attention this comment of Ronald in TimerImpl:

     

        // I am not sure here, timeout() just needs to be called in case we are
        // executing for a task activity
        
        ActivityImpl activity = execution.getActivity();
        ExternalActivityBehaviour behaviour = (ExternalActivityBehaviour) activity.getActivityBehaviour();
        behaviour.timeout(execution, signalName);  
    

     

       It said that Ronald didn't make sure this solution is complete right, too.  Because the timeout() method is only used by TaskActivity.

     

       So I will try to find a solution without changed public API.  Then we could discuss about which solution could be taken.

  • 6. Re: JBPM-2537
    HuiSheng Xu Master

    Hi Sebastian and Maciej,

     

      Please have a look at this.  I just add two lines in TaskActivity and TaskTimeoutTest could pass successfully.

     

        DbSession taskDbSession = EnvironmentImpl .getFromCurrent(DbSession.class);
        TaskImpl task = (TaskImpl) taskDbSession.findTaskByExecution(execution);
        if (task!=null) {
          task.setSignalling(false);
    +     HistoryEvent.fire(new TaskTimeout("time out"), (ExecutionImpl)execution);
    +     taskDbSession.delete(task);
        }
    

     

      The testcase project is in the attach.  Please think about it.  If there is any chance, please don't change ExternalActivityBehaviour,  at least not this time.  Thank you very much.

  • 7. Re: JBPM-2537
    Sebastian Schneider Master

    I agreed with Maciej that this is a serious issue. I don't like work arounds. I know that this affects some more internal things. One more reason for me too fix this as soon as possible. It will only get more difficult if you postpone this. IMHO this is a design problem. Could you point out the differences a bit more cleary, Huisheng Xu? When and where are you taking care of the tasks which stays in the left after the execution is triggered?

  • 8. Re: JBPM-2537
    HuiSheng Xu Master

    Hi Sebastian,

     

      Because of an already fixed issue, Tom move dbSession.delete(task) from TaskActivity.signal() to TaskImpl.complete().  So if we call TaskService.completeTask()  and signal the related execution.  The task will alway be deleted before execution.signal().  So we could use it to judge who are calling signal() method.  If we can not find the task for current execution, it will be the normal way, and task had been completed normally.  If there is still a task for current execution. This task must be out of date.

     

      Here is the issue.

     

    https://jira.jboss.org/jira/browse/JBPM-2607

     

      Yes, I know we still cannot tell whether there is a timerout event occurred and it is surely a design problem.  But directly add a timeout() to ExternalActivityBehaviour is not a good design, too.

  • 9. Re: JBPM-2537
    Maciej Swiderski Master

    I just wondering if in this scenario task should be completed. To me it sounds more like cancellation than completion.

    Especially if we are considering some kind of reviews of process and its history. Because if you complete a task that no one has taken any action on that could be confusing.

     

    But that is my opinion...

  • 10. Re: JBPM-2537
    Sebastian Schneider Master

    Huisheng Xu: I was not aware of this issue and its bugfix. Thank you for pointing out. Nevertheless I don't see why adding this method to ExternalActivityBehaviour is not a good design. Ronald has been thinking this carefully. When implementating custom nodes which also need a timeout behaviour this would be already useful.

     

    Please also consider Maciej's point:

     

    For me cancelling a task and timeouts have to be clearly distinguished from the completion and deletion of tasks because they all mean different history data being related. If there is an escalation/timeout I don't want the task to be treated as if it was deleted. Deleting tasks should only be possible for tasks not belonging to a process/execution.

  • 11. Re: JBPM-2537
    HuiSheng Xu Master

    Hi Sebastian.

     

       I am agree that timeout is very useful and it is necessary for TaskActivity and other Waitable Activities.  But there are many situations to activate the Waitable Acitivity.  the 'time out' situation is just one of these situations.  So I want to find a way to handle all of them.  There may be cancel() or interrupt() method with a 'state' argument, but not timeout().

     

      For example,  now we saw TimerImpl could complete a TaskActivity with timeout status.  So we add a timeout() method in ExternalActivityBehaviour.  Some days later, we want to let people cancel a TaskActivity by hande,  does we need add a cancelByHuman() method in  ExternalActivityBehaviour, too?  And so on.  If there are more cancel activity situations,  does we need add more xxx() methods in ExternalActivityBehaviour?

     

      So, in my opinion, we should make a better plan for how to handle these activity cancellation situations, not just add a timeout() method in ExternalActivityBehaviour.

     

      And I am totally agree with you on not deleting tasks.  In my demo, I am using your TaskTimeout.java to store history record.  Thank you for your work.

  • 12. Re: JBPM-2537
    Maciej Swiderski Master

    Hi guys,

     

    finally I found some time to look into this issue.

     

    First of all I don't see anything wrong with adding methods (such as timeout or cancel) to activity behavior - to me they are regular life cycle methods of the activity. Especially that one of the core funtinalities is timers.

    Next, ExternalActivityBehaviour is not modified. That's why new interface was created (TimeoutActivityBehaviour) and this interface is only implemented by TaskActivity.

     

    I have tried the patch from jira (20100227-JBPM-2753) but unfortunately it is incomplete. There are few things missing - mainly those that Ronald posted inline. I made some modification to make it to work. So, currently both issues seems to have a solution:

    - JBPM-2753 - uses timeout to close task

    - JBPM-2807 - uses cancel to close task

     

    Main change compared to Ronlad's changes is to make Timer to make use of new signaling - marking it as timeout aware. It uses new signal constructor introduced by Ronald.

     

    I will put some comments and the patch into jira later today.

     

    Let me know your comments.

     

    Cheers,

    Maciej

  • 13. Re: JBPM-2537
    HuiSheng Xu Master

    Hi Maciej,

     

      Seems I make a mistake.  The latest verion is 20100227-JBPM-2753.  I'm glad to see we needn't modify the ExternalActivityBehaviour. and I will wait for your patcher.  Thank you.

  • 14. Re: JBPM-2537
    Sebastian Schneider Master

    Hi Maciej!

     

    Thanks for taking the time to look deeper into the issue. I now recall the discussion I had with Ronald. He was not satisfied with the first solution because we have been changing an Interface and was afraid of breaking backwards compatibility. That was the reason he introduced a new Interface in his version of the patch. So it seems we can go ahead and apply the patch.

     

    Sebastian

1 2 3 Previous Next