diff --git a/src/darwin/Framework/CHIP/MTREndpointInfo.mm b/src/darwin/Framework/CHIP/MTREndpointInfo.mm index ca0c6ec3340205..a99950a1a6cf50 100644 --- a/src/darwin/Framework/CHIP/MTREndpointInfo.mm +++ b/src/darwin/Framework/CHIP/MTREndpointInfo.mm @@ -52,6 +52,7 @@ - (instancetype)initWithEndpointID:(NSNumber *)endpointID _deviceTypes = [deviceTypes copy]; _partsList = [partsList copy]; _children = @[]; + _mark = EndpointMark::NotVisited; return self; } @@ -78,7 +79,7 @@ + (BOOL)populateChildrenForEndpoints:(NSDictionary deque; // stack followed by sorted list deque.emplace_front(root->_endpointID); @@ -109,6 +110,11 @@ + (BOOL)populateChildrenForEndpoints:(NSDictionary_mark == EndpointMark::Visited */ { + // Nodes can be visited multiple times due to Full-Family + // ancestors like the root node, or in scenarios where a + // nodes is erroneously in the PartsList of two separate + // branches of the tree. There is no easy way to distinguish + // these cases here, so we are not setting valid = NO. deque.pop_front(); // nothing else to do } } @@ -157,8 +163,7 @@ + (BOOL)populateChildrenForEndpoints:(NSDictionary * deviceTypeList = MTRDecodeAttributeValue(path, *cache, &err); if (!deviceTypeList) { - MTR_LOG_ERROR("Ignoring endpoint %u, failed to parse DeviceTypeList: %" CHIP_ERROR_FORMAT, path.mEndpointId, err.Format()); - return CHIP_NO_ERROR; + MTR_LOG_ERROR("Ignoring invalid DeviceTypeList for endpoint %u: %" CHIP_ERROR_FORMAT, path.mEndpointId, err.Format()); } NSMutableArray * deviceTypes = [[NSMutableArray alloc] initWithCapacity:deviceTypeList.count]; @@ -172,15 +177,11 @@ + (BOOL)populateChildrenForEndpoints:(NSDictionary * partsList = MTRDecodeAttributeValue(partsListPath, *cache, &err); if (!partsList) { - MTR_LOG_ERROR("Ignoring PartsList for endpoint %u: %" CHIP_ERROR_FORMAT, path.mEndpointId, err.Format()); + MTR_LOG_ERROR("Ignoring invalid PartsList for endpoint %u: %" CHIP_ERROR_FORMAT, path.mEndpointId, err.Format()); partsList = @[]; } diff --git a/src/darwin/Framework/CHIPTests/MTREndpointInfoTests.m b/src/darwin/Framework/CHIPTests/MTREndpointInfoTests.m index 1ed79adca1ef99..1948639abf2517 100644 --- a/src/darwin/Framework/CHIPTests/MTREndpointInfoTests.m +++ b/src/darwin/Framework/CHIPTests/MTREndpointInfoTests.m @@ -33,6 +33,13 @@ @implementation MTREndpointInfoTests return [[endpoint.children valueForKey:@"endpointID"] sortedArrayUsingSelector:@selector(compare:)]; } +static NSArray * Exclude(NSArray * numbers, NSNumber * numberToExclude) +{ + NSMutableArray * result = [numbers mutableCopy]; + [result removeObject:numberToExclude]; + return result; +} + - (NSDictionary *)indexEndpoints:(NSArray *)endpoints { NSMutableDictionary * indexed = [[NSMutableDictionary alloc] init]; @@ -64,6 +71,28 @@ - (void)testPopulateChildren XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@6]), (@[])); } +- (void)testPopulateChildren2 +{ + // Same as testPopulateChildren, but with reversed PartsLists + NSDictionary * endpoints = [self indexEndpoints:@[ + MakeEndpoint(@0, @[ @6, @5, @4, @3, @2, @1 ]), // full-family pattern + MakeEndpoint(@1, @[ @3, @2 ]), + MakeEndpoint(@2, @[]), + MakeEndpoint(@3, @[]), + MakeEndpoint(@4, @[ @6, @5 ]), // full-family pattern + MakeEndpoint(@5, @[ @6 ]), + MakeEndpoint(@6, @[]), + ]]; + XCTAssertTrue([MTREndpointInfo populateChildrenForEndpoints:endpoints]); + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@0]), (@[ @1, @4 ])); + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@1]), (@[ @2, @3 ])); + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@2]), (@[])); + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@3]), (@[])); + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@4]), (@[ @5 ])); + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@5]), (@[ @6 ])); + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@6]), (@[])); +} + - (void)testPopulateChildrenRootOnly { NSDictionary * endpoints = [self indexEndpoints:@[ @@ -73,7 +102,7 @@ - (void)testPopulateChildrenRootOnly XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@0]), (@[])); } -- (void)testPopulateChildrenWithCompositionCycle +- (void)testPopulateChildrenInvalidCompositionCycle { NSDictionary * endpoints = [self indexEndpoints:@[ MakeEndpoint(@0, @[ @1, @2, @3, @4, @5, @6 ]), // full-family pattern @@ -82,7 +111,7 @@ - (void)testPopulateChildrenWithCompositionCycle MakeEndpoint(@3, @[]), MakeEndpoint(@4, @[ @5, @6 ]), // full-family pattern MakeEndpoint(@5, @[ @6 ]), - MakeEndpoint(@6, @[ @4 ]), // cycle 4 -> 5 -> 6 -> 4 + MakeEndpoint(@6, @[ @4 ]), // not valid per spec: cycle 4 -> 5 -> 6 -> 4 ]]; XCTAssertFalse([MTREndpointInfo populateChildrenForEndpoints:endpoints]); XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@0]), (@[ @1, @4 ])); @@ -92,4 +121,26 @@ - (void)testPopulateChildrenWithCompositionCycle // We make no promises about child lists for endpoints involved in a cycle } +- (void)testPopulateChildrenInvalidNonTree +{ + NSDictionary * endpoints = [self indexEndpoints:@[ + MakeEndpoint(@0, @[ @1, @2, @3, @4, @5, @6 ]), // full-family pattern + MakeEndpoint(@1, @[ @2, @3, @6 ]), + MakeEndpoint(@2, @[]), + MakeEndpoint(@3, @[]), + MakeEndpoint(@4, @[ @5, @6 ]), // full-family pattern + MakeEndpoint(@5, @[ @6 ]), + MakeEndpoint(@6, @[]), // not valid per spec: 6 is a child of both 1 and 5 + ]]; + // Note: Not asserting a false return value here, this scenario is currently not detected. + [MTREndpointInfo populateChildrenForEndpoints:endpoints]; + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@0]), (@[ @1, @4 ])); + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@2]), (@[])); + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@3]), (@[])); + XCTAssertEqualObjects(ChildEndpointIDs(endpoints[@4]), (@[ @5 ])); + // Endpoint 6 has multiple parents, so we make no guarantees where (or if) it shows up + XCTAssertEqualObjects(Exclude(ChildEndpointIDs(endpoints[@1]), @6), (@[ @2, @3 ])); + XCTAssertEqualObjects(Exclude(ChildEndpointIDs(endpoints[@5]), @6), (@[])); +} + @end