[IM] Write Chunking Support (#13641)

* [IM] Implement write chunking with set+append for large lists * Python API update * Update LongListAttribute * Update TestSuite * Add more tests * Fix WriteClient construct call * Run Codegen * Address comments * Restyle * Fix test * Run Codegen * Fix dirty merge * Address comments * Update access-control-server * Fix merge * Updated code-gen * Address comments * Fix unclean merge * Fix GroupWrite Encoding * Fix merge and make access-control-server diff readable * Fix dirty merge * Update group-key-mgmt-server * Fix GroupWrite attribute path on the server side * Adapt WriteHandler for #14821 * Add ACL check cache * Resolve comments * Fix typo and fix merge * Fix * Comment out #if CONFIG_IM_BUILD_FOR_UNIT_TEST in WriteClient.h * Comment out #if CONFIG_IM_BUILD_FOR_UNIT_TEST in WriteClient.h Co-authored-by: Jerry Johns <[email protected]>
project-chip · Sep 1, 2023 · a05f741 · a05f741
1 parent e5a1fd0
commit a05f741
Show file tree

Hide file tree

Showing 46 changed files with 4,917 additions and 3,372 deletions.
diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter
@@ -2957,7 +2957,7 @@ server cluster TestCluster = 1295 {
   attribute int8s rangeRestrictedInt8s = 39;
   attribute int16u rangeRestrictedInt16u = 40;
   attribute int16s rangeRestrictedInt16s = 41;
-  readonly attribute LONG_OCTET_STRING listLongOctetString[] = 42;
+  attribute LONG_OCTET_STRING listLongOctetString[] = 42;
   readonly attribute TestFabricScoped listFabricScoped[] = 43;
   attribute boolean timedWriteBoolean = 48;
   attribute boolean generalErrorBoolean = 49;

diff --git a/examples/chip-tool/commands/clusters/WriteAttributeCommand.h b/examples/chip-tool/commands/clusters/WriteAttributeCommand.h
@@ -58,7 +58,7 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb
     }
 
     /////////// WriteClient Callback Interface /////////
-    void OnResponse(const chip::app::WriteClient * client, const chip::app::ConcreteAttributePath & path,
+    void OnResponse(const chip::app::WriteClient * client, const chip::app::ConcreteDataAttributePath & path,
                     chip::app::StatusIB status) override
     {
         CHIP_ERROR error = status.ToChipError();
@@ -98,7 +98,7 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb
 
         mWriteClient = std::make_unique<chip::app::WriteClient>(device->GetExchangeManager(), this, mTimedInteractionTimeoutMs);
 
-        ReturnErrorOnFailure(mWriteClient->EncodeAttributeWritePayload(attributePathParams, value));
+        ReturnErrorOnFailure(mWriteClient->EncodeAttribute(attributePathParams, value));
         return mWriteClient->SendWriteRequest(device->GetSecureSession().Value());
     }
 

diff --git a/src/app/AttributeAccessToken.h b/src/app/AttributeAccessToken.h
@@ -0,0 +1,45 @@
+/*
+ *
+ *    Copyright (c) 2022 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 <access/Privilege.h>
+#include <app/ConcreteAttributePath.h>
+
+namespace chip {
+namespace app {
+
+/**
+ * AttributeAccessToken records the privilege granted for accessing the specified attribute. This struct is used in chunked write
+ * to avoid losing privilege when updating ACL items.
+ */
+struct AttributeAccessToken
+{
+    ConcreteAttributePath mPath;
+    Access::Privilege mPrivilege;
+
+    bool operator==(const AttributeAccessToken & other) const { return mPath == other.mPath && mPrivilege == other.mPrivilege; }
+
+    bool Matches(const ConcreteAttributePath & aPath, const Access::Privilege & aPrivilege) const
+    {
+        return mPath == aPath && mPrivilege == aPrivilege;
+    }
+};
+
+} // namespace app
+} // namespace chip
diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
@@ -48,6 +48,8 @@ static_library("app") {
     "CASEClientPool.h",
     "CASESessionManager.cpp",
     "CASESessionManager.h",
+    "ChunkedWriteCallback.cpp",
+    "ChunkedWriteCallback.h",
     "CommandHandler.cpp",
     "CommandResponseHelper.h",
     "CommandSender.cpp",

diff --git a/src/app/ChunkedWriteCallback.cpp b/src/app/ChunkedWriteCallback.cpp
@@ -0,0 +1,91 @@
+/*
+ *
+ *    Copyright (c) 2022 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 <app/ChunkedWriteCallback.h>
+
+namespace chip {
+namespace app {
+
+void ChunkedWriteCallback::OnResponse(const WriteClient * apWriteClient, const ConcreteDataAttributePath & aPath, StatusIB aStatus)
+{
+    // We may send a chunked list. To make the behavior consistent whether a list is being chunked or not,
+    // we merge the write responses for a chunked list here and provide our consumer with a single status response.
+    if (mLastAttributePath.HasValue())
+    {
+        // This is not the first write response.
+        if (IsAppendingToLastItem(aPath))
+        {
+            // This is a response on the same path as what we already have stored. Report the first
+            // failure status we encountered, and ignore subsequent ones.
+            if (mAttributeStatus.IsSuccess())
+            {
+                mAttributeStatus = aStatus;
+            }
+            return;
+        }
+        else
+        {
+            // This is a response to another attribute write. Report the final result of last attribute write.
+            callback->OnResponse(apWriteClient, mLastAttributePath.Value(), mAttributeStatus);
+        }
+    }
+
+    // This is the first report for a new attribute.  We assume it will never be a list item operation.
+    if (aPath.IsListItemOperation())
+    {
+        aStatus = StatusIB(CHIP_ERROR_INCORRECT_STATE);
+    }
+
+    mLastAttributePath.SetValue(aPath);
+    mAttributeStatus = aStatus;
+    // For the last status in the response, we will call the application callback in OnDone()
+}
+
+void ChunkedWriteCallback::OnError(const WriteClient * apWriteClient, CHIP_ERROR aError)
+{
+    callback->OnError(apWriteClient, aError);
+}
+
+void ChunkedWriteCallback::OnDone(WriteClient * apWriteClient)
+{
+    if (mLastAttributePath.HasValue())
+    {
+        // We have a cached status that has yet to be reported to the application so report it now.
+        // If we failed to receive the response, or we received a malformed response, OnResponse won't be called,
+        // mLastAttributePath will be Missing() in this case.
+        callback->OnResponse(apWriteClient, mLastAttributePath.Value(), mAttributeStatus);
+    }
+
+    callback->OnDone(apWriteClient);
+}
+
+bool ChunkedWriteCallback::IsAppendingToLastItem(const ConcreteDataAttributePath & aPath)
+{
+    if (!aPath.IsListItemOperation())
+    {
+        return false;
+    }
+    if (!mLastAttributePath.HasValue() || !(mLastAttributePath.Value() == aPath))
+    {
+        return false;
+    }
+    return aPath.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem;
+}
+
+} // namespace app
+} // namespace chip
diff --git a/src/app/ChunkedWriteCallback.h b/src/app/ChunkedWriteCallback.h
@@ -0,0 +1,55 @@
+/*
+ *
+ *    Copyright (c) 2022 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 <app/WriteClient.h>
+
+namespace chip {
+namespace app {
+
+/*
+ * This is an adapter that intercepts calls that deliver status codes from the WriteClient and
+ * selectively "merge"s the status codes for a chunked list write as follows:
+ * - If the whole list was successfully written, callback->OnResponse will be called with success.
+ * - If any element in the list was not successfully written, callback->OnResponse will be called with the first error received.
+ * - callback->OnResponse will always have NotList as mListOp since we have merged the chunked responses.
+ * The merge logic assumes all list operations are part of list chunking.
+ */
+class ChunkedWriteCallback : public WriteClient::Callback
+{
+public:
+    ChunkedWriteCallback(WriteClient::Callback * apCallback) : callback(apCallback) {}
+
+    void OnResponse(const WriteClient * apWriteClient, const ConcreteDataAttributePath & aPath, StatusIB status) override;
+    void OnError(const WriteClient * apWriteClient, CHIP_ERROR aError) override;
+    void OnDone(WriteClient * apWriteClient) override;
+
+private:
+    bool IsAppendingToLastItem(const ConcreteDataAttributePath & aPath);
+
+    // We are using the casts between ConcreteAttributePath and ConcreteDataAttributePath, then all paths passed to upper
+    // applications will always have NotList as mListOp.
+    Optional<ConcreteAttributePath> mLastAttributePath;
+    StatusIB mAttributeStatus;
+
+    WriteClient::Callback * callback;
+};
+
+} // namespace app
+} // namespace chip
diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h
@@ -337,7 +337,8 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr
 /**
  * TODO: Document.
  */
-CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, ClusterInfo & aClusterInfo,
-                                  TLV::TLVReader & aReader, WriteHandler * apWriteHandler);
+CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor,
+                                  const ConcreteDataAttributePath & aAttributePath, TLV::TLVReader & aReader,
+                                  WriteHandler * apWriteHandler);
 } // namespace app
 } // namespace chip
diff --git a/src/app/MessageDef/AttributePathIB.cpp b/src/app/MessageDef/AttributePathIB.cpp
@@ -198,6 +198,33 @@ CHIP_ERROR AttributePathIB::Parser::GetListIndex(DataModel::Nullable<ListIndex>
     return GetNullableUnsignedInteger(to_underlying(Tag::kListIndex), apListIndex);
 }
 
+CHIP_ERROR AttributePathIB::Parser::GetListIndex(ConcreteDataAttributePath & aAttributePath) const
+{
+    CHIP_ERROR err = CHIP_NO_ERROR;
+    DataModel::Nullable<ListIndex> listIndex;
+    err = GetListIndex(&(listIndex));
+    if (err == CHIP_NO_ERROR)
+    {
+        if (listIndex.IsNull())
+        {
+            aAttributePath.mListOp = ConcreteDataAttributePath::ListOperation::AppendItem;
+        }
+        else
+        {
+            // TODO: Add ListOperation::ReplaceItem support. (Attribute path with valid list index)
+            err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
+        }
+    }
+    else if (CHIP_END_OF_TLV == err)
+    {
+        // We do not have the context for the actual data type here. We always set the list operation to not list and the users
+        // should interpret it as ReplaceAll when the attribute type is a list.
+        aAttributePath.mListOp = ConcreteDataAttributePath::ListOperation::NotList;
+        err                    = CHIP_NO_ERROR;
+    }
+    return err;
+}
+
 AttributePathIB::Builder & AttributePathIB::Builder::EnableTagCompression(const bool aEnableTagCompression)
 {
     // skip if error has already been set
@@ -300,5 +327,29 @@ CHIP_ERROR AttributePathIB::Builder::Encode(const AttributePathParams & aAttribu
     return GetError();
 }
 
+CHIP_ERROR AttributePathIB::Builder::Encode(const ConcreteDataAttributePath & aAttributePath)
+{
+    Endpoint(aAttributePath.mEndpointId);
+    Cluster(aAttributePath.mClusterId);
+    Attribute(aAttributePath.mAttributeId);
+
+    if (!aAttributePath.IsListOperation() || aAttributePath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll)
+    {
+        /* noop */
+    }
+    else if (aAttributePath.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
+    {
+        ListIndex(DataModel::NullNullable);
+    }
+    else
+    {
+        // TODO: Add ListOperation::ReplaceItem support. (Attribute path with valid list index)
+        return CHIP_ERROR_INVALID_ARGUMENT;
+    }
+
+    EndOfAttributePathIB();
+    return GetError();
+}
+
 } // namespace app
 } // namespace chip
