Add routeEnum with validation and enhance error handling in BaseService [Twitter Gem TeamCity Packagist MozillaObservatory HexPm Greasyfork]

core/base-service/base.js

@@ -99,8 +99,24 @@ class BaseService {
   }
 
   /**
-   * Extract an array of allowed values from this service's route pattern
-   * for a given route parameter
+   * If the route pattern includes an enum, this property should be set to the
+   * array of allowed values. This is used to validate the route pattern and
+   * generate the OpenAPI spec.
+   * enum applies to the first param in the route pattern.
+   *
+   * @abstract
+   * @type {string[]} array of allowed values for the enum in the route pattern
+   */
+  static routeEnum = undefined
+
+  /**
+   * If old route enum is used (legacy):
+   *  Extract an array of allowed values from this service's route pattern
+   *  for a given route parameter
+   *
+   * If new routeEnum is used:
+   *  Return the array of allowed values for a given route parameter from the
+   *  routeEnum property
    *
    * @param {string} param The name of a param in this service's route pattern
    * @returns {string[]} Array of allowed values for this param
@@ -109,6 +125,23 @@ class BaseService {
     if (!('pattern' in this.route)) {
       throw new Error('getEnum() requires route to have a .pattern property')
     }
+
+    if (this.routeEnum) {
+      if (
+        !Array.isArray(this.routeEnum) ||
+        this.routeEnum.length === 0 ||
+        !this.routeEnum.every(item => typeof item === 'string')
+      ) {
+        throw new Error(
+          `getEnum() requires routeEnum for ${this.name} to be a non-empty array of strings`,
+        )
+      }
+
+      return this.routeEnum
+    }
+
+    // TODO Remove after #11371 is merged the old route extraction.
+    // replace with error if routeEnum and this function is called.
     const enumeration = getEnum(this.route.pattern, param)
     if (!Array.isArray(enumeration)) {
       throw new Error(
@@ -413,6 +446,17 @@ class BaseService {
       serviceError = new ImproperlyConfigured({ prettyMessage })
     }
 
+    if (!serviceError && this.routeEnum) {
+      const firstParamName = Object.keys(namedParams || {})[0]
+      if (firstParamName) {
+        if (!this.routeEnum.includes(namedParams[firstParamName])) {
+          serviceError = new InvalidParameter({
+            prettyMessage: `invalid parameter ${firstParamName}: ${namedParams[firstParamName]}`,
+          })
+        }
+      }
+    }
+
     const { queryParamSchema } = this.route
     let transformedQueryParams
     if (!serviceError && queryParamSchema) {

core/base-service/base.spec.js

@@ -618,5 +618,104 @@ describe('BaseService', function () {
         'getEnum() requires route to have a .pattern property',
       )
     })
+
+    it('returns routeEnum when set', function () {
+      class RouteEnumService extends DummyService {
+        static route = {
+          base: 'foo',
+          pattern: ':namedParamA',
+          queryParamSchema,
+        }
+        static routeEnum = ['alpha', 'beta']
+      }
+
+      expect(RouteEnumService.getEnum('namedParamA')).to.deep.equal([
+        'alpha',
+        'beta',
+      ])
+    })
+
+    it('invoke rejects first named param not in routeEnum', async function () {
+      class RouteEnumService extends DummyService {
+        static route = {
+          base: 'foo',
+          pattern: ':namedParamA',
+          queryParamSchema,
+        }
+        static routeEnum = ['alpha', 'beta']
+      }
+
+      const result = await RouteEnumService.invoke({}, defaultConfig, {
+        namedParamA: 'gamma',
+      })
+      expect(result).to.deep.equal({
+        isError: true,
+        color: 'red',
+        message: 'invalid parameter namedParamA: gamma',
+      })
+    })
+
+    it('invoke allows first named param present in routeEnum', async function () {
+      class RouteEnumService extends DummyService {
+        static route = {
+          base: 'foo',
+          pattern: ':namedParamA',
+          queryParamSchema,
+        }
+        static routeEnum = ['alpha', 'beta']
+      }
+
+      const result = await RouteEnumService.invoke({}, defaultConfig, {
+        namedParamA: 'alpha',
+      })
+      expect(result).to.deep.equal({
+        message: 'Hello namedParamA: alpha with queryParamA: undefined',
+      })
+    })
+
+    it('throws when routeEnum is not a non-empty array of strings (not an array)', function () {
+      class BadRouteEnumService extends DummyService {
+        static route = {
+          base: 'foo',
+          pattern: ':namedParamA',
+          queryParamSchema,
+        }
+        static routeEnum = { alpha: true }
+      }
+
+      expect(() => BadRouteEnumService.getEnum('namedParamA')).to.throw(
+        `getEnum() requires routeEnum for ${BadRouteEnumService.name} to be a non-empty array of strings`,
+      )
+    })
+
+    it('throws when routeEnum is an empty array', function () {
+      class BadRouteEnumService extends DummyService {
+        static route = {
+          base: 'foo',
+          pattern: ':namedParamA',
+          queryParamSchema,
+        }
+        static routeEnum = []
+      }
+
+      expect(() => BadRouteEnumService.getEnum('namedParamA')).to.throw(
+        `getEnum() requires routeEnum for ${BadRouteEnumService.name} to be a non-empty array of strings`,
+      )
+    })
+
+    it('throws when routeEnum contains non-string values', function () {
+      class BadRouteEnumService extends DummyService {
+        static route = {
+          base: 'foo',
+          pattern: ':namedParamA',
+          queryParamSchema,
+        }
+        static routeEnum = ['alpha', 42]
+      }
+
+      expect(() => BadRouteEnumService.getEnum('namedParamA')).to.throw(
+        `getEnum() requires routeEnum for ${BadRouteEnumService.name} to be a non-empty array of strings`,
+      )
+    })
   })
 })

core/base-service/redirector.js

@@ -18,6 +18,7 @@ const attrSchema = Joi.object({
   category: isValidCategory,
   isDeprecated: Joi.boolean().default(true),
   route: isValidRoute,
+  routeEnum: Joi.array().items(Joi.string()).optional(),
   openApi: openApiSchema,
   transformPath: Joi.func()
     .maxArity(1)
@@ -37,6 +38,7 @@ export default function redirector(attrs) {
     category,
     isDeprecated,
     route,
+    routeEnum,
     openApi,
     transformPath,
     transformQueryParams,
@@ -53,6 +55,7 @@ export default function redirector(attrs) {
     static category = category
     static isDeprecated = isDeprecated
     static route = route
+    static routeEnum = routeEnum
     static openApi = openApi
 
     static register({ camp, metricInstance }, { rasterUrl }) {

doc/TUTORIAL.md

@@ -108,7 +108,26 @@ Description of the code:
 4. `route` declares the URL path at which the service operates. It also maps components of the URL path to handler parameters.
    - `base` defines the first part of the URL that doesn't change, e.g. `/example/`.
    - `pattern` defines the variable part of the route, everything that comes after `/example/`. It can include any number of named parameters. These are converted into regular expressions by [`path-to-regexp`][path-to-regexp]. Because a service instance won't be created until it's time to handle a request, the route and other metadata must be obtained by examining the classes themselves. [That's why they're marked `static`.][static]
-   - There is additional documentation on conventions for [designing badge URLs](./badge-urls.md)
+
+- `routeEnum` (optional): an array of strings that defines a finite set of allowed values for an enum-like parameter in the route. When `routeEnum` is present, the first named parameter in the route `pattern` is treated as the enum value to validate against `routeEnum`.
+  - Example: for a route with pattern `:type/:user/:repo` you would set `static routeEnum = ['dt', 'dm', 'dd']` and the incoming `type` value will be validated against that array.
+  - If the first named parameter is not present in `routeEnum`, the request will be rejected before `handle()` is invoked.
+
+  ```js
+  // Example service that uses routeEnum
+  export default class Downloads extends BaseService {
+    static route = { base: 'downloads', pattern: ':type/:user/:repo' }
+    static routeEnum = ['dt', 'dm', 'dd']
+
+    async handle({ type, user, repo }) {
+      // `type` is guaranteed to be one of 'dt', 'dm', or 'dd'
+      // implement fetch/render logic here
+    }
+  }
+  ```
+
+- There is additional documentation on conventions for [designing badge URLs](./badge-urls.md)
+
 5. All badges must implement the `async handle()` function that receives parameters to render the badge. Parameters of `handle()` will match the name defined in `route` Because we're capturing a single variable called `text` our function signature is `async handle({ text })`. `async` is needed to let JavaScript do other things while we are waiting for result from external API. Although in this simple case, we don't make any external calls. Our `handle()` function should return an object with 3 properties:
    - `label`: the text on the left side of the badge
    - `message`: the text on the right side of the badge - here we are passing through the parameter we captured in the route regex

doc/badge-redirectors.md

@@ -60,8 +60,9 @@ export default redirector({
   category: 'analysis',
   route: {
     base: 'scrutinizer',
-    pattern: ':vcs(g|b)/:user/:repo/:branch*',
+    pattern: ':vcs/:user/:repo/:branch*',
   },
+  routeEnum: ['g', 'b'],
   transformPath: ({ vcs, user, repo, branch }) =>
     `/scrutinizer/quality/${vcs}/${user}/${repo}${branch ? `/${branch}` : ''}`,
   dateAdded: new Date('2025-02-02'),
@@ -81,8 +82,9 @@ export default redirector({
   category: 'monitoring',
   route: {
     base: 'website',
-    pattern: ':protocol(https|http)/:hostAndPath+',
+    pattern: ':protocol/:hostAndPath+',
   },
+  routeEnum: ['https', 'http'],
   transformPath: () => '/website',
   transformQueryParams: ({ protocol, hostAndPath }) => ({
     url: `${protocol}://${hostAndPath}`,

doc/badge-urls.md

@@ -5,6 +5,8 @@
   - singular if the badge message represents a single entity, such as the current status of a build (e.g: `/build`), or a more abstract or aggregate representation of the thing (e.g.: `/coverage`, `/quality`)
   - plural if there are (or may) be many of the thing (e.g: `/dependencies`, `/stars`)
 - Parameters should always be part of the route if they are required to display a badge e.g: `:packageName` and should be lower camelCase.
+- Parameters should always be part of the route if they are required to display a badge e.g: `:packageName` and should be lower camelCase.
+- `routeEnum` support: when a service defines a finite set of allowed values for a parameter, the service can declare `static routeEnum = [...]`. In that case the first named parameter in the route `pattern` is treated as the enum to validate against `routeEnum`.
 - Common optional params like, `:branch` or `:tag` should also be passed as part of the route.
 - Query string parameters should be used when:
   - The parameter is related to formatting. e.g: `/appveyor/tests/:user/:repo?compact_message`.

services/gem/gem-downloads.service.js

@@ -29,7 +29,8 @@ const versionSchema = Joi.array()
 
 export default class GemDownloads extends BaseJsonService {
   static category = 'downloads'
-  static route = { base: 'gem', pattern: ':variant(dt|dtv|dv)/:gem/:version?' }
+  static route = { base: 'gem', pattern: ':variant/:gem/:version?' }
+  static routeEnum = ['dt', 'dtv', 'dv']
   static openApi = {
     '/gem/dt/{gem}': {
       get: {

services/greasyfork/greasyfork-downloads.service.js

@@ -4,7 +4,8 @@ import BaseGreasyForkService from './greasyfork-base.js'
 
 export default class GreasyForkInstalls extends BaseGreasyForkService {
   static category = 'downloads'
-  static route = { base: 'greasyfork', pattern: ':variant(dt|dd)/:scriptId' }
+  static route = { base: 'greasyfork', pattern: ':variant/:scriptId' }
+  static routeEnum = ['dt', 'dd']
 
   static openApi = {
     '/greasyfork/{variant}/{scriptId}': {

services/hexpm/hexpm.service.js

@@ -127,8 +127,9 @@ class HexPmDownloads extends BaseHexPmService {
 
   static route = {
     base: 'hexpm',
-    pattern: ':interval(dd|dw|dt)/:packageName',
+    pattern: ':interval/:packageName',
   }
+  static routeEnum = ['dd', 'dw', 'dt']
 
   static openApi = {
     '/hexpm/{interval}/{packageName}': {

services/mozilla-observatory/mozilla-observatory.service.js

@@ -21,8 +21,9 @@ export default class MozillaObservatory extends BaseJsonService {
 
   static route = {
     base: 'mozilla-observatory',
-    pattern: ':format(grade|grade-score)/:host',
+    pattern: ':format/:host',
   }
+  static routeEnum = ['grade', 'grade-score']
 
   static openApi = {
     '/mozilla-observatory/{format}/{host}': {

services/packagist/packagist-downloads.service.js

@@ -42,9 +42,10 @@ export default class PackagistDownloads extends BasePackagistService {
 
   static route = {
     base: 'packagist',
-    pattern: ':interval(dd|dm|dt)/:user/:repo',
+    pattern: ':interval/:user/:repo',
     queryParamSchema,
   }
+  static routeEnum = ['dd', 'dm', 'dt']
 
   static openApi = {
     '/packagist/{interval}/{user}/{repo}': {

services/teamcity/teamcity-build.service.js

@@ -17,9 +17,10 @@ export default class TeamCityBuild extends TeamCityBase {
 
   static route = {
     base: 'teamcity/build',
-    pattern: ':verbosity(s|e)/:buildId',
+    pattern: ':verbosity/:buildId',
     queryParamSchema,
   }
+  static routeEnum = ['s', 'e']
 
   static openApi = {
     '/teamcity/build/s/{buildId}': {

services/twitter/twitter-redirect.service.js

@@ -6,8 +6,9 @@ export default [
     name: 'TwitterUrlRedirect',
     route: {
       base: 'twitter/url',
-      pattern: ':protocol(https|http)/:hostAndPath+',
+      pattern: ':protocol/:hostAndPath+',
     },
+    routeEnum: ['https', 'http'],
     transformPath: () => '/twitter/url',
     transformQueryParams: ({ protocol, hostAndPath }) => ({
       url: `${protocol}://${hostAndPath}`,