Skip to content
This repository has been archived by the owner on Jun 23, 2023. It is now read-only.

Commit

Permalink
Check config.json URLs for corresponding files (gravitational#123)
Browse files Browse the repository at this point in the history
* Check config.json URLs for corresponding files

Fixes gravitational#101

When loading the `docs/config.json` file for each version of the docs
site, we have some logic in `normalizeDocsUrl` that validates URLs
(i.e., checks if the URL has a trailing slash) and adds a
version-specific path segment to the URL path.

This changes `normalizeDocsUrl` to also check if a file exists at the
given URL path. This way, docs site builds fail if (a) a navigation
entry exists for a nonexistent docs page; or (b) a redirect points to a
nonexistent docs page.

Unfortunately, throughout the code that handles `config.json`, there is
logic that assumes the file lives in
`content/<version>/docs/config.json`. To test this change on a test data
directory, rather than an actual docs site directory, we would need to
refactor a lot of `server/config-docs.ts` and the code that imports from
it.

As a result, in order to test only `normalizeDocsUrl`, I had to make it
an exported function. I also had to make assumptions in the tests that
there would be a `content` directory populated with per-version
subdirectories.

* Respond to PR feedback

* Refactor the config.json checker

We can't guarantee that our CI server will have loaded this repo's git
submodules before running tests. This change refactors the
config.json checker so we can test it with any docs content directory,
e.g., the `server/fixtures` directory.

This also provides better separation of concerns between the
URL normalization code (which doesn't need to know about a directory
tree) and the file path checker code.
  • Loading branch information
ptgott authored Aug 9, 2022
1 parent b0eb44c commit 53124b5
Show file tree
Hide file tree
Showing 8 changed files with 241 additions and 5 deletions.
115 changes: 115 additions & 0 deletions server/config-docs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import { Redirect } from "next/dist/lib/load-custom-routes";
import { suite } from "uvu";
import * as assert from "uvu/assert";
import { Config, checkURLsForCorrespondingFiles } from "./config-docs";
import { randomUUID } from "crypto";
import { join } from "path";
import { opendirSync } from "fs";

const Suite = suite("server/config-docs");

Suite("Ensures that URLs correspond to docs pages", () => {
const conf: Config = {
navigation: [
{
icon: "bolt",
title: "Home",
entries: [
{
title: "Welcome",
slug: "/",
forScopes: ["oss"],
},
],
},
{
icon: "bolt",
title: "About",
entries: [
{
title: "Introduction",
slug: "/about/",
forScopes: ["oss"],
},
{
title: "Projects",
slug: "/about/projects/",
forScopes: ["oss"],
entries: [
{
title: "Project 1",
slug: "/about/projects/project1/",
forScopes: ["oss"],
},
{
title: "Project 2",
slug: "/about/projects/project2/",
forScopes: ["oss"],
},
{
title: "Project 3",
slug: "/about/projects/project3/",
forScopes: ["oss"],
},
],
},
{
title: "Team",
slug: "/about/team/",
forScopes: ["oss"],
},
],
},
{
icon: "bolt",
title: "Contact",
entries: [
{
title: "Welcome",
slug: "/contact/",
forScopes: ["oss"],
},
{
title: "Offices",
slug: "/contact/offices/",
forScopes: ["oss"],
},
],
},
],
redirects: [
{
source: "/offices/",
destination: "/contact/offices/",
permanent: true,
},
{
source: "/project4/",
destination: "/about/projects/project4/",
permanent: true,
},
{
source: "/project3",
destination: "/about/projects/project3/",
permanent: true,
},
],
};

const actual = checkURLsForCorrespondingFiles(
join("server", "fixtures", "fake-content"),
conf.navigation,
conf.redirects
);

const expected = [
"/about/projects/",
"/about/projects/project3/",
"/contact/",
"/about/projects/project4/",
];

assert.equal(actual, expected);
});

Suite.run();
107 changes: 102 additions & 5 deletions server/config-docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { Redirect } from "next/dist/lib/load-custom-routes";

import Ajv from "ajv";
import { validateConfig } from "./config-common";
import { resolve } from "path";
import { resolve, join } from "path";
import { existsSync, readFileSync } from "fs";
import { isExternalLink, isHash, splitPath } from "../utils/url";
import { NavigationCategory, NavigationItem } from "../layouts/DocsPage/types";
Expand Down Expand Up @@ -150,23 +150,105 @@ const validator = ajv.compile({
* Also we check that all links ends with "/: for consistency.
*/

const normalizeDocsUrl = (version: string, url: string, raw?: boolean) => {
/*
* normalizeDocsUrl ensures that internal docs URLs include trailing slashes and
* adds the docs version to the URL.*
*/
export const normalizeDocsUrl = (version: string, url: string) => {
if (isExternalLink(url) || isHash(url)) {
return url;
}

if (!splitPath(url).path.endsWith("/")) {
const configPath = getConfigPath(version);
const path = splitPath(url).path;
const configPath = getConfigPath(version);

if (!path.endsWith("/")) {
throw Error(`File ${configPath} misses trailing slash in '${url}' path.`);
}

const addVersion = raw || latest !== version;
const addVersion = latest !== version;
const prefix = `${addVersion ? `/ver/${version}` : ""}`;

return prefix + url;
};

const getPathsForNavigationEntries = (entries: NavigationItem[]): string[] => {
return entries.reduce((allSlugs, currentEntry) => {
let slugs = [currentEntry.slug];
if (currentEntry.entries) {
const moreSlugs = getPathsForNavigationEntries(currentEntry.entries);
slugs.push(...moreSlugs);
}
allSlugs.push(...slugs);
return allSlugs;
}, []);
};

// checkURLsForCorrespondingFiles attempts to open the URL paths provided in the
// navigation config provided by categories and the destination paths provided
// in redirects. It converts each URL path into a filename in the content
// directory rooted at dirRoot. If it fails to open a file corresponding to a
// URL path, it adds the file to an array of strings. It returns the resulting
// array.
export const checkURLsForCorrespondingFiles = (
dirRoot: string,
categories: NavigationCategory[],
redirects: Redirect[]
): string[] => {
let result: string[] = [];
categories.forEach((cat) => {
let slugs = getPathsForNavigationEntries(cat.entries);
result = result.concat(slugs.flat());
});

result = result.concat(
redirects.map((r) => {
// We only check destinations because there is no expectation that the
// source URL corresponds with a file.
return r.destination;
})
);

// Deduplicate result
result = Array.from(new Set(result).values());

return result.reduce((prev, curr) => {
if (correspondingFileExistsForURL(dirRoot, curr)) {
return prev;
}
prev.push(curr);
return prev;
}, []);
};

// checkURLForCorrespondingFile determines whether a file exists in the content
// directory rooted at dirRoot for the file corresponding to the provided URL path.// If a file does not exist, it returns false.
const correspondingFileExistsForURL = (
dirRoot: string,
urlpath: string
): boolean => {
// Each URL in the docs config begins at docs/pages within a given version's
// content directory. Get the MDX file for a given URL and check if it
// exists in the filesystem. URL paths must point to (a) an MDX file with
// the same name as the final path segment; (b) a file named "index.mdx"; or
// (c) a file named "introduction.mdx".
const mdxPath = urlpath.replace(/\/$/, ".mdx");
const docsPagePath = resolve(join(dirRoot, mdxPath));

const indexPath = resolve(join(dirRoot, urlpath + "index.mdx"));

const introPath = resolve(join(dirRoot, urlpath + "introduction.mdx"));

if (
[docsPagePath, indexPath, introPath].find((p) => {
return existsSync(p);
}) == undefined
) {
return false;
}
return true;
};

const normalizeDocsUrls = (
version: string,
entries: NavigationItem[]
Expand Down Expand Up @@ -210,6 +292,7 @@ const normalizeRedirects = (
return redirects.map((redirect) => {
return {
...redirect,
// Don't check for the existence of an MDX file for the redirect source
source: normalizeDocsUrl(version, redirect.source),
destination: normalizeDocsUrl(version, redirect.destination),
};
Expand Down Expand Up @@ -239,6 +322,20 @@ export const normalize = (config: Config, version: string): Config => {
export const loadConfig = (version: string) => {
const config = load(version);

const badSlugs = checkURLsForCorrespondingFiles(
join("content", version, "docs", "pages"),
config.navigation,
config.redirects
);

if (badSlugs.length > 0) {
throw new Error(
"The following navigation slugs or redirect destinations do not" +
" correspond to an actual MDX file:\n\t- " +
badSlugs.join("\n\t- ")
);
}

validateConfig<Config>(validator, config);

return normalize(config, version);
Expand Down
4 changes: 4 additions & 0 deletions server/fixtures/fake-content/about/introduction.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
title: About
description: This is the "about" section
---
4 changes: 4 additions & 0 deletions server/fixtures/fake-content/about/projects/project1.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
title: Project 1
description: "Here's information about Project 1"
---
4 changes: 4 additions & 0 deletions server/fixtures/fake-content/about/projects/project2.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
title: Project 2
description: "Here's information about Project 2"
---
4 changes: 4 additions & 0 deletions server/fixtures/fake-content/about/team.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
title: Team
description: Information about the team
---
4 changes: 4 additions & 0 deletions server/fixtures/fake-content/contact/offices.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
title: Offices
description: Information about our offices
---
4 changes: 4 additions & 0 deletions server/fixtures/fake-content/index.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
title: Home Page
description: There's no place like it.
---

0 comments on commit 53124b5

Please sign in to comment.