Make use of DataModel::Provider in writes (#34754)

* Implement DM::Provider::Write usage * Fix compile * Fix java builds * Update src/app/InteractionModelEngine.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Make codegen data model call increasing the cluster data version * Restyle * Make sure that attribute changed information can be propagated out of ember * Optimize storage size of WriteHandler * Restyle * Make the code more obvious identical with what was there before * Change Provider to not double-call the dirty and version increase * Update src/app/InteractionModelEngine.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Cleaner usage: no need of a separate function that is used in one place only * Attempt an API update * Fix typos in the Accessors src * Fix typo and regen * More fixes on accessors * Update signature for emAfWriteAttributeExternal * Add a comment about all the checks being vague * Update src/app/util/af-types.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/af-types.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/mock/CodegenEmberMocks.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <[email protected]> * zap regen and restyle * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-table.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-table.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-table.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-table.h Co-authored-by: Boris Zbarsky <[email protected]> * Rename ChangedPathListener to AttributesChangedListener * Remove chip:: and chip::app * Update constructors of AttributePathParams and add nodiscard according to the linter to never call const methods without considering their return value * Restyled by clang-format * Remove auto-inserted include * Update again and zap regen: removed extra namespace prefixes in accessors.h/cpp * Add comment about uint8_t non-const usage... * Another rename given that the listener is now an attributes and not a path listener * Update src/app/util/attribute-table.h Co-authored-by: Tennessee Carmel-Veilleux <[email protected]> * Update after merge to have more things compile * Everything compiles now * Restyle * Make unit tests pass: mocks also have to call the change listeners * Comment update to talk more about AttributesChangedListener * Restyle * Add support for a previous path write ... this is similar to what ACL caching and tokenizing currently does, but in a explicit manner describing the ACL use case * Remove some extra added includes * Another include fix * Follow the comment and do not restrict the cache to ACL cluster, since this is what current ember does * More self code review changes * Make tests pass, more code cleanup * Match ordering to ember-compatibility functions. This is ODD because we check attribute existance before access! * Adjust test ordering and add a large note that we are probably doing the wrong thing, however tests force us to do the wrong thing * Fix unit test * Remove odd comment * Add a few ending endifs * Renamed ScopedExchangeContext * Update src/app/InteractionModelEngine.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/WriteHandler.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/WriteHandler.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/data-model-provider/OperationTypes.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/data-model-provider/OperationTypes.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/data-model-provider/OperationTypes.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Restyle to reorder includes * Add issue link * Update equality logic * Rename member to mLastSuccessfullyWrittenPath * Update argument logic * Make ActionContext private in IME so it is not such a public API * Clean up comments * fix up compares * Restyle --------- Co-authored-by: Andrei Litvin <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]> Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Tennessee Carmel-Veilleux <[email protected]>
project-chip · Sep 18, 2024 · 0b8ffb7 · 0b8ffb7
1 parent 2396bb4
commit 0b8ffb7
Show file tree

Hide file tree

Showing 20 changed files with 513 additions and 77 deletions.
diff --git a/src/app/AttributeValueDecoder.h b/src/app/AttributeValueDecoder.h
@@ -65,6 +65,8 @@ class AttributeValueDecoder
     const Access::SubjectDescriptor & GetSubjectDescriptor() const { return mSubjectDescriptor; }
 
 private:
+    friend class TestOnlyAttributeValueDecoderAccessor;
+
     TLV::TLVReader & mReader;
     bool mTriedDecode = false;
     const Access::SubjectDescriptor mSubjectDescriptor;

diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp
@@ -855,6 +855,12 @@ void EventManagement::SetScheduledEventInfo(EventNumber & aEventNumber, uint32_t
     aInitialWrittenEventBytes = mBytesWritten;
 }
 
+CHIP_ERROR EventManagement::GenerateEvent(EventLoggingDelegate * eventPayloadWriter, const EventOptions & options,
+                                          EventNumber & generatedEventNumber)
+{
+    return LogEvent(eventPayloadWriter, options, generatedEventNumber);
+}
+
 void CircularEventBuffer::Init(uint8_t * apBuffer, uint32_t aBufferLength, CircularEventBuffer * apPrev,
                                CircularEventBuffer * apNext, PriorityLevel aPriorityLevel)
 {
@@ -914,5 +920,6 @@ CHIP_ERROR CircularEventBufferWrapper::GetNextBuffer(TLVReader & aReader, const
 exit:
     return err;
 }
+
 } // namespace app
 } // namespace chip
diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h
@@ -31,6 +31,7 @@
 #include <app/EventLoggingTypes.h>
 #include <app/MessageDef/EventDataIB.h>
 #include <app/MessageDef/StatusIB.h>
