add logic of no callbacks for an activate-before-revert happening after the revert

Author: wqx6

src/app/clusters/thread-border-router-management-server/thread-br-delegate.h

@@ -48,7 +48,7 @@ class Delegate
         // If the dataset is set successfully, OnActivateDatasetComplete should be called with CHIP_NO_ERROR when the
         // Border Router is attached to the Thread network.
         // If an error occurs while setting the active dataset, this callback should be called with the error.
-        virtual void OnActivateDatasetComplete(CHIP_ERROR error) = 0;
+        virtual void OnActivateDatasetComplete(uint32_t randomNumber, CHIP_ERROR error) = 0;
     };
 
     enum class DatasetType : uint8_t
@@ -72,15 +72,14 @@ class Delegate
     virtual CHIP_ERROR GetDataset(Thread::OperationalDataset & dataset, DatasetType type) = 0;
 
     // The Delegate implementation should back up the old active dataset at the beginning of function if no backup already exists.
-    virtual CHIP_ERROR SetActiveDataset(const Thread::OperationalDataset & activeDataset, ActivateDatasetCallback * callback) = 0;
+    // The Delegate implementation should store the random number and pass it to OnActivateDatasetComplete.
+    virtual CHIP_ERROR SetActiveDataset(const Thread::OperationalDataset & activeDataset, uint32_t randomNum,
+                                        ActivateDatasetCallback * callback) = 0;
 
     // The Delegate implementation should remove the backup active dataset in this function.
     virtual CHIP_ERROR CommitActiveDataset() = 0;
 
     // The Delegate implementation should restore the backup active dataset in this function.
-    // If RevertActiveDataset is called before the ActivateDatasetCallback that would result from a
-    // previous SetActiveDataset, the delegate must ensure that the ActivateDatasetCallback
-    // for that previous SetActiveDataset call will not happen.
     virtual CHIP_ERROR RevertActiveDataset() = 0;
 
     virtual CHIP_ERROR SetPendingDataset(const Thread::OperationalDataset & pendingDataset) = 0;

src/app/clusters/thread-border-router-management-server/thread-br-mgmt-server.cpp

@@ -30,6 +30,7 @@
 #include "app/clusters/general-commissioning-server/general-commissioning-server.h"
 #include "app/data-model/Nullable.h"
 #include "app/server/Server.h"
+#include "crypto/RandUtils.h"
 #include "lib/core/CHIPError.h"
 #include "lib/support/CodeUtils.h"
 #include "lib/support/Span.h"
@@ -102,7 +103,12 @@ Status ServerInstance::HandleSetActiveDatasetRequest(bool failSafeArmed,
     }
 
     mBreadcrumb    = req.breadcrumb;
-    CHIP_ERROR err = mDelegate->SetActiveDataset(activeDataset, this);
+    mRandomNumber  = Crypto::GetRandU32();
+    CHIP_ERROR err = mDelegate->SetActiveDataset(activeDataset, mRandomNumber, this);
+    if (err != CHIP_NO_ERROR)
+    {
+        mRandomNumber = 0;
+    }
     return StatusIB(err).mStatus;
 }
 
@@ -162,7 +168,7 @@ void ServerInstance::InvokeCommand(HandlerContext & ctxt)
                 Status status = HandleSetActiveDatasetRequest(IsFailSafeArmed(ctx), req);
                 if (status != Status::Success)
                 {
-                    OnActivateDatasetComplete(ChipError(ChipError::SdkPart::kIMGlobalStatus, to_underlying(status)));
+                    OnActivateDatasetComplete(mRandomNumber, ChipError(ChipError::SdkPart::kIMGlobalStatus, to_underlying(status)));
                 }
             }
             else
@@ -318,14 +324,20 @@ void ServerInstance::CommitSavedBreadcrumb()
     mBreadcrumb.ClearValue();
 }
 
-void ServerInstance::OnActivateDatasetComplete(CHIP_ERROR error)
+void ServerInstance::OnActivateDatasetComplete(uint32_t randomNumber, CHIP_ERROR error)
 {
     auto commandHandleRef = std::move(mAsyncCommandHandle);
     auto commandHandle    = commandHandleRef.Get();
     if (commandHandle == nullptr)
     {
         return;
     }
+    if (mRandomNumber != randomNumber)
+    {
+        // Previous SetActiveDatasetRequest was handled.
+        return;
+    }
+    mRandomNumber = 0;
     if (error == CHIP_NO_ERROR)
     {
         // The successful completion of the activation process SHALL disarm the fail-safe timer.
@@ -352,6 +364,8 @@ void ServerInstance::OnFailSafeTimerExpired()
     {
         return;
     }
+    // Reset the RandomNumeber so that the OnActivateDatasetComplete will not handle the previous SetActiveDatasetRequest command.
+    mRandomNumber = 0;
     commandHandle->AddStatus(mPath, Status::Timeout);
 }
 

src/app/clusters/thread-border-router-management-server/thread-br-mgmt-server.h

@@ -53,7 +53,7 @@ class ServerInstance : public CommandHandlerInterface, public AttributeAccessInt
     CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
 
     // ActivateDatasetCallbackInterface
-    void OnActivateDatasetComplete(CHIP_ERROR error) override;
+    void OnActivateDatasetComplete(uint32_t randomNum, CHIP_ERROR error) override;
 
 private:
     friend class TestThreadBorderRouterManagementCluster;
@@ -86,6 +86,7 @@ class ServerInstance : public CommandHandlerInterface, public AttributeAccessInt
     app::CommandHandler::Handle mAsyncCommandHandle;
     ConcreteCommandPath mPath = ConcreteCommandPath(0, 0, 0);
     Optional<uint64_t> mBreadcrumb;
+    uint32_t mRandomNumber;
 };
 
 } // namespace ThreadBorderRouterManagement

