[IM] Add Fabric Filtered Write Support (#13756)

* [IM] Add helper for fabric scoped / fabric filtered writes * Run Codegen * Update access control server * Fix merge * Remove DecodeWithoutFabricIndex * Resolve conflicts and ready for merge * Run Codegen * Fix build * Address Comments * Fix code * Removed extraneous SFINAE logic in Decode.h * Run Codegen Co-authored-by: Jerry Johns <[email protected]>
project-chip · Feb 10, 2022 · 5381145 · 5381145
1 parent bc9406b
commit 5381145
Show file tree

Hide file tree

Showing 11 changed files with 2,005 additions and 33 deletions.
diff --git a/src/app/AttributeAccessInterface.h b/src/app/AttributeAccessInterface.h
@@ -316,13 +316,24 @@ class AttributeValueDecoder
         mReader(aReader), mSubjectDescriptor(aSubjectDescriptor)
     {}
 
-    template <typename T>
+    template <typename T, typename std::enable_if_t<!DataModel::IsFabricScoped<T>::value, bool> = true>
     CHIP_ERROR Decode(T & aArg)
     {
         mTriedDecode = true;
         return DataModel::Decode(mReader, aArg);
     }
 
+    template <typename T, typename std::enable_if_t<DataModel::IsFabricScoped<T>::value, bool> = true>
+    CHIP_ERROR Decode(T & aArg)
+    {
+        mTriedDecode = true;
+        // TODO: We may want to reject kUndefinedFabricIndex for writing fabric scoped data. mAccessingFabricIndex will be
+        // kUndefinedFabricIndex on PASE sessions.
+        ReturnErrorOnFailure(DataModel::Decode(mReader, aArg));
+        aArg.SetFabricIndex(AccessingFabricIndex());
+        return CHIP_NO_ERROR;
+    }
+
     bool TriedDecode() const { return mTriedDecode; }
 
     /**

diff --git a/src/app/clusters/access-control-server/access-control-server.cpp b/src/app/clusters/access-control-server/access-control-server.cpp
@@ -282,6 +282,8 @@ struct AccessControlEntryCodec
         return fabricIndex;
     }
 
+    void SetFabricIndex(FabricIndex fabricIndex) { entry.SetFabricIndex(fabricIndex); }
+
     AccessControl::Entry entry;
 };
 
@@ -435,30 +437,43 @@ CHIP_ERROR AccessControlAttribute::Write(const ConcreteDataAttributePath & aPath
 
 CHIP_ERROR AccessControlAttribute::WriteAcl(AttributeValueDecoder & aDecoder)
 {
+    FabricIndex accessingFabricIndex = aDecoder.AccessingFabricIndex();
+
     DataModel::DecodableList<AccessControlEntryCodec> list;
     ReturnErrorOnFailure(aDecoder.Decode(list));
 
-    size_t oldCount;
+    size_t oldCount = 0;
+    size_t allCount;
     size_t newCount;
     size_t maxCount;
-    ReturnErrorOnFailure(GetAccessControl().GetEntryCount(oldCount));
+
+    AccessControl::EntryIterator it;
+    AccessControl::Entry entry;
+    ReturnErrorOnFailure(GetAccessControl().Entries(it, &accessingFabricIndex));
+    while (it.Next(entry) == CHIP_NO_ERROR)
+    {
+        oldCount++;
+    }
+
+    ReturnErrorOnFailure(GetAccessControl().GetEntryCount(allCount));
     ReturnErrorOnFailure(list.ComputeSize(&newCount));
     ReturnErrorOnFailure(GetAccessControl().GetMaxEntryCount(maxCount));
-    ReturnErrorCodeIf(newCount > maxCount, CHIP_ERROR_INVALID_LIST_LENGTH);
+    VerifyOrReturnError(allCount >= oldCount, CHIP_ERROR_INTERNAL);
+    VerifyOrReturnError(static_cast<size_t>(allCount - oldCount + newCount) <= maxCount, CHIP_ERROR_INVALID_LIST_LENGTH);
 
     auto iterator = list.begin();
     size_t i      = 0;
     while (iterator.Next())
     {
         if (i < oldCount)
         {
-            ReturnErrorOnFailure(GetAccessControl().UpdateEntry(i, iterator.GetValue().entry));
+            ReturnErrorOnFailure(GetAccessControl().UpdateEntry(i, iterator.GetValue().entry, &accessingFabricIndex));
             ReturnErrorOnFailure(LogAccessControlEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
                                                        AccessControlCluster::ChangeTypeEnum::kChanged));
         }
         else
         {
-            ReturnErrorOnFailure(GetAccessControl().CreateEntry(nullptr, iterator.GetValue().entry));
+            ReturnErrorOnFailure(GetAccessControl().CreateEntry(nullptr, iterator.GetValue().entry, &accessingFabricIndex));
             ReturnErrorOnFailure(LogAccessControlEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
                                                        AccessControlCluster::ChangeTypeEnum::kAdded));
         }
@@ -469,7 +484,7 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(AttributeValueDecoder & aDecoder)
     while (i < oldCount)
     {
         --oldCount;
-        ReturnErrorOnFailure(GetAccessControl().DeleteEntry(oldCount));
+        ReturnErrorOnFailure(GetAccessControl().DeleteEntry(oldCount, &accessingFabricIndex));
         ReturnErrorOnFailure(LogAccessControlEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
                                                    AccessControlCluster::ChangeTypeEnum::kRemoved));
     }

diff --git a/src/app/data-model/DecodableList.h b/src/app/data-model/DecodableList.h
@@ -20,6 +20,7 @@
 
 #include <app/data-model/Decode.h>
 #include <app/data-model/Encode.h>
+#include <app/data-model/FabricScoped.h>
 #include <lib/core/CHIPTLV.h>
 
 namespace chip {
@@ -39,6 +40,8 @@ class DecodableList
 public:
     DecodableList() { ClearReader(); }
 
+    static constexpr bool kIsFabricScoped = DataModel::IsFabricScoped<T>::value;
+
     /*
      * @brief
      *
@@ -56,6 +59,12 @@ class DecodableList
      */
     void ClearReader() { mReader.Init(nullptr, 0); }
 
+    template <typename T0 = T, std::enable_if_t<DataModel::IsFabricScoped<T0>::value, bool> = true>
+    void SetFabricIndex(FabricIndex fabricIndex)
+    {
+        mFabricIndex.SetValue(fabricIndex);
+    }
+
     class Iterator
     {
     public:
@@ -67,7 +76,7 @@ class DecodableList
          * have a `kTLVType_NotSpecified` container type if there is
          * no list.
          */
-        Iterator(const TLV::TLVReader & reader)
+        Iterator(const TLV::TLVReader & reader, Optional<FabricIndex> fabricIndex) : mFabricIndex(fabricIndex)
         {
             mStatus = CHIP_NO_ERROR;
             mReader.Init(reader);
@@ -88,24 +97,23 @@ class DecodableList
          * this shall return false as well. The caller is expected to invoke GetStatus()
          * to retrieve the status of the operation.
          */
+        template <typename T0 = T, std::enable_if_t<!DataModel::IsFabricScoped<T0>::value, bool> = true>
         bool Next()
         {
-            if (mReader.GetContainerType() == TLV::kTLVType_NotSpecified)
-            {
-                return false;
-            }
+            return DoNext();
+        }
 
-            if (mStatus == CHIP_NO_ERROR)
-            {
-                mStatus = mReader.Next();
-            }
+        template <typename T0 = T, std::enable_if_t<DataModel::IsFabricScoped<T0>::value, bool> = true>
+        bool Next()
+        {
+            bool hasNext = DoNext();
 
-            if (mStatus == CHIP_NO_ERROR)
+            if (hasNext && mFabricIndex.HasValue())
             {
-                mStatus = Decode(mReader, mValue);
+                mValue.SetFabricIndex(mFabricIndex.Value());
             }
 
-            return (mStatus == CHIP_NO_ERROR);
+            return hasNext;
         }
 
         /*
@@ -133,12 +141,35 @@ class DecodableList
         }
 
     private:
+        bool DoNext()
+        {
+            if (mReader.GetContainerType() == TLV::kTLVType_NotSpecified)
+            {
+                return false;
+            }
+
+            if (mStatus == CHIP_NO_ERROR)
+            {
+                mStatus = mReader.Next();
+            }
+
+            if (mStatus == CHIP_NO_ERROR)
+            {
+                mStatus = DataModel::Decode(mReader, mValue);
+            }
+
+            return (mStatus == CHIP_NO_ERROR);
+        }
+
         T mValue;
         CHIP_ERROR mStatus;
         TLV::TLVReader mReader;
+        // TODO: Consider some setup where this field does not exist when T
+        // is not a fabric scoped struct.
+        const Optional<FabricIndex> mFabricIndex;
     };
 
-    Iterator begin() const { return Iterator(mReader); }
+    Iterator begin() const { return Iterator(mReader, mFabricIndex); }
 
     /*
      * Compute the size of the list. This can fail if the TLV is malformed. If
@@ -160,21 +191,21 @@ class DecodableList
         }
     }
 
+    CHIP_ERROR Decode(TLV::TLVReader & reader)
+    {
+        VerifyOrReturnError(reader.GetType() == TLV::kTLVType_Array, CHIP_ERROR_SCHEMA_MISMATCH);
+        TLV::TLVType type;
+        ReturnErrorOnFailure(reader.EnterContainer(type));
+        SetReader(reader);
+        ReturnErrorOnFailure(reader.ExitContainer(type));
+        return CHIP_NO_ERROR;
+    }
+
 private:
     TLV::TLVReader mReader;
+    chip::Optional<FabricIndex> mFabricIndex;
 };
 
-template <typename X>
-CHIP_ERROR Decode(TLV::TLVReader & reader, DecodableList<X> & x)
-{
-    VerifyOrReturnError(reader.GetType() == TLV::kTLVType_Array, CHIP_ERROR_SCHEMA_MISMATCH);
-    TLV::TLVType type;
-    ReturnErrorOnFailure(reader.EnterContainer(type));
-    x.SetReader(reader);
-    ReturnErrorOnFailure(reader.ExitContainer(type));
-    return CHIP_NO_ERROR;
-}
-
 } // namespace DataModel
 } // namespace app
 } // namespace chip
diff --git a/src/app/data-model/Nullable.h b/src/app/data-model/Nullable.h
@@ -74,6 +74,9 @@ struct Nullable : protected Optional<T>
         return true;
     }
 
+    // The only fabric-scoped objects in the spec are events and structs inside lists, and neither one can be nullable.
+    static constexpr bool kIsFabricScoped = false;
+
     bool operator==(const Nullable & other) const { return Optional<T>::operator==(other); }
     bool operator!=(const Nullable & other) const { return !(*this == other); }
 };

diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn
@@ -42,6 +42,7 @@ chip_test_suite("tests") {
 
   test_sources = [
     "TestAttributePathExpandIterator.cpp",
+    "TestAttributeValueDecoder.cpp",
     "TestAttributeValueEncoder.cpp",
     "TestBuilderParser.cpp",
     "TestClusterInfo.cpp",