8000 feat: datafile accessor by pthompson127 · Pull Request #535 · optimizely/javascript-sdk · GitHub
[go: up one dir, main page]

Skip to content

feat: datafile accessor #535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
Sep 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
674c9bd
refactor: modify datafile-manager to require, use, and return datafil…
pthompson127 Jul 22, 2020
09bf4bc
docs: modify params of createInstance methods (in browser, Node, and …
pthompson127 Jul 22, 2020
27ceed7
refactor: modify datafile setter to handle case for datafile object/i…
pthompson127 Jul 23, 2020
434ab3e
refactor: add helper function to take care of setting datafile and cr…
pthompson127 Jul 24, 2020
9a136d0
feat: add toDatafile() method in OptimizelyConfig
pthompson127 Jul 24, 2020
38d7b92
refactor: remove accidental formatting changes
pthompson127 Jul 24, 2020
c2ff808
Merge branch 'master' of https://github.com/optimizely/javascript-sdk…
pthompson127 Jul 24, 2020
6762ff5
getDatafile tweaks
Jul 27, 2020
48b2d44
Don't set null
Jul 28, 2020
5e3bea5
refactor: address PR requested changes
pthompson127 Jul 28, 2020
68b532c
docs: fix datafile accessor methods descriptions
pthompson127 Jul 28, 2020
f8120f4
style: fix styling and typos
pthompson127 Jul 28, 2020
229a971
test: update httpPollingDatafileManager test to account for datafile …
pthompson127 Jul 28, 2020
f7dec26
test: update tests to comply with change in datafile format
pthompson127 Jul 29, 2020
8fc3baf
merge: pull from master after datafile-manager updates and resolve me…
pthompson127 Aug 5, 2020
1e7f54e
test: modify existing tests to account for datafile accessor changes
pthompson127 Aug 8, 2020
5957dce
test: re-adding logs to comply with browser tests
pthompson127 Aug 21, 2020
efe28e8
fix: add import statement for log messages
pthompson127 Aug 21, 2020
244f8ff
updated dfm package
msohailhussain Sep 1, 2020
e9535f8
stringified datafile
msohailhussain Sep 3, 2020
6f85f62
Merge branch 'master' into peter/getDatafile
msohailhussain Sep 3, 2020
8e7688d
conflicts resolved.
msohailhussain Sep 3, 2020
b7278bc
lint fix
msohailhussain Sep 3, 2020
efbebb2
comments resolved
mnoman09 Sep 8, 2020
e87d8a1
reverting these changes as its causing error
mnoman09 Sep 8, 2020
1abe037
returning optimizelyConfig as default
mnoman09 Sep 8, 2020
1623409
resolved fixes
msohailhussain Sep 8, 2020
87a1746
remove this.createProjectConfig stubbing
msohailhussain Sep 8, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/optimizely-sdk/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ module.exports = {
Promise: 'readonly',
},
parserOptions: {
ecmaVersion: 6,
// Note: The TS compiler determines what syntax is accepted. We're using TS version 3.3.3333.
// This seems to roughly correspond to "2018" for this setting.
ecmaVersion: 2018,
Comment on lines +17 to +19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops...I thought your branch was based on https://github.com/optimizely/javascript-sdk/tree/mcarroll/optimizely-sdk-typescript, so I suggested adding some newer syntax. But I see now this PR is against master. I suppose the easiest thing now is to wait until we can merge https://github.com/optimizely/javascript-sdk/tree/mcarroll/optimizely-sdk-typescript into master before merging these changes.

We can address this later, as the first step for this PR should be separating out and pushing through the datafile manager changes.

sourceType: 'module',
},
overrides: [
Expand Down
32 changes: 23 additions & 9 deletions packages/optimizely-sdk/lib/core/optimizely_config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,26 @@ function getFeaturesMap(configObj, allExperiments) {
}, {});
}

export var getOptimizelyConfig = function(configObj) {
// Fetch all feature variables from feature flags to merge them with variation variables
var experimentsMap = getExperimentsMap(configObj);
return {
experimentsMap: experimentsMap,
featuresMap: getFeaturesMap(configObj, experimentsMap),
revision: configObj.revision,
};
};
/**
* The OptimizelyConfig class
* @param {Object} configObj
* @param {string} datafile
*/
export function OptimizelyConfig(configObj, datafile) {
this.experimentsMap = getExperimentsMap(configObj);
this.featuresMap = getFeaturesMap(configObj, this.experimentsMap);
this.revision = configObj.revision;
this.__datafile = datafile;
}

/**
* Get the datafile
* @returns {string} JSON string representation of the datafile that was used to create the current config object
*/
OptimizelyConfig.prototype.getDatafile = function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a JSDoc comment with the return type.

