Fix isEqual and hash on various path classes to do the right thing. (#…

…31168) * Fix isEqual and hash on various path classes to do the right thing. Some of them were falling through to the MTRClusterPath implementations, leading to different paths testing equal. Fixes #30969 * Address review comments.
project-chip · Jan 17, 2024 · 1054603 · 1054603
1 parent 827adcb
commit 1054603
Show file tree

Hide file tree

Showing 3 changed files with 127 additions and 46 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
@@ -2248,7 +2248,7 @@ - (BOOL)isEqualToAttributeRequestPath:(MTRAttributeRequestPath *)path
 
 - (BOOL)isEqual:(id)object
 {
-    if (![object isKindOfClass:[self class]]) {
+    if ([object class] != [self class]) {
         return NO;
     }
     return [self isEqualToAttributeRequestPath:object];
@@ -2320,7 +2320,7 @@ - (BOOL)isEqualToEventRequestPath:(MTREventRequestPath *)path
 
 - (BOOL)isEqual:(id)object
 {
-    if (![object isKindOfClass:[self class]]) {
+    if ([object class] != [self class]) {
         return NO;
     }
     return [self isEqualToEventRequestPath:object];
@@ -2389,7 +2389,7 @@ - (BOOL)isEqualToClusterPath:(MTRClusterPath *)clusterPath
 
 - (BOOL)isEqual:(id)object
 {
-    if (![object isKindOfClass:[self class]]) {
+    if ([object class] != [self class]) {
         return NO;
     }
     return [self isEqualToClusterPath:object];
@@ -2441,7 +2441,7 @@ - (BOOL)isEqualToAttributePath:(MTRAttributePath *)attributePath
 
 - (BOOL)isEqual:(id)object
 {
-    if (![object isKindOfClass:[self class]]) {
+    if ([object class] != [self class]) {
         return NO;
     }
     return [self isEqualToAttributePath:object];
@@ -2497,6 +2497,24 @@ ConcreteEventPath path(static_cast<chip::EndpointId>([endpointID unsignedShortVa
     return [[MTREventPath alloc] initWithPath:path];
 }
 
+- (BOOL)isEqualToEventPath:(MTREventPath *)eventPath
+{
+    return [self isEqualToClusterPath:eventPath] && [_event isEqualToNumber:eventPath.event];
+}
+
+- (BOOL)isEqual:(id)object
+{
+    if ([object class] != [self class]) {
+        return NO;
+    }
+    return [self isEqualToEventPath:object];
+}
+
+- (NSUInteger)hash
+{
+    return self.endpoint.unsignedShortValue ^ self.cluster.unsignedLongValue ^ _event.unsignedLongValue;
+}
+
 - (id)copyWithZone:(NSZone *)zone
 {
     return [MTREventPath eventPathWithEndpointID:self.endpoint clusterID:self.cluster eventID:_event];
@@ -2540,6 +2558,24 @@ ConcreteCommandPath path(static_cast<chip::EndpointId>([endpointID unsignedShort
     return [[MTRCommandPath alloc] initWithPath:path];
 }
 
+- (BOOL)isEqualToCommandPath:(MTRCommandPath *)commandPath
+{
+    return [self isEqualToClusterPath:commandPath] && [_command isEqualToNumber:commandPath.command];
+}
+
+- (BOOL)isEqual:(id)object
+{
+    if ([object class] != [self class]) {
+        return NO;
+    }
+    return [self isEqualToCommandPath:object];
+}
+
+- (NSUInteger)hash
+{
+    return self.endpoint.unsignedShortValue ^ self.cluster.unsignedLongValue ^ _command.unsignedLongValue;
+}
+
 - (id)copyWithZone:(NSZone *)zone
 {
     return [MTRCommandPath commandPathWithEndpointID:self.endpoint clusterID:self.cluster commandID:_command];

diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
@@ -2805,6 +2805,93 @@ - (void)test900_SubscribeAllAttributes
     [self waitForExpectations:[NSArray arrayWithObject:reportExpectation] timeout:kTimeoutInSeconds];
 }
 
+- (void)test029_PathsBehavior
+{
+    __auto_type * commandPath1 = [MTRCommandPath commandPathWithEndpointID:@(1) clusterID:@(2) commandID:@(3)];
+    __auto_type * commandPath2 = [MTRCommandPath commandPathWithEndpointID:@(1) clusterID:@(2) commandID:@(4)];
+    __auto_type * commandPath3 = [MTRCommandPath commandPathWithEndpointID:@(2) clusterID:@(2) commandID:@(3)];
+    __auto_type * commandPath4 = [MTRCommandPath commandPathWithEndpointID:@(1) clusterID:@(1) commandID:@(3)];
+    __auto_type * commandPath5 = [MTRCommandPath commandPathWithEndpointID:@(1) clusterID:@(2) commandID:@(3)];
+
+    __auto_type * eventPath1 = [MTREventPath eventPathWithEndpointID:@(1) clusterID:@(2) eventID:@(3)];
+    __auto_type * eventPath2 = [MTREventPath eventPathWithEndpointID:@(1) clusterID:@(2) eventID:@(4)];
+    __auto_type * eventPath3 = [MTREventPath eventPathWithEndpointID:@(2) clusterID:@(2) eventID:@(3)];
+    __auto_type * eventPath4 = [MTREventPath eventPathWithEndpointID:@(1) clusterID:@(1) eventID:@(3)];
+    __auto_type * eventPath5 = [MTREventPath eventPathWithEndpointID:@(1) clusterID:@(2) eventID:@(3)];
+
+    __auto_type * attributePath1 = [MTRAttributePath attributePathWithEndpointID:@(1) clusterID:@(2) attributeID:@(3)];
+    __auto_type * attributePath2 = [MTRAttributePath attributePathWithEndpointID:@(1) clusterID:@(2) attributeID:@(4)];
+    __auto_type * attributePath3 = [MTRAttributePath attributePathWithEndpointID:@(2) clusterID:@(2) attributeID:@(3)];
+    __auto_type * attributePath4 = [MTRAttributePath attributePathWithEndpointID:@(1) clusterID:@(1) attributeID:@(3)];
+    __auto_type * attributePath5 = [MTRAttributePath attributePathWithEndpointID:@(1) clusterID:@(2) attributeID:@(3)];
+
+    __auto_type * clusterPath1 = [MTRClusterPath clusterPathWithEndpointID:@(1) clusterID:@(2)];
+    __auto_type * clusterPath2 = [MTRClusterPath clusterPathWithEndpointID:@(1) clusterID:@(3)];
+    __auto_type * clusterPath3 = [MTRClusterPath clusterPathWithEndpointID:@(2) clusterID:@(2)];
+    __auto_type * clusterPath4 = [MTRClusterPath clusterPathWithEndpointID:@(1) clusterID:@(2)];
+
+    // Command paths
+    XCTAssertTrue([commandPath1 isEqual:commandPath5]);
+    XCTAssertEqualObjects(commandPath1, commandPath5);
+
+    XCTAssertFalse([commandPath1 isEqual:commandPath2]);
+    XCTAssertNotEqualObjects(commandPath1, commandPath2);
+
+    XCTAssertFalse([commandPath1 isEqual:commandPath3]);
+    XCTAssertNotEqualObjects(commandPath1, commandPath3);
+
+    XCTAssertFalse([commandPath1 isEqual:commandPath4]);
+    XCTAssertNotEqualObjects(commandPath1, commandPath4);
+
+    // Event paths
+    XCTAssertTrue([eventPath1 isEqual:eventPath5]);
+    XCTAssertEqualObjects(eventPath1, eventPath5);
+
+    XCTAssertFalse([eventPath1 isEqual:eventPath2]);
+    XCTAssertNotEqualObjects(eventPath1, eventPath2);
+
+    XCTAssertFalse([eventPath1 isEqual:eventPath3]);
+    XCTAssertNotEqualObjects(eventPath1, eventPath3);
+
+    XCTAssertFalse([eventPath1 isEqual:eventPath4]);
+    XCTAssertNotEqualObjects(eventPath1, eventPath4);
+
+    // Attribute paths
+    XCTAssertTrue([attributePath1 isEqual:attributePath5]);
+    XCTAssertEqualObjects(attributePath1, attributePath5);
+
+    XCTAssertFalse([attributePath1 isEqual:attributePath2]);
+    XCTAssertNotEqualObjects(attributePath1, attributePath2);
+
+    XCTAssertFalse([attributePath1 isEqual:attributePath3]);
+    XCTAssertNotEqualObjects(attributePath1, attributePath3);
+
+    XCTAssertFalse([attributePath1 isEqual:attributePath4]);
+    XCTAssertNotEqualObjects(attributePath1, attributePath4);
+
+    // Clusters
+    XCTAssertTrue([clusterPath1 isEqual:clusterPath4]);
+    XCTAssertEqualObjects(clusterPath1, clusterPath4);
+
+    XCTAssertFalse([clusterPath1 isEqual:clusterPath2]);
+    XCTAssertNotEqualObjects(clusterPath1, clusterPath2);
+
+    XCTAssertFalse([clusterPath1 isEqual:clusterPath3]);
+    XCTAssertNotEqualObjects(clusterPath1, clusterPath3);
+
+    // Mix
+    XCTAssertFalse([commandPath1 isEqual:eventPath1]);
+    XCTAssertFalse([eventPath1 isEqual:commandPath1]);
+
+    XCTAssertFalse([commandPath1 isEqual:attributePath1]);
+    XCTAssertFalse([attributePath1 isEqual:commandPath1]);
+
+    XCTAssertFalse([attributePath1 isEqual:eventPath1]);
+    XCTAssertFalse([eventPath1 isEqual:attributePath1]);
+    XCTAssertFalse([clusterPath1 isEqual:attributePath1]);
+    XCTAssertFalse([clusterPath1 isEqual:eventPath1]);
+}
+
 - (void)test999_TearDown
 {
     ResetCommissionee(GetConnectedDevice(), dispatch_get_main_queue(), self, kTimeoutInSeconds);

diff --git a/src/darwin/Framework/CHIPTests/MTRXPCProtocolTests.m b/src/darwin/Framework/CHIPTests/MTRXPCProtocolTests.m
@@ -34,48 +34,6 @@
 // Inverted expectation timeout
 static const uint16_t kNegativeTimeoutInSeconds = 1;
 
-@interface MTRAttributePath (Test)
-- (BOOL)isEqual:(id)object;
-@end
-
-@implementation MTRAttributePath (Test)
-- (BOOL)isEqual:(id)object
-{
-    if ([object isKindOfClass:[MTRAttributePath class]]) {
-        MTRAttributePath * other = object;
-        return [self.endpoint isEqualToNumber:other.endpoint] && [self.cluster isEqualToNumber:other.cluster] &&
-            [self.attribute isEqualToNumber:other.attribute];
-    }
-    return NO;
-}
-
-- (NSString *)description
-{
-    return [NSString stringWithFormat:@"MTRAttributePath(%@,%@,%@)", self.endpoint, self.cluster, self.attribute];
-}
-@end
-
-@interface MTRCommandPath (Test)
-- (BOOL)isEqual:(id)object;
-@end
-
-@implementation MTRCommandPath (Test)
-- (BOOL)isEqual:(id)object
-{
-    if ([object isKindOfClass:[MTRCommandPath class]]) {
-        MTRCommandPath * other = object;
-        return [self.endpoint isEqualToNumber:other.endpoint] && [self.cluster isEqualToNumber:other.cluster] &&
-            [self.command isEqualToNumber:other.command];
-    }
-    return NO;
-}
-
-- (NSString *)description
-{
-    return [NSString stringWithFormat:@"MTRCommandPath(%@,%@,%@)", self.endpoint, self.cluster, self.command];
-}
-@end
-
 @interface MTRClusterStateCacheContainer (Test)
 // Obsolete method is moved to this test suite to keep tests compatible
 - (void)subscribeWithDeviceController:(MTRDeviceController *)deviceController