Addressing more review comments.

project-chip · Jan 31, 2024 · 06febe9 · 06febe9
1 parent 5b60d08
commit 06febe9
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 28 deletions.
diff --git a/src/darwin/Framework/CHIP/ServerEndpoint/MTRAccessGrant.mm b/src/darwin/Framework/CHIP/ServerEndpoint/MTRAccessGrant.mm
@@ -80,7 +80,7 @@ + (MTRAccessGrant *)accessGrantForAllNodesWithPrivilege:(MTRAccessControlEntryPr
 
 // initWithSubject assumes that the subject has already been validated and, if
 // needed, copied from the input.
-- (instancetype)initWithSubject:(nullable NSNumber *)subject privilege:(MTRAccessControlEntryPrivilege)privilege authenticationMode:(MTRAccessControlEntryAuthMode)authenticationMode
+- (nullable instancetype)initWithSubject:(nullable NSNumber *)subject privilege:(MTRAccessControlEntryPrivilege)privilege authenticationMode:(MTRAccessControlEntryAuthMode)authenticationMode
 {
     if (!(self = [super init])) {
         return nil;
@@ -115,4 +115,41 @@ - (NSUInteger)hash
     return _subjectID.unsignedLongLongValue ^ _grantedPrivilege ^ _authenticationMode;
 }
 
+- (NSString *)description
+{
+    NSString * privilege = @"Unknown";
+    switch (_grantedPrivilege) {
+    case MTRAccessControlEntryPrivilegeView:
+        privilege = @"View";
+        break;
+    case MTRAccessControlEntryPrivilegeProxyView:
+        privilege = @"ProxyView";
+        break;
+    case MTRAccessControlEntryPrivilegeOperate:
+        privilege = @"Operate";
+        break;
+    case MTRAccessControlEntryPrivilegeManage:
+        privilege = @"Manage";
+        break;
+    case MTRAccessControlEntryPrivilegeAdminister:
+        privilege = @"Administer";
+        break;
+    }
+
+    if (_subjectID == nil) {
+        return [NSString stringWithFormat:@"<%@ all nodes can %@>", self.class, privilege];
+    }
+
+    NodeId nodeId = static_cast<NodeId>(_subjectID.unsignedLongLongValue);
+    if (IsGroupId(nodeId)) {
+        return [NSString stringWithFormat:@"<%@ group 0x%x can %@>", self.class, GroupIdFromNodeId(nodeId), privilege];
+    }
+
+    if (IsCASEAuthTag(nodeId)) {
+        return [NSString stringWithFormat:@"<%@ nodes with CASE Authenticated Tag 0x%08x can %@>", self.class, CASEAuthTagFromNodeId(nodeId), privilege];
+    }
+
+    return [NSString stringWithFormat:@"<%@ node 0x%016llx can %@>", self.class, nodeId, privilege];
+}
+
 @end
diff --git a/src/darwin/Framework/CHIP/ServerEndpoint/MTRServerCluster.h b/src/darwin/Framework/CHIP/ServerEndpoint/MTRServerCluster.h
@@ -32,18 +32,18 @@ MTR_NEWLY_AVAILABLE
 
 /**
  * The provided clusterID must not be MTRClusterIDTypeDescriptorID; see
- * initDescriptorCluster.
+ * newDescriptorCluster.
  *
  * Otherwise, it must be a valid cluster identifier.  That means:
  *
  * * In the range 0-0x7FFF for standard clusters.
  * * In the range 0xVVVVFC00-0xVVVVFFFE for vendor-specific clusters, where VVVV
  *   is the vendor identifier.
  *
- * The provided clusterRevision must be in the range 1-65535.
+ * The provided revision must be in the range 1-65535.
  *
  */
-- (nullable instancetype)initWithClusterID:(NSNumber *)clusterID clusterRevision:(NSNumber *)clusterRevision;
+- (nullable instancetype)initWithClusterID:(NSNumber *)clusterID revision:(NSNumber *)revision;
 
 /**
  * Add an access grant to the cluster.  If the same access grant is added
@@ -87,7 +87,7 @@ MTR_NEWLY_AVAILABLE
  * Create a cluster description for the descriptor cluster.  This will set
  * clusterRevision to the current version implemented by Matter.framework.
  */
-- (instancetype)initDescriptorCluster;
++ (MTRServerCluster *)newDescriptorCluster;
 
 @property (nonatomic, copy, readonly) NSNumber * clusterID;
 

diff --git a/src/darwin/Framework/CHIP/ServerEndpoint/MTRServerCluster.mm b/src/darwin/Framework/CHIP/ServerEndpoint/MTRServerCluster.mm
@@ -41,7 +41,7 @@ @implementation MTRServerCluster {
     MTRDeviceController * __weak _deviceController;
 }
 
-- (nullable instancetype)initWithClusterID:(NSNumber *)clusterID clusterRevision:(NSNumber *)clusterRevision
+- (nullable instancetype)initWithClusterID:(NSNumber *)clusterID revision:(NSNumber *)revision
 {
     auto clusterIDValue = clusterID.unsignedLongLongValue;
     if (!CanCastTo<ClusterId>(clusterIDValue)) {
@@ -56,32 +56,32 @@ - (nullable instancetype)initWithClusterID:(NSNumber *)clusterID clusterRevision
     }
 
     if (id == MTRClusterIDTypeDescriptorID) {
-        MTR_LOG_ERROR("Should be using initDescriptorCluster to initialize an MTRServerCluster for Descriptor");
+        MTR_LOG_ERROR("Should be using newDescriptorCluster to initialize an MTRServerCluster for Descriptor");
         return nil;
     }
 
-    auto revisionValue = clusterRevision.unsignedLongLongValue;
+    auto revisionValue = revision.unsignedLongLongValue;
     if (revisionValue < 1 || revisionValue > 0xFFFF) {
         MTR_LOG_ERROR("MTRServerCluster provided invalid cluster revision: 0x%llx", revisionValue);
         return nil;
     }
 
-    return [self initInternalWithClusterID:clusterID clusterRevision:clusterRevision accessGrants:[NSSet set] attributes:@[]];
+    return [self initInternalWithClusterID:clusterID revision:revision accessGrants:[NSSet set] attributes:@[]];
 }
 
-- (instancetype)initDescriptorCluster
++ (MTRServerCluster *)newDescriptorCluster
 {
-    return [self initInternalWithClusterID:@(MTRClusterIDTypeDescriptorID) clusterRevision:@(app::Clusters::Descriptor::kClusterRevision) accessGrants:[NSSet set] attributes:@[]];
+    return [[MTRServerCluster alloc] initInternalWithClusterID:@(MTRClusterIDTypeDescriptorID) revision:@(app::Clusters::Descriptor::kClusterRevision) accessGrants:[NSSet set] attributes:@[]];
 }
 
-- (instancetype)initInternalWithClusterID:(NSNumber *)clusterID clusterRevision:(NSNumber *)clusterRevision accessGrants:(NSSet *)accessGrants attributes:(NSArray *)attributes
+- (instancetype)initInternalWithClusterID:(NSNumber *)clusterID revision:(NSNumber *)revision accessGrants:(NSSet *)accessGrants attributes:(NSArray *)attributes
 {
     if (!(self = [super init])) {
         return nil;
     }
 
     _clusterID = [clusterID copy];
-    _clusterRevision = [clusterRevision copy];
+    _clusterRevision = [revision copy];
     _accessGrants = [[NSMutableSet alloc] init];
     _attributes = [[NSMutableArray alloc] init];
     _matterAccessGrants = [NSSet set];
@@ -90,13 +90,13 @@ - (instancetype)initInternalWithClusterID:(NSNumber *)clusterID clusterRevision:
     for (MTRAccessGrant * grant in accessGrants) {
         // Since our state is MTRServerEndpointStateInitializing, we know this
         // will succeed.
-        [self addAccessGrant:[grant copy]];
+        [self addAccessGrant:grant];
     }
 
     for (MTRServerAttribute * attr in attributes) {
         // Since our state is MTRServerEndpointStateInitializing and the initial
         // attribute array was valid, we know this will succeed.
-        [self addAttribute:[attr copy]];
+        [self addAttribute:attr];
     }
 
     return self;

diff --git a/src/darwin/Framework/CHIPTests/MTRServerEndpointTests.m b/src/darwin/Framework/CHIPTests/MTRServerEndpointTests.m
@@ -57,6 +57,7 @@ - (void)testAccessGrant
         XCTAssertEqualObjects(grant.subjectID, nodeID);
         XCTAssertEqual(grant.grantedPrivilege, MTRAccessControlEntryPrivilegeView);
         XCTAssertEqual(grant.authenticationMode, MTRAccessControlEntryAuthModeCASE);
+        XCTAssertEqualObjects([grant description], @"<MTRAccessGrant node 0x0000000000000002 can View>");
     }
 
     // Try different privileges
@@ -67,6 +68,7 @@ - (void)testAccessGrant
         XCTAssertEqualObjects(grant.subjectID, nodeID);
         XCTAssertEqual(grant.grantedPrivilege, MTRAccessControlEntryPrivilegeAdminister);
         XCTAssertEqual(grant.authenticationMode, MTRAccessControlEntryAuthModeCASE);
+        XCTAssertEqualObjects([grant description], @"<MTRAccessGrant node 0x0000000000000002 can Administer>");
     }
 
     // Try a CAT
@@ -76,6 +78,7 @@ - (void)testAccessGrant
         XCTAssertEqualObjects(grant.subjectID, @(0xFFFFFFFD00020003));
         XCTAssertEqual(grant.grantedPrivilege, MTRAccessControlEntryPrivilegeManage);
         XCTAssertEqual(grant.authenticationMode, MTRAccessControlEntryAuthModeCASE);
+        XCTAssertEqualObjects([grant description], @"<MTRAccessGrant nodes with CASE Authenticated Tag 0x00020003 can Manage>");
     }
 
     // Try some invalid CATs
