From 1a5aab54aaa88ce51c2086ab0e3c11ab3c1b8ecb Mon Sep 17 00:00:00 2001 From: Offir Golan Date: Fri, 15 Apr 2016 20:03:30 -0700 Subject: [PATCH 1/3] Only call super on validations class if it exists via shouldCallSuper --- addon/utils/should-call-super.js | 27 ++++++++ addon/utils/utils.js | 5 ++ addon/validations/factory.js | 30 +++++++-- .../validations/factory-general-test.js | 67 +++++++++++++++++++ tests/unit/utils/should-call-super-test.js | 37 ++++++++++ 5 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 addon/utils/should-call-super.js create mode 100644 tests/unit/utils/should-call-super-test.js diff --git a/addon/utils/should-call-super.js b/addon/utils/should-call-super.js new file mode 100644 index 00000000..e14c4cfe --- /dev/null +++ b/addon/utils/should-call-super.js @@ -0,0 +1,27 @@ +/** + * Copyright 2016, Yahoo! Inc. + * Copyrights licensed under the New BSD License. See the accompanying LICENSE file for terms. + */ + +/** + * Checks if the give key exists on the object's super. + * If so, we can successfuly call the obj[key] _super + * + * Created by @rwjblue + */ +export default function shouldCallSuper(obj, key) { + let current = Object.getPrototypeOf(obj); + current = Object.getPrototypeOf(current); + + while (current) { + let descriptor = Object.getOwnPropertyDescriptor(current, key); + + if (descriptor) { + return true; + } + + current = Object.getPrototypeOf(current); + } + + return false; +} diff --git a/addon/utils/utils.js b/addon/utils/utils.js index 6600816a..d4d39c04 100644 --- a/addon/utils/utils.js +++ b/addon/utils/utils.js @@ -1,3 +1,8 @@ +/** + * Copyright 2016, Yahoo! Inc. + * Copyrights licensed under the New BSD License. See the accompanying LICENSE file for terms. + */ + export function hasEmberData() { return typeof self.DS !== 'undefined'; } diff --git a/addon/validations/factory.js b/addon/validations/factory.js index d79cd916..06ae8cb5 100644 --- a/addon/validations/factory.js +++ b/addon/validations/factory.js @@ -11,6 +11,7 @@ import ValidationResult from './result'; import ValidationResultCollection from './result-collection'; import BaseValidator from '../validators/base'; import cycleBreaker from '../utils/cycle-breaker'; +import shouldCallSuper from '../utils/should-call-super'; const { get, @@ -98,14 +99,27 @@ function buildValidations(validations = {}, globalOptions = {}) { let Validations; return Ember.Mixin.create({ - _validationsClass: computed(function () { + init() { + this._super(...arguments); + + // Count number of mixins to bypass super check if there is more than 1 + this.__validationsMixinCount__ = this.__validationsMixinCount__|| 0; + this.__validationsMixinCount__++; + }, + __validationsClass__: computed(function () { if (!Validations) { - Validations = createValidationsClass(this._super(), validations); + let inheritedClass; + + if(shouldCallSuper(this, '__validationsClass__') || this.__validationsMixinCount__ > 1) { + inheritedClass = this._super(); + } + + Validations = createValidationsClass(inheritedClass, validations); } return Validations; }).readOnly(), validations: computed(function () { - return this.get('_validationsClass').create({ + return this.get('__validationsClass__').create({ model: this }); }).readOnly(), @@ -181,7 +195,7 @@ function createValidationsClass(inheritedValidationsClass, validations = {}) { let validatableAttributes = Object.keys(validations); // Setup validation inheritance - if (inheritedValidationsClass) { + if (inheritedValidationsClass && inheritedValidationsClass.__isCPValidationsClass__) { const inheritedValidations = inheritedValidationsClass.create(); validationRules = merge(validationRules, inheritedValidations.get('_validationRules')); @@ -235,7 +249,7 @@ function createValidationsClass(inheritedValidationsClass, validations = {}) { }); // Create `validations` class - return Ember.Object.extend(TopLevelProps, { + const ValidationClass = Ember.Object.extend(TopLevelProps, { model: null, attrs: null, isValidations: true, @@ -286,6 +300,12 @@ function createValidationsClass(inheritedValidationsClass, validations = {}) { }); } }); + + ValidationClass.reopenClass({ + __isCPValidationsClass__: true + }); + + return ValidationClass; } /** diff --git a/tests/integration/validations/factory-general-test.js b/tests/integration/validations/factory-general-test.js index a6f2f76e..46037b4c 100644 --- a/tests/integration/validations/factory-general-test.js +++ b/tests/integration/validations/factory-general-test.js @@ -650,3 +650,70 @@ test("nested keys - inheritance", function(assert) { }); +test("call super in validations class with no super property", function(assert) { + // see https://github.com/offirgolan/ember-cp-validations/issues/149 + assert.expect(1); + + var Validations = buildValidations({ + 'firstName': validator(Validators.presence) + }); + + var mixin = Ember.Mixin.create({ + actions: { + foo() { + // The issue is that __validationsClass__ is calling super but since + // this is no __validationsClass__ method on the super, it is wrapped + // with the closest context which is the 'foo' action + assert.ok(false); // should not reach here + } + } + }); + + var component = setupObject(this, Ember.Component.extend(Validations, mixin, { + actions: { + foo() { + assert.ok(true); + const validations = this.get('validations'); + } + } + })); + + component.send('foo'); +}); + + +test("multiple mixins", function(assert) { + var Validations1 = buildValidations({ + 'firstName': validator(Validators.presence) + }); + + var Validations2 = buildValidations({ + 'middleName': validator(Validators.presence) + }); + + var Validations3 = buildValidations({ + 'lastName': validator(Validators.presence) + }); + + var object = setupObject(this, Ember.Object.extend(Validations1, Validations2, Validations3)); + + assert.deepEqual(object.get('validations.validatableAttributes').sort(), ['firstName', 'middleName', 'lastName'].sort()); + assert.equal(object.get('validations.attrs.firstName.isValid'), false); + assert.equal(object.get('validations.attrs.lastName.isValid'), false); + assert.equal(object.get('validations.isValid'), false); + + object.set('firstName', 'Offir'); + + assert.equal(object.get('validations.attrs.firstName.isValid'), true); + assert.equal(object.get('validations.isValid'), false); + + object.set('middleName', 'David'); + + assert.equal(object.get('validations.attrs.middleName.isValid'), true); + assert.equal(object.get('validations.isValid'), false); + + object.set('lastName', 'Golan'); + + assert.equal(object.get('validations.attrs.lastName.isValid'), true); + assert.equal(object.get('validations.isValid'), true); +}); diff --git a/tests/unit/utils/should-call-super-test.js b/tests/unit/utils/should-call-super-test.js new file mode 100644 index 00000000..73db368d --- /dev/null +++ b/tests/unit/utils/should-call-super-test.js @@ -0,0 +1,37 @@ +import Ember from 'ember'; +import { module, test } from 'qunit'; +import shouldCallSuper from 'ember-cp-validations/utils/should-call-super'; + +const { + computed +} = Ember; + +module('Unit | Utils | shouldCallSuper'); + +test('shouldCallSuper - true', function(assert) { + const Parent = Ember.Object.extend({ + foo: computed(function() {}) + }); + + const Child = Parent.extend({ + foo: computed(function() { + assert.ok(shouldCallSuper(this, 'foo')); + }) + }); + + const child = Child.create(); + child.get('foo'); +}); + +test('shouldCallSuper - false', function(assert) { + const Parent = Ember.Object.extend(); + + const Child = Parent.extend({ + foo: computed(function() { + assert.ok(!shouldCallSuper(this, 'foo')); + }) + }); + + const child = Child.create(); + child.get('foo'); +}); From bfc1a18d51d3d566a9ac2a139614e82ef8b2a6c6 Mon Sep 17 00:00:00 2001 From: Offir Golan Date: Fri, 15 Apr 2016 20:07:31 -0700 Subject: [PATCH 2/3] fix naming convention --- addon/validations/factory.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/addon/validations/factory.js b/addon/validations/factory.js index 06ae8cb5..d71db08e 100644 --- a/addon/validations/factory.js +++ b/addon/validations/factory.js @@ -249,7 +249,7 @@ function createValidationsClass(inheritedValidationsClass, validations = {}) { }); // Create `validations` class - const ValidationClass = Ember.Object.extend(TopLevelProps, { + const ValidationsClass = Ember.Object.extend(TopLevelProps, { model: null, attrs: null, isValidations: true, @@ -301,11 +301,11 @@ function createValidationsClass(inheritedValidationsClass, validations = {}) { } }); - ValidationClass.reopenClass({ + ValidationsClass.reopenClass({ __isCPValidationsClass__: true }); - return ValidationClass; + return ValidationsClass; } /** From 4dcb03142458ac7bac8b54185fc3f1732bdfba6e Mon Sep 17 00:00:00 2001 From: Offir Golan Date: Fri, 15 Apr 2016 20:28:18 -0700 Subject: [PATCH 3/3] store mixin count in closure --- addon/validations/factory.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/addon/validations/factory.js b/addon/validations/factory.js index d71db08e..e7a43556 100644 --- a/addon/validations/factory.js +++ b/addon/validations/factory.js @@ -96,21 +96,21 @@ default function buildValidations(validations = {}, globalOptions = {}) { normalizeOptions(validations, globalOptions); - let Validations; + let Validations, validationMixinCount; return Ember.Mixin.create({ init() { this._super(...arguments); // Count number of mixins to bypass super check if there is more than 1 - this.__validationsMixinCount__ = this.__validationsMixinCount__|| 0; - this.__validationsMixinCount__++; + this.__validationsMixinCount__ = this.__validationsMixinCount__ || 0; + validationMixinCount = ++this.__validationsMixinCount__; }, __validationsClass__: computed(function () { if (!Validations) { let inheritedClass; - - if(shouldCallSuper(this, '__validationsClass__') || this.__validationsMixinCount__ > 1) { + console.log(shouldCallSuper(this, '__validationsClass__'), validationMixinCount); + if(shouldCallSuper(this, '__validationsClass__') || validationMixinCount > 1) { inheritedClass = this._super(); }