Skip to content
This repository was archived by the owner on Sep 3, 2022. It is now read-only.

Commit 497822c

Browse files
aultimusMatthew Ault
andauthored
Add dest mw take two (#150)
* 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(); * feat: add addDestinationMiddleware function WIP * feat: correct doc * feat: update addDestinationMiddleware docs * feat: update dest mw signature to be consistent with source mw * chore: fix syntax error * chore: fix failing tests * chore: fix failing test by enabling integration * feat: fix multiple invoke events being emitted * chore: touch HISTORY.md to fix build error * chore: fix HISTORY.md * chore: resolve review comments * chore: bump package version Co-authored-by: Matthew Ault <[email protected]>
1 parent 41c1285 commit 497822c

File tree

5 files changed

+130
-6
lines changed

5 files changed

+130
-6
lines changed

HISTORY.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
# 3.13.3 / 2020-05-26
2+
3+
- feat: add destination middleware
4+
15
# 3.13.2 / 2020-05-21
26

37
- fix: null values should delete cookies

lib/analytics.js

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ var Identify = require('segmentio-facade').Identify;
1414
var SourceMiddlewareChain = require('./middleware').SourceMiddlewareChain;
1515
var IntegrationMiddlewareChain = require('./middleware')
1616
.IntegrationMiddlewareChain;
17+
var DestinationMiddlewareChain = require('./middleware')
18+
.DestinationMiddlewareChain;
1719
var Page = require('segmentio-facade').Page;
1820
var Track = require('segmentio-facade').Track;
1921
var bindAll = require('bind-all');
@@ -50,6 +52,7 @@ function Analytics() {
5052
this.Integrations = {};
5153
this._sourceMiddlewares = new SourceMiddlewareChain();
5254
this._integrationMiddlewares = new IntegrationMiddlewareChain();
55+
this._destinationMiddlewares = {};
5356
this._integrations = {};
5457
this._readied = false;
5558
this._timeout = 300;
@@ -111,6 +114,7 @@ Analytics.prototype.addSourceMiddleware = function(middleware) {
111114

112115
/**
113116
* Define a new `IntegrationMiddleware`
117+
* DEPRECATED
114118
*
115119
* @param {Function} Middleware
116120
* @return {Analytics}
@@ -121,6 +125,32 @@ Analytics.prototype.addIntegrationMiddleware = function(middleware) {
121125
return this;
122126
};
123127

128+
/**
129+
* Define a new `DestinationMiddleware`
130+
* Destination Middleware is chained after integration middleware
131+
*
132+
* @param {String} integrationName
133+
* @param {Array} Middlewares
134+
* @return {Analytics}
135+
*/
136+
137+
Analytics.prototype.addDestinationMiddleware = function(
138+
integrationName,
139+
middlewares
140+
) {
141+
var self = this;
142+
middlewares.forEach(function(middleware) {
143+
if (!self._destinationMiddlewares[integrationName]) {
144+
self._destinationMiddlewares[
145+
integrationName
146+
] = new DestinationMiddlewareChain();
147+
}
148+
149+
self._destinationMiddlewares[integrationName].add(middleware);
150+
});
151+
return self;
152+
};
153+
124154
/**
125155
* Initialize with the given integration `settings` and `options`.
126156
*
@@ -791,12 +821,43 @@ Analytics.prototype._invoke = function(method, facade) {
791821
result = new Facade(result);
792822
}
793823

794-
metrics.increment('analytics_js.integration.invoke', {
795-
method: method,
796-
integration_name: integration.name
797-
});
824+
// apply destination middlewares
825+
// Apply any integration middlewares that exist, then invoke the integration with the result.
826+
if (self._destinationMiddlewares[integration.name]) {
827+
self._destinationMiddlewares[integration.name].applyMiddlewares(
828+
facadeCopy,
829+
integration.name,
830+
function(result) {
831+
// A nullified payload should not be sent to an integration.
832+
if (result === null) {
833+
self.log(
834+
'Payload to destination "%s" was null and dropped by a middleware.',
835+
name
836+
);
837+
return;
838+
}
839+
840+
// Check if the payload is still a Facade. If not, convert it to one.
841+
if (!(result instanceof Facade)) {
842+
result = new Facade(result);
843+
}
844+
845+
metrics.increment('analytics_js.integration.invoke', {
846+
method: method,
847+
integration_name: integration.name
848+
});
849+
850+
integration.invoke.call(integration, method, result);
851+
}
852+
);
853+
} else {
854+
metrics.increment('analytics_js.integration.invoke', {
855+
method: method,
856+
integration_name: integration.name
857+
});
798858

799-
integration.invoke.call(integration, method, result);
859+
integration.invoke.call(integration, method, result);
860+
}
800861
}
801862
);
802863
} catch (e) {

lib/middleware.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,20 @@ module.exports.IntegrationMiddlewareChain = function IntegrationMiddlewareChain(
3434
};
3535
};
3636

37+
module.exports.DestinationMiddlewareChain = function DestinationMiddlewareChain() {
38+
var apply = middlewareChain(this);
39+
40+
this.applyMiddlewares = function(facade, integration, callback) {
41+
return apply(
42+
function(mw, payload, next) {
43+
mw({ payload: payload, integration: integration, next: next });
44+
},
45+
facade,
46+
callback
47+
);
48+
};
49+
};
50+
3751
// Chain is essentially a linked list of middlewares to run in order.
3852
function middlewareChain(dest) {
3953
var middlewares = [];

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@segment/analytics.js-core",
33
"author": "Segment <[email protected]>",
4-
"version": "3.13.2",
4+
"version": "3.13.3",
55
"description": "The hassle-free way to integrate analytics into any web application.",
66
"keywords": [
77
"analytics",

test/analytics.test.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2058,6 +2058,51 @@ describe('Analytics', function() {
20582058
});
20592059
});
20602060

2061+
describe('#addDestinationMiddleware', function() {
2062+
it('should have a defined _integrationMiddlewares property', function() {
2063+
assert(analytics._destinationMiddlewares !== undefined);
2064+
});
2065+
2066+
it('should allow users to add a valid Middleware', function() {
2067+
try {
2068+
analytics.addDestinationMiddleware('foo', [function() {}]);
2069+
} catch (e) {
2070+
// This assert should not run.
2071+
assert(false, 'error was incorrectly thrown!');
2072+
}
2073+
});
2074+
2075+
it('should throw an error if the selected Middleware is not a function', function() {
2076+
try {
2077+
analytics.addDestinationMiddleware('foo', [7]);
2078+
2079+
// This assert should not run.
2080+
assert(false, 'error was not thrown!');
2081+
} catch (e) {
2082+
assert(
2083+
e.message === 'attempted to add non-function middleware',
2084+
'wrong error return'
2085+
);
2086+
}
2087+
});
2088+
2089+
it('should not throw an error if AJS has already initialized', function() {
2090+
analytics.init();
2091+
try {
2092+
analytics.addDestinationMiddleware('foo', [function() {}]);
2093+
} catch (e) {
2094+
// This assert should not run.
2095+
assert(false, 'error was thrown!');
2096+
}
2097+
});
2098+
2099+
it('should return the analytics object', function() {
2100+
assert(
2101+
analytics === analytics.addDestinationMiddleware('foo', [function() {}])
2102+
);
2103+
});
2104+
});
2105+
20612106
describe('#addSourceMiddleware', function() {
20622107
it('should have a defined _sourceMiddlewares property', function() {
20632108
assert(analytics._sourceMiddlewares !== undefined);

0 commit comments

Comments
 (0)