Repository: badges/shields (js)

Issue:
Redirect aliased badges to the canonical form
In #2333 I replaced an old route with a redirect.

As I wrote there:

> In the case of URLs which have permanently changed, an approach I’d like to try is issuing a 301 Redirect.
> 
> The benefit is that if a user pastes the URL into the address bar while they are previewing or editing it, the browser will replace the address with the corrected URL when it loads. I figure this will cause some people to update their URLs with no effort, simply because they previewed the badge in their browser, and others to change over, if they notice it.
> 
> We incur a slight cost, which is a second request. However many browsers cache the 301’s indefinitely, and we can set an effectively infinite cache duration so the CDN and most other downstream caches will keep them a long time. And handling the redirect is extremely cheap.
> 
> This is a nice way to preserve backward compatibility of old routes without having to complicate the new route, such as in the case of vso -> azure-devops. For maintenance purposes, the route that redirects can effectively be treated separately.
> 
> It’s also a nice, gentle, and confidence-inspiring way to signal that users should update their URLs.

Some possibilities:

- [ ] vso
- [ ] Wordpress stars
- [ ] snap-ci
- [ ] codacy?

Others?

Pull request:
Redirect from alias to canonical URL [azuredevops vso]
As described in #2340, this provides a way to replace an old alias with a redirect. This makes it easier to migrate our URLs over time without making our matching patterns more complicated.

The 301 redirect is sent back to the requester. If a user pastes the aliased URL into the address bar, it'll be replaced in the browser with the new URL, which gently encourages them to migrate.

We can do some follow-ons to migrate a few more of these.

Close #2340

Patch:
diff --git a/core/base-service/index.js b/core/base-service/index.js
index bdedfe9bd9..2f021c5185 100644
--- a/core/base-service/index.js
+++ b/core/base-service/index.js
@@ -9,6 +9,7 @@ const BaseXmlService = require('./base-xml')
 const BaseYamlService = require('./base-yaml')
 
 const deprecatedService = require('./deprecated-service')
