From df3264de942cf45752ebef0e26507634aa09e392 Mon Sep 17 00:00:00 2001
From: Ross Boucher <ross@280north.com>
Date: Wed, 24 Sep 2008 13:36:25 -0700
Subject: [PATCH] Updates to KVO and KVO tests.

---
 Foundation/CPKeyValueObserving.j | 165 +++++++++++++++++++++++--------
 Tests/Foundation/CPKVOTest.j     | 143 ++++++++++++++++++++++++---
 2 files changed, 253 insertions(+), 55 deletions(-)

diff --git a/Foundation/CPKeyValueObserving.j b/Foundation/CPKeyValueObserving.j
index 6b16584bc..8870225cd 100644
--- a/Foundation/CPKeyValueObserving.j
+++ b/Foundation/CPKeyValueObserving.j
@@ -31,7 +31,6 @@ import "CPObject.j"
 
 }
 
-
 - (void)didChangeValueForKey:(CPString)aKey
 {
 
@@ -60,15 +59,31 @@ import "CPObject.j"
 
 @end
 
+// KVO Options
+CPKeyValueObservingOptionNew        = 1 << 0;
+CPKeyValueObservingOptionOld        = 1 << 1;
+CPKeyValueObservingOptionInitial    = 1 << 2;
+CPKeyValueObservingOptionPrior      = 1 << 3;
+
+// KVO Change Dictionary Keys
+CPKeyValueChangeKindKey                 = @"CPKeyValueChangeKindKey";
+CPKeyValueChangeNewKey                  = @"CPKeyValueChangeNewKey";
+CPKeyValueChangeOldKey                  = @"CPKeyValueChangeOldKey";
+CPKeyValueChangeIndexesKey              = @"CPKeyValueChangeIndexesKey";
+CPKeyValueChangeNotificationIsPriorKey  = @"CPKeyValueChangeNotificationIsPriorKey";
+
+// Map of real objects to their KVO proxy
 var KVOProxyMap = [CPDictionary dictionary];
 
 //rule of thumb: _ methods are called on the real proxy object, others are called on the "fake" proxy object (aka the real object)
 
+/* @ignore */
 @implementation _CPKVOProxy : CPObject
 {
     id              _targetObject;
     Class           _nativeClass;
-    CPDictionary    _observerInfos;
+    CPDictionary    _changesForKey;
+    CPDictionary    _observersForKey;
     CPDictionary    _replacementMethods;
 }
 
@@ -96,8 +111,9 @@ var KVOProxyMap = [CPDictionary dictionary];
     
     _targetObject       = aTarget;
     _nativeClass        = [aTarget class];
-    _observerInfos      = [CPDictionary dictionary];
     _replacementMethods = [CPDictionary dictionary];
+    _observersForKey    = [CPDictionary dictionary];
+    _changesForKey      = [CPDictionary dictionary];
     
     return self;
 }
@@ -113,7 +129,7 @@ var KVOProxyMap = [CPDictionary dictionary];
             
         for (var i=0; i<count; i++)
         {
-            var newMethod = _kvoMethodForMethod(methodList[i]);
+            var newMethod = _kvoMethodForMethod(_targetObject, methodList[i]);
 
             if (newMethod)
                 [_replacementMethods setObject:newMethod forKey:methodList[i].name];
@@ -123,11 +139,16 @@ var KVOProxyMap = [CPDictionary dictionary];
     }
 }
 
-- (id)class
+- (Class)class
 {
     return [KVOProxyMap objectForKey:[self hash]]._nativeClass;
 }
 