diff --git a/src/app/MessageDef/AttributePathIB.h b/src/app/MessageDef/AttributePathIB.h
@@ -139,6 +139,18 @@ class Parser : public ListParser
      *          #CHIP_END_OF_TLV if there is no such element
      */
     CHIP_ERROR GetListIndex(DataModel::Nullable<ListIndex> * const apListIndex) const;
+
+    /**
+     * @brief Get the ListIndex, and set the mListIndex and mListOp fields in the ConcreteDataAttributePath accordingly. It will set
+     * ListOp to NotList when the list index is missing, users should interpret it as ReplaceAll according to the context.
+     *
+     *  @param [in] aAttributePath    The attribute path object for setting list index and list op.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     */
+    CHIP_ERROR GetListIndex(ConcreteDataAttributePath & aAttributePath) const;
+
+    // TODO(#14934) Add a function to get ConcreteDataAttributePath from AttributePathIB::Parser directly.
 };
 
 class Builder : public ListBuilder
@@ -209,6 +221,7 @@ class Builder : public ListBuilder
     AttributePathIB::Builder & EndOfAttributePathIB();
 
     CHIP_ERROR Encode(const AttributePathParams & aAttributePathParams);
+    CHIP_ERROR Encode(const ConcreteDataAttributePath & aAttributePathParams);
 };
 } // namespace AttributePathIB
 } // namespace app

diff --git a/src/app/MessageDef/WriteRequestMessage.cpp b/src/app/MessageDef/WriteRequestMessage.cpp
@@ -125,8 +125,7 @@ CHIP_ERROR WriteRequestMessage::Parser::CheckSchemaValidity() const
 
     if (CHIP_END_OF_TLV == err)
     {
-        const int RequiredFields = (1 << to_underlying(Tag::kIsFabricFiltered)) | (1 << to_underlying(Tag::kTimedRequest)) |
-            (1 << to_underlying(Tag::kWriteRequests));
+        const int RequiredFields = ((1 << to_underlying(Tag::kTimedRequest)) | (1 << to_underlying(Tag::kWriteRequests)));
 
         if ((tagPresenceMask & RequiredFields) == RequiredFields)
         {

diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp
@@ -533,22 +533,7 @@ CHIP_ERROR ReadClient::ProcessAttributePath(AttributePathIB::Parser & aAttribute
     VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
     err = aAttributePathParser.GetAttribute(&(aAttributePath.mAttributeId));
     VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
-
-    DataModel::Nullable<ListIndex> listIndex;
-    err = aAttributePathParser.GetListIndex(&(listIndex));
-    if (CHIP_END_OF_TLV == err)
-    {
-        err = CHIP_NO_ERROR;
-    }
-    else if (listIndex.IsNull())
-    {
-        aAttributePath.mListOp = ConcreteDataAttributePath::ListOperation::AppendItem;
-    }
-    else
-    {
-        // TODO: Add ListOperation::ReplaceItem support. (Attribute path with valid list index)
-        err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
-    }
+    err = aAttributePathParser.GetListIndex(aAttributePath);
     VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
     return CHIP_NO_ERROR;
 }