-
Notifications
You must be signed in to change notification settings - Fork 487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BPMN Linter #803
Comments
Such thing resided on the back of my head for a long time already. Unfortunately we were not able to pick such thing up due to time constraints. To make the model linter most powerful we should probably hook it into the BPMN model exposed via bpmn-moddle rather than diagram elements. That would allow us to validate diagrams via the command line, too (similar to eslint). Are you open to contribute an initial API and a small number of sane rules as defaults? Missing input/output mappings for call activities sounds like a good one but I guess there can be many more ranging from actual errors to simple style issues. |
Do you have any ideas on whether this should be a custom API/lib hooked into moodle or use any lib? Like using ESlint and having it use the js metamodels that are generated by bpmn moodle? I have not figured out what the architecture would look like. Your insights and thoughts would be great. |
My current design idea has been: use Eslint preprossor plugin to convert the bpmn file into bpmn-moodle js and the. Pass that js into eslint rules. |
ESLint transforms JavaScript programs to their AST / CST form and runs numerous processors over them. I'm not sure that this is the desired behavior for us. I could rather imagine a tree traverser that walks the entire model, allowing plug-ins to hook into it. How would you like to create a custom rule? Could you share a piece of code that sketches it? |
so reviewing this further:
yes, there are likely a few different approaches on this, depending on the behaviour we are trying to achieve.
While the AST/CST is not a exact fit (given its more code static code focused), it seem to have some good carry over when looking at eslint from how Markdown editors use it: You can setup a processor/pre-processor that ingests a .md file and captures all of the code blocks within the md file and process each block individually through the linter. So when i think about this processes, this is very very similar to a setup we need from a bpmn linter: a bpmn/cmn/dmn file/xml is ingested, and either linted at the XML level or converted into another form (moodle-bpmn for example) and the tree can be read as a whole or split into multiple items (like linting each element individually, rather than traversing the entire tree. eslint rules imo are very similar style of rules to what i have imagined a bpmn linter would preform. Style, configurations, poor formatting, double sequence flows, overlaping sequence flows, malformed expressions, missing I personally prefer using/finding a linter that already has figured out the overhead/boiler plate and we can adapt to work within that tool. BPMN is not a "mass adoption" language that everyone is using. So generating our own linter engine + rules system/configurations + cli + managing all of that, is imo a lot of overhead that just creates a developer dependency. The modeler is built in electron, so using a linter than can execute on JS seems to have its benefits. And as modeler could be injected into other IDEs (like VScode) and using the lint tools that are already baked in. Another angle i started to scope was using a object validation framework: if we assume that we generate moodle-bpmn and we build object object validations then we would just need to build rules that represent the object validation, and the object would come from the moddle-bpmn Maybe something like a JSON Schema Validator? |
This is the big difference between text formats (such as It really boils down to the rules we'd like two write (and with how much boilerplate we'd like two write them). So again, how would you like to write a classic BPMN rule such as:
I lack the insights into how ESlint can be extended to traverse custom AST(s) and provide validations for it. But generally speaking it is worth to integrate into existing linters, if that is possible. |
@nikku okay so i have moved forward with JSON Schema validation. It seems to be working so far. The "leg work" here is going to be defining all of the definitions to layer the validations... BUT... here is a starting example (very rough) just meant as a proof of concept Given the following JSON created by bpmn-moddle and doing a {
"$type": "bpmn:Definitions",
"id": "Definitions_1",
"targetNamespace": "http://bpmn.io/schema/bpmn",
"exporter": "Camunda Modeler",
"exporterVersion": "1.11.2",
"rootElements": [
{
"$type": "bpmn:Collaboration",
"id": "Collaboration_12y05km",
"participants": [
{
"$type": "bpmn:Participant",
"id": "Participant_0mcjimc"
},
{
"$type": "bpmn:Participant",
"id": "Participant_0ujwmw1"
}
]
},
{
"$type": "bpmn:Process",
"id": "Process_1",
"isExecutable": true,
"flowElements": [
{
"$type": "bpmn:StartEvent",
"id": "StartEvent_1"
},
{
"$type": "bpmn:Task",
"id": "Task_1rahgaz",
"name": "do something",
"extensionElements": {
"$type": "bpmn:ExtensionElements",
"values": [
{
"$type": "camunda:properties",
"$children": [
{
"$type": "camunda:property",
"value": "dog"
}
]
}
]
}
},
{
"$type": "bpmn:Task",
"id": "Task_0mvt8tb",
"name": "do something else",
"extensionElements": {
"$type": "bpmn:ExtensionElements",
"values": [
{
"$type": "camunda:properties",
"$children": [
{
"$type": "camunda:property",
"value": "cat"
}
]
}
]
}
},
{
"$type": "bpmn:EndEvent",
"id": "EndEvent_0rad2du"
},
{
"$type": "bpmn:ScriptTask",
"id": "Task_01jj8vs",
"name": "run a script",
"scriptFormat": "javascript1",
"script": "111"
},
{
"$type": "bpmn:SequenceFlow",
"id": "SequenceFlow_12encqd"
},
{
"$type": "bpmn:SequenceFlow",
"id": "SequenceFlow_1x999h5"
},
{
"$type": "bpmn:SequenceFlow",
"id": "SequenceFlow_0rszop9"
},
{
"$type": "bpmn:SequenceFlow",
"id": "SequenceFlow_0abg4mt"
}
]
},
{
"$type": "bpmn:Process",
"id": "Process_042g3ej",
"isExecutable": false,
"flowElements": [
{
"$type": "bpmn:Task",
"id": "Task_0rauq3b",
"name": "Do something else else"
},
{
"$type": "bpmn:StartEvent",
"id": "StartEvent_1gio3o0"
},
{
"$type": "bpmn:SequenceFlow",
"id": "SequenceFlow_1oi22b6"
}
]
}
],
"diagrams": [
{
"$type": "bpmndi:BPMNDiagram",
"id": "BPMNDiagram_1",
"plane": {
"$type": "bpmndi:BPMNPlane",
"id": "BPMNPlane_1",
"planeElement": [
{
"$type": "bpmndi:BPMNShape",
"id": "Participant_0mcjimc_di",
"bounds": {
"$type": "dc:Bounds",
"x": 209.5,
"y": 246,
"width": 729,
"height": 250
}
},
{
"$type": "bpmndi:BPMNShape",
"id": "_BPMNShape_StartEvent_2",
"bounds": {
"$type": "dc:Bounds",
"x": 260,
"y": 288,
"width": 36,
"height": 36
},
"label": {
"$type": "bpmndi:BPMNLabel",
"bounds": {
"$type": "dc:Bounds",
"x": 233,
"y": 324,
"width": 90,
"height": 20
}
}
},
{
"$type": "bpmndi:BPMNShape",
"id": "Task_1rahgaz_di",
"bounds": {
"$type": "dc:Bounds",
"x": 365,
"y": 266,
"width": 100,
"height": 80
}
},
{
"$type": "bpmndi:BPMNShape",
"id": "Task_0mvt8tb_di",
"bounds": {
"$type": "dc:Bounds",
"x": 548,
"y": 266,
"width": 100,
"height": 80
}
},
{
"$type": "bpmndi:BPMNShape",
"id": "EndEvent_0rad2du_di",
"bounds": {
"$type": "dc:Bounds",
"x": 883,
"y": 288,
"width": 36,
"height": 36
},
"label": {
"$type": "bpmndi:BPMNLabel",
"bounds": {
"$type": "dc:Bounds",
"x": 901,
"y": 327,
"width": 0,
"height": 13
}
}
},
{
"$type": "bpmndi:BPMNShape",
"id": "ScriptTask_0uq5bj8_di",
"bounds": {
"$type": "dc:Bounds",
"x": 712,
"y": 266,
"width": 100,
"height": 80
}
},
{
"$type": "bpmndi:BPMNEdge",
"id": "SequenceFlow_12encqd_di",
"waypoint": [
{
"$type": "dc:Point",
"x": 296,
"y": 306
},
{
"$type": "dc:Point",
"x": 365,
"y": 306
}
],
"label": {
"$type": "bpmndi:BPMNLabel",
"bounds": {
"$type": "dc:Bounds",
"x": 330.5,
"y": 284,
"width": 0,
"height": 13
}
}
},
{
"$type": "bpmndi:BPMNEdge",
"id": "SequenceFlow_1x999h5_di",
"waypoint": [
{
"$type": "dc:Point",
"x": 465,
"y": 306
},
{
"$type": "dc:Point",
"x": 548,
"y": 306
}
],
"label": {
"$type": "bpmndi:BPMNLabel",
"bounds": {
"$type": "dc:Bounds",
"x": 506.5,
"y": 284,
"width": 0,
"height": 13
}
}
},
{
"$type": "bpmndi:BPMNEdge",
"id": "SequenceFlow_0rszop9_di",
"waypoint": [
{
"$type": "dc:Point",
"x": 648,
"y": 306
},
{
"$type": "dc:Point",
"x": 712,
"y": 306
}
],
"label": {
"$type": "bpmndi:BPMNLabel",
"bounds": {
"$type": "dc:Bounds",
"x": 680,
"y": 284.5,
"width": 0,
"height": 13
}
}
},
{
"$type": "bpmndi:BPMNEdge",
"id": "SequenceFlow_0abg4mt_di",
"waypoint": [
{
"$type": "dc:Point",
"x": 812,
"y": 306
},
{
"$type": "dc:Point",
"x": 848,
"y": 306
},
{
"$type": "dc:Point",
"x": 848,
"y": 306
},
{
"$type": "dc:Point",
"x": 883,
"y": 306
}
],
"label": {
"$type": "bpmndi:BPMNLabel",
"bounds": {
"$type": "dc:Bounds",
"x": 863,
"y": 299.5,
"width": 0,
"height": 13
}
}
},
{
"$type": "bpmndi:BPMNShape",
"id": "Participant_0ujwmw1_di",
"bounds": {
"$type": "dc:Bounds",
"x": 209.5,
"y": 539,
"width": 600,
"height": 250
}
},
{
"$type": "bpmndi:BPMNShape",
"id": "Task_0rauq3b_di",
"bounds": {
"$type": "dc:Bounds",
"x": 409,
"y": 613,
"width": 100,
"height": 80
}
},
{
"$type": "bpmndi:BPMNShape",
"id": "StartEvent_1gio3o0_di",
"bounds": {
"$type": "dc:Bounds",
"x": 279,
"y": 635,
"width": 36,
"height": 36
},
"label": {
"$type": "bpmndi:BPMNLabel",
"bounds": {
"$type": "dc:Bounds",
"x": 297,
"y": 674,
"width": 0,
"height": 13
}
}
},
{
"$type": "bpmndi:BPMNEdge",
"id": "SequenceFlow_1oi22b6_di",
"waypoint": [
{
"$type": "dc:Point",
"x": 315,
"y": 653
},
{
"$type": "dc:Point",
"x": 409,
"y": 653
}
],
"label": {
"$type": "bpmndi:BPMNLabel",
"bounds": {
"$type": "dc:Bounds",
"x": 362,
"y": 631,
"width": 0,
"height": 13
}
}
}
]
}
}
]
} this was based on the following XML:
and so we build the following definition: {
"type": "object",
"properties": {
"rootElements": {
"type": "array",
"items": {
"type": "object",
"properties": {
"flowElements": {
"type": "array",
"items": {
"type": "object",
"properties": {
"$type": {
"enum": [
"bpmn:StartEvent",
"bpmn:Task",
"bpmn:EndEvent",
"bpmn:SequenceFlow",
"bpmn:ScriptTask"
]
},
"id": {
"type": "string"
},
"name": {
"type": "string"
}
},
"required": [
"$type",
"id"
],
"allOf": [
{
"if": {
"properties": {
"$type": {
"enum": [
"bpmn:ScriptTask"
]
}
}
},
"then": {
"required": [
"scriptFormat",
"script"
]
}
},
{
"if": {
"properties": {
"$type": {
"enum": [
"bpmn:ScriptTask"
]
}
}
},
"then": {
"properties": {
"script": {
"type": "string",
"minLength": 1
}
}
}
},
{
"if": {
"properties": {
"$type": {
"enum": [
"bpmn:ScriptTask"
]
}
}
},
"then": {
"properties": {
"scriptFormat": {
"enum": [
"javascript",
"groovy",
"jruby",
"jpython"
]
}
}
}
}
]
}
}
}
}
}
}
} We use the AJV schema validator https://github.com/epoberezkin/ajv and we install the CLI: https://github.com/jessedc/ajv-cli ( Then we run the following in the terminal:
which will return a error such as: myJson1 invalid
[
{
"keyword": "enum",
"dataPath": ".rootElements[1].flowElements[4].scriptFormat",
"schemaPath": "#/properties/rootElements/items/properties/flowElements/items/allOf/2/then/properties/scriptFormat/enum",
"params": {
"allowedValues": [
"javascript",
"groovy",
"jruby",
"jpython"
]
},
"message": "should be equal to one of the allowed values"
}
] |
In my view at the moment the base work would be:
|
@nikku here is a example of validating Camunda Property Extensions: {
"type": "object",
"properties": {
"rootElements": {
"type": "array",
"items": {
"type": "object",
"properties": {
"flowElements": {
"type": "array",
"items": {
"type": "object",
"properties": {
"extensionElements": {
"type": "object",
"properties": {
"$type": {
"type": "string"
},
"values": {
"type": "array",
"items": {
"type": "object",
"properties": {
"$children": {
"type": "array",
"items": {
"type": "object",
"properties": {
"$type": {
"type": "string"
},
"key": {
"type": "string"
},
"value": {
"type": "string"
}
},
"required": [
"key",
"value",
"$type"
]
}
}
}
}
}
}
}
}
}
}
}
}
}
}
} When run against the BPMN listed previously, it would throw the following error: [
{
"keyword": "required",
"dataPath": ".rootElements[1].flowElements[1].extensionElements.values[0].$children[0]",
"schemaPath": "#/properties/rootElements/items/properties/flowElements/items/properties/extensionElements/properties/values/items/properties/%24children/items/required",
"params": {
"missingProperty": "key"
},
"message": "should have required property 'key'"
}
] Based on this sort of pattern is seems like it would be good to create a bunch of meta-schemas (part of the json schema spec) that will provide rapid access to common BPMN elements: various bpmn element types, configurations such as the extension properties, flow elements, etc. |
The design consideration to be figured out here is how to build reusable and extendable schemas against the single tree that is the BPMN JSON. in this schema example: {
"$id": "def2.json",
"definitions": {
"base": {
"$id": "base",
"type": "object",
"properties": {
"$type": {
"enum": [
"bpmn:Definitions"
]
},
"id": {
"type": "string"
},
"targetNamespace": {
"type": "string"
},
"exporter": {
"type": "string"
},
"exporterVersion": {
"type": "string"
},
"rootElements": {
"$ref": "rootElements"
}
}
},
"rootElements": {
"$id": "rootElements",
"type": "array",
"items": {
"anyOf": [
{
"$ref": "collaboration"
}
]
}
},
"collaboration": {
"$id": "collaboration",
"type": "object",
"properties": {
"$type": {
"enum": [
"bpmn:Collaboration"
]
},
"id": {
"type": "string"
},
"participants": {
"type": "array",
"items": {
"anyOf": [
{
"$ref": "collaboration_participants"
}
]
}
}
}
},
"collaboration_participants": {
"$id": "collaboration_participants",
"type": "object",
"properties": {
"$type": {
"enum": [
"bpmn:Participant"
]
},
"id": {
"type": "string"
}
}
}
}
} which is for the following BPMN: {
"$type": "bpmn:Definitions",
"id": "Definitions_1",
"targetNamespace": "http://bpmn.io/schema/bpmn",
"exporter": "Camunda Modeler",
"exporterVersion": "1.11.2",
"rootElements": [
{
"$type": "bpmn:Collaboration",
"id": "Collaboration_12y05km",
"participants": [
{
"$type": "bpmn:Participant",
"id": "Participant_0mcjimc",
"dog":"111"
},
{
"$type": "bpmn:Participant",
"id": "Participant_0ujwmw1",
"dog":"22"
}
]
}
]
} It becomes (as far as i can tell from the current spec), to be able to extend the |
@nikku any thoughts? |
Thanks for providing such a comprehensive list of examples. The JSON schema validation approach you present seems not too bad to me for the few simple cases you present. That said, I'd probably never be the best friend of such schema matching, as it most of the time turns out to be way to verbose for my taste. So if I had a wish I'd personally prefer to build an API that allows linters to be programmed, too. Both approaches don't need to exclude each other, of course. More complex examples, such as the ones I've mentioned previously may not be that easy to capture but are still absolutely in the scope of a good linter:
|
I guess it boils down to hacking a prototype and checking the actual validation limits. I'll prototype the AST / tree visitor based approach if I find the time to. |
@nikku Have you done any performance benchmarks on the time it takes to generate json from the XML defintion? Large vs small definitions, multiple pools, etc. and then doing a Partial vs Full conversion (if there is such a thing): example: Can we drastically reduce the json meta object creation time if only subsets were generated (such as: "all user tasks", all sequence flows, all tasks that are external, each pool without the subset of data within the pool, etc): The biggest issue i have with the JSON schema approach is the perceived complexity and duplication that would be involved in writing the various schema rules. It would be a process of essentially writing much of the same boilerplate over and over again creating a very large pyramid of json. If you write a custom linter, I foresee you would be dealing with some of the same issues: continual conversion of the xml into static code/static json for json eval. Assuming that the continual conversion of the xml into json meta objects becomes a cpu intensive issue: Any thoughts on following a listener approach? Allowing for many common listeners to be created, and schemas can be attached to those listeners. The listeners would only provide the sub-set of data, and only be firing when something is registered to that listener. So someone could attach at the parent level, but they could also just attach to a list of sequence flows and eval the sequence flows similar to your "No duplicate sequence flows from A to B".
What is too verbose? The schema tools like ajv can provide really customized responses and of different styles. So the verbosity imo would really just be about what level of rule is written. Are there any examples that come to mind where the schema apporach would not work? (where a more classic linter would be more appropriate? |
As an example, how would the following structural rule be implemented using JSON schema?
This is the option I'd prefer, I just called it AST / tree based visitor approach. In the best case a validator only traverses the object model (be it JSON / AST / XML / object tree) once and all validators are able to hook in where appropriate for them. Whether they apply a JSON schema validation on a subset of the tree or do the validation programmatically is an implementation detail. |
What I'd go for is the following:
For lint rules this has the following implications:
|
In my understanding of this rule: "After a XOR for the outgoing sequence flows, you are checking if each one has a configuration (script, expression, or delegate)" Assuming this is correct, my schema would be something like: "For each sequence flow object in the array of sequence flows that are outbound on an XOR, each object in the array must have
You can see this type of rule here: ...
"collaboration": {
"$id": "collaboration",
"type": "object",
"properties": {
"$type": {
"enum": [
"bpmn:Collaboration"
]
},
"id": {
"type": "string"
},
"participants": {
"type": "array",
"items": {
"anyOf": [
{
"$ref": "collaboration_participants"
}
]
}
}
}
}
... Which is a example from above. The anyOf is doing a ref to the collaboration_participants definition. In the XOR condition we are creating definitions for each of the rules (script, expression, delegate), and then setting the validation as Additionally would likely be adding some additional logic for being able to handle default outgoing flow, as a config is not required on the |
In this example, are you assuming that the linter is a custom engine/api or something like esLinter ? and that rules are a custom rules engine/some sort of function execution? |
ESlint rules are simple functions, something like this: // 'foo.notAllowed' rule implementation
function nodeVisitor(node, context) {
if (node.type === 'FOO') {
context.addError('foo.notAllowed');
}
} In my understanding what linters such as ESLint ensure is that:
I'd build something similar with |
@nikku so i guess this issue can be closed in favour of the BPMN linter you have started to release over the past few days? |
Let's keep this issue open to track a proper integration of the linting functionality into the Camunda Modeler. |
In the mean time you're welcome to checkout bpmnlint. We'd appreciate your feedback! |
Available as a plug-in. |
Would be a really interesting feature to expose JSLint / ES Lint style functionality in the Modeler.
A sort of mix between https://github.com/umb/camunda-modeler-property-info-plugin and Es Lint, where we can define rules in a file (similar to how plugins currently function for the modeler), and warning or error icons can be shown where lint issues were detected.
Examples can be:
etc.
The idea being that a generic set of rules can be created and reused by the community, but many of the rules will be tailored per project.
The lint confg file could also be detected in the same folder as the bpmn file, allowing for per bpmn file configurations rather than a overall modeler configuration.
The text was updated successfully, but these errors were encountered: