Add better APIs for handling FeatureMap in MTRServerAttribute. (#33760)

* Add better APIs for handling FeatureMap in MTRServerAttribute. Do more sanity checks when using the initializer, and provide a factory method that makes creating FeatureMap attributes easier. * Apply suggestions from code review Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com> --------- Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com>
project-chip · Aug 9, 2024 · 1116432 · 1116432
1 parent d8ffa7e
commit 1116432
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 2 deletions.
diff --git a/src/darwin/Framework/CHIP/ServerEndpoint/MTRServerAttribute.h b/src/darwin/Framework/CHIP/ServerEndpoint/MTRServerAttribute.h
@@ -41,7 +41,9 @@ MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6))
  * Will fail if the attribute ID is not valid per the Matter specification or
  * the attribute value is not a valid data-value.
  *
- * requiredPrivilege is the privilege required to read the attribute.
+ * requiredPrivilege is the privilege required to read the attribute. This
+ * initializer may fail if the provided attributeID is a global attribute and
+ * the provided requiredPrivilege value is not correct for that attribute ID.
  */
 - (nullable instancetype)initReadonlyAttributeWithID:(NSNumber *)attributeID initialValue:(NSDictionary<NSString *, id> *)value requiredPrivilege:(MTRAccessControlEntryPrivilege)requiredPrivilege;
 
@@ -53,6 +55,14 @@ MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6))
  */
 - (BOOL)setValue:(NSDictionary<NSString *, id> *)value;
 
