diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm index 602d608a42e6dc..a10c7d81e71492 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -3319,55 +3319,70 @@ - (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)one isEqualToDataValue 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 +// _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 - if (!one && !theOther) { - MTR_LOG_ERROR("%@ attribute data-value comparison does not expect comparing two nil dictionaries", self); + // 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 (!one || !theOther) { - // Comparing against nil is expected, and should return NO quietly + 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 (![one[MTRTypeKey] isEqual:theOther[MTRTypeKey]]) { - // Different types, not equivalent. + if (![observed[MTRTypeKey] isEqual:expected[MTRTypeKey]]) { + // Different types, does not satisfy expectation. 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); + 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 *> * oneArray = one[MTRValueKey]; - NSArray *> * theOtherArray = theOther[MTRValueKey]; + NSArray *> * observedArray = observed[MTRValueKey]; + NSArray *> * expectedArray = expected[MTRValueKey]; - if (oneArray.count != theOtherArray.count) { + if (observedArray.count != expectedArray.count) { return NO; } - for (NSUInteger i = 0; i < oneArray.count; ++i) { - NSDictionary * oneEntry = oneArray[i]; - NSDictionary * theOtherEntry = theOtherArray[i]; + for (NSUInteger i = 0; i < observedArray.count; ++i) { + NSDictionary * observedEntry = observedArray[i]; + NSDictionary * expectedEntry = expectedArray[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); + 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:oneEntry[MTRDataKey] isEquivalentToDataValue:theOtherEntry[MTRDataKey]]) { + if (![self _attributeDataValue:observedEntry[MTRDataKey] satisfiesValueExpectation:expectedEntry[MTRDataKey]]) { return NO; } } @@ -3375,50 +3390,50 @@ - (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)one isEquivalentToData 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]; + 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 (![one[MTRValueKey] isKindOfClass:NSArray.class] || ![theOther[MTRValueKey] isKindOfClass:NSArray.class]) { + if (![observed[MTRValueKey] isKindOfClass:NSArray.class] || ![expected[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); + MTR_LOG_ERROR("%@ at least one of observed and expected value is not an NSArrray: %@, %@", self, observed, expected); return NO; } - NSArray *> * oneArray = one[MTRValueKey]; - NSArray *> * theOtherArray = theOther[MTRValueKey]; + NSArray *> * observedArray = observed[MTRValueKey]; + NSArray *> * expectedArray = expected[MTRValueKey]; - if (oneArray.count != theOtherArray.count) { + if (observedArray.count != expectedArray.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); + 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 * oneContextTag = oneField[MTRContextTagKey]; + 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 * theOtherField in theOtherArray) { - if (![theOtherField[MTRContextTagKey] isKindOfClass:NSNumber.class] || ![theOtherField[MTRDataKey] isKindOfClass:NSDictionary.class]) { - MTR_LOG_ERROR("%@ structure-value contains invalid field %@", self, theOtherField); + 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 * theOtherContextTag = theOtherField[MTRContextTagKey]; - if ([oneContextTag isEqual:theOtherContextTag]) { + NSNumber * observedContextTag = observedField[MTRContextTagKey]; + if ([expectedContextTag isEqual:observedContextTag]) { found = YES; // Compare the data. - if (![self _attributeDataValue:oneField[MTRDataKey] isEquivalentToDataValue:theOtherField[MTRDataKey]]) { + if (![self _attributeDataValue:observedField[MTRDataKey] satisfiesValueExpectation:expectedField[MTRDataKey]]) { return NO; } @@ -3428,7 +3443,7 @@ - (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)one isEquivalentToData } if (!found) { - // Context tag present in one but not theOther. + // Context tag present in expected but not observed. return NO; } } diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 1a8805915e0af2..9d33649057ddb1 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -4467,63 +4467,59 @@ - (void)test039_GetAllAttributesReport } } -- (void)test040_AttributeValueEquivalence +- (void)test040_AttributeValueExpectationSatisfaction { - // 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" : @ { + @"expected" : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(7), }, - @"second" : @ { + @"observed" : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(7) }, + // Equal unsigned integer should satisfy expectation. @"expectedComparison" : @(YES), }, @{ - // Check that two unequal unsigned integers are equivalent. - @"first" : @ { + @"expected" : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(7), }, - @"second" : @ { + @"observed" : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(9), }, + // Unequal unsigned integer should not satisfy expectation @"expectedComparison" : @(NO), }, @{ - // Check that a signed and unsigned integer are not equivalent, even - // if the value is the same. - @"first" : @ { + @"expected" : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @(7), }, - @"second" : @ { + @"observed" : @ { MTRTypeKey : MTRSignedIntegerValueType, MTRValueKey : @(7), }, + // A signed integer does not satisfy expectation for an unsigned integer. @"expectedComparison" : @(NO), }, @{ - // Check that null is equivalent to null. - @"first" : @ { + @"expected" : @ { MTRTypeKey : MTRNullValueType, }, - @"second" : @ { + @"observed" : @ { MTRTypeKey : MTRNullValueType, }, + // Null satisfies expectation for null. @"expectedComparison" : @(YES), }, @{ - // Check that two different-length arrays are not equivalent. - @"first" : @ { + @"expected" : @ { MTRTypeKey : MTRArrayValueType, MTRValueKey : @[ @{ @@ -4540,7 +4536,7 @@ - (void)test040_AttributeValueEquivalence }, ], }, - @"second" : @ { + @"observed" : @ { MTRTypeKey : MTRArrayValueType, MTRValueKey : @[ @{ @@ -4563,11 +4559,55 @@ - (void)test040_AttributeValueEquivalence }, ], }, + // A longer list does not satisfy expectation for a shorter array. @"expectedComparison" : @(NO), }, @{ - // Check that identical arrays are equivalent. - @"first" : @ { + @"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 : @[ @{ @@ -4584,7 +4624,7 @@ - (void)test040_AttributeValueEquivalence }, ], }, - @"second" : @ { + @"observed" : @ { MTRTypeKey : MTRArrayValueType, MTRValueKey : @[ @{ @@ -4601,12 +4641,11 @@ - (void)test040_AttributeValueEquivalence }, ], }, + // An observed array identical to an expected one satisfies the expectation. @"expectedComparison" : @(YES), }, @{ - // Check that arrays with the same entries in different orders are - // not equivalent. - @"first" : @ { + @"expected" : @ { MTRTypeKey : MTRArrayValueType, MTRValueKey : @[ @{ @@ -4623,7 +4662,7 @@ - (void)test040_AttributeValueEquivalence }, ], }, - @"second" : @ { + @"observed" : @ { MTRTypeKey : MTRArrayValueType, MTRValueKey : @[ @{ @@ -4640,12 +4679,11 @@ - (void)test040_AttributeValueEquivalence }, ], }, + // An array with entries in a different order does not satisfy the expectation. @"expectedComparison" : @(NO), }, @{ - // Check that two structs that have the same fields in the same - // order are equivalent. - @"first" : @ { + @"expected" : @ { MTRTypeKey : MTRStructureValueType, MTRValueKey : @[ @{ @@ -4664,7 +4702,7 @@ - (void)test040_AttributeValueEquivalence }, ], }, - @"second" : @ { + @"observed" : @ { MTRTypeKey : MTRStructureValueType, MTRValueKey : @[ @{ @@ -4683,12 +4721,12 @@ - (void)test040_AttributeValueEquivalence }, ], }, + // A struct that has the same fields in the same order satisfiess the + // expectation. @"expectedComparison" : @(YES), }, @{ - // Check that two structs that have different fields in the same - // order are equivalent. - @"first" : @ { + @"expected" : @ { MTRTypeKey : MTRStructureValueType, MTRValueKey : @[ @{ @@ -4707,7 +4745,7 @@ - (void)test040_AttributeValueEquivalence }, ], }, - @"second" : @ { + @"observed" : @ { MTRTypeKey : MTRStructureValueType, MTRValueKey : @[ @{ @@ -4726,12 +4764,12 @@ - (void)test040_AttributeValueEquivalence }, ], }, + // A struct that has different fields in the same order does not + // satisfy the expectation. @"expectedComparison" : @(NO), }, @{ - // Check that two structs that have the same fields in different orders - // are equivalent. - @"first" : @ { + @"expected" : @ { MTRTypeKey : MTRStructureValueType, MTRValueKey : @[ @{ @@ -4750,7 +4788,7 @@ - (void)test040_AttributeValueEquivalence }, ], }, - @"second" : @ { + @"observed" : @ { MTRTypeKey : MTRStructureValueType, MTRValueKey : @[ @{ @@ -4769,15 +4807,15 @@ - (void)test040_AttributeValueEquivalence }, ], }, + // 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[@"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"]); + XCTAssertEqual([device _attributeDataValue:test[@"observed"] satisfiesValueExpectation:test[@"expected"]], [test[@"expectedComparison"] boolValue], + "observed: %@, expected: %@", test[@"observed"], test[@"expected"]); } } diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h index b8935d6bd4a7b9..fc3eeec3cd0c3d 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h @@ -50,7 +50,7 @@ NS_ASSUME_NONNULL_BEGIN @interface MTRDevice (Test) - (BOOL)_attributeDataValue:(NSDictionary *)one isEqualToDataValue:(NSDictionary *)theOther; -- (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)one isEquivalentToDataValue:(MTRDeviceDataValueDictionary)theOther; +- (BOOL)_attributeDataValue:(MTRDeviceDataValueDictionary)observed satisfiesValueExpectation:(MTRDeviceDataValueDictionary)expected; - (NSMutableArray *)arrayOfNumbersFromAttributeValue:(MTRDeviceDataValueDictionary)dataDictionary; - (void)setStorageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration; @end