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

…roject-chip#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 project-chip#30969 * Address review comments.
Jerry-ESP · Jan 4, 2024 · b88f21a · b88f21a
1 parent 82d105e
commit b88f21a
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