Allow runtime init of some Server members (1/2) (#16065)

* Allow runtime init of some `Server` members (1/2) This PR is on the path towards having Server::Server no longer statically initialize its members with storage, and instead relying on Server::Init(). This will simplify organization of unit tests and also the convergence of Controller/Server storage to address issue #16028 Issue #16028 * Restyled by clang-format * Move a comment to the correct place Co-authored-by: Restyled.io <[email protected]>
project-chip · Mar 10, 2022 · 665a4ae · 665a4ae
1 parent 244969f
commit 665a4ae
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 9 deletions.
diff --git a/src/app/DefaultAttributePersistenceProvider.cpp b/src/app/DefaultAttributePersistenceProvider.cpp
@@ -25,6 +25,8 @@ namespace app {
 CHIP_ERROR DefaultAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & aPath,
                                                            const EmberAfAttributeMetadata * aMetadata, const ByteSpan & aValue)
 {
+    VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE);
+
     // TODO: we may want to have a small cache for values that change a lot, so
     // we only write them once a bunch of changes happen or on timer or
     // shutdown.
@@ -33,15 +35,17 @@ CHIP_ERROR DefaultAttributePersistenceProvider::WriteValue(const ConcreteAttribu
     {
         return CHIP_ERROR_BUFFER_TOO_SMALL;
     }
-    return mStorage.SyncSetKeyValue(key.AttributeValue(aPath), aValue.data(), static_cast<uint16_t>(aValue.size()));
+    return mStorage->SyncSetKeyValue(key.AttributeValue(aPath), aValue.data(), static_cast<uint16_t>(aValue.size()));
 }
 
 CHIP_ERROR DefaultAttributePersistenceProvider::ReadValue(const ConcreteAttributePath & aPath,
                                                           const EmberAfAttributeMetadata * aMetadata, MutableByteSpan & aValue)
 {
+    VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE);
+
     DefaultStorageKeyAllocator key;
     uint16_t size = static_cast<uint16_t>(min(aValue.size(), static_cast<size_t>(UINT16_MAX)));
-    ReturnErrorOnFailure(mStorage.SyncGetKeyValue(key.AttributeValue(aPath), aValue.data(), size));
+    ReturnErrorOnFailure(mStorage->SyncGetKeyValue(key.AttributeValue(aPath), aValue.data(), size));
     EmberAfAttributeType type = aMetadata->attributeType;
     if (emberAfIsStringAttributeType(type))
     {

diff --git a/src/app/DefaultAttributePersistenceProvider.h b/src/app/DefaultAttributePersistenceProvider.h
@@ -33,17 +33,29 @@ namespace app {
 class DefaultAttributePersistenceProvider : public AttributePersistenceProvider
 {
 public:
-    // aStorage must outlive this object.
-    DefaultAttributePersistenceProvider(PersistentStorageDelegate & aStorage) : mStorage(aStorage) {}
+    DefaultAttributePersistenceProvider() {}
+
+    // Passed-in storage must outlive this object.
+    CHIP_ERROR Init(PersistentStorageDelegate * storage)
+    {
+        if (storage == nullptr)
+        {
+            return CHIP_ERROR_INVALID_ARGUMENT;
+        }
+        mStorage = storage;
+        return CHIP_NO_ERROR;
+    }
+
+    void Shutdown() {}
 
     // AttributePersistenceProvider implementation.
     CHIP_ERROR WriteValue(const ConcreteAttributePath & aPath, const EmberAfAttributeMetadata * aMetadata,
                           const ByteSpan & aValue) override;
     CHIP_ERROR ReadValue(const ConcreteAttributePath & aPath, const EmberAfAttributeMetadata * aMetadata,
                          MutableByteSpan & aValue) override;
 
-private:
-    PersistentStorageDelegate & mStorage;
+protected:
+    PersistentStorageDelegate * mStorage;
 };
 
 } // namespace app

diff --git a/src/app/server/CommissioningWindowManager.h b/src/app/server/CommissioningWindowManager.h
@@ -39,7 +39,17 @@ class Server;
 class CommissioningWindowManager : public SessionEstablishmentDelegate, public app::CommissioningModeProvider
 {
 public:
-    CommissioningWindowManager(Server * server) : mAppDelegate(nullptr), mServer(server) {}
+    CommissioningWindowManager() {}
+
+    CHIP_ERROR Init(Server * server)
+    {
+        if (server == nullptr)
+        {
+            return CHIP_ERROR_INVALID_ARGUMENT;
+        }
+        mServer = server;
+        return CHIP_NO_ERROR;
+    }
 
     void SetAppDelegate(AppDelegate * delegate) { mAppDelegate = delegate; }
 

diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp
@@ -107,8 +107,7 @@ Server::Server() :
         .devicePool        = &mDevicePool,
         .dnsResolver       = nullptr,
     }),
-    mCommissioningWindowManager(this), mGroupsProvider(mDeviceStorage), mAttributePersister(mDeviceStorage),
-    mAccessControl(Access::Examples::GetAccessControlDelegate(&mDeviceStorage))
+    mGroupsProvider(mDeviceStorage), mAccessControl(Access::Examples::GetAccessControlDelegate(&mDeviceStorage))
 {}
 
 CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint16_t unsecureServicePort,
@@ -120,13 +119,16 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
 
     CHIP_ERROR err = CHIP_NO_ERROR;
 
+    // TODO: Remove chip::Platform::MemoryInit() call from Server class, it belongs to outer code
     chip::Platform::MemoryInit();
 
+    SuccessOrExit(err = mCommissioningWindowManager.Init(this));
     mCommissioningWindowManager.SetAppDelegate(delegate);
     mCommissioningWindowManager.SetSessionIDAllocator(&mSessionIDAllocator);
 
     // Set up attribute persistence before we try to bring up the data model
     // handler.
+    SuccessOrExit(mAttributePersister.Init(&mDeviceStorage));
     SetAttributePersistenceProvider(&mAttributePersister);
 
     InitDataModelHandler(&mExchangeMgr);
@@ -337,8 +339,12 @@ void Server::Shutdown()
     }
     mSessions.Shutdown();
     mTransports.Close();
+
+    mAttributePersister.Shutdown();
     mCommissioningWindowManager.Shutdown();
     mCASESessionManager.Shutdown();
+
+    // TODO: Remove chip::Platform::MemoryInit() call from Server class, it belongs to outer code
     chip::Platform::MemoryShutdown();
 }