camunda BPMN model API support for Text Annotation

[Filip Hrisafov]

Hi,

I am using the Camunda BPMN Model API to read some BPMN XML files. When trying to access the Source and Target of an Association I sometimes get a ClassCastException. The reason for this is that there are some text annotations in the files that I have and TextAnnotation is not supported by the Camunda BPMN Model API. There might be some other cases as well, I haven't tested out with all the possible elements.

Do you perhaps have a plan to support it in some future releases or I can provide a pull request for it? I haven't done anything, but it seems that it won't be too complicated to do it.

Thanks
Filip Hrisafov

[Christian Lipphardt]

Hi Filip,

As you have experienced the Model API is not feature complete regarding the number of supported BPMN constructs. We jsut implemented the necessary ones. But how it is with OSS, pull requests are always welcome. Just create an issue inside the model api Github repo or submit your pull request. We will provide answers for implementation details.

Cheers,

Christian

[Sebastian Menski]

Hi Filip,

it would be really nice if you can provide a pull request for new elements you need to work with the model API. If you are not experienced with pull requests

or are confused by the strange source code of the model API please feel free to ask further questions. Also you can have a look at an earlier pull request [1]

which added the elements DataObject and DataObjectReference to the model API. Maybe this helps to better estimate the effort to add a new element to the model API.

But even without a pull request thanks for using the model API in the first place.

Cheers,

Sebastian

[1] https://github.com/camunda/camunda-bpmn-model/pull/5

[Sebastian Menski]

Hi Filip,

nice to hear that you are already working on the implementation. Regarding a code formatter I can't really recommend something. I'm

personally using IntelliJ but no real code formatting features.

In the camunda BPM platform project we provide some Eclipse settings [1] but I never used them. You could give it a try or just

use the existing code as reference.

If you got questions regarding implementation details or need a second opinion you can send me a link to your branch or something.

Cheers,
Sebastian

[1] https://github.com/camunda/camunda-bpm-platform/tree/master/settings/eclipse

[Filip Hrisafov]

Hi Sebastian,

I will provide the pull request. I finished the TextAnnotation element, I used the reference and the pull request. I also created tests for it.

In the meantime, I also saw this CAM-2561 and I wanted to implement that as well. I hope that I would finish it by the end of the week and send you the pull request.

There are things which were a bit confusing for me, but I looked at how some other elements are implemented and I am following them. In the meantime do you perhaps some codeformatter that I can use?

Cheers

[Filip Hrisafov]

Hi Sebastian,

I used the eclipse settings from the camunda BPM platform for the formatting of the code.

I finished the implementation for the CAM-2561 feature request, I only need to add tests to it. The branch I am working on is here https://github.com/filiphr/camunda-bpmn-model/tree/feature-additionalBpmnStructs. I hope that it is fine. I had some doubts about the DataInputRef and DataOutputRef, I looked at the EventDefinitionRef from the ThrowEventImpl to do it same as there, without an interface. Let me know if I need to change it. I would appreciate if you can point me to some bpmn files with MultiInstanceLoopCharacteristics, so I can write the tests for it.

I am also not sure with the text element for the textAnnotation, should I add a field in the textAnnotationImpl or how to do it? Currently the field is not used anywhere in the class.

Thanks for the help
Cheers
Filip

P.S. I reposted, since I realized, that my email was in my signature in the previous posts. Sorry for the mix up

On Tuesday, October 7, 2014 11:28:56 PM UTC+2, Sebastian Menski wrote:

[Sebastian Menski]

Hi Filip,

I just had a look at your branch. Some comments:

1. The text of a TextAnnotation is a child element, so you have to create another element (interface + implementation) Text
    which should than be reference from the TextAnnotation like:

      protected static ChildElement<Text> textChild;

   and be accessible through a getter/setter. I think TextAnnotation/Text is basically the same as ScriptTask/Script so if
   you have a look at their implementation (ignoring the camunda extensions) you will see what is missing.

2. Also please let your TextAnnotationTest extend the BpmnModelElementInstanceTest class and implement the required methods,
    this tests are only to verify that the element is correctly defined after the standard. Again have a look at the ScriptTaskTest/ScriptTest
    ignoring the camunda extensions.

3. Currently your TextAnnotationTest isn't working for me cause the bpmn file is not found. I think you have to add it to a new folder 'instance'
    so its in the same folder hierachy as the test source file. If you added the Text child element you should use it in your testGetTextAnnotationById()
    Test like:

      assertEquals("Attached text annotation", textAnnotation.getText().getTextContent());

