From 85ad313fc1a6b7d56d1183cfe777b0bbbcf523d4 Mon Sep 17 00:00:00 2001 From: Maciej Barelkowski Date: Mon, 15 Jan 2024 17:30:47 +0100 Subject: [PATCH] feat: disallow condition depending on the containing property Closes #125 --- .../src/base-error-messages.json | 15 ++ .../src/defs/base-properties.json | 10 +- .../src/defs/condition.json | 195 +++++++++++------- .../test/helpers/index.js | 3 +- .../fixtures/condition-allMatch-on-itself.js | 69 +++++++ .../test/fixtures/condition-on-itself.js | 64 ++++++ .../test/fixtures/condition.js | 1 + .../test/spec/validationSpec.js | 8 + 8 files changed, 287 insertions(+), 78 deletions(-) create mode 100644 packages/zeebe-element-templates-json-schema/test/fixtures/condition-allMatch-on-itself.js create mode 100644 packages/zeebe-element-templates-json-schema/test/fixtures/condition-on-itself.js diff --git a/packages/element-templates-json-schema-shared/src/base-error-messages.json b/packages/element-templates-json-schema-shared/src/base-error-messages.json index d264d6b..7c72cc2 100644 --- a/packages/element-templates-json-schema-shared/src/base-error-messages.json +++ b/packages/element-templates-json-schema-shared/src/base-error-messages.json @@ -149,5 +149,20 @@ "property": "missing property name for condition" } } + }, + { + "path": [ + "definitions", + "properties", + "allOf", + 0, + "items", + "allOf", + 1, + "allOf", + 0, + "then" + ], + "errorMessage": "Invalid condition.property, must be different than property.id" } ] \ No newline at end of file diff --git a/packages/element-templates-json-schema-shared/src/defs/base-properties.json b/packages/element-templates-json-schema-shared/src/defs/base-properties.json index 3a4288b..ea237dd 100644 --- a/packages/element-templates-json-schema-shared/src/defs/base-properties.json +++ b/packages/element-templates-json-schema-shared/src/defs/base-properties.json @@ -24,13 +24,12 @@ "choices" ] } + }, + { + "$ref": "condition.json" } ], "properties": { - "id": { - "type": "string", - "description": "Unique identifier of the property." - }, "value": { "$id": "#/properties/property/value", "type": [ @@ -143,9 +142,6 @@ "$id": "#/properties/property/group", "type": "string", "description": "The custom group of a control field." - }, - "condition": { - "$ref": "condition.json" } } } diff --git a/packages/element-templates-json-schema-shared/src/defs/condition.json b/packages/element-templates-json-schema-shared/src/defs/condition.json index f0b16a6..e4b81cd 100644 --- a/packages/element-templates-json-schema-shared/src/defs/condition.json +++ b/packages/element-templates-json-schema-shared/src/defs/condition.json @@ -1,94 +1,149 @@ { - "$id": "#/condition", - "type": "object", - "description": "Condition(s) to activate the binding.", "allOf": [ { - "$ref": "examples.json#/condition" + "if": { + "required": [ + "id", + "condition" + ] + }, + "then": { + "not": { + "properties": { + "condition": { + "anyOf": [ + { + "required": [ + "property" + ], + "properties": { + "property": { + "const": { + "$data": "2/id" + } + } + } + }, + { + "required": [ + "allMatch" + ], + "allMatch": { + "contains": { + "properties": { + "property": { + "const": { + "$data": "2/id" + } + } + } + } + } + } + ] + } + } + } + } } ], - "definitions": { + "properties": { + "id": { + "type": "string", + "description": "Unique identifier of the property." + }, "condition": { "type": "object", - "required": [ - "property" - ], - "properties": { - "type": { - "$id": "#/condition/type", - "const": "simple", - "description": "The type of the condition.", - "default": "simple" - }, - "property": { - "$id": "#/condition/property", - "type": "string", - "description": "The id of the property to check." - } - }, - "oneOf": [ + "description": "Condition(s) to activate the binding.", + "allOf": [ { + "$ref": "examples.json#/condition" + } + ], + "definitions": { + "condition": { + "type": "object", + "required": [ + "property" + ], "properties": { - "equals": { - "type": [ - "string", - "number", - "boolean" - ] + "type": { + "$id": "#/condition/type", + "const": "simple", + "description": "The type of the condition.", + "default": "simple" + }, + "property": { + "$id": "#/condition/property", + "type": "string", + "description": "The id of the property to check." } }, - "required": [ - "equals" + "oneOf": [ + { + "properties": { + "equals": { + "type": [ + "string", + "number", + "boolean" + ] + } + }, + "required": [ + "equals" + ] + }, + { + "properties": { + "oneOf": { + "type": "array", + "items": { + "type": [ + "string", + "number" + ] + } + } + }, + "required": [ + "oneOf" + ] + }, + { + "properties": { + "isActive": { + "type": "boolean", + "description": "For `true`, activates the property when given property is active" + } + }, + "required": [ + "isActive" + ] + } ] + } + }, + "oneOf": [ + { + "$ref": "#/properties/condition/definitions/condition" }, { "properties": { - "oneOf": { + "allMatch": { + "$id": "#/allMatch", "type": "array", "items": { - "type": [ - "string", - "number" - ] - } + "$ref": "#/properties/condition/definitions/condition" + }, + "minItems": 1 } }, "required": [ - "oneOf" - ] - }, - { - "properties": { - "isActive": { - "type": "boolean", - "description": "For `true`, activates the property when given property is active" - } - }, - "required": [ - "isActive" + "allMatch" ] } ] } - }, - "oneOf": [ - { - "$ref": "#/definitions/condition" - }, - { - "properties": { - "allMatch": { - "$id": "#/allMatch", - "type": "array", - "items": { - "$ref": "#/definitions/condition" - }, - "minItems": 1 - } - }, - "required": [ - "allMatch" - ] - } - ] -} \ No newline at end of file + } +} diff --git a/packages/element-templates-json-schema-shared/test/helpers/index.js b/packages/element-templates-json-schema-shared/test/helpers/index.js index 1a3eb5c..f0f484f 100644 --- a/packages/element-templates-json-schema-shared/test/helpers/index.js +++ b/packages/element-templates-json-schema-shared/test/helpers/index.js @@ -15,7 +15,8 @@ function createValidator(schema, errors) { const ajv = new Ajv({ allErrors: true, - strict: false + strict: false, + $data: true }); AjvErrors(ajv); diff --git a/packages/zeebe-element-templates-json-schema/test/fixtures/condition-allMatch-on-itself.js b/packages/zeebe-element-templates-json-schema/test/fixtures/condition-allMatch-on-itself.js new file mode 100644 index 0000000..58cc35a --- /dev/null +++ b/packages/zeebe-element-templates-json-schema/test/fixtures/condition-allMatch-on-itself.js @@ -0,0 +1,69 @@ +export const template = { + 'name': 'Condition', + 'id': 'example.com.condition', + 'appliesTo': [ + 'bpmn:ServiceTask' + ], + 'properties': [ + { + 'id': 'myId', + 'label': 'input 1', + 'type': 'String', + 'binding': { + 'type': 'property', + 'name': 'input1' + }, + 'condition': { + 'allMatch': [ + { + 'type': 'simple', + property: 'myId', + equals: 'text' + } + ] + } + } + ] +}; + +export const errors = [ + { + keyword: 'errorMessage', + dataPath: '/properties/0', + schemaPath: '#/allOf/0/items/allOf/1/allOf/0/then/errorMessage', + params: { + errors: [ + { + keyword: 'not', + dataPath: '/properties/0', + schemaPath: '#/allOf/0/items/allOf/1/allOf/0/then/not', + params: {}, + message: 'should NOT be valid', + emUsed: true + } + ] + }, + message: 'Invalid condition.property, must be different than property.id' + }, + { + keyword: 'if', + dataPath: '/properties/0', + schemaPath: '#/allOf/0/items/allOf/1/allOf/0/if', + params: { failingKeyword: 'then' }, + message: 'should match "then" schema' + }, + { + keyword: 'type', + dataPath: '', + schemaPath: '#/oneOf/1/type', + params: { type: 'array' }, + message: 'should be array' + }, + { + keyword: 'oneOf', + dataPath: '', + schemaPath: '#/oneOf', + params: { passingSchemas: null }, + message: 'should match exactly one schema in oneOf' + } +]; diff --git a/packages/zeebe-element-templates-json-schema/test/fixtures/condition-on-itself.js b/packages/zeebe-element-templates-json-schema/test/fixtures/condition-on-itself.js new file mode 100644 index 0000000..4c15a4c --- /dev/null +++ b/packages/zeebe-element-templates-json-schema/test/fixtures/condition-on-itself.js @@ -0,0 +1,64 @@ +export const template = { + 'name': 'Condition', + 'id': 'example.com.condition', + 'appliesTo': [ + 'bpmn:ServiceTask' + ], + 'properties': [ + { + 'id': 'myId', + 'label': 'input 1', + 'type': 'String', + 'binding': { + 'type': 'property', + 'name': 'input1' + }, + 'condition': { + property: 'myId', + equals: 'text' + } + } + ] +}; + +export const errors = [ + { + keyword: 'errorMessage', + dataPath: '/properties/0', + schemaPath: '#/allOf/0/items/allOf/1/allOf/0/then/errorMessage', + params: { + errors: [ + { + keyword: 'not', + dataPath: '/properties/0', + schemaPath: '#/allOf/0/items/allOf/1/allOf/0/then/not', + params: {}, + message: 'should NOT be valid', + emUsed: true + } + ] + }, + message: 'Invalid condition.property, must be different than property.id' + }, + { + keyword: 'if', + dataPath: '/properties/0', + schemaPath: '#/allOf/0/items/allOf/1/allOf/0/if', + params: { failingKeyword: 'then' }, + message: 'should match "then" schema' + }, + { + keyword: 'type', + dataPath: '', + schemaPath: '#/oneOf/1/type', + params: { type: 'array' }, + message: 'should be array' + }, + { + keyword: 'oneOf', + dataPath: '', + schemaPath: '#/oneOf', + params: { passingSchemas: null }, + message: 'should match exactly one schema in oneOf' + } +]; diff --git a/packages/zeebe-element-templates-json-schema/test/fixtures/condition.js b/packages/zeebe-element-templates-json-schema/test/fixtures/condition.js index 0a91480..7cf9101 100644 --- a/packages/zeebe-element-templates-json-schema/test/fixtures/condition.js +++ b/packages/zeebe-element-templates-json-schema/test/fixtures/condition.js @@ -80,6 +80,7 @@ export const template = { } }, { + 'id': 'someId', 'label': 'default condition type', 'type': 'String', 'condition': { diff --git a/packages/zeebe-element-templates-json-schema/test/spec/validationSpec.js b/packages/zeebe-element-templates-json-schema/test/spec/validationSpec.js index 4503863..88135fc 100644 --- a/packages/zeebe-element-templates-json-schema/test/spec/validationSpec.js +++ b/packages/zeebe-element-templates-json-schema/test/spec/validationSpec.js @@ -48,6 +48,8 @@ describe('validation', function() { errors } = validateTemplate(template); + printNested(errors); + // then expect(errors).to.eql(expectedErrors); }); @@ -289,6 +291,12 @@ describe('validation', function() { testTemplate('condition-dropdown-choices-invalid'); + + + testTemplate('condition-on-itself'); + + + testTemplate('condition-allMatch-on-itself'); });