fix(client-presence): disabled (failing) attendees unit tests

packages/framework/presence/src/systemWorkspace.ts

@@ -17,6 +17,7 @@ import type {
 } from "./presence.js";
 import { SessionClientStatus } from "./presence.js";
 import type { PresenceStatesInternal } from "./presenceStates.js";
+import { TimerManager } from "./timerManager.js";
 import type { PresenceStates, PresenceStatesSchema } from "./types.js";
 
 /**
@@ -37,17 +38,12 @@ class SessionClient implements ISessionClient {
 	 */
 	public order: number = 0;
 
-	private connectionStatus: SessionClientStatus;
+	private connectionStatus: SessionClientStatus = SessionClientStatus.Disconnected;
 
 	public constructor(
 		public readonly sessionId: ClientSessionId,
-		private connectionId: ClientConnectionId | undefined = undefined,
-	) {
-		this.connectionStatus =
-			connectionId === undefined
-				? SessionClientStatus.Disconnected
-				: SessionClientStatus.Connected;
-	}
+		public connectionId: ClientConnectionId | undefined = undefined,
+	) {}
 
 	public getConnectionId(): ClientConnectionId {
 		if (this.connectionId === undefined) {
@@ -60,8 +56,7 @@ class SessionClient implements ISessionClient {
 		return this.connectionStatus;
 	}
 
-	public setConnectionId(connectionId: ClientConnectionId): void {
-		this.connectionId = connectionId;
+	public setConnected(): void {
 		this.connectionStatus = SessionClientStatus.Connected;
 	}
 
@@ -103,6 +98,12 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace {
 	 */
 	private readonly attendees = new Map<ClientConnectionId | ClientSessionId, SessionClient>();
 
+	// When local client disconnects, we lose the connectivity status updates for remote attendees in the session.
+	// Upon reconnect, we mark all other attendees connections as stale and update their status to disconnected after 30 seconds of inactivity.
+	private readonly staleConnectionClients = new Set<SessionClient>();
+
+	private readonly timer = new TimerManager();
+
 	public constructor(
 		clientSessionId: ClientSessionId,
 		private readonly datastore: SystemWorkspaceDatastore,
@@ -137,34 +138,23 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace {
 	): void {
 		const postUpdateActions: (() => void)[] = [];
 		const audienceMembers = this.audience.getMembers();
-		const connectedAttendees = new Set<SessionClient>();
+		const joiningAttendees = new Set<SessionClient>();
 		for (const [clientConnectionId, value] of Object.entries(
 			remoteDatastore.clientToSessionId,
 		)) {
 			const clientSessionId = value.value;
-			const { attendee, isNew } = this.ensureAttendee(
+			const { attendee, isJoining } = this.ensureAttendee(
 				clientSessionId,
 				clientConnectionId,
 				/* order */ value.rev,
+				// If the attendee is present in audience OR if the attendee update is from the sending remote client itself,
+				// then the attendee is considered connected.
+				/* isConnected */ senderConnectionId === clientConnectionId ||
+					audienceMembers.has(clientConnectionId),
 			);
-
-			// Check new attendee against audience to see if they're currently connected
-			const isAttendeeConnected = audienceMembers.has(clientConnectionId);
-
-			if (isAttendeeConnected) {
-				connectedAttendees.add(attendee);
-				if (attendee.getConnectionStatus() === SessionClientStatus.Disconnected) {
-					attendee.setConnectionId(clientConnectionId);
-				}
-				if (isNew) {
-					// If the attendee is both new and in audience (i.e. currently connected), emit an attendeeJoined event.
-					postUpdateActions.push(() => this.events.emit("attendeeJoined", attendee));
-				}
-			}
-
-			// If the attendee is not in the audience, they are considered disconnected.
-			if (!connectedAttendees.has(attendee)) {
-				attendee.setDisconnected();
+			// If the attendee is joining the session, add them to the list of joining attendees to be announced later.
+			if (isJoining) {
+				joiningAttendees.add(attendee);
 			}
 
 			const knownSessionId: InternalTypes.ValueRequiredState<ClientSessionId> | undefined =
@@ -176,6 +166,10 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace {
 			}
 		}
 
+		for (const announcedAttendee of joiningAttendees) {
+			postUpdateActions.push(() => this.events.emit("attendeeJoined", announcedAttendee));
+		}
+
 		// TODO: reorganize processUpdate and caller to process actions after all updates are processed.
 		for (const action of postUpdateActions) {
 			action();
@@ -189,8 +183,25 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace {
 			value: this.selfAttendee.sessionId,
 		};
 
-		this.selfAttendee.setConnectionId(clientConnectionId);
+		this.selfAttendee.connectionId = clientConnectionId;
+		this.selfAttendee.setConnected();
 		this.attendees.set(clientConnectionId, this.selfAttendee);
+
+		if (!this.timer.hasExpired()) {
+			this.timer.clearTimeout();
+		}
+
+		for (const staleConnecionClient of this.attendees.values()) {
+			this.staleConnectionClients.add(staleConnecionClient);
+		}
+
+		this.timer.setTimeout(() => {
+			for (const client of this.staleConnectionClients) {
+				// Mark the client as disconnected and remove from the stale connection set
+				client.setDisconnected();
+				this.staleConnectionClients.delete(client);
+			}
+		}, 30_000);
 	}
 
 	public removeClientConnectionId(clientConnectionId: ClientConnectionId): void {
@@ -239,27 +250,42 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace {
 		clientSessionId: ClientSessionId,
 		clientConnectionId: ClientConnectionId,
 		order: number,
-	): { attendee: SessionClient; isNew: boolean } {
+		isConnected: boolean,
+	): { attendee: SessionClient; isJoining: boolean } {
 		let attendee = this.attendees.get(clientSessionId);
-		let isNew = false;
+		let isJoining = false;
 
 		if (attendee === undefined) {
 			// New attendee. Create SessionClient and add session ID based
 			// entry to map.
 			attendee = new SessionClient(clientSessionId, clientConnectionId);
 			this.attendees.set(clientSessionId, attendee);
-			isNew = true;
+			if (isConnected) {
+				attendee.setConnected();
+				isJoining = true;
+			}
 		} else if (order > attendee.order) {
 			// The given association is newer than the one we have.
 			// Update the order and current connection ID.
 			attendee.order = order;
-			attendee.setConnectionId(clientConnectionId);
-			isNew = true;
+			// Known attendee is joining the session if they are currently disconnected
+			if (attendee.getConnectionStatus() === SessionClientStatus.Disconnected && isConnected) {
+				attendee.setConnected();
+				isJoining = true;
+			}
+			attendee.connectionId = clientConnectionId;
 		}
+
+		if (this.staleConnectionClients.has(attendee) && isConnected) {
+			// If the attendee is marked as stale, update the connection status to connected
+			attendee.setConnected();
+			this.staleConnectionClients.delete(attendee);
+		}
+
 		// Always update entry for the connection ID. (Okay if already set.)
 		this.attendees.set(clientConnectionId, attendee);
 
-		return { attendee, isNew };
+		return { attendee, isJoining };
 	}
 }
 

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

@@ -203,7 +203,7 @@ describe("Presence", () => {
 						verifyAttendee(joinedAttendees[0], rejoinAttendeeConnectionId, attendeeSessionId);
 					});
 
-					it.skip('second time is announced once via `attendeeJoined` with status "Connected" when prior is still connected', () => {
+					it('second time is announced once via `attendeeJoined` with status "Connected" when prior is still connected', () => {
 						// Act - simulate join message from client
 						const joinedAttendees = processJoinSignals([rejoinAttendeeSignal]);
 
@@ -217,7 +217,7 @@ describe("Presence", () => {
 						verifyAttendee(joinedAttendees[0], rejoinAttendeeConnectionId, attendeeSessionId);
 					});
 
-					it.skip('first time is announced via `attendeeJoined` with status "Connected" even if unknown to audience', () => {
+					it('first time is announced via `attendeeJoined` with status "Connected" even if unknown to audience', () => {
 						// Setup - remove connection from audience
 						runtime.removeMember(initialAttendeeConnectionId);
 
@@ -282,6 +282,62 @@ describe("Presence", () => {
 
 						verifyAttendee(joinedAttendees[0], rejoinAttendeeConnectionId, attendeeSessionId);
 					});
+
+					it("as collateral with old connection info and connected is NOT announced via `attendeeJoined`", () => {
+						// Setup
+						const oldCollateralAttendeeConnectionId = "client9";
+						const newCollateralAttendeeConnectionId = "client10";
+
+						const rejoinSignal = generateBasicClientJoin(clock.now - 10, {
+							averageLatency: 40,
+							clientSessionId: "collateral-id",
+							clientConnectionId: newCollateralAttendeeConnectionId,
+							updateProviders: [initialAttendeeConnectionId],
+							connectionOrder: 1,
+						});
+
+						const responseSignal = generateBasicClientJoin(clock.now - 5, {
+							averageLatency: 20,
+							clientSessionId: attendeeSessionId,
+							clientConnectionId: initialAttendeeConnectionId,
+							priorClientToSessionId: {
+								...initialAttendeeSignal.content.data["system:presence"].clientToSessionId,
+								[oldCollateralAttendeeConnectionId]: {
+									rev: 0,
+									timestamp: 0,
+									value: "collateral-id",
+								},
+							},
+						});
+
+						const joinedAttendees = processJoinSignals([initialAttendeeSignal]);
+						assert.strictEqual(
+							joinedAttendees.length,
+							1,
+							"Expected exactly one attendee to be announced",
+						);
+
+						// Act - simulate join message from client
+						const rejoinAttendees = processJoinSignals([rejoinSignal]);
+						const responseAttendees = processJoinSignals([responseSignal]);
+
+						// Verify - only the rejoining attendee is announced
+						assert.strictEqual(
+							rejoinAttendees.length,
+							1,
+							"Expected exactly one attendee to be announced",
+						);
+						assert.strictEqual(
+							responseAttendees.length,
+							0,
+							"Expected no attendees to be announced",
+						);
+						verifyAttendee(
+							rejoinAttendees[0],
+							newCollateralAttendeeConnectionId,
+							"collateral-id",
+						);
+					});
 				});
 
 				describe("that is already known", () => {
@@ -314,7 +370,7 @@ describe("Presence", () => {
 					// To retain symmetry across Joined and Disconnected events, do not announce
 					// attendeeJoined when the attendee is already connected and we only see
 					// a connection id update. This can happen when audience removal is late.
-					it.skip('is not announced via `attendeeJoined` when already "Connected"', () => {
+					it('is not announced via `attendeeJoined` when already "Connected"', () => {
 						// Setup
 						afterCleanUp.push(
 							presence.events.on("attendeeJoined", () => {