From f4eaba8a7692806f0bf994f35308a941ff0f0388 Mon Sep 17 00:00:00 2001 From: Rafael Weinstein Date: Thu, 12 Dec 2013 10:10:06 -0800 Subject: [PATCH] Refactor to support Observable interface This patch formalizes the Observable interface. This is part of making the Node.bind() take an Observable, rather than model & path. The main implications to observe-js are: -Path/CompoundPath/Object/ArrayObserver all implement the Observable interface -PathObserver and CompoundObserver no longer have a .value property -PathObserver and CompoundObserver no longer have optional transform functions. These are now concerns of the ObserverTransform. R=arv@chromium.org, arv BUG= Review URL: https://codereview.appspot.com/40460045 --- src/observe.js | 336 +++++++++++++---------- tests/array_fuzzer.js | 3 +- tests/test.js | 570 ++++++++++++++++++++++------------------ util/array_reduction.js | 11 +- 4 files changed, 515 insertions(+), 405 deletions(-) diff --git a/src/observe.js b/src/observe.js index 24e4b9c..6d0b5d5 100644 --- a/src/observe.js +++ b/src/observe.js @@ -345,13 +345,23 @@ return copy; } - function Observer(object, callback, target) { - this.closed_ = false; - this.object_ = object; - this.callback = callback; // TODO(rafaelw): Used in NodeBind - // TODO(rafaelw): Hold this.target weakly when WeakRef is available. - this.target = target; // TODO(rafaelw): Used in NodeBind + function isObservable(obj) { + return obj && + typeof obj.open == 'function' && + typeof obj.close == 'function'; + } + + var UNOPENED = 0; + var OPENED = 1; + var CLOSED = 2; + function Observer() { + this.state_ = UNOPENED; + + this.value_ = undefined; + this.callback_ = undefined; + this.target_ = undefined; // TODO(rafaelw): Should be WeakRef this.reporting_ = true; + this.reportArgs_ = undefined; if (hasObserve) { var self = this; this.boundInternalCallback_ = function(records) { @@ -363,8 +373,20 @@ } Observer.prototype = { + open: function(callback, target) { + if (this.state_ != UNOPENED) + throw Error('Observer has already been opened.'); + + this.callback_ = callback; + this.target_ = target; + this.state_ = OPENED; + this.connect_(); + this.sync_(true); + return this.value_; + }, + internalCallback_: function(records) { - if (this.closed_) + if (this.state_ != OPENED) return; if (this.reporting_ && this.check_(records)) { this.report_(); @@ -374,19 +396,20 @@ }, close: function() { - if (this.closed_) + if (this.state_ == CLOSED) return; - if (this.object_ && typeof this.object_.close === 'function') - this.object_.close(); + this.state_ = CLOSED; this.disconnect_(); - this.object_ = undefined; - this.closed_ = true; + this.value_ = undefined; + this.callback_ = undefined; + this.target_ = undefined; }, deliver: function(testingResults) { - if (this.closed_) + if (this.state_ != OPENED) return; + if (hasObserve) { this.testingResults = testingResults; Object.deliverChangeRecords(this.boundInternalCallback_); @@ -401,15 +424,15 @@ return; this.sync_(false); - if (this.callback) { - this.invokeCallback_(this.reportArgs); + if (this.callback_) { + this.invokeCallback_(this.reportArgs_); } - this.reportArgs = undefined; + this.reportArgs_ = undefined; }, invokeCallback_: function(args) { try { - this.callback.apply(this.target, args); + this.callback_.apply(this.target_, args); } catch (ex) { Observer._errorThrownDuringCallback = true; console.error('Exception caught during observer callback: ' + (ex.stack || ex)); @@ -417,8 +440,8 @@ }, reset: function() { - if (this.closed_) - return; + if (this.state_ != OPENED) + throw Error('Observer is not open'); if (hasObserve) { this.reporting_ = false; @@ -427,6 +450,7 @@ } this.sync_(true); + return this.value_; } } @@ -477,7 +501,7 @@ for (var i = 0; i < toCheck.length; i++) { var observer = toCheck[i]; - if (observer.closed_) + if (observer.state_ != OPENED) continue; if (hasObserve) { @@ -504,10 +528,10 @@ }; } - function ObjectObserver(object, callback, target) { - Observer.call(this, object, callback, target); - this.connect_(); - this.sync_(true); + function ObjectObserver(object) { + Observer.call(this); + this.value_ = object; + this.oldObject_ = undefined; } ObjectObserver.prototype = createObject({ @@ -515,12 +539,12 @@ connect_: function() { if (hasObserve) - Object.observe(this.object_, this.boundInternalCallback_); + Object.observe(this.value_, this.boundInternalCallback_); }, sync_: function(hard) { if (!hasObserve) - this.oldObject = copyObject(this.object_); + this.oldObject_ = copyObject(this.value_); }, check_: function(changeRecords) { @@ -531,19 +555,19 @@ return false; oldValues = {}; - diff = diffObjectFromChangeRecords(this.object_, changeRecords, + diff = diffObjectFromChangeRecords(this.value_, changeRecords, oldValues); } else { - oldValues = this.oldObject; - diff = diffObjectFromOldObject(this.object_, this.oldObject); + oldValues = this.oldObject_; + diff = diffObjectFromOldObject(this.value_, this.oldObject_); } if (diffIsEmpty(diff)) return false; - this.reportArgs = + this.reportArgs_ = [diff.added || {}, diff.removed || {}, diff.changed || {}]; - this.reportArgs.push(function(property) { + this.reportArgs_.push(function(property) { return oldValues[property]; }); @@ -552,16 +576,16 @@ disconnect_: function() { if (!hasObserve) - this.oldObject = undefined; - else if (this.object_) - Object.unobserve(this.object_, this.boundInternalCallback_); + this.oldObject_ = undefined; + else if (this.value_) + Object.unobserve(this.value_, this.boundInternalCallback_); } }); - function ArrayObserver(array, callback, target) { + function ArrayObserver(array) { if (!Array.isArray(array)) throw Error('Provided object is not an Array'); - ObjectObserver.call(this, array, callback, target); + ObjectObserver.call(this, array); } ArrayObserver.prototype = createObject({ @@ -569,12 +593,12 @@ connect_: function() { if (hasObserve) - Array.observe(this.object_, this.boundInternalCallback_); + Array.observe(this.value_, this.boundInternalCallback_); }, sync_: function() { if (!hasObserve) - this.oldObject = this.object_.slice(); + this.oldObject_ = this.value_.slice(); }, check_: function(changeRecords) { @@ -582,16 +606,16 @@ if (hasObserve) { if (!changeRecords) return false; - splices = projectArraySplices(this.object_, changeRecords); + splices = projectArraySplices(this.value_, changeRecords); } else { - splices = calcSplices(this.object_, 0, this.object_.length, - this.oldObject, 0, this.oldObject.length); + splices = calcSplices(this.value_, 0, this.value_.length, + this.oldObject_, 0, this.oldObject_.length); } if (!splices || !splices.length) return false; - this.reportArgs = [splices]; + this.reportArgs_ = [splices]; return true; } }); @@ -611,7 +635,7 @@ function ObservedSet(callback) { this.arr = []; - this.callback = callback; + this.callback_ = callback; this.isObserved = true; } @@ -634,7 +658,7 @@ if (i < 0) { i = this.arr.length; this.arr[i] = obj; - Object.observe(obj, this.callback); + Object.observe(obj, this.callback_); } this.arr[i+1] = this.isObserved; @@ -653,7 +677,7 @@ } i += 2; } else { - Object.unobserve(obj, this.callback); + Object.unobserve(obj, this.callback_); } j += 2; } @@ -662,23 +686,11 @@ } }; - function PathObserver(object, path, callback, target, transformFn, - setValueFn) { - var path = path instanceof Path ? path : getPath(path); - if (!path.valid || !path.length || !isObject(object)) { - this.value_ = path.getValueFrom(object); - this.value = transformFn ? transformFn(this.value_) : this.value_; - this.closed_ = true; - return; - } - - Observer.call(this, object, callback, target); - this.transformFn_ = transformFn; - this.setValueFn_ = setValueFn; - this.path_ = path; - - this.connect_(); - this.sync_(true); + function PathObserver(object, path) { + Observer.call(this); + this.object_ = object; + this.path_ = path instanceof Path ? path : getPath(path); + this.observedSet_ = undefined; } PathObserver.prototype = createObject({ @@ -705,17 +717,16 @@ if (this.observedSet_) this.observedSet_.reset(); - this.value_ = this.path_.getValueFrom(this.object_, this.observedSet_); + var newValue = this.path_.getValueFrom(this.object_, this.observedSet_); if (this.observedSet_) this.observedSet_.cleanup(); - if (areSameValue(this.value_, this.oldValue_)) + if (areSameValue(this.value_, newValue)) return false; - this.value = this.transformFn_ ? this.transformFn_(this.value_) - : this.value_; - this.reportArgs = [this.value, this.oldValue]; + this.reportArgs_ = [newValue, this.value_]; + this.value_ = newValue; return true; }, @@ -725,140 +736,173 @@ this.observedSet_.reset(); this.value_ = this.path_.getValueFrom(this.object_, this.observedSet_); - this.value = this.transformFn_ ? this.transformFn_(this.value_) - : this.value_; if (this.observedSet_) this.observedSet_.cleanup(); } - - this.oldValue_ = this.value_; - this.oldValue = this.value; }, setValue: function(newValue) { - if (this.setValueFn_) - this.setValueFn_(newValue); - else if (this.path_) + if (this.path_) this.path_.setValueFrom(this.object_, newValue); } }); - function CompoundPathObserver(callback, target, transformFn, setValueFn) { - Observer.call(this, undefined, callback, target); - this.transformFn_ = transformFn; - this.setValueFn_ = setValueFn; + function CompoundObserver() { + Observer.call(this); this.observed_ = []; - this.values_ = []; - this.value = undefined; - this.oldValue = undefined; - this.oldValues_ = undefined; - this.changeFlags_ = undefined; - this.started_ = false; + this.value_ = []; } - CompoundPathObserver.prototype = createObject({ + var observerSentinel = {}; + + CompoundObserver.prototype = createObject({ __proto__: PathObserver.prototype, // TODO(rafaelw): Consider special-casing when |object| is a PathObserver // and path 'value' to avoid explicit observation. addPath: function(object, path) { - if (this.started_) - throw Error('Cannot add more paths once started.'); - - var path = path instanceof Path ? path : getPath(path); - var value = path.getValueFrom(object); + if (this.state_ != UNOPENED) + throw Error('Cannot add paths once started.'); - this.observed_.push(object, path); - this.values_.push(value); + this.observed_.push(object, path instanceof Path ? path : getPath(path)); }, - start: function() { - this.started_ = true; - this.connect_(); - this.sync_(true); + addObserver: function(observer) { + if (this.state_ != UNOPENED) + throw Error('Cannot add observers once started.'); + + if (!isObservable(observer)) + throw Error('Object must be observable'); + + observer.open(this.deliver, this); + var value = observer.value; + + this.observed_.push(observerSentinel, observer); + this.value_.push(value); }, - getValues_: function() { + getValues_: function(sync) { if (this.observedSet_) this.observedSet_.reset(); - var anyChanged = false; - for (var i = 0; i < this.observed_.length; i = i+2) { - var path = this.observed_[i+1]; + var oldValues; + for (var i = 0; i < this.observed_.length; i = i + 2) { + var pathOrObserver = this.observed_[i+1]; var object = this.observed_[i]; - var value = path.getValueFrom(object, this.observedSet_); - var oldValue = this.values_[i/2]; - if (!areSameValue(value, oldValue)) { - if (!anyChanged && !this.transformFn_) { - this.oldValues_ = this.oldValues_ || []; - this.changeFlags_ = this.changeFlags_ || []; - for (var j = 0; j < this.values_.length; j++) { - this.oldValues_[j] = this.values_[j]; - this.changeFlags_[j] = false; - } - } + var value = object === observerSentinel ? pathOrObserver.reset() : + pathOrObserver.getValueFrom(object, this.observedSet_) - if (!this.transformFn_) - this.changeFlags_[i/2] = true; - - this.values_[i/2] = value; - anyChanged = true; + if (sync) { + this.value_[i / 2] = value; + continue; } + + if (areSameValue(value, this.value_[i / 2])) + continue; + + oldValues = oldValues || []; + oldValues[i / 2] = this.value_[i / 2]; + this.value_[i / 2] = value; } if (this.observedSet_) this.observedSet_.cleanup(); - return anyChanged; + return oldValues; }, check_: function() { - if (!this.getValues_()) + var oldValues = this.getValues_(); + if (!oldValues) return; - if (this.transformFn_) { - this.value = this.transformFn_(this.values_); - - if (areSameValue(this.value, this.oldValue)) - return false; - - this.reportArgs = [this.value, this.oldValue]; - } else { - this.reportArgs = [this.values_, this.oldValues_, this.changeFlags_, - this.observed_]; - } - + this.reportArgs_ = [this.value_, oldValues]; return true; }, sync_: function(hard) { - if (hard) { - this.getValues_(); - if (this.transformFn_) - this.value = this.transformFn_(this.values_); - } - - if (this.transformFn_) - this.oldValue = this.value; + if (hard) + this.getValues_(true); }, close: function() { if (this.observed_) { - for (var i = 0; i < this.observed_.length; i = i + 2) { - var object = this.observed_[i]; - if (object && typeof object.close === 'function') - object.close(); + for (var i = 0; i < this.observed_.length; i += 2) { + if (this.observed_[i] === observerSentinel) + this.observed_[i + 1].close(); } this.observed_ = undefined; - this.values_ = undefined; + this.value_ = undefined; } Observer.prototype.close.call(this); } }); + function identFn(value) { return value; } + + function ObserverTransform(observable, getValueFn, setValueFn, + dontPassThroughSet) { + this.callback_ = undefined; + this.target_ = undefined; + this.value_ = undefined; + this.observable_ = observable; + this.getValueFn_ = getValueFn || identFn; + this.setValueFn_ = setValueFn || identFn; + // TODO(rafaelw): This is a temporary hack. PolymerExpressions needs this + // at the moment because of a bug in it's dependency tracking. + this.dontPassThroughSet_ = dontPassThroughSet; + } + + ObserverTransform.prototype = { + open: function(callback, target) { + this.callback_ = callback; + this.target_ = target; + this.value_ = + this.getValueFn_(this.observable_.open(this.observedCallback_, this)); + return this.value_; + }, + + observedCallback_: function(value) { + value = this.getValueFn_(value); + if (areSameValue(value, this.value_)) + return; + var oldValue = this.value_; + this.value_ = value; + this.callback_.call(this.target_, this.value_, oldValue); + }, + + reset: function() { + this.value_ = this.getValueFn_(this.observable_.reset()); + return this.value_; + }, + + deliver: function() { + return this.observable_.deliver(); + }, + + setValue: function(value) { + this.value_ = this.setValueFn_(value); + if (this.dontPassThroughSet_ || !this.observable_.setValue) + return; + + return this.observable_.setValue(this.value_); + }, + + close: function() { + if (this.observable_) + this.observable_.close(); + this.callback_ = undefined; + this.target_ = undefined; + this.observable_ = undefined; + this.value_ = undefined; + this.getValueFn_ = undefined; + this.setValueFn_ = undefined; + } + } + var expectedRecordTypes = {}; expectedRecordTypes[PROP_ADD_TYPE] = true; expectedRecordTypes[PROP_UPDATE_TYPE] = true; @@ -1379,6 +1423,7 @@ global.Observer = Observer; global.Observer.hasObjectObserve = hasObserve; + global.Observer.isObservable = isObservable; global.ArrayObserver = ArrayObserver; global.ArrayObserver.calculateSplices = function(current, previous) { return arraySplice.calculateSplices(current, previous); @@ -1387,8 +1432,9 @@ global.ArraySplice = ArraySplice; global.ObjectObserver = ObjectObserver; global.PathObserver = PathObserver; - global.CompoundPathObserver = CompoundPathObserver; + global.CompoundObserver = CompoundObserver; global.Path = Path; + global.ObserverTransform = ObserverTransform; // TODO(rafaelw): Only needed for testing until new change record names // make it to release. diff --git a/tests/array_fuzzer.js b/tests/array_fuzzer.js index d40aaef..0f756e0 100644 --- a/tests/array_fuzzer.js +++ b/tests/array_fuzzer.js @@ -124,7 +124,8 @@ ArrayFuzzer.prototype.go = function() { var copy = this.copy = this.arr.slice(); this.origCopy = this.copy.slice(); - var observer = new ArrayObserver(this.arr, function(splices) { + var observer = new ArrayObserver(this.arr); + observer.open(function(splices) { ArrayObserver.applySplices(copy, orig, splices); }); diff --git a/tests/test.js b/tests/test.js index ba60008..23a22ba 100644 --- a/tests/test.js +++ b/tests/test.js @@ -18,6 +18,8 @@ var callbackInvoked = false; window.testingExposeCycleCount = true; +function noop() {} + function callback() { callbackArgs = Array.prototype.slice.apply(arguments); callbackInvoked = true; @@ -53,20 +55,15 @@ function assertPathChanges(expectNewValue, expectOldValue) { callbackInvoked = false; } -function assertCompoundPathChanges(expectNewValues, expectOldValues, - expectChangeFlags, expectObserverArg) { +function assertCompoundPathChanges(expectNewValues, expectOldValues) { observer.deliver(); assert.isTrue(callbackInvoked); var newValues = callbackArgs[0]; var oldValues = callbackArgs[1]; - var changeFlags = callbackArgs[2]; - var observerArg = callbackArgs[3]; assert.deepEqual(expectNewValues, newValues); assert.deepEqual(expectOldValues, oldValues); - assert.deepEqual(expectChangeFlags, changeFlags); - assert.deepEqual(expectObserverArg, observerArg); assert.isTrue(window.dirtyCheckCycleCount === undefined || window.dirtyCheckCycleCount === 1); @@ -204,16 +201,22 @@ suite('Basic Tests', function() { var model = [1]; var count = 0; - var observer1 = new ObjectObserver(model, function() { + var observer1 = new ObjectObserver(model); + observer1.open(function() { count++; + throw 'ouch'; }); - var observer2 = new PathObserver(model, '0', function() { + var observer2 = new PathObserver(model, '0'); + observer2.open(function() { count++; + throw 'ouch'; }); - var observer3 = new ArrayObserver(model, function() { + var observer3 = new ArrayObserver(model); + observer3.open(function() { count++; + throw 'ouch'; }); model[0] = 2; @@ -230,6 +233,38 @@ suite('Basic Tests', function() { observer3.close(); }); + test('Can only open once', function() { + observer = new PathObserver({ id: 1 }, 'id'); + observer.open(callback); + assert.throws(function() { + observer.open(callback); + }); + observer.close(); + + observer = new CompoundObserver(new PathObserver({ id: 1 }, 'id'), + noop, noop); + observer.open(callback); + assert.throws(function() { + observer.open(callback); + }); + observer.close(); + + observer = new ObjectObserver({}, 'id'); + observer.open(callback); + assert.throws(function() { + observer.open(callback); + }); + observer.close(); + + observer = new ArrayObserver([], 'id'); + observer.open(callback); + assert.throws(function() { + observer.open(callback); + }); + observer.close(); + + }); + test('No Object.observe performMicrotaskCheckpoint', function() { if (typeof Object.observe == 'function') return; @@ -237,15 +272,18 @@ suite('Basic Tests', function() { var model = [1]; var count = 0; - var observer1 = new ObjectObserver(model, function() { + var observer1 = new ObjectObserver(model); + observer1.open(function() { count++; }); - var observer2 = new PathObserver(model, '0', function() { + var observer2 = new PathObserver(model, '0'); + observer2.open(function() { count++; }); - var observer3 = new ArrayObserver(model, function() { + var observer3 = new ArrayObserver(model); + observer3.open(function() { count++; }); @@ -261,6 +299,91 @@ suite('Basic Tests', function() { }); }); +suite('ObserverTransform', function() { + + test('Close Invokes Close', function() { + var count = 0; + var observer = { + open: function() {}, + close: function() { count++; } + }; + + var observer = new ObserverTransform(observer); + observer.open(); + observer.close(); + assert.strictEqual(1, count); + }); + + test('valueFn/setValueFn', function() { + var obj = { foo: 1 }; + + function valueFn(value) { return value * 2; } + + function setValueFn(value) { return value / 2; } + + observer = new ObserverTransform(new PathObserver(obj, 'foo'), + valueFn, + setValueFn); + observer.open(callback); + + obj.foo = 2; + + assert.strictEqual(4, observer.reset()); + assertNoChanges(); + + observer.setValue(2); + assert.strictEqual(obj.foo, 1); + assert.strictEqual(2, observer.reset()); + assertNoChanges(); + + observer.close(); + }); + + test('valueFn - object literal', function() { + var model = {}; + + function valueFn(value) { + return [ value ]; + } + + observer = new ObserverTransform(new PathObserver(model, 'foo'), valueFn); + observer.open(callback); + + model.foo = 1; + assertPathChanges([1], [undefined]); + + model.foo = 3; + assertPathChanges([3], [1]); + + observer.close(); + }); + + test('CompoundObserver - valueFn reduction', function() { + var model = { a: 1, b: 2, c: 3 }; + + function valueFn(values) { + return values.reduce(function(last, cur) { + return typeof cur === 'number' ? last + cur : undefined; + }, 0); + } + + var compound = new CompoundObserver(); + compound.addPath(model, 'a'); + compound.addPath(model, 'b'); + compound.addPath(model, Path.get('c')); + + observer = new ObserverTransform(compound, valueFn); + assert.strictEqual(6, observer.open(callback)); + + model.a = -10; + model.b = 20; + model.c = 30; + assertPathChanges(40, 6); + + observer.close(); + }); +}) + suite('PathObserver Tests', function() { setup(doSetup); @@ -268,20 +391,13 @@ suite('PathObserver Tests', function() { teardown(doTeardown); test('invalid', function() { - var observer = new PathObserver({ a: { b: 1 }} , 'a b', callback); + var observer = new PathObserver({ a: { b: 1 }} , 'a b'); + observer.open(callback); assert.strictEqual(undefined, observer.value); observer.deliver(); assert.isFalse(callbackInvoked); }); - test('Close Invokes Close', function() { - var called = false; - var obj = { foo: 1, close: function() { called = true }}; - var observer = new PathObserver(obj, 'foo', function() {}); - observer.close(); - assert.isTrue(called); - }); - test('Optional target for callback', function() { var target = { changed: function(value, oldValue) { @@ -289,7 +405,8 @@ suite('PathObserver Tests', function() { } }; var obj = { foo: 1 }; - var observer = new PathObserver(obj, 'foo', target.changed, target); + var observer = new PathObserver(obj, 'foo'); + observer.open(target.changed, target); obj.foo = 2; observer.deliver(); assert.isTrue(target.called); @@ -300,7 +417,8 @@ suite('PathObserver Tests', function() { test('Delivery Until No Changes', function() { var obj = { foo: { bar: 5 }}; var callbackCount = 0; - var observer = new PathObserver(obj, 'foo . bar', function() { + var observer = new PathObserver(obj, 'foo . bar'); + observer.open(function() { callbackCount++; if (!obj.foo.bar) return; @@ -320,13 +438,15 @@ suite('PathObserver Tests', function() { var arr = {}; arr.foo = 'bar'; - observer = new PathObserver(arr, 'foo', callback); + observer = new PathObserver(arr, 'foo'); + observer.open(callback); arr.foo = 'baz'; assertPathChanges('baz', 'bar'); arr.foo = 'bar'; observer.close(); + arr.foo = 'boo'; assertNoChanges(); }); @@ -335,7 +455,8 @@ suite('PathObserver Tests', function() { var arr = {}; arr.foo = 'bar'; - observer = new PathObserver(arr, 'foo', callback); + observer = new PathObserver(arr, 'foo'); + observer.open(callback); arr.foo = 'baz'; assertPathChanges('baz', 'bar'); @@ -353,7 +474,8 @@ suite('PathObserver Tests', function() { var obj = {}; obj.foo = 'bar'; - observer = new PathObserver(obj, 'foo', callback); + observer = new PathObserver(obj, 'foo'); + observer.open(callback); obj.foo = 'baz'; observer.setValue('bat'); @@ -367,55 +489,36 @@ suite('PathObserver Tests', function() { observer.close(); }); - test('Path setValueFn', function() { - var obj = { foo: 1 }; - function setValueFn(value) { - obj.foo = 2 * value; - } - - observer = new PathObserver(obj, 'foo', callback, undefined, undefined, - setValueFn); - obj.foo = 2; - - observer.setValue(2); - assert.strictEqual(obj.foo, 4); - assertPathChanges(4, 1); - - observer.setValue(8); - observer.reset(); - assertNoChanges(); - - observer.close(); - }); - test('Degenerate Values', function() { var emptyPath = Path.get(); - observer = new PathObserver(null, '', callback); + observer = new PathObserver(null, ''); + observer.open(callback); assert.equal(null, observer.value); observer.close(); var foo = {}; - observer = new PathObserver(foo, '', callback); - assert.equal(foo, observer.value); + observer = new PathObserver(foo, ''); + assert.equal(foo, observer.open(callback)); observer.close(); - observer = new PathObserver(3, '', callback); - assert.equal(3, observer.value); + observer = new PathObserver(3, ''); + assert.equal(3, observer.open(callback)); observer.close(); - observer = new PathObserver(undefined, 'a', callback); - assert.equal(undefined, observer.value); + observer = new PathObserver(undefined, 'a'); + assert.equal(undefined, observer.open(callback)); observer.close(); var bar = { id: 23 }; - observer = new PathObserver(undefined, 'a/3!', callback); - assert.equal(undefined, observer.value); + observer = new PathObserver(undefined, 'a/3!'); + assert.equal(undefined, observer.open(callback)); observer.close(); }); test('Path NaN', function() { var foo = { val: 1 }; - observer = new PathObserver(foo, 'val', callback); + observer = new PathObserver(foo, 'val'); + observer.open(callback); foo.val = 0/0; // Can't use assertSummary because deepEqual() will fail with NaN @@ -433,16 +536,14 @@ suite('PathObserver Tests', function() { path.setValueFrom(obj, 3); assert.equal(3, obj.foo); - observer = new PathObserver(obj, 'foo', callback); - assert.equal(3, observer.value); + observer = new PathObserver(obj, 'foo'); + assert.equal(3, observer.open(callback)); path.setValueFrom(obj, 2); - observer.reset(); - assert.equal(2, observer.value); + assert.equal(2, observer.reset()); path.setValueFrom(obj, 3); - observer.reset(); - assert.equal(3, observer.value); + assert.equal(3, observer.reset()); assertNoChanges(); @@ -452,7 +553,8 @@ suite('PathObserver Tests', function() { test('Path Triple Equals', function() { var model = { }; - observer = new PathObserver(model, 'foo', callback); + observer = new PathObserver(model, 'foo'); + observer.open(callback); model.foo = null; assertPathChanges(null, undefined); @@ -466,7 +568,8 @@ suite('PathObserver Tests', function() { test('Path Simple', function() { var model = { }; - observer = new PathObserver(model, 'foo', callback); + observer = new PathObserver(model, 'foo'); + observer.open(callback); model.foo = 1; assertPathChanges(1, undefined); @@ -484,7 +587,8 @@ suite('PathObserver Tests', function() { var model = { }; var path = Path.get('foo'); - observer = new PathObserver(model, path, callback); + observer = new PathObserver(model, path); + observer.open(callback); model.foo = 1; assertPathChanges(1, undefined); @@ -498,49 +602,11 @@ suite('PathObserver Tests', function() { observer.close(); }); - test('valueFn', function() { - var model = { }; - - function valueFn(value) { - return isNaN(value) ? value : value * 3; - } - - observer = new PathObserver(model, 'foo', callback, undefined, valueFn); - - model.foo = 1; - assertPathChanges(3, undefined); - - model.foo = 2; - assertPathChanges(6, 3); - - delete model.foo; - assertPathChanges(undefined, 6); - - observer.close(); - }); - - test('valueFn - return object literal', function() { - var model = { }; - - function valueFn(value) { - return isNaN(value) ? value : [ value ]; - } - - observer = new PathObserver(model, 'foo', callback, undefined, valueFn); - - model.foo = 1; - assertPathChanges([1], undefined); - - model.foo = 3; - assertPathChanges([3], [1]); - - observer.close(); - }); - test('Path With Indices', function() { var model = []; - observer = new PathObserver(model, '0', callback); + observer = new PathObserver(model, '0'); + observer.open(callback); model.push(1); assertPathChanges(1, undefined); @@ -557,7 +623,8 @@ suite('PathObserver Tests', function() { } }; - observer = new PathObserver(model, 'a.b.c', callback); + observer = new PathObserver(model, 'a.b.c'); + observer.open(callback); model.a.b.c = 'hello, mom'; assertPathChanges('hello, mom', 'hello, world'); @@ -584,7 +651,8 @@ suite('PathObserver Tests', function() { assertNoChanges(); // Resume observing - observer = new PathObserver(model, 'a.b.c', callback); + observer = new PathObserver(model, 'a.b.c'); + observer.open(callback); model.a.b.c = 'hello. Back for reals'; assertPathChanges('hello. Back for reals', @@ -600,7 +668,8 @@ suite('PathObserver Tests', function() { } }); - observer = new PathObserver(model, 'id', callback); + observer = new PathObserver(model, 'id'); + observer.open(callback); model.id = 1; assertNoChanges(); @@ -614,7 +683,8 @@ suite('PathObserver Tests', function() { writable: false, value: 1 }); - observer = new PathObserver(model, 'x', callback); + observer = new PathObserver(model, 'x'); + observer.open(callback); model.x = 2; @@ -629,7 +699,8 @@ suite('PathObserver Tests', function() { } }); - observer = new PathObserver(model, 'x', callback); + observer = new PathObserver(model, 'x'); + observer.open(callback); model.x = 2; assertPathChanges(2, 1); observer.close(); @@ -643,7 +714,8 @@ suite('PathObserver Tests', function() { x: 1 }); - observer = new PathObserver(model, 'x', callback); + observer = new PathObserver(model, 'x'); + observer.open(callback); delete model.x; assertNoChanges(); observer.close(); @@ -657,7 +729,8 @@ suite('PathObserver Tests', function() { x: 2 }); - observer = new PathObserver(model, 'x', callback); + observer = new PathObserver(model, 'x'); + observer.open(callback); delete model.x; assertPathChanges(1, 2); observer.close(); @@ -671,7 +744,8 @@ suite('PathObserver Tests', function() { __proto__: proto }); - observer = new PathObserver(model, 'x', callback); + observer = new PathObserver(model, 'x'); + observer.open(callback); model.x = 2; assertPathChanges(2, 1); @@ -692,7 +766,8 @@ suite('PathObserver Tests', function() { value: 1 }); - observer = new PathObserver(model, 'x', callback); + observer = new PathObserver(model, 'x'); + observer.open(callback); delete model.x; assertNoChanges(); @@ -722,7 +797,8 @@ suite('PathObserver Tests', function() { } }); - observer = new PathObserver(model, 'a.b', callback); + observer = new PathObserver(model, 'a.b'); + observer.open(callback); _b = 3; // won't be observed. assertNoChanges(); @@ -846,7 +922,7 @@ suite('PathObserver Tests', function() { }); -suite('CompoundPathObserver Tests', function() { +suite('CompoundObserver Tests', function() { setup(doSetup); @@ -855,11 +931,11 @@ suite('CompoundPathObserver Tests', function() { test('Simple', function() { var model = { a: 1, b: 2, c: 3 }; - observer = new CompoundPathObserver(callback); + observer = new CompoundObserver(); observer.addPath(model, 'a'); observer.addPath(model, 'b'); observer.addPath(model, Path.get('c')); - observer.start(); + observer.open(callback); var observerCallbackArg = [model, Path.get('a'), model, Path.get('b'), @@ -868,90 +944,29 @@ suite('CompoundPathObserver Tests', function() { model.b = 20; model.c = 30; assertCompoundPathChanges([-10, 20, 30], [1, 2, 3], - [true, true, true], observerCallbackArg); model.a = 'a'; model.c = 'c'; - assertCompoundPathChanges(['a', 20, 'c'], [-10, 20, 30], - [true, false, true], + assertCompoundPathChanges(['a', 20, 'c'], [-10,, 30], observerCallbackArg); observer.close(); }); - test('Simple - valueFn', function() { - var model = { a: 1, b: 2, c: 3 }; - - function valueFn(values) { - return values.reduce(function(last, cur) { - return typeof cur === 'number' ? last + cur : undefined; - }, 0); - } - - observer = new CompoundPathObserver(callback, undefined, valueFn); - observer.addPath(model, 'a'); - observer.addPath(model, 'b'); - observer.addPath(model, Path.get('c')); - observer.start(); - - assert.strictEqual(6, observer.value); - - model.a = -10; - model.b = 20; - model.c = 30; - assertPathChanges(40, 6); - - observer.close(); - }); - - test('setValueFn', function() { - var obj = { foo: 1, bar: 2 }; - - function valueFn(values) { - return values[0] + values[1]; - } - - function setValueFn(value) { - obj.foo = value; - } - - observer = new CompoundPathObserver(callback, undefined, valueFn, - setValueFn); - observer.addPath(obj, 'foo'); - observer.addPath(obj, 'bar'); - observer.start(); - - observer.setValue(2); - assert.strictEqual(obj.foo, 2); - assertPathChanges(4, 3); - - observer.setValue(8); - observer.reset(); - assertNoChanges(); - - observer.close(); - }); - - test('Degenerate Values', function() { var model = {}; - - function valueFn(values) { - assert.strictEqual(4, values.length); - assert.strictEqual(undefined, values[0]); - assert.strictEqual('obj-value', values[1]); - assert.strictEqual(undefined, values[2]); - assert.strictEqual(undefined, values[3]); - } - - observer = new CompoundPathObserver(callback, undefined, undefined, - valueFn); + observer = new CompoundObserver(); observer.addPath({}, '.'); // invalid path observer.addPath('obj-value', ''); // empty path observer.addPath({}, 'foo'); // unreachable observer.addPath(3, 'bar'); // non-object with non-empty path - observer.start(); + var values = observer.open(callback); + assert.strictEqual(4, values.length); + assert.strictEqual(undefined, values[0]); + assert.strictEqual('obj-value', values[1]); + assert.strictEqual(undefined, values[2]); + assert.strictEqual(undefined, values[3]); observer.close(); }); @@ -962,10 +977,10 @@ suite('CompoundPathObserver Tests', function() { return {}; } - observer = new CompoundPathObserver(callback, undefined, valueFn); + observer = new CompoundObserver(valueFn); observer.addPath(model, 'a'); - observer.start(); + observer.open(callback); model.a = 2; observer.deliver(); @@ -973,6 +988,53 @@ suite('CompoundPathObserver Tests', function() { window.dirtyCheckCycleCount === 1); observer.close(); }); + + test('Heterogeneous', function() { + var model = { a: 1, b: 2 }; + var otherModel = { c: 3 }; + + function valueFn(value) { return value * 2; } + function setValueFn(value) { return value / 2; } + + var compound = new CompoundObserver; + assert.throws(function () { + compound.addObserver({ open: function() {} }); + }); + + assert.throws(function () { + compound.addObserver({ close: function() {} }); + }); + + assert.throws(function () { + compound.addObserver(1); + }); + + compound.addPath(model, 'a'); + compound.addObserver(new ObserverTransform(new PathObserver(model, 'b'), + valueFn, setValueFn)); + compound.addObserver(new PathObserver(otherModel, 'c')); + + function combine(values) { + return values[0] + values[1] + values[2]; + }; + observer = new ObserverTransform(compound, combine); + assert.strictEqual(8, observer.open(callback)); + + model.a = 2; + model.b = 4; + assertPathChanges(13, 8); + + model.b = 10; + otherModel.c = 5; + assertPathChanges(27, 13); + + model.a = 20; + model.b = 1; + otherModel.c = 5; + assertNoChanges(); + + observer.close(); + }) }); suite('ArrayObserver Tests', function() { @@ -1040,7 +1102,8 @@ suite('ArrayObserver Tests', function() { function arrayMutationTest(arr, operations) { var copy = arr.slice(); - observer = new ArrayObserver(arr, callback); + observer = new ArrayObserver(arr); + observer.open(callback); operations.forEach(function(op) { switch(op.name) { case 'delete': @@ -1061,15 +1124,6 @@ suite('ArrayObserver Tests', function() { observer.close(); } - test('Close Invokes Close', function() { - var called = false; - var obj = []; - obj.close = function() { called = true }; - var observer = new ArrayObserver(obj, function() {}); - observer.close(); - assert.isTrue(called); - }); - test('Optional target for callback', function() { var target = { changed: function(splices) { @@ -1077,7 +1131,8 @@ suite('ArrayObserver Tests', function() { } }; var obj = []; - var observer = new ArrayObserver(obj, target.changed, target); + var observer = new ArrayObserver(obj); + observer.open(target.changed, target); obj.length = 1; observer.deliver(); assert.isTrue(target.called); @@ -1087,7 +1142,8 @@ suite('ArrayObserver Tests', function() { test('Delivery Until No Changes', function() { var arr = [0, 1, 2, 3, 4]; var callbackCount = 0; - var observer = new ArrayObserver(arr, function() { + var observer = new ArrayObserver(arr); + observer.open(function() { callbackCount++; arr.shift(); }); @@ -1103,7 +1159,8 @@ suite('ArrayObserver Tests', function() { test('Array disconnect', function() { var arr = [ 0 ]; - observer = new ArrayObserver(arr, callback); + observer = new ArrayObserver(arr); + observer.open(callback); arr[0] = 1; @@ -1122,7 +1179,8 @@ suite('ArrayObserver Tests', function() { var arr = []; arr.push(1); - observer = new ArrayObserver(arr, callback); + observer = new ArrayObserver(arr); + observer.open(callback); arr.push(2); assertArrayChanges([{ @@ -1147,7 +1205,8 @@ suite('ArrayObserver Tests', function() { test('Array', function() { var model = [0, 1]; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model[0] = 2; @@ -1169,24 +1228,27 @@ suite('ArrayObserver Tests', function() { test('Array observe non-array throws', function() { assert.throws(function () { - observer = new ArrayObserver({}, callback); + observer = new ArrayObserver({}); }); }); test('Array Set Same', function() { var model = [1]; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model[0] = 1; - + observer.deliver(); + assert.isFalse(callbackInvoked); observer.close(); }); test('Array Splice', function() { var model = [0, 1] - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.splice(1, 1, 2, 3); // [0, 2, 3] assertArrayChanges([{ @@ -1238,7 +1300,8 @@ suite('ArrayObserver Tests', function() { test('Array Splice Truncate And Expand With Length', function() { var model = ['a', 'b', 'c', 'd', 'e']; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.length = 2; @@ -1262,7 +1325,8 @@ suite('ArrayObserver Tests', function() { test('Array Splice Delete Too Many', function() { var model = ['a', 'b', 'c']; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.splice(2, 3); // ['a', 'b'] assertArrayChanges([{ @@ -1277,7 +1341,8 @@ suite('ArrayObserver Tests', function() { test('Array Length', function() { var model = [0, 1]; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.length = 5; // [0, 1, , , ,]; assertArrayChanges([{ @@ -1302,7 +1367,8 @@ suite('ArrayObserver Tests', function() { test('Array Push', function() { var model = [0, 1]; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.push(2, 3); // [0, 1, 2, 3] assertArrayChanges([{ @@ -1320,7 +1386,8 @@ suite('ArrayObserver Tests', function() { test('Array Pop', function() { var model = [0, 1]; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.pop(); // [0] assertArrayChanges([{ @@ -1345,7 +1412,8 @@ suite('ArrayObserver Tests', function() { test('Array Shift', function() { var model = [0, 1]; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.shift(); // [1] assertArrayChanges([{ @@ -1370,7 +1438,8 @@ suite('ArrayObserver Tests', function() { test('Array Unshift', function() { var model = [0, 1]; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.unshift(-1); // [-1, 0, 1] assertArrayChanges([{ @@ -1527,7 +1596,8 @@ suite('ArrayObserver Tests', function() { var model = ['a','b']; var copy = model.slice(); - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.splice(0, 1, 'c', 'd', 'e'); model.splice(4,0,'f'); @@ -1540,7 +1610,8 @@ suite('ArrayObserver Tests', function() { var model = [3,4]; var copy = model.slice(); - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.splice(2,0,8); model.splice(0,1,0,5); @@ -1553,7 +1624,8 @@ suite('ArrayObserver Tests', function() { var model = [1,3,6]; var copy = model.slice(); - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.splice(1,1); model.splice(0,2,1,7); @@ -1580,21 +1652,24 @@ suite('ArrayObserver Tests', function() { test('Array Tracker No Proxies Edits', function() { model = []; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.length = 0; model.push(1, 2, 3); assertEditDistance(model, 3); observer.close(); model = ['x', 'x', 'x', 'x', '1', '2', '3']; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.length = 0; model.push('1', '2', '3', 'y', 'y', 'y', 'y'); assertEditDistance(model, 8); observer.close(); model = ['1', '2', '3', '4', '5']; - observer = new ArrayObserver(model, callback); + observer = new ArrayObserver(model); + observer.open(callback); model.length = 0; model.push('a', '2', 'y', 'y', '4', '5', 'z', 'z'); assertEditDistance(model, 7); @@ -1637,15 +1712,6 @@ suite('ObjectObserver Tests', function() { callbackInvoked = false; } - test('Close Invokes Close', function() { - var called = false; - var obj = {}; - obj.close = function() { called = true }; - var observer = new ObjectObserver(obj, function() {}); - observer.close(); - assert.isTrue(called); - }); - test('Optional target for callback', function() { var target = { changed: function(value, oldValue) { @@ -1653,7 +1719,8 @@ suite('ObjectObserver Tests', function() { } }; var obj = { foo: 1 }; - var observer = new PathObserver(obj, 'foo', target.changed, target); + var observer = new PathObserver(obj, 'foo'); + observer.open(target.changed, target); obj.foo = 2; observer.deliver(); assert.isTrue(target.called); @@ -1661,25 +1728,11 @@ suite('ObjectObserver Tests', function() { observer.close(); }); - test('Optional target for callback', function() { - var target = { - changed: function(added, removed, changed, oldValues) { - this.called = true; - } - }; - var obj = {}; - var observer = new ObjectObserver(obj, target.changed, target); - obj.foo = 1; - observer.deliver(); - assert.isTrue(target.called); - - observer.close(); - }); - test('Delivery Until No Changes', function() { var obj = { foo: 5 }; var callbackCount = 0; - var observer = new ObjectObserver(obj, function() { + var observer = new ObjectObserver(obj); + observer.open(function() { callbackCount++; if (!obj.foo) return; @@ -1699,7 +1752,8 @@ suite('ObjectObserver Tests', function() { var obj = {}; obj.foo = 'bar'; - observer = new ObjectObserver(obj, callback); + observer = new ObjectObserver(obj); + observer.open(callback); obj.foo = 'baz'; obj.bat = 'bag'; @@ -1734,7 +1788,8 @@ suite('ObjectObserver Tests', function() { var obj = {}; obj.foo = 'bar'; - observer = new ObjectObserver(obj, callback); + observer = new ObjectObserver(obj); + observer.open(callback); obj.foo = 'baz'; assertObjectChanges({ @@ -1769,7 +1824,8 @@ suite('ObjectObserver Tests', function() { test('Object observe array', function() { var arr = []; - observer = new ObjectObserver(arr, callback); + observer = new ObjectObserver(arr); + observer.open(callback); arr.length = 5; arr.foo = 'bar'; @@ -1797,7 +1853,8 @@ suite('ObjectObserver Tests', function() { test('Object', function() { var model = {}; - observer = new ObjectObserver(model, callback); + observer = new ObjectObserver(model); + observer.open(callback); model.id = 0; assertObjectChanges({ added: { @@ -1828,7 +1885,8 @@ suite('ObjectObserver Tests', function() { assertNoChanges(); // Re-observe -- should see an new event again. - observer = new ObjectObserver(model, callback); + observer = new ObjectObserver(model); + observer.open(callback); model.id2 = 202;; assertObjectChanges({ added: { @@ -1847,7 +1905,8 @@ suite('ObjectObserver Tests', function() { test('Object Delete Add Delete', function() { var model = { id: 1 }; - observer = new ObjectObserver(model, callback); + observer = new ObjectObserver(model); + observer.open(callback); // If mutation occurs in seperate "runs", two events fire. delete model.id; @@ -1885,7 +1944,8 @@ suite('ObjectObserver Tests', function() { test('Object Set Undefined', function() { var model = {}; - observer = new ObjectObserver(model, callback); + observer = new ObjectObserver(model); + observer.open(callback); model.x = undefined; assertObjectChanges({ diff --git a/util/array_reduction.js b/util/array_reduction.js index e36bc24..8c3958f 100644 --- a/util/array_reduction.js +++ b/util/array_reduction.js @@ -17,7 +17,8 @@ function ArrayReduction(array, path, reduceFn, initial) { this.path = path ? path.trim() : undefined; this.reduceFn = reduceFn; this.initial = initial; - this.arrayObserver = new ArrayObserver(array, this.handleSplices, this); + this.arrayObserver = new ArrayObserver(array); + this.arrayObserver.open(this.handleSplices, this); this.observers = this.path ? [] : undefined; this.handleSplices([{ @@ -33,8 +34,10 @@ ArrayReduction.prototype = { var splice = splices[i]; var added = []; for (var j = 0; j < splice.addedCount; j++) { - added.push(new PathObserver(this.array[splice.index + j], this.path, - this.reduce, this)); + var observer = new PathObserver(this.array[splice.index + j], this.path); + observer.open(this.reduce, this); + added.push(observer); + } var spliceArgs = [splice.index, splice.removed.length].concat(added); @@ -106,4 +109,4 @@ ArrayReduction.defineProperty = function(object, name, descriptor) { }) return observer; -} \ No newline at end of file +}