configurable options for headers & refactoring [fixes #91, 104] (#110)

* configurable options for headers & refactoring [fixes #91, 104] * make filterheaders more readable
zalando · Jan 18, 2017 · 44ce50a · 44ce50a
1 parent 07f07dc
commit 44ce50a
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 75 deletions.
diff --git a/README.md b/README.md
@@ -43,14 +43,18 @@ server.listen(process.env.PORT || 8080);
 
 ## Options
 
-* `fetchContext(request)` a function that returns a promise of the context, that is an object that maps fragment id to fragment url, to be able to override urls of the fragments on the page, defaults to `Promise.resolve({})`
-* `fetchTemplate(request, parseTemplate)` a function that should fetch the template, call `parseTemplate` and return a promise of the result. Useful to implement your own way to retrieve and cache the templates, e.g. from s3.
+* `fetchContext(request)` - Function that returns a promise of the context, that is an object that maps fragment id to fragment url, to be able to override urls of the fragments on the page, defaults to `Promise.resolve({})`
+* `fetchTemplate(request, parseTemplate)` - Function that should fetch the template, call `parseTemplate` and return a promise of the result. Useful to implement your own way to retrieve and cache the templates, e.g. from s3.
 Default implementation [`lib/fetch-template.js`](https://github.com/zalando/tailor/blob/master/lib/fetch-template.js) fetches the template from  the file system
-* `fragmentTag` a name of the fragment tag, defaults to `fragment`
-* `handledTags` an array of custom tags, check [`tests/handle-tag`](https://github.com/zalando/tailor/blob/master/tests/handle-tag.js) for more info
-* `handleTag(request, tag)` receives a tag or closing tag and serializes it to a string or returns a stream
-* `requestFragment(url, fragmentAttributes, request)` a function that returns a promise of request to a fragment server, check the default implementation in [`lib/request-fragment`](https://github.com/zalando/tailor/blob/master/lib/request-fragment.js)
+* `templatesPath` - To specify the path where the templates are stored locally, Defaults to `/templates/`
+* `fragmentTag` - Name of the fragment tag, defaults to `fragment`
+* `handledTags` - An array of custom tags, check [`tests/handle-tag`](https://github.com/zalando/tailor/blob/master/tests/handle-tag.js) for more info
+* `handleTag(request, tag)` - Receives a tag or closing tag and serializes it to a string or returns a stream
+* `filterHeaders(attributes, request)` - Function that filters the request headers that are passed to fragment request, check default implementation in [`lib/filter-headers`](https://github.com/zalando/tailor/blob/master/lib/filter-headers.js)
+* `requestFragment(filterHeaders)(url, attributes, request)` - Function that returns a promise of request to a fragment server, check the default implementation in [`lib/request-fragment`](https://github.com/zalando/tailor/blob/master/lib/request-fragment.js)
 * `amdLoaderUrl` - URL to AMD loader. We use [RequireJS from cdnjs](https://cdnjs.com/libraries/require.js) as deafult
+* `pipeInstanceName` - Pipe instance name that is available in the browser window for consuming frontend hooks.
+* `pipeAttributes(attributes)` - Function that returns the minimal set of fragment attributes available on the frontend [hooks](https://github.com/zalando/tailor/blob/master/docs/hooks.md).
 
 # Template
 

diff --git a/index.js b/index.js
@@ -7,24 +7,25 @@ const requestHandler = require('./lib/request-handler');
 const fetchTemplate = require('./lib/fetch-template');
 const parseTemplate = require('./lib/parse-template');
 const requestFragment = require('./lib/request-fragment');
+const filterHeadersFn = require('./lib/filter-headers');
 const PIPE_DEFINITION = fs.readFileSync(path.resolve(__dirname, 'src/pipe.min.js'));
 const AMD_LOADER_URL = 'https://cdnjs.cloudflare.com/ajax/libs/require.js/2.1.22/require.min.js';
 
 
 const stripUrl = (fileUrl) => path.normalize(fileUrl.replace('file://', ''));
 const getPipeAttributes = (attributes) => {
     const { primary, id } = attributes;
-    return { 
-        primary: !!(primary || primary === ''), 
-        id 
+    return {
+        primary: !!(primary || primary === ''),
+        id
     };
 };
 
 module.exports = class Tailor extends EventEmitter {
 
     constructor (options) {
         super();
-        const { amdLoaderUrl = AMD_LOADER_URL, templatesPath } = options;
+        const { amdLoaderUrl = AMD_LOADER_URL, templatesPath, filterHeaders = filterHeadersFn } = options;
         let memoizedDefinition;
         const pipeChunk = (amdLoaderUrl, pipeInstanceName) => {
             if (!memoizedDefinition) {
@@ -48,17 +49,17 @@ module.exports = class Tailor extends EventEmitter {
             fragmentTag: 'fragment',
             handledTags: [],
             handleTag: () => '',
-            requestFragment,
+            requestFragment: requestFragment(filterHeaders),
+            pipeInstanceName: 'Pipe',
             pipeDefinition: pipeChunk.bind(null, amdLoaderUrl),
-            pipeInstanceName: () => 'Pipe',
             pipeAttributes: getPipeAttributes
         }, options);
 
         requestOptions.parseTemplate = parseTemplate(
             [requestOptions.fragmentTag].concat(requestOptions.handledTags),
             ['script', requestOptions.fragmentTag]
         );
-        
+
         this.requestHandler = requestHandler.bind(this, requestOptions);
         // To Prevent from exiting the process - https://nodejs.org/api/events.html#events_error_events
         this.on('error', () => {});

diff --git a/lib/filter-headers.js b/lib/filter-headers.js
@@ -2,16 +2,22 @@
 const ACCEPT_HEADERS = ['accept-language', 'referer', 'user-agent'];
 /**
  * Filter the request headers that are passed to fragment request.
+ * @callback filterHeaders
  *
  * @param {Object} attributes - Attributes object of the fragment node
- * @param {string} attributes.public - Denotes the public fragemnt. Headers are not forward in this case
- * @param {Object} headers - Request header object
+ * @param {string} attributes.public - Denotes the public fragment.
+ * @param {Object} request - HTTP Request object
+ * @param {Object} request.headers - request header object
  * @returns {Object} New filtered header object
  */
-module.exports = ({ public: publicFragment }, headers) => 
-    publicFragment 
-        ? {} 
+module.exports = (attributes, request) => {
+    const { public: publicFragment } = attributes;
+    const { headers = {} } = request;
+    // Headers are not forwarded to public fragment for security reasons
+    return publicFragment
+        ? {}
         : ACCEPT_HEADERS.reduce((newHeaders, key) => {
             headers[key] && (newHeaders[key] = headers[key]);
             return newHeaders;
         }, {});
+};
diff --git a/lib/fragment.js b/lib/fragment.js
@@ -43,9 +43,16 @@ module.exports = class Fragment extends EventEmitter {
      * @param {object} context - Context object for the given fragment
      * @param {number} index - Order of the fragment
      * @param {function} requestFragment - Function to request the fragment
-     * @param {string} pipeInstanceName - Pipe instance name generated randomly and used in client side for the layout
+     * @param {string} pipeInstanceName - Pipe instance name that is available in the browser window for consuming hooks
      */
-    constructor (tag, context, index, requestFragment, pipeInstanceName, pipeAttributes) {
+    constructor ({
+        tag,
+        context,
+        index,
+        requestFragment,
+        pipeInstanceName,
+        pipeAttributes = () => {}
+    } = {}) {
         super();
         this.attributes = getAttributes(tag, context, index);
         ['async', 'primary', 'public'].forEach((key) => {
@@ -57,8 +64,7 @@ module.exports = class Fragment extends EventEmitter {
             }
             this.attributes[key] = value;
         });
-        pipeAttributes = pipeAttributes || (() => {});
-        this.pipeAttributes = pipeAttributes(Object.assign({}, {id: index}, tag.attributes));
+        this.pipeAttributes = pipeAttributes(Object.assign({}, { id: index }, tag.attributes));
         this.index = index;
         this.requestFragment = requestFragment;
         this.pipeInstanceName = pipeInstanceName;

diff --git a/lib/request-fragment.js b/lib/request-fragment.js
@@ -3,7 +3,6 @@
 const http = require('http');
 const https = require('https');
 const url = require('url');
-const filterHeaders = require('./filter-headers');
 // By default tailor supports gzipped response from fragments
 const requiredHeaders = {
     'accept-encoding': 'gzip, deflate'
@@ -14,35 +13,37 @@ const requiredHeaders = {
  *  - filtered headers
  *  - Specified timeout from fragment attributes
  *
+ * @param {filterHeaders} - Function that handles the header forwarding
  * @param {string} fragmentUrl - URL of the fragment server
  * @param {Object} fragmentAttributes - Attributes passed via fragment tags
  * @param {Object} request - HTTP request stream
  * @returns {Promise} Response from the fragment server
  */
-module.exports = (fragmentUrl, fragmentAttributes, request) =>
-    new Promise((resolve, reject) => {
-        const parsedUrl = url.parse(fragmentUrl);
-        const options = Object.assign({
-            headers: Object.assign(
-                filterHeaders(fragmentAttributes, request.headers),
-                requiredHeaders
-            ),
-            keepAlive: true,
-            timeout: fragmentAttributes.timeout
-        }, parsedUrl);
-        const { protocol: reqProtocol, timeout } = options;
-        const protocol = reqProtocol === 'https:' ? https : http;
-        const fragmentRequest = protocol.request(options);
-        if (timeout) {
-            fragmentRequest.setTimeout(timeout, fragmentRequest.abort);
-        }
-        fragmentRequest.on('response', (response) => {
-            if (response.statusCode >= 500) {
-                reject(new Error('Internal Server Error'));
-            } else {
-                resolve(response);
+module.exports = (filterHeaders) =>
+    (fragmentUrl, fragmentAttributes, request) =>
+        new Promise((resolve, reject) => {
+            const parsedUrl = url.parse(fragmentUrl);
+            const options = Object.assign({
+                headers: Object.assign(
+                    filterHeaders(fragmentAttributes, request),
+                    requiredHeaders
+                ),
+                keepAlive: true,
+                timeout: fragmentAttributes.timeout
+            }, parsedUrl);
+            const { protocol: reqProtocol, timeout } = options;
+            const protocol = reqProtocol === 'https:' ? https : http;
+            const fragmentRequest = protocol.request(options);
+            if (timeout) {
+                fragmentRequest.setTimeout(timeout, fragmentRequest.abort);
             }
+            fragmentRequest.on('response', (response) => {
+                if (response.statusCode >= 500) {
+                    reject(new Error('Internal Server Error'));
+                } else {
+                    resolve(response);
+                }
+            });
+            fragmentRequest.on('error', reject);
+            fragmentRequest.end();
         });
-        fragmentRequest.on('error', reject);
-        fragmentRequest.end();
-    });
diff --git a/lib/request-handler.js b/lib/request-handler.js
@@ -16,10 +16,9 @@ const { TEMPLATE_NOT_FOUND } = require('./fetch-template');
 module.exports = function processRequest (options, request, response) {
     this.emit('start', request);
 
-    const { fetchContext, fetchTemplate, handleTag, 
+    const { fetchContext, fetchTemplate, handleTag,
             parseTemplate, requestFragment, fragmentTag,
             pipeAttributes, pipeInstanceName, pipeDefinition} = options;
-    const pipeName = pipeInstanceName();
 
     const asyncStream = new AsyncStream();
     const contextPromise = fetchContext(request).catch((err) => {
@@ -46,7 +45,7 @@ module.exports = function processRequest (options, request, response) {
         const resultStream = new StringifierStream((tag) => {
             const { placeholder, name, } = tag;
             if (placeholder === 'pipe') {
-                return pipeDefinition(pipeName);
+                return pipeDefinition(pipeInstanceName);
             }
 
             if (placeholder === 'async') {
@@ -55,14 +54,14 @@ module.exports = function processRequest (options, request, response) {
             }
 
             if (name === fragmentTag) {
-                const fragment = new Fragment(
+                const fragment = new Fragment({
                     tag,
                     context,
-                    index++,
+                    index: index++,
                     requestFragment,
-                    pipeName,
+                    pipeInstanceName,
                     pipeAttributes
-                );
+                });
 
                 FRAGMENT_EVENTS.forEach((eventName) => {
                     fragment.on(eventName, (...args) => {

diff --git a/tests/filter-headers.js b/tests/filter-headers.js
@@ -14,11 +14,11 @@ describe('filter-headers', () => {
 
     it('keeps only certain headers', () => {
         const after = {'accept-language': '1', 'referer': '2', 'user-agent': '3'};
-        assert.deepEqual(filterHeaders({}, headers), after);
+        assert.deepEqual(filterHeaders({}, { headers }), after);
     });
 
     it('removes headers if fragment is public', () => {
-        assert.deepEqual(filterHeaders({public: true}, headers), {});
+        assert.deepEqual(filterHeaders({ public: true }, { headers }), {});
     });
 
 });
diff --git a/tests/fragment.events.js b/tests/fragment.events.js
@@ -4,26 +4,33 @@ const assert = require('assert');
 const nock = require('nock');
 const TAG = {attributes: {src: 'https://fragment'}};
 const TAG_FALLBACK = {attributes: {src: 'https://fragment', 'fallback-src': 'https://fallback-fragment'}};
-const REQUEST = {
-    headers: {}
-};
+const REQUEST = { headers: {} };
 const RESPONSE_HEADERS = {connection: 'close'};
+const filterHeaderFn = () => ({});
 const sinon = require('sinon');
 const requestFragment = require('../lib/request-fragment');
+const getOptions = (tag) => {
+    return {
+        tag,
+        context: {},
+        index: false,
+        requestFragment: requestFragment(filterHeaderFn)
+    };
+};
 
 describe('Fragment events', () => {
 
     it('triggers `start` event', (done) => {
         nock('https://fragment').get('/').reply(200, 'OK');
-        const fragment = new Fragment(TAG, {}, false, requestFragment);
+        const fragment = new Fragment(getOptions(TAG));
         fragment.on('start', done);
         fragment.fetch(REQUEST, false);
     });
 
     it('triggers `fallback` event', (done) => {
         nock('https://fragment').get('/').reply(500, 'Server Error');
         nock('https://fallback-fragment').get('/').reply(200, 'OK');
-        const fragment = new Fragment(TAG_FALLBACK, {}, false, requestFragment);
+        const fragment = new Fragment(getOptions(TAG_FALLBACK));
         fragment.on('fallback', () => {
             done();
         });
@@ -36,7 +43,7 @@ describe('Fragment events', () => {
 
         nock('https://fragment').get('/').reply(500, 'Server Error');
         nock('https://fallback-fragment').get('/').reply(200);
-        const fragment = new Fragment(TAG_FALLBACK, {}, false, requestFragment);
+        const fragment = new Fragment(getOptions(TAG_FALLBACK));
         fragment.on('fallback',onFallback);
         fragment.on('error', onError);
         fragment.stream.on('end', () => {
@@ -50,7 +57,7 @@ describe('Fragment events', () => {
 
     it('triggers `response(statusCode, headers)` when received headers', (done) => {
         nock('https://fragment').get('/').reply(200, 'OK', RESPONSE_HEADERS);
-        const fragment = new Fragment(TAG, {}, false, requestFragment);
+        const fragment = new Fragment(getOptions(TAG));
         fragment.on('response', (statusCode, headers) => {
             assert.equal(statusCode, 200);
             assert.deepEqual(headers, RESPONSE_HEADERS);
@@ -61,7 +68,7 @@ describe('Fragment events', () => {
 
     it('triggers `end(contentSize)` when the content is succesfully retreived', (done) => {
         nock('https://fragment').get('/').reply(200, '12345');
-        const fragment = new Fragment(TAG, {}, false, requestFragment);
+        const fragment = new Fragment(getOptions(TAG));
         fragment.on('end', (contentSize) => {
             assert.equal(contentSize, 5);
             done();
@@ -72,7 +79,7 @@ describe('Fragment events', () => {
 
     it('triggers `error(error)` when fragment responds with 50x', (done) => {
         nock('https://fragment').get('/').reply(500);
-        const fragment = new Fragment(TAG, {}, false, requestFragment);
+        const fragment = new Fragment(getOptions(TAG));
         fragment.on('error', (error) => {
             assert.ok(error);
             done();
@@ -85,7 +92,7 @@ describe('Fragment events', () => {
         const onEnd = sinon.spy();
         const onFallback = sinon.spy();
         nock('https://fragment').get('/').reply(500);
-        const fragment = new Fragment(TAG_FALLBACK, {}, false, requestFragment);
+        const fragment = new Fragment(getOptions(TAG_FALLBACK));
         fragment.on('response', onResponse);
         fragment.on('end', onEnd);
         fragment.on('fallback', onFallback);
@@ -104,7 +111,7 @@ describe('Fragment events', () => {
         const onEnd = sinon.spy();
         const onError = sinon.spy();
         nock('https://fragment').get('/').reply(500);
-        const fragment = new Fragment(TAG, {}, false, requestFragment);
+        const fragment = new Fragment(getOptions(TAG));
         fragment.on('response', onResponse);
         fragment.on('end', onEnd);
         fragment.on('error', onError);
@@ -126,7 +133,7 @@ describe('Fragment events', () => {
         nock('https://fragment')
             .get('/')
             .replyWithError(ERROR);
-        const fragment = new Fragment(TAG, {}, false, requestFragment);
+        const fragment = new Fragment(getOptions(TAG));
         fragment.on('error', (error) => {
             assert.equal(error.message, ERROR.message);
             done();
@@ -137,7 +144,7 @@ describe('Fragment events', () => {
     it('triggers `error(error)` when fragment times out', (done) => {
         nock('https://fragment').get('/').socketDelay(101).reply(200);
         const tag = {attributes: {src: 'https://fragment', timeout: '100'}};
-        const fragment = new Fragment(tag, {}, false, requestFragment);
+        const fragment = new Fragment(getOptions(tag));
         fragment.on('error', (err) => {
             assert.equal(err.message, 'Request aborted');
             done();

diff --git a/tests/request-fragment.js b/tests/request-fragment.js
@@ -1,8 +1,9 @@
 'use strict';
 
-const requestFragment = require('../lib/request-fragment');
 const assert = require('assert');
 const nock = require('nock');
+const filterHeaderFn = () => ({});
+const requestFragment = require('../lib/request-fragment')(filterHeaderFn);
 
 describe('requestFragment', () => {