Skip to content

Commit

Permalink
Emit Leave events before the fabric is actually removed. (#21191)
Browse files Browse the repository at this point in the history
Otherwise we stop being able to do things like send a report for the
event.

Fixes #13930
Fixes #21012
  • Loading branch information
bzbarsky-apple authored and web-flow committed Jul 26, 2022
1 parent e556daa commit 2cee8a6
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -325,20 +325,12 @@ void OnPlatformEventHandler(const chip::DeviceLayer::ChipDeviceEvent * event, in
// As per specifications section 11.22.5.1. Constant RESP_MAX
constexpr size_t kMaxRspLen = 900;

// TODO: The code currently has two sources of truths for fabrics, the fabricInfo table + the attributes. There should only be one,
// the attributes list. Currently the attributes are not persisted so we are keeping the fabric table to have the
// fabrics/admrins be persisted. Once attributes are persisted, there should only be one sorce of truth, the attributes list and
// only that should be modifed to perosst/read/write fabrics.
// TODO: Once attributes are persisted, implement reading/writing/manipulation fabrics around that and remove fabricTable
// logic.
class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
{
public:
// Gets called when a fabric is deleted from KVS store
void OnFabricRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) override
// Gets called when a fabric is about to be deleted
void FabricWillBeRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) override
{
ChipLogProgress(Zcl, "OpCreds: Fabric index 0x%x was removed", static_cast<unsigned>(fabricIndex));

// The Leave event SHOULD be emitted by a Node prior to permanently leaving the Fabric.
for (auto endpoint : EnabledEndpointsWithServerCluster(Basic::Id))
{
Expand All @@ -359,8 +351,14 @@ class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
// - removing the fabric removes all associated access control entries, so generating
// subsequent reports containing the leave event will fail the access control check.
InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleUrgentEventDeliverySync();
EventManagement::GetInstance().FabricRemoved(fabricIndex);
}

// Gets called when a fabric is deleted
void OnFabricRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) override
{
ChipLogProgress(Zcl, "OpCreds: Fabric index 0x%x was removed", static_cast<unsigned>(fabricIndex));

EventManagement::GetInstance().FabricRemoved(fabricIndex);
NotifyFabricTableChanged();
}

Expand Down
12 changes: 12 additions & 0 deletions src/credentials/FabricTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,18 @@ CHIP_ERROR FabricTable::Delete(FabricIndex fabricIndex)
VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(IsValidFabricIndex(fabricIndex), CHIP_ERROR_INVALID_ARGUMENT);

{
FabricTable::Delegate * delegate = mDelegateListRoot;
while (delegate)
{
// It is possible that delegate will remove itself from the list in FabricWillBeRemoved,
// so we grab the next delegate in the list now.
FabricTable::Delegate * nextDelegate = delegate->next;
delegate->FabricWillBeRemoved(*this, fabricIndex);
delegate = nextDelegate;
}
}

FabricInfo * fabricInfo = GetMutableFabricByIndex(fabricIndex);
if (fabricInfo == &mPendingFabric)
{
Expand Down
7 changes: 7 additions & 0 deletions src/credentials/FabricTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,13 @@ class DLL_EXPORT FabricTable
Delegate() {}
virtual ~Delegate() {}

/**
* Gets called when a fabric is about to be deleted, such as on
* FabricTable::Delete(). This allows actions to be taken that need the
* fabric to still be around before we delete it.
**/
virtual void FabricWillBeRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) {}

/**
* Gets called when a fabric is deleted, such as on FabricTable::Delete().
**/
Expand Down

0 comments on commit 2cee8a6

Please sign in to comment.