From 36ad3b71b0eab539f2240fb7b055603855600a2e Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 18 Oct 2024 16:35:45 -0400 Subject: [PATCH] Add an MTRDevice_Concrete method for checking whether attribute data-values are quivalent. --- .../CHIP/MTRDeviceDataValueDictionary.h | 3 + .../Framework/CHIP/MTRDevice_Concrete.mm | 135 +++++++- .../Framework/CHIPTests/MTRDeviceTests.m | 314 ++++++++++++++++++ .../TestHelpers/MTRTestDeclarations.h | 1 + 4 files changed, 449 insertions(+), 4 deletions(-) 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 48d7f7f99d0fe7..602d608a42e6dc 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -3303,7 +3303,7 @@ - (void)_performScheduledExpirationCheck return nil; } -- (BOOL)_attributeDataValue:(NSDictionary *)one isEqualToDataValue:(NSDictionary *)theOther +- (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)one isEqualToDataValue:(MTRDeviceDataValueDictionary)theOther { // Sanity check for nil cases if (!one && !theOther) { @@ -3319,6 +3319,133 @@ - (BOOL)_attributeDataValue:(NSDictionary *)one isEqualToDataValue:(NSDictionary return [one[MTRTypeKey] isEqual:theOther[MTRTypeKey]] && ((one[MTRValueKey] == theOther[MTRValueKey]) || [one[MTRValueKey] isEqual:theOther[MTRValueKey]]); } +// _attributeDataValue:isEquivalentToDataValue: differs from +// _attributeDataValue:isEqualToDataValue in that it does not insist on the +// order of fields in structs matching between the two values. 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. +- (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)one isEquivalentToDataValue:(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; + } + + if (![one[MTRTypeKey] isEqual:theOther[MTRTypeKey]]) { + // Different types, not equivalent. + return NO; + } + + if ([MTRArrayValueType isEqual:one[MTRTypeKey]]) { + // For array-values, check that sizes are same and entries are equivalent. + if (![one[MTRValueKey] isKindOfClass:NSArray.class] || ![theOther[MTRValueKey] isKindOfClass:NSArray.class]) { + // Malformed data, just claim not equivalent. + MTR_LOG_ERROR("%@ array-values not equivalent because at least one is not an NSArrray: %@, %@", self, one, theOther); + return NO; + } + + NSArray *> * oneArray = one[MTRValueKey]; + NSArray *> * theOtherArray = theOther[MTRValueKey]; + + if (oneArray.count != theOtherArray.count) { + return NO; + } + + for (NSUInteger i = 0; i < oneArray.count; ++i) { + NSDictionary * oneEntry = oneArray[i]; + NSDictionary * theOtherEntry = theOtherArray[i]; + + if (![oneEntry isKindOfClass:NSDictionary.class] || ![theOtherEntry isKindOfClass:NSDictionary.class]) { + MTR_LOG_ERROR("%@ array-values not equivalent because they contain entries that are not NSDictionary: %@, %@", self, oneEntry, theOtherEntry); + return NO; + } + + if (![self _attributeDataValue:oneEntry[MTRDataKey] isEquivalentToDataValue:theOtherEntry[MTRDataKey]]) { + return NO; + } + } + + return YES; + } + + if (![MTRStructureValueType isEqual:one[MTRTypeKey]]) { + // For everything except arrays and structs, equivalence and equality + // are the same. + return [self _attributeDataValue:one isEqualToDataValue:theOther]; + } + + // Now we have two structure-values. Make sure they have the same number of fields + // in them. + if (![one[MTRValueKey] isKindOfClass:NSArray.class] || ![theOther[MTRValueKey] isKindOfClass:NSArray.class]) { + // Malformed data, just claim not equivalent. + MTR_LOG_ERROR("%@ structure-values not equivalent because at least one is not an NSArrray: %@, %@", self, one, theOther); + return NO; + } + + NSArray *> * oneArray = one[MTRValueKey]; + NSArray *> * theOtherArray = theOther[MTRValueKey]; + + if (oneArray.count != theOtherArray.count) { + return NO; + } + + for (NSDictionary * oneField in oneArray) { + if (![oneField[MTRContextTagKey] isKindOfClass:NSNumber.class] || ![oneField[MTRDataKey] isKindOfClass:NSDictionary.class]) { + MTR_LOG_ERROR("%@ structure-value contains invalid field %@", self, oneField); + return NO; + } + + NSNumber * oneContextTag = oneField[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 * theOtherField in theOtherArray) { + if (![theOtherField[MTRContextTagKey] isKindOfClass:NSNumber.class] || ![theOtherField[MTRDataKey] isKindOfClass:NSDictionary.class]) { + MTR_LOG_ERROR("%@ structure-value contains invalid field %@", self, theOtherField); + return NO; + } + + NSNumber * theOtherContextTag = theOtherField[MTRContextTagKey]; + if ([oneContextTag isEqual:theOtherContextTag]) { + found = YES; + + // Compare the data. + if (![self _attributeDataValue:oneField[MTRDataKey] isEquivalentToDataValue:theOtherField[MTRDataKey]]) { + return NO; + } + + // Found a match for the context tag, stop looking. + break; + } + } + + if (!found) { + // Context tag present in one but not theOther. + 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; +} + // Utility to return data value dictionary without data version - (NSDictionary *)_dataValueWithoutDataVersion:(NSDictionary *)attributeValue { @@ -3534,9 +3661,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/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 5fb3549a49216a..1a8805915e0af2 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -4467,6 +4467,320 @@ - (void)test039_GetAllAttributesReport } } +- (void)test040_AttributeValueEquivalence +{ + // This shouldn't really need an MTRDevice instance, but for convenience the + // functionality we are testing is an instance method on MTRDevice. + __auto_type * device = [MTRDevice deviceWithNodeID:kDeviceId deviceController:sController]; + + __auto_type * testData = @[ + @{ + // Check that two equal unsigned integers are equivalent. + @"first" : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7), + }, + @"second" : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7) + }, + @"expectedComparison" : @(YES), + }, + @{ + // Check that two unequal unsigned integers are equivalent. + @"first" : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7), + }, + @"second" : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(9), + }, + @"expectedComparison" : @(NO), + }, + @{ + // Check that a signed and unsigned integer are not equivalent, even + // if the value is the same. + @"first" : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7), + }, + @"second" : @ { + MTRTypeKey : MTRSignedIntegerValueType, + MTRValueKey : @(7), + }, + @"expectedComparison" : @(NO), + }, + @{ + // Check that null is equivalent to null. + @"first" : @ { + MTRTypeKey : MTRNullValueType, + }, + @"second" : @ { + MTRTypeKey : MTRNullValueType, + }, + @"expectedComparison" : @(YES), + }, + @{ + // Check that two different-length arrays are not equivalent. + @"first" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + ], + }, + @"second" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(7), + } + }, + ], + }, + @"expectedComparison" : @(NO), + }, + @{ + // Check that identical arrays are equivalent. + @"first" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + ], + }, + @"second" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + ], + }, + @"expectedComparison" : @(YES), + }, + @{ + // Check that arrays with the same entries in different orders are + // not equivalent. + @"first" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + ], + }, + @"second" : @ { + MTRTypeKey : MTRArrayValueType, + MTRValueKey : @[ + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + } + }, + @{ + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(5), + } + }, + ], + }, + @"expectedComparison" : @(NO), + }, + @{ + // Check that two structs that have the same fields in the same + // order are equivalent. + @"first" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abc"), + }, + }, + ], + }, + @"second" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abc"), + }, + }, + ], + }, + @"expectedComparison" : @(YES), + }, + @{ + // Check that two structs that have different fields in the same + // order are equivalent. + @"first" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abc"), + }, + }, + ], + }, + @"second" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abcd"), + }, + }, + ], + }, + @"expectedComparison" : @(NO), + }, + @{ + // Check that two structs that have the same fields in different orders + // are equivalent. + @"first" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abc"), + }, + }, + ], + }, + @"second" : @ { + MTRTypeKey : MTRStructureValueType, + MTRValueKey : @[ + @{ + MTRContextTagKey : @(2), + MTRDataKey : @ { + MTRTypeKey : MTRUTF8StringValueType, + MTRValueKey : @("abc"), + }, + }, + @{ + MTRContextTagKey : @(1), + MTRDataKey : @ { + MTRTypeKey : MTRUnsignedIntegerValueType, + MTRValueKey : @(6), + }, + }, + ], + }, + @"expectedComparison" : @(YES), + }, + ]; + + for (NSDictionary * test in testData) { + XCTAssertEqual([device _attributeDataValue:test[@"first"] isEquivalentToDataValue:test[@"second"]], [test[@"expectedComparison"] boolValue], + "first: %@, second: %@", test[@"first"], test[@"second"]); + XCTAssertEqual([device _attributeDataValue:test[@"second"] isEquivalentToDataValue:test[@"first"]], [test[@"expectedComparison"] boolValue], + "first: %@, second: %@", test[@"first"], test[@"second"]); + } +} + @end @interface MTRDeviceEncoderTests : XCTestCase diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h index 74a7dc280b1216..b8935d6bd4a7b9 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h @@ -50,6 +50,7 @@ NS_ASSUME_NONNULL_BEGIN @interface MTRDevice (Test) - (BOOL)_attributeDataValue:(NSDictionary *)one isEqualToDataValue:(NSDictionary *)theOther; +- (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)one isEquivalentToDataValue:(MTRDeviceDataValueDictionary)theOther; - (NSMutableArray *)arrayOfNumbersFromAttributeValue:(MTRDeviceDataValueDictionary)dataDictionary; - (void)setStorageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration; @end