Cleanup of peer components (#1253)

Removes several bits of cruft left over from refactor. Modifies session resumption record cleanup slightly after Discord conversation.
project-chip · Oct 1, 2024 · e7b377c · e7b377c
1 parent 2d9d1cb
commit e7b377c
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 34 deletions.
diff --git a/packages/protocol/src/common/FailsafeContext.ts b/packages/protocol/src/common/FailsafeContext.ts
@@ -144,7 +144,7 @@ export abstract class FailsafeContext {
 
     async updateFabric(fabric: Fabric) {
         await this.#fabrics.updateFabric(fabric);
-        await this.#sessions.updateFabricForResumptionRecords(fabric);
+        await this.#sessions.deleteResumptionRecordsForFabric(fabric);
     }
 
     /**

diff --git a/packages/protocol/src/peer/ControllerCommissioningFlow.ts b/packages/protocol/src/peer/ControllerCommissioningFlow.ts
@@ -133,12 +133,6 @@ class RecoverableCommissioningError extends CommissioningError {}
 
 const DEFAULT_FAILSAFE_TIME_MS = 60_000; // 60 seconds
 
-/**
- * The operative connection callback may return this value to skip PASE commissioning.
- */
-export const SKIP_CASE_COMMISSIONING = Symbol("skip-pase-commissioning");
-export type SKIP_PASE_COMMISSIONING = typeof SKIP_CASE_COMMISSIONING;
-
 /**
  * Class to abstract the Device commission flow in a step wise way as defined in Specs. The specs are not 100%
  */

diff --git a/packages/protocol/src/peer/PeerSet.ts b/packages/protocol/src/peer/PeerSet.ts
@@ -75,13 +75,6 @@ interface RunningDiscovery {
     timer?: Timer;
 }
 
-/**
- * API for establishing a connection to a peer.
- */
-export interface PeerConnector {
-    connect(address: PeerAddress, discoveryOptions: DiscoveryOptions): Promise<InteractionClient>;
-}
-
 /**
  * Interfaces {@link PeerSet} with other components.
  */
@@ -97,7 +90,7 @@ export interface PeerSetContext {
 /**
  * Manages operational connections to peers on shared fabric.
  */
-export class PeerSet implements ImmutableSet<OperationalPeer>, ObservableSet<OperationalPeer>, PeerConnector {
+export class PeerSet implements ImmutableSet<OperationalPeer>, ObservableSet<OperationalPeer> {
     readonly #sessions: SessionManager;
     readonly #channels: ChannelManager;
     readonly #exchanges: ExchangeManager;
@@ -280,7 +273,7 @@ export class PeerSet implements ImmutableSet<OperationalPeer>, ObservableSet<Ope
         this.#peers.delete(actual);
         await this.#store.deletePeer(actual.address);
         await this.disconnect(actual);
-        await this.#sessions.removeResumptionRecord(actual.address);
+        await this.#sessions.deleteResumptionRecord(actual.address);
     }
 
     async close() {

diff --git a/packages/protocol/src/protocol/DeviceCommissioner.ts b/packages/protocol/src/protocol/DeviceCommissioner.ts
@@ -22,7 +22,7 @@ import {
 import { SecureChannelProtocol } from "#securechannel/SecureChannelProtocol.js";
 import { PaseServer, SessionManager } from "#session/index.js";
 import { CommissioningOptions, StatusCode, StatusResponseError } from "#types";
-import type { ControllerCommissioningFlow } from "../peer/ControllerCommissioningFlow.js";
+import type { ControllerCommissioner } from "../peer/ControllerCommissioner.js";
 import { DeviceAdvertiser } from "./DeviceAdvertiser.js";
 
 const logger = Logger.get("DeviceCommissioner");
@@ -48,7 +48,7 @@ export interface DeviceCommissionerContext {
 /**
  * Implements commissioning for devices.
  *
- * Note this implements commissioning for a *local* device; use {@link ControllerCommissioningFlow} to commission a *remote*
+ * Note this implements commissioning for a *local* device; use {@link ControllerCommissioner} to commission a *remote*
  * device.
  */
 export class DeviceCommissioner {

diff --git a/packages/protocol/src/session/SessionManager.ts b/packages/protocol/src/session/SessionManager.ts
@@ -126,7 +126,7 @@ export class SessionManager {
 
         // When fabric is removed, also remove the resumption record
         this.#observers.on(context.fabrics.events.deleted, async fabric =>
-            this.removeResumptionRecord(fabric.addressOf(fabric.rootNodeId)),
+            this.deleteResumptionRecordsForFabric(fabric),
         );
 
         this.#construction = Construction(this, () => this.#initialize());
@@ -285,11 +285,23 @@ export class SessionManager {
         return session;
     }
 
-    async removeResumptionRecord(address: PeerAddress) {
+    async deleteResumptionRecord(address: PeerAddress) {
         await this.#construction;
 
         this.#resumptionRecords.delete(address);
-        await this.storeResumptionRecords();
+        await this.#storeResumptionRecords();
+    }
+
+    async deleteResumptionRecordsForFabric(fabric: Fabric) {
+        await this.#construction;
+
+        for (const address of this.#resumptionRecords.keys()) {
+            if (address.fabricIndex === fabric.fabricIndex) {
+                this.#resumptionRecords.delete(address);
+            }
+        }
+
+        await this.#storeResumptionRecords();
     }
 
     findOldestInactiveSession() {
@@ -397,20 +409,10 @@ export class SessionManager {
     async saveResumptionRecord(resumptionRecord: ResumptionRecord) {
         await this.#construction;
         this.#resumptionRecords.set(resumptionRecord.fabric.addressOf(resumptionRecord.peerNodeId), resumptionRecord);
-        await this.storeResumptionRecords();
-    }
-
-    async updateFabricForResumptionRecords(fabric: Fabric) {
-        await this.#construction;
-        const record = this.#resumptionRecords.get(fabric.addressOf(fabric.rootNodeId));
-        if (record === undefined) {
-            throw new MatterFlowError("Resumption record not found. Should never happen.");
-        }
-        this.#resumptionRecords.set(fabric.addressOf(fabric.rootNodeId), { ...record, fabric });
-        await this.storeResumptionRecords();
+        await this.#storeResumptionRecords();
     }
 
-    async storeResumptionRecords() {
+    async #storeResumptionRecords() {
         await this.#construction;
         await this.#context.storage.set(
             "resumptionRecords",
@@ -517,7 +519,7 @@ export class SessionManager {
         await this.#subscriptionUpdateMutex;
 
         this.#observers.close();
-        await this.storeResumptionRecords();
+        await this.#storeResumptionRecords();
         for (const session of this.#sessions) {
             await session?.end(false);
             this.#sessions.delete(session);