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

@@ -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');

src/html/fetch-markdown.js

@@ -107,5 +107,7 @@ async function fetch(
   }
 }
 
+fetch.ext = 'fetch';
+
 module.exports = fetch;
 module.exports.uri = uri;

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';

src/html/get-metadata.js

@@ -166,4 +166,6 @@ function getmetadata({ content: { sections = [] } }, { logger }) {
   return { content: { meta: {} } };
 }
 
+getmetadata.ext = 'meta';
+
 module.exports = getmetadata;

src/html/make-html.js

@@ -23,4 +23,6 @@ function html({ content: { mdast }, request }, { logger, secrets }) {
   return { content };
 }
 
+html.ext = 'html';
+
 module.exports = html;

src/html/parse-markdown.js

@@ -63,4 +63,6 @@ function parse({ content: { body = '' } = {} }, { logger }) {
   return { content: { mdast } };
 }
 
+parse.ext = 'parse';
+
 module.exports = parse;

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.');

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/));
   });

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');
     };