+const redirector = require('./redirector')
 
 const {
   NotFound,
@@ -27,6 +28,7 @@ module.exports = {
   BaseXmlService,
   BaseYamlService,
   deprecatedService,
+  redirector,
   NotFound,
   InvalidResponse,
   Inaccessible,
diff --git a/core/base-service/redirector.js b/core/base-service/redirector.js
new file mode 100644
index 0000000000..07798d67e4
--- /dev/null
+++ b/core/base-service/redirector.js
@@ -0,0 +1,78 @@
+'use strict'
+
+const emojic = require('emojic')
+const Joi = require('joi')
+const BaseService = require('./base')
+const {
+  serverHasBeenUpSinceResourceCached,
+  setCacheHeadersForStaticResource,
+} = require('./cache-headers')
+const { prepareRoute, namedParamsForMatch } = require('./route')
+const trace = require('./trace')
+
+module.exports = function redirector({ category, route, target, dateAdded }) {
+  return class Redirector extends BaseService {
+    static get category() {
+      return category
+    }
+
+    static get route() {
+      return route
+    }
+
+    static get isDeprecated() {
+      return true
+    }
+
+    static validateDefinition() {
+      super.validateDefinition()
+      Joi.assert(
+        { dateAdded },
+        Joi.object({
+          dateAdded: Joi.date().required(),
+        }),
+        `Redirector for ${route.base}`
+      )
+    }
+
+    static register({ camp }) {
+      const { regex, captureNames } = prepareRoute(this.route)
+
+      camp.route(regex, async (queryParams, match, end, ask) => {
+        if (serverHasBeenUpSinceResourceCached(ask.req)) {
+          // Send Not Modified.
+          ask.res.statusCode = 304
+          ask.res.end()
+          return
+        }
+
+        const namedParams = namedParamsForMatch(captureNames, match, this)
+        trace.logTrace(
+          'inbound',
+          emojic.arrowHeadingUp,
+          'Redirector',
+          route.base
+        )
+        trace.logTrace('inbound', emojic.ticket, 'Named params', namedParams)
+        trace.logTrace('inbound', emojic.crayon, 'Query params', queryParams)
+
+        const targetUrl = target(namedParams)
+        trace.logTrace('validate', emojic.dart, 'Target', targetUrl)
+
+        // The final capture group is the extension.
+        const format = match.slice(-1)[0]
+        const redirectUrl = `${targetUrl}.${format}${ask.uri.search || ''}`
+        trace.logTrace('outbound', emojic.shield, 'Redirect URL', redirectUrl)
+
+        ask.res.statusCode = 301
+        ask.res.setHeader('Location', redirectUrl)
+
+        // To avoid caching mistakes for a long time, and to make this simpler
+        // to reason about, use the same cache semantics as the static badge.
+        setCacheHeadersForStaticResource(ask.res)
+
+        ask.res.end()
+      })
+    }
+  }
+}
diff --git a/core/base-service/redirector.spec.js b/core/base-service/redirector.spec.js
new file mode 100644
index 0000000000..4a6eb54037
--- /dev/null
+++ b/core/base-service/redirector.spec.js
@@ -0,0 +1,105 @@
+'use strict'
+
+const Camp = require('camp')
+const got = require('got')
+const portfinder = require('portfinder')
+const { expect } = require('chai')
+const redirector = require('./redirector')
+
+describe('Redirector', function() {
+  const route = {
+    base: 'very/old/service',
+    pattern: ':namedParamA',
+  }
+  const category = 'analysis'
+  const target = () => {}
+  const attrs = {
+    category,
+    route,
+    target,
+    dateAdded: new Date(),
+  }
+
+  it('returns true on isDeprecated', function() {
+    expect(redirector(attrs).isDeprecated).to.be.true
+  })
+
+  it('sets specified route', function() {
+    expect(redirector(attrs).route).to.deep.equal(route)
+  })
+
+  it('sets specified category', function() {
+    expect(redirector(attrs).category).to.equal(category)
+  })
+
+  it('throws the expected error when dateAdded is missing', function() {
+    expect(() =>
+      redirector({ route, category, target }).validateDefinition()
+    ).to.throw('"dateAdded" is required')
+  })
+
+  describe('ScoutCamp integration', function() {
+    let port, baseUrl
+    beforeEach(async function() {
+      port = await portfinder.getPortPromise()
+      baseUrl = `http://127.0.0.1:${port}`
+    })
+
+    let camp
+    beforeEach(async function() {
+      camp = Camp.start({ port, hostname: '::' })
+      await new Promise(resolve => camp.on('listening', () => resolve()))
+    })
+    afterEach(async function() {
+      if (camp) {
+        await new Promise(resolve => camp.close(resolve))
+        camp = undefined
+      }
+    })
+
+    const target = ({ namedParamA }) => `/new/service/${namedParamA}`
+
+    beforeEach(function() {
+      const ServiceClass = redirector({ route, target })
+      ServiceClass.register({ camp })
+    })
+
+    it('should redirect as configured', async function() {
+      const { statusCode, headers } = await got(
+        `${baseUrl}/very/old/service/hello-world.svg`,
+        {
+          followRedirect: false,
+        }
+      )
+
+      expect(statusCode).to.equal(301)
+      expect(headers.location).to.equal('/new/service/hello-world.svg')
+    })
+
+    it('should preserve the extension', async function() {
+      const { statusCode, headers } = await got(
+        `${baseUrl}/very/old/service/hello-world.png`,
+        {
+          followRedirect: false,
+        }
+      )
+
+      expect(statusCode).to.equal(301)
+      expect(headers.location).to.equal('/new/service/hello-world.png')
+    })
+
+    it('should forward the query params', async function() {
+      const { statusCode, headers } = await got(
+        `${baseUrl}/very/old/service/hello-world.svg?colorB=123&style=flat-square`,
+        {
+          followRedirect: false,
+        }
+      )
+
+      expect(statusCode).to.equal(301)
+      expect(headers.location).to.equal(
+        '/new/service/hello-world.svg?colorB=123&style=flat-square'
+      )
+    })
+  })
+})
diff --git a/services/azure-devops/azure-devops-build.service.js b/services/azure-devops/azure-devops-build.service.js
index 52d9070781..e713638af5 100644
--- a/services/azure-devops/azure-devops-build.service.js
+++ b/services/azure-devops/azure-devops-build.service.js
@@ -34,9 +34,8 @@ module.exports = class AzureDevOpsBuild extends BaseSvgScrapingService {
 
   static get route() {
     return {
-      base: '',
-      format: '(?:azure-devops|vso)/build/([^/]+)/([^/]+)/([^/]+)(?:/(.+))?',
-      capture: ['organization', 'projectId', 'definitionId', 'branch'],
+      base: 'azure-devops/build',
+      pattern: ':organization/:projectId/:definitionId/:branch*',
     }
   }
 
@@ -44,7 +43,7 @@ module.exports = class AzureDevOpsBuild extends BaseSvgScrapingService {
     return [
       {
         title: 'Azure DevOps builds',
-        pattern: 'azure-devops/build/:organization/:projectId/:definitionId',
+        pattern: ':organization/:projectId/:definitionId',
         namedParams: {
           organization: 'totodem',
           projectId: '8cf3ec0e-d0c2-4fcd-8206-ad204f254a96',
@@ -56,8 +55,7 @@ module.exports = class AzureDevOpsBuild extends BaseSvgScrapingService {
       },
       {
         title: 'Azure DevOps builds (branch)',
-        pattern:
-          'azure-devops/build/:organization/:projectId/:definitionId/:branch',
+        pattern: ':organization/:projectId/:definitionId/:branch',
         namedParams: {
           organization: 'totodem',
           projectId: '8cf3ec0e-d0c2-4fcd-8206-ad204f254a96',
diff --git a/services/azure-devops/azure-devops-release.service.js b/services/azure-devops/azure-devops-release.service.js
index 331d59f7ca..88f7449e9b 100644
--- a/services/azure-devops/azure-devops-release.service.js
+++ b/services/azure-devops/azure-devops-release.service.js
@@ -29,9 +29,8 @@ module.exports = class AzureDevOpsRelease extends BaseSvgScrapingService {
 
   static get route() {
     return {
-      base: '',
-      format: '(?:azure-devops|vso)/release/([^/]+)/([^/]+)/([^/]+)/([^/]+)',
-      capture: ['organization', 'projectId', 'definitionId', 'environmentId'],
+      base: 'azure-devops/release',
+      pattern: ':organization/:projectId/:definitionId/:environmentId',
     }
   }
 
@@ -39,8 +38,6 @@ module.exports = class AzureDevOpsRelease extends BaseSvgScrapingService {
     return [
       {
         title: 'Azure DevOps releases',
-        pattern:
-          'azure-devops/release/:organization/:projectId/:definitionId/:environmentId',
         namedParams: {
           organization: 'totodem',
           projectId: '8cf3ec0e-d0c2-4fcd-8206-ad204f254a96',
diff --git a/services/azure-devops/vso-redirect.service.js b/services/azure-devops/vso-redirect.service.js
new file mode 100644
index 0000000000..0589750f0f
--- /dev/null
+++ b/services/azure-devops/vso-redirect.service.js
@@ -0,0 +1,31 @@
+'use strict'
+
+const { redirector } = require('..')
+
+module.exports = [
+  redirector({
+    category: 'build',
+    route: {
+      base: 'vso/build',
+      pattern: ':organization/:projectId/:definitionId/:branch*',
+    },
+    target: ({ organization, projectId, definitionId, branch }) => {
+      let path = `/azure-devops/build/${organization}/${projectId}/${definitionId}`
+      if (branch) {
+        path += `/${branch}`
+      }
+      return path
+    },
+    dateAdded: new Date('2019-02-08'),
+  }),
+  redirector({
+    category: 'build',
+    route: {
+      base: 'vso/release',
+      pattern: ':organization/:projectId/:definitionId/:environmentId',
+    },
+    target: ({ organization, projectId, definitionId, environmentId }) =>
+      `/azure-devops/release/${organization}/${projectId}/${definitionId}/${environmentId}`,
+    dateAdded: new Date('2019-02-08'),
+  }),
+]


Test patch:
diff --git a/services/azure-devops/azure-devops-build.tester.js b/services/azure-devops/azure-devops-build.tester.js
index d7a47a4f9d..2a50302012 100644
--- a/services/azure-devops/azure-devops-build.tester.js
+++ b/services/azure-devops/azure-devops-build.tester.js
@@ -9,9 +9,7 @@ const { isBuildStatus } = require('../../lib/build-status')
 const t = (module.exports = require('../tester').createServiceTester())
 
 t.create('default branch')
-  .get(
-    '/azure-devops/build/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/2.json'
-  )
+  .get('/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/2.json')
   .expectJSONTypes(
     Joi.object().keys({
       name: 'build',
@@ -20,9 +18,7 @@ t.create('default branch')
   )
 
 t.create('named branch')
-  .get(
-    '/azure-devops/build/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/2/master.json'
-  )
+  .get('/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/2/master.json')
   .expectJSONTypes(
     Joi.object().keys({
       name: 'build',
@@ -31,22 +27,20 @@ t.create('named branch')
   )
 
 t.create('unknown definition')
-  .get(
-    '/azure-devops/build/larsbrinkhoff/953a34b9-5966-4923-a48a-c41874cfb5f5/515.json'
-  )
+  .get('/larsbrinkhoff/953a34b9-5966-4923-a48a-c41874cfb5f5/515.json')
   .expectJSON({ name: 'build', value: 'definition not found' })
 
 t.create('unknown project')
-  .get('/azure-devops/build/larsbrinkhoff/foo/515.json')
+  .get('/larsbrinkhoff/foo/515.json')
   .expectJSON({ name: 'build', value: 'user or project not found' })
 
 t.create('unknown user')
-  .get('/azure-devops/build/notarealuser/foo/515.json')
+  .get('/notarealuser/foo/515.json')
   .expectJSON({ name: 'build', value: 'user or project not found' })
 
 // The following build definition has always a partially succeeded status
 t.create('partially succeeded build')
   .get(
-    '/azure-devops/build/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/4.json?style=_shields_test'
+    '/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/4.json?style=_shields_test'
   )
   .expectJSON({ name: 'build', value: 'passing', color: 'orange' })
diff --git a/services/azure-devops/azure-devops-release.tester.js b/services/azure-devops/azure-devops-release.tester.js
index e1c5104125..203e951b9d 100644
--- a/services/azure-devops/azure-devops-release.tester.js
+++ b/services/azure-devops/azure-devops-release.tester.js
@@ -9,9 +9,7 @@ const { isBuildStatus } = require('../../lib/build-status')
 const t = (module.exports = require('../tester').createServiceTester())
 
 t.create('release status is succeeded')
-  .get(
-    '/azure-devops/release/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/1/1.json'
-  )
+  .get('/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/1/1.json')
   .expectJSONTypes(
     Joi.object().keys({
       name: 'deployment',
@@ -20,24 +18,20 @@ t.create('release status is succeeded')
   )
 
 t.create('unknown environment')
-  .get(
-    '/azure-devops/release/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/1/515.json'
-  )
+  .get('/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/1/515.json')
   .expectJSON({ name: 'deployment', value: 'user or environment not found' })
 
 t.create('unknown definition')
-  .get(
-    '/azure-devops/release/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/515/515.json'
-  )
+  .get('/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/515/515.json')
   .expectJSON({
     name: 'deployment',
     value: 'inaccessible or definition not found',
   })
 
 t.create('unknown project')
-  .get('/azure-devops/release/totodem/515/515/515.json')
+  .get('/totodem/515/515/515.json')
   .expectJSON({ name: 'deployment', value: 'project not found' })
 
 t.create('unknown user')
-  .get('/azure-devops/release/this-repo/does-not-exist/1/2.json')
+  .get('/this-repo/does-not-exist/1/2.json')
   .expectJSON({ name: 'deployment', value: 'user or environment not found' })
diff --git a/services/azure-devops/vso-redirect.tester.js b/services/azure-devops/vso-redirect.tester.js
new file mode 100644
index 0000000000..29490de3e9
--- /dev/null
+++ b/services/azure-devops/vso-redirect.tester.js
@@ -0,0 +1,38 @@
+'use strict'
+
+const { ServiceTester } = require('../tester')
+
+const t = (module.exports = new ServiceTester({
+  id: 'vso',
+  title: 'VSO',
+}))
+
+t.create('Build: default branch')
+  .get('/build/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/2.svg', {
+    followRedirect: false,
+  })
+  .expectStatus(301)
+  .expectHeader(
+    'Location',
+    '/azure-devops/build/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/2.svg'
+  )
+
+t.create('Build: named branch')
+  .get('/build/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/2/master.svg', {
+    followRedirect: false,
+  })
+  .expectStatus(301)
+  .expectHeader(
+    'Location',
+    '/azure-devops/build/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/2/master.svg'
+  )
+
+t.create('Release status')
+  .get('/release/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/1/1.svg', {
+    followRedirect: false,
+  })
+  .expectStatus(301)
+  .expectHeader(
+    'Location',
+    '/azure-devops/release/totodem/8cf3ec0e-d0c2-4fcd-8206-ad204f254a96/1/1.svg'
+  )