From 0b4d67aab7949dc5542c3e28706d318fb9481b71 Mon Sep 17 00:00:00 2001 From: Jerry Johns Date: Tue, 29 Mar 2022 22:24:11 -0700 Subject: [PATCH] Add Application Callback for Subscription Handling (#16517) * Add Application Callback for Subscription Handling Adds a new ReadHandler::ApplicationCallback that permits notifying the application of notable happenings on subscriptions as well as giving them a chance to alter the min/max intervals setup as part of subscriptions. Tests: - Add some unit tests to TestRead to validate the positive/negative behaviors added. * Apply suggestions from code review Co-authored-by: Boris Zbarsky * Review feedback. * Removing a bunch of accidentally added files... Co-authored-by: Boris Zbarsky --- src/app/EventLoggingTypes.h | 8 +- src/app/EventManagement.cpp | 2 +- src/app/EventManagement.h | 2 +- src/app/InteractionModelEngine.h | 17 +- src/app/ReadClient.cpp | 3 +- src/app/ReadClient.h | 15 ++ src/app/ReadHandler.cpp | 40 +++- src/app/ReadHandler.h | 114 +++++++++-- src/app/reporting/Engine.cpp | 12 +- src/app/reporting/Engine.h | 3 +- src/app/tests/TestReadInteraction.cpp | 3 +- src/app/tests/TestReportingEngine.cpp | 3 +- src/controller/CHIPCluster.h | 2 +- src/controller/ReadInteraction.h | 4 + src/controller/TypedReadCallback.h | 4 +- src/controller/tests/data_model/TestRead.cpp | 196 ++++++++++++++++++- 16 files changed, 383 insertions(+), 45 deletions(-) diff --git a/src/app/EventLoggingTypes.h b/src/app/EventLoggingTypes.h index a3805a81a89a06..c4fa7ca22f8400 100644 --- a/src/app/EventLoggingTypes.h +++ b/src/app/EventLoggingTypes.h @@ -152,10 +152,10 @@ struct EventLoadOutContext EventNumber mStartingEventNumber = 0; Timestamp mPreviousTime; Timestamp mCurrentTime; - EventNumber mCurrentEventNumber = 0; - size_t mEventCount = 0; - ObjectList * mpInterestedEventPaths = nullptr; - bool mFirst = true; + EventNumber mCurrentEventNumber = 0; + size_t mEventCount = 0; + const ObjectList * mpInterestedEventPaths = nullptr; + bool mFirst = true; Access::SubjectDescriptor mSubjectDescriptor; }; } // namespace app diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index fa72c5355c2c43..ad657d819ddb7a 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -733,7 +733,7 @@ CHIP_ERROR EventManagement::CopyEventsSince(const TLVReader & aReader, size_t aD return err; } -CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ObjectList * apEventPathList, +CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, const ObjectList * apEventPathList, EventNumber & aEventMin, size_t & aEventCount, const Access::SubjectDescriptor & aSubjectDescriptor) { diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index e4dbe9a6bbc530..60db1cd9a3e5df 100644 --- a/src/app/EventManagement.h +++ b/src/app/EventManagement.h @@ -340,7 +340,7 @@ class EventManagement * available. * */ - CHIP_ERROR FetchEventsSince(chip::TLV::TLVWriter & aWriter, ObjectList * apEventPathList, + CHIP_ERROR FetchEventsSince(chip::TLV::TLVWriter & aWriter, const ObjectList * apEventPathList, EventNumber & aEventMin, size_t & aEventCount, const Access::SubjectDescriptor & aSubjectDescriptor); diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 6f1482c63ea769..ffe0acc7a3f546 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -68,7 +68,9 @@ namespace app { * handlers * */ -class InteractionModelEngine : public Messaging::ExchangeDelegate, public CommandHandler::Callback, public ReadHandler::Callback +class InteractionModelEngine : public Messaging::ExchangeDelegate, + public CommandHandler::Callback, + public ReadHandler::ManagementCallback { public: /** @@ -157,6 +159,15 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman CommandHandlerInterface * FindCommandHandler(EndpointId endpointId, ClusterId clusterId); void UnregisterCommandHandlers(EndpointId endpointId); + /* + * Register an application callback to be notified of notable events when handling reads/subscribes. + */ + void RegisterReadHandlerAppCallback(ReadHandler::ApplicationCallback * mpApplicationCallback) + { + mpReadHandlerApplicationCallback = mpApplicationCallback; + } + void UnregisterReadHandlerAppCallback() { mpReadHandlerApplicationCallback = nullptr; } + /** * Called when a timed interaction has failed (i.e. the exchange it was * happening on has closed while the exchange delegate was the timed @@ -252,6 +263,8 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman void OnDone(CommandHandler & apCommandObj) override; void OnDone(ReadHandler & apReadObj) override; + ReadHandler::ApplicationCallback * GetAppCallback() override { return mpReadHandlerApplicationCallback; } + /** * Called when Interaction Model receives a Command Request message. Errors processing * the Command Request are handled entirely within this function. The caller pre-sets status to failure and the callee is @@ -325,6 +338,8 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman ObjectPool, CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS> mDataVersionFilterPool; ReadClient * mpActiveReadClientList = nullptr; + ReadHandler::ApplicationCallback * mpReadHandlerApplicationCallback = nullptr; + #if CONFIG_IM_BUILD_FOR_UNIT_TEST int mReadHandlerCapacityOverride = -1; #endif diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index e68b48bf2d69e9..47a097b7b310da 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -760,10 +760,11 @@ CHIP_ERROR ReadClient::ProcessSubscribeResponse(System::PacketBufferHandle && aP ChipLogValueX64(mPeerNodeId)); ReturnErrorOnFailure(subscribeResponse.ExitContainer()); - mpCallback.OnSubscriptionEstablished(subscriptionId); MoveToState(ClientState::SubscriptionActive); + mpCallback.OnSubscriptionEstablished(subscriptionId); + if (mReadPrepareParams.mResubscribePolicy != nullptr) { mNumRetries = 0; diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 03320c27d435eb..b23326722683b9 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -238,6 +238,21 @@ class ReadClient : public Messaging::ExchangeDelegate bool IsReadType() { return mInteractionType == InteractionType::Read; } bool IsSubscriptionType() const { return mInteractionType == InteractionType::Subscribe; }; + /* + * Retrieve the reporting intervals associated with an active subscription. This should only be called if we're of subscription + * interaction type and after a subscription has been established. + */ + CHIP_ERROR GetReportingIntervals(uint16_t & aMinIntervalFloorSeconds, uint16_t & aMaxIntervalCeilingSeconds) const + { + VerifyOrReturnError(IsSubscriptionType(), CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsSubscriptionIdle(), CHIP_ERROR_INCORRECT_STATE); + + aMinIntervalFloorSeconds = mMinIntervalFloorSeconds; + aMaxIntervalCeilingSeconds = mMaxIntervalCeilingSeconds; + + return CHIP_NO_ERROR; + } + ReadClient * GetNextClient() { return mpNext; } void SetNextClient(ReadClient * apClient) { mpNext = apClient; } diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index b5e7ba7d033872..f409fc0aceb7d6 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -36,8 +36,9 @@ namespace chip { namespace app { -ReadHandler::ReadHandler(Callback & apCallback, Messaging::ExchangeContext * apExchangeContext, InteractionType aInteractionType) : - mCallback(apCallback) +ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeContext * apExchangeContext, + InteractionType aInteractionType) : + mManagementCallback(apCallback) { mpExchangeMgr = apExchangeContext->GetExchangeMgr(); mpExchangeCtx = apExchangeContext; @@ -80,6 +81,12 @@ void ReadHandler::Abort(bool aCalledFromDestructor) ReadHandler::~ReadHandler() { + auto * appCallback = mManagementCallback.GetAppCallback(); + if (mActiveSubscription && appCallback) + { + appCallback->OnSubscriptionTerminated(*this); + } + Abort(true); if (IsType(InteractionType::Subscribe)) @@ -109,7 +116,7 @@ void ReadHandler::Close() } MoveToState(HandlerState::AwaitingDestruction); - mCallback.OnDone(*this); + mManagementCallback.OnDone(*this); } CHIP_ERROR ReadHandler::OnInitialRequest(System::PacketBufferHandle && aPayload) @@ -159,10 +166,18 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange { if (IsPriming()) { - err = SendSubscribeResponse(); + err = SendSubscribeResponse(); + mpExchangeCtx = nullptr; SuccessOrExit(err); + mActiveSubscription = true; + + auto * appCallback = mManagementCallback.GetAppCallback(); + if (appCallback) + { + appCallback->OnSubscriptionEstablished(*this); + } } else { @@ -697,6 +712,23 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP ReturnErrorOnFailure(subscribeRequestParser.GetMinIntervalFloorSeconds(&mMinIntervalFloorSeconds)); ReturnErrorOnFailure(subscribeRequestParser.GetMaxIntervalCeilingSeconds(&mMaxIntervalCeilingSeconds)); VerifyOrReturnError(mMinIntervalFloorSeconds <= mMaxIntervalCeilingSeconds, CHIP_ERROR_INVALID_ARGUMENT); + + // + // Notify the application (if requested) of the impending subscription and check whether we should still proceed to set it up. + // This also provides the application an opportunity to modify the negotiated min/max intervals set above. + // + auto * appCallback = mManagementCallback.GetAppCallback(); + if (appCallback) + { + if (appCallback->OnSubscriptionRequested(*this, *mpExchangeCtx->GetSessionHandle()->AsSecureSession()) != CHIP_NO_ERROR) + { + return CHIP_ERROR_TRANSACTION_CANCELED; + } + } + + ChipLogProgress(DataManagement, "Final negotiated min/max parameters: Min = %ds, Max = %ds", mMinIntervalFloorSeconds, + mMaxIntervalCeilingSeconds); + ReturnErrorOnFailure(subscribeRequestParser.GetIsFabricFiltered(&mIsFabricFiltered)); ReturnErrorOnFailure(Crypto::DRBG_get_bytes(reinterpret_cast(&mSubscriptionId), sizeof(mSubscriptionId))); ReturnErrorOnFailure(subscribeRequestParser.ExitContainer()); diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 78062a16f84777..8b8b4749333721 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -54,7 +54,10 @@ namespace app { // namespace reporting { class Engine; -} +class TestReportingEngine; +} // namespace reporting + +class InteractionModelEngine; /** * @class ReadHandler @@ -74,18 +77,80 @@ class ReadHandler : public Messaging::ExchangeDelegate Subscribe, }; - class Callback + /* + * A callback used to interact with the application. + */ + class ApplicationCallback { public: - virtual ~Callback() = default; + virtual ~ApplicationCallback() = default; + + /* + * Called right after a SubscribeRequest has been parsed and processed. This notifies an interested application + * of a subscription that is about to be established. It also provides an avenue for altering the parameters of the + * subscription (specifically, the min/max negotiated intervals) or even outright rejecting the subscription for + * application-specific reasons. + * + * TODO: Need a new IM status code to convey application-rejected subscribes. Currently, a Failure IM status code is sent + * back to the subscriber, which isn't sufficient. + * + * To reject the subscription, a CHIP_ERROR code that is not equivalent to CHIP_NO_ERROR should be returned. + * + * More information about the set of paths associated with this subscription can be retrieved by calling the appropriate + * Get* methods below. + * + * aReadHandler: Reference to the ReadHandler associated with the subscription. + * aSecureSession: A reference to the underlying secure session associated with the subscription. + * + */ + virtual CHIP_ERROR OnSubscriptionRequested(ReadHandler & aReadHandler, Transport::SecureSession & aSecureSession) + { + return CHIP_NO_ERROR; + } + + /* + * Called after a subscription has been fully established. + */ + virtual void OnSubscriptionEstablished(ReadHandler & aReadHandler){}; + + /* + * Called right before a subscription is about to get terminated. This is only called on subscriptions that were terminated + * after they had been fully established (and therefore had called OnSubscriptionEstablished). + * OnSubscriptionEstablishment(). + */ + virtual void OnSubscriptionTerminated(ReadHandler & aReadHandler){}; + }; + + /* + * A callback used to manage the lifetime of the ReadHandler object. + */ + class ManagementCallback + { + public: + virtual ~ManagementCallback() = default; /* * Method that signals to a registered callback that this object * has completed doing useful work and is now safe for release/destruction. */ virtual void OnDone(ReadHandler & apReadHandlerObj) = 0; + + /* + * Retrieve the ApplicationCallback (if a valid one exists) from our management entity. This avoids + * storing multiple references to the application provided callback and having to subsequently manage lifetime + * issues w.r.t the ReadHandler itself. + */ + virtual ApplicationCallback * GetAppCallback() = 0; }; + /* + * Destructor - as part of destruction, it will abort the exchange context + * if a valid one still exists. + * + * See Abort() for details on when that might occur. + */ + ~ReadHandler() override; + /** * * Constructor. @@ -93,15 +158,35 @@ class ReadHandler : public Messaging::ExchangeDelegate * The callback passed in has to outlive this handler object. * */ - ReadHandler(Callback & apCallback, Messaging::ExchangeContext * apExchangeContext, InteractionType aInteractionType); + ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeContext * apExchangeContext, InteractionType aInteractionType); + + const ObjectList * GetAttributePathList() const { return mpAttributePathList; } + const ObjectList * GetEventPathList() const { return mpEventPathList; } + const ObjectList * GetDataVersionFilterList() const { return mpDataVersionFilterList; } + + void GetReportingIntervals(uint16_t & aMinInterval, uint16_t & aMaxInterval) const + { + aMinInterval = mMinIntervalFloorSeconds; + aMaxInterval = mMaxIntervalCeilingSeconds; + } /* - * Destructor - as part of destruction, it will abort the exchange context - * if a valid one still exists. - * - * See Abort() for details on when that might occur. + * Set the reporting intervals for the subscription. This SHALL only be called + * from the OnSubscriptionRequested callback above. */ - ~ReadHandler() override; + CHIP_ERROR SetReportingIntervals(uint16_t aMinInterval, uint16_t aMaxInterval) + { + VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(aMinInterval <= aMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); + + mMinIntervalFloorSeconds = aMinInterval; + mMaxIntervalCeilingSeconds = aMaxInterval; + return CHIP_NO_ERROR; + } + +private: + PriorityLevel GetCurrentPriority() const { return mCurrentPriority; } + EventNumber & GetEventMin() { return mEventMin; } /** * Process a read/subscribe request. Parts of the processing may end up being asynchronous, but the ReadHandler @@ -131,6 +216,7 @@ class ReadHandler : public Messaging::ExchangeDelegate */ bool IsFromSubscriber(Messaging::ExchangeContext & apExchangeContext) const; + bool IsIdle() const { return mState == HandlerState::Idle; } bool IsReportable() const { return mState == HandlerState::GeneratingReports && !mHoldReport && (IsDirty() || !mHoldSync); } bool IsGeneratingReports() const { return mState == HandlerState::GeneratingReports; } bool IsAwaitingReportResponse() const { return mState == HandlerState::AwaitingReportResponse; } @@ -139,11 +225,6 @@ class ReadHandler : public Messaging::ExchangeDelegate void ResetPathIterator(); CHIP_ERROR ProcessDataVersionFilterList(DataVersionFilterIBs::Parser & aDataVersionFilterListParser); - ObjectList * GetAttributePathList() { return mpAttributePathList; } - ObjectList * GetEventPathList() { return mpEventPathList; } - ObjectList * GetDataVersionFilterList() const { return mpDataVersionFilterList; } - EventNumber & GetEventMin() { return mEventMin; } - PriorityLevel GetCurrentPriority() { return mCurrentPriority; } // if current priority is in the middle, it has valid snapshoted last event number, it check cleaness via comparing // with snapshotted last event number. if current priority is in the end, no valid @@ -185,8 +266,8 @@ class ReadHandler : public Messaging::ExchangeDelegate uint32_t GetLastWrittenEventsBytes() const { return mLastWrittenEventsBytes; } CHIP_ERROR SendStatusReport(Protocols::InteractionModel::Status aStatus); -private: friend class TestReadInteraction; + friend class chip::app::reporting::TestReportingEngine; // // The engine needs to be able to Abort/Close a ReadHandler instance upon completion of work for a given read/subscribe @@ -194,6 +275,7 @@ class ReadHandler : public Messaging::ExchangeDelegate // should really be taking application usage considerations as well. Hence, make it a friend. // friend class chip::app::reporting::Engine; + friend class chip::app::InteractionModelEngine; enum class HandlerState { @@ -259,7 +341,7 @@ class ReadHandler : public Messaging::ExchangeDelegate // The last schedule event number snapshoted in the beginning when preparing to fill new events to reports EventNumber mLastScheduledEventNumber = 0; Messaging::ExchangeManager * mpExchangeMgr = nullptr; - Callback & mCallback; + ManagementCallback & mManagementCallback; // Tracks whether we're in the initial phase of receiving priming // reports, which is always true for reads and true for subscriptions diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index 86dc17cfefc95c..2880979be4b689 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -50,7 +50,7 @@ void Engine::Shutdown() mGlobalDirtySet.ReleaseAll(); } -bool Engine::IsClusterDataVersionMatch(ObjectList * aDataVersionFilterList, +bool Engine::IsClusterDataVersionMatch(const ObjectList * aDataVersionFilterList, const ConcreteReadAttributePath & aPath) { bool existPathMatch = false; @@ -296,11 +296,11 @@ CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder CHIP_ERROR err = CHIP_NO_ERROR; size_t eventCount = 0; TLV::TLVWriter backup; - bool eventClean = true; - ObjectList * eventList = apReadHandler->GetEventPathList(); - EventNumber & eventMin = apReadHandler->GetEventMin(); - EventManagement & eventManager = EventManagement::GetInstance(); - bool hasMoreChunks = false; + bool eventClean = true; + const auto * eventList = apReadHandler->GetEventPathList(); + auto & eventMin = apReadHandler->GetEventMin(); + EventManagement & eventManager = EventManagement::GetInstance(); + bool hasMoreChunks = false; aReportDataBuilder.Checkpoint(backup); diff --git a/src/app/reporting/Engine.h b/src/app/reporting/Engine.h index bc6980959c1576..1fcd8fc283a682 100644 --- a/src/app/reporting/Engine.h +++ b/src/app/reporting/Engine.h @@ -156,7 +156,8 @@ class Engine // of those will fail to match. This function should return false if either nothing in the list matches the given // endpoint+cluster in the path or there is an entry in the list that matches the endpoint+cluster in the path but does not // match the current data version of that cluster. - bool IsClusterDataVersionMatch(ObjectList * aDataVersionFilterList, const ConcreteReadAttributePath & aPath); + bool IsClusterDataVersionMatch(const ObjectList * aDataVersionFilterList, + const ConcreteReadAttributePath & aPath); /** * Check all active subscription, if the subscription has no paths that intersect with global dirty set, diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 7cf1032ae3d175..5ae2a69d0a8a78 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -214,10 +214,11 @@ class MockInteractionModelApp : public chip::app::ReadClient::Callback // The typical callback implementor is the engine, but that would proceed to return the object // back to the handler pool (which we obviously don't want in this case). This just no-ops those calls. // -class NullReadHandlerCallback : public chip::app::ReadHandler::Callback +class NullReadHandlerCallback : public chip::app::ReadHandler::ManagementCallback { public: void OnDone(chip::app::ReadHandler & apReadHandlerObj) override {} + chip::app::ReadHandler::ApplicationCallback * GetAppCallback() override { return nullptr; } }; } // namespace diff --git a/src/app/tests/TestReportingEngine.cpp b/src/app/tests/TestReportingEngine.cpp index f4e22fa8a30f79..63627f36216687 100644 --- a/src/app/tests/TestReportingEngine.cpp +++ b/src/app/tests/TestReportingEngine.cpp @@ -66,10 +66,11 @@ class TestExchangeDelegate : public Messaging::ExchangeDelegate void OnResponseTimeout(Messaging::ExchangeContext * ec) override {} }; -class DummyDelegate : public ReadHandler::Callback +class DummyDelegate : public ReadHandler::ManagementCallback { public: void OnDone(ReadHandler & apHandler) override {} + chip::app::ReadHandler::ApplicationCallback * GetAppCallback() override { return nullptr; } }; void TestReportingEngine::TestBuildAndSendSingleReportData(nlTestSuite * apSuite, void * apContext) diff --git a/src/controller/CHIPCluster.h b/src/controller/CHIPCluster.h index bfeb9b289f00b2..01c7b4cc39cc8d 100644 --- a/src/controller/CHIPCluster.h +++ b/src/controller/CHIPCluster.h @@ -296,7 +296,7 @@ class DLL_EXPORT ClusterBase } }; - auto onSubscriptionEstablishedCb = [context, subscriptionEstablishedCb]() { + auto onSubscriptionEstablishedCb = [context, subscriptionEstablishedCb](const app::ReadClient & readClient) { if (subscriptionEstablishedCb != nullptr) { subscriptionEstablishedCb(context); diff --git a/src/controller/ReadInteraction.h b/src/controller/ReadInteraction.h index aec8717ab07311..fa333682ffb1f4 100644 --- a/src/controller/ReadInteraction.h +++ b/src/controller/ReadInteraction.h @@ -164,6 +164,10 @@ CHIP_ERROR SubscribeAttribute( * A typed way to subscribe to the value of a single attribute. See * documentation for ReadAttribute above for details on how AttributeTypeInfo * works. + * + * A const view-only reference to the underlying ReadClient is passed in through the OnSubscriptionEstablishedCallbackType + * argument. This reference is valid until the error callback is invoked at which point, this reference is no longer valid + * and should not be used any more. */ template CHIP_ERROR SubscribeAttribute( diff --git a/src/controller/TypedReadCallback.h b/src/controller/TypedReadCallback.h index da1c5f241a314c..289868a3c0f87e 100644 --- a/src/controller/TypedReadCallback.h +++ b/src/controller/TypedReadCallback.h @@ -52,7 +52,7 @@ class TypedReadAttributeCallback final : public app::ReadClient::Callback std::function; using OnErrorCallbackType = std::function; using OnDoneCallbackType = std::function; - using OnSubscriptionEstablishedCallbackType = std::function; + using OnSubscriptionEstablishedCallbackType = std::function; TypedReadAttributeCallback(ClusterId aClusterId, AttributeId aAttributeId, OnSuccessCallbackType aOnSuccess, OnErrorCallbackType aOnError, OnDoneCallbackType aOnDone, @@ -102,7 +102,7 @@ class TypedReadAttributeCallback final : public app::ReadClient::Callback { if (mOnSubscriptionEstablished) { - mOnSubscriptionEstablished(); + mOnSubscriptionEstablished(*mReadClient.get()); } } diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 9e325a26a444e0..7ce7700f1d16d9 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -16,6 +16,7 @@ * limitations under the License. */ +#include "transport/SecureSession.h" #include #include #include @@ -156,7 +157,7 @@ bool IsClusterDataVersionEqual(const ConcreteClusterPath & aConcreteClusterPath, namespace { -class TestReadInteraction +class TestReadInteraction : public app::ReadHandler::ApplicationCallback { public: TestReadInteraction() {} @@ -169,22 +170,51 @@ class TestReadInteraction static void TestReadFabricScopedWithoutFabricFilter(nlTestSuite * apSuite, void * apContext); static void TestReadFabricScopedWithFabricFilter(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_MultipleSubscriptions(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionAppRejection(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_MultipleReads(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_OneSubscribeMultipleReads(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_TwoSubscribesMultipleReads(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_MultipleSubscriptionsWithDataVersionFilter(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionAlteredReportingIntervals(nlTestSuite * apSuite, void * apContext); static void TestReadHandlerResourceExhaustion_MultipleSubscriptions(nlTestSuite * apSuite, void * apContext); static void TestReadHandlerResourceExhaustion_MultipleReads(nlTestSuite * apSuite, void * apContext); private: + static constexpr uint16_t kTestMinInterval = 33; + static constexpr uint16_t kTestMaxInterval = 66; + + CHIP_ERROR OnSubscriptionRequested(app::ReadHandler & aReadHandler, Transport::SecureSession & aSecureSession) + { + VerifyOrReturnError(!mEmitSubscriptionError, CHIP_ERROR_INVALID_ARGUMENT); + + if (mAlterSubscriptionIntervals) + { + ReturnErrorOnFailure(aReadHandler.SetReportingIntervals(kTestMinInterval, kTestMaxInterval)); + } + + return CHIP_NO_ERROR; + } + + void OnSubscriptionEstablished(app::ReadHandler & aReadHandler) { mNumActiveSubscriptions++; } + + void OnSubscriptionTerminated(app::ReadHandler & aReadHandler) { mNumActiveSubscriptions--; } + // Issue the given number of reads in parallel and wait for them all to // succeed. static void MultipleReadHelper(nlTestSuite * apSuite, TestContext & aCtx, size_t aReadCount); + // Establish the given number of subscriptions, then issue the given number // of reads in parallel and wait for them all to succeed. static void SubscribeThenReadHelper(nlTestSuite * apSuite, TestContext & aCtx, size_t aSubscribeCount, size_t aReadCount); + +private: + bool mEmitSubscriptionError = false; + int32_t mNumActiveSubscriptions = 0; + bool mAlterSubscriptionIntervals = false; }; +TestReadInteraction gTestReadInteraction; + void TestReadInteraction::TestReadAttributeResponse(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); @@ -408,7 +438,15 @@ void TestReadInteraction::TestReadHandler_MultipleSubscriptions(nlTestSuite * ap NL_TEST_ASSERT(apSuite, false); }; - auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls]() { numSubscriptionEstablishedCalls++; }; + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls](const app::ReadClient & readClient) { + numSubscriptionEstablishedCalls++; + }; + + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); // // Try to issue parallel subscriptions that will exceed the value for CHIP_IM_MAX_NUM_READ_HANDLER. @@ -427,10 +465,151 @@ void TestReadInteraction::TestReadHandler_MultipleSubscriptions(nlTestSuite * ap NL_TEST_ASSERT(apSuite, numSuccessCalls == (CHIP_IM_MAX_NUM_READ_HANDLER + 1)); NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == (CHIP_IM_MAX_NUM_READ_HANDLER + 1)); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == (CHIP_IM_MAX_NUM_READ_HANDLER + 1)); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); +} + +void TestReadInteraction::TestReadHandler_SubscriptionAppRejection(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + uint32_t numSuccessCalls = 0; + uint32_t numFailureCalls = 0; + uint32_t numSubscriptionEstablishedCalls = 0; + + responseDirective = kSendDataResponse; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { + numSuccessCalls++; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&numFailureCalls](const app::ConcreteDataAttributePath * attributePath, CHIP_ERROR aError) { + numFailureCalls++; + }; + + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls](const app::ReadClient & readClient) { + numSubscriptionEstablishedCalls++; + }; + + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); + + // + // Test the application rejecting subscriptions. + // + gTestReadInteraction.mEmitSubscriptionError = true; + + NL_TEST_ASSERT(apSuite, + Controller::SubscribeAttribute( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 10, + onSubscriptionEstablishedCb, false, true) == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, numSuccessCalls == 0); + + // + // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute + // implementation. + // + NL_TEST_ASSERT(apSuite, numFailureCalls == 0); + NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 0); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); + gTestReadInteraction.mEmitSubscriptionError = false; +} + +void TestReadInteraction::TestReadHandler_SubscriptionAlteredReportingIntervals(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + uint32_t numSuccessCalls = 0; + uint32_t numFailureCalls = 0; + uint32_t numSubscriptionEstablishedCalls = 0; + + responseDirective = kSendDataResponse; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { + numSuccessCalls++; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&numFailureCalls](const app::ConcreteDataAttributePath * attributePath, CHIP_ERROR aError) { + numFailureCalls++; + }; + + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls, &apSuite](const app::ReadClient & readClient) { + uint16_t minInterval = 0, maxInterval = 0; + + CHIP_ERROR err = readClient.GetReportingIntervals(minInterval, maxInterval); + + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + NL_TEST_ASSERT(apSuite, minInterval == kTestMinInterval); + NL_TEST_ASSERT(apSuite, maxInterval == kTestMaxInterval); + + numSubscriptionEstablishedCalls++; + }; + + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); + + // + // Test the server-side application altering the subscription intervals. + // + gTestReadInteraction.mAlterSubscriptionIntervals = true; + + NL_TEST_ASSERT(apSuite, + Controller::SubscribeAttribute( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 10, + onSubscriptionEstablishedCb, false, true) == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + // + // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute + // implementation. + // + NL_TEST_ASSERT(apSuite, numSuccessCalls != 0); + NL_TEST_ASSERT(apSuite, numFailureCalls == 0); + NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 1); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 1); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); + gTestReadInteraction.mAlterSubscriptionIntervals = false; } void TestReadInteraction::TestReadHandler_MultipleReads(nlTestSuite * apSuite, void * apContext) @@ -501,7 +680,8 @@ void TestReadInteraction::SubscribeThenReadHelper(nlTestSuite * apSuite, TestCon NL_TEST_ASSERT(apSuite, false); }; - auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls, &apSuite, &aCtx, aSubscribeCount, aReadCount]() { + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls, &apSuite, &aCtx, aSubscribeCount, + aReadCount](const app::ReadClient & readClient) { numSubscriptionEstablishedCalls++; if (numSubscriptionEstablishedCalls == aSubscribeCount) { @@ -588,7 +768,9 @@ void TestReadInteraction::TestReadHandler_MultipleSubscriptionsWithDataVersionFi NL_TEST_ASSERT(apSuite, false); }; - auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls]() { numSubscriptionEstablishedCalls++; }; + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls](const app::ReadClient & readClient) { + numSubscriptionEstablishedCalls++; + }; // // Try to issue parallel subscriptions that will exceed the value for CHIP_IM_MAX_NUM_READ_HANDLER. @@ -639,7 +821,9 @@ void TestReadInteraction::TestReadHandlerResourceExhaustion_MultipleSubscription NL_TEST_ASSERT(apSuite, attributePath == nullptr); }; - auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls]() { numSubscriptionEstablishedCalls++; }; + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls](const app::ReadClient & readClient) { + numSubscriptionEstablishedCalls++; + }; // // Artifically limit the capacity to 2 ReadHandlers. This will also validate reservation of handlers for Reads, @@ -823,6 +1007,7 @@ const nlTest sTests[] = NL_TEST_DEF("TestReadFabricScopedWithoutFabricFilter", TestReadInteraction::TestReadFabricScopedWithoutFabricFilter), NL_TEST_DEF("TestReadFabricScopedWithFabricFilter", TestReadInteraction::TestReadFabricScopedWithFabricFilter), NL_TEST_DEF("TestReadHandler_MultipleSubscriptions", TestReadInteraction::TestReadHandler_MultipleSubscriptions), + NL_TEST_DEF("TestReadHandler_SubscriptionAppRejection", TestReadInteraction::TestReadHandler_SubscriptionAppRejection), NL_TEST_DEF("TestReadHandler_MultipleSubscriptionsWithDataVersionFilter", TestReadInteraction::TestReadHandler_MultipleSubscriptionsWithDataVersionFilter), NL_TEST_DEF("TestReadHandler_MultipleReads", TestReadInteraction::TestReadHandler_MultipleReads), NL_TEST_DEF("TestReadHandler_OneSubscribeMultipleReads", TestReadInteraction::TestReadHandler_OneSubscribeMultipleReads), @@ -830,6 +1015,7 @@ const nlTest sTests[] = NL_TEST_DEF("TestReadHandlerResourceExhaustion_MultipleSubscriptions", TestReadInteraction::TestReadHandlerResourceExhaustion_MultipleSubscriptions), NL_TEST_DEF("TestReadHandlerResourceExhaustion_MultipleReads", TestReadInteraction::TestReadHandlerResourceExhaustion_MultipleReads), NL_TEST_DEF("TestReadAttributeTimeout", TestReadInteraction::TestReadAttributeTimeout), + NL_TEST_DEF("TestReadHandler_SubscriptionAlteredReportingIntervals", TestReadInteraction::TestReadHandler_SubscriptionAlteredReportingIntervals), NL_TEST_SENTINEL() }; // clang-format on