Redirect from alias to canonical URL [azuredevops vso] (#2939)

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.

Close #2340

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,

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()
+      })
+    }
+  }
+}

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'
+      )
+    })
+  })
+})

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',

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' })

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',

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' })

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'),
+  }),
+]

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'
+  )