+#include <app/data-model-provider/EventsGenerator.h>
 #include <app/util/basic-types.h>
 #include <lib/core/TLVCircularBuffer.h>
 #include <lib/support/CHIPCounter.h>
@@ -196,7 +197,7 @@ struct LogStorageResources
  *   more space for new events.
  */
 
-class EventManagement
+class EventManagement : public DataModel::EventsGenerator
 {
 public:
     /**
@@ -387,6 +388,10 @@ class EventManagement
      */
     void SetScheduledEventInfo(EventNumber & aEventNumber, uint32_t & aInitialWrittenEventBytes) const;
 
+    /* EventsGenerator implementation */
+    CHIP_ERROR GenerateEvent(EventLoggingDelegate * eventPayloadWriter, const EventOptions & options,
+                             EventNumber & generatedEventNumber) override;
+
 private:
     /**
      * @brief
@@ -559,5 +564,6 @@ class EventManagement
 
     System::Clock::Milliseconds64 mMonotonicStartupTime;
 };
+
 } // namespace app
 } // namespace chip
diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
@@ -895,7 +895,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnWriteRequest(Messa
     {
         if (writeHandler.IsFree())
         {
-            VerifyOrReturnError(writeHandler.Init(this) == CHIP_NO_ERROR, Status::Busy);
+            VerifyOrReturnError(writeHandler.Init(GetDataModelProvider(), this) == CHIP_NO_ERROR, Status::Busy);
             return writeHandler.OnWriteRequest(apExchangeContext, std::move(aPayload), aIsTimedWrite);
         }
     }
@@ -996,6 +996,9 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext
 
     Protocols::InteractionModel::Status status = Status::Failure;
 
+    // Ensure that DataModel::Provider has access to the exchange the message was received on.
+    CurrentExchangeValueScope scopedExchangeContext(*this, apExchangeContext);
+
     // Group Message can only be an InvokeCommandRequest or WriteRequest
     if (apExchangeContext->IsGroupExchangeContext() &&
         !aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::InvokeCommandRequest) &&
@@ -1749,16 +1752,44 @@ DataModel::Provider * InteractionModelEngine::SetDataModelProvider(DataModel::Pr
     // Alternting data model should not be done while IM is actively handling requests.
     VerifyOrDie(mReadHandlers.begin() == mReadHandlers.end());
 
-    DataModel::Provider * oldModel = GetDataModelProvider();
-    mDataModelProvider             = model;
+    DataModel::Provider * oldModel = mDataModelProvider;
+    if (oldModel != nullptr)
+    {
+        CHIP_ERROR err = oldModel->Shutdown();
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(InteractionModel, "Failure on interaction model shutdown: %" CHIP_ERROR_FORMAT, err.Format());
+        }
+    }
+
+    mDataModelProvider = model;
+    if (mDataModelProvider != nullptr)
+    {
+        DataModel::InteractionModelContext context;
+
+        context.eventsGenerator         = &EventManagement::GetInstance();
+        context.dataModelChangeListener = &mReportingEngine;
+        context.actionContext           = this;
+
+        CHIP_ERROR err = mDataModelProvider->Startup(context);
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(InteractionModel, "Failure on interaction model startup: %" CHIP_ERROR_FORMAT, err.Format());
+        }
+    }
+
     return oldModel;
 }
 
-DataModel::Provider * InteractionModelEngine::GetDataModelProvider() const
+DataModel::Provider * InteractionModelEngine::GetDataModelProvider()
 {
 #if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
-    // TODO: this should be temporary, we should fully inject the data model
-    VerifyOrReturnValue(mDataModelProvider != nullptr, CodegenDataModelProviderInstance());
+    if (mDataModelProvider == nullptr)
+    {
+        // These should be called within the CHIP processing loop.
+        assertChipStackLockedByCurrentThread();
+        SetDataModelProvider(CodegenDataModelProviderInstance());
+    }
 #endif
     return mDataModelProvider;
 }

diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h
@@ -86,6 +86,7 @@ namespace app {
  */
 class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
                                public Messaging::ExchangeDelegate,
+                               private DataModel::ActionContext,
                                public CommandResponseSender::Callback,
                                public CommandHandlerImpl::Callback,
                                public ReadHandler::ManagementCallback,
@@ -402,7 +403,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
     }
 #endif
 
-    DataModel::Provider * GetDataModelProvider() const;
+    // Temporarily NOT const because the data model provider will be auto-set
+    // to codegen on first usage. This behaviour will be changed once each
+    // application must explicitly set the data model provider.
+    DataModel::Provider * GetDataModelProvider();
 
     // MUST NOT be used while the interaction model engine is running as interaction
     // model functionality (e.g. active reads/writes/subscriptions) rely on data model
