Add a way to forget a node ID to Matter.framework. (project-chip#37198)

The intent is to tear down any MTRDevice connection to the node and remove any stored data for the node ID.
espressif · Jan 24, 2025 · fd216ec · fd216ec
1 parent 12235cd
commit fd216ec
Show file tree

Hide file tree

Showing 9 changed files with 109 additions and 13 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.h b/src/darwin/Framework/CHIP/MTRDeviceController.h
@@ -269,6 +269,12 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
  */
 - (void)removeServerEndpoint:(MTRServerEndpoint *)endpoint MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6));
 
+/**
+ * Forget any information we have about the device with the given node ID.  That
+ * includes clearing any information we have stored about it.
+ */
+- (void)forgetDeviceWithNodeID:(NSNumber *)nodeID MTR_NEWLY_AVAILABLE;
+
 /**
  * Compute a PASE verifier for the desired setup passcode.
  *

diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -386,6 +386,18 @@ - (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
     return [self _deviceForNodeID:nodeID createIfNeeded:YES];
 }
 
+- (void)forgetDeviceWithNodeID:(NSNumber *)nodeID
+{
+    MTRDevice * deviceToRemove;
+    {
+        std::lock_guard lock(*self.deviceMapLock);
+        deviceToRemove = [_nodeIDToDeviceMap objectForKey:nodeID];
+    }
+    if (deviceToRemove != nil) {
+        [self removeDevice:deviceToRemove];
+    }
+}
+
 - (void)removeDevice:(MTRDevice *)device
 {
     std::lock_guard lock(*self.deviceMapLock);

diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h
@@ -61,6 +61,7 @@ typedef void (^MTRDeviceControllerDataStoreClusterDataHandler)(NSDictionary<NSNu
 - (nullable MTRCASESessionResumptionInfo *)findResumptionInfoByNodeID:(NSNumber *)nodeID;
 - (nullable MTRCASESessionResumptionInfo *)findResumptionInfoByResumptionID:(NSData *)resumptionID;
 - (void)storeResumptionInfo:(MTRCASESessionResumptionInfo *)resumptionInfo;
+- (void)clearResumptionInfoForNodeID:(NSNumber *)nodeID;
 - (void)clearAllResumptionInfo;
 
 /**
@@ -92,6 +93,7 @@ typedef void (^MTRDeviceControllerDataStoreClusterDataHandler)(NSDictionary<NSNu
  */
 - (nullable NSDictionary<NSString *, id> *)getStoredDeviceDataForNodeID:(NSNumber *)nodeID;
 - (void)storeDeviceData:(NSDictionary<NSString *, id> *)data forNodeID:(NSNumber *)nodeID;
