Skip to content

Commit

Permalink
[Terms and Conditions] Introduce TermsAndConditionsManager singleton
Browse files Browse the repository at this point in the history
Refactor Terms and Conditions handling by introducing a dedicated

TermsAndConditionsManager singleton class that manages all T&C operations.

- Removes T&C provider from Server class to reduce coupling
- Adds new TermsAndConditionsManager class with singleton pattern
- Updates all T&C provider references to use the manager
- Moves initialization logic from CommonCaseDeviceServerInitParams to manager
- Updates Linux example app to use the new manager

The change simplifies T&C management and improves code organization by
centralizing T&C functionality in a dedicated manager class.
  • Loading branch information
swan-amazon committed Dec 5, 2024
1 parent 0c99ddc commit 485bae5
Show file tree
Hide file tree
Showing 8 changed files with 153 additions and 36 deletions.
9 changes: 9 additions & 0 deletions examples/platform/linux/AppMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@
#include <thread>
#endif

#if CHIP_CONFIG_TC_REQUIRED
#include <app/server/TermsAndConditionsManager.h>
#endif

#if defined(PW_RPC_ENABLED)
#include <Rpc.h>
#endif
Expand Down Expand Up @@ -536,6 +540,11 @@ void ChipLinuxAppMainLoop(AppMainLoopImplementation * impl)
VerifyOrDie(initParams.InitializeStaticResourcesBeforeServerInit() == CHIP_NO_ERROR);
initParams.dataModelProvider = app::CodegenDataModelProviderInstance(initParams.persistentStorageDelegate);

#if CHIP_CONFIG_TC_REQUIRED
const Optional<app::TermsAndConditions> termsAndConditions = Optional<app::TermsAndConditions>(app::TermsAndConditions(CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS, CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION));
chip::app::TermsAndConditionsManager::GetInstance()->Init(&Server::GetInstance().GetPersistentStorage(), termsAndConditions);
#endif

#if defined(ENABLE_CHIP_SHELL)
Engine::Root().Init();
Shell::RegisterCommissioneeCommands();
Expand Down
6 changes: 5 additions & 1 deletion examples/terms-and-conditions-app/linux/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ import("${chip_root}/build/chip/tools.gni")
import("${chip_root}/src/app/common_flags.gni")

