From db4bffb300a8d9c794fbd089a2f0f9586415e500 Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Sat, 11 Aug 2018 10:43:05 -0400 Subject: [PATCH] Split BaseService and BaseJsonService into separate modules (#1889) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There’s a lot of behavior here, and going to be even more, so I think it makes sense to split these up as I’ve done with the tests. --- services/apm/apm.service.js | 2 +- services/appveyor/appveyor.service.js | 2 +- services/base-json.js | 62 +++++++++++++++++++++++++++ services/base-json.spec.js | 2 +- services/base.js | 59 +------------------------ services/base.spec.js | 2 +- services/cdnjs/cdnjs.service.js | 2 +- services/clojars/clojars.service.js | 2 +- services/gem/gem-downloads.service.js | 2 +- services/gem/gem-owner.service.js | 2 +- services/gem/gem-rank.service.js | 2 +- services/gem/gem-version.service.js | 2 +- services/npm/npm-base.js | 2 +- services/npm/npm-downloads.service.js | 2 +- services/time/time.service.js | 2 +- 15 files changed, 76 insertions(+), 71 deletions(-) create mode 100644 services/base-json.js diff --git a/services/apm/apm.service.js b/services/apm/apm.service.js index 0725124e9c30f..6ad55f8e72488 100644 --- a/services/apm/apm.service.js +++ b/services/apm/apm.service.js @@ -1,7 +1,7 @@ 'use strict' const Joi = require('joi') -const { BaseJsonService } = require('../base') +const BaseJsonService = require('../base-json') const { InvalidResponse } = require('../errors') const { version: versionColor } = require('../../lib/color-formatters') const { metric, addv } = require('../../lib/text-formatters') diff --git a/services/appveyor/appveyor.service.js b/services/appveyor/appveyor.service.js index ec4c9f4b98b92..d7660416d428e 100644 --- a/services/appveyor/appveyor.service.js +++ b/services/appveyor/appveyor.service.js @@ -1,7 +1,7 @@ 'use strict' const Joi = require('joi') -const { BaseJsonService } = require('../base') +const BaseJsonService = require('../base-json') const appVeyorSchema = Joi.object({ build: Joi.object({ diff --git a/services/base-json.js b/services/base-json.js new file mode 100644 index 0000000000000..8425a2d1926c8 --- /dev/null +++ b/services/base-json.js @@ -0,0 +1,62 @@ +'use strict' + +// See available emoji at http://emoji.muan.co/ +const emojic = require('emojic') +const Joi = require('joi') +const { checkErrorResponse, asJson } = require('../lib/error-helper') +const BaseService = require('./base') +const { InvalidResponse } = require('./errors') + +class BaseJsonService extends BaseService { + static _validate(json, schema) { + const { error, value } = Joi.validate(json, schema, { + allowUnknown: true, + stripUnknown: true, + }) + if (error) { + this.logTrace( + 'error', + emojic.womanShrugging, + 'Response did not match schema', + error.message + ) + throw new InvalidResponse({ + prettyMessage: 'invalid json response', + underlyingError: error, + }) + } else { + this.logTrace('validate', emojic.bathtub, 'JSON after validation', value) + return value + } + } + + async _requestJson({ schema, url, options = {}, notFoundMessage }) { + const logTrace = (...args) => this.constructor.logTrace('fetch', ...args) + if (!schema || !schema.isJoi) { + throw Error('A Joi schema is required') + } + const mergedOptions = { + ...{ headers: { Accept: 'application/json' } }, + ...options, + } + logTrace(emojic.bowAndArrow, 'Request', url, '\n', mergedOptions) + return this._sendAndCacheRequest(url, mergedOptions) + .then(({ res, buffer }) => { + logTrace(emojic.dart, 'Response status code', res.statusCode) + return { res, buffer } + }) + .then( + checkErrorResponse.asPromise( + notFoundMessage ? { notFoundMessage: notFoundMessage } : undefined + ) + ) + .then(asJson) + .then(json => { + logTrace(emojic.dart, 'Response JSON (before validation)', json) + return json + }) + .then(json => this.constructor._validate(json, schema)) + } +} + +module.exports = BaseJsonService diff --git a/services/base-json.spec.js b/services/base-json.spec.js index 012e718da0a73..5ad73a12d117a 100644 --- a/services/base-json.spec.js +++ b/services/base-json.spec.js @@ -4,7 +4,7 @@ const Joi = require('joi') const chai = require('chai') const { expect } = chai -const { BaseJsonService } = require('./base') +const BaseJsonService = require('./base-json') const { invalidJSON } = require('./response-fixtures') chai.use(require('chai-as-promised')) diff --git a/services/base.js b/services/base.js index 67e510b8c499c..d36819815ed3d 100644 --- a/services/base.js +++ b/services/base.js @@ -1,6 +1,5 @@ 'use strict' -const Joi = require('joi') // See available emoji at http://emoji.muan.co/ const emojic = require('emojic') const chalk = require('chalk') @@ -12,7 +11,6 @@ const { makeColor, setBadgeColor, } = require('../lib/badge-data') -const { checkErrorResponse, asJson } = require('../lib/error-helper') // Config is loaded globally but it would be better to inject it. To do that, // there needs to be one instance of the service created at registration time, // which gets the config injected into it, instead of one instance per request. @@ -308,59 +306,4 @@ class BaseService { } } -class BaseJsonService extends BaseService { - static _validate(json, schema) { - const { error, value } = Joi.validate(json, schema, { - allowUnknown: true, - stripUnknown: true, - }) - if (error) { - this.logTrace( - 'error', - emojic.womanShrugging, - 'Response did not match schema', - error.message - ) - throw new InvalidResponse({ - prettyMessage: 'invalid json response', - underlyingError: error, - }) - } else { - this.logTrace('validate', emojic.bathtub, 'JSON after validation', value) - return value - } - } - - async _requestJson({ schema, url, options = {}, notFoundMessage }) { - const logTrace = (...args) => this.constructor.logTrace('fetch', ...args) - if (!schema || !schema.isJoi) { - throw Error('A Joi schema is required') - } - const mergedOptions = { - ...{ headers: { Accept: 'application/json' } }, - ...options, - } - logTrace(emojic.bowAndArrow, 'Request', url, '\n', mergedOptions) - return this._sendAndCacheRequest(url, mergedOptions) - .then(({ res, buffer }) => { - logTrace(emojic.dart, 'Response status code', res.statusCode) - return { res, buffer } - }) - .then( - checkErrorResponse.asPromise( - notFoundMessage ? { notFoundMessage: notFoundMessage } : undefined - ) - ) - .then(asJson) - .then(json => { - logTrace(emojic.dart, 'Response JSON (before validation)', json) - return json - }) - .then(json => this.constructor._validate(json, schema)) - } -} - -module.exports = { - BaseService, - BaseJsonService, -} +module.exports = BaseService diff --git a/services/base.spec.js b/services/base.spec.js index 5faf615d7eec2..2881bc2b671a1 100644 --- a/services/base.spec.js +++ b/services/base.spec.js @@ -4,7 +4,7 @@ const { expect } = require('chai') const { test, given, forCases } = require('sazerac') const sinon = require('sinon') -const { BaseService } = require('./base') +const BaseService = require('./base') require('../lib/register-chai-plugins.spec') diff --git a/services/cdnjs/cdnjs.service.js b/services/cdnjs/cdnjs.service.js index 6b9cf7596f673..1a80be9538fad 100644 --- a/services/cdnjs/cdnjs.service.js +++ b/services/cdnjs/cdnjs.service.js @@ -1,7 +1,7 @@ 'use strict' const Joi = require('joi') -const { BaseJsonService } = require('../base') +const BaseJsonService = require('../base-json') const { NotFound } = require('../errors') const { addv: versionText } = require('../../lib/text-formatters') const { version: versionColor } = require('../../lib/color-formatters') diff --git a/services/clojars/clojars.service.js b/services/clojars/clojars.service.js index 43f3f36ebb352..a73cea9d42ab6 100644 --- a/services/clojars/clojars.service.js +++ b/services/clojars/clojars.service.js @@ -1,7 +1,7 @@ 'use strict' const Joi = require('joi') -const { BaseJsonService } = require('../base') +const BaseJsonService = require('../base-json') const { NotFound } = require('../errors') const { version: versionColor } = require('../../lib/color-formatters') diff --git a/services/gem/gem-downloads.service.js b/services/gem/gem-downloads.service.js index 6d0fa3bf15d93..4c2de124eb49a 100644 --- a/services/gem/gem-downloads.service.js +++ b/services/gem/gem-downloads.service.js @@ -3,7 +3,7 @@ const semver = require('semver') const Joi = require('joi') -const { BaseJsonService } = require('../base') +const BaseJsonService = require('../base-json') const { InvalidResponse } = require('../errors') const { downloadCount: downloadCountColor, diff --git a/services/gem/gem-owner.service.js b/services/gem/gem-owner.service.js index 2ac085499f0a6..1a8d25d927b3f 100644 --- a/services/gem/gem-owner.service.js +++ b/services/gem/gem-owner.service.js @@ -2,7 +2,7 @@ const Joi = require('joi') -const { BaseJsonService } = require('../base') +const BaseJsonService = require('../base-json') const { floorCount: floorCountColor } = require('../../lib/color-formatters') const ownerSchema = Joi.array().required() diff --git a/services/gem/gem-rank.service.js b/services/gem/gem-rank.service.js index 8e50f1b570d92..e73ddd16525a4 100644 --- a/services/gem/gem-rank.service.js +++ b/services/gem/gem-rank.service.js @@ -2,7 +2,7 @@ const Joi = require('joi') -const { BaseJsonService } = require('../base') +const BaseJsonService = require('../base-json') const { floorCount: floorCountColor } = require('../../lib/color-formatters') const { ordinalNumber } = require('../../lib/text-formatters') const { nonNegativeInteger } = require('../validators.js') diff --git a/services/gem/gem-version.service.js b/services/gem/gem-version.service.js index 615f0d3cca9e2..b44eb9ff179fb 100644 --- a/services/gem/gem-version.service.js +++ b/services/gem/gem-version.service.js @@ -2,7 +2,7 @@ const Joi = require('joi') -const { BaseJsonService } = require('../base') +const BaseJsonService = require('../base-json') const { addv: versionText } = require('../../lib/text-formatters') const { version: versionColor } = require('../../lib/color-formatters') diff --git a/services/npm/npm-base.js b/services/npm/npm-base.js index 956978c05d622..692d0e272aa0c 100644 --- a/services/npm/npm-base.js +++ b/services/npm/npm-base.js @@ -1,7 +1,7 @@ 'use strict' const Joi = require('joi') -const { BaseJsonService } = require('../base') +const BaseJsonService = require('../base-json') const { InvalidResponse, NotFound } = require('../errors') const deprecatedLicenseObjectSchema = Joi.object({ diff --git a/services/npm/npm-downloads.service.js b/services/npm/npm-downloads.service.js index 1604e343e5984..401a693db5dd9 100644 --- a/services/npm/npm-downloads.service.js +++ b/services/npm/npm-downloads.service.js @@ -1,7 +1,7 @@ 'use strict' const Joi = require('joi') -const { BaseJsonService } = require('../base') +const BaseJsonService = require('../base-json') const { metric } = require('../../lib/text-formatters') const { nonNegativeInteger } = require('../validators.js') diff --git a/services/time/time.service.js b/services/time/time.service.js index 04f83eb50f7fd..76ef1e62d194f 100644 --- a/services/time/time.service.js +++ b/services/time/time.service.js @@ -1,6 +1,6 @@ 'use strict' -const { BaseService } = require('../base') +const BaseService = require('../base') module.exports = class Time extends BaseService { async handle() {