From c296fe3861135fda5aa254741ce8480a3f31c4fb Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Wed, 13 May 2020 13:40:07 -0700 Subject: [PATCH 01/13] feat: fix bug whereby changes made by integration mw were not persisted This bug meant that this code did not send an event with a userId=bar to segment.io. This change will result in this code sending an event with userId=bar to segment.io: f=function(payload, integration, next) { payload.obj.userId = "bar";next(payload); }; analytics.addIntegrationMiddleware(f); analytics.identify(); --- lib/analytics.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/analytics.js b/lib/analytics.js index 5f1d5c14..2faacad0 100644 --- a/lib/analytics.js +++ b/lib/analytics.js @@ -734,11 +734,6 @@ Analytics.prototype._invoke = function(method, facade) { result = new Facade(result); } - self.emit('invoke', result); - metrics.increment('analytics_js.invoke', { - method: method - }); - applyIntegrationMiddlewares(result); } ); @@ -791,6 +786,11 @@ Analytics.prototype._invoke = function(method, facade) { result = new Facade(result); } + self.emit('invoke', result); + metrics.increment('analytics_js.invoke', { + method: method + }); + metrics.increment('analytics_js.integration.invoke', { method: method, integration_name: integration.name From 8e2508706aa5cdaabf085c1687c90e18cdd40905 Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Tue, 12 May 2020 10:21:56 -0700 Subject: [PATCH 02/13] feat: add addDestinationMiddleware function WIP --- lib/analytics.js | 85 ++++++++++++++++++++++++++++++++++++------ test/analytics.test.js | 45 ++++++++++++++++++++++ 2 files changed, 119 insertions(+), 11 deletions(-) diff --git a/lib/analytics.js b/lib/analytics.js index 2faacad0..e1e4b51c 100644 --- a/lib/analytics.js +++ b/lib/analytics.js @@ -50,6 +50,7 @@ function Analytics() { this.Integrations = {}; this._sourceMiddlewares = new SourceMiddlewareChain(); this._integrationMiddlewares = new IntegrationMiddlewareChain(); + this._destinationMiddlewares = {}; this._integrations = {}; this._readied = false; this._timeout = 300; @@ -111,6 +112,7 @@ Analytics.prototype.addSourceMiddleware = function(middleware) { /** * Define a new `IntegrationMiddleware` + * DEPRECATED * * @param {Function} Middleware * @return {Analytics} @@ -121,6 +123,31 @@ Analytics.prototype.addIntegrationMiddleware = function(middleware) { return this; }; +/** + * Define a new `DestinationMiddleware` + * Destination Middleware and Integration middleware cannot be chained together. + * + * @param {Function} Middleware + * @return {Analytics} + */ + +Analytics.prototype.addDestinationMiddleware = function( + integrationName, + middlewares +) { + self = this; + middlewares.forEach(function(middleware) { + if (!self._destinationMiddlewares[integrationName]) { + self._destinationMiddlewares[ + integrationName + ] = new IntegrationMiddlewareChain(); + } + + self._destinationMiddlewares[integrationName].add(middleware); + }); + return self; +}; + /** * Initialize with the given integration `settings` and `options`. * @@ -786,17 +813,53 @@ Analytics.prototype._invoke = function(method, facade) { result = new Facade(result); } - self.emit('invoke', result); - metrics.increment('analytics_js.invoke', { - method: method - }); - - metrics.increment('analytics_js.integration.invoke', { - method: method, - integration_name: integration.name - }); - - integration.invoke.call(integration, method, result); + // apply destination middlewares + // Apply any integration middlewares that exist, then invoke the integration with the result. + if (self._destinationMiddlewares[integration.name]) { + self._destinationMiddlewares[integration.name].applyMiddlewares( + facadeCopy, + integration.name, + function(result) { + // A nullified payload should not be sent to an integration. + if (result === null) { + self.log( + 'Payload to destination "%s" was null and dropped by a middleware.', + name + ); + return; + } + + // Check if the payload is still a Facade. If not, convert it to one. + if (!(result instanceof Facade)) { + result = new Facade(result); + } + + self.emit('invoke', result); + metrics.increment('analytics_js.invoke', { + method: method + }); + + metrics.increment('analytics_js.integration.invoke', { + method: method, + integration_name: integration.name + }); + + integration.invoke.call(integration, method, result); + } + ); + } else { + self.emit('invoke', result); + metrics.increment('analytics_js.invoke', { + method: method + }); + + metrics.increment('analytics_js.integration.invoke', { + method: method, + integration_name: integration.name + }); + + integration.invoke.call(integration, method, result); + } } ); } catch (e) { diff --git a/test/analytics.test.js b/test/analytics.test.js index 897d6870..4db0a6b5 100644 --- a/test/analytics.test.js +++ b/test/analytics.test.js @@ -2058,6 +2058,51 @@ describe('Analytics', function() { }); }); + describe('#addDestinationMiddleware', function() { + it('should have a defined _integrationMiddlewares property', function() { + assert(analytics._destinationMiddlewares !== undefined); + }); + + it('should allow users to add a valid Middleware', function() { + try { + analytics.addDestinationMiddleware('foo', [function() {}]); + } catch (e) { + // This assert should not run. + assert(false, 'error was incorrectly thrown!'); + } + }); + + it('should throw an error if the selected Middleware is not a function', function() { + try { + analytics.addDestinationMiddleware('foo', [7]); + + // This assert should not run. + assert(false, 'error was not thrown!'); + } catch (e) { + assert( + e.message === 'attempted to add non-function middleware', + 'wrong error return' + ); + } + }); + + it('should not throw an error if AJS has already initialized', function() { + analytics.init(); + try { + analytics.addDestinationMiddleware('foo', [function() {}]); + } catch (e) { + // This assert should not run. + assert(false, 'error was thrown!'); + } + }); + + it('should return the analytics object', function() { + assert( + analytics === analytics.addDestinationMiddleware('foo', [function() {}]) + ); + }); + }); + describe('#addSourceMiddleware', function() { it('should have a defined _sourceMiddlewares property', function() { assert(analytics._sourceMiddlewares !== undefined); From 5a8c0a3cda35012724b9360ff273cb6ae5754056 Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Wed, 13 May 2020 14:45:20 -0700 Subject: [PATCH 03/13] feat: correct doc --- lib/analytics.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/analytics.js b/lib/analytics.js index e1e4b51c..2e63d656 100644 --- a/lib/analytics.js +++ b/lib/analytics.js @@ -125,7 +125,7 @@ Analytics.prototype.addIntegrationMiddleware = function(middleware) { /** * Define a new `DestinationMiddleware` - * Destination Middleware and Integration middleware cannot be chained together. + * Destination Middleware is chained after integration middleware * * @param {Function} Middleware * @return {Analytics} From 679378042d8fccfbe64b33ff962c7d22d13f5ef6 Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Wed, 13 May 2020 15:02:25 -0700 Subject: [PATCH 04/13] feat: update addDestinationMiddleware docs --- lib/analytics.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/analytics.js b/lib/analytics.js index 2e63d656..b94cb52d 100644 --- a/lib/analytics.js +++ b/lib/analytics.js @@ -127,7 +127,8 @@ Analytics.prototype.addIntegrationMiddleware = function(middleware) { * Define a new `DestinationMiddleware` * Destination Middleware is chained after integration middleware * - * @param {Function} Middleware + * @param {String} integrationName + * @param {Array} Middlewares * @return {Analytics} */ From 02a8905aa6c3ab72d87f13ff24a0120de0b29869 Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Wed, 13 May 2020 16:50:20 -0700 Subject: [PATCH 05/13] feat: update dest mw signature to be consistent with source mw --- lib/analytics.js | 4 +++- lib/middleware.js | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/analytics.js b/lib/analytics.js index b94cb52d..cffed69c 100644 --- a/lib/analytics.js +++ b/lib/analytics.js @@ -14,6 +14,8 @@ var Identify = require('segmentio-facade').Identify; var SourceMiddlewareChain = require('./middleware').SourceMiddlewareChain; var IntegrationMiddlewareChain = require('./middleware') .IntegrationMiddlewareChain; +var DestinationMiddlewareChain = require('./middleware') + .DestinationMiddlewareChain; var Page = require('segmentio-facade').Page; var Track = require('segmentio-facade').Track; var bindAll = require('bind-all'); @@ -141,7 +143,7 @@ Analytics.prototype.addDestinationMiddleware = function( if (!self._destinationMiddlewares[integrationName]) { self._destinationMiddlewares[ integrationName - ] = new IntegrationMiddlewareChain(); + ] = new DestinationMiddlewareChain(); } self._destinationMiddlewares[integrationName].add(middleware); diff --git a/lib/middleware.js b/lib/middleware.js index 308a8100..8f13e68f 100644 --- a/lib/middleware.js +++ b/lib/middleware.js @@ -34,6 +34,20 @@ module.exports.IntegrationMiddlewareChain = function IntegrationMiddlewareChain( }; }; +module.exports.DestinationMiddlewareChain = function DestinationMiddlewareChain() { + var apply = middlewareChain(this); + + this.applyMiddlewares = function(facade, integration, callback) { + return apply( + function(mw, payload, next) { + mw({ payload, integration, next }); + }, + facade, + callback + ); + }; +}; + // Chain is essentially a linked list of middlewares to run in order. function middlewareChain(dest) { var middlewares = []; From 533867793400b2052343a4c6a873860ad6e35015 Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Mon, 18 May 2020 09:02:12 -0700 Subject: [PATCH 06/13] chore: fix syntax error --- lib/middleware.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/middleware.js b/lib/middleware.js index 8f13e68f..39a2a6ce 100644 --- a/lib/middleware.js +++ b/lib/middleware.js @@ -40,7 +40,7 @@ module.exports.DestinationMiddlewareChain = function DestinationMiddlewareChain( this.applyMiddlewares = function(facade, integration, callback) { return apply( function(mw, payload, next) { - mw({ payload, integration, next }); + mw({ payload: payload, integration: integration, next: next }); }, facade, callback From 723127c895b3302249407b905317a1a5f8fb122f Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Tue, 19 May 2020 09:24:20 -0700 Subject: [PATCH 07/13] chore: fix failing tests --- lib/analytics.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/analytics.js b/lib/analytics.js index cffed69c..e425d96b 100644 --- a/lib/analytics.js +++ b/lib/analytics.js @@ -138,7 +138,7 @@ Analytics.prototype.addDestinationMiddleware = function( integrationName, middlewares ) { - self = this; + var self = this; middlewares.forEach(function(middleware) { if (!self._destinationMiddlewares[integrationName]) { self._destinationMiddlewares[ From bc1397b28c8002ad7401adcadad866a0241aa7cd Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Wed, 20 May 2020 09:21:53 -0700 Subject: [PATCH 08/13] chore: fix failing test by enabling integration --- test/analytics.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/analytics.test.js b/test/analytics.test.js index 4db0a6b5..b719ced0 100644 --- a/test/analytics.test.js +++ b/test/analytics.test.js @@ -495,7 +495,7 @@ describe('Analytics', function() { }); it('should emit "invoke" with facade', function(done) { - var opts = { All: false }; + var opts = { All: true }; var identify = new Identify({ testVal: 'success', options: opts }); analytics.on('invoke', function(msg) { assert(msg instanceof Facade); From 010219b675e859d35deafaf8c1e387461839c694 Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Wed, 20 May 2020 13:13:33 -0700 Subject: [PATCH 09/13] feat: fix multiple invoke events being emitted --- lib/analytics.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/analytics.js b/lib/analytics.js index e425d96b..b70c7cdc 100644 --- a/lib/analytics.js +++ b/lib/analytics.js @@ -764,6 +764,11 @@ Analytics.prototype._invoke = function(method, facade) { result = new Facade(result); } + self.emit('invoke', result); + metrics.increment('analytics_js.invoke', { + method: method + }); + applyIntegrationMiddlewares(result); } ); @@ -837,11 +842,6 @@ Analytics.prototype._invoke = function(method, facade) { result = new Facade(result); } - self.emit('invoke', result); - metrics.increment('analytics_js.invoke', { - method: method - }); - metrics.increment('analytics_js.integration.invoke', { method: method, integration_name: integration.name @@ -851,11 +851,6 @@ Analytics.prototype._invoke = function(method, facade) { } ); } else { - self.emit('invoke', result); - metrics.increment('analytics_js.invoke', { - method: method - }); - metrics.increment('analytics_js.integration.invoke', { method: method, integration_name: integration.name From 094ef73fe590ca948b6fc38e3898f41e972627b8 Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Wed, 20 May 2020 15:50:30 -0700 Subject: [PATCH 10/13] chore: touch HISTORY.md to fix build error From 114c771a819ddbe5b4045bca15f08afec1434a52 Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Wed, 20 May 2020 15:58:41 -0700 Subject: [PATCH 11/13] chore: fix HISTORY.md --- HISTORY.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index 2214bce1..2c6dc910 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,6 @@ +- Revert "Add destination mw (#148)" +- Add destination mw (#148) + # 3.13.1 / 2020-05-04 - refactor: remove third party dependency ndhoule/each From fccfece9c33838866a52cb792e85cc3f3060d51b Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Tue, 26 May 2020 09:18:40 -0700 Subject: [PATCH 12/13] chore: resolve review comments --- HISTORY.md | 1 - test/analytics.test.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 2c6dc910..ac4d00e6 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,4 +1,3 @@ -- Revert "Add destination mw (#148)" - Add destination mw (#148) # 3.13.1 / 2020-05-04 diff --git a/test/analytics.test.js b/test/analytics.test.js index b719ced0..4db0a6b5 100644 --- a/test/analytics.test.js +++ b/test/analytics.test.js @@ -495,7 +495,7 @@ describe('Analytics', function() { }); it('should emit "invoke" with facade', function(done) { - var opts = { All: true }; + var opts = { All: false }; var identify = new Identify({ testVal: 'success', options: opts }); analytics.on('invoke', function(msg) { assert(msg instanceof Facade); From e6797dd65f4014543f5e4b68932d3542bb0e85d8 Mon Sep 17 00:00:00 2001 From: Matthew Ault Date: Tue, 26 May 2020 10:47:02 -0700 Subject: [PATCH 13/13] chore: bump package version --- HISTORY.md | 4 ++++ package.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/HISTORY.md b/HISTORY.md index ab571e33..96129b33 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,7 @@ +# 3.13.3 / 2020-05-26 + +- feat: add destination middleware + # 3.13.2 / 2020-05-21 - fix: null values should delete cookies diff --git a/package.json b/package.json index cf764d9d..54c2cadc 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@segment/analytics.js-core", "author": "Segment ", - "version": "3.13.2", + "version": "3.13.3", "description": "The hassle-free way to integrate analytics into any web application.", "keywords": [ "analytics",