Skip to content

Commit

Permalink
feat (datafile management): Implement automatic updates, datafile man…
Browse files Browse the repository at this point in the history
…ager options, and stopping datafile manager (#15)

Summary:

- Refactor project config management into a ProjectConfigManager class, which uses DatafileManager internally
- Update Optimizely to get project config by calling projectConfigManager.getConfig() instead of keeping its own reference to a project config object
- Pass through datafileOptions to project config manager (allowing use of autoUpdate and updateInterval)
- Add notification center event for new project config
- Add an update listener to datafile manager that sends project config update notification
- Call this.datafileManager.stop in close method
- Add default timeout to onReady method
- Update behavior of onReady to reject early when datafile manager emits invalid datafile


Test plan:

Unit tests

Issues:

https://optimizely.atlassian.net/browse/OASIS-4385
https://optimizely.atlassian.net/browse/OASIS-4386
https://optimizely.atlassian.net/browse/OASIS-4387
  • Loading branch information
mjc1283 committed Apr 8, 2019
1 parent b53d102 commit c987725
Show file tree
Hide file tree
Showing 7 changed files with 833 additions and 578 deletions.
30 changes: 10 additions & 20 deletions packages/optimizely-sdk/lib/core/project_config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -600,32 +600,22 @@ module.exports = {
/**
* Try to create a project config object from the given datafile and
* configuration properties.
* If successful, return the project config object, otherwise return null.
* If successful, return the project config object, otherwise throws an error
* @param {Object} config
* @param {Object} config.datafile
* @param {Object} config.errorHandler
* @param {Object} config.jsonSchemaValidator
* @param {Object} config.logger
* @param {Object} config.skipJSONValidation
* @return {Object|null} Project config object if datafile was valid, otherwise null
* @return {Object} Project config object
*/
tryCreatingProjectConfig: function(config) {
var configObj = null;
try {
configValidator.validateDatafile(config.datafile);
if (config.skipJSONValidation === true) {
configObj = module.exports.createProjectConfig(config.datafile);
config.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SKIPPING_JSON_VALIDATION, MODULE_NAME));
} else if (!config.jsonSchemaValidator) {
configObj = module.exports.createProjectConfig(config.datafile);
} else if (config.jsonSchemaValidator.validate(projectConfigSchema, config.datafile)) {
configObj = module.exports.createProjectConfig(config.datafile);
config.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.VALID_DATAFILE, MODULE_NAME));
}
} catch (ex) {
config.logger.log(LOG_LEVEL.ERROR, ex.message);
config.errorHandler.handleError(ex);
configValidator.validateDatafile(config.datafile);
if (config.skipJSONValidation === true) {
config.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SKIPPING_JSON_VALIDATION, MODULE_NAME));
} else if (config.jsonSchemaValidator) {
config.jsonSchemaValidator.validate(projectConfigSchema, config.datafile);
config.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.VALID_DATAFILE, MODULE_NAME));
}
return configObj;
}
return module.exports.createProjectConfig(config.datafile);
},
};
81 changes: 30 additions & 51 deletions packages/optimizely-sdk/lib/core/project_config/index.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@ var projectConfig = require('./');
var enums = require('../../utils/enums');
var testDatafile = require('../../tests/test_data');
var configValidator = require('../../utils/config_validator');
var logging = require('@optimizely/js-sdk-logging');

var logger = logging.getLogger();

var _ = require('lodash/core');
var fns = require('../../utils/fns');
var chai = require('chai');
var assert = chai.assert;
var logger = require('../../plugins/logger');
var loggerPlugin = require('../../plugins/logger');
var sinon = require('sinon');
var sprintf = require('@optimizely/js-sdk-utils').sprintf;

