Refactor route functions in BaseService (#2860)

The route helper functions are fairly well isolated from the rest of BaseService, with a few convenient entry points. They are easier to test in isolation.

The way the code was written before, `pathToRegexp` was invoked once for every request, which seems inefficient.

`route` was validated when it was used, though it seems more helpful to validate it up front.

This breaks out `_makeFullUrl`, `_regex`, `_regexFromPath` into new helper functions `makeFullUrl`, `assertValidRoute`, `prepareRoute`, and `namedParamsForMatch`.

It adds validation to route, and updates the services without patterns to include one, in order to pass the new validation rules.

core/base-service/base-non-memory-caching.js

@@ -5,6 +5,7 @@ const BaseService = require('./base')
 const { setCacheHeaders } = require('./cache-headers')
 const { makeSend } = require('./legacy-result-sender')
 const coalesceBadge = require('./coalesce-badge')
+const { prepareRoute, namedParamsForMatch } = require('./route')
 
 // Badges are subject to two independent types of caching: in-memory and
 // downstream.
@@ -26,9 +27,10 @@ module.exports = class NonMemoryCachingBaseService extends BaseService {
   static register({ camp }, serviceConfig) {
     const { cacheHeaders: cacheHeaderConfig } = serviceConfig
     const { _cacheLength: serviceDefaultCacheLengthSeconds } = this
+    const { regex, captureNames } = prepareRoute(this.route)
 
-    camp.route(this._regex, async (queryParams, match, end, ask) => {
-      const namedParams = this._namedParamsForMatch(match)
+    camp.route(regex, async (queryParams, match, end, ask) => {
+      const namedParams = namedParamsForMatch(captureNames, match, this)
       const serviceData = await this.invoke(
         {},
         serviceConfig,

core/base-service/base-static.js

@@ -9,14 +9,16 @@ const {
 } = require('./cache-headers')
 const { makeSend } = require('./legacy-result-sender')
 const coalesceBadge = require('./coalesce-badge')
+const { prepareRoute, namedParamsForMatch } = require('./route')
 
 module.exports = class BaseStaticService extends BaseService {
   static register({ camp }, serviceConfig) {
     const {
       profiling: { makeBadge: shouldProfileMakeBadge },
     } = serviceConfig
+    const { regex, captureNames } = prepareRoute(this.route)
 
-    camp.route(this._regex, async (queryParams, match, end, ask) => {
+    camp.route(regex, async (queryParams, match, end, ask) => {
       analytics.noteRequest(queryParams, match)
 
       if (serverHasBeenUpSinceResourceCached(ask.req)) {
@@ -26,7 +28,7 @@ module.exports = class BaseStaticService extends BaseService {
         return
       }
 
-      const namedParams = this._namedParamsForMatch(match)
+      const namedParams = namedParamsForMatch(captureNames, match, this)
       const serviceData = await this.invoke(
         {},
         serviceConfig,

core/base-service/base.js

@@ -2,7 +2,6 @@
 
 // See available emoji at http://emoji.muan.co/
 const emojic = require('emojic')
-const pathToRegexp = require('path-to-regexp')
 const Joi = require('joi')
 const { checkErrorResponse } = require('../../lib/error-helper')
 const { assertValidCategory } = require('../../services/categories')
@@ -14,6 +13,12 @@ const {
   InvalidParameter,
   Deprecated,
 } = require('./errors')
+const {
+  makeFullUrl,
+  assertValidRoute,
+  prepareRoute,
+  namedParamsForMatch,
+} = require('./route')
 const { assertValidServiceDefinition } = require('./service-definitions')
 const trace = require('./trace')
 const { validateExample, transformExample } = require('./transform-example')
@@ -150,13 +155,11 @@ class BaseService {
     return []
   }
 
-  static _makeFullUrl(partialUrl) {
-    return `/${[this.route.base, partialUrl].filter(Boolean).join('/')}`
-  }
-
   static validateDefinition() {
     assertValidCategory(this.category, `Category for ${this.name}`)
 
+    assertValidRoute(this.route, `Route for ${this.name}`)
+
     Joi.assert(
       this.defaultBadgeData,
       defaultBadgeDataSchema,
@@ -171,9 +174,9 @@ class BaseService {
   static getDefinition() {
     const { category, name, isDeprecated } = this
 
-    let format, pattern, queryParams
+    let base, format, pattern, queryParams
     try {
-      ;({ format, pattern, query: queryParams = [] } = this.route)
+      ;({ base, format, pattern, query: queryParams = [] } = this.route)
     } catch (e) {
       // Legacy services do not have a route.
     }
@@ -184,7 +187,7 @@ class BaseService {
 
     let route
     if (pattern) {
-      route = { pattern: this._makeFullUrl(pattern), queryParams }
+      route = { pattern: makeFullUrl(base, pattern), queryParams }
     } else if (format) {
       route = { format, queryParams }
     } else {
@@ -198,44 +201,6 @@ class BaseService {
     return result
   }
 
-  static get _regexFromPath() {
-    const { pattern } = this.route
-    const fullPattern = `${this._makeFullUrl(
-      pattern
-    )}.:ext(svg|png|gif|jpg|json)`
-
-    const keys = []
-    const regex = pathToRegexp(fullPattern, keys, {
-      strict: true,
-      sensitive: true,
-    })
-    const capture = keys.map(item => item.name).slice(0, -1)
-
-    return { regex, capture }
-  }
-
-  static get _regex() {
-    const { pattern, format, capture } = this.route
-    if (
-      pattern !== undefined &&
-      (format !== undefined || capture !== undefined)
-    ) {
-      throw Error(
-        `Since the route for ${
-          this.name
-        } includes a pattern, it should not include a format or capture`
-      )
-    } else if (pattern !== undefined) {
-      return this._regexFromPath.regex
-    } else if (format !== undefined) {
-      return new RegExp(
-        `^${this._makeFullUrl(this.route.format)}\\.(svg|png|gif|jpg|json)$`
-      )
-    } else {
-      throw Error(`The route for ${this.name} has neither pattern nor format`)
-    }
-  }
-
   static get _cacheLength() {
     const cacheLengths = {
       build: 30,
@@ -246,28 +211,6 @@ class BaseService {
     return cacheLengths[this.category]
   }
 
-  static _namedParamsForMatch(match) {
-    const { pattern, capture } = this.route
-    const names = pattern ? this._regexFromPath.capture : capture || []
-
-    // Assume the last match is the format, and drop match[0], which is the
-    // entire match.
-    const captures = match.slice(1, -1)
-
-    if (names.length !== captures.length) {
-      throw new Error(
-        `Service ${this.name} declares incorrect number of capture groups ` +
-          `(expected ${names.length}, got ${captures.length})`
-      )
-    }
-
-    const result = {}
-    names.forEach((name, index) => {
-      result[name] = captures[index]
-    })
-    return result
-  }
-
   _handleError(error) {
     if (error instanceof NotFound || error instanceof InvalidParameter) {
       trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
@@ -344,12 +287,14 @@ class BaseService {
 
   static register({ camp, handleRequest, githubApiProvider }, serviceConfig) {
     const { cacheHeaders: cacheHeaderConfig, fetchLimitBytes } = serviceConfig
+    const { regex, captureNames } = prepareRoute(this.route)
+
     camp.route(
-      this._regex,
+      regex,
       handleRequest(cacheHeaderConfig, {
         queryParams: this.route.queryParams,
         handler: async (queryParams, match, sendBadge, request) => {
-          const namedParams = this._namedParamsForMatch(match)
+          const namedParams = namedParamsForMatch(captureNames, match, this)
           const serviceData = await this.invoke(
             {
               sendAndCacheRequest: request.asPromise,

core/base-service/base.spec.js

@@ -2,7 +2,6 @@
 
 const Joi = require('joi')
 const { expect } = require('chai')
-const { test, given, forCases } = require('sazerac')
 const sinon = require('sinon')
 const trace = require('./trace')
 
@@ -60,6 +59,7 @@ class DummyService extends BaseService {
       },
     ]
   }
+
   static get route() {
     return {
       base: 'foo',
@@ -72,128 +72,6 @@ class DummyService extends BaseService {
 describe('BaseService', function() {
   const defaultConfig = { handleInternalErrors: false }
 
-  describe('URL pattern matching', function() {
-    context('A `pattern` with a named param is declared', function() {
-      const regexExec = str => DummyService._regex.exec(str)
-      const getNamedParamA = str => {
-        const [, namedParamA] = regexExec(str)
-        return namedParamA
-      }
-      const namedParams = str => {
-        const match = regexExec(str)
-        return DummyService._namedParamsForMatch(match)
-      }
-
-      test(regexExec, () => {
-        forCases([
-          given('/foo/bar.bar.bar.zip'),
-          given('/foo/bar/bar.svg'),
-          // This is a valid example with the wrong extension separator, to
-          // test that we only accept a `.`.
-          given('/foo/bar.bar.bar_svg'),
-        ]).expect(null)
-      })
-
-      test(getNamedParamA, () => {
-        forCases([
-          given('/foo/bar.bar.bar.svg'),
-          given('/foo/bar.bar.bar.png'),
-          given('/foo/bar.bar.bar.gif'),
-          given('/foo/bar.bar.bar.jpg'),
-          given('/foo/bar.bar.bar.json'),
-        ]).expect('bar.bar.bar')
-      })
-
-      test(namedParams, () => {
-        forCases([
-          given('/foo/bar.bar.bar.svg'),
-          given('/foo/bar.bar.bar.png'),
-          given('/foo/bar.bar.bar.gif'),
-          given('/foo/bar.bar.bar.jpg'),
-          given('/foo/bar.bar.bar.json'),
-        ]).expect({ namedParamA: 'bar.bar.bar' })
-      })
-    })
-
-    context('A `format` with a named param is declared', function() {
-      class ServiceWithFormat extends BaseService {
-        static get route() {
-          return {
-            base: 'foo',
-            format: '([^/]+)',
-            capture: ['namedParamA'],
-          }
-        }
-      }
-
-      const regexExec = str => ServiceWithFormat._regex.exec(str)
-      const getNamedParamA = str => {
-        const [, namedParamA] = regexExec(str)
-        return namedParamA
-      }
-      const namedParams = str => {
-        const match = regexExec(str)
-        return ServiceWithFormat._namedParamsForMatch(match)
-      }
-
-      test(regexExec, () => {
-        forCases([
-          given('/foo/bar.bar.bar.zip'),
-          given('/foo/bar/bar.svg'),
-          // This is a valid example with the wrong extension separator, to
-          // test that we only accept a `.`.
-          given('/foo/bar.bar.bar_svg'),
-        ]).expect(null)
-      })
-
-      test(getNamedParamA, () => {
-        forCases([
-          given('/foo/bar.bar.bar.svg'),
-          given('/foo/bar.bar.bar.png'),
-          given('/foo/bar.bar.bar.gif'),
-          given('/foo/bar.bar.bar.jpg'),
-          given('/foo/bar.bar.bar.json'),
-        ]).expect('bar.bar.bar')
-      })
-
-      test(namedParams, () => {
-        forCases([
-          given('/foo/bar.bar.bar.svg'),
-          given('/foo/bar.bar.bar.png'),
-          given('/foo/bar.bar.bar.gif'),
-          given('/foo/bar.bar.bar.jpg'),
-          given('/foo/bar.bar.bar.json'),
-        ]).expect({ namedParamA: 'bar.bar.bar' })
-      })
-    })
-
-    context('No named params are declared', function() {
-      class ServiceWithZeroNamedParams extends BaseService {
-        static get route() {
-          return {
-            base: 'foo',
-            format: '(?:[^/]+)',
-          }
-        }
-      }
-
-      const namedParams = str => {
-        const match = ServiceWithZeroNamedParams._regex.exec(str)
-        return ServiceWithZeroNamedParams._namedParamsForMatch(match)
-      }
-
-      test(namedParams, () => {
-        forCases([
-          given('/foo/bar.bar.bar.svg'),
-          given('/foo/bar.bar.bar.png'),
-          given('/foo/bar.bar.bar.gif'),
-          given('/foo/bar.bar.bar.jpg'),
-          given('/foo/bar.bar.bar.json'),
-        ]).expect({})
-      })
-    })
-  })
-
   it('Invokes the handler as expected', async function() {
     expect(
       await DummyService.invoke(

core/base-service/route.js

@@ -0,0 +1,75 @@
+'use strict'
+
+const Joi = require('joi')
+const pathToRegexp = require('path-to-regexp')
+
+function makeFullUrl(base, partialUrl) {
+  return `/${[base, partialUrl].filter(Boolean).join('/')}`
+}
+
+const routeSchema = Joi.object({
+  base: Joi.string().allow(''),
+  pattern: Joi.string().allow(''),
+  format: Joi.string(),
+  capture: Joi.alternatives().when('format', {
+    is: Joi.string().required(),
+    then: Joi.array().items(Joi.string().required()),
+  }),
+  queryParams: Joi.array().items(Joi.string().required()),
+})
+  .xor('pattern', 'format')
+  .required()
+
+function assertValidRoute(route, message = undefined) {
+  Joi.assert(route, routeSchema, message)
+}
+
+function prepareRoute({ base, pattern, format, capture }) {
+  let regex, captureNames
+  if (pattern === undefined) {
+    regex = new RegExp(
+      `^${makeFullUrl(base, format)}\\.(svg|png|gif|jpg|json)$`
+    )
+    captureNames = capture || []
+  } else {
+    const fullPattern = `${makeFullUrl(
+      base,
+      pattern
+    )}.:ext(svg|png|gif|jpg|json)`
+    const keys = []
+    regex = pathToRegexp(fullPattern, keys, {
+      strict: true,
+      sensitive: true,
+    })
+    captureNames = keys.map(item => item.name).slice(0, -1)
+  }
+  return { regex, captureNames }
+}
+
+function namedParamsForMatch(captureNames = [], match, ServiceClass) {
+  // Assume the last match is the format, and drop match[0], which is the
+  // entire match.
+  const captures = match.slice(1, -1)
+
+  if (captureNames.length !== captures.length) {
+    throw new Error(
+      `Service ${
+        ServiceClass.name
+      } declares incorrect number of capture groups ` +
+        `(expected ${captureNames.length}, got ${captures.length})`
+    )
+  }
+
+  const result = {}
+  captureNames.forEach((name, index) => {
+    result[name] = captures[index]
+  })
+  return result
+}
+
+module.exports = {
+  makeFullUrl,
+  assertValidRoute,
+  prepareRoute,
+  namedParamsForMatch,
+}

core/base-service/route.spec.js

@@ -0,0 +1,87 @@
+'use strict'
+
+const { test, given, forCases } = require('sazerac')
+const { prepareRoute, namedParamsForMatch } = require('./route')
+
+describe('Route helpers', function() {
+  context('A `pattern` with a named param is declared', function() {
+    const { regex, captureNames } = prepareRoute({
+      base: 'foo',
+      pattern: ':namedParamA',
+      queryParams: ['queryParamA'],
+    })
+
+    const regexExec = str => regex.exec(str)
+    test(regexExec, () => {
+      forCases([
+        given('/foo/bar.bar.bar.zip'),
+        given('/foo/bar/bar.svg'),
+        // This is a valid example with the wrong extension separator, to
+        // test that we only accept a `.`.
+        given('/foo/bar.bar.bar_svg'),
+      ]).expect(null)
+    })
+
+    const namedParams = str =>
+      namedParamsForMatch(captureNames, regex.exec(str))
+    test(namedParams, () => {
+      forCases([
+        given('/foo/bar.bar.bar.svg'),
+        given('/foo/bar.bar.bar.png'),
+        given('/foo/bar.bar.bar.gif'),
+        given('/foo/bar.bar.bar.jpg'),
+        given('/foo/bar.bar.bar.json'),
+      ]).expect({ namedParamA: 'bar.bar.bar' })
+    })
+  })
+
+  context('A `format` with a named param is declared', function() {
+    const { regex, captureNames } = prepareRoute({
+      base: 'foo',
+      format: '([^/]+)',
+      capture: ['namedParamA'],
+    })
+
+    const regexExec = str => regex.exec(str)
+    test(regexExec, () => {
+      forCases([
+        given('/foo/bar.bar.bar.zip'),
+        given('/foo/bar/bar.svg'),
+        // This is a valid example with the wrong extension separator, to
+        // test that we only accept a `.`.
+        given('/foo/bar.bar.bar_svg'),
+      ]).expect(null)
+    })
+
+    const namedParams = str =>
+      namedParamsForMatch(captureNames, regex.exec(str))
+    test(namedParams, () => {
+      forCases([
+        given('/foo/bar.bar.bar.svg'),
+        given('/foo/bar.bar.bar.png'),
+        given('/foo/bar.bar.bar.gif'),
+        given('/foo/bar.bar.bar.jpg'),
+        given('/foo/bar.bar.bar.json'),
+      ]).expect({ namedParamA: 'bar.bar.bar' })
+    })
+  })
+
+  context('No named params are declared', function() {
+    const { regex, captureNames } = prepareRoute({
+      base: 'foo',
+      format: '(?:[^/]+)',
+    })
+
+    const namedParams = str =>
+      namedParamsForMatch(captureNames, regex.exec(str))
+    test(namedParams, () => {
+      forCases([
+        given('/foo/bar.bar.bar.svg'),
+        given('/foo/bar.bar.bar.png'),
+        given('/foo/bar.bar.bar.gif'),
+        given('/foo/bar.bar.bar.jpg'),
+        given('/foo/bar.bar.bar.json'),
+      ]).expect({})
+    })
+  })
+})

core/base-service/transform-example.js

@@ -3,6 +3,7 @@
 const Joi = require('joi')
 const pathToRegexp = require('path-to-regexp')
 const coalesceBadge = require('./coalesce-badge')
+const { makeFullUrl } = require('./route')
 
 const optionalObjectOfKeyValues = Joi.object().pattern(
   /./,
@@ -127,13 +128,16 @@ function transformExample(inExample, index, ServiceClass) {
   let example
   if (namedParams) {
     example = {
-      pattern: ServiceClass._makeFullUrl(pattern || ServiceClass.route.pattern),
+      pattern: makeFullUrl(
+        ServiceClass.route.base,
+        pattern || ServiceClass.route.pattern
+      ),
       namedParams,
       queryParams,
     }
   } else {
     example = {
-      path: ServiceClass._makeFullUrl(previewUrl),
+      path: makeFullUrl(ServiceClass.route.base, previewUrl),
       queryParams,
     }
   }
@@ -152,7 +156,7 @@ function transformExample(inExample, index, ServiceClass) {
     preview = { label, message: `${message}`, color }
   } else {
     preview = {
-      path: ServiceClass._makeFullUrl(previewUrl),
+      path: makeFullUrl(ServiceClass.route.base, previewUrl),
       queryParams,
     }
   }

services/amo/amo.service.js

@@ -85,6 +85,7 @@ class AmoRating extends LegacyService {
   static get route() {
     return {
       base: 'amo',
+      pattern: '',
     }
   }
 

services/bitrise/bitrise.service.js

@@ -17,6 +17,7 @@ module.exports = class Bitrise extends LegacyService {
   static get route() {
     return {
       base: 'bitrise',
+      pattern: ':appId/:branch',
     }
   }
 
@@ -24,7 +25,6 @@ module.exports = class Bitrise extends LegacyService {
     return [
       {
         title: 'Bitrise',
-        pattern: ':appId/:branch',
         namedParams: { appId: 'cde737473028420d', branch: 'master' },
         queryParams: { token: 'GCIdEzacE4GW32jLVrZb7A' },
         staticPreview: {

services/bugzilla/bugzilla.service.js

@@ -24,6 +24,7 @@ module.exports = class Bugzilla extends LegacyService {
   static get route() {
     return {
       base: 'bugzilla',
+      pattern: ':bugNumber',
     }
   }
 
@@ -31,7 +32,6 @@ module.exports = class Bugzilla extends LegacyService {
     return [
       {
         title: 'Bugzilla bug status',
-        pattern: ':bugNumber',
         namedParams: { bugNumber: '996038' },
         staticPreview: {
           label: 'bug 996038',

services/buildkite/buildkite.service.js

@@ -18,6 +18,7 @@ module.exports = class Buildkite extends LegacyService {
   static get route() {
     return {
       base: 'buildkite',
+      pattern: '',
     }
   }
 

services/bundlephobia/bundlephobia.service.js

@@ -20,6 +20,7 @@ module.exports = class Bundlephobia extends LegacyService {
   static get route() {
     return {
       base: 'bundlephobia',
+      pattern: '',
     }
   }
 

services/chrome-web-store/chrome-web-store.service.js

@@ -18,7 +18,6 @@ const {
 
 const commonExample = {
   title: 'Chrome Web Store',
-  pattern: ':storeId',
   namedParams: { storeId: 'ogffaloegjglncjfehdfplabnoondfjo' },
 }
 
@@ -30,6 +29,7 @@ class ChromeWebStoreDownloads extends LegacyService {
   static get route() {
     return {
       base: 'chrome-web-store/users',
+      pattern: ':storeId',
     }
   }
 
@@ -53,6 +53,7 @@ class ChromeWebStoreVersion extends LegacyService {
   static get route() {
     return {
       base: 'chrome-web-store/v',
+      pattern: ':storeId',
     }
   }
 
@@ -80,6 +81,7 @@ class ChromeWebStorePrice extends LegacyService {
   static get route() {
     return {
       base: 'chrome-web-store/price',
+      pattern: ':storeId',
     }
   }
 
@@ -103,6 +105,7 @@ class ChromeWebStoreRating extends LegacyService {
   static get route() {
     return {
       base: 'chrome-web-store',
+      pattern: ':storeId',
     }
   }
 

services/cocoapods/cocoapods-metrics.service.js

@@ -21,6 +21,7 @@ module.exports = class CocoapodsMetrics extends LegacyService {
   static get route() {
     return {
       base: 'cocoapods/metrics/doc-percent',
+      pattern: ':spec',
     }
   }
 
@@ -28,7 +29,6 @@ module.exports = class CocoapodsMetrics extends LegacyService {
     return [
       {
         title: 'Cocoapods doc percentage',
-        pattern: ':spec',
         namedParams: { spec: 'AFNetworking' },
         staticPreview: { label: 'docs', message: '94%', color: 'green' },
       },

services/cocoapods/cocoapods-version.service.js

@@ -10,6 +10,7 @@ module.exports = class CocoapodsVersion extends LegacyService {
   static get route() {
     return {
       base: 'cocoapods/v',
+      pattern: ':spec',
     }
   }
 

services/codeclimate/codeclimate.service.js

@@ -12,6 +12,7 @@ class CodeclimateCoverage extends LegacyService {
   static get route() {
     return {
       base: 'codeclimate',
+      pattern: ':which(coverage|coverage-letter)/:userRepo*',
     }
   }
 
@@ -57,6 +58,8 @@ class Codeclimate extends LegacyService {
   static get route() {
     return {
       base: 'codeclimate',
+      pattern:
+        ':which(issues|maintainability|maintainability-percentage|tech-debt)/:userRepo*',
     }
   }
 

services/codecov/codecov.service.js

@@ -21,6 +21,7 @@ module.exports = class Codecov extends LegacyService {
   static get route() {
     return {
       base: 'codecov/c',
+      pattern: '',
     }
   }
 

services/codeship/codeship.service.js

@@ -17,6 +17,7 @@ module.exports = class Codeship extends LegacyService {
   static get route() {
     return {
       base: 'codeship',
+      pattern: '',
     }
   }
 

services/continuousphp/continuousphp.service.js

@@ -15,7 +15,10 @@ module.exports = class ContinuousPhp extends LegacyService {
   }
 
   static get route() {
-    return { base: 'continuousphp' }
+    return {
+      base: 'continuousphp',
+      pattern: '',
+    }
   }
 
   static get examples() {

services/cookbook/cookbook.service.js

@@ -19,14 +19,16 @@ module.exports = class Cookbook extends LegacyService {
   }
 
   static get route() {
-    return { base: 'cookbook/v' }
+    return {
+      base: 'cookbook/v',
+      pattern: ':cookbook',
+    }
   }
 
   static get examples() {
     return [
       {
         title: 'Chef cookbook',
-        pattern: ':cookbook',
         namedParams: { cookbook: 'chef-sugar' },
         staticPreview: { label: 'cookbook', message: 'v5.0.0', color: 'blue' },
       },

services/coveralls/coveralls.service.js

@@ -20,6 +20,7 @@ module.exports = class Coveralls extends LegacyService {
   static get route() {
     return {
       base: 'coveralls',
+      pattern: '',
     }
   }
 

services/david/david.service.js

@@ -17,6 +17,7 @@ module.exports = class David extends LegacyService {
   static get route() {
     return {
       base: 'david',
+      pattern: '',
     }
   }
 

services/github/github-commit-activity.service.js

@@ -23,6 +23,7 @@ module.exports = class GithubCommitActivity extends LegacyService {
   static get route() {
     return {
       base: 'github/commit-activity',
+      pattern: ':interval(y|4w|w)/:user/:repo',
     }
   }
 

services/github/github-commit-status.service.js

@@ -21,6 +21,7 @@ module.exports = class GithubCommitStatus extends LegacyService {
   static get route() {
     return {
       base: 'github/commit-status',
+      pattern: ':user/:repo/:branch/:commit',
     }
   }
 
@@ -28,7 +29,6 @@ module.exports = class GithubCommitStatus extends LegacyService {
     return [
       {
         title: 'GitHub commit merge status',
-        pattern: ':user/:repo/:branch/:commit',
         namedParams: {
           user: 'badges',
           repo: 'shields',

services/github/github-downloads.service.js

@@ -23,6 +23,7 @@ module.exports = class GithubDownloads extends LegacyService {
   static get route() {
     return {
       base: 'github',
+      pattern: '',
     }
   }
 

services/github/github-followers.service.js

@@ -22,6 +22,7 @@ module.exports = class GithubFollowers extends LegacyService {
   static get route() {
     return {
       base: 'github/followers',
+      pattern: ':user',
     }
   }
 
@@ -29,7 +30,6 @@ module.exports = class GithubFollowers extends LegacyService {
     return [
       {
         title: 'GitHub followers',
-        pattern: ':user',
         previewUrl: 'espadrine',
         // https://github.com/badges/shields/issues/2479
         // namedParams: {

services/github/github-forks.service.js

@@ -22,6 +22,7 @@ module.exports = class GithubForks extends LegacyService {
   static get route() {
     return {
       base: 'github/forks',
+      pattern: ':user/:repo',
     }
   }
 

services/github/github-stars.service.js

@@ -23,6 +23,7 @@ module.exports = class GithubStars extends LegacyService {
   static get route() {
     return {
       base: 'github/stars',
+      pattern: ':user/:repo',
     }
   }
 

services/github/github-watchers.service.js

@@ -22,6 +22,7 @@ module.exports = class GithubWatchers extends LegacyService {
   static get route() {
     return {
       base: 'github/watchers',
+      pattern: ':user/:repo',
     }
   }
 

services/jenkins/jenkins-plugin-version.service.js

@@ -12,7 +12,7 @@ const { version: versionColor } = require('../../lib/color-formatters')
 // https://github.com/badges/shields/blob/master/doc/rewriting-services.md
 //
 // Do not base new services on this code.
-module.exports = class JenkinsPlugin extends LegacyService {
+module.exports = class JenkinsPluginVersion extends LegacyService {
   static get category() {
     return 'version'
   }
@@ -20,6 +20,7 @@ module.exports = class JenkinsPlugin extends LegacyService {
   static get route() {
     return {
       base: 'jenkins/plugin/v',
+      pattern: ':plugin',
     }
   }
 
@@ -27,7 +28,6 @@ module.exports = class JenkinsPlugin extends LegacyService {
     return [
       {
         title: 'Jenkins Plugins',
-        pattern: ':plugin',
         namedParams: {
           plugin: 'blueocean',
         },

services/legacy-service.js

@@ -12,6 +12,11 @@ const { BaseService } = require('.')
 // BaseJsonService. Refer to the tutorial:
 // https://github.com/badges/shields/blob/master/doc/TUTORIAL.md
 class LegacyService extends BaseService {
+  // Provide a placeholder for services which do not define a route.
+  static get route() {
+    return { pattern: '' }
+  }
+
   static registerLegacyRouteHandler({ camp, cache, githubApiProvider }) {
     throw Error(`registerLegacyRouteHandler() not implemented for ${this.name}`)
   }

services/lgtm/lgtm-alerts.service.js

@@ -19,6 +19,7 @@ module.exports = class LgtmAlerts extends LegacyService {
   static get route() {
     return {
       base: 'lgtm/alerts',
+      pattern: '',
     }
   }
 

services/lgtm/lgtm-grade.service.js

@@ -18,6 +18,7 @@ module.exports = class LgtmGrade extends LegacyService {
   static get route() {
     return {
       base: 'lgtm/grade',
+      pattern: '',
     }
   }
 

services/liberapay/liberapay.service.js

@@ -20,6 +20,7 @@ module.exports = class Liberapay extends LegacyService {
   static get route() {
     return {
       base: 'liberapay',
+      pattern: '',
     }
   }
 

services/librariesio/librariesio-dependencies.service.js

@@ -18,6 +18,7 @@ module.exports = class LibrariesioDependencies extends LegacyService {
   static get route() {
     return {
       base: 'librariesio',
+      pattern: '',
     }
   }
 

services/luarocks/luarocks.service.js

@@ -22,6 +22,7 @@ module.exports = class Luarocks extends LegacyService {
   static get route() {
     return {
       base: 'luarocks/v',
+      pattern: ':user/:moduleName',
     }
   }
 
@@ -29,7 +30,6 @@ module.exports = class Luarocks extends LegacyService {
     return [
       {
         title: 'LuaRocks',
-        pattern: ':user/:moduleName',
         namedParams: {
           user: 'mpeterv',
           moduleName: 'luacheck',

services/maven-central/maven-central.service.js

@@ -20,6 +20,7 @@ module.exports = class MavenCentral extends LegacyService {
   static get route() {
     return {
       base: 'maven-central/v',
+      pattern: '',
     }
   }
 

services/maven-metadata/maven-metadata.service.js

@@ -20,6 +20,7 @@ module.exports = class MavenMetadata extends LegacyService {
   static get route() {
     return {
       base: 'maven-metadata/v',
+      pattern: '',
     }
   }
 

services/microbadger/microbadger.service.js

@@ -21,6 +21,7 @@ module.exports = class MicroBadger extends LegacyService {
   static get route() {
     return {
       base: 'microbadger',
+      pattern: '',
     }
   }
 

services/packagist/packagist-downloads.service.js

@@ -21,6 +21,7 @@ module.exports = class PackagistDownloads extends LegacyService {
   static get route() {
     return {
       base: 'packagist',
+      pattern: ':interval(dm|dd|dt)/:user/:repo',
     }
   }
 

services/packagist/packagist-php-version.service.js

@@ -18,6 +18,7 @@ module.exports = class PackagistPhpVersion extends LegacyService {
   static get route() {
     return {
       base: 'packagist/php-v',
+      pattern: '',
     }
   }
 

services/packagist/packagist-version.service.js

@@ -24,6 +24,7 @@ module.exports = class PackagistVersion extends LegacyService {
   static get route() {
     return {
       base: 'packagist',
+      pattern: ':which(v|vpre)/:user/:repo',
     }
   }
 

services/php-eye/php-eye-hhvm.service.js

@@ -21,6 +21,7 @@ module.exports = class PhpeyeHhvm extends LegacyService {
   static get route() {
     return {
       base: 'hhvm',
+      pattern: '',
     }
   }
 

services/php-eye/php-eye-php-version.service.js

@@ -22,6 +22,7 @@ module.exports = class PhpEyePhpVersion extends LegacyService {
   static get route() {
     return {
       base: 'php-eye',
+      pattern: ':user/:repo',
     }
   }
 

services/puppetforge/puppetforge-modules.service.js

@@ -20,6 +20,7 @@ class PuppetforgeModuleVersion extends LegacyService {
   static get route() {
     return {
       base: 'puppetforge/v',
+      pattern: ':user/:moduleName',
     }
   }
 
@@ -43,6 +44,7 @@ class PuppetforgeModulePdkVersion extends LegacyService {
   static get route() {
     return {
       base: 'puppetforge/pdk-version',
+      pattern: ':user/:moduleName',
     }
   }
 
@@ -50,7 +52,6 @@ class PuppetforgeModulePdkVersion extends LegacyService {
     return [
       {
         title: 'Puppet Forge – PDK version',
-        pattern: ':user/:moduleName',
         namedParams: {
           user: 'tragiccode',
           moduleName: 'azure_key_vault',
@@ -75,6 +76,7 @@ class PuppetforgeModuleDownloads extends LegacyService {
   static get route() {
     return {
       base: 'puppetforge/dt',
+      pattern: ':user/:moduleName',
     }
   }
 
@@ -98,6 +100,7 @@ class PuppetforgeModuleEndorsement extends LegacyService {
   static get route() {
     return {
       base: 'puppetforge/e',
+      pattern: ':user/:moduleName',
     }
   }
 
@@ -121,6 +124,7 @@ class PuppetforgeModuleFeedback extends LegacyService {
   static get route() {
     return {
       base: 'puppetforge/f',
+      pattern: ':user/:moduleName',
     }
   }
 

services/puppetforge/puppetforge-users.service.js

@@ -16,6 +16,7 @@ class PuppetforgeUserReleases extends LegacyService {
   static get route() {
     return {
       base: 'puppetforge/rc',
+      pattern: ':user',
     }
   }
 
@@ -39,6 +40,7 @@ class PuppetforgeUserModules extends LegacyService {
   static get route() {
     return {
       base: 'puppetforge/mc',
+      pattern: ':user',
     }
   }
 

services/scrutinizer/scrutinizer.service.js

@@ -15,6 +15,7 @@ class ScrutinizerBuild extends LegacyService {
   static get route() {
     return {
       base: 'scrutinizer',
+      pattern: '',
     }
   }
 
@@ -38,6 +39,7 @@ class ScrutinizerCoverage extends LegacyService {
   static get route() {
     return {
       base: 'scrutinizer',
+      pattern: '',
     }
   }
 
@@ -71,6 +73,7 @@ class Scrutinizer extends LegacyService {
   static get route() {
     return {
       base: 'scrutinizer',
+      pattern: '',
     }
   }
 

services/sonarqube/sonarqube.service.js

@@ -16,6 +16,7 @@ class SonarqubeCoverage extends LegacyService {
   static get route() {
     return {
       base: 'sonar',
+      pattern: '',
     }
   }
 
@@ -51,6 +52,7 @@ class Sonarqube extends LegacyService {
   static get route() {
     return {
       base: 'sonar',
+      pattern: '',
     }
   }
 

services/twitter/twitter.service.js

@@ -19,6 +19,7 @@ class TwitterUrl extends LegacyService {
   static get route() {
     return {
       base: 'twitter/url',
+      pattern: '',
     }
   }
 
@@ -73,6 +74,7 @@ class TwitterFollow extends LegacyService {
   static get route() {
     return {
       base: 'twitter/follow',
+      pattern: ':user',
     }
   }
 

services/vaadin-directory/vaadin-directory.service.js

@@ -20,6 +20,7 @@ class VaadinDirectoryRating extends LegacyService {
   static get route() {
     return {
       base: 'vaadin-directory',
+      pattern: ':which(rating|stars|rating-count)/:packageName',
     }
   }
 

services/waffle/waffle.service.js

@@ -21,6 +21,7 @@ module.exports = class Waffle extends LegacyService {
   static get route() {
     return {
       base: 'waffle/label',
+      pattern: ':user/:repo/:query',
     }
   }
 
@@ -28,7 +29,6 @@ module.exports = class Waffle extends LegacyService {
     return [
       {
         title: 'Waffle.io',
-        pattern: ':user/:repo/:query',
         namedParams: {
           user: 'evancohen',
           repo: 'smart-mirror',

services/website/website.service.js

@@ -88,6 +88,7 @@ module.exports = class Website extends LegacyService {
   static get route() {
     return {
       base: '',
+      pattern: '',
     }
   }
 

test-fixtures/valid-array.fixture.js

@@ -7,11 +7,19 @@ class GoodServiceOne extends BaseJsonService {
   static get category() {
     return 'build'
   }
+
+  static get route() {
+    return { pattern: 'good/one' }
+  }
 }
 class GoodServiceTwo extends LegacyService {
   static get category() {
     return 'build'
   }
+
+  static get route() {
+    return { pattern: 'good/two' }
+  }
 }
 
 module.exports = [GoodServiceOne, GoodServiceTwo]

test-fixtures/valid-class.fixture.js

@@ -6,6 +6,10 @@ class GoodService extends BaseJsonService {
   static get category() {
     return 'build'
   }
+
+  static get route() {
+    return { pattern: 'good' }
+  }
 }
 
 module.exports = GoodService

test-fixtures/valid-object.fixture.js

@@ -7,11 +7,19 @@ class GoodServiceOne extends BaseJsonService {
   static get category() {
     return 'build'
   }
+
+  static get route() {
+    return { pattern: 'good/one' }
+  }
 }
 class GoodServiceTwo extends LegacyService {
   static get category() {
     return 'build'
   }
+
+  static get route() {
+    return { pattern: 'good/two' }
+  }
 }
 
 module.exports = { GoodServiceOne, GoodServiceTwo }