@@ -412,6 +416,9 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
     DataModel::Provider * SetDataModelProvider(DataModel::Provider * model);
 
 private:
+    /* DataModel::ActionContext implementation */
+    Messaging::ExchangeContext * CurrentExchange() override { return mCurrentExchange; }
+
     friend class reporting::Engine;
     friend class TestCommandInteraction;
     friend class TestInteractionModelEngine;
@@ -698,7 +705,23 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
 
     SubscriptionResumptionStorage * mpSubscriptionResumptionStorage = nullptr;
 
-    DataModel::Provider * mDataModelProvider = nullptr;
+    DataModel::Provider * mDataModelProvider      = nullptr;
+    Messaging::ExchangeContext * mCurrentExchange = nullptr;
+
+    // Changes the current exchange context of a InteractionModelEngine to a given context
+    class CurrentExchangeValueScope
+    {
+    public:
+        CurrentExchangeValueScope(InteractionModelEngine & engine, Messaging::ExchangeContext * context) : mEngine(engine)
+        {
+            mEngine.mCurrentExchange = context;
+        }
+
+        ~CurrentExchangeValueScope() { mEngine.mCurrentExchange = nullptr; }
+
+    private:
+        InteractionModelEngine & mEngine;
+    };
 };
 
 } // namespace app

diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp
@@ -18,30 +18,40 @@
 
 #include <app/AppConfig.h>
 #include <app/AttributeAccessInterfaceRegistry.h>
+#include <app/AttributeValueDecoder.h>
 #include <app/InteractionModelEngine.h>
 #include <app/MessageDef/EventPathIB.h>
 #include <app/MessageDef/StatusIB.h>
 #include <app/StatusResponse.h>
 #include <app/WriteHandler.h>
+#include <app/data-model-provider/OperationTypes.h>
 #include <app/reporting/Engine.h>
 #include <app/util/MatterCallbacks.h>
 #include <app/util/ember-compatibility-functions.h>
 #include <credentials/GroupDataProvider.h>
+#include <lib/core/CHIPError.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/TypeTraits.h>
+#include <messaging/ExchangeContext.h>
 #include <protocols/interaction_model/StatusCode.h>
 
