From a161318fdc3b44365a144427d8a00f3aafa297aa Mon Sep 17 00:00:00 2001 From: Offir Golan Date: Fri, 16 Feb 2018 11:25:30 -0800 Subject: [PATCH] Inline Validator (#569) --- addon/validations/factory.js | 11 +- addon/validations/validator.js | 14 +- addon/validators/base.js | 21 +-- addon/validators/inline.js | 49 ++++++ app/validators/inline.js | 1 + tests/dummy/app/models/signup.js | 6 +- .../factory-dependent-keys-test.js | 60 +++---- .../validations/factory-general-test.js | 156 ++++++++++-------- .../validations/model-relationships-test.js | 16 +- tests/unit/validations/ds-model-test.js | 2 +- tests/unit/validators/inline-test.js | 30 ++++ 11 files changed, 218 insertions(+), 148 deletions(-) create mode 100755 addon/validators/inline.js create mode 100644 app/validators/inline.js create mode 100644 tests/unit/validators/inline-test.js diff --git a/addon/validations/factory.js b/addon/validations/factory.js index edf4170a..69f2e28b 100644 --- a/addon/validations/factory.js +++ b/addon/validations/factory.js @@ -751,7 +751,6 @@ function createValidatorsFor(attribute, model) { let validatorCache = get(validations, '_validators'); let owner = getOwner(model); let validators = []; - let validator; // We must have an owner to be able to lookup our validators if (isNone(owner)) { @@ -763,15 +762,7 @@ function createValidatorsFor(attribute, model) { validationRules.forEach(v => { v.attribute = attribute; v.model = model; - - // If validate function exists, that means validator was created with a function so use the base class - if (v._type === 'function') { - validator = BaseValidator.create(owner.ownerInjection(), v); - } else { - validator = lookupValidator(owner, v._type).create(v); - } - - validators.push(validator); + validators.push(lookupValidator(owner, v._type).create(v)); }); // Add validators to model instance cache diff --git a/addon/validations/validator.js b/addon/validations/validator.js index b50cfb82..4442690a 100644 --- a/addon/validations/validator.js +++ b/addon/validations/validator.js @@ -1,4 +1,5 @@ import { isNone } from '@ember/utils'; +import { deprecate } from '@ember/application/deprecations'; /** * @module Validators @@ -210,13 +211,20 @@ export default function(arg1, options) { }; if (typeof arg1 === 'function') { - props.validate = arg1; - props._type = 'function'; + deprecate( + '[ember-cp-validations] `validator` no longer directly accepts ' + + 'a function. Please use the inline validator syntax:' + + "\n\nvalidator('inline', { validate() {} )\n\n", + false, + { id: 'ember-cp-validations.inline-validator', until: '4.2.0' } + ); + props.options.validate = arg1; + props._type = 'inline'; } else if (typeof arg1 === 'string') { props._type = arg1; } else { throw new TypeError( - '[ember-cp-validations] Unexpected type for first validator argument. It should either be a string or a function' + '[ember-cp-validations] Unexpected type for first validator argument — It must be a string.' ); } diff --git a/addon/validators/base.js b/addon/validators/base.js index d635f950..8b55d140 100644 --- a/addon/validators/base.js +++ b/addon/validators/base.js @@ -115,7 +115,7 @@ const Base = EmberObject.extend({ /** * Build options hook. Merges default options into options object. * This method gets called on init and is the ideal place to normalize your options. - * The [presence validator](https://github.com/offirgolan/ember-cp-validations/blob/master/app/validators/presence.js) is a good example to checkout + * The [presence validator](https://github.com/offirgolan/ember-cp-validations/blob/master/addon/validators/presence.js) is a good example to checkout * @method buildOptions * @param {Object} options * @param {Object} defaultOptions @@ -302,7 +302,7 @@ const Base = EmberObject.extend({ Base.reopenClass({ /** - * Generate the needed depenent keys for this validator + * Generate the needed dependent keys for this validator * * @method getDependentsFor * @static @@ -466,20 +466,3 @@ export default Base; * @module Validators * @extends Base */ - -/** - * A validator can also be declared with an inline function. The function will be then wrapped in the {{#crossLink 'Base'}}Base Validator{{/crossLink}} class and used just like any other pre-defined validator. - * - * ```javascript - * // Example - * validator(function(value, options, model, attribute) { - * return value === options.username ? true : `{description} must be ${options.username}`; - * } , { - * username: 'John' // Any options can be passed here - * }) - * ``` - * - * @class Inline - * @module Validators - * @extends Base - */ diff --git a/addon/validators/inline.js b/addon/validators/inline.js new file mode 100755 index 00000000..74f1f94b --- /dev/null +++ b/addon/validators/inline.js @@ -0,0 +1,49 @@ +import Base from 'ember-cp-validations/validators/base'; +import { assign } from '@ember/polyfills'; +import { assert } from '@ember/debug'; + +/** + * Accepts a custom `validate` function. + * + * ## Examples + * + * ```javascript + * validator('inline', { + * username: 'offirgolan', + * validate(value, options, model, attribute) { + * return value === options.username ? + * true : + * `Username must be ${options.username}`; + * } + * }); + * ``` + * + * @class Inline + * @module Validators + * @extends Base + */ +export default Base.extend({ + /** + * Override the validator's `validate` method with the one that was + * passed in via the options. + * + * @method buildOptions + * @param {Object} options + * @param {Object} defaultOptions + * @param {Object} globalOptions + * @return {Object} + */ + buildOptions(options = {}, ...args) { + assert( + `[validator:inline] You must pass in a validate function`, + options && typeof options.validate === 'function' + ); + + const opts = assign({}, options); + + this.validate = opts.validate; + delete opts.validate; + + return this._super(opts, ...args); + } +}); diff --git a/app/validators/inline.js b/app/validators/inline.js new file mode 100644 index 00000000..471dd231 --- /dev/null +++ b/app/validators/inline.js @@ -0,0 +1 @@ +export { default } from 'ember-cp-validations/validators/inline'; diff --git a/tests/dummy/app/models/signup.js b/tests/dummy/app/models/signup.js index abaa54f3..7aa0836f 100644 --- a/tests/dummy/app/models/signup.js +++ b/tests/dummy/app/models/signup.js @@ -3,8 +3,10 @@ import { validator, buildValidations } from 'ember-cp-validations'; let Validations = buildValidations({ name: validator('presence', true), - acceptTerms: validator(value => { - return value || 'You must accept the terms.'; + acceptTerms: validator('inline', { + validate(value) { + return value || 'You must accept the terms.'; + } }) }); diff --git a/tests/integration/validations/factory-dependent-keys-test.js b/tests/integration/validations/factory-dependent-keys-test.js index cc738b67..b774a521 100644 --- a/tests/integration/validations/factory-dependent-keys-test.js +++ b/tests/integration/validations/factory-dependent-keys-test.js @@ -91,19 +91,17 @@ test('nested ds-error validator creates correct dependent keys', function(assert test('custom dependent keys - simple', function(assert) { let Validations = buildValidations({ - fullName: validator( - function(value, options, model) { + fullName: validator('inline', { + dependentKeys: ['model.firstName', 'model.lastName'], + validate(value, options, model) { let firstName = model.get('firstName'); let lastName = model.get('lastName'); if (!firstName || !lastName) { return 'Full name requires first and last name'; } return true; - }, - { - dependentKeys: ['model.firstName', 'model.lastName'] } - ) + }) }); let obj = setupObject(this, EmberObject.extend(Validations)); @@ -127,19 +125,17 @@ test('custom dependent keys - default options', function(assert) { fullName: { dependentKeys: ['model.firstName'], validators: [ - validator( - function(value, options, model) { + validator('inline', { + dependentKeys: ['model.lastName'], + validate(value, options, model) { let firstName = model.get('firstName'); let lastName = model.get('lastName'); if (!firstName || !lastName) { return 'Full name requires first and last name'; } return true; - }, - { - dependentKeys: ['model.lastName'] } - ) + }) ] } }); @@ -166,8 +162,9 @@ test('custom dependent keys - global options', function(assert) { fullName: { dependentKeys: ['model.firstName'], validators: [ - validator( - function(value, options, model) { + validator('inline', { + dependentKeys: ['model.lastName'], + validate(value, options, model) { let firstName = model.get('firstName'); let lastName = model.get('lastName'); let middleName = model.get('middleName'); @@ -175,11 +172,8 @@ test('custom dependent keys - global options', function(assert) { return 'Full name requires first, middle, and last name'; } return true; - }, - { - dependentKeys: ['model.lastName'] } - ) + }) ] } }, @@ -211,19 +205,17 @@ test('custom dependent keys - global options', function(assert) { test('custom dependent keys - nested object', function(assert) { let Validations = buildValidations({ - page: validator( - function(value, options, model) { + page: validator('inline', { + dependentKeys: ['model.currPage', 'model.meta.pages.last'], + validate(value, options, model) { let currPage = model.get('currPage'); let lastPage = model.get('meta.pages.last'); if (currPage > lastPage) { return 'Cannot exceed max page'; } return true; - }, - { - dependentKeys: ['model.currPage', 'model.meta.pages.last'] } - ) + }) }); let obj = setupObject(this, EmberObject.extend(Validations), { @@ -255,18 +247,16 @@ test('custom dependent keys - nested object', function(assert) { test('custom dependent keys - array', function(assert) { let Validations = buildValidations({ - friends: validator( - function(value, options, model) { + friends: validator('inline', { + dependentKeys: ['model.friends.[]'], + validate(value, options, model) { let friends = model.get('friends'); if (!friends || friends.length === 0) { return 'User must have a friend'; } return true; - }, - { - dependentKeys: ['model.friends.[]'] } - ) + }) }); let obj = setupObject(this, EmberObject.extend(Validations), { @@ -297,8 +287,9 @@ test('custom dependent keys - array', function(assert) { test('custom dependent keys - array of objects', function(assert) { let Validations = buildValidations({ - friends: validator( - function(value, options, model) { + friends: validator('inline', { + dependentKeys: ['model.friends.@each.name'], + validate(value, options, model) { let friends = model.get('friends'); if (!friends || friends.length === 0) { return 'User must have a friend'; @@ -309,11 +300,8 @@ test('custom dependent keys - array of objects', function(assert) { } } return true; - }, - { - dependentKeys: ['model.friends.@each.name'] } - ) + }) }); let obj = setupObject(this, EmberObject.extend(Validations), { diff --git a/tests/integration/validations/factory-general-test.js b/tests/integration/validations/factory-general-test.js index b779d7b5..9c919af6 100644 --- a/tests/integration/validations/factory-general-test.js +++ b/tests/integration/validations/factory-general-test.js @@ -24,8 +24,8 @@ const Validators = { }; let Validations = buildValidations({ - firstName: validator(Validators.presence), - lastName: validator(Validators.presence) + firstName: validator('inline', { validate: Validators.presence }), + lastName: validator('inline', { validate: Validators.presence }) }); moduleFor('foo', 'Integration | Validations | Factory - General', { @@ -348,7 +348,7 @@ test('basic sync validation - API - #validationSync', function(assert) { test('basic sync validation returns null', function(assert) { let Validations = buildValidations({ - firstName: validator(() => null) + firstName: validator('inline', { validate: () => null }) }); let object = setupObject(this, EmberObject.extend(Validations), { firstName: 'Offir' @@ -382,10 +382,12 @@ test('basic sync validation returns null', function(assert) { test('shallow isAsync test', function(assert) { let Validations = buildValidations({ - firstName: validator(function() { - return new EmberPromise(resolve => { - resolve(true); - }); + firstName: validator('inline', { + validate() { + return new EmberPromise(resolve => { + resolve(true); + }); + } }) }); @@ -404,7 +406,10 @@ test('default options', function(assert) { let Validations = buildValidations({ firstName: { description: 'Test field', - validators: [validator(Validators.presence), validator(() => false)] + validators: [ + validator('inline', { validate: Validators.presence }), + validator('inline', { validate: () => false }) + ] } }); let object = setupObject(this, EmberObject.extend(Validations), { @@ -458,8 +463,10 @@ test('global options', function(assert) { test('custom messages object', function(assert) { this.register('validator:messages', DefaultMessages); let Validations = buildValidations({ - firstName: validator(function(value) { - return this.createErrorMessage('test', value); + firstName: validator('inline', { + validate(value) { + return this.createErrorMessage('test', value); + } }) }); @@ -494,8 +501,9 @@ test('debounced validations', function(assert) { let done = assert.async(); let initSetup = true; let Validations = buildValidations({ - firstName: validator(Validators.presence), - lastName: validator(Validators.presence, { + firstName: validator('inline', { validate: Validators.presence }), + lastName: validator('inline', { + validate: Validators.presence, debounce: computed(function() { return initSetup ? 0 : 500; // Do not debounce on initial object creation }).volatile() @@ -548,7 +556,8 @@ test('debounced validator should only be called once', function(assert) { let done = assert.async(); let Validations = buildValidations({ - firstName: validator(() => count++, { + firstName: validator('inline', { + validate: () => count++, debounce: 500 }) }); @@ -574,20 +583,18 @@ test('debounced validations should cleanup on object destroy', function(assert) let done = assert.async(); let initSetup = true; - let debouncedValidator = validator( - (value, options, model, attr) => { + let debouncedValidator = validator('inline', { + debounce: computed(function() { + return initSetup ? 0 : 500; + }).volatile(), + validate(value, options, model, attr) { model.set('foo', 'bar'); return Validators.presence(value, options, model, attr); - }, - { - debounce: computed(function() { - return initSetup ? 0 : 500; - }).volatile() } - ); + }); let Validations = buildValidations({ - firstName: validator(Validators.presence), + firstName: validator('inline', { validate: Validators.presence }), lastName: debouncedValidator, 'details.url': debouncedValidator }); @@ -646,8 +653,9 @@ test('debounced validations should cleanup on object destroy', function(assert) test('disabled validations - simple', function(assert) { let Validations = buildValidations({ - firstName: validator(Validators.presence), - lastName: validator(Validators.presence, { + firstName: validator('inline', { validate: Validators.presence }), + lastName: validator('inline', { + validate: Validators.presence, disabled: true }) }); @@ -684,8 +692,9 @@ test('disabled validations - simple', function(assert) { test('disabled validations - cp with dependent key', function(assert) { let Validations = buildValidations({ - firstName: validator(Validators.presence), - lastName: validator(Validators.presence, { + firstName: validator('inline', { validate: Validators.presence }), + lastName: validator('inline', { + validate: Validators.presence, disabled: not('model.validateLastName') }) }); @@ -735,8 +744,8 @@ test('attribute validation result options hash', function(assert) { firstName: { description: 'First Name', validators: [ - validator(Validators.presence, {}), - validator(Validators.presence, { presence: true }), + validator('inline', { validate: Validators.presence }), + validator('inline', { validate: Validators.presence, presence: true }), validator('presence', true), validator('length', { min: 1, @@ -751,17 +760,17 @@ test('attribute validation result options hash', function(assert) { assert.ok(options); assert.deepEqual( Object.keys(options).sort(), - ['presence', 'length', 'function'].sort() + ['presence', 'length', 'inline'].sort() ); - assert.ok(isArray(options['function']) && options['function'].length === 2); + assert.ok(isArray(options['inline']) && options['inline'].length === 2); assert.ok(options.presence.presence); assert.equal(options.length.min, 1); - assert.ok(options['function'][1].presence); + assert.ok(options['inline'][1].presence); // Default options built into option objects assert.equal(options.length.description, 'First Name'); assert.equal(options.presence.description, 'First Name'); - assert.equal(options['function'][0].description, 'First Name'); + assert.equal(options['inline'][0].description, 'First Name'); }); test('attribute validation result options hash with cps', function(assert) { @@ -790,15 +799,15 @@ test('attribute validation result options hash with cps', function(assert) { test('validations persist with inheritance', function(assert) { let Parent = EmberObject.extend( buildValidations({ - firstName: validator(Validators.presence), - lastName: validator(Validators.presence) + firstName: validator('inline', { validate: Validators.presence }), + lastName: validator('inline', { validate: Validators.presence }) }) ); let Child = Parent.extend( buildValidations({ - middleName: validator(Validators.presence), - dob: validator(Validators.presence) + middleName: validator('inline', { validate: Validators.presence }), + dob: validator('inline', { validate: Validators.presence }) }) ); @@ -832,22 +841,22 @@ test('validations persist with inheritance', function(assert) { test('validations persist with deep inheritance', function(assert) { let Parent = EmberObject.extend( buildValidations({ - firstName: validator(Validators.presence), - lastName: validator(Validators.presence) + firstName: validator('inline', { validate: Validators.presence }), + lastName: validator('inline', { validate: Validators.presence }) }) ); let Child = Parent.extend( buildValidations({ - middleName: validator(Validators.presence), - dob: validator(Validators.presence) + middleName: validator('inline', { validate: Validators.presence }), + dob: validator('inline', { validate: Validators.presence }) }) ); let Baby = Child.extend( buildValidations({ - diaper: validator(Validators.presence), - favParent: validator(Validators.presence) + diaper: validator('inline', { validate: Validators.presence }), + favParent: validator('inline', { validate: Validators.presence }) }) ); @@ -887,8 +896,8 @@ test('validations persist with deep inheritance', function(assert) { test('nested keys - simple', function(assert) { let Validations = buildValidations({ - 'user.firstName': validator(Validators.presence), - 'user.lastName': validator(Validators.presence) + 'user.firstName': validator('inline', { validate: Validators.presence }), + 'user.lastName': validator('inline', { validate: Validators.presence }) }); let object = setupObject(this, EmberObject.extend(Validations), { user: {} @@ -911,9 +920,9 @@ test('nested keys - simple', function(assert) { test('nested keys - complex', function(assert) { let Validations = buildValidations({ - firstName: validator(Validators.presence), - 'user.foo.bar.baz': validator(Validators.presence), - 'user.foo.boop': validator(Validators.presence) + firstName: validator('inline', { validate: Validators.presence }), + 'user.foo.bar.baz': validator('inline', { validate: Validators.presence }), + 'user.foo.boop': validator('inline', { validate: Validators.presence }) }); let object = setupObject(this, EmberObject.extend(Validations)); @@ -953,17 +962,19 @@ test('nested keys - complex', function(assert) { test('nested keys - inheritance', function(assert) { let Parent = EmberObject.extend( buildValidations({ - firstName: validator(Validators.presence), - 'user.firstName': validator(Validators.presence), - 'user.middleName': validator(Validators.presence) + firstName: validator('inline', { validate: Validators.presence }), + 'user.firstName': validator('inline', { validate: Validators.presence }), + 'user.middleName': validator('inline', { validate: Validators.presence }) }) ); let Child = Parent.extend( buildValidations({ - 'user.lastName': validator(Validators.presence), - 'user.middleName': validator(function() { - return 'Middle name invalid'; + 'user.lastName': validator('inline', { validate: Validators.presence }), + 'user.middleName': validator('inline', { + validate() { + return 'Middle name invalid'; + } }) }) ); @@ -1006,7 +1017,7 @@ test('call super in validations class with no super property', function(assert) assert.expect(1); let Validations = buildValidations({ - firstName: validator(Validators.presence) + firstName: validator('inline', { validate: Validators.presence }) }); let mixin = Mixin.create({ @@ -1039,15 +1050,15 @@ test('call super in validations class with no super property', function(assert) test('multiple mixins', function(assert) { let Validations1 = buildValidations({ - firstName: validator(Validators.presence) + firstName: validator('inline', { validate: Validators.presence }) }); let Validations2 = buildValidations({ - middleName: validator(Validators.presence) + middleName: validator('inline', { validate: Validators.presence }) }); let Validations3 = buildValidations({ - lastName: validator(Validators.presence) + lastName: validator('inline', { validate: Validators.presence }) }); let object = setupObject( @@ -1081,7 +1092,10 @@ test('multiple mixins', function(assert) { test('validateAttribute - sync validations', function(assert) { let Validations = buildValidations({ - firstName: [validator(Validators.presence), validator(() => true)] + firstName: [ + validator('inline', { validate: Validators.presence }), + validator('inline', { validate: () => true }) + ] }); let object = setupObject(this, EmberObject.extend(Validations), { firstName: 'Offir' @@ -1100,8 +1114,12 @@ test('validateAttribute - sync validations', function(assert) { test('validateAttribute - async validations', function(assert) { let Validations = buildValidations({ firstName: [ - validator(() => EmberPromise.resolve('firstName is invalid')), - validator(() => EmberPromise.resolve('firstName is really invalid')) + validator('inline', { + validate: () => EmberPromise.resolve('firstName is invalid') + }), + validator('inline', { + validate: () => EmberPromise.resolve('firstName is really invalid') + }) ] }); let object = setupObject(this, EmberObject.extend(Validations), { @@ -1308,9 +1326,11 @@ test('lazy validators are actually lazy', function(assert) { validator('length', { min: 5 }), - validator(() => { - customValidatorCount++; - return 'Password is not valid'; + validator('inline', { + validate() { + customValidatorCount++; + return 'Password is not valid'; + } }) ] } @@ -1372,15 +1392,13 @@ test('none lazy validators are actually not lazy', function(assert) { min: 5, lazy: false }), - validator( - () => { + validator('inline', { + lazy: false, + validate() { customValidatorCount++; return 'Password is not valid'; - }, - { - lazy: false } - ) + }) ] } }); diff --git a/tests/integration/validations/model-relationships-test.js b/tests/integration/validations/model-relationships-test.js index 461c0fc3..bc4bdc89 100644 --- a/tests/integration/validations/model-relationships-test.js +++ b/tests/integration/validations/model-relationships-test.js @@ -23,8 +23,8 @@ const Validators = { }; const Validations = buildValidations({ - firstName: validator(Validators.presence), - lastName: validator(Validators.presence) + firstName: validator('inline', { validate: Validators.presence }), + lastName: validator('inline', { validate: Validators.presence }) }); const BelongsToValidations = buildValidations({ @@ -400,8 +400,8 @@ test('alias validation - simple', function(assert) { this, EmberObject.extend( buildValidations({ - firstName: validator(Validators.presence), - lastName: validator(Validators.presence), + firstName: validator('inline', { validate: Validators.presence }), + lastName: validator('inline', { validate: Validators.presence }), fullName: validator('alias', 'firstName') }) ) @@ -445,8 +445,8 @@ test('alias validation - firstMessageOnly', function(assert) { buildValidations( { firstName: [ - validator(() => 'First error message'), - validator(() => 'Second error message') + validator('inline', { validate: () => 'First error message' }), + validator('inline', { validate: () => 'Second error message' }) ], fullName: validator('alias', { alias: 'firstName', @@ -480,8 +480,8 @@ test('alias validation - multiple', function(assert) { EmberObject.extend( buildValidations( { - firstName: validator(Validators.presence), - lastName: validator(Validators.presence), + firstName: validator('inline', { validate: Validators.presence }), + lastName: validator('inline', { validate: Validators.presence }), fullName: [ validator('alias', 'firstName'), validator('alias', 'lastName') diff --git a/tests/unit/validations/ds-model-test.js b/tests/unit/validations/ds-model-test.js index 4ba34f6e..6d643c0e 100644 --- a/tests/unit/validations/ds-model-test.js +++ b/tests/unit/validations/ds-model-test.js @@ -2,7 +2,7 @@ import { run } from '@ember/runloop'; import { moduleForModel, test } from 'ember-qunit'; moduleForModel('signup', 'Unit | Validations | DS.Model', { - needs: ['validator:presence'] + needs: ['validator:presence', 'validator:inline'] }); test('create model with defaults', function(assert) { diff --git a/tests/unit/validators/inline-test.js b/tests/unit/validators/inline-test.js new file mode 100644 index 00000000..fe8cb95c --- /dev/null +++ b/tests/unit/validators/inline-test.js @@ -0,0 +1,30 @@ +import { moduleFor, test } from 'ember-qunit'; + +moduleFor('validator:inline', 'Unit | Validator | inline'); + +test('no options', function(assert) { + assert.expect(1); + + try { + this.subject(); + } catch (e) { + assert.ok(true); + } +}); + +test('it works', function(assert) { + assert.expect(3); + + const validator = this.subject({ + options: { + foo: 'bar', + validate(value, options) { + assert.equal(this, validator, 'Context is preserved'); + assert.equal(options.foo, 'bar', 'It receives options'); + assert.notOk(options.validate, 'Validate fn removed from the options'); + } + } + }); + + validator.validate('foo', validator.get('options').toObject()); +});