From 211bc3ff8fd09ed2a29cc1f0208d179bda5e7860 Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Sat, 11 Apr 2020 03:06:49 +0800 Subject: [PATCH] Reorganise variable processing (part 1) Processing variables involves a lot of boilerplate code with minor variations. This decentralizes variable processing, decreasing code maintainability and readability. Let's embark on a two stage refactor to improve variable processing. In this first refactor, we create the variablePreprocessor abstraction, and completely migrate the use of userDefinedVariablesMap for site variables to variablePreprocessor, reducing boilerplate code relating to site variables. Additionally, we move all page variable processing related methods to variablePreprocessor from Parser, abstracting such methods into smaller units of functionality where possible, and enhancing in code documentation. In the second part, we will similarly refactor page variable processing methods to reduce more boilerplate, increasing code maintainability. --- src/Page.js | 24 +- src/Site.js | 49 +- src/lib/markbind/src/parser.js | 286 +---------- .../preprocessors/componentPreprocessor.js | 24 +- .../src/preprocessors/variablePreprocessor.js | 468 ++++++++++++++++++ test/unit/Site.test.js | 10 +- test/unit/parser.test.js | 33 +- 7 files changed, 557 insertions(+), 337 deletions(-) create mode 100644 src/lib/markbind/src/preprocessors/variablePreprocessor.js diff --git a/src/Page.js b/src/Page.js index 6780991727..f246322b2a 100644 --- a/src/Page.js +++ b/src/Page.js @@ -77,7 +77,7 @@ class Page { * @property {string} pageTemplate template used for this page * @property {string} title * @property {string} titlePrefix https://markbind.org/userGuide/siteConfiguration.html#titleprefix - * @property {Object} userDefinedVariablesMap + * @property {VariablePreprocessor} variablePreprocessor * @property {string} sourcePath the source file for rendering this page * @property {string} tempPath the temp path for writing intermediate result * @property {string} resultPath the output path of this page @@ -167,9 +167,9 @@ class Page { */ this.titlePrefix = pageConfig.titlePrefix; /** - * @type {Object} + * @type {VariablePreprocessor} */ - this.userDefinedVariablesMap = pageConfig.userDefinedVariablesMap; + this.variablePreprocessor = pageConfig.variablePreprocessor; /** * The source file for rendering this page @@ -643,8 +643,7 @@ class Page { this.includedFiles.add(headerPath); // Map variables const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap); - const userDefinedVariables = this.userDefinedVariablesMap[parentSite]; - return `${njUtil.renderRaw(headerContent, userDefinedVariables)}\n${pageData}`; + return `${this.variablePreprocessor.renderSiteVariables(parentSite, headerContent)}\n${pageData}`; } /** @@ -673,8 +672,7 @@ class Page { this.includedFiles.add(footerPath); // Map variables const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap); - const userDefinedVariables = this.userDefinedVariablesMap[parentSite]; - return `${pageData}\n${njUtil.renderRaw(footerContent, userDefinedVariables)}`; + return `${pageData}\n${this.variablePreprocessor.renderSiteVariables(parentSite, footerContent)}`; } /** @@ -710,8 +708,7 @@ class Page { this.includedFiles.add(siteNavPath); // Map variables const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap); - const userDefinedVariables = this.userDefinedVariablesMap[parentSite]; - const siteNavMappedData = njUtil.renderRaw(siteNavContent, userDefinedVariables); + const siteNavMappedData = this.variablePreprocessor.renderSiteVariables(parentSite, siteNavContent); // Convert to HTML const siteNavDataSelector = cheerio.load(siteNavMappedData); if (siteNavDataSelector('navigation').length > 1) { @@ -847,8 +844,7 @@ class Page { this.includedFiles.add(headFilePath); // Map variables const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap); - const userDefinedVariables = this.userDefinedVariablesMap[parentSite]; - const headFileMappedData = njUtil.renderRaw(headFileContent, userDefinedVariables) + const headFileMappedData = this.variablePreprocessor.renderSiteVariables(parentSite, headFileContent) .trim(); // Split top and bottom contents const $ = cheerio.load(headFileMappedData, { xmlMode: false }); @@ -907,7 +903,7 @@ class Page { * @typedef {Object} FileConfig * @property {Set} baseUrlMap the set of urls representing the sites' base directories * @property {string} rootPath - * @property {Object} userDefinedVariablesMap + * @property {VariablePreprocessor} variablePreprocessor * @property {Object} headerIdMap * @property {boolean} fixedHeader indicates whether the header of the page is fixed */ @@ -925,7 +921,7 @@ class Page { const fileConfig = { baseUrlMap: this.baseUrlMap, rootPath: this.rootPath, - userDefinedVariablesMap: this.userDefinedVariablesMap, + variablePreprocessor: this.variablePreprocessor, headerIdMap: this.headerIdMap, fixedHeader: this.fixedHeader, }; @@ -1241,7 +1237,7 @@ class Page { }); return markbinder.includeFile(dependency.to, { baseUrlMap: this.baseUrlMap, - userDefinedVariablesMap: this.userDefinedVariablesMap, + variablePreprocessor: this.variablePreprocessor, rootPath: this.rootPath, cwf: file, }).then(result => Page.removeFrontMatter(result)) diff --git a/src/Site.js b/src/Site.js index f54dd9753b..f74a2e74ca 100644 --- a/src/Site.js +++ b/src/Site.js @@ -6,7 +6,7 @@ const path = require('path'); const Promise = require('bluebird'); const ProgressBar = require('progress'); const walkSync = require('walk-sync'); -const MarkBind = require('./lib/markbind/src/parser'); +const VariablePreprocessor = require('./lib/markbind/src/preprocessors/variablePreprocessor'); const njUtil = require('./lib/markbind/src/utils/nunjuckUtils'); const injectHtmlParser2SpecialTags = require('./lib/markbind/src/patches/htmlparser2'); const injectMarkdownItSpecialTags = require( @@ -134,7 +134,9 @@ class Site { */ this.siteConfig = undefined; this.siteConfigPath = siteConfigPath; - this.userDefinedVariablesMap = {}; + + // Site wide variable preprocessor + this.variablePreprocessor = undefined; // Lazy reload properties this.onePagePath = onePagePath; @@ -271,7 +273,7 @@ class Site { title: config.title || '', titlePrefix: this.siteConfig.titlePrefix, headingIndexingLevel: this.siteConfig.headingIndexingLevel, - userDefinedVariablesMap: this.userDefinedVariablesMap, + userDefinedVariablesMap: this.variablePreprocessor, sourcePath, resultPath, asset: { @@ -513,6 +515,7 @@ class Site { .map(x => path.resolve(this.rootPath, x)); this.baseUrlMap = new Set(candidates.map(candidate => path.dirname(candidate))); + this.variablePreprocessor = new VariablePreprocessor(this.rootPath, this.baseUrlMap); return Promise.resolve(); } @@ -523,12 +526,9 @@ class Site { collectUserDefinedVariablesMap() { // The key is the base directory of the site/subsites, // while the value is a mapping of user defined variables - this.userDefinedVariablesMap = {}; - const markbindVariable = { MarkBind: MARKBIND_LINK_HTML }; + this.variablePreprocessor.resetUserDefinedVariablesMap(); this.baseUrlMap.forEach((base) => { - const userDefinedVariables = {}; - Object.assign(userDefinedVariables, markbindVariable); const userDefinedVariablesPath = path.resolve(base, USER_VARIABLES_PATH); const userDefinedVariablesDir = path.dirname(userDefinedVariablesPath); let content; @@ -539,15 +539,17 @@ class Site { logger.warn(e.message); } - // This is to prevent the first nunjuck call from converting {{baseUrl}} to an empty string - // and let the baseUrl value be injected later. - userDefinedVariables.baseUrl = '{{baseUrl}}'; - this.userDefinedVariablesMap[base] = userDefinedVariables; + /* + This is to prevent the first nunjuck call from converting {{baseUrl}} to an empty string + and let the baseUrl value be injected later. + */ + this.variablePreprocessor.addUserDefinedVariable(base, 'baseUrl', '{{baseUrl}}'); + this.variablePreprocessor.addUserDefinedVariable(base, 'MarkBind', MARKBIND_LINK_HTML); const $ = cheerio.load(content); - $('variable,span').each(function () { - const name = $(this).attr('name') || $(this).attr('id'); - const variableSource = $(this).attr('from'); + $('variable,span').each((index, element) => { + const name = $(element).attr('name') || $(element).attr('id'); + const variableSource = $(element).attr('from'); if (variableSource !== undefined) { try { @@ -555,18 +557,13 @@ class Site { const jsonData = fs.readFileSync(variableFilePath); const varData = JSON.parse(jsonData); Object.entries(varData).forEach(([varName, varValue]) => { - // Process the content of the variable with nunjucks, in case it refers to other variables. - const variableValue = njUtil.renderRaw(varValue, userDefinedVariables, {}, false); - - userDefinedVariables[varName] = variableValue; + this.variablePreprocessor.addAndRenderUserDefinedVariable(base, varName, varValue); }); } catch (err) { logger.warn(`Error ${err.message}`); } } else { - // Process the content of the variable with nunjucks, in case it refers to other variables. - const html = njUtil.renderRaw($(this).html(), userDefinedVariables, {}, false); - userDefinedVariables[name] = html; + this.variablePreprocessor.addAndRenderUserDefinedVariable(base, name, $(element).html()); } }); }); @@ -691,7 +688,7 @@ class Site { const filePathArray = Array.isArray(filePaths) ? filePaths : [filePaths]; const uniquePaths = _.uniq(filePathArray); logger.info('Rebuilding affected source files'); - MarkBind.resetVariables(); + this.variablePreprocessor.resetVariables(); return new Promise((resolve, reject) => { this.regenerateAffectedPages(uniquePaths) .then(() => fs.removeAsync(this.tempPath)) @@ -709,7 +706,7 @@ class Site { const uniqueUrls = _.uniq(normalizedUrlArray); uniqueUrls.forEach(normalizedUrl => logger.info( `Building ${normalizedUrl} as some of its dependencies were changed since the last visit`)); - MarkBind.resetVariables(); + this.variablePreprocessor.resetVariables(); /* Lazy loading only builds the page being viewed, but the user may be quick enough @@ -729,7 +726,6 @@ class Site { } this.toRebuild.delete(normalizedUrl); - pageToRebuild.userDefinedVariablesMap = this.userDefinedVariablesMap; pageToRebuild.generate(new Set()) .then(() => { pageToRebuild.collectHeadingsAndKeywords(); @@ -1062,7 +1058,6 @@ class Site { } } - page.userDefinedVariablesMap = this.userDefinedVariablesMap; processingFiles.push(page.generate(builtFiles) .catch((err) => { logger.error(err); @@ -1281,9 +1276,7 @@ class Site { timeZoneName: 'short', }; const time = new Date().toLocaleTimeString(this.siteConfig.locale, options); - Object.keys(this.userDefinedVariablesMap).forEach((base) => { - this.userDefinedVariablesMap[base].timestamp = time; - }); + this.variablePreprocessor.addUserDefinedVariableForAllSites('timestamp', time); return Promise.resolve(); } } diff --git a/src/lib/markbind/src/parser.js b/src/lib/markbind/src/parser.js index 3fb28f84a6..3ef7d66b1b 100644 --- a/src/lib/markbind/src/parser.js +++ b/src/lib/markbind/src/parser.js @@ -7,7 +7,6 @@ const slugify = require('@sindresorhus/slugify'); const ensurePosix = require('ensure-posix-path'); const componentParser = require('./parsers/componentParser'); const componentPreprocessor = require('./preprocessors/componentPreprocessor'); -const logger = require('../../../util/logger'); const njUtil = require('./utils/nunjuckUtils'); const _ = {}; @@ -27,7 +26,6 @@ cheerio.prototype.options.decodeEntities = false; // Don't escape HTML entities const { ATTRIB_INCLUDE_PATH, ATTRIB_CWF, - IMPORTED_VARIABLE_PREFIX, } = require('./constants'); class Parser { @@ -35,103 +33,12 @@ class Parser { this._options = options || {}; // eslint-disable-next-line no-console this._onError = this._options.errorHandler || console.error; - this._fileCache = {}; this.dynamicIncludeSrc = []; this.staticIncludeSrc = []; this.boilerplateIncludeSrc = []; this.missingIncludeSrc = []; } - /** - * Returns an object containing the imported variables for specified file - * @param file file name to get the imported variables for - */ - // eslint-disable-next-line class-methods-use-this - getImportedVariableMap(file) { - const innerVariables = {}; - Parser.FILE_ALIASES.get(file).forEach((actualPath, alias) => { - innerVariables[alias] = {}; - const variables = Parser.VARIABLE_LOOKUP.get(actualPath); - variables.forEach((value, name) => { - innerVariables[alias][name] = value; - }); - }); - return innerVariables; - } - - /** - * Extract page variables from a page - * @param filename for error printing - * @param data to extract variables from - * @param userDefinedVariables global variables - * @param includedVariables variables from parent include - */ - static extractPageVariables(fileName, data, userDefinedVariables, includedVariables) { - const fileDir = path.dirname(fileName); - const $ = cheerio.load(data); - const pageVariables = {}; - Parser.VARIABLE_LOOKUP.set(fileName, new Map()); - /** - * ed variables have not been processed yet, we replace such variables with itself first. - */ - const importedVariables = {}; - $('import[from]').each((index, element) => { - const variableNames = Object.keys(element.attribs).filter(name => name !== 'from' && name !== 'as'); - // If no namespace is provided, we use the smallest name as one... - const largestName = variableNames.sort()[0]; - // ... and prepend it with $__MARKBIND__ to reduce collisions. - const generatedAlias = IMPORTED_VARIABLE_PREFIX + largestName; - const hasAlias = _.hasIn(element.attribs, 'as'); - const alias = hasAlias ? element.attribs.as : generatedAlias; - importedVariables[alias] = new Proxy({}, { - get(obj, prop) { - return `{{${alias}.${prop}}}`; - }, - }); - variableNames.forEach((name) => { - importedVariables[name] = `{{${alias}.${name}}}`; - }); - }); - const setPageVariable = (variableName, rawVariableValue) => { - const otherVariables = { - ...importedVariables, - ...pageVariables, - ...userDefinedVariables, - ...includedVariables, - }; - const variableValue = njUtil.renderRaw(rawVariableValue, otherVariables, {}, false); - if (!pageVariables[variableName]) { - pageVariables[variableName] = variableValue; - Parser.VARIABLE_LOOKUP.get(fileName).set(variableName, variableValue); - } - }; - $('variable').each(function () { - const variableElement = $(this); - const variableName = variableElement.attr('name'); - const variableSource = $(this).attr('from'); - if (variableSource !== undefined) { - try { - const variableFilePath = path.resolve(fileDir, variableSource); - const jsonData = fs.readFileSync(variableFilePath); - const varData = JSON.parse(jsonData); - Object.entries(varData).forEach(([varName, varValue]) => { - setPageVariable(varName, varValue); - }); - } catch (err) { - logger.warn(`Error ${err.message}`); - } - } else { - if (!variableName) { - // eslint-disable-next-line no-console - console.warn(`Missing 'name' for variable in ${fileName}\n`); - return; - } - setPageVariable(variableName, md.renderInline(variableElement.html())); - } - }); - return { ...importedVariables, ...pageVariables }; - } - getDynamicIncludeSrc() { return _.clone(this.dynamicIncludeSrc); } @@ -148,82 +55,6 @@ class Parser { return _.clone(this.missingIncludeSrc); } - _renderIncludeFile(filePath, node, context, config, asIfAt = filePath) { - try { - this._fileCache[filePath] = this._fileCache[filePath] || fs.readFileSync(filePath, 'utf8'); - } catch (e) { - // Read file fail - const missingReferenceErrorMessage = `Missing reference in: ${node.attribs[ATTRIB_CWF]}`; - e.message += `\n${missingReferenceErrorMessage}`; - this._onError(e); - return utils.createErrorNode(node, e); - } - const fileContent = this._fileCache[filePath]; // cache the file contents to save some I/O - const parentSitePath = urlUtils.getParentSiteAbsolutePath(asIfAt, config.rootPath, config.baseUrlMap); - const userDefinedVariables = config.userDefinedVariablesMap[parentSitePath]; - // Extract included variables from the PARENT file - const includeVariables = Parser.extractIncludeVariables(node, context.variables); - // Extract page variables from the CHILD file - const pageVariables = Parser.extractPageVariables(asIfAt, fileContent, - userDefinedVariables, includeVariables); - const content = njUtil.renderRaw(fileContent, { - ...pageVariables, - ...includeVariables, - ...userDefinedVariables, - }, { - path: filePath, - }); - const childContext = _.cloneDeep(context); - childContext.cwf = asIfAt; - childContext.variables = includeVariables; - return { content, childContext, userDefinedVariables }; - } - - _extractInnerVariables(content, context, config) { - const { cwf } = context; - const $ = cheerio.load(content, { - xmlMode: false, - decodeEntities: false, - }); - const aliases = new Map(); - Parser.FILE_ALIASES.set(cwf, aliases); - $('import[from]').each((index, node) => { - const filePath = path.resolve(path.dirname(cwf), node.attribs.from); - const variableNames = Object.keys(node.attribs) - .filter(name => name !== 'from' && name !== 'as'); - // If no namespace is provided, we use the smallest name as one - const largestName = variableNames.sort()[0]; - // ... and prepend it with $__MARKBIND__ to reduce collisions. - const generatedAlias = IMPORTED_VARIABLE_PREFIX + largestName; - const alias = _.hasIn(node.attribs, 'as') - ? node.attribs.as - : generatedAlias; - aliases.set(alias, filePath); - this.staticIncludeSrc.push({ from: context.cwf, to: filePath }); - // Render inner file content - const { - content: renderedContent, - childContext, - userDefinedVariables, - } = this._renderIncludeFile(filePath, node, context, config); - this.extractInnerVariablesIfNotProcessed(renderedContent, childContext, config, filePath); - const innerVariables = this.getImportedVariableMap(filePath); - - Parser.VARIABLE_LOOKUP.get(filePath).forEach((value, variableName, map) => { - map.set(variableName, njUtil.renderRaw(value, { - ...userDefinedVariables, ...innerVariables, - })); - }); - }); - } - - extractInnerVariablesIfNotProcessed(content, childContext, config, filePathToExtract) { - if (!Parser.PROCESSED_INNER_VARIABLES.has(filePathToExtract)) { - Parser.PROCESSED_INNER_VARIABLES.add(filePathToExtract); - this._extractInnerVariables(content, childContext, config); - } - } - processDynamicResources(context, html) { const $ = cheerio.load(html, { xmlMode: false, @@ -422,27 +253,21 @@ class Parser { reject(err); return; } - const parentSitePath = urlUtils.getParentSiteAbsolutePath(file, config.rootPath, config.baseUrlMap); - const userDefinedVariables = config.userDefinedVariablesMap[parentSitePath]; - const pageVariables = Parser.extractPageVariables(file, data, userDefinedVariables, {}); - let fileContent = njUtil.renderRaw(data, { - ...pageVariables, - ...userDefinedVariables, - }, { - path: actualFilePath, - }); - this._extractInnerVariables(fileContent, context, config); - const innerVariables = this.getImportedVariableMap(context.cwf); - fileContent = njUtil.renderRaw(fileContent, { - ...userDefinedVariables, ...innerVariables, - }); + const { variablePreprocessor } = config; + + const outerFileContentRendered = variablePreprocessor.renderOuterVariables(data, file); + variablePreprocessor.extractInnerVariables(outerFileContentRendered, context.cwf) + .forEach(src => this.staticIncludeSrc.push(src)); + const innerFileContentRendered = variablePreprocessor.renderInnerVariables(outerFileContentRendered, + file, context.cwf); + const fileExt = utils.getExt(file); if (utils.isMarkdownFileExt(fileExt)) { context.source = 'md'; - parser.parseComplete(fileContent.toString()); + parser.parseComplete(innerFileContentRendered.toString()); } else if (fileExt === 'html') { context.source = 'html'; - parser.parseComplete(fileContent); + parser.parseComplete(innerFileContentRendered); } else { const error = new Error(`Unsupported File Extension: '${fileExt}'`); reject(error); @@ -456,14 +281,6 @@ class Parser { context.cwf = config.cwf || file; // current working file return new Promise((resolve, reject) => { - let actualFilePath = file; - if (!utils.fileExists(file)) { - const boilerplateFilePath = urlUtils.calculateBoilerplateFilePath(path.basename(file), file, config); - if (utils.fileExists(boilerplateFilePath)) { - actualFilePath = boilerplateFilePath; - } - } - const currentContext = context; currentContext.mode = 'include'; currentContext.callStack = []; @@ -478,7 +295,7 @@ class Parser { try { processed = componentPreprocessor.preProcessComponent(d, currentContext, config, this); } catch (err) { - err.message += `\nError while preprocessing '${actualFilePath}'`; + err.message += `\nError while preprocessing '${file}'`; this._onError(err); processed = utils.createErrorNode(d, err); } @@ -487,37 +304,28 @@ class Parser { resolve(cheerio.html(nodes)); }); + const { additionalVariables, variablePreprocessor } = config; + + const outerFileContentRendered = variablePreprocessor.renderOuterVariables(pageData, file, + additionalVariables); + variablePreprocessor.extractInnerVariables(outerFileContentRendered, currentContext.cwf) + .forEach(src => this.staticIncludeSrc.push(src)); + const innerFileContentRendered = variablePreprocessor.renderInnerVariables(outerFileContentRendered, + file, currentContext.cwf, + additionalVariables); + const parser = new htmlparser.Parser(handler, { xmlMode: true, decodeEntities: true, }); - const parentSitePath = urlUtils.getParentSiteAbsolutePath(file, config.rootPath, config.baseUrlMap); - const userDefinedVariables = config.userDefinedVariablesMap[parentSitePath]; - const { additionalVariables } = config; - const pageVariables = Parser.extractPageVariables(actualFilePath, pageData, userDefinedVariables, {}); - - let fileContent = njUtil.renderRaw(pageData, { - ...pageVariables, - ...userDefinedVariables, - ...additionalVariables, - }, { - path: actualFilePath, - }); - this._extractInnerVariables(fileContent, currentContext, config); - const innerVariables = this.getImportedVariableMap(currentContext.cwf); - fileContent = njUtil.renderRaw(fileContent, { - ...userDefinedVariables, - ...additionalVariables, - ...innerVariables, - }); - const fileExt = utils.getExt(actualFilePath); + const fileExt = utils.getExt(file); if (utils.isMarkdownFileExt(fileExt)) { currentContext.source = 'md'; - parser.parseComplete(fileContent.toString()); + parser.parseComplete(innerFileContentRendered.toString()); } else if (fileExt === 'html') { currentContext.source = 'html'; - parser.parseComplete(fileContent); + parser.parseComplete(innerFileContentRendered); } else { const error = new Error(`Unsupported File Extension: '${fileExt}'`); reject(error); @@ -639,55 +447,9 @@ class Parser { return node; } - static resetVariables() { - Parser.VARIABLE_LOOKUP.clear(); - Parser.FILE_ALIASES.clear(); - Parser.PROCESSED_INNER_VARIABLES.clear(); - } - static isText(node) { return node.type === 'text' || node.type === 'comment'; } - - /** - * Extract variables from an include element - * @param includeElement include element to extract variables from - * @param contextVariables variables defined by parent pages - */ - static extractIncludeVariables(includeElement, contextVariables) { - const includedVariables = { ...contextVariables }; - Object.keys(includeElement.attribs).forEach((attribute) => { - if (!attribute.startsWith('var-')) { - return; - } - const variableName = attribute.replace(/^var-/, ''); - if (!includedVariables[variableName]) { - includedVariables[variableName] = includeElement.attribs[attribute]; - } - }); - if (includeElement.children) { - includeElement.children.forEach((child) => { - if (child.name !== 'variable' && child.name !== 'span') { - return; - } - const variableName = child.attribs.name || child.attribs.id; - if (!variableName) { - // eslint-disable-next-line no-console - console.warn(`Missing reference in ${includeElement.attribs[ATTRIB_CWF]}\n` - + `Missing 'name' or 'id' in variable for ${includeElement.attribs.src} include.`); - return; - } - if (!includedVariables[variableName]) { - includedVariables[variableName] = cheerio.html(child.children); - } - }); - } - return includedVariables; - } } -Parser.VARIABLE_LOOKUP = new Map(); -Parser.FILE_ALIASES = new Map(); -Parser.PROCESSED_INNER_VARIABLES = new Set(); - module.exports = Parser; diff --git a/src/lib/markbind/src/preprocessors/componentPreprocessor.js b/src/lib/markbind/src/preprocessors/componentPreprocessor.js index 0829c727eb..a3c1f1c0b4 100644 --- a/src/lib/markbind/src/preprocessors/componentPreprocessor.js +++ b/src/lib/markbind/src/preprocessors/componentPreprocessor.js @@ -6,7 +6,6 @@ const CyclicReferenceError = require('../handlers/cyclicReferenceError.js'); const utils = require('../utils'); const urlUtils = require('../utils/urls'); -const njUtil = require('../utils/nunjuckUtils'); const _ = {}; _.has = require('lodash/has'); @@ -252,16 +251,12 @@ function _preprocessInclude(node, context, config, parser) { const isIncludeSrcMd = _isHtmlIncludingMarkdown(element, context, filePath); - const { content, childContext, userDefinedVariables } - = parser._renderIncludeFile(actualFilePath, element, context, config, filePath); - childContext.source = isIncludeSrcMd ? 'md' : 'html'; - childContext.callStack.push(context.cwf); - parser.extractInnerVariablesIfNotProcessed(content, childContext, config, filePath); - - const innerVariables = parser.getImportedVariableMap(filePath); - const fileContent = njUtil.renderRaw(content, { - ...userDefinedVariables, ...innerVariables, - }); + const { variablePreprocessor } = config; + const { content, childContext } = variablePreprocessor.renderIncludeFile(actualFilePath, element, + context, filePath); + variablePreprocessor.extractInnerVariablesIfNotProcessed(content, childContext.cwf, filePath) + .forEach(src => parser.staticIncludeSrc.push(src)); + const innerContentRendered = variablePreprocessor.renderInnerVariables(content, filePath, filePath); _deleteIncludeAttributes(element); @@ -272,7 +267,7 @@ function _preprocessInclude(node, context, config, parser) { if (hash) { // Keep scripts in the fileContent - const src = cheerio.parseHTML(fileContent, true); + const src = cheerio.parseHTML(innerContentRendered, true); const $ = cheerio.load(src); const hashContent = $(hash).html(); @@ -293,7 +288,7 @@ function _preprocessInclude(node, context, config, parser) { // optional includes of segments have now been handled, so delete the attribute if (isOptional) delete element.attribs.optional; } else { - actualContent = (fileContent && isTrim) ? fileContent.trim() : fileContent; + actualContent = (innerContentRendered && isTrim) ? innerContentRendered.trim() : innerContentRendered; } if (isIncludeSrcMd) { @@ -307,6 +302,9 @@ function _preprocessInclude(node, context, config, parser) { element.children = cheerio.parseHTML(childrenHtml, true); if (element.children && element.children.length > 0) { + childContext.source = isIncludeSrcMd ? 'md' : 'html'; + childContext.callStack.push(context.cwf); + if (childContext.callStack.length > CyclicReferenceError.MAX_RECURSIVE_DEPTH) { const error = new CyclicReferenceError(childContext.callStack); parser._onError(error); diff --git a/src/lib/markbind/src/preprocessors/variablePreprocessor.js b/src/lib/markbind/src/preprocessors/variablePreprocessor.js new file mode 100644 index 0000000000..74bc17d24c --- /dev/null +++ b/src/lib/markbind/src/preprocessors/variablePreprocessor.js @@ -0,0 +1,468 @@ +const cheerio = require('cheerio'); +const fs = require('fs'); +const path = require('path'); + +const _ = {}; +_.clone = require('lodash/clone'); +_.cloneDeep = require('lodash/cloneDeep'); +_.has = require('lodash/has'); +_.isArray = require('lodash/isArray'); +_.isEmpty = require('lodash/isEmpty'); + +const md = require('../lib/markdown-it'); +const utils = require('../utils'); +const njUtil = require('../utils/nunjuckUtils'); +const urlUtils = require('../utils/urls'); + +const { + ATTRIB_CWF, + IMPORTED_VARIABLE_PREFIX, +} = require('../constants'); + + +/** + * Variable preprocessor to recursively extract and render pages' variables. + * Houses a site-wide variable cache. + */ +class VariablePreprocessor { + constructor(rootPath, baseUrlMap) { + /** + * Root site path + * @type {string} + */ + this.rootPath = rootPath; + + /** + * Set of sites' root paths + * @type {Set} + */ + this.baseUrlMap = baseUrlMap; + + /** + * Map of sites to their variables + * @type {Object} + */ + this.userDefinedVariablesMap = {}; + + /* + * Page level + */ + + /** + * Map of files to another object containing its variable names matched to values + * @type {Object>} + */ + this.variableLookup = {}; + + /** + * Map of files to another object containing its + * variable aliases matched to the respective imported file + * @type {Object>} + */ + this.fileAliasesMap = {}; + + /** + * Set of files that were already processed + * @type {Set} + */ + this.processedFiles = new Set(); + } + + + /* + * -------------------------------------------------- + * Site level variable storage methods + */ + + + /** + * Adds a variable for a site with the specified name and value. + * @param site path of the site, as reflected by the Site instance's baseUrlMap + * @param name of the variable to add + * @param value of the variable + */ + addUserDefinedVariable(site, name, value) { + this.userDefinedVariablesMap[site] = this.userDefinedVariablesMap[site] || {}; + this.userDefinedVariablesMap[site][name] = value; + } + + /** + * Renders the variable in addition to adding it, unlike {@link addUserDefinedVariable}. + */ + addAndRenderUserDefinedVariable(site, name, value) { + const renderedVal = njUtil.renderRaw(value, this.userDefinedVariablesMap[site], {}, false); + this.addUserDefinedVariable(site, name, renderedVal); + } + + /** + * Version of {@link addUserDefinedVariable} that adds to all sites. + */ + addUserDefinedVariableForAllSites(name, value) { + Object.keys(this.userDefinedVariablesMap) + .forEach(base => this.addUserDefinedVariable(base, name, value)); + } + + resetUserDefinedVariablesMap() { + this.userDefinedVariablesMap = {}; + } + + /* + * -------------------------------------------------- + * Site level variable rendering methods + */ + + + /** + * Renders the supplied content using only the site variables belonging to the specified base. + */ + renderSiteVariables(siteBase, content) { + return njUtil.renderRaw(content, this.userDefinedVariablesMap[siteBase]); + } + + + /* + * -------------------------------------------------- + * Page level variable storage methods + */ + + + resetVariables() { + this.variableLookup = {}; + this.fileAliasesMap = {}; + this.processedFiles.clear(); + } + + /** + * Returns an object containing the ed variables for the specified file. + * For variables specified with aliases, we construct the sub object containing the + * names and values of the variables tied to the alias. + * @param file to get the imported variables for + */ + getImportedVariableMap(file) { + const innerVariables = {}; + const fileAliases = this.fileAliasesMap[file]; + + Object.entries(fileAliases).forEach(([alias, actualPath]) => { + innerVariables[alias] = {}; + const variables = this.variableLookup[actualPath]; + + Object.entries(variables).forEach(([name, value]) => { + innerVariables[alias][name] = value; + }); + }); + + return innerVariables; + } + + /** + * Retrieves a semi-unique alias for the given array of variable names from an import element + * by prepending the smallest variable name with $__MARKBIND__. + * Uses the alias declared in the 'as' attribute if there is one. + * @param variableNames of the import element, not referenced from the 'as' attribute + * @param node of the import element + * @return {string} Alias for the import element + */ + static getAliasForImportVariables(variableNames, node) { + if (_.has(node.attribs, 'as')) { + return node.attribs.as; + } + const largestName = variableNames.sort()[0]; + return IMPORTED_VARIABLE_PREFIX + largestName; + } + + /** + * Extracts a pseudo imported variables for the supplied content. + * @param $ loaded cheerio object of the content + * @return {{}} pseudo imported variables of the content + */ + static extractPseudoImportedVariables($) { + const importedVariables = {}; + $('import[from]').each((index, element) => { + const variableNames = Object.keys(element.attribs).filter(name => name !== 'from' && name !== 'as'); + + // Add pseudo variables for the alias's variables + const alias = VariablePreprocessor.getAliasForImportVariables(variableNames, element); + importedVariables[alias] = new Proxy({}, { + get(obj, prop) { + return `{{${alias}.${prop}}}`; + }, + }); + + // Add pseudo variables for inline variables in the import element + variableNames.forEach((name) => { + importedVariables[name] = `{{${alias}.${name}}}`; + }); + }); + return importedVariables; + } + + /** + * Extract page variables from a page. + * @param fileName for error printing + * @param data to extract variables from + * @param includedVariables variables from parent include + */ + extractPageVariables(fileName, data, includedVariables) { + const fileDir = path.dirname(fileName); + const $ = cheerio.load(data); + + const importedVariables = VariablePreprocessor.extractPseudoImportedVariables($); + const pageVariables = {}; + const userDefinedVariables = this.getContentParentSiteVariables(fileName); + + const setPageVariableIfNotSetBefore = (variableName, rawVariableValue) => { + if (pageVariables[variableName]) { + return; + } + + pageVariables[variableName] = njUtil.renderRaw(rawVariableValue, { + ...importedVariables, + ...pageVariables, + ...userDefinedVariables, + ...includedVariables, + }, {}, false); + }; + + $('variable').each((index, node) => { + const variableSource = $(node).attr('from'); + if (variableSource !== undefined) { + try { + const variableFilePath = path.resolve(fileDir, variableSource); + const jsonData = fs.readFileSync(variableFilePath); + const varData = JSON.parse(jsonData); + Object.entries(varData).forEach(([varName, varValue]) => { + setPageVariableIfNotSetBefore(varName, varValue); + }); + } catch (err) { + console.warn(`Error ${err.message}`); + } + } else { + const variableName = $(node).attr('name'); + if (!variableName) { + console.warn(`Missing 'name' for variable in ${fileName}\n`); + return; + } + setPageVariableIfNotSetBefore(variableName, md.renderInline($(node).html())); + } + }); + + this.variableLookup[fileName] = pageVariables; + + return { ...importedVariables, ...pageVariables }; + } + + /** + * Extracts inner variables of the content, resolving new imports with respect to + * the supplied contentFilePath. + * @param content to extract from + * @param contentFilePath of the content + * @return {[]} Array of files that were imported + */ + extractInnerVariables(content, contentFilePath) { + let staticIncludeSrcs = []; + const $ = cheerio.load(content, { + xmlMode: false, + decodeEntities: false, + }); + + const aliases = {}; + this.fileAliasesMap[contentFilePath] = aliases; + + $('import[from]').each((index, node) => { + const filePath = path.resolve(path.dirname(contentFilePath), node.attribs.from); + staticIncludeSrcs.push({ from: contentFilePath, to: filePath }); + + const variableNames = Object.keys(node.attribs).filter(name => name !== 'from' && name !== 'as'); + const alias = VariablePreprocessor.getAliasForImportVariables(variableNames, node); + aliases[alias] = filePath; + + // Render inner file content and extract inner variables recursively + const { + content: renderedContent, + childContext, + } = this.renderIncludeFile(filePath, node, {}); + const userDefinedVariables = this.getContentParentSiteVariables(filePath); + staticIncludeSrcs = [ + ...staticIncludeSrcs, + ...this.extractInnerVariablesIfNotProcessed(renderedContent, childContext.cwf, filePath), + ]; + + // Re-render page variables with imported variables + const innerVariables = this.getImportedVariableMap(filePath); + const variables = this.variableLookup[filePath]; + Object.entries(variables).forEach(([variableName, value]) => { + variables[variableName] = njUtil.renderRaw(value, { + ...userDefinedVariables, ...innerVariables, + }); + }); + }); + + return staticIncludeSrcs; + } + + /** + * Extracts inner variables {@link extractInnerVariables} only if not processed before + */ + extractInnerVariablesIfNotProcessed(content, contentFilePath, filePathToExtract) { + if (!this.processedFiles.has(filePathToExtract)) { + this.processedFiles.add(filePathToExtract); + return this.extractInnerVariables(content, contentFilePath); + } + return []; + } + + /** + * Extracts variables specified as in include elements, + * adding them to the supplied variables if it does not already exist. + * @param includeElement to extract inline variables from + * @param variables to add the extracted variables to + */ + static extractIncludeInlineVariables(includeElement, variables) { + Object.keys(includeElement.attribs).forEach((attribute) => { + if (!attribute.startsWith('var-')) { + return; + } + const variableName = attribute.replace(/^var-/, ''); + if (!variables[variableName]) { + variables[variableName] = includeElement.attribs[attribute]; + } + }); + } + + /** + * Extracts variables specified as in include elements, + * adding them to the supplied variables if it does not already exist. + * @param includeElement to search child nodes for + * @param variables to add the extracted variables to + */ + static extractIncludeChildElementVariables(includeElement, variables) { + if (!(includeElement.children && includeElement.children.length)) { + return; + } + + includeElement.children.forEach((child) => { + if (child.name !== 'variable' && child.name !== 'span') { + return; + } + const variableName = child.attribs.name || child.attribs.id; + if (!variableName) { + // eslint-disable-next-line no-console + console.warn(`Missing reference in ${includeElement.attribs[ATTRIB_CWF]}\n` + + `Missing 'name' or 'id' in variable for ${includeElement.attribs.src} include.`); + return; + } + if (!variables[variableName]) { + variables[variableName] = cheerio.html(child.children); + } + }); + } + + + /** + * Extract variables from an include element. + * Subroutine for {@link renderIncludeFile} + * @param includeElement include element to extract variables from + * @param parentVariables defined by parent pages, which have priority + */ + static extractIncludeVariables(includeElement, parentVariables) { + const includedVariables = { ...parentVariables }; + VariablePreprocessor.extractIncludeInlineVariables(includeElement, includedVariables); + VariablePreprocessor.extractIncludeChildElementVariables(includeElement, includedVariables); + + return includedVariables; + } + + + /* + * -------------------------------------------------- + * Combined rendering methods + */ + + + /** + * Renders an include file with the supplied context, returning the rendered + * content and new context with respect to the child content. + * @param filePath of the included file source + * @param node of the include element + * @param context object containing the variables for the current context + * @param asIfAt where the included file should be rendered from + */ + renderIncludeFile(filePath, node, context, asIfAt = filePath) { + try { + // Extract included variables from the include element, merging with the parent context variables + const includeVariables = VariablePreprocessor.extractIncludeVariables(node, context.variables); + + // Extract page variables from the included file + const fileContent = fs.readFileSync(filePath, 'utf8'); + const userDefinedVariables = this.getContentParentSiteVariables(asIfAt); + const pageVariables = this.extractPageVariables(asIfAt, fileContent, includeVariables); + + // Render the included content with the extracted variables + const content = njUtil.renderRaw(fileContent, { + ...pageVariables, + ...includeVariables, + ...userDefinedVariables, + }); + + // Set up the next child context + const childContext = _.cloneDeep(context); + childContext.cwf = asIfAt; + childContext.variables = includeVariables; + + return { content, childContext }; + } catch (e) { + const missingReferenceErrorMessage = `Missing reference in: ${node.attribs[ATTRIB_CWF]}`; + e.message += `\n${missingReferenceErrorMessage}`; + this._onError(e); + return utils.createErrorNode(node, e); + } + } + + + /** + * Extracts the content's page variables, then renders it. + * @param content to render + * @param contentFilePath of the content to render + * @param additionalVariables if any + */ + renderOuterVariables(content, contentFilePath, additionalVariables = {}) { + const pageVariables = this.extractPageVariables(contentFilePath, content, {}); + const userDefinedVariables = this.getContentParentSiteVariables(contentFilePath); + + return njUtil.renderRaw(content, { + ...pageVariables, + ...userDefinedVariables, + ...additionalVariables, + }); + } + + /** + * Extracts the content's page's inner variables, then renders it. + * @param content to render + * @param contentFilePath of the content to render + * @param contentAsIfAtFilePath + * @param additionalVariables if any + */ + renderInnerVariables(content, contentFilePath, contentAsIfAtFilePath, additionalVariables = {}) { + const userDefinedVariables = this.getContentParentSiteVariables(contentFilePath); + const innerVariables = this.getImportedVariableMap(contentAsIfAtFilePath); + + return njUtil.renderRaw(content, { + ...userDefinedVariables, + ...additionalVariables, + ...innerVariables, + }); + } + + /** + * Utility methods + */ + + getContentParentSiteVariables(contentFilePath) { + const parentSitePath = urlUtils.getParentSiteAbsolutePath(contentFilePath, this.rootPath, + this.baseUrlMap); + return this.userDefinedVariablesMap[parentSitePath]; + } +} + +module.exports = VariablePreprocessor; diff --git a/test/unit/Site.test.js b/test/unit/Site.test.js index a6f8a4f74c..d4009ad8ec 100644 --- a/test/unit/Site.test.js +++ b/test/unit/Site.test.js @@ -361,7 +361,7 @@ test('Site resolves variables referencing other variables', async () => { await site.collectBaseUrl(); await site.collectUserDefinedVariablesMap(); - const root = site.userDefinedVariablesMap[path.resolve('')]; + const root = site.variablePreprocessor.userDefinedVariablesMap[path.resolve('')]; // check all variables expect(root.level1).toEqual('variable'); @@ -391,10 +391,10 @@ test('Site read correct user defined variables', async () => { await site.collectBaseUrl(); await site.collectUserDefinedVariablesMap(); - const root = site.userDefinedVariablesMap[path.resolve('')]; - const sub = site.userDefinedVariablesMap[path.resolve('sub')]; - const subsub = site.userDefinedVariablesMap[path.resolve('sub/sub')]; - const othersub = site.userDefinedVariablesMap[path.resolve('otherSub/sub')]; + const root = site.variablePreprocessor.userDefinedVariablesMap[path.resolve('')]; + const sub = site.variablePreprocessor.userDefinedVariablesMap[path.resolve('sub')]; + const subsub = site.variablePreprocessor.userDefinedVariablesMap[path.resolve('sub/sub')]; + const othersub = site.variablePreprocessor.userDefinedVariablesMap[path.resolve('otherSub/sub')]; // check all baseUrls const baseUrl = '{{baseUrl}}'; diff --git a/test/unit/parser.test.js b/test/unit/parser.test.js index b51e04a5d0..fee0f8f8ca 100644 --- a/test/unit/parser.test.js +++ b/test/unit/parser.test.js @@ -1,6 +1,7 @@ const path = require('path'); const fs = require('fs'); const MarkBind = require('../../src/lib/markbind/src/parser.js'); +const VariablePreprocessor = require('../../src/lib/markbind/src/preprocessors/variablePreprocessor'); const { USER_VARIABLES_DEFAULT } = require('./utils/data'); jest.mock('fs'); @@ -9,11 +10,13 @@ jest.mock('../../src/util/logger'); afterEach(() => fs.vol.reset()); const ROOT_PATH = path.resolve(''); -const DEFAULT_USER_DEFINED_VARIABLES_MAP = {}; -DEFAULT_USER_DEFINED_VARIABLES_MAP[ROOT_PATH] = { - example: USER_VARIABLES_DEFAULT, - baseUrl: '{{baseUrl}}', -}; +function getNewDefaultVariablePreprocessor() { + const DEFAULT_VARIABLE_PREPROCESSOR = new VariablePreprocessor(ROOT_PATH, new Set([ROOT_PATH])); + DEFAULT_VARIABLE_PREPROCESSOR.addUserDefinedVariable(ROOT_PATH, 'example', USER_VARIABLES_DEFAULT); + DEFAULT_VARIABLE_PREPROCESSOR.addUserDefinedVariable(ROOT_PATH, 'baseUrl', '{{baseUrl}}'); + + return DEFAULT_VARIABLE_PREPROCESSOR; +} test('includeFile replaces with
', async () => { const indexPath = path.resolve('index.md'); @@ -40,7 +43,7 @@ test('includeFile replaces with
', async () => { const result = await markbinder.includeFile(indexPath, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, + variablePreprocessor: getNewDefaultVariablePreprocessor(), }); const expected = [ @@ -80,7 +83,7 @@ test('includeFile replaces with
', async const result = await markbinder.includeFile(indexPath, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, + variablePreprocessor: getNewDefaultVariablePreprocessor(), }); const expected = [ @@ -116,7 +119,7 @@ test('includeFile replaces with empty < const result = await markbinder.includeFile(indexPath, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, + variablePreprocessor: getNewDefaultVariablePreprocessor(), }); const expected = [ @@ -155,7 +158,7 @@ test('includeFile replaces with
', async const result = await markbinder.includeFile(indexPath, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, + variablePreprocessor: getNewDefaultVariablePreprocessor(), }); const expected = [ @@ -199,7 +202,7 @@ test('includeFile replaces with inline const result = await markbinder.includeFile(indexPath, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, + variablePreprocessor: getNewDefaultVariablePreprocessor(), }); const expected = [ @@ -239,7 +242,7 @@ test('includeFile replaces with trimmed c const result = await markbinder.includeFile(indexPath, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, + variablePreprocessor: getNewDefaultVariablePreprocessor(), }); const expected = [ @@ -286,7 +289,7 @@ test('includeFile replaces with error with
const result = await markbinder.includeFile(indexPath, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, + variablePreprocessor: getNewDefaultVariablePreprocessor(), }); const expected = [ @@ -365,7 +368,7 @@ test('includeFile replaces with const result = await markbinder.includeFile(indexPath, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, + variablePreprocessor: getNewDefaultVariablePreprocessor(), }); const expected = [ @@ -423,7 +426,7 @@ test('includeFile detects cyclic references for static cyclic includes', async ( const result = await markbinder.includeFile(indexPath, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, + variablePreprocessor: getNewDefaultVariablePreprocessor(), }); const expected = `
${expectedErrorMessage}
`;