diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index e0154fdb8934e6..8e379980578f58 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -539,6 +539,8 @@ - (BOOL)deviceCachePrimed return NO; } +#pragma mark - Suspend/resume management + - (void)controllerSuspended { // Nothing to do for now. @@ -549,6 +551,166 @@ - (void)controllerResumed // Nothing to do for now. } +#pragma mark - Value comparisons + +- (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)one isEqualToDataValue:(MTRDeviceDataValueDictionary)theOther +{ + // Sanity check for nil cases + if (!one && !theOther) { + MTR_LOG_ERROR("%@ attribute data-value comparison does not expect comparing two nil dictionaries", self); + return YES; + } + if (!one || !theOther) { + // Comparing against nil is expected, and should return NO quietly + return NO; + } + + // Attribute data-value dictionaries are equal if type and value are equal, and specifically, this should return true if values are both nil + return [one[MTRTypeKey] isEqual:theOther[MTRTypeKey]] && ((one[MTRValueKey] == theOther[MTRValueKey]) || [one[MTRValueKey] isEqual:theOther[MTRValueKey]]); +} + +// _attributeDataValue:satisfiesExpectedDataValue: checks whether the newly +// received attribute data value satisfies the expectation we have. +// +// For now, a value is considered to satisfy the expectation if it's equal to +// the expected value, though we allow the fields of structs to be in a +// different order than expected: while in theory the spec does require a +// specific ordering for struct fields, in practice we should not force certain +// API consumers to deal with knowing what that ordering is. +// +// Things to consider for future: +// +// 1) Should a value that has _extra_ fields in a struct compared to the expected +// value be considered as satisfying the expectation? Arguably, yes. +// +// 2) Should lists actually enforce order (as now), or should they allow +// reordering entries? +// +// 3) For fabric-scoped lists, should we have a way to check for just "our +// fabric's" entries? +- (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)observed satisfiesValueExpectation:(MTRDeviceDataValueDictionary)expected +{ + // Sanity check for nil cases (which really should not happen!) + if (!observed && !expected) { + MTR_LOG_ERROR("%@ observed to expected attribute data-value comparison does not expect comparing two nil dictionaries", self); + return YES; + } + + if (!observed || !expected) { + // Again, not expected here. But clearly the expectation is not really + // satisfied, in some sense. + MTR_LOG_ERROR("@ observed to expected attribute data-value comparison does not expect a nil %s", observed ? "expected" : "observed"); + return NO; + } + + if (![observed[MTRTypeKey] isEqual:expected[MTRTypeKey]]) { + // Different types, does not satisfy expectation. + return NO; + } + + if ([MTRArrayValueType isEqual:expected[MTRTypeKey]]) { + // For array-values, check that sizes are same and entries satisfy expectations. + if (![observed[MTRValueKey] isKindOfClass:NSArray.class] || ![expected[MTRValueKey] isKindOfClass:NSArray.class]) { + // Malformed data, just claim expectation is not satisfied. + MTR_LOG_ERROR("%@ at least one of observed and expected value is not an NSArrray: %@, %@", self, observed, expected); + return NO; + } + + NSArray *> * observedArray = observed[MTRValueKey]; + NSArray *> * expectedArray = expected[MTRValueKey]; + + if (observedArray.count != expectedArray.count) { + return NO; + } + + for (NSUInteger i = 0; i < observedArray.count; ++i) { + NSDictionary * observedEntry = observedArray[i]; + NSDictionary * expectedEntry = expectedArray[i]; + + if (![observedEntry isKindOfClass:NSDictionary.class] || ![expectedEntry isKindOfClass:NSDictionary.class]) { + MTR_LOG_ERROR("%@ expected or observed array-value contains entries that are not NSDictionary: %@, %@", self, observedEntry, expectedEntry); + return NO; + } + + if (![self _attributeDataValue:observedEntry[MTRDataKey] satisfiesValueExpectation:expectedEntry[MTRDataKey]]) { + return NO; + } + } + + return YES; + } + + if (![MTRStructureValueType isEqual:expected[MTRTypeKey]]) { + // For everything except arrays and structs, expectation is satisfied + // exactly when the values are equal. + return [self _attributeDataValue:observed isEqualToDataValue:expected]; + } + + // Now we have two structure-values. Make sure they have the same number of fields + // in them. + if (![observed[MTRValueKey] isKindOfClass:NSArray.class] || ![expected[MTRValueKey] isKindOfClass:NSArray.class]) { + // Malformed data, just claim not equivalent. + MTR_LOG_ERROR("%@ at least one of observed and expected value is not an NSArrray: %@, %@", self, observed, expected); + return NO; + } + + NSArray *> * observedArray = observed[MTRValueKey]; + NSArray *> * expectedArray = expected[MTRValueKey]; + + if (observedArray.count != expectedArray.count) { + return NO; + } + + for (NSDictionary * expectedField in expectedArray) { + if (![expectedField[MTRContextTagKey] isKindOfClass:NSNumber.class] || ![expectedField[MTRDataKey] isKindOfClass:NSDictionary.class]) { + MTR_LOG_ERROR("%@ expected structure-value contains invalid field %@", self, expectedField); + return NO; + } + + NSNumber * expectedContextTag = expectedField[MTRContextTagKey]; + + // Make sure it's present in the other array. In practice, these are + // pretty small arrays, so the O(N^2) behavior here is ok. + BOOL found = NO; + for (NSDictionary * observedField in observedArray) { + if (![observedField[MTRContextTagKey] isKindOfClass:NSNumber.class] || ![observedField[MTRDataKey] isKindOfClass:NSDictionary.class]) { + MTR_LOG_ERROR("%@ observed structure-value contains invalid field %@", self, observedField); + return NO; + } + + NSNumber * observedContextTag = observedField[MTRContextTagKey]; + if ([expectedContextTag isEqual:observedContextTag]) { + found = YES; + + // Compare the data. + if (![self _attributeDataValue:observedField[MTRDataKey] satisfiesValueExpectation:expectedField[MTRDataKey]]) { + return NO; + } + + // Found a match for the context tag, stop looking. + break; + } + } + + if (!found) { + // Context tag present in expected but not observed. + return NO; + } + } + + // All entries in the first field array matched entries in the second field + // array. Since the lengths are equal, the two arrays must match, as long + // as all the context tags listed are distinct. If someone produces invalid + // TLV with the same context tag set in it multiple times, this method could + // claim two structure-values are equivalent when the first has two fields + // with context tag N and the second has a field with context tag N and + // another field with context tag M. That should be ok, in practice, but if + // we discover it's not we will need a better algorithm here. It's not + // clear what "equivalent" should mean for such malformed TLV, expecially if + // the same context tag maps to different values in one of the structs. + return YES; +} + @end /* BEGIN DRAGONS: Note methods here cannot be renamed, and are used by private callers, do not rename, remove or modify behavior here */ diff --git a/src/darwin/Framework/CHIP/MTRDeviceDataValueDictionary.h b/src/darwin/Framework/CHIP/MTRDeviceDataValueDictionary.h index 34e46b7ccd7ef3..53a6b2e6f914b7 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceDataValueDictionary.h +++ b/src/darwin/Framework/CHIP/MTRDeviceDataValueDictionary.h @@ -18,6 +18,9 @@ NS_ASSUME_NONNULL_BEGIN +/** + * A data-value as defined in MTRBaseDevice.h. + */ typedef NSDictionary * MTRDeviceDataValueDictionary; NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm index 7276a5b5342eb1..2423e4e8769c45 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -3307,22 +3307,6 @@ - (void)_performScheduledExpirationCheck return nil; } -- (BOOL)_attributeDataValue:(NSDictionary *)one isEqualToDataValue:(NSDictionary *)theOther -{ - // Sanity check for nil cases - if (!one && !theOther) { - MTR_LOG_ERROR("%@ attribute data-value comparison does not expect comparing two nil dictionaries", self); - return YES; - } - if (!one || !theOther) { - // Comparing against nil is expected, and should return NO quietly - return NO; - } - - // Attribute data-value dictionaries are equal if type and value are equal, and specifically, this should return true if values are both nil - return [one[MTRTypeKey] isEqual:theOther[MTRTypeKey]] && ((one[MTRValueKey] == theOther[MTRValueKey]) || [one[MTRValueKey] isEqual:theOther[MTRValueKey]]); -} - // Utility to return data value dictionary without data version - (NSDictionary *)_dataValueWithoutDataVersion:(NSDictionary *)attributeValue { @@ -3538,9 +3522,9 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray * attributeResponseValue in reportedAttributeValues) { MTRAttributePath * attributePath = attributeResponseValue[MTRAttributePathKey]; - NSDictionary * attributeDataValue = attributeResponseValue[MTRDataKey]; - NSError * attributeError = attributeResponseValue[MTRErrorKey]; - NSDictionary * previousValue; + MTRDeviceDataValueDictionary _Nullable attributeDataValue = attributeResponseValue[MTRDataKey]; + NSError * _Nullable attributeError = attributeResponseValue[MTRErrorKey]; + MTRDeviceDataValueDictionary _Nullable previousValue; // sanity check either data value or error must exist if (!attributeDataValue && !attributeError) { diff --git a/src/darwin/Framework/CHIP/MTRDevice_Internal.h b/src/darwin/Framework/CHIP/MTRDevice_Internal.h index f954979f131084..df4a8265538dfe 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDevice_Internal.h @@ -22,6 +22,7 @@ #import "MTRAsyncWorkQueue.h" #import "MTRDefines_Internal.h" +#import "MTRDeviceDataValueDictionary.h" #import "MTRDeviceStorageBehaviorConfiguration_Internal.h" NS_ASSUME_NONNULL_BEGIN @@ -164,6 +165,10 @@ MTR_DIRECT_MEMBERS - (void)controllerSuspended; - (void)controllerResumed; +// Methods for comparing attribute data values. +- (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)one isEqualToDataValue:(MTRDeviceDataValueDictionary)theOther; +- (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)observed satisfiesValueExpectation:(MTRDeviceDataValueDictionary)expected; + @end #pragma mark - MTRDevice internal state monitoring diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 5fb3549a49216a..9d33649057ddb1 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -4467,6 +4467,358 @@ - (void)test039_GetAllAttributesReport } } +- (void)test040_AttributeValueExpectationSatisfaction +{ + __auto_type * device = [MTRDevice deviceWithNodeID:kDeviceId deviceController:sController]; + + __auto_type * testData = @[ + @{ + @"expected" : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7), + }, + @"observed" : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7) + }, + // Equal unsigned integer should satisfy expectation. + @"expectedComparison" : @(YES), + }, + @{ + @"expected" : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7), + }, + @"observed" : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(9), + }, + // Unequal unsigned integer should not satisfy expectation + @"expectedComparison" : @(NO), + }, + @{ + @"expected" : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7), + }, + @"observed" : @ { + MTRTypeKey : MTRSignedIntegerValueType, + MTRValueKey : @(7), + }, + // A signed integer does not satisfy expectation for an unsigned integer. + @"expectedComparison" : @(NO), + }, + @{ + @"expected" : @ { + MTRTypeKey : MTRNullValueType, + }, + @"observed" : @ { + MTRTypeKey : MTRNullValueType, + }, + // Null satisfies expectation for null. + @"expectedComparison" : @(YES), + }, + @{ + @"expected" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + ], + }, + @"observed" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7), + } + }, + ], + }, + // A longer list does not satisfy expectation for a shorter array. + @"expectedComparison" : @(NO), + }, + @{ + @"expected" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7), + } + }, + ], + }, + @"observed" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + ], + }, + // A shorter list does not satisfy expectation for a longer array. + @"expectedComparison" : @(NO), + }, + @{ + @"expected" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + ], + }, + @"observed" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + ], + }, + // An observed array identical to an expected one satisfies the expectation. + @"expectedComparison" : @(YES), + }, + @{ + @"expected" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + ], + }, + @"observed" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + ], + }, + // An array with entries in a different order does not satisfy the expectation. + @"expectedComparison" : @(NO), + }, + @{ + @"expected" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abc"), + }, + }, + ], + }, + @"observed" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abc"), + }, + }, + ], + }, + // A struct that has the same fields in the same order satisfiess the + // expectation. + @"expectedComparison" : @(YES), + }, + @{ + @"expected" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abc"), + }, + }, + ], + }, + @"observed" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abcd"), + }, + }, + ], + }, + // A struct that has different fields in the same order does not + // satisfy the expectation. + @"expectedComparison" : @(NO), + }, + @{ + @"expected" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abc"), + }, + }, + ], + }, + @"observed" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abc"), + }, + }, + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + ], + }, + // A struct that has the same fields in a different order satisfies + // the expectation. + @"expectedComparison" : @(YES), + }, + ]; + + for (NSDictionary * test in testData) { + XCTAssertEqual([device _attributeDataValue:test[@"observed"] satisfiesValueExpectation:test[@"expected"]], [test[@"expectedComparison"] boolValue], + "observed: %@, expected: %@", test[@"observed"], test[@"expected"]); + } +} + @end @interface MTRDeviceEncoderTests : XCTestCase diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h index 74a7dc280b1216..b00507d665a9f0 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h @@ -49,7 +49,6 @@ NS_ASSUME_NONNULL_BEGIN @end @interface MTRDevice (Test) -- (BOOL)_attributeDataValue:(NSDictionary *)one isEqualToDataValue:(NSDictionary *)theOther; - (NSMutableArray *)arrayOfNumbersFromAttributeValue:(MTRDeviceDataValueDictionary)dataDictionary; - (void)setStorageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration; @end