diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index e7e80f83eab..0e50e806310 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -167,11 +167,11 @@ class RangeInput extends React.Component { } RangeInput.propTypes = { - value: React.PropTypes.number, + value: React.PropTypes.number.isRequired, upperBound: React.PropTypes.bool, validationState: React.PropTypes.string, - boundIncluded: React.PropTypes.bool, - disabled: React.PropTypes.bool, + boundIncluded: React.PropTypes.bool.isRequired, + disabled: React.PropTypes.bool.isRequired, onChange: React.PropTypes.func, width: React.PropTypes.number }; diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index f2b0278fa6c..9a9d0e49ae1 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -43,12 +43,26 @@ class RuleCategoryRange extends React.Component { return result; } + static validateKeyAndValue(key, value) { + if (!_.includes(['$gt', '$gte', '$lt', '$lte'], key)) { + return false; + } + // Check that we have only numeric (or null) types. + // String types are a possible extension, + // but documents, arrays, BinData, undefined and other BSON types + // make little sense http://bsonspec.org/spec.html + if (typeof(value) !== 'number') { + return false; + } + return !isNaN(value) && Math.abs(value) !== Infinity; + } + static queryToParams(query) { // if not every key in the object is one of the comparison operators, // this rule cannot represent the query const keys = _.keys(query); if (!_.every(keys, (key) => { - return _.contains(['$gt', '$gte', '$lt', '$lte'], key); + return RuleCategoryRange.validateKeyAndValue(key, query[key]); })) { return false; } @@ -58,12 +72,17 @@ class RuleCategoryRange extends React.Component { lowerBoundValue: query.$gte || query.$gt || null, lowerBoundType: _.intersection(keys, ['$gte', '$gt']) }; - - if (result.upperBoundType.length === 0) { - result.upperBoundType = null; + if (result.upperBoundType.length > 1 || result.lowerBoundType.length > 1) { + return false; } - if (result.lowerBoundType.length === 0) { - result.lowerBoundType = null; + result.upperBoundType = result.upperBoundType[0] || null; + result.lowerBoundType = result.lowerBoundType[0] || null; + + // No documents could possibly satisfy these cases, e.g. 5 <= value < 5 + if (typeof(result.upperBoundValue) === 'number' && + typeof(result.lowerBoundValue) === 'number' && + result.upperBoundValue <= result.lowerBoundValue) { + return false; } return result; } @@ -76,8 +95,17 @@ class RuleCategoryRange extends React.Component { render() { return ( - - + + ); } diff --git a/test/validation.range.test.js b/test/validation.range.test.js index 4ba46aa37a4..084fef566aa 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -15,12 +15,12 @@ chai.use(chaiEnzyme()); describe('', () => { context('when rendering the default control', () => { it('has label `LOWER BOUND`', () => { - const component = shallow(); + const component = shallow(); const labelText = component.find(ControlLabel).dive().text(); expect(labelText).to.be.equal('LOWER BOUND'); }); it('has placeholder text of `enter lower bound`', () => { - const component = shallow(); + const component = shallow(); const placeholderText = component.find(FormControl).props().placeholder; expect(placeholderText).to.be.equal('enter lower bound'); }); @@ -28,12 +28,12 @@ describe('', () => { context('when rendering an upperBound control', () => { it('has label `UPPER BOUND`', () => { - const component = shallow(); + const component = shallow(); const labelText = component.find(ControlLabel).dive().text(); expect(labelText).to.be.equal('UPPER BOUND'); }); it('has placeholder text of `enter upper bound`', () => { - const component = shallow(); + const component = shallow(); const placeholderText = component.find(FormControl).props().placeholder; expect(placeholderText).to.be.equal('enter upper bound'); }); diff --git a/src/internal-packages/validation/test/rule.test.js b/test/validation.rule.test.js similarity index 96% rename from src/internal-packages/validation/test/rule.test.js rename to test/validation.rule.test.js index b0867392ea1..db5b57fada7 100644 --- a/src/internal-packages/validation/test/rule.test.js +++ b/test/validation.rule.test.js @@ -1,11 +1,11 @@ -/* eslint no-unused-expressions: 0 */ +/* eslint no-unused-vars: 0 */ const chai = require('chai'); const chaiEnzyme = require('chai-enzyme'); const expect = chai.expect; const React = require('react'); const mount = require('enzyme').mount; -const Rule = require('../lib/components/rule'); +const Rule = require('../src/internal-packages/validation/lib/components/rule'); const _ = require('lodash'); // const debug = require('debug')('compass:validation:test'); diff --git a/src/internal-packages/validation/test/store.test.js b/test/validation.store.test.js similarity index 68% rename from src/internal-packages/validation/test/store.test.js rename to test/validation.store.test.js index fb9ba5fdde3..d522960a094 100644 --- a/src/internal-packages/validation/test/store.test.js +++ b/test/validation.store.test.js @@ -1,7 +1,7 @@ /* eslint no-unused-expressions: 0 */ const expect = require('chai').expect; -const ValidationStore = require('../lib/stores'); +const ValidationStore = require('../src/internal-packages/validation/lib/stores'); const sinon = require('sinon'); const _ = require('lodash'); @@ -53,6 +53,7 @@ describe('ValidationStore', function() { unsubscribe = ValidationStore.listen((state) => { expect(state.fetchState).to.be.equal('fetching'); + unsubscribe(); done(); }); ValidationStore.fetchValidationRules(); @@ -861,4 +862,344 @@ describe('ValidationStore', function() { done(); }, 10); }); + + describe('Range Rule when passing in values that are valid on the server', function() { + context.skip('values accepted by the rule builder UI', function() { + it('accepts {$gte: 21}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gte': 21 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = result.rules[0]; + expect(_.omit(rule, 'id')).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gte', + 'lowerBoundValue': 21, + 'upperBoundType': null, + 'upperBoundValue': null + } + }); + }); + + it('accepts {$lt: 21.1234567890}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$lt': 21.1234567890 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': null, + 'lowerBoundValue': null, + 'upperBoundType': '$lt', + 'upperBoundValue': 21.123456789 + } + }); + }); + + it('accepts {$gt: 20, $lte: 21}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': 20, + '$lte': 21 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gt', + 'lowerBoundValue': 20, + 'upperBoundType': '$lte', + 'upperBoundValue': 21 + } + }); + }); + + it('accepts {$gt: -20, $lte: -0.000001}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': -20, + '$lte': -0.000001 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gt', + 'lowerBoundValue': -20, + 'upperBoundType': '$lte', + 'upperBoundValue': -0.000001 + } + }); + }); + }); + + // Note: Server allows these cases, but we'd drop back to JSON view here + context('values rejected by the rule builder UI', function() { + // Only documents with value = 5 could be inserted, + // which being a constant probably should be at the application layer + it('rejects equality constant range "5 <= value <= 5"', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gte': 5, + '$lte': 5 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + // Bad as users couldn't insert any documents into the collection + it('rejects empty range "5 < value <= 5"', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': 5, + '$lte': 5 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + // Bad as users couldn't insert any documents into the collection + it('rejects empty range "5 <= value < 5"', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gte': 5, + '$lt': 5 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + // Bad as users couldn't insert any documents into the collection + it('rejects empty range "6 < value < 5"', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': 6, + '$lt': 5 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + // Better represented in GUI with the "None" operator drop down value + it('rejects {$gte: -Infinity}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gte': -Infinity + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + it('rejects {$lt: Infinity}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$lt': Infinity + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + // https://github.com/mongodb/js-bson/blob/0.5/lib/bson/decimal128.js#L6 + it("rejects {$gte: 'inf'}", function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gte': 'inf' + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + // Otherwise we'd silently type-convert which does not feel intuitive + it("rejects a number-string {$gte: '-2.01'}", function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gte': '-2.01' + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + // This actually works in that the server allows the validation rule + // {key: {$gte: NaN}}, but it's not very useful in that you + // can only insert NaN, so drop back to JSON + it('rejects NaN {$gte: NaN}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gte': NaN + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + it('rejects NaN {$lt: NaN}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$lt': NaN + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + it('rejects similar operators {$gt: 20, $gte: 21}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': 20, + '$gte': 21 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + it('rejects similar operators {$lt: 20, $lte: 21}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$lt': 20, + '$lte': 21 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + // These might be useful, but we'd need to figure out things like + // the minimum string, maximum string, and + // understand l10n, i18n and collation properly + it('rejects strings {$gte: "a", $lte: "z"}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$lte': 'a', + '$gte': 'z' + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + it('rejects a document {$lt: {}}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$lt': '{}' + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + + it('rejects an array {$lt: []}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$lt': '[]' + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + expect(result.rules).to.be.false; + }); + }); + }); });