Skip to content

Commit

Permalink
re-read content on every request (github#30646)
Browse files Browse the repository at this point in the history
  • Loading branch information
peterbe authored Sep 13, 2022
1 parent c1feb44 commit edc25dd
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 5 deletions.
2 changes: 1 addition & 1 deletion contributing/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ npm run build
npm start
```

You should now have a running server! Visit [localhost:4000](http://localhost:4000) in your browser. It will automatically restart as you make changes to site content.
You should now have a running server! Visit [localhost:4000](http://localhost:4000) in your browser.

When you're ready to stop your local server, type <kbd>Ctrl</kbd>+<kbd>C</kbd> in your terminal window.

Expand Down
55 changes: 53 additions & 2 deletions middleware/find-page.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,32 @@
export default function findPage(req, res, next) {
const page = req.context.pages[req.pagePath]
import { fileURLToPath } from 'url'
import path from 'path'
import { existsSync } from 'fs'

import Page from '../lib/page.js'
import { languageKeys } from '../lib/languages.js'

const languagePrefixRegex = new RegExp(`^/(${languageKeys.join('|')})(/|$)`)
const englishPrefixRegex = /^\/en(\/|$)/
const __dirname = path.dirname(fileURLToPath(import.meta.url))
const CONTENT_ROOT = path.posix.join(__dirname, '..', 'content')

export default async function findPage(
req,
res,
next,
// Express won't execute these but it makes it easier to unit test
// the middleware.
{ isDev = process.env.NODE_ENV === 'development', contentRoot = CONTENT_ROOT } = {}
) {
// Filter out things like `/will/redirect` or `/_next/data/...`
if (!languagePrefixRegex.test(req.pagePath)) {
return next()
}

const page =
isDev && englishPrefixRegex.test(req.pagePath)
? await rereadByPath(req.pagePath, contentRoot, req.context.currentVersion)
: req.context.pages[req.pagePath]

if (page) {
req.context.page = page
Expand All @@ -8,3 +35,27 @@ export default function findPage(req, res, next) {

return next()
}

async function rereadByPath(uri, contentRoot, currentVersion) {
const languageCode = uri.match(languagePrefixRegex)[1]
const withoutLanguage = uri.replace(languagePrefixRegex, '/')
const withoutVersion = withoutLanguage.replace(`/${currentVersion}/`, '/')
// TODO: Support loading translations the same way.
// NOTE: No one is going to test translations like this in development
// but perhaps one day we can always and only do these kinds of lookups
// at runtime.
const possible = path.join(contentRoot, withoutVersion)
const filePath = existsSync(possible) ? path.join(possible, 'index.md') : possible + '.md'
const relativePath = path.relative(contentRoot, filePath)
const basePath = contentRoot

// Remember, the Page.init() can return a Promise that resolves to falsy
// if it can't read the file in from disk. E.g. a request for /en/non/existent.
// In other words, it's fine if it can't be read from disk. It'll get
// handled and turned into a nice 404 message.
return await Page.init({
basePath,
relativePath,
languageCode,
})
}
2 changes: 1 addition & 1 deletion middleware/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ export default function (app) {
app.use(instrument(handleRedirects, './redirects/handle-redirects')) // Must come before contextualizers

// *** Config and context for rendering ***
app.use(instrument(findPage, './find-page')) // Must come before archived-enterprise-versions, breadcrumbs, featured-links, products, render-page
app.use(asyncMiddleware(instrument(findPage, './find-page'))) // Must come before archived-enterprise-versions, breadcrumbs, featured-links, products, render-page
app.use(instrument(blockRobots, './block-robots'))

// Check for a dropped connection before proceeding
Expand Down
3 changes: 2 additions & 1 deletion nodemon.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"script",
"translations",
"stylesheets",
"tests"
"tests",
"content"
]
}
89 changes: 89 additions & 0 deletions tests/unit/find-page-middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { fileURLToPath } from 'url'
import path from 'path'
import http from 'http'

import { expect, describe, test } from '@jest/globals'

import Page from '../../lib/page.js'
import findPage from '../../middleware/find-page.js'

const __dirname = path.dirname(fileURLToPath(import.meta.url))

function makeRequestResponse(url, currentVersion = 'free-pro-team@latest') {
const req = new http.IncomingMessage(null)
req.method = 'GET'
req.url = url
req.path = url
req.cookies = {}
req.headers = {}

// Custom keys on the request
req.pagePath = url
req.context = {}
req.context.currentVersion = currentVersion
req.context.pages = {}

return [req, new http.ServerResponse(req)]
}

describe('find page middleware', () => {
test('attaches page on req.context', async () => {
const url = '/en/foo/bar'
const [req, res] = makeRequestResponse(url)
req.context.pages = {
'/en/foo/bar': await Page.init({
relativePath: 'page-with-redirects.md',
basePath: path.join(__dirname, '../fixtures'),
languageCode: 'en',
}),
}

let nextCalls = 0
await findPage(req, res, () => {
nextCalls++
})
expect(nextCalls).toBe(1)
expect(req.context.page).toBeInstanceOf(Page)
})

test('does not attach page on req.context if not found', async () => {
const [req, res] = makeRequestResponse('/en/foo/bar')

let nextCalls = 0
await findPage(req, res, () => {
nextCalls++
})
expect(nextCalls).toBe(1)
expect(req.context.page).toBe(undefined)
})

test('re-reads from disk if in development mode', async () => {
const [req, res] = makeRequestResponse('/en/page-with-redirects')

await findPage(req, res, () => {}, {
isDev: true,
contentRoot: path.join(__dirname, '../fixtures'),
})
expect(req.context.page).toBeInstanceOf(Page)
})

test('finds it for non-fpt version URLS', async () => {
const [req, res] = makeRequestResponse('/en/page-with-redirects', 'enterprise-cloud@latest')

await findPage(req, res, () => {}, {
isDev: true,
contentRoot: path.join(__dirname, '../fixtures'),
})
expect(req.context.page).toBeInstanceOf(Page)
})

test('re-reads from disk if in development mode and finds nothing', async () => {
const [req, res] = makeRequestResponse('/en/never/heard/of')

await findPage(req, res, () => {}, {
isDev: true,
contentRoot: path.join(__dirname, '../fixtures'),
})
expect(req.context.page).toBe(undefined)
})
})

0 comments on commit edc25dd

Please sign in to comment.