ServiceTask - Parsing and persistence for custom types

[christhonie]

Dear Activiti team,

Thanks for an awesome product!  It is clear that years of experience has gone into this.

I would like to extend the behavior of a ServiceTask, which I can easily do injecting my own ServiceTaskParseHandler into customDefaultBpmnParseHandlers and providing my own implementation of AbstractBpmnActivityBehavior.  I am however running into problems with how Activiti parse BPMN and convert to and from JSON.  In the former case it is not compliant with the BPMN 2.0 specification and I want to help with a fix.  Let me briefly explain the various deficiencies I have found in master (5.18.1-SNAPSHOT).

Issue 1:
The Common Executable Conformance Sub-Class of the BPMN specification requires a ServiceTask to persist, among others, the attributes "implementation" and "operationref", but this is only done when "implementation" equals "##WebService".  The specification states that implementation could be the constants "##undefined" or "##WebService", as well as any URI.

Issue 2:
Even if I work around this when the model is marshaled back to XML the ImplementationType.IMPLEMENTATION_TYPE_WEBSERVICE is never handled.

Issue 3:
The ServiceTaskJsonConverter also does not handle the IMPLEMENTATION_TYPE_WEBSERVICE (either direction), nor does it persist the operationref.  This is also addressed in https://activiti.atlassian.net/browse/ACT-2010 / https://github.com/Activiti/Activiti/issues/272.

I have solutions and working fixes for these, but I want to check with you first in preparing a pull request that would meet your approval for 5.18.1 and 6.

Question 1:
Would it be acceptable and advisable to introduce 2 new ImplementationTypes; IMPLEMENTATION_TYPE_UNDEFINED and IMPLEMENTATION_TYPE_URI to cater for these 2 scenarios?  This will complement the existing IMPLEMENTATION_TYPE_CLASS, …EXPRESSION, …DELEGATE_EXPRESSION and …WEBSERVICE.  Any other suggestions?

Question 2:
It would be better not to use ServiceTask.Type for this, as this seems to be mapped to the different stencil types.  Agree?

Question 3:
To support the JSON persistence to/from JSON I need to introduce 2 service task properties; PROPERTY_SERVICETASK_IMPLEMENTATION = "servicetaskimplementation" and PROPERTY_SERVICETASK_OPERATION = "servicetaskoperation".  Agree?

Question 4:
With the view on Activiti 6, is the JSON format still relevant?  I read somewhere you will persist to BPMN XML.  Could you explain when and where the JSON is used?  That bit is still a bit confusing.

Question 5:
<Not a critical issue for me and dependent on question 4>  To fully support persistence of a web service service task the JSON format will need to include the persistence of the Interface, Operation, ItemDefinition, etc. specifications, which fall outside the <process> section, which seems to be the only bit being persisted. What is your view on how this should be done?  Maybe this is a topic for another day?

Question 6:
Now assuming you are OK with the persistence of the WebService ServiceTask in JSON but without the interface persisted as in question 5, when retrieving the model again from persistence it will always give a ServiceTaskValidator error, as it expects the operation to be present.  Should this validation be relaxed until the operation is persisted in JSON, or do you have another idea?

Please note I am not trying to go down the WebService implementation rabbit hole, as I realize that one might go quite deep.  I am merely trying to find a standards compliant way for persisting all the required properties of a service task so that I can leverage that to bolt on top of that a custom ActivityBehavior.  The specific nature of my problem will be best addressed by extending the modeler (to introduce my own servicetask) and not using delegates.  This is similar to cdeneux's request in https://forums.activiti.org/content/custom-service-task-type-loaded-through-java-spi.  So, there seem to be similar use cases around.

This post ended up being much longer than I anticipated. Sorry!

Chris

[jbarrez]

Phew … that is a seriously long post. I'm sure I'm missing things when responding to this:

i1. Yes, that is correct from a BPMN spec point of view. However adding those particular attributes won't make it better importable in other products.

i2. That might be correct. Internally it's the WebServiceActivityBehavior that is responsible for the webservice calls. Does it marshal to a servicetask xml with that class?

i3. It sounds right, but I think a PR with actual code makes it easier to discuss and understand what we're talking about. So I'd say yes to getting such a PR read 🙂

Q1. I'm not sure. You say complement, but it will have an impact on existing process definitions, no? Or wat scenario's are you talking about here?

Q2. Not sure what you mean by this. Can you elaborate?

Q3. Same as Q2. I think code would make this a lot easier to understand.

Q4. Yes, the JSON conversion will still matter. The v6 Modeler uses that JSON, so it's not going away anytime soon.

Q5. Yes, if webservice is to be fully supported there is no other solution than storing it in the json too.

Q6. I think the validator simply must become aware of this new kind of service task and change the validation rules there.

Looking forward to discuss further in the PR!