Merge branch 'master' into feature/openactive-test-suite-test

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:
 
@@ -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

README.md

@@ -229,10 +229,10 @@ 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
+$ npm run run-tests
 ```
 
 ### Contributing

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@openactive/data-model-validator",
-  "version": "2.0.74",
+  "version": "2.0.83",
   "description": "A library to allow a developer to validate a JSON document against the OpenActive Modelling Opportunity Specification",
   "homepage": "https://openactive.io",
   "author": "OpenActive Community <[email protected]>",
@@ -38,19 +38,23 @@
     "write-file-atomic": "^5.0.1"
   },
   "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": "^5.1.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",
-    "test": "npm run test-no-lint",
-    "test-no-lint": "jasmine",
+    "pretest": "npm run lint && tsc",
+    "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"

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;

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,

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);
   }
 

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<any>}
  */
 async function getFileLoadRemoteAndCacheToFs(url, options) {
   {

src/helpers/raw.js

@@ -1,31 +1,25 @@
+const _ = require('lodash');
+
 const RawHelper = class {
   static isRpdeFeed(data) {
-    if (
-      typeof data !== 'object'
-      || data === null
-      || data instanceof Array
-    ) {
+    if (!_.isPlainObject(data)) {
+      return false;
+    }
+    const type = data['@type'] || data.type;
+    // This is a JSON-LD object with a @type
+    if (!_.isNil(type)) {
+      return false;
+    }
+    if (!Array.isArray(data.items)) {
       return false;
     }
-    if (
-      typeof data.type === 'undefined'
-      && typeof data['@type'] === 'undefined'
-      && typeof data.items !== 'undefined'
-      && data.items instanceof Array
-    ) {
-      for (const item of data.items) {
-        if (
-          typeof item.state === 'string'
-          && (
-            item.state === 'updated'
-            || item.state === 'deleted'
-          )
-        ) {
-          return true;
-        }
+    for (const item of data.items) {
+      if (item.state !== 'updated' && item.state !== 'deleted') {
+        return false;
       }
     }
-    return false;
+    // If the page has no items (e.g. a last page), it's still considered an RPDE feed.
+    return true;
   }
 };
 

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<TKey, TKey>}
+   */
+  create(keys) {
+    return keys.reduce((acc, key) => {
+      acc[key] = key;
+      return acc;
+    }, /** @type {any} */({}));
+  },
+};
+
+module.exports = {
+  SelfIndexingObject,
+};

src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js

@@ -0,0 +1,54 @@
+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', 'latest'),
+    'latest',
+    true,
+  );
+
+  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);
+  });
+});

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 does not point to a recognised discussion board. Currently recognised discussion board formats: `https://github.com/<ORG>/<REPO>/issues`.',
+          category: ValidationErrorCategory.CONFORMANCE,
+          severity: ValidationErrorSeverity.WARNING,
+          type: ValidationErrorType.INVALID_FORMAT,
+        },
+      },
+    };
+  }
+
+  /**
+   * @param {import('../../classes/model-node').ModelNodeType} node
+   * @param {string} field
+   */
+  async 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),
+        },
+      ),
+    ];
+  }
+};

src/rules/raw/rpde-feed-rule-spec.js

@@ -44,6 +44,20 @@ describe('RpdeFeedRule', () => {
     expect(errors[0].severity).toBe(ValidationErrorSeverity.NOTICE);
   });
 
+  it('should return a notice for an empty RPDE feed', async () => {
+    const data = {
+      items: [],
+      next: 'https://example.org/api/feed/?afterId=ABCDEF09001015&afterTimestamp=1533206202992&limit=500',
+      license: 'https://creativecommons.org/licenses/by/4.0/',
+    };
+
+    const { errors } = await rule.validate(data);
+    expect(errors.length).toBe(1);
+
+    expect(errors[0].type).toBe(ValidationErrorType.FOUND_RPDE_FEED);
+    expect(errors[0].severity).toBe(ValidationErrorSeverity.NOTICE);
+  });
+
   it('should return a notice for an RPDE feed, and modify the data with a limit to the number of items', async () => {
     const feed = {
       items: [