From 043029cf41b2fd4505dd16b8ba5c015f9a12b880 Mon Sep 17 00:00:00 2001 From: daboe01 Date: Fri, 3 Apr 2026 13:30:13 +0200 Subject: [PATCH 1/3] new: _CPKVOCollectionOperatorObserver --- Foundation/CPKeyValueObserving.j | 176 ++++++++++++++++++++++++++++++- 1 file changed, 174 insertions(+), 2 deletions(-) diff --git a/Foundation/CPKeyValueObserving.j b/Foundation/CPKeyValueObserving.j index f789fcbf9..3d3aa4315 100644 --- a/Foundation/CPKeyValueObserving.j +++ b/Foundation/CPKeyValueObserving.j @@ -763,9 +763,12 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti if (!anObserver) return; - var forwarder = nil; + var forwarder = nil, + collectionOperatorMatch = aPath.match(/^(.*)\.(@\w+)(?:\.(.*))?$/); - if (aPath.indexOf('.') !== CPNotFound && aPath.charAt(0) !== '@') + if (collectionOperatorMatch) + forwarder = [[_CPKVOCollectionOperatorObserver alloc] initWithTarget:_targetObject observer:anObserver keyPath:aPath options:options context:aContext]; + else if (aPath.indexOf('.') !== CPNotFound && aPath.charAt(0) !== '@') forwarder = [[_CPKVOForwardingObserver alloc] initWithKeyPath:aPath object:_targetObject observer:anObserver options:options context:aContext]; else [self _replaceModifiersForKey:aPath]; @@ -1265,6 +1268,175 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti @end + +@implementation _CPKVOCollectionOperatorObserver : CPObject +{ + id _target; + id _originalObserver; + CPString _fullKeyPath; + unsigned _options; + id _context; + + CPString _collectionKeyPath; + CPString _operator; + CPString _valueKeyPath; + + CPArray _observedItems; +} + +- (id)initWithTarget:(id)aTarget observer:(id)anObserver keyPath:(CPString)aKeyPath options:(unsigned)options context:(id)aContext +{ + self = [super init]; + + if (self) + { + _target = aTarget; + _originalObserver = anObserver; + _fullKeyPath = aKeyPath; + _options = options; + _context = aContext; + _observedItems = [CPArray array]; + + // Parse key path like "collectionKeyPath.@sum.valueKeyPath" + // Captures: 1=collectionKeyPath, 2=@sum, 3=valueKeyPath + var match = aKeyPath.match(/^(.*)\.(@\w+)(?:\.(.*))?$/); + if (match) + { + _collectionKeyPath = match[1]; + _operator = match[2]; + _valueKeyPath = match[3]; + } + + // Observe the collection itself on the target to catch array resets/mutations + [_target addObserver:self forKeyPath:_collectionKeyPath options:CPKeyValueObservingOptionNew | CPKeyValueObservingOptionOld context:nil]; + + [self _setupItemObservers]; + } + + return self; +} + +- (void)_setupItemObservers +{ + var collection = [_target valueForKeyPath:_collectionKeyPath]; + + if (collection && [collection respondsToSelector:@selector(objectEnumerator)]) + { + var enumerator = [collection objectEnumerator], + item; + + while ((item = [enumerator nextObject]) !== nil) + { + if (_valueKeyPath && [item respondsToSelector:@selector(addObserver:forKeyPath:options:context:)]) + [item addObserver:self forKeyPath:_valueKeyPath options:_options context:nil]; + + [_observedItems addObject:item]; + } + } +} + +- (void)_tearDownItemObservers +{ + if (_valueKeyPath) + { + var count = [_observedItems count]; + while (count--) + { + var item = [_observedItems objectAtIndex:count]; + if ([item respondsToSelector:@selector(removeObserver:forKeyPath:context:)]) + [item removeObserver:self forKeyPath:_valueKeyPath context:nil]; + } + } + + [_observedItems removeAllObjects]; +} + +- (void)observeValueForKeyPath:(CPString)keyPath ofObject:(id)object change:(CPDictionary)change context:(id)context +{ + // Branch 1: The collection on the target was modified + if (object === _target && [keyPath isEqualToString:_collectionKeyPath]) + { + var kind = [change objectForKey:CPKeyValueChangeKindKey]; + + if (kind === CPKeyValueChangeSetting) + { + [self _tearDownItemObservers]; + [self _setupItemObservers]; + } + else if (kind === CPKeyValueChangeInsertion || kind === CPKeyValueChangeReplacement) + { + if (kind === CPKeyValueChangeReplacement) + { + var oldItems = [change objectForKey:CPKeyValueChangeOldKey]; + if (oldItems) + { + for (var i = 0, len = oldItems.length; i < len; i++) + { + var item = oldItems[i]; + if (_valueKeyPath && [item respondsToSelector:@selector(removeObserver:forKeyPath:context:)]) + [item removeObserver:self forKeyPath:_valueKeyPath context:nil]; + [_observedItems removeObject:item]; + } + } + } + + var newItems = [change objectForKey:CPKeyValueChangeNewKey]; + if (newItems) + { + for (var i = 0, len = newItems.length; i < len; i++) + { + var item = newItems[i]; + if (_valueKeyPath && [item respondsToSelector:@selector(addObserver:forKeyPath:options:context:)]) + [item addObserver:self forKeyPath:_valueKeyPath options:_options context:nil]; + [_observedItems addObject:item]; + } + } + } + else if (kind === CPKeyValueChangeRemoval) + { + var oldItems = [change objectForKey:CPKeyValueChangeOldKey]; + if (oldItems) + { + for (var i = 0, len = oldItems.length; i < len; i++) + { + var item = oldItems[i]; + if (_valueKeyPath && [item respondsToSelector:@selector(removeObserver:forKeyPath:context:)]) + [item removeObserver:self forKeyPath:_valueKeyPath context:nil]; + [_observedItems removeObject:item]; + } + } + } + + [self _notifyOriginalObserver]; + } + // Branch 2: One of the children items has updated + else if (_valueKeyPath && [keyPath isEqualToString:_valueKeyPath] && [_observedItems containsObject:object]) + { + [self _notifyOriginalObserver]; + } +} + +- (void)_notifyOriginalObserver +{ + // Ask standard KVC to evaluate the entire aggregate string (e.g., @sum.value) + var newValue = [_target valueForKeyPath:_fullKeyPath], + change = @{ + CPKeyValueChangeKindKey: CPKeyValueChangeSetting, + CPKeyValueChangeNewKey: (newValue !== nil ? newValue : [CPNull null]) + }; + + [_originalObserver observeValueForKeyPath:_fullKeyPath ofObject:_target change:change context:_context]; +} + +- (void)finalize +{ + [_target removeObserver:self forKeyPath:_collectionKeyPath]; + [self _tearDownItemObservers]; +} + +@end + + @implementation _CPKVOForwardingObserver : CPObject { id _object; From 4b81001330e83c51d62ad16dec023008decba8f4 Mon Sep 17 00:00:00 2001 From: daboe01 Date: Fri, 3 Apr 2026 13:33:06 +0200 Subject: [PATCH 2/3] new: testCollectionOperatorKeyPath --- Tests/Foundation/CPKVOTest.j | 63 ++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/Tests/Foundation/CPKVOTest.j b/Tests/Foundation/CPKVOTest.j index 631e2ac80..b3215ec4e 100644 --- a/Tests/Foundation/CPKVOTest.j +++ b/Tests/Foundation/CPKVOTest.j @@ -3,6 +3,8 @@ @class CarTester @class ToManyTester +@class AccountTester +@class TransactionTester @implementation CPKVOTest : OJTestCase { @@ -436,6 +438,35 @@ [self assertTrue:_sawObservation message:@"Never recieved an observation"]; } +- (void)testCollectionOperatorKeyPath +{ + var account = [[AccountTester alloc] init], + t1 = [[TransactionTester alloc] initWithAmount:10.0], + t2 = [[TransactionTester alloc] initWithAmount:20.0]; + + [account setTransactions:[t1, t2]]; + + // 1. Test updating a child property triggers an aggregate update + [account addObserver:self forKeyPath:@"transactions.@sum.amount" options:CPKeyValueObservingOptionNew | CPKeyValueObservingOptionOld context:@"testCollectionOperatorKeyPath_ItemChange"]; + + _sawObservation = NO; + [t1 setAmount:15.0]; // KVC sum goes from 30.0 -> 35.0 + + [self assertTrue:_sawObservation message:@"Never received an observation when child item's property changed"]; + [account removeObserver:self forKeyPath:@"transactions.@sum.amount"]; + + // 2. Test replacing the whole array triggers an aggregate update + [account addObserver:self forKeyPath:@"transactions.@sum.amount" options:CPKeyValueObservingOptionNew | CPKeyValueObservingOptionOld context:@"testCollectionOperatorKeyPath_ArrayReplace"]; + + var t3 = [[TransactionTester alloc] initWithAmount:5.0]; + + _sawObservation = NO; + [account setTransactions:[t1, t2, t3]]; // KVC sum goes from 35.0 -> 40.0 + + [self assertTrue:_sawObservation message:@"Never received an observation when collection was replaced"]; + [account removeObserver:self forKeyPath:@"transactions.@sum.amount"]; +} + - (void)testPerformance { bob = [PersonTester new]; @@ -777,6 +808,16 @@ testNestedNotificationsBobCount += 1; break; + case "testCollectionOperatorKeyPath_ItemChange": + [self assert:newValue equals:35.0]; + [self assert:oldValue equals:30.0]; + break; + + case "testCollectionOperatorKeyPath_ArrayReplace": + [self assert:newValue equals:40.0]; + [self assert:oldValue equals:35.0]; + break; + default: [self assertFalse:YES message:@"unhandled observation, must be an error"]; return; @@ -955,3 +996,25 @@ } @end + +@implementation TransactionTester : CPObject +{ + float amount @accessors; +} + +- (id)initWithAmount:(float)anAmount +{ + self = [super init]; + if (self) { + amount = anAmount; + } + return self; +} + +@end + +@implementation AccountTester : CPObject +{ + CPArray transactions @accessors; +} +@end From 0e5f3b1697b0b99be46e04591381aa1e1303ee6d Mon Sep 17 00:00:00 2001 From: daboe01 Date: Fri, 3 Apr 2026 13:49:33 +0200 Subject: [PATCH 3/3] fixed caching --- Foundation/CPKeyValueObserving.j | 122 ++++++++++++++++++------------- 1 file changed, 70 insertions(+), 52 deletions(-) diff --git a/Foundation/CPKeyValueObserving.j b/Foundation/CPKeyValueObserving.j index 3d3aa4315..855342d6a 100644 --- a/Foundation/CPKeyValueObserving.j +++ b/Foundation/CPKeyValueObserving.j @@ -1282,6 +1282,7 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti CPString _valueKeyPath; CPArray _observedItems; + id _cachedValue; } - (id)initWithTarget:(id)aTarget observer:(id)anObserver keyPath:(CPString)aKeyPath options:(unsigned)options context:(id)aContext @@ -1297,8 +1298,6 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti _context = aContext; _observedItems = [CPArray array]; - // Parse key path like "collectionKeyPath.@sum.valueKeyPath" - // Captures: 1=collectionKeyPath, 2=@sum, 3=valueKeyPath var match = aKeyPath.match(/^(.*)\.(@\w+)(?:\.(.*))?$/); if (match) { @@ -1307,8 +1306,9 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti _valueKeyPath = match[3]; } - // Observe the collection itself on the target to catch array resets/mutations - [_target addObserver:self forKeyPath:_collectionKeyPath options:CPKeyValueObservingOptionNew | CPKeyValueObservingOptionOld context:nil]; + _cachedValue = [_target valueForKeyPath:_fullKeyPath]; + + [_target addObserver:self forKeyPath:_collectionKeyPath options:_options context:nil]; [self _setupItemObservers]; } @@ -1353,19 +1353,50 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti - (void)observeValueForKeyPath:(CPString)keyPath ofObject:(id)object change:(CPDictionary)change context:(id)context { - // Branch 1: The collection on the target was modified + var isPrior = [[change objectForKey:CPKeyValueChangeNotificationIsPriorKey] boolValue]; + if (object === _target && [keyPath isEqualToString:_collectionKeyPath]) { - var kind = [change objectForKey:CPKeyValueChangeKindKey]; + // Only tear down/rebuild observers when the actual change happens, not during the "Prior" notification + if (!isPrior) + { + var kind = [change objectForKey:CPKeyValueChangeKindKey]; - if (kind === CPKeyValueChangeSetting) - { - [self _tearDownItemObservers]; - [self _setupItemObservers]; - } - else if (kind === CPKeyValueChangeInsertion || kind === CPKeyValueChangeReplacement) - { - if (kind === CPKeyValueChangeReplacement) + if (kind === CPKeyValueChangeSetting) + { + [self _tearDownItemObservers]; + [self _setupItemObservers]; + } + else if (kind === CPKeyValueChangeInsertion || kind === CPKeyValueChangeReplacement) + { + if (kind === CPKeyValueChangeReplacement) + { + var oldItems = [change objectForKey:CPKeyValueChangeOldKey]; + if (oldItems) + { + for (var i = 0, len = oldItems.length; i < len; i++) + { + var item = oldItems[i]; + if (_valueKeyPath && [item respondsToSelector:@selector(removeObserver:forKeyPath:context:)]) + [item removeObserver:self forKeyPath:_valueKeyPath context:nil]; + [_observedItems removeObject:item]; + } + } + } + + var newItems = [change objectForKey:CPKeyValueChangeNewKey]; + if (newItems) + { + for (var i = 0, len = newItems.length; i < len; i++) + { + var item = newItems[i]; + if (_valueKeyPath && [item respondsToSelector:@selector(addObserver:forKeyPath:options:context:)]) + [item addObserver:self forKeyPath:_valueKeyPath options:_options context:nil]; + [_observedItems addObject:item]; + } + } + } + else if (kind === CPKeyValueChangeRemoval) { var oldItems = [change objectForKey:CPKeyValueChangeOldKey]; if (oldItems) @@ -1379,53 +1410,41 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti } } } - - var newItems = [change objectForKey:CPKeyValueChangeNewKey]; - if (newItems) - { - for (var i = 0, len = newItems.length; i < len; i++) - { - var item = newItems[i]; - if (_valueKeyPath && [item respondsToSelector:@selector(addObserver:forKeyPath:options:context:)]) - [item addObserver:self forKeyPath:_valueKeyPath options:_options context:nil]; - [_observedItems addObject:item]; - } - } - } - else if (kind === CPKeyValueChangeRemoval) - { - var oldItems = [change objectForKey:CPKeyValueChangeOldKey]; - if (oldItems) - { - for (var i = 0, len = oldItems.length; i < len; i++) - { - var item = oldItems[i]; - if (_valueKeyPath && [item respondsToSelector:@selector(removeObserver:forKeyPath:context:)]) - [item removeObserver:self forKeyPath:_valueKeyPath context:nil]; - [_observedItems removeObject:item]; - } - } } - [self _notifyOriginalObserver]; + [self _notifyOriginalObserverIsPrior:isPrior]; } - // Branch 2: One of the children items has updated else if (_valueKeyPath && [keyPath isEqualToString:_valueKeyPath] && [_observedItems containsObject:object]) { - [self _notifyOriginalObserver]; + [self _notifyOriginalObserverIsPrior:isPrior]; } } -- (void)_notifyOriginalObserver +- (void)_notifyOriginalObserverIsPrior:(BOOL)isPrior { - // Ask standard KVC to evaluate the entire aggregate string (e.g., @sum.value) - var newValue = [_target valueForKeyPath:_fullKeyPath], - change = @{ - CPKeyValueChangeKindKey: CPKeyValueChangeSetting, - CPKeyValueChangeNewKey: (newValue !== nil ? newValue : [CPNull null]) - }; + var change = [CPMutableDictionary dictionaryWithObject:CPKeyValueChangeSetting forKey:CPKeyValueChangeKindKey]; - [_originalObserver observeValueForKeyPath:_fullKeyPath ofObject:_target change:change context:_context]; + if (isPrior) + [change setObject:YES forKey:CPKeyValueChangeNotificationIsPriorKey]; + + if (_options & CPKeyValueObservingOptionOld) + [change setObject:(_cachedValue !== nil ? _cachedValue : [CPNull null]) forKey:CPKeyValueChangeOldKey]; + + if (!isPrior) + { + var newValue = [_target valueForKeyPath:_fullKeyPath]; + if (_options & CPKeyValueObservingOptionNew) + [change setObject:(newValue !== nil ? newValue : [CPNull null]) forKey:CPKeyValueChangeNewKey]; + + [_originalObserver observeValueForKeyPath:_fullKeyPath ofObject:_target change:change context:_context]; + + // Update the cache for the next notification + _cachedValue = newValue; + } + else + { + [_originalObserver observeValueForKeyPath:_fullKeyPath ofObject:_target change:change context:_context]; + } } - (void)finalize @@ -1436,7 +1455,6 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti @end - @implementation _CPKVOForwardingObserver : CPObject { id _object;