From 7c407fb37b9d7c2c02e67bbf1d0ed709ed39fdd5 Mon Sep 17 00:00:00 2001 From: Luke Winship Date: Tue, 5 Mar 2024 16:55:40 +0000 Subject: [PATCH 1/7] feat: Add discussionUrl rules checking against GH issues --- CONTRIBUTING.md | 2 +- src/helpers/self-indexing-object.js | 27 +++ ...o-recognised-discussion-board-rule-spec.js | 171 ++++++++++++++++++ ...int-to-recognised-discussion-board-rule.js | 79 ++++++++ src/rules/rule.js | 21 +++ 5 files changed, 299 insertions(+), 1 deletion(-) create mode 100644 src/helpers/self-indexing-object.js create mode 100644 src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js create mode 100644 src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4b2b273..b945f82 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -129,7 +129,7 @@ this.targetValidationModes = ['C1Request', 'C2Response']; Set `this.meta` to explain what the rule is testing for. -Defining this detail here makes it easier for libaries scrape all of the rules that the validator will run. +Defining this detail here makes it easier for libaries to scrape all of the rules that the validator will run. This meta object should include: diff --git a/src/helpers/self-indexing-object.js b/src/helpers/self-indexing-object.js new file mode 100644 index 0000000..5362138 --- /dev/null +++ b/src/helpers/self-indexing-object.js @@ -0,0 +1,27 @@ +/** + * A shorthand to create an object where keys and values are the same. + * + * e.g. + * + * ```js + * > SelfIndexingObject.create(['a', 'b', 'c']) + * { a: 'a', b: 'b', c: 'c' } + * ``` + */ +const SelfIndexingObject = { + /** + * @template TKey + * @param {TKey[]} keys + * @returns {Record} + */ + create(keys) { + return keys.reduce((acc, key) => { + acc[key] = key; + return acc; + }, {}); + }, +}; + +module.exports = { + SelfIndexingObject, +}; diff --git a/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js b/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js new file mode 100644 index 0000000..25320a6 --- /dev/null +++ b/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js @@ -0,0 +1,171 @@ +const DiscussionUrlShouldPointToRecognisedDiscussionBoardRule = require('./discussion-url-should-point-to-recognised-discussion-board-rule'); +const ModelNode = require('../../classes/model-node'); +const Model = require('../../classes/model'); +const DataModelHelper = require('../../helpers/data-model'); +const ValidationErrorType = require('../../errors/validation-error-type'); +const ValidationErrorCategory = require('../../errors/validation-error-category'); +const ValidationErrorSeverity = require('../../errors/validation-error-severity'); + +describe('DiscussionUrlShouldPointToRecognisedDiscussionBoardRule', () => { + const rule = new DiscussionUrlShouldPointToRecognisedDiscussionBoardRule(); + + const model = new Model( + DataModelHelper.loadModel('Dataset', '2.0'), + '2.0', + true, + ); + // const model = ApplyRules.loadModel('Dataset', '2.0'); + // const model = loadModel('Dataset', '2.0'); + // const data = { + // '@context': [ + // 'https://schema.org/', + // 'https://openactive.io/', + // ], + // '@type': 'Dataset', + // '@id': 'https://opendata.fusion-lifestyle.com/OpenActive/', + // url: 'https://opendata.fusion-lifestyle.com/OpenActive/', + // name: 'Fusion Lifestyle Sessions and Facilities', + // description: 'Near real-time availability and rich descriptions relating to the sessions and facilities available from Fusion Lifestyle', + // accessService: { + // '@type': 'WebAPI', + // name: 'Open Booking API', + // description: 'An API that allows for seamless booking experiences to be created for sessions and facilities available from Fusion Lifestyle', + // authenticationAuthority: 'https://auth.reference-implementation.openactive.io', + // conformsTo: [ + // 'https://openactive.io/open-booking-api/EditorsDraft/', + // ], + // documentation: 'https://permalink.openactive.io/dataset-site/open-booking-api-documentation', + // endpointDescription: 'https://www.openactive.io/open-booking-api/EditorsDraft/swagger.json', + // endpointUrl: 'https://reference-implementation.openactive.io/api/openbooking', + // landingPage: 'https://example.com/api-landing-page', + // termsOfService: 'https://example.com/api-terms-page', + // }, + // keywords: [ + // 'Sessions', + // 'Facilities', + // 'Activities', + // 'Sports', + // 'Physical Activity', + // 'OpenActive', + // ], + // license: 'https://creativecommons.org/licenses/by/4.0/', + // distribution: [ + // { + // '@type': 'DataDownload', + // name: 'ScheduledSession', + // additionalType: 'https://openactive.io/ScheduledSession', + // contentUrl: 'https://www.example.com/feeds/scheduled-sessions', + // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', + // identifier: 'ScheduledSession', + // }, + // { + // '@type': 'DataDownload', + // name: 'SessionSeries', + // additionalType: 'https://openactive.io/SessionSeries', + // contentUrl: 'https://www.example.com/feeds/session-series', + // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', + // identifier: 'SessionSeries', + // }, + // { + // '@type': 'DataDownload', + // name: 'Event', + // additionalType: 'https://schema.org/Event', + // contentUrl: 'https://www.example.com/feeds/events', + // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', + // identifier: 'Event', + // }, + // { + // '@type': 'DataDownload', + // name: 'FacilityUse', + // additionalType: 'https://openactive.io/FacilityUse', + // contentUrl: 'https://www.example.com/feeds/facility-uses', + // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', + // identifier: 'FacilityUse', + // }, + // { + // '@type': 'DataDownload', + // name: 'Slot for FacilityUse', + // additionalType: 'https://openactive.io/Slot', + // contentUrl: 'https://www.example.com/feeds/facility-use-slots', + // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', + // identifier: 'FacilityUseSlot', + // }, + // { + // '@type': 'DataDownload', + // name: 'CourseInstance', + // additionalType: 'https://schema.org/CourseInstance', + // contentUrl: 'https://www.example.com/feeds/course-instances', + // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', + // identifier: 'CourseInstance', + // }, + // ], + // discussionUrl: 'https://github.com/gladstonemrm/FusionLifestyle/schmissues', + // documentation: 'https://permalink.openactive.io/dataset-site/open-data-documentation', + // inLanguage: [ + // 'en-GB', + // ], + // publisher: { + // '@type': 'Organization', + // name: 'Fusion Lifestyle', + // description: 'Fusion Lifestyle was established in April 2000 following a decision by a London Borough Council to outsource the management of the leisure facilities. Fusion now manages a diverse portfolio of facilities using our specialist expertise to provide sustainable solutions for councils and exciting products and programmes for our customers. Fusion is a national operator managing leisure facilities from Wales to London and as far north as Newcastle. \n\nNo two Fusion sites are the same, we retain the heritage of centres when we refurbish and we are experienced at managing centres from ice rinks to outward bound residential centres, town halls and expansive leisure facilities. We put our energies into providing facilities and programmes that are an attractive proposition to the local community. We respect the history of our centres and it is not uncommon for generations of local residents to hold fond memories of learning to swim in our centres, playing football matches over the years and hosting birthday celebrations at our sites.\n\nFusion is a registered charity, there are therefore no shareholders. We are able to continually reinvest in facilities, products and importantly people.\n', + // email: 'info@fusion-lifestyle.com', + // legalName: 'Fusion Lifestyle', + // logo: { + // '@type': 'ImageObject', + // url: 'https://res.cloudinary.com/gladstone/image/upload/fusion-lifestyle-live/ydokan4mlia7zigqd79d', + // }, + // url: 'https://www.fusion-lifestyle.com/', + // }, + // dateModified: '2020-04-23T09:01:05+01:00', + // datePublished: '2019-04-25T19:32:14+00:00', + // schemaVersion: 'https://openactive.io/modelling-opportunity-data/2.0/', + // bookingService: { + // '@type': 'BookingService', + // name: 'Gladstone', + // url: 'https://www.gladstonesoftware.co.uk', + // softwareVersion: '3.0.2', + // hasCredential: 'https://certificates.reference-implementation.openactive.io/examples/all-features/controlled/', + // }, + // backgroundImage: { + // '@type': 'ImageObject', + // url: 'https://res.cloudinary.com/gladstone/image/upload/fusion-lifestyle-live/jjtttkudkodzfzulijsu?', + // }, + // }; + + it('should target Dataset discussionUrl', () => { + const isTargeted = rule.isFieldTargeted(model, 'discussionUrl'); + expect(isTargeted).toBe(true); + }); + + it('should return no error when discussionUrl points to a GitHub repo\'s /issues', async () => { + const nodeToTest = new ModelNode('$', { + discussionUrl: 'https://github.com/openactive/openactive-test-suite/issues', + }, null, model); + const errors = await rule.validate(nodeToTest); + expect(errors).toHaveSize(0); + }); + + it('should return an error when discussionUrl points to a GitHub repo but not to /issues', async () => { + const nodeToTest = new ModelNode('$', { + discussionUrl: 'https://github.com/openactive/openactive-test-suite', + }, null, model); + const errors = await rule.validate(nodeToTest); + expect(errors).toHaveSize(1); + expect(errors[0].rule).toEqual('DiscussionUrlShouldPointToRecognisedDiscussionBoardRule'); + expect(errors[0].category).toEqual(ValidationErrorCategory.DATA_QUALITY); + expect(errors[0].type).toEqual(ValidationErrorType.INVALID_FORMAT); + expect(errors[0].severity).toEqual(ValidationErrorSeverity.FAILURE); + }); + + it('should return a warning when discussionUrl points to an unrecognised place', async () => { + const nodeToTest = new ModelNode('$', { + discussionUrl: 'https://example.com/some-place', + }, null, model); + const errors = await rule.validate(nodeToTest); + expect(errors).toHaveSize(1); + expect(errors[0].rule).toEqual('DiscussionUrlShouldPointToRecognisedDiscussionBoardRule'); + expect(errors[0].category).toEqual(ValidationErrorCategory.CONFORMANCE); + expect(errors[0].type).toEqual(ValidationErrorType.INVALID_FORMAT); + expect(errors[0].severity).toEqual(ValidationErrorSeverity.WARNING); + }); +}); diff --git a/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js b/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js new file mode 100644 index 0000000..ca5ce25 --- /dev/null +++ b/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js @@ -0,0 +1,79 @@ +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'); +const { SelfIndexingObject } = require('../../helpers/self-indexing-object'); + +const TEST_KEYS = SelfIndexingObject.create(/** @type {const} */([ + 'githubButNotIssues', + 'unrecognisedFormat', +])); + +module.exports = class DiscussionUrlShouldPointToRecognisedDiscussionBoardRule extends Rule { + constructor(options) { + super(options); + this.targetFields = { + Dataset: 'discussionUrl', + }; + this.meta = { + name: 'DiscussionUrlShouldPointToRecognisedDiscussionBoardRule', + description: 'Validates that the `discussionUrl` property points to a recognised discussion board.', + tests: { + [TEST_KEYS.githubButNotIssues]: { + message: 'If `discussionUrl` points to GitHub, it should be to the project\'s /issues page e.g. `https://github.com/openactive/OpenActive.Server.NET/issues`.', + category: ValidationErrorCategory.DATA_QUALITY, + severity: ValidationErrorSeverity.FAILURE, + type: ValidationErrorType.INVALID_FORMAT, + }, + [TEST_KEYS.unrecognisedFormat]: { + message: 'The `discussionUrl` property must point to a recognised discussion board. Currently recognised discussion board formats: `https://github.com///issues`.', + category: ValidationErrorCategory.CONFORMANCE, + severity: ValidationErrorSeverity.WARNING, + type: ValidationErrorType.INVALID_FORMAT, + }, + }, + }; + } + + /** + * @param {import('../../classes/model-node').ModelNodeType} node + * @param {string} field + */ + validateField(node, field) { + const discussionUrlRaw = node.getValue(field); + const discussionUrl = new URL(discussionUrlRaw); + if (discussionUrl.hostname === 'github.com') { + return this.validateGitHubUrl(discussionUrl, node, field); + } + return [ + this.createError( + TEST_KEYS.unrecognisedFormat, + { + value: node.getValue(field), + path: node.getPath(field), + }, + ), + ]; + } + + /** + * @param {URL} discussionUrl + * @param {import('../../classes/model-node').ModelNodeType} node + * @param {string} field + */ + validateGitHubUrl(discussionUrl, node, field) { + const isGhIssuesUrl = discussionUrl.pathname.endsWith('/issues'); + if (isGhIssuesUrl) { + return []; + } + return [ + this.createError( + TEST_KEYS.githubButNotIssues, + { + value: node.getValue(field), + path: node.getPath(field), + }, + ), + ]; + } +}; diff --git a/src/rules/rule.js b/src/rules/rule.js index 1daceb3..72dff15 100644 --- a/src/rules/rule.js +++ b/src/rules/rule.js @@ -5,9 +5,26 @@ const ValidationError = require('../errors/validation-error'); class Rule { constructor(options) { this.options = options || new OptionsHelper(); + /** @type {'*' | string[]} */ this.targetModels = []; + /** @type {'*' | {[model: string]: '*' | string[]}} */ this.targetFields = {}; + /** @type {'*' | string[]} */ this.targetValidationModes = '*'; + /** + * @type {{ + * name: string; + * description: string; + * tests: {[key: string]: { + * description?: string; + * message: string; + * sampleValues?: { [messageTemplateArg: string], string }; + * category: string; + * severity: string; + * type: string; + * }} + * }} + */ this.meta = { name: 'Rule', description: 'This is a base rule description that should be overridden.', @@ -88,6 +105,10 @@ class Rule { ); } + /** + * @param {import('../classes/model')} model + * @param {string} field + */ isFieldTargeted(model, field) { if (this.targetFields === '*') { return true; From ca270d940a5f0e92a76007468f052c526e3b106b Mon Sep 17 00:00:00 2001 From: Luke Winship Date: Tue, 5 Mar 2024 16:57:24 +0000 Subject: [PATCH 2/7] ye --- ...o-recognised-discussion-board-rule-spec.js | 121 +----------------- 1 file changed, 2 insertions(+), 119 deletions(-) diff --git a/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js b/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js index 25320a6..c2dadb6 100644 --- a/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js +++ b/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js @@ -10,127 +10,10 @@ describe('DiscussionUrlShouldPointToRecognisedDiscussionBoardRule', () => { const rule = new DiscussionUrlShouldPointToRecognisedDiscussionBoardRule(); const model = new Model( - DataModelHelper.loadModel('Dataset', '2.0'), - '2.0', + DataModelHelper.loadModel('Dataset', 'latest'), + 'latest', true, ); - // const model = ApplyRules.loadModel('Dataset', '2.0'); - // const model = loadModel('Dataset', '2.0'); - // const data = { - // '@context': [ - // 'https://schema.org/', - // 'https://openactive.io/', - // ], - // '@type': 'Dataset', - // '@id': 'https://opendata.fusion-lifestyle.com/OpenActive/', - // url: 'https://opendata.fusion-lifestyle.com/OpenActive/', - // name: 'Fusion Lifestyle Sessions and Facilities', - // description: 'Near real-time availability and rich descriptions relating to the sessions and facilities available from Fusion Lifestyle', - // accessService: { - // '@type': 'WebAPI', - // name: 'Open Booking API', - // description: 'An API that allows for seamless booking experiences to be created for sessions and facilities available from Fusion Lifestyle', - // authenticationAuthority: 'https://auth.reference-implementation.openactive.io', - // conformsTo: [ - // 'https://openactive.io/open-booking-api/EditorsDraft/', - // ], - // documentation: 'https://permalink.openactive.io/dataset-site/open-booking-api-documentation', - // endpointDescription: 'https://www.openactive.io/open-booking-api/EditorsDraft/swagger.json', - // endpointUrl: 'https://reference-implementation.openactive.io/api/openbooking', - // landingPage: 'https://example.com/api-landing-page', - // termsOfService: 'https://example.com/api-terms-page', - // }, - // keywords: [ - // 'Sessions', - // 'Facilities', - // 'Activities', - // 'Sports', - // 'Physical Activity', - // 'OpenActive', - // ], - // license: 'https://creativecommons.org/licenses/by/4.0/', - // distribution: [ - // { - // '@type': 'DataDownload', - // name: 'ScheduledSession', - // additionalType: 'https://openactive.io/ScheduledSession', - // contentUrl: 'https://www.example.com/feeds/scheduled-sessions', - // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', - // identifier: 'ScheduledSession', - // }, - // { - // '@type': 'DataDownload', - // name: 'SessionSeries', - // additionalType: 'https://openactive.io/SessionSeries', - // contentUrl: 'https://www.example.com/feeds/session-series', - // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', - // identifier: 'SessionSeries', - // }, - // { - // '@type': 'DataDownload', - // name: 'Event', - // additionalType: 'https://schema.org/Event', - // contentUrl: 'https://www.example.com/feeds/events', - // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', - // identifier: 'Event', - // }, - // { - // '@type': 'DataDownload', - // name: 'FacilityUse', - // additionalType: 'https://openactive.io/FacilityUse', - // contentUrl: 'https://www.example.com/feeds/facility-uses', - // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', - // identifier: 'FacilityUse', - // }, - // { - // '@type': 'DataDownload', - // name: 'Slot for FacilityUse', - // additionalType: 'https://openactive.io/Slot', - // contentUrl: 'https://www.example.com/feeds/facility-use-slots', - // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', - // identifier: 'FacilityUseSlot', - // }, - // { - // '@type': 'DataDownload', - // name: 'CourseInstance', - // additionalType: 'https://schema.org/CourseInstance', - // contentUrl: 'https://www.example.com/feeds/course-instances', - // encodingFormat: 'application/vnd.openactive.rpde+json; version=1', - // identifier: 'CourseInstance', - // }, - // ], - // discussionUrl: 'https://github.com/gladstonemrm/FusionLifestyle/schmissues', - // documentation: 'https://permalink.openactive.io/dataset-site/open-data-documentation', - // inLanguage: [ - // 'en-GB', - // ], - // publisher: { - // '@type': 'Organization', - // name: 'Fusion Lifestyle', - // description: 'Fusion Lifestyle was established in April 2000 following a decision by a London Borough Council to outsource the management of the leisure facilities. Fusion now manages a diverse portfolio of facilities using our specialist expertise to provide sustainable solutions for councils and exciting products and programmes for our customers. Fusion is a national operator managing leisure facilities from Wales to London and as far north as Newcastle. \n\nNo two Fusion sites are the same, we retain the heritage of centres when we refurbish and we are experienced at managing centres from ice rinks to outward bound residential centres, town halls and expansive leisure facilities. We put our energies into providing facilities and programmes that are an attractive proposition to the local community. We respect the history of our centres and it is not uncommon for generations of local residents to hold fond memories of learning to swim in our centres, playing football matches over the years and hosting birthday celebrations at our sites.\n\nFusion is a registered charity, there are therefore no shareholders. We are able to continually reinvest in facilities, products and importantly people.\n', - // email: 'info@fusion-lifestyle.com', - // legalName: 'Fusion Lifestyle', - // logo: { - // '@type': 'ImageObject', - // url: 'https://res.cloudinary.com/gladstone/image/upload/fusion-lifestyle-live/ydokan4mlia7zigqd79d', - // }, - // url: 'https://www.fusion-lifestyle.com/', - // }, - // dateModified: '2020-04-23T09:01:05+01:00', - // datePublished: '2019-04-25T19:32:14+00:00', - // schemaVersion: 'https://openactive.io/modelling-opportunity-data/2.0/', - // bookingService: { - // '@type': 'BookingService', - // name: 'Gladstone', - // url: 'https://www.gladstonesoftware.co.uk', - // softwareVersion: '3.0.2', - // hasCredential: 'https://certificates.reference-implementation.openactive.io/examples/all-features/controlled/', - // }, - // backgroundImage: { - // '@type': 'ImageObject', - // url: 'https://res.cloudinary.com/gladstone/image/upload/fusion-lifestyle-live/jjtttkudkodzfzulijsu?', - // }, - // }; it('should target Dataset discussionUrl', () => { const isTargeted = rule.isFieldTargeted(model, 'discussionUrl'); From 5686972db47d04a299f2129c7b2c8df5ae534356 Mon Sep 17 00:00:00 2001 From: Luke Winship Date: Tue, 5 Mar 2024 16:58:37 +0000 Subject: [PATCH 3/7] loosen up language --- ...sion-url-should-point-to-recognised-discussion-board-rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js b/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js index ca5ce25..28fcd11 100644 --- a/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js +++ b/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js @@ -26,7 +26,7 @@ module.exports = class DiscussionUrlShouldPointToRecognisedDiscussionBoardRule e type: ValidationErrorType.INVALID_FORMAT, }, [TEST_KEYS.unrecognisedFormat]: { - message: 'The `discussionUrl` property must point to a recognised discussion board. Currently recognised discussion board formats: `https://github.com///issues`.', + message: 'The `discussionUrl` property does not point to a recognised discussion board. Currently recognised discussion board formats: `https://github.com///issues`.', category: ValidationErrorCategory.CONFORMANCE, severity: ValidationErrorSeverity.WARNING, type: ValidationErrorType.INVALID_FORMAT, From 46949581cc394a40581b158d001a38db95e684e3 Mon Sep 17 00:00:00 2001 From: Luke Winship Date: Tue, 5 Mar 2024 17:32:31 +0000 Subject: [PATCH 4/7] lil fix --- ...on-url-should-point-to-recognised-discussion-board-rule.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js b/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js index 28fcd11..cda2caf 100644 --- a/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js +++ b/src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js @@ -13,7 +13,7 @@ module.exports = class DiscussionUrlShouldPointToRecognisedDiscussionBoardRule e constructor(options) { super(options); this.targetFields = { - Dataset: 'discussionUrl', + Dataset: ['discussionUrl'], }; this.meta = { name: 'DiscussionUrlShouldPointToRecognisedDiscussionBoardRule', @@ -39,7 +39,7 @@ module.exports = class DiscussionUrlShouldPointToRecognisedDiscussionBoardRule e * @param {import('../../classes/model-node').ModelNodeType} node * @param {string} field */ - validateField(node, field) { + async validateField(node, field) { const discussionUrlRaw = node.getValue(field); const discussionUrl = new URL(discussionUrlRaw); if (discussionUrl.hostname === 'github.com') { From f39a609a66f33f2cd08e69b67b94c11d5d7401f8 Mon Sep 17 00:00:00 2001 From: Luke Winship Date: Tue, 5 Mar 2024 17:47:43 +0000 Subject: [PATCH 5/7] chore: Check for errors with TypeScript --- CONTRIBUTING.md | 1 + README.md | 2 +- package.json | 8 ++++++-- src/classes/field.js | 2 ++ src/classes/model-node.js | 3 +++ src/classes/model.js | 2 ++ src/helpers/json-loader.js | 5 ++++- src/rules/rule.js | 36 ++++++++++++++++++++++++++++++++++-- tsconfig.json | 22 ++++++++++++++++++++++ 9 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 tsconfig.json diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4b2b273..bb009c4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -247,6 +247,7 @@ this.createError( #### Adding to the core library * You should write a test for your rule. +* Add the rule's file, as well as its test file to tsconfig.json's `include` array, so that TypeScript can check for errors. * Add the rule to the list in `rules/index`, so that it is processed. #### Adding to your own application diff --git a/README.md b/README.md index 27bfbe3..874dd28 100644 --- a/README.md +++ b/README.md @@ -229,7 +229,7 @@ To run tests locally, run: $ npm test ``` -The test run will also include a run of [eslint](https://eslint.org/). To run the tests without these, use: +The test run will also include a run of [eslint](https://eslint.org/) and [TypeScript](https://www.typescriptlang.org/). To run the tests without these, use: ```shell $ npm run test-no-lint diff --git a/package.json b/package.json index 30d6ce1..9152f9a 100644 --- a/package.json +++ b/package.json @@ -38,17 +38,21 @@ "write-file-atomic": "^3.0.3" }, "devDependencies": { + "@types/jasmine": "^5.1.4", + "@types/jasmine-expect": "^3.8.1", + "@types/node": "^20.11.24", "eslint": "^8.36.0", "eslint-config-airbnb": "^19.0.4", "eslint-plugin-import": "^2.27.5", "jasmine": "^4.6.0", "nock": "^13.3.0", - "sync-request": "^6.1.0" + "sync-request": "^6.1.0", + "typescript": "^5.3.3" }, "scripts": { "lint": "eslint \"src/**/*.js\"", "lint-fix": "eslint \"src/**/*.js\" --fix", - "pretest": "npm run lint", + "pretest": "npm run lint && tsc", "test": "npm run test-no-lint", "test-no-lint": "jasmine", "test-debug": "node --inspect-brk -i ./node_modules/jasmine/bin/jasmine.js", diff --git a/src/classes/field.js b/src/classes/field.js index 776be3d..0a265b9 100644 --- a/src/classes/field.js +++ b/src/classes/field.js @@ -301,7 +301,9 @@ const Field = class { actualTypeKey = actualTypeKey.substr(1); } if ( + // @ts-expect-error typeof (this.constructor.canBeTypeOfMapping[testTypeKey]) !== 'undefined' + // @ts-expect-error && this.constructor.canBeTypeOfMapping[testTypeKey] === actualTypeKey ) { return true; diff --git a/src/classes/model-node.js b/src/classes/model-node.js index 618288c..73866c9 100644 --- a/src/classes/model-node.js +++ b/src/classes/model-node.js @@ -128,6 +128,7 @@ const ModelNode = class { // Does our property allow us to inherit? && typeof this.parentNode.model.fields[this.cleanName] !== 'undefined' && typeof this.parentNode.model.fields[this.cleanName].inheritsTo !== 'undefined' + // @ts-expect-error && this.constructor.checkInheritRule( this.parentNode.model.fields[this.cleanName].inheritsTo, field, @@ -142,6 +143,7 @@ const ModelNode = class { const modelField = this.model.fields[fieldKey]; const fieldValue = this.getValue(fieldKey); if (typeof modelField.inheritsFrom !== 'undefined' + // @ts-expect-error && this.constructor.checkInheritRule(modelField.inheritsFrom, field) && typeof fieldValue === 'object' && !(fieldValue instanceof Array) @@ -162,6 +164,7 @@ const ModelNode = class { } } if (parentModel) { + // @ts-expect-error const parentNode = new this.constructor( modelField.fieldName, fieldValue, diff --git a/src/classes/model.js b/src/classes/model.js index 490415d..596a224 100644 --- a/src/classes/model.js +++ b/src/classes/model.js @@ -80,6 +80,7 @@ const Model = class { } hasRequiredField(field) { + // @ts-expect-error return PropertyHelper.arrayHasField(this.requiredFields, field, this.version); } @@ -143,6 +144,7 @@ const Model = class { } hasRecommendedField(field) { + // @ts-expect-error return PropertyHelper.arrayHasField(this.recommendedFields, field, this.version); } diff --git a/src/helpers/json-loader.js b/src/helpers/json-loader.js index 44fd9fa..2f6e082 100644 --- a/src/helpers/json-loader.js +++ b/src/helpers/json-loader.js @@ -99,6 +99,7 @@ async function getFromFsCacheIfExists(baseCachePath, url) { // Probably just doesn't exist return { exists: false }; } + // @ts-expect-error const parsed = JSON.parse(rawCacheContents); return { exists: true, @@ -132,6 +133,7 @@ async function saveToFsCache(baseCachePath, url, fileObject) { async function getFromRemoteUrl(url) { let response; try { + // @ts-expect-error response = await axios.get(url, { headers: { 'Content-Type': 'application/ld+json', @@ -146,6 +148,7 @@ async function getFromRemoteUrl(url) { if (match !== null) { const { origin } = new URL(url); const linkUrl = match[1]; + // @ts-expect-error response = await axios.get(origin + linkUrl, { headers: { 'Content-Type': 'application/ld+json', @@ -217,7 +220,7 @@ async function getFileLoadRemote(url) { * * @param {string} url * @param {Object} options - * @returns {Object} + * @returns {Promise} */ async function getFileLoadRemoteAndCacheToFs(url, options) { { diff --git a/src/rules/rule.js b/src/rules/rule.js index 1daceb3..c1b1519 100644 --- a/src/rules/rule.js +++ b/src/rules/rule.js @@ -5,9 +5,26 @@ const ValidationError = require('../errors/validation-error'); class Rule { constructor(options) { this.options = options || new OptionsHelper(); + /** @type {string[] | '*'} */ this.targetModels = []; + /** @type {'*' | {[model: string]: '*' | string[]}} */ this.targetFields = {}; + /** @type {string[] | '*'} */ this.targetValidationModes = '*'; + /** + * @type {{ + * name: string; + * description: string; + * tests: {[key: string]: { + * description?: string; + * message: string; + * sampleValues?: { [messageTemplateArg: string]: string }; + * category: string; + * severity: string; + * type: string; + * }} + * }} + */ this.meta = { name: 'Rule', description: 'This is a base rule description that should be overridden.', @@ -38,11 +55,22 @@ class Rule { return errors; } - validateModel(/* node */) { + /** + * @param {import('../classes/model-node').ModelNodeType} node + * @returns {Promise} + */ + // eslint-disable-next-line no-unused-vars + validateModel(node) { throw Error('Model validation rule not implemented'); } - async validateField(/* node, field */) { + /** + * @param {import('../classes/model-node').ModelNodeType} node + * @param {string} field + * @returns {Promise} + */ + // eslint-disable-next-line no-unused-vars + async validateField(node, field) { throw Error('Field validation rule not implemented'); } @@ -88,6 +116,10 @@ class Rule { ); } + /** + * @param {import('../classes/model')} model + * @param {string} field + */ isFieldTargeted(model, field) { if (this.targetFields === '*') { return true; diff --git a/tsconfig.json b/tsconfig.json new file mode 100644 index 0000000..a3b356a --- /dev/null +++ b/tsconfig.json @@ -0,0 +1,22 @@ +{ + "compilerOptions": { + "noEmit": true, + "allowJs": true, + "checkJs": true, + "downlevelIteration": true, + "target": "ES2019", + "moduleResolution": "node", + "types": ["jasmine", "node", "jasmine-expect"] + }, + "include": [ + "src/classes/**/*.js", + "src/errors/**/*.js", + "src/helpers/**/*.js", + "src/rules/rule.js", + // TODO add other rules + "src/exceptions.js" + ], + "exclude": [ + "src/helpers/graph.js" + ] +} From d8586a88b9e6771f57afeab9bc85f1ee07bbdb22 Mon Sep 17 00:00:00 2001 From: Luke Winship Date: Tue, 5 Mar 2024 18:08:54 +0000 Subject: [PATCH 6/7] fixes --- src/helpers/self-indexing-object.js | 2 +- tsconfig.json | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/helpers/self-indexing-object.js b/src/helpers/self-indexing-object.js index 5362138..f98515e 100644 --- a/src/helpers/self-indexing-object.js +++ b/src/helpers/self-indexing-object.js @@ -18,7 +18,7 @@ const SelfIndexingObject = { return keys.reduce((acc, key) => { acc[key] = key; return acc; - }, {}); + }, /** @type {any} */({})); }, }; diff --git a/tsconfig.json b/tsconfig.json index a3b356a..be33275 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -11,10 +11,12 @@ "include": [ "src/classes/**/*.js", "src/errors/**/*.js", + "src/exceptions.js", "src/helpers/**/*.js", "src/rules/rule.js", + "src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js", + "src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js", // TODO add other rules - "src/exceptions.js" ], "exclude": [ "src/helpers/graph.js" From 7a04eafb2240cb85e0ffb62fcef7aae4ec084c92 Mon Sep 17 00:00:00 2001 From: Luke Winship Date: Mon, 11 Mar 2024 16:10:34 +0000 Subject: [PATCH 7/7] test-no-lint -> run-tests --- README.md | 2 +- package.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 874dd28..3888114 100644 --- a/README.md +++ b/README.md @@ -232,7 +232,7 @@ $ npm test The test run will also include a run of [eslint](https://eslint.org/) and [TypeScript](https://www.typescriptlang.org/). To run the tests without these, use: ```shell -$ npm run test-no-lint +$ npm run run-tests ``` ### Contributing diff --git a/package.json b/package.json index 9152f9a..9837dd4 100644 --- a/package.json +++ b/package.json @@ -53,8 +53,8 @@ "lint": "eslint \"src/**/*.js\"", "lint-fix": "eslint \"src/**/*.js\" --fix", "pretest": "npm run lint && tsc", - "test": "npm run test-no-lint", - "test-no-lint": "jasmine", + "test": "npm run run-tests", + "run-tests": "jasmine", "test-debug": "node --inspect-brk -i ./node_modules/jasmine/bin/jasmine.js", "postpublish": "git push", "publish-patch": "npm test && git pull && git push && npm version patch && npm publish"