src/app/tests/BUILD.gn

@@ -123,11 +123,10 @@ source_set("operational-state-test-srcs") {
 source_set("thread-border-router-management-test-srcs") {
   sources = [ "${chip_root}/src/app/clusters/thread-border-router-management-server/thread-br-mgmt-server.cpp" ]
 
-  deps = [ "${chip_root}/src/app/server" ]
-
   public_deps = [
     "${chip_root}/src/app",
     "${chip_root}/src/app/common:cluster-objects",
+    "${chip_root}/src/app/server",
     "${chip_root}/src/app/util/mock:mock_ember",
     "${chip_root}/src/lib/core",
   ]

src/app/tests/TestThreadBorderRouterManagementCluster.cpp

@@ -103,7 +103,8 @@ class TestDelegate : public Delegate
         return CHIP_IM_GLOBAL_STATUS(NotFound);
     }
 
-    CHIP_ERROR SetActiveDataset(const Thread::OperationalDataset & activeDataset, ActivateDatasetCallback * callback) override
+    CHIP_ERROR SetActiveDataset(const Thread::OperationalDataset & activeDataset, uint32_t randomNumber,
+                                ActivateDatasetCallback * callback) override
     {
         memcpy(mActiveDataset, activeDataset.AsByteSpan().data(), activeDataset.AsByteSpan().size());
         mActiveDatasetLen = activeDataset.AsByteSpan().size();

src/platform/OpenThread/GenericThreadBorderRouterDelegate.cpp

@@ -119,7 +119,7 @@ CHIP_ERROR GenericThreadBorderRouterDelegate::GetDataset(Thread::OperationalData
 }
 
 CHIP_ERROR GenericThreadBorderRouterDelegate::SetActiveDataset(const Thread::OperationalDataset & activeDataset,
-                                                               ActivateDatasetCallback * callback)
+                                                               uint32_t randomNumber, ActivateDatasetCallback * callback)
 {
     CHIP_ERROR err = BackupActiveDataset();
     if (err == CHIP_NO_ERROR)
@@ -128,7 +128,8 @@ CHIP_ERROR GenericThreadBorderRouterDelegate::SetActiveDataset(const Thread::Ope
     }
     if (err == CHIP_NO_ERROR)
     {
-        mCallback = callback;
+        mRandomNumber = randomNumber;
+        mCallback     = callback;
     }
     return err;
 }
@@ -141,7 +142,7 @@ void GenericThreadBorderRouterDelegate::OnPlatformEventHandler(const DeviceLayer
         if (event->Type == DeviceLayer::DeviceEventType::kThreadConnectivityChange &&
             event->ThreadConnectivityChange.Result == DeviceLayer::kConnectivity_Established)
         {
-            delegate->mCallback->OnActivateDatasetComplete(CHIP_NO_ERROR);
+            delegate->mCallback->OnActivateDatasetComplete(delegate->mRandomNumber, CHIP_NO_ERROR);
             // Delete Failsafe Keys after activating dataset is completed
             DeviceLayer::PersistedStorage::KeyValueStoreMgr().Delete(kFailsafeThreadDatasetTlvsKey);
             delegate->mCallback = nullptr;

src/platform/OpenThread/GenericThreadBorderRouterDelegate.h

@@ -65,7 +65,8 @@ class GenericThreadBorderRouterDelegate : public Delegate
 
     CHIP_ERROR GetDataset(Thread::OperationalDataset & dataset, DatasetType type) override;
 
-    CHIP_ERROR SetActiveDataset(const Thread::OperationalDataset & activeDataset, ActivateDatasetCallback * callback) override;
+    CHIP_ERROR SetActiveDataset(const Thread::OperationalDataset & activeDataset, uint32_t randomNumber,
+                                ActivateDatasetCallback * callback) override;
 
     CHIP_ERROR RevertActiveDataset() override;
 
@@ -79,6 +80,7 @@ class GenericThreadBorderRouterDelegate : public Delegate
     CHIP_ERROR BackupActiveDataset();
     ActivateDatasetCallback * mCallback        = nullptr;
     Thread::OperationalDataset mStagingDataset = {};
+    uint32_t mRandomNumber;
 };
 } // namespace ThreadBorderRouterManagement
 } // namespace Clusters