Merge DeviceProxyInitParams with CASEClientInitParams

Damian-Nordic · Damian-Nordic · commit 3639ccb5e9ad · 2022-12-14T11:47:04.000+01:00
Both structures are almost the same and as we tend to pass
more and more interfaces down the stack, translating between
all the different structures becomes cumbersome.
diff --git a/src/app/CASEClient.h b/src/app/CASEClient.h
@@ -34,8 +34,19 @@ struct CASEClientInitParams
     Messaging::ExchangeManager * exchangeMgr                           = nullptr;
     FabricTable * fabricTable                                          = nullptr;
     Credentials::GroupDataProvider * groupDataProvider                 = nullptr;
+    Optional<ReliableMessageProtocolConfig> mrpLocalConfig             = Optional<ReliableMessageProtocolConfig>::Missing();
 
-    Optional<ReliableMessageProtocolConfig> mrpLocalConfig = Optional<ReliableMessageProtocolConfig>::Missing();
+    CHIP_ERROR Validate() const
+    {
+        // sessionResumptionStorage can be nullptr when resumption is disabled.
+        // certificateValidityPolicy is optional, too.
+        ReturnErrorCodeIf(sessionManager == nullptr, CHIP_ERROR_INCORRECT_STATE);
+        ReturnErrorCodeIf(exchangeMgr == nullptr, CHIP_ERROR_INCORRECT_STATE);
+        ReturnErrorCodeIf(fabricTable == nullptr, CHIP_ERROR_INCORRECT_STATE);
+        ReturnErrorCodeIf(groupDataProvider == nullptr, CHIP_ERROR_INCORRECT_STATE);
+
+        return CHIP_NO_ERROR;
+    }
 };
 
 class DLL_EXPORT CASEClient
diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp
@@ -41,7 +41,7 @@ void CASESessionManager::FindOrEstablishSession(const ScopedNodeId & peerId, Cal
     {
         ChipLogDetail(CASESessionManager, "FindOrEstablishSession: No existing OperationalSessionSetup instance found");
 
-        session = mConfig.sessionSetupPool->Allocate(mConfig.sessionInitParams, peerId, this);
+        session = mConfig.sessionSetupPool->Allocate(mConfig.sessionInitParams, mConfig.clientPool, peerId, this);
 
         if (session == nullptr)
         {
@@ -83,7 +83,7 @@ void CASESessionManager::UpdatePeerAddress(ScopedNodeId peerId)
     {
         ChipLogDetail(CASESessionManager, "UpdatePeerAddress: No existing OperationalSessionSetup instance found");
 
-        session = mConfig.sessionSetupPool->Allocate(mConfig.sessionInitParams, peerId, this);
+        session = mConfig.sessionSetupPool->Allocate(mConfig.sessionInitParams, mConfig.clientPool, peerId, this);
         if (session == nullptr)
         {
             ChipLogDetail(CASESessionManager, "UpdatePeerAddress: Failed to allocate OperationalSessionSetup instance");
diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h
@@ -36,7 +36,8 @@ class OperationalSessionSetupPoolDelegate;
 
 struct CASESessionManagerConfig
 {
-    DeviceProxyInitParams sessionInitParams;
+    CASEClientInitParams sessionInitParams;
+    CASEClientPoolDelegate * clientPool                    = nullptr;
     OperationalSessionSetupPoolDelegate * sessionSetupPool = nullptr;
 };
 
diff --git a/src/app/OperationalSessionSetup.cpp b/src/app/OperationalSessionSetup.cpp
@@ -221,18 +221,10 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad
 
 CHIP_ERROR OperationalSessionSetup::EstablishConnection(const ReliableMessageProtocolConfig & config)
 {
-    mCASEClient = mInitParams.clientPool->Allocate();
+    mCASEClient = mClientPool->Allocate();
     ReturnErrorCodeIf(mCASEClient == nullptr, CHIP_ERROR_NO_MEMORY);
 
-    CASEClientInitParams initParams = { mInitParams.sessionManager,
-                                        mInitParams.sessionResumptionStorage,
-                                        mInitParams.certificateValidityPolicy,
-                                        mInitParams.exchangeMgr,
-                                        mFabricTable,
-                                        mInitParams.groupDataProvider,
-                                        mInitParams.mrpLocalConfig };
-
-    CHIP_ERROR err = mCASEClient->EstablishSession(initParams, mPeerId, mDeviceAddress, config, this);
+    CHIP_ERROR err = mCASEClient->EstablishSession(mInitParams, mPeerId, mDeviceAddress, config, this);
     if (err != CHIP_NO_ERROR)
     {
         CleanupCASEClient();
@@ -336,7 +328,7 @@ void OperationalSessionSetup::CleanupCASEClient()
 {
     if (mCASEClient)
     {
-        mInitParams.clientPool->Release(mCASEClient);
+        mClientPool->Release(mCASEClient);
         mCASEClient = nullptr;
     }
 }
@@ -370,7 +362,7 @@ OperationalSessionSetup::~OperationalSessionSetup()
     if (mCASEClient)
     {
         // Make sure we don't leak it.
-        mInitParams.clientPool->Release(mCASEClient);
+        mClientPool->Release(mCASEClient);
     }
 }
 
@@ -388,7 +380,7 @@ CHIP_ERROR OperationalSessionSetup::LookupPeerAddress()
         return CHIP_NO_ERROR;
     }
 
-    auto const * fabricInfo = mFabricTable->FindFabricWithIndex(mPeerId.GetFabricIndex());
+    auto const * fabricInfo = mInitParams.fabricTable->FindFabricWithIndex(mPeerId.GetFabricIndex());
     VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_INVALID_FABRIC_INDEX);
 
     PeerId peerId(fabricInfo->GetCompressedFabricId(), mPeerId.GetNodeId());
diff --git a/src/app/OperationalSessionSetup.h b/src/app/OperationalSessionSetup.h
@@ -45,31 +45,6 @@
 
 namespace chip {
 
-struct DeviceProxyInitParams
-{
-    SessionManager * sessionManager                                    = nullptr;
-    SessionResumptionStorage * sessionResumptionStorage                = nullptr;
-    Credentials::CertificateValidityPolicy * certificateValidityPolicy = nullptr;
-    Messaging::ExchangeManager * exchangeMgr                           = nullptr;
-    FabricTable * fabricTable                                          = nullptr;
-    CASEClientPoolDelegate * clientPool                                = nullptr;
-    Credentials::GroupDataProvider * groupDataProvider                 = nullptr;
-
-    Optional<ReliableMessageProtocolConfig> mrpLocalConfig = Optional<ReliableMessageProtocolConfig>::Missing();
-
-    CHIP_ERROR Validate() const
-    {
-        ReturnErrorCodeIf(sessionManager == nullptr, CHIP_ERROR_INCORRECT_STATE);
-        // sessionResumptionStorage can be nullptr when resumption is disabled
-        ReturnErrorCodeIf(exchangeMgr == nullptr, CHIP_ERROR_INCORRECT_STATE);
-        ReturnErrorCodeIf(fabricTable == nullptr, CHIP_ERROR_INCORRECT_STATE);
-        ReturnErrorCodeIf(groupDataProvider == nullptr, CHIP_ERROR_INCORRECT_STATE);
-        ReturnErrorCodeIf(clientPool == nullptr, CHIP_ERROR_INCORRECT_STATE);
-
-        return CHIP_NO_ERROR;
-    }
-};
-
 class OperationalSessionSetup;
 
 /**
@@ -171,20 +146,20 @@ class DLL_EXPORT OperationalSessionSetup : public SessionDelegate,
 public:
     ~OperationalSessionSetup() override;
 
-    OperationalSessionSetup(DeviceProxyInitParams & params, ScopedNodeId peerId,
+    OperationalSessionSetup(const CASEClientInitParams & params, CASEClientPoolDelegate * clientPool, ScopedNodeId peerId,
                             OperationalSessionReleaseDelegate * releaseDelegate) :
         mSecureSession(*this)
     {
         mInitParams = params;
-        if (params.Validate() != CHIP_NO_ERROR || releaseDelegate == nullptr)
+        if (params.Validate() != CHIP_NO_ERROR || clientPool == nullptr || releaseDelegate == nullptr)
         {
             mState = State::Uninitialized;
             return;
         }
 
+        mClientPool      = clientPool;
         mSystemLayer     = params.exchangeMgr->GetSessionManager()->SystemLayer();
         mPeerId          = peerId;
-        mFabricTable     = params.fabricTable;
         mReleaseDelegate = releaseDelegate;
         mState           = State::NeedsAddress;
         mAddressLookupHandle.SetListener(this);
@@ -260,8 +235,8 @@ class DLL_EXPORT OperationalSessionSetup : public SessionDelegate,
         SecureConnected,  // CASE session established.
     };
 
-    DeviceProxyInitParams mInitParams;
-    FabricTable * mFabricTable = nullptr;
+    CASEClientInitParams mInitParams;
+    CASEClientPoolDelegate * mClientPool = nullptr;
     System::Layer * mSystemLayer;
 
     // mCASEClient is only non-null if we are in State::Connecting or just
diff --git a/src/app/OperationalSessionSetupPool.h b/src/app/OperationalSessionSetupPool.h
@@ -27,8 +27,8 @@ namespace chip {
 class OperationalSessionSetupPoolDelegate
 {
 public:
-    virtual OperationalSessionSetup * Allocate(DeviceProxyInitParams & params, ScopedNodeId peerId,
-                                               OperationalSessionReleaseDelegate * releaseDelegate) = 0;
+    virtual OperationalSessionSetup * Allocate(const CASEClientInitParams & params, CASEClientPoolDelegate * clientPool,
+                                               ScopedNodeId peerId, OperationalSessionReleaseDelegate * releaseDelegate) = 0;
 
     virtual void Release(OperationalSessionSetup * device) = 0;
 
@@ -47,10 +47,10 @@ class OperationalSessionSetupPool : public OperationalSessionSetupPoolDelegate
 public:
     ~OperationalSessionSetupPool() override { mSessionSetupPool.ReleaseAll(); }
 
-    OperationalSessionSetup * Allocate(DeviceProxyInitParams & params, ScopedNodeId peerId,
-                                       OperationalSessionReleaseDelegate * releaseDelegate) override
+    OperationalSessionSetup * Allocate(const CASEClientInitParams & params, CASEClientPoolDelegate * clientPool,
+                                       ScopedNodeId peerId, OperationalSessionReleaseDelegate * releaseDelegate) override
     {
-        return mSessionSetupPool.CreateObject(params, peerId, releaseDelegate);
+        return mSessionSetupPool.CreateObject(params, clientPool, peerId, releaseDelegate);
     }
 
     void Release(OperationalSessionSetup * device) override { mSessionSetupPool.ReleaseObject(device); }
diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp
@@ -290,11 +290,11 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
             .certificateValidityPolicy = mCertificateValidityPolicy,
             .exchangeMgr       = &mExchangeMgr,
             .fabricTable       = &mFabrics,
-            .clientPool        = &mCASEClientPool,
             .groupDataProvider = mGroupsProvider,
             .mrpLocalConfig    = GetLocalMRPConfig(),
         },
-        .sessionSetupPool        = &mSessionSetupPool,
+        .clientPool            = &mCASEClientPool,
+        .sessionSetupPool      = &mSessionSetupPool,
     };
 
     err = mCASESessionManager.Init(&DeviceLayer::SystemLayer(), caseSessionManagerConfig);
diff --git a/src/app/tests/TestOperationalDeviceProxy.cpp b/src/app/tests/TestOperationalDeviceProxy.cpp
@@ -69,7 +69,7 @@ void TestOperationalDeviceProxy_EstablishSessionDirectly(nlTestSuite * inSuite,
     VerifyOrDie(groupDataProvider.Init() == CHIP_NO_ERROR);
     // TODO: Set IPK in groupDataProvider
 
-    DeviceProxyInitParams params = {
+    CASEClientInitParams params = {
         .sessionManager           = &sessionManager,
         .sessionResumptionStorage = &sessionResumptionStorage,
         .exchangeMgr              = &exchangeMgr,
diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp
@@ -245,18 +245,18 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
     stateParams.sessionSetupPool = Platform::New<DeviceControllerSystemStateParams::SessionSetupPool>();
     stateParams.caseClientPool   = Platform::New<DeviceControllerSystemStateParams::CASEClientPool>();
 
-    DeviceProxyInitParams deviceInitParams = {
+    CASEClientInitParams sessionInitParams = {
         .sessionManager           = stateParams.sessionMgr,
         .sessionResumptionStorage = stateParams.sessionResumptionStorage.get(),
         .exchangeMgr              = stateParams.exchangeMgr,
         .fabricTable              = stateParams.fabricTable,
-        .clientPool               = stateParams.caseClientPool,
         .groupDataProvider        = stateParams.groupDataProvider,
         .mrpLocalConfig           = GetLocalMRPConfig(),
     };
 
     CASESessionManagerConfig sessionManagerConfig = {
-        .sessionInitParams = deviceInitParams,
+        .sessionInitParams = sessionInitParams,
+        .clientPool        = stateParams.caseClientPool,
         .sessionSetupPool  = stateParams.sessionSetupPool,
     };
 

Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ void CASESessionManager::FindOrEstablishSession(const ScopedNodeId & peerId, Cal`
`41`	`41`	`{`
`42`	`42`	`ChipLogDetail(CASESessionManager, "FindOrEstablishSession: No existing OperationalSessionSetup instance found");`
`43`	`43`
`44`		`- session = mConfig.sessionSetupPool->Allocate(mConfig.sessionInitParams, peerId, this);`
	`44`	`+ session = mConfig.sessionSetupPool->Allocate(mConfig.sessionInitParams, mConfig.clientPool, peerId, this);`
`45`	`45`
`46`	`46`	`if (session == nullptr)`
`47`	`47`	`{`
`@@ -83,7 +83,7 @@ void CASESessionManager::UpdatePeerAddress(ScopedNodeId peerId)`
`83`	`83`	`{`
`84`	`84`	`ChipLogDetail(CASESessionManager, "UpdatePeerAddress: No existing OperationalSessionSetup instance found");`
`85`	`85`
`86`		`- session = mConfig.sessionSetupPool->Allocate(mConfig.sessionInitParams, peerId, this);`
	`86`	`+ session = mConfig.sessionSetupPool->Allocate(mConfig.sessionInitParams, mConfig.clientPool, peerId, this);`
`87`	`87`	`if (session == nullptr)`
`88`	`88`	`{`
`89`	`89`	`ChipLogDetail(CASESessionManager, "UpdatePeerAddress: Failed to allocate OperationalSessionSetup instance");`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,8 @@ class OperationalSessionSetupPoolDelegate;`
`36`	`36`
`37`	`37`	`struct CASESessionManagerConfig`
`38`	`38`	`{`
`39`		`- DeviceProxyInitParams sessionInitParams;`
	`39`	`+ CASEClientInitParams sessionInitParams;`
	`40`	`+ CASEClientPoolDelegate * clientPool = nullptr;`
`40`	`41`	`OperationalSessionSetupPoolDelegate * sessionSetupPool = nullptr;`
`41`	`42`	`};`
`42`	`43`
Original file line number	Diff line number	Diff line change
`@@ -221,18 +221,10 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad`
`221`	`221`
`222`	`222`	`CHIP_ERROR OperationalSessionSetup::EstablishConnection(const ReliableMessageProtocolConfig & config)`
`223`	`223`	`{`
`224`		`- mCASEClient = mInitParams.clientPool->Allocate();`
	`224`	`+ mCASEClient = mClientPool->Allocate();`
`225`	`225`	`ReturnErrorCodeIf(mCASEClient == nullptr, CHIP_ERROR_NO_MEMORY);`
`226`	`226`
`227`		`- CASEClientInitParams initParams = { mInitParams.sessionManager,`
`228`		`- mInitParams.sessionResumptionStorage,`
`229`		`- mInitParams.certificateValidityPolicy,`
`230`		`- mInitParams.exchangeMgr,`
`231`		`- mFabricTable,`
`232`		`- mInitParams.groupDataProvider,`
`233`		`- mInitParams.mrpLocalConfig };`
`234`		`-`
`235`		`- CHIP_ERROR err = mCASEClient->EstablishSession(initParams, mPeerId, mDeviceAddress, config, this);`
	`227`	`+ CHIP_ERROR err = mCASEClient->EstablishSession(mInitParams, mPeerId, mDeviceAddress, config, this);`
`236`	`228`	`if (err != CHIP_NO_ERROR)`
`237`	`229`	`{`
`238`	`230`	`CleanupCASEClient();`
`@@ -336,7 +328,7 @@ void OperationalSessionSetup::CleanupCASEClient()`
`336`	`328`	`{`
`337`	`329`	`if (mCASEClient)`
`338`	`330`	`{`
`339`		`- mInitParams.clientPool->Release(mCASEClient);`
	`331`	`+ mClientPool->Release(mCASEClient);`
`340`	`332`	`mCASEClient = nullptr;`
`341`	`333`	`}`
`342`	`334`	`}`
`@@ -370,7 +362,7 @@ OperationalSessionSetup::~OperationalSessionSetup()`
`370`	`362`	`if (mCASEClient)`
`371`	`363`	`{`
`372`	`364`	`// Make sure we don't leak it.`
`373`		`- mInitParams.clientPool->Release(mCASEClient);`
	`365`	`+ mClientPool->Release(mCASEClient);`
`374`	`366`	`}`
`375`	`367`	`}`
`376`	`368`
`@@ -388,7 +380,7 @@ CHIP_ERROR OperationalSessionSetup::LookupPeerAddress()`
`388`	`380`	`return CHIP_NO_ERROR;`
`389`	`381`	`}`
`390`	`382`
`391`		`- auto const * fabricInfo = mFabricTable->FindFabricWithIndex(mPeerId.GetFabricIndex());`
	`383`	`+ auto const * fabricInfo = mInitParams.fabricTable->FindFabricWithIndex(mPeerId.GetFabricIndex());`
`392`	`384`	`VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_INVALID_FABRIC_INDEX);`
`393`	`385`
`394`	`386`	`PeerId peerId(fabricInfo->GetCompressedFabricId(), mPeerId.GetNodeId());`