Some interface cleanup and make AttributeValueEncoder/Decoder more similar

src/app/AttributeValueDecoder.h

@@ -46,19 +46,14 @@ class AttributeValueDecoder
         mTriedDecode = true;
         // The WriteRequest comes with no fabric index, this will happen when receiving a write request on a PASE session before
         // AddNOC.
-        VerifyOrReturnError(AccessingFabricIndex() != kUndefinedFabricIndex, CHIP_IM_GLOBAL_STATUS(UnsupportedAccess));
+        VerifyOrReturnError(GetSubjectDescriptor().fabricIndex != kUndefinedFabricIndex, CHIP_IM_GLOBAL_STATUS(UnsupportedAccess));
         ReturnErrorOnFailure(DataModel::Decode(mReader, aArg));
-        aArg.SetFabricIndex(AccessingFabricIndex());
+        aArg.SetFabricIndex(GetSubjectDescriptor().fabricIndex);
         return CHIP_NO_ERROR;
     }
 
     bool TriedDecode() const { return mTriedDecode; }
 
-    /**
-     * The accessing fabric index for this write interaction.
-     */
-    FabricIndex AccessingFabricIndex() const { return mSubjectDescriptor.fabricIndex; }
-
     /**
      * The accessing subject descriptor for this write interaction.
      */

src/app/AttributeValueEncoder.cpp

@@ -32,13 +32,13 @@ CHIP_ERROR AttributeValueEncoder::EnsureListStarted()
 {
     VerifyOrDie(mCurrentEncodingListIndex == kInvalidListIndex);
 
-    mEncodingInitialList = (mEncodeState.mCurrentEncodingListIndex == kInvalidListIndex);
+    mEncodingInitialList = (mEncodeState.CurrentEncodingListIndex() == kInvalidListIndex);
     if (mEncodingInitialList)
     {
         // Clear mAllowPartialData flag here since this encode procedure is not atomic.
         // The most common error in this function is CHIP_ERROR_NO_MEMORY / CHIP_ERROR_BUFFER_TOO_SMALL, just revert and try
         // next time is ok.
-        mEncodeState.mAllowPartialData = false;
+        mEncodeState.SetAllowPartialData(false);
 
         AttributeReportBuilder builder;
 
@@ -59,7 +59,7 @@ CHIP_ERROR AttributeValueEncoder::EnsureListStarted()
         ReturnErrorOnFailure(
             mAttributeReportIBsBuilder.GetWriter()->ReserveBuffer(kEndOfAttributeReportIBByteCount + kEndOfListByteCount));
 
-        mEncodeState.mCurrentEncodingListIndex = 0;
+        mEncodeState.SetCurrentEncodingListIndex(0);
     }
     else
     {
@@ -72,7 +72,7 @@ CHIP_ERROR AttributeValueEncoder::EnsureListStarted()
 
     // After encoding the initial list start, the remaining items are atomically encoded into the buffer. Tell report engine to not
     // revert partial data.
-    mEncodeState.mAllowPartialData = true;
+    mEncodeState.SetAllowPartialData(true);
 
     return CHIP_NO_ERROR;
 }
@@ -105,7 +105,7 @@ void AttributeValueEncoder::EnsureListEnded()
         // If we succeeded at encoding the whole list (i.e. the list is in fact
         // empty and we fit in the packet), mAllowPartialData will be ignored,
         // so it's safe to set it to false even if encoding succeeded.
-        mEncodeState.mAllowPartialData = false;
+        mEncodeState.SetAllowPartialData(false);
     }
 }
 

src/app/AttributeValueEncoder.h

@@ -16,6 +16,7 @@
  */
 #pragma once
 
+#include <access/SubjectDescriptor.h>
 #include <app/AttributeReportBuilder.h>
 #include <app/ConcreteAttributePath.h>
 #include <app/MessageDef/AttributeReportIBs.h>