return this.__datafile;
}

export default {
OptimizelyConfig: OptimizelyConfig
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unable to stub, that's why passing object.

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import { assert } from 'chai';
import { cloneDeep } from 'lodash';

import { getOptimizelyConfig } from './index';
import { OptimizelyConfig } from './index';
import { createProjectConfig } from '../project_config';
import { getTestProjectConfigWithFeatures } from '../../tests/test_data';

Expand All @@ -41,7 +41,7 @@ describe('lib/core/optimizely_config', function() {
var projectConfigObject;
beforeEach(function() {
projectConfigObject = createProjectConfig(cloneDeep(datafile));
optimizelyConfigObject = getOptimizelyConfig(projectConfigObject);
optimizelyConfigObject = new OptimizelyConfig(projectConfigObject, JSON.stringify(datafile));
});

it('should return all experiments except rollouts', function() {
Expand Down
89 changes: 66 additions & 23 deletions packages/optimizely-sdk/lib/core/project_config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import { sprintf, objectValues } from '@optimizely/js-sdk-utils';
import { assign, keyBy } from '../../utils/fns';
import {
ERROR_MESSAGES,
LOG_MESSAGES,
LOG_LEVEL,
LOG_MESSAGES,
FEATURE_VARIABLE_TYPES,
} from '../../utils/enums';
import configValidator from '../../utils/config_validator';
Expand Down Expand Up @@ -58,11 +58,14 @@ function createMutationSafeDatafileCopy(datafile) {

/**
* Creates projectConfig object to be used for quick project property lookup
* @param {Object} datafile JSON datafile representing the project
* @return {Object} Object representing project configuration
* @param {Object} datafileObj JSON datafile representing the project
* @param {string=} datafileStr JSON string representation of the datafile
* @return {Object} Object representing project configuration
*/
export var createProjectConfig = function(datafile) {
var projectConfig = createMutationSafeDatafileCopy(datafile);
export var createProjectConfig = function(datafileObj, datafileStr=null) {
var projectConfig = createMutationSafeDatafileCopy(datafileObj);

projectConfig.__datafileStr = datafileStr === null ? JSON.stringify(datafileObj) : datafileStr;

/*
* Conditions of audiences in projectConfig.typedAudiences are not
Expand Down Expand Up @@ -512,7 +515,7 @@ export var getTypeCastValue = function(variableValue, variableType, logger) {
/**
* Returns an object containing all audiences in the project config. Keys are audience IDs
* and values are audience objects.
* @param projectConfig
* @param {Object} projectConfig
* @returns {Object}
*/
export var getAudiencesById = function(projectConfig) {
Expand All @@ -521,44 +524,83 @@ export var getAudiencesById = function(projectConfig) {

/**
* Returns true if an event with the given key exists in the datafile, and false otherwise
* @param {Object} projectConfig
* @param {string} eventKey
* @param {Object} projectConfig
* @param {string} eventKey
* @returns {boolean}
*/
export var eventWithKeyExists = function(projectConfig, eventKey) {
return projectConfig.eventKeyMap.hasOwnProperty(eventKey);
};

/**
*
* Returns true if experiment belongs to any feature, false otherwise.
* @param {Object} projectConfig
* @param {string} experimentId
* @returns {boolean} Returns true if experiment belongs to
* any feature, false otherwise.
* @returns {boolean}
*/
export var isFeatureExperiment = function(projectConfig, experimentId) {
return projectConfig.experimentFeatureMap.hasOwnProperty(experimentId);
};

/**
* Returns the JSON string representation of the datafile
* @param {Object} projectConfig
* @returns {string}
*/
export var toDatafile = function(projectConfig) {
return projectConfig.__datafileStr;
}

/**
* @typedef {Object} TryCreatingProjectConfigResult
* @property {Object|null} configObj
* @property {Error|null} error
*/

/**
* Try to create a project config object from the given datafile and
* configuration properties.
* If successful, return the project config object, otherwise throws an error
* @param {Object} config
* @param {Object} config.datafile
* @param {Object} config.jsonSchemaValidator
* @param {Object} config.logger
* @return {Object} Project config object
* Returns an object with configObj and error properties.
* If successful, configObj is the project config object, and error is null.
* Otherwise, configObj is null and error is an error with more information.
* @param {Object} config
* @param {Object|string} config.datafile
* @param {Object} config.jsonSchemaValidator
* @param {Object} config.logger
* @returns {TryCreatingProjectConfigResult}
*/
export var tryCreatingProjectConfig = function(config) {
configValidator.validateDatafile(config.datafile);
if (!config.jsonSchemaValidator) {
config.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SKIPPING_JSON_VALIDATION, MODULE_NAME));

var newDatafileObj;
try {
newDatafileObj = configValidator.validateDatafile(config.datafile);
} catch (error) {
return { configObj: null, error };
}

if (config.jsonSchemaValidator) {
try {
config.jsonSchemaValidator.validate(newDatafileObj);
config.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.VALID_DATAFILE, MODULE_NAME));
} catch (error) {
return { configObj: null, error };
}
} else {
config.jsonSchemaValidator.validate(config.datafile);
config.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.VALID_DATAFILE, MODULE_NAME));
config.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SKIPPING_JSON_VALIDATION, MODULE_NAME));
}

var createProjectConfigArgs = [newDatafileObj];
if (typeof config.datafile === 'string') {
// Since config.datafile was validated above, we know that it is a valid JSON string
createProjectConfigArgs.push(config.datafile);
}
return this.createProjectConfig(config.datafile);

var newConfigObj = createProjectConfig(...createProjectConfigArgs);

return {
configObj: newConfigObj,
error: null,
};
};

export default {
Expand All @@ -583,5 +625,6 @@ export default {
getAudiencesById: getAudiencesById,
eventWithKeyExists: eventWithKeyExists,
isFeatureExperiment: isFeatureExperiment,
toDatafile: toDatafile,
tryCreatingProjectConfig: tryCreatingProjectConfig,
};
76 changes: 46 additions & 30 deletions packages/optimizely-sdk/lib/core/project_config/index.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/
import sinon from 'sinon';
import { assert } from 'chai';
import { assert, expect, config } from 'chai';
import { forEach, cloneDeep } from 'lodash';
import { getLogger } from '@optimizely/js-sdk-logging';
import { sprintf } from '@optimizely/js-sdk-utils';
Expand Down Expand Up @@ -712,75 +712,91 @@ describe('lib/core/project_config', function() {
stubJsonSchemaValidator = {
validate: sinon.stub().returns(true),
};
sinon.stub(projectConfig, 'createProjectConfig').returns({});
sinon.stub(configValidator, 'validateDatafile').returns(true);
sinon.spy(logger, 'error');
});

afterEach(function() {
projectConfig.createProjectConfig.restore();
configValidator.validateDatafile.restore();
logger.error.restore();
});

it('returns a project config object created by createProjectConfig when all validation is applied and there are no errors', function() {
configValidator.validateDatafile.returns(true);
stubJsonSchemaValidator.validate.returns(true);
var configDatafile = {
foo: 'bar',
experiments: [
{key: 'a'},
{key: 'b'}
]
}
configValidator.validateDatafile.returns(configDatafile);
var configObj = {
foo: 'bar',
experimentKeyMap: {
a: { key: 'a' },
b: { key: 'b' },
"a": { key: "a", variationKeyMap: {} },
"b": { key: "b", variationKeyMap: {} }
},
};
projectConfig.createProjectConfig.returns(configObj);

stubJsonSchemaValidator.validate.returns(true);

var result = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
datafile: configDatafile,
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
});
assert.deepEqual(result, configObj);

assert.deepInclude(result.configObj, configObj)
});

it('throws an error when validateDatafile throws', function() {
it('returns an error when validateDatafile throws', function() {
configValidator.validateDatafile.throws();
stubJsonSchemaValidator.validate.returns(true);
assert.throws(function() {
projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
});
var { error } = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
});
assert.isNotNull(error);
});

it('throws an error when jsonSchemaValidator.validate throws', function() {
it('returns an error when jsonSchemaValidator.validate throws', function() {
configValidator.validateDatafile.returns(true);
stubJsonSchemaValidator.validate.throws();
assert.throws(function() {
var result = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
});
var { error } = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
});
assert.isNotNull(error);
});

it('skips json validation when jsonSchemaValidator is not provided', function() {
configValidator.validateDatafile.returns(true);

var configDatafile = {
foo: 'bar',
experiments: [
{key: 'a'},
{key: 'b'}
]
}

configValidator.validateDatafile.returns(configDatafile);

var configObj = {
foo: 'bar',
experimentKeyMap: {
a: { key: 'a' },
b: { key: 'b' },
a: { key: 'a', variationKeyMap: {} },
b: { key: 'b', variationKeyMap: {} },
},
};
projectConfig.createProjectConfig.returns(configObj);

var result = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
datafile: configDatafile,
logger: logger,
});
assert.deepEqual(result, configObj);

assert.deepInclude(result.configObj, configObj);
sinon.assert.notCalled(logger.error);
});
});
Expand Down
Loading
0