Skip to content

Commit

Permalink
Fix wrong change is sent to observer when top level object is changed.
Browse files Browse the repository at this point in the history
When you change the top level object, observer MUST receive value
for the key path of the previous top level object as old and
value for the key path of the new top level object as new.
  • Loading branch information
Kentzo committed Jul 5, 2012
1 parent 8afb675 commit 4094a1f
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 7 deletions.
9 changes: 8 additions & 1 deletion Foundation/CPKeyValueObserving.j
Expand Up @@ -1114,7 +1114,14 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti
{
if (aKeyPath === _firstPart)
{
[_observer observeValueForKeyPath:_firstPart ofObject:_object change:changes context:_context];
var oldValue = [_value valueForKeyPath:_secondPart],
newValue = [_object valueForKeyPath:_firstPart+"."+_secondPart],
pathChanges = [CPDictionary dictionaryWithObjectsAndKeys:
newValue ? newValue : [CPNull null], CPKeyValueChangeNewKey,
oldValue ? oldValue : [CPNull null], CPKeyValueChangeOldKey,
CPKeyValueChangeSetting, CPKeyValueChangeKindKey];

[_observer observeValueForKeyPath:_firstPart+"."+_secondPart ofObject:_object change:pathChanges context:_context];

//since a has changed, we should remove ourselves as an observer of the old a, and observe the new one
if (_value)
Expand Down
42 changes: 36 additions & 6 deletions Tests/Foundation/CPKVOTest.j
Expand Up @@ -290,7 +290,10 @@

[a addObserver:self forKeyPath:"b.c.d.e.f" options:0 context:"testCrazyKeyPathChanges"];

[a setValue:[D new] forKeyPath:"b.c.d"];
var newD = [D new];
[newD setValue:[E new] forKeyPath:@"e"];
[newD setValue:[F new] forKeyPath:@"e.f"];
[a setValue:newD forKeyPath:"b.c.d"];

[self assertTrue: _sawObservation message:"Never recieved an observation"];
}
Expand Down Expand Up @@ -382,6 +385,22 @@
[self assertTrue: _sawObservation message:"Never recieved an observation"];
}

- (void)testChangeTopLevelObject
{
var tester = [IndirectToManyTester new];

tester.tester = [ToManyTester new];

[tester.tester setValue:[1, 2, 3, 4] forKey:@"subviews"];
[tester addObserver:self forKeyPath:@"tester.subviews" options:0 context:"testChangeTopLevelObject"];

var newTesterTester = [ToManyTester new];
[newTesterTester setValue:[5, 6, 7, 8] forKey:@"subviews"];
[tester setValue:newTesterTester forKey:@"tester"];

[self assertTrue: _sawObservation message:"Never recieved an observation"];
}

- (void)testPerformance
{
bob = [PersonTester new];
Expand Down Expand Up @@ -537,21 +556,21 @@
break;

case "testThreePartKeyPart2":
[self assertTrue: aKeyPath == "teacher.car" message:"Keypath should be: teacher.car, was: "+aKeyPath];
[self assertTrue: newValue.year == "2000" message:"New value should be a car with year: 2000, was: "+[newValue description]];
[self assertTrue: aKeyPath == "teacher.car.year" message:"Keypath should be: teacher.car, was: "+aKeyPath];
[self assertTrue: newValue == "2000" message:"New value should be a car with year: 2000, was: "+[newValue description]];
[self assertTrue: anObject == cs101 message: "anObject should be: "+[cs101 description]+", was: "+[anObject description]];
break;

case "testCrazyKeyPathChanges":
[self assertTrue: [anObject class] == A message:"Should be observing an A class, was: "+[anObject class]];
[self assertTrue: [newValue class] == D message:"Changed class was a D class, got: "+[newValue class]];
[self assertTrue: aKeyPath == "b.c.d" message:"Expected keyPath b.c.d, got: "+aKeyPath];
[self assertTrue: [newValue class] == F message:"Changed class was a F class, got: "+[newValue class]];
[self assertTrue: aKeyPath == "b.c.d.e.f" message:"Expected keyPath b.c.d.e.f, got: "+aKeyPath];
break;

case "testCrazyKeyPathChanges2":
[self assertTrue: [anObject class] == A message:"Should be observing an A class, was: "+[anObject class]];
[self assertTrue: newValue == [CPNull null] message:"Expected null, got: "+newValue];
[self assertTrue: aKeyPath == "b.c" message:"Expected keyPath b.c, got: "+aKeyPath];
[self assertTrue: aKeyPath == "b.c.d.e.f" message:"Expected keyPath b.c.d.e.f, got: "+aKeyPath];
break;

case "testCrazyKeyPathChanges3":
Expand Down Expand Up @@ -616,6 +635,17 @@

break;

case "testChangeTopLevelObject":
var type = [changes objectForKey:CPKeyValueChangeKindKey];
[self assertTrue: type == CPKeyValueChangeSetting message: "Should have been a set, was: "+type];

var oldValue = [changes objectForKey:CPKeyValueChangeOldKey],
newValue = [changes objectForKey:CPKeyValueChangeNewKey];
[self assert:oldValue equals:[1, 2, 3, 4]];
[self assert:newValue equals:[5, 6, 7, 8]];

break;

case "testRemoveFromToManyProperty":
var type = [changes objectForKey:CPKeyValueChangeKindKey];
[self assertTrue: type == CPKeyValueChangeRemoval message: "Should have been a removal, was: "+type];
Expand Down

0 comments on commit 4094a1f

Please sign in to comment.