From 3ddf53d9faf7aef86dad0aa5b93825ead67f50fd Mon Sep 17 00:00:00 2001 From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> Date: Fri, 12 Apr 2024 18:20:14 -0700 Subject: [PATCH] [Darwin] MTRDeviceControllerStorageDelegate should support bulk store/fetch (#32858) * [Darwin] MTRDeviceControllerStorageDelegate should support bulk store/fetch * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky * MTRDeviceControllerStorageDelegate header documentation clarification * Update src/darwin/Framework/CHIP/MTRDevice.mm Co-authored-by: Boris Zbarsky * Update src/darwin/Framework/CHIP/MTRDeviceController.mm Co-authored-by: Boris Zbarsky * Update src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h Co-authored-by: Boris Zbarsky * Update src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m Co-authored-by: Boris Zbarsky * Update src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m Co-authored-by: Boris Zbarsky * Address review comments * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky * Update src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h Co-authored-by: Boris Zbarsky * Move processing out of storage delegate queue dispatch_sync --------- Co-authored-by: Boris Zbarsky Co-authored-by: Justin Wood --- src/darwin/Framework/CHIP/MTRDevice.mm | 22 +++ .../Framework/CHIP/MTRDeviceController.mm | 66 +++++-- .../CHIP/MTRDeviceControllerDataStore.h | 9 + .../CHIP/MTRDeviceControllerDataStore.mm | 169 ++++++++++++++--- .../MTRDeviceControllerLocalTestStorage.mm | 48 ++++- .../CHIP/MTRDeviceControllerStorageDelegate.h | 44 +++-- .../Framework/CHIP/MTRDevice_Internal.h | 4 + .../CHIPTests/MTRPerControllerStorageTests.m | 173 +++++++++++++++++- .../TestHelpers/MTRTestDeclarations.h | 5 + .../TestHelpers/MTRTestPerControllerStorage.h | 11 ++ .../TestHelpers/MTRTestPerControllerStorage.m | 31 ++++ 11 files changed, 530 insertions(+), 52 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 9997b370320e7b..ee72c4cc815ad1 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -236,6 +236,20 @@ - (id)copyWithZone:(NSZone *)zone return [[MTRDeviceClusterData alloc] initWithDataVersion:_dataVersion attributes:_attributes]; } +- (BOOL)isEqualToClusterData:(MTRDeviceClusterData *)otherClusterData +{ + return [_dataVersion isEqual:otherClusterData.dataVersion] && [_attributes isEqual:otherClusterData.attributes]; +} + +- (BOOL)isEqual:(id)object +{ + if ([object class] != [self class]) { + return NO; + } + + return [self isEqualToClusterData:object]; +} + @end @interface MTRDevice () @@ -2250,6 +2264,14 @@ - (void)setAttributeValues:(NSArray *)attributeValues reportChan [self _setAttributeValues:attributeValues reportChanges:reportChanges]; } +#ifdef DEBUG +- (NSUInteger)unitTestAttributeCount +{ + std::lock_guard lock(_lock); + return _readCache.count; +} +#endif + - (void)setClusterData:(NSDictionary *)clusterData { MTR_LOG_INFO("%@ setClusterData count: %lu", self, static_cast(clusterData.count)); diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 4c7c7c7c964659..ab2bba520b2675 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -576,6 +576,20 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams return NO; } + if (_controllerDataStore) { + // If the storage delegate supports the bulk read API, then a dictionary of nodeID => cluster data dictionary would be passed to the handler. Otherwise this would be a no-op, and stored attributes for MTRDevice objects will be loaded lazily in -deviceForNodeID:. + [_controllerDataStore fetchAttributeDataForAllDevices:^(NSDictionary *> * _Nonnull clusterDataByNode) { + MTR_LOG_INFO("Loaded attribute values for %lu nodes from storage for controller uuid %@", static_cast(clusterDataByNode.count), self->_uniqueIdentifier); + + std::lock_guard lock(self->_deviceMapLock); + for (NSNumber * nodeID in clusterDataByNode) { + NSDictionary * clusterData = clusterDataByNode[nodeID]; + MTRDevice * device = [self _setupDeviceForNodeID:nodeID prefetchedClusterData:clusterData]; + MTR_LOG_INFO("Loaded %lu cluster data from storage for %@", static_cast(clusterData.count), device); + } + }]; + } + return YES; } @@ -919,20 +933,25 @@ - (MTRBaseDevice *)baseDeviceForNodeID:(NSNumber *)nodeID return [[MTRBaseDevice alloc] initWithNodeID:nodeID controller:self]; } -- (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID +// If prefetchedClusterData is not provided, load attributes individually from controller data store +- (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(NSDictionary *)prefetchedClusterData { - std::lock_guard lock(_deviceMapLock); - MTRDevice * deviceToReturn = _nodeIDToDeviceMap[nodeID]; - if (!deviceToReturn) { - deviceToReturn = [[MTRDevice alloc] initWithNodeID:nodeID controller:self]; - // If we're not running, don't add the device to our map. That would - // create a cycle that nothing would break. Just return the device, - // which will be in exactly the state it would be in if it were created - // while we were running and then we got shut down. - if ([self isRunning]) { - _nodeIDToDeviceMap[nodeID] = deviceToReturn; - } + os_unfair_lock_assert_owner(&_deviceMapLock); + + MTRDevice * deviceToReturn = [[MTRDevice alloc] initWithNodeID:nodeID controller:self]; + // If we're not running, don't add the device to our map. That would + // create a cycle that nothing would break. Just return the device, + // which will be in exactly the state it would be in if it were created + // while we were running and then we got shut down. + if ([self isRunning]) { + _nodeIDToDeviceMap[nodeID] = deviceToReturn; + } + if (prefetchedClusterData) { + if (prefetchedClusterData.count) { + [deviceToReturn setClusterData:prefetchedClusterData]; + } + } else { #if !MTRDEVICE_ATTRIBUTE_CACHE_STORE_ATTRIBUTES_BY_CLUSTER // Load persisted attributes if they exist. NSArray * attributesFromCache = [_controllerDataStore getStoredAttributesForNodeID:nodeID]; @@ -952,6 +971,17 @@ - (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID return deviceToReturn; } +- (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID +{ + std::lock_guard lock(_deviceMapLock); + MTRDevice * deviceToReturn = _nodeIDToDeviceMap[nodeID]; + if (!deviceToReturn) { + deviceToReturn = [self _setupDeviceForNodeID:nodeID prefetchedClusterData:nil]; + } + + return deviceToReturn; +} + - (void)removeDevice:(MTRDevice *)device { std::lock_guard lock(_deviceMapLock); @@ -965,6 +995,18 @@ - (void)removeDevice:(MTRDevice *)device } } +#ifdef DEBUG +- (NSDictionary *)unitTestGetDeviceAttributeCounts +{ + std::lock_guard lock(_deviceMapLock); + NSMutableDictionary * deviceAttributeCounts = [NSMutableDictionary dictionary]; + for (NSNumber * nodeID in _nodeIDToDeviceMap) { + deviceAttributeCounts[nodeID] = @([_nodeIDToDeviceMap[nodeID] unitTestAttributeCount]); + } + return deviceAttributeCounts; +} +#endif + - (void)setDeviceControllerDelegate:(id)delegate queue:(dispatch_queue_t)queue { [self diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h index 27019b06df1acb..bc3b8f37f90fbb 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h @@ -44,6 +44,15 @@ NS_ASSUME_NONNULL_BEGIN storageDelegate:(id)storageDelegate storageDelegateQueue:(dispatch_queue_t)storageDelegateQueue; +// clusterDataByNode a dictionary: nodeID => cluster data dictionary +typedef void (^MTRDeviceControllerDataStoreClusterDataHandler)(NSDictionary *> * clusterDataByNode); + +/** + * Asks the data store to load cluster data for nodes in bulk. If the storageDelegate supports it, the handler will be called synchronously. + * If the storageDelegate does not support it, the handler will not be called at all. + */ +- (void)fetchAttributeDataForAllDevices:(MTRDeviceControllerDataStoreClusterDataHandler)clusterDataHandler; + /** * Resumption info APIs. */ diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm index 4c7c43586aa29c..d6c7e8c00dfeb1 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm @@ -151,6 +151,20 @@ - (nullable instancetype)initWithController:(MTRDeviceController *)controller return self; } +- (void)fetchAttributeDataForAllDevices:(MTRDeviceControllerDataStoreClusterDataHandler)clusterDataHandler +{ + __block NSDictionary * dataStoreSecureLocalValues = nil; + dispatch_sync(_storageDelegateQueue, ^{ + if ([self->_storageDelegate respondsToSelector:@selector(valuesForController:securityLevel:sharingType:)]) { + dataStoreSecureLocalValues = [self->_storageDelegate valuesForController:self->_controller securityLevel:MTRStorageSecurityLevelSecure sharingType:MTRStorageSharingTypeNotShared]; + } + }); + + if (dataStoreSecureLocalValues.count) { + clusterDataHandler([self _getClusterDataFromSecureLocalValues:dataStoreSecureLocalValues]); + } +} + - (nullable MTRCASESessionResumptionInfo *)findResumptionInfoByNodeID:(NSNumber *)nodeID { return [self _findResumptionInfoWithKey:ResumptionByNodeIDKey(nodeID)]; @@ -337,6 +351,14 @@ - (BOOL)_storeAttributeCacheValue:(id)value forKey:(NSString *)key sharingType:MTRStorageSharingTypeNotShared]; } +- (BOOL)_bulkStoreAttributeCacheValues:(NSDictionary> *)values +{ + return [_storageDelegate controller:_controller + storeValues:values + securityLevel:MTRStorageSecurityLevelSecure + sharingType:MTRStorageSharingTypeNotShared]; +} + - (BOOL)_removeAttributeCacheValueForKey:(NSString *)key { return [_storageDelegate controller:_controller @@ -968,6 +990,76 @@ - (void)clearAllStoredAttributes return clusterDataToReturn; } +// Utility for constructing dictionary of nodeID to cluster data from dictionary of storage keys +- (nullable NSDictionary *> *)_getClusterDataFromSecureLocalValues:(NSDictionary *)secureLocalValues +{ + NSMutableDictionary *> * clusterDataByNodeToReturn = nil; + + if (![secureLocalValues isKindOfClass:[NSDictionary class]]) { + return nil; + } + + // Fetch node index + NSArray * nodeIndex = secureLocalValues[sAttributeCacheNodeIndexKey]; + + if (![nodeIndex isKindOfClass:[NSArray class]]) { + return nil; + } + + for (NSNumber * nodeID in nodeIndex) { + if (![nodeID isKindOfClass:[NSNumber class]]) { + continue; + } + + NSMutableDictionary * clusterDataForNode = nil; + NSArray * endpointIndex = secureLocalValues[[self _endpointIndexKeyForNodeID:nodeID]]; + + if (![endpointIndex isKindOfClass:[NSArray class]]) { + continue; + } + + for (NSNumber * endpointID in endpointIndex) { + if (![endpointID isKindOfClass:[NSNumber class]]) { + continue; + } + + NSArray * clusterIndex = secureLocalValues[[self _clusterIndexKeyForNodeID:nodeID endpointID:endpointID]]; + + if (![clusterIndex isKindOfClass:[NSArray class]]) { + continue; + } + + for (NSNumber * clusterID in clusterIndex) { + if (![clusterID isKindOfClass:[NSNumber class]]) { + continue; + } + + MTRDeviceClusterData * clusterData = secureLocalValues[[self _clusterDataKeyForNodeID:nodeID endpointID:endpointID clusterID:clusterID]]; + if (!clusterData) { + continue; + } + if (![clusterData isKindOfClass:[MTRDeviceClusterData class]]) { + continue; + } + MTRClusterPath * clusterPath = [MTRClusterPath clusterPathWithEndpointID:endpointID clusterID:clusterID]; + if (!clusterDataForNode) { + clusterDataForNode = [NSMutableDictionary dictionary]; + } + clusterDataForNode[clusterPath] = clusterData; + } + } + + if (clusterDataForNode.count) { + if (!clusterDataByNodeToReturn) { + clusterDataByNodeToReturn = [NSMutableDictionary dictionary]; + } + clusterDataByNodeToReturn[nodeID] = clusterDataForNode; + } + } + + return clusterDataByNodeToReturn; +} + - (void)storeClusterData:(NSDictionary *)clusterData forNodeID:(NSNumber *)nodeID { if (!nodeID) { @@ -983,6 +1075,11 @@ - (void)storeClusterData:(NSDictionary dispatch_async(_storageDelegateQueue, ^{ NSUInteger storeFailures = 0; + NSMutableDictionary> * bulkValuesToStore = nil; + if ([self->_storageDelegate respondsToSelector:@selector(controller:storeValues:securityLevel:sharingType:)]) { + bulkValuesToStore = [NSMutableDictionary dictionary]; + } + // A map of endpoint => list of clusters modified for that endpoint so cluster indexes can be updated later NSMutableDictionary *> * clustersModified = [NSMutableDictionary dictionary]; @@ -993,11 +1090,15 @@ - (void)storeClusterData:(NSDictionary MTR_LOG_INFO("Attempt to store clusterData @ node 0x%016llX endpoint %u cluster 0x%08lX", nodeID.unsignedLongLongValue, path.endpoint.unsignedShortValue, path.cluster.unsignedLongValue); #endif - // Store cluster data - BOOL storeFailed = ![self _storeClusterData:data forNodeID:nodeID endpointID:path.endpoint clusterID:path.cluster]; - if (storeFailed) { - storeFailures++; - MTR_LOG_INFO("Store failed for clusterDAta @ node 0x%016llX endpoint %u cluster 0x%08lX", nodeID.unsignedLongLongValue, path.endpoint.unsignedShortValue, path.cluster.unsignedLongValue); + if (bulkValuesToStore) { + bulkValuesToStore[[self _clusterDataKeyForNodeID:nodeID endpointID:path.endpoint clusterID:path.cluster]] = data; + } else { + // Store cluster data + BOOL storeFailed = ![self _storeClusterData:data forNodeID:nodeID endpointID:path.endpoint clusterID:path.cluster]; + if (storeFailed) { + storeFailures++; + MTR_LOG_INFO("Store failed for clusterData @ node 0x%016llX endpoint %u cluster 0x%08lX", nodeID.unsignedLongLongValue, path.endpoint.unsignedShortValue, path.cluster.unsignedLongValue); + } } // Note the cluster as modified for the endpoint @@ -1046,36 +1147,60 @@ - (void)storeClusterData:(NSDictionary } if (clusterIndexModified) { - BOOL storeFailed = ![self _storeClusterIndex:clusterIndexToStore forNodeID:nodeID endpointID:endpointID]; - if (storeFailed) { - storeFailures++; - MTR_LOG_INFO("Store failed for clusterIndex @ node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue); - continue; + if (bulkValuesToStore) { + bulkValuesToStore[[self _clusterIndexKeyForNodeID:nodeID endpointID:endpointID]] = clusterIndexToStore; + } else { + BOOL storeFailed = ![self _storeClusterIndex:clusterIndexToStore forNodeID:nodeID endpointID:endpointID]; + if (storeFailed) { + storeFailures++; + MTR_LOG_INFO("Store failed for clusterIndex @ node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue); + continue; + } } } } // Update endpoint index as needed if (endpointIndexModified) { - BOOL storeFailed = ![self _storeEndpointIndex:endpointIndexToStore forNodeID:nodeID]; - if (storeFailed) { - storeFailures++; - MTR_LOG_INFO("Store failed for endpointIndex @ node 0x%016llX", nodeID.unsignedLongLongValue); + if (bulkValuesToStore) { + bulkValuesToStore[[self _endpointIndexKeyForNodeID:nodeID]] = endpointIndexToStore; + } else { + BOOL storeFailed = ![self _storeEndpointIndex:endpointIndexToStore forNodeID:nodeID]; + if (storeFailed) { + storeFailures++; + MTR_LOG_INFO("Store failed for endpointIndex @ node 0x%016llX", nodeID.unsignedLongLongValue); + } } } - // Ensure node index exists + // Check if node index needs updating / creation NSArray * nodeIndex = [self _fetchNodeIndex]; - BOOL storeFailed = NO; + NSArray * nodeIndexToStore = nil; if (!nodeIndex) { - nodeIndex = [NSArray arrayWithObject:nodeID]; - storeFailed = ![self _storeNodeIndex:nodeIndex]; + // Ensure node index exists + nodeIndexToStore = [NSArray arrayWithObject:nodeID]; } else if (![nodeIndex containsObject:nodeID]) { - storeFailed = ![self _storeNodeIndex:[nodeIndex arrayByAddingObject:nodeID]]; + nodeIndexToStore = [nodeIndex arrayByAddingObject:nodeID]; } - if (storeFailed) { - storeFailures++; - MTR_LOG_INFO("Store failed for nodeIndex"); + + if (nodeIndexToStore) { + if (bulkValuesToStore) { + bulkValuesToStore[sAttributeCacheNodeIndexKey] = nodeIndexToStore; + } else { + BOOL storeFailed = ![self _storeNodeIndex:nodeIndexToStore]; + if (storeFailed) { + storeFailures++; + MTR_LOG_INFO("Store failed for nodeIndex"); + } + } + } + + if (bulkValuesToStore) { + BOOL storeFailed = ![self _bulkStoreAttributeCacheValues:bulkValuesToStore]; + if (storeFailed) { + storeFailures++; + MTR_LOG_INFO("Store failed for bulk values count %lu", static_cast(bulkValuesToStore.count)); + } } // In the rare event that store fails, allow all attribute store attempts to go through and prune empty branches at the end altogether. diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerLocalTestStorage.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerLocalTestStorage.mm index 9ccd651760a47d..f51dae7bbbcaff 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerLocalTestStorage.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerLocalTestStorage.mm @@ -42,6 +42,7 @@ + (void)setLocalTestStorageEnabled:(BOOL)localTestStorageEnabled } } +// TODO: Add another init argument for controller so that this can support multiple-controllers. - (instancetype)initWithPassThroughStorage:(id)passThroughStorage { if (self = [super init]) { @@ -79,8 +80,12 @@ - (BOOL)controller:(MTRDeviceController *)controller sharingType:(MTRStorageSharingType)sharingType { if (sharingType == MTRStorageSharingTypeNotShared) { - NSError * error; + NSError * error = nil; NSData * data = [NSKeyedArchiver archivedDataWithRootObject:value requiringSecureCoding:YES error:&error]; + if (error) { + MTR_LOG_INFO("MTRDeviceControllerLocalTestStorage storeValue: failed to convert value object to data %@", error); + return NO; + } NSUserDefaults * defaults = [[NSUserDefaults alloc] initWithSuiteName:kLocalTestUserDefaultDomain]; [defaults setObject:data forKey:key]; return YES; @@ -112,4 +117,45 @@ - (BOOL)controller:(MTRDeviceController *)controller } } } + +- (NSDictionary> *)valuesForController:(MTRDeviceController *)controller securityLevel:(MTRStorageSecurityLevel)securityLevel sharingType:(MTRStorageSharingType)sharingType +{ + if (sharingType == MTRStorageSharingTypeNotShared) { + NSUserDefaults * defaults = [[NSUserDefaults alloc] initWithSuiteName:kLocalTestUserDefaultDomain]; + return [defaults dictionaryRepresentation]; + } else { + if (_passThroughStorage && [_passThroughStorage respondsToSelector:@selector(valuesForController:securityLevel:sharingType:)]) { + return [_passThroughStorage valuesForController:controller securityLevel:securityLevel sharingType:sharingType]; + } else { + MTR_LOG_INFO("MTRDeviceControllerLocalTestStorage valuesForController: shared type but no pass-through storage"); + return nil; + } + } +} + +- (BOOL)controller:(MTRDeviceController *)controller storeValues:(NSDictionary> *)values securityLevel:(MTRStorageSecurityLevel)securityLevel sharingType:(MTRStorageSharingType)sharingType +{ + if (sharingType == MTRStorageSharingTypeNotShared) { + NSUserDefaults * defaults = [[NSUserDefaults alloc] initWithSuiteName:kLocalTestUserDefaultDomain]; + BOOL success = YES; + for (NSString * key in values) { + NSError * error = nil; + NSData * data = [NSKeyedArchiver archivedDataWithRootObject:values[key] requiringSecureCoding:YES error:&error]; + if (error) { + MTR_LOG_INFO("MTRDeviceControllerLocalTestStorage storeValues: failed to convert value object to data %@", error); + success = NO; + continue; + } + [defaults setObject:data forKey:key]; + } + return success; + } else { + if (_passThroughStorage && [_passThroughStorage respondsToSelector:@selector(controller:storeValues:securityLevel:sharingType:)]) { + return [_passThroughStorage controller:controller storeValues:values securityLevel:securityLevel sharingType:sharingType]; + } else { + MTR_LOG_INFO("MTRDeviceControllerLocalTestStorage valuesForController: shared type but no pass-through storage"); + return NO; + } + } +} @end diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h b/src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h index bdf9bdb1a44c56..3e4e0575e56834 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h @@ -47,7 +47,7 @@ typedef NS_ENUM(NSUInteger, MTRStorageSharingType) { /** * Protocol for storing and retrieving controller-specific data. * - * Implementations of this protocol MUST keep two things in mind: + * Implementations of this protocol MUST keep these things in mind: * * 1) The controller provided to the delegate methods may not be fully * initialized when the callbacks are called. The only safe thing to do with @@ -60,6 +60,10 @@ typedef NS_ENUM(NSUInteger, MTRStorageSharingType) { * this delegate, apart from de-serializing and serializing the items being * stored and calling MTRDeviceControllerStorageClasses(), is likely to lead * to deadlocks. + * + * 3) Security level and sharing type will always be the same for any given key value + * and are provided to describe the data should the storage delegate choose to + * implement separating storage location by security level and sharing type. */ MTR_NEWLY_AVAILABLE @protocol MTRDeviceControllerStorageDelegate @@ -68,10 +72,6 @@ MTR_NEWLY_AVAILABLE * Return the stored value for the given key, if any, for the provided * controller. Returns nil if there is no stored value. * - * securityLevel and dataType will always be the same for any given key value - * and are just present here to help locate the data if storage location is - * separated out by security level and data type. - * * The set of classes that might be decoded by this function is available by * calling MTRDeviceControllerStorageClasses(). */ @@ -82,9 +82,6 @@ MTR_NEWLY_AVAILABLE /** * Store a value for the given key. Returns whether the store succeeded. - * - * securityLevel and dataType will always be the same for any given key value - * and are present here as a hint to how the value should be stored. */ - (BOOL)controller:(MTRDeviceController *)controller storeValue:(id)value @@ -94,15 +91,38 @@ MTR_NEWLY_AVAILABLE /** * Remove the stored value for the given key. Returns whether the remove succeeded. - * - * securityLevel and dataType will always be the same for any given key value - * and are just present here to help locate the data if storage location is - * separated out by security level and data type. */ - (BOOL)controller:(MTRDeviceController *)controller removeValueForKey:(NSString *)key securityLevel:(MTRStorageSecurityLevel)securityLevel sharingType:(MTRStorageSharingType)sharingType; + +@optional +/** + * Return all keys and values stored, if any, for the provided controller, in a + * dictionary. Returns nil if there are no stored values. + * + * securityLevel and sharingType are provided as a hint for the storage delegate + * to load from the right security level and sharing type, if the implementation + * stores them separately. If the implementation includes key/value pairs from other + * security levels or sharing types, they will be ignored by the caller. + * + * The set of classes that might be decoded by this function is available by + * calling MTRDeviceControllerStorageClasses(). + */ +- (nullable NSDictionary> *)valuesForController:(MTRDeviceController *)controller + securityLevel:(MTRStorageSecurityLevel)securityLevel + sharingType:(MTRStorageSharingType)sharingType; + +/** + * Store a list of key/value pairs in the form of a dictionary. Returns whether + * the store succeeded. Specifically, if any keys in this dictionary fail to store, + * the storage delegate should return NO. + */ +- (BOOL)controller:(MTRDeviceController *)controller + storeValues:(NSDictionary> *)values + securityLevel:(MTRStorageSecurityLevel)securityLevel + sharingType:(MTRStorageSharingType)sharingType; @end MTR_EXTERN MTR_NEWLY_AVAILABLE diff --git a/src/darwin/Framework/CHIP/MTRDevice_Internal.h b/src/darwin/Framework/CHIP/MTRDevice_Internal.h index 1444e602bd5247..28833060ba6c27 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDevice_Internal.h @@ -90,6 +90,10 @@ MTR_TESTABLE // Currently contains data version information - (void)setClusterData:(NSDictionary *)clusterData; +#ifdef DEBUG +- (NSUInteger)unitTestAttributeCount; +#endif + @end #pragma mark - Utility for clamping numbers diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m index 3355790c584e83..e652d2f9fddc0e 100644 --- a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m +++ b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m @@ -1035,6 +1035,23 @@ - (void)test007_TestMultipleControllers XCTAssertFalse([controller3 isRunning]); } +- (BOOL)_array:(NSArray *)one containsSameElementsAsArray:(NSArray *)other +{ + for (id object in one) { + if (![other containsObject:object]) { + return NO; + } + } + + for (id object in other) { + if (![one containsObject:object]) { + return NO; + } + } + + return YES; +} + - (void)test008_TestDataStoreDirect { __auto_type * factory = [MTRDeviceControllerFactory sharedInstance]; @@ -1046,7 +1063,7 @@ - (void)test008_TestDataStoreDirect __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); - __auto_type * storageDelegate = [[MTRTestPerControllerStorage alloc] initWithControllerID:[NSUUID UUID]]; + __auto_type * storageDelegate = [[MTRTestPerControllerStorageWithBulkReadWrite alloc] initWithControllerID:[NSUUID UUID]]; NSNumber * nodeID = @(123); NSNumber * fabricID = @(456); @@ -1103,6 +1120,7 @@ - (void)test008_TestDataStoreDirect XCTAssertEqual(dataStoreValues.count, 9); // Check values + NSUInteger unexpectedValues = 0; for (NSDictionary * responseValue in dataStoreValues) { MTRAttributePath * path = responseValue[MTRAttributePathKey]; XCTAssertNotNil(path); @@ -1132,8 +1150,11 @@ - (void)test008_TestDataStoreDirect XCTAssertEqualObjects(value, @(212)); } else if ([path.endpoint isEqualToNumber:@(2)] && [path.cluster isEqualToNumber:@(1)] && [path.attribute isEqualToNumber:@(3)]) { XCTAssertEqualObjects(value, @(213)); + } else { + unexpectedValues++; } } + XCTAssertEqual(unexpectedValues, 0); NSDictionary * dataStoreClusterData = [controller.controllerDataStore getStoredClusterDataForNodeID:@(1001)]; for (MTRClusterPath * path in testClusterData) { @@ -1188,11 +1209,100 @@ - (void)test008_TestDataStoreDirect id testNodeIndex = [storageDelegate controller:controller valueForKey:@"attrCacheNodeIndex" securityLevel:MTRStorageSecurityLevelSecure sharingType:MTRStorageSharingTypeNotShared]; XCTAssertNil(testNodeIndex); + // Now test bulk write + MTRClusterPath * bulkTestClusterPath11 = [MTRClusterPath clusterPathWithEndpointID:@(1) clusterID:@(1)]; + MTRDeviceClusterData * bulkTestClusterData11 = [[MTRDeviceClusterData alloc] init]; + bulkTestClusterData11.dataVersion = @(11); + bulkTestClusterData11.attributes = @{ + @(1) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(111) }, + @(2) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(112) }, + @(3) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(113) }, + }; + MTRClusterPath * bulkTestClusterPath12 = [MTRClusterPath clusterPathWithEndpointID:@(1) clusterID:@(2)]; + MTRDeviceClusterData * bulkTestClusterData12 = [[MTRDeviceClusterData alloc] init]; + bulkTestClusterData12.dataVersion = @(12); + bulkTestClusterData12.attributes = @{ + @(1) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(121) }, + @(2) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(122) }, + @(3) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(123) }, + }; + MTRClusterPath * bulkTestClusterPath13 = [MTRClusterPath clusterPathWithEndpointID:@(1) clusterID:@(3)]; + MTRDeviceClusterData * bulkTestClusterData13 = [[MTRDeviceClusterData alloc] init]; + bulkTestClusterData13.dataVersion = @(13); + bulkTestClusterData13.attributes = @{ + @(1) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(131) }, + @(2) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(132) }, + @(3) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(133) }, + }; + MTRClusterPath * bulkTestClusterPath21 = [MTRClusterPath clusterPathWithEndpointID:@(2) clusterID:@(1)]; + MTRDeviceClusterData * bulkTestClusterData21 = [[MTRDeviceClusterData alloc] init]; + bulkTestClusterData21.dataVersion = @(21); + bulkTestClusterData21.attributes = @{ + @(1) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(211) }, + @(2) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(212) }, + @(3) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(213) }, + }; + MTRClusterPath * bulkTestClusterPath22 = [MTRClusterPath clusterPathWithEndpointID:@(2) clusterID:@(2)]; + MTRDeviceClusterData * bulkTestClusterData22 = [[MTRDeviceClusterData alloc] init]; + bulkTestClusterData22.dataVersion = @(22); + bulkTestClusterData22.attributes = @{ + @(1) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(221) }, + @(2) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(222) }, + @(3) : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(223) }, + }; + NSDictionary * bulkTestClusterDataDictionary = @{ + bulkTestClusterPath11 : bulkTestClusterData11, + bulkTestClusterPath12 : bulkTestClusterData12, + bulkTestClusterPath13 : bulkTestClusterData13, + bulkTestClusterPath21 : bulkTestClusterData21, + bulkTestClusterPath22 : bulkTestClusterData22, + }; + + // Manually construct what the total dictionary should look like + NSDictionary> * testBulkValues = @{ + @"attrCacheNodeIndex" : @[ @(3001) ], + [controller.controllerDataStore _endpointIndexKeyForNodeID:@(3001)] : @[ @(1), @(2) ], + [controller.controllerDataStore _clusterIndexKeyForNodeID:@(3001) endpointID:@(1)] : @[ @(1), @(2), @(3) ], + [controller.controllerDataStore _clusterIndexKeyForNodeID:@(3001) endpointID:@(2)] : @[ @(1), @(2) ], + [controller.controllerDataStore _clusterDataKeyForNodeID:@(3001) endpointID:@(1) clusterID:@(1)] : bulkTestClusterData11, + [controller.controllerDataStore _clusterDataKeyForNodeID:@(3001) endpointID:@(1) clusterID:@(2)] : bulkTestClusterData12, + [controller.controllerDataStore _clusterDataKeyForNodeID:@(3001) endpointID:@(1) clusterID:@(3)] : bulkTestClusterData13, + [controller.controllerDataStore _clusterDataKeyForNodeID:@(3001) endpointID:@(2) clusterID:@(1)] : bulkTestClusterData21, + [controller.controllerDataStore _clusterDataKeyForNodeID:@(3001) endpointID:@(2) clusterID:@(2)] : bulkTestClusterData22, + }; + // Bulk store with delegate + dispatch_sync(_storageQueue, ^{ + [storageDelegate controller:controller storeValues:testBulkValues securityLevel:MTRStorageSecurityLevelSecure sharingType:MTRStorageSharingTypeNotShared]; + }); + // Verify that the store resulted in the correct values + dataStoreClusterData = [controller.controllerDataStore getStoredClusterDataForNodeID:@(3001)]; + XCTAssertEqualObjects(dataStoreClusterData, bulkTestClusterDataDictionary); + + // clear information before the next test + [controller.controllerDataStore clearStoredAttributesForNodeID:@(3001)]; + + // Now test bulk store through data store + [controller.controllerDataStore storeClusterData:bulkTestClusterDataDictionary forNodeID:@(3001)]; + dataStoreClusterData = [controller.controllerDataStore getStoredClusterDataForNodeID:@(3001)]; + XCTAssertEqualObjects(dataStoreClusterData, bulkTestClusterDataDictionary); + + // Now test bulk read directly from storage delegate + NSDictionary> * dataStoreBulkValues = [storageDelegate valuesForController:controller securityLevel:MTRStorageSecurityLevelSecure sharingType:MTRStorageSharingTypeNotShared]; + // Due to dictionary enumeration in storeClusterData:forNodeID:, the elements could be stored in a different order, but still be valid and equivalent + XCTAssertTrue(([self _array:(NSArray *) dataStoreBulkValues[[controller.controllerDataStore _endpointIndexKeyForNodeID:@(3001)]] containsSameElementsAsArray:@[ @(1), @(2) ]])); + XCTAssertTrue(([self _array:(NSArray *) dataStoreBulkValues[[controller.controllerDataStore _clusterIndexKeyForNodeID:@(3001) endpointID:@(1)]] containsSameElementsAsArray:@[ @(1), @(2), @(3) ]])); + XCTAssertTrue(([self _array:(NSArray *) dataStoreBulkValues[[controller.controllerDataStore _clusterIndexKeyForNodeID:@(3001) endpointID:@(2)]] containsSameElementsAsArray:@[ @(1), @(2) ]])); + XCTAssertEqualObjects(dataStoreBulkValues[[controller.controllerDataStore _clusterDataKeyForNodeID:@(3001) endpointID:@(1) clusterID:@(1)]], bulkTestClusterData11); + XCTAssertEqualObjects(dataStoreBulkValues[[controller.controllerDataStore _clusterDataKeyForNodeID:@(3001) endpointID:@(1) clusterID:@(2)]], bulkTestClusterData12); + XCTAssertEqualObjects(dataStoreBulkValues[[controller.controllerDataStore _clusterDataKeyForNodeID:@(3001) endpointID:@(1) clusterID:@(3)]], bulkTestClusterData13); + XCTAssertEqualObjects(dataStoreBulkValues[[controller.controllerDataStore _clusterDataKeyForNodeID:@(3001) endpointID:@(2) clusterID:@(1)]], bulkTestClusterData21); + XCTAssertEqualObjects(dataStoreBulkValues[[controller.controllerDataStore _clusterDataKeyForNodeID:@(3001) endpointID:@(2) clusterID:@(2)]], bulkTestClusterData22); + [controller shutdown]; XCTAssertFalse([controller isRunning]); } -- (void)test009_TestDataStoreMTRDevice +- (void)doDataStoreMTRDeviceTestWithStorageDelegate:(id)storageDelegate { __auto_type * factory = [MTRDeviceControllerFactory sharedInstance]; XCTAssertNotNil(factory); @@ -1205,8 +1315,6 @@ - (void)test009_TestDataStoreMTRDevice __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); - __auto_type * storageDelegate = [[MTRTestPerControllerStorage alloc] initWithControllerID:[NSUUID UUID]]; - NSNumber * nodeID = @(123); NSNumber * fabricID = @(456); @@ -1311,7 +1419,7 @@ - (void)test009_TestDataStoreMTRDevice double storedAttributeDifferFromMTRDevicePercentage = storedAttributeDifferFromMTRDeviceCount * 100.0 / dataStoreValuesCount; XCTAssertTrue(storedAttributeDifferFromMTRDevicePercentage < 10.0); - // Now + // Now set up new delegate for the new device and verify that once subscription reestablishes, the data version filter loaded from storage will work __auto_type * newDelegate = [[MTRDeviceTestDelegate alloc] init]; XCTestExpectation * newDeviceSubscriptionExpectation = [self expectationWithDescription:@"Subscription has been set up for new device"]; @@ -1340,6 +1448,61 @@ - (void)test009_TestDataStoreMTRDevice XCTAssertFalse([controller isRunning]); } +- (void)test009_TestDataStoreMTRDevice +{ + [self doDataStoreMTRDeviceTestWithStorageDelegate:[[MTRTestPerControllerStorage alloc] initWithControllerID:[NSUUID UUID]]]; +} + +- (void)test010_TestDataStoreMTRDeviceWithBulkReadWrite +{ + __auto_type * storageDelegate = [[MTRTestPerControllerStorageWithBulkReadWrite alloc] initWithControllerID:[NSUUID UUID]]; + + // First do the same test as the above + [self doDataStoreMTRDeviceTestWithStorageDelegate:storageDelegate]; + + // Then restart controller with same storage and see that bulk read through MTRDevice initialization works + + __auto_type * factory = [MTRDeviceControllerFactory sharedInstance]; + XCTAssertNotNil(factory); + + __auto_type * rootKeys = [[MTRTestKeys alloc] init]; + XCTAssertNotNil(rootKeys); + + __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; + XCTAssertNotNil(operationalKeys); + + NSNumber * nodeID = @(123); + NSNumber * fabricID = @(456); + + NSError * error; + + MTRPerControllerStorageTestsCertificateIssuer * certificateIssuer; + MTRDeviceController * controller = [self startControllerWithRootKeys:rootKeys + operationalKeys:operationalKeys + fabricID:fabricID + nodeID:nodeID + storage:storageDelegate + error:&error + certificateIssuer:&certificateIssuer]; + XCTAssertNil(error); + XCTAssertNotNil(controller); + XCTAssertTrue([controller isRunning]); + + XCTAssertEqualObjects(controller.controllerNodeID, nodeID); + + // No need to commission device - just look at device count + NSDictionary * deviceAttributeCounts = [controller unitTestGetDeviceAttributeCounts]; + XCTAssertTrue(deviceAttributeCounts.count > 0); + NSUInteger totalAttributes = 0; + for (NSNumber * nodeID in deviceAttributeCounts) { + totalAttributes += deviceAttributeCounts[nodeID].unsignedIntegerValue; + } + XCTAssertTrue(totalAttributes > 300); + + [controller shutdown]; + XCTAssertFalse([controller isRunning]); +} + // TODO: This might want to go in a separate test file, with some shared setup // across multiple tests, maybe. Would need to factor out // startControllerWithRootKeys into a test helper. diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h index 68abb9a8fa319f..b3ef0327cb1bea 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h @@ -34,6 +34,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)unitTestPruneEmptyStoredAttributesBranches; - (NSString *)_endpointIndexKeyForNodeID:(NSNumber *)nodeID; - (NSString *)_clusterIndexKeyForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID; +- (NSString *)_clusterDataKeyForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID; - (NSString *)_attributeIndexKeyForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID; - (NSString *)_attributeValueKeyForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID attributeID:(NSNumber *)attributeID; @end @@ -52,6 +53,10 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - Declarations for items compiled only for DEBUG configuration #ifdef DEBUG +@interface MTRDeviceController (TestDebug) +- (NSDictionary *)unitTestGetDeviceAttributeCounts; +@end + @interface MTRBaseDevice (TestDebug) - (void)failSubscribers:(dispatch_queue_t)queue completion:(void (^)(void))completion; diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestPerControllerStorage.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestPerControllerStorage.h index b3052a9929a1af..b02bff9c01a46b 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestPerControllerStorage.h +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestPerControllerStorage.h @@ -40,4 +40,15 @@ NS_ASSUME_NONNULL_BEGIN sharingType:(MTRStorageSharingType)sharingType; @end +@interface MTRTestPerControllerStorageWithBulkReadWrite : MTRTestPerControllerStorage +- (nullable NSDictionary> *)valuesForController:(MTRDeviceController *)controller + securityLevel:(MTRStorageSecurityLevel)securityLevel + sharingType:(MTRStorageSharingType)sharingType; +- (BOOL)controller:(MTRDeviceController *)controller + storeValues:(NSDictionary> *)values + securityLevel:(MTRStorageSecurityLevel)securityLevel + sharingType:(MTRStorageSharingType)sharingType; + +@end + NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestPerControllerStorage.m b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestPerControllerStorage.m index 1898bd14832cf9..f0bce87bc47af1 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestPerControllerStorage.m +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestPerControllerStorage.m @@ -83,3 +83,34 @@ - (BOOL)controller:(MTRDeviceController *)controller } @end + +@implementation MTRTestPerControllerStorageWithBulkReadWrite + +- (NSDictionary> *)valuesForController:(MTRDeviceController *)controller securityLevel:(MTRStorageSecurityLevel)securityLevel sharingType:(MTRStorageSharingType)sharingType +{ + XCTAssertEqualObjects(self.controllerID, controller.uniqueIdentifier); + + if (!self.storage.count) { + return nil; + } + + NSMutableDictionary * valuesToReturn = [NSMutableDictionary dictionary]; + for (NSString * key in self.storage) { + valuesToReturn[key] = [self controller:controller valueForKey:key securityLevel:securityLevel sharingType:sharingType]; + } + + return valuesToReturn; +} + +- (BOOL)controller:(MTRDeviceController *)controller storeValues:(NSDictionary> *)values securityLevel:(MTRStorageSecurityLevel)securityLevel sharingType:(MTRStorageSharingType)sharingType +{ + XCTAssertEqualObjects(self.controllerID, controller.uniqueIdentifier); + + for (NSString * key in values) { + [self controller:controller storeValue:values[key] forKey:key securityLevel:securityLevel sharingType:sharingType]; + } + + return YES; +} + +@end