@@ -27,6 +28,60 @@
 namespace chip {
 namespace app {
 
+/**
+ * Maintains the internal state of list encoding
+ */
+class AttributeEncodeState
+{
+public:
+    AttributeEncodeState() = default;
+
+    bool AllowPartialData() const { return mAllowPartialData; }
+    ListIndex CurrentEncodingListIndex() const { return mCurrentEncodingListIndex; }
+
+    AttributeEncodeState & SetAllowPartialData(bool allow)
+    {
+        mAllowPartialData = allow;
+        return *this;
+    }
+
+    AttributeEncodeState & SetCurrentEncodingListIndex(ListIndex idx)
+    {
+        mCurrentEncodingListIndex = idx;
+        return *this;
+    }
+
+    void Reset()
+    {
+        mCurrentEncodingListIndex = kInvalidListIndex;
+        mAllowPartialData         = false;
+    }
+
+private:
+    /**
+     * If set to kInvalidListIndex, indicates that we have not encoded any data for the list yet and
+     * need to start by encoding an empty list before we start encoding any list items.
+     *
+     * When set to a valid ListIndex value, indicates the index of the next list item that needs to be
+     * encoded (i.e. the count of items encoded so far).
+     */
+    ListIndex mCurrentEncodingListIndex = kInvalidListIndex;
+
+    /**
+     * When an attempt to encode an attribute returns an error, the buffer may contain tailing dirty data
+     * (since the put was aborted).  The report engine normally rolls back the buffer to right before encoding
+     * of the attribute started on errors.
+     *
+     * When chunking a list, EncodeListItem will atomically encode list items, ensuring that the
+     * state of the buffer is valid to send (i.e. contains no trailing garbage), and return an error
+     * if the list doesn't entirely fit.  In this situation, mAllowPartialData is set to communicate to the
+     * report engine that it should not roll back the list items.
+     *
+     * TODO: There might be a better name for this variable.
+     */
+    bool mAllowPartialData = false;
+};
+
 /**
  * The AttributeValueEncoder is a helper class for filling report payloads into AttributeReportIBs.
  * The attribute value encoder can be initialized with a AttributeEncodeState for saving and recovering its state between encode
@@ -51,9 +106,10 @@ class AttributeValueEncoder
             // If we are encoding for a fabric filtered attribute read and the fabric index does not match that present in the
             // request, skip encoding this list item.
             VerifyOrReturnError(!mAttributeValueEncoder.mIsFabricFiltered ||
-                                    aArg.GetFabricIndex() == mAttributeValueEncoder.mAccessingFabricIndex,
+                                    aArg.GetFabricIndex() == mAttributeValueEncoder.GetSubjectDescriptor().fabricIndex,
                                 CHIP_NO_ERROR);
-            return mAttributeValueEncoder.EncodeListItem(mAttributeValueEncoder.mAccessingFabricIndex, std::forward<T>(aArg));
+            return mAttributeValueEncoder.EncodeListItem(mAttributeValueEncoder.GetSubjectDescriptor().fabricIndex,
+                                                         std::forward<T>(aArg));
         }
 
         template <typename T, std::enable_if_t<!DataModel::IsFabricScoped<T>::value, bool> = true>
@@ -66,42 +122,11 @@ class AttributeValueEncoder
         AttributeValueEncoder & mAttributeValueEncoder;
     };
 
-    class AttributeEncodeState
-    {
-    public:
-        AttributeEncodeState() : mAllowPartialData(false), mCurrentEncodingListIndex(kInvalidListIndex) {}
-        bool AllowPartialData() const { return mAllowPartialData; }
-
-    private:
-        friend class AttributeValueEncoder;
-        /**
-         * When an attempt to encode an attribute returns an error, the buffer may contain tailing dirty data
-         * (since the put was aborted).  The report engine normally rolls back the buffer to right before encoding
-         * of the attribute started on errors.
-         *
-         * When chunking a list, EncodeListItem will atomically encode list items, ensuring that the
-         * state of the buffer is valid to send (i.e. contains no trailing garbage), and return an error
-         * if the list doesn't entirely fit.  In this situation, mAllowPartialData is set to communicate to the
-         * report engine that it should not roll back the list items.
-         *
-         * TODO: There might be a better name for this variable.
-         */
-        bool mAllowPartialData = false;
-        /**
-         * If set to kInvalidListIndex, indicates that we have not encoded any data for the list yet and
-         * need to start by encoding an empty list before we start encoding any list items.
-         *
-         * When set to a valid ListIndex value, indicates the index of the next list item that needs to be
-         * encoded (i.e. the count of items encoded so far).
-         */
-        ListIndex mCurrentEncodingListIndex = kInvalidListIndex;
-    };
-
-    AttributeValueEncoder(AttributeReportIBs::Builder & aAttributeReportIBsBuilder, FabricIndex aAccessingFabricIndex,
+    AttributeValueEncoder(AttributeReportIBs::Builder & aAttributeReportIBsBuilder, Access::SubjectDescriptor subjectDescriptor,
                           const ConcreteAttributePath & aPath, DataVersion aDataVersion, bool aIsFabricFiltered = false,
                           const AttributeEncodeState & aState = AttributeEncodeState()) :
         mAttributeReportIBsBuilder(aAttributeReportIBsBuilder),
-        mAccessingFabricIndex(aAccessingFabricIndex), mPath(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId),
+        mSubjectDescriptor(subjectDescriptor), mPath(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId),
         mDataVersion(aDataVersion), mIsFabricFiltered(aIsFabricFiltered), mEncodeState(aState)
     {}
 
@@ -176,10 +201,7 @@ class AttributeValueEncoder
 
     bool TriedEncode() const { return mTriedEncode; }
 
-    /**
-     * The accessing fabric index for this read or subscribe interaction.
-     */
-    FabricIndex AccessingFabricIndex() const { return mAccessingFabricIndex; }
+    const Access::SubjectDescriptor & GetSubjectDescriptor() const { return mSubjectDescriptor; }
 
     /**
      * AttributeValueEncoder is a short lived object, and the state is persisted by mEncodeState and restored by constructor.
@@ -195,7 +217,7 @@ class AttributeValueEncoder
     {
         // EncodeListItem must be called after EnsureListStarted(), thus mCurrentEncodingListIndex and
         // mEncodeState.mCurrentEncodingListIndex are not invalid values.
-        if (mCurrentEncodingListIndex < mEncodeState.mCurrentEncodingListIndex)
+        if (mCurrentEncodingListIndex < mEncodeState.CurrentEncodingListIndex())
         {
             // We have encoded this element in previous chunks, skip it.
             mCurrentEncodingListIndex++;
@@ -226,7 +248,7 @@ class AttributeValueEncoder
         }
 
         mCurrentEncodingListIndex++;
-        mEncodeState.mCurrentEncodingListIndex++;
+        mEncodeState.SetCurrentEncodingListIndex(mCurrentEncodingListIndex);
         mEncodedAtLeastOneListItem = true;
         return CHIP_NO_ERROR;
     }
@@ -266,20 +288,20 @@ class AttributeValueEncoder
      */
     void EnsureListEnded();
 
-    bool mTriedEncode = false;
     AttributeReportIBs::Builder & mAttributeReportIBsBuilder;
-    const FabricIndex mAccessingFabricIndex;
+    const Access::SubjectDescriptor mSubjectDescriptor;
     ConcreteDataAttributePath mPath;
     DataVersion mDataVersion;
+    bool mTriedEncode      = false;
     bool mIsFabricFiltered = false;
     // mEncodingInitialList is true if we're encoding a list and we have not
     // started chunking it yet, so we're encoding a single attribute report IB
     // for the whole list, not one per item.
     bool mEncodingInitialList = false;
     // mEncodedAtLeastOneListItem becomes true once we successfully encode a list item.
-    bool mEncodedAtLeastOneListItem = false;
-    AttributeEncodeState mEncodeState;
+    bool mEncodedAtLeastOneListItem     = false;
     ListIndex mCurrentEncodingListIndex = kInvalidListIndex;
+    AttributeEncodeState mEncodeState;
 };
 
 } // namespace app

