Permalink
Browse files

mixins/composite: fixed flakiness with composite property caches

  • Loading branch information...
Erick Johnson
Erick Johnson committed Dec 16, 2011
1 parent 82010e4 commit 6a557794315d97ff782a30e19bcbc5377e55297c
View
@@ -217,21 +217,20 @@ DataStructures.Composite = {
doCompositeOperation: function(op,args) {
if (!this.get('isCompositePiece')) return null;
- var composites = this.get('compositeList'),
+ var composites = [this].concat(this.get('compositeChildren')),
ret = [];
args = SC.A(args);
- composites.forEach(function(c) {
- if (!arguments[0]) return;
- try {
- c._cmpst_isCollecting = YES;
- var v = c[op].apply(c,args);
- if (SC.none(v)) return;
- ret = ret.concat(v);
- } finally {
- c._cmpst_isCollecting = NO;
- }
- });
+ for (var i=0;i<composites.length;i++) {
+ var c = composites[i];
+ if (!c) continue;
+
+ c._cmpst_isCollecting = YES;
+
+ var v = c[op].apply(c,args);
+ if (!SC.none(v)) ret = ret.concat(v);
+ c._cmpst_isCollecting = NO;
+ };
return ret;
}.dsProfile('doCompositeOperation'),
@@ -607,9 +606,6 @@ DataStructures.Composite = {
*/
_cmpst_pendingChildPropertyChanges: null,
_cmpst_notifyOfChildProvidedCompositePropertiesChange: function(child,keys) {
- if (this.DEBUG_COMPOSITE)
- SC.Logger.log('Composite._cmpst_notifyOfChildProvidedCompositePropertiesChange: %@ (%@) - notified of child %@ (%@) property changes'.fmt(this.toString(), SC.hashFor(this), child.toString(),SC.hashFor(this)));
-
if (!this._cmpst_pendingChildPropertyChanges) {
this._cmpst_pendingChildPropertyChanges = [];
}
@@ -621,13 +617,19 @@ DataStructures.Composite = {
if (scheduled) return;
+ if (this.DEBUG_COMPOSITE)
+ SC.Logger.log('Composite._cmpst_notifyOfChildProvidedCompositePropertiesChange: %@ scheduled for keys %@'.fmt(this.toString(), keys));
+
var task = SC.Task.create({ run: function() {
if (!this._cmpst_pendingChildPropertyChanges) return;
var myCompProps = this.get('compositeProperties');
var changed = false;
var keys = this._cmpst_pendingChildPropertyChanges.fastFlatten().uniq();
+ if (this.DEBUG_COMPOSITE)
+ SC.Logger.log('Composite._cmpst_notifyOfChildProvidedCompositePropertiesChange.task: %@ running for keys %@'.fmt(this.toString(), keys));
+
this._cmpst_pendingChildPropertyChanges = null;
keys.forEach(function childCompPropsIterator(p) {
@@ -641,6 +643,17 @@ DataStructures.Composite = {
// notifications through the call to
// notifyPropertyChange('compositeProperties') below
this.notifyPropertyChange(p);
+
+ // TODO: track this hack down more - this hack fixes the
+ // cached property key from being incorrect. here's what I
+ // know, after an object calls notifyPropertyChange(p)
+ // above, this._kvo_cache is cleared for the current dynamic
+ // property. Somewhere between here and the end of the
+ // runLoop this._kvo_cache is repopulated with the old
+ // value. Getting the value here seems to cache the correct
+ // value. Commenting out this line will make two of the
+ // assertions in the composite/kvo_and_properties test fail
+// this.get(p);
}
}, this);
@@ -763,6 +776,14 @@ DataStructures.Composite = {
*
* There be dragons here
*/
+ set: function(key,value) {
+ var func = this[key];
+ if (func && func.isProperty && func.isCacheable && this._kvo_cache) {
+ this._kvo_cache[func.cacheKey] = undefined ;
+ }
+ return sc_super();
+ },
+
_cmpst_dynamicPropertyScheduledNotify: null,
_cmpst_propertyCache: null,
_cmpst_addDynamicProperty: function(fullPath) {
@@ -794,8 +815,23 @@ DataStructures.Composite = {
}
}
- var ret = null, cached = this._cmpst_accessPropertyCache(key);
+ var cached = this._cmpst_accessPropertyCache(key),
+ ret = cached;
+
+ if (!this.get('compositeIsLeaf')) {
+ ret = SC.A(cached);
+
+ var children = this.compositeSortChildren(); //this.get('compositeChildren');
+ for (var i=0;i<children.length;i++) {
+ if (!children[i]) continue;
+ ret = ret.concat(children[i].get(key)).compact().without(undefined);
+ }
+ if (this.compositeUniqueValues && ret.uniq)
+ ret = ret.uniq();
+ }
+
+/*
if (!this._cmpst_isCollecting) {
ret = this.doCompositeOperation('get',[key]);
@@ -810,6 +846,7 @@ DataStructures.Composite = {
} else {
ret = cached;
}
+*/
return ret;
}.dsProfile('_cmpst_dynamicProperty.%@'.fmt(prop)).property().cacheable(); //.dynamicCompositeProperty();
@@ -31,20 +31,14 @@ module("DataStructures.Composite DAG Managment", {
var component = {
horsePower: null,
getHorsePower: function() {
- var hp = this.doCompositeOperation('get', 'horsePower');
-
- return hp.reduce(function(prev,item) {
- return prev + item;
- },0);
+ var hp = this.get('horsePower');
+ return hp.reduce ? hp.reduce(function(prev,item) { return prev + item; },0) : hp;
},
weight: null,
getWeight: function() {
- var componentWeights = this.doCompositeOperation('get', 'weight');
-
- return componentWeights.reduce(function(prev,item) {
- return prev + item;
- },0);
+ var w = this.get('weight');
+ return w.reduce ? w.reduce(function(prev,item) { return prev + item; },0) : w;
}
};
@@ -190,31 +184,35 @@ test("composite supplant", function() {
var camaro, camaro2011, v8Engine, superCharger;
camaro = Car.create({
+ compositeProperties: ['weight','horsePower'],
weight: 2000,
horsePower: 250
});
SC.run(function() {
v8Engine = Part.create({
+ compositeProperties: ['weight','horsePower'],
compositeParents: camaro,
weight: 300,
horsePower: 100
});
superCharger = Part.create({
+ compositeProperties: ['weight','horsePower'],
compositeParents: v8Engine,
weight: 10,
horsePower: 50
});
camaro2011 = Car.create({
+ compositeProperties: ['weight','horsePower'],
weight: 1000,
horsePower: 300
});
});
- equals(camaro.getHorsePower(), 400, 'camaro should have children as normal');
- equals(camaro2011.getHorsePower(), 300, 'camaro2011 should have no children');
+ equals(camaro.getHorsePower(), 400, 'prereq - camaro should have children as normal');
+ equals(camaro2011.getHorsePower(), 300, 'prereq - camaro2011 should have no children');
SC.run(function() {
camaro2011.compositeSupplant(camaro);
@@ -232,26 +230,29 @@ test("composite supplant", function() {
*/
test("a composite can add/remove children and perform operations", function () {
var camaro = Car.create({
+ compositeProperties: ['weight','horsePower'],
weight: 2000,
horsePower: 250
});
equals(camaro.getWeight(), 2000, 'calculating stock camaro weight');
var v8Engine = Part.create({
+ compositeProperties: ['weight','horsePower'],
weight: 300,
horsePower: 100
});
var superCharger = Part.create({
+ compositeProperties: ['weight','horsePower'],
weight: 10,
horsePower: 50
});
- SC.run(function() {
- v8Engine.addCompositeChild(superCharger);
- camaro.addCompositeChild(v8Engine);
- });
+ SC.RunLoop.begin();
+ v8Engine.addCompositeChild(superCharger);
+ camaro.addCompositeChild(v8Engine);
+ SC.RunLoop.end();
equals(v8Engine.getWeight(), 310, 'calculating engine weight');
equals(camaro.getWeight(), 2310, 'calculating modified camaro weight');
@@ -268,6 +269,7 @@ test("a composite can add/remove children and perform operations", function () {
equals(camaro.getWeight(), 2300, 'calculating un-supercharged camaro weight');
var racingSeats = Part.create({
+ compositeProperties: ['weight','horsePower'],
weight: 50,
horsePower: null
});
@@ -281,6 +283,7 @@ test("a composite can add/remove children and perform operations", function () {
// composing bottom up also works
var mustang = Car.create({
+ compositeProperties: ['weight','horsePower'],
weight: 2200,
horsePower: 280
});
@@ -292,6 +295,7 @@ test("a composite can add/remove children and perform operations", function () {
equals(mustang.getWeight(), 2500, 'can also add composite pieces bottom up');
var restrictorPlate = Part.create({
+ compositeProperties: ['weight','horsePower'],
weight: 30,
horsePower: -50
});
@@ -306,16 +310,19 @@ test("a composite can add/remove children and perform operations", function () {
test("composite should throw error when adding a destroyed child/parent", function() {
var camaro = Car.create({
+ compositeProperties: ['weight','horsePower'],
weight: 2000,
horsePower: 250
});
var v8Engine = Part.create({
+ compositeProperties: ['weight','horsePower'],
weight: 300,
horsePower: 100
});
var superCharger = Part.create({
+ compositeProperties: ['weight','horsePower'],
weight: 10,
horsePower: 50
});
@@ -401,6 +408,7 @@ test("a composite member can have multiple parents... watch out for paradoxes",
equals(characters.length, 2, 'the year %@ should have 2 people'.fmt(y.get('year')));
});
+ SC.RunLoop.begin();
// also test that the composites can be different
_1955.set('characters', ['Martys Teen Mom',
'BTTF 1: Marty McFly',
@@ -409,10 +417,30 @@ test("a composite member can have multiple parents... watch out for paradoxes",
'Old Biff from the Future']);
_1885.set('characters', ['Clara Clayton', 'Maddog Tannen']);
+ SC.RunLoop.end();
- equals(_1985.get('characters').length, 2, '1985 has 2 characters');
- equals(_1955.get('characters').length, 7, '1985 has 7 characters');
- equals(_1885.get('characters').length, 4, '1885 has 4 characters');
+ var _1985Characters = _1985.get('characters');
+ var _1955Characters = _1955.get('characters');
+ var _1885Characters = _1885.get('characters');
+
+ equals(_1985Characters.length, 2, '1985 has 2 characters');
+ ok(_1985Characters.contains('Marty McFly'),'Marty McFly should be in 1985');
+ ok(_1985Characters.contains('Doc Brown'),'Doc Brown should be in 1985');
+
+ equals(_1955Characters.length, 7, '1955 has 7 characters');
+ ok(_1955Characters.contains('Marty McFly'),'Marty McFly should be in 1955');
+ ok(_1955Characters.contains('Doc Brown'),'Doc Brown should be in 1955');
+ ok(_1955Characters.contains('Martys Teen Mom'),'Martys Teen Mom should be in 1955');
+ ok(_1955Characters.contains('BTTF 1: Marty McFly'),'BTTF 1: Marty McFly should be in 1955');
+ ok(_1955Characters.contains('BTTF 1: Doc Brown'),'BTTF 1: Doc Brown should be in 1955');
+ ok(_1955Characters.contains('Young Biff'),'Young Biff should be in 1955');
+ ok(_1955Characters.contains('Old Biff from the Future'),'Old Biff from the Future should be in 1955');
+
+ equals(_1885Characters.length, 4, '1885 has 4 characters');
+ ok(_1885Characters.contains('Clara Clayton'), 'Clara Clayton should be in 1885');
+ ok(_1885Characters.contains('Maddog Tannen'), 'Maddog Tannen should be in 1885');
+ ok(_1885Characters.contains('Doc Brown'), 'Doc Brown should be in 1885');
+ ok(_1885Characters.contains('Marty McFly'), 'Marty McFly should be in 1885');
SC.RunLoop.begin();
var timeTrain = SC.Object.create(DataStructures.Composite, {
@@ -423,10 +451,12 @@ test("a composite member can have multiple parents... watch out for paradoxes",
SC.RunLoop.end();
equals(_1985.get('characters').length, 4, 'after train enters 1985 and 1885, 1985 has 4 characters');
- equals(_1955.get('characters').length, 7, 'after train enters 1985 and 1885, 1985 has 7 characters');
+ equals(_1955.get('characters').length, 7, 'after train enters 1985 and 1885, 1955 has 7 characters');
equals(_1885.get('characters').length, 6, 'after train enters 1985 and 1885, 1885 has 6 characters');
+ SC.RunLoop.begin();
_1885.removeCompositeChild(delorian);
+ SC.RunLoop.end();
equals(_1985.get('characters').length, 4, 'after delorian leave 1885, 1985 has 4 characters');
equals(_1955.get('characters').length, 7, 'after delorian leave 1885, 1955 has 7 characters');
@@ -139,10 +139,10 @@ test("compositeProperties should be observable and propogate changes up the comp
name: 'forest',
didChangeCount: 0,
allTheLeaves: function() {
- return this.doCompositeOperation('get','leaves')
+ return SC.A(this.get('leaves'))
.reduce(function(prev,cur) {
return prev + cur;
- });
+ },0);
},
_leavesDidChange: function() {
var val = this.allTheLeaves();
@@ -182,7 +182,7 @@ test("compositeProperties should be observable and propogate changes up the comp
ok(tree1.compositeProperties.indexOf('branches') > -1, "branches should be in tree1's compositeProperties array");
var branches = forest.get('branches'),
- branchCount = branches.reduce(function(p,c) {
+ branchCount = SC.A(branches).reduce(function(p,c) {
return p+c;
});
equals(branchCount, 2, '2 branches should have been auto added to the compositeProperty list (one from each tree)');
@@ -194,7 +194,6 @@ test("compositeProperties should be observable and propogate changes up the comp
SC.RunLoop.begin();
SC.Logger.log('begin setting leaves values on tree1 and tree2');
-//debugger;
tree1.set('leaves', 150);
tree2.set('leaves', 50);
SC.Logger.log('end setting leaves values on tree1 and tree2');
@@ -234,8 +233,8 @@ test("compositeProperties should be observable and propogate changes up the comp
});
SC.RunLoop.end();
- equals(tree1.get('leaves').reduce(function(p,c){return p+c;}), 1150);
- equals(tree2.get('leaves').reduce(function(p,c){return p+c;}), 650);
+ equals(SC.A(tree1.get('leaves')).reduce(function(p,c){return p+c;}), 1150);
+ equals(SC.A(tree2.get('leaves')).reduce(function(p,c){return p+c;}), 650);
equals(forest._unboundValue, 1800, 'branch additions should percolate up');
SC.run(function() {
@@ -299,7 +298,7 @@ test("composite should notify of property change when a composite child is remov
equals(contact.propUpdateCounts.fuzzy,2,'there should be 2 update for fuzzy after delete');
});
-test("composite shoudl propogate new computed properties even in the face of caching", function() {
+test("composite should propogate new computed properties through caches", function() {
var Comp = SC.Object.extend(DataStructures.Composite);
var contact = Comp.create();
@@ -407,6 +406,7 @@ test('orig values remain as primitive or array', function() {
SC.RunLoop.end();
ok(aComposite.get('aPrimitive') === 1, 'aPrimitive should equal 1 - while aComposite is a leaf');
+ equals(SC.typeOf(aComposite.get('anArray')),SC.T_ARRAY,'anArray should be an array');
ok(aComposite.get('anArray').indexOf(1) >= 0, 'anArray should contain "1"');
SC.RunLoop.begin();
@@ -418,10 +418,12 @@ test('orig values remain as primitive or array', function() {
});
SC.RunLoop.end();
+ SC.RunLoop.begin();
aComposite.set('aPrimitive',2);
aComposite.set('anArray',['a','b','c']);
+ SC.RunLoop.end();
- ok(aComposite.get('aPrimitive').indexOf(2) >= 0, 'aPrimitive should be an array that contains 2 once aComposite hasChildren');
+ ok(aComposite.get('aPrimitive').indexOf(2) === 0, 'aPrimitive should be an array that contains 2 once aComposite hasChildren');
ok(aComposite.get('anArray').indexOf('a') >= 0, 'anArray should contain "a"');
ok(aComposite.get('anArray').indexOf('d') >= 0, 'anArray should contain "a"');
});
Oops, something went wrong.

0 comments on commit 6a55779

Please sign in to comment.