Refactor [GemDownloads] for readability; improve error messages (#2870)

Close #1960

services/gem/gem-downloads.service.js

@@ -5,17 +5,17 @@ const Joi = require('joi')
 const { downloadCount } = require('../../lib/color-formatters')
 const { metric } = require('../../lib/text-formatters')
 const { latest: latestVersion } = require('../../lib/version')
-const { BaseJsonService, InvalidResponse } = require('..')
+const { BaseJsonService, InvalidParameter, InvalidResponse } = require('..')
 const { nonNegativeInteger } = require('../validators')
 
 const keywords = ['ruby']
 
-const gemsSchema = Joi.object({
+const gemSchema = Joi.object({
   downloads: nonNegativeInteger,
   version_downloads: nonNegativeInteger,
 }).required()
 
-const versionsSchema = Joi.array()
+const versionSchema = Joi.array()
   .items(
     Joi.object({
       prerelease: Joi.boolean().required(),
@@ -27,17 +27,56 @@ const versionsSchema = Joi.array()
   .required()
 
 module.exports = class GemDownloads extends BaseJsonService {
-  async fetch({ repo, info }) {
-    const endpoint = info === 'dv' ? 'versions/' : 'gems/'
-    const schema = info === 'dv' ? versionsSchema : gemsSchema
-    const url = `https://rubygems.org/api/v1/${endpoint}${repo}.json`
-    return this._requestJson({
-      url,
-      schema,
+  async fetchDownloadCountForVersion({ gem, version }) {
+    const json = await this._requestJson({
+      url: `https://rubygems.org/api/v1/versions/${gem}.json`,
+      schema: versionSchema,
+      errorMessages: {
+        404: 'gem not found',
+      },
     })
+
+    let wantedVersion
+    if (version === 'stable') {
+      wantedVersion = latestVersion(
+        json.filter(({ prerelease }) => !prerelease).map(({ number }) => number)
+      )
+    } else {
+      wantedVersion = version
+    }
+
+    const versionData = json.find(({ number }) => number === wantedVersion)
+    if (versionData) {
+      return versionData.downloads_count
+    } else {
+      throw new InvalidResponse({
+        prettyMessage: 'version not found',
+      })
+    }
   }
 
-  static render({ label, downloads }) {
+  async fetchDownloadCountForGem({ gem }) {
+    const {
+      downloads: totalDownloads,
+      version_downloads: versionDownloads,
+    } = await this._requestJson({
+      url: `https://rubygems.org/api/v1/gems/${gem}.json`,
+      schema: gemSchema,
+      errorMessages: {
+        404: 'gem not found',
+      },
+    })
+    return { totalDownloads, versionDownloads }
+  }
+
+  static render({ which, version, downloads }) {
+    let label
+    if (version) {
+      label = `downloads@${version}`
+    } else if (which === 'dtv') {
+      label = 'downloads@latest'
+    }
+
     return {
       label,
       message: metric(downloads),
@@ -45,58 +84,28 @@ module.exports = class GemDownloads extends BaseJsonService {
     }
   }
 
-  static _getLabel(version, info) {
-    if (version) {
-      return `downloads@${version}`
-    } else {
-      if (info === 'dtv') {
-        return 'downloads@latest'
-      } else {
-        return 'downloads'
-      }
-    }
-  }
-
-  async handle({ info, rubygem }) {
-    const splitRubygem = rubygem.split('/')
-    const repo = splitRubygem[0]
-    let version =
-      splitRubygem.length > 1 ? splitRubygem[splitRubygem.length - 1] : null
-    version = version === 'stable' ? version : semver.valid(version)
-    const label = this.constructor._getLabel(version, info)
-    const json = await this.fetch({ repo, info })
-
+  async handle({ which, gem, version }) {
     let downloads
-    if (info === 'dt') {
-      downloads = json.downloads
-    } else if (info === 'dtv') {
-      downloads = json.version_downloads
-    } else if (info === 'dv') {
-      let versionData
-      if (version !== null && version === 'stable') {
-        const versions = json
-          .filter(ver => ver.prerelease === false)
-          .map(ver => ver.number)
-        // Found latest stable version.
-        const stableVersion = latestVersion(versions)
-        versionData = json.filter(ver => ver.number === stableVersion)[0]
-        downloads = versionData.downloads_count
-      } else if (version !== null) {
-        versionData = json.filter(ver => ver.number === version)[0]
-
-        downloads = versionData.downloads_count
-      } else {
-        throw new InvalidResponse({
-          underlyingError: new Error('version is null'),
+    if (which === 'dv') {
+      if (!version) {
+        throw new InvalidParameter({
+          prettyMessage: 'version downloads requires a version',
         })
       }
+      if (version !== 'stable' && !semver.valid(version)) {
+        throw new InvalidParameter({
+          prettyMessage: 'version should be "stable" or valid semver',
+        })
+      }
+      downloads = await this.fetchDownloadCountForVersion({ gem, version })
     } else {
-      throw new InvalidResponse({
-        underlyingError: new Error('info is invalid'),
-      })
+      const {
+        totalDownloads,
+        versionDownloads,
+      } = await this.fetchDownloadCountForGem({ gem, which })
+      downloads = which === 'dtv' ? versionDownloads : totalDownloads
     }
-
-    return this.constructor.render({ label, downloads })
+    return this.constructor.render({ which, version, downloads })
   }
 
   // Metadata
@@ -111,8 +120,7 @@ module.exports = class GemDownloads extends BaseJsonService {
   static get route() {
     return {
       base: 'gem',
-      format: '(dt|dtv|dv)/(.+)',
-      capture: ['info', 'rubygem'],
+      pattern: ':which(dt|dtv|dv)/:gem/:version?',
     }
   }
 
@@ -126,7 +134,8 @@ module.exports = class GemDownloads extends BaseJsonService {
           version: 'stable',
         },
         staticPreview: this.render({
-          label: this._getLabel('stable', 'dv'),
+          which: 'dv',
+          version: 'stable',
           downloads: 70000,
         }),
         keywords,
@@ -139,7 +148,8 @@ module.exports = class GemDownloads extends BaseJsonService {
           version: '4.1.0',
         },
         staticPreview: this.render({
-          label: this._getLabel('4.1.0', 'dv'),
+          which: 'dv',
+          version: '4.1.0',
           downloads: 50000,
         }),
         keywords,
@@ -149,7 +159,7 @@ module.exports = class GemDownloads extends BaseJsonService {
         pattern: 'dtv/:gem',
         namedParams: { gem: 'rails' },
         staticPreview: this.render({
-          label: this._getLabel(undefined, 'dtv'),
+          which: 'dtv',
           downloads: 70000,
         }),
         keywords,
@@ -159,7 +169,7 @@ module.exports = class GemDownloads extends BaseJsonService {
         pattern: 'dt/:gem',
         namedParams: { gem: 'rails' },
         staticPreview: this.render({
-          label: this._getLabel(undefined, 'dt'),
+          which: 'dt',
           downloads: 900000,
         }),
         keywords,

services/gem/gem-downloads.tester.js

@@ -5,7 +5,6 @@ const { isMetric } = require('../test-validators')
 
 const t = (module.exports = require('..').createServiceTester())
 
-// total downloads
 t.create('total downloads (valid)')
   .get('/dt/rails.json')
   .expectJSONTypes(
@@ -17,9 +16,8 @@ t.create('total downloads (valid)')
 
 t.create('total downloads (not found)')
   .get('/dt/not-a-package.json')
-  .expectJSON({ name: 'downloads', value: 'not found' })
+  .expectJSON({ name: 'downloads', value: 'gem not found' })
 
-// version downloads
 t.create('version downloads (valid, stable version)')
   .get('/dv/rails/stable.json')
   .expectJSONTypes(
@@ -40,17 +38,29 @@ t.create('version downloads (valid, specific version)')
 
 t.create('version downloads (package not found)')
   .get('/dv/not-a-package/4.1.0.json')
-  .expectJSON({ name: 'downloads', value: 'not found' })
+  .expectJSON({ name: 'downloads', value: 'gem not found' })
 
 t.create('version downloads (valid package, invalid version)')
   .get('/dv/rails/not-a-version.json')
-  .expectJSON({ name: 'downloads', value: 'invalid' })
+  .expectJSON({
+    name: 'downloads',
+    value: 'version should be "stable" or valid semver',
+  })
+
+t.create('version downloads (valid package, version not found)')
+  .get('/dv/rails/8.8.8.json')
+  .expectJSON({
+    name: 'downloads',
+    value: 'version not found',
+  })
 
 t.create('version downloads (valid package, version not specified)')
   .get('/dv/rails.json')
-  .expectJSON({ name: 'downloads', value: 'invalid' })
+  .expectJSON({
+    name: 'downloads',
+    value: 'version downloads requires a version',
+  })
 
-// latest version downloads
 t.create('latest version downloads (valid)')
   .get('/dtv/rails.json')
   .expectJSONTypes(
@@ -62,4 +72,4 @@ t.create('latest version downloads (valid)')
 
 t.create('latest version downloads (not found)')
   .get('/dtv/not-a-package.json')
-  .expectJSON({ name: 'downloads', value: 'not found' })
+  .expectJSON({ name: 'downloads', value: 'gem not found' })