From c2c89e9e3af8651f2e9ee401579222daff1c354a Mon Sep 17 00:00:00 2001 From: Civ Sivakumaran Date: Thu, 28 Apr 2022 12:56:50 +0100 Subject: [PATCH 1/9] feat: add validation for id references for requests only and objects for responses --- CONTRIBUTING.md | 2 +- src/classes/model-node.js | 4 + src/errors/validation-error-type.js | 2 + .../id-references-for-requests-rule-spec.js | 107 ++++++++++++++++++ .../core/id-references-for-requests-rule.js | 75 ++++++++++++ ...o-id-references-for-responses-rule-spec.js | 95 ++++++++++++++++ .../no-id-references-for-responses-rule.js | 74 ++++++++++++ 7 files changed, 358 insertions(+), 1 deletion(-) create mode 100644 src/rules/core/id-references-for-requests-rule-spec.js create mode 100644 src/rules/core/id-references-for-requests-rule.js create mode 100644 src/rules/core/no-id-references-for-responses-rule-spec.js create mode 100644 src/rules/core/no-id-references-for-responses-rule.js diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0c0e2545..4b2b2731 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -195,7 +195,7 @@ this.meta = { }; ``` -### validateModel and validateField +### `validateModel` and `validateField` Only one of these methods is expected to be implemented on each rule. diff --git a/src/classes/model-node.js b/src/classes/model-node.js index 45c3d29b..ef6be9db 100644 --- a/src/classes/model-node.js +++ b/src/classes/model-node.js @@ -181,4 +181,8 @@ const ModelNode = class { } }; +/** + * @typedef {ModelNode} ModelNodeType + */ + module.exports = ModelNode; diff --git a/src/errors/validation-error-type.js b/src/errors/validation-error-type.js index 79e9c46d..29b2b408 100644 --- a/src/errors/validation-error-type.js +++ b/src/errors/validation-error-type.js @@ -43,6 +43,8 @@ const ValidationErrorType = { BELOW_MIN_VALUE_INCLUSIVE: 'below_min_value_inclusive', VALUE_OUTWITH_CONSTRAINT: 'value_outwith_constraint', INVALID_ID: 'invalid_id', + FIELD_NOT_ID_REFERENCE: 'field_not_id_reference', + FIELD_SHOUlD_NOT_BE_ID_REFERENCE: 'field_should_not_be_id_reference', }; module.exports = Object.freeze(ValidationErrorType); diff --git a/src/rules/core/id-references-for-requests-rule-spec.js b/src/rules/core/id-references-for-requests-rule-spec.js new file mode 100644 index 00000000..aee2dace --- /dev/null +++ b/src/rules/core/id-references-for-requests-rule-spec.js @@ -0,0 +1,107 @@ +const Model = require('../../classes/model'); +const ModelNode = require('../../classes/model-node'); +const { IdReferencesForRequestsRule } = require('./id-references-for-requests-rule'); +const OptionsHelper = require('../../helpers/options'); +const ValidationErrorType = require('../../errors/validation-error-type'); +const ValidationErrorSeverity = require('../../errors/validation-error-severity'); + + +describe('IdReferencesForRequestsRule', () => { + const rule = new IdReferencesForRequestsRule(); + + const model = new Model({ + type: 'OrderItem', + }, 'latest'); + model.hasSpecification = true; + + it('should return a failure if a request object does not have `acceptedOffer` as a compact ID reference', async () => { + const options = new OptionsHelper({ validationMode: 'C1Request' }); + const data = { + '@context': 'https://openactive.io/', + '@type': 'OrderItem', + acceptedOffer: { + '@type': 'Offer', + '@id': 'https://example.com/offer/1', + }, + }; + + const nodeToTest = new ModelNode( + '$', + data, + null, + model, + options, + ); + const errors = await rule.validate(nodeToTest); + + expect(errors.length).toBe(1); + expect(errors[0].type).toBe(ValidationErrorType.FIELD_NOT_ID_REFERENCE); + expect(errors[0].severity).toBe(ValidationErrorSeverity.FAILURE); + }); + it('should return a failure if a request object does not have `orderedItem` as a compact ID reference', async () => { + const options = new OptionsHelper({ validationMode: 'C1Request' }); + const data = { + '@context': 'https://openactive.io/', + '@type': 'OrderItem', + orderedItem: { + '@type': 'ScheduledSession', + '@id': 'https://example.com/session/1', + }, + }; + + const nodeToTest = new ModelNode( + '$', + data, + null, + model, + options, + ); + const errors = await rule.validate(nodeToTest); + + expect(errors.length).toBe(1); + expect(errors[0].type).toBe(ValidationErrorType.FIELD_NOT_ID_REFERENCE); + expect(errors[0].severity).toBe(ValidationErrorSeverity.FAILURE); + }); + it('should return no errors if a response object has `acceptedOffer` as a data object', async () => { + const options = new OptionsHelper({ validationMode: 'C1Response' }); + const data = { + '@context': 'https://openactive.io/', + '@type': 'OrderItem', + acceptedOffer: { + '@type': 'ScheduledSession', + '@id': 'https://example.com/offer/1', + }, + }; + + const nodeToTest = new ModelNode( + '$', + data, + null, + model, + options, + ); + const errors = await rule.validate(nodeToTest); + expect(errors.length).toBe(0); + }); + it('should return no errors if a response object has `orderedItem` as a data object', async () => { + const options = new OptionsHelper({ validationMode: 'C1Response' }); + const data = { + '@context': 'https://openactive.io/', + '@type': 'OrderItem', + orderedItem: { + '@type': 'ScheduledSession', + '@id': 'https://example.com/session/1', + }, + }; + + const nodeToTest = new ModelNode( + '$', + data, + null, + model, + options, + ); + const errors = await rule.validate(nodeToTest); + expect(errors.length).toBe(0); + }); +}); diff --git a/src/rules/core/id-references-for-requests-rule.js b/src/rules/core/id-references-for-requests-rule.js new file mode 100644 index 00000000..ebc0ebc9 --- /dev/null +++ b/src/rules/core/id-references-for-requests-rule.js @@ -0,0 +1,75 @@ +const Rule = require('../rule'); +const PropertyHelper = require('../../helpers/property'); +const ValidationErrorCategory = require('../../errors/validation-error-category'); +const ValidationErrorSeverity = require('../../errors/validation-error-severity'); +const validationErrorType = require('../../errors/validation-error-type'); + +/** @typedef {import('../../classes/model-node').ModelNodeType} ModelNode */ + +class IdReferencesForRequestsRule extends Rule { + constructor(options) { + super(options); + this.targetValidationModes = [ + 'C1Request', + 'C2Request', + 'PRequest', + 'BRequest', + 'BOrderProposalRequest', + 'OrderPatch', + ]; + this.targetFields = { OrderItem: ['acceptedOffer', 'orderedItem'] }; + this.meta = { + name: 'IdReferencesForRequestsRule', + description: 'Validates that acceptedOffer and orderedItem are ID references and not objects for requests (C1, C2 etc)', + tests: { + default: { + description: `Raises a failure if the acceptedOffer or orderedItem within the OrderItem of a request is not a URL + (ie a reference to the object and not the object itself)`, + message: 'For requests, {{field}} must be an compact ID reference, not the object representing the data itself', + sampleValues: { + field: 'acceptedOffer', + }, + category: ValidationErrorCategory.CONFORMANCE, + severity: ValidationErrorSeverity.FAILURE, + type: validationErrorType.FIELD_NOT_ID_REFERENCE, + }, + }, + }; + } + + /** + * + * @param {ModelNode} node + * @param {string} field + */ + validateField(node, field) { + // Don't do this check for models that we don't actually have a spec for or for models that aren't JSON-LD + if (!node.model.hasSpecification || !node.model.isJsonLd) { + return []; + } + + const errors = []; + const fieldValue = node.getValue(field); + + if (typeof fieldValue !== 'string' || !PropertyHelper.isUrl(fieldValue)) { + errors.push( + this.createError( + 'default', + { + fieldValue, + path: node.getPath(field), + }, + { field }, + ), + ); + } else { + return []; + } + + return errors; + } +} + +module.exports = { + IdReferencesForRequestsRule, +}; diff --git a/src/rules/core/no-id-references-for-responses-rule-spec.js b/src/rules/core/no-id-references-for-responses-rule-spec.js new file mode 100644 index 00000000..050486ff --- /dev/null +++ b/src/rules/core/no-id-references-for-responses-rule-spec.js @@ -0,0 +1,95 @@ +const Model = require('../../classes/model'); +const ModelNode = require('../../classes/model-node'); +const OptionsHelper = require('../../helpers/options'); +const ValidationErrorType = require('../../errors/validation-error-type'); +const ValidationErrorSeverity = require('../../errors/validation-error-severity'); +const { NoIdReferencesForResponsesRule } = require('./no-id-references-for-responses-rule'); + + +describe('NoIdReferencesForResponsesRule', () => { + const rule = new NoIdReferencesForResponsesRule(); + + const model = new Model({ + type: 'OrderItem', + }, 'latest'); + model.hasSpecification = true; + + it('should return a failure if a response object does not have `acceptedOffer` as a data object', async () => { + const options = new OptionsHelper({ validationMode: 'C1Response' }); + const data = { + '@context': 'https://openactive.io/', + '@type': 'OrderItem', + acceptedOffer: 'https://example.com/offer/1', + }; + + const nodeToTest = new ModelNode( + '$', + data, + null, + model, + options, + ); + const errors = await rule.validate(nodeToTest); + + expect(errors.length).toBe(1); + expect(errors[0].type).toBe(ValidationErrorType.FIELD_SHOUlD_NOT_BE_ID_REFERENCE); + expect(errors[0].severity).toBe(ValidationErrorSeverity.FAILURE); + }); + it('should return a failure if a response object does not have `orderedItem` as a data object', async () => { + const options = new OptionsHelper({ validationMode: 'C1Response' }); + const data = { + '@context': 'https://openactive.io/', + '@type': 'OrderItem', + orderedItem: 'https://example.com/session/1', + }; + + const nodeToTest = new ModelNode( + '$', + data, + null, + model, + options, + ); + const errors = await rule.validate(nodeToTest); + + expect(errors.length).toBe(1); + expect(errors[0].type).toBe(ValidationErrorType.FIELD_SHOUlD_NOT_BE_ID_REFERENCE); + expect(errors[0].severity).toBe(ValidationErrorSeverity.FAILURE); + }); + it('should return no errors if a request object has `acceptedOffer` as a compact ID reference', async () => { + const options = new OptionsHelper({ validationMode: 'C1Request' }); + const data = { + '@context': 'https://openactive.io/', + '@type': 'OrderItem', + acceptedOffer: 'https://example.com/offer/1', + }; + + const nodeToTest = new ModelNode( + '$', + data, + null, + model, + options, + ); + const errors = await rule.validate(nodeToTest); + expect(errors.length).toBe(0); + }); + it('should return no errors if a request object has `orderedItem` as a compact ID reference', async () => { + const options = new OptionsHelper({ validationMode: 'C1Request' }); + const data = { + '@context': 'https://openactive.io/', + '@type': 'OrderItem', + orderedItem: 'https://example.com/session/1', + }; + + const nodeToTest = new ModelNode( + '$', + data, + null, + model, + options, + ); + const errors = await rule.validate(nodeToTest); + expect(errors.length).toBe(0); + }); +}); diff --git a/src/rules/core/no-id-references-for-responses-rule.js b/src/rules/core/no-id-references-for-responses-rule.js new file mode 100644 index 00000000..9a021a8e --- /dev/null +++ b/src/rules/core/no-id-references-for-responses-rule.js @@ -0,0 +1,74 @@ +const Rule = require('../rule'); +const ValidationErrorCategory = require('../../errors/validation-error-category'); +const ValidationErrorSeverity = require('../../errors/validation-error-severity'); +const validationErrorType = require('../../errors/validation-error-type'); + +/** @typedef {import('../../classes/model-node').ModelNodeType} ModelNode */ + +class NoIdReferencesForResponsesRule extends Rule { + constructor(options) { + super(options); + this.targetValidationModes = [ + 'C1Response', + 'C2Response', + 'PResponse', + 'BResponse', + 'OrdersFeed', + 'OrderStatus', + ]; + this.targetFields = { OrderItem: ['acceptedOffer', 'orderedItem'] }; + this.meta = { + name: 'NoIdReferencesForResponsesRule', + description: 'Validates that acceptedOffer and orderedItem are not ID references and not objects for responses (C1, C2 etc)', + tests: { + default: { + description: `Raises a failure if the acceptedOffer or orderedItem within the OrderItem of a response is a URL + (ie a reference to the object and not the object itself)`, + message: 'For responses, {{field}} must not be an compact ID reference, but the object representing the data itself', + sampleValues: { + field: 'acceptedOffer', + }, + category: ValidationErrorCategory.CONFORMANCE, + severity: ValidationErrorSeverity.FAILURE, + type: validationErrorType.FIELD_SHOUlD_NOT_BE_ID_REFERENCE, + }, + }, + }; + } + + /** + * + * @param {ModelNode} node + * @param {string} field + */ + validateField(node, field) { + // Don't do this check for models that we don't actually have a spec for or for models that aren't JSON-LD + if (!node.model.hasSpecification || !node.model.isJsonLd) { + return []; + } + + const errors = []; + const fieldValue = node.getValue(field); + + if (typeof fieldValue !== 'object') { + errors.push( + this.createError( + 'default', + { + fieldValue, + path: node.getPath(field), + }, + { field }, + ), + ); + } else { + return []; + } + + return errors; + } +} + +module.exports = { + NoIdReferencesForResponsesRule, +}; From 21f316e50e6b42980ba5b3b578525d3b08adf10a Mon Sep 17 00:00:00 2001 From: civsiv Date: Mon, 16 May 2022 10:55:41 +0100 Subject: [PATCH 2/9] Update src/rules/core/no-id-references-for-responses-rule.js Co-authored-by: Luke Winship --- src/rules/core/no-id-references-for-responses-rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/core/no-id-references-for-responses-rule.js b/src/rules/core/no-id-references-for-responses-rule.js index 9a021a8e..b2c6262c 100644 --- a/src/rules/core/no-id-references-for-responses-rule.js +++ b/src/rules/core/no-id-references-for-responses-rule.js @@ -19,7 +19,7 @@ class NoIdReferencesForResponsesRule extends Rule { this.targetFields = { OrderItem: ['acceptedOffer', 'orderedItem'] }; this.meta = { name: 'NoIdReferencesForResponsesRule', - description: 'Validates that acceptedOffer and orderedItem are not ID references and not objects for responses (C1, C2 etc)', + description: 'Validates that acceptedOffer and orderedItem are not ID references and are objects for responses (C1, C2 etc)', tests: { default: { description: `Raises a failure if the acceptedOffer or orderedItem within the OrderItem of a response is a URL From 1c1947dd3b27a3b5bdaa907791f6518843b02cb6 Mon Sep 17 00:00:00 2001 From: civsiv Date: Mon, 16 May 2022 10:55:46 +0100 Subject: [PATCH 3/9] Update src/rules/core/id-references-for-requests-rule.js Co-authored-by: Luke Winship --- src/rules/core/id-references-for-requests-rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/core/id-references-for-requests-rule.js b/src/rules/core/id-references-for-requests-rule.js index ebc0ebc9..922d2482 100644 --- a/src/rules/core/id-references-for-requests-rule.js +++ b/src/rules/core/id-references-for-requests-rule.js @@ -25,7 +25,7 @@ class IdReferencesForRequestsRule extends Rule { default: { description: `Raises a failure if the acceptedOffer or orderedItem within the OrderItem of a request is not a URL (ie a reference to the object and not the object itself)`, - message: 'For requests, {{field}} must be an compact ID reference, not the object representing the data itself', + message: 'For requests, {{field}} must be a compact ID reference, not the object representing the data itself', sampleValues: { field: 'acceptedOffer', }, From b3d0a0f025d981f1af7004cc8c7276c370e9a9e4 Mon Sep 17 00:00:00 2001 From: civsiv Date: Mon, 16 May 2022 10:55:51 +0100 Subject: [PATCH 4/9] Update src/rules/core/no-id-references-for-responses-rule.js Co-authored-by: Luke Winship --- src/rules/core/no-id-references-for-responses-rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/core/no-id-references-for-responses-rule.js b/src/rules/core/no-id-references-for-responses-rule.js index b2c6262c..24fa0fe2 100644 --- a/src/rules/core/no-id-references-for-responses-rule.js +++ b/src/rules/core/no-id-references-for-responses-rule.js @@ -24,7 +24,7 @@ class NoIdReferencesForResponsesRule extends Rule { default: { description: `Raises a failure if the acceptedOffer or orderedItem within the OrderItem of a response is a URL (ie a reference to the object and not the object itself)`, - message: 'For responses, {{field}} must not be an compact ID reference, but the object representing the data itself', + message: 'For responses, {{field}} must not be a compact ID reference, but the object representing the data itself', sampleValues: { field: 'acceptedOffer', }, From 58238e64d5e6f51e290a9cc6e40bd6a3f047a84e Mon Sep 17 00:00:00 2001 From: Civ Sivakumaran Date: Mon, 16 May 2022 11:02:51 +0100 Subject: [PATCH 5/9] replace typeof with lodash helper function --- package.json | 2 ++ src/rules/core/no-id-references-for-responses-rule.js | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index bc0a1628..32ddaaeb 100644 --- a/package.json +++ b/package.json @@ -24,10 +24,12 @@ "license": "MIT", "dependencies": { "@openactive/data-models": "^2.0.219", + "@types/lodash": "^4.14.182", "axios": "^0.19.2", "currency-codes": "^1.5.1", "html-entities": "^1.3.1", "jsonpath": "^1.0.2", + "lodash": "^4.17.21", "moment": "^2.24.0", "rrule": "^2.6.2", "striptags": "^3.1.1", diff --git a/src/rules/core/no-id-references-for-responses-rule.js b/src/rules/core/no-id-references-for-responses-rule.js index 9a021a8e..a3cc93b4 100644 --- a/src/rules/core/no-id-references-for-responses-rule.js +++ b/src/rules/core/no-id-references-for-responses-rule.js @@ -1,3 +1,4 @@ +const _ = require('lodash'); const Rule = require('../rule'); const ValidationErrorCategory = require('../../errors/validation-error-category'); const ValidationErrorSeverity = require('../../errors/validation-error-severity'); @@ -50,7 +51,7 @@ class NoIdReferencesForResponsesRule extends Rule { const errors = []; const fieldValue = node.getValue(field); - if (typeof fieldValue !== 'object') { + if (!_.isPlainObject(fieldValue)) { errors.push( this.createError( 'default', From 97ca8a321af08246539f859d74dbb74b58b48f2c Mon Sep 17 00:00:00 2001 From: Civ Sivakumaran Date: Tue, 7 Jun 2022 14:20:17 +0100 Subject: [PATCH 6/9] use referencedFields in data models for compact ID reference tests --- src/classes/model.js | 11 ++++++ .../id-references-for-requests-rule-spec.js | 25 +++++++++++++ .../core/id-references-for-requests-rule.js | 36 +++++++++---------- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/classes/model.js b/src/classes/model.js index 57ace1c2..f5f47a68 100644 --- a/src/classes/model.js +++ b/src/classes/model.js @@ -116,6 +116,17 @@ const Model = class { return this.data.shallNotInclude || []; } + getReferencedFields(validationMode, containingFieldName) { + const specificContextualImperativeConfiguration = this.getImperativeConfigurationWithContext(validationMode, containingFieldName); + const specificImperativeConfiguration = this.getImperativeConfiguration(validationMode); + + if (specificContextualImperativeConfiguration && specificContextualImperativeConfiguration.referencedFields) return specificContextualImperativeConfiguration.referencedFields; + + if (specificImperativeConfiguration && specificImperativeConfiguration.referencedFields) return specificImperativeConfiguration.referencedFields; + + return this.data.referencedFields || []; + } + hasRecommendedField(field) { return PropertyHelper.arrayHasField(this.recommendedFields, field, this.version); } diff --git a/src/rules/core/id-references-for-requests-rule-spec.js b/src/rules/core/id-references-for-requests-rule-spec.js index aee2dace..7da34674 100644 --- a/src/rules/core/id-references-for-requests-rule-spec.js +++ b/src/rules/core/id-references-for-requests-rule-spec.js @@ -11,6 +11,10 @@ describe('IdReferencesForRequestsRule', () => { const model = new Model({ type: 'OrderItem', + referencedFields: [ + 'orderedItem', + 'acceptedOffer', + ], }, 'latest'); model.hasSpecification = true; @@ -23,6 +27,7 @@ describe('IdReferencesForRequestsRule', () => { '@type': 'Offer', '@id': 'https://example.com/offer/1', }, + orderedItem: 'https://example.com/item/2', }; const nodeToTest = new ModelNode( @@ -47,6 +52,7 @@ describe('IdReferencesForRequestsRule', () => { '@type': 'ScheduledSession', '@id': 'https://example.com/session/1', }, + acceptedOffer: 'https://example.com/offer/1', }; const nodeToTest = new ModelNode( @@ -94,6 +100,25 @@ describe('IdReferencesForRequestsRule', () => { }, }; + const nodeToTest = new ModelNode( + '$', + data, + null, + model, + options, + ); + const errors = await rule.validate(nodeToTest); + expect(errors.length).toBe(0); + }); + it('should return no errors if a request object has `orderedItem` as a compact ID reference', async () => { + const options = new OptionsHelper({ validationMode: 'C1Request' }); + const data = { + '@context': 'https://openactive.io/', + '@type': 'OrderItem', + orderedItem: 'https://example.com/session/1', + acceptedOffer: 'https://example.com/offer/1', + }; + const nodeToTest = new ModelNode( '$', data, diff --git a/src/rules/core/id-references-for-requests-rule.js b/src/rules/core/id-references-for-requests-rule.js index 922d2482..3d4380e2 100644 --- a/src/rules/core/id-references-for-requests-rule.js +++ b/src/rules/core/id-references-for-requests-rule.js @@ -17,7 +17,7 @@ class IdReferencesForRequestsRule extends Rule { 'BOrderProposalRequest', 'OrderPatch', ]; - this.targetFields = { OrderItem: ['acceptedOffer', 'orderedItem'] }; + this.targetModels = '*'; this.meta = { name: 'IdReferencesForRequestsRule', description: 'Validates that acceptedOffer and orderedItem are ID references and not objects for requests (C1, C2 etc)', @@ -38,34 +38,34 @@ class IdReferencesForRequestsRule extends Rule { } /** - * * @param {ModelNode} node - * @param {string} field */ - validateField(node, field) { + validateModel(node) { // Don't do this check for models that we don't actually have a spec for or for models that aren't JSON-LD if (!node.model.hasSpecification || !node.model.isJsonLd) { return []; } const errors = []; - const fieldValue = node.getValue(field); + const referencedFields = node.model.getReferencedFields(node.options.validationMode, node.name); + for (const field of referencedFields) { + const fieldValue = node.getValue(field); - if (typeof fieldValue !== 'string' || !PropertyHelper.isUrl(fieldValue)) { - errors.push( - this.createError( - 'default', - { - fieldValue, - path: node.getPath(field), - }, - { field }, - ), - ); - } else { - return []; + if (typeof fieldValue !== 'string' || !PropertyHelper.isUrl(fieldValue)) { + errors.push( + this.createError( + 'default', + { + fieldValue, + path: node.getPath(field), + }, + { referencedField: field }, + ), + ); + } } + return errors; } } From 2abee72df951814494955bac8b520d9d2abe3b22 Mon Sep 17 00:00:00 2001 From: Civ Sivakumaran Date: Fri, 10 Jun 2022 15:20:20 +0100 Subject: [PATCH 7/9] add rules to index --- src/rules/core/id-references-for-requests-rule-spec.js | 2 +- src/rules/core/id-references-for-requests-rule.js | 4 +--- src/rules/core/no-id-references-for-responses-rule-spec.js | 2 +- src/rules/core/no-id-references-for-responses-rule.js | 4 +--- src/rules/index.js | 2 ++ 5 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/rules/core/id-references-for-requests-rule-spec.js b/src/rules/core/id-references-for-requests-rule-spec.js index 7da34674..cb207dfd 100644 --- a/src/rules/core/id-references-for-requests-rule-spec.js +++ b/src/rules/core/id-references-for-requests-rule-spec.js @@ -1,6 +1,6 @@ const Model = require('../../classes/model'); const ModelNode = require('../../classes/model-node'); -const { IdReferencesForRequestsRule } = require('./id-references-for-requests-rule'); +const IdReferencesForRequestsRule = require('./id-references-for-requests-rule'); const OptionsHelper = require('../../helpers/options'); const ValidationErrorType = require('../../errors/validation-error-type'); const ValidationErrorSeverity = require('../../errors/validation-error-severity'); diff --git a/src/rules/core/id-references-for-requests-rule.js b/src/rules/core/id-references-for-requests-rule.js index 3d4380e2..374607d6 100644 --- a/src/rules/core/id-references-for-requests-rule.js +++ b/src/rules/core/id-references-for-requests-rule.js @@ -70,6 +70,4 @@ class IdReferencesForRequestsRule extends Rule { } } -module.exports = { - IdReferencesForRequestsRule, -}; +module.exports = IdReferencesForRequestsRule; diff --git a/src/rules/core/no-id-references-for-responses-rule-spec.js b/src/rules/core/no-id-references-for-responses-rule-spec.js index 050486ff..bc7b57c6 100644 --- a/src/rules/core/no-id-references-for-responses-rule-spec.js +++ b/src/rules/core/no-id-references-for-responses-rule-spec.js @@ -3,7 +3,7 @@ const ModelNode = require('../../classes/model-node'); const OptionsHelper = require('../../helpers/options'); const ValidationErrorType = require('../../errors/validation-error-type'); const ValidationErrorSeverity = require('../../errors/validation-error-severity'); -const { NoIdReferencesForResponsesRule } = require('./no-id-references-for-responses-rule'); +const NoIdReferencesForResponsesRule = require('./no-id-references-for-responses-rule'); describe('NoIdReferencesForResponsesRule', () => { diff --git a/src/rules/core/no-id-references-for-responses-rule.js b/src/rules/core/no-id-references-for-responses-rule.js index c148b0c7..324aeda3 100644 --- a/src/rules/core/no-id-references-for-responses-rule.js +++ b/src/rules/core/no-id-references-for-responses-rule.js @@ -70,6 +70,4 @@ class NoIdReferencesForResponsesRule extends Rule { } } -module.exports = { - NoIdReferencesForResponsesRule, -}; +module.exports = NoIdReferencesForResponsesRule; diff --git a/src/rules/index.js b/src/rules/index.js index a7d0e3b3..fea8dfb6 100644 --- a/src/rules/index.js +++ b/src/rules/index.js @@ -23,6 +23,8 @@ module.exports = { require('./core/valueconstraint-rule'), require('./core/minvalueinclusive-rule'), require('./core/id-rule'), + require('./core/id-references-for-requests-rule'), + require('./core/no-id-references-for-responses-rule'), // Formatting rules require('./format/duration-format-rule'), From c00645162d6e2e01fe75974ea2ee9797e65bba6f Mon Sep 17 00:00:00 2001 From: Civ Sivakumaran Date: Fri, 10 Jun 2022 16:08:53 +0100 Subject: [PATCH 8/9] add shallNotBeReferencedFields functionality --- src/classes/model.js | 11 ++++ ...o-id-references-for-responses-rule-spec.js | 58 +++++++++++++++++++ .../no-id-references-for-responses-rule.js | 35 ++++++----- 3 files changed, 86 insertions(+), 18 deletions(-) diff --git a/src/classes/model.js b/src/classes/model.js index f5f47a68..7fe2b64f 100644 --- a/src/classes/model.js +++ b/src/classes/model.js @@ -127,6 +127,17 @@ const Model = class { return this.data.referencedFields || []; } + getShallNotBeReferencedFields(validationMode, containingFieldName) { + const specificContextualImperativeConfiguration = this.getImperativeConfigurationWithContext(validationMode, containingFieldName); + const specificImperativeConfiguration = this.getImperativeConfiguration(validationMode); + + if (specificContextualImperativeConfiguration && specificContextualImperativeConfiguration.shallNotBeReferencedFields) return specificContextualImperativeConfiguration.shallNotBeReferencedFields; + + if (specificImperativeConfiguration && specificImperativeConfiguration.shallNotBeReferencedFields) return specificImperativeConfiguration.shallNotBeReferencedFields; + + return this.data.shallNotBeReferencedFields || []; + } + hasRecommendedField(field) { return PropertyHelper.arrayHasField(this.recommendedFields, field, this.version); } diff --git a/src/rules/core/no-id-references-for-responses-rule-spec.js b/src/rules/core/no-id-references-for-responses-rule-spec.js index bc7b57c6..ae41d45b 100644 --- a/src/rules/core/no-id-references-for-responses-rule-spec.js +++ b/src/rules/core/no-id-references-for-responses-rule-spec.js @@ -11,6 +11,58 @@ describe('NoIdReferencesForResponsesRule', () => { const model = new Model({ type: 'OrderItem', + validationMode: { + C1Request: 'request', + C1Response: 'Cresponse', + }, + imperativeConfiguration: { + request: { + requiredFields: [ + 'type', + 'acceptedOffer', + 'orderedItem', + 'position', + ], + recommendedFields: [], + shallNotInclude: [ + 'id', + 'orderItemStatus', + 'unitTaxSpecification', + 'accessCode', + 'error', + 'cancellationMessage', + 'customerNotice', + 'orderItemIntakeForm', + ], + requiredOptions: [], + referencedFields: [ + 'orderedItem', + 'acceptedOffer', + ], + }, + Cresponse: { + requiredFields: [ + 'type', + 'acceptedOffer', + 'orderedItem', + 'position', + ], + shallNotInclude: [ + 'id', + 'orderItemStatus', + 'cancellationMessage', + 'customerNotice', + 'accessCode', + 'accessPass', + 'error', + ], + requiredOptions: [], + shallNotBeReferencedFields: [ + 'orderedItem', + 'acceptedOffer', + ], + }, + }, }, 'latest'); model.hasSpecification = true; @@ -20,6 +72,9 @@ describe('NoIdReferencesForResponsesRule', () => { '@context': 'https://openactive.io/', '@type': 'OrderItem', acceptedOffer: 'https://example.com/offer/1', + orderedItem: { + '@id': 'https://example.com/item/1', + }, }; const nodeToTest = new ModelNode( @@ -41,6 +96,9 @@ describe('NoIdReferencesForResponsesRule', () => { '@context': 'https://openactive.io/', '@type': 'OrderItem', orderedItem: 'https://example.com/session/1', + acceptedOffer: { + '@id': 'https://example.com/offer/1', + }, }; const nodeToTest = new ModelNode( diff --git a/src/rules/core/no-id-references-for-responses-rule.js b/src/rules/core/no-id-references-for-responses-rule.js index 324aeda3..6366ba67 100644 --- a/src/rules/core/no-id-references-for-responses-rule.js +++ b/src/rules/core/no-id-references-for-responses-rule.js @@ -17,7 +17,7 @@ class NoIdReferencesForResponsesRule extends Rule { 'OrdersFeed', 'OrderStatus', ]; - this.targetFields = { OrderItem: ['acceptedOffer', 'orderedItem'] }; + this.targetModels = '*'; this.meta = { name: 'NoIdReferencesForResponsesRule', description: 'Validates that acceptedOffer and orderedItem are not ID references and are objects for responses (C1, C2 etc)', @@ -38,32 +38,31 @@ class NoIdReferencesForResponsesRule extends Rule { } /** - * * @param {ModelNode} node - * @param {string} field */ - validateField(node, field) { + validateModel(node) { // Don't do this check for models that we don't actually have a spec for or for models that aren't JSON-LD if (!node.model.hasSpecification || !node.model.isJsonLd) { return []; } const errors = []; - const fieldValue = node.getValue(field); + const shouldNotBeReferencedFields = node.model.getShallNotBeReferencedFields(node.options.validationMode, node.name); + for (const field of shouldNotBeReferencedFields) { + const fieldValue = node.getValue(field); - if (!_.isPlainObject(fieldValue)) { - errors.push( - this.createError( - 'default', - { - fieldValue, - path: node.getPath(field), - }, - { field }, - ), - ); - } else { - return []; + if (!_.isPlainObject(fieldValue)) { + errors.push( + this.createError( + 'default', + { + fieldValue, + path: node.getPath(field), + }, + { field }, + ), + ); + } } return errors; From d0c9e24cfbd45d72107c57c6fec72c3cb8fa4dd0 Mon Sep 17 00:00:00 2001 From: nickevansuk <2616208+nickevansuk@users.noreply.github.com> Date: Sat, 16 Jul 2022 01:01:43 +0100 Subject: [PATCH 9/9] Refactor and include other fixes --- package.json | 2 +- src/errors/validation-error-type.js | 4 +- .../booking/booking-root-type-correct-rule.js | 4 + .../consumer-notes/assume-age-range-rule.js | 4 + .../assume-event-status-rule.js | 4 + .../assume-no-gender-restriction-rule.js | 4 + src/rules/core/fields-correct-type-rule.js | 34 +++++--- ... id-references-not-permitted-rule-spec.js} | 10 +-- ...js => id-references-not-permitted-rule.js} | 29 ++++--- ...js => id-references-required-rule-spec.js} | 52 ++---------- ...rule.js => id-references-required-rule.js} | 27 ++++--- ...t-remaining-attendee-capacity-rule-spec.js | 81 ------------------- .../event-remaining-attendee-capacity-rule.js | 49 ----------- ...scheduled-session-must-be-subevent-rule.js | 4 + src/rules/index.js | 5 +- 15 files changed, 94 insertions(+), 219 deletions(-) rename src/rules/core/{no-id-references-for-responses-rule-spec.js => id-references-not-permitted-rule-spec.js} (91%) rename src/rules/core/{no-id-references-for-responses-rule.js => id-references-not-permitted-rule.js} (64%) rename src/rules/core/{id-references-for-requests-rule-spec.js => id-references-required-rule-spec.js} (62%) rename src/rules/core/{id-references-for-requests-rule.js => id-references-required-rule.js} (63%) delete mode 100644 src/rules/data-quality/event-remaining-attendee-capacity-rule-spec.js delete mode 100644 src/rules/data-quality/event-remaining-attendee-capacity-rule.js diff --git a/package.json b/package.json index 32ddaaeb..ca2f952d 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ }, "license": "MIT", "dependencies": { - "@openactive/data-models": "^2.0.219", + "@openactive/data-models": "^2.0.289", "@types/lodash": "^4.14.182", "axios": "^0.19.2", "currency-codes": "^1.5.1", diff --git a/src/errors/validation-error-type.js b/src/errors/validation-error-type.js index 29b2b408..46b44831 100644 --- a/src/errors/validation-error-type.js +++ b/src/errors/validation-error-type.js @@ -43,8 +43,8 @@ const ValidationErrorType = { BELOW_MIN_VALUE_INCLUSIVE: 'below_min_value_inclusive', VALUE_OUTWITH_CONSTRAINT: 'value_outwith_constraint', INVALID_ID: 'invalid_id', - FIELD_NOT_ID_REFERENCE: 'field_not_id_reference', - FIELD_SHOUlD_NOT_BE_ID_REFERENCE: 'field_should_not_be_id_reference', + FIELD_MUST_BE_ID_REFERENCE: 'FIELD_MUST_BE_ID_REFERENCE', + FIELD_MUST_NOT_BE_ID_REFERENCE: 'FIELD_MUST_NOT_BE_ID_REFERENCE', }; module.exports = Object.freeze(ValidationErrorType); diff --git a/src/rules/booking/booking-root-type-correct-rule.js b/src/rules/booking/booking-root-type-correct-rule.js index e4589e9f..3d4be3b0 100644 --- a/src/rules/booking/booking-root-type-correct-rule.js +++ b/src/rules/booking/booking-root-type-correct-rule.js @@ -10,13 +10,17 @@ module.exports = class BookingRootTypeCorrectRule extends Rule { this.targetValidationModes = [ 'C1Request', 'C1Response', + 'C1ResponseOrderItemError', 'C2Request', 'C2Response', + 'C2ResponseOrderItemError', 'PRequest', 'PResponse', + 'PResponseOrderItemError', 'BRequest', 'BOrderProposalRequest', 'BResponse', + 'BResponseOrderItemError', 'OrderProposalPatch', 'OrderPatch', 'OrdersFeed', diff --git a/src/rules/consumer-notes/assume-age-range-rule.js b/src/rules/consumer-notes/assume-age-range-rule.js index 348d8b0f..38a0fd29 100644 --- a/src/rules/consumer-notes/assume-age-range-rule.js +++ b/src/rules/consumer-notes/assume-age-range-rule.js @@ -11,9 +11,13 @@ module.exports = class AssumeAgeRangeRule extends Rule { 'RPDEFeed', 'BookableRPDEFeed', 'C1Response', + 'C1ResponseOrderItemError', 'C2Response', + 'C2ResponseOrderItemError', 'PResponse', + 'PResponseOrderItemError', 'BResponse', + 'BResponseOrderItemError', ]; this.meta = { name: 'AssumeAgeRangeRule', diff --git a/src/rules/consumer-notes/assume-event-status-rule.js b/src/rules/consumer-notes/assume-event-status-rule.js index 0890e376..3223970a 100644 --- a/src/rules/consumer-notes/assume-event-status-rule.js +++ b/src/rules/consumer-notes/assume-event-status-rule.js @@ -11,9 +11,13 @@ module.exports = class AssumeEventStatusRule extends Rule { 'RPDEFeed', 'BookableRPDEFeed', 'C1Response', + 'C1ResponseOrderItemError', 'C2Response', + 'C2ResponseOrderItemError', 'PResponse', + 'PResponseOrderItemError', 'BResponse', + 'BResponseOrderItemError', ]; this.meta = { name: 'AssumeEventStatusRule', diff --git a/src/rules/consumer-notes/assume-no-gender-restriction-rule.js b/src/rules/consumer-notes/assume-no-gender-restriction-rule.js index da29ec98..a53b76cc 100644 --- a/src/rules/consumer-notes/assume-no-gender-restriction-rule.js +++ b/src/rules/consumer-notes/assume-no-gender-restriction-rule.js @@ -11,9 +11,13 @@ module.exports = class AssumeNoGenderRestrictionRule extends Rule { 'RPDEFeed', 'BookableRPDEFeed', 'C1Response', + 'C1ResponseOrderItemError', 'C2Response', + 'C2ResponseOrderItemError', 'PResponse', + 'PResponseOrderItemError', 'BResponse', + 'BResponseOrderItemError', ]; this.meta = { name: 'AssumeNoGenderRestrictionRule', diff --git a/src/rules/core/fields-correct-type-rule.js b/src/rules/core/fields-correct-type-rule.js index 7917d4a6..885ed715 100644 --- a/src/rules/core/fields-correct-type-rule.js +++ b/src/rules/core/fields-correct-type-rule.js @@ -21,11 +21,11 @@ module.exports = class FieldsCorrectTypeRule extends Rule { }, singleType: { description: 'Validates that a property conforms to a single type.', - message: 'Invalid type, expected {{expectedType}} but found {{foundType}}.{{examples}}', + message: 'Invalid type, expected {{expectedType}}{{idReferencingMessage}} but found {{foundType}}.{{examples}}', sampleValues: { expectedType: this.constructor.getHumanReadableType('https://schema.org/Text'), foundType: this.constructor.getHumanReadableType('https://schema.org/Number'), - examples: this.constructor.makeExamples('property', ['https://schema.org/Text'], this.options.version), + examples: this.constructor.makeExamples('property', ['https://schema.org/Text'], this.options.version, true), }, category: ValidationErrorCategory.CONFORMANCE, severity: ValidationErrorSeverity.FAILURE, @@ -37,7 +37,7 @@ module.exports = class FieldsCorrectTypeRule extends Rule { sampleValues: { expectedType: this.constructor.getHumanReadableType('LocationFeatureSpecification'), foundType: this.constructor.getHumanReadableType('ChangingRooms'), - examples: this.constructor.makeExamples('property', ['LocationFeatureSpecification'], this.options.version), + examples: this.constructor.makeExamples('property', ['LocationFeatureSpecification'], this.options.version, true), }, category: ValidationErrorCategory.CONFORMANCE, severity: ValidationErrorSeverity.FAILURE, @@ -49,7 +49,7 @@ module.exports = class FieldsCorrectTypeRule extends Rule { sampleValues: { expectedType: this.constructor.getHumanReadableType('LocationFeatureSpecification'), foundTypes: this.constructor.makeExpectedTypeList(['ChangingRooms', 'GolfCourse']), - examples: this.constructor.makeExamples('property', ['LocationFeatureSpecification'], this.options.version), + examples: this.constructor.makeExamples('property', ['LocationFeatureSpecification'], this.options.version, true), }, category: ValidationErrorCategory.CONFORMANCE, severity: ValidationErrorSeverity.FAILURE, @@ -57,11 +57,11 @@ module.exports = class FieldsCorrectTypeRule extends Rule { }, multipleTypes: { description: 'Validates that a property conforms one of a list of types.', - message: 'Invalid type, expected one of {{expectedTypes}} but found {{foundType}}.{{examples}}', + message: 'Invalid type, expected one of {{expectedTypes}}{{idReferencingMessage}} but found {{foundType}}.{{examples}}', sampleValues: { expectedTypes: this.constructor.makeExpectedTypeList(['https://schema.org/Text', 'ArrayOf#https://schema.org/Text', '#Concept', 'ArrayOf#Concept']), foundType: this.constructor.getHumanReadableType('https://schema.org/Number'), - examples: this.constructor.makeExamples('property', ['https://schema.org/Text', 'ArrayOf#https://schema.org/Text', '#Concept', 'ArrayOf#Concept'], this.options.version), + examples: this.constructor.makeExamples('property', ['https://schema.org/Text', 'ArrayOf#https://schema.org/Text', '#Concept', 'ArrayOf#Concept'], this.options.version, true), }, category: ValidationErrorCategory.CONFORMANCE, severity: ValidationErrorSeverity.FAILURE, @@ -109,6 +109,8 @@ module.exports = class FieldsCorrectTypeRule extends Rule { return `[\`string${plural}\` containing the URL of a property](${type}) from the [OpenActive](https://openactive.io/ns) or [schema.org](https://schema.org/) vocabularies`; case 'https://schema.org/URL': return `[\`string${plural}\` containing a url](${type})`; + case 'https://openactive.io/IdReference': + return '[`@id` reference](https://permalink.openactive.io/data-model-validator/id-references)'; default: return `\`${type.replace(/^#/, '')}\``; } @@ -125,7 +127,7 @@ module.exports = class FieldsCorrectTypeRule extends Rule { } const humanReadableType = this.getHumanReadableRawType(readableType, isArray); let aOrAn = 'A'; - if (isArray || humanReadableType.match(/^\[?`[aeiouAEIOU]/)) { + if (isArray || humanReadableType.match(/^\[?`[aeiouAEIOU`]/)) { aOrAn = 'An'; } const hint = `${aOrAn} ${isArray ? 'array of ' : ''}${humanReadableType} looks like this:`; @@ -162,6 +164,9 @@ module.exports = class FieldsCorrectTypeRule extends Rule { case 'https://schema.org/URL': example = `${prefix}"https://www.example.org/"`; break; + case 'https://openactive.io/IdReference': + example = `${prefix}"https://id.example.com/api/session-series/1402CBP20150217"`; + break; default: if (PropertyHelper.isEnum(readableType, version)) { const allowedOptions = PropertyHelper.getEnumOptions(readableType, version); @@ -185,7 +190,7 @@ module.exports = class FieldsCorrectTypeRule extends Rule { return `${expectedTypes}`; } - static makeExamples(property, types, version, renderedExample) { + static makeExamples(property, types, version, renderedExample, allowReferencing) { let examples = ''; for (const type of types) { examples = `${examples}\n\n${this.getHumanReadableExample(property, type, version)}`; @@ -194,6 +199,7 @@ module.exports = class FieldsCorrectTypeRule extends Rule { const hint = types.length > 1 ? 'A full example of the preferred approach looks like this:' : 'A full example looks like this:'; examples = `${examples}\n\n${hint}\n\n${renderedExample}`; } + if (allowReferencing) examples = `${examples}\n\nA URI reference which matches the \`@id\` of another object may also be used in place of the object itself.${this.getHumanReadableExample(property, 'https://openactive.io/IdReference', version)}`; return examples; } @@ -241,6 +247,8 @@ module.exports = class FieldsCorrectTypeRule extends Rule { // Pass check if referencing via a URL that matches an @id elsewhere is allowed, and in use || (fieldObj.allowReferencing && typeof fieldValue === 'string' && PropertyHelper.isUrl(fieldValue)); + const idReferencingMessage = fieldObj.allowReferencing ? ' or a reference URI to an `@id`' : ''; + if (!checkPass) { let testKey; let messageValues = {}; @@ -280,14 +288,14 @@ module.exports = class FieldsCorrectTypeRule extends Rule { messageValues = { expectedType: this.constructor.getHumanReadableType(typeChecks[0]), foundType: this.constructor.getHumanReadableType(notAllowed[0]), - examples: this.constructor.makeExamples(propName, typeChecks, node.options.version, fieldObj.getRenderedExample()), + examples: this.constructor.makeExamples(propName, typeChecks, node.options.version, fieldObj.getRenderedExample(), fieldObj.allowReferencing), }; } else if (notAllowed.length > 1) { testKey = 'singleTypeSubclassMultipleError'; messageValues = { expectedType: this.constructor.getHumanReadableType(typeChecks[0]), foundTypes: this.constructor.makeExpectedTypeList(notAllowed), - examples: this.constructor.makeExamples(propName, typeChecks, node.options.version, fieldObj.getRenderedExample()), + examples: this.constructor.makeExamples(propName, typeChecks, node.options.version, fieldObj.getRenderedExample(), fieldObj.allowReferencing), }; } } else { @@ -295,7 +303,8 @@ module.exports = class FieldsCorrectTypeRule extends Rule { messageValues = { expectedType: this.constructor.getHumanReadableType(typeChecks[0]), foundType: this.constructor.getHumanReadableType(derivedType), - examples: this.constructor.makeExamples(propName, typeChecks, node.options.version, fieldObj.getRenderedExample()), + examples: this.constructor.makeExamples(propName, typeChecks, node.options.version, fieldObj.getRenderedExample(), fieldObj.allowReferencing), + idReferencingMessage, }; } } else { @@ -304,7 +313,8 @@ module.exports = class FieldsCorrectTypeRule extends Rule { messageValues = { expectedTypes, foundType: this.constructor.getHumanReadableType(derivedType), - examples: this.constructor.makeExamples(propName, typeChecks, node.options.version, fieldObj.getRenderedExample()), + examples: this.constructor.makeExamples(propName, typeChecks, node.options.version, fieldObj.getRenderedExample(), fieldObj.allowReferencing), + idReferencingMessage, }; } errors.push( diff --git a/src/rules/core/no-id-references-for-responses-rule-spec.js b/src/rules/core/id-references-not-permitted-rule-spec.js similarity index 91% rename from src/rules/core/no-id-references-for-responses-rule-spec.js rename to src/rules/core/id-references-not-permitted-rule-spec.js index ae41d45b..4afaafba 100644 --- a/src/rules/core/no-id-references-for-responses-rule-spec.js +++ b/src/rules/core/id-references-not-permitted-rule-spec.js @@ -3,11 +3,11 @@ const ModelNode = require('../../classes/model-node'); const OptionsHelper = require('../../helpers/options'); const ValidationErrorType = require('../../errors/validation-error-type'); const ValidationErrorSeverity = require('../../errors/validation-error-severity'); -const NoIdReferencesForResponsesRule = require('./no-id-references-for-responses-rule'); +const IdReferencesNotPermittedRule = require('./id-references-not-permitted-rule'); -describe('NoIdReferencesForResponsesRule', () => { - const rule = new NoIdReferencesForResponsesRule(); +describe('IdReferencesNotPermittedRule', () => { + const rule = new IdReferencesNotPermittedRule(); const model = new Model({ type: 'OrderItem', @@ -87,7 +87,7 @@ describe('NoIdReferencesForResponsesRule', () => { const errors = await rule.validate(nodeToTest); expect(errors.length).toBe(1); - expect(errors[0].type).toBe(ValidationErrorType.FIELD_SHOUlD_NOT_BE_ID_REFERENCE); + expect(errors[0].type).toBe(ValidationErrorType.FIELD_MUST_NOT_BE_ID_REFERENCE); expect(errors[0].severity).toBe(ValidationErrorSeverity.FAILURE); }); it('should return a failure if a response object does not have `orderedItem` as a data object', async () => { @@ -111,7 +111,7 @@ describe('NoIdReferencesForResponsesRule', () => { const errors = await rule.validate(nodeToTest); expect(errors.length).toBe(1); - expect(errors[0].type).toBe(ValidationErrorType.FIELD_SHOUlD_NOT_BE_ID_REFERENCE); + expect(errors[0].type).toBe(ValidationErrorType.FIELD_MUST_NOT_BE_ID_REFERENCE); expect(errors[0].severity).toBe(ValidationErrorSeverity.FAILURE); }); it('should return no errors if a request object has `acceptedOffer` as a compact ID reference', async () => { diff --git a/src/rules/core/no-id-references-for-responses-rule.js b/src/rules/core/id-references-not-permitted-rule.js similarity index 64% rename from src/rules/core/no-id-references-for-responses-rule.js rename to src/rules/core/id-references-not-permitted-rule.js index 6366ba67..a824b3af 100644 --- a/src/rules/core/no-id-references-for-responses-rule.js +++ b/src/rules/core/id-references-not-permitted-rule.js @@ -1,4 +1,3 @@ -const _ = require('lodash'); const Rule = require('../rule'); const ValidationErrorCategory = require('../../errors/validation-error-category'); const ValidationErrorSeverity = require('../../errors/validation-error-severity'); @@ -6,32 +5,42 @@ const validationErrorType = require('../../errors/validation-error-type'); /** @typedef {import('../../classes/model-node').ModelNodeType} ModelNode */ -class NoIdReferencesForResponsesRule extends Rule { +class IdReferencesNotPermittedRule extends Rule { constructor(options) { super(options); this.targetValidationModes = [ + 'C1Request', 'C1Response', + 'C1ResponseOrderItemError', + 'C2Request', 'C2Response', + 'C2ResponseOrderItemError', + 'PRequest', 'PResponse', + 'PResponseOrderItemError', + 'BRequest', + 'BOrderProposalRequest', 'BResponse', + 'BResponseOrderItemError', + 'OrderProposalPatch', + 'OrderPatch', 'OrdersFeed', 'OrderStatus', ]; this.targetModels = '*'; this.meta = { - name: 'NoIdReferencesForResponsesRule', - description: 'Validates that acceptedOffer and orderedItem are not ID references and are objects for responses (C1, C2 etc)', + name: 'IdReferencesNotPermittedRule', + description: 'Validates that ID references are not used where not permitted', tests: { default: { - description: `Raises a failure if the acceptedOffer or orderedItem within the OrderItem of a response is a URL - (ie a reference to the object and not the object itself)`, - message: 'For responses, {{field}} must not be a compact ID reference, but the object representing the data itself', + description: 'Raises a failure if the value of a property is a URL (i.e. it is a reference to the object and not the object itself)', + message: 'In this validation mode `{{field}}` must be an object representing the data itself, not a compact `@id` reference or string', sampleValues: { field: 'acceptedOffer', }, category: ValidationErrorCategory.CONFORMANCE, severity: ValidationErrorSeverity.FAILURE, - type: validationErrorType.FIELD_SHOUlD_NOT_BE_ID_REFERENCE, + type: validationErrorType.FIELD_MUST_NOT_BE_ID_REFERENCE, }, }, }; @@ -51,7 +60,7 @@ class NoIdReferencesForResponsesRule extends Rule { for (const field of shouldNotBeReferencedFields) { const fieldValue = node.getValue(field); - if (!_.isPlainObject(fieldValue)) { + if (typeof fieldValue === 'string') { errors.push( this.createError( 'default', @@ -69,4 +78,4 @@ class NoIdReferencesForResponsesRule extends Rule { } } -module.exports = NoIdReferencesForResponsesRule; +module.exports = IdReferencesNotPermittedRule; diff --git a/src/rules/core/id-references-for-requests-rule-spec.js b/src/rules/core/id-references-required-rule-spec.js similarity index 62% rename from src/rules/core/id-references-for-requests-rule-spec.js rename to src/rules/core/id-references-required-rule-spec.js index cb207dfd..4b8e362c 100644 --- a/src/rules/core/id-references-for-requests-rule-spec.js +++ b/src/rules/core/id-references-required-rule-spec.js @@ -1,13 +1,13 @@ const Model = require('../../classes/model'); const ModelNode = require('../../classes/model-node'); -const IdReferencesForRequestsRule = require('./id-references-for-requests-rule'); const OptionsHelper = require('../../helpers/options'); const ValidationErrorType = require('../../errors/validation-error-type'); const ValidationErrorSeverity = require('../../errors/validation-error-severity'); +const IdReferencesRequiredRule = require('./id-references-required-rule'); -describe('IdReferencesForRequestsRule', () => { - const rule = new IdReferencesForRequestsRule(); +describe('IdReferencesRequiredRule', () => { + const rule = new IdReferencesRequiredRule(); const model = new Model({ type: 'OrderItem', @@ -40,7 +40,7 @@ describe('IdReferencesForRequestsRule', () => { const errors = await rule.validate(nodeToTest); expect(errors.length).toBe(1); - expect(errors[0].type).toBe(ValidationErrorType.FIELD_NOT_ID_REFERENCE); + expect(errors[0].type).toBe(ValidationErrorType.FIELD_MUST_BE_ID_REFERENCE); expect(errors[0].severity).toBe(ValidationErrorSeverity.FAILURE); }); it('should return a failure if a request object does not have `orderedItem` as a compact ID reference', async () => { @@ -65,51 +65,9 @@ describe('IdReferencesForRequestsRule', () => { const errors = await rule.validate(nodeToTest); expect(errors.length).toBe(1); - expect(errors[0].type).toBe(ValidationErrorType.FIELD_NOT_ID_REFERENCE); + expect(errors[0].type).toBe(ValidationErrorType.FIELD_MUST_BE_ID_REFERENCE); expect(errors[0].severity).toBe(ValidationErrorSeverity.FAILURE); }); - it('should return no errors if a response object has `acceptedOffer` as a data object', async () => { - const options = new OptionsHelper({ validationMode: 'C1Response' }); - const data = { - '@context': 'https://openactive.io/', - '@type': 'OrderItem', - acceptedOffer: { - '@type': 'ScheduledSession', - '@id': 'https://example.com/offer/1', - }, - }; - - const nodeToTest = new ModelNode( - '$', - data, - null, - model, - options, - ); - const errors = await rule.validate(nodeToTest); - expect(errors.length).toBe(0); - }); - it('should return no errors if a response object has `orderedItem` as a data object', async () => { - const options = new OptionsHelper({ validationMode: 'C1Response' }); - const data = { - '@context': 'https://openactive.io/', - '@type': 'OrderItem', - orderedItem: { - '@type': 'ScheduledSession', - '@id': 'https://example.com/session/1', - }, - }; - - const nodeToTest = new ModelNode( - '$', - data, - null, - model, - options, - ); - const errors = await rule.validate(nodeToTest); - expect(errors.length).toBe(0); - }); it('should return no errors if a request object has `orderedItem` as a compact ID reference', async () => { const options = new OptionsHelper({ validationMode: 'C1Request' }); const data = { diff --git a/src/rules/core/id-references-for-requests-rule.js b/src/rules/core/id-references-required-rule.js similarity index 63% rename from src/rules/core/id-references-for-requests-rule.js rename to src/rules/core/id-references-required-rule.js index 374607d6..024b2bda 100644 --- a/src/rules/core/id-references-for-requests-rule.js +++ b/src/rules/core/id-references-required-rule.js @@ -6,32 +6,42 @@ const validationErrorType = require('../../errors/validation-error-type'); /** @typedef {import('../../classes/model-node').ModelNodeType} ModelNode */ -class IdReferencesForRequestsRule extends Rule { +class IdReferencesRequiredRule extends Rule { constructor(options) { super(options); this.targetValidationModes = [ 'C1Request', + 'C1Response', + 'C1ResponseOrderItemError', 'C2Request', + 'C2Response', + 'C2ResponseOrderItemError', 'PRequest', + 'PResponse', + 'PResponseOrderItemError', 'BRequest', 'BOrderProposalRequest', + 'BResponse', + 'BResponseOrderItemError', + 'OrderProposalPatch', 'OrderPatch', + 'OrdersFeed', + 'OrderStatus', ]; this.targetModels = '*'; this.meta = { - name: 'IdReferencesForRequestsRule', - description: 'Validates that acceptedOffer and orderedItem are ID references and not objects for requests (C1, C2 etc)', + name: 'IdReferencesRequiredRule', + description: 'Validates that ID references are used where permitted', tests: { default: { - description: `Raises a failure if the acceptedOffer or orderedItem within the OrderItem of a request is not a URL - (ie a reference to the object and not the object itself)`, - message: 'For requests, {{field}} must be a compact ID reference, not the object representing the data itself', + description: 'Raises a failure if the value of a property is not a URL (i.e. it is the object itself, not a reference to the object)', + message: 'In this validation mode `{{field}}` must be a compact [`@id` reference](https://permalink.openactive.io/data-model-validator/id-references), not the object representing the data itself. An `@id` reference looks like this:\n\n```\n"{{field}}": "https://id.example.com/api/session-series/1402CBP20150217"\n```', sampleValues: { field: 'acceptedOffer', }, category: ValidationErrorCategory.CONFORMANCE, severity: ValidationErrorSeverity.FAILURE, - type: validationErrorType.FIELD_NOT_ID_REFERENCE, + type: validationErrorType.FIELD_MUST_BE_ID_REFERENCE, }, }, }; @@ -65,9 +75,8 @@ class IdReferencesForRequestsRule extends Rule { } } - return errors; } } -module.exports = IdReferencesForRequestsRule; +module.exports = IdReferencesRequiredRule; diff --git a/src/rules/data-quality/event-remaining-attendee-capacity-rule-spec.js b/src/rules/data-quality/event-remaining-attendee-capacity-rule-spec.js deleted file mode 100644 index 620ae55f..00000000 --- a/src/rules/data-quality/event-remaining-attendee-capacity-rule-spec.js +++ /dev/null @@ -1,81 +0,0 @@ -const EventRemainingAttendeeCapacityRule = require('./event-remaining-attendee-capacity-rule'); -const Model = require('../../classes/model'); -const ModelNode = require('../../classes/model-node'); -const ValidationErrorType = require('../../errors/validation-error-type'); -const ValidationErrorSeverity = require('../../errors/validation-error-severity'); -const OptionsHelper = require('../../helpers/options'); - -describe('EventRemainingAttendeeCapacityRule', () => { - const rule = new EventRemainingAttendeeCapacityRule(); - - const model = new Model({ - type: 'Event', - fields: { - remainingAttendeeCapacity: { - fieldName: 'remainingAttendeeCapacity', - requiredType: 'https://schema.org/Integer', - }, - }, - }, 'latest'); - - it('should target remainingAttendeeCapacity fields', () => { - const isTargeted = rule.isFieldTargeted(model, 'remainingAttendeeCapacity'); - expect(isTargeted).toBe(true); - }); - - describe('isValidationModeTargeted', () => { - const modesToTest = ['C1Response', 'C2Response', 'PResponse', 'BResponse']; - - for (const mode of modesToTest) { - it(`should target ${mode}`, () => { - const isTargeted = rule.isValidationModeTargeted(mode); - expect(isTargeted).toBe(true); - }); - } - - it('should not target RPDEFeed validation mode', () => { - const isTargeted = rule.isValidationModeTargeted('RPDEFeed'); - expect(isTargeted).toBe(false); - }); - }); - - describe('when in a booking mode like C1Response', () => { - const options = new OptionsHelper({ validationMode: 'C1Response' }); - - it('should return no error when remainingAttendeeCapacity is > 0', async () => { - const data = { - '@type': 'Event', - remainingAttendeeCapacity: 1, - }; - - const nodeToTest = new ModelNode( - '$', - data, - null, - model, - options, - ); - const errors = await rule.validate(nodeToTest); - expect(errors.length).toBe(0); - }); - - it('should return no error when remainingAttendeeCapacity is < 0', async () => { - const data = { - '@type': 'Event', - remainingAttendeeCapacity: -1, - }; - - const nodeToTest = new ModelNode( - '$', - data, - null, - model, - options, - ); - const errors = await rule.validate(nodeToTest); - expect(errors.length).toBe(1); - expect(errors[0].type).toBe(ValidationErrorType.FIELD_NOT_IN_DEFINED_VALUES); - expect(errors[0].severity).toBe(ValidationErrorSeverity.FAILURE); - }); - }); -}); diff --git a/src/rules/data-quality/event-remaining-attendee-capacity-rule.js b/src/rules/data-quality/event-remaining-attendee-capacity-rule.js deleted file mode 100644 index beea33ea..00000000 --- a/src/rules/data-quality/event-remaining-attendee-capacity-rule.js +++ /dev/null @@ -1,49 +0,0 @@ -const Rule = require('../rule'); -const ValidationErrorType = require('../../errors/validation-error-type'); -const ValidationErrorCategory = require('../../errors/validation-error-category'); -const ValidationErrorSeverity = require('../../errors/validation-error-severity'); - -module.exports = class EventRemainingAttendeeCapacityRule extends Rule { - constructor(options) { - super(options); - this.targetFields = { Event: ['remainingAttendeeCapacity'] }; - this.targetValidationModes = [ - 'C1Response', - 'C2Response', - 'PResponse', - 'BResponse', - ]; - this.meta = { - name: 'EventRemainingAttendeeCapacityRule', - description: 'Validates that the remainingAttendeeCapacity of an Event is greater than or equal to 0', - tests: { - default: { - description: 'Raises a failure if the remainingAttendeeCapacity of an Event is not greater than or equal to 0', - message: 'The `remainingAttendeeCapacity` of an `Event` must be greater than or equal to 0.', - category: ValidationErrorCategory.DATA_QUALITY, - severity: ValidationErrorSeverity.FAILURE, - type: ValidationErrorType.FIELD_NOT_IN_DEFINED_VALUES, - }, - }, - }; - } - - validateField(node, field) { - const errors = []; - - const fieldValue = node.getValue(field); - - if (fieldValue < 0) { - errors.push( - this.createError( - 'default', - { - path: node.getPath(field), - }, - ), - ); - } - - return errors; - } -}; diff --git a/src/rules/data-quality/scheduled-session-must-be-subevent-rule.js b/src/rules/data-quality/scheduled-session-must-be-subevent-rule.js index 438f6934..cb2fd242 100644 --- a/src/rules/data-quality/scheduled-session-must-be-subevent-rule.js +++ b/src/rules/data-quality/scheduled-session-must-be-subevent-rule.js @@ -11,9 +11,13 @@ module.exports = class ScheduledSessionMustBeSubeventRule extends Rule { 'RPDEFeed', 'BookableRPDEFeed', 'C1Response', + 'C1ResponseOrderItemError', 'C2Response', + 'C2ResponseOrderItemError', 'PResponse', + 'PResponseOrderItemError', 'BResponse', + 'BResponseOrderItemError', ]; this.meta = { name: 'ScheduledSessionMustBeSubeventRule', diff --git a/src/rules/index.js b/src/rules/index.js index fea8dfb6..4bf29279 100644 --- a/src/rules/index.js +++ b/src/rules/index.js @@ -23,8 +23,8 @@ module.exports = { require('./core/valueconstraint-rule'), require('./core/minvalueinclusive-rule'), require('./core/id-rule'), - require('./core/id-references-for-requests-rule'), - require('./core/no-id-references-for-responses-rule'), + require('./core/id-references-required-rule'), + require('./core/id-references-not-permitted-rule'), // Formatting rules require('./format/duration-format-rule'), @@ -57,7 +57,6 @@ module.exports = { require('./data-quality/session-series-schedule-type-rule'), require('./data-quality/currency-if-non-zero-price-rule'), require('./data-quality/if-needs-booking-must-have-valid-offer-rule'), - require('./data-quality/event-remaining-attendee-capacity-rule'), require('./data-quality/available-channel-for-prepayment-rule'), // Notes on the data consumer