From b5ac5d604408c6c264622916bee41a22a1608083 Mon Sep 17 00:00:00 2001 From: chris48s Date: Sun, 16 Dec 2018 20:29:20 +0000 Subject: [PATCH] [dynamicyaml f-droid] add yaml base service class (#2540) --- services/base-yaml.js | 49 +++++++ services/base-yaml.spec.js | 156 +++++++++++++++++++++++ services/dynamic/dynamic-yaml.service.js | 39 +----- services/f-droid/f-droid.service.js | 33 ++--- services/f-droid/f-droid.tester.js | 6 +- 5 files changed, 225 insertions(+), 58 deletions(-) create mode 100644 services/base-yaml.js create mode 100644 services/base-yaml.spec.js diff --git a/services/base-yaml.js b/services/base-yaml.js new file mode 100644 index 0000000000..c8d857e455 --- /dev/null +++ b/services/base-yaml.js @@ -0,0 +1,49 @@ +'use strict' + +const BaseService = require('./base') +const emojic = require('emojic') +const { InvalidResponse } = require('./errors') +const trace = require('./trace') +const yaml = require('js-yaml') + +class BaseYamlService extends BaseService { + async _requestYaml({ + schema, + url, + options = {}, + errorMessages = {}, + encoding = 'utf8', + }) { + const logTrace = (...args) => trace.logTrace('fetch', ...args) + const mergedOptions = { + ...{ + headers: { + Accept: + 'text/x-yaml, text/yaml, application/x-yaml, application/yaml, text/plain', + }, + }, + ...options, + } + const { buffer } = await this._request({ + url, + options: mergedOptions, + errorMessages, + }) + let parsed + try { + parsed = yaml.safeLoad(buffer.toString(), encoding) + } catch (err) { + logTrace(emojic.dart, 'Response YAML (unparseable)', buffer) + throw new InvalidResponse({ + prettyMessage: 'unparseable yaml response', + underlyingError: err, + }) + } + logTrace(emojic.dart, 'Response YAML (before validation)', parsed, { + deep: true, + }) + return this.constructor._validate(parsed, schema) + } +} + +module.exports = BaseYamlService diff --git a/services/base-yaml.spec.js b/services/base-yaml.spec.js new file mode 100644 index 0000000000..134236472b --- /dev/null +++ b/services/base-yaml.spec.js @@ -0,0 +1,156 @@ +'use strict' + +const Joi = require('joi') +const { expect } = require('chai') +const sinon = require('sinon') +const BaseYamlService = require('./base-yaml') + +const dummySchema = Joi.object({ + requiredString: Joi.string().required(), +}).required() + +class DummyYamlService extends BaseYamlService { + static get category() { + return 'cat' + } + + static get route() { + return { + base: 'foo', + } + } + + async handle() { + const { requiredString } = await this._requestYaml({ + schema: dummySchema, + url: 'http://example.com/foo.yaml', + }) + return { message: requiredString } + } +} + +const expectedYaml = ` +--- +requiredString: some-string +` + +const unexpectedYaml = ` +--- +unexpectedKey: some-string +` + +const invalidYaml = ` +--- +foo: bar +foo: baz +` + +describe('BaseYamlService', function() { + describe('Making requests', function() { + let sendAndCacheRequest + beforeEach(function() { + sendAndCacheRequest = sinon.stub().returns( + Promise.resolve({ + buffer: expectedYaml, + res: { statusCode: 200 }, + }) + ) + }) + + it('invokes _sendAndCacheRequest', async function() { + await DummyYamlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + + expect(sendAndCacheRequest).to.have.been.calledOnceWith( + 'http://example.com/foo.yaml', + { + headers: { + Accept: + 'text/x-yaml, text/yaml, application/x-yaml, application/yaml, text/plain', + }, + } + ) + }) + + it('forwards options to _sendAndCacheRequest', async function() { + class WithOptions extends DummyYamlService { + async handle() { + const { value } = await this._requestYaml({ + schema: dummySchema, + url: 'http://example.com/foo.yaml', + options: { method: 'POST', qs: { queryParam: 123 } }, + }) + return { message: value } + } + } + + await WithOptions.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + + expect(sendAndCacheRequest).to.have.been.calledOnceWith( + 'http://example.com/foo.yaml', + { + headers: { + Accept: + 'text/x-yaml, text/yaml, application/x-yaml, application/yaml, text/plain', + }, + method: 'POST', + qs: { queryParam: 123 }, + } + ) + }) + }) + + describe('Making badges', function() { + it('handles valid yaml responses', async function() { + const sendAndCacheRequest = async () => ({ + buffer: expectedYaml, + res: { statusCode: 200 }, + }) + expect( + await DummyYamlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ + message: 'some-string', + }) + }) + + it('handles yaml responses which do not match the schema', async function() { + const sendAndCacheRequest = async () => ({ + buffer: unexpectedYaml, + res: { statusCode: 200 }, + }) + expect( + await DummyYamlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ + color: 'lightgray', + message: 'invalid response data', + }) + }) + + it('handles unparseable yaml responses', async function() { + const sendAndCacheRequest = async () => ({ + buffer: invalidYaml, + res: { statusCode: 200 }, + }) + expect( + await DummyYamlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ + color: 'lightgray', + message: 'unparseable yaml response', + }) + }) + }) +}) diff --git a/services/dynamic/dynamic-yaml.service.js b/services/dynamic/dynamic-yaml.service.js index 6210d8fae5..6ee391f3d5 100644 --- a/services/dynamic/dynamic-yaml.service.js +++ b/services/dynamic/dynamic-yaml.service.js @@ -1,11 +1,9 @@ 'use strict' -const yaml = require('js-yaml') -const jp = require('jsonpath') -const emojic = require('emojic') -const BaseService = require('../base') +const BaseYamlService = require('../base-yaml') const { InvalidResponse } = require('../errors') -const trace = require('../trace') +const Joi = require('joi') +const jp = require('jsonpath') const { createRoute, queryParamSchema, @@ -13,7 +11,7 @@ const { renderDynamicBadge, } = require('./dynamic-helpers') -module.exports = class DynamicYaml extends BaseService { +module.exports = class DynamicYaml extends BaseYamlService { static get category() { return 'dynamic' } @@ -28,24 +26,6 @@ module.exports = class DynamicYaml extends BaseService { } } - parseYml(buffer) { - const logTrace = (...args) => trace.logTrace('fetch', ...args) - let parsed - try { - parsed = yaml.safeLoad(buffer) - } catch (err) { - logTrace(emojic.dart, 'Response YAML (unparseable)', buffer) - throw new InvalidResponse({ - prettyMessage: 'unparseable yaml response', - underlyingError: err, - }) - } - logTrace(emojic.dart, 'Response YAML (before validation)', parsed, { - deep: true, - }) - return parsed - } - async handle(namedParams, queryParams) { const { url, @@ -54,19 +34,12 @@ module.exports = class DynamicYaml extends BaseService { suffix, } = this.constructor._validateQueryParams(queryParams, queryParamSchema) - const { buffer } = await this._request({ + const data = await this._requestYaml({ + schema: Joi.any(), url, - options: { - headers: { - Accept: - 'text/x-yaml, text/yaml, application/x-yaml, application/yaml, text/plain', - }, - }, errorMessages, }) - const data = this.parseYml(buffer) - const values = jp.query(data, pathExpression) if (!values.length) { diff --git a/services/f-droid/f-droid.service.js b/services/f-droid/f-droid.service.js index 30ea7af568..5ca02ca28d 100644 --- a/services/f-droid/f-droid.service.js +++ b/services/f-droid/f-droid.service.js @@ -1,13 +1,18 @@ 'use strict' const Joi = require('joi') -const yaml = require('js-yaml') -const BaseService = require('../base') +const BaseYamlService = require('../base-yaml') const { addv: versionText } = require('../../lib/text-formatters') const { version: versionColor } = require('../../lib/color-formatters') const { InvalidResponse } = require('../errors') -module.exports = class FDroid extends BaseService { +const schema = Joi.object({ + CurrentVersion: Joi.alternatives() + .try(Joi.number(), Joi.string()) + .required(), +}).required() + +module.exports = class FDroid extends BaseYamlService { static render({ version }) { return { message: versionText(version), @@ -45,28 +50,12 @@ module.exports = class FDroid extends BaseService { } async fetchYaml(url, options) { - const { buffer } = await this._request({ + const yaml = await this._requestYaml({ + schema, url: `${url}.yml`, ...options, }) - - // we assume the yaml layout as provided here: - // https://gitlab.com/fdroid/fdroiddata/raw/master/metadata/org.dystopia.email.yml - try { - const { CurrentVersion: version } = yaml.safeLoad( - buffer.toString(), - 'utf8' - ) - if (!version) { - throw new Error('could not find version on website') - } - return { version } - } catch (error) { - throw new InvalidResponse({ - prettyMessage: 'invalid response', - underlyingError: error, - }) - } + return { version: yaml['CurrentVersion'] } } async fetchText(url, options) { diff --git a/services/f-droid/f-droid.tester.js b/services/f-droid/f-droid.tester.js index 8ec9d04217..65510c37f4 100644 --- a/services/f-droid/f-droid.tester.js +++ b/services/f-droid/f-droid.tester.js @@ -155,7 +155,7 @@ t.create('Package is found yml matadata format with missing "CurrentVersion"') .get(`${path}.yml`) .reply(200, 'Categories: System') ) - .expectJSON({ name: 'f-droid', value: 'invalid response' }) + .expectJSON({ name: 'f-droid', value: 'invalid response data' }) t.create('Package is found with bad yml matadata format') .get('/v/axp.tool.apkextractor.json?metadata_format=yml') @@ -164,7 +164,7 @@ t.create('Package is found with bad yml matadata format') .get(`${path}.yml`) .reply(200, '.CurrentVersion: 1.4') ) - .expectJSON({ name: 'f-droid', value: 'invalid response' }) + .expectJSON({ name: 'f-droid', value: 'invalid response data' }) t.create('Package is not found') .get('/v/axp.tool.apkextractor.json') @@ -187,7 +187,7 @@ t.create('The api changed') .get(`${path}.yml`) .reply(200, '') ) - .expectJSON({ name: 'f-droid', value: 'invalid response' }) + .expectJSON({ name: 'f-droid', value: 'invalid response data' }) t.create('Package is not found due invalid metadata format') .get('/v/axp.tool.apkextractor.json?metadata_format=xml')