@@ -96,6 +99,7 @@ - (void)testAccessGrant
         XCTAssertEqualObjects(grant.subjectID, @(0xFFFFFFFFFFFF0005));
         XCTAssertEqual(grant.grantedPrivilege, MTRAccessControlEntryPrivilegeOperate);
         XCTAssertEqual(grant.authenticationMode, MTRAccessControlEntryAuthModeGroup);
+        XCTAssertEqualObjects([grant description], @"<MTRAccessGrant group 0x5 can Operate>");
     }
 
     // Try an invalid group ID
@@ -111,6 +115,7 @@ - (void)testAccessGrant
         XCTAssertNil(grant.subjectID);
         XCTAssertEqual(grant.grantedPrivilege, MTRAccessControlEntryPrivilegeView);
         XCTAssertEqual(grant.authenticationMode, MTRAccessControlEntryAuthModeCASE);
+        XCTAssertEqualObjects([grant description], @"<MTRAccessGrant all nodes can View>");
     }
 }
 
@@ -234,7 +239,7 @@ - (void)testClusterDescription
     {
         NSNumber * clusterID = @(6);
         NSNumber * clusterRevision = @(1);
-        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID clusterRevision:clusterRevision];
+        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID revision:clusterRevision];
         XCTAssertNotNil(cluster);
         XCTAssertEqualObjects(cluster.clusterID, clusterID);
         XCTAssertEqualObjects(cluster.clusterRevision, clusterRevision);