+- (void)clearDeviceDataForNodeID:(NSNumber *)nodeID;
 
 /**
  * Mechanism for an API client to perform a block after previous async operations (writes) on the storage queue have executed.

diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm
@@ -228,24 +228,38 @@ - (void)clearAllResumptionInfo
     // Can we do less dispatch?  We would need to have a version of
     // _findResumptionInfoWithKey that assumes we are already on the right queue.
     for (NSNumber * nodeID in _nodesWithResumptionInfo) {
-        auto * oldInfo = [self findResumptionInfoByNodeID:nodeID];
-        if (oldInfo != nil) {
-            dispatch_sync(_storageDelegateQueue, ^{
-                [_storageDelegate controller:controller
-                           removeValueForKey:ResumptionByResumptionIDKey(oldInfo.resumptionID)
-                               securityLevel:MTRStorageSecurityLevelSecure
-                                 sharingType:MTRStorageSharingTypeNotShared];
-                [_storageDelegate controller:controller
-                           removeValueForKey:ResumptionByNodeIDKey(oldInfo.nodeID)
-                               securityLevel:MTRStorageSecurityLevelSecure
-                                 sharingType:MTRStorageSharingTypeNotShared];
-            });
-        }
+        [self _clearResumptionInfoForNodeID:nodeID controller:controller];
     }
 
     [_nodesWithResumptionInfo removeAllObjects];
 }
 
+- (void)clearResumptionInfoForNodeID:(NSNumber *)nodeID
+{
+    MTRDeviceController * controller = _controller;
+    VerifyOrReturn(controller != nil); // No way to call delegate without controller.
+
+    [self _clearResumptionInfoForNodeID:nodeID controller:controller];
+    [_nodesWithResumptionInfo removeObject:nodeID];
+}
+
+- (void)_clearResumptionInfoForNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
+{
+    auto * oldInfo = [self findResumptionInfoByNodeID:nodeID];
+    if (oldInfo != nil) {
+        dispatch_sync(_storageDelegateQueue, ^{
+            [_storageDelegate controller:controller
+                       removeValueForKey:ResumptionByResumptionIDKey(oldInfo.resumptionID)
+                           securityLevel:MTRStorageSecurityLevelSecure
+                             sharingType:MTRStorageSharingTypeNotShared];
+            [_storageDelegate controller:controller
+                       removeValueForKey:ResumptionByNodeIDKey(oldInfo.nodeID)
+                           securityLevel:MTRStorageSecurityLevelSecure
+                             sharingType:MTRStorageSharingTypeNotShared];
+        });
+    }
+}
+
 - (CHIP_ERROR)storeLastLocallyUsedNOC:(MTRCertificateTLVBytes)noc
 {
     MTRDeviceController * controller = _controller;
@@ -1169,6 +1183,20 @@ - (void)storeDeviceData:(NSDictionary<NSString *, id> *)data forNodeID:(NSNumber
     });
 }
 
+- (void)clearDeviceDataForNodeID:(NSNumber *)nodeID
+{
+    dispatch_async(_storageDelegateQueue, ^{
+        MTRDeviceController * controller = self->_controller;
+        VerifyOrReturn(controller != nil); // No way to call delegate without controller.
+
+        // Ignore store failures, since they are not actionable for us here.
+        [self->_storageDelegate controller:controller
+                         removeValueForKey:[self _deviceDataKeyForNodeID:nodeID]
+                             securityLevel:MTRStorageSecurityLevelSecure
+                               sharingType:MTRStorageSharingTypeNotShared];
+    });
+}
+
 - (void)synchronouslyPerformBlock:(void (^_Nullable)(void))block
 {
     dispatch_sync(_storageDelegateQueue, ^{

diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm
@@ -1193,6 +1193,21 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N
     return deviceToReturn;
 }
 
+- (void)forgetDeviceWithNodeID:(NSNumber *)nodeID
+{
+    MTR_LOG("%@: Forgetting device with node ID: %@", self, nodeID);
+
+    // Tear down any existing MTRDevice for this nodeID first, so we don't run
+    // into issues with it storing data after we have deleted it.
+    [super forgetDeviceWithNodeID:nodeID];
+
+    if (_controllerDataStore) {
+        [_controllerDataStore clearResumptionInfoForNodeID:nodeID];
+        [_controllerDataStore clearDeviceDataForNodeID:nodeID];
+        [_controllerDataStore clearStoredClusterDataForNodeID:nodeID];
+    }
+}
+
 #ifdef DEBUG
 - (NSDictionary<NSNumber *, NSNumber *> *)unitTestGetDeviceAttributeCounts
 {

diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm b/src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm
@@ -60,6 +60,10 @@ @implementation MTRDeviceController_XPC
                                                : (NSDictionary *) controllerState, updateControllerConfiguration
                                                : (NSDictionary *) controllerState)
 
+MTR_DEVICECONTROLLER_SIMPLE_REMOTE_XPC_COMMAND(deleteNodeID
+                                               : (NSNumber *) nodeID, deleteNodeID
+                                               : (NSNumber *) nodeID)
+
 - (void)_updateRegistrationInfo
 {
     NSMutableDictionary * registrationInfo = [NSMutableDictionary dictionary];
@@ -95,6 +99,13 @@ - (void)removeDevice:(MTRDevice *)device
     [self _updateRegistrationInfo];
 }
 
+- (void)forgetDeviceWithNodeID:(NSNumber *)nodeID
+{
+    MTR_LOG("%@: Forgetting device with node ID: %@", self, nodeID);
+    [self deleteNodeID:nodeID];
+    [super forgetDeviceWithNodeID:nodeID];
+}
+
 #pragma mark - XPC
 @synthesize controllerNodeID = _controllerNodeID;
 @synthesize compressedFabricID = _compressedFabricID;

diff --git a/src/darwin/Framework/CHIP/XPC Protocol/MTRXPCServerProtocol.h b/src/darwin/Framework/CHIP/XPC Protocol/MTRXPCServerProtocol.h
@@ -71,6 +71,8 @@ MTR_AVAILABLE(ios(18.2), macos(15.2), watchos(11.2), tvos(18.2))
 // - (oneway void)deviceController:(NSUUID *)controller addServerEndpoint:(MTRServerEndpoint *)endpoint withReply:(void (^)(BOOL success))reply;
 // - (oneway void)deviceController:(NSUUID *)controller removeServerEndpoint:(MTRServerEndpoint *)endpoint;
 
+- (oneway void)deviceController:(NSUUID *)controller deleteNodeID:(NSNumber *)nodeID MTR_NEWLY_AVAILABLE;
+
 - (oneway void)deviceController:(NSUUID *)controller registerNodeID:(NSNumber *)nodeID;
 - (oneway void)deviceController:(NSUUID *)controller unregisterNodeID:(NSNumber *)nodeID;
 - (oneway void)deviceController:(NSUUID *)controller updateControllerConfiguration:(NSDictionary *)controllerState;

diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
@@ -1764,6 +1764,22 @@ - (void)test010_TestDataStoreMTRDeviceWithBulkReadWrite
     }
     XCTAssertTrue(totalAttributes > 300);
 
+    // Now try forgetting this device and make sure all the info we had for it
+    // goes away.
+    NSNumber * deviceID = @(17);
+    __auto_type * dataStore = controller.controllerDataStore;
+    XCTAssertNotNil(deviceAttributeCounts[deviceID]);
+    XCTAssertNotNil([dataStore findResumptionInfoByNodeID:deviceID]);
+    XCTAssertNotNil([dataStore getStoredDeviceDataForNodeID:deviceID]);
+    XCTAssertNotNil([dataStore getStoredClusterDataForNodeID:deviceID]);
+
+    [controller forgetDeviceWithNodeID:deviceID];
+    deviceAttributeCounts = [controller unitTestGetDeviceAttributeCounts];
+    XCTAssertNil(deviceAttributeCounts[deviceID]);
+    XCTAssertNil([dataStore findResumptionInfoByNodeID:deviceID]);
+    XCTAssertNil([dataStore getStoredDeviceDataForNodeID:deviceID]);
+    XCTAssertNil([dataStore getStoredClusterDataForNodeID:deviceID]);
+
     [controller shutdown];
     XCTAssertFalse([controller isRunning]);
 }

diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h
@@ -26,8 +26,11 @@ NS_ASSUME_NONNULL_BEGIN
 
 #pragma mark - Declarations for internal methods
 
+@class MTRCASESessionResumptionInfo;
+
 // MTRDeviceControllerDataStore.h includes C++ header, and so we need to declare the methods separately
 @protocol MTRDeviceControllerDataStoreAttributeStoreMethods
+- (nullable MTRCASESessionResumptionInfo *)findResumptionInfoByNodeID:(NSNumber *)nodeID;
 - (nullable NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)getStoredClusterDataForNodeID:(NSNumber *)nodeID;
 - (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData forNodeID:(NSNumber *)nodeID;
 - (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID;
@@ -39,6 +42,7 @@ NS_ASSUME_NONNULL_BEGIN
 - (nullable NSArray<NSNumber *> *)_fetchEndpointIndexForNodeID:(NSNumber *)nodeID;
 - (nullable NSArray<NSNumber *> *)_fetchClusterIndexForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID;
 - (nullable MTRDeviceClusterData *)_fetchClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID;
+- (nullable NSDictionary<NSString *, id> *)getStoredDeviceDataForNodeID:(NSNumber *)nodeID;
 @end
 
 // Declare internal methods for testing