diff --git a/src/article-api/middleware/pagelist.ts b/src/article-api/middleware/pagelist.ts index a4e2c11f7a..eab002c929 100644 --- a/src/article-api/middleware/pagelist.ts +++ b/src/article-api/middleware/pagelist.ts @@ -1,68 +1,104 @@ import express from 'express' -import type { Response } from 'express' +import type { Response, RequestHandler } from 'express' import type { ExtendedRequest } from '@/types' import { defaultCacheControl } from '@/frame/middleware/cache-control.js' import { getProductStringFromPath, getVersionStringFromPath } from '#src/frame/lib/path-utils.js' -import { latest } from '#src/versions/lib/enterprise-server-releases.js' +import { getLanguageCodeFromPath } from '#src/languages/middleware/detect-language.js' +import { pagelistValidationMiddleware } from './validation' +import catchMiddlewareError from '#src/observability/middleware/catch-middleware-error.js' const router = express.Router() -router.get('/v1/enterprise-server@latest', (req, res) => { - res.redirect( - 307, - req.originalUrl.replace( - '/pagelist/v1/enterprise-server@latest', - `/pagelist/v1/enterprise-server@${latest}`, - ), - ) -}) +// pagelistValidationMiddleware is used for every route to normalize the lang and version from the path -router.get('/v1/:product@:version', (req: ExtendedRequest, res: Response) => { - const { product, version } = req.params +// If no version or lang is provided we'll assume english and fpt and redirect there +router.get( + '/', + pagelistValidationMiddleware as RequestHandler, + catchMiddlewareError(async function (req: ExtendedRequest, res: Response) { + res.redirect( + 308, + req.originalUrl.replace( + '/pagelist', + `/pagelist/${req.context!.currentLanguage}/${req.context!.currentVersion}`, + ), + ) + }), +) - if (!req.context || !req.context.pages) throw new Error('Request not contextualized.') +// handles paths with fragments that could be the language or the version +router.get( + '/:someParam', + pagelistValidationMiddleware as RequestHandler, + catchMiddlewareError(async function (req: ExtendedRequest, res: Response) { + const { someParam } = req.params + res.redirect( + 308, + req.originalUrl.replace( + `/pagelist/${someParam}`, + `/pagelist/${req.context!.currentLanguage}/${req.context!.currentVersion}`, + ), + ) + }), +) - const pages = req.context.pages +// for a fully qualified path with language and product version, we'll serve up the pagelist +router.get( + '/:lang/:productVersion', + pagelistValidationMiddleware as RequestHandler, + catchMiddlewareError(async function (req: ExtendedRequest, res: Response) { + if (!req.context || !req.context.pages) throw new Error('Request not contextualized.') - // the keys of `context.pages` are permalinks - const keys = Object.keys(pages) + const pages = req.context.pages - // we filter the permalinks to get only our target version - const filteredPermalinks = keys.filter((key) => versionMatcher(key, `${product}@${version}`)) - const expression = /^\/en/ + // the keys of `context.pages` are permalinks + const keys = Object.keys(pages) - if (!filteredPermalinks.length) { - res.status(400).type('text').send('Invalid version') - return - } + // we filter the permalinks to get only our target version and language + const filteredPermalinks = keys.filter((key) => + versionMatcher(key, req.context!.currentVersion!, req.context!.currentLanguage!), + ) - //right now we only need english permalinks perhaps we can use the language from the request in the future - const englishPermalinks = filteredPermalinks.filter((permalink) => expression.test(permalink)) + // if we've filtered it out of existence, there's no articles to return so we must've + // gotten a bad language or version + if (!filteredPermalinks.length) { + const { lang, productVersion } = req.params - defaultCacheControl(res) + res + .status(400) + .type('application/json') + .send( + JSON.stringify({ + error: 'Invalid version or language code', + language: lang, + version: productVersion, + }), + ) + return + } - // new line added at the end so `wc` works as expected with `-l` and `-w`. - res.type('text').send(englishPermalinks.join('\n').concat('\n')) -}) + defaultCacheControl(res) -router.get('/:product@:version', (req, res) => { - res.redirect(307, req.originalUrl.replace('/pagelist', '/pagelist/v1')) -}) + // new line added at the end so `wc` works as expected with `-l` and `-w`. + res.type('text').send(filteredPermalinks.join('\n').concat('\n')) + }), +) -// If no version is provided we'll assume API v1 and Docs version FPT -router.get('/', (req, res) => { - res.redirect(307, req.originalUrl.replace('/pagelist', '/pagelist/v1/free-pro-team@latest')) -}) +function versionMatcher(key: string, targetVersion: string, targetLang: string) { + const versionFromPermalink = getVersionStringFromPath(key) -function versionMatcher(key: string, targetVersion: string) { - const versionFromPath = getVersionStringFromPath(key) - - if (!versionFromPath) { + if (!versionFromPermalink) { throw new Error(`Couldn't get version from the permalink ${key} when generating the pagelist.`) } if (getProductStringFromPath(key) === 'early-access') return null - if (versionFromPath === targetVersion) return key + + const langFromPermalink = getLanguageCodeFromPath(key) + if (!langFromPermalink) { + throw new Error(`Couldn't get language from the permalink ${key} when generating the pagelist.`) + } + + if (versionFromPermalink === targetVersion && langFromPermalink === targetLang) return key } export default router diff --git a/src/article-api/middleware/validation.ts b/src/article-api/middleware/validation.ts index 9964413b66..5fd0bb8720 100644 --- a/src/article-api/middleware/validation.ts +++ b/src/article-api/middleware/validation.ts @@ -1,8 +1,41 @@ import { ExtendedRequestWithPageInfo } from '../types' import type { NextFunction, Response } from 'express' + +import { ExtendedRequest, Page } from '#src/types.js' import { isArchivedVersionByPath } from '@/archives/lib/is-archived-version' import getRedirect from '@/redirects/lib/get-redirect.js' -import { Page } from '#src/types.js' +import { getVersionStringFromPath, getLangFromPath } from '#src/frame/lib/path-utils.js' +import nonEnterpriseDefaultVersion from '#src/versions/lib/non-enterprise-default-version.js' + +// validates the path for pagelist endpoint +// specifically, defaults to `/en/free-pro-team@latest` when those values are missing +// when they're provided, checks and cleans them up so we don't just lookup bad lang codes or versions +export const pagelistValidationMiddleware = ( + req: ExtendedRequest, + res: Response, + next: NextFunction, +) => { + // get version from path, fallback to default version if it can't be resolved + const versionFromPath = getVersionStringFromPath(req.path) || nonEnterpriseDefaultVersion + + // in the rare case that this failed, probably won't be reached + if (!versionFromPath) + return res.status(400).json({ error: `Couldn't get version from the given path.` }) + + // get the language from path, fallback to english if it can't be resolved + const langFromPath = getLangFromPath(req.path) || 'en' + + // in the rare case that the language fallback failed + if (!langFromPath) + return res.status(400).json({ + error: `Couldn't get language from the from the given path.`, + }) + + // set the version and language in the context, we'll use it later + req.context!.currentVersion = versionFromPath + req.context!.currentLanguage = langFromPath + return next() +} export const pathValidationMiddleware = ( req: ExtendedRequestWithPageInfo, diff --git a/src/article-api/tests/pagelist.ts b/src/article-api/tests/pagelist.ts index 1a985758a5..bc569adafb 100644 --- a/src/article-api/tests/pagelist.ts +++ b/src/article-api/tests/pagelist.ts @@ -4,19 +4,6 @@ import { get } from '#src/tests/helpers/e2etest.js' import { allVersionKeys } from '#src/versions/lib/all-versions.js' import nonEnterpriseDefaultVersion from '#src/versions/lib/non-enterprise-default-version.js' -import { latest } from '#src/versions/lib/enterprise-server-releases.js' - -test('redirects without version suffix', async () => { - const res = await get(`/api/pagelist`) - expect(res.statusCode).toBe(307) - expect(res.headers.location).toBe(`/api/pagelist/v1/${nonEnterpriseDefaultVersion}`) -}) - -test('redirects for ghes@latest', async () => { - const res = await get(`/api/pagelist/v1/enterprise-server@latest`) - expect(res.statusCode).toBe(307) - expect(res.headers.location).toBe(`/api/pagelist/v1/enterprise-server@${latest}`) -}) describe.each(allVersionKeys)('pagelist api for %s', async (versionKey) => { beforeAll(() => { @@ -37,7 +24,7 @@ describe.each(allVersionKeys)('pagelist api for %s', async (versionKey) => { }) // queries the pagelist API for each version - const res = await get(`/api/pagelist/v1/${versionKey}`) + const res = await get(`/api/pagelist/en/${versionKey}`) test('is reachable, returns 200 OK', async () => { expect(res.statusCode).toBe(200) @@ -45,7 +32,7 @@ describe.each(allVersionKeys)('pagelist api for %s', async (versionKey) => { // there's a large assortment of possible URLs, // even "/en" is an acceptable URL, so regexes capture lots - test('contains valid urls', async () => { + test('contains valid urls matching the requested version', async () => { let expression // if we're testing the default version, it may be missing @@ -62,7 +49,7 @@ describe.each(allVersionKeys)('pagelist api for %s', async (versionKey) => { }) }) - test('only returns urls that contain /en', async () => { + test('English requests only returns urls that contain /en', async () => { const expression = new RegExp(`^/en(/${nonEnterpriseDefaultVersion})?/?.*`) res.body .trim() @@ -72,3 +59,23 @@ describe.each(allVersionKeys)('pagelist api for %s', async (versionKey) => { }) }) }) + +describe('Redirect Tests', () => { + test('redirects without version suffix', async () => { + const res = await get(`/api/pagelist`) + expect(res.statusCode).toBe(308) + expect(res.headers.location).toBe(`/api/pagelist/en/${nonEnterpriseDefaultVersion}`) + }) + + test('should redirect to /pagelist/en/:product@:version when URL does not include /en', async () => { + const res = await get('/api/pagelist/free-pro-team@latest') + expect(res.statusCode).toBe(308) + expect(res.headers.location).toBe('/api/pagelist/en/free-pro-team@latest') + }) + + test('should redirect to /pagelist/en/free-pro-team@lateset when URL does not include version', async () => { + const res = await get('/api/pagelist/en') + expect(res.statusCode).toBe(308) + expect(res.headers.location).toBe('/api/pagelist/en/free-pro-team@latest') + }) +}) diff --git a/src/frame/lib/path-utils.js b/src/frame/lib/path-utils.js index 1c7c770292..9bf528040f 100644 --- a/src/frame/lib/path-utils.js +++ b/src/frame/lib/path-utils.js @@ -7,6 +7,14 @@ import { allVersions } from '#src/versions/lib/all-versions.js' import nonEnterpriseDefaultVersion from '#src/versions/lib/non-enterprise-default-version.js' const supportedVersions = new Set(Object.keys(allVersions)) +// Extracts the language code from the path +// if href is '/en/something', returns 'en' +export function getLangFromPath(href) { + // first remove the version from the path so we don't match, say, `/free-pro-team` as `/fr/` + const match = getPathWithoutVersion(href).match(patterns.getLanguageCode) + return match ? match[1] : null +} + // Add the language to the given HREF // /articles/foo -> /en/articles/foo export function getPathWithLanguage(href, languageCode) {