executable("chip-terms-and-conditions-app") {
sources = [ "main.cpp" ]
sources = [
"main.cpp",
"${chip_root}/src/app/server/TermsAndConditionsManager.cpp",
"${chip_root}/src/app/server/TermsAndConditionsManager.h",
]

deps = [
"${chip_root}/examples/platform/linux:app-main",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@
#include <platform/DeviceControlServer.h>
#include <tracing/macros.h>

#if CHIP_CONFIG_TC_REQUIRED
#include <app/server/TermsAndConditionsManager.h>
#endif

using namespace chip;
using namespace chip::app;
using namespace chip::app::Clusters;
Expand Down Expand Up @@ -100,7 +104,7 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath
}
#if CHIP_CONFIG_TC_REQUIRED
case TCAcceptedVersion::Id: {
TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider();
TermsAndConditionsProvider * const termsAndConditionsProvider = TermsAndConditionsManager::GetInstance();
Optional<TermsAndConditions> outTermsAndConditions;

if (nullptr == termsAndConditionsProvider)
Expand All @@ -117,7 +121,7 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath
return aEncoder.Encode(outTermsAndConditions.ValueOr(TermsAndConditions(0, 0)).GetVersion());
}
case TCMinRequiredVersion::Id: {
TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider();
TermsAndConditionsProvider * const termsAndConditionsProvider = TermsAndConditionsManager::GetInstance();
Optional<TermsAndConditions> outTermsAndConditions;

if (nullptr == termsAndConditionsProvider)
Expand All @@ -134,7 +138,7 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath
return aEncoder.Encode(outTermsAndConditions.ValueOr(TermsAndConditions(0, 0)).GetVersion());
}
case TCAcknowledgements::Id: {
TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider();
TermsAndConditionsProvider * const termsAndConditionsProvider = TermsAndConditionsManager::GetInstance();
Optional<TermsAndConditions> outTermsAndConditions;

if (nullptr == termsAndConditionsProvider)
Expand All @@ -151,7 +155,7 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath
return aEncoder.Encode(outTermsAndConditions.ValueOr(TermsAndConditions(0, 0)).GetValue());
}
case TCAcknowledgementsRequired::Id: {
TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider();
TermsAndConditionsProvider * const termsAndConditionsProvider = TermsAndConditionsManager::GetInstance();
Optional<TermsAndConditions> outTermsAndConditions;
TermsAndConditionsState termsAndConditionsState;

Expand All @@ -177,7 +181,7 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath
return aEncoder.Encode(setTermsAndConditionsCallRequiredBeforeCommissioningCompleteSuccess);
}
case TCUpdateDeadline::Id: {
TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider();
TermsAndConditionsProvider * const termsAndConditionsProvider = TermsAndConditionsManager::GetInstance();
Optional<uint32_t> outUpdateAcceptanceDeadline;

if (nullptr == termsAndConditionsProvider)
Expand Down Expand Up @@ -404,7 +408,7 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback(
}

#if CHIP_CONFIG_TC_REQUIRED
TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider();
TermsAndConditionsProvider * const termsAndConditionsProvider = TermsAndConditionsManager::GetInstance();

// Ensure required terms and conditions have been accepted, then attempt to commit
if (nullptr != termsAndConditionsProvider)
Expand Down Expand Up @@ -537,7 +541,7 @@ bool emberAfGeneralCommissioningClusterSetTCAcknowledgementsCallback(
MATTER_TRACE_SCOPE("SetTCAcknowledgements", "GeneralCommissioning");

auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider();
TermsAndConditionsProvider * const termsAndConditionsProvider = TermsAndConditionsManager::GetInstance();

Optional<TermsAndConditions> acceptedTermsAndConditions = Optional<TermsAndConditions>(TermsAndConditions(commandData.TCUserResponse, commandData.TCVersion));

Expand Down Expand Up @@ -581,7 +585,7 @@ void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t
if (event->FailSafeTimerExpired.updateTermsAndConditionsHasBeenInvoked)
{
#if CHIP_CONFIG_TC_REQUIRED
TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider();
TermsAndConditionsProvider * const termsAndConditionsProvider = TermsAndConditionsManager::GetInstance();

if (nullptr == termsAndConditionsProvider)
{
Expand Down Expand Up @@ -611,7 +615,7 @@ class GeneralCommissioningFabricTableDelegate : public chip::FabricTable::Delega
static_cast<unsigned>(fabricIndex));

#if CHIP_CONFIG_TC_REQUIRED
TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider();
TermsAndConditionsProvider * const termsAndConditionsProvider = TermsAndConditionsManager::GetInstance();

if (nullptr != termsAndConditionsProvider)
{
Expand Down
1 change: 1 addition & 0 deletions src/app/server/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ static_library("server") {
"Server.cpp",
"Server.h",
"TermsAndConditionsProvider.h",
"TermsAndConditionsManager.h",
]

public_configs = [ ":server_config" ]
Expand Down
4 changes: 1 addition & 3 deletions src/app/server/Server.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* Copyright (c) 2021-2024 Project CHIP Authors
* Copyright (c) 2021 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -209,8 +209,6 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)

mReportScheduler = initParams.reportScheduler;

mTermsAndConditionsProvider = initParams.termsAndConditionsProvider;

mTestEventTriggerDelegate = initParams.testEventTriggerDelegate;
if (mTestEventTriggerDelegate == nullptr)
{
Expand Down
24 changes: 1 addition & 23 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* Copyright (c) 2020-2024 Project CHIP Authors
* Copyright (c) 2020 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -33,7 +33,6 @@
#include <app/server/AppDelegate.h>
#include <app/server/CommissioningWindowManager.h>
#include <app/server/DefaultAclStorage.h>
#include <app/server/DefaultTermsAndConditionsProvider.h>
#include <credentials/CertificateValidityPolicy.h>
#include <credentials/FabricTable.h>
#include <credentials/GroupDataProvider.h>
Expand Down Expand Up @@ -194,9 +193,6 @@ struct ServerInitParams
// data model it wants to use. Backwards-compatibility can use `CodegenDataModelProviderInstance`
// for ember/zap-generated models.
chip::app::DataModel::Provider * dataModelProvider = nullptr;

// Optional. Terms and conditions provider to support enhanced setup flow feature.
app::TermsAndConditionsProvider * termsAndConditionsProvider = nullptr;
};

/**
Expand Down Expand Up @@ -318,21 +314,6 @@ struct CommonCaseDeviceServerInitParams : public ServerInitParams
}
#endif

#if CHIP_CONFIG_TC_REQUIRED
static app::DefaultTermsAndConditionsProvider sDefaultTermsAndConditionsProviderInstance;
static app::DefaultTermsAndConditionsStorageDelegate sDefaultTermsAndConditionsStorageDelegateInstance;

Optional<app::TermsAndConditions> termsAndConditions = Optional<app::TermsAndConditions>(
app::TermsAndConditions(CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS, CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION));

ReturnErrorOnFailure(sDefaultTermsAndConditionsStorageDelegateInstance.Init(this->persistentStorageDelegate));
ReturnErrorOnFailure(sDefaultTermsAndConditionsProviderInstance.Init(&sDefaultTermsAndConditionsStorageDelegateInstance,
termsAndConditions));
this->termsAndConditionsProvider = &sDefaultTermsAndConditionsProviderInstance;
#else
this->termsAndConditionsProvider = nullptr;
#endif

return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -435,8 +416,6 @@ class Server

app::reporting::ReportScheduler * GetReportScheduler() { return mReportScheduler; }

app::TermsAndConditionsProvider * GetTermsAndConditionsProvider() { return mTermsAndConditionsProvider; }

#if CHIP_CONFIG_ENABLE_ICD_SERVER
app::ICDManager & GetICDManager() { return mICDManager; }

Expand Down Expand Up @@ -710,7 +689,6 @@ class Server
GroupDataProviderListener mListener;
ServerFabricDelegate mFabricDelegate;
app::reporting::ReportScheduler * mReportScheduler;
app::TermsAndConditionsProvider * mTermsAndConditionsProvider;

Access::AccessControl mAccessControl;
app::AclStorage * mAclStorage;
Expand Down
79 changes: 79 additions & 0 deletions src/app/server/TermsAndConditionsManager.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
*
* Copyright (c) 2024 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "DefaultTermsAndConditionsProvider.h"
#include "TermsAndConditionsManager.h"

static chip::app::TermsAndConditionsManager sTermsAndConditionsManager;
static chip::app::DefaultTermsAndConditionsProvider sTermsAndConditionsProviderInstance;
static chip::app::DefaultTermsAndConditionsStorageDelegate sTermsAndConditionsStorageDelegateInstance;

chip::app::TermsAndConditionsManager * chip::app::TermsAndConditionsManager::GetInstance()
{
return &sTermsAndConditionsManager;
}

CHIP_ERROR chip::app::TermsAndConditionsManager::Init(chip::PersistentStorageDelegate * const inPersistentStorageDelegate, const Optional<TermsAndConditions> & inRequiredTermsAndConditions)
{
Optional<app::TermsAndConditions> termsAndConditions = Optional<app::TermsAndConditions>(app::TermsAndConditions(CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS, CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION));

ReturnErrorOnFailure(sTermsAndConditionsStorageDelegateInstance.Init(inPersistentStorageDelegate));
ReturnErrorOnFailure(sTermsAndConditionsProviderInstance.Init(&sTermsAndConditionsStorageDelegateInstance, termsAndConditions));

return CHIP_NO_ERROR;
}

CHIP_ERROR chip::app::TermsAndConditionsManager::CheckAcceptance(const Optional<TermsAndConditions> & inTermsAndConditions, TermsAndConditionsState & outState) const
{
return sTermsAndConditionsProviderInstance.CheckAcceptance(inTermsAndConditions, outState);
}

CHIP_ERROR chip::app::TermsAndConditionsManager::CommitAcceptance()
{
return sTermsAndConditionsProviderInstance.CommitAcceptance();
}

CHIP_ERROR chip::app::TermsAndConditionsManager::GetAcceptance(Optional<TermsAndConditions> & outTermsAndConditions) const
{
return sTermsAndConditionsProviderInstance.GetAcceptance(outTermsAndConditions);
}

CHIP_ERROR chip::app::TermsAndConditionsManager::GetRequirements(Optional<TermsAndConditions> & outTermsAndConditions) const
{
return sTermsAndConditionsProviderInstance.GetRequirements(outTermsAndConditions);
}

CHIP_ERROR chip::app::TermsAndConditionsManager::GetUpdateAcceptanceDeadline(Optional<uint32_t> & outUpdateAcceptanceDeadline) const
{
return sTermsAndConditionsProviderInstance.GetUpdateAcceptanceDeadline(outUpdateAcceptanceDeadline);
}

CHIP_ERROR chip::app::TermsAndConditionsManager::ResetAcceptance()
{
return sTermsAndConditionsProviderInstance.ResetAcceptance();
}

CHIP_ERROR chip::app::TermsAndConditionsManager::RevertAcceptance()
{
return sTermsAndConditionsProviderInstance.RevertAcceptance();
}

CHIP_ERROR chip::app::TermsAndConditionsManager::SetAcceptance(const Optional<TermsAndConditions> & inTermsAndConditions)
{
return sTermsAndConditionsProviderInstance.SetAcceptance(inTermsAndConditions);
}
44 changes: 44 additions & 0 deletions src/app/server/TermsAndConditionsManager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
*
* Copyright (c) 2024 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

#include <lib/core/CHIPPersistentStorageDelegate.h>

#include "TermsAndConditionsProvider.h"

namespace chip {
namespace app {

class TermsAndConditionsManager : public TermsAndConditionsProvider
{
public:
static TermsAndConditionsManager * GetInstance();
CHIP_ERROR Init(PersistentStorageDelegate * const inPersistentStorageDelegate, const Optional<TermsAndConditions> & inRequiredTermsAndConditions);
CHIP_ERROR CheckAcceptance(const Optional<TermsAndConditions> & inTermsAndConditions, TermsAndConditionsState & outState) const;
CHIP_ERROR CommitAcceptance();
CHIP_ERROR GetAcceptance(Optional<TermsAndConditions> & outTermsAndConditions) const;
CHIP_ERROR GetRequirements(Optional<TermsAndConditions> & outTermsAndConditions) const;
CHIP_ERROR GetUpdateAcceptanceDeadline(Optional<uint32_t> & outUpdateAcceptanceDeadline) const;
CHIP_ERROR ResetAcceptance();
CHIP_ERROR RevertAcceptance();
CHIP_ERROR SetAcceptance(const Optional<TermsAndConditions> & inTermsAndConditions);
};

} // namespace app
} // namespace chip

0 comments on commit 485bae5

Please sign in to comment.