Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Darwin] MTRDeviceControllerStorageDelegate should support bulk store/fetch #32858

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
272f01a
[Darwin] MTRDeviceControllerStorageDelegate should support bulk store…
jtung-apple Apr 4, 2024
d7df770
Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h
jtung-apple Apr 5, 2024
9631f2a
MTRDeviceControllerStorageDelegate header documentation clarification
jtung-apple Apr 5, 2024
ae5a3e1
Update src/darwin/Framework/CHIP/MTRDevice.mm
jtung-apple Apr 11, 2024
92aec71
Update src/darwin/Framework/CHIP/MTRDeviceController.mm
jtung-apple Apr 11, 2024
fc33a56
Update src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h
jtung-apple Apr 11, 2024
1c43330
Update src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
jtung-apple Apr 11, 2024
3fc44eb
Update src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
jtung-apple Apr 11, 2024
a6b25e5
Address review comments
jtung-apple Apr 12, 2024
ae6184e
Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h
jtung-apple Apr 12, 2024
63765ff
Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h
jtung-apple Apr 12, 2024
ed69505
Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h
jtung-apple Apr 12, 2024
ec9348c
Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h
jtung-apple Apr 12, 2024
2cf0860
Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h
jtung-apple Apr 12, 2024
a6ef665
Update src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h
jtung-apple Apr 12, 2024
5b689c3
Move processing out of storage delegate queue dispatch_sync
jtung-apple Apr 12, 2024
70116ac
Merge branch 'master' into jtung/darwin-storage-delegate-bulk-read-wr…
woody-apple Apr 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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 isKindOfClass:[MTRDeviceClusterData class]]) {
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
return NO;
}

return [self isEqualToClusterData:object];
}

@end

@interface MTRDevice ()
Expand Down Expand Up @@ -2241,6 +2255,14 @@ - (void)setAttributeValues:(NSArray<NSDictionary *> *)attributeValues reportChan
[self _setAttributeValues:attributeValues reportChanges:reportChanges];
}

#ifdef DEBUG
- (NSUInteger)unitTestAttributeCount
{
std::lock_guard lock(_lock);
return _readCache.count;
}
#endif

- (void)setClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData
{
MTR_LOG_INFO("%@ setClusterData count: %lu", self, static_cast<unsigned long>(clusterData.count));
Expand Down
28 changes: 28 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,22 @@ - (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 startUpWithClusterDataHandler:^(NSDictionary<NSNumber *, NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *> * _Nonnull clusterDataByNode) {
MTR_LOG_INFO("Loaded %lu nodes from storage", static_cast<unsigned long>(clusterDataByNode.count));
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
for (NSNumber * nodeID in clusterDataByNode) {
MTRDevice * device = [[MTRDevice alloc] initWithNodeID:nodeID controller:self];
NSDictionary * clusterData = clusterDataByNode[nodeID];
MTR_LOG_INFO("Loaded %lu cluster data from storage for %@", static_cast<unsigned long>(clusterData.count), device);
if (clusterData.count) {
[device setClusterData:clusterData];
}
self->_nodeIDToDeviceMap[nodeID] = device;
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
}
}];
}

return YES;
}

Expand Down Expand Up @@ -979,6 +995,18 @@ - (void)removeDevice:(MTRDevice *)device
}
}

#ifdef DEBUG
- (NSDictionary<NSNumber *, NSNumber *> *)unitTestGetDeviceAttributeCounts
{
std::lock_guard lock(_deviceMapLock);
NSMutableDictionary<NSNumber *, NSNumber *> * deviceAttributeCounts = [NSMutableDictionary dictionary];
for (NSNumber * nodeID in _nodeIDToDeviceMap) {
deviceAttributeCounts[nodeID] = @([_nodeIDToDeviceMap[nodeID] unitTestAttributeCount]);
}
return deviceAttributeCounts;
}
#endif

- (void)setDeviceControllerDelegate:(id<MTRDeviceControllerDelegate>)delegate queue:(dispatch_queue_t)queue
{
[self
Expand Down
7 changes: 7 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ NS_ASSUME_NONNULL_BEGIN
storageDelegate:(id<MTRDeviceControllerStorageDelegate>)storageDelegate
storageDelegateQueue:(dispatch_queue_t)storageDelegateQueue;

// clusterDataByNode a dictionary: nodeID => cluster data dictionary
typedef void (^MTRDeviceControllerDataStoreClusterDataHandler)(NSDictionary<NSNumber *, NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *> * clusterDataByNode);
/**
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
* Asks the data store to load cluster data for nodes in bulk, if the storageDelegate supports it.
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
*/
- (void)startUpWithClusterDataHandler:(MTRDeviceControllerDataStoreClusterDataHandler)clusterDataHandler;

/**
* Resumption info APIs.
*/
Expand Down
138 changes: 116 additions & 22 deletions src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,17 @@ - (nullable instancetype)initWithController:(MTRDeviceController *)controller
return self;
}

- (void)startUpWithClusterDataHandler:(MTRDeviceControllerDataStoreClusterDataHandler)clusterDataHandler
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
{
dispatch_sync(_storageDelegateQueue, ^{
if ([self->_storageDelegate respondsToSelector:@selector(valuesForController:securityLevel:sharingType:)]) {
NSDictionary<NSString *, id> * dataStoreSecureLocalValues = [self->_storageDelegate valuesForController:self->_controller securityLevel:MTRStorageSecurityLevelSecure sharingType:MTRStorageSharingTypeNotShared];

clusterDataHandler([self getClusterDataFromSecureLocalValues:dataStoreSecureLocalValues]);
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
}
});
}

- (nullable MTRCASESessionResumptionInfo *)findResumptionInfoByNodeID:(NSNumber *)nodeID
{
return [self _findResumptionInfoWithKey:ResumptionByNodeIDKey(nodeID)];
Expand Down Expand Up @@ -337,6 +348,14 @@ - (BOOL)_storeAttributeCacheValue:(id)value forKey:(NSString *)key
sharingType:MTRStorageSharingTypeNotShared];
}

