From dfcb6defc8e8d25ef0e1d0d7f295eef38b92fa0a Mon Sep 17 00:00:00 2001 From: Marcin Mielnicki Date: Thu, 5 Dec 2019 01:03:05 +0100 Subject: [PATCH] Refactor JSONPath based services, run [DynamicJson DynamicYaml] (#4272) * Subclass factory for JSON path services * Common methods moved to JSON path class * should throw error if _getData is not overridden * Test JSON path factory using chai-as-promised * Using chai-as-promised in more tests * JSDoc for json-path * Error message adopted to JSON and YAML * Dynamic YAML badge handles YAML with a string * 'fetch' naming covention * Strict string validation in error message --- core/base-service/base.spec.js | 41 +++++------ package-lock.json | 26 +++---- package.json | 1 + services/dynamic/dynamic-json.service.js | 55 ++------------- services/dynamic/dynamic-json.tester.js | 2 +- services/dynamic/dynamic-yaml.service.js | 32 ++------- services/dynamic/dynamic-yaml.tester.js | 13 ++++ services/dynamic/json-path.js | 86 ++++++++++++++++++++++++ services/dynamic/json-path.spec.js | 22 ++++++ 9 files changed, 161 insertions(+), 117 deletions(-) create mode 100644 services/dynamic/json-path.js create mode 100644 services/dynamic/json-path.spec.js diff --git a/core/base-service/base.spec.js b/core/base-service/base.spec.js index 9acdd90a48..11025c6856 100644 --- a/core/base-service/base.spec.js +++ b/core/base-service/base.spec.js @@ -1,7 +1,8 @@ 'use strict' const Joi = require('@hapi/joi') -const { expect } = require('chai') +const chai = require('chai') +const { expect } = chai const sinon = require('sinon') const trace = require('./trace') const { @@ -14,6 +15,7 @@ const { const BaseService = require('./base') require('../register-chai-plugins.spec') +chai.use(require('chai-as-promised')) const queryParamSchema = Joi.object({ queryParamA: Joi.string(), @@ -97,17 +99,14 @@ describe('BaseService', function() { describe('Required overrides', function() { it('Should throw if render() is not overridden', function() { expect(() => BaseService.render()).to.throw( - 'render() function not implemented for BaseService' + /^render\(\) function not implemented for BaseService$/ ) }) - it('Should throw if route is not overridden', async function() { - try { - await BaseService.invoke({}, {}, {}) - expect.fail('Expected to throw') - } catch (e) { - expect(e.message).to.equal('Route not defined for BaseService') - } + it('Should throw if route is not overridden', function() { + return expect(BaseService.invoke({}, {}, {})).to.be.rejectedWith( + /^Route not defined for BaseService$/ + ) }) class WithRoute extends BaseService { @@ -115,18 +114,15 @@ describe('BaseService', function() { return {} } } - it('Should throw if handle() is not overridden', async function() { - try { - await WithRoute.invoke({}, {}, {}) - expect.fail('Expected to throw') - } catch (e) { - expect(e.message).to.equal('Handler not implemented for WithRoute') - } + it('Should throw if handle() is not overridden', function() { + return expect(WithRoute.invoke({}, {}, {})).to.be.rejectedWith( + /^Handler not implemented for WithRoute$/ + ) }) it('Should throw if category is not overridden', function() { expect(() => BaseService.category).to.throw( - 'Category not set for BaseService' + /^Category not set for BaseService$/ ) }) }) @@ -427,16 +423,15 @@ describe('BaseService', function() { requiredString: Joi.string().required(), }).required() - it('throws error for invalid responses', async function() { - try { + it('throws error for invalid responses', function() { + expect(() => DummyService._validate( { requiredString: ['this', "shouldn't", 'work'] }, dummySchema ) - expect.fail('Expected to throw') - } catch (e) { - expect(e).to.be.an.instanceof(InvalidResponse) - } + ) + .to.throw() + .instanceof(InvalidResponse) }) }) diff --git a/package-lock.json b/package-lock.json index 09d16fd8a1..3dd5145ec3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1395,15 +1395,6 @@ "@babel/helper-plugin-utils": "^7.0.0" } }, - "@babel/plugin-syntax-dynamic-import": { - "version": "7.2.0", - "resolved": "https://registry.npmjs.org/@babel/plugin-syntax-dynamic-import/-/plugin-syntax-dynamic-import-7.2.0.tgz", - "integrity": "sha512-mVxuJ0YroI/h/tbFTPGZR8cv6ai+STMKNBq0f8hFxsxWjl94qqhsb+wXbpNMDPU3cfR1TIsVFzU3nXyZMqyK4w==", - "dev": true, - "requires": { - "@babel/helper-plugin-utils": "^7.0.0" - } - }, "@babel/plugin-syntax-json-strings": { "version": "7.7.4", "resolved": "https://registry.npmjs.org/@babel/plugin-syntax-json-strings/-/plugin-syntax-json-strings-7.7.4.tgz", @@ -2032,15 +2023,6 @@ } } }, - "@babel/plugin-transform-spread": { - "version": "7.6.2", - "resolved": "https://registry.npmjs.org/@babel/plugin-transform-spread/-/plugin-transform-spread-7.6.2.tgz", - "integrity": "sha512-DpSvPFryKdK1x+EDJYCy28nmAaIMdxmhot62jAXF/o99iA33Zj2Lmcp3vDmz+MUh0LNYVPvfj5iC3feb3/+PFg==", - "dev": true, - "requires": { - "@babel/helper-plugin-utils": "^7.0.0" - } - }, "@babel/plugin-transform-sticky-regex": { "version": "7.7.4", "resolved": "https://registry.npmjs.org/@babel/plugin-transform-sticky-regex/-/plugin-transform-sticky-regex-7.7.4.tgz", @@ -7137,6 +7119,14 @@ } } }, + "chai-as-promised": { + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/chai-as-promised/-/chai-as-promised-7.1.1.tgz", + "integrity": "sha512-azL6xMoi+uxu6z4rhWQ1jbdUhOMhis2PvscD/xjLqNMkv3BPPp2JyyuTHOrf9BOosGpNQ11v6BKv/g57RXbiaA==", + "requires": { + "check-error": "^1.0.2" + } + }, "chai-datetime": { "version": "1.5.0", "resolved": "https://registry.npmjs.org/chai-datetime/-/chai-datetime-1.5.0.tgz", diff --git a/package.json b/package.json index 2129f116e2..911ac47636 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "bytes": "^3.1.0", "camelcase": "^5.3.1", "camp": "~17.2.4", + "chai-as-promised": "^7.1.1", "chalk": "^3.0.0", "check-node-version": "^4.0.1", "chrome-web-store-item-property": "~1.2.0", diff --git a/services/dynamic/dynamic-json.service.js b/services/dynamic/dynamic-json.service.js index 2f2c7c1ab0..398bd53842 100644 --- a/services/dynamic/dynamic-json.service.js +++ b/services/dynamic/dynamic-json.service.js @@ -1,62 +1,19 @@ 'use strict' -const Joi = require('@hapi/joi') -const jp = require('jsonpath') -const { renderDynamicBadge, errorMessages } = require('../dynamic-common') const { createRoute } = require('./dynamic-helpers') -const { BaseJsonService, InvalidParameter, InvalidResponse } = require('..') - -module.exports = class DynamicJson extends BaseJsonService { - static get category() { - return 'dynamic' - } +const jsonPath = require('./json-path') +const { BaseJsonService } = require('..') +module.exports = class DynamicJson extends jsonPath(BaseJsonService) { static get route() { return createRoute('json') } - static get defaultBadgeData() { - return { - label: 'custom badge', - } - } - - async handle(namedParams, { url, query: pathExpression, prefix, suffix }) { - const data = await this._requestJson({ - schema: Joi.any(), + async fetch({ schema, url, errorMessages }) { + return this._requestJson({ + schema, url, errorMessages, }) - - // JSONPath only works on objects and arrays. - // https://github.com/badges/shields/issues/4018 - if (typeof data !== 'object') { - throw new InvalidResponse({ - prettyMessage: 'json must contain an object or array', - }) - } - - let values - try { - values = jp.query(data, pathExpression) - } catch (e) { - const { message } = e - if ( - message.startsWith('Lexical error') || - message.startsWith('Parse error') - ) { - throw new InvalidParameter({ - prettyMessage: 'unparseable jsonpath query', - }) - } else { - throw e - } - } - - if (!values.length) { - throw new InvalidResponse({ prettyMessage: 'no result' }) - } - - return renderDynamicBadge({ value: values, prefix, suffix }) } } diff --git a/services/dynamic/dynamic-json.tester.js b/services/dynamic/dynamic-json.tester.js index d8d43f4e90..562d9af4dd 100644 --- a/services/dynamic/dynamic-json.tester.js +++ b/services/dynamic/dynamic-json.tester.js @@ -181,6 +181,6 @@ t.create('JSON contains a string') ) .expectBadge({ label: 'custom badge', - message: 'json must contain an object or array', + message: 'resource must contain an object or array', color: 'lightgrey', }) diff --git a/services/dynamic/dynamic-yaml.service.js b/services/dynamic/dynamic-yaml.service.js index e8d6503823..6e0e40eeb9 100644 --- a/services/dynamic/dynamic-yaml.service.js +++ b/services/dynamic/dynamic-yaml.service.js @@ -1,39 +1,19 @@ 'use strict' -const Joi = require('@hapi/joi') -const jp = require('jsonpath') -const { renderDynamicBadge, errorMessages } = require('../dynamic-common') const { createRoute } = require('./dynamic-helpers') -const { BaseYamlService, InvalidResponse } = require('..') - -module.exports = class DynamicYaml extends BaseYamlService { - static get category() { - return 'dynamic' - } +const jsonPath = require('./json-path') +const { BaseYamlService } = require('..') +module.exports = class DynamicYaml extends jsonPath(BaseYamlService) { static get route() { return createRoute('yaml') } - static get defaultBadgeData() { - return { - label: 'custom badge', - } - } - - async handle(namedParams, { url, query: pathExpression, prefix, suffix }) { - const data = await this._requestYaml({ - schema: Joi.any(), + async fetch({ schema, url, errorMessages }) { + return this._requestYaml({ + schema, url, errorMessages, }) - - const values = jp.query(data, pathExpression) - - if (!values.length) { - throw new InvalidResponse({ prettyMessage: 'no result' }) - } - - return renderDynamicBadge({ value: values, prefix, suffix }) } } diff --git a/services/dynamic/dynamic-yaml.tester.js b/services/dynamic/dynamic-yaml.tester.js index ad52a5ec0f..cfcc4f3dd2 100644 --- a/services/dynamic/dynamic-yaml.tester.js +++ b/services/dynamic/dynamic-yaml.tester.js @@ -101,3 +101,16 @@ t.create('YAML from url | error color overrides user specified') message: 'invalid query parameter: url', color: 'red', }) + +t.create('YAML contains a string') + .get('.json?url=https://example.test/yaml&query=$.foo,') + .intercept(nock => + nock('https://example.test') + .get('/yaml') + .reply(200, '"foo"') + ) + .expectBadge({ + label: 'custom badge', + message: 'resource must contain an object or array', + color: 'lightgrey', + }) diff --git a/services/dynamic/json-path.js b/services/dynamic/json-path.js new file mode 100644 index 0000000000..00537dbfe9 --- /dev/null +++ b/services/dynamic/json-path.js @@ -0,0 +1,86 @@ +/** + * @module + */ + +'use strict' + +const Joi = require('@hapi/joi') +const jp = require('jsonpath') +const { renderDynamicBadge, errorMessages } = require('../dynamic-common') +const { InvalidParameter, InvalidResponse } = require('..') + +/** + * Dynamic service class factory which wraps {@link module:core/base-service/base~BaseService} with support of {@link https://jsonpath.com/|JSONPath}. + * + * @param {Function} superclass class to extend + * @returns {Function} wrapped class + */ +module.exports = superclass => + class extends superclass { + static get category() { + return 'dynamic' + } + + static get defaultBadgeData() { + return { + label: 'custom badge', + } + } + + /** + * Request data from an upstream API, transform it to JSON and validate against a schema + * + * @param {object} attrs Refer to individual attrs + * @param {Joi} attrs.schema Joi schema to validate the response transformed to JSON + * @param {string} attrs.url URL to request + * @param {object} [attrs.errorMessages={}] Key-value map of status codes + * and custom error messages e.g: `{ 404: 'package not found' }`. + * This can be used to extend or override the + * [default](https://github.com/badges/shields/blob/master/services/dynamic-common.js#L8) + * @returns {object} Parsed response + */ + async fetch({ schema, url, errorMessages }) { + throw new Error( + `fetch() function not implemented for ${this.constructor.name}` + ) + } + + async handle(namedParams, { url, query: pathExpression, prefix, suffix }) { + const data = await this.fetch({ + schema: Joi.any(), + url, + errorMessages, + }) + + // JSONPath only works on objects and arrays. + // https://github.com/badges/shields/issues/4018 + if (typeof data !== 'object') { + throw new InvalidResponse({ + prettyMessage: 'resource must contain an object or array', + }) + } + + let values + try { + values = jp.query(data, pathExpression) + } catch (e) { + const { message } = e + if ( + message.startsWith('Lexical error') || + message.startsWith('Parse error') + ) { + throw new InvalidParameter({ + prettyMessage: 'unparseable jsonpath query', + }) + } else { + throw e + } + } + + if (!values.length) { + throw new InvalidResponse({ prettyMessage: 'no result' }) + } + + return renderDynamicBadge({ value: values, prefix, suffix }) + } + } diff --git a/services/dynamic/json-path.spec.js b/services/dynamic/json-path.spec.js new file mode 100644 index 0000000000..decaf38779 --- /dev/null +++ b/services/dynamic/json-path.spec.js @@ -0,0 +1,22 @@ +'use strict' + +const chai = require('chai') +const { expect } = chai +const jsonPath = require('./json-path') + +chai.use(require('chai-as-promised')) + +describe('JSON Path service factory', function() { + describe('fetch()', function() { + it('should throw error if it is not overridden', function() { + class BaseService {} + class JsonPathService extends jsonPath(BaseService) {} + const jsonPathServiceInstance = new JsonPathService() + + return expect(jsonPathServiceInstance.fetch({})).to.be.rejectedWith( + Error, + 'fetch() function not implemented for JsonPathService' + ) + }) + }) +})