8000 fix(project config): Don't mutate the datafile object (#462) · optimizely/javascript-sdk@862408c · GitHub
[go: up one dir, main page]
Skip to content

Commit 862408c

Browse files
mjc1283mjc1283
authored
fix(project config): Don't mutate the datafile object (#462)
Summary: createProjectConfig was destructively mutating its datafile argument. After that mutation, the datafile object could not be re-used to create another instance. With this change, createProjectConfig copies the objects it needs to mutate and mutates only those copies. It's not a full deep clone; it only shallowly clones audiences, experiments, feature flags, groups, rollouts, group experiments, and rollout experiments. Test plan: - Updated & new unit tests - Compatibility suite should pass Issues: https://optimizely.atlassian.net/browse/OASIS-6301
1 parent a093132 commit 862408c

File tree

3 files changed

+65
-3
lines changed

3 files changed

+65
-3
lines changed

‎packages/optimizely-sdk/lib/core/project_config/index.js

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,41 @@ var EXPERIMENT_RUNNING_STATUS = 'Running';
2828
var RESERVED_ATTRIBUTE_PREFIX = '$opt_';
2929
var MODULE_NAME = 'PROJECT_CONFIG';
3030

31+
function createMutationSafeDatafileCopy(datafile) {
32+
var datafileCopy = fns.assign({}, datafile);
33+
datafileCopy.audiences = (datafile.audiences || []).map(function(audience) {
34+
return fns.assign({}, audience);
35+
});
36+
datafileCopy.experiments = (datafile.experiments || []).map(function(experiment) {
37+
return fns.assign({}, experiment);
38+
});
39+
datafileCopy.featureFlags = (datafile.featureFlags || []).map(function(featureFlag) {
40+
return fns.assign({}, featureFlag);
41+
});
42+
datafileCopy.groups = (datafile.groups || []).map(function(group) {
43+
var groupCopy = fns.assign({}, group);
44+
groupCopy.experiments = (group.experiments || []).map(function(experiment) {
45+
return fns.assign({}, experiment);
46+
});
47+
return groupCopy;
48+
});
49+
datafileCopy.rollouts = (datafile.rollouts || []).map(function(rollout) {
50+
var rolloutCopy = fns.assign({}, rollout);
51+
rolloutCopy.experiments = (rollout.experiments || []).map(function(experiment) {
52+
return fns.assign({}, experiment);
53+
});
54+
return rolloutCopy;
55+
});
56+
return datafileCopy;
57+
}
58+
3159
/**
3260
* Creates projectConfig object to be used for quick project property lookup
3361
* @param {Object} datafile JSON datafile representing the project
3462
* @return {Object} Object representing project configuration
3563
*/
3664
export var createProjectConfig = function(datafile) {
37-
var projectConfig = fns.assign({}, datafile);
65+
var projectConfig = createMutationSafeDatafileCopy(datafile);
3866

3967
/*
4068
* Conditions of audiences in projectConfig.typedAudiences are not

‎packages/optimizely-sdk/lib/core/project_config/index.tests.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,26 @@ import configValidator from '../../utils/config_validator';
3333
var logger = getLogger();
3434

3535
describe('lib/core/project_config', function() {
36-
var parsedAudiences = testDatafile.getParsedAudiences;
3736
describe('createProjectConfig method', function() {
3837
it('should set properties correctly when createProjectConfig is called', function() {
3938
var testData = testDatafile.getTestProjectConfig();
4039
var configObj = projectConfig.createProjectConfig(testData);
4140

4241
forEach(testData.audiences, function(audience) {
43-
audience.conditions = audience.conditions;
42+
audience.conditions = JSON.parse(audience.conditions);
4443
});
4544

4645
assert.strictEqual(configObj.accountId, testData.accountId);
4746
assert.strictEqual(configObj.projectId, testData.projectId);
4847
assert.strictEqual(configObj.revision, testData.revision);
4948
assert.deepEqual(configObj.events, testData.events);
5049
assert.deepEqual(configObj.audiences, testData.audiences);
50+
testData.groups.forEach(function(group) {
51+
group.experiments.forEach(function(experiment) {
52+
experiment.groupId = group.id;
53+
experiment.variationKeyMap = fns.keyBy(experiment.variations, 'key');
54+
});
55+
});
5156
assert.deepEqual(configObj.groups, testData.groups);
5257

5358
var expectedGroupIdMap = {
@@ -170,6 +175,13 @@ describe('lib/core/project_config', function() {
170175
assert.deepEqual(configObj.variationIdMap, expectedVariationIdMap);
171176
});
172177

178+
it('should not mutate the datafile', function() {
179+
var datafile = testDatafile.getTypedAudiencesConfig();
180+
var datafileClone = cloneDeep(datafile);
181+
projectConfig.createProjectConfig(datafile);
182+
assert.deepEqual(datafileClone, datafile);
183+
});
184+
173185
describe('feature management', function() {
174186
var configObj;
175187
beforeEach(function() {

‎packages/optimizely-sdk/lib/optimizely/index.tests.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,28 @@ describe('lib/optimizely', function() {
245245
});
246246
});
247247
});
248+
249+
it('should support constructing two instances using the same datafile object', function() {
250+
var datafile = testData.getTypedAudiencesConfig();
251+
var optlyInstance = new Optimizely({
252+
clientEngine: 'node-sdk',
253+
datafile: datafile,
254+
errorHandler: stubErrorHandler,
255+
eventDispatcher: stubEventDispatcher,
256+
jsonSchemaValidator: jsonSchemaValidator,
257+
logger: createdLogger,
258+
});
259+
assert.instanceOf(optlyInstance, Optimizely);
260+
var optlyInstance2 = new Optimizely({
261+
clientEngine: 'node-sdk',
262+
datafile: datafile,
263+
errorHandler: stubErrorHandler,
264+
eventDispatcher: stubEventDispatcher,
265+
jsonSchemaValidator: jsonSchemaValidator,
266+
logger: createdLogger,
267+
});
268+
assert.instanceOf(optlyInstance2, Optimizely);
269+
});
248270
});
249271
});
250272

0 commit comments

Comments
 (0)
0