- (BOOL)_bulkStoreAttributeCacheValues:(NSDictionary<NSString *, id<NSSecureCoding>> *)values
{
return [_storageDelegate controller:_controller
storeValues:values
securityLevel:MTRStorageSecurityLevelSecure
sharingType:MTRStorageSharingTypeNotShared];
}

- (BOOL)_removeAttributeCacheValueForKey:(NSString *)key
{
return [_storageDelegate controller:_controller
Expand Down Expand Up @@ -519,6 +538,8 @@ - (BOOL)_deleteAttributeValueForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *

#pragma - Attribute Cache management

#define ATTRIBUTE_CACHE_VERBOSE_LOGGING 1
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved

#ifndef ATTRIBUTE_CACHE_VERBOSE_LOGGING
#define ATTRIBUTE_CACHE_VERBOSE_LOGGING 0
#endif
Expand Down Expand Up @@ -968,6 +989,46 @@ - (void)clearAllStoredAttributes
return clusterDataToReturn;
}

// Utility for constructing dictionary of nodeID to cluster data from dictionary of storage keys
- (nullable NSDictionary<NSNumber *, NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *> *)getClusterDataFromSecureLocalValues:(NSDictionary<NSString *, id> *)secureLocalValues
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
{
NSMutableDictionary<NSNumber *, NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *> * clusterDataByNodeToReturn = nil;

// Fetch node index
NSArray<NSNumber *> * nodeIndex = secureLocalValues[sAttributeCacheNodeIndexKey];

for (NSNumber * nodeID in nodeIndex) {
NSMutableDictionary<MTRClusterPath *, MTRDeviceClusterData *> * clusterDataForNode = nil;
NSArray<NSNumber *> * endpointIndex = secureLocalValues[[self _endpointIndexKeyForNodeID:nodeID]];
for (NSNumber * endpointID in endpointIndex) {
NSArray<NSNumber *> * clusterIndex = secureLocalValues[[self _clusterIndexKeyForNodeID:nodeID endpointID:endpointID]];
for (NSNumber * clusterID in clusterIndex) {
MTRDeviceClusterData * clusterData = secureLocalValues[[self _clusterDataKeyForNodeID:nodeID endpointID:endpointID clusterID:clusterID]];
if (!clusterData) {
continue;
}
if (![clusterData isKindOfClass:[MTRDeviceClusterData class]]) {
continue;
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
}
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<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData forNodeID:(NSNumber *)nodeID
{
if (!nodeID) {
Expand All @@ -983,6 +1044,11 @@ - (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *>
dispatch_async(_storageDelegateQueue, ^{
NSUInteger storeFailures = 0;

NSMutableDictionary<NSString *, id<NSSecureCoding>> * 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<NSNumber *, NSMutableSet<NSNumber *> *> * clustersModified = [NSMutableDictionary dictionary];

Expand All @@ -993,11 +1059,15 @@ - (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *>
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
Expand Down Expand Up @@ -1046,36 +1116,60 @@ - (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *>
}

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<NSNumber *> * nodeIndex = [self _fetchNodeIndex];
BOOL storeFailed = NO;
NSArray<NSNumber *> * 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<unsigned long>(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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ + (void)setLocalTestStorageEnabled:(BOOL)localTestStorageEnabled
}
}

// TODO: Add another init argument for controller so that this can support multiple-controllers.
- (instancetype)initWithPassThroughStorage:(id<MTRDeviceControllerStorageDelegate>)passThroughStorage
{
if (self = [super init]) {
Expand Down Expand Up @@ -81,8 +82,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;
Expand Down Expand Up @@ -114,6 +119,47 @@ - (BOOL)controller:(MTRDeviceController *)controller
}
}
}

- (NSDictionary<NSString *, id<NSSecureCoding>> *)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<NSString *, id<NSSecureCoding>> *)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

#endif // MTR_PER_CONTROLLER_STORAGE_ENABLED
Loading
Loading