Split BaseService and BaseJsonService into separate modules (#1889)

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.
badges · Aug 11, 2018 · db4bffb · db4bffb
1 parent ed81dcb
commit db4bffb
Show file tree

Hide file tree

Showing 15 changed files with 76 additions and 71 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,6 +1,6 @@
 'use strict'
 
-const { BaseService } = require('../base')
+const BaseService = require('../base')
 
 module.exports = class Time extends BaseService {
   async handle() {