From 485bae5b160b8e54e1483f9c58934f91562baee0 Mon Sep 17 00:00:00 2001 From: James Swan <122404367+swan-amazon@users.noreply.github.com> Date: Thu, 5 Dec 2024 18:44:22 +0000 Subject: [PATCH] [Terms and Conditions] Introduce TermsAndConditionsManager singleton 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. --- examples/platform/linux/AppMain.cpp | 9 +++ .../terms-and-conditions-app/linux/BUILD.gn | 6 +- .../general-commissioning-server.cpp | 22 +++--- src/app/server/BUILD.gn | 1 + src/app/server/Server.cpp | 4 +- src/app/server/Server.h | 24 +----- src/app/server/TermsAndConditionsManager.cpp | 79 +++++++++++++++++++ src/app/server/TermsAndConditionsManager.h | 44 +++++++++++ 8 files changed, 153 insertions(+), 36 deletions(-) create mode 100644 src/app/server/TermsAndConditionsManager.cpp create mode 100644 src/app/server/TermsAndConditionsManager.h diff --git a/examples/platform/linux/AppMain.cpp b/examples/platform/linux/AppMain.cpp index 2f69e4da048cd0..f0b2040d455157 100644 --- a/examples/platform/linux/AppMain.cpp +++ b/examples/platform/linux/AppMain.cpp @@ -57,6 +57,10 @@ #include #endif +#if CHIP_CONFIG_TC_REQUIRED +#include +#endif + #if defined(PW_RPC_ENABLED) #include #endif @@ -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 termsAndConditions = Optional(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(); diff --git a/examples/terms-and-conditions-app/linux/BUILD.gn b/examples/terms-and-conditions-app/linux/BUILD.gn index 6a1233b00ebb15..2a5312a758e49a 100644 --- a/examples/terms-and-conditions-app/linux/BUILD.gn +++ b/examples/terms-and-conditions-app/linux/BUILD.gn @@ -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", diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp index 90345092a5cfd0..cc3d808c2b206c 100644 --- a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp +++ b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp @@ -39,6 +39,10 @@ #include #include +#if CHIP_CONFIG_TC_REQUIRED +#include +#endif + using namespace chip; using namespace chip::app; using namespace chip::app::Clusters; @@ -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 outTermsAndConditions; if (nullptr == termsAndConditionsProvider) @@ -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 outTermsAndConditions; if (nullptr == termsAndConditionsProvider) @@ -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 outTermsAndConditions; if (nullptr == termsAndConditionsProvider) @@ -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 outTermsAndConditions; TermsAndConditionsState termsAndConditionsState; @@ -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 outUpdateAcceptanceDeadline; if (nullptr == termsAndConditionsProvider) @@ -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) @@ -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 acceptedTermsAndConditions = Optional(TermsAndConditions(commandData.TCUserResponse, commandData.TCVersion)); @@ -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) { @@ -611,7 +615,7 @@ class GeneralCommissioningFabricTableDelegate : public chip::FabricTable::Delega static_cast(fabricIndex)); #if CHIP_CONFIG_TC_REQUIRED - TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider(); + TermsAndConditionsProvider * const termsAndConditionsProvider = TermsAndConditionsManager::GetInstance(); if (nullptr != termsAndConditionsProvider) { diff --git a/src/app/server/BUILD.gn b/src/app/server/BUILD.gn index ee04495673133d..52baeb05abbb16 100644 --- a/src/app/server/BUILD.gn +++ b/src/app/server/BUILD.gn @@ -48,6 +48,7 @@ static_library("server") { "Server.cpp", "Server.h", "TermsAndConditionsProvider.h", + "TermsAndConditionsManager.h", ] public_configs = [ ":server_config" ] diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index ce1bc8a77ea570..7363d519b5c5a4 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -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. @@ -209,8 +209,6 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams) mReportScheduler = initParams.reportScheduler; - mTermsAndConditionsProvider = initParams.termsAndConditionsProvider; - mTestEventTriggerDelegate = initParams.testEventTriggerDelegate; if (mTestEventTriggerDelegate == nullptr) { diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 9aa8693345aff0..315b65977329d7 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -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. @@ -33,7 +33,6 @@ #include #include #include -#include #include #include #include @@ -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; }; /** @@ -318,21 +314,6 @@ struct CommonCaseDeviceServerInitParams : public ServerInitParams } #endif -#if CHIP_CONFIG_TC_REQUIRED - static app::DefaultTermsAndConditionsProvider sDefaultTermsAndConditionsProviderInstance; - static app::DefaultTermsAndConditionsStorageDelegate sDefaultTermsAndConditionsStorageDelegateInstance; - - Optional termsAndConditions = Optional( - 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; } @@ -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; } @@ -710,7 +689,6 @@ class Server GroupDataProviderListener mListener; ServerFabricDelegate mFabricDelegate; app::reporting::ReportScheduler * mReportScheduler; - app::TermsAndConditionsProvider * mTermsAndConditionsProvider; Access::AccessControl mAccessControl; app::AclStorage * mAclStorage; diff --git a/src/app/server/TermsAndConditionsManager.cpp b/src/app/server/TermsAndConditionsManager.cpp new file mode 100644 index 00000000000000..094a296e787a46 --- /dev/null +++ b/src/app/server/TermsAndConditionsManager.cpp @@ -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 & inRequiredTermsAndConditions) +{ + Optional termsAndConditions = Optional(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 & 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 & outTermsAndConditions) const +{ + return sTermsAndConditionsProviderInstance.GetAcceptance(outTermsAndConditions); +} + +CHIP_ERROR chip::app::TermsAndConditionsManager::GetRequirements(Optional & outTermsAndConditions) const +{ + return sTermsAndConditionsProviderInstance.GetRequirements(outTermsAndConditions); +} + +CHIP_ERROR chip::app::TermsAndConditionsManager::GetUpdateAcceptanceDeadline(Optional & 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 & inTermsAndConditions) +{ + return sTermsAndConditionsProviderInstance.SetAcceptance(inTermsAndConditions); +} diff --git a/src/app/server/TermsAndConditionsManager.h b/src/app/server/TermsAndConditionsManager.h new file mode 100644 index 00000000000000..b6fa9fa5bb5b79 --- /dev/null +++ b/src/app/server/TermsAndConditionsManager.h @@ -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 + +#include "TermsAndConditionsProvider.h" + +namespace chip { +namespace app { + +class TermsAndConditionsManager : public TermsAndConditionsProvider +{ +public: + static TermsAndConditionsManager * GetInstance(); + CHIP_ERROR Init(PersistentStorageDelegate * const inPersistentStorageDelegate, const Optional & inRequiredTermsAndConditions); + CHIP_ERROR CheckAcceptance(const Optional & inTermsAndConditions, TermsAndConditionsState & outState) const; + CHIP_ERROR CommitAcceptance(); + CHIP_ERROR GetAcceptance(Optional & outTermsAndConditions) const; + CHIP_ERROR GetRequirements(Optional & outTermsAndConditions) const; + CHIP_ERROR GetUpdateAcceptanceDeadline(Optional & outUpdateAcceptanceDeadline) const; + CHIP_ERROR ResetAcceptance(); + CHIP_ERROR RevertAcceptance(); + CHIP_ERROR SetAcceptance(const Optional & inTermsAndConditions); +}; + +} // namespace app +} // namespace chip