+#include <optional>
+
 namespace chip {
 namespace app {
 
 using namespace Protocols::InteractionModel;
 using Status                         = Protocols::InteractionModel::Status;
 constexpr uint8_t kListAttributeType = 0x48;
 
-CHIP_ERROR WriteHandler::Init(WriteHandlerDelegate * apWriteHandlerDelegate)
+CHIP_ERROR WriteHandler::Init(DataModel::Provider * apProvider, WriteHandlerDelegate * apWriteHandlerDelegate)
 {
     VerifyOrReturnError(!mExchangeCtx, CHIP_ERROR_INCORRECT_STATE);
     VerifyOrReturnError(apWriteHandlerDelegate, CHIP_ERROR_INVALID_ARGUMENT);
+#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+    VerifyOrReturnError(apProvider, CHIP_ERROR_INVALID_ARGUMENT);
+    mDataModelProvider = apProvider;
+#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
 
     mDelegate = apWriteHandlerDelegate;
     MoveToState(State::Initialized);
@@ -63,6 +73,9 @@ void WriteHandler::Close()
     DeliverFinalListWriteEnd(false /* wasSuccessful */);
     mExchangeCtx.Release();
     mStateFlags.Clear(StateBits::kSuppressResponse);
+#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+    mDataModelProvider = nullptr;
+#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
     MoveToState(State::Uninitialized);
 }
 
@@ -354,7 +367,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData
             err = CHIP_NO_ERROR;
         }
         SuccessOrExit(err);
-        err = WriteSingleClusterData(subjectDescriptor, dataAttributePath, dataReader, this);
+        err = WriteClusterData(subjectDescriptor, dataAttributePath, dataReader);
         if (err != CHIP_NO_ERROR)
         {
             mWriteResponseBuilder.GetWriteResponses().Rollback(backup);
@@ -501,7 +514,7 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut
 
             DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Write,
                                                                   DataModelCallbacks::OperationOrder::Pre, dataAttributePath);
-            err = WriteSingleClusterData(subjectDescriptor, dataAttributePath, tmpDataReader, this);
+            err = WriteClusterData(subjectDescriptor, dataAttributePath, tmpDataReader);
             if (err != CHIP_NO_ERROR)
             {
                 ChipLogError(DataManagement,
@@ -552,14 +565,18 @@ Status WriteHandler::ProcessWriteRequest(System::PacketBufferHandle && aPayload,
     // our callees hand out Status as well.
     Status status = Status::InvalidAction;
 
+#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+    mLastSuccessfullyWrittenPath = std::nullopt;
+#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+
     reader.Init(std::move(aPayload));
 
     err = writeRequestParser.Init(reader);
     SuccessOrExit(err);
 
 #if CHIP_CONFIG_IM_PRETTY_PRINT
     writeRequestParser.PrettyPrint();
-#endif
+#endif // CHIP_CONFIG_IM_PRETTY_PRINT
     bool boolValue;
 
     boolValue = mStateFlags.Has(StateBits::kSuppressResponse);
@@ -703,5 +720,32 @@ void WriteHandler::MoveToState(const State aTargetState)
     ChipLogDetail(DataManagement, "IM WH moving to [%s]", GetStateStr());
 }
 
+CHIP_ERROR WriteHandler::WriteClusterData(const Access::SubjectDescriptor & aSubject, const ConcreteDataAttributePath & aPath,
+                                          TLV::TLVReader & aData)
+{
+    // Writes do not have a checked-path. If data model interface is enabled (both checked and only version)
+    // the write is done via the DataModel interface
+#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+    VerifyOrReturnError(mDataModelProvider != nullptr, CHIP_ERROR_INCORRECT_STATE);
+
+    DataModel::WriteAttributeRequest request;
+
+    request.path                = aPath;
+    request.subjectDescriptor   = aSubject;
+    request.previousSuccessPath = mLastSuccessfullyWrittenPath;
+    request.writeFlags.Set(DataModel::WriteFlags::kTimed, IsTimedWrite());
+
+    AttributeValueDecoder decoder(aData, aSubject);
+
+    DataModel::ActionReturnStatus status = mDataModelProvider->WriteAttribute(request, decoder);
+
+    mLastSuccessfullyWrittenPath = status.IsSuccess() ? std::make_optional(aPath) : std::nullopt;
+
+    return AddStatusInternal(aPath, StatusIB(status.GetStatusCode()));
+#else
+    return WriteSingleClusterData(aSubject, aPath, aData, this);
+#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+}
+
 } // namespace app
 } // namespace chip
diff --git a/src/app/WriteHandler.h b/src/app/WriteHandler.h
@@ -21,8 +21,10 @@
 #include <app/AppConfig.h>
 #include <app/AttributeAccessToken.h>
 #include <app/AttributePathParams.h>
+#include <app/ConcreteAttributePath.h>
 #include <app/InteractionModelDelegatePointers.h>
 #include <app/MessageDef/WriteResponseMessage.h>
+#include <app/data-model-provider/Provider.h>
 #include <lib/core/CHIPCore.h>
 #include <lib/core/TLVDebug.h>
 #include <lib/support/BitFlags.h>
@@ -69,6 +71,7 @@ class WriteHandler : public Messaging::ExchangeDelegate
      *  construction until a call to Close is made to terminate the
      *  instance.
      *
+     *  @param[in] apProvider              A valid pointer to the model used to forward writes towards
      *  @param[in] apWriteHandlerDelegate  A Valid pointer to the WriteHandlerDelegate.
      *
      *  @retval #CHIP_ERROR_INVALID_ARGUMENT on invalid pointers
@@ -77,7 +80,7 @@ class WriteHandler : public Messaging::ExchangeDelegate
      *  @retval #CHIP_NO_ERROR On success.
      *
      */
-    CHIP_ERROR Init(WriteHandlerDelegate * apWriteHandlerDelegate);
+    CHIP_ERROR Init(DataModel::Provider * apProvider, WriteHandlerDelegate * apWriteHandlerDelegate);
 
     /**
      *  Process a write request.  Parts of the processing may end up being asynchronous, but the WriteHandler
@@ -182,11 +185,20 @@ class WriteHandler : public Messaging::ExchangeDelegate
                                  System::PacketBufferHandle && aPayload) override;
     void OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) override;
 
+    // Write the given data to the given path
+    CHIP_ERROR WriteClusterData(const Access::SubjectDescriptor & aSubject, const ConcreteDataAttributePath & aPath,
+                                TLV::TLVReader & aData);
+
     Messaging::ExchangeHolder mExchangeCtx;
     WriteResponseMessage::Builder mWriteResponseBuilder;
     Optional<ConcreteAttributePath> mProcessingAttributePath;
     Optional<AttributeAccessToken> mACLCheckCache = NullOptional;
 
+#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+    DataModel::Provider * mDataModelProvider = nullptr;
+    std::optional<ConcreteAttributePath> mLastSuccessfullyWrittenPath;
+#endif
+
     // This may be a "fake" pointer or a real delegate pointer, depending
     // on CHIP_CONFIG_STATIC_GLOBAL_INTERACTION_MODEL_ENGINE setting.
     //