Only read the frontmatter from files in warm-server (#17222)

* Add read-frontmatter.js

* Use it

* Page static read/init are async now

* Fix some blockers

* I'm confused

* Fix some more bugs

* Use frontmatter schema, ensure end fence exists

* Fix a bug

* Still read full contents for index.md files

* Remove comment

* Only get ToC items for index pages

* Readd frontmatter error and verdadero handling

* Fix some borked tests

* Simplify the code

* Add a comment

* Remove redundant variable

* Re-simplify the Page construction

* End chunk _after_ endline

* Just use Page.init

Author: JasonEtco

lib/page.js

@@ -21,16 +21,18 @@ const frontmatter = require('./frontmatter')
 const products = require('./all-products')
 const slash = require('slash')
 const statsd = require('./statsd')
+const fmfromf = require('./read-frontmatter')
 const getLinkData = require('./get-link-data')
 
 class Page {
-  static init (opts) {
-    opts = Page.read(opts)
+  static async init (opts) {
+    opts = await Page.read(opts)
     if (!opts) return
     return new Page(opts)
   }
 
-  static read (opts) {
+  static async read (opts) {
+    assert(opts.languageCode, 'languageCode is required')
     assert(opts.relativePath, 'relativePath is required')
     assert(opts.basePath, 'basePath is required')
 
@@ -40,40 +42,29 @@ class Page {
     // Per https://nodejs.org/api/fs.html#fs_fs_exists_path_callback
     // its better to read and handle errors than to check access/stats first
     try {
-      const raw = fs.readFileSync(fullPath, 'utf8')
-      return { ...opts, relativePath, fullPath, raw }
+      const { data, content, errors: frontmatterErrors } = await fmfromf(fullPath, opts.languageCode)
+
+      return {
+        ...opts,
+        relativePath,
+        fullPath,
+        ...data,
+        markdown: content,
+        frontmatterErrors
+      }
     } catch (err) {
       if (err.code === 'ENOENT') return false
       console.error(err)
     }
   }
 
   constructor (opts) {
-    assert(opts.languageCode, 'languageCode is required')
-
     Object.assign(this, { ...opts })
 
-    // TODO remove this when crowdin-support issue 66 has been resolved
-    if (this.languageCode !== 'en' && this.raw.includes(': verdadero')) {
-      this.raw = this.raw.replace(': verdadero', ': true')
-    }
-
-    // parse frontmatter and save any errors for validation in the test suite
-    const { content, data, errors: frontmatterErrors } = frontmatter(this.raw, { filepath: this.fullPath })
-    this.frontmatterErrors = frontmatterErrors
-
     if (this.frontmatterErrors.length) {
       throw new Error(JSON.stringify(this.frontmatterErrors, null, 2))
     }
 
-    // preserve the frontmatter-free markdown content,
-    this.markdown = content
-
-    // prevent `[foo] (bar)` strings with a space between from being interpreted as markdown links
-    this.markdown = encodeBracketedParentheses(this.markdown)
-
-    Object.assign(this, data)
-
     // Store raw data so we can cache parsed versions
     this.rawIntro = this.intro
     this.rawTitle = this.title
@@ -94,8 +85,10 @@ class Page {
     // derive array of Permalink objects
     this.permalinks = Permalink.derive(this.languageCode, this.relativePath, this.title, this.versions)
 
-    // get an array of linked items in product and category TOCs
-    this.tocItems = getTocItems(this)
+    if (this.relativePath.endsWith('index.md')) {
+      // get an array of linked items in product and category TOCs
+      this.tocItems = getTocItems(this)
+    }
 
     // if this is an article and it doesn't have showMiniToc = false, set mini TOC to true
     if (!this.relativePath.endsWith('index.md') && !this.mapTopic) {
@@ -146,7 +139,20 @@ class Page {
       : this.renderProp('title', context, opts)
   }
 
+  async getMarkdown () {
+    const raw = fs.readFileSync(this.fullPath, 'utf8')
+    const { content } = frontmatter(raw, { filepath: this.fullPath })
+    // prevent `[foo] (bar)` strings with a space between from being interpreted as markdown links
+    const encodedMarkdown = encodeBracketedParentheses(content)
+    return encodedMarkdown
+  }
+
   async _render (context) {
+    // Get the raw markdown if we need to
+    if (!this.markdown) {
+      this.markdown = await this.getMarkdown()
+    }
+
     this.intro = await renderContent(this.rawIntro, context)
 
     // rewrite local links in the intro to include current language code and GHE version if needed

lib/pages.js

@@ -3,19 +3,19 @@ const walk = require('walk-sync').entries
 const Page = require('./page')
 const languages = require('./languages')
 
-function loadPageList () {
+async function loadPageList () {
   // load english pages
   const englishPath = path.join(__dirname, '..', languages.en.dir, 'content')
   const englishPaths = walk(englishPath, {
     globs: ['**/*.md'],
     ignore: ['**/README.md']
   })
-  const englishPages = englishPaths.map(
-    opts => Page.read({
+  const englishPages = await Promise.all(englishPaths.map(
+    async opts => Page.init({
       ...opts,
       languageCode: languages.en.code
     })
-  )
+  ))
 
   // load matching pages in other languages
   const localizedPaths = Object.values(languages)
@@ -30,16 +30,18 @@ function loadPageList () {
     })
     .flat()
 
-  const localizedPages = localizedPaths.map(
-    ({ basePath, relativePath, languageCode }) =>
-      Page.read({ basePath, relativePath, languageCode })
-  )
+  const localizedPages = await Promise.all(localizedPaths.map(
+    async ({ basePath, relativePath, languageCode }) => Page.init({
+      basePath,
+      relativePath,
+      languageCode
+    })
+  ))
 
   // Build out the list of prepared pages
   return englishPages
     .concat(localizedPages)
     .filter(Boolean)
-    .map(opts => new Page(opts))
 }
 
 function createMapFromArray (pageList) {
@@ -58,8 +60,8 @@ function createMapFromArray (pageList) {
   return pageMap
 }
 
-function loadPageMap (pageList) {
-  const pages = pageList || loadPageList()
+async function loadPageMap (pageList) {
+  const pages = pageList || await loadPageList()
   return createMapFromArray(pages)
 }
 

lib/read-frontmatter.js

@@ -0,0 +1,47 @@
+const fs = require('fs')
+const fm = require('./frontmatter')
+
+const endLine = '\n---\n'
+
+/**
+ * Reads the given filepath, but only up until `endLine`, using streams to
+ * read each chunk and close the stream early.
+ */
+async function readFrontmatter (filepath) {
+  const readStream = fs.createReadStream(filepath, { encoding: 'utf8', emitClose: true })
+  return new Promise((resolve, reject) => {
+    let frontmatter = ''
+    readStream
+      .on('data', function (chunk) {
+        const endOfFrontmatterIndex = chunk.indexOf(endLine)
+        if (endOfFrontmatterIndex !== -1) {
+          frontmatter += chunk.slice(0, endOfFrontmatterIndex + endLine.length)
+          // Stop early!
+          readStream.destroy()
+        } else {
+          frontmatter += chunk
+        }
+      })
+      .on('error', (error) => reject(error))
+      // Stream has been destroyed and file has been closed
+      .on('close', () => resolve(frontmatter))
+  })
+}
+
+/**
+ * Read only the frontmatter from a file
+ */
+module.exports = async function fmfromf (filepath, languageCode) {
+  let fileContent = filepath.endsWith('index.md')
+    // For index files, we need to read the whole file because they contain ToC info
+    ? await fs.promises.readFile(filepath, 'utf8')
+    // For everything else, only read the frontmatter
+    : await readFrontmatter(filepath)
+
+  // TODO remove this when crowdin-support issue 66 has been resolved
+  if (languageCode !== 'en' && fileContent.includes(': verdadero')) {
+    fileContent = fileContent.replace(': verdadero', ': true')
+  }
+
+  return fm(fileContent, { filepath })
+}

lib/warm-server.js

@@ -40,15 +40,15 @@ async function warmServer () {
   }
 
   if (!pageList) {
-    pageList = dog.loadPages()
+    pageList = await dog.loadPages()
   }
 
   if (!site) {
     site = dog.loadSiteData()
   }
 
   if (!pageMap) {
-    pageMap = dog.loadPageMap(pageList)
+    pageMap = await dog.loadPageMap(pageList)
   }
 
   if (!redirects) {

tests/unit/page.js

@@ -430,7 +430,7 @@ describe('Page class', () => {
 })
 
 describe('catches errors thrown in Page class', () => {
-  test('frontmatter parsing error', () => {
+  test('frontmatter parsing error', async () => {
     async function getPage () {
       return await Page.init({
         relativePath: 'page-with-frontmatter-error.md',
@@ -439,10 +439,10 @@ describe('catches errors thrown in Page class', () => {
       })
     }
 
-    expect(getPage).rejects.toThrowError('invalid frontmatter entry')
+    await expect(getPage).rejects.toThrowError('invalid frontmatter entry')
   })
 
-  test('missing versions frontmatter', () => {
+  test('missing versions frontmatter', async () => {
     async function getPage () {
       return await Page.init({
         relativePath: 'page-with-missing-product-versions.md',
@@ -451,10 +451,10 @@ describe('catches errors thrown in Page class', () => {
       })
     }
 
-    expect(getPage).rejects.toThrowError('versions')
+    await expect(getPage).rejects.toThrowError('versions')
   })
 
-  test('English page with a version in frontmatter that its parent product is not available in', () => {
+  test('English page with a version in frontmatter that its parent product is not available in', async () => {
     async function getPage () {
       return await Page.init({
         relativePath: 'admin/some-category/some-article-with-mismatched-versions-frontmatter.md',
@@ -466,7 +466,7 @@ describe('catches errors thrown in Page class', () => {
     expect(getPage).rejects.toThrowError(/`versions` frontmatter.*? product is not available in/)
   })
 
-  test('non-English page with a version in frontmatter that its parent product is not available in', () => {
+  test('non-English page with a version in frontmatter that its parent product is not available in', async () => {
     async function getPage () {
       return await Page.init({
         relativePath: 'admin/some-category/some-article-with-mismatched-versions-frontmatter.md',
@@ -475,6 +475,6 @@ describe('catches errors thrown in Page class', () => {
       })
     }
 
-    expect(getPage).rejects.toThrowError(/`versions` frontmatter.*? product is not available in/)
+    await expect(getPage).rejects.toThrowError(/`versions` frontmatter.*? product is not available in/)
   })
 })