From 988e68fa98313a0fb3289c4cf50a95b01e85db70 Mon Sep 17 00:00:00 2001 From: Peter Bengtsson Date: Thu, 17 Nov 2022 14:08:49 +0100 Subject: [PATCH 1/2] JIT data (#32140) --- components/context/MainContext.tsx | 23 +++++- .../get-started/quickstart/github-glossary.md | 2 +- lib/get-applicable-versions.js | 11 +-- lib/get-data.js | 6 +- lib/get-english-headings.js | 4 +- lib/liquid-tags/data.js | 14 +--- lib/liquid-tags/indented-data-reference.js | 19 +---- lib/page-data.js | 12 ++- lib/process-learning-tracks.js | 18 ++++- lib/warm-server.js | 6 +- middleware/context.js | 10 ++- middleware/contextualizers/features.js | 54 +++++++++----- .../contextualizers/ghae-release-notes.js | 4 +- .../contextualizers/ghes-release-notes.js | 3 +- middleware/contextualizers/glossaries.js | 4 +- .../contextualizers/product-examples.js | 42 +++++++++-- middleware/index.js | 2 +- middleware/learning-track.js | 9 ++- nodemon.json | 7 +- tests/content/category-pages.js | 7 +- tests/content/glossary.js | 7 +- .../liquid-tags/good-data-variable.md | 2 +- tests/unit/liquid-helpers.js | 73 ++++--------------- tests/unit/liquid-tags/data.js | 53 +++++++++----- 24 files changed, 215 insertions(+), 177 deletions(-) diff --git a/components/context/MainContext.tsx b/components/context/MainContext.tsx index 44e803927f..8c072327f5 100644 --- a/components/context/MainContext.tsx +++ b/components/context/MainContext.tsx @@ -40,7 +40,7 @@ type DataT = { version_was_deprecated: string version_will_be_deprecated: string deprecation_details: string - isOldestReleaseDeprecated: boolean + isOldestReleaseDeprecated?: boolean } policies: { translation: string @@ -130,12 +130,27 @@ export const getMainContext = async (req: any, res: any): Promise error: req.context.error ? req.context.error.toString() : '', data: { ui: req.context.site.data.ui, + reusables: { - enterprise_deprecation: req.context.site.data.reusables.enterprise_deprecation, - policies: req.context.site.data.reusables.policies, + enterprise_deprecation: { + version_was_deprecated: req.context.getDottedData( + 'reusables.enterprise_deprecation.version_was_deprecated' + ), + version_will_be_deprecated: req.context.getDottedData( + 'reusables.enterprise_deprecation.version_will_be_deprecated' + ), + deprecation_details: req.context.getDottedData( + 'reusables.enterprise_deprecation.deprecation_details' + ), + }, + policies: { + translation: req.context.getDottedData('reusables.policies.translation'), + }, }, variables: { - release_candidate: req.context.site.data.variables.release_candidate, + release_candidate: { + version: req.context.getDottedData('variables.release_candidate.version') || null, + }, }, }, currentCategory: req.context.currentCategory || '', diff --git a/content/get-started/quickstart/github-glossary.md b/content/get-started/quickstart/github-glossary.md index 2b36e09b0e..18a85238e5 100644 --- a/content/get-started/quickstart/github-glossary.md +++ b/content/get-started/quickstart/github-glossary.md @@ -13,7 +13,7 @@ versions: --- {% for glossary in glossaries %} ### {{ glossary.term }} - {{ glossary.description}} + {{ glossary.description }} --- {% endfor %} diff --git a/lib/get-applicable-versions.js b/lib/get-applicable-versions.js index b12243202d..7f454d88ec 100644 --- a/lib/get-applicable-versions.js +++ b/lib/get-applicable-versions.js @@ -1,12 +1,8 @@ -import path from 'path' import { reduce, sortBy } from 'lodash-es' import { allVersions } from './all-versions.js' import versionSatisfiesRange from './version-satisfies-range.js' import checkIfNextVersionOnly from './check-if-next-version-only.js' -import dataDirectory from './data-directory.js' -import encodeBracketedParentheses from './encode-bracketed-parentheses.js' - -const featuresDir = path.join('data', 'features') +import { getDeepDataByLanguage } from './get-data.js' let featureData = null @@ -24,10 +20,7 @@ function getApplicableVersions(frontmatterVersions, filepath) { } if (!featureData) { - featureData = dataDirectory(featuresDir, { - preprocess: (dataString) => encodeBracketedParentheses(dataString.trimEnd()), - ignorePatterns: [/README\.md$/], - }) + featureData = getDeepDataByLanguage('features', 'en') } // Check for frontmatter that includes a feature name, like: diff --git a/lib/get-data.js b/lib/get-data.js index 92f8a4cfa3..219759c1c2 100644 --- a/lib/get-data.js +++ b/lib/get-data.js @@ -7,6 +7,10 @@ import { merge, get } from 'lodash-es' import languages from './languages.js' +// If you run `export DEBUG_JIT_DATA_READS=true` in your terminal, +// next time it will mention every file it reads from disk. +const DEBUG_JIT_DATA_READS = Boolean(JSON.parse(process.env.DEBUG_JIT_DATA_READS || 'false')) + // This is a list of files that we should always immediately fall back to // English for. // Having this is safer than trying to wrangle the translations to NOT @@ -237,7 +241,7 @@ const getMarkdownContent = memoize((root, relPath, englishRoot) => { const getFileContent = (root, relPath, englishRoot) => { const filePath = root ? path.join(root, relPath) : relPath - if (process.env.NODE_ENV === 'development') console.log('READ', filePath) + if (DEBUG_JIT_DATA_READS) console.log('READ', filePath) try { return fs.readFileSync(filePath, 'utf-8') } catch (err) { diff --git a/lib/get-english-headings.js b/lib/get-english-headings.js index 8eca64219f..9f201a1084 100644 --- a/lib/get-english-headings.js +++ b/lib/get-english-headings.js @@ -1,7 +1,9 @@ import { fromMarkdown } from 'mdast-util-from-markdown' import { toString } from 'mdast-util-to-string' import { visit } from 'unist-util-visit' + import findPage from './find-page.js' +import { getDataByLanguage } from './get-data.js' // for any translated page, first get corresponding English markdown // then get the headings on both the translated and English pageMap @@ -11,7 +13,7 @@ export default function getEnglishHeadings(page, context) { // generated programatically. if (page.relativePath.endsWith('/github-glossary.md')) { // Return an object of `{ localized-term: english-slug }` - const languageGlossary = context.site.data.glossaries.external + const languageGlossary = getDataByLanguage('glossaries.external', 'en') return languageGlossary.reduce((prev, curr) => { prev[curr.term] = curr.slug return prev diff --git a/lib/liquid-tags/data.js b/lib/liquid-tags/data.js index 03251661dd..b6e3dfbcaa 100644 --- a/lib/liquid-tags/data.js +++ b/lib/liquid-tags/data.js @@ -1,7 +1,7 @@ import { TokenizationError } from 'liquidjs' -import matter from 'gray-matter' import { THROW_ON_EMPTY, DataReferenceError } from './error-handling.js' +import { getDataByLanguage } from '../get-data.js' const Syntax = /([a-z0-9/\\_.\-[\]]+)/i const SyntaxHelp = "Syntax Error in 'data' - Valid syntax: data [path]" @@ -16,8 +16,8 @@ export default { }, async render(scope) { - let value = await this.liquid.evalValue(`site.data.${this.path}`, scope) - if (typeof value !== 'string') { + const text = getDataByLanguage(this.path, scope.environments.currentLanguage) + if (text === undefined) { const message = `Can't find the key 'site.data.${this.path}' in the scope.` if (THROW_ON_EMPTY) { throw new DataReferenceError(message) @@ -26,12 +26,6 @@ export default { return } - // Drop any frontmatter since reusable Markdown files aren't currently - // expected to use frontmatter. This is useful since our current translation - // process adds YAML frontmatter properties to any Markdown file that gets - // translated and we don't want to render that information. - ;({ content: value } = matter(value)) - - return this.liquid.parseAndRender(value, scope.environments) + return this.liquid.parseAndRender(text, scope.environments) }, } diff --git a/lib/liquid-tags/indented-data-reference.js b/lib/liquid-tags/indented-data-reference.js index f5310e5a81..cb38db3ffe 100644 --- a/lib/liquid-tags/indented-data-reference.js +++ b/lib/liquid-tags/indented-data-reference.js @@ -1,6 +1,7 @@ import assert from 'assert' import { THROW_ON_EMPTY, IndentedDataReferenceError } from './error-handling.js' +import { getDataByLanguage } from '../get-data.js' // This class supports a tag that expects two parameters, a data reference and `spaces=NUMBER`: // @@ -33,20 +34,8 @@ export default { assert(parseInt(numSpaces) || numSpaces === '0', '"spaces=NUMBER" must include a number') // Get the referenced value from the context - const value = await this.liquid.evalValue(`site.data.${dataReference}`, scope) - - // If value is falsy it can be because we completely failed to look - // it up. But it can also be literally an empty string. - // For example, the reusable could be entirely something like this: - // - // {% if some condition %}The meat{% endif %} - // - // Then it's working as expected. But if the reference is wrong, e.g. - // - // {% indented_data_reference reusables.foo.tyypu spaces=3 %} - // - // Or if the file simple doesn't exist, you get an undefined for the value. - if (typeof value !== 'string' && !value) { + const text = getDataByLanguage(dataReference, scope.environments.currentLanguage) + if (text === undefined) { const message = `Can't find the key 'site.data.${dataReference}' in the scope.` if (THROW_ON_EMPTY) { throw new IndentedDataReferenceError(message) @@ -56,7 +45,7 @@ export default { } // add spaces to each line - const renderedReferenceWithIndent = value.replace(/^/gm, ' '.repeat(numSpaces)) + const renderedReferenceWithIndent = text.replace(/^/gm, ' '.repeat(numSpaces)) return this.liquid.parseAndRender(renderedReferenceWithIndent, scope.environments) }, diff --git a/lib/page-data.js b/lib/page-data.js index 0498fca7d7..ae2443e6d3 100644 --- a/lib/page-data.js +++ b/lib/page-data.js @@ -1,8 +1,8 @@ import path from 'path' + import languages from './languages.js' import { allVersions } from './all-versions.js' import createTree, { getBasePath } from './create-tree.js' -import loadSiteData from './site-data.js' import nonEnterpriseDefaultVersion from './non-enterprise-default-version.js' import Page from './page.js' @@ -60,8 +60,7 @@ export async function loadUnversionedTree(languagesOnly = null) { * * Order of languages and versions doesn't matter, but order of child page arrays DOES matter (for navigation). */ -export async function loadSiteTree(unversionedTree, siteData) { - const site = siteData || loadSiteData() +export async function loadSiteTree(unversionedTree) { const rawTree = Object.assign({}, unversionedTree || (await loadUnversionedTree())) const siteTree = {} @@ -76,8 +75,7 @@ export async function loadSiteTree(unversionedTree, siteData) { treePerVersion[version] = await versionPages( Object.assign({}, rawTree[langCode]), version, - langCode, - site + langCode ) }) ) @@ -89,7 +87,7 @@ export async function loadSiteTree(unversionedTree, siteData) { return siteTree } -export async function versionPages(obj, version, langCode, site) { +export async function versionPages(obj, version, langCode) { // Add a versioned href as a convenience for use in layouts. obj.href = obj.page.permalinks.find( (pl) => @@ -103,7 +101,7 @@ export async function versionPages(obj, version, langCode, site) { // Drop child pages that do not apply to the current version .filter((childPage) => childPage.page.applicableVersions.includes(version)) // Version the child pages recursively. - .map((childPage) => versionPages(Object.assign({}, childPage), version, langCode, site)) + .map((childPage) => versionPages(Object.assign({}, childPage), version, langCode)) ) obj.childPages = [...versionedChildPages] diff --git a/lib/process-learning-tracks.js b/lib/process-learning-tracks.js index 98f5d5fc5b..bbef38c608 100644 --- a/lib/process-learning-tracks.js +++ b/lib/process-learning-tracks.js @@ -1,6 +1,7 @@ import renderContent from './render-content/index.js' import getLinkData from './get-link-data.js' import getApplicableVersions from './get-applicable-versions.js' +import { getDataByLanguage } from './get-data.js' const renderOpts = { textOnly: true, encodeEntities: true } @@ -23,11 +24,20 @@ export default async function processLearningTracks(rawLearningTracks, context) if (!renderedTrackName) continue // Find the data for the current product and track name. - const trackDataForProduct = context.site.data['learning-tracks'][context.currentProduct] - if (!trackDataForProduct) { - throw new Error(`No learning track data for product "${context.currentProduct}".`) + + if (context.currentProduct.includes('.')) { + throw new Error(`currentProduct can not contain a . (${context.currentProduct})`) } - const track = trackDataForProduct[renderedTrackName] + if (renderedTrackName.includes('.')) { + throw new Error(`renderedTrackName can not contain a . (${renderedTrackName})`) + } + + // Note: this will use the translated learning tracks and automatically + // fall back to English if they don't exist on disk in the translation. + const track = getDataByLanguage( + `learning-tracks.${context.currentProduct}.${renderedTrackName}`, + context.currentLanguage + ) if (!track) { throw new Error(`No learning track called '${renderedTrackName}'.`) } diff --git a/lib/warm-server.js b/lib/warm-server.js index 18b0a50a49..808889c784 100644 --- a/lib/warm-server.js +++ b/lib/warm-server.js @@ -1,7 +1,6 @@ import statsd from './statsd.js' import { loadUnversionedTree, loadSiteTree, loadPages, loadPageMap } from './page-data.js' import loadRedirects from './redirects/precompile.js' -import loadSiteData from './site-data.js' // Instrument these functions so that // it's wrapped in a timer that reports to Datadog @@ -11,7 +10,6 @@ const dog = { loadPages: statsd.asyncTimer(loadPages, 'load_pages'), loadPageMap: statsd.asyncTimer(loadPageMap, 'load_page_map'), loadRedirects: statsd.asyncTimer(loadRedirects, 'load_redirects'), - loadSiteData: statsd.timer(loadSiteData, 'load_site_data'), } // For multiple-triggered Promise sharing @@ -25,8 +23,7 @@ async function warmServer() { } const unversionedTree = await dog.loadUnversionedTree() - const site = dog.loadSiteData() - const siteTree = await dog.loadSiteTree(unversionedTree, site) + const siteTree = await dog.loadSiteTree(unversionedTree) const pageList = await dog.loadPages(unversionedTree) const pageMap = await dog.loadPageMap(pageList) const redirects = await dog.loadRedirects(pageList) @@ -43,7 +40,6 @@ async function warmServer() { return { pages: pageMap, - site, redirects, unversionedTree, siteTree, diff --git a/middleware/context.js b/middleware/context.js index 6c892a780d..8d5c88134a 100644 --- a/middleware/context.js +++ b/middleware/context.js @@ -7,6 +7,7 @@ import productNames from '../lib/product-names.js' import warmServer from '../lib/warm-server.js' import searchVersions from '../lib/search/versions.js' import nonEnterpriseDefaultVersion from '../lib/non-enterprise-default-version.js' +import { getDataByLanguage, getUIDataMerged } from '../lib/get-data.js' const activeProducts = Object.values(productMap).filter( (product) => !product.wip && !product.hidden ) @@ -26,7 +27,7 @@ const enterpriseServerVersions = Object.keys(allVersions).filter((version) => // Note that additional middleware in middleware/index.js adds to this context object export default async function contextualize(req, res, next) { // Ensure that we load some data only once on first request - const { site, redirects, siteTree, pages: pageMap } = await warmServer() + const { redirects, siteTree, pages: pageMap } = await warmServer() req.context = {} req.context.process = { env: {} } @@ -49,7 +50,12 @@ export default async function contextualize(req, res, next) { req.context.enterpriseServerReleases = enterpriseServerReleases req.context.enterpriseServerVersions = enterpriseServerVersions req.context.redirects = redirects - req.context.site = site[req.language].site + req.context.site = { + data: { + ui: getUIDataMerged(req.language), + }, + } + req.context.getDottedData = (dottedPath) => getDataByLanguage(dottedPath, req.language) req.context.siteTree = siteTree req.context.pages = pageMap req.context.searchVersions = searchVersions diff --git a/middleware/contextualizers/features.js b/middleware/contextualizers/features.js index 731a7cb5e2..0de51a8781 100644 --- a/middleware/contextualizers/features.js +++ b/middleware/contextualizers/features.js @@ -1,25 +1,43 @@ -import warmServer from '../../lib/warm-server.js' import getApplicableVersions from '../../lib/get-applicable-versions.js' +import { getDeepDataByLanguage } from '../../lib/get-data.js' -export default async function features(req, res, next) { +export default function features(req, res, next) { if (!req.context.page) return next() - const { site } = await warmServer() - - // Determine whether the currentVersion belongs to the list of versions the feature is available in. - // Note that we always exclusively use the English `data.features` because - // we don't want any of these translated (and possibly corrupt). - Object.keys(site.en.site.data.features).forEach((featureName) => { - const { versions } = site.en.site.data.features[featureName] - const applicableVersions = getApplicableVersions(versions, `data/features/${featureName}.yml`) - - // Adding the resulting boolean to the context object gives us the ability to use - // `{% if featureName ... %}` conditionals in content files. - const isFeatureAvailableInCurrentVersion = applicableVersions.includes( - req.context.currentVersion - ) - req.context[featureName] = isFeatureAvailableInCurrentVersion - }) + Object.entries(getFeaturesByVersion(req.context.currentVersion)).forEach( + ([featureName, isFeatureAvailableInCurrentVersion]) => { + req.context[featureName] = isFeatureAvailableInCurrentVersion + } + ) return next() } + +let allFeatures + +const cache = new Map() +function getFeaturesByVersion(currentVersion) { + if (!cache.has(currentVersion)) { + if (!allFeatures) { + // As of Oct 2022, the `data/features/**` reading is *not* JIT. + // The `data/features` is deliberately not ignored in nodemon.json. + // See internal issue #2389 + allFeatures = getDeepDataByLanguage('features', 'en') + } + + const features = {} + // Determine whether the currentVersion belongs to the list of versions the feature is available in. + for (const [featureName, feature] of Object.entries(allFeatures)) { + const { versions } = feature + const applicableVersions = getApplicableVersions(versions, `data/features/${featureName}.yml`) + + // Adding the resulting boolean to the context object gives us the ability to use + // `{% if featureName ... %}` conditionals in content files. + const isFeatureAvailableInCurrentVersion = applicableVersions.includes(currentVersion) + features[featureName] = isFeatureAvailableInCurrentVersion + } + cache.set(currentVersion, features) + } + + return cache.get(currentVersion) +} diff --git a/middleware/contextualizers/ghae-release-notes.js b/middleware/contextualizers/ghae-release-notes.js index 25ea905070..e832000796 100644 --- a/middleware/contextualizers/ghae-release-notes.js +++ b/middleware/contextualizers/ghae-release-notes.js @@ -1,4 +1,5 @@ import { formatReleases, renderPatchNotes } from '../../lib/release-notes-utils.js' +import { getDeepDataByLanguage } from '../../lib/get-data.js' import { allVersions } from '../../lib/all-versions.js' export default async function ghaeReleaseNotesContext(req, res, next) { @@ -9,7 +10,8 @@ export default async function ghaeReleaseNotesContext(req, res, next) { ) return next() - const ghaeReleaseNotes = req.context.site.data['release-notes']['github-ae'] + const ghaeReleaseNotes = getDeepDataByLanguage('release-notes.github-ae', req.language) + // internalLatestRelease is set in lib/all-versions, e.g., '3.5' but UI still displays '@latest'. let requestedRelease = req.context.currentVersionObj.internalLatestRelease diff --git a/middleware/contextualizers/ghes-release-notes.js b/middleware/contextualizers/ghes-release-notes.js index 3e77407e10..3432f4f740 100644 --- a/middleware/contextualizers/ghes-release-notes.js +++ b/middleware/contextualizers/ghes-release-notes.js @@ -1,12 +1,13 @@ import { formatReleases, renderPatchNotes } from '../../lib/release-notes-utils.js' import { all } from '../../lib/enterprise-server-releases.js' +import { getDeepDataByLanguage } from '../../lib/get-data.js' export default async function ghesReleaseNotesContext(req, res, next) { if (!(req.pagePath.endsWith('/release-notes') || req.pagePath.endsWith('/admin'))) return next() const [requestedPlan, requestedRelease] = req.context.currentVersion.split('@') if (requestedPlan !== 'enterprise-server') return next() - const ghesReleaseNotes = req.context.site.data['release-notes']['enterprise-server'] + const ghesReleaseNotes = getDeepDataByLanguage('release-notes.enterprise-server', req.language) // If the requested GHES release isn't found in data/release-notes/enterprise-server/*, // and it IS a valid GHES release, try being helpful and redirecting to the old location. diff --git a/middleware/contextualizers/glossaries.js b/middleware/contextualizers/glossaries.js index b5fbed7924..abf3470f9d 100644 --- a/middleware/contextualizers/glossaries.js +++ b/middleware/contextualizers/glossaries.js @@ -1,7 +1,9 @@ +import { getDataByLanguage } from '../../lib/get-data.js' + export default function glossaries(req, res, next) { if (!req.pagePath.endsWith('get-started/quickstart/github-glossary')) return next() - const glossaries = req.context.site.data.glossaries.external + const glossaries = getDataByLanguage('glossaries.external', req.context.currentLanguage) req.context.glossaries = glossaries.sort((a, b) => a.term.localeCompare(b.term)) return next() diff --git a/middleware/contextualizers/product-examples.js b/middleware/contextualizers/product-examples.js index 765b6123b6..c653c694a6 100644 --- a/middleware/contextualizers/product-examples.js +++ b/middleware/contextualizers/product-examples.js @@ -1,20 +1,50 @@ import getApplicableVersions from '../../lib/get-applicable-versions.js' +import { getDataByLanguage } from '../../lib/get-data.js' + +function getProductExampleData(product, key, language) { + // Because getDataByLanguage() depends on reading data files from + // disk, asking for something that doesn't exist would throw a + // `ENOENT` error from `fs.readFile` but we want that to default + // to `undefined` because certain product's don't have all product + // examples. + try { + return getDataByLanguage(`product-examples.${product}.${key}`, language) + } catch (error) { + if (error.code === 'ENOENT') return + throw error + } +} export default async function productExamples(req, res, next) { if (!req.context.page) return next() if (req.context.currentLayoutName !== 'product-landing') return next() - const productExamples = req.context.site.data['product-examples'][req.context.currentProduct] - if (!productExamples) return next() + const { currentProduct, currentLanguage } = req.context + if (currentProduct.includes('.')) + throw new Error(`currentProduct can not contain a . (${currentProduct})`) - req.context.productCommunityExamples = productExamples['community-examples'] - req.context.productUserExamples = productExamples['user-examples'] + req.context.productCommunityExamples = getProductExampleData( + currentProduct, + 'community-examples', + currentLanguage + ) + req.context.productUserExamples = getProductExampleData( + currentProduct, + 'user-examples', + currentLanguage + ) + + const productCodeExamples = getProductExampleData( + currentProduct, + 'code-examples', + currentLanguage + ) // We currently only support versioning in code examples. // TODO support versioning across all example types. req.context.productCodeExamples = - productExamples['code-examples'] && - productExamples['code-examples'].filter((example) => { + productCodeExamples && + productCodeExamples.filter((example) => { // If an example block does NOT contain the versions prop, assume it's available in all versions return ( !example.versions || diff --git a/middleware/index.js b/middleware/index.js index bee098fc2d..5320507d77 100644 --- a/middleware/index.js +++ b/middleware/index.js @@ -269,7 +269,7 @@ export default function (app) { app.use(asyncMiddleware(instrument(currentProductTree, './contextualizers/current-product-tree'))) app.use(asyncMiddleware(instrument(genericToc, './contextualizers/generic-toc'))) app.use(instrument(breadcrumbs, './contextualizers/breadcrumbs')) - app.use(asyncMiddleware(instrument(features, './contextualizers/features'))) + app.use(instrument(features, './contextualizers/features')) app.use(asyncMiddleware(instrument(productExamples, './contextualizers/product-examples'))) app.use(asyncMiddleware(instrument(productGroups, './contextualizers/product-groups'))) app.use(instrument(glossaries, './contextualizers/glossaries')) diff --git a/middleware/learning-track.js b/middleware/learning-track.js index 1b8b42e77d..11e058a08e 100644 --- a/middleware/learning-track.js +++ b/middleware/learning-track.js @@ -1,6 +1,7 @@ import { getPathWithoutLanguage, getPathWithoutVersion } from '../lib/path-utils.js' import getLinkData from '../lib/get-link-data.js' import renderContent from '../lib/render-content/renderContent.js' +import { getDeepDataByLanguage } from '../lib/get-data.js' export default async function learningTrack(req, res, next) { const noTrack = () => { @@ -14,7 +15,8 @@ export default async function learningTrack(req, res, next) { if (!trackName) return noTrack() let trackProduct = req.context.currentProduct - let tracksPerProduct = req.context.site.data['learning-tracks'][trackProduct] + const allLearningTracks = getDeepDataByLanguage('learning-tracks', req.language) + let tracksPerProduct = allLearningTracks[trackProduct] // If there are no learning tracks for the current product, try and fall // back to the learning track product set as a URL parameter. This handles @@ -22,12 +24,11 @@ export default async function learningTrack(req, res, next) { // than the current learning track product. if (!tracksPerProduct) { trackProduct = req.query.learnProduct - tracksPerProduct = req.context.site.data['learning-tracks'][trackProduct] + tracksPerProduct = allLearningTracks[trackProduct] } - if (!tracksPerProduct) return noTrack() - const track = req.context.site.data['learning-tracks'][trackProduct][trackName] + const track = allLearningTracks[trackProduct][trackName] if (!track) return noTrack() const currentLearningTrack = { trackName, trackProduct } diff --git a/nodemon.json b/nodemon.json index e5c16a7537..03f870fc3b 100644 --- a/nodemon.json +++ b/nodemon.json @@ -6,6 +6,11 @@ "translations", "stylesheets", "tests", - "content" + "content", + "data/reusables", + "data/variables", + "data/glossaries", + "data/product-examples", + "data/learning-tracks" ] } diff --git a/tests/content/category-pages.js b/tests/content/category-pages.js index b607856fb6..810ecdcce6 100644 --- a/tests/content/category-pages.js +++ b/tests/content/category-pages.js @@ -6,7 +6,6 @@ import matter from '../../lib/read-frontmatter.js' import { zip, difference } from 'lodash-es' import GithubSlugger from 'github-slugger' import { decode } from 'html-entities' -import loadSiteData from '../../lib/site-data.js' import renderContent from '../../lib/render-content/index.js' import getApplicableVersions from '../../lib/get-applicable-versions.js' const __dirname = path.dirname(fileURLToPath(import.meta.url)) @@ -16,8 +15,6 @@ const slugger = new GithubSlugger() const contentDir = path.join(__dirname, '../../content') describe('category pages', () => { - const siteData = loadSiteData().en.site - const walkOptions = { globs: ['*/index.md', 'enterprise/*/index.md'], ignore: [ @@ -100,7 +97,9 @@ describe('category pages', () => { }) // Save the index title for later testing - indexTitle = await renderContent(data.title, { site: siteData }, { textOnly: true }) + indexTitle = data.title.includes('{') + ? await renderContent(data.title, { currentLanguage: 'en' }, { textOnly: true }) + : data.title publishedArticlePaths = ( await Promise.all( diff --git a/tests/content/glossary.js b/tests/content/glossary.js index 38d3459737..1443832292 100644 --- a/tests/content/glossary.js +++ b/tests/content/glossary.js @@ -1,7 +1,10 @@ -import loadSiteData from '../../lib/site-data.js' +import { getDataByLanguage } from '../../lib/get-data' describe('glossaries', () => { - const glossaries = loadSiteData().en.site.data.glossaries + const glossaries = { + external: getDataByLanguage('glossaries.external', 'en'), + candidates: getDataByLanguage('glossaries.candidates', 'en'), + } test('are broken into external, internal, and candidates', async () => { const keys = Object.keys(glossaries) diff --git a/tests/fixtures/liquid-tags/good-data-variable.md b/tests/fixtures/liquid-tags/good-data-variable.md index 0fc5275ec4..8b838798f1 100644 --- a/tests/fixtures/liquid-tags/good-data-variable.md +++ b/tests/fixtures/liquid-tags/good-data-variable.md @@ -3,4 +3,4 @@ title: Good sample page versions: '*' --- -{% data variables.foo %} +{% data variables.stuff.foo %} diff --git a/tests/unit/liquid-helpers.js b/tests/unit/liquid-helpers.js index 7589132112..bd54e3039c 100644 --- a/tests/unit/liquid-helpers.js +++ b/tests/unit/liquid-helpers.js @@ -1,33 +1,30 @@ -import { jest } from '@jest/globals' +import { afterAll, jest, beforeAll, expect } from '@jest/globals' import { liquid } from '../../lib/render-content/index.js' -import { loadPageMap } from '../../lib/page-data.js' -import nonEnterpriseDefaultVersion from '../../lib/non-enterprise-default-version.js' +import languages from '../../lib/languages.js' +import { DataDirectory } from '../helpers/data-directory.js' describe('liquid helper tags', () => { jest.setTimeout(60 * 1000) const context = {} - let pageMap - beforeAll(async () => { - pageMap = await loadPageMap() + let dd + const enDirBefore = languages.en.dir + + beforeAll(() => { context.currentLanguage = 'en' - context.currentVersion = nonEnterpriseDefaultVersion - context.pages = pageMap - context.redirects = { - '/en/desktop/contributing-and-collaborating-using-github-desktop': `/en/${nonEnterpriseDefaultVersion}/desktop/contributing-and-collaborating-using-github-desktop`, - '/en/desktop/contributing-and-collaborating-using-github-desktop/adding-and-cloning-repositories': `/en/${nonEnterpriseDefaultVersion}/desktop/contributing-and-collaborating-using-github-desktop/adding-and-cloning-repositories`, - '/en/github/writing-on-github/basic-writing-and-formatting-syntax': `/en/${nonEnterpriseDefaultVersion}/github/writing-on-github/basic-writing-and-formatting-syntax`, - } - context.site = { + dd = new DataDirectory({ data: { reusables: { example: 'a rose by any other name\nwould smell as sweet', }, }, - } - context.page = { - relativePath: 'desktop/index.md', - } + }) + languages.en.dir = dd.root + }) + + afterAll(() => { + dd.destroy() + languages.en.dir = enDirBefore }) describe('indented_data_reference tag', () => { @@ -63,44 +60,4 @@ would smell as sweet` expect(output).toBe(expected) }) }) - - describe('data tag', () => { - test('handles bracketed array access within for-in loop', async () => { - const template = ` -{% for term in site.data.glossaries.external %} -### {% data glossaries.external[forloop.index0].term %} -{% data glossaries.external[forloop.index0].description %} ---- -{% endfor %}` - - const localContext = { ...context } - localContext.site = { - data: { - variables: { - fire_emoji: ':fire:', - }, - glossaries: { - external: [ - { term: 'lit', description: 'Awesome things. {% data variables.fire_emoji %}' }, - { term: 'Zhu Li', description: '_"Zhu Li, do the thing!"_ :point_up:' }, - ], - }, - }, - } - - const expected = ` - -### lit -Awesome things. :fire: ---- - -### Zhu Li -_"Zhu Li, do the thing!"_ :point_up: ---- -` - - const output = await liquid.parseAndRender(template, localContext) - expect(output).toBe(expected) - }) - }) }) diff --git a/tests/unit/liquid-tags/data.js b/tests/unit/liquid-tags/data.js index c57ce5f08d..f19833ff57 100644 --- a/tests/unit/liquid-tags/data.js +++ b/tests/unit/liquid-tags/data.js @@ -1,12 +1,37 @@ -import { expect } from '@jest/globals' -import path from 'path' -import Page from '../../../lib/page.js' -import nonEnterpriseDefaultVersion from '../../../lib/non-enterprise-default-version.js' import { fileURLToPath } from 'url' +import path from 'path' + +import { afterAll, beforeAll, expect, describe, it } from '@jest/globals' + +import Page from '../../../lib/page.js' +import languages from '../../../lib/languages.js' +import nonEnterpriseDefaultVersion from '../../../lib/non-enterprise-default-version.js' +import { DataDirectory } from '../../helpers/data-directory.js' const __dirname = path.dirname(fileURLToPath(import.meta.url)) describe('data tag', () => { + let dd + const enDirBefore = languages.en.dir + + beforeAll(() => { + dd = new DataDirectory({ + data: { + variables: { + stuff: { + foo: 'Foo', + }, + }, + }, + }) + languages.en.dir = dd.root + }) + + afterAll(() => { + if (dd) dd.destroy() + languages.en.dir = enDirBefore + }) + it('should render fine if data is found', async () => { const page = await Page.init({ relativePath: 'liquid-tags/good-data-variable.md', @@ -18,22 +43,9 @@ describe('data tag', () => { currentLanguage: 'en', currentPath: '/en/liquid-tags/good-data-variable', } - const rendered = await page.render( - Object.assign( - { - site: { - data: { - variables: { - foo: 'Foo', - }, - }, - }, - }, - context - ) - ) + const rendered = await page.render(context) // The test fixture contains: - // {% data variables.foo %} + // {% data variables.stuff.foo %} // which we control the value of here in the test. expect(rendered.includes('Foo')).toBeTruthy() }) @@ -45,9 +57,10 @@ describe('data tag', () => { }) const context = { currentPath: '/en/liquid-tags/bad-data-variable', + currentLanguage: 'en', } await expect(page.render(context)).rejects.toThrow( - "Can't find the key 'site.data.foo.bar.tipu' in the scope., line:2, col:1" + "Can't find the key 'foo.bar.tipu' in the scope., line:2, col:1" ) }) }) From c95a5c9bdf3ae54ad77b074739b262e7d0e8a077 Mon Sep 17 00:00:00 2001 From: Peter Bengtsson Date: Thu, 17 Nov 2022 14:19:32 +0100 Subject: [PATCH 2/2] Treat "failing" external link checks as warning (#32456) --- .../actions/rendered-content-link-checker.js | 51 +++++++++++++++++-- .github/workflows/link-check-daily.yml | 3 ++ lib/excluded-links.js | 1 - script/rendered-content-link-checker.js | 1 + 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/.github/actions/rendered-content-link-checker.js b/.github/actions/rendered-content-link-checker.js index d897ab0ba8..49e1f9f21a 100755 --- a/.github/actions/rendered-content-link-checker.js +++ b/.github/actions/rendered-content-link-checker.js @@ -77,8 +77,15 @@ const deprecatedVersionPrefixesRegex = new RegExp( // When this file is invoked directly from action as opposed to being imported if (import.meta.url.endsWith(process.argv[1])) { // Optional env vars - const { ACTION_RUN_URL, LEVEL, FILES_CHANGED, REPORT_REPOSITORY, REPORT_AUTHOR, REPORT_LABEL } = - process.env + const { + ACTION_RUN_URL, + LEVEL, + FILES_CHANGED, + REPORT_REPOSITORY, + REPORT_AUTHOR, + REPORT_LABEL, + EXTERNAL_SERVER_ERRORS_AS_WARNINGS, + } = process.env const octokit = github() @@ -113,6 +120,7 @@ if (import.meta.url.endsWith(process.argv[1])) { reportLabel: REPORT_LABEL, reportAuthor: REPORT_AUTHOR, actionContext: getActionContext(), + externalServerErrorsAsWarning: EXTERNAL_SERVER_ERRORS_AS_WARNINGS, } if (opts.shouldComment || opts.createReport) { @@ -155,6 +163,7 @@ if (import.meta.url.endsWith(process.argv[1])) { * random {boolean} - Randomize page order for debugging when true * patient {boolean} - Wait longer and retry more times for rate-limited external URLS * bail {boolean} - Throw an error on the first page (not permalink) that has >0 flaws + * externalServerErrorsAsWarning {boolean} - Treat >=500 errors or temporary request errors as warning * */ async function main(core, octokit, uploadArtifact, opts = {}) { @@ -583,6 +592,7 @@ async function processPermalink(core, permalink, page, pageMap, redirects, opts, checkExternalLinks, verbose, patient, + externalServerErrorsAsWarning, } = opts const html = await renderInnerHTML(page, permalink) const $ = cheerio.load(html) @@ -613,6 +623,7 @@ async function processPermalink(core, permalink, page, pageMap, redirects, opts, pageMap, checkAnchors, checkExternalLinks, + externalServerErrorsAsWarning, { verbose, patient }, db ) @@ -758,6 +769,7 @@ async function checkHrefLink( pageMap, checkAnchors = false, checkExternalLinks = false, + externalServerErrorsAsWarning = false, { verbose = false, patient = false } = {}, db = null ) { @@ -815,11 +827,39 @@ async function checkHrefLink( } const { ok, ...info } = await checkExternalURLCached(core, href, { verbose, patient }, db) if (!ok) { - return { CRITICAL: `Broken external link (${JSON.stringify(info)})`, isExternal: true } + // By default, an not-OK problem with an external link is CRITICAL + // but if it was a `responseError` or the statusCode was >= 500 + // then downgrade it to WARNING. + let problem = 'CRITICAL' + if (externalServerErrorsAsWarning) { + if ( + (info.statusCode && info.statusCode >= 500) || + (info.requestError && isTemporaryRequestError(info.requestError)) + ) { + problem = 'WARNING' + } + } + return { [problem]: `Broken external link (${JSON.stringify(info)})`, isExternal: true } } } } +// Return true if the request error is sufficiently temporary. For example, +// a request to `https://exammmmple.org` will fail with `ENOTFOUND` because +// the DNS entry doesn't exist. It means it won't have much hope if you +// simply try again later. +// However, an `ETIMEDOUT` means it could work but it didn't this time but +// might if we try again a different hour or day. +function isTemporaryRequestError(requestError) { + if (typeof requestError === 'string') { + // See https://betterstack.com/community/guides/scaling-nodejs/nodejs-errors/ + // for a definition of each one. + const errorEnums = ['ECONNRESET', 'ECONNREFUSED', 'ETIMEDOUT', 'ECONNABORTED'] + return errorEnums.some((enum_) => requestError.includes(enum_)) + } + return false +} + // Can't do this memoization within the checkExternalURL because it can // return a Promise since it already collates multiple URLs under the // same cache key. @@ -844,7 +884,10 @@ async function checkExternalURLCached(core, href, { verbose, patient }, db) { } } - const result = await checkExternalURL(core, href, { verbose, patient }) + const result = await checkExternalURL(core, href, { + verbose, + patient, + }) if (cacheMaxAge) { // By only cache storing successful results, we give the system a chance diff --git a/.github/workflows/link-check-daily.yml b/.github/workflows/link-check-daily.yml index be9aabf207..7a55af00d5 100644 --- a/.github/workflows/link-check-daily.yml +++ b/.github/workflows/link-check-daily.yml @@ -77,6 +77,9 @@ jobs: # But mind you that the number has a 10% chance of "jitter" # to avoid a stampeding herd when they all expire some day. EXTERNAL_LINK_CHECKER_MAX_AGE_DAYS: 7 + # If we're unable to connect or the server returns a 50x error, + # treat it as a warning and not as a broken link. + EXTERNAL_SERVER_ERRORS_AS_WARNINGS: true timeout-minutes: 30 run: node .github/actions/rendered-content-link-checker.js diff --git a/lib/excluded-links.js b/lib/excluded-links.js index 65d3281956..0f80af68d3 100644 --- a/lib/excluded-links.js +++ b/lib/excluded-links.js @@ -47,5 +47,4 @@ export default [ 'https://developer.android.com/studio', 'https://lastpass.com/', 'https://lastpass.com/auth/', - 'https://jqplay.org', ] diff --git a/script/rendered-content-link-checker.js b/script/rendered-content-link-checker.js index afa54806c1..7f76384cb8 100755 --- a/script/rendered-content-link-checker.js +++ b/script/rendered-content-link-checker.js @@ -69,6 +69,7 @@ program .option('--bail', 'Exit on the first possible flaw') .option('--verbose-url ', 'Print the absolute URL if set') .option('--fail-on-flaw', 'Throw error on link flaws (default: false)') + .option('--external-server-errors-as-warning', 'Treat server errors as warning (default: false)') .option('--max ', 'integer argument (default: none)', (value) => { const parsed = parseInt(value, 10) if (isNaN(parsed)) {