refactor [shippable] service (#2576)

services/shippable/shippable.service.js

@@ -1,16 +1,44 @@
 'use strict'
 
-const LegacyService = require('../legacy-service')
-const { makeBadgeData: getBadgeData } = require('../../lib/badge-data')
-const { checkErrorResponse } = require('../../lib/error-helper')
+const BaseJsonService = require('../base-json')
+const Joi = require('joi')
+const { NotFound } = require('../errors')
+
+// source: https://github.com/badges/shields/pull/1362#discussion_r161693830
+const statusCodes = {
+  0: { color: '#5183A0', label: 'waiting' },
+  10: { color: '#5183A0', label: 'queued' },
+  20: { color: '#5183A0', label: 'processing' },
+  30: { color: '#44CC11', label: 'success' },
+  40: { color: '#F8A97D', label: 'skipped' },
+  50: { color: '#CEA61B', label: 'unstable' },
+  60: { color: '#555555', label: 'timeout' },
+  70: { color: '#6BAFBD', label: 'cancelled' },
+  80: { color: '#DC5F59', label: 'failed' },
+  90: { color: '#555555', label: 'stopped' },
+}
+
+const schema = Joi.array()
+  .items(
+    Joi.object({
+      branchName: Joi.string().required(),
+      statusCode: Joi.number()
+        .valid(Object.keys(statusCodes).map(key => parseInt(key)))
+        .required(),
+    }).required()
+  )
+  .required()
+
+module.exports = class Shippable extends BaseJsonService {
+  async fetch({ projectId }) {
+    const url = `https://api.shippable.com/projects/${projectId}/branchRunStatus`
+    return this._requestJson({ schema, url })
+  }
+
+  static get defaultBadgeData() {
+    return { label: 'shippable' }
+  }
 
-// This legacy service should be rewritten to use e.g. BaseJsonService.
-//
-// Tips for rewriting:
-// https://github.com/badges/shields/blob/master/doc/rewriting-services.md
-//
-// Do not base new services on this code.
-module.exports = class Shippable extends LegacyService {
   static get category() {
     return 'build'
   }
@@ -18,6 +46,8 @@ module.exports = class Shippable extends LegacyService {
   static get route() {
     return {
       base: 'shippable',
+      format: '([^/]+)(?:/(.+))?',
+      capture: ['projectId', 'branch'],
     }
   }
 
@@ -27,7 +57,7 @@ module.exports = class Shippable extends LegacyService {
         title: 'Shippable',
         pattern: ':projectId',
         namedParams: { projectId: '5444c5ecb904a4b21567b0ff' },
-        staticExample: { label: 'build', message: 'success', color: '#44CC11' },
+        staticExample: this.render({ code: 30 }),
       },
       {
         title: 'Shippable branch',
@@ -36,66 +66,25 @@ module.exports = class Shippable extends LegacyService {
           projectId: '5444c5ecb904a4b21567b0ff',
           branch: 'master',
         },
-        staticExample: { label: 'build', message: 'success', color: '#44CC11' },
+        staticExample: this.render({ code: 30 }),
       },
     ]
   }
 
-  static registerLegacyRouteHandler({ camp, cache }) {
-    camp.route(
-      /^\/shippable\/([^/]+)(?:\/(.+))?\.(svg|png|gif|jpg|json)$/,
-      cache((data, match, sendBadge, request) => {
-        // source: https://github.com/badges/shields/pull/1362#discussion_r161693830
-        const statusCodes = {
-          0: { color: '#5183A0', label: 'waiting' },
-          10: { color: '#5183A0', label: 'queued' },
-          20: { color: '#5183A0', label: 'processing' },
-          30: { color: '#44CC11', label: 'success' },
-          40: { color: '#F8A97D', label: 'skipped' },
-          50: { color: '#CEA61B', label: 'unstable' },
-          60: { color: '#555555', label: 'timeout' },
-          70: { color: '#6BAFBD', label: 'cancelled' },
-          80: { color: '#DC5F59', label: 'failed' },
-          90: { color: '#555555', label: 'stopped' },
-        }
+  static render({ code }) {
+    return {
+      label: 'build',
+      message: statusCodes[code].label,
+      color: statusCodes[code].color,
+    }
+  }
 
-        const projectId = match[1] // eg, 54d119db5ab6cc13528ab183
-        let targetBranch = match[2]
-        if (targetBranch == null) {
-          targetBranch = 'master'
-        }
-        const format = match[3]
-        const url = `https://api.shippable.com/projects/${projectId}/branchRunStatus`
-        const options = {
-          method: 'GET',
-          uri: url,
-        }
-
-        const badgeData = getBadgeData('build', data)
-
-        request(options, (err, res, buffer) => {
-          if (checkErrorResponse(badgeData, err, res)) {
-            sendBadge(format, badgeData)
-            return
-          }
-          try {
-            res = JSON.parse(buffer)
-            for (const branch of res) {
-              if (branch.branchName === targetBranch) {
-                badgeData.text[1] = statusCodes[branch.statusCode].label
-                badgeData.colorB = statusCodes[branch.statusCode].color
-                sendBadge(format, badgeData)
-                return
-              }
-            }
-            badgeData.text[1] = 'branch not found'
-            sendBadge(format, badgeData)
-          } catch (e) {
-            badgeData.text[1] = 'invalid'
-            sendBadge(format, badgeData)
-          }
-        })
-      })
-    )
+  async handle({ projectId, branch = 'master' }) {
+    const data = await this.fetch({ projectId })
+    const builds = data.filter(result => result.branchName === branch)
+    if (builds.length === 0) {
+      throw new NotFound({ prettyMessage: 'branch not found' })
+    }
+    return this.constructor.render({ code: builds[0].statusCode })
   }
 }

services/shippable/shippable.tester.js

@@ -1,7 +1,6 @@
 'use strict'
 
 const Joi = require('joi')
-const { invalidJSON } = require('../response-fixtures')
 const { isBuildStatus } = require('../test-validators')
 
 const t = (module.exports = require('../create-service-tester')())
@@ -26,25 +25,11 @@ t.create('build status (valid, with branch)')
 
 t.create('build status (branch not found)')
   .get('/5444c5ecb904a4b21567b0ff/not-a-branch.json')
-  .expectJSON({ name: 'build', value: 'branch not found' })
+  .expectJSON({ name: 'shippable', value: 'branch not found' })
 
 t.create('build status (not found)')
   .get('/not-a-build.json')
-  .expectJSON({ name: 'build', value: 'not found' })
-
-t.create('build status (connection error)')
-  .get('/5444c5ecb904a4b21567b0ff.json')
-  .networkOff()
-  .expectJSON({ name: 'build', value: 'inaccessible' })
-
-t.create('build status (unexpected response)')
-  .get('/5444c5ecb904a4b21567b0ff.json')
-  .intercept(nock =>
-    nock('https://api.shippable.com/')
-      .get('/projects/5444c5ecb904a4b21567b0ff/branchRunStatus')
-      .reply(invalidJSON)
-  )
-  .expectJSON({ name: 'build', value: 'invalid' })
+  .expectJSON({ name: 'shippable', value: 'not found' })
 
 t.create('build status (unexpected status code)')
   .get('/5444c5ecb904a4b21567b0ff.json')
@@ -53,4 +38,4 @@ t.create('build status (unexpected status code)')
       .get('/projects/5444c5ecb904a4b21567b0ff/branchRunStatus')
       .reply(200, '[{ "branchName": "master", "statusCode": 63 }]')
   )
-  .expectJSON({ name: 'build', value: 'invalid' })
+  .expectJSON({ name: 'shippable', value: 'invalid response data' })