Skip to content
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

Change functionality of JSON schema validation by removing skipJSONValidation entirely. #442

Merged
merged 2 commits into from
Apr 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 2 additions & 3 deletions packages/optimizely-sdk/lib/core/project_config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -499,14 +499,13 @@ module.exports = {
* @param {Object} config.datafile
* @param {Object} config.jsonSchemaValidator
* @param {Object} config.logger
* @param {Object} config.skipJSONValidation
* @return {Object} Project config object
*/
tryCreatingProjectConfig: function(config) {
configValidator.validateDatafile(config.datafile);
if (config.skipJSONValidation === true) {
if (!config.jsonSchemaValidator) {
config.logger.log(LOG_LEVEL.INFO, jsSdkUtils.sprintf(LOG_MESSAGES.SKIPPING_JSON_VALIDATION, MODULE_NAME));
} else if (config.jsonSchemaValidator) {
} else {
config.jsonSchemaValidator.validate(config.datafile);
config.logger.log(LOG_LEVEL.INFO, jsSdkUtils.sprintf(LOG_MESSAGES.VALID_DATAFILE, MODULE_NAME));
}
Expand Down
13 changes: 0 additions & 13 deletions packages/optimizely-sdk/lib/core/project_config/index.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,6 @@ describe('lib/core/project_config', function() {
datafile: { foo: 'bar' },
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
skipJSONValidation: false,
});
assert.deepEqual(result, configObj);
});
Expand All @@ -739,7 +738,6 @@ describe('lib/core/project_config', function() {
datafile: { foo: 'bar' },
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
skipJSONValidation: false,
});
});
});
Expand All @@ -752,21 +750,10 @@ describe('lib/core/project_config', function() {
datafile: { foo: 'bar' },
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
skipJSONValidation: false,
});
});
});

it('does not call jsonSchemaValidator.validate when skipJSONValidation is true', function() {
projectConfig.tryCreatingProjectConfig({
datafile: { foo: 'bar' },
jsonSchemaValidator: stubJsonSchemaValidator,
logger: logger,
skipJSONValidation: true,
});
sinon.assert.notCalled(stubJsonSchemaValidator.validate);
});

