server: reformat/refactor around redirects

server.js

@@ -10,7 +10,8 @@
  */
 
 const { createServer } = require('http')
-const { parse } = require('url')
+const { parse: parseURL } = require('url')
+const { parse: parseQuery } = require('querystring')
 const next = require('next')
 
 const { getItemByPath } = require('./src/utils/sidebar')
@@ -23,23 +24,21 @@ const port = process.env.PORT || 3000
 
 app.prepare().then(() => {
   createServer((req, res) => {
-    const parsedUrl = parse(req.url, true)
-    const { pathname, query } = parsedUrl
+    const parsedUrl = parseURL(req.url)
+    const { pathname, queryStr } = parsedUrl
     const host = req.headers.host
 
-    /*
-     * HTTP redirects
-     */
     let [redirectCode, redirectLocation] = getRedirect(host, pathname, {
       req,
       dev
     })
-
     if (redirectLocation) {
-      // should be getting the query as a string
-      const { query } = parse(req.url)
-      if (query) {
-        redirectLocation += '?' + query
+      /*
+       * HTTP redirects
+       */
+
+      if (queryStr) {
+        redirectLocation += '?' + queryStr
       }
       res.writeHead(redirectCode, {
         'Cache-control': 'no-cache',
@@ -57,9 +56,12 @@ app.prepare().then(() => {
       }
 
       // Custom route for all docs
-      app.render(req, res, '/doc', query)
+      app.render(req, res, '/doc', parseQuery(queryStr))
     } else {
-      // Regular Next.js handler
+      /*
+       * Regular Next.js handler
+       */
+
       handle(req, res, parsedUrl)
     }
   }).listen(port, err => {

src/utils/redirects.js

@@ -17,7 +17,7 @@ const processRedirectString = redirectString => {
 
 exports.processRedirectString = processRedirectString
 
-// Parse redirects when starting up
+// Parse redirects when starting up.
 redirects = redirects.map(processRedirectString)
 
 const matchRedirectList = (host, pathname) => {

src/utils/redirects.test.js

@@ -3,41 +3,41 @@ const { processRedirectString, getRedirect } = require('./redirects')
 
 describe('processRedirectString', () => {
   it('reads the regex, replacement and code', () => {
-    const { regex, replace, code } = processRedirectString('^/foo /bar 418')
+    const { regex, replace, code } = processRedirectString('^/foo /bar 308')
 
     expect(regex).toBeInstanceOf(RegExp)
     expect(regex.source).toEqual('^\\/foo')
     expect(replace).toEqual('/bar')
-    expect(code).toEqual(418)
+    expect(code).toEqual(308)
   })
 
-  it('code defaults to 301', () => {
-    const { code } = processRedirectString('^/x /x')
+  it('defaults to 301 status code', () => {
+    const { code } = processRedirectString('^/x /y')
 
     expect(code).toEqual(301)
   })
 
-  it('detects whether we are matching a pathname or the whole url', () => {
-    const { matchPathname } = processRedirectString('^/pathname /x')
-    expect(matchPathname).toEqual(true)
-
+  it('detects whether matching a full URL or just the path', () => {
     const { matchPathname: matchPathnameFalse } = processRedirectString(
       '^https://example.com/foo /x'
     )
     expect(matchPathnameFalse).toEqual(false)
+
+    const { matchPathname } = processRedirectString('^/path /y')
+    expect(matchPathname).toEqual(true)
   })
 })
 
 describe('getRedirect', () => {
-  it('redirects to https while removing www', () => {
-    const fakeReq = {
+  it('redirects to https:// while removing www', () => {
+    const mockReq = {
       headers: {
         'x-forwarded-proto': 'http'
       },
       url: '/foo/bar?baz'
     }
     expect(
-      getRedirect('www.example.com', '/not-used', { req: fakeReq, dev: false })
+      getRedirect('www.example.com', '/not-used', { req: mockReq, dev: false })
     ).toEqual([301, 'https://example.com/foo/bar?baz'])
   })
 
@@ -57,18 +57,17 @@ describe('getRedirect', () => {
       expect(rLocation).toEqual(target)
       expect(rCode).toEqual(code)
 
-      // Detect redirect loops
+      // Detect redirect loops.
       const secondUrl = url.parse(addHost(rLocation))
       const secondRedirect = getRedirect(secondUrl.hostname, secondUrl.pathname)
       expect(secondRedirect).toEqual([])
     })
   }
 
-  describe('host redirects', () => {
-    // remove the www (when already HTTPS)
+  describe('Subdomain-based redirects', () => {
+    // Remove www (when already HTTPS)
     itRedirects('https://www.dvc.org/foo', 'https://dvc.org/foo')
 
-    // short and sweet hosts
     itRedirects(
       'https://man.dvc.org/',
       'https://dvc.org/doc/command-reference/',
@@ -94,7 +93,7 @@ describe('getRedirect', () => {
     )
   })
 
-  describe('redirects to s3', () => {
+  describe('S3 redirects', () => {
     itRedirects(
       'https://code.dvc.org/foo/bar',
       'https://s3-us-east-2.amazonaws.com/dvc-public/code/foo/bar',
@@ -126,22 +125,23 @@ describe('getRedirect', () => {
     )
   })
 
-  describe('discord', () => {
+  describe('Discord', () => {
     itRedirects('/help', 'https://discordapp.com/invite/dvwXA2N', 303)
 
     itRedirects('/chat', 'https://discordapp.com/invite/dvwXA2N', 303)
   })
 
-  describe('in-site moves', () => {
+  describe('Path redirects', () => {
     itRedirects('/docs/x', '/doc/x')
 
     itRedirects('/documentation/x', '/doc/x')
 
-    itRedirects('/doc/commands-reference/add', '/doc/command-reference/add')
+    itRedirects('/doc/commands-reference/foo', '/doc/command-reference/foo')
 
+    itRedirects('/doc/tutorial', '/doc/tutorials')
     itRedirects('/doc/tutorial/', '/doc/tutorials')
 
-    itRedirects('/doc/tutorial/subject', '/doc/tutorials/deep/subject')
+    itRedirects('/doc/tutorial/bar', '/doc/tutorials/deep/bar')
 
     itRedirects(
       '/doc/use-cases/data-and-model-files-versioning',