+/**
+ * Create an attribute description for a FeatureMap attribute with the provided
+ * value (expected to be an unsigned integer representing the value of the
+ * bitmap). This will automatically set requiredPrivilege to the right value
+ * for FeatureMap.
+ */
++ (MTRServerAttribute *)newFeatureMapAttributeWithInitialValue:(NSNumber *)value;
+
 @property (atomic, copy, readonly) NSNumber * attributeID;
 @property (atomic, copy, readonly) NSDictionary<NSString *, id> * value;
 /**

diff --git a/src/darwin/Framework/CHIP/ServerEndpoint/MTRServerAttribute.mm b/src/darwin/Framework/CHIP/ServerEndpoint/MTRServerAttribute.mm
@@ -23,6 +23,7 @@
 #import "MTRUnfairLock.h"
 #import "NSDataSpanConversion.h"
 
+#import <Matter/MTRClusterConstants.h>
 #import <Matter/MTRServerAttribute.h>
 
 #include <app/reporting/reporting.h>
@@ -57,6 +58,20 @@ - (nullable instancetype)initAttributeWithID:(NSNumber *)attributeID initialValu
         return nil;
     }
 
+    if (attrId == MTRAttributeIDTypeGlobalAttributeFeatureMapID) {
+        // Some sanity checks: value should be an unsigned-integer NSNumber, and
+        // requiredReadPrivilege should be View.
+        if (requiredReadPrivilege != MTRAccessControlEntryPrivilegeView) {
+            MTR_LOG_ERROR("MTRServerAttribute for FeatureMap provided with invalid read privilege %d", requiredReadPrivilege);
+            return nil;
+        }
+
+        if (![MTRUnsignedIntegerValueType isEqual:value[MTRTypeKey]]) {
+            MTR_LOG_ERROR("MTRServerAttribute for FeatureMap provided with value that is not an unsigned integer: %@", value);
+            return nil;
+        }
+    }
+
     return [self initWithAttributeID:[attributeID copy] value:[value copy] requiredReadPrivilege:requiredReadPrivilege writable:writable];
 }
 
@@ -80,7 +95,8 @@ - (nullable instancetype)initWithAttributeID:(NSNumber *)attributeID value:(NSDi
     _writable = writable;
     _parentCluster = app::ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId);
 
-    // Now call setValue to store the value and its serialization.
+    // Now store the value and its serialization. This will also check that the
+    // value is serializable to TLV.
     if ([self setValueInternal:value logIfNotAssociated:NO] == NO) {
         return nil;
     }
@@ -93,6 +109,16 @@ - (BOOL)setValue:(NSDictionary<NSString *, id> *)value
     return [self setValueInternal:value logIfNotAssociated:YES];
 }
 
++ (MTRServerAttribute *)newFeatureMapAttributeWithInitialValue:(NSNumber *)value
+{
+    return [[MTRServerAttribute alloc] initReadonlyAttributeWithID:@(MTRAttributeIDTypeGlobalAttributeFeatureMapID)
+                                                      initialValue:@{
+                                                          MTRTypeKey : MTRUnsignedIntegerValueType,
+                                                          MTRValueKey : value,
+                                                      }
+                                                 requiredPrivilege:MTRAccessControlEntryPrivilegeView];
+}
+
 - (BOOL)setValueInternal:(NSDictionary<NSString *, id> *)value logIfNotAssociated:(BOOL)logIfNotAssociated
 {
     id serializedValue;

diff --git a/src/darwin/Framework/CHIPTests/MTRServerEndpointTests.m b/src/darwin/Framework/CHIPTests/MTRServerEndpointTests.m
@@ -126,6 +126,11 @@ - (void)testServerAttribute
         MTRValueKey : @(5),
     };
 
+    __auto_type * signedIntValue = @{
+        MTRTypeKey : MTRSignedIntegerValueType,
+        MTRValueKey : @(5),
+    };
+
     __auto_type * listOfStringsValue = @{
         MTRTypeKey : MTRArrayValueType,
         MTRValueKey : @[
@@ -192,6 +197,36 @@ - (void)testServerAttribute
         __auto_type * attr = [[MTRServerAttribute alloc] initReadonlyAttributeWithID:invalidID initialValue:unsignedIntValue requiredPrivilege:MTRAccessControlEntryPrivilegeView];
         XCTAssertNil(attr);
     }
+
+    // Valid FeatureMap attribute
+    {
+        __auto_type * attr = [[MTRServerAttribute alloc] initReadonlyAttributeWithID:@(MTRAttributeIDTypeGlobalAttributeFeatureMapID) initialValue:unsignedIntValue requiredPrivilege:MTRAccessControlEntryPrivilegeView];
+        XCTAssertNotNil(attr);
+        XCTAssertEqualObjects(attr.attributeID, @(MTRAttributeIDTypeGlobalAttributeFeatureMapID));
+        XCTAssertEqualObjects(attr.value, unsignedIntValue);
+        XCTAssertEqual(attr.writable, NO);
+    }
+
+    // FeatureMap attribute with wrong value type
+    {
+        __auto_type * attr = [[MTRServerAttribute alloc] initReadonlyAttributeWithID:@(MTRAttributeIDTypeGlobalAttributeFeatureMapID) initialValue:signedIntValue requiredPrivilege:MTRAccessControlEntryPrivilegeView];
+        XCTAssertNil(attr);
+    }
+
+    // FeatureMap attribute with wrong permissions
+    {
+        __auto_type * attr = [[MTRServerAttribute alloc] initReadonlyAttributeWithID:@(MTRAttributeIDTypeGlobalAttributeFeatureMapID) initialValue:unsignedIntValue requiredPrivilege:MTRAccessControlEntryPrivilegeOperate];
+        XCTAssertNil(attr);
+    }
+
+    // FeatureMap attribute via factory
+    {
+        __auto_type * attr = [MTRServerAttribute newFeatureMapAttributeWithInitialValue:@(5)];
+        XCTAssertNotNil(attr);
+        XCTAssertEqualObjects(attr.attributeID, @(MTRAttributeIDTypeGlobalAttributeFeatureMapID));
+        XCTAssertEqualObjects(attr.value, unsignedIntValue);
+        XCTAssertEqual(attr.writable, NO);
+    }
 }
 
 - (void)testDeviceType