it('skips json validation when jsonSchemaValidator is not provided', function() {
configValidator.validateDatafile.returns(true);
var configObj = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ function getErrorMessage(maybeError, defaultMessage) {
* @param {Object=} config.datafileOptions
* @param {Object=} config.jsonSchemaValidator
* @param {string=} config.sdkKey
* @param {boolean=} config.skipJSONValidation
*/
function ProjectConfigManager(config) {
try {
Expand All @@ -80,12 +79,10 @@ function ProjectConfigManager(config) {
* @param {Object=} config.datafileOptions
* @param {Object=} config.jsonSchemaValidator
* @param {string=} config.sdkKey
* @param {boolean=} config.skipJSONValidation
*/
ProjectConfigManager.prototype.__initialize = function(config) {
this.__updateListeners = [];
this.jsonSchemaValidator = config.jsonSchemaValidator;
this.skipJSONValidation = config.skipJSONValidation;

if (!config.datafile && !config.sdkKey) {
this.__configObj = null;
Expand All @@ -106,7 +103,6 @@ ProjectConfigManager.prototype.__initialize = function(config) {
datafile: initialDatafile,
jsonSchemaValidator: this.jsonSchemaValidator,
logger: logger,
skipJSONValidation: this.skipJSONValidation,
});
this.__optimizelyConfigObj = optimizelyConfig.getOptimizelyConfig(this.__configObj);
} catch (ex) {
Expand Down Expand Up @@ -162,7 +158,6 @@ ProjectConfigManager.prototype.__onDatafileManagerReadyFulfill = function() {
datafile: newDatafile,
jsonSchemaValidator: this.jsonSchemaValidator,
logger: logger,
skipJSONValidation: this.skipJSONValidation,
});
} catch (ex) {
logger.error(ex);
Expand Down Expand Up @@ -204,7 +199,6 @@ ProjectConfigManager.prototype.__onDatafileManagerUpdate = function() {
datafile: newDatafile,
jsonSchemaValidator: this.jsonSchemaValidator,
logger: logger,
skipJSONValidation: this.skipJSONValidation,
});
} catch (ex) {
logger.error(ex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ describe('lib/core/project_config/project_config_manager', function() {

it('should call the error handler and fulfill onReady with an unsuccessful result if neither datafile nor sdkKey are passed into the constructor', function() {
var manager = new projectConfigManager.ProjectConfigManager({
skipJSONValidation: true,
});
sinon.assert.calledOnce(globalStubErrorHandler.handleError);
var errorMessage = globalStubErrorHandler.handleError.lastCall.args[0].message;
Expand All @@ -75,7 +74,6 @@ describe('lib/core/project_config/project_config_manager', function() {
var invalidDatafileJSON = 'abc';
var manager = new projectConfigManager.ProjectConfigManager({
datafile: invalidDatafileJSON,
skipJSONValidation: true,
});
sinon.assert.calledOnce(globalStubErrorHandler.handleError);
var errorMessage = globalStubErrorHandler.handleError.lastCall.args[0].message;
Expand Down Expand Up @@ -131,20 +129,18 @@ describe('lib/core/project_config/project_config_manager', function() {
jsonSchemaValidator.validate.restore();
});

it('should skip JSON schema validation if skipJSONValidation is passed into instance args with `true` value', function() {
it('should skip JSON schema validation if jsonSchemaValidator is not provided', function() {
var manager = new projectConfigManager.ProjectConfigManager({
datafile: testData.getTestProjectConfig(),
skipJSONValidation: true,
});
sinon.assert.notCalled(jsonSchemaValidator.validate);
return manager.onReady();
});

it('should not skip JSON schema validation if skipJSONValidation is passed into instance args with any value other than true', function() {
it('should not skip JSON schema validation if jsonSchemaValidator is provided', function() {
var manager = new projectConfigManager.ProjectConfigManager({
datafile: testData.getTestProjectConfig(),
jsonSchemaValidator: jsonSchemaValidator,
skipJSONValidation: 'hi',
});
sinon.assert.calledOnce(jsonSchemaValidator.validate);
sinon.assert.calledOnce(stubLogHandler.log);
Expand Down
6 changes: 0 additions & 6 deletions packages/optimizely-sdk/lib/index.browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,6 @@ module.exports = {
config.isValidInstance = false;
}

// Explicitly check for null or undefined
// prettier-ignore
if (config.skipJSONValidation == null) { // eslint-disable-line eqeqeq
config.skipJSONValidation = true;
}

var eventDispatcher;
// prettier-ignore
if (config.eventDispatcher == null) { // eslint-disable-line eqeqeq
Expand Down
2 changes: 0 additions & 2 deletions packages/optimizely-sdk/lib/index.browser.umdtests.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ describe('javascript-sdk', function() {
// checking that INFO logs log for an unspecified logLevel
var optlyInstance = window.optimizelySdk.createInstance({
datafile: testData.getTestProjectConfig(),
skipJSONValidation: true,
});
assert.strictEqual(console.info.getCalls().length, 1);
call = console.info.getCalls()[0];
Expand All @@ -87,7 +86,6 @@ describe('javascript-sdk', function() {
var optlyInstance = window.optimizelySdk.createInstance({
datafile: testData.getTestProjectConfig(),
logLevel: enums.LOG_LEVEL.ERROR,
skipJSONValidation: true,
});
assert.strictEqual(console.log.getCalls().length, 0);

Expand Down
1 change: 0 additions & 1 deletion packages/optimizely-sdk/lib/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ declare module '@optimizely/optimizely-sdk' {
| enums.LOG_LEVEL.INFO
| enums.LOG_LEVEL.NOTSET
| enums.LOG_LEVEL.WARNING;
skipJSONValidation?: boolean;
jsonSchemaValidator?: object;
userProfileService?: UserProfileService | null;
eventBatchSize?: number;
Expand Down
4 changes: 0 additions & 4 deletions packages/optimizely-sdk/lib/index.node.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ var defaultErrorHandler = require('./plugins/error_handler');
var defaultEventDispatcher = require('./plugins/event_dispatcher/index.node');
var enums = require('./utils/enums');
var fns = require('./utils/fns');
var jsonSchemaValidator = require('./utils/json_schema_validator');
var loggerPlugin = require('./plugins/logger');
var Optimizely = require('./optimizely');
var eventProcessorConfigValidator = require('./utils/event_processor_config_validator');
Expand Down Expand Up @@ -48,7 +47,6 @@ module.exports = {
* @param {Object} config.datafile
* @param {Object} config.errorHandler
* @param {Object} config.eventDispatcher
* @param {Object} config.jsonSchemaValidator
* @param {Object} config.logger
* @param {Object} config.userProfileService
* @param {Object} config.eventBatchSize
Expand Down Expand Up @@ -93,8 +91,6 @@ module.exports = {
eventBatchSize: DEFAULT_EVENT_BATCH_SIZE,
eventDispatcher: defaultEventDispatcher,
eventFlushInterval: DEFAULT_EVENT_FLUSH_INTERVAL,
jsonSchemaValidator: jsonSchemaValidator,
skipJSONValidation: false,
},
config,
{
Expand Down
6 changes: 0 additions & 6 deletions packages/optimizely-sdk/lib/index.react_native.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,6 @@ module.exports = {
config.isValidInstance = false;
}

// Explicitly check for null or undefined
// prettier-ignore
if (config.skipJSONValidation == null) { // eslint-disable-line eqeqeq
config.skipJSONValidation = true;
}

config = fns.assign(
{
clientEngine: enums.REACT_NATIVE_JS_CLIENT_ENGINE,
Expand Down
2 changes: 0 additions & 2 deletions packages/optimizely-sdk/lib/optimizely/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ var DEFAULT_ONREADY_TIMEOUT = 30000;
* @param {Object} config.errorHandler
* @param {Object} config.eventDispatcher
* @param {Object} config.logger
* @param {Object} config.skipJSONValidation
* @param {Object} config.userProfileService
* @param {Object} config.eventBatchSize
* @param {Object} config.eventFlushInterval
Expand All @@ -76,7 +75,6 @@ function Optimizely(config) {
datafileOptions: config.datafileOptions,
jsonSchemaValidator: config.jsonSchemaValidator,
sdkKey: config.sdkKey,
skipJSONValidation: config.skipJSONValidation,
});

this.__disposeOnUpdate = this.projectConfigManager.onUpdate(
Expand Down
3 changes: 0 additions & 3 deletions packages/optimizely-sdk/lib/optimizely/index.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ describe('lib/optimizely', function() {
jsonSchemaValidator: jsonSchemaValidator,
logger: createdLogger,
sdkKey: '12345',
skipJSONValidation: false,
});
sinon.assert.notCalled(stubErrorHandler.handleError);
});
Expand All @@ -236,7 +235,6 @@ describe('lib/optimizely', function() {
jsonSchemaValidator: jsonSchemaValidator,
logger: createdLogger,
sdkKey: '12345',
skipJSONValidation: false,
});
sinon.assert.calledOnce(projectConfigManager.ProjectConfigManager);
sinon.assert.calledWithExactly(projectConfigManager.ProjectConfigManager, {
Expand All @@ -247,7 +245,6 @@ describe('lib/optimizely', function() {
},
jsonSchemaValidator: jsonSchemaValidator,
sdkKey: '12345',
skipJSONValidation: false,
});
});
});
Expand Down