@@ -246,7 +251,7 @@ - (void)testClusterDescription
     {
         NSNumber * clusterID = @(0xFFF1FC01);
         NSNumber * clusterRevision = @(1);
-        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID clusterRevision:clusterRevision];
+        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID revision:clusterRevision];
         XCTAssertNotNil(cluster);
         XCTAssertEqualObjects(cluster.clusterID, clusterID);
         XCTAssertEqualObjects(cluster.clusterRevision, clusterRevision);
@@ -258,47 +263,47 @@ - (void)testClusterDescription
     {
         NSNumber * clusterID = @(0x8000);
         NSNumber * clusterRevision = @(1);
-        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID clusterRevision:clusterRevision];
+        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID revision:clusterRevision];
         XCTAssertNil(cluster);
     }
 
     // Invalid vendor-specific cluster.
     {
         NSNumber * clusterID = @(0xFFF10002);
         NSNumber * clusterRevision = @(1);
-        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID clusterRevision:clusterRevision];
+        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID revision:clusterRevision];
         XCTAssertNil(cluster);
     }
 
     // Cluster ID out of range.
     {
         NSNumber * clusterID = @(0x100000000);
         NSNumber * clusterRevision = @(1);
-        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID clusterRevision:clusterRevision];
+        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID revision:clusterRevision];
         XCTAssertNil(cluster);
     }
 
     // Revision too small.
     {
         NSNumber * clusterID = @(6);
         NSNumber * clusterRevision = @(0);
-        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID clusterRevision:clusterRevision];
+        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID revision:clusterRevision];
         XCTAssertNil(cluster);
     }
 
     // Revision too big.
     {
         NSNumber * clusterID = @(6);
         NSNumber * clusterRevision = @(0x10000);
-        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID clusterRevision:clusterRevision];
+        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID revision:clusterRevision];
         XCTAssertNil(cluster);
     }
 
     // Descriptor cluster wrong method.
     {
         NSNumber * clusterID = @(0x001D);
         NSNumber * clusterRevision = @(0x10000);
-        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID clusterRevision:clusterRevision];
+        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID revision:clusterRevision];
         XCTAssertNil(cluster);
     }
 
