From 649f757fc0bce725f7179bfe7331157ffc3ab880 Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Mon, 28 Apr 2025 18:41:44 +0100 Subject: [PATCH 01/18] initial changes --- .../presence/src/presenceDatastoreManager.ts | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index bb64e27f9424..a9556c6175e3 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -90,9 +90,19 @@ interface ClientJoinMessage extends IInboundSignalMessage { }; } +const ackknowledgementMessageType = "Pres:Ack"; + +interface AcknowledgementMessage extends IInboundSignalMessage { + type: typeof ackknowledgementMessageType; + content: { + sendTimestamp: number; + avgLatency: number; + }; +} + function isPresenceMessage( message: IInboundSignalMessage, -): message is DatastoreUpdateMessage | ClientJoinMessage { +): message is DatastoreUpdateMessage | ClientJoinMessage | AcknowledgementMessage { return message.type.startsWith("Pres:"); } /** @@ -356,7 +366,11 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { // IExtensionMessage is a subset of IInboundSignalMessage so this is safe. // Change types of DatastoreUpdateMessage | ClientJoinMessage to // IExtensionMessage<> derivatives to see the issues. - message: IInboundSignalMessage | DatastoreUpdateMessage | ClientJoinMessage, + message: + | IInboundSignalMessage + | DatastoreUpdateMessage + | ClientJoinMessage + | AcknowledgementMessage, local: boolean, ): void { const received = Date.now(); @@ -390,6 +404,9 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } // It is okay to continue processing the contained updates even if we are not // connected. + } else if (message.type === ackknowledgementMessageType) { + // TODO: Handle acknowledgement message type once implemented. + return; } else { assert(message.type === datastoreUpdateMessageType, 0xa3b /* Unexpected message type */); if (message.content.isComplete) { From f3d92c5e971cf05c0f4b336b711c93a35e98cd1a Mon Sep 17 00:00:00 2001 From: WillieHabi <143546745+WillieHabi@users.noreply.github.com> Date: Mon, 28 Apr 2025 19:12:02 +0100 Subject: [PATCH 02/18] Update packages/framework/presence/src/presenceDatastoreManager.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- packages/framework/presence/src/presenceDatastoreManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index a9556c6175e3..61829262efde 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -90,10 +90,10 @@ interface ClientJoinMessage extends IInboundSignalMessage { }; } -const ackknowledgementMessageType = "Pres:Ack"; +const acknowledgementMessageType = "Pres:Ack"; interface AcknowledgementMessage extends IInboundSignalMessage { - type: typeof ackknowledgementMessageType; + type: typeof acknowledgementMessageType; content: { sendTimestamp: number; avgLatency: number; From 36e46f6f7e30572e1f1ded825e51fb4b8f68b19c Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Mon, 28 Apr 2025 19:13:41 +0100 Subject: [PATCH 03/18] change comment --- packages/framework/presence/src/presenceDatastoreManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index a9556c6175e3..3201a03a01fc 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -405,7 +405,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { // It is okay to continue processing the contained updates even if we are not // connected. } else if (message.type === ackknowledgementMessageType) { - // TODO: Handle acknowledgement message type once implemented. + // TODO: Handle acknowledgement message type. return; } else { assert(message.type === datastoreUpdateMessageType, 0xa3b /* Unexpected message type */); From a5bcfa707952a36c9b68f65c9b89fbf77b579000 Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Mon, 28 Apr 2025 19:14:02 +0100 Subject: [PATCH 04/18] fix mispelling --- packages/framework/presence/src/presenceDatastoreManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index c6324920b722..29ab1eb8f0e8 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -404,7 +404,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } // It is okay to continue processing the contained updates even if we are not // connected. - } else if (message.type === ackknowledgementMessageType) { + } else if (message.type === acknowledgementMessageType) { // TODO: Handle acknowledgement message type. return; } else { From ad1793fa06fb0b10ca342feb917b348f249561b7 Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Tue, 29 Apr 2025 09:31:21 +0100 Subject: [PATCH 05/18] req changes --- .../presence/src/presenceDatastoreManager.ts | 16 +++++++------ .../src/test/presenceDatastoreManager.spec.ts | 24 +++++++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index 29ab1eb8f0e8..fb5bdbafe17a 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -94,10 +94,7 @@ const acknowledgementMessageType = "Pres:Ack"; interface AcknowledgementMessage extends IInboundSignalMessage { type: typeof acknowledgementMessageType; - content: { - sendTimestamp: number; - avgLatency: number; - }; + content: unknown; } function isPresenceMessage( @@ -378,6 +375,14 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { if (!isPresenceMessage(message)) { return; } + + // Here we accept acknowledgement messages passively. + // Once implemented, these will be used for tracking message delivery status. + // For now, we just skip processing these messages. + if (message.type === acknowledgementMessageType) { + return; + } + if (local) { const deliveryDelta = received - message.content.sendTimestamp; // Limit returnedMessages count to 256 such that newest message @@ -404,9 +409,6 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } // It is okay to continue processing the contained updates even if we are not // connected. - } else if (message.type === acknowledgementMessageType) { - // TODO: Handle acknowledgement message type. - return; } else { assert(message.type === datastoreUpdateMessageType, 0xa3b /* Unexpected message type */); if (message.content.isComplete) { diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index 10f2b2d9f9f0..20631c27d2a2 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -438,5 +438,29 @@ describe("Presence", () => { assert.strictEqual(listener.callCount, 1); }); }); + + describe("receiving AcknowledgementMessage", () => { + it("accpets passively without failing", () => { + const presence = prepareConnectedPresence( + runtime, + "attendeeId-2", + "client2", + clock, + logger, + ); + + presence.processSignal( + "", + { + type: "Pres:Ack", + content: { + messageId: "messageId", + }, + clientId: "client1", + }, + false, + ); + }); + }); }); }); From 35757eb2520f0b46955935368bc900aec557eb9c Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Thu, 1 May 2025 14:55:47 +0100 Subject: [PATCH 06/18] more comments --- .../framework/presence/src/presenceDatastoreManager.ts | 5 +++++ .../presence/src/test/presenceDatastoreManager.spec.ts | 9 ++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index fb5bdbafe17a..2ffd252d84fa 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -90,10 +90,15 @@ interface ClientJoinMessage extends IInboundSignalMessage { }; } +// Message type identifier for presence acknowledgment messages. const acknowledgementMessageType = "Pres:Ack"; +// Signal message format used to acknowledge receipt of presence-related messages. +// These are lightweight and typically contain only the messageId being acknowledged. interface AcknowledgementMessage extends IInboundSignalMessage { type: typeof acknowledgementMessageType; + // TODO: Define what ack content will be shaped like ({ messageId: string }), + // Keeping generic for now to allow evolution of this message type. content: unknown; } diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index 20631c27d2a2..0747100084a1 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -440,7 +440,7 @@ describe("Presence", () => { }); describe("receiving AcknowledgementMessage", () => { - it("accpets passively without failing", () => { + it("accepts passively without failing", () => { const presence = prepareConnectedPresence( runtime, "attendeeId-2", @@ -448,13 +448,16 @@ describe("Presence", () => { clock, logger, ); - + // This test ensures that PresenceManager can safely receive and process + // a signal of type 'Pres:Ack' without throwing or misbehaving, + // even if the acknowledgment does not correspond to any known pending message. + // We do not assert specific outcomes here — success is defined by the absence of error presence.processSignal( "", { type: "Pres:Ack", content: { - messageId: "messageId", + messageId: "messageId", // Arbitrary ID that is not expected to match anything }, clientId: "client1", }, From e6c52b15f91133b74e36fcdb5ab28596a6c8512c Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Tue, 13 May 2025 09:03:47 -0700 Subject: [PATCH 07/18] ackRequested and supportedFeatures --- .../framework/presence/src/internalTypes.ts | 4 +++- .../presence/src/presenceDatastoreManager.ts | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/framework/presence/src/internalTypes.ts b/packages/framework/presence/src/internalTypes.ts index 5af893f7695b..a5a3a83667db 100644 --- a/packages/framework/presence/src/internalTypes.ts +++ b/packages/framework/presence/src/internalTypes.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. */ +import type { ILayerCompatDetails } from "@fluid-internal/client-utils"; import type { IContainerRuntime } from "@fluidframework/container-runtime-definitions/internal"; import type { IFluidDataStoreRuntime } from "@fluidframework/datastore-definitions/internal"; @@ -34,7 +35,8 @@ export type IEphemeralRuntime = Pick< (IContainerRuntime & IRuntimeInternal) | IFluidDataStoreRuntime, "clientId" | "connected" | "getAudience" | "getQuorum" | "off" | "on" | "submitSignal" > & - Partial>; + Partial> & + Partial>; /** * @internal diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index 2ffd252d84fa..0f92058864f6 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -75,6 +75,7 @@ interface DatastoreUpdateMessage extends IInboundSignalMessage { sendTimestamp: number; avgLatency: number; isComplete?: true; + ackRequested?: true; data: DatastoreMessageContent; }; } @@ -419,6 +420,22 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { if (message.content.isComplete) { this.refreshBroadcastRequested = false; } + // If the message requests an acknowledgement, we will send one back. + if ( + message.content.ackRequested && + this.runtime.supportedFeatures?.has("submit_signals_v2") === true + ) { + this.runtime.submitSignal( + acknowledgementMessageType, + { + type: acknowledgementMessageType, + // Using client Id + datastore update message content as messageId for now. + // This is a temporary solution until we settle on a final unique message id format. + content: { messageId: message.clientId + JSON.stringify(message.content.data) }, + } satisfies AcknowledgementMessage["content"], + message.clientId, + ); + } } // Handle activation of unregistered workspaces before processing updates. From 433df8b0f7c806c7586352d40ad4ed375c24faf3 Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Tue, 13 May 2025 10:56:59 -0700 Subject: [PATCH 08/18] ILayerCompatDetails --- packages/framework/presence/src/internalTypes.ts | 6 +++--- .../presence/src/presenceDatastoreManager.ts | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/framework/presence/src/internalTypes.ts b/packages/framework/presence/src/internalTypes.ts index a5a3a83667db..99acc47a9c6a 100644 --- a/packages/framework/presence/src/internalTypes.ts +++ b/packages/framework/presence/src/internalTypes.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. */ -import type { ILayerCompatDetails } from "@fluid-internal/client-utils"; import type { IContainerRuntime } from "@fluidframework/container-runtime-definitions/internal"; import type { IFluidDataStoreRuntime } from "@fluidframework/datastore-definitions/internal"; @@ -35,8 +34,9 @@ export type IEphemeralRuntime = Pick< (IContainerRuntime & IRuntimeInternal) | IFluidDataStoreRuntime, "clientId" | "connected" | "getAudience" | "getQuorum" | "off" | "on" | "submitSignal" > & - Partial> & - Partial>; + Partial> & { + ILayerCompatDetails?: unknown; + }; /** * @internal diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index 0f92058864f6..d5ca142e2a8a 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. */ +import type { ILayerCompatDetails } from "@fluid-internal/client-utils"; import type { IEmitter } from "@fluidframework/core-interfaces/internal"; import { assert } from "@fluidframework/core-utils/internal"; import type { IInboundSignalMessage } from "@fluidframework/runtime-definitions/internal"; @@ -108,6 +109,16 @@ function isPresenceMessage( ): message is DatastoreUpdateMessage | ClientJoinMessage | AcknowledgementMessage { return message.type.startsWith("Pres:"); } + +function isLayerCompatDetails(value: unknown): value is ILayerCompatDetails { + return ( + typeof value === "object" && + value !== null && + "supportedFeatures" in value && + value.supportedFeatures instanceof Set + ); +} + /** * @internal */ @@ -423,13 +434,14 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { // If the message requests an acknowledgement, we will send one back. if ( message.content.ackRequested && - this.runtime.supportedFeatures?.has("submit_signals_v2") === true + isLayerCompatDetails(this.runtime.ILayerCompatDetails) && + this.runtime.ILayerCompatDetails?.supportedFeatures?.has("submit_signals_v2") === true ) { this.runtime.submitSignal( acknowledgementMessageType, { type: acknowledgementMessageType, - // Using client Id + datastore update message content as messageId for now. + // Using client Id + datastore update message content as messageId for now. // This is a temporary solution until we settle on a final unique message id format. content: { messageId: message.clientId + JSON.stringify(message.content.data) }, } satisfies AcknowledgementMessage["content"], From bd69e1dcdbda901ca3a77a8e38fedda0f65a16ed Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Wed, 14 May 2025 09:12:19 -0700 Subject: [PATCH 09/18] acknowledgmentId --- .../presence/src/presenceDatastoreManager.ts | 20 +++++++------- .../src/test/presenceDatastoreManager.spec.ts | 27 ------------------- 2 files changed, 10 insertions(+), 37 deletions(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index d5ca142e2a8a..2484429e9db7 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -76,7 +76,7 @@ interface DatastoreUpdateMessage extends IInboundSignalMessage { sendTimestamp: number; avgLatency: number; isComplete?: true; - ackRequested?: true; + acknowledgementId?: number; data: DatastoreMessageContent; }; } @@ -101,7 +101,9 @@ interface AcknowledgementMessage extends IInboundSignalMessage { type: typeof acknowledgementMessageType; // TODO: Define what ack content will be shaped like ({ messageId: string }), // Keeping generic for now to allow evolution of this message type. - content: unknown; + content: { + id: number; + }; } function isPresenceMessage( @@ -393,10 +395,11 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { return; } - // Here we accept acknowledgement messages passively. - // Once implemented, these will be used for tracking message delivery status. - // For now, we just skip processing these messages. if (message.type === acknowledgementMessageType) { + // TODO: Handle acknowledgement messages here. + // Placeholder for future implementation. + // These acknowledgement messages will be used to confirm receipt + // of pending messages that were previously sent. return; } @@ -433,17 +436,14 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } // If the message requests an acknowledgement, we will send one back. if ( - message.content.ackRequested && + message.content.acknowledgementId !== undefined && isLayerCompatDetails(this.runtime.ILayerCompatDetails) && this.runtime.ILayerCompatDetails?.supportedFeatures?.has("submit_signals_v2") === true ) { this.runtime.submitSignal( acknowledgementMessageType, { - type: acknowledgementMessageType, - // Using client Id + datastore update message content as messageId for now. - // This is a temporary solution until we settle on a final unique message id format. - content: { messageId: message.clientId + JSON.stringify(message.content.data) }, + id: message.content.acknowledgementId, } satisfies AcknowledgementMessage["content"], message.clientId, ); diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index 0747100084a1..10f2b2d9f9f0 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -438,32 +438,5 @@ describe("Presence", () => { assert.strictEqual(listener.callCount, 1); }); }); - - describe("receiving AcknowledgementMessage", () => { - it("accepts passively without failing", () => { - const presence = prepareConnectedPresence( - runtime, - "attendeeId-2", - "client2", - clock, - logger, - ); - // This test ensures that PresenceManager can safely receive and process - // a signal of type 'Pres:Ack' without throwing or misbehaving, - // even if the acknowledgment does not correspond to any known pending message. - // We do not assert specific outcomes here — success is defined by the absence of error - presence.processSignal( - "", - { - type: "Pres:Ack", - content: { - messageId: "messageId", // Arbitrary ID that is not expected to match anything - }, - clientId: "client1", - }, - false, - ); - }); - }); }); }); From d50f7d37a68e33e6ff0f65cc899ea697c55302e3 Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Wed, 14 May 2025 09:15:21 -0700 Subject: [PATCH 10/18] remove comment --- packages/framework/presence/src/presenceDatastoreManager.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index 2484429e9db7..493b51df3b52 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -99,8 +99,6 @@ const acknowledgementMessageType = "Pres:Ack"; // These are lightweight and typically contain only the messageId being acknowledged. interface AcknowledgementMessage extends IInboundSignalMessage { type: typeof acknowledgementMessageType; - // TODO: Define what ack content will be shaped like ({ messageId: string }), - // Keeping generic for now to allow evolution of this message type. content: { id: number; }; From b524bd8cdc7ea720feb9de9b68493d15edd1b649 Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Thu, 15 May 2025 11:52:14 -0700 Subject: [PATCH 11/18] update ts-doc for ack message type --- packages/framework/presence/src/presenceDatastoreManager.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index 493b51df3b52..55071b00c003 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -95,8 +95,9 @@ interface ClientJoinMessage extends IInboundSignalMessage { // Message type identifier for presence acknowledgment messages. const acknowledgementMessageType = "Pres:Ack"; -// Signal message format used to acknowledge receipt of presence-related messages. -// These are lightweight and typically contain only the messageId being acknowledged. +/** + * Message type used to acknowledge receipt of remote update messages. + */ interface AcknowledgementMessage extends IInboundSignalMessage { type: typeof acknowledgementMessageType; content: { From 3f5564626c3c816705377530c2f3d953b77a735b Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Thu, 15 May 2025 13:15:01 -0700 Subject: [PATCH 12/18] requested changes --- .../presence/src/presenceDatastoreManager.ts | 35 +++++++--------- .../presence/src/test/mockEphemeralRuntime.ts | 18 ++++++++ .../src/test/presenceDatastoreManager.spec.ts | 41 +++++++++++++++++++ 3 files changed, 74 insertions(+), 20 deletions(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index 55071b00c003..143dfb34da3f 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -107,7 +107,7 @@ interface AcknowledgementMessage extends IInboundSignalMessage { function isPresenceMessage( message: IInboundSignalMessage, -): message is DatastoreUpdateMessage | ClientJoinMessage | AcknowledgementMessage { +): message is DatastoreUpdateMessage | ClientJoinMessage { return message.type.startsWith("Pres:"); } @@ -182,6 +182,9 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { private refreshBroadcastRequested = false; private readonly timer = new TimerManager(); private readonly workspaces = new Map>(); + private readonly supportsTargetedSignals = + isLayerCompatDetails(this.runtime.ILayerCompatDetails) && + this.runtime.ILayerCompatDetails.supportedFeatures.has("submit_signals_v2"); public constructor( private readonly attendeeId: AttendeeId, @@ -381,11 +384,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { // IExtensionMessage is a subset of IInboundSignalMessage so this is safe. // Change types of DatastoreUpdateMessage | ClientJoinMessage to // IExtensionMessage<> derivatives to see the issues. - message: - | IInboundSignalMessage - | DatastoreUpdateMessage - | ClientJoinMessage - | AcknowledgementMessage, + message: IInboundSignalMessage | DatastoreUpdateMessage | ClientJoinMessage, local: boolean, ): void { const received = Date.now(); @@ -394,14 +393,6 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { return; } - if (message.type === acknowledgementMessageType) { - // TODO: Handle acknowledgement messages here. - // Placeholder for future implementation. - // These acknowledgement messages will be used to confirm receipt - // of pending messages that were previously sent. - return; - } - if (local) { const deliveryDelta = received - message.content.sendTimestamp; // Limit returnedMessages count to 256 such that newest message @@ -433,12 +424,9 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { if (message.content.isComplete) { this.refreshBroadcastRequested = false; } - // If the message requests an acknowledgement, we will send one back. - if ( - message.content.acknowledgementId !== undefined && - isLayerCompatDetails(this.runtime.ILayerCompatDetails) && - this.runtime.ILayerCompatDetails?.supportedFeatures?.has("submit_signals_v2") === true - ) { + // If the message requests an acknowledgement, we will send a targeted acknowledgement message back to just the requestor. + if (message.content.acknowledgementId !== undefined) { + assert(this.supportsTargetedSignals, "Targeted signals not supported"); this.runtime.submitSignal( acknowledgementMessageType, { @@ -446,6 +434,13 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } satisfies AcknowledgementMessage["content"], message.clientId, ); + this.logger?.sendTelemetryEvent({ + eventName: "AckSent", + details: { + requestor: message.clientId, + responder: this.runtime.clientId, + }, + }); } } diff --git a/packages/framework/presence/src/test/mockEphemeralRuntime.ts b/packages/framework/presence/src/test/mockEphemeralRuntime.ts index f966c14a04cc..69b72acab153 100644 --- a/packages/framework/presence/src/test/mockEphemeralRuntime.ts +++ b/packages/framework/presence/src/test/mockEphemeralRuntime.ts @@ -5,6 +5,7 @@ import { strict as assert } from "node:assert"; +import type { ILayerCompatDetails } from "@fluid-internal/client-utils"; import type { ITelemetryBaseLogger } from "@fluidframework/core-interfaces"; import type { IClient, ISequencedClient } from "@fluidframework/driver-definitions"; import { MockAudience, MockQuorumClients } from "@fluidframework/test-runtime-utils/internal"; @@ -78,6 +79,7 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { connected: [], disconnected: [], }; + private readonly supportedFeatures = new Set(["submit_signals_v2"]); private isSupportedEvent(event: string): event is keyof typeof this.listeners { return event in this.listeners; } @@ -136,6 +138,14 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { ); } + public setTargetSignalSupport(supported: boolean): void { + if (supported) { + this.supportedFeatures.add("submit_signals_v2"); + } else { + this.supportedFeatures.delete("submit_signals_v2"); + } + } + public removeMember(clientId: ClientConnectionId): void { const client = this.audience.getMember(clientId); assert(client !== undefined, `Attempting to remove unknown connection: ${clientId}`); @@ -187,5 +197,13 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { assert.deepStrictEqual(args, expected, "Unexpected signal"); }; + public get ILayerCompatDetails(): ILayerCompatDetails { + return { + supportedFeatures: this.supportedFeatures, + generation: 1, + pkgVersion: "2.33.0", + }; + } + // #endregion } diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index 10f2b2d9f9f0..3ad084507ab3 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -437,6 +437,47 @@ describe("Presence", () => { // Verify assert.strictEqual(listener.callCount, 1); }); + + it("with requested acknowledgment sends acknowledgment back to requestor", () => { + // Setup + logger.registerExpectedEvent({ + eventName: "Presence:AckSent", + details: JSON.stringify({ + requestor: "client4", + responder: "client2", + }), + }); + // We expect to send a targeted acknowledgment back to the requestor + runtime.signalsExpected.push([ + "Pres:Ack", + { + "id": 1, + }, + "client4" /* targetClientId */, + ]); + + // Act - send generic datastore update with acknowledgement id specified + presence.processSignal( + "", + { + type: "Pres:DatastoreUpdate", + content: { + sendTimestamp: clock.now - 10, + avgLatency: 20, + data: { + "system:presence": systemWorkspaceUpdate, + "s:name:testStateWorkspace": statesWorkspaceUpdate, + }, + acknowledgementId: 1, + }, + clientId: "client4", + }, + false, + ); + + // Verify + assertFinalExpectations(runtime, logger); + }); }); }); }); From a8208928f03c51242abe329f3e7344b2af42207e Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Thu, 15 May 2025 13:15:47 -0700 Subject: [PATCH 13/18] test title --- .../presence/src/test/presenceDatastoreManager.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index 3ad084507ab3..baf47daaf4e3 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -438,7 +438,7 @@ describe("Presence", () => { assert.strictEqual(listener.callCount, 1); }); - it("with requested acknowledgment sends acknowledgment back to requestor", () => { + it("with acknowledgementId sends targeted acknowledgment messsage back to requestor", () => { // Setup logger.registerExpectedEvent({ eventName: "Presence:AckSent", From cb28f2a9c64e7e80889b03dec5fdf4bbaa59edac Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Wed, 21 May 2025 14:41:24 -0700 Subject: [PATCH 14/18] FluidObject + correct changes --- .../framework/presence/src/internalTypes.ts | 4 +-- .../presence/src/presenceDatastoreManager.ts | 28 ++++++------------- .../presence/src/test/mockEphemeralRuntime.ts | 1 + .../src/test/presenceDatastoreManager.spec.ts | 8 ------ 4 files changed, 10 insertions(+), 31 deletions(-) diff --git a/packages/framework/presence/src/internalTypes.ts b/packages/framework/presence/src/internalTypes.ts index 99acc47a9c6a..5af893f7695b 100644 --- a/packages/framework/presence/src/internalTypes.ts +++ b/packages/framework/presence/src/internalTypes.ts @@ -34,9 +34,7 @@ export type IEphemeralRuntime = Pick< (IContainerRuntime & IRuntimeInternal) | IFluidDataStoreRuntime, "clientId" | "connected" | "getAudience" | "getQuorum" | "off" | "on" | "submitSignal" > & - Partial> & { - ILayerCompatDetails?: unknown; - }; + Partial>; /** * @internal diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index 143dfb34da3f..e9b6847acffc 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -4,7 +4,7 @@ */ import type { ILayerCompatDetails } from "@fluid-internal/client-utils"; -import type { IEmitter } from "@fluidframework/core-interfaces/internal"; +import type { FluidObject, IEmitter } from "@fluidframework/core-interfaces/internal"; import { assert } from "@fluidframework/core-utils/internal"; import type { IInboundSignalMessage } from "@fluidframework/runtime-definitions/internal"; import type { ITelemetryLoggerExt } from "@fluidframework/telemetry-utils/internal"; @@ -111,15 +111,6 @@ function isPresenceMessage( return message.type.startsWith("Pres:"); } -function isLayerCompatDetails(value: unknown): value is ILayerCompatDetails { - return ( - typeof value === "object" && - value !== null && - "supportedFeatures" in value && - value.supportedFeatures instanceof Set - ); -} - /** * @internal */ @@ -182,9 +173,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { private refreshBroadcastRequested = false; private readonly timer = new TimerManager(); private readonly workspaces = new Map>(); - private readonly supportsTargetedSignals = - isLayerCompatDetails(this.runtime.ILayerCompatDetails) && - this.runtime.ILayerCompatDetails.supportedFeatures.has("submit_signals_v2"); + private readonly supportsTargetedSignals: boolean; public constructor( private readonly attendeeId: AttendeeId, @@ -199,6 +188,12 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { // eslint-disable-next-line @typescript-eslint/consistent-type-assertions this.datastore = { "system:presence": systemWorkspaceDatastore } as PresenceDatastore; this.workspaces.set("system:presence", systemWorkspace); + // Determine if the runtime supports targeted signals + const maybeLayerCompatDetails = this.runtime as FluidObject; + this.supportsTargetedSignals = + maybeLayerCompatDetails.ILayerCompatDetails?.supportedFeatures.has( + "submit_signals_v2", + ) ?? false; } public joinSession(clientId: ClientConnectionId): void { @@ -434,13 +429,6 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } satisfies AcknowledgementMessage["content"], message.clientId, ); - this.logger?.sendTelemetryEvent({ - eventName: "AckSent", - details: { - requestor: message.clientId, - responder: this.runtime.clientId, - }, - }); } } diff --git a/packages/framework/presence/src/test/mockEphemeralRuntime.ts b/packages/framework/presence/src/test/mockEphemeralRuntime.ts index 69b72acab153..65da97713efb 100644 --- a/packages/framework/presence/src/test/mockEphemeralRuntime.ts +++ b/packages/framework/presence/src/test/mockEphemeralRuntime.ts @@ -200,6 +200,7 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { public get ILayerCompatDetails(): ILayerCompatDetails { return { supportedFeatures: this.supportedFeatures, + // arbitrary generation number and package version for testing generation: 1, pkgVersion: "2.33.0", }; diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index baf47daaf4e3..dd06c8ab1e34 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -439,14 +439,6 @@ describe("Presence", () => { }); it("with acknowledgementId sends targeted acknowledgment messsage back to requestor", () => { - // Setup - logger.registerExpectedEvent({ - eventName: "Presence:AckSent", - details: JSON.stringify({ - requestor: "client4", - responder: "client2", - }), - }); // We expect to send a targeted acknowledgment back to the requestor runtime.signalsExpected.push([ "Pres:Ack", From 3815ef3af84ab7a74549a96dd4dc9e3ecfee9582 Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Tue, 27 May 2025 17:08:59 -0700 Subject: [PATCH 15/18] additional changes --- .../src/datastorePresenceManagerFactory.ts | 9 +++++-- .../framework/presence/src/internalTypes.ts | 6 ++++- .../presence/src/presenceDatastoreManager.ts | 24 ++++++++++++------- packages/framework/presence/src/protocol.ts | 23 +++++++++++++++++- .../src/test/presenceDatastoreManager.spec.ts | 8 +++---- 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/packages/framework/presence/src/datastorePresenceManagerFactory.ts b/packages/framework/presence/src/datastorePresenceManagerFactory.ts index dcce018a5d54..eda0fe3ecadd 100644 --- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts +++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts @@ -21,6 +21,7 @@ import { BasicDataStoreFactory, LoadableFluidObject } from "./datastoreSupport.j import type { PresenceWithNotifications as Presence } from "./presence.js"; import { createPresenceManager } from "./presenceManager.js"; import type { + OutboundAcknowledgementMessage, OutboundClientJoinMessage, OutboundDatastoreUpdateMessage, SignalMessages, @@ -60,8 +61,12 @@ class PresenceManagerDataObject extends LoadableFluidObject { events, getQuorum: runtime.getQuorum.bind(runtime), getAudience: runtime.getAudience.bind(runtime), - submitSignal: (message: OutboundClientJoinMessage | OutboundDatastoreUpdateMessage) => - runtime.submitSignal(message.type, message.content, message.targetClientId), + submitSignal: ( + message: + | OutboundClientJoinMessage + | OutboundDatastoreUpdateMessage + | OutboundAcknowledgementMessage, + ) => runtime.submitSignal(message.type, message.content, message.targetClientId), }); this.runtime.on("signal", (message: IInboundSignalMessage, local: boolean) => { assertSignalMessageIsValid(message); diff --git a/packages/framework/presence/src/internalTypes.ts b/packages/framework/presence/src/internalTypes.ts index 1aabba9d66b1..2c157cb97983 100644 --- a/packages/framework/presence/src/internalTypes.ts +++ b/packages/framework/presence/src/internalTypes.ts @@ -8,6 +8,7 @@ import type { ExtensionHost as ContainerExtensionHost } from "@fluidframework/co import type { InternalTypes } from "./exposedInternalTypes.js"; import type { AttendeeId, Attendee } from "./presence.js"; import type { + OutboundAcknowledgementMessage, OutboundClientJoinMessage, OutboundDatastoreUpdateMessage, SignalMessages, @@ -55,7 +56,10 @@ export type IEphemeralRuntime = Omit void; }; diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index 5600244d9b0d..dabb12ea5ffd 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -38,7 +38,11 @@ import type { SignalMessages, SystemDatastore, } from "./protocol.js"; -import { datastoreUpdateMessageType, joinMessageType } from "./protocol.js"; +import { + acknowledgementMessageType, + datastoreUpdateMessageType, + joinMessageType, +} from "./protocol.js"; import type { SystemWorkspaceDatastore } from "./systemWorkspace.js"; import { TimerManager } from "./timerManager.js"; import type { @@ -66,7 +70,11 @@ const internalWorkspaceTypes: Readonly, ): message is InboundDatastoreUpdateMessage | InboundClientJoinMessage { @@ -389,13 +397,11 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { // If the message requests an acknowledgement, we will send a targeted acknowledgement message back to just the requestor. if (message.content.acknowledgementId !== undefined) { assert(this.supportsTargetedSignals, "Targeted signals not supported"); - this.runtime.submitSignal( - acknowledgementMessageType, - { - id: message.content.acknowledgementId, - } satisfies AcknowledgementMessage["content"], - message.clientId, - ); + this.runtime.submitSignal({ + type: acknowledgementMessageType, + content: { id: message.content.acknowledgementId }, + targetClientId: message.clientId, + }); } } diff --git a/packages/framework/presence/src/protocol.ts b/packages/framework/presence/src/protocol.ts index d4259a223513..0c61e150b385 100644 --- a/packages/framework/presence/src/protocol.ts +++ b/packages/framework/presence/src/protocol.ts @@ -42,6 +42,7 @@ interface DatastoreUpdateMessage { content: { sendTimestamp: number; avgLatency: number; + acknowledgementId?: number; isComplete?: true; data: DatastoreMessageContent; }; @@ -72,6 +73,23 @@ interface ClientJoinMessage { }; } +/** + * @internal + */ +export const acknowledgementMessageType = "Pres:Ack"; + +interface AcknowledgementMessage { + type: typeof acknowledgementMessageType; + content: { + id: number; + }; +} + +/** + * @internal + */ +export type OutboundAcknowledgementMessage = OutboundExtensionMessage; + /** * @internal */ @@ -85,4 +103,7 @@ export type InboundClientJoinMessage = VerifiedInboundExtensionMessage { it("with acknowledgementId sends targeted acknowledgment messsage back to requestor", () => { // We expect to send a targeted acknowledgment back to the requestor runtime.signalsExpected.push([ - "Pres:Ack", { - "id": 1, + type: "Pres:Ack", + content: { id: 1 }, + targetClientId: "client4", }, - "client4" /* targetClientId */, ]); // Act - send generic datastore update with acknowledgement id specified presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { From c7b1ed21b5ad570fabf75701e9f06e27146008fa Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Tue, 27 May 2025 17:28:05 -0700 Subject: [PATCH 16/18] add targeted signal support --- .../presence/src/experimentalAccess.ts | 20 ++++++++++++++----- .../presence/src/presenceDatastoreManager.ts | 13 +++--------- .../framework/presence/src/presenceManager.ts | 12 +++++++++-- .../presence/src/test/mockEphemeralRuntime.ts | 20 +------------------ .../framework/presence/src/test/testUtils.ts | 2 +- 5 files changed, 30 insertions(+), 37 deletions(-) diff --git a/packages/framework/presence/src/experimentalAccess.ts b/packages/framework/presence/src/experimentalAccess.ts index 1d85995c1ae9..9732d47ef6f5 100644 --- a/packages/framework/presence/src/experimentalAccess.ts +++ b/packages/framework/presence/src/experimentalAccess.ts @@ -3,11 +3,13 @@ * Licensed under the MIT License. */ +import type { ILayerCompatDetails } from "@fluid-internal/client-utils"; import type { ContainerExtension, ContainerExtensionFactory, InboundExtensionMessage, } from "@fluidframework/container-runtime-definitions/internal"; +import type { FluidObject } from "@fluidframework/core-interfaces"; import { assert } from "@fluidframework/core-utils/internal"; import type { IFluidContainer } from "@fluidframework/fluid-static"; import { isInternalFluidContainer } from "@fluidframework/fluid-static/internal"; @@ -35,12 +37,20 @@ class ContainerPresenceManager private readonly manager: PresenceExtensionInterface; public constructor(host: ExtensionHost) { - this.interface = this.manager = createPresenceManager({ - ...host, - submitSignal: (message) => { - host.submitAddressedSignal([], message); + const maybeLayerCompatDetails = host as FluidObject; + const targetedSignalSupport = + maybeLayerCompatDetails.ILayerCompatDetails?.supportedFeatures.has( + "submit_signals_v2", + ) ?? false; + this.interface = this.manager = createPresenceManager( + { + ...host, + submitSignal: (message) => { + host.submitAddressedSignal([], message); + }, }, - }); + targetedSignalSupport, + ); } public onNewUse(): void { diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index dabb12ea5ffd..a867a1b28be1 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -3,9 +3,8 @@ * Licensed under the MIT License. */ -import type { ILayerCompatDetails } from "@fluid-internal/client-utils"; import type { InboundExtensionMessage } from "@fluidframework/container-runtime-definitions/internal"; -import type { FluidObject, IEmitter } from "@fluidframework/core-interfaces/internal"; +import type { IEmitter } from "@fluidframework/core-interfaces/internal"; import { assert } from "@fluidframework/core-utils/internal"; import type { ITelemetryLoggerExt } from "@fluidframework/telemetry-utils/internal"; @@ -147,7 +146,6 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { private refreshBroadcastRequested = false; private readonly timer = new TimerManager(); private readonly workspaces = new Map>(); - private readonly supportsTargetedSignals: boolean; public constructor( private readonly attendeeId: AttendeeId, @@ -158,16 +156,11 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { private readonly presence: Presence, systemWorkspaceDatastore: SystemWorkspaceDatastore, systemWorkspace: AnyWorkspaceEntry, + private readonly targetedSignalSupport: boolean, ) { // eslint-disable-next-line @typescript-eslint/consistent-type-assertions this.datastore = { "system:presence": systemWorkspaceDatastore } as PresenceDatastore; this.workspaces.set("system:presence", systemWorkspace); - // Determine if the runtime supports targeted signals - const maybeLayerCompatDetails = this.runtime as FluidObject; - this.supportsTargetedSignals = - maybeLayerCompatDetails.ILayerCompatDetails?.supportedFeatures.has( - "submit_signals_v2", - ) ?? false; } public joinSession(clientId: ClientConnectionId): void { @@ -396,7 +389,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } // If the message requests an acknowledgement, we will send a targeted acknowledgement message back to just the requestor. if (message.content.acknowledgementId !== undefined) { - assert(this.supportsTargetedSignals, "Targeted signals not supported"); + assert(this.targetedSignalSupport, "Targeted signals not supported"); this.runtime.submitSignal({ type: acknowledgementMessageType, content: { id: message.content.acknowledgementId }, diff --git a/packages/framework/presence/src/presenceManager.ts b/packages/framework/presence/src/presenceManager.ts index b2b94d8ebaec..0328cc5b5c0d 100644 --- a/packages/framework/presence/src/presenceManager.ts +++ b/packages/framework/presence/src/presenceManager.ts @@ -76,7 +76,11 @@ class PresenceManager implements Presence, PresenceExtensionInterface { private readonly mc: MonitoringContext | undefined = undefined; - public constructor(runtime: IEphemeralRuntime, attendeeId: AttendeeId) { + public constructor( + runtime: IEphemeralRuntime, + attendeeId: AttendeeId, + targetedSignalSupport: boolean, + ) { const logger = runtime.logger; if (logger) { this.mc = createChildMonitoringContext({ logger, namespace: "Presence" }); @@ -89,6 +93,7 @@ class PresenceManager implements Presence, PresenceExtensionInterface { this.events, this.mc?.logger, this, + targetedSignalSupport, ); this.attendees = this.systemWorkspace; @@ -160,6 +165,7 @@ function setupSubComponents( IEmitter, logger: ITelemetryLoggerExt | undefined, presence: Presence, + targetedSignalSupport: boolean, ): [PresenceDatastoreManager, SystemWorkspace] { const systemWorkspaceDatastore: SystemWorkspaceDatastore = { clientToSessionId: {}, @@ -179,6 +185,7 @@ function setupSubComponents( presence, systemWorkspaceDatastore, systemWorkspaceConfig.statesEntry, + targetedSignalSupport, ); return [datastoreManager, systemWorkspaceConfig.workspace]; } @@ -190,7 +197,8 @@ function setupSubComponents( */ export function createPresenceManager( runtime: IEphemeralRuntime, + targetedSignalSupport: boolean = false, attendeeId: AttendeeId = createSessionId() as AttendeeId, ): Presence & PresenceExtensionInterface { - return new PresenceManager(runtime, attendeeId); + return new PresenceManager(runtime, attendeeId, targetedSignalSupport); } diff --git a/packages/framework/presence/src/test/mockEphemeralRuntime.ts b/packages/framework/presence/src/test/mockEphemeralRuntime.ts index d796bc1ce94b..85a7dbcd65f0 100644 --- a/packages/framework/presence/src/test/mockEphemeralRuntime.ts +++ b/packages/framework/presence/src/test/mockEphemeralRuntime.ts @@ -5,7 +5,6 @@ import { strict as assert } from "node:assert"; -import type { ILayerCompatDetails } from "@fluid-internal/client-utils"; import type { ITelemetryBaseLogger } from "@fluidframework/core-interfaces"; import type { IClient, ISequencedClient } from "@fluidframework/driver-definitions"; import { MockAudience, MockQuorumClients } from "@fluidframework/test-runtime-utils/internal"; @@ -81,7 +80,7 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { connected: [], disconnected: [], }; - private readonly supportedFeatures = new Set(["submit_signals_v2"]); + private isSupportedEvent(event: string): event is keyof typeof this.listeners { return event in this.listeners; } @@ -145,14 +144,6 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { ); } - public setTargetSignalSupport(supported: boolean): void { - if (supported) { - this.supportedFeatures.add("submit_signals_v2"); - } else { - this.supportedFeatures.delete("submit_signals_v2"); - } - } - public removeMember(clientId: ClientConnectionId): void { const client = this.audience.getMember(clientId); assert(client !== undefined, `Attempting to remove unknown connection: ${clientId}`); @@ -195,14 +186,5 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { assert.deepStrictEqual(args, expected, "Unexpected signal"); }; - public get ILayerCompatDetails(): ILayerCompatDetails { - return { - supportedFeatures: this.supportedFeatures, - // arbitrary generation number and package version for testing - generation: 1, - pkgVersion: "2.33.0", - }; - } - // #endregion } diff --git a/packages/framework/presence/src/test/testUtils.ts b/packages/framework/presence/src/test/testUtils.ts index ab2cbc5271fe..242ab16e0133 100644 --- a/packages/framework/presence/src/test/testUtils.ts +++ b/packages/framework/presence/src/test/testUtils.ts @@ -155,7 +155,7 @@ export function prepareConnectedPresence( delete expectedClientJoin.clientId; runtime.signalsExpected.push([expectedClientJoin]); - const presence = createPresenceManager(runtime, attendeeId as AttendeeId); + const presence = createPresenceManager(runtime, true, attendeeId as AttendeeId); // Validate expectations post initialization to make sure logger // and runtime are left in a clean expectation state. From bb621279f0938cbc7d6b0b346edb8aa36043bc33 Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Tue, 27 May 2025 18:06:31 -0700 Subject: [PATCH 17/18] Add supportedFeatures to ExtensionHost --- .../presence/src/datastorePresenceManagerFactory.ts | 8 +++++--- packages/framework/presence/src/experimentalAccess.ts | 9 +-------- packages/framework/presence/src/internalTypes.ts | 4 ++-- .../framework/presence/src/test/mockEphemeralRuntime.ts | 2 ++ packages/framework/presence/src/test/testUtils.ts | 6 +++++- .../src/containerExtension.ts | 3 ++- .../runtime/container-runtime/src/containerRuntime.ts | 1 + 7 files changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/framework/presence/src/datastorePresenceManagerFactory.ts b/packages/framework/presence/src/datastorePresenceManagerFactory.ts index eda0fe3ecadd..cd6e962aa9fe 100644 --- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts +++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts @@ -7,7 +7,7 @@ * Hacky support for internal datastore based usages. */ -import { createEmitter } from "@fluid-internal/client-utils"; +import { createEmitter, type ILayerCompatDetails } from "@fluid-internal/client-utils"; import type { ExtensionHostEvents, RawInboundExtensionMessage, @@ -63,10 +63,12 @@ class PresenceManagerDataObject extends LoadableFluidObject { getAudience: runtime.getAudience.bind(runtime), submitSignal: ( message: + | OutboundAcknowledgementMessage | OutboundClientJoinMessage - | OutboundDatastoreUpdateMessage - | OutboundAcknowledgementMessage, + | OutboundDatastoreUpdateMessage, ) => runtime.submitSignal(message.type, message.content, message.targetClientId), + supportedFeatures: + (runtime.ILayerCompatDetails as ILayerCompatDetails)?.supportedFeatures ?? new Set(), }); this.runtime.on("signal", (message: IInboundSignalMessage, local: boolean) => { assertSignalMessageIsValid(message); diff --git a/packages/framework/presence/src/experimentalAccess.ts b/packages/framework/presence/src/experimentalAccess.ts index 9732d47ef6f5..6023b5d0f245 100644 --- a/packages/framework/presence/src/experimentalAccess.ts +++ b/packages/framework/presence/src/experimentalAccess.ts @@ -3,13 +3,11 @@ * Licensed under the MIT License. */ -import type { ILayerCompatDetails } from "@fluid-internal/client-utils"; import type { ContainerExtension, ContainerExtensionFactory, InboundExtensionMessage, } from "@fluidframework/container-runtime-definitions/internal"; -import type { FluidObject } from "@fluidframework/core-interfaces"; import { assert } from "@fluidframework/core-utils/internal"; import type { IFluidContainer } from "@fluidframework/fluid-static"; import { isInternalFluidContainer } from "@fluidframework/fluid-static/internal"; @@ -37,11 +35,6 @@ class ContainerPresenceManager private readonly manager: PresenceExtensionInterface; public constructor(host: ExtensionHost) { - const maybeLayerCompatDetails = host as FluidObject; - const targetedSignalSupport = - maybeLayerCompatDetails.ILayerCompatDetails?.supportedFeatures.has( - "submit_signals_v2", - ) ?? false; this.interface = this.manager = createPresenceManager( { ...host, @@ -49,7 +42,7 @@ class ContainerPresenceManager host.submitAddressedSignal([], message); }, }, - targetedSignalSupport, + host.supportedFeatures.has("submit_signals_v2"), ); } diff --git a/packages/framework/presence/src/internalTypes.ts b/packages/framework/presence/src/internalTypes.ts index 2c157cb97983..122a6b15ee22 100644 --- a/packages/framework/presence/src/internalTypes.ts +++ b/packages/framework/presence/src/internalTypes.ts @@ -57,9 +57,9 @@ export type IEphemeralRuntime = Omit void; }; diff --git a/packages/framework/presence/src/test/mockEphemeralRuntime.ts b/packages/framework/presence/src/test/mockEphemeralRuntime.ts index 85a7dbcd65f0..e903bbec9d14 100644 --- a/packages/framework/presence/src/test/mockEphemeralRuntime.ts +++ b/packages/framework/presence/src/test/mockEphemeralRuntime.ts @@ -186,5 +186,7 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { assert.deepStrictEqual(args, expected, "Unexpected signal"); }; + public supportedFeatures: ReadonlySet = new Set(["submit_signals_v2"]); + // #endregion } diff --git a/packages/framework/presence/src/test/testUtils.ts b/packages/framework/presence/src/test/testUtils.ts index 242ab16e0133..417ff9e06812 100644 --- a/packages/framework/presence/src/test/testUtils.ts +++ b/packages/framework/presence/src/test/testUtils.ts @@ -155,7 +155,11 @@ export function prepareConnectedPresence( delete expectedClientJoin.clientId; runtime.signalsExpected.push([expectedClientJoin]); - const presence = createPresenceManager(runtime, true, attendeeId as AttendeeId); + const presence = createPresenceManager( + runtime, + true /* targetedSignalSupport */, + attendeeId as AttendeeId, + ); // Validate expectations post initialization to make sure logger // and runtime are left in a clean expectation state. diff --git a/packages/runtime/container-runtime-definitions/src/containerExtension.ts b/packages/runtime/container-runtime-definitions/src/containerExtension.ts index 28f40e031937..8fff85eaa160 100644 --- a/packages/runtime/container-runtime-definitions/src/containerExtension.ts +++ b/packages/runtime/container-runtime-definitions/src/containerExtension.ts @@ -22,7 +22,6 @@ import type { IQuorumClients } from "@fluidframework/driver-definitions/internal * @internal */ export type ClientConnectionId = string; - /** * Common structure between incoming and outgoing extension signals. * @@ -205,6 +204,8 @@ export interface ExtensionHost; + /** * Submits a signal to be sent to other clients. * @param addressChain - Custom address sequence for the signal. diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index b414b0832c16..68974b8ff7eb 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -4986,6 +4986,7 @@ export class ContainerRuntime }, getQuorum: this.getQuorum.bind(this), getAudience: this.getAudience.bind(this), + supportedFeatures: this.ILayerCompatDetails.supportedFeatures, } satisfies ExtensionHost; entry = new factory(runtime, ...useContext); this.extensions.set(id, entry); From 354fc29742434e0419b8b01239165a4f92229d2d Mon Sep 17 00:00:00 2001 From: Willie Habimana Date: Mon, 2 Jun 2025 09:47:42 -0700 Subject: [PATCH 18/18] req changes --- .../src/datastorePresenceManagerFactory.ts | 19 +++++-------------- .../presence/src/experimentalAccess.ts | 13 +++++-------- .../presence/src/presenceDatastoreManager.ts | 8 ++++++-- .../framework/presence/src/presenceManager.ts | 12 ++---------- packages/framework/presence/src/protocol.ts | 18 ++++++++++++++---- .../src/test/presenceDatastoreManager.spec.ts | 4 ++-- .../framework/presence/src/test/testUtils.ts | 6 +----- .../package.json | 1 + .../src/containerExtension.ts | 4 +++- pnpm-lock.yaml | 3 +++ 10 files changed, 42 insertions(+), 46 deletions(-) diff --git a/packages/framework/presence/src/datastorePresenceManagerFactory.ts b/packages/framework/presence/src/datastorePresenceManagerFactory.ts index cd6e962aa9fe..85bb348fc48b 100644 --- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts +++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts @@ -7,7 +7,7 @@ * Hacky support for internal datastore based usages. */ -import { createEmitter, type ILayerCompatDetails } from "@fluid-internal/client-utils"; +import { createEmitter } from "@fluid-internal/client-utils"; import type { ExtensionHostEvents, RawInboundExtensionMessage, @@ -20,12 +20,7 @@ import type { SharedObjectKind } from "@fluidframework/shared-object-base"; import { BasicDataStoreFactory, LoadableFluidObject } from "./datastoreSupport.js"; import type { PresenceWithNotifications as Presence } from "./presence.js"; import { createPresenceManager } from "./presenceManager.js"; -import type { - OutboundAcknowledgementMessage, - OutboundClientJoinMessage, - OutboundDatastoreUpdateMessage, - SignalMessages, -} from "./protocol.js"; +import type { OutboundPresenceMessage, SignalMessages } from "./protocol.js"; /** * This provides faux validation of the signal message. @@ -61,14 +56,10 @@ class PresenceManagerDataObject extends LoadableFluidObject { events, getQuorum: runtime.getQuorum.bind(runtime), getAudience: runtime.getAudience.bind(runtime), - submitSignal: ( - message: - | OutboundAcknowledgementMessage - | OutboundClientJoinMessage - | OutboundDatastoreUpdateMessage, - ) => runtime.submitSignal(message.type, message.content, message.targetClientId), + submitSignal: (message: OutboundPresenceMessage) => + runtime.submitSignal(message.type, message.content, message.targetClientId), supportedFeatures: - (runtime.ILayerCompatDetails as ILayerCompatDetails)?.supportedFeatures ?? new Set(), + new Set() /* We do not implement feature detection here since this is a deprecated path */, }); this.runtime.on("signal", (message: IInboundSignalMessage, local: boolean) => { assertSignalMessageIsValid(message); diff --git a/packages/framework/presence/src/experimentalAccess.ts b/packages/framework/presence/src/experimentalAccess.ts index 6023b5d0f245..1d85995c1ae9 100644 --- a/packages/framework/presence/src/experimentalAccess.ts +++ b/packages/framework/presence/src/experimentalAccess.ts @@ -35,15 +35,12 @@ class ContainerPresenceManager private readonly manager: PresenceExtensionInterface; public constructor(host: ExtensionHost) { - this.interface = this.manager = createPresenceManager( - { - ...host, - submitSignal: (message) => { - host.submitAddressedSignal([], message); - }, + this.interface = this.manager = createPresenceManager({ + ...host, + submitSignal: (message) => { + host.submitAddressedSignal([], message); }, - host.supportedFeatures.has("submit_signals_v2"), - ); + }); } public onNewUse(): void { diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index a867a1b28be1..daf7b43ced60 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -146,6 +146,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { private refreshBroadcastRequested = false; private readonly timer = new TimerManager(); private readonly workspaces = new Map>(); + private readonly targetedSignalSupport: boolean; public constructor( private readonly attendeeId: AttendeeId, @@ -156,11 +157,11 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { private readonly presence: Presence, systemWorkspaceDatastore: SystemWorkspaceDatastore, systemWorkspace: AnyWorkspaceEntry, - private readonly targetedSignalSupport: boolean, ) { // eslint-disable-next-line @typescript-eslint/consistent-type-assertions this.datastore = { "system:presence": systemWorkspaceDatastore } as PresenceDatastore; this.workspaces.set("system:presence", systemWorkspace); + this.targetedSignalSupport = this.runtime.supportedFeatures.has("submit_signals_v2"); } public joinSession(clientId: ClientConnectionId): void { @@ -389,7 +390,10 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } // If the message requests an acknowledgement, we will send a targeted acknowledgement message back to just the requestor. if (message.content.acknowledgementId !== undefined) { - assert(this.targetedSignalSupport, "Targeted signals not supported"); + assert( + this.targetedSignalSupport, + "Acknowledgment message was requested while targeted signal capability is not supported", + ); this.runtime.submitSignal({ type: acknowledgementMessageType, content: { id: message.content.acknowledgementId }, diff --git a/packages/framework/presence/src/presenceManager.ts b/packages/framework/presence/src/presenceManager.ts index 0328cc5b5c0d..b2b94d8ebaec 100644 --- a/packages/framework/presence/src/presenceManager.ts +++ b/packages/framework/presence/src/presenceManager.ts @@ -76,11 +76,7 @@ class PresenceManager implements Presence, PresenceExtensionInterface { private readonly mc: MonitoringContext | undefined = undefined; - public constructor( - runtime: IEphemeralRuntime, - attendeeId: AttendeeId, - targetedSignalSupport: boolean, - ) { + public constructor(runtime: IEphemeralRuntime, attendeeId: AttendeeId) { const logger = runtime.logger; if (logger) { this.mc = createChildMonitoringContext({ logger, namespace: "Presence" }); @@ -93,7 +89,6 @@ class PresenceManager implements Presence, PresenceExtensionInterface { this.events, this.mc?.logger, this, - targetedSignalSupport, ); this.attendees = this.systemWorkspace; @@ -165,7 +160,6 @@ function setupSubComponents( IEmitter, logger: ITelemetryLoggerExt | undefined, presence: Presence, - targetedSignalSupport: boolean, ): [PresenceDatastoreManager, SystemWorkspace] { const systemWorkspaceDatastore: SystemWorkspaceDatastore = { clientToSessionId: {}, @@ -185,7 +179,6 @@ function setupSubComponents( presence, systemWorkspaceDatastore, systemWorkspaceConfig.statesEntry, - targetedSignalSupport, ); return [datastoreManager, systemWorkspaceConfig.workspace]; } @@ -197,8 +190,7 @@ function setupSubComponents( */ export function createPresenceManager( runtime: IEphemeralRuntime, - targetedSignalSupport: boolean = false, attendeeId: AttendeeId = createSessionId() as AttendeeId, ): Presence & PresenceExtensionInterface { - return new PresenceManager(runtime, attendeeId, targetedSignalSupport); + return new PresenceManager(runtime, attendeeId); } diff --git a/packages/framework/presence/src/protocol.ts b/packages/framework/presence/src/protocol.ts index 0c61e150b385..9b31116560f8 100644 --- a/packages/framework/presence/src/protocol.ts +++ b/packages/framework/presence/src/protocol.ts @@ -42,7 +42,7 @@ interface DatastoreUpdateMessage { content: { sendTimestamp: number; avgLatency: number; - acknowledgementId?: number; + acknowledgementId?: AcknowledgmentIdType; isComplete?: true; data: DatastoreMessageContent; }; @@ -74,19 +74,21 @@ interface ClientJoinMessage { } /** - * @internal + * Acknowledgement message type. */ export const acknowledgementMessageType = "Pres:Ack"; interface AcknowledgementMessage { type: typeof acknowledgementMessageType; content: { - id: number; + id: AcknowledgmentIdType; }; } +type AcknowledgmentIdType = string; + /** - * @internal + * Outbound acknowledgement message. */ export type OutboundAcknowledgementMessage = OutboundExtensionMessage; @@ -100,6 +102,14 @@ export type OutboundClientJoinMessage = OutboundExtensionMessage; +/** + * Outbound presence message. + */ +export type OutboundPresenceMessage = + | OutboundAcknowledgementMessage + | OutboundClientJoinMessage + | OutboundDatastoreUpdateMessage; + /** * @internal */ diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index d6f5c926f845..5627f9fd39cc 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -477,7 +477,7 @@ describe("Presence", () => { runtime.signalsExpected.push([ { type: "Pres:Ack", - content: { id: 1 }, + content: { id: "ackID" }, targetClientId: "client4", }, ]); @@ -494,7 +494,7 @@ describe("Presence", () => { "system:presence": systemWorkspaceUpdate, "s:name:testStateWorkspace": statesWorkspaceUpdate, }, - acknowledgementId: 1, + acknowledgementId: "ackID", }, clientId: "client4", }, diff --git a/packages/framework/presence/src/test/testUtils.ts b/packages/framework/presence/src/test/testUtils.ts index 417ff9e06812..ab2cbc5271fe 100644 --- a/packages/framework/presence/src/test/testUtils.ts +++ b/packages/framework/presence/src/test/testUtils.ts @@ -155,11 +155,7 @@ export function prepareConnectedPresence( delete expectedClientJoin.clientId; runtime.signalsExpected.push([expectedClientJoin]); - const presence = createPresenceManager( - runtime, - true /* targetedSignalSupport */, - attendeeId as AttendeeId, - ); + const presence = createPresenceManager(runtime, attendeeId as AttendeeId); // Validate expectations post initialization to make sure logger // and runtime are left in a clean expectation state. diff --git a/packages/runtime/container-runtime-definitions/package.json b/packages/runtime/container-runtime-definitions/package.json index 1b7ac0be3cca..0f08da520d04 100644 --- a/packages/runtime/container-runtime-definitions/package.json +++ b/packages/runtime/container-runtime-definitions/package.json @@ -79,6 +79,7 @@ "typetests:prepare": "flub typetests --dir . --reset --previous --normalize" }, "dependencies": { + "@fluid-internal/client-utils": "workspace:~", "@fluidframework/container-definitions": "workspace:~", "@fluidframework/core-interfaces": "workspace:~", "@fluidframework/driver-definitions": "workspace:~", diff --git a/packages/runtime/container-runtime-definitions/src/containerExtension.ts b/packages/runtime/container-runtime-definitions/src/containerExtension.ts index 8fff85eaa160..81afd1d4f7b5 100644 --- a/packages/runtime/container-runtime-definitions/src/containerExtension.ts +++ b/packages/runtime/container-runtime-definitions/src/containerExtension.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. */ +import type { ILayerCompatDetails } from "@fluid-internal/client-utils"; import type { IAudience } from "@fluidframework/container-definitions/internal"; // eslint-disable-next-line @typescript-eslint/consistent-type-imports -- BrandedType is a class declaration only import type { @@ -22,6 +23,7 @@ import type { IQuorumClients } from "@fluidframework/driver-definitions/internal * @internal */ export type ClientConnectionId = string; + /** * Common structure between incoming and outgoing extension signals. * @@ -204,7 +206,7 @@ export interface ExtensionHost; + readonly supportedFeatures: ILayerCompatDetails["supportedFeatures"]; /** * Submits a signal to be sent to other clients. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0ed25eb74962..31830867029a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -12303,6 +12303,9 @@ importers: packages/runtime/container-runtime-definitions: dependencies: + '@fluid-internal/client-utils': + specifier: workspace:~ + version: link:../../common/client-utils '@fluidframework/container-definitions': specifier: workspace:~ version: link:../../common/container-definitions