+- (Class)superclass
+{
+    return [self class].super_class;
+}
+
 - (BOOL)respondsToSelector:(SEL)aSelector
 {
     var proxy = [_CPKVOProxy proxyForObject:self],
@@ -167,76 +188,134 @@ var KVOProxyMap = [CPDictionary dictionary];
     if (!anObserver)
         return;
 
-    var info = [_observerInfos objectForKey:[anObserver hash]];
-    
-    if (!info)
-    {
-        info = _CPKVOInfoMake(anObserver);
-        [_observerInfos setObject:info forKey:[anObserver hash]];
-    }
-
     if (aPath.indexOf('.') != CPNotFound)
-        return print("WHOA, don't go crazy...");
+        return CPLog.error("WHOA, don't go crazy...");
     else
-        [info.keyPaths setObject:_CPKVOInfoRecordMake(options, aContext) forKey:aPath];
+    {
+        var observers = [_observersForKey objectForKey:aPath];
+        
+        if (!observers)
+        {
+            observers = [CPDictionary dictionary];
+            [_observersForKey setObject:observers forKey:aPath];
+        }
+        
+        [observers setObject:_CPKVOInfoMake(anObserver, options, aContext) forKey:[anObserver hash]];
+
+        var changes = [CPDictionary dictionary];
+        [changes setObject:[_targetObject valueForKey:aPath] forKey:CPKeyValueChangeNewKey];
+
+        [_changesForKey setObject:changes forKey:aPath];
+
+        if (options & CPKeyValueObservingOptionInitial)
+            [anObserver observeValueForKeyPath:aPath ofObject:self change:changes context:aContext];
+    }
 }
 
 - (void)_removeObserver:(id)anObserver forKeyPath:(CPString)aPath
 {        
-    var info = [_observerInfos objectForKey:[anObserver hash]],
-        path = [info.keyPaths objectForKey:aPath];
-    
-    if (path)
-        [info.keyPaths removeObjectForKey:aPath];
-    
-    if (![info.keyPaths count])
-        [_observerInfos removeObjectForKey:anObserver];
-
-    if (![_observerInfos count])
+    if (aPath.indexOf('.') != CPNotFound)
+        return CPLog.error("WHOA, don't go crazy...");
+    else
     {
-        _targetObject.isa = _nativeClass; //restore the original class
-        [KVOProxyMap removeObjectForKey:[_targetObject hash]];
+        var observers = [_observersForKey objectForKey:aPath];
+
+        [observers removeObjectForKey:[anObserver hash]];
+        
+        if (![observers count])
+            [_observersForKey removeObjectForKey:aPath];
+            
+        if (![_observersForKey count])
+        {
+            _targetObject.isa = _nativeClass; //restore the original class
+            [KVOProxyMap removeObjectForKey:[_targetObject hash]];
+        }
     }
 }
 
 - (void)willChangeValueForKey:(CPString)aKey
 {
-    print("WILL CHANGE FOR: "+aKey);
+    if (!aKey) 
+        return;
+
+    [[_CPKVOProxy proxyForObject:self] _sendNotificationsForKey:aKey isBefore:YES];
 }
 
 
 - (void)didChangeValueForKey:(CPString)aKey
 {
-    print("DID CHANGE FOR: "+aKey);
+    if (!aKey) 
+        return;
+
+    [[_CPKVOProxy proxyForObject:self] _sendNotificationsForKey:aKey isBefore:NO];
+}
+
+- (void)_sendNotificationsForKey:(CPString)aKey isBefore:(BOOL)isBefore
+{
+    var changes = [_changesForKey objectForKey:aKey],
+        oldValue = [changes objectForKey:CPKeyValueChangeOldKey],
+        newValue = [changes objectForKey:CPKeyValueChangeNewKey];
+    
+    if (!oldValue && oldValue !== "")
+        oldValue = [CPNull null];
+
+    if (!newValue && newValue !== "")
+        newValue = [CPNull null];
+        
+    if (isBefore)
+    {
+        changes = [CPDictionary dictionary];
+        
+        [changes setObject:1 forKey:CPKeyValueChangeNotificationIsPriorKey];
+        [changes setObject:newValue forKey:CPKeyValueChangeOldKey];
+        
+        [_changesForKey setObject:changes forKey:aKey];
+    }
+    else
+    {
+        [changes removeObjectForKey:CPKeyValueChangeNotificationIsPriorKey];
+        
+        var newValue = [_targetObject valueForKey:aKey];
+        
+        if (!newValue && newValue !== "")
+            newValue = [CPNull null];
+
+        [changes setObject:newValue forKey:CPKeyValueChangeNewKey];
+    }
+    
+    var observers = [[_observersForKey objectForKey:aKey] allValues],
+        count = [observers count];
+    
+    while (count--)
+    {
+        var observerInfo = observers[count];
+        
+        if (isBefore && (observerInfo.options & CPKeyValueObservingOptionPrior))
+            [observerInfo.observer observeValueForKeyPath:aKey ofObject:_targetObject change:changes context:observerInfo.context];
+        else if (!isBefore)
+            [observerInfo.observer observeValueForKeyPath:aKey ofObject:_targetObject change:changes context:observerInfo.context];
+    }
 }
 
 @end
 
-var _CPKVOInfoMake = function _CPKVOInfoMake(anObserver)
+var _CPKVOInfoMake = function _CPKVOInfoMake(anObserver, theOptions, aContext)
 {
     return {
         observer: anObserver, 
-        keyPaths: [CPDictionary dictionary],
-        changes:  [CPDictionary dictionary]
+        options: theOptions,
+        context: aContext
     };
 }
 