src/app/ConcreteAttributePath.h

@@ -101,7 +101,7 @@ struct ConcreteReadAttributePath : public ConcreteAttributePath
  */
 struct ConcreteDataAttributePath : public ConcreteAttributePath
 {
-    enum class ListOperation
+    enum class ListOperation : uint8_t
     {
         NotList,     // Path points to an attribute that isn't a list.
         ReplaceAll,  // Path points to an attribute that is a list, indicating that the contents of the list should be replaced in

src/app/ReadHandler.cpp

@@ -841,7 +841,7 @@ void ReadHandler::PersistSubscription()
 void ReadHandler::ResetPathIterator()
 {
     mAttributePathExpandIterator = AttributePathExpandIterator(mpAttributePathList);
-    mAttributeEncoderState       = AttributeValueEncoder::AttributeEncodeState();
+    mAttributeEncoderState.Reset();
 }
 
 void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeChanged)
@@ -870,7 +870,7 @@ void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeCha
         // our iterator to point back to the beginning of that cluster. This ensures that the receiver will get a coherent view of
         // the state of the cluster as present on the server
         mAttributePathExpandIterator.ResetCurrentCluster();
-        mAttributeEncoderState = AttributeValueEncoder::AttributeEncodeState();
+        mAttributeEncoderState.Reset();
     }
 
     // ReportScheduler will take care of verifying the reportability of the handler and schedule the run