@@ -315,7 +320,7 @@ - (void)testClusterDescription
     // Descriptor cluster right method
     {
         NSNumber * clusterID = @(0x001D);
-        __auto_type * cluster = [[MTRServerCluster alloc] initDescriptorCluster];
+        __auto_type * cluster = [MTRServerCluster newDescriptorCluster];
         XCTAssertNotNil(cluster);
         XCTAssertEqualObjects(cluster.clusterID, clusterID);
         // Don't hardcode the cluster revision here; we want it to be able to
@@ -358,7 +363,7 @@ - (void)testClusterDescription
     {
         NSNumber * clusterID = @(0xFFF1FC01);
         NSNumber * clusterRevision = @(1);
-        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID clusterRevision:clusterRevision];
+        __auto_type * cluster = [[MTRServerCluster alloc] initWithClusterID:clusterID revision:clusterRevision];
         XCTAssertNotNil(cluster);
         XCTAssertEqualObjects(cluster.accessGrants, @[]);
         XCTAssertEqualObjects(cluster.attributes, @[]);
@@ -384,7 +389,7 @@ - (void)testClusterDescription
         }
         XCTAssertEqualObjects(cluster.attributes, attributes);
 
-        __auto_type * otherCluster = [[MTRServerCluster alloc] initWithClusterID:clusterID clusterRevision:clusterRevision];
+        __auto_type * otherCluster = [[MTRServerCluster alloc] initWithClusterID:clusterID revision:clusterRevision];
 
         // Adding an already-added attribute should fail.
         XCTAssertFalse([otherCluster addAttribute:attributes[0]]);
@@ -462,7 +467,7 @@ - (void)testEndpointDescription
         XCTAssertEqualObjects(endpoint.accessGrants, grants);
 
         __auto_type * clusters = @[
-            [[MTRServerCluster alloc] initWithClusterID:@(6) clusterRevision:@(1)],
+            [[MTRServerCluster alloc] initWithClusterID:@(6) revision:@(1)],
         ];
         for (MTRServerCluster * cluster in clusters) {
             XCTAssertTrue([endpoint addServerCluster:cluster]);
@@ -474,7 +479,7 @@ - (void)testEndpointDescription
         // Adding an already-added cluster should fail.
         XCTAssertFalse([otherEndpoint addServerCluster:clusters[0]]);
 
-        MTRServerCluster * otherCluster = [[MTRServerCluster alloc] initWithClusterID:@(6) clusterRevision:@(1)];
+        MTRServerCluster * otherCluster = [[MTRServerCluster alloc] initWithClusterID:@(6) revision:@(1)];
 
         // Adding same-id cluster should fail.
         XCTAssertFalse([endpoint addServerCluster:otherCluster]);