Skip to content

Commit

Permalink
Unravel pages (the array + map object) (github#16708)
Browse files Browse the repository at this point in the history
* Revise the 'pages' module to export two methods: 'loadPages' and 'loadPageMap'

Update all existing references to use 'loadPages' for now

* Remove explicit Promise resolutions on loadPage* methods

* Condense reduction method into its now-singular usage spot

* Opt for for-of instead of forEach

* Make require of pages in warm-server more explicit

* Be more explicit about find-page using a pageMap

* Be more explicit about find-page-in-version using a pageMap

* Be more explicit about site-tree using a pageMap

* Extract the map creation from loadPageMap

* Be more explicit about using a pageMap

* Update redirects precompile to take two arguments: pageList, pageMap

* Rename internal loadPages method to loadPageList

* Clarify pageMap is what is stored in context.pages

* Use loadPageMap in tests and stuff
  • Loading branch information
JamesMGreene authored Dec 3, 2020
1 parent 510d503 commit fb30a07
Show file tree
Hide file tree
Showing 25 changed files with 262 additions and 198 deletions.
2 changes: 1 addition & 1 deletion lib/algolia/find-indexable-pages.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const loadPages = require('../pages')
const { loadPages } = require('../pages')

module.exports = async function findIndexablePages () {
const allPages = await loadPages()
Expand Down
4 changes: 2 additions & 2 deletions lib/find-page-in-version.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const findPage = require('./find-page')
const getApplicableVersions = require('./get-applicable-versions')

module.exports = function findPageInVersion (href, pages, redirects, languageCode, version, isDotcomOnly = false) {
module.exports = function findPageInVersion (href, pageMap, redirects, languageCode, version, isDotcomOnly = false) {
// findPage() will throw an error if an English page can't be found
const page = findPage(href, pages, redirects, languageCode)
const page = findPage(href, pageMap, redirects, languageCode)
if (!page) return null

// if the link is on the homepage, return the page as soon as it's found
Expand Down
8 changes: 4 additions & 4 deletions lib/find-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const patterns = require('./patterns')
const allVersions = Object.keys(require('./all-versions'))
const { getVersionedPathWithLanguage } = require('./path-utils')

module.exports = function findPage (href, pages, redirects = {}, languageCode = 'en', sourceLanguage = null) {
module.exports = function findPage (href, pageMap, redirects = {}, languageCode = 'en', sourceLanguage = null) {
// Convert Windows backslashes to forward slashes
// remove trailing slash
href = slash(href).replace(patterns.trailingSlash, '$1')
Expand All @@ -16,14 +16,14 @@ module.exports = function findPage (href, pages, redirects = {}, languageCode =
// get the first found path of the page (account for redirects)
let pathToPage = versionedPathsToCheck.find(path => {
path = redirects[path] || path
return pages[removeFragment(path)]
return pageMap[removeFragment(path)]
})

// need to account for redirects again
pathToPage = redirects[pathToPage] || pathToPage

// find the page
const page = pages[removeFragment(pathToPage)]
const page = pageMap[removeFragment(pathToPage)]

if (page) return page

Expand Down Expand Up @@ -52,7 +52,7 @@ module.exports = function findPage (href, pages, redirects = {}, languageCode =
// because localized content is not yet synced
if (languageCode !== 'en') {
// pass the source language so we can surface it in error messages
return findPage(href, pages, redirects, 'en', languageCode)
return findPage(href, pageMap, redirects, 'en', languageCode)
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/find-unused-assets.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const walk = require('walk-sync')
const { execSync } = require('child_process')
const assert = require('assert')
const loadSiteData = require('./site-data')
const loadPages = require('./pages')
const { loadPages } = require('./pages')
const patterns = require('./patterns')
const getDataReferences = require('./get-liquid-data-references')
const imagesPath = '/assets/images'
Expand Down
6 changes: 3 additions & 3 deletions lib/get-english-headings.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
const headingRegex = /^###?#? (.*?)$/gm

// for any translated page, first get corresponding English markdown
// then get the headings on both the translated and English pages
// then get the headings on both the translated and English pageMap
// finally, create a map of translation:English for all headings on the page
module.exports = function getEnglishHeadings (page, pages) {
module.exports = function getEnglishHeadings (page, pageMap) {
const translatedHeadings = page.markdown.match(headingRegex)
if (!translatedHeadings) return

const englishPage = pages[`/en/${page.relativePath.replace(/.md$/, '')}`]
const englishPage = pageMap[`/en/${page.relativePath.replace(/.md$/, '')}`]
if (!englishPage) return

// FIX there may be bugs if English headings are updated before Crowdin syncs up :/
Expand Down
4 changes: 2 additions & 2 deletions lib/get-map-topic-content.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const findPage = require('./find-page')

// get the page.childArticles set on english map topics in lib/site-tree.js
module.exports = function getMapTopicContent (page, pages, redirects) {
module.exports = function getMapTopicContent (page, pageMap, redirects) {
const englishPage = page.languageCode !== 'en'
? findPage(`/${page.relativePath.replace(/.md$/, '')}`, pages, redirects, 'en')
? findPage(`/${page.relativePath.replace(/.md$/, '')}`, pageMap, redirects, 'en')
: page

if (!englishPage) {
Expand Down
43 changes: 32 additions & 11 deletions lib/pages.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ const Page = require('./page')
const languages = require('./languages')
const fs = require('fs')

module.exports = async function () {
const pages = []
async function loadPageList () {
const pageList = []

// load english pages
const englishPath = path.join(__dirname, '..', languages.en.dir, 'content')
Expand All @@ -15,7 +15,8 @@ module.exports = async function () {
!relativePath.includes('README')
})
.map(fileData => new Page({ ...fileData, languageCode: languages.en.code }))
pages.push(...englishPages)

pageList.push(...englishPages)

// load matching pages in other languages
for (const page of englishPages) {
Expand All @@ -29,20 +30,40 @@ module.exports = async function () {
} catch (_) {
continue
}
pages.push(new Page({

pageList.push(new Page({
relativePath: page.relativePath,
basePath,
languageCode: language.code
}))
}
}

// add named keys to the array for fast object-like reference
for (const page of pages) {
page.permalinks.forEach(permalink => {
pages[permalink.href] = page
})
}
return pageList
}

function createMapFromArray (pageList) {
// add keys to the object for each of the page's permalinks for fast lookup
const pageMap =
pageList.reduce(
(pageMap, page) => {
for (const permalink of page.permalinks) {
pageMap[permalink.href] = page
}
return pageMap
},
{}
)

return pageMap
}

async function loadPageMap (pageList) {
const pages = pageList || await loadPageList()
return createMapFromArray(pages)
}

return Promise.resolve(pages)
module.exports = {
loadPages: loadPageList,
loadPageMap
}
4 changes: 2 additions & 2 deletions lib/redirects/get-docs-path-from-developer-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { getVersionedPathWithLanguage } = require('../path-utils')

// This function takes a known pre-migration path from developer.github.com and
// infers and returns a current, correct docs.github.com path.
module.exports = function getDocsPathFromDeveloperPath (oldDeveloperPath, allRedirects, pages) {
module.exports = function getDocsPathFromDeveloperPath (oldDeveloperPath, allRedirects, pageMap) {
let newPath = oldDeveloperPath

// Look up the old path in the redirects object BEFORE modifying /v3 and /v4 paths
Expand Down Expand Up @@ -59,7 +59,7 @@ module.exports = function getDocsPathFromDeveloperPath (oldDeveloperPath, allRed

// show an error if the page to be redirected to doesn't exist
// but only if the current collection of pages includes REST and GraphQL reference pages
if (process.env.NODE_ENV !== 'test' && !pages[newPath] && pages['/en/rest/reference'] && pages['/en/graphql/reference']) {
if (process.env.NODE_ENV !== 'test' && !pageMap[newPath] && pageMap['/en/rest/reference'] && pageMap['/en/graphql/reference']) {
console.error(`can't find ${newPath}! based on ${oldDeveloperPath}`)
}

Expand Down
6 changes: 3 additions & 3 deletions lib/redirects/precompile.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ const nonEnterpriseDefaultVersion = require('../non-enterprise-default-version')

// This function runs at server warmup and precompiles possible redirect routes.
// It outputs them in key-value pairs within a neat Javascript object: { oldPath: newPath }
module.exports = async function precompileRedirects (pages) {
module.exports = async function precompileRedirects (pageList, pageMap) {
const allRedirects = {}

// 1. CURRENT PAGES PERMALINKS AND FRONTMATTER
// These redirects are created in lib/page.js via lib/redirects/permalinks.js
pages.forEach(page => Object.assign(allRedirects, page.redirects))
pageList.forEach(page => Object.assign(allRedirects, page.redirects))

// 2. REDIRECTS FOR ARCHIVED ROUTES FROM 2.13 TO 2.17
// Starting with 2.18, we updated the archival script to create stubbed HTML redirect files,
Expand All @@ -41,7 +41,7 @@ module.exports = async function precompileRedirects (pages) {
// Note that the list only includes supported enterprise paths up to 2.21, which is
// the last version that was available on developer.github.com before the migration.
DEVELOPER_ROUTES.forEach(developerRoute => {
const newPath = getDocsPathFromDevPath(developerRoute, allRedirects, pages)
const newPath = getDocsPathFromDevPath(developerRoute, allRedirects, pageMap)

// add the redirect to the object
allRedirects[developerRoute] = newPath
Expand Down
24 changes: 12 additions & 12 deletions lib/site-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const allVersions = Object.keys(require('./all-versions'))
// or if a category has direct child articles:
// siteTree[languageCode][version].products[productHref].categories[categoryHref].articles

module.exports = async function buildSiteTree (pages, site, redirects) {
module.exports = async function buildSiteTree (pageMap, site, redirects) {
const siteTree = {}

languageCodes.forEach(languageCode => {
Expand All @@ -38,12 +38,12 @@ module.exports = async function buildSiteTree (pages, site, redirects) {
product.href = item.href

// find the product TOC page and get TOC items
const page = findPageInVersion(item.href, pages, redirects, languageCode, version)
const page = findPageInVersion(item.href, pageMap, redirects, languageCode, version)

// skip if page can't be found in this version
if (!page) return

product.categories = buildCategoriesTree(page.tocItems, item.href, pages, redirects, version, languageCode)
product.categories = buildCategoriesTree(page.tocItems, item.href, pageMap, redirects, version, languageCode)

productTree[item.id] = product
return null
Expand All @@ -58,7 +58,7 @@ module.exports = async function buildSiteTree (pages, site, redirects) {
return siteTree
}

function buildCategoriesTree (tocItems, productHref, pages, redirects, version, languageCode) {
function buildCategoriesTree (tocItems, productHref, pageMap, redirects, version, languageCode) {
const categoryTree = {}

// for every category in a product TOC...
Expand All @@ -71,7 +71,7 @@ function buildCategoriesTree (tocItems, productHref, pages, redirects, version,
category.href = versionedCategoryHref

// find the category TOC page and get its TOC items
const page = findPageInVersion(categoryHref, pages, redirects, languageCode, version)
const page = findPageInVersion(categoryHref, pageMap, redirects, languageCode, version)

// skip if page can't be found in this version
if (!page) return
Expand All @@ -90,9 +90,9 @@ function buildCategoriesTree (tocItems, productHref, pages, redirects, version,
// if TOC contains maptopics, build a maptopics tree
// otherwise build an articles tree
if (hasMaptopics) {
category.maptopics = buildMaptopicsTree(page.tocItems, categoryHref, pages, redirects, version, languageCode)
category.maptopics = buildMaptopicsTree(page.tocItems, categoryHref, pageMap, redirects, version, languageCode)
} else {
category.articles = buildArticlesTree(page.tocItems, categoryHref, pages, redirects, version, languageCode)
category.articles = buildArticlesTree(page.tocItems, categoryHref, pageMap, redirects, version, languageCode)
}
}

Expand All @@ -102,7 +102,7 @@ function buildCategoriesTree (tocItems, productHref, pages, redirects, version,
return categoryTree
}

function buildMaptopicsTree (tocItems, categoryHref, pages, redirects, version, languageCode) {
function buildMaptopicsTree (tocItems, categoryHref, pageMap, redirects, version, languageCode) {
const maptopicTree = {}

// for every maptopic in a category TOC...
Expand All @@ -118,7 +118,7 @@ function buildMaptopicsTree (tocItems, categoryHref, pages, redirects, version,

// we already have access to the child articles via the category TOC items
// but we still need the page to get the available versions
const page = findPageInVersion(maptopicHref, pages, redirects, languageCode, version)
const page = findPageInVersion(maptopicHref, pageMap, redirects, languageCode, version)

// skip if page can't be found in this version
if (!page) return
Expand All @@ -135,14 +135,14 @@ function buildMaptopicsTree (tocItems, categoryHref, pages, redirects, version,
// make the child articles accessible to the page object for maptopic rendering
if (!page.childArticles) page.childArticles = childArticles

maptopic.articles = buildArticlesTree(childArticles, categoryHref, pages, redirects, version, languageCode)
maptopic.articles = buildArticlesTree(childArticles, categoryHref, pageMap, redirects, version, languageCode)
maptopicTree[versionedMaptopicHref] = maptopic
})

return maptopicTree
}

function buildArticlesTree (tocItems, categoryHref, pages, redirects, version, languageCode) {
function buildArticlesTree (tocItems, categoryHref, pageMap, redirects, version, languageCode) {
const articleTree = {}

// REST categories may not have TOC items
Expand All @@ -157,7 +157,7 @@ function buildArticlesTree (tocItems, categoryHref, pages, redirects, version, l
const versionedArticleHref = getVersionedPathWithoutLanguage(articleHref, version)
article.href = versionedArticleHref

const page = findPageInVersion(articleHref, pages, redirects, languageCode, version)
const page = findPageInVersion(articleHref, pageMap, redirects, languageCode, version)

// skip if page can't be found in this version
if (!page) return
Expand Down
24 changes: 15 additions & 9 deletions lib/warm-server.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
const statsd = require('./statsd')
const loadPages = require('./pages')
const { loadPages, loadPageMap } = require('./pages')
const loadRedirects = require('./redirects/precompile')
const loadSiteData = require('./site-data')
const loadSiteTree = require('./site-tree')

// For local caching
let pages, site, redirects, siteTree
let pageList, pageMap, site, redirects, siteTree

function isFullyWarmed () {
return Boolean(pages && site && redirects && siteTree)
// NOTE: Yes, `pageList` is specifically excluded here as it is transient data
const fullyWarmed = !!(pageMap && site && redirects && siteTree)
return fullyWarmed
}

function getWarmedCache () {
return {
pages,
pages: pageMap,
site,
redirects,
siteTree
Expand All @@ -27,20 +29,24 @@ async function warmServer () {
console.log('Priming context information...')
}

if (!pages || !site) {
if (!pageList || !site) {
// Promise.all is used to load multiple things in parallel
[pages, site] = await Promise.all([
pages || loadPages(),
[pageList, site] = await Promise.all([
pageList || loadPages(),
site || loadSiteData()
])
}

if (!pageMap) {
pageMap = await loadPageMap(pageList)
}

if (!redirects) {
redirects = await loadRedirects(pages)
redirects = await loadRedirects(pageList, pageMap)
}

if (!siteTree) {
siteTree = await loadSiteTree(pages, site, redirects)
siteTree = await loadSiteTree(pageMap, site, redirects)
}

if (process.env.NODE_ENV !== 'test') {
Expand Down
5 changes: 3 additions & 2 deletions middleware/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ const featureFlags = Object.keys(require('../feature-flags'))
// Note that additional middleware in middleware/index.js adds to this context object
module.exports = async function contextualize (req, res, next) {
// Ensure that we load some data only once on first request
const { site, redirects, pages, siteTree } = await warmServer()
const { site, redirects, siteTree, pages: pageMap } = await warmServer()

req.context = {}

// make feature flag environment variables accessible in layouts
Expand All @@ -39,7 +40,7 @@ module.exports = async function contextualize (req, res, next) {
req.context.redirects = redirects
req.context.site = site[req.language].site
req.context.siteTree = siteTree
req.context.pages = pages
req.context.pages = pageMap

return next()
}
2 changes: 1 addition & 1 deletion script/check-s3-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const path = require('path')
const { chain, difference } = require('lodash')
const loadPages = require('../lib/pages')
const { loadPages } = require('../lib/pages')
const loadSiteData = require('../lib/site-data')
const renderContent = require('../lib/render-content')
const allVersions = require('../lib/all-versions')
Expand Down
Loading

0 comments on commit fb30a07

Please sign in to comment.