From f1484c02b79e27580dcee08b4fab50c795d60b1b Mon Sep 17 00:00:00 2001 From: Mathieu Kardous Date: Fri, 26 Jul 2024 13:45:57 -0400 Subject: [PATCH] Refactor ICDManager init to leverage a builder pattern --- src/app/icd/server/ICDManager.cpp | 34 +++++--------- src/app/icd/server/ICDManager.h | 49 +++++++++++++++++++-- src/app/icd/server/tests/TestICDManager.cpp | 25 +++++++++-- src/app/server/Server.cpp | 12 ++++- 4 files changed, 89 insertions(+), 31 deletions(-) diff --git a/src/app/icd/server/ICDManager.cpp b/src/app/icd/server/ICDManager.cpp index 278ea34c87783a..2ba08990aef4b6 100644 --- a/src/app/icd/server/ICDManager.cpp +++ b/src/app/icd/server/ICDManager.cpp @@ -52,17 +52,19 @@ using chip::Protocols::InteractionModel::Status; static_assert(UINT8_MAX >= CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS, "ICDManager::mOpenExchangeContextCount cannot hold count for the max exchange count"); -void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, Crypto::SymmetricKeystore * symmetricKeystore, - Messaging::ExchangeManager * exchangeManager, SubscriptionsInfoProvider * subInfoProvider, - ICDCheckInBackOffStrategy * strategy) +void ICDManager::Init() { #if CHIP_CONFIG_ENABLE_ICD_CIP - VerifyOrDie(storage != nullptr); - VerifyOrDie(fabricTable != nullptr); - VerifyOrDie(symmetricKeystore != nullptr); - VerifyOrDie(exchangeManager != nullptr); - VerifyOrDie(subInfoProvider != nullptr); - VerifyOrDie(strategy != nullptr); + VerifyOrDie(mStorage != nullptr); + VerifyOrDie(mFabricTable != nullptr); + VerifyOrDie(mSymmetricKeystore != nullptr); + VerifyOrDie(mExchangeManager != nullptr); + VerifyOrDie(mSubInfoProvider != nullptr); + VerifyOrDie(mICDCheckInBackOffStrategy != nullptr); + + VerifyOrDie(ICDConfigurationData::GetInstance().GetICDCounter().Init(mStorage, DefaultStorageKeyAllocator::ICDCheckInCounter(), + ICDConfigurationData::kICDCounterPersistenceIncrement) == + CHIP_NO_ERROR); #endif // CHIP_CONFIG_ENABLE_ICD_CIP #if CHIP_CONFIG_ENABLE_ICD_LIT @@ -84,19 +86,6 @@ void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricT VerifyOrDie(ICDNotifier::GetInstance().Subscribe(this) == CHIP_NO_ERROR); -#if CHIP_CONFIG_ENABLE_ICD_CIP - mStorage = storage; - mFabricTable = fabricTable; - mSymmetricKeystore = symmetricKeystore; - mExchangeManager = exchangeManager; - mSubInfoProvider = subInfoProvider; - mICDCheckInBackOffStrategy = strategy; - - VerifyOrDie(ICDConfigurationData::GetInstance().GetICDCounter().Init(mStorage, DefaultStorageKeyAllocator::ICDCheckInCounter(), - ICDConfigurationData::kICDCounterPersistenceIncrement) == - CHIP_NO_ERROR); -#endif // CHIP_CONFIG_ENABLE_ICD_CIP - UpdateICDMode(); UpdateOperationState(OperationalState::IdleMode); } @@ -197,7 +186,6 @@ void ICDManager::SendCheckInMsgs() continue; } - // Validate Check-In BackOff strategy if we should send a Check-In message. if (!mICDCheckInBackOffStrategy->ShouldSendCheckInMessage(entry)) { // continue to next entry diff --git a/src/app/icd/server/ICDManager.h b/src/app/icd/server/ICDManager.h index a697071bd98dd1..4ec1dfe65231d3 100644 --- a/src/app/icd/server/ICDManager.h +++ b/src/app/icd/server/ICDManager.h @@ -115,9 +115,52 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler ICDManager() = default; ~ICDManager() = default; - void Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, Crypto::SymmetricKeystore * symmetricKeyStore, - Messaging::ExchangeManager * exchangeManager, SubscriptionsInfoProvider * subInfoProvider, - ICDCheckInBackOffStrategy * strategy); + /* + Builder function to set all necessary members for the ICDManager class + */ + +#if CHIP_CONFIG_ENABLE_ICD_CIP + ICDManager & SetPersistentStorageDelegate(PersistentStorageDelegate * storage) + { + mStorage = storage; + return *this; + }; + + ICDManager & SetFabricTable(FabricTable * fabricTable) + { + mFabricTable = fabricTable; + return *this; + }; + + ICDManager & SetSymmetricKeyStore(Crypto::SymmetricKeystore * symmetricKeystore) + { + mSymmetricKeystore = symmetricKeystore; + return *this; + }; + + ICDManager & SetExchangeManager(Messaging::ExchangeManager * exchangeManager) + { + mExchangeManager = exchangeManager; + return *this; + }; + + ICDManager & SetSubscriptionsInfoProvider(SubscriptionsInfoProvider * subInfoProvider) + { + mSubInfoProvider = subInfoProvider; + return *this; + }; + + ICDManager & SetICDCheckInBackOffStrategy(ICDCheckInBackOffStrategy * strategy) + { + mICDCheckInBackOffStrategy = strategy; + return *this; + }; +#endif // CHIP_CONFIG_ENABLE_ICD_CIP + + /** + * @brief Validates that the ICDManager has all the necessary members to function and initializes the class + */ + void Init(); void Shutdown(); /** diff --git a/src/app/icd/server/tests/TestICDManager.cpp b/src/app/icd/server/tests/TestICDManager.cpp index f4b5db2eb42c2a..9b5cb532e1b69c 100644 --- a/src/app/icd/server/tests/TestICDManager.cpp +++ b/src/app/icd/server/tests/TestICDManager.cpp @@ -198,7 +198,14 @@ class TestICDManager : public Test::LoopbackMessagingContext mICDStateObserver.ResetAll(); mICDManager.RegisterObserver(&mICDStateObserver); - mICDManager.Init(&testStorage, &GetFabricTable(), &mKeystore, &GetExchangeManager(), &mSubInfoProvider, &mStrategy); + + mICDManager.SetPersistentStorageDelegate(&testStorage) + .SetFabricTable(&GetFabricTable()) + .SetSymmetricKeyStore(&mKeystore) + .SetExchangeManager(&GetExchangeManager()) + .SetSubscriptionsInfoProvider(&mSubInfoProvider) + .SetICDCheckInBackOffStrategy(&mStrategy) + .Init(); } // Performs teardown for each individual test in the test suite @@ -570,7 +577,13 @@ TEST_F(TestICDManager, TestICDCounter) // Shut down and reinit ICDManager to increment counter mICDManager.Shutdown(); - mICDManager.Init(&(testStorage), &GetFabricTable(), &(mKeystore), &GetExchangeManager(), &(mSubInfoProvider), &mStrategy); + mICDManager.SetPersistentStorageDelegate(&testStorage) + .SetFabricTable(&GetFabricTable()) + .SetSymmetricKeyStore(&mKeystore) + .SetExchangeManager(&GetExchangeManager()) + .SetSubscriptionsInfoProvider(&mSubInfoProvider) + .SetICDCheckInBackOffStrategy(&mStrategy) + .Init(); mICDManager.RegisterObserver(&(mICDStateObserver)); EXPECT_EQ(counter + ICDConfigurationData::kICDCounterPersistenceIncrement, @@ -975,7 +988,13 @@ TEST_F(TestICDManager, TestICDStateObserverOnICDModeChangeOnInit) // Shut down and reinit ICDManager - We should go to LIT mode since we have a registration mICDManager.Shutdown(); mICDManager.RegisterObserver(&(mICDStateObserver)); - mICDManager.Init(&testStorage, &GetFabricTable(), &mKeystore, &GetExchangeManager(), &mSubInfoProvider, &mStrategy); + mICDManager.SetPersistentStorageDelegate(&testStorage) + .SetFabricTable(&GetFabricTable()) + .SetSymmetricKeyStore(&mKeystore) + .SetExchangeManager(&GetExchangeManager()) + .SetSubscriptionsInfoProvider(&mSubInfoProvider) + .SetICDCheckInBackOffStrategy(&mStrategy) + .Init(); // We have a registration, transition to LIT mode EXPECT_TRUE(mICDStateObserver.mOnICDModeChangeCalled); diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 23d3054179edaa..33f657f7ec1030 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -359,8 +359,16 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams) mICDManager.RegisterObserver(mReportScheduler); mICDManager.RegisterObserver(&app::DnssdServer::Instance()); - mICDManager.Init(mDeviceStorage, &GetFabricTable(), mSessionKeystore, &mExchangeMgr, - chip::app::InteractionModelEngine::GetInstance(), initParams.icdCheckInBackOffStrategy); +#if CHIP_CONFIG_ENABLE_ICD_CIP + mICDManager.SetPersistentStorageDelegate(mDeviceStorage) + .SetFabricTable(&GetFabricTable()) + .SetSymmetricKeyStore(mSessionKeystore) + .SetExchangeManager(&mExchangeMgr) + .SetSubscriptionsInfoProvider(chip::app::InteractionModelEngine::GetInstance()) + .SetICDCheckInBackOffStrategy(initParams.icdCheckInBackOffStrategy); + +#endif // CHIP_CONFIG_ENABLE_ICD_CIP + mICDManager.Init(); // Register Test Event Trigger Handler if (mTestEventTriggerDelegate != nullptr)