fix(client-presence): ISessionClient naming consistency (#22973)

1. `ISessionClient` method names updated for consistency to
`getConnectionId()` and `getConnectionStatus()`.
2. Implementation of `ISessionClient` moved to a full class object.
3. Changeset provided for Presence changes since 2.4.
4. Updated `id` to `ID` in comments (public and most internal).

No behavior is changed.

[AB#21446](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/21446)

---------

Co-authored-by: Willie Habimana <whabimana@microsoft.com>
Co-authored-by: Tyler Butler <tylerbu@microsoft.com>

.changeset/ninety-zoos-trade.md

@@ -0,0 +1,12 @@
+---
+"@fluid-experimental/presence": minor
+---
+---
+"section": feature
+---
+
+ISessionClient now exposes connectivity information
+
+1. `ISessionClient` has a new method, `getConnectionStatus()`, with two possible states: `Connected` and `Disconnected`.
+2. `ISessionClient`'s `connectionId()` member has been renamed to `getConnectionId()` for consistency.
+3. `IPresence` event `attendeeDisconnected` is now implemented.

examples/service-clients/azure-client/external-controller/src/view.ts

@@ -167,7 +167,7 @@ function makePresenceView(
 	logContentDiv.style.border = "1px solid black";
 	if (audience !== undefined) {
 		presenceConfig.presence.events.on("attendeeJoined", (attendee) => {
-			const name = audience.getMembers().get(attendee.connectionId())?.name;
+			const name = audience.getMembers().get(attendee.getConnectionId())?.name;
 			const update = `client ${name === undefined ? "(unnamed)" : `named ${name}`} with id ${attendee.sessionId} joined`;
 			addLogEntry(logContentDiv, update);
 		});

packages/framework/presence/api-report/presence.alpha.api.md

@@ -37,9 +37,8 @@ export interface IPresence {
 
 // @alpha @sealed
 export interface ISessionClient<SpecificSessionClientId extends ClientSessionId = ClientSessionId> {
-    connectionId(): ClientConnectionId;
-    getStatus(): SessionClientStatus;
-    // (undocumented)
+    getConnectionId(): ClientConnectionId;
+    getConnectionStatus(): SessionClientStatus;
     readonly sessionId: SpecificSessionClientId;
 }
 

packages/framework/presence/src/baseTypes.ts

@@ -10,7 +10,7 @@
  * Each client connection is given a unique identifier for the duration of the
  * connection. If a client disconnects and reconnects, it will be given a new
  * identifier. Prefer use of {@link ISessionClient} as a way to identify clients
- * in a session. {@link ISessionClient.connectionId} will provide the current
+ * in a session. {@link ISessionClient.getConnectionId} will provide the current
  * connection identifier for a logical session client.
  *
  * @privateRemarks

packages/framework/presence/src/presence.ts

@@ -24,7 +24,7 @@ import type { ISubscribable } from "@fluid-experimental/presence/internal/events
  * duration of the session. If a client disconnects and reconnects, it will
  * retain its identifier. Prefer use of {@link ISessionClient} as a way to
  * identify clients in a session. {@link ISessionClient.sessionId} will provide
- * the session id.
+ * the session ID.
  *
  * @alpha
  */
@@ -56,8 +56,8 @@ export type SessionClientStatus =
  *
  * `ISessionClient` should be used as key to distinguish between different
  * clients as they join, rejoin, and disconnect from a session. While a
- * client's {@link ClientConnectionId} may change over time `ISessionClient`
- * will be fixed.
+ * client's {@link ClientConnectionId} from {@link ISessionClient.getConnectionStatus}
+ * may change over time, `ISessionClient` will be fixed.
  *
  * @privateRemarks
  * As this is evolved, pay attention to how this relates to Audience, Service
@@ -69,32 +69,35 @@ export type SessionClientStatus =
 export interface ISessionClient<
 	SpecificSessionClientId extends ClientSessionId = ClientSessionId,
 > {
+	/**
+	 * The session ID of the client that is stable over all connections.
+	 */
 	readonly sessionId: SpecificSessionClientId;
 
 	/**
-	 * Get current client connection id.
+	 * Get current client connection ID.
 	 *
-	 * @returns Current client connection id.
+	 * @returns Current client connection ID.
 	 *
 	 * @remarks
-	 * Connection id will change on reconnect.
+	 * Connection ID will change on reconnect.
 	 *
-	 * If {@link ISessionClient.getStatus} is {@link (SessionClientStatus:variable).Disconnected}, this will represent the last known connection id.
+	 * If {@link ISessionClient.getConnectionStatus} is {@link (SessionClientStatus:variable).Disconnected}, this will represent the last known connection ID.
 	 */
-	connectionId(): ClientConnectionId;
+	getConnectionId(): ClientConnectionId;
 
 	/**
-	 * Get status of session client.
+	 * Get connection status of session client.
 	 *
-	 * @returns Status of session client.
+	 * @returns Connection status of session client.
 	 *
 	 */
-	getStatus(): SessionClientStatus;
+	getConnectionStatus(): SessionClientStatus;
 }
 
 /**
  * Utility type limiting to a specific session client. (A session client with
- * a specific session id - not just any session id.)
+ * a specific session ID - not just any session ID.)
  *
  * @internal
  */
@@ -162,7 +165,7 @@ export interface IPresence {
 	/**
 	 * Lookup a specific attendee in the session.
 	 *
-	 * @param clientId - Client connection or session id
+	 * @param clientId - Client connection or session ID
 	 */
 	getAttendee(clientId: ClientConnectionId | ClientSessionId): ISessionClient;
 

packages/framework/presence/src/presenceManager.ts

@@ -49,9 +49,9 @@ export type PresenceExtensionInterface = Required<
  */
 export interface ClientConnectionManager {
 	/**
-	 * Remove the current client connection id from the corresponding disconnected attendee.
+	 * Remove the current client connection ID from the corresponding disconnected attendee.
 	 *
-	 * @param clientConnectionId - The current client connection id to be removed.
+	 * @param clientConnectionId - The current client connection ID to be removed.
 	 */
 	removeClientConnectionId(clientConnectionId: ClientConnectionId): void;
 }

packages/framework/presence/src/systemWorkspace.ts

@@ -30,20 +30,49 @@ export interface SystemWorkspaceDatastore {
 	};
 }
 
-/**
- * There is no implementation class for this interface.
- * It is a simple structure. Most complicated aspect is that
- * `connectionId()` member is replaced with a new
- * function when a more recent connection is added.
- *
- * See {@link SystemWorkspaceImpl.ensureAttendee}.
- */
-interface SessionClient extends ISessionClient {
+class SessionClient implements ISessionClient {
 	/**
 	 * Order is used to track the most recent client connection
 	 * during a session.
 	 */
-	order: number;
+	public order: number = 0;
+
+	private connectionStatus: SessionClientStatus;
+
+	public constructor(
+		public readonly sessionId: ClientSessionId,
+		private connectionId: ClientConnectionId | undefined = undefined,
+	) {
+		this.connectionStatus =
+			connectionId === undefined
+				? SessionClientStatus.Disconnected
+				: SessionClientStatus.Connected;
+	}
+
+	public getConnectionId(): ClientConnectionId {
+		if (this.connectionId === undefined) {
+			throw new Error("Client has never been connected");
+		}
+		return this.connectionId;
+	}
+
+	public getConnectionStatus(): SessionClientStatus {
+		return this.connectionStatus;
+	}
+
+	public setConnectionId(
+		connectionId: ClientConnectionId,
+		updateStatus: boolean = true,
+	): void {
+		this.connectionId = connectionId;
+		if (updateStatus) {
+			this.connectionStatus = SessionClientStatus.Connected;
+		}
+	}
+
+	public setDisconnected(): void {
+		this.connectionStatus = SessionClientStatus.Disconnected;
+	}
 }
 
 /**
@@ -56,14 +85,14 @@ export interface SystemWorkspace
 	/**
 	 * Must be called when the current client acquires a new connection.
 	 *
-	 * @param clientConnectionId - The new client connection id.
+	 * @param clientConnectionId - The new client connection ID.
 	 */
 	onConnectionAdded(clientConnectionId: ClientConnectionId): void;
 
 	/**
-	 * Removes the client connection id from the system workspace.
+	 * Removes the client connection ID from the system workspace.
 	 *
-	 * @param clientConnectionId - The client connection id to remove.
+	 * @param clientConnectionId - The client connection ID to remove.
 	 */
 	removeClientConnectionId(clientConnectionId: ClientConnectionId): void;
 }
@@ -75,7 +104,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace {
 	 * session. The map covers entries for both session ids and connection
 	 * ids, which are never expected to collide, but if they did for same
 	 * client that would be fine.
-	 * An entry is for session id if the value's `sessionId` matches the key.
+	 * An entry is for session ID if the value's `sessionId` matches the key.
 	 */
 	private readonly attendees = new Map<ClientConnectionId | ClientSessionId, SessionClient>();
 
@@ -86,14 +115,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace {
 			Pick<PresenceEvents, "attendeeJoined" | "attendeeDisconnected">
 		>,
 	) {
-		this.selfAttendee = {
-			sessionId: clientSessionId,
-			order: 0,
-			connectionId: () => {
-				throw new Error("Client has never been connected");
-			},
-			getStatus: () => SessionClientStatus.Disconnected,
-		};
+		this.selfAttendee = new SessionClient(clientSessionId);
 		this.attendees.set(clientSessionId, this.selfAttendee);
 	}
 
@@ -150,8 +172,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace {
 			value: this.selfAttendee.sessionId,
 		};
 
-		this.selfAttendee.connectionId = () => clientConnectionId;
-		this.selfAttendee.getStatus = () => SessionClientStatus.Connected;
+		this.selfAttendee.setConnectionId(clientConnectionId);
 		this.attendees.set(clientConnectionId, this.selfAttendee);
 	}
 
@@ -161,11 +182,11 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace {
 			return;
 		}
 
-		// If the last known connectionID is different from the connection id being removed, the attendee has reconnected,
+		// If the last known connectionID is different from the connection ID being removed, the attendee has reconnected,
 		// therefore we should not change the attendee connection status or emit a disconnect event.
-		const attendeeReconnected = attendee.connectionId() !== clientConnectionId;
+		const attendeeReconnected = attendee.getConnectionId() !== clientConnectionId;
 		if (!attendeeReconnected) {
-			attendee.getStatus = () => SessionClientStatus.Disconnected;
+			attendee.setDisconnected();
 			this.events.emit("attendeeDisconnected", attendee);
 		}
 	}
@@ -192,36 +213,33 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace {
 	}
 
 	/**
-	 * Make sure the given client session and connection id pair are represented
+	 * Make sure the given client session and connection ID pair are represented
 	 * in the attendee map. If not present, SessionClient is created and added
-	 * to map. If present, make sure the current connection id is updated.
+	 * to map. If present, make sure the current connection ID is updated.
 	 */
 	private ensureAttendee(
 		clientSessionId: ClientSessionId,
 		clientConnectionId: ClientConnectionId,
 		order: number,
 	): { attendee: SessionClient; isNew: boolean } {
-		const connectionId = (): ClientConnectionId => clientConnectionId;
 		let attendee = this.attendees.get(clientSessionId);
 		let isNew = false;
+		// TODO #22616: Check for a current connection to determine best status.
+		// For now, always leave existing state as was last determined and
+		// assume new client is connected.
 		if (attendee === undefined) {
-			// New attendee. Create SessionClient and add session id based
+			// New attendee. Create SessionClient and add session ID based
 			// entry to map.
-			attendee = {
-				sessionId: clientSessionId,
-				order,
-				connectionId,
-				getStatus: () => SessionClientStatus.Connected,
-			};
+			attendee = new SessionClient(clientSessionId, clientConnectionId);
 			this.attendees.set(clientSessionId, attendee);
 			isNew = true;
 		} else if (order > attendee.order) {
 			// The given association is newer than the one we have.
-			// Update the order and current connection id.
+			// Update the order and current connection ID.
 			attendee.order = order;
-			attendee.connectionId = connectionId;
+			attendee.setConnectionId(clientConnectionId, /* updateStatus */ false);
 		}
-		// Always update entry for the connection id. (Okay if already set.)
+		// Always update entry for the connection ID. (Okay if already set.)
 		this.attendees.set(clientConnectionId, attendee);
 		return { attendee, isNew };
 	}

packages/framework/presence/src/test/presenceManager.spec.ts

@@ -125,7 +125,7 @@ describe("Presence", () => {
 						"Attendee has wrong session id",
 					);
 					assert.equal(
-						newAttendee.connectionId(),
+						newAttendee.getConnectionId(),
 						initialAttendeeConnectionId,
 						"Attendee has wrong client connection id",
 					);
@@ -163,7 +163,7 @@ describe("Presence", () => {
 							"Disconnected attendee has wrong session id",
 						);
 						assert.equal(
-							disconnectedAttendee.connectionId(),
+							disconnectedAttendee.getConnectionId(),
 							initialAttendeeConnectionId,
 							"Disconnected attendee has wrong client connection id",
 						);
@@ -176,7 +176,7 @@ describe("Presence", () => {
 							"No attendee was disconnected in beforeEach",
 						);
 						assert.equal(
-							disconnectedAttendee.getStatus(),
+							disconnectedAttendee.getConnectionStatus(),
 							SessionClientStatus.Disconnected,
 							"Disconnected attendee has wrong status",
 						);
@@ -251,7 +251,7 @@ describe("Presence", () => {
 						);
 						// Current connection id is updated
 						assert(
-							newAttendee.connectionId() === updatedClientConnectionId,
+							newAttendee.getConnectionId() === updatedClientConnectionId,
 							"Attendee does not have updated client connection id",
 						);
 						// Attendee is available via new connection id