From dc5dfe4667bc8c8a14437cc2339e5105c4f111d3 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 16 Dec 2022 17:58:46 -0500 Subject: [PATCH] Add a test for a subscription trying to send reports on a defunct session. (#24113) This is testing the situation described in https://github.com/project-chip/connectedhomeip/pull/24093 --- src/app/tests/TestReadInteraction.cpp | 88 +++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index af2e7a15672b8b..3333e97d7ac81e 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -339,6 +339,7 @@ class TestReadInteraction static void TestReadHandlerInvalidSubscribeRequest(nlTestSuite * apSuite, void * apContext); static void TestSubscribeInvalidateFabric(nlTestSuite * apSuite, void * apContext); static void TestShutdownSubscription(nlTestSuite * apSuite, void * apContext); + static void TestSubscriptionReportWithDefunctSession(nlTestSuite * apSuite, void * apContext); static void TestReadHandlerMalformedSubscribeRequest(nlTestSuite * apSuite, void * apContext); private: @@ -4148,6 +4149,92 @@ void TestReadInteraction::TestShutdownSubscription(nlTestSuite * apSuite, void * NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); } +/** + * Tests what happens when a subscription tries to deliver reports but the + * session it has is defunct. Makes sure we correctly tear down the ReadHandler + * and don't increment the "reports in flight" count. + */ +void TestReadInteraction::TestSubscriptionReportWithDefunctSession(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + + Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + // Shouldn't have anything in the retransmit table when starting the test. + NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); + + MockInteractionModelApp delegate; + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + AttributePathParams subscribePath(Test::kMockEndpoint3, Test::MockClusterId(2), Test::MockAttributeId(4)); + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + readPrepareParams.mpAttributePathParamsList = &subscribePath; + readPrepareParams.mAttributePathParamsListSize = 1; + + readPrepareParams.mMinIntervalFloorSeconds = 0; + readPrepareParams.mMaxIntervalCeilingSeconds = 0; + + { + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Subscribe); + + delegate.mGotReport = false; + + err = readClient.SendSubscribeRequest(std::move(readPrepareParams)); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, delegate.mGotReport); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Subscribe) == 1); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Read) == 0); + NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().GetNumReportsInFlight() == 0); + + NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(0) != nullptr); + auto * readHandler = engine->ActiveHandlerAt(0); + + // Verify that the session we will reset later is the one we will mess + // with now. + NL_TEST_ASSERT(apSuite, SessionHandle(*readHandler->GetSession()) == ctx.GetSessionAliceToBob()); + + // Test that we send reports as needed. + readHandler->mFlags.Set(ReadHandler::ReadHandlerFlags::HoldReport, false); + delegate.mGotReport = false; + engine->GetReportingEngine().SetDirty(subscribePath); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, delegate.mGotReport); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Subscribe) == 1); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Read) == 0); + NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().GetNumReportsInFlight() == 0); + + // Test that if the session is defunct we don't send reports and clean + // up properly. + readHandler->GetSession()->MarkAsDefunct(); + readHandler->mFlags.Set(ReadHandler::ReadHandlerFlags::HoldReport, false); + delegate.mGotReport = false; + engine->GetReportingEngine().SetDirty(subscribePath); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, !delegate.mGotReport); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Subscribe) == 0); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Read) == 0); + NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().GetNumReportsInFlight() == 0); + } + engine->Shutdown(); + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + // Get rid of our defunct session. + ctx.ExpireSessionAliceToBob(); + ctx.CreateSessionAliceToBob(); +} + } // namespace app } // namespace chip @@ -4212,6 +4299,7 @@ const nlTest sTests[] = NL_TEST_DEF("TestPostSubscribeRoundtripChunkStatusReportTimeout", chip::app::TestReadInteraction::TestPostSubscribeRoundtripChunkStatusReportTimeout), NL_TEST_DEF("TestPostSubscribeRoundtripChunkReportTimeout", chip::app::TestReadInteraction::TestPostSubscribeRoundtripChunkReportTimeout), NL_TEST_DEF("TestReadShutdown", chip::app::TestReadInteraction::TestReadShutdown), + NL_TEST_DEF("TestSubscriptionReportWithDefunctSession", chip::app::TestReadInteraction::TestSubscriptionReportWithDefunctSession), NL_TEST_SENTINEL() }; // clang-format on