Skip to content

Commit

Permalink
review change
Browse files Browse the repository at this point in the history
  • Loading branch information
@wqx6
wqx6 committed Aug 5, 2024
1 parent 6ebe9fc commit e887c8c
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 46 deletions.
37 changes: 20 additions & 17 deletions ...lusters/thread-border-router-management-server/thread-border-router-management-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,24 @@ namespace ThreadBorderRouterManagement {

using Protocols::InteractionModel::Status;

static bool IsCommandOverCASESession(CommandHandlerInterface::HandlerContext & ctx)
bool ServerInstance::IsCommandOverCASESession(CommandHandlerInterface::HandlerContext & ctx)
{
#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
if (mSkipCASESessionCheck)
{
return true;
}
#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST
Messaging::ExchangeContext * exchangeCtx = ctx.mCommandHandler.GetExchangeContext();
return exchangeCtx && exchangeCtx->HasSessionHandle() && exchangeCtx->GetSessionHandle()->IsSecureSession() &&
exchangeCtx->GetSessionHandle()->AsSecureSession()->GetSecureSessionType() == Transport::SecureSession::Type::kCASE;
}

Status ServerInstance::HandleGetDatasetRequest(bool isOverCASESession, Delegate::DatasetType type,
Status ServerInstance::HandleGetDatasetRequest(CommandHandlerInterface::HandlerContext & ctx, Delegate::DatasetType type,
Thread::OperationalDataset & dataset)
{
VerifyOrDie(mDelegate);
if (!isOverCASESession)
{
return Status::UnsupportedAccess;
}
VerifyOrReturnValue(IsCommandOverCASESession(ctx), Status::UnsupportedAccess);

CHIP_ERROR err = mDelegate->GetDataset(dataset, type);
if (err != CHIP_NO_ERROR)
Expand All @@ -70,7 +73,7 @@ Status ServerInstance::HandleGetDatasetRequest(bool isOverCASESession, Delegate:
return Status::Success;
}

Status ServerInstance::HandleSetActiveDatasetRequest(bool isOverCASESession, CommandHandler * commandHandler,
Status ServerInstance::HandleSetActiveDatasetRequest(CommandHandlerInterface::HandlerContext & ctx,
const Commands::SetActiveDatasetRequest::DecodableType & req)
{
// The SetActiveDatasetRequest command SHALL be FailSafeArmed. Upon receiving this command, the Thread BR will set its
Expand All @@ -80,8 +83,8 @@ Status ServerInstance::HandleSetActiveDatasetRequest(bool isOverCASESession, Com
// reverted. If the FailSafe timer expires before the Thread BR responds, the Thread BR will respond with a timeout status and
// the active dataset should also be reverted.
VerifyOrDie(mDelegate);
VerifyOrReturnValue(isOverCASESession, Status::UnsupportedAccess);
VerifyOrReturnValue(mFailsafeContext.IsFailSafeArmed(commandHandler->GetAccessingFabricIndex()), Status::FailsafeRequired);
VerifyOrReturnValue(IsCommandOverCASESession(ctx), Status::UnsupportedAccess);
VerifyOrReturnValue(mFailsafeContext.IsFailSafeArmed(ctx.mCommandHandler.GetAccessingFabricIndex()), Status::FailsafeRequired);

Thread::OperationalDataset activeDataset;
Thread::OperationalDataset currentActiveDataset;
Expand All @@ -102,19 +105,19 @@ Status ServerInstance::HandleSetActiveDatasetRequest(bool isOverCASESession, Com
{
return Status::Busy;
}
commandHandler->FlushAcksRightAwayOnSlowCommand();
mAsyncCommandHandle = CommandHandler::Handle(commandHandler);
ctx.mCommandHandler.FlushAcksRightAwayOnSlowCommand();
mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler);
mBreadcrumb = req.breadcrumb;
mSetActiveDatasetSequenceNumber++;
mDelegate->SetActiveDataset(activeDataset, mSetActiveDatasetSequenceNumber, this);
return Status::Success;
}

Status ServerInstance::HandleSetPendingDatasetRequest(bool isOverCASESession,
Status ServerInstance::HandleSetPendingDatasetRequest(CommandHandlerInterface::HandlerContext & ctx,
const Commands::SetPendingDatasetRequest::DecodableType & req)
{
VerifyOrDie(mDelegate);
VerifyOrReturnValue(isOverCASESession, Status::UnsupportedAccess);
VerifyOrReturnValue(IsCommandOverCASESession(ctx), Status::UnsupportedAccess);
if (!mDelegate->GetPanChangeSupported())
{
return Status::UnsupportedCommand;
Expand Down Expand Up @@ -146,21 +149,21 @@ void ServerInstance::InvokeCommand(HandlerContext & ctxt)
case Commands::GetActiveDatasetRequest::Id:
HandleCommand<Commands::GetActiveDatasetRequest::DecodableType>(ctxt, [this](HandlerContext & ctx, const auto & req) {
Thread::OperationalDataset dataset;
Status status = HandleGetActiveDatasetRequest(IsCommandOverCASESession(ctx), dataset);
Status status = HandleGetActiveDatasetRequest(ctx, dataset);
AddDatasetResponse(ctx, status, dataset);
});
break;
case Commands::GetPendingDatasetRequest::Id:
HandleCommand<Commands::GetPendingDatasetRequest::DecodableType>(ctxt, [this](HandlerContext & ctx, const auto & req) {
Thread::OperationalDataset dataset;
Status status = HandleGetPendingDatasetRequest(IsCommandOverCASESession(ctx), dataset);
Status status = HandleGetPendingDatasetRequest(ctx, dataset);
AddDatasetResponse(ctx, status, dataset);
});
break;
case Commands::SetActiveDatasetRequest::Id:
HandleCommand<Commands::SetActiveDatasetRequest::DecodableType>(ctxt, [this](HandlerContext & ctx, const auto & req) {
mPath = ctx.mRequestPath;
Status status = HandleSetActiveDatasetRequest(IsCommandOverCASESession(ctx), &ctx.mCommandHandler, req);
Status status = HandleSetActiveDatasetRequest(ctx, req);
if (status != Status::Success)
{
// If status is not Success, we should immediately report the status. Otherwise the async work will report the
Expand All @@ -171,7 +174,7 @@ void ServerInstance::InvokeCommand(HandlerContext & ctxt)
break;
case Commands::SetPendingDatasetRequest::Id:
HandleCommand<Commands::SetPendingDatasetRequest::DecodableType>(ctxt, [this](HandlerContext & ctx, const auto & req) {
Status status = HandleSetPendingDatasetRequest(IsCommandOverCASESession(ctx), req);
Status status = HandleSetPendingDatasetRequest(ctx, req);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status);
});
break;
Expand Down
20 changes: 12 additions & 8 deletions .../clusters/thread-border-router-management-server/thread-border-router-management-server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,17 @@ class ServerInstance : public CommandHandlerInterface,
// TODO: Split the business logic from the unit test class
friend class TestThreadBorderRouterManagementCluster;
// Command Handlers
Status HandleGetActiveDatasetRequest(bool isOverCASESession, Thread::OperationalDataset & dataset)
Status HandleGetActiveDatasetRequest(HandlerContext & ctx, Thread::OperationalDataset & dataset)
{
return HandleGetDatasetRequest(isOverCASESession, Delegate::DatasetType::kActive, dataset);
return HandleGetDatasetRequest(ctx, Delegate::DatasetType::kActive, dataset);
}
Status HandleGetPendingDatasetRequest(bool isOverCASESession, Thread::OperationalDataset & dataset)
Status HandleGetPendingDatasetRequest(HandlerContext & ctx, Thread::OperationalDataset & dataset)
{
return HandleGetDatasetRequest(isOverCASESession, Delegate::DatasetType::kPending, dataset);
return HandleGetDatasetRequest(ctx, Delegate::DatasetType::kPending, dataset);
}
Status HandleSetActiveDatasetRequest(bool isOverCASESession, CommandHandler * commandHandler,
const Commands::SetActiveDatasetRequest::DecodableType & req);
Status HandleSetPendingDatasetRequest(bool isOverCASESession, const Commands::SetPendingDatasetRequest::DecodableType & req);
Status HandleGetDatasetRequest(bool isOverCASESession, Delegate::DatasetType type, Thread::OperationalDataset & dataset);
Status HandleSetActiveDatasetRequest(HandlerContext & ctx, const Commands::SetActiveDatasetRequest::DecodableType & req);
Status HandleSetPendingDatasetRequest(HandlerContext & ctx, const Commands::SetPendingDatasetRequest::DecodableType & req);
Status HandleGetDatasetRequest(HandlerContext & ctx, Delegate::DatasetType type, Thread::OperationalDataset & dataset);

// Attribute Read handlers
void ReadFeatureMap(BitFlags<Feature> & feature);
Expand All @@ -88,6 +87,11 @@ class ServerInstance : public CommandHandlerInterface,
CHIP_ERROR ReadBorderRouterName(MutableCharSpan & borderRouterName);
CHIP_ERROR ReadBorderAgentID(MutableByteSpan & borderAgentId);

#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
void SetSkipCASESessionCheck(bool skipCheck) { mSkipCASESessionCheck = skipCheck; }
bool mSkipCASESessionCheck;
#endif
bool IsCommandOverCASESession(CommandHandlerInterface::HandlerContext & ctx);
static void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg);
void OnFailSafeTimerExpired();
void CommitSavedBreadcrumb();
Expand Down
51 changes: 30 additions & 21 deletions src/app/tests/TestThreadBorderRouterManagementCluster.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@

#include <app-common/zap-generated/cluster-objects.h>
#include <app/CommandHandler.h>
#include <app/CommandHandlerInterface.h>
#include <app/ConcreteCommandPath.h>
#include <app/FailSafeContext.h>
#include <app/clusters/thread-border-router-management-server/thread-border-router-management-server.h>
#include <cstdint>
#include <cstring>
#include <lib/core/CASEAuthTag.h>
#include <lib/core/CHIPError.h>
#include <lib/core/DataModelTypes.h>
#include <lib/core/Optional.h>
#include <lib/core/TLVReader.h>
#include <lib/support/BitFlags.h>
#include <lib/support/CHIPMem.h>
#include <lib/support/Span.h>
Expand Down Expand Up @@ -153,10 +157,10 @@ static FailSafeContext sTestFailsafeContext;
static TestDelegate sTestDelegate;
static ServerInstance sTestSeverInstance(kTestEndpointId, &sTestDelegate, sTestFailsafeContext);

class TestSetActiveDatasetCommandHandler : public CommandHandler
class TestCommandHandler : public CommandHandler
{
public:
TestSetActiveDatasetCommandHandler() : mClusterStatus(Protocols::InteractionModel::Status::Success) {}
TestCommandHandler() : mClusterStatus(Protocols::InteractionModel::Status::Success) {}
CHIP_ERROR FallibleAddStatus(const ConcreteCommandPath & aRequestCommandPath,
const Protocols::InteractionModel::ClusterStatusCode & aStatus, const char * context = nullptr)
{
Expand Down Expand Up @@ -197,7 +201,7 @@ class TestSetActiveDatasetCommandHandler : public CommandHandler
Protocols::InteractionModel::ClusterStatusCode mClusterStatus;
};

TestSetActiveDatasetCommandHandler sTestCommandHandler;
TestCommandHandler sTestCommandHandler;

class TestThreadBorderRouterManagementCluster : public ::testing::Test
{
Expand Down Expand Up @@ -264,17 +268,23 @@ TEST_F_FROM_FIXTURE(TestThreadBorderRouterManagementCluster, TestCommandHandle)
{
// Test GetActiveDatasetRequest and GetPendingDatasetRequest commands
Thread::OperationalDataset dataset;
ThreadBorderRouterManagement::Commands::SetActiveDatasetRequest::DecodableType req1;
Commands::SetPendingDatasetRequest::DecodableType req2;
using DatasetType = Delegate::DatasetType;
using Status = Protocols::InteractionModel::Status;
// The GetDataset requests should over CASE session.
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(false /* isOverCASESession */, DatasetType::kActive, dataset),
Status::UnsupportedAccess);
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(false, DatasetType::kPending, dataset), Status::UnsupportedAccess);
ConcreteCommandPath testPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId);
TLV::TLVReader testTLVReader;
CommandHandlerInterface::HandlerContext ctx(sTestCommandHandler, testPath, testTLVReader);
// All the command should be over CASE session.
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(ctx, DatasetType::kActive, dataset), Status::UnsupportedAccess);
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(ctx, DatasetType::kPending, dataset), Status::UnsupportedAccess);
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(ctx, req1), Status::UnsupportedAccess);
EXPECT_EQ(sTestSeverInstance.HandleSetPendingDatasetRequest(ctx, req2), Status::UnsupportedAccess);
sTestSeverInstance.SetSkipCASESessionCheck(true);
// The GetDataset should return NotFound when no dataset is configured.
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(true, DatasetType::kActive, dataset), Status::NotFound);
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(true, DatasetType::kPending, dataset), Status::NotFound);
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(ctx, DatasetType::kActive, dataset), Status::NotFound);
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(ctx, DatasetType::kPending, dataset), Status::NotFound);
// Test SetActiveDatasetRequest
ThreadBorderRouterManagement::Commands::SetActiveDatasetRequest::DecodableType req1;
uint8_t invalidDataset[] = { 0x11, 0x22, 0x33, 0x44, 0x55, 0x66 };
uint8_t validDataset[] = { 0x0e, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x0b, 0x35, 0x06,
0x00, 0x04, 0x00, 0x1f, 0xff, 0xe0, 0x02, 0x08, 0xde, 0xaa, 0x00, 0xbe, 0xef, 0x00, 0xca, 0xef, 0x07,
Expand All @@ -287,14 +297,14 @@ TEST_F_FROM_FIXTURE(TestThreadBorderRouterManagementCluster, TestCommandHandle)
EXPECT_FALSE(activeDatasetTimestamp.HasValue());
req1.activeDataset = ByteSpan(invalidDataset);
// SetActiveDatasetRequest is FailsafeRequired.
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(true, &sTestCommandHandler, req1), Status::FailsafeRequired);
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(ctx, req1), Status::FailsafeRequired);
EXPECT_EQ(sTestFailsafeContext.ArmFailSafe(kTestAccessingFabricIndex, System::Clock::Seconds16(1)), CHIP_NO_ERROR);
// SetActiveDatasetRequest should return InvalidCommand when dataset is invalid.
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(true, &sTestCommandHandler, req1), Status::InvalidCommand);
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(ctx, req1), Status::InvalidCommand);
req1.activeDataset = ByteSpan(validDataset);
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(true, &sTestCommandHandler, req1), Status::Success);
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(ctx, req1), Status::Success);
// When the Server is handling a SetActiveDatasetRequest command, it should return Busy after receiving another one.
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(true, &sTestCommandHandler, req1), Status::Busy);
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(ctx, req1), Status::Busy);
EXPECT_FALSE(sTestDelegate.mInterfaceEnabled);
EXPECT_EQ(sTestDelegate.mSetActiveDatasetCommandSequenceNum, static_cast<unsigned int>(1));
// Activate the dataset.
Expand All @@ -303,30 +313,29 @@ TEST_F_FROM_FIXTURE(TestThreadBorderRouterManagementCluster, TestCommandHandle)
Protocols::InteractionModel::ClusterStatusCode(Protocols::InteractionModel::Status::Success));
sTestFailsafeContext.DisarmFailSafe();
// The Dataset should be updated.
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(true, DatasetType::kActive, dataset), Status::Success);
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(ctx, DatasetType::kActive, dataset), Status::Success);
EXPECT_TRUE(dataset.AsByteSpan().data_equal(ByteSpan(validDataset)));
EXPECT_TRUE(sTestDelegate.mInterfaceEnabled);
activeDatasetTimestamp = sTestSeverInstance.ReadActiveDatasetTimestamp();
// activeDatasetTimestamp should have value.
EXPECT_TRUE(activeDatasetTimestamp.HasValue());
EXPECT_EQ(sTestFailsafeContext.ArmFailSafe(kTestAccessingFabricIndex, System::Clock::Seconds16(1)), CHIP_NO_ERROR);
// When ActiveDatasetTimestamp is not null, the set active dataset request should return InvalidInState.
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(true, &sTestCommandHandler, req1), Status::InvalidInState);
EXPECT_EQ(sTestSeverInstance.HandleSetActiveDatasetRequest(ctx, req1), Status::InvalidInState);
sTestFailsafeContext.DisarmFailSafe();
// Test SetPendingDatasetRequest command
Commands::SetPendingDatasetRequest::DecodableType req2;
sTestDelegate.mPanChangeSupported = false;
req2.pendingDataset = ByteSpan(validDataset);
// SetPendingDatasetRequest is supported when PANChange feature is enabled.
EXPECT_EQ(sTestSeverInstance.HandleSetPendingDatasetRequest(true, req2), Status::UnsupportedCommand);
EXPECT_EQ(sTestSeverInstance.HandleSetPendingDatasetRequest(ctx, req2), Status::UnsupportedCommand);
sTestDelegate.mPanChangeSupported = true;
req2.pendingDataset = ByteSpan(invalidDataset);
// SetPendingDatasetRequest should return InvalidCommand when dataset is invalid.
EXPECT_EQ(sTestSeverInstance.HandleSetPendingDatasetRequest(true, req2), Status::InvalidCommand);
EXPECT_EQ(sTestSeverInstance.HandleSetPendingDatasetRequest(ctx, req2), Status::InvalidCommand);
req2.pendingDataset = ByteSpan(validDataset);
// Success SetPendingDatasetRequest
EXPECT_EQ(sTestSeverInstance.HandleSetPendingDatasetRequest(true, req2), Status::Success);
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(true, DatasetType::kPending, dataset), Status::Success);
EXPECT_EQ(sTestSeverInstance.HandleSetPendingDatasetRequest(ctx, req2), Status::Success);
EXPECT_EQ(sTestSeverInstance.HandleGetDatasetRequest(ctx, DatasetType::kPending, dataset), Status::Success);
EXPECT_TRUE(dataset.AsByteSpan().data_equal(ByteSpan(validDataset)));
}

Expand Down

0 comments on commit e887c8c

Please sign in to comment.