4. I also noticed that in your TextAnnotation related classes the formatting is different, maybe you started to use the eclipse settings at a later point?

    Could you please at least indent the function body with 2 spaces? :)

5. Regarding your MultiInstance implemenation I would say it looks quite nice. I currently not aware about the input/outputDataItem. I never

    have seen them, so I have to think about it. But I think your current implementation is fine. Please again provide a BpmnModelElementInstanceTest extending

    test for every new element.

Regarding example files I only now that we have some test files in the platform [1]. But they aren't really complex. Maybe just craft some artificial examples, which

don't have to be complete or executable.

Thanks again for your contributing.

Cheers,

Sebastian

[1] https://github.com/camunda/camunda-bpm-platform/tree/master/engine/src/test/resources/org/camunda/bpm/engine/test/bpmn/multiinstance

[Filip Hrisafov]

Hi Christian,

Thank you a lot for the input, it helped me a lot. I took note of all your comments and implemented it. 

The tests for all of the new elements are created and they all pass, I moved the bpmn file to the required folder. I also am extending the BpmnModelElementInstanceTest, the DataObject and DataObjectReference  tests though aren't, and I was looking at them. Do you think that a test bpmn file for the MultiInstanceLoopCharacteristics is required?

Cheers

Filip

[Sebastian Menski]

Hi Filip,

there are DataObject [1] and DataObjectReference [2] tests which extend BpmnModelElementInstanceTest. But Dario also provided a DataObjectsTest [3]

which tested its implemenation by parsing an example bpmn file. But this is generally optional in the model API so you don't have to provide this sort of test

important are only the BpmnModelElementInstanceTest. So it is okay if your pull request doesn't contain such test.

Thanks again for your work.

Cheers,

Sebastian

[1] https://github.com/camunda/camunda-bpmn-model/blob/master/src/test/java/org/camunda/bpm/model/bpmn/instance/DataObjectTest.java

[2] https://github.com/camunda/camunda-bpmn-model/blob/master/src/test/java/org/camunda/bpm/model/bpmn/instance/DataObjectReferenceTest.java

[3] https://github.com/camunda/camunda-bpmn-model/blob/master/src/test/java/org/camunda/bpm/model/bpmn/DataObjectsTest.java

[Filip Hrisafov]

Hi,

I am terribly sorry for the mistake. I didn't manage to see the tests. By the end of the weekend I will make a pull request. I will git squash the different commits into 2, one for the TextAnnotation and the other one for the MultiInstanceLoopCharacteristics and then do a force push on the feature branch before the pull request, so there are 2 commits for the 2 different elements.

Cheers

Filip

[Sebastian Menski]

Hi Filip,

no problem. I'm looking forward to the pull request. 

Cheers,

Sebastian

[Filip Hrisafov]

Hi Sebastian,

I am preparing the branch for the pull request, but I have some problems with the test [1] getEventDefinition, it fails since the condition there is null. Can you please help me out with that one? My changes shouldn't have affected the test. Maybe because of the addition of the ImplicitThrowEvent, but why would it affect it?

Cheers

Filip

[1] https://github.com/filiphr/camunda-bpmn-model/blob/master/src/test/java/org/camunda/bpm/model/bpmn/instance/ConditionalEventDefinitionTest.java

[Sebastian Menski]

Hi Filip,

I think I tracked down the problem. The problem is that Condition and ComplexBehaviorDefinitionCondition define a element with the same

namespace and same name also they have basically a different type tExpression vs. tFormalExpression. And since you inserted

ComplexBehaviorDefinitionConditionImpl below ConditionImpl in the BPMN doRegisterTypes method it overrides the element defined

by ConditionImpl. That is why the Condition child element of ConditionalEventDefinition is no longer known to the model API.

But as tFormalExpression is not fully supported by the model API I would suggest that you use Condition also as child element of 

ComplexBehaviorDefinition and remove the new ComplexBehaviorDefinitionCondition.

Is that okay for you?

Cheers,

Sebastian

[Filip Hrisafov]

Hey Sebastian,

Thanks for tracking down the problem, I suspected something like that, but I was not sure, I couldn't track it down.

I changed ComplexBehaviorDefinition to use the Condition that is already there. I have no problems with it. I added a comment there so people would now.

I just created a pull request with the changes. Let me know if everything is fine.

Cheers

Filip

[Sebastian Menski]

Hi Filip,

thanks for the pull request. I will review them in the next days. See JIRA tickets:

https://app.camunda.com/jira/browse/CAM-2866

https://app.camunda.com/jira/browse/CAM-2561

Thanks again for your contribution.

Cheers,

Sebastian