Expand Down Expand Up @@ -224,7 +227,7 @@ describe('lib/core/project_config', function() {
describe('projectConfig helper methods', function() {
var testData = testDatafile.getTestProjectConfig();
var configObj;
var createdLogger = logger.createLogger({logLevel: LOG_LEVEL.INFO});
var createdLogger = loggerPlugin.createLogger({logLevel: LOG_LEVEL.INFO});

beforeEach(function() {
configObj = projectConfig.createProjectConfig(testData);
Expand Down Expand Up @@ -351,7 +354,7 @@ describe('lib/core/project_config', function() {
});

describe('feature management', function() {
var featureManagementLogger = logger.createLogger({logLevel: LOG_LEVEL.INFO});
var featureManagementLogger = loggerPlugin.createLogger({logLevel: LOG_LEVEL.INFO});
beforeEach(function() {
configObj = projectConfig.createProjectConfig(testDatafile.getTestProjectConfigWithFeatures());
sinon.stub(featureManagementLogger, 'log');
Expand Down Expand Up @@ -559,7 +562,7 @@ describe('lib/core/project_config', function() {
});

describe('#getForcedVariation', function() {
var createdLogger = logger.createLogger({
var createdLogger = loggerPlugin.createLogger({
logLevel: LOG_LEVEL.INFO,
logToConsole: false,
});
Expand All @@ -582,7 +585,7 @@ describe('lib/core/project_config', function() {
});

describe('#setForcedVariation', function() {
var createdLogger = logger.createLogger({
var createdLogger = loggerPlugin.createLogger({
logLevel: LOG_LEVEL.INFO,
logToConsole: false,
});
Expand Down Expand Up @@ -730,26 +733,20 @@ describe('lib/core/project_config', function() {
});

describe('#tryCreatingProjectConfig', function() {
var stubErrorHandler;
var stubJsonSchemaValidator;
var stubLogger;
beforeEach(function() {
stubErrorHandler = {
handleError: sinon.stub(),
};
stubJsonSchemaValidator = {
validate: sinon.stub().returns(true),
};
stubLogger = {
log: sinon.stub(),
};
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() {
Expand All @@ -765,61 +762,44 @@ describe('lib/core/project_config', function() {
projectConfig.createProjectConfig.returns(configObj);
var result = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
errorHandler: stubErrorHandler,
jsonSchemaValidator: stubJsonSchemaValidator,
logger: stubLogger,
logger: logger,
skipJSONValidation: false,
});
assert.deepEqual(result, configObj);
});

it('returns null and calls handleError when validateDatafile throws', function() {
it('throws an error when validateDatafile throws', function() {
configValidator.validateDatafile.throws();
stubJsonSchemaValidator.validate.returns(true);
var result = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
errorHandler: stubErrorHandler,
jsonSchemaValidator: stubJsonSchemaValidator,
logger: stubLogger,
skipJSONValidation: false,
assert.throws(function() {
projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
skipJSONValidation: false,
});
});
assert.strictEqual(result, null);
sinon.assert.calledOnce(stubErrorHandler.handleError);
});

it('returns null and calls handleError when jsonSchemaValidator.validate throws', function() {
it('throws an error when jsonSchemaValidator.validate throws', function() {
configValidator.validateDatafile.returns(true);
stubJsonSchemaValidator.validate.throws();
var result = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
errorHandler: stubErrorHandler,
jsonSchemaValidator: stubJsonSchemaValidator,
logger: stubLogger,
skipJSONValidation: false,
});
assert.strictEqual(result, null);
sinon.assert.calledOnce(stubErrorHandler.handleError);
});

it('returns null when jsonSchemaValidator.validate returns false', function() {
configValidator.validateDatafile.returns(true);
stubJsonSchemaValidator.validate.returns(false);
var result = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
errorHandler: stubErrorHandler,
jsonSchemaValidator: stubJsonSchemaValidator,
logger: stubLogger,
skipJSONValidation: false,
assert.throws(function() {
var result = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
skipJSONValidation: false,
});
});
assert.strictEqual(result, null);
});

it('does not call jsonSchemaValidator.validate when skipJSONValidation is true', function() {
projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
errorHandler: stubErrorHandler,
jsonSchemaValidator: stubJsonSchemaValidator,
logger: stubLogger,
logger: logger,
skipJSONValidation: true,
});
sinon.assert.notCalled(stubJsonSchemaValidator.validate);
Expand All @@ -837,11 +817,10 @@ describe('lib/core/project_config', function() {
projectConfig.createProjectConfig.returns(configObj);
var result = projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
errorHandler: stubErrorHandler,
logger: stubLogger,
logger: logger,
});
assert.deepEqual(result, configObj);
sinon.assert.notCalled(stubErrorHandler.handleError);
sinon.assert.notCalled(logger.error);
});
});
});

0 comments on commit c987725

Please sign in to comment.