src/app/ReadHandler.h

@@ -412,8 +412,8 @@ class ReadHandler : public Messaging::ExchangeDelegate
     /// or after the min interval is reached if it has not yet been reached.
     void ForceDirtyState();
 
-    const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; }
-    void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) { mAttributeEncoderState = aState; }
+    const AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; }
+    void SetAttributeEncodeState(const AttributeEncodeState & aState) { mAttributeEncoderState = aState; }
     uint32_t GetLastWrittenEventsBytes() const { return mLastWrittenEventsBytes; }
 
     // Returns the number of interested paths, including wildcard and concrete paths.
@@ -562,7 +562,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
 
     // The detailed encoding state for a single attribute, used by list chunking feature.
     // The size of AttributeEncoderState is 2 bytes for now.
-    AttributeValueEncoder::AttributeEncodeState mAttributeEncoderState;
+    AttributeEncodeState mAttributeEncoderState;
 
     // Current Handler state
     HandlerState mState            = HandlerState::Idle;

src/app/clusters/access-control-server/access-control-server.cpp

@@ -230,7 +230,7 @@ CHIP_ERROR AccessControlAttribute::WriteImpl(const ConcreteDataAttributePath & a
 
 CHIP_ERROR AccessControlAttribute::WriteAcl(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
 {
-    FabricIndex accessingFabricIndex = aDecoder.AccessingFabricIndex();
+    FabricIndex accessingFabricIndex = aDecoder.GetSubjectDescriptor().fabricIndex;
 
     size_t oldCount;
     ReturnErrorOnFailure(GetAccessControl().GetEntryCount(accessingFabricIndex, oldCount));
@@ -293,7 +293,7 @@ CHIP_ERROR AccessControlAttribute::WriteExtension(const ConcreteDataAttributePat
 {
     auto & storage = Server::GetInstance().GetPersistentStorage();
 
-    FabricIndex accessingFabricIndex = aDecoder.AccessingFabricIndex();
+    FabricIndex accessingFabricIndex = aDecoder.GetSubjectDescriptor().fabricIndex;
 
     uint8_t buffer[kExtensionDataMaxLength] = { 0 };
     uint16_t size                           = static_cast<uint16_t>(sizeof(buffer));

src/app/clusters/bindings/bindings.cpp

@@ -200,7 +200,7 @@ void BindingTableAccess::OnListWriteEnd(const app::ConcreteAttributePath & aPath
 
 CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath & path, AttributeValueDecoder & decoder)
 {
-    mAccessingFabricIndex = decoder.AccessingFabricIndex();
+    mAccessingFabricIndex = decoder.GetSubjectDescriptor().fabricIndex;
     if (!path.IsListOperation() || path.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll)
     {
         DecodableBindingListType newBindingList;

src/app/clusters/group-key-mgmt-server/group-key-mgmt-server.cpp

@@ -192,7 +192,7 @@ class GroupKeyManagementAttributeAccess : public AttributeAccessInterface
 
     CHIP_ERROR WriteGroupKeyMap(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
     {
-        auto fabric_index = aDecoder.AccessingFabricIndex();
+        auto fabric_index = aDecoder.GetSubjectDescriptor().fabricIndex;
         auto provider     = GetGroupDataProvider();
 
         if (!aPath.IsListItemOperation())

src/app/clusters/operational-credentials-server/operational-credentials-server.cpp

@@ -117,7 +117,7 @@ class OperationalCredentialsAttrAccess : public AttributeAccessInterface
 
 CHIP_ERROR OperationalCredentialsAttrAccess::ReadNOCs(EndpointId endpoint, AttributeValueEncoder & aEncoder)
 {
-    auto accessingFabricIndex = aEncoder.AccessingFabricIndex();
+    auto accessingFabricIndex = aEncoder.GetSubjectDescriptor().fabricIndex;
 
     return aEncoder.EncodeList([accessingFabricIndex](const auto & encoder) -> CHIP_ERROR {
         const auto & fabricTable = Server::GetInstance().GetFabricTable();
@@ -252,7 +252,7 @@ CHIP_ERROR OperationalCredentialsAttrAccess::Read(const ConcreteReadAttributePat
         return ReadRootCertificates(aPath.mEndpointId, aEncoder);
     }
     case Attributes::CurrentFabricIndex::Id: {
-        return aEncoder.Encode(aEncoder.AccessingFabricIndex());
+        return aEncoder.Encode(aEncoder.GetSubjectDescriptor().fabricIndex);
     }
     default:
         break;

src/app/clusters/ota-requestor/ota-requestor-server.cpp

@@ -120,7 +120,7 @@ CHIP_ERROR OtaSoftwareUpdateRequestorAttrAccess::WriteDefaultOtaProviders(const
         DataModel::DecodableList<OtaSoftwareUpdateRequestor::Structs::ProviderLocation::DecodableType> list;
         ReturnErrorOnFailure(aDecoder.Decode(list));
 
-        ReturnErrorOnFailure(requestor->ClearDefaultOtaProviderList(aDecoder.AccessingFabricIndex()));
+        ReturnErrorOnFailure(requestor->ClearDefaultOtaProviderList(aDecoder.GetSubjectDescriptor().fabricIndex));
 
         auto iter = list.begin();
         while (iter.Next())

src/app/clusters/test-cluster-server/test-cluster-server.cpp

@@ -661,7 +661,7 @@ CHIP_ERROR TestAttrAccess::WriteListFabricScopedAttribute(const ConcreteDataAttr
         size_t srcIndex = 0, dstIndex = 0;
         while (srcIndex < gListFabricScopedAttributeLen)
         {
-            if (gListFabricScopedAttributeValue[srcIndex].fabricIndex != aDecoder.AccessingFabricIndex())
+            if (gListFabricScopedAttributeValue[srcIndex].fabricIndex != aDecoder.GetSubjectDescriptor().fabricIndex)
             {
                 auto & dstEntry = gListFabricScopedAttributeValue[dstIndex];
                 auto & srcEntry = gListFabricScopedAttributeValue[srcIndex];

src/app/dynamic_server/DynamicDispatcher.cpp

@@ -123,7 +123,7 @@ Status DetermineAttributeStatus(const ConcreteAttributePath & aPath, bool aIsWri
 
 CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
                                  const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports,
-                                 AttributeValueEncoder::AttributeEncodeState * aEncoderState)
+                                 AttributeEncodeState * aEncoderState)
 {
     Status status = DetermineAttributeStatus(aPath, /* aIsWrite = */ false);
     return aAttributeReports.EncodeAttributeStatus(aPath, StatusIB(status));