-var _CPKVOInfoRecordMake = function _CPKVOInfoRecordMake(theOptions, aContext)
-{
-    return {
-        options: theOptions ? theOptions : 0,
-        context: aContext ? aContext : [CPNull null]
-    };
-}
-
-var _kvoMethodForMethod = function _kvoMethodForMethod(theMethod)
+var _kvoMethodForMethod = function _kvoMethodForMethod(theObject, theMethod)
 {
     var methodName = theMethod.name,
         methodImplementation = theMethod.method_imp,
         setterKey = kvoKeyForSetter(methodName);
     
-    if (setterKey)
-    {
+    if (setterKey && objj_msgSend(theObject, @selector(automaticallyNotifiesObserversForKey:), setterKey))
+    {            
         var newMethodImp = function(self) 
         {
             [self willChangeValueForKey:setterKey];
diff --git a/Tests/Foundation/CPKVOTest.j b/Tests/Foundation/CPKVOTest.j
index b04dafdc3..590ec09f1 100644
--- a/Tests/Foundation/CPKVOTest.j
+++ b/Tests/Foundation/CPKVOTest.j
@@ -2,12 +2,16 @@ import <Foundation/CPKeyValueCoding.j>
 import <Foundation/CPKeyValueObserving.j>
 
 @implementation CPKVOTest : OJTestCase
+{
+    BOOL    _sawInitialObservation;
+    BOOL    _sawPriorObservation;
+}
 
 - (void)testAddObserver
 {
     var bob = [[PersonTester alloc] init];
     
-    [bob addObserver:[CPObject new] forKeyPath:@"name" options:nil context:nil];
+    [bob addObserver:self forKeyPath:@"name" options:nil context:"testAddObserver"];
         
     [bob setValue:@"bob" forKey:@"name"];
     
@@ -19,8 +23,8 @@ import <Foundation/CPKeyValueObserving.j>
 {
     var bob = [[PersonTester alloc] init];
     
-    [bob addObserver:[CPArray new] forKeyPath:@"name" options:nil context:nil];
-    [bob addObserver:[CPString new] forKeyPath:@"name" options:nil context:nil];
+    [bob addObserver:self forKeyPath:@"name" options:nil context:"testAddTwoObservers"];
+    [bob addObserver:[CPObject new] forKeyPath:@"name" options:nil context:"testAddTwoObservers"];
         
     [bob setValue:@"bob" forKey:@"name"];
     
@@ -32,7 +36,7 @@ import <Foundation/CPKeyValueObserving.j>
 {
     var bob = [[PersonTester alloc] init];
     
-    [bob addObserver:[CPObject new] forKeyPath:@"phoneNumber" options:nil context:nil];
+    [bob addObserver:self forKeyPath:@"phoneNumber" options:nil context:"testDirectIVarObservation"];
     
     [bob setValue:@"555" forKey:@"phoneNumber"];
     
@@ -42,11 +46,25 @@ import <Foundation/CPKeyValueObserving.j>
 
 - (void)testRemoveObserver
 {
-    var bob = [[PersonTester alloc] init],
-        obj = [CPArray new];
+    var bob = [[PersonTester alloc] init];
         
-    [bob addObserver:obj forKeyPath:@"name" options:nil context:nil];
-    [bob addObserver:[CPString new] forKeyPath:@"name" options:nil context:nil];
+    [bob addObserver:self forKeyPath:@"name" options:nil context:"testRemoveObserver"];
+    [bob addObserver:[CPString new] forKeyPath:@"name" options:nil context:"testRemoveObserver"];
+    
+    [bob removeObserver:self forKeyPath:@"name"];
+    
+    [bob setValue:@"bob" forKey:@"name"];
+    
+    [self assertTrue: [bob valueForKey:@"name"] == @"set_bob" message: "valueForKey:'name' should be bob, was: "+[bob valueForKey:@"name"]];
+}
+
+- (void)testRemoveOtherObserver
+{
+    var bob = [[PersonTester alloc] init],
+        obj = [CPString new];
+        
+    [bob addObserver:self forKeyPath:@"name" options:nil context:"testRemoveOtherObserver"];
+    [bob addObserver:obj forKeyPath:@"name" options:nil context:"testRemoveOtherObserver"];
     
     [bob removeObserver:obj forKeyPath:@"name"];
     
@@ -61,8 +79,8 @@ import <Foundation/CPKeyValueObserving.j>
         obj1 = [CPArray new],
         obj2 = [CPString new];
         
-    [bob addObserver:obj1 forKeyPath:@"name" options:nil context:nil];
-    [bob addObserver:obj2 forKeyPath:@"name" options:nil context:nil];
+    [bob addObserver:obj1 forKeyPath:@"name" options:nil context:"testRemoveAllObservers"];
+    [bob addObserver:obj2 forKeyPath:@"name" options:nil context:"testRemoveAllObservers"];
     
     [bob removeObserver:obj1 forKeyPath:@"name"];
     [bob removeObserver:obj2 forKeyPath:@"name"];
@@ -74,8 +92,109 @@ import <Foundation/CPKeyValueObserving.j>
 
 }
 
-//test actual observing
-//test each observing option
+- (void)testPriorObservationOption
+{
+    _sawPriorObservation = NO;
+    
+    var bob = [[PersonTester alloc] init];
+    
+    [bob addObserver:self forKeyPath:@"name" options:CPKeyValueObservingOptionPrior context:"testPriorObservationOption"];
+
+    [bob setValue:@"bob" forKey:@"name"];
+
+    [self assertTrue: _sawPriorObservation message: "asked for CPKeyValueObservingOptionPrior but did not recieve corresponding notification"];
+}
+
+- (void)testInitialObservationOption
+{
+    _sawInitialObservation = NO;
+
+    var bob = [[PersonTester alloc] init];
+    
+    bob.name = "paul";    
+
+    [bob addObserver:self forKeyPath:@"name" options:CPKeyValueObservingOptionInitial context:"testInitialObservationOption"];
+    [bob removeObserver:self forKeyPath:@"name"];
+
+    [bob setValue:@"bob" forKey:@"name"];
+
+    [self assertTrue: _sawInitialObservation message: "asked for CPKeyValueObservingOptionInitial but did not recieve corresponding notification"];
+}
+
+- (void)observeValueForKeyPath:(CPString)aKeyPath ofObject:(id)anObject change:(CPDictionary)changes context:(id)aContext
+{
+    var oldValue = [changes objectForKey:CPKeyValueChangeOldKey],
+        newValue = [changes objectForKey:CPKeyValueChangeNewKey];
+
+    switch (aContext)
+    {
+        case "testAddObserver": 
+            [self assertTrue: newValue == "set_bob" message: "newValue should be: set_bob was: "+newValue];
+            [self assertTrue: oldValue == [CPNull null] message: "oldValue should be CPNull was: "+oldValue];
+            break;
+            
+        case "testAddTwoObservers":             
+            [self assertTrue: newValue == "set_bob" message: "newValue should be: set_bob was: "+newValue];
+            [self assertTrue: oldValue == [CPNull null] message: "oldValue should be CPNull was: "+oldValue];
+            break;
+            
+        case "testDirectIVarObservation":
+            [self assertTrue: newValue == "555" message: "newValue should be: 555 was: "+newValue];
+            [self assertTrue: oldValue == [CPNull null] message: "oldValue should be CPNull was: "+oldValue];
+            break;
+
+        case "testRemoveObserver":
+            [self assertTrue: NO message: "observer was removed, but notification was still received"];
+            break;
+
+        case "testRemoveOtherObserver":
+            [self assertTrue: newValue == "set_bob" message: "newValue should be: set_bob was: "+newValue];
+            [self assertTrue: oldValue == [CPNull null] message: "oldValue should be CPNull was: "+oldValue];
+            break;
+
+        case "testRemoveAllObservers":
+            [self assertTrue: NO message: "all observers were removed, but notification was still received"];
+            break;
+            
+        case "testPriorObservationOption":
+            var prior = [changes objectForKey:CPKeyValueChangeNotificationIsPriorKey];
+            
+            if (!_sawPriorObservation)
+            {
+                [self assertTrue:prior message:"Have not been sent the prior notification, but it should have been sent"];
+                [self assertTrue:oldValue == [CPNull null] message: "Shoudl be no initial value"];
+                [self assertFalse:newValue message: "Should be no object for the new value key on the prior notification"];
+                _sawPriorObservation = YES;
+            }
+            else
+            {
+                [self assertFalse: prior message: "there should be no value for the notification is prior key, that notification was already sent"];
+                [self assertTrue: newValue == "set_bob" message: "newValue should be: set_bob was: "+newValue];
+            }
+            break;
+            
+        case "testInitialObservationOption":
+            if (!_sawInitialObservation)
+            {
+                [self assertTrue: newValue == "paul" message:"Expected old value to be: paul was: "+oldValue];
+                [self assertFalse: oldValue message:"Should be no value for new change key on initial observation"];
+                _sawInitialObservation = YES;
+            }
+            else
+                [self assertFalse:YES message:"Should never have received this notification"];
+
+            break;
+    }
+}
+
+@end
+
+@implementation CPObject (KVO)
+
+- (void)observeValueForKeyPath:(CPString)aKeyPath ofObject:(id)anObject change:(CPDictionary)changes context:(id)aContext
+{
+
+}
 
 @end