Skip to content

Commit

Permalink
fix(element-templates): always log id and name with validation error
Browse files Browse the repository at this point in the history 
related to camunda/camunda-modeler#2111
fix upstream camunda/camunda-modeler#2111
  • Loading branch information
@MaxTru
MaxTru committed Feb 23, 2021
1 parent 487a1e5 commit 79c7cac
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 54 deletions.
76 changes: 51 additions & 25 deletions lib/provider/camunda/element-templates/Validator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,26 +110,25 @@ function Validator() {
schemaVersion = template.$schema && getSchemaVersion(template.$schema);

if (!id) {
return this._logError('missing template id');
return this._logError('missing template id', template);
}

if (!name) {
return this._logError('missing template name');
return this._logError('missing template name', template);
}

if (schemaVersion &&
(semver.compare(SUPPORTED_SCHEMA_VERSION, schemaVersion) < 0)) {
return this._logError('template name <' + name + '> with id <' + id +
'> uses element template schema version <' + schemaVersion +
'>. Your installation only supports up to version <' + SUPPORTED_SCHEMA_VERSION +
'>. Please update your installation.');
return this._logError('unsupported element template schema version <' + schemaVersion +
'>. Your installation only supports up to version <' + SUPPORTED_SCHEMA_VERSION +
'>. Please update your installation', template);
}

if (this._templatesById[ id ] && this._templatesById[ id ][ version ]) {
if (version === '_') {
return this._logError('template id <' + id + '> already used');
return this._logError('template id <' + id + '> already used', template);
} else {
return this._logError('template id <' + id + '> and version <' + version + '> already used');
return this._logError('template id <' + id + '> and version <' + version + '> already used', template);
}
}

Expand All @@ -140,7 +139,7 @@ function Validator() {
if (!isArray(properties)) {
err = this._logError('missing properties=[]', template);
} else {
if (!this._validateProperties(properties)) {
if (!this._validateProperties(template, properties)) {
err = new Error('invalid properties');
}
}
Expand All @@ -152,6 +151,14 @@ function Validator() {
return err;
};

/**
* Validate given scopes and return error (if any).
*
* @param {TemplateDescriptor} template
* @param {ScopesDescriptor} scopes
*
* @return {Error} validation error, if any
*/
this._validateScopes = function(template, scopes) {

var err,
Expand All @@ -174,7 +181,7 @@ function Validator() {
'missing properties=[] in scope <' + scopeName + '>', template
);
} else {
if (!this._validateProperties(scope.properties)) {
if (!this._validateProperties(template, scope.properties)) {
err = new Error('invalid properties in scope <' + scopeName + '>');
}
}
Expand All @@ -186,12 +193,13 @@ function Validator() {
/**
* Validate properties and return false if any is invalid.
*
* @param {TemplateDescriptor} template
* @param {Array<PropertyDescriptor>} properties
*
* @return {Boolean} true if all properties are valid
*/
this._validateProperties = function(properties) {
var validProperties = properties.filter(this._validateProperty, this);
this._validateProperties = function(template, properties) {
var validProperties = properties.filter(function(ele) { return this._validateProperty(template, ele); }, this);

return properties.length === validProperties.length;
};
Expand All @@ -200,11 +208,12 @@ function Validator() {
* Validate property and return false, if there was
* a validation error.
*
* @param {TemplateDescriptor} template
* @param {PropertyDescriptor} property
*
* @return {Boolean} true if property is valid
*/
this._validateProperty = function(property) {
this._validateProperty = function(template, property) {

var type = property.type,
binding = property.binding;
Expand All @@ -216,33 +225,37 @@ function Validator() {
if (type && VALID_TYPES.indexOf(type) === -1) {
err = this._logError(
'invalid property type <' + type + '>; ' +
'must be any of { ' + VALID_TYPES.join(', ') + ' }'
'must be any of { ' + VALID_TYPES.join(', ') + ' }',
template
);
}

if (type === DROPDOWN_TYPE && bindingType !== CAMUNDA_EXECUTION_LISTENER) {
if (!isArray(property.choices)) {
err = this._logError(
'must provide choices=[] with ' + DROPDOWN_TYPE + ' type'
'must provide choices=[] with ' + DROPDOWN_TYPE + ' type',
template
);
} else

if (!property.choices.every(isDropdownChoiceValid)) {
err = this._logError(
'{ name, value } must be specified for ' +
DROPDOWN_TYPE + ' choices'
DROPDOWN_TYPE + ' choices',
template
);
}
}

if (!binding) {
return this._logError('property missing binding');
return this._logError('property missing binding', template);
}

if (VALID_BINDING_TYPES.indexOf(bindingType) === -1) {
err = this._logError(
'invalid property.binding type <' + bindingType + '>; ' +
'must be any of { ' + VALID_BINDING_TYPES.join(', ') + ' }'
'must be any of { ' + VALID_BINDING_TYPES.join(', ') + ' }',
template
);
}

Expand All @@ -253,15 +266,17 @@ function Validator() {

if (!binding.name) {
err = this._logError(
'property.binding <' + bindingType + '> requires name'
'property.binding <' + bindingType + '> requires name',
template
);
}
}

if (bindingType === CAMUNDA_OUTPUT_PARAMETER_TYPE) {
if (!binding.source) {
err = this._logError(
'property.binding <' + bindingType + '> requires source'
'property.binding <' + bindingType + '> requires source',
template
);
}
}
Expand All @@ -271,7 +286,8 @@ function Validator() {
if (!binding.variables && !binding.target) {
err = this._logError(
'property.binding <' + bindingType + '> requires ' +
'variables or target'
'variables or target',
template
);
}
}
Expand All @@ -281,7 +297,8 @@ function Validator() {
if (!binding.variables && !binding.source && !binding.sourceExpression) {
err = this._logError(
'property.binding <' + bindingType + '> requires ' +
'variables, sourceExpression or source'
'variables, sourceExpression or source',
template
);
}
}
Expand All @@ -291,20 +308,29 @@ function Validator() {
if (type && type !== 'Hidden') {
err = this._logError(
'invalid property type <' + type + '> for ' + CAMUNDA_EXECUTION_LISTENER + '; ' +
'must be <Hidden>'
'must be <Hidden>',
template
);
}
}

return !err;
};


/**
* Log an error for the given template
*
* @param {(String|Error)} err
* @param {TemplateDescriptor} template
*
* @return {Error} logged validation errors
*/
this._logError = function(err, template) {

if (typeof err === 'string') {

if (template) {
err = 'template(id: ' + template.id + ') ' + err;
err = 'template(id: <' + template.id + '>, name: <' + template.name + '>): ' + err;
}

err = new Error(err);
Expand Down
6 changes: 3 additions & 3 deletions test/spec/provider/camunda/element-templates/ElementTemplatesLoaderSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ describe('element-templates - ElementTemplatesLoader', function() {
var errors = e.errors;

expect(messages(errors)).to.eql([
'template id <foo> already used',
'template id <foo> already used'
'template(id: <foo>, name: <Foo>): template id <foo> already used',
'template(id: <foo>, name: <Foo>): template id <foo> already used'
]);
});

Expand Down Expand Up @@ -221,4 +221,4 @@ function messages(errors) {
return errors.map(function(e) {
return e.message;
});
}
}
52 changes: 26 additions & 26 deletions test/spec/provider/camunda/element-templates/ValidatorSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ describe('element-templates - Validator', function() {
// then
expect(errors(templates)).to.have.length(6);

expect(errors(templates)[0]).to.eql('template name <Foo> with id <foo> uses element template schema version <99.99.99>. Your installation only supports up to version <0.2.0>. Please update your installation.');
expect(errors(templates)[0]).to.eql('template(id: <foo>, name: <Foo>): unsupported element template schema version <99.99.99>. Your installation only supports up to version <0.2.0>. Please update your installation');
});

});
Expand Down Expand Up @@ -226,7 +226,7 @@ describe('element-templates - Validator', function() {
templates.addAll(templateDescriptors);

// then
expect(errors(templates)).to.contain('missing template name');
expect(errors(templates)).to.contain('template(id: <invalid>, name: <undefined>): missing template name');

expect(valid(templates)).to.be.empty;
});
Expand All @@ -243,7 +243,7 @@ describe('element-templates - Validator', function() {
templates.addAll(templateDescriptors);

// then
expect(errors(templates)).to.contain('missing template id');
expect(errors(templates)).to.contain('template(id: <undefined>, name: <Invalid>): missing template id');

expect(valid(templates)).to.be.empty;
});
Expand All @@ -260,7 +260,7 @@ describe('element-templates - Validator', function() {
templates.addAll(templateDescriptors);

// then
expect(errors(templates)).to.contain('template id <foo> already used');
expect(errors(templates)).to.contain('template(id: <foo>, name: <Foo 2>): template id <foo> already used');

expect(valid(templates)).to.have.length(1);
});
Expand All @@ -277,7 +277,7 @@ describe('element-templates - Validator', function() {
templates.addAll(templateDescriptors);

// then
expect(errors(templates)).to.contain('template id <foo> and version <1> already used');
expect(errors(templates)).to.contain('template(id: <foo>, name: <Foo 2>): template id <foo> and version <1> already used');

expect(valid(templates)).to.have.length(1);
});
Expand All @@ -294,7 +294,7 @@ describe('element-templates - Validator', function() {
templates.addAll(templateDescriptors);

// then
expect(errors(templates)).to.contain('template(id: foo) missing appliesTo=[]');
expect(errors(templates)).to.contain('template(id: <foo>, name: <Invalid>): missing appliesTo=[]');

expect(valid(templates)).to.be.empty;
});
Expand All @@ -311,7 +311,7 @@ describe('element-templates - Validator', function() {
templates.addAll(templateDescriptors);

// then
expect(errors(templates)).to.contain('template(id: foo) missing properties=[]');
expect(errors(templates)).to.contain('template(id: <foo>, name: <Invalid>): missing properties=[]');

expect(valid(templates)).to.be.empty;
});
Expand All @@ -329,7 +329,7 @@ describe('element-templates - Validator', function() {

// then
expect(errors(templates)).to.eql([
'must provide choices=[] with Dropdown type'
'template(id: <bar>, name: <Task>): must provide choices=[] with Dropdown type'
]);

expect(valid(templates)).to.be.empty;
Expand All @@ -348,7 +348,7 @@ describe('element-templates - Validator', function() {

// then
expect(errors(templates)).to.eql([
'{ name, value } must be specified for Dropdown choices'
'template(id: <bar>, name: <Task>): { name, value } must be specified for Dropdown choices'
]);

expect(valid(templates)).to.be.empty;
Expand All @@ -367,10 +367,10 @@ describe('element-templates - Validator', function() {

// then
expect(errors(templates)).to.eql([
'invalid property type <InvalidType>; must be any of { ' +
'template(id: <foo>, name: <Invalid>): invalid property type <InvalidType>; must be any of { ' +
'String, Text, Boolean, Hidden, Dropdown ' +
'}',
'invalid property.binding type <alsoInvalid>; must be any of { ' +
'template(id: <foo>, name: <Invalid>): invalid property.binding type <alsoInvalid>; must be any of { ' +
'property, camunda:property, camunda:inputParameter, ' +
'camunda:outputParameter, camunda:in, camunda:out, ' +
'camunda:in:businessKey, camunda:executionListener, ' +
Expand All @@ -394,12 +394,12 @@ describe('element-templates - Validator', function() {

// then
expect(errors(templates)).to.eql([
'property.binding <property> requires name',
'property.binding <camunda:property> requires name',
'property.binding <camunda:inputParameter> requires name',
'property.binding <camunda:outputParameter> requires source',
'property.binding <camunda:in> requires variables or target',
'property.binding <camunda:out> requires variables, sourceExpression or source'
'template(id: <foo>, name: <Invalid>): property.binding <property> requires name',
'template(id: <foo>, name: <Invalid>): property.binding <camunda:property> requires name',
'template(id: <foo>, name: <Invalid>): property.binding <camunda:inputParameter> requires name',
'template(id: <foo>, name: <Invalid>): property.binding <camunda:outputParameter> requires source',
'template(id: <foo>, name: <Invalid>): property.binding <camunda:in> requires variables or target',
'template(id: <foo>, name: <Invalid>): property.binding <camunda:out> requires variables, sourceExpression or source'
]);

expect(valid(templates)).to.be.empty;
Expand Down Expand Up @@ -452,10 +452,10 @@ describe('element-templates - Validator', function() {

// then
expect(errors(templates)).to.eql([
'invalid property type <String> for camunda:executionListener; must be <Hidden>',
'invalid property type <Text> for camunda:executionListener; must be <Hidden>',
'invalid property type <Boolean> for camunda:executionListener; must be <Hidden>',
'invalid property type <Dropdown> for camunda:executionListener; must be <Hidden>'
'template(id: <my.execution.listener.task>, name: <Execution Listener>): invalid property type <String> for camunda:executionListener; must be <Hidden>',
'template(id: <my.execution.listener.task>, name: <Execution Listener>): invalid property type <Text> for camunda:executionListener; must be <Hidden>',
'template(id: <my.execution.listener.task>, name: <Execution Listener>): invalid property type <Boolean> for camunda:executionListener; must be <Hidden>',
'template(id: <my.execution.listener.task>, name: <Execution Listener>): invalid property type <Dropdown> for camunda:executionListener; must be <Hidden>'
]);

expect(valid(templates)).to.have.length(0);
Expand All @@ -473,7 +473,7 @@ describe('element-templates - Validator', function() {
templates.addAll(templateDescriptors);

// then
expect(errors(templates)).to.contain('template(id: foo) invalid scopes, should be scopes={}');
expect(errors(templates)).to.contain('template(id: <foo>, name: <Invalid>): invalid scopes, should be scopes={}');

expect(valid(templates)).to.be.empty;
});
Expand All @@ -490,7 +490,7 @@ describe('element-templates - Validator', function() {
templates.addAll(templateDescriptors);

// then
expect(errors(templates)).to.contain('template(id: foo) invalid scope, should be scope={}');
expect(errors(templates)).to.contain('template(id: <foo>, name: <Invalid>): invalid scope, should be scope={}');

expect(valid(templates)).to.be.empty;
});
Expand All @@ -508,7 +508,7 @@ describe('element-templates - Validator', function() {

// then
expect(errors(templates)).to.contain(
'template(id: foo) missing properties=[] in scope <camunda:Connector>'
'template(id: <foo>, name: <Invalid>): missing properties=[] in scope <camunda:Connector>'
);

expect(valid(templates)).to.be.empty;
Expand All @@ -527,10 +527,10 @@ describe('element-templates - Validator', function() {

// then
expect(errors(templates)).to.eql([
'invalid property type <InvalidType>; must be any of { ' +
'template(id: <foo>, name: <Invalid>): invalid property type <InvalidType>; must be any of { ' +
'String, Text, Boolean, Hidden, Dropdown ' +
'}',
'invalid property.binding type <alsoInvalid>; must be any of { ' +
'template(id: <foo>, name: <Invalid>): invalid property.binding type <alsoInvalid>; must be any of { ' +
'property, camunda:property, camunda:inputParameter, ' +
'camunda:outputParameter, camunda:in, camunda:out, ' +
'camunda:in:businessKey, camunda:executionListener, ' +
Expand Down

0 comments on commit 79c7cac

Please sign in to comment.