From 461d3f64f48d3c055b304dda627c4199714ef207 Mon Sep 17 00:00:00 2001 From: Lars Trieloff Date: Tue, 19 Mar 2019 07:58:19 +0000 Subject: [PATCH] feat(pipeline): Introduced explicit, named extension points The previous iteration of pipeline extensibility exposed an implementation detail (the function name) to enable extensibility, implicitly making each step in the pipeline a potential extension point, and worse, part of the API contract. This change introduces an explicit extension point marker, called `ext`. It can be used in two ways: (a) by setting the `ext` property of the function that you want to make an extension point to the name of the extension point (b) by calling the `ext(name)` chain method on `Pipeline`, which will set the extension point name for the last function added to the pipeline. --- src/defaults/html.pipe.js | 14 ++++++++------ src/html/fetch-markdown.js | 2 ++ src/html/flag-esi.js | 3 +++ src/html/get-metadata.js | 2 ++ src/html/make-html.js | 2 ++ src/html/parse-markdown.js | 2 ++ src/pipeline.js | 28 +++++++++++++++++----------- test/testHTML.js | 11 +++++++++-- test/testPipeline.js | 12 ++++++++++++ 9 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/defaults/html.pipe.js b/src/defaults/html.pipe.js index 8870121fe..8799731b3 100644 --- a/src/defaults/html.pipe.js +++ b/src/defaults/html.pipe.js @@ -32,6 +32,11 @@ const { cache, uncached } = require('../html/shared-cache'); const embeds = require('../html/find-embeds'); /* eslint no-param-reassign: off */ +/* eslint newline-per-chained-call: off */ + +function hascontent({ content }) { + return !(content !== undefined && content.body !== undefined); +} const htmlpipe = (cont, payload, action) => { action.logger = action.logger || log; @@ -40,8 +45,7 @@ const htmlpipe = (cont, payload, action) => { pipe .every(dump).when(() => !production()) .every(validate).when(() => !production()) - .before(fetch) - .when(({ content }) => !(content !== undefined && content.body !== undefined)) + .before(fetch).when(hascontent) .before(parse) .before(embeds) .before(smartypants) @@ -53,12 +57,10 @@ const htmlpipe = (cont, payload, action) => { .before(responsive) .once(cont) .after(type('text/html')) - .after(cache) - .when(uncached) + .after(cache).when(uncached) .after(key) .after(debug) - .after(flag) - .when(esi) // flag ESI when there is ESI in the response + .after(flag).when(esi).ext('esi') // flag ESI when there is ESI in the response .error(selectStatus(production())); action.logger.log('debug', 'Running HTML pipeline'); diff --git a/src/html/fetch-markdown.js b/src/html/fetch-markdown.js index 35b3c6b2f..c5451d16d 100644 --- a/src/html/fetch-markdown.js +++ b/src/html/fetch-markdown.js @@ -107,5 +107,7 @@ async function fetch( } } +fetch.ext = 'fetch'; + module.exports = fetch; module.exports.uri = uri; diff --git a/src/html/flag-esi.js b/src/html/flag-esi.js index adeb250ce..dfc55cc51 100644 --- a/src/html/flag-esi.js +++ b/src/html/flag-esi.js @@ -34,3 +34,6 @@ function flag() { module.exports = { esi, flag, }; + +// make flag esi a pipeline extension point +module.exports.flag.ext = 'flag-esi'; diff --git a/src/html/get-metadata.js b/src/html/get-metadata.js index 1bcc9d36c..ec5975b68 100644 --- a/src/html/get-metadata.js +++ b/src/html/get-metadata.js @@ -166,4 +166,6 @@ function getmetadata({ content: { sections = [] } }, { logger }) { return { content: { meta: {} } }; } +getmetadata.ext = 'meta'; + module.exports = getmetadata; diff --git a/src/html/make-html.js b/src/html/make-html.js index 933a2352e..030732f5b 100644 --- a/src/html/make-html.js +++ b/src/html/make-html.js @@ -23,4 +23,6 @@ function html({ content: { mdast }, request }, { logger, secrets }) { return { content }; } +html.ext = 'html'; + module.exports = html; diff --git a/src/html/parse-markdown.js b/src/html/parse-markdown.js index 776b3879e..949111e4a 100644 --- a/src/html/parse-markdown.js +++ b/src/html/parse-markdown.js @@ -63,4 +63,6 @@ function parse({ content: { body = '' } = {} }, { logger }) { return { content: { mdast } }; } +parse.ext = 'parse'; + module.exports = parse; diff --git a/src/pipeline.js b/src/pipeline.js index ec440a3fb..e674d829e 100644 --- a/src/pipeline.js +++ b/src/pipeline.js @@ -115,21 +115,14 @@ class Pipeline { * @param {integer} before - where to insert the new function (true = before, false = after) */ this.attach.generic = (name, f, before = true) => { - const offset = before ? 0 : 2; - // see the describe function below for the generated names - // we parse the extracted name to get the matching extension - // point - const re = new RegExp(`^.*\\:${name} from `); - - // find the index of the function where the resolved alias + const offset = before ? 0 : 1; + // find the index of the function where the resolved ext name // matches the provided name by searching the list of pre and // post functions const foundpres = this._pres - .filter(pre => pre.alias) - .findIndex(pre => re.test(pre.alias)); + .findIndex(pre => pre && pre.ext && pre.ext === name); const foundposts = this._posts - .filter(post => post.alias) - .findIndex(post => re.test(post.alias)); + .findIndex(post => post && post.ext && post.ext === name); // if something has been found in either lists, insert the // new function into the list, with the correct offset @@ -139,6 +132,9 @@ class Pipeline { if (foundposts !== -1) { this._posts.splice(foundposts + offset, 0, f); } + if (foundpres === -1 && foundposts === -1) { + this._action.logger.warn(`Unknown extension point ${name}`); + } }; /** * Registers an extension to the pipeline. The function `f` will be run in @@ -194,6 +190,15 @@ class Pipeline { return this; } + /** + * Declares the last function that has been added to be a named extension point + * @param {string} name - name of the new extension point + */ + ext(name) { + this._last.slice(-1).pop().ext = name; + return this; + } + /** * Adds a condition to the previously defined `pre` or `post` function. The previously defined * function will only be executed if the predicate evaluates to something truthy or returns a @@ -222,6 +227,7 @@ class Pipeline { return args[0]; }; wrappedfunc.alias = lastfunc.alias; + wrappedfunc.ext = lastfunc.ext; this._last.push(wrappedfunc); } else { throw new Error('when() needs function to operate on.'); diff --git a/test/testHTML.js b/test/testHTML.js index 21522d60f..f99a6909e 100644 --- a/test/testHTML.js +++ b/test/testHTML.js @@ -166,6 +166,7 @@ ${content.document.body.innerHTML}`, let calledfoo = false; let calledbar = false; + let calledbaz = false; function foo() { assert.equal(calledfoo, false, 'foo has not yet been called'); assert.equal(calledbar, false, 'bar has not yet been called'); @@ -178,6 +179,10 @@ ${content.document.body.innerHTML}`, calledbar = true; } + function baz() { + calledbaz = true; + } + function shouttitle(p) { return { content: { @@ -191,10 +196,11 @@ ${content.document.body.innerHTML}`, }; myfunc.after = { - flag: bar, + esi: bar, // after the metadata has been retrived, make sure that // the title is being shouted - getmetadata: shouttitle, + meta: shouttitle, + never: baz, }; const res = await pipe( @@ -213,6 +219,7 @@ ${content.document.body.innerHTML}`, assert.equal(calledfoo, true, 'foo has been called'); assert.equal(calledbar, true, 'bar has been called'); + assert.equal(calledbaz, false, 'baz has never been called'); assert.ok(res.response.body.match(/HELLO WORLD/)); }); diff --git a/test/testPipeline.js b/test/testPipeline.js index d03b56ffb..1246db45d 100644 --- a/test/testPipeline.js +++ b/test/testPipeline.js @@ -75,6 +75,12 @@ describe('Testing Pipeline', () => { order.push('four'); }; + // inject explicit extension points + [first, second, third, fourth].forEach((f) => { + // eslint-disable-next-line no-param-reassign + f.ext = f.name; + }); + const pipe = new Pipeline({ logger }) .before(second) .once(() => { @@ -108,6 +114,12 @@ describe('Testing Pipeline', () => { order.push('four'); }; + // inject explicit extension points + [first, second, third, fourth].forEach((f) => { + // eslint-disable-next-line no-param-reassign + f.ext = f.name; + }); + const middle = function middle() { order.push('middle'); };