Add support to intercept attribute report and prune any removed endpoints and clusters and attributes from both persisted clusters data and data storage

nivi-apple · nivi-apple · commit 1b4c1885ecf6 · 2024-05-20T12:03:54.000-07:00
- When an attribute report is received, check if there are any changes to parts list or server list for the descriptor cluster or attribute list for any cluster.

- If any endpoints were removed, make sure to delete the cluster index for the endpoint and all cluster data for that endpoint should be deleted.

- If any clusters were removed from an endpoint, make sure to delete the cluster from the cluster index for that endpoint and also clear the cluster data for that cluster.

- If any attributes were removed from a cluster, make sure to update the cluster data and delete the entry for the attribute that was removed.
diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
@@ -220,6 +220,11 @@ - (void)storeValue:(MTRDeviceDataValueDictionary _Nullable)value forAttribute:(N
     _attributes[attribute] = value;
 }
 
+- (void)_removeValueForAttribute:(NSNumber *)attribute
+{
+    [_attributes removeObjectForKey:attribute];
+}
+
 - (NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> *)attributes
 {
     return _attributes;
@@ -1609,6 +1614,18 @@ - (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value f
     _clusterDataToPersist[clusterPath] = clusterData;
 }
 
+- (void)_removeCachedAttributeValue:(MTRClusterPath *)clusterPath forPath:(MTRAttributePath *)attributePath
+{
+    os_unfair_lock_assert_owner(&self->_lock);
+
+    if (_clusterDataToPersist == nil) {
+        return;
+    }
+    auto * clusterData = [_clusterDataToPersist objectForKey:clusterPath];
+    [clusterData _removeValueForAttribute:attributePath.attribute];
+    [_clusterDataToPersist setObject:clusterData forKey:clusterPath];
+}
+
 - (void)_createDataVersionFilterListFromDictionary:(NSDictionary<MTRClusterPath *, NSNumber *> *)dataVersions dataVersionFilterList:(DataVersionFilter **)dataVersionFilterList count:(size_t *)count sizeReduction:(size_t)sizeReduction
 {
     size_t maxDataVersionFilterSize = dataVersions.count;
@@ -2666,7 +2683,7 @@ - (BOOL)_attributeDataValue:(NSDictionary *)one isEqualToDataValue:(NSDictionary
 }
 
 // Utility to return data value dictionary without data version
-- (NSDictionary *)_dataValueWithoutDataVersion:(NSDictionary *)attributeValue;
+- (NSDictionary *)_dataValueWithoutDataVersion:(NSDictionary *)attributeValue
 {
     // Sanity check for nil - return the same input to fail gracefully
     if (!attributeValue || !attributeValue[MTRTypeKey]) {
@@ -2728,6 +2745,137 @@ - (BOOL)_attributeAffectsDeviceConfiguration:(MTRAttributePath *)attributePath
     return NO;
 }
 
+- (BOOL)_needsPruningOfEndpointsAndClusters:(MTRAttributePath *)attributePath
+{
+    // Check for attributes in the descriptor cluster that could cause removal of endpoints and clusters.
+    if (attributePath.cluster.unsignedLongValue == MTRClusterIDTypeDescriptorID) {
+        switch (attributePath.attribute.unsignedLongValue) {
+        case MTRAttributeIDTypeClusterDescriptorAttributePartsListID:
+        case MTRAttributeIDTypeClusterDescriptorAttributeServerListID:
+            return YES;
+        }
+    }
+
+    // Check for global attribute - attribute list that could cause removal of attributes.
+    switch (attributePath.attribute.unsignedLongValue) {
+    case MTRAttributeIDTypeGlobalAttributeAttributeListID:
+        return YES;
+    }
+    return NO;
+}
+
+- (void)_pruneOrphanedEndpointsAndClusters:(MTRAttributePath *)attributePath
+                             previousValue:(NSDictionary *)previousValue
+                        attributeDataValue:(NSDictionary *)attributeDataValue
+{
+    os_unfair_lock_assert_owner(&self->_lock);
+    
+    if (_persistedClusters == nil || _persistedClusterData == nil || !previousValue.count)
+    {
+        return;
+    }
+    // Check if parts list changed or server list changed for the descriptor cluster or the attribute list changed for a cluster.
+    // If yes, we might need to prune any deleted endpoints, clusters or attributes from the storage and persisted cluster data.
+    if (attributePath.cluster.unsignedLongValue == MTRClusterIDTypeDescriptorID) {
+        switch (attributePath.attribute.unsignedLongValue) {
+                
+            // If the parts list changed and one or more endpoints were removed, remove all the clusters in _persistedClusters and _persistedClusterData for all those endpoints.
+            // Also remove it from the data store.
+            case MTRAttributeIDTypeClusterDescriptorAttributePartsListID:
+            {
+                NSMutableSet * toBeRemovedEndpoints = [NSMutableSet setWithArray:[self arrayOfNumbersFromAttributeValue:previousValue]];
+                NSSet * endpointsOnDevice = [NSSet setWithArray:[self arrayOfNumbersFromAttributeValue:attributeDataValue]];
+                [toBeRemovedEndpoints minusSet:endpointsOnDevice];
+                
+                for (NSNumber * endpoint in toBeRemovedEndpoints)
+                {
+                    NSMutableSet<MTRClusterPath *> * clusterPathsToRemove = [[NSMutableSet alloc]init];
+                    for (MTRClusterPath * path in _persistedClusters)
+                    {
+                        if ([path.endpoint isEqualToNumber:endpoint])
+                        {
+                            [clusterPathsToRemove addObject:path];
+                            [_persistedClusterData removeObjectForKey:path];
+                            [self.deviceController.controllerDataStore clearStoredClusterDataForNodeIDWithEndpointID:self.nodeID endpointID:endpoint];
+                        }
+                    }
+                    [_persistedClusters minusSet:clusterPathsToRemove];
+                }
+                break;
+            }
+                
+            // If the server list changed and clusters were removed, remove the clusters from the _persistedClusters and _persistedClusterData for that endpoint
+            // Also remove it from the data store.
+            case MTRAttributeIDTypeClusterDescriptorAttributeServerListID:
+            {
+                NSMutableSet * toBeRemovedClusters= [NSMutableSet setWithArray:[self arrayOfNumbersFromAttributeValue:previousValue]];
+                NSSet * clustersOnDevice = [NSSet setWithArray:[self arrayOfNumbersFromAttributeValue:attributeDataValue]];
+                [toBeRemovedClusters minusSet:clustersOnDevice];
+                
+                NSMutableSet<MTRClusterPath *> * clusterPathsToRemove = [[NSMutableSet alloc]init];
+                for (NSNumber * cluster in toBeRemovedClusters)
+                {
+                    for (MTRClusterPath * path in _persistedClusters)
+                    {
+                        if ([path.endpoint isEqualToNumber:attributePath.endpoint] && [path.cluster isEqualToNumber:cluster])
+                        {
+                            [clusterPathsToRemove addObject:path];
+                            [_persistedClusterData removeObjectForKey:path];
+                            
+                            [self.deviceController.controllerDataStore clearStoredClusterDataForNodeIDWithClusterID:self.nodeID endpointID:path.endpoint clusterID:path.cluster];
+                        }
+                    }
+                }
+                [_persistedClusters minusSet:clusterPathsToRemove];
+                break;
+            }
+        }
+    }
+    
+    switch (attributePath.attribute.unsignedLongValue) {
+        // If the attribute list changed and attributes were removed, remove the attributes from the _persistedClusterData for that cluster and endpoint.
+        // Also remove it from the data store cluster data.
+        case MTRAttributeIDTypeGlobalAttributeAttributeListID:
+        {
+            NSMutableSet * toBeRemovedAttributes= [NSMutableSet setWithArray:[self arrayOfNumbersFromAttributeValue:[self _cachedAttributeValueForPath:attributePath]]];
+            NSSet * attributesOnDevice = [NSSet setWithArray:[self arrayOfNumbersFromAttributeValue:attributeDataValue]];
+            
+            [toBeRemovedAttributes minusSet:attributesOnDevice];
+            for (NSNumber * attribute in toBeRemovedAttributes)
+            {
+                for (MTRClusterPath * path in _persistedClusters)
+                {
+                    if ([path.endpoint isEqualToNumber:attributePath.endpoint] && [path.cluster isEqualToNumber:attributePath.cluster])
+                    {
+                        MTRDeviceClusterData * clusterData = [self _clusterDataForPath:path];
+                        if (clusterData == nil)
+                        {
+                            return;
+                        }
+                        [clusterData _removeValueForAttribute:attribute];
+                        [self->_persistedClusterData setObject:clusterData forKey:path];
+                        
+                        NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> * dataStoreClusterData = [self.deviceController.controllerDataStore getStoredClusterDataForNodeID:self.nodeID];
+                        NSMutableDictionary<MTRClusterPath *, MTRDeviceClusterData *> * dataStoreClusterDataCopy = [dataStoreClusterData mutableCopy];
+                        for (MTRClusterPath * dataStorePath in dataStoreClusterData)
+                        {
+                            if ([dataStorePath isEqualTo:path])
+                            {
+                                [dataStoreClusterDataCopy removeObjectForKey:path];
+                                [dataStoreClusterDataCopy setObject:clusterData forKey:path];
+                                [self.deviceController.controllerDataStore storeClusterData:dataStoreClusterDataCopy forNodeID:self.nodeID];
+                                dataStoreClusterData = [NSMutableDictionary dictionaryWithDictionary:[self.deviceController.controllerDataStore getStoredClusterDataForNodeID:self.nodeID]];
+                            }
+                        }
+                        [self _removeCachedAttributeValue:path forPath:attributePath];
+                    }
+                }
+            }
+            break;
+        }
+    }
+}
+
 // assume lock is held
 - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSString *, id> *> *)reportedAttributeValues fromSubscription:(BOOL)isFromSubscription
 {
@@ -2781,13 +2929,20 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
                     [self _noteDataVersion:dataVersion forClusterPath:clusterPath];
                 }
 
-                [self _setCachedAttributeValue:attributeDataValue forPath:attributePath fromSubscription:isFromSubscription];
+                if ([self _needsPruningOfEndpointsAndClusters:attributePath])
+                {
+                    previousValue = [self _dataValueWithoutDataVersion:previousValue];
+                    [self _pruneOrphanedEndpointsAndClusters:attributePath previousValue:previousValue attributeDataValue:attributeDataValue];
+                }
+                
                 if (!_deviceConfigurationChanged) {
                     _deviceConfigurationChanged = [self _attributeAffectsDeviceConfiguration:attributePath];
                     if (_deviceConfigurationChanged) {
                         MTR_LOG_INFO("Device configuration changed due to changes in attribute %@", attributePath);
                     }
                 }
+                
+                [self _setCachedAttributeValue:attributeDataValue forPath:attributePath fromSubscription:isFromSubscription];
             }
 
 #ifdef DEBUG
@@ -2941,6 +3096,47 @@ - (void)_storePersistedDeviceData
     [datastore storeDeviceData:[data copy] forNodeID:self.nodeID];
 }
 
+- (void)_removePersistedClusterDataForPath:(MTRClusterPath *)path
+{
+    os_unfair_lock_assert_owner(&self->_lock);
+    if (_persistedClusters == nil || _persistedClusterData == nil)
+    {
+        return;
+    }
+
+    [_persistedClusterData removeObjectForKey:path];
+    [_persistedClusters removeObject:path];
+}
+
+- (NSMutableSet<MTRClusterPath *> *)_getPersistedClusters
+{
+    std::lock_guard lock(_lock);
+    
+    return _persistedClusters;
+}
+
+- (MTRDeviceClusterData *)_getPersistedClusterDataForPath:(MTRClusterPath *)path
+{
+    std::lock_guard lock(_lock);
+    
+    if ([_persistedClusters containsObject:path])
+    {
+        return [_persistedClusterData objectForKey:path];
+    }
+    return nil;
+}
+
+- (BOOL)_persistedClusterContains:(MTRClusterPath *)path
+{
+    std::lock_guard lock(_lock);
+    
+    if ([_persistedClusters containsObject:path])
+    {
+        return YES;
+    }
+    return NO;
+}
+
 - (BOOL)deviceCachePrimed
 {
     std::lock_guard lock(_lock);
diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h
@@ -76,6 +76,8 @@ typedef void (^MTRDeviceControllerDataStoreClusterDataHandler)(NSDictionary<NSNu
 - (nullable MTRDeviceClusterData *)getStoredClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID;
 - (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData forNodeID:(NSNumber *)nodeID;
 - (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID;
+- (void)clearStoredClusterDataForNodeIDWithEndpointID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID;
+- (void)clearStoredClusterDataForNodeIDWithClusterID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID;
 - (void)clearAllStoredClusterData;
 
 /**
diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm
@@ -446,6 +446,25 @@ - (BOOL)_storeEndpointIndex:(NSArray<NSNumber *> *)endpointIndex forNodeID:(NSNu
     return [self _storeAttributeCacheValue:endpointIndex forKey:[self _endpointIndexKeyForNodeID:nodeID]];
 }
 
+- (BOOL)_deleteEndpointIndex:(NSNumber *)endpointID forNodeID:(NSNumber *)nodeID
+{
+    dispatch_assert_queue(_storageDelegateQueue);
+
+    if (!endpointID || !nodeID) {
+        MTR_LOG_ERROR("%s: unexpected nil input", __func__);
+        return NO;
+    }
+    
+    NSMutableArray * endpointIndex = [NSMutableArray arrayWithArray:[self _fetchEndpointIndexForNodeID:nodeID]];
+    if (endpointIndex == nil)
+    {
+        return NO;
+    }
+    
+    [endpointIndex removeObject:endpointID];
+    return [self _storeAttributeCacheValue:endpointIndex forKey:[self _endpointIndexKeyForNodeID:nodeID]];
+}
+
 - (BOOL)_deleteEndpointIndexForNodeID:(NSNumber *)nodeID
 {
     dispatch_assert_queue(_storageDelegateQueue);
@@ -497,7 +516,6 @@ - (BOOL)_deleteClusterIndexForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)e
         MTR_LOG_ERROR("%s: unexpected nil input", __func__);
         return NO;
     }
-
     return [self _removeAttributeCacheValueForKey:[self _clusterIndexKeyForNodeID:nodeID endpointID:endpointID]];
 }
 
@@ -540,8 +558,8 @@ - (BOOL)_deleteClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)en
         MTR_LOG_ERROR("%s: unexpected nil input", __func__);
         return NO;
     }
-
-    return [self _removeAttributeCacheValueForKey:[self _clusterDataKeyForNodeID:nodeID endpointID:endpointID clusterID:clusterID]];
+    BOOL value = [self _removeAttributeCacheValueForKey:[self _clusterDataKeyForNodeID:nodeID endpointID:endpointID clusterID:clusterID]];
+    return value;
 }
 
 #pragma - Attribute Cache management
@@ -699,6 +717,65 @@ - (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID
     });
 }
 
+- (BOOL)_clearStoredClusterDataForNodeIDWithEndpointID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID
+{
+    dispatch_assert_queue(_storageDelegateQueue);
+    NSArray<NSNumber *> * clusterIndex = [self _fetchClusterIndexForNodeID:nodeID endpointID:endpointID];
+
+    BOOL success = NO;
+    for (NSNumber * clusterID in clusterIndex) {
+        success = [self _deleteClusterDataForNodeID:nodeID endpointID:endpointID clusterID:clusterID];
+        if (!success) {
+            MTR_LOG_INFO("_clearStoredClusterDataForNodeIDWithEndpointID: Delete failed for clusterData @ node 0x%016llX endpoint %u cluster 0x%08lX", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue, clusterID.unsignedLongValue);
+        }
+    }
+
+    success = [self _deleteClusterIndexForNodeID:nodeID endpointID:endpointID];
+    if (!success) {
+        MTR_LOG_INFO("Delete failed for clusterIndex @ node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue);
+    }
+    
+    success = [self _deleteEndpointIndex:endpointID forNodeID:(nodeID)];
+    if (!success) {
+        MTR_LOG_INFO("Delete failed for endpoint index %@ for @ node 0x%016llX", endpointID, nodeID.unsignedLongLongValue);
+    }
+    return success;
+}
+
+- (void)clearStoredClusterDataForNodeIDWithEndpointID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID
+{
+    dispatch_async(_storageDelegateQueue, ^{
+        BOOL success = [self _clearStoredClusterDataForNodeIDWithEndpointID:nodeID endpointID:endpointID];
+        if (!success) {
+            MTR_LOG_INFO("clearStoredClusterDataForNodeIDWithEndpointID: Delete failed for clusterData for @ node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue);
+        }
+        MTR_LOG_INFO("clearStoredClusterDataForNodeIDWithEndpointID: Successfully deleted cluster index and data for @ node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue);
+    });
+}
+
+- (void)clearStoredClusterDataForNodeIDWithClusterID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID
+{
+    dispatch_async(_storageDelegateQueue, ^{
+        BOOL success = [self _deleteClusterDataForNodeID:nodeID endpointID:endpointID clusterID:clusterID];
+        if (!success) {
+            MTR_LOG_INFO("clearStoredClusterDataForNodeIDWithClusterID: _deleteClusterDataForNodeID failed for @ node 0x%016llX endpoint %u cluster 0x%08lX", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue, clusterID.unsignedLongValue);
+            return;
+        }
+               
+        NSArray<NSNumber *> * clusterIndex = [self _fetchClusterIndexForNodeID:nodeID endpointID:endpointID];
+        NSMutableArray<NSNumber *> * clusterIndexCopy = [clusterIndex mutableCopy];
+        [clusterIndexCopy removeObject:clusterID];
+
+        if (clusterIndexCopy.count != clusterIndex.count) {
+            success = [self _storeClusterIndex:clusterIndexCopy forNodeID:nodeID endpointID:endpointID];
+            if (!success) {
+                MTR_LOG_INFO("clearStoredClusterDataForNodeIDWithClusterID: _storeClusterIndex failed for @ node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue);
+            }
+        }
+        MTR_LOG_INFO("clearStoredClusterDataForNodeIDWithClusterID: Deleted endpoint %u cluster 0x%08lX for @ node 0x%016llX successfully", endpointID.unsignedShortValue, clusterID.unsignedLongValue, nodeID.unsignedLongLongValue);
+    });
+}
+
 - (void)clearAllStoredClusterData
 {
     dispatch_async(_storageDelegateQueue, ^{
diff --git a/src/darwin/Framework/CHIP/MTRDevice_Internal.h b/src/darwin/Framework/CHIP/MTRDevice_Internal.h
@@ -39,6 +39,7 @@ MTR_TESTABLE
 @property (nonatomic, readonly) NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> * attributes; // attributeID => data-value dictionary
 
 - (void)storeValue:(MTRDeviceDataValueDictionary _Nullable)value forAttribute:(NSNumber *)attribute;
+- (void)_removeValueForAttribute:(NSNumber *)attribute;
 
 - (nullable instancetype)initWithDataVersion:(NSNumber * _Nullable)dataVersion attributes:(NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> * _Nullable)attributes;
 @end
diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h
