From c207f58cce7d55a91025757c1479335f015c9020 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Tue, 15 Apr 2025 17:46:55 -0700 Subject: [PATCH 01/21] feat(client-container-runtime): path-based address signal routing For Signals, support using a `/` separated address in `IEnvelope` (at ContainerRuntime). No change to Ops. - Reading is always supported. - Writing (sending) for all signals requires `IContainerRuntimeOptionsInternal.enablePathBasedAddressing` to be enabled and is not externally exposed nor internally enabled apart from testing. Refactor signal submission so that application of `applyTrackingToBroadcastSignalEnvelope` is centralized and use `UnsequencedSignalEnvelope` to make `clientBroadcastSignalSequenceNumber` stamping (or lack of) more obvious. Add two helpers: - `submitWithPathBasedSignalAddress` will promote addressing to always use paths. - `submitAssertingLegacySignalAddressing` confirms envelopes coming through don't start with `/` that is reserved for path-based addressing. docs(client): changeset format update and additional comment. style(client): shorten option name improvement(client-contain-runtime): remove pathBasedAddressing from DocSchema impact --- .changeset/wet-towns-ask.md | 8 + .../container-runtime/src/compatUtils.ts | 3 + .../container-runtime/src/containerRuntime.ts | 183 +++++++++++++++--- .../src/test/containerRuntime.spec.ts | 12 +- .../test-service-load/src/optionsMatrix.ts | 1 + 5 files changed, 178 insertions(+), 29 deletions(-) create mode 100644 .changeset/wet-towns-ask.md diff --git a/.changeset/wet-towns-ask.md b/.changeset/wet-towns-ask.md new file mode 100644 index 000000000000..774483e07e8c --- /dev/null +++ b/.changeset/wet-towns-ask.md @@ -0,0 +1,8 @@ +--- +"@fluidframework/container-runtime": minor +"__section": feature +--- + +Introduce path based message routing + +Add ability for runtime to address messages with a `/` separated path scheme. `/runtime/` is reserved for runtime where `undefined` was previously used and data store messages are prefixed with `/channels/`. To enable general sending messages with this scheme, internal `IContainerRuntimeOptionsInternal.pathBasedAddressing` must be enabled. Internally `presence` requires this support under the `/ext/` path and thus should only be used once involved clients are using version 2.33 or later. diff --git a/packages/runtime/container-runtime/src/compatUtils.ts b/packages/runtime/container-runtime/src/compatUtils.ts index d8bd0abbc016..94bb017e75d3 100644 --- a/packages/runtime/container-runtime/src/compatUtils.ts +++ b/packages/runtime/container-runtime/src/compatUtils.ts @@ -85,6 +85,9 @@ export type RuntimeOptionsAffectingDocSchema = Omit< | "maxBatchSizeInBytes" | "loadSequenceNumberVerification" | "summaryOptions" + // This feature will eventually impact but it does not yet impact ops and should + // should never be enabled outside of experimentation. + | "pathBasedAddressing" >; /** diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 782e71ded758..e7c4914d36b8 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -451,6 +451,15 @@ export interface ContainerRuntimeOptionsInternal extends ContainerRuntimeOptions * In that case, batched messages will be sent individually (but still all at the same time). */ readonly enableGroupedBatching: boolean; + + /** + * When this property is enabled, runtime will use flexible address encoding + * pattern to route messages to the correct channel or extensible location. + * Minimum client runtime version is 2.33. + * Client runtimes older than 2.33 will log errors receiving a signal with + * path-based address and ultimately ignore the signal. + */ + readonly pathBasedAddressing: true | undefined; } /** @@ -661,6 +670,75 @@ export let getSingleUseLegacyLogCallback = (logger: ITelemetryLoggerExt, type: s }; }; +/** + * Address parts extracted from an address using the pathed address format. + */ +interface PathedAddressInfo { + /** + * First address in path - extracted from {@link PathedAddressInfo.fullAddress} + */ + top: string; + /** + * When true, it is considered an error if {@link PathedAddressInfo.top} is not found as a destination + */ + critical: boolean; + /** + * Address after {@link PathedAddressInfo.top} - extracted from {@link PathedAddressInfo.fullAddress} + */ + subaddress: string; + /** + * Full original address + */ + fullAddress: string; +} + +/** + * Mostly undefined address parts extracted from an address not using pathed address format. + */ +interface LegacyAddressInfo { + top: undefined; + critical: undefined; + subaddress: undefined; + fullAddress: string; +} + +/** + * Union of the two address info types. + */ +type NonContainerAddressInfo = PathedAddressInfo | LegacyAddressInfo; + +type UnsequencedSignalEnvelope = Omit; + +function submitWithPathBasedSignalAddress( + submitUnsequencedSignalFn: ( + contents: UnsequencedSignalEnvelope, + targetClientId?: string, + ) => void, +): (contents: UnsequencedSignalEnvelope, targetClientId?: string) => void { + return (envelope: UnsequencedSignalEnvelope, targetClientId?: string) => { + if (envelope.address === undefined) { + envelope.address = "/runtime"; + } else if (!envelope.address.startsWith("/")) { + envelope.address = `/channels/${envelope.address}`; + } + submitUnsequencedSignalFn(envelope, targetClientId); + }; +} + +function submitAssertingLegacySignalAddressing( + submitUnsequencedSignalFn: ( + contents: UnsequencedSignalEnvelope, + targetClientId?: string, + ) => void, +): (envelope: UnsequencedSignalEnvelope, targetClientId?: string) => void { + return (envelope: UnsequencedSignalEnvelope, targetClientId?: string) => { + if (envelope.address?.startsWith("/")) { + throw new Error("Path based addressing is not enabled"); + } + submitUnsequencedSignalFn(envelope, targetClientId); + }; +} + /** * This object holds the parameters necessary for the {@link loadContainerRuntime} function. * @legacy @@ -805,13 +883,15 @@ export class ContainerRuntime getCompatibilityVersionDefaults(compatibilityVersion); // The following are the default values for the options that do not affect the DocumentSchema. - const defaultConfigsNonVersionDependent: Required< - Omit + const defaultConfigsNonVersionDependent: Omit< + ContainerRuntimeOptionsInternal, + keyof RuntimeOptionsAffectingDocSchema > = { summaryOptions: {}, loadSequenceNumberVerification: "close", maxBatchSizeInBytes: defaultMaxBatchSizeInBytes, chunkSizeInBytes: defaultChunkSizeInBytes, + pathBasedAddressing: undefined, }; const defaultConfigs = { @@ -837,6 +917,7 @@ export class ContainerRuntime ? disabledCompressionConfig : defaultConfigs.compressionOptions, createBlobPayloadPending = defaultConfigs.createBlobPayloadPending, + pathBasedAddressing = defaultConfigs.pathBasedAddressing, }: IContainerRuntimeOptionsInternal = runtimeOptions; // The logic for enableRuntimeIdCompressor is a bit different. Since `undefined` represents a logical state (off) @@ -1047,6 +1128,7 @@ export class ContainerRuntime enableGroupedBatching, explicitSchemaControl, createBlobPayloadPending, + pathBasedAddressing, }; const runtime = new containerRuntimeCtor( @@ -1114,10 +1196,10 @@ export class ContainerRuntime summaryOp: ISummaryContent, referenceSequenceNumber?: number, ) => number; - /** - * Do not call directly - use submitAddressesSignal - */ - private readonly submitSignalFn: (content: ISignalEnvelope, targetClientId?: string) => void; + private readonly submitSignalFn: ( + content: UnsequencedSignalEnvelope, + targetClientId?: string, + ) => void; public readonly disposeFn: (error?: ICriticalContainerError) => void; public readonly closeFn: (error?: ICriticalContainerError) => void; @@ -1469,7 +1551,21 @@ export class ContainerRuntime this.submitSummaryFn = submitSummaryFn ?? ((summaryOp, refseq) => submitFn(MessageType.Summarize, summaryOp, false)); - this.submitSignalFn = submitSignalFn; + + const sequenceAndSubmitSignal = ( + envelope: UnsequencedSignalEnvelope, + targetClientId?: string, + ): void => { + if (targetClientId === undefined) { + this.signalTelemetryManager.applyTrackingToBroadcastSignalEnvelope(envelope); + } + submitSignalFn(envelope, targetClientId); + }; + this.submitSignalFn = ( + runtimeOptions.pathBasedAddressing + ? submitWithPathBasedSignalAddress + : submitAssertingLegacySignalAddressing + )(sequenceAndSubmitSignal); // TODO: After IContainerContext.options is removed, we'll just create a new blank object {} here. // Values are generally expected to be set from the runtime side. @@ -1728,9 +1824,6 @@ export class ContainerRuntime // verifyNotClosed is called in FluidDataStoreContext, which is *the* expected caller. const envelope1 = content as IEnvelope; const envelope2 = createNewSignalEnvelope(envelope1.address, type, envelope1.contents); - if (targetClientId === undefined) { - this.signalTelemetryManager.applyTrackingToBroadcastSignalEnvelope(envelope2); - } this.submitSignalFn(envelope2, targetClientId); }; @@ -3165,22 +3258,67 @@ export class ContainerRuntime ); } - if (envelope.address === undefined) { + const fullAddress = envelope.address; + if (fullAddress === undefined || fullAddress === "/runtime/") { // No address indicates a container signal message. this.emit("signal", transformed, local); return; } - // Due to a mismatch between different layers in terms of - // what is the interface of passing signals, we need to adjust - // the signal envelope before sending it to the datastores to be processed - const envelope2: IEnvelope = { - address: envelope.address, - contents: transformed.content, + // Note that this pattern allows for ! prefix in top-address but + // does not give it any more special treatment than without it. + const topAddressAndSubaddress = fullAddress.match( + /^\/(?[!?]?)(?[^/]*)\/(?.*)$/, + ); + const { optional, top, subaddress } = topAddressAndSubaddress?.groups ?? { + optional: undefined, + top: undefined, + subaddress: undefined, }; - transformed.content = envelope2; + this.routeNonContainerSignal( + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- cast required + { + top, + // Could just check !== "?", but would not correctly meet strict typing. + critical: optional === undefined ? undefined : optional !== "?", + subaddress, + fullAddress, + } as NonContainerAddressInfo, + transformed, + local, + ); + } + + private routeNonContainerSignal( + address: NonContainerAddressInfo, + signalMessage: IInboundSignalMessage, + local: boolean, + ): void { + // channelCollection signals are identified by no top address (use full address) or by the top address of "channels". + const isChannelAddress = address.top === undefined || address.top === "channels"; + if (isChannelAddress) { + // Due to a mismatch between different layers in terms of + // what is the interface of passing signals, we need to adjust + // the signal envelope before sending it to the datastores to be processed + const envelope: IEnvelope = { + address: address.subaddress ?? address.fullAddress, + contents: signalMessage.content, + }; + signalMessage.content = envelope; + + this.channelCollection.processSignal(signalMessage, local); + return; + } - this.channelCollection.processSignal(transformed, local); + if (address.critical) { + assert(!local, "No recipient found for critical local signal"); + this.mc.logger.sendTelemetryEvent({ + eventName: "SignalCriticalAddressNotFound", + ...tagCodeArtifacts({ + address: address.top, + }), + }); + } } /** @@ -3419,9 +3557,6 @@ export class ContainerRuntime public submitSignal(type: string, content: unknown, targetClientId?: string): void { this.verifyNotClosed(); const envelope = createNewSignalEnvelope(undefined /* address */, type, content); - if (targetClientId === undefined) { - this.signalTelemetryManager.applyTrackingToBroadcastSignalEnvelope(envelope); - } this.submitSignalFn(envelope, targetClientId); } @@ -4796,8 +4931,8 @@ export function createNewSignalEnvelope( address: string | undefined, type: string, content: unknown, -): Omit { - const newEnvelope: Omit = { +): UnsequencedSignalEnvelope { + const newEnvelope: UnsequencedSignalEnvelope = { address, contents: { type, content }, }; diff --git a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts index 620ad691adec..d86c578cbf35 100644 --- a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts +++ b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts @@ -81,6 +81,7 @@ import { IPendingRuntimeState, defaultPendingOpsWaitTimeoutMs, getSingleUseLegacyLogCallback, + type ContainerRuntimeOptionsInternal, type IContainerRuntimeOptionsInternal, } from "../containerRuntime.js"; import { @@ -1564,7 +1565,7 @@ describe("Runtime", () => { mockLogger = new MockLogger(); }); - const runtimeOptions: IContainerRuntimeOptionsInternal = { + const runtimeOptions = { compressionOptions: { minimumBatchSizeInBytes: 1024 * 1024, compressionAlgorithm: CompressionAlgorithms.lz4, @@ -1572,9 +1573,9 @@ describe("Runtime", () => { chunkSizeInBytes: 800 * 1024, flushMode: FlushModeExperimental.Async as unknown as FlushMode, enableGroupedBatching: true, - }; + } as const satisfies IContainerRuntimeOptionsInternal; - const defaultRuntimeOptions: IContainerRuntimeOptionsInternal = { + const defaultRuntimeOptions = { summaryOptions: {}, gcOptions: {}, loadSequenceNumberVerification: "close", @@ -1589,8 +1590,9 @@ describe("Runtime", () => { enableGroupedBatching: true, // Redundant, but makes the JSON.stringify yield the same result as the logs explicitSchemaControl: false, createBlobPayloadPending: false, // Redundant, but makes the JSON.stringify yield the same result as the logs - }; - const mergedRuntimeOptions = { ...defaultRuntimeOptions, ...runtimeOptions }; + pathBasedAddressing: undefined, + } as const satisfies ContainerRuntimeOptionsInternal; + const mergedRuntimeOptions = { ...defaultRuntimeOptions, ...runtimeOptions } as const; it("Container load stats", async () => { await ContainerRuntime.loadRuntime({ diff --git a/packages/test/test-service-load/src/optionsMatrix.ts b/packages/test/test-service-load/src/optionsMatrix.ts index bbd38d51b119..a0fc7d36138d 100644 --- a/packages/test/test-service-load/src/optionsMatrix.ts +++ b/packages/test/test-service-load/src/optionsMatrix.ts @@ -118,6 +118,7 @@ export function generateRuntimeOptions( enableGroupedBatching: [true, false], createBlobPayloadPending: [true, false], explicitSchemaControl: [true, false], + pathBasedAddressing: [true, undefined], }; const pairwiseOptions = generatePairwiseOptions( From 8b219d73397d2b5b0d3fc5d038947d9367c15d2b Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Sat, 12 Apr 2025 17:24:49 -0700 Subject: [PATCH 02/21] improvement(client): use generic for signal content and use to type ContainerRuntime signals received. --- .../core-interfaces.legacy.alpha.api.md | 6 +++ packages/common/core-interfaces/src/index.ts | 2 +- .../common/core-interfaces/src/messages.ts | 30 +++++++++++--- .../driver-definitions.legacy.alpha.api.md | 8 ++-- .../src/protocol/protocol.ts | 14 ++++--- .../container-runtime/src/containerRuntime.ts | 17 +++++--- .../src/test/containerRuntime.spec.ts | 40 +++++++++++-------- .../runtime-definitions.legacy.alpha.api.md | 4 +- .../runtime-definitions/src/protocol.ts | 6 ++- 9 files changed, 86 insertions(+), 41 deletions(-) diff --git a/packages/common/core-interfaces/api-report/core-interfaces.legacy.alpha.api.md b/packages/common/core-interfaces/api-report/core-interfaces.legacy.alpha.api.md index b89e27fbca7b..94e0eb2ab47f 100644 --- a/packages/common/core-interfaces/api-report/core-interfaces.legacy.alpha.api.md +++ b/packages/common/core-interfaces/api-report/core-interfaces.legacy.alpha.api.md @@ -407,6 +407,12 @@ export type TelemetryBaseEventPropertyType = string | number | boolean | undefin // @public export type TransformedEvent = (event: E, listener: (...args: ReplaceIEventThisPlaceHolder) => void) => TThis; +// @alpha @legacy +export interface TypedMessage { + content: unknown; + type: string; +} + // (No @packageDocumentation comment for this package) ``` diff --git a/packages/common/core-interfaces/src/index.ts b/packages/common/core-interfaces/src/index.ts index 0d6e8f70f55c..71d95915b0ce 100644 --- a/packages/common/core-interfaces/src/index.ts +++ b/packages/common/core-interfaces/src/index.ts @@ -47,7 +47,7 @@ export type { export { LogLevel } from "./logger.js"; export type { FluidObjectProviderKeys, FluidObject, FluidObjectKeys } from "./provider.js"; export type { ConfigTypes, IConfigProviderBase } from "./config.js"; -export type { ISignalEnvelope } from "./messages.js"; +export type { ISignalEnvelope, TypedMessage } from "./messages.js"; export type { ErasedType } from "./erasedType.js"; export type { diff --git a/packages/common/core-interfaces/src/messages.ts b/packages/common/core-interfaces/src/messages.ts index 92d7d2ad9360..c53c571e3d17 100644 --- a/packages/common/core-interfaces/src/messages.ts +++ b/packages/common/core-interfaces/src/messages.ts @@ -3,6 +3,28 @@ * Licensed under the MIT License. */ +/** + * A message that has a string `type` associated with `content`. + * + * @remarks + * This type is meant to be used indirectly. Most commonly as a constraint + * for generics of message structures. + * + * @legacy + * @alpha + */ +export interface TypedMessage { + /** + * The type of the message. + */ + type: string; + + /** + * The contents of the message. + */ + content: unknown; +} + /** * @internal * @@ -13,7 +35,7 @@ * * See at `server/routerlicious/packages/lambdas/src/utils/messageGenerator.ts`. */ -export interface ISignalEnvelope { +export interface ISignalEnvelope { /** * The target for the envelope, undefined for the container */ @@ -27,9 +49,5 @@ export interface ISignalEnvelope { /** * The contents of the envelope */ - contents: { - type: string; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - content: any; - }; + contents: TMessage; } diff --git a/packages/common/driver-definitions/api-report/driver-definitions.legacy.alpha.api.md b/packages/common/driver-definitions/api-report/driver-definitions.legacy.alpha.api.md index 8d038339eeed..b6fb41972185 100644 --- a/packages/common/driver-definitions/api-report/driver-definitions.legacy.alpha.api.md +++ b/packages/common/driver-definitions/api-report/driver-definitions.legacy.alpha.api.md @@ -468,17 +468,17 @@ export interface ISignalClient { } // @alpha @legacy -export interface ISignalMessage extends ISignalMessageBase { +export interface ISignalMessage extends ISignalMessageBase { clientId: string | null; } // @alpha @legacy -export interface ISignalMessageBase { +export interface ISignalMessageBase { clientConnectionNumber?: number; - content: unknown; + content: TMessage["content"]; referenceSequenceNumber?: number; targetClientId?: string; - type?: string; + type?: TMessage["type"]; } // @alpha @legacy diff --git a/packages/common/driver-definitions/src/protocol/protocol.ts b/packages/common/driver-definitions/src/protocol/protocol.ts index c82a7b5c973f..347cff22ce64 100644 --- a/packages/common/driver-definitions/src/protocol/protocol.ts +++ b/packages/common/driver-definitions/src/protocol/protocol.ts @@ -3,6 +3,8 @@ * Licensed under the MIT License. */ +import type { TypedMessage } from "@fluidframework/core-interfaces/internal"; + /** * @legacy * @alpha @@ -341,16 +343,16 @@ export interface ISequencedDocumentAugmentedMessage extends ISequencedDocumentMe * @legacy * @alpha */ -export interface ISignalMessageBase { +export interface ISignalMessageBase { /** * Signal content */ - content: unknown; + content: TMessage["content"]; /** * Signal type */ - type?: string; + type?: TMessage["type"]; /** * Counts the number of signals sent by the sending client. @@ -374,7 +376,8 @@ export interface ISignalMessageBase { * @legacy * @alpha */ -export interface ISignalMessage extends ISignalMessageBase { +export interface ISignalMessage + extends ISignalMessageBase { /** * The client ID that submitted the message. * For server generated messages the clientId will be null. @@ -387,7 +390,8 @@ export interface ISignalMessage extends ISignalMessageBase { * Interface for signals sent by clients to the server. * @internal */ -export type ISentSignalMessage = ISignalMessageBase; +export type ISentSignalMessage = + ISignalMessageBase; /** * @legacy diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index e7c4914d36b8..6e27c2dac99b 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -41,6 +41,7 @@ import type { IFluidHandleInternal, IProvideFluidHandleContext, ISignalEnvelope, + JsonDeserialized, } from "@fluidframework/core-interfaces/internal"; import { assert, @@ -3240,9 +3241,15 @@ export class ContainerRuntime } } - public processSignal(message: ISignalMessage, local: boolean): void { - const envelope = message.content as ISignalEnvelope; - const transformed: IInboundSignalMessage = { + public processSignal( + message: ISignalMessage<{ + type: string; + content: ISignalEnvelope<{ type: string; content: JsonDeserialized }>; + }>, + local: boolean, + ): void { + const envelope = message.content; + const transformed = { clientId: message.clientId, content: envelope.contents.content, type: envelope.contents.type, @@ -3291,7 +3298,7 @@ export class ContainerRuntime private routeNonContainerSignal( address: NonContainerAddressInfo, - signalMessage: IInboundSignalMessage, + signalMessage: IInboundSignalMessage<{ type: string; content: JsonDeserialized }>, local: boolean, ): void { // channelCollection signals are identified by no top address (use full address) or by the top address of "channels". @@ -3300,7 +3307,7 @@ export class ContainerRuntime // Due to a mismatch between different layers in terms of // what is the interface of passing signals, we need to adjust // the signal envelope before sending it to the datastores to be processed - const envelope: IEnvelope = { + const envelope = { address: address.subaddress ?? address.fullAddress, contents: signalMessage.content, }; diff --git a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts index d86c578cbf35..fe2ed7da1ee4 100644 --- a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts +++ b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts @@ -27,6 +27,7 @@ import { ISignalEnvelope, type IErrorBase, type ITelemetryBaseLogger, + type JsonDeserialized, } from "@fluidframework/core-interfaces/internal"; import { ISummaryTree } from "@fluidframework/driver-definitions"; import { @@ -134,12 +135,14 @@ const changeConnectionState = ( }; interface ISignalEnvelopeWithClientIds { - envelope: ISignalEnvelope; + envelope: ISignalEnvelope<{ type: string; content: JsonDeserialized }>; clientId: string; targetClientId?: string; } -function isSignalEnvelope(obj: unknown): obj is ISignalEnvelope { +function isSignalEnvelope( + obj: unknown, +): obj is ISignalEnvelope<{ type: string; content: JsonDeserialized }> { return ( typeof obj === "object" && obj !== null && @@ -1016,7 +1019,7 @@ describe("Runtime", () => { function patchRuntime( pendingStateManager: PendingStateManager, _maxReconnects: number | undefined = undefined, - ) { + ): void { // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment -- Modifying private properties const runtime = containerRuntime as any; // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access @@ -1025,7 +1028,6 @@ describe("Runtime", () => { runtime.channelCollection = getMockChannelCollection(); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment runtime.maxConsecutiveReconnects = _maxReconnects ?? runtime.maxConsecutiveReconnects; - return runtime as ContainerRuntime; } /** @@ -1960,7 +1962,10 @@ describe("Runtime", () => { * This always returns the same snapshot. Basically, when container runtime receives an ack for the * deleted snapshot and tries to fetch the latest snapshot, return the latest snapshot. */ - async getSnapshotTree(version?: IVersion, scenarioName?: string) { + async getSnapshotTree( + version?: IVersion, + scenarioName?: string, + ): Promise { assert.strictEqual( version, latestVersion, @@ -1975,7 +1980,7 @@ describe("Runtime", () => { count: number, scenarioName?: string, fetchSource?: FetchSource, - ) { + ): Promise { return [latestVersion]; } @@ -1983,7 +1988,10 @@ describe("Runtime", () => { * Validates that this is not called by container runtime with the deleted snapshot id even * though it received an ack for it. */ - async uploadSummaryWithContext(summary: ISummaryTree, context: ISummaryContext) { + async uploadSummaryWithContext( + summary: ISummaryTree, + context: ISummaryContext, + ): Promise { assert.notStrictEqual( context.ackHandle, deletedSnapshotId, @@ -1996,7 +2004,7 @@ describe("Runtime", () => { * Called by container runtime to read document attributes. Return the sequence number as 0 which * is lower than the deleted snapshot's reference sequence number. */ - async readBlob(id: string) { + async readBlob(id: string): Promise { assert.strictEqual(id, "attributesBlob", "Not implemented"); const attributes: IDocumentAttributes = { sequenceNumber: 0, @@ -2497,7 +2505,7 @@ describe("Runtime", () => { logger.clear(); }); - function createSnapshot(addMissingDatastore: boolean, setGroupId: boolean = true) { + function createSnapshot(addMissingDatastore: boolean, setGroupId: boolean = true): void { if (addMissingDatastore) { snapshotTree.trees[".channels"].trees.missingDataStore = { blobs: { ".component": "id" }, @@ -2848,7 +2856,7 @@ describe("Runtime", () => { logger.clear(); }); - function sendSignals(count: number) { + function sendSignals(count: number): void { for (let i = 0; i < count; i++) { containerRuntime.submitSignal("TestSignalType", `TestSignalContent ${i + 1}`); assert( @@ -2864,7 +2872,7 @@ describe("Runtime", () => { } } - function processSignals(signals: ISignalEnvelopeWithClientIds[], count: number) { + function processSignals(signals: ISignalEnvelopeWithClientIds[], count: number): void { const signalsToProcess = signals.splice(0, count); for (const signal of signalsToProcess) { if (signal.targetClientId === undefined) { @@ -2902,7 +2910,7 @@ describe("Runtime", () => { } } - function processWithNoTargetSupport(count: number) { + function processWithNoTargetSupport(count: number): void { const signalsToProcess = submittedSignals.splice(0, count); for (const signal of signalsToProcess) { for (const runtime of runtimes.values()) { @@ -2921,15 +2929,15 @@ describe("Runtime", () => { } } - function processSubmittedSignals(count: number) { + function processSubmittedSignals(count: number): void { processSignals(submittedSignals, count); } - function processDroppedSignals(count: number) { + function processDroppedSignals(count: number): void { processSignals(droppedSignals, count); } - function dropSignals(count: number) { + function dropSignals(count: number): void { const signalsToDrop = submittedSignals.splice(0, count); droppedSignals.push(...signalsToDrop); } @@ -3392,7 +3400,7 @@ describe("Runtime", () => { let remoteContainerRuntime: ContainerRuntime; let remoteLogger: MockLogger; - function sendRemoteSignals(count: number) { + function sendRemoteSignals(count: number): void { for (let i = 0; i < count; i++) { remoteContainerRuntime.submitSignal( "TestSignalType", diff --git a/packages/runtime/runtime-definitions/api-report/runtime-definitions.legacy.alpha.api.md b/packages/runtime/runtime-definitions/api-report/runtime-definitions.legacy.alpha.api.md index 3198a93dd497..04067edc5d92 100644 --- a/packages/runtime/runtime-definitions/api-report/runtime-definitions.legacy.alpha.api.md +++ b/packages/runtime/runtime-definitions/api-report/runtime-definitions.legacy.alpha.api.md @@ -244,9 +244,9 @@ export interface IGarbageCollectionDetailsBase { } // @alpha @legacy -export interface IInboundSignalMessage extends ISignalMessage { +export interface IInboundSignalMessage extends ISignalMessage { // (undocumented) - readonly type: string; + readonly type: TMessage["type"]; } // @alpha @legacy diff --git a/packages/runtime/runtime-definitions/src/protocol.ts b/packages/runtime/runtime-definitions/src/protocol.ts index 762aaf02c2a5..7168bbfc3b22 100644 --- a/packages/runtime/runtime-definitions/src/protocol.ts +++ b/packages/runtime/runtime-definitions/src/protocol.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. */ +import type { TypedMessage } from "@fluidframework/core-interfaces/internal"; import type { ITree, ISignalMessage, @@ -32,8 +33,9 @@ export interface IEnvelope { * @legacy * @alpha */ -export interface IInboundSignalMessage extends ISignalMessage { - readonly type: string; +export interface IInboundSignalMessage + extends ISignalMessage { + readonly type: TMessage["type"]; } /** From 1872d870cd422e7cf647bf391f6073be80e2696d Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Wed, 16 Apr 2025 12:37:11 -0700 Subject: [PATCH 03/21] feat(client): internal container extensions interface refactor `ContainerExtension` interface from `presence` with several updates to `ExtensionRuntime` and accommodating `presence` changes. --- .../src/containerExtension.ts} | 54 +++++++++---- .../common/container-definitions/src/index.ts | 11 +++ .../container-definitions/src/runtime.ts | 6 ++ packages/framework/presence/package.json | 4 - .../src/container-definitions/index.ts | 9 --- .../src/container-definitions/runtime.ts | 13 ---- .../src/datastorePresenceManagerFactory.ts | 27 +++++-- .../presence/src/experimentalAccess.ts | 31 ++++---- .../framework/presence/src/internalTypes.ts | 26 ++++--- .../presence/src/presenceDatastoreManager.ts | 25 +++--- .../framework/presence/src/presenceManager.ts | 28 +++---- .../presence/src/test/mockEphemeralRuntime.ts | 76 +++++++++---------- .../framework/presence/src/test/testUtils.ts | 6 +- 13 files changed, 176 insertions(+), 140 deletions(-) rename packages/{framework/presence/src/container-definitions/containerExtensions.ts => common/container-definitions/src/containerExtension.ts} (72%) delete mode 100644 packages/framework/presence/src/container-definitions/index.ts delete mode 100644 packages/framework/presence/src/container-definitions/runtime.ts diff --git a/packages/framework/presence/src/container-definitions/containerExtensions.ts b/packages/common/container-definitions/src/containerExtension.ts similarity index 72% rename from packages/framework/presence/src/container-definitions/containerExtensions.ts rename to packages/common/container-definitions/src/containerExtension.ts index a3114590db5a..f74b10888bff 100644 --- a/packages/framework/presence/src/container-definitions/containerExtensions.ts +++ b/packages/common/container-definitions/src/containerExtension.ts @@ -4,9 +4,14 @@ */ import type { + ITelemetryBaseLogger, JsonDeserialized, JsonSerializable, + Listenable, } from "@fluidframework/core-interfaces/internal"; +import type { IQuorumClients } from "@fluidframework/driver-definitions/internal"; + +import type { IAudience } from "./audience.js"; /** * While connected, the id of a client within a session. @@ -16,12 +21,12 @@ import type { export type ClientConnectionId = string; /** - * Common interface between incoming and outgoing extension signals. + * Common structure between incoming and outgoing extension signals. * * @sealed * @internal */ -export interface IExtensionMessage { +export interface ExtensionMessage { /** * Message type */ @@ -51,7 +56,7 @@ export interface IExtensionMessage { +export interface ContainerExtension { /** * Notifies the extension of a new use context. * @@ -66,21 +71,33 @@ export interface IContainerExtension { * @param signal - Signal content and metadata * @param local - True if signal was sent by this client */ - processSignal?(address: string, signal: IExtensionMessage, local: boolean): void; + processSignal?(address: string, signal: ExtensionMessage, local: boolean): void; +} + +/** + * Events emitted by the {@link ExtensionRuntime}. + * + * @internal + */ +export interface ExtensionRuntimeEvents { + "disconnected": () => void; + "connected": (clientId: ClientConnectionId) => void; } /** * Defines the runtime interface an extension may access. - * In most cases this is a subset of {@link @fluidframework/container-runtime-definitions#IContainerRuntime}. + * In most cases this is a logical subset of {@link @fluidframework/container-runtime-definitions#IContainerRuntime}. * * @sealed * @internal */ -export interface IExtensionRuntime { - /** - * {@inheritdoc @fluidframework/container-runtime-definitions#IContainerRuntime.clientId} - */ - get clientId(): ClientConnectionId | undefined; +export interface ExtensionRuntime { + readonly isConnected: () => boolean; + readonly getClientId: () => ClientConnectionId | undefined; + + readonly events: Listenable; + + readonly logger: ITelemetryBaseLogger; /** * Submits a signal to be sent to other clients. @@ -89,15 +106,22 @@ export interface IExtensionRuntime { * @param content - Custom content of the signal. Should be a JSON serializable object or primitive via {@link https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify|JSON.stringify}. * @param targetClientId - When specified, the signal is only sent to the provided client id. * - * Upon receipt of signal, {@link IContainerExtension.processSignal} will be called with the same + * Upon receipt of signal, {@link ContainerExtension.processSignal} will be called with the same * address, type, and content (less any non-{@link https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify|JSON.stringify}-able data). */ - submitAddressedSignal( + submitAddressedSignal: ( address: string, type: string, content: JsonSerializable, targetClientId?: ClientConnectionId, - ): void; + ) => void; + + /** + * The collection of write clients which were connected as of the current sequence number. + * Also contains a map of key-value pairs that must be agreed upon by all clients before being accepted. + */ + getQuorum: () => IQuorumClients; + getAudience: () => IAudience; } /** @@ -120,9 +144,9 @@ export interface IExtensionRuntime { * @internal */ export type ContainerExtensionFactory = new ( - runtime: IExtensionRuntime, + runtime: ExtensionRuntime, ...context: TContext -) => { readonly interface: T; readonly extension: IContainerExtension }; +) => { readonly interface: T; readonly extension: ContainerExtension }; /** * Unique identifier for extension diff --git a/packages/common/container-definitions/src/index.ts b/packages/common/container-definitions/src/index.ts index 6e587f42fa65..03330c591acd 100644 --- a/packages/common/container-definitions/src/index.ts +++ b/packages/common/container-definitions/src/index.ts @@ -15,6 +15,16 @@ export type { IFluidBrowserPackageEnvironment, } from "./browserPackage.js"; export { isFluidBrowserPackage } from "./browserPackage.js"; +export type { + ClientConnectionId, + ContainerExtensionFactory, + ContainerExtensionId, + ContainerExtensionStore, + ContainerExtension, + ExtensionMessage, + ExtensionRuntime, + ExtensionRuntimeEvents, +} from "./containerExtension.js"; export type { IConnectionDetails, IDeltaManager, @@ -64,6 +74,7 @@ export type { IContainerContext, IProvideRuntimeFactory, IRuntime, + IRuntimeInternal, IGetPendingLocalStateProps, } from "./runtime.js"; export { AttachState, IRuntimeFactory } from "./runtime.js"; diff --git a/packages/common/container-definitions/src/runtime.ts b/packages/common/container-definitions/src/runtime.ts index 4f036e5ea711..e343b887843b 100644 --- a/packages/common/container-definitions/src/runtime.ts +++ b/packages/common/container-definitions/src/runtime.ts @@ -25,6 +25,7 @@ import type { } from "@fluidframework/driver-definitions/internal"; import type { IAudience } from "./audience.js"; +import type { ContainerExtensionStore } from "./containerExtension.js"; import type { IDeltaManager } from "./deltas.js"; import type { ICriticalContainerError } from "./error.js"; import type { ILoader } from "./loader.js"; @@ -114,6 +115,11 @@ export interface IRuntime extends IDisposable { getEntryPoint(): Promise; } +/** + * @internal + */ +export interface IRuntimeInternal extends IRuntime, ContainerExtensionStore {} + /** * Payload type for IContainerContext.submitBatchFn() * @legacy diff --git a/packages/framework/presence/package.json b/packages/framework/presence/package.json index 86e2ca4a7815..dc93e5f045b3 100644 --- a/packages/framework/presence/package.json +++ b/packages/framework/presence/package.json @@ -22,10 +22,6 @@ "types": "./dist/alpha.d.ts", "default": "./dist/index.js" } - }, - "./internal/container-definitions/internal": { - "import": "./lib/container-definitions/index.js", - "require": "./dist/container-definitions/index.js" } }, "files": [ diff --git a/packages/framework/presence/src/container-definitions/index.ts b/packages/framework/presence/src/container-definitions/index.ts deleted file mode 100644 index 0a896ffee9f8..000000000000 --- a/packages/framework/presence/src/container-definitions/index.ts +++ /dev/null @@ -1,9 +0,0 @@ -/*! - * Copyright (c) Microsoft Corporation and contributors. All rights reserved. - * Licensed under the MIT License. - */ - -export type { IRuntimeInternal } from "./runtime.js"; - -// eslint-disable-next-line no-restricted-syntax -export type * from "./containerExtensions.js"; diff --git a/packages/framework/presence/src/container-definitions/runtime.ts b/packages/framework/presence/src/container-definitions/runtime.ts deleted file mode 100644 index 2157e5993520..000000000000 --- a/packages/framework/presence/src/container-definitions/runtime.ts +++ /dev/null @@ -1,13 +0,0 @@ -/*! - * Copyright (c) Microsoft Corporation and contributors. All rights reserved. - * Licensed under the MIT License. - */ - -import type { IRuntime } from "@fluidframework/container-definitions/internal"; - -import type { ContainerExtensionStore } from "./containerExtensions.js"; - -/** - * @internal - */ -export interface IRuntimeInternal extends IRuntime, ContainerExtensionStore {} diff --git a/packages/framework/presence/src/datastorePresenceManagerFactory.ts b/packages/framework/presence/src/datastorePresenceManagerFactory.ts index 8309199e3f56..40fb27ce7eaf 100644 --- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts +++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts @@ -7,6 +7,11 @@ * Hacky support for internal datastore based usages. */ +import { createEmitter } from "@fluid-internal/client-utils"; +import type { + ExtensionMessage, + ExtensionRuntimeEvents, +} from "@fluidframework/container-definitions/internal"; import type { IFluidLoadable } from "@fluidframework/core-interfaces"; import { assert } from "@fluidframework/core-utils/internal"; import type { IInboundSignalMessage } from "@fluidframework/runtime-definitions/internal"; @@ -16,14 +21,12 @@ import { BasicDataStoreFactory, LoadableFluidObject } from "./datastoreSupport.j import type { Presence } from "./presence.js"; import { createPresenceManager } from "./presenceManager.js"; -import type { IExtensionMessage } from "@fluidframework/presence/internal/container-definitions/internal"; - function assertSignalMessageIsValid( - message: IInboundSignalMessage | IExtensionMessage, -): asserts message is IExtensionMessage { + message: IInboundSignalMessage | ExtensionMessage, +): asserts message is ExtensionMessage { assert(message.clientId !== null, 0xa58 /* Signal must have a client ID */); // The other difference between messages is that `content` for - // IExtensionMessage is JsonDeserialized and we are fine assuming that. + // ExtensionMessage is JsonDeserialized and we are fine assuming that. } /** @@ -38,7 +41,19 @@ class PresenceManagerDataObject extends LoadableFluidObject { if (!this._presenceManager) { // TODO: investigate if ContainerExtensionStore (path-based address routing for // Signals) is readily detectable here and use that presence manager directly. - const manager = createPresenceManager(this.runtime); + const runtime = this.runtime; + const events = createEmitter(); + runtime.on("connected", (clientId) => events.emit("connected", clientId)); + runtime.on("disconnected", () => events.emit("disconnected")); + + const manager = createPresenceManager({ + isConnected: () => runtime.connected, + getClientId: () => runtime.clientId, + events, + getQuorum: runtime.getQuorum.bind(runtime), + getAudience: runtime.getAudience.bind(runtime), + submitSignal: runtime.submitSignal.bind(runtime), + }); this.runtime.on("signal", (message: IInboundSignalMessage, local: boolean) => { assertSignalMessageIsValid(message); manager.processSignal("", message, local); diff --git a/packages/framework/presence/src/experimentalAccess.ts b/packages/framework/presence/src/experimentalAccess.ts index e478dc50e8e0..cece68fcb016 100644 --- a/packages/framework/presence/src/experimentalAccess.ts +++ b/packages/framework/presence/src/experimentalAccess.ts @@ -3,24 +3,22 @@ * Licensed under the MIT License. */ +import type { + ContainerExtensionStore, + ContainerExtension, + ExtensionMessage, + ExtensionRuntime, +} from "@fluidframework/container-definitions/internal"; import type { IContainerExperimental } from "@fluidframework/container-loader/internal"; import { assert } from "@fluidframework/core-utils/internal"; import type { IFluidContainer } from "@fluidframework/fluid-static"; import { isInternalFluidContainer } from "@fluidframework/fluid-static/internal"; import type { IContainerRuntimeBase } from "@fluidframework/runtime-definitions/internal"; -import type { IEphemeralRuntime } from "./internalTypes.js"; import type { Presence } from "./presence.js"; import type { PresenceExtensionInterface } from "./presenceManager.js"; import { createPresenceManager } from "./presenceManager.js"; -import type { - ContainerExtensionStore, - IContainerExtension, - IExtensionMessage, - IExtensionRuntime, -} from "@fluidframework/presence/internal/container-definitions/internal"; - function isContainerExtensionStore( manager: ContainerExtensionStore | IContainerRuntimeBase | IContainerExperimental, ): manager is ContainerExtensionStore { @@ -30,16 +28,19 @@ function isContainerExtensionStore( /** * Common Presence manager for a container */ -class ContainerPresenceManager implements IContainerExtension { +class ContainerPresenceManager implements ContainerExtension { public readonly interface: Presence; public readonly extension = this; private readonly manager: PresenceExtensionInterface; - public constructor(runtime: IExtensionRuntime) { - // TODO create the appropriate ephemeral runtime (map address must be in submitSignal, etc.) - this.interface = this.manager = createPresenceManager( - runtime as unknown as IEphemeralRuntime, - ); + public constructor(runtime: ExtensionRuntime) { + this.interface = this.manager = createPresenceManager({ + ...runtime, + submitSignal: (type, content, targetClientId) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument -- eslint see content as `any` (IntelliSense claims `JsonSerialized` as expected) + runtime.submitAddressedSignal("", type, content, targetClientId); + }, + }); } public onNewContext(): void { @@ -48,7 +49,7 @@ class ContainerPresenceManager implements IContainerExtension { public static readonly extensionId = "dis:bb89f4c0-80fd-4f0c-8469-4f2848ee7f4a"; - public processSignal(address: string, message: IExtensionMessage, local: boolean): void { + public processSignal(address: string, message: ExtensionMessage, local: boolean): void { this.manager.processSignal(address, message, local); } } diff --git a/packages/framework/presence/src/internalTypes.ts b/packages/framework/presence/src/internalTypes.ts index 5af893f7695b..c89d6c36cc38 100644 --- a/packages/framework/presence/src/internalTypes.ts +++ b/packages/framework/presence/src/internalTypes.ts @@ -3,14 +3,12 @@ * Licensed under the MIT License. */ -import type { IContainerRuntime } from "@fluidframework/container-runtime-definitions/internal"; -import type { IFluidDataStoreRuntime } from "@fluidframework/datastore-definitions/internal"; +import type { ExtensionRuntime } from "@fluidframework/container-definitions/internal"; +import type { JsonSerializable } from "@fluidframework/core-interfaces/internal"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { AttendeeId, Attendee } from "./presence.js"; -import type { IRuntimeInternal } from "@fluidframework/presence/internal/container-definitions/internal"; - /** * @internal */ @@ -30,11 +28,21 @@ export interface ClientRecord & - Partial>; +export type IEphemeralRuntime = Omit & + // Apart from tests, there is always a logger. So this could be promoted to required. + Partial> & { + /** + * Submits the signal to be sent to other clients. + * @param type - Type of the signal. + * @param content - Content of the signal. Should be a JSON serializable object or primitive. + * @param targetClientId - When specified, the signal is only sent to the provided client id. + */ + submitSignal: ( + type: string, + content: JsonSerializable, + targetClientId?: string, + ) => void; + }; /** * @internal diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index bb64e27f9424..90e1e4a73358 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 { ExtensionMessage } from "@fluidframework/container-definitions/internal"; import type { IEmitter } from "@fluidframework/core-interfaces/internal"; import { assert } from "@fluidframework/core-utils/internal"; import type { IInboundSignalMessage } from "@fluidframework/runtime-definitions/internal"; @@ -35,8 +36,6 @@ import type { WorkspaceAddress, } from "./types.js"; -import type { IExtensionMessage } from "@fluidframework/presence/internal/container-definitions/internal"; - interface AnyWorkspaceEntry { public: AnyWorkspace; internal: PresenceStatesInternal; @@ -109,7 +108,7 @@ export interface PresenceDatastoreManager { internalWorkspaceAddress: `n:${WorkspaceAddress}`, requestedContent: TSchema, ): NotificationsWorkspace; - processSignal(message: IExtensionMessage, local: boolean): void; + processSignal(message: ExtensionMessage, local: boolean): void; } function mergeGeneralDatastoreMessageContent( @@ -212,7 +211,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { options: RuntimeLocalUpdateOptions, ): void => { // Check for connectivity before sending updates. - if (!this.runtime.connected) { + if (!this.runtime.isConnected()) { return; } @@ -304,14 +303,14 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } // Check for connectivity before sending updates. - if (!this.runtime.connected) { + if (!this.runtime.isConnected()) { // Clear the queued data since we're disconnected. We don't want messages // to queue infinitely while disconnected. this.queuedData = undefined; return; } - const clientConnectionId = this.runtime.clientId; + const clientConnectionId = this.runtime.getClientId(); assert(clientConnectionId !== undefined, 0xa59 /* Client connected without clientId */); const currentClientToSessionValueState = // When connected, `clientToSessionId` must always have current connection entry. @@ -350,12 +349,12 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } public processSignal( - // Note: IInboundSignalMessage is used here in place of IExtensionMessage - // as IExtensionMessage's strictly JSON `content` creates type compatibility + // Note: IInboundSignalMessage is used here in place of ExtensionMessage + // as ExtensionMessage's strictly JSON `content` creates type compatibility // issues with `AttendeeId` keys and really unknown value content. - // IExtensionMessage is a subset of IInboundSignalMessage so this is safe. + // ExtensionMessage is a subset of IInboundSignalMessage so this is safe. // Change types of DatastoreUpdateMessage | ClientJoinMessage to - // IExtensionMessage<> derivatives to see the issues. + // ExtensionMessage<> derivatives to see the issues. message: IInboundSignalMessage | DatastoreUpdateMessage | ClientJoinMessage, local: boolean, ): void { @@ -385,7 +384,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { // It is possible for some signals to come in while client is not connected due // to how work is scheduled. If we are not connected, we can't respond to the // join request. We will make our own Join request once we are connected. - if (this.runtime.connected) { + if (this.runtime.isConnected()) { this.prepareJoinResponse(message.content.updateProviders, message.clientId); } // It is okay to continue processing the contained updates even if we are not @@ -472,7 +471,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { // We must be connected to receive this message, so clientId should be defined. // If it isn't then, not really a problem; just won't be in provider or quorum list. // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const clientId = this.runtime.clientId!; + const clientId = this.runtime.getClientId()!; // const requestor = message.clientId; if (updateProviders.includes(clientId)) { // Send all current state to the new client @@ -513,7 +512,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { setTimeout(() => { // Make sure a broadcast is still needed and we are currently connected. // If not connected, nothing we can do. - if (this.refreshBroadcastRequested && this.runtime.connected) { + if (this.refreshBroadcastRequested && this.runtime.isConnected()) { this.broadcastAllKnownState(); this.logger?.sendTelemetryEvent({ eventName: "JoinResponse", diff --git a/packages/framework/presence/src/presenceManager.ts b/packages/framework/presence/src/presenceManager.ts index 980054084e36..fee6dc941df1 100644 --- a/packages/framework/presence/src/presenceManager.ts +++ b/packages/framework/presence/src/presenceManager.ts @@ -4,6 +4,10 @@ */ import { createEmitter } from "@fluid-internal/client-utils"; +import type { + ContainerExtension, + ExtensionMessage, +} from "@fluidframework/container-definitions/internal"; import type { IEmitter, Listenable } from "@fluidframework/core-interfaces/internal"; import { createSessionId } from "@fluidframework/id-compressor/internal"; import type { @@ -28,18 +32,13 @@ import type { WorkspaceAddress, } from "./types.js"; -import type { - IContainerExtension, - IExtensionMessage, -} from "@fluidframework/presence/internal/container-definitions/internal"; - /** - * Portion of the container extension requirements ({@link IContainerExtension}) that are delegated to presence manager. + * Portion of the container extension requirements ({@link ContainerExtension}) that are delegated to presence manager. * * @internal */ export type PresenceExtensionInterface = Required< - Pick, "processSignal"> + Pick, "processSignal"> >; /** @@ -87,11 +86,12 @@ class PresenceManager implements Presence, PresenceExtensionInterface { ); this.attendees = this.systemWorkspace; - runtime.on("connected", this.onConnect.bind(this)); + runtime.events.on("connected", this.onConnect.bind(this)); - runtime.on("disconnected", () => { - if (runtime.clientId !== undefined) { - this.removeClientConnectionId(runtime.clientId); + runtime.events.on("disconnected", () => { + const currentClientId = runtime.getClientId(); + if (currentClientId !== undefined) { + this.removeClientConnectionId(currentClientId); } }); @@ -103,8 +103,8 @@ class PresenceManager implements Presence, PresenceExtensionInterface { // delayed we expect that "connected" event has passed. // Note: In some manual testing, this does not appear to be enough to // always trigger an initial connect. - const clientId = runtime.clientId; - if (clientId !== undefined && runtime.connected) { + const clientId = runtime.getClientId(); + if (clientId !== undefined && runtime.isConnected()) { this.onConnect(clientId); } } @@ -124,7 +124,7 @@ class PresenceManager implements Presence, PresenceExtensionInterface { * @param message - Message to be processed * @param local - Whether the message originated locally (`true`) or remotely (`false`) */ - public processSignal(address: string, message: IExtensionMessage, local: boolean): void { + public processSignal(address: string, message: ExtensionMessage, local: boolean): void { this.datastoreManager.processSignal(message, local); } } diff --git a/packages/framework/presence/src/test/mockEphemeralRuntime.ts b/packages/framework/presence/src/test/mockEphemeralRuntime.ts index f966c14a04cc..0d5274671ada 100644 --- a/packages/framework/presence/src/test/mockEphemeralRuntime.ts +++ b/packages/framework/presence/src/test/mockEphemeralRuntime.ts @@ -67,6 +67,8 @@ function makeMockAudience(clients: ClientData[]): MockAudience { * Mock ephemeral runtime for testing */ export class MockEphemeralRuntime implements IEphemeralRuntime { + public clientId: string | undefined; + public connected: boolean = false; public logger?: ITelemetryBaseLogger; public readonly quorum: MockQuorumClients; public readonly audience: MockAudience; @@ -95,31 +97,36 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { /* count of write clients (in quorum) */ 6, ); this.quorum = makeMockQuorum(clientsData); - this.getQuorum = () => this.quorum; this.audience = makeMockAudience(clientsData); - this.getAudience = () => this.audience; - this.on = ( - event: string, - listener: (...args: any[]) => void, - // Events style eventing does not lend itself to union that - // IEphemeralRuntime is derived from, so we are using `any` here - // but meet the intent of the interface. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ): any => { - if (!this.isSupportedEvent(event)) { - throw new Error(`Event ${event} is not supported`); - } - // Switch to allowing a single listener as commented when - // implementation uses a single "connected" listener. - // if (this.listeners[event]) { - // throw new Error(`Event ${event} already has a listener`); - // } - // this.listeners[event] = listener; - if (this.listeners[event].length > 1) { - throw new Error(`Event ${event} already has multiple listeners`); - } - this.listeners[event].push(listener); - return this; + this.events = { + on: ( + event: string, + listener: (...args: any[]) => void, + // Events style eventing does not lend itself to union that + // IEphemeralRuntime is derived from, so we are using `any` here + // but meet the intent of the interface. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ): any => { + if (!this.isSupportedEvent(event)) { + throw new Error(`Event ${event} is not supported`); + } + // Switch to allowing a single listener as commented when + // implementation uses a single "connected" listener. + // if (this.listeners[event]) { + // throw new Error(`Event ${event} already has a listener`); + // } + // this.listeners[event] = listener; + if (this.listeners[event].length > 1) { + throw new Error(`Event ${event} already has multiple listeners`); + } + this.listeners[event].push(listener); + return this; + }, + off: ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ): any => { + throw new Error("IEphemeralRuntime.off method not implemented."); + }, }; } @@ -162,24 +169,15 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { // #region IEphemeralRuntime - public clientId: string | undefined; - public connected: boolean = false; - - public on: IEphemeralRuntime["on"]; - - public off: IEphemeralRuntime["off"] = ( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ): any => { - throw new Error("IEphemeralRuntime.off method not implemented."); - }; + public isConnected = (): ReturnType => this.connected; + public getClientId = (): ReturnType => this.clientId; - public getAudience: () => ReturnType; + public events: IEphemeralRuntime["events"]; - public getQuorum: () => ReturnType; + public getQuorum: () => ReturnType = () => this.quorum; + public getAudience: () => ReturnType = () => this.audience; - public submitSignal: IEphemeralRuntime["submitSignal"] = ( - ...args: Parameters - ) => { + public submitSignal: IEphemeralRuntime["submitSignal"] = (...args: unknown[]) => { if (this.signalsExpected.length === 0) { throw new Error(`Unexpected signal: ${JSON.stringify(args)}`); } diff --git a/packages/framework/presence/src/test/testUtils.ts b/packages/framework/presence/src/test/testUtils.ts index ec1112bcb838..3a6a637d8514 100644 --- a/packages/framework/presence/src/test/testUtils.ts +++ b/packages/framework/presence/src/test/testUtils.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. */ +import type { ExtensionMessage } from "@fluidframework/container-definitions/internal"; import type { InternalUtilityTypes } from "@fluidframework/core-interfaces/internal"; import type { EventAndErrorTrackingLogger } from "@fluidframework/test-utils/internal"; import { getUnexpectedLogErrorException } from "@fluidframework/test-utils/internal"; @@ -12,8 +13,7 @@ import { createPresenceManager } from "../presenceManager.js"; import type { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; -import type { ClientConnectionId, AttendeeId } from "@fluidframework/presence/alpha"; -import type { IExtensionMessage } from "@fluidframework/presence/internal/container-definitions/internal"; +import type { AttendeeId, ClientConnectionId } from "@fluidframework/presence/alpha"; /** * Use to compile-time assert types of two variables are identical. @@ -77,7 +77,7 @@ export function generateBasicClientJoin( updateProviders, }, clientId: clientConnectionId, - } satisfies IExtensionMessage<"Pres:ClientJoin">; + } satisfies ExtensionMessage<"Pres:ClientJoin">; } /** From 62d3fa63201240c012ff9d436542e3bea177aea5 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 17 Apr 2025 00:28:44 -0700 Subject: [PATCH 04/21] improvement(client): enhance and tighten types for container extension messages across submitting and processing signals. + Improve incoming signal validation: - Check for exactly known type names - Allow for "optional" unrecognized signal; if a signal is not optional it must be recognized to avoid assert (throw) + Clean up test data types and content to conform --- .../src/containerExtension.ts | 132 +++- .../common/container-definitions/src/index.ts | 3 + .../src/exposedInternalUtilityTypes.ts | 21 +- .../common/core-interfaces/src/internal.ts | 2 + .../src/datastorePresenceManagerFactory.ts | 26 +- .../presence/src/experimentalAccess.ts | 20 +- .../framework/presence/src/internalTypes.ts | 25 +- .../framework/presence/src/internalUtils.ts | 15 +- .../presence/src/presenceDatastoreManager.ts | 110 ++- .../framework/presence/src/presenceManager.ts | 26 +- packages/framework/presence/src/protocol.ts | 87 +++ .../presence/src/test/batching.spec.ts | 723 +++++++++--------- .../presence/src/test/eventing.spec.ts | 22 +- .../presence/src/test/mockEphemeralRuntime.ts | 4 +- .../src/test/notificationsManager.spec.ts | 99 +-- .../src/test/presenceDatastoreManager.spec.ts | 96 ++- .../presence/src/test/presenceManager.spec.ts | 13 +- .../framework/presence/src/test/testUtils.ts | 64 +- 18 files changed, 896 insertions(+), 592 deletions(-) create mode 100644 packages/framework/presence/src/protocol.ts diff --git a/packages/common/container-definitions/src/containerExtension.ts b/packages/common/container-definitions/src/containerExtension.ts index f74b10888bff..a98f1dc64a31 100644 --- a/packages/common/container-definitions/src/containerExtension.ts +++ b/packages/common/container-definitions/src/containerExtension.ts @@ -4,10 +4,12 @@ */ import type { + InternalUtilityTypes, ITelemetryBaseLogger, JsonDeserialized, JsonSerializable, Listenable, + TypedMessage, } from "@fluidframework/core-interfaces/internal"; import type { IQuorumClients } from "@fluidframework/driver-definitions/internal"; @@ -23,32 +25,68 @@ export type ClientConnectionId = string; /** * Common structure between incoming and outgoing extension signals. * + * @remarks + * Do not use directly, use {@link OutboundExtensionMessage} or {@link InboundExtensionMessage} instead. + * * @sealed * @internal */ -export interface ExtensionMessage { - /** - * Message type - */ - type: TType; +export type ExtensionMessage< + TMessage extends TypedMessage = { + type: string; + content: JsonSerializable | JsonDeserialized; + }, +> = // `TMessage extends TypedMessage` encourages processing union elements individually + TMessage extends TypedMessage + ? InternalUtilityTypes.FlattenIntersection< + TMessage & { + /** + * Client ID of the singular client the message is being (or has been) sent to. + * May only be specified when IConnect.supportedFeatures['submit_signals_v2'] is true, will throw otherwise. + */ + targetClientId?: ClientConnectionId; + } + > + : never; - /** - * Message content - */ - content: JsonDeserialized; +/** + * Outgoing extension signals. + * + * @sealed + * @internal + */ +export type OutboundExtensionMessage = + ExtensionMessage<{ type: TMessage["type"]; content: JsonSerializable }>; - /** - * The client ID that submitted the message. - * For server generated messages the clientId will be null. - */ - // eslint-disable-next-line @rushstack/no-new-null - clientId: ClientConnectionId | null; +/** + * Incoming extension signals. + * + * @sealed + * @internal + */ +export type InboundExtensionMessage = + // `TMessage extends TypedMessage` encourages processing union elements individually + TMessage extends TypedMessage + ? InternalUtilityTypes.FlattenIntersection< + ExtensionMessage<{ + type: TMessage["type"]; + content: JsonDeserialized; + }> & { + /** + * The client ID that submitted the message. + * For server generated messages the clientId will be null. + */ + // eslint-disable-next-line @rushstack/no-new-null + clientId: ClientConnectionId | null; + } + > + : never; - /** - * Client ID of the singular client the message is being (or has been) sent to. - * May only be specified when IConnect.supportedFeatures['submit_signals_v2'] is true, will throw otherwise. - */ - targetClientId?: ClientConnectionId; +/** + * @internal + */ +export interface ExtensionRuntimeProperties { + SignalMessages: TypedMessage; } /** @@ -56,22 +94,29 @@ export interface ExtensionMessage { +export interface ContainerExtension< + TUseContext extends unknown[], + TRuntimeProperties extends ExtensionRuntimeProperties, +> { /** * Notifies the extension of a new use context. * * @param context - Context new reference to extension is acquired within */ - onNewContext(...context: TContext): void; + onNewContext(...context: TUseContext): void; /** * Callback for signal sent by this extension. * * @param address - Address of the signal - * @param signal - Signal content and metadata + * @param signalMessage - Unvalidated signal content and metadata * @param local - True if signal was sent by this client */ - processSignal?(address: string, signal: ExtensionMessage, local: boolean): void; + processSignal?: ( + address: string, + signalMessage: InboundExtensionMessage, + local: boolean, + ) => void; } /** @@ -91,7 +136,7 @@ export interface ExtensionRuntimeEvents { * @sealed * @internal */ -export interface ExtensionRuntime { +export interface ExtensionRuntime { readonly isConnected: () => boolean; readonly getClientId: () => ClientConnectionId | undefined; @@ -102,18 +147,14 @@ export interface ExtensionRuntime { /** * Submits a signal to be sent to other clients. * @param address - Custom address for the signal. - * @param type - Custom type of the signal. - * @param content - Custom content of the signal. Should be a JSON serializable object or primitive via {@link https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify|JSON.stringify}. - * @param targetClientId - When specified, the signal is only sent to the provided client id. + * @param message - Custom message content of the signal. * * Upon receipt of signal, {@link ContainerExtension.processSignal} will be called with the same - * address, type, and content (less any non-{@link https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify|JSON.stringify}-able data). + * address and message (less any non-{@link https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify|JSON.stringify}-able data). */ - submitAddressedSignal: ( + submitAddressedSignal: ( address: string, - type: string, - content: JsonSerializable, - targetClientId?: ClientConnectionId, + message: OutboundExtensionMessage, ) => void; /** @@ -143,10 +184,17 @@ export interface ExtensionRuntime { * * @internal */ -export type ContainerExtensionFactory = new ( - runtime: ExtensionRuntime, - ...context: TContext -) => { readonly interface: T; readonly extension: ContainerExtension }; +export type ContainerExtensionFactory< + T, + TUseContext extends unknown[], + TRuntimeProperties extends ExtensionRuntimeProperties, +> = new ( + runtime: ExtensionRuntime, + ...context: TUseContext +) => { + readonly interface: T; + readonly extension: ContainerExtension; +}; /** * Unique identifier for extension @@ -178,9 +226,13 @@ export interface ContainerExtensionStore { * @param factory - Factory to create the extension if not found * @returns The extension */ - acquireExtension( + acquireExtension< + T, + TUseContext extends unknown[], + TRuntimeProperties extends ExtensionRuntimeProperties, + >( id: ContainerExtensionId, - factory: ContainerExtensionFactory, - ...context: TContext + factory: ContainerExtensionFactory, + ...context: TUseContext ): T; } diff --git a/packages/common/container-definitions/src/index.ts b/packages/common/container-definitions/src/index.ts index 03330c591acd..77eeca44e849 100644 --- a/packages/common/container-definitions/src/index.ts +++ b/packages/common/container-definitions/src/index.ts @@ -24,6 +24,9 @@ export type { ExtensionMessage, ExtensionRuntime, ExtensionRuntimeEvents, + ExtensionRuntimeProperties, + InboundExtensionMessage, + OutboundExtensionMessage, } from "./containerExtension.js"; export type { IConnectionDetails, diff --git a/packages/common/core-interfaces/src/exposedInternalUtilityTypes.ts b/packages/common/core-interfaces/src/exposedInternalUtilityTypes.ts index 4fd2c790c097..491e80084834 100644 --- a/packages/common/core-interfaces/src/exposedInternalUtilityTypes.ts +++ b/packages/common/core-interfaces/src/exposedInternalUtilityTypes.ts @@ -535,20 +535,27 @@ export namespace InternalUtilityTypes { */ export type IsExactlyObject = IsSameType; + /** + * Any Record type. + * + * @system + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- `any` for property types is required to avoid "Index signature for type 'string' is missing in type" in some outside `FlattenIntersection` uses. + export type AnyRecord = Record; + /** * Creates a simple object type from an intersection of multiple. * @privateRemarks - * `T extends Record` within the implementation encourages tsc to process + * `T extends AnyRecord` within the implementation encourages tsc to process * intersections within unions. * * @system */ - export type FlattenIntersection> = - T extends Record - ? { - [K in keyof T]: T[K]; - } - : T; + export type FlattenIntersection = T extends AnyRecord + ? { + [K in keyof T]: T[K]; + } + : T; /** * Extracts Function portion from an intersection (&) type returning diff --git a/packages/common/core-interfaces/src/internal.ts b/packages/common/core-interfaces/src/internal.ts index 35090d64c0eb..6294fd1649c9 100644 --- a/packages/common/core-interfaces/src/internal.ts +++ b/packages/common/core-interfaces/src/internal.ts @@ -77,6 +77,8 @@ export type ReadonlyNonNullJsonObjectWith = ExposedReadonlyNonNullJsonObjectW // eslint-disable-next-line @typescript-eslint/no-namespace export namespace InternalUtilityTypes { /* eslint-disable jsdoc/require-jsdoc */ + export type FlattenIntersection = + ExposedInternalUtilityTypes.FlattenIntersection; export type IfSameType< X, Y, diff --git a/packages/framework/presence/src/datastorePresenceManagerFactory.ts b/packages/framework/presence/src/datastorePresenceManagerFactory.ts index 40fb27ce7eaf..fa72c1e1679c 100644 --- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts +++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts @@ -8,10 +8,7 @@ */ import { createEmitter } from "@fluid-internal/client-utils"; -import type { - ExtensionMessage, - ExtensionRuntimeEvents, -} from "@fluidframework/container-definitions/internal"; +import type { ExtensionRuntimeEvents } from "@fluidframework/container-definitions/internal"; import type { IFluidLoadable } from "@fluidframework/core-interfaces"; import { assert } from "@fluidframework/core-utils/internal"; import type { IInboundSignalMessage } from "@fluidframework/runtime-definitions/internal"; @@ -20,13 +17,25 @@ import type { SharedObjectKind } from "@fluidframework/shared-object-base"; import { BasicDataStoreFactory, LoadableFluidObject } from "./datastoreSupport.js"; import type { Presence } from "./presence.js"; import { createPresenceManager } from "./presenceManager.js"; +import type { + InboundClientJoinMessage, + InboundDatastoreUpdateMessage, + OutboundClientJoinMessage, + OutboundDatastoreUpdateMessage, +} from "./protocol.js"; +/** + * This provides faux validation of the signal message. + * This is needed while {@link InboundExtensionMessage} is fully typed even + * though not validated as used by {@link ContainerExtension.processSignal}. + * The preferred assertion that `message is InboundExtensionMessage`. + */ function assertSignalMessageIsValid( - message: IInboundSignalMessage | ExtensionMessage, -): asserts message is ExtensionMessage { + message: IInboundSignalMessage | InboundClientJoinMessage | InboundDatastoreUpdateMessage, +): asserts message is InboundClientJoinMessage | InboundDatastoreUpdateMessage { assert(message.clientId !== null, 0xa58 /* Signal must have a client ID */); // The other difference between messages is that `content` for - // ExtensionMessage is JsonDeserialized and we are fine assuming that. + // InboundExtensionMessage is JsonDeserialized and we are fine assuming that. } /** @@ -52,7 +61,8 @@ class PresenceManagerDataObject extends LoadableFluidObject { events, getQuorum: runtime.getQuorum.bind(runtime), getAudience: runtime.getAudience.bind(runtime), - submitSignal: runtime.submitSignal.bind(runtime), + submitSignal: (message: OutboundClientJoinMessage | OutboundDatastoreUpdateMessage) => + 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/experimentalAccess.ts b/packages/framework/presence/src/experimentalAccess.ts index cece68fcb016..49deefcfc991 100644 --- a/packages/framework/presence/src/experimentalAccess.ts +++ b/packages/framework/presence/src/experimentalAccess.ts @@ -6,8 +6,7 @@ import type { ContainerExtensionStore, ContainerExtension, - ExtensionMessage, - ExtensionRuntime, + InboundExtensionMessage, } from "@fluidframework/container-definitions/internal"; import type { IContainerExperimental } from "@fluidframework/container-loader/internal"; import { assert } from "@fluidframework/core-utils/internal"; @@ -15,9 +14,11 @@ import type { IFluidContainer } from "@fluidframework/fluid-static"; import { isInternalFluidContainer } from "@fluidframework/fluid-static/internal"; import type { IContainerRuntimeBase } from "@fluidframework/runtime-definitions/internal"; +import type { ExtensionRuntime, ExtensionRuntimeProperties } from "./internalTypes.js"; import type { Presence } from "./presence.js"; import type { PresenceExtensionInterface } from "./presenceManager.js"; import { createPresenceManager } from "./presenceManager.js"; +import type { SignalMessages } from "./protocol.js"; function isContainerExtensionStore( manager: ContainerExtensionStore | IContainerRuntimeBase | IContainerExperimental, @@ -28,7 +29,9 @@ function isContainerExtensionStore( /** * Common Presence manager for a container */ -class ContainerPresenceManager implements ContainerExtension { +class ContainerPresenceManager + implements ContainerExtension +{ public readonly interface: Presence; public readonly extension = this; private readonly manager: PresenceExtensionInterface; @@ -36,9 +39,8 @@ class ContainerPresenceManager implements ContainerExtension { public constructor(runtime: ExtensionRuntime) { this.interface = this.manager = createPresenceManager({ ...runtime, - submitSignal: (type, content, targetClientId) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument -- eslint see content as `any` (IntelliSense claims `JsonSerialized` as expected) - runtime.submitAddressedSignal("", type, content, targetClientId); + submitSignal: (message) => { + runtime.submitAddressedSignal("", message); }, }); } @@ -49,7 +51,11 @@ class ContainerPresenceManager implements ContainerExtension { public static readonly extensionId = "dis:bb89f4c0-80fd-4f0c-8469-4f2848ee7f4a"; - public processSignal(address: string, message: ExtensionMessage, local: boolean): void { + public processSignal( + address: string, + message: InboundExtensionMessage, + local: boolean, + ): void { this.manager.processSignal(address, message, local); } } diff --git a/packages/framework/presence/src/internalTypes.ts b/packages/framework/presence/src/internalTypes.ts index c89d6c36cc38..f23290831124 100644 --- a/packages/framework/presence/src/internalTypes.ts +++ b/packages/framework/presence/src/internalTypes.ts @@ -3,11 +3,26 @@ * Licensed under the MIT License. */ -import type { ExtensionRuntime } from "@fluidframework/container-definitions/internal"; -import type { JsonSerializable } from "@fluidframework/core-interfaces/internal"; +import type { ExtensionRuntime as ContainerExtensionRuntime } from "@fluidframework/container-definitions/internal"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { AttendeeId, Attendee } from "./presence.js"; +import type { + OutboundClientJoinMessage, + OutboundDatastoreUpdateMessage, + SignalMessages, +} from "./protocol.js"; + +/** + * @internal + */ +export interface ExtensionRuntimeProperties { + SignalMessages: SignalMessages; +} +/** + * @internal + */ +export type ExtensionRuntime = ContainerExtensionRuntime; /** * @internal @@ -37,10 +52,8 @@ export type IEphemeralRuntime = Omit( - type: string, - content: JsonSerializable, - targetClientId?: string, + submitSignal: ( + message: OutboundClientJoinMessage | OutboundDatastoreUpdateMessage, ) => void; }; diff --git a/packages/framework/presence/src/internalUtils.ts b/packages/framework/presence/src/internalUtils.ts index dc52e39c563d..8a078191adb0 100644 --- a/packages/framework/presence/src/internalUtils.ts +++ b/packages/framework/presence/src/internalUtils.ts @@ -25,9 +25,14 @@ type RequiredAndNotUndefined = { /** * Object.entries retyped to preserve known keys and their types. * + * @privateRemarks + * The is a defect in this utility when a string index appears in the object. + * In such a case, the only result is `[string, T]`, where `T` is the type + * of the string index entry. + * * @internal */ -export const objectEntries = Object.entries as (o: T) => KeyValuePairs; +export const objectEntries = Object.entries as (o: T) => KeyValuePairs; /** * Object.entries retyped to preserve known keys and their types. @@ -39,7 +44,7 @@ export const objectEntries = Object.entries as (o: T) => KeyValuePairs; * * @internal */ -export const objectEntriesWithoutUndefined = Object.entries as ( +export const objectEntriesWithoutUndefined = Object.entries as ( o: T, ) => KeyValuePairs>; @@ -48,7 +53,9 @@ export const objectEntriesWithoutUndefined = Object.entries as ( * * @internal */ -export const objectKeys = Object.keys as (o: T) => (keyof MapNumberIndicesToStrings)[]; +export const objectKeys = Object.keys as ( + o: T, +) => (keyof MapNumberIndicesToStrings)[]; /** * Retrieve a value from a record with the given key, or create a new entry if @@ -62,7 +69,7 @@ export const objectKeys = Object.keys as (o: T) => (keyof MapNumberIndicesToS * @returns either the existing value for the given key, or the newly-created * value (the result of `defaultValue`) */ -export function getOrCreateRecord( +export function getOrCreateRecord( record: Record, key: K, defaultValue: (key: K) => V, diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index 90e1e4a73358..dae495636025 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -3,10 +3,9 @@ * Licensed under the MIT License. */ -import type { ExtensionMessage } from "@fluidframework/container-definitions/internal"; +import type { InboundExtensionMessage } from "@fluidframework/container-definitions/internal"; import type { 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"; import type { ClientConnectionId } from "./baseTypes.js"; @@ -25,6 +24,14 @@ import { mergeUntrackedDatastore, mergeValueDirectory, } from "./presenceStates.js"; +import type { + GeneralDatastoreMessageContent, + InboundClientJoinMessage, + InboundDatastoreUpdateMessage, + OutboundDatastoreUpdateMessage, + SystemDatastore, +} from "./protocol.js"; +import { datastoreUpdateMessageType, joinMessageType } from "./protocol.js"; import type { SystemWorkspaceDatastore } from "./systemWorkspace.js"; import { TimerManager } from "./timerManager.js"; import type { @@ -41,59 +48,24 @@ interface AnyWorkspaceEntry { internal: PresenceStatesInternal; } -interface SystemDatastore { - "system:presence": SystemWorkspaceDatastore; -} - -type InternalWorkspaceAddress = `${"s" | "n"}:${WorkspaceAddress}`; - type PresenceDatastore = SystemDatastore & { [WorkspaceAddress: string]: ValueElementMap; }; -interface GeneralDatastoreMessageContent { - [WorkspaceAddress: string]: { - [StateValueManagerKey: string]: { - [AttendeeId: AttendeeId]: ClientUpdateEntry; - }; - }; -} - -type DatastoreMessageContent = SystemDatastore & GeneralDatastoreMessageContent; - -const datastoreUpdateMessageType = "Pres:DatastoreUpdate"; +type InternalWorkspaceAddress = `${"s" | "n"}:${WorkspaceAddress}`; const internalWorkspaceTypes: Readonly> = { s: "States", n: "Notifications", } as const; -interface DatastoreUpdateMessage extends IInboundSignalMessage { - type: typeof datastoreUpdateMessageType; - content: { - sendTimestamp: number; - avgLatency: number; - isComplete?: true; - data: DatastoreMessageContent; - }; -} - -const joinMessageType = "Pres:ClientJoin"; -interface ClientJoinMessage extends IInboundSignalMessage { - type: typeof joinMessageType; - content: { - updateProviders: ClientConnectionId[]; - sendTimestamp: number; - avgLatency: number; - data: DatastoreMessageContent; - }; -} - +const knownMessageTypes = new Set([joinMessageType, datastoreUpdateMessageType]); function isPresenceMessage( - message: IInboundSignalMessage, -): message is DatastoreUpdateMessage | ClientJoinMessage { - return message.type.startsWith("Pres:"); + message: InboundExtensionMessage, +): message is InboundDatastoreUpdateMessage | InboundClientJoinMessage { + return knownMessageTypes.has(message.type); } + /** * @internal */ @@ -108,7 +80,7 @@ export interface PresenceDatastoreManager { internalWorkspaceAddress: `n:${WorkspaceAddress}`, requestedContent: TSchema, ): NotificationsWorkspace; - processSignal(message: ExtensionMessage, local: boolean): void; + processSignal(message: InboundExtensionMessage, local: boolean, optional: boolean): void; } function mergeGeneralDatastoreMessageContent( @@ -182,12 +154,15 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { if (updateProviders.length > 3) { updateProviders.length = 3; } - this.runtime.submitSignal(joinMessageType, { - sendTimestamp: Date.now(), - avgLatency: this.averageLatency, - data: this.datastore, - updateProviders, - } satisfies ClientJoinMessage["content"]); + this.runtime.submitSignal({ + type: joinMessageType, + content: { + sendTimestamp: Date.now(), + avgLatency: this.averageLatency, + data: this.datastore, + updateProviders, + }, + }); } public getWorkspace( @@ -333,34 +308,33 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { }, ...this.queuedData, }, - } satisfies DatastoreUpdateMessage["content"]; + } satisfies OutboundDatastoreUpdateMessage["content"]; this.queuedData = undefined; - this.runtime.submitSignal(datastoreUpdateMessageType, newMessage); + this.runtime.submitSignal({ type: datastoreUpdateMessageType, content: newMessage }); } private broadcastAllKnownState(): void { - this.runtime.submitSignal(datastoreUpdateMessageType, { - sendTimestamp: Date.now(), - avgLatency: this.averageLatency, - isComplete: true, - data: this.datastore, - } satisfies DatastoreUpdateMessage["content"]); + this.runtime.submitSignal({ + type: datastoreUpdateMessageType, + content: { + sendTimestamp: Date.now(), + avgLatency: this.averageLatency, + isComplete: true, + data: this.datastore, + }, + }); this.refreshBroadcastRequested = false; } public processSignal( - // Note: IInboundSignalMessage is used here in place of ExtensionMessage - // as ExtensionMessage's strictly JSON `content` creates type compatibility - // issues with `AttendeeId` keys and really unknown value content. - // ExtensionMessage is a subset of IInboundSignalMessage so this is safe. - // Change types of DatastoreUpdateMessage | ClientJoinMessage to - // ExtensionMessage<> derivatives to see the issues. - message: IInboundSignalMessage | DatastoreUpdateMessage | ClientJoinMessage, + message: InboundExtensionMessage, local: boolean, + optional: boolean, ): void { const received = Date.now(); assert(message.clientId !== null, 0xa3a /* Map received signal without clientId */); if (!isPresenceMessage(message)) { + assert(optional, "Unrecognized message type in critical message"); return; } if (local) { @@ -390,7 +364,6 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { // It is okay to continue processing the contained updates even if we are not // connected. } else { - assert(message.type === datastoreUpdateMessageType, 0xa3b /* Unexpected message type */); if (message.content.isComplete) { this.refreshBroadcastRequested = false; } @@ -423,7 +396,10 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } const postUpdateActions: PostUpdateAction[] = []; - for (const [workspaceAddress, remoteDatastore] of Object.entries(message.content.data)) { + // While the system workspace is processed here too, it is declared as + // conforming to the general schema. So drop its override. + const data = message.content.data as Omit; + for (const [workspaceAddress, remoteDatastore] of Object.entries(data)) { // Direct to the appropriate Presence Workspace, if present. const workspace = this.workspaces.get(workspaceAddress); if (workspace) { diff --git a/packages/framework/presence/src/presenceManager.ts b/packages/framework/presence/src/presenceManager.ts index fee6dc941df1..d206ddb793b6 100644 --- a/packages/framework/presence/src/presenceManager.ts +++ b/packages/framework/presence/src/presenceManager.ts @@ -6,7 +6,7 @@ import { createEmitter } from "@fluid-internal/client-utils"; import type { ContainerExtension, - ExtensionMessage, + InboundExtensionMessage, } from "@fluidframework/container-definitions/internal"; import type { IEmitter, Listenable } from "@fluidframework/core-interfaces/internal"; import { createSessionId } from "@fluidframework/id-compressor/internal"; @@ -18,7 +18,7 @@ import { createChildMonitoringContext } from "@fluidframework/telemetry-utils/in import type { ClientConnectionId } from "./baseTypes.js"; import type { BroadcastControlSettings } from "./broadcastControls.js"; -import type { IEphemeralRuntime } from "./internalTypes.js"; +import type { ExtensionRuntimeProperties, IEphemeralRuntime } from "./internalTypes.js"; import type { AttendeesEvents, AttendeeId, Presence, PresenceEvents } from "./presence.js"; import type { PresenceDatastoreManager } from "./presenceDatastoreManager.js"; import { PresenceDatastoreManagerImpl } from "./presenceDatastoreManager.js"; @@ -38,7 +38,7 @@ import type { * @internal */ export type PresenceExtensionInterface = Required< - Pick, "processSignal"> + Pick, "processSignal"> >; /** @@ -121,11 +121,25 @@ class PresenceManager implements Presence, PresenceExtensionInterface { * Check for Presence message and process it. * * @param address - Address of the message - * @param message - Message to be processed + * @param message - Unvalidated message to be processed * @param local - Whether the message originated locally (`true`) or remotely (`false`) + * + * @remarks + * generic InboundExtensionMessage is used here in place of specific inbound + * message types that are expected. This is to facilitate this code doing at + * least some validation of the message type and content before use. + * A better solution would be to brand the messages are validated/unvalidated. */ - public processSignal(address: string, message: ExtensionMessage, local: boolean): void { - this.datastoreManager.processSignal(message, local); + public processSignal( + address: string, + message: InboundExtensionMessage, + local: boolean, + ): void { + this.datastoreManager.processSignal( + message, + local, + /* optional */ address.startsWith("?"), + ); } } diff --git a/packages/framework/presence/src/protocol.ts b/packages/framework/presence/src/protocol.ts new file mode 100644 index 000000000000..49d1aaad85db --- /dev/null +++ b/packages/framework/presence/src/protocol.ts @@ -0,0 +1,87 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import type { + InboundExtensionMessage, + OutboundExtensionMessage, +} from "@fluidframework/container-definitions/internal"; + +import type { ClientConnectionId } from "./baseTypes.js"; +import type { AttendeeId } from "./presence.js"; +import type { ClientUpdateEntry } from "./presenceStates.js"; +import type { SystemWorkspaceDatastore } from "./systemWorkspace.js"; + +/** + * @internal + */ +export interface SystemDatastore { + "system:presence": SystemWorkspaceDatastore; +} + +/** + * @internal + */ +export interface GeneralDatastoreMessageContent { + [WorkspaceAddress: string]: { + [StateValueManagerKey: string]: { + [AttendeeId: AttendeeId]: ClientUpdateEntry; + }; + }; +} + +type DatastoreMessageContent = GeneralDatastoreMessageContent & SystemDatastore; + +/** + * @internal + */ +export const datastoreUpdateMessageType = "Pres:DatastoreUpdate"; +interface DatastoreUpdateMessage { + type: typeof datastoreUpdateMessageType; + content: { + sendTimestamp: number; + avgLatency: number; + isComplete?: true; + data: DatastoreMessageContent; + }; +} + +/** + * @internal + */ +export type OutboundDatastoreUpdateMessage = OutboundExtensionMessage; + +/** + * @internal + */ +export type InboundDatastoreUpdateMessage = InboundExtensionMessage; + +/** + * @internal + */ +export const joinMessageType = "Pres:ClientJoin"; +interface ClientJoinMessage { + type: typeof joinMessageType; + content: { + updateProviders: ClientConnectionId[]; + sendTimestamp: number; + avgLatency: number; + data: DatastoreMessageContent; + }; +} + +/** + * @internal + */ +export type OutboundClientJoinMessage = OutboundExtensionMessage; + +/** + * @internal + */ +export type InboundClientJoinMessage = InboundExtensionMessage; + +/** + * @internal + */ +export type SignalMessages = ClientJoinMessage | DatastoreUpdateMessage; diff --git a/packages/framework/presence/src/test/batching.spec.ts b/packages/framework/presence/src/test/batching.spec.ts index d578df18c78f..fa74fefc29c5 100644 --- a/packages/framework/presence/src/test/batching.spec.ts +++ b/packages/framework/presence/src/test/batching.spec.ts @@ -12,7 +12,12 @@ import { Notifications, StateFactory } from "../index.js"; import type { createPresenceManager } from "../presenceManager.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; -import { assertFinalExpectations, prepareConnectedPresence } from "./testUtils.js"; +import { + assertFinalExpectations, + connectionId2, + prepareConnectedPresence, + attendeeId2, +} from "./testUtils.js"; describe("Presence", () => { describe("batching", () => { @@ -35,7 +40,7 @@ describe("Presence", () => { clock.setSystemTime(initialTime); // Set up the presence connection. - presence = prepareConnectedPresence(runtime, "attendeeId-2", "client2", clock, logger); + presence = prepareConnectedPresence(runtime, attendeeId2, connectionId2, clock, logger); }); afterEach(() => { @@ -53,27 +58,29 @@ describe("Presence", () => { it("sends signal immediately when allowable latency is 0", async () => { runtime.signalsExpected.push( [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1010, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1010, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 0, - "timestamp": 1010, - "value": { - "num": 0, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 0, + "timestamp": 1010, + "value": { + "num": 0, + }, }, }, }, @@ -82,27 +89,29 @@ describe("Presence", () => { }, ], [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1020, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1020, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 1, - "timestamp": 1020, - "value": { - "num": 42, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 1, + "timestamp": 1020, + "value": { + "num": 42, + }, }, }, }, @@ -111,27 +120,29 @@ describe("Presence", () => { }, ], [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1020, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1020, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 2, - "timestamp": 1020, - "value": { - "num": 84, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 2, + "timestamp": 1020, + "value": { + "num": 84, + }, }, }, }, @@ -165,27 +176,29 @@ describe("Presence", () => { it("sets timer for default allowableUpdateLatency", async () => { runtime.signalsExpected.push([ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1070, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1070, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 0, - "timestamp": 1010, - "value": { - "num": 0, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 0, + "timestamp": 1010, + "value": { + "num": 0, + }, }, }, }, @@ -211,27 +224,29 @@ describe("Presence", () => { it("batches signals sent within default allowableUpdateLatency", async () => { runtime.signalsExpected.push( [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1070, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1070, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 3, - "timestamp": 1060, - "value": { - "num": 22, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 3, + "timestamp": 1060, + "value": { + "num": 22, + }, }, }, }, @@ -240,27 +255,29 @@ describe("Presence", () => { }, ], [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1150, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1150, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 6, - "timestamp": 1140, - "value": { - "num": 90, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 6, + "timestamp": 1140, + "value": { + "num": 90, + }, }, }, }, @@ -315,27 +332,29 @@ describe("Presence", () => { it("batches signals sent within a specified allowableUpdateLatency", async () => { runtime.signalsExpected.push( [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1110, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1110, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 2, - "timestamp": 1100, - "value": { - "num": 34, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 2, + "timestamp": 1100, + "value": { + "num": 34, + }, }, }, }, @@ -344,27 +363,29 @@ describe("Presence", () => { }, ], [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1240, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1240, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 5, - "timestamp": 1220, - "value": { - "num": 90, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 5, + "timestamp": 1220, + "value": { + "num": 90, + }, }, }, }, @@ -417,36 +438,38 @@ describe("Presence", () => { it("queued signal is sent immediately with immediate update message", async () => { runtime.signalsExpected.push( [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1010, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1010, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 0, - "timestamp": 1010, - "value": { - "num": 0, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 0, + "timestamp": 1010, + "value": { + "num": 0, + }, }, }, - }, - "immediateUpdate": { - "attendeeId-2": { - "rev": 0, - "timestamp": 1010, - "value": { - "num": 0, + "immediateUpdate": { + [attendeeId2]: { + "rev": 0, + "timestamp": 1010, + "value": { + "num": 0, + }, }, }, }, @@ -455,36 +478,38 @@ describe("Presence", () => { }, ], [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1110, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1110, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 2, - "timestamp": 1100, - "value": { - "num": 34, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 2, + "timestamp": 1100, + "value": { + "num": 34, + }, }, }, - }, - "immediateUpdate": { - "attendeeId-2": { - "rev": 1, - "timestamp": 1110, - "value": { - "num": 56, + "immediateUpdate": { + [attendeeId2]: { + "rev": 1, + "timestamp": 1110, + "value": { + "num": 56, + }, }, }, }, @@ -527,36 +552,38 @@ describe("Presence", () => { it("batches signals with different allowed latencies", async () => { runtime.signalsExpected.push( [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1060, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1060, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 2, - "timestamp": 1050, - "value": { - "num": 34, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 2, + "timestamp": 1050, + "value": { + "num": 34, + }, }, }, - }, - "note": { - "attendeeId-2": { - "rev": 1, - "timestamp": 1020, - "value": { - "message": "will be queued", + "note": { + [attendeeId2]: { + "rev": 1, + "timestamp": 1020, + "value": { + "message": "will be queued", + }, }, }, }, @@ -565,22 +592,24 @@ describe("Presence", () => { }, ], [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1110, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { "rev": 0, "timestamp": 1000, "value": "attendeeId-2" }, - }, - }, - "s:name:testStateWorkspace": { - "note": { - "attendeeId-2": { - "rev": 2, - "timestamp": 1060, - "value": { "message": "final message" }, + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1110, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, + }, + }, + "s:name:testStateWorkspace": { + "note": { + [attendeeId2]: { + "rev": 2, + "timestamp": 1060, + "value": { "message": "final message" }, + }, }, }, }, @@ -627,38 +656,40 @@ describe("Presence", () => { it("batches signals from multiple workspaces", async () => { runtime.signalsExpected.push([ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1070, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1070, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 2, - "timestamp": 1050, - "value": { - "num": 34, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 2, + "timestamp": 1050, + "value": { + "num": 34, + }, }, }, }, - }, - "s:name:testStateWorkspace2": { - "note": { - "attendeeId-2": { - "rev": 2, - "timestamp": 1060, - "value": { - "message": "final message", + "s:name:testStateWorkspace2": { + "note": { + [attendeeId2]: { + "rev": 2, + "timestamp": 1060, + "value": { + "message": "final message", + }, }, }, }, @@ -708,23 +739,25 @@ describe("Presence", () => { it("notification signals are sent immediately", async () => { runtime.signalsExpected.push( [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1050, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { "rev": 0, "timestamp": 1000, "value": "attendeeId-2" }, - }, - }, - "n:name:testNotificationWorkspace": { - "testEvents": { - "attendeeId-2": { - "rev": 0, - "timestamp": 0, - "value": { "name": "newId", "args": [77] }, - "ignoreUnmonitored": true, + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1050, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, + }, + }, + "n:name:testNotificationWorkspace": { + "testEvents": { + [attendeeId2]: { + "rev": 0, + "timestamp": 0, + "value": { "name": "newId", "args": [77] }, + "ignoreUnmonitored": true, + }, }, }, }, @@ -732,23 +765,25 @@ describe("Presence", () => { }, ], [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1060, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { "rev": 0, "timestamp": 1000, "value": "attendeeId-2" }, - }, - }, - "n:name:testNotificationWorkspace": { - "testEvents": { - "attendeeId-2": { - "rev": 0, - "timestamp": 0, - "value": { "name": "newId", "args": [88] }, - "ignoreUnmonitored": true, + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1060, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, + }, + }, + "n:name:testNotificationWorkspace": { + "testEvents": { + [attendeeId2]: { + "rev": 0, + "timestamp": 0, + "value": { "name": "newId", "args": [88] }, + "ignoreUnmonitored": true, + }, }, }, }, @@ -792,41 +827,43 @@ describe("Presence", () => { it("notification signals cause queued messages to be sent immediately", async () => { runtime.signalsExpected.push( [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1060, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1060, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "s:name:testStateWorkspace": { - "count": { - "attendeeId-2": { - "rev": 3, - "timestamp": 1040, - "value": { - "num": 56, + "s:name:testStateWorkspace": { + "count": { + [attendeeId2]: { + "rev": 3, + "timestamp": 1040, + "value": { + "num": 56, + }, }, }, }, - }, - "n:name:testNotificationWorkspace": { - "testEvents": { - "attendeeId-2": { - "rev": 0, - "timestamp": 0, - "value": { - "name": "newId", - "args": [99], + "n:name:testNotificationWorkspace": { + "testEvents": { + [attendeeId2]: { + "rev": 0, + "timestamp": 0, + "value": { + "name": "newId", + "args": [99], + }, + "ignoreUnmonitored": true, }, - "ignoreUnmonitored": true, }, }, }, @@ -834,30 +871,32 @@ describe("Presence", () => { }, ], [ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1090, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": 1000, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1090, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": attendeeId2, + }, }, }, - }, - "n:name:testNotificationWorkspace": { - "testEvents": { - "attendeeId-2": { - "rev": 0, - "timestamp": 0, - "value": { - "name": "newId", - "args": [111], + "n:name:testNotificationWorkspace": { + "testEvents": { + [attendeeId2]: { + "rev": 0, + "timestamp": 0, + "value": { + "name": "newId", + "args": [111], + }, + "ignoreUnmonitored": true, }, - "ignoreUnmonitored": true, }, }, }, diff --git a/packages/framework/presence/src/test/eventing.spec.ts b/packages/framework/presence/src/test/eventing.spec.ts index 4fc11d8c2e5b..8ae7d022864d 100644 --- a/packages/framework/presence/src/test/eventing.spec.ts +++ b/packages/framework/presence/src/test/eventing.spec.ts @@ -12,7 +12,11 @@ import { useFakeTimers, spy } from "sinon"; import type { Attendee, WorkspaceAddress } from "../index.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; -import { assertFinalExpectations, prepareConnectedPresence } from "./testUtils.js"; +import { + assertFinalExpectations, + prepareConnectedPresence, + attendeeId1, +} from "./testUtils.js"; import type { LatestRaw, @@ -31,13 +35,13 @@ const attendeeUpdate = { "client1": { "rev": 0, "timestamp": 0, - "value": "attendeeId-1", + "value": attendeeId1, }, }, } as const; const latestUpdate = { "latest": { - "attendeeId-1": { + [attendeeId1]: { "rev": 1, "timestamp": 0, "value": { x: 1, y: 1, z: 1 }, @@ -46,7 +50,7 @@ const latestUpdate = { } as const; const latestMapUpdate = { "latestMap": { - "attendeeId-1": { + [attendeeId1]: { "rev": 1, "items": { "key1": { @@ -65,7 +69,7 @@ const latestMapUpdate = { } as const; const latestUpdateRev2 = { "latest": { - "attendeeId-1": { + [attendeeId1]: { "rev": 2, "timestamp": 50, "value": { x: 2, y: 2, z: 2 }, @@ -74,7 +78,7 @@ const latestUpdateRev2 = { } as const; const itemRemovedMapUpdate = { "latestMap": { - "attendeeId-1": { + [attendeeId1]: { "rev": 2, "items": { "key2": { @@ -87,7 +91,7 @@ const itemRemovedMapUpdate = { } as const; const itemRemovedAndItemUpdatedMapUpdate = { "latestMap": { - "attendeeId-1": { + [attendeeId1]: { "rev": 2, "items": { "key2": { @@ -105,7 +109,7 @@ const itemRemovedAndItemUpdatedMapUpdate = { }; const itemUpdatedAndItemRemoveddMapUpdate = { "latestMap": { - "attendeeId-1": { + [attendeeId1]: { "rev": 2, "items": { "key1": { @@ -127,7 +131,7 @@ const latestMapItemRemovedAndLatestUpdate = { } as const; const notificationsUpdate = { "testEvents": { - "attendeeId-1": { + [attendeeId1]: { "rev": 0, "timestamp": 0, "value": { "name": "newId", "args": [42] }, diff --git a/packages/framework/presence/src/test/mockEphemeralRuntime.ts b/packages/framework/presence/src/test/mockEphemeralRuntime.ts index 0d5274671ada..127e44dc229b 100644 --- a/packages/framework/presence/src/test/mockEphemeralRuntime.ts +++ b/packages/framework/presence/src/test/mockEphemeralRuntime.ts @@ -136,8 +136,8 @@ export class MockEphemeralRuntime implements IEphemeralRuntime { 0, `Missing signals [\n${this.signalsExpected .map( - (a) => - `\t{ type: ${a[0]}, content: ${JSON.stringify(a[1], undefined, "\t")}, targetClientId: ${a[2]} }`, + ([m]) => + `\t{ type: ${m.type}, content: ${JSON.stringify(m.content, undefined, "\t")}, targetClientId: ${m.targetClientId} }`, ) .join(",\n\t")}\n]`, ); diff --git a/packages/framework/presence/src/test/notificationsManager.spec.ts b/packages/framework/presence/src/test/notificationsManager.spec.ts index 43e9f8aebbf5..177062cc4bd0 100644 --- a/packages/framework/presence/src/test/notificationsManager.spec.ts +++ b/packages/framework/presence/src/test/notificationsManager.spec.ts @@ -16,10 +16,17 @@ import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; import { assertFinalExpectations, assertIdenticalTypes, + connectionId2, createInstanceOf, + createSpecificAttendeeId, prepareConnectedPresence, + attendeeId2, } from "./testUtils.js"; +const attendeeId3 = createSpecificAttendeeId("attendeeId-3"); +// Really a ClientConnectionId, but typed as AttendeeId for TypeScript workaround. +const connectionId3 = createSpecificAttendeeId("client3"); + describe("Presence", () => { describe("NotificationsManager", () => { // Note: this test setup mimics the setup in src/test/presenceManager.spec.ts @@ -112,23 +119,25 @@ describe("Presence", () => { clock.tick(10); runtime.signalsExpected.push([ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1020, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { "rev": 0, "timestamp": 1000, "value": "attendeeId-2" }, + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1020, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, + }, }, - }, - "n:name:testNotificationWorkspace": { - "testEvents": { - "attendeeId-2": { - "rev": 0, - "timestamp": 0, - "value": { "name": "newId", "args": [42] }, - "ignoreUnmonitored": true, + "n:name:testNotificationWorkspace": { + "testEvents": { + [attendeeId2]: { + "rev": 0, + "timestamp": 0, + "value": { "name": "newId", "args": [42] }, + "ignoreUnmonitored": true, + }, }, }, }, @@ -163,30 +172,32 @@ describe("Presence", () => { clock.tick(10); runtime.signalsExpected.push([ - "Pres:DatastoreUpdate", { - "sendTimestamp": 1020, - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { "rev": 0, "timestamp": 1000, "value": "attendeeId-2" }, + type: "Pres:DatastoreUpdate", + content: { + "sendTimestamp": 1020, + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, + }, }, - }, - "n:name:testNotificationWorkspace": { - "testEvents": { - "attendeeId-2": { - "rev": 0, - "timestamp": 0, - "value": { "name": "newId", "args": [42] }, - "ignoreUnmonitored": true, + "n:name:testNotificationWorkspace": { + "testEvents": { + [attendeeId2]: { + "rev": 0, + "timestamp": 0, + "value": { "name": "newId", "args": [42] }, + "ignoreUnmonitored": true, + }, }, }, }, }, + // Targeting self for simplicity + targetClientId: "client2", }, - // Targeting self for simplicity - "client2", ]); // Act & Verify @@ -204,7 +215,7 @@ describe("Presence", () => { }; function originalEventHandler(attendee: Attendee, id: number): void { - assert.equal(attendee.attendeeId, "attendeeId-3"); + assert.equal(attendee.attendeeId, attendeeId3); assert.equal(id, 42); eventHandlerCalls.original.push({ attendee, id }); } @@ -248,12 +259,12 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - "client3": { "rev": 0, "timestamp": 1000, "value": "attendeeId-3" }, + [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, }, }, "n:name:testNotificationWorkspace": { "testEvents": { - "attendeeId-3": { + [attendeeId3]: { "rev": 0, "timestamp": 0, "value": { "name": "newId", "args": [42] }, @@ -309,7 +320,7 @@ describe("Presence", () => { testEvents.events.on("unattendedNotification", (name, sender, ...content) => { assert.equal(name, "oldId"); - assert.equal(sender.attendeeId, "attendeeId-3"); + assert.equal(sender.attendeeId, attendeeId3); assert.deepEqual(content, [41]); assert(!unattendedEventCalled); unattendedEventCalled = true; @@ -326,12 +337,12 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - "client3": { "rev": 0, "timestamp": 1000, "value": "attendeeId-3" }, + [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, }, }, "n:name:testNotificationWorkspace": { "testEvents": { - "attendeeId-3": { + [attendeeId3]: { "rev": 0, "timestamp": 0, "value": { "name": "oldId", "args": [41] }, @@ -373,7 +384,7 @@ describe("Presence", () => { testEvents.events.on("unattendedNotification", (name, sender, ...content) => { assert.equal(name, "newId"); - assert.equal(sender.attendeeId, "attendeeId-3"); + assert.equal(sender.attendeeId, attendeeId3); assert.deepEqual(content, [43]); assert(!unattendedEventCalled); unattendedEventCalled = true; @@ -392,12 +403,12 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - "client3": { "rev": 0, "timestamp": 1000, "value": "attendeeId-3" }, + [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, }, }, "n:name:testNotificationWorkspace": { "testEvents": { - "attendeeId-3": { + [attendeeId3]: { "rev": 0, "timestamp": 0, "value": { "name": "newId", "args": [43] }, @@ -419,7 +430,7 @@ describe("Presence", () => { let originalEventHandlerCalled = false; function originalEventHandler(attendee: Attendee, id: number): void { - assert.equal(attendee.attendeeId, "attendeeId-3"); + assert.equal(attendee.attendeeId, attendeeId3); assert.equal(id, 44); assert.equal(originalEventHandlerCalled, false); originalEventHandlerCalled = true; @@ -464,12 +475,12 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - "client3": { "rev": 0, "timestamp": 1000, "value": "attendeeId-3" }, + [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, }, }, "n:name:testNotificationWorkspace": { "testEvents": { - "attendeeId-3": { + [attendeeId3]: { "rev": 0, "timestamp": 0, "value": { "name": "newId", "args": [44] }, diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index 10f2b2d9f9f0..df2283df75a9 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -9,10 +9,29 @@ import { EventAndErrorTrackingLogger } from "@fluidframework/test-utils/internal import type { SinonFakeTimers } from "sinon"; import { useFakeTimers, spy } from "sinon"; +import type { AttendeeId } from "../presence.js"; import { createPresenceManager } from "../presenceManager.js"; +import type { SystemWorkspaceDatastore } from "../systemWorkspace.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; -import { assertFinalExpectations, prepareConnectedPresence } from "./testUtils.js"; +import { + assertFinalExpectations, + connectionId2, + createSpecificAttendeeId, + prepareConnectedPresence, + attendeeId1, + attendeeId2, +} from "./testUtils.js"; + +const attendee4SystemWorkspaceDatastore = { + "clientToSessionId": { + ["client4" as AttendeeId]: { + "rev": 0, + "timestamp": 700, + "value": createSpecificAttendeeId("attendeeId-4"), + }, + }, +} as const satisfies SystemWorkspaceDatastore; describe("Presence", () => { describe("protocol handling", () => { @@ -76,22 +95,24 @@ describe("Presence", () => { }), }); runtime.signalsExpected.push([ - "Pres:DatastoreUpdate", { - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": initialTime, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + [connectionId2]: { + "rev": 0, + "timestamp": initialTime, + "value": attendeeId2, + }, }, }, }, + "isComplete": true, + "sendTimestamp": clock.now, }, - "isComplete": true, - "sendTimestamp": clock.now, }, ]); @@ -103,7 +124,9 @@ describe("Presence", () => { content: { sendTimestamp: clock.now - 50, avgLatency: 50, - data: {}, + data: { + "system:presence": attendee4SystemWorkspaceDatastore, + }, updateProviders: ["client2"], }, clientId: "client4", @@ -125,7 +148,9 @@ describe("Presence", () => { content: { sendTimestamp: clock.now - 20, avgLatency: 0, - data: {}, + data: { + "system:presence": attendee4SystemWorkspaceDatastore, + }, updateProviders: ["client0", "client1"], }, clientId: "client4", @@ -146,22 +171,25 @@ describe("Presence", () => { }), }); runtime.signalsExpected.push([ - "Pres:DatastoreUpdate", { - "avgLatency": 10, - "data": { - "system:presence": { - "clientToSessionId": { - "client2": { - "rev": 0, - "timestamp": initialTime, - "value": "attendeeId-2", + type: "Pres:DatastoreUpdate", + content: { + "avgLatency": 10, + "data": { + "system:presence": { + "clientToSessionId": { + ...attendee4SystemWorkspaceDatastore.clientToSessionId, + [connectionId2]: { + "rev": 0, + "timestamp": initialTime, + "value": attendeeId2, + }, }, }, }, + "isComplete": true, + "sendTimestamp": clock.now + 180, }, - "isComplete": true, - "sendTimestamp": clock.now + 180, }, ]); @@ -182,14 +210,14 @@ describe("Presence", () => { "client1": { "rev": 0, "timestamp": 0, - "value": "attendeeId-1", + "value": attendeeId1, }, }, }; const statesWorkspaceUpdate = { "latest": { - "attendeeId-1": { + [attendeeId1]: { "rev": 1, "timestamp": 0, "value": {}, @@ -199,17 +227,23 @@ describe("Presence", () => { const notificationsWorkspaceUpdate = { "testEvents": { - "attendeeId-1": { + [attendeeId1]: { "rev": 0, "timestamp": 0, "value": {}, "ignoreUnmonitored": true, }, }, - }; + } as const; beforeEach(() => { - presence = prepareConnectedPresence(runtime, "attendeeId-2", "client2", clock, logger); + presence = prepareConnectedPresence( + runtime, + attendeeId2, + connectionId2, + clock, + logger, + ); // Pass a little time (to mimic reality) clock.tick(10); @@ -291,7 +325,7 @@ describe("Presence", () => { "system:presence": systemWorkspaceUpdate, "u:name:testUnknownWorkspace": { "latest": { - "attendeeId-1": { + [attendeeId1]: { "rev": 1, "timestamp": 0, "value": { x: 1, y: 1, z: 1 }, diff --git a/packages/framework/presence/src/test/presenceManager.spec.ts b/packages/framework/presence/src/test/presenceManager.spec.ts index 8889b1cdf1d0..d65de5bb9de6 100644 --- a/packages/framework/presence/src/test/presenceManager.spec.ts +++ b/packages/framework/presence/src/test/presenceManager.spec.ts @@ -16,10 +16,13 @@ import { createPresenceManager } from "../presenceManager.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; import { assertFinalExpectations, + createSpecificAttendeeId, generateBasicClientJoin, prepareConnectedPresence, } from "./testUtils.js"; +const collateralSessionId = createSpecificAttendeeId("collateral-id"); + describe("Presence", () => { describe("PresenceManager", () => { let runtime: MockEphemeralRuntime; @@ -266,7 +269,7 @@ describe("Presence", () => { [collateralAttendeeConnectionId]: { rev: 0, timestamp: 0, - value: "collateral-id", + value: collateralSessionId, }, }, }); @@ -296,7 +299,7 @@ describe("Presence", () => { // Rejoin signal for the collateral attendee unknown to audience const rejoinSignal = generateBasicClientJoin(clock.now - 10, { averageLatency: 40, - attendeeId: "collateral-id", + attendeeId: collateralSessionId, clientConnectionId: newAttendeeConnectionId, updateProviders: [initialAttendeeConnectionId], connectionOrder: 1, @@ -304,7 +307,7 @@ describe("Presence", () => { [oldAttendeeConnectionId]: { rev: 0, timestamp: 0, - value: "collateral-id", + value: collateralSessionId, }, }, }); @@ -321,7 +324,7 @@ describe("Presence", () => { [oldAttendeeConnectionId]: { rev: 0, timestamp: 0, - value: "collateral-id", + value: collateralSessionId, }, }, }); @@ -353,7 +356,7 @@ describe("Presence", () => { "Expected no attendees to be announced", ); // Check attendee information remains unchanged - verifyAttendee(rejoinAttendees[0], newAttendeeConnectionId, "collateral-id"); + verifyAttendee(rejoinAttendees[0], newAttendeeConnectionId, collateralSessionId); }); }); diff --git a/packages/framework/presence/src/test/testUtils.ts b/packages/framework/presence/src/test/testUtils.ts index 3a6a637d8514..eaa6895baa04 100644 --- a/packages/framework/presence/src/test/testUtils.ts +++ b/packages/framework/presence/src/test/testUtils.ts @@ -3,13 +3,14 @@ * Licensed under the MIT License. */ -import type { ExtensionMessage } from "@fluidframework/container-definitions/internal"; import type { InternalUtilityTypes } from "@fluidframework/core-interfaces/internal"; import type { EventAndErrorTrackingLogger } from "@fluidframework/test-utils/internal"; import { getUnexpectedLogErrorException } from "@fluidframework/test-utils/internal"; import type { SinonFakeTimers } from "sinon"; import { createPresenceManager } from "../presenceManager.js"; +import type { InboundClientJoinMessage, OutboundClientJoinMessage } from "../protocol.js"; +import type { SystemWorkspaceDatastore } from "../systemWorkspace.js"; import type { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; @@ -32,15 +33,51 @@ export function createInstanceOf(): T { return undefined as T; } +type SpecificAttendeeId = string extends T + ? never + : Exclude; + +/** + * Forms {@link AttendeeId} for a specific attendee + */ +export function createSpecificAttendeeId( + id: T, +): SpecificAttendeeId { + return id as SpecificAttendeeId; +} + +/** + * Mock {@link AttendeeId}. + */ +export const attendeeId1 = createSpecificAttendeeId("attendeeId-1"); +/** + * Mock {@link AttendeeId}. + */ +export const attendeeId2 = createSpecificAttendeeId("attendeeId-2"); +/** + * Mock {@link ClientConnectionId}. + * + * @remarks + * This is an {@link AttendeeId} as a workaround to TypeScript expectation + * that specific properties overriding an indexed property still conform + * to the index signature. This makes cases where it is used as + * `clientConnectionId` key in {@link SystemWorkspaceDatastore} also + * satisfy {@link GeneralDatastoreMessageContent}'s `AttendeeId` key. + * + * The only known alternative is to use + * `satisfies SystemWorkspaceDatastore as SystemWorkspaceDatastore` + * wherever "system:presence" is defined. + */ +export const connectionId2 = createSpecificAttendeeId("client2"); + /** - * Generates expected join signal for a client that was initialized while connected. + * Generates expected inbound join signal for a client that was initialized while connected. */ -// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/explicit-function-return-type export function generateBasicClientJoin( fixedTime: number, { - attendeeId = "attendeeId-2", - clientConnectionId = "client2", + attendeeId = attendeeId2, + clientConnectionId = connectionId2, updateProviders = ["client0", "client1", "client3"], connectionOrder = 0, averageLatency = 0, @@ -51,12 +88,9 @@ export function generateBasicClientJoin( updateProviders?: string[]; connectionOrder?: number; averageLatency?: number; - priorClientToSessionId?: Record< - ClientConnectionId, - { rev: number; timestamp: number; value: string } - >; + priorClientToSessionId?: SystemWorkspaceDatastore["clientToSessionId"]; }, -) { +): InboundClientJoinMessage { return { type: "Pres:ClientJoin", content: { @@ -68,7 +102,7 @@ export function generateBasicClientJoin( [clientConnectionId]: { "rev": connectionOrder, "timestamp": fixedTime, - "value": attendeeId, + "value": attendeeId as AttendeeId, }, }, }, @@ -77,7 +111,7 @@ export function generateBasicClientJoin( updateProviders, }, clientId: clientConnectionId, - } satisfies ExtensionMessage<"Pres:ClientJoin">; + }; } /** @@ -112,12 +146,14 @@ export function prepareConnectedPresence( quorumClientIds.length = 3; } - const expectedClientJoin = generateBasicClientJoin(clock.now, { + const expectedClientJoin: OutboundClientJoinMessage & + Partial> = generateBasicClientJoin(clock.now, { attendeeId, clientConnectionId, updateProviders: quorumClientIds, }); - runtime.signalsExpected.push([expectedClientJoin.type, expectedClientJoin.content]); + delete expectedClientJoin.clientId; + runtime.signalsExpected.push([expectedClientJoin]); const presence = createPresenceManager(runtime, attendeeId as AttendeeId); From b1f82325df2caecd6feb6f838571295e350c0496 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 17 Apr 2025 14:06:12 -0700 Subject: [PATCH 05/21] improvement(client): distinguish raw incoming signals from [at least partially] validated/verified ones --- .../src/containerExtension.ts | 61 ++++++++++++++++++- .../common/container-definitions/src/index.ts | 3 + .../common/core-interfaces/src/brandedType.ts | 43 +++++++++++++ packages/common/core-interfaces/src/index.ts | 2 + .../src/datastorePresenceManagerFactory.ts | 17 +++--- .../presence/src/presenceDatastoreManager.ts | 11 +++- .../framework/presence/src/presenceManager.ts | 11 +--- packages/framework/presence/src/protocol.ts | 7 ++- 8 files changed, 129 insertions(+), 26 deletions(-) create mode 100644 packages/common/core-interfaces/src/brandedType.ts diff --git a/packages/common/container-definitions/src/containerExtension.ts b/packages/common/container-definitions/src/containerExtension.ts index a98f1dc64a31..b3fb1ff6a114 100644 --- a/packages/common/container-definitions/src/containerExtension.ts +++ b/packages/common/container-definitions/src/containerExtension.ts @@ -3,7 +3,9 @@ * Licensed under the MIT License. */ +// eslint-disable-next-line @typescript-eslint/consistent-type-imports -- BrandedType is a class declaration only import type { + BrandedType, InternalUtilityTypes, ITelemetryBaseLogger, JsonDeserialized, @@ -59,12 +61,54 @@ export type OutboundExtensionMessage }>; /** - * Incoming extension signals. + * Brand for value that has not been verified. + * + * Usage: + * + * - Cast to with `as unknown as UnverifiedBrand` when value of or containing expected type `T` is yet unknown. + * + * - Cast from with `as unknown` when "instance" will be parsed to `T`. * * @sealed * @internal */ -export type InboundExtensionMessage = +export declare class UnverifiedBrand extends BrandedType { + private readonly UnverifiedValue: T; + private constructor(); +} + +/** + * Unverified incoming extension signals. + * + * @sealed + * @internal + */ +export type RawInboundExtensionMessage = + // `TMessage extends TypedMessage` encourages processing union elements individually + TMessage extends TypedMessage + ? InternalUtilityTypes.FlattenIntersection< + ExtensionMessage<{ + type: string; + content: JsonDeserialized; + }> & { + /** + * The client ID that submitted the message. + * For server generated messages the clientId will be null. + */ + // eslint-disable-next-line @rushstack/no-new-null + clientId: ClientConnectionId | null; + } + > & + UnverifiedBrand + : never; + +/** + * Verified incoming extension signals. + * + * @sealed + * @internal + */ +export type VerifiedInboundExtensionMessage = // `TMessage extends TypedMessage` encourages processing union elements individually TMessage extends TypedMessage ? InternalUtilityTypes.FlattenIntersection< @@ -82,6 +126,16 @@ export type InboundExtensionMessage : never; +/** + * Incoming extension signal that may be of the known type or has not yet been validated. + * + * @sealed + * @internal + */ +export type InboundExtensionMessage = + | RawInboundExtensionMessage + | VerifiedInboundExtensionMessage; + /** * @internal */ @@ -109,8 +163,9 @@ export interface ContainerExtension< * Callback for signal sent by this extension. * * @param address - Address of the signal - * @param signalMessage - Unvalidated signal content and metadata + * @param signalMessage - Signal unverified content and metadata * @param local - True if signal was sent by this client + * */ processSignal?: ( address: string, diff --git a/packages/common/container-definitions/src/index.ts b/packages/common/container-definitions/src/index.ts index 77eeca44e849..8c947755acbd 100644 --- a/packages/common/container-definitions/src/index.ts +++ b/packages/common/container-definitions/src/index.ts @@ -27,6 +27,9 @@ export type { ExtensionRuntimeProperties, InboundExtensionMessage, OutboundExtensionMessage, + RawInboundExtensionMessage, + UnverifiedBrand, + VerifiedInboundExtensionMessage, } from "./containerExtension.js"; export type { IConnectionDetails, diff --git a/packages/common/core-interfaces/src/brandedType.ts b/packages/common/core-interfaces/src/brandedType.ts new file mode 100644 index 000000000000..231eba11b1ac --- /dev/null +++ b/packages/common/core-interfaces/src/brandedType.ts @@ -0,0 +1,43 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +/** + * Base branded type which can be used to annotate other type. + * + * @remarks + * To use derive another class declaration and ideally add additional private + * properties to further distinguish the type. + * + * Since branded types are not real value types, they will always need to be + * created using `as` syntax and very often `as unknown` first. + * + * This class should never exist at runtime, so it is only declared. + * + * @sealed + * @internal + */ +export declare class BrandedType { + /** + * Compile time only marker to make type checking more strict. + * This method will not exist at runtime and accessing it is invalid. + * + * @privateRemarks + * `Brand` is used as the return type of a method rather than a simple + * readonly property as this allows types with two brands to be + * intersected without getting `never`. + * The method takes in `never` to help emphasize that it's not callable. + */ + protected readonly brand: (dummy: never) => Brand; + + protected constructor(); + + /** + * Since this class is a compile time only type brand, `instanceof` will + * never work with it. * This `Symbol.hasInstance` implementation ensures + * that `instanceof` will error if used, and in TypeScript 5.3 and newer + * will produce a compile time error if used. + */ + public static [Symbol.hasInstance](value: never): value is never; +} diff --git a/packages/common/core-interfaces/src/index.ts b/packages/common/core-interfaces/src/index.ts index 71d95915b0ce..dad9ef1abfd4 100644 --- a/packages/common/core-interfaces/src/index.ts +++ b/packages/common/core-interfaces/src/index.ts @@ -3,6 +3,8 @@ * Licensed under the MIT License. */ +export type { BrandedType } from "./brandedType.js"; + export type { IDisposable } from "./disposable.js"; export type { IErrorBase, IGenericError, IUsageError, IThrottlingWarning } from "./error.js"; diff --git a/packages/framework/presence/src/datastorePresenceManagerFactory.ts b/packages/framework/presence/src/datastorePresenceManagerFactory.ts index fa72c1e1679c..d07008c68b5c 100644 --- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts +++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts @@ -8,7 +8,10 @@ */ import { createEmitter } from "@fluid-internal/client-utils"; -import type { ExtensionRuntimeEvents } from "@fluidframework/container-definitions/internal"; +import type { + ExtensionRuntimeEvents, + RawInboundExtensionMessage, +} from "@fluidframework/container-definitions/internal"; import type { IFluidLoadable } from "@fluidframework/core-interfaces"; import { assert } from "@fluidframework/core-utils/internal"; import type { IInboundSignalMessage } from "@fluidframework/runtime-definitions/internal"; @@ -18,24 +21,20 @@ import { BasicDataStoreFactory, LoadableFluidObject } from "./datastoreSupport.j import type { Presence } from "./presence.js"; import { createPresenceManager } from "./presenceManager.js"; import type { - InboundClientJoinMessage, - InboundDatastoreUpdateMessage, OutboundClientJoinMessage, OutboundDatastoreUpdateMessage, + SignalMessages, } from "./protocol.js"; /** * This provides faux validation of the signal message. - * This is needed while {@link InboundExtensionMessage} is fully typed even - * though not validated as used by {@link ContainerExtension.processSignal}. - * The preferred assertion that `message is InboundExtensionMessage`. */ function assertSignalMessageIsValid( - message: IInboundSignalMessage | InboundClientJoinMessage | InboundDatastoreUpdateMessage, -): asserts message is InboundClientJoinMessage | InboundDatastoreUpdateMessage { + message: IInboundSignalMessage | RawInboundExtensionMessage, +): asserts message is RawInboundExtensionMessage { assert(message.clientId !== null, 0xa58 /* Signal must have a client ID */); // The other difference between messages is that `content` for - // InboundExtensionMessage is JsonDeserialized and we are fine assuming that. + // RawInboundExtensionMessage is JsonDeserialized and we are fine assuming that. } /** diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index dae495636025..64bbaf21b653 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -29,6 +29,7 @@ import type { InboundClientJoinMessage, InboundDatastoreUpdateMessage, OutboundDatastoreUpdateMessage, + SignalMessages, SystemDatastore, } from "./protocol.js"; import { datastoreUpdateMessageType, joinMessageType } from "./protocol.js"; @@ -61,7 +62,7 @@ const internalWorkspaceTypes: Readonly, ): message is InboundDatastoreUpdateMessage | InboundClientJoinMessage { return knownMessageTypes.has(message.type); } @@ -80,7 +81,11 @@ export interface PresenceDatastoreManager { internalWorkspaceAddress: `n:${WorkspaceAddress}`, requestedContent: TSchema, ): NotificationsWorkspace; - processSignal(message: InboundExtensionMessage, local: boolean, optional: boolean): void; + processSignal( + message: InboundExtensionMessage, + local: boolean, + optional: boolean, + ): void; } function mergeGeneralDatastoreMessageContent( @@ -327,7 +332,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } public processSignal( - message: InboundExtensionMessage, + message: InboundExtensionMessage, local: boolean, optional: boolean, ): void { diff --git a/packages/framework/presence/src/presenceManager.ts b/packages/framework/presence/src/presenceManager.ts index d206ddb793b6..4047f73527dd 100644 --- a/packages/framework/presence/src/presenceManager.ts +++ b/packages/framework/presence/src/presenceManager.ts @@ -22,6 +22,7 @@ import type { ExtensionRuntimeProperties, IEphemeralRuntime } from "./internalTy import type { AttendeesEvents, AttendeeId, Presence, PresenceEvents } from "./presence.js"; import type { PresenceDatastoreManager } from "./presenceDatastoreManager.js"; import { PresenceDatastoreManagerImpl } from "./presenceDatastoreManager.js"; +import type { SignalMessages } from "./protocol.js"; import type { SystemWorkspace, SystemWorkspaceDatastore } from "./systemWorkspace.js"; import { createSystemWorkspace } from "./systemWorkspace.js"; import type { @@ -121,18 +122,12 @@ class PresenceManager implements Presence, PresenceExtensionInterface { * Check for Presence message and process it. * * @param address - Address of the message - * @param message - Unvalidated message to be processed + * @param message - Unverified message to be processed * @param local - Whether the message originated locally (`true`) or remotely (`false`) - * - * @remarks - * generic InboundExtensionMessage is used here in place of specific inbound - * message types that are expected. This is to facilitate this code doing at - * least some validation of the message type and content before use. - * A better solution would be to brand the messages are validated/unvalidated. */ public processSignal( address: string, - message: InboundExtensionMessage, + message: InboundExtensionMessage, local: boolean, ): void { this.datastoreManager.processSignal( diff --git a/packages/framework/presence/src/protocol.ts b/packages/framework/presence/src/protocol.ts index 49d1aaad85db..1958661cee36 100644 --- a/packages/framework/presence/src/protocol.ts +++ b/packages/framework/presence/src/protocol.ts @@ -4,8 +4,8 @@ */ import type { - InboundExtensionMessage, OutboundExtensionMessage, + VerifiedInboundExtensionMessage, } from "@fluidframework/container-definitions/internal"; import type { ClientConnectionId } from "./baseTypes.js"; @@ -55,7 +55,8 @@ export type OutboundDatastoreUpdateMessage = OutboundExtensionMessage; +export type InboundDatastoreUpdateMessage = + VerifiedInboundExtensionMessage; /** * @internal @@ -79,7 +80,7 @@ export type OutboundClientJoinMessage = OutboundExtensionMessage; +export type InboundClientJoinMessage = VerifiedInboundExtensionMessage; /** * @internal From 112853bfacc300d2431dbf46655291389a64ff0c Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 17 Apr 2025 16:18:54 -0700 Subject: [PATCH 06/21] feat(client): implement internal container extensions support and use in presence examples and tests docs: fix presence.md typo Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- docs/docs/build/presence.md | 19 +-- examples/apps/presence-tracker/src/app.ts | 16 ++- .../external-controller/src/app.ts | 10 +- .../loader/container-loader/src/container.ts | 31 ++++- .../container-runtime/src/containerRuntime.ts | 115 ++++++++++++++++-- .../src/test/multiprocess/childClient.ts | 12 +- 6 files changed, 158 insertions(+), 45 deletions(-) diff --git a/docs/docs/build/presence.md b/docs/docs/build/presence.md index 686fceeca40d..9a5d43692053 100644 --- a/docs/docs/build/presence.md +++ b/docs/docs/build/presence.md @@ -5,8 +5,7 @@ sidebar_postition: 10 ## Overview -We are introducting a new way to power your ephemeral experiences wth Fluid. Introducing the new Presence APIs (currently in alpha) that provide session-focused utilities for lightweight data sharing and messaging. - +We are introducing a new way to power your ephemeral experiences with Fluid. Introducing the new Presence APIs (currently in alpha) that provide session-focused utilities for lightweight data sharing and messaging. Collaborative features typically rely on each user maintaining their own temporary state, which is subsequently shared with others. For example, in applications featuring multiplayer cursors, the cursor position of each user signifies their state. This state can be further utilized for various purposes such as indicating typing activity or displaying a user's current selection. This concept is referred to as _presence_. By leveraging this shared state, applications can provide a seamless and interactive collaborative experience, ensuring that users are always aware of each other's actions and selections in real-time. @@ -57,21 +56,13 @@ Notifications are special case where no data is retained during a session and al ## Onboarding -While this package is developing as experimental and other Fluid Framework internals are being updated to accommodate it, a temporary Shared Object must be added within container to gain access. +To access Presence APIs, use `getPresence()` with any `IFluidContainer`. ```typescript -import { - getPresenceViaDataObject, - ExperimentalPresenceManager, -} from "@fluidframework/presence/alpha"; - -const containerSchema = { - initialObjects: { - presence: ExperimentalPresenceManager, - }, -} satisfies ContainerSchema; +import { getPresence } from "@fluidframework/presence/alpha"; -const presence = await getPresenceViaDataObject(container.initialObjects.presence); +function usePresence(container: IFluidContainer): void { + const presence = await getPresence(container); ``` ## Limitations diff --git a/examples/apps/presence-tracker/src/app.ts b/examples/apps/presence-tracker/src/app.ts index c392258362de..9a6b8ebcaae6 100644 --- a/examples/apps/presence-tracker/src/app.ts +++ b/examples/apps/presence-tracker/src/app.ts @@ -4,6 +4,7 @@ */ import { + getPresence, getPresenceViaDataObject, ExperimentalPresenceManager, } from "@fluidframework/presence/alpha"; @@ -16,10 +17,14 @@ import { initializeReactions } from "./reactions.js"; import { renderControlPanel, renderFocusPresence, renderMousePresence } from "./view.js"; // Define the schema of the Fluid container. -// This example uses the presence features only, so only that data object is added. +// This example uses the presence features only, so no data object is required. +// But the old experimental presence data object is used to check that old path still works. +// Besides initialObjects is not currently allowed to be empty. +// That version of presence is compatible with all 2.x runtimes. Long-term support without +// data object requires 2.32 or later. const containerSchema = { initialObjects: { - // A Presence Manager object temporarily needs to be placed within container schema + // A Presence Manager object placed within container schema for experimental presence access // https://github.com/microsoft/FluidFramework/blob/main/packages/framework/presence/README.md#onboarding presence: ExperimentalPresenceManager, }, @@ -56,8 +61,11 @@ async function start() { ({ container } = await client.getContainer(id, containerSchema, "2")); } - // Retrieve a reference to the presence APIs via the data object. - const presence = getPresenceViaDataObject(container.initialObjects.presence); + const useDataObject = location.search.includes("useDataObject"); + const presence = useDataObject + ? // Retrieve a reference to the presence APIs via the data object. + getPresenceViaDataObject(container.initialObjects.presence) + : getPresence(container); // Get the states workspace for the tracker data. This workspace will be created if it doesn't exist. // We create it with no states; we will pass the workspace to the Mouse and Focus trackers, and they will create value diff --git a/examples/service-clients/azure-client/external-controller/src/app.ts b/examples/service-clients/azure-client/external-controller/src/app.ts index d1eff40133bb..e344a427f55e 100644 --- a/examples/service-clients/azure-client/external-controller/src/app.ts +++ b/examples/service-clients/azure-client/external-controller/src/app.ts @@ -11,10 +11,7 @@ import { } from "@fluidframework/azure-client"; import { createDevtoolsLogger, initializeDevtools } from "@fluidframework/devtools/beta"; import { ISharedMap, IValueChanged, SharedMap } from "@fluidframework/map/legacy"; -import { - getPresenceViaDataObject, - ExperimentalPresenceManager, -} from "@fluidframework/presence/alpha"; +import { getPresence } from "@fluidframework/presence/alpha"; import { createChildLogger } from "@fluidframework/telemetry-utils/legacy"; // eslint-disable-next-line import/no-internal-modules -- #26985: `test-runtime-utils` internal used in example import { InsecureTokenProvider } from "@fluidframework/test-runtime-utils/internal"; @@ -76,9 +73,6 @@ const containerSchema = { /* [id]: DataObject */ map1: SharedMap, map2: SharedMap, - // A Presence Manager object temporarily needs to be placed within container schema - // https://github.com/microsoft/FluidFramework/blob/main/packages/framework/presence/README.md#onboarding - presence: ExperimentalPresenceManager, }, } satisfies ContainerSchema; type DiceRollerContainerSchema = typeof containerSchema; @@ -182,7 +176,7 @@ async function start(): Promise { // Biome insist on no semicolon - https://dev.azure.com/fluidframework/internal/_workitems/edit/9083 const lastRoll: { die1?: DieValue; die2?: DieValue } = {}; - const presence = getPresenceViaDataObject(container.initialObjects.presence); + const presence = getPresence(container); const states = buildDicePresence(presence).states; // Initialize Devtools diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index ea96cc0cb54b..675ce3dc4213 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -15,8 +15,12 @@ import { IAudience, ICriticalContainerError, } from "@fluidframework/container-definitions"; -import { +import type { + ContainerExtensionFactory, + ContainerExtensionId, + ContainerExtensionStore, ContainerWarning, + ExtensionRuntimeProperties, IBatchMessage, ICodeDetailsLoader, IContainer, @@ -30,11 +34,12 @@ import { IProvideFluidCodeDetailsComparer, IProvideRuntimeFactory, IRuntime, - isFluidCodeDetails, + IRuntimeInternal, ReadOnlyInfo, - type ILoader, - type ILoaderOptions, + ILoader, + ILoaderOptions, } from "@fluidframework/container-definitions/internal"; +import { isFluidCodeDetails } from "@fluidframework/container-definitions/internal"; import { FluidObject, IEvent, @@ -1223,6 +1228,22 @@ export class Container return this.getPendingLocalStateCore({ notifyImminentClosure: false }); } + public acquireExtension< + T, + TUseContext extends unknown[], + TRuntimeProperties extends ExtensionRuntimeProperties, + >( + id: ContainerExtensionId, + factory: ContainerExtensionFactory, + ...context: TUseContext + ): T { + const runtime = this.runtime as Partial; + if (runtime.acquireExtension === undefined) { + throw new Error("Runtime does not support container extensions feature"); + } + return runtime.acquireExtension(id, factory, ...context); + } + private async getPendingLocalStateCore(props: IGetPendingLocalStateProps): Promise { if (this.closed || this._disposed) { throw new UsageError( @@ -2562,7 +2583,7 @@ export class Container * IContainer interface that includes experimental features still under development. * @internal */ -export interface IContainerExperimental extends IContainer { +export interface IContainerExperimental extends IContainer, Partial { /** * Get pending state from container. WARNING: misuse of this API can result in duplicate op * submission and potential document corruption. The blob returned MUST be deleted if and when this diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 6e27c2dac99b..d06e1ff04443 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -7,7 +7,7 @@ import type { ILayerCompatDetails, IProvideLayerCompatDetails, } from "@fluid-internal/client-utils"; -import { Trace, TypedEventEmitter } from "@fluid-internal/client-utils"; +import { createEmitter, Trace, TypedEventEmitter } from "@fluid-internal/client-utils"; import type { IAudience, ISelf, @@ -16,12 +16,19 @@ import type { } from "@fluidframework/container-definitions"; import { AttachState } from "@fluidframework/container-definitions"; import type { + ContainerExtensionFactory, + ContainerExtensionId, IContainerContext, + ExtensionRuntime, + ExtensionRuntimeEvents, + ExtensionRuntimeProperties, IGetPendingLocalStateProps, IRuntime, + IRuntimeInternal, IDeltaManager, IDeltaManagerFull, ILoader, + OutboundExtensionMessage, } from "@fluidframework/container-definitions/internal"; import { isIDeltaManagerFull } from "@fluidframework/container-definitions/internal"; import type { @@ -34,6 +41,7 @@ import type { IRequest, IResponse, ITelemetryBaseLogger, + Listenable, } from "@fluidframework/core-interfaces"; import type { IErrorBase, @@ -42,10 +50,12 @@ import type { IProvideFluidHandleContext, ISignalEnvelope, JsonDeserialized, + TypedMessage, } from "@fluidframework/core-interfaces/internal"; import { assert, Deferred, + Lazy, LazyPromise, PromiseCache, delay, @@ -274,6 +284,16 @@ import { } from "./summary/index.js"; import { Throttler, formExponentialFn } from "./throttler.js"; +/** + * A {@link ContainerExtension}'s factory function as stored in extension map. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any -- `any` required to allow typed factory to be assignable per ContainerExtension.processSignal +type ExtensionEntry = ContainerExtensionFactory extends new ( + ...args: any[] +) => infer T + ? T + : never; + /** * Creates an error object to be thrown / passed to Container's close fn in case of an unknown message type. * The parameters are typed to support compile-time enforcement of handling all known types/behaviors @@ -803,8 +823,8 @@ export class ContainerRuntime extends TypedEventEmitter implements IContainerRuntime, - IRuntime, IGarbageCollectionRuntime, + IRuntimeInternal, ISummarizerRuntime, ISummarizerInternalsProvider, IFluidParentContext, @@ -1460,6 +1480,8 @@ export class ContainerRuntime */ private readonly skipSafetyFlushDuringProcessStack: boolean; + private readonly extensions = new Map(); + /***/ protected constructor( context: IContainerContext, @@ -1562,11 +1584,24 @@ export class ContainerRuntime } submitSignalFn(envelope, targetClientId); }; - this.submitSignalFn = ( - runtimeOptions.pathBasedAddressing - ? submitWithPathBasedSignalAddress - : submitAssertingLegacySignalAddressing - )(sequenceAndSubmitSignal); + const submitPathAddressedSignal = + submitWithPathBasedSignalAddress(sequenceAndSubmitSignal); + this.submitSignalFn = runtimeOptions.pathBasedAddressing + ? submitPathAddressedSignal + : submitAssertingLegacySignalAddressing(sequenceAndSubmitSignal); + this.submitExtensionSignal = ( + id: string, + subaddress: string, + message: OutboundExtensionMessage, + ): void => { + this.verifyNotClosed(); + const envelope = createNewSignalEnvelope( + `/ext/${id}/${subaddress}`, + message.type, + message.content, + ); + submitPathAddressedSignal(envelope, message.targetClientId); + }; // TODO: After IContainerContext.options is removed, we'll just create a new blank object {} here. // Values are generally expected to be set from the runtime side. @@ -3317,6 +3352,20 @@ export class ContainerRuntime return; } + if (address.top === "ext") { + const idAndSubaddress = address.subaddress.match( + /^(?[^/]*:[^/]*)\/(?.*)$/, + ); + const { id, subaddress } = idAndSubaddress?.groups ?? {}; + if (id !== undefined && subaddress !== undefined) { + const entry = this.extensions.get(id as ContainerExtensionId); + if (entry !== undefined) { + entry.extension.processSignal?.(subaddress, signalMessage, local); + return; + } + } + } + if (address.critical) { assert(!local, "No recipient found for critical local signal"); this.mc.logger.sendTelemetryEvent({ @@ -4929,6 +4978,58 @@ export class ContainerRuntime } } + // While internal, ContainerRuntime has not been converted to use the new events support. + // Recreate the required events (new pattern) with injected, wrapper new emitter. + // It is lazily create to avoid listeners (old events) that ultimately go nowhere. + private readonly lazyEventsForExtensions = new Lazy>( + () => { + const eventEmitter = createEmitter(); + this.on("connected", (clientId) => eventEmitter.emit("connected", clientId)); + this.on("disconnected", () => eventEmitter.emit("disconnected")); + return eventEmitter; + }, + ); + + private readonly submitExtensionSignal: ( + id: string, + subaddress: string, + message: OutboundExtensionMessage, + ) => void; + + public acquireExtension< + T, + TUseContext extends unknown[], + TRuntimeProperties extends ExtensionRuntimeProperties, + >( + id: ContainerExtensionId, + factory: ContainerExtensionFactory, + ...context: TUseContext + ): T { + let entry = this.extensions.get(id); + if (entry === undefined) { + const runtime = { + isConnected: () => this.connected, + getClientId: () => this.clientId, + events: this.lazyEventsForExtensions.value, + logger: this.baseLogger, + submitAddressedSignal: ( + address: string, + message: OutboundExtensionMessage, + ) => { + this.submitExtensionSignal(id, address, message); + }, + getQuorum: this.getQuorum.bind(this), + getAudience: this.getAudience.bind(this), + } satisfies ExtensionRuntime; + entry = new factory(runtime, ...context); + this.extensions.set(id, entry); + } else { + assert(entry instanceof factory, "Extension entry is not of the expected type"); + entry.extension.onNewContext(...context); + } + return entry.interface as T; + } + private get groupedBatchingEnabled(): boolean { return this.sessionSchema.opGroupingEnabled === true; } diff --git a/packages/service-clients/end-to-end-tests/azure-client/src/test/multiprocess/childClient.ts b/packages/service-clients/end-to-end-tests/azure-client/src/test/multiprocess/childClient.ts index 5cdee9d43b01..f7883c1098d7 100644 --- a/packages/service-clients/end-to-end-tests/azure-client/src/test/multiprocess/childClient.ts +++ b/packages/service-clients/end-to-end-tests/azure-client/src/test/multiprocess/childClient.ts @@ -15,11 +15,10 @@ import { AttachState } from "@fluidframework/container-definitions"; import { ConnectionState } from "@fluidframework/container-loader"; import { ContainerSchema, type IFluidContainer } from "@fluidframework/fluid-static"; import { - getPresenceViaDataObject, + getPresence, + type Attendee, ExperimentalPresenceManager, - type ExperimentalPresenceDO, type Presence, - type Attendee, // eslint-disable-next-line import/no-internal-modules } from "@fluidframework/presence/alpha"; import { InsecureTokenProvider } from "@fluidframework/test-runtime-utils/internal"; @@ -83,7 +82,8 @@ const getOrCreatePresenceContainer = async ( const client = new AzureClient({ connection: connectionProps }); const schema: ContainerSchema = { initialObjects: { - presence: ExperimentalPresenceManager, + // A DataObject is added as otherwise fluid-static complains "Container cannot be initialized without any DataTypes" + _unused: ExperimentalPresenceManager, }, }; let services: AzureContainerServices; @@ -107,9 +107,7 @@ const getOrCreatePresenceContainer = async ( "Container is not attached after attach is called", ); - const presence = getPresenceViaDataObject( - container.initialObjects.presence as ExperimentalPresenceDO, - ); + const presence = getPresence(container); return { client, container, From 11c4a19504499a2120212d25fc25ce1eeaef000b Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 25 Apr 2025 12:58:15 -0700 Subject: [PATCH 07/21] docs(client-container-definitions): comments for `IRuntimeInternal` --- .vscode/settings.json | 1 + packages/common/container-definitions/src/runtime.ts | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/.vscode/settings.json b/.vscode/settings.json index 89012c7451b3..056aa3903906 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -53,6 +53,7 @@ "mocharc", "multinomial", "nonfinite", + "privateremarks", "pseudorandomly", "reconnections", "Routerlicious", diff --git a/packages/common/container-definitions/src/runtime.ts b/packages/common/container-definitions/src/runtime.ts index e343b887843b..bc8fa72d1d3f 100644 --- a/packages/common/container-definitions/src/runtime.ts +++ b/packages/common/container-definitions/src/runtime.ts @@ -57,6 +57,10 @@ export enum AttachState { /** * The IRuntime represents an instantiation of a code package within a Container. * Primarily held by the ContainerContext to be able to interact with the running instance of the Container. + * + * @privateremarks + * Implementors of this interface should implement {@link IRuntimeInternal} instead/directly. + * * @legacy * @alpha */ @@ -116,6 +120,13 @@ export interface IRuntime extends IDisposable { } /** + * Version of {@link IRuntime} that is constructed internally by the framework + * and provides access to `@internal` APIs. + * + * @remarks + * Without another guarantee, users given an {@link IRuntime} instance should + * not call internal APIs without checking for the presence of those properties. + * * @internal */ export interface IRuntimeInternal extends IRuntime, ContainerExtensionStore {} From f2e7304e4ace66b33bef31128531ac8294fa7660 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 25 Apr 2025 13:01:13 -0700 Subject: [PATCH 08/21] improvement(client-container-loader): replace `as` with typed target --- packages/loader/container-loader/src/container.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index 675ce3dc4213..92d383da20c3 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -1237,7 +1237,7 @@ export class Container factory: ContainerExtensionFactory, ...context: TUseContext ): T { - const runtime = this.runtime as Partial; + const runtime: Partial = this.runtime; if (runtime.acquireExtension === undefined) { throw new Error("Runtime does not support container extensions feature"); } From beb23e224063c27a154c6388f96b46669080852b Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 25 Apr 2025 13:05:02 -0700 Subject: [PATCH 09/21] improvement(client-example): use URLSearchParams to set a good example --- examples/apps/presence-tracker/src/app.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/apps/presence-tracker/src/app.ts b/examples/apps/presence-tracker/src/app.ts index 9a6b8ebcaae6..4056117d604b 100644 --- a/examples/apps/presence-tracker/src/app.ts +++ b/examples/apps/presence-tracker/src/app.ts @@ -61,7 +61,7 @@ async function start() { ({ container } = await client.getContainer(id, containerSchema, "2")); } - const useDataObject = location.search.includes("useDataObject"); + const useDataObject = new URLSearchParams(location.search).has("useDataObject"); const presence = useDataObject ? // Retrieve a reference to the presence APIs via the data object. getPresenceViaDataObject(container.initialObjects.presence) From 2f3b12e416e5c9609644b9482512e055954c96ef Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 22 May 2025 09:33:09 -0700 Subject: [PATCH 10/21] refactor(client): remove ContainerExtensionStore from container (loader) layer --- .../common/container-definitions/src/index.ts | 17 -------------- .../container-definitions/src/runtime.ts | 16 ------------- .../src/datastorePresenceManagerFactory.ts | 2 +- .../presence/src/experimentalAccess.ts | 23 ++++++------------- .../framework/presence/src/internalTypes.ts | 6 ++--- .../presence/src/presenceDatastoreManager.ts | 2 +- .../framework/presence/src/presenceManager.ts | 2 +- packages/framework/presence/src/protocol.ts | 2 +- .../loader/container-loader/src/container.ts | 23 +------------------ .../src/containerExtension.ts | 3 +-- .../src/containerRuntime.ts | 11 +++++++++ .../src/index.ts | 17 ++++++++++++++ .../container-runtime/src/containerRuntime.ts | 18 +++++++-------- 13 files changed, 53 insertions(+), 89 deletions(-) rename packages/{common/container-definitions => runtime/container-runtime-definitions}/src/containerExtension.ts (99%) diff --git a/packages/common/container-definitions/src/index.ts b/packages/common/container-definitions/src/index.ts index 8c947755acbd..6e587f42fa65 100644 --- a/packages/common/container-definitions/src/index.ts +++ b/packages/common/container-definitions/src/index.ts @@ -15,22 +15,6 @@ export type { IFluidBrowserPackageEnvironment, } from "./browserPackage.js"; export { isFluidBrowserPackage } from "./browserPackage.js"; -export type { - ClientConnectionId, - ContainerExtensionFactory, - ContainerExtensionId, - ContainerExtensionStore, - ContainerExtension, - ExtensionMessage, - ExtensionRuntime, - ExtensionRuntimeEvents, - ExtensionRuntimeProperties, - InboundExtensionMessage, - OutboundExtensionMessage, - RawInboundExtensionMessage, - UnverifiedBrand, - VerifiedInboundExtensionMessage, -} from "./containerExtension.js"; export type { IConnectionDetails, IDeltaManager, @@ -80,7 +64,6 @@ export type { IContainerContext, IProvideRuntimeFactory, IRuntime, - IRuntimeInternal, IGetPendingLocalStateProps, } from "./runtime.js"; export { AttachState, IRuntimeFactory } from "./runtime.js"; diff --git a/packages/common/container-definitions/src/runtime.ts b/packages/common/container-definitions/src/runtime.ts index bc8fa72d1d3f..d0b9f9974f4a 100644 --- a/packages/common/container-definitions/src/runtime.ts +++ b/packages/common/container-definitions/src/runtime.ts @@ -25,7 +25,6 @@ import type { } from "@fluidframework/driver-definitions/internal"; import type { IAudience } from "./audience.js"; -import type { ContainerExtensionStore } from "./containerExtension.js"; import type { IDeltaManager } from "./deltas.js"; import type { ICriticalContainerError } from "./error.js"; import type { ILoader } from "./loader.js"; @@ -58,9 +57,6 @@ export enum AttachState { * The IRuntime represents an instantiation of a code package within a Container. * Primarily held by the ContainerContext to be able to interact with the running instance of the Container. * - * @privateremarks - * Implementors of this interface should implement {@link IRuntimeInternal} instead/directly. - * * @legacy * @alpha */ @@ -119,18 +115,6 @@ export interface IRuntime extends IDisposable { getEntryPoint(): Promise; } -/** - * Version of {@link IRuntime} that is constructed internally by the framework - * and provides access to `@internal` APIs. - * - * @remarks - * Without another guarantee, users given an {@link IRuntime} instance should - * not call internal APIs without checking for the presence of those properties. - * - * @internal - */ -export interface IRuntimeInternal extends IRuntime, ContainerExtensionStore {} - /** * Payload type for IContainerContext.submitBatchFn() * @legacy diff --git a/packages/framework/presence/src/datastorePresenceManagerFactory.ts b/packages/framework/presence/src/datastorePresenceManagerFactory.ts index d07008c68b5c..43beb7e5dfe9 100644 --- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts +++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts @@ -11,7 +11,7 @@ import { createEmitter } from "@fluid-internal/client-utils"; import type { ExtensionRuntimeEvents, RawInboundExtensionMessage, -} from "@fluidframework/container-definitions/internal"; +} from "@fluidframework/container-runtime-definitions/internal"; import type { IFluidLoadable } from "@fluidframework/core-interfaces"; import { assert } from "@fluidframework/core-utils/internal"; import type { IInboundSignalMessage } from "@fluidframework/runtime-definitions/internal"; diff --git a/packages/framework/presence/src/experimentalAccess.ts b/packages/framework/presence/src/experimentalAccess.ts index 49deefcfc991..607cedb5715a 100644 --- a/packages/framework/presence/src/experimentalAccess.ts +++ b/packages/framework/presence/src/experimentalAccess.ts @@ -7,12 +7,10 @@ import type { ContainerExtensionStore, ContainerExtension, InboundExtensionMessage, -} from "@fluidframework/container-definitions/internal"; -import type { IContainerExperimental } from "@fluidframework/container-loader/internal"; +} from "@fluidframework/container-runtime-definitions/internal"; import { assert } from "@fluidframework/core-utils/internal"; import type { IFluidContainer } from "@fluidframework/fluid-static"; import { isInternalFluidContainer } from "@fluidframework/fluid-static/internal"; -import type { IContainerRuntimeBase } from "@fluidframework/runtime-definitions/internal"; import type { ExtensionRuntime, ExtensionRuntimeProperties } from "./internalTypes.js"; import type { Presence } from "./presence.js"; @@ -20,12 +18,6 @@ import type { PresenceExtensionInterface } from "./presenceManager.js"; import { createPresenceManager } from "./presenceManager.js"; import type { SignalMessages } from "./protocol.js"; -function isContainerExtensionStore( - manager: ContainerExtensionStore | IContainerRuntimeBase | IContainerExperimental, -): manager is ContainerExtensionStore { - return (manager as ContainerExtensionStore).acquireExtension !== undefined; -} - /** * Common Presence manager for a container */ @@ -60,6 +52,11 @@ class ContainerPresenceManager } } +// Placeholder to isolate incomplete piece from the rest of the code +function getExtensionStoreFromContainer(_container: IFluidContainer): ContainerExtensionStore { + throw new Error("getPresence connection to runtime extension store is not implemented yet"); +} + /** * Acquire an Presence from a Fluid Container * @param fluidContainer - Fluid Container to acquire the map from @@ -72,14 +69,8 @@ export function getPresence(fluidContainer: IFluidContainer): Presence { isInternalFluidContainer(fluidContainer), 0xa2f /* IFluidContainer was not recognized. Only Containers generated by the Fluid Framework are supported. */, ); - const innerContainer = fluidContainer.container; - - assert( - isContainerExtensionStore(innerContainer), - 0xa39 /* Container does not support extensions. Use getPresenceViaDataObject. */, - ); - const presence = innerContainer.acquireExtension( + const presence = getExtensionStoreFromContainer(fluidContainer).acquireExtension( ContainerPresenceManager.extensionId, ContainerPresenceManager, ); diff --git a/packages/framework/presence/src/internalTypes.ts b/packages/framework/presence/src/internalTypes.ts index f23290831124..820836dc7c41 100644 --- a/packages/framework/presence/src/internalTypes.ts +++ b/packages/framework/presence/src/internalTypes.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import type { ExtensionRuntime as ContainerExtensionRuntime } from "@fluidframework/container-definitions/internal"; +import type { ExtensionRuntime as ContainerExtensionRuntime } from "@fluidframework/container-runtime-definitions/internal"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { AttendeeId, Attendee } from "./presence.js"; @@ -35,8 +35,8 @@ export interface ClientRecord( - id: ContainerExtensionId, - factory: ContainerExtensionFactory, - ...context: TUseContext - ): T { - const runtime: Partial = this.runtime; - if (runtime.acquireExtension === undefined) { - throw new Error("Runtime does not support container extensions feature"); - } - return runtime.acquireExtension(id, factory, ...context); - } - private async getPendingLocalStateCore(props: IGetPendingLocalStateProps): Promise { if (this.closed || this._disposed) { throw new UsageError( @@ -2562,7 +2541,7 @@ export class Container * IContainer interface that includes experimental features still under development. * @internal */ -export interface IContainerExperimental extends IContainer, Partial { +export interface IContainerExperimental extends IContainer { /** * Get pending state from container. WARNING: misuse of this API can result in duplicate op * submission and potential document corruption. The blob returned MUST be deleted if and when this diff --git a/packages/common/container-definitions/src/containerExtension.ts b/packages/runtime/container-runtime-definitions/src/containerExtension.ts similarity index 99% rename from packages/common/container-definitions/src/containerExtension.ts rename to packages/runtime/container-runtime-definitions/src/containerExtension.ts index b3fb1ff6a114..4e4e35cd6ff2 100644 --- a/packages/common/container-definitions/src/containerExtension.ts +++ b/packages/runtime/container-runtime-definitions/src/containerExtension.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. */ +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 { BrandedType, @@ -15,8 +16,6 @@ import type { } from "@fluidframework/core-interfaces/internal"; import type { IQuorumClients } from "@fluidframework/driver-definitions/internal"; -import type { IAudience } from "./audience.js"; - /** * While connected, the id of a client within a session. * diff --git a/packages/runtime/container-runtime-definitions/src/containerRuntime.ts b/packages/runtime/container-runtime-definitions/src/containerRuntime.ts index e10d65b9afca..e778d48661bd 100644 --- a/packages/runtime/container-runtime-definitions/src/containerRuntime.ts +++ b/packages/runtime/container-runtime-definitions/src/containerRuntime.ts @@ -26,6 +26,8 @@ import type { IProvideFluidDataStoreRegistry, } from "@fluidframework/runtime-definitions/internal"; +import type { ContainerExtensionStore } from "./containerExtension.js"; + /** * @deprecated Will be removed in future major release. Migrate all usage of IFluidRouter to the "entryPoint" pattern. Refer to Removing-IFluidRouter.md * @legacy @@ -198,3 +200,12 @@ export interface IContainerRuntime */ getAbsoluteUrl(relativeUrl: string): Promise; } + +/** + * Represents the internal version of the runtime of the container. + * + * @internal + */ +export interface IContainerRuntimeInternal + extends IContainerRuntime, + ContainerExtensionStore {} diff --git a/packages/runtime/container-runtime-definitions/src/index.ts b/packages/runtime/container-runtime-definitions/src/index.ts index 10d6eaaa23b9..f523a6568be9 100644 --- a/packages/runtime/container-runtime-definitions/src/index.ts +++ b/packages/runtime/container-runtime-definitions/src/index.ts @@ -3,10 +3,27 @@ * Licensed under the MIT License. */ +export type { + ClientConnectionId, + ContainerExtensionFactory, + ContainerExtensionId, + ContainerExtensionStore, + ContainerExtension, + ExtensionMessage, + ExtensionRuntime, + ExtensionRuntimeEvents, + ExtensionRuntimeProperties, + InboundExtensionMessage, + OutboundExtensionMessage, + RawInboundExtensionMessage, + UnverifiedBrand, + VerifiedInboundExtensionMessage, +} from "./containerExtension.js"; export type { IContainerRuntime, IContainerRuntimeBaseWithCombinedEvents, IContainerRuntimeEvents, + IContainerRuntimeInternal, IContainerRuntimeWithResolveHandle_Deprecated, SummarizerStopReason, ISummarizeEventProps, diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 6c610e0ac413..c42ac22e5bba 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -16,24 +16,24 @@ import type { } from "@fluidframework/container-definitions"; import { AttachState } from "@fluidframework/container-definitions"; import type { - ContainerExtensionFactory, - ContainerExtensionId, IContainerContext, - ExtensionRuntime, - ExtensionRuntimeEvents, - ExtensionRuntimeProperties, IGetPendingLocalStateProps, IRuntime, - IRuntimeInternal, IDeltaManager, IDeltaManagerFull, ILoader, - OutboundExtensionMessage, } from "@fluidframework/container-definitions/internal"; import { isIDeltaManagerFull } from "@fluidframework/container-definitions/internal"; import type { + ContainerExtensionFactory, + ContainerExtensionId, + ExtensionRuntime, + ExtensionRuntimeEvents, + ExtensionRuntimeProperties, IContainerRuntime, IContainerRuntimeEvents, + IContainerRuntimeInternal, + OutboundExtensionMessage, } from "@fluidframework/container-runtime-definitions/internal"; import type { FluidObject, @@ -856,11 +856,11 @@ const defaultMaxConsecutiveReconnects = 7; export class ContainerRuntime extends TypedEventEmitter implements - IContainerRuntime, + IContainerRuntimeInternal, // eslint-disable-next-line import/no-deprecated IContainerRuntimeBaseExperimental, IGarbageCollectionRuntime, - IRuntimeInternal, + IRuntime, ISummarizerRuntime, ISummarizerInternalsProvider, IFluidParentContext, From b0d8db2475af51fd9100cbb4ba7f146aa19f7434 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 22 May 2025 11:00:32 -0700 Subject: [PATCH 11/21] feat(client): declarative Presence support Connect `presence` through `IFluidContainer` that acquires `ContainerRuntime` (as `ContainerExtensionStore`) via fluis-static runtime factory --- packages/framework/fluid-static/package.json | 1 + .../fluid-static/src/fluidContainer.ts | 21 ++++-- packages/framework/fluid-static/src/index.ts | 3 +- .../fluid-static/src/rootDataObject.ts | 66 ++++++++++++++----- packages/framework/fluid-static/src/types.ts | 25 ++++--- .../presence/src/experimentalAccess.ts | 8 +-- .../azure-client/src/AzureClient.ts | 24 +++---- .../odsp-client/src/odspClient.ts | 18 ++--- .../src/TinyliciousClient.ts | 20 +++--- pnpm-lock.yaml | 3 + 10 files changed, 120 insertions(+), 69 deletions(-) diff --git a/packages/framework/fluid-static/package.json b/packages/framework/fluid-static/package.json index ec8589a4e121..b682f79399bc 100644 --- a/packages/framework/fluid-static/package.json +++ b/packages/framework/fluid-static/package.json @@ -103,6 +103,7 @@ "@fluidframework/container-runtime": "workspace:~", "@fluidframework/container-runtime-definitions": "workspace:~", "@fluidframework/core-interfaces": "workspace:~", + "@fluidframework/core-utils": "workspace:~", "@fluidframework/datastore-definitions": "workspace:~", "@fluidframework/driver-definitions": "workspace:~", "@fluidframework/request-handler": "workspace:~", diff --git a/packages/framework/fluid-static/src/fluidContainer.ts b/packages/framework/fluid-static/src/fluidContainer.ts index 27620fccc445..c68a3e42c91c 100644 --- a/packages/framework/fluid-static/src/fluidContainer.ts +++ b/packages/framework/fluid-static/src/fluidContainer.ts @@ -10,10 +10,16 @@ import { type ICriticalContainerError, } from "@fluidframework/container-definitions"; import type { IContainer } from "@fluidframework/container-definitions/internal"; +import type { ContainerExtensionStore } from "@fluidframework/container-runtime-definitions/internal"; import type { IEvent, IEventProvider, IFluidLoadable } from "@fluidframework/core-interfaces"; import type { SharedObjectKind } from "@fluidframework/shared-object-base"; -import type { ContainerAttachProps, ContainerSchema, IRootDataObject } from "./types.js"; +import type { + ContainerAttachProps, + ContainerSchema, + IRootDataObject, + IStaticEntryPoint, +} from "./types.js"; /** * Extract the type of 'initialObjects' from the given {@link ContainerSchema} type. @@ -235,7 +241,7 @@ export interface IFluidContainer(props: { container: IContainer; - rootDataObject: IRootDataObject; + staticEntryPoint: IStaticEntryPoint; }): IFluidContainer { - return new FluidContainer(props.container, props.rootDataObject); + return new FluidContainer( + props.container, + props.staticEntryPoint.rootDataObject, + props.staticEntryPoint.extensionStore, + ); } /** @@ -291,12 +301,15 @@ class FluidContainer this.emit("disposed", error); private readonly savedHandler = (): boolean => this.emit("saved"); private readonly dirtyHandler = (): boolean => this.emit("dirty"); + public readonly acquireExtension: ContainerExtensionStore["acquireExtension"]; public constructor( public readonly container: IContainer, private readonly rootDataObject: IRootDataObject, + extensionStore: ContainerExtensionStore, ) { super(); + this.acquireExtension = extensionStore.acquireExtension.bind(extensionStore); container.on("connected", this.connectedHandler); container.on("closed", this.disposedHandler); container.on("disconnected", this.disconnectedHandler); diff --git a/packages/framework/fluid-static/src/index.ts b/packages/framework/fluid-static/src/index.ts index 4d9f853035a2..59688878d3fb 100644 --- a/packages/framework/fluid-static/src/index.ts +++ b/packages/framework/fluid-static/src/index.ts @@ -25,10 +25,11 @@ export type { ContainerAttachProps, IConnection, IMember, - IProvideRootDataObject, + IProvideStaticEntryPoint, IRootDataObject, IServiceAudience, IServiceAudienceEvents, + IStaticEntryPoint, LoadableObjectRecord, MemberChangedListener, Myself, diff --git a/packages/framework/fluid-static/src/rootDataObject.ts b/packages/framework/fluid-static/src/rootDataObject.ts index 4ff9f936ddd3..ec703ff8e688 100644 --- a/packages/framework/fluid-static/src/rootDataObject.ts +++ b/packages/framework/fluid-static/src/rootDataObject.ts @@ -14,8 +14,16 @@ import { FluidDataStoreRegistry, type MinimumVersionForCollab, } from "@fluidframework/container-runtime/internal"; -import type { IContainerRuntime } from "@fluidframework/container-runtime-definitions/internal"; -import type { FluidObject, IFluidLoadable } from "@fluidframework/core-interfaces"; +import type { + IContainerRuntime, + IContainerRuntimeInternal, +} from "@fluidframework/container-runtime-definitions/internal"; +import type { + FluidObject, + FluidObjectKeys, + IFluidLoadable, +} from "@fluidframework/core-interfaces"; +import { assert } from "@fluidframework/core-utils/internal"; import type { IChannelFactory } from "@fluidframework/datastore-definitions/internal"; import type { IDirectory } from "@fluidframework/map/internal"; import type { IFluidDataStoreRegistry } from "@fluidframework/runtime-definitions/internal"; @@ -29,6 +37,7 @@ import type { CompatibilityMode, ContainerSchema, IRootDataObject, + IStaticEntryPoint, LoadableObjectKind, LoadableObjectKindRecord, LoadableObjectRecord, @@ -59,18 +68,22 @@ interface RootDataObjectProps { readonly initialObjects: LoadableObjectKindRecord; } +interface IProvideRootDataObject { + readonly RootDataObject: RootDataObject; +} + /** * The entry-point/root collaborative object of the {@link IFluidContainer | Fluid Container}. * Abstracts the dynamic code required to build a Fluid Container into a static representation for end customers. */ class RootDataObject extends DataObject<{ InitialState: RootDataObjectProps }> - implements IRootDataObject + implements IRootDataObject, IProvideRootDataObject { private readonly initialObjectsDirKey = "initial-objects-key"; private readonly _initialObjects: LoadableObjectRecord = {}; - public get IRootDataObject(): IRootDataObject { + public get RootDataObject(): RootDataObject { return this; } @@ -173,8 +186,9 @@ const rootDataStoreId = "rootDOId"; /** * Creates an {@link @fluidframework/aqueduct#BaseContainerRuntimeFactory} which constructs containers - * with a single {@link IRootDataObject} as their entry point, where the root data object's registry - * and initial objects are configured based on the provided schema (and optionally, data store registry). + * with a {@link IStaticEntryPoint} (containing single {@link IRootDataObject}) as their entry point, + * where the root data object's registry and initial objects are configured based on the provided + * schema (and optionally, data store registry). * * @internal */ @@ -203,9 +217,35 @@ export function createDOProviderContainerRuntimeFactory(props: { ); } +function makeFluidObject = FluidObjectKeys>( + object: Omit, + providerKey: K, +): T { + return Object.defineProperty(object, providerKey, { value: object }) as T; +} + +async function provideEntryPoint( + containerRuntime: IContainerRuntime, +): Promise { + const entryPoint = await containerRuntime.getAliasedDataStoreEntryPoint(rootDataStoreId); + if (entryPoint === undefined) { + throw new Error(`default dataStore [${rootDataStoreId}] must exist`); + } + const rootDataObject = ((await entryPoint.get()) as FluidObject) + .RootDataObject; + assert(rootDataObject !== undefined, "entryPoint must be of type RootDataObject"); + return makeFluidObject( + { + rootDataObject, + extensionStore: containerRuntime as IContainerRuntimeInternal, + }, + "IStaticEntryPoint", + ); +} + /** - * Factory for Container Runtime instances that provide a single {@link IRootDataObject} - * as their entry point. + * Factory for Container Runtime instances that provide a {@link IStaticEntryPoint} + * (containing single {@link IRootDataObject}) as their entry point. */ class DOProviderContainerRuntimeFactory extends BaseContainerRuntimeFactory { private readonly rootDataObjectFactory: DataObjectFactory< @@ -239,16 +279,6 @@ class DOProviderContainerRuntimeFactory extends BaseContainerRuntimeFactory { { InitialState: RootDataObjectProps } >, ) { - const provideEntryPoint = async ( - containerRuntime: IContainerRuntime, - // eslint-disable-next-line unicorn/consistent-function-scoping - ): Promise => { - const entryPoint = await containerRuntime.getAliasedDataStoreEntryPoint(rootDataStoreId); - if (entryPoint === undefined) { - throw new Error(`default dataStore [${rootDataStoreId}] must exist`); - } - return entryPoint.get(); - }; super({ registryEntries: [rootDataObjectFactory.registryEntry], runtimeOptions: compatibilityModeRuntimeOptions[compatibilityMode], diff --git a/packages/framework/fluid-static/src/types.ts b/packages/framework/fluid-static/src/types.ts index 20e5e8f2f231..3c8c6872ebab 100644 --- a/packages/framework/fluid-static/src/types.ts +++ b/packages/framework/fluid-static/src/types.ts @@ -4,6 +4,7 @@ */ import type { DataObjectKind } from "@fluidframework/aqueduct/internal"; +import type { ContainerExtensionStore } from "@fluidframework/container-runtime-definitions/internal"; import type { IEvent, IEventProvider, IFluidLoadable } from "@fluidframework/core-interfaces"; import type { SharedObjectKind } from "@fluidframework/shared-object-base"; import type { ISharedObjectKind } from "@fluidframework/shared-object-base/internal"; @@ -94,19 +95,12 @@ export interface ContainerSchema { readonly dynamicObjectTypes?: readonly SharedObjectKind[]; } -/** - * @internal - */ -export interface IProvideRootDataObject { - readonly IRootDataObject: IRootDataObject; -} - /** * Holds the collection of objects that the container was initially created with, as well as provides the ability * to dynamically create further objects during usage. * @internal */ -export interface IRootDataObject extends IProvideRootDataObject { +export interface IRootDataObject { /** * Provides a record of the initial objects defined on creation. */ @@ -122,6 +116,21 @@ export interface IRootDataObject extends IProvideRootDataObject { create(objectClass: SharedObjectKind): Promise; } +/** + * @internal + */ +export interface IProvideStaticEntryPoint { + readonly IStaticEntryPoint: IStaticEntryPoint; +} + +/** + * @internal + */ +export interface IStaticEntryPoint extends IProvideStaticEntryPoint { + readonly rootDataObject: IRootDataObject; + readonly extensionStore: ContainerExtensionStore; +} + /** * Signature for {@link IMember} change events. * diff --git a/packages/framework/presence/src/experimentalAccess.ts b/packages/framework/presence/src/experimentalAccess.ts index 607cedb5715a..87772db3de02 100644 --- a/packages/framework/presence/src/experimentalAccess.ts +++ b/packages/framework/presence/src/experimentalAccess.ts @@ -4,7 +4,6 @@ */ import type { - ContainerExtensionStore, ContainerExtension, InboundExtensionMessage, } from "@fluidframework/container-runtime-definitions/internal"; @@ -52,11 +51,6 @@ class ContainerPresenceManager } } -// Placeholder to isolate incomplete piece from the rest of the code -function getExtensionStoreFromContainer(_container: IFluidContainer): ContainerExtensionStore { - throw new Error("getPresence connection to runtime extension store is not implemented yet"); -} - /** * Acquire an Presence from a Fluid Container * @param fluidContainer - Fluid Container to acquire the map from @@ -70,7 +64,7 @@ export function getPresence(fluidContainer: IFluidContainer): Presence { 0xa2f /* IFluidContainer was not recognized. Only Containers generated by the Fluid Framework are supported. */, ); - const presence = getExtensionStoreFromContainer(fluidContainer).acquireExtension( + const presence = fluidContainer.acquireExtension( ContainerPresenceManager.extensionId, ContainerPresenceManager, ); diff --git a/packages/service-clients/azure-client/src/AzureClient.ts b/packages/service-clients/azure-client/src/AzureClient.ts index 914fc17d991c..17e729f56610 100644 --- a/packages/service-clients/azure-client/src/AzureClient.ts +++ b/packages/service-clients/azure-client/src/AzureClient.ts @@ -32,8 +32,8 @@ import type { IFluidContainer, CompatibilityMode, } from "@fluidframework/fluid-static"; +import type { IStaticEntryPoint } from "@fluidframework/fluid-static/internal"; import { - type IRootDataObject, createDOProviderContainerRuntimeFactory, createFluidContainer, createServiceAudience, @@ -185,10 +185,10 @@ export class AzureClient { ...loaderProps, request: { url: url.href }, }); - const rootDataObject = await this.getContainerEntryPoint(container); + const staticEntryPoint = await this.getContainerEntryPoint(container); const fluidContainer = createFluidContainer({ container, - rootDataObject, + staticEntryPoint, }); const services = this.getContainerServices(container); return { container: fluidContainer, services }; @@ -224,10 +224,10 @@ export class AzureClient { url: url.href, headers: { [LoaderHeader.version]: version.id }, }); - const rootDataObject = await this.getContainerEntryPoint(container); + const staticEntryPoint = await this.getContainerEntryPoint(container); const fluidContainer = createFluidContainer({ container, - rootDataObject, + staticEntryPoint, }); return { container: fluidContainer }; } @@ -322,7 +322,7 @@ export class AzureClient { getTenantId(connection), ); - const rootDataObject = await this.getContainerEntryPoint(container); + const staticEntryPoint = await this.getContainerEntryPoint(container); /** * See {@link FluidContainer.attach} @@ -339,19 +339,19 @@ export class AzureClient { }; const fluidContainer = createFluidContainer({ container, - rootDataObject, + staticEntryPoint, }); fluidContainer.attach = attach; return fluidContainer; } - private async getContainerEntryPoint(container: IContainer): Promise { - const rootDataObject: FluidObject = await container.getEntryPoint(); + private async getContainerEntryPoint(container: IContainer): Promise { + const entryPoint: FluidObject = await container.getEntryPoint(); assert( - rootDataObject.IRootDataObject !== undefined, - 0x90a /* entryPoint must be of type IRootDataObject */, + entryPoint.IStaticEntryPoint !== undefined, + "entryPoint must be of type IStaticEntryPoint", ); - return rootDataObject.IRootDataObject; + return entryPoint.IStaticEntryPoint; } // #endregion } diff --git a/packages/service-clients/odsp-client/src/odspClient.ts b/packages/service-clients/odsp-client/src/odspClient.ts index 8ee74ff4f3fd..034a0eb93fdc 100644 --- a/packages/service-clients/odsp-client/src/odspClient.ts +++ b/packages/service-clients/odsp-client/src/odspClient.ts @@ -27,7 +27,7 @@ import type { ContainerSchema, IFluidContainer, } from "@fluidframework/fluid-static"; -import type { IRootDataObject } from "@fluidframework/fluid-static/internal"; +import type { IStaticEntryPoint } from "@fluidframework/fluid-static/internal"; import { createDOProviderContainerRuntimeFactory, createFluidContainer, @@ -160,7 +160,7 @@ export class OdspClient { const fluidContainer = createFluidContainer({ container, - rootDataObject: await this.getContainerEntryPoint(container), + staticEntryPoint: await this.getContainerEntryPoint(container), }); const services = await this.getContainerServices(container); return { container: fluidContainer as IFluidContainer, services }; @@ -203,7 +203,7 @@ export class OdspClient { container: IContainer, connection: OdspConnectionConfig, ): Promise { - const rootDataObject = await this.getContainerEntryPoint(container); + const staticEntryPoint = await this.getContainerEntryPoint(container); /** * See {@link FluidContainer.attach} @@ -237,7 +237,7 @@ export class OdspClient { */ return resolvedUrl.itemId; }; - const fluidContainer = createFluidContainer({ container, rootDataObject }); + const fluidContainer = createFluidContainer({ container, staticEntryPoint }); fluidContainer.attach = attach; return fluidContainer; } @@ -251,12 +251,12 @@ export class OdspClient { }; } - private async getContainerEntryPoint(container: IContainer): Promise { - const rootDataObject: FluidObject = await container.getEntryPoint(); + private async getContainerEntryPoint(container: IContainer): Promise { + const entryPoint: FluidObject = await container.getEntryPoint(); assert( - rootDataObject.IRootDataObject !== undefined, - 0x878 /* entryPoint must be of type IRootDataObject */, + entryPoint.IStaticEntryPoint !== undefined, + "entryPoint must be of type IStaticEntryPoint", ); - return rootDataObject.IRootDataObject; + return entryPoint.IStaticEntryPoint; } } diff --git a/packages/service-clients/tinylicious-client/src/TinyliciousClient.ts b/packages/service-clients/tinylicious-client/src/TinyliciousClient.ts index 3099f230049e..22e43ed4e56a 100644 --- a/packages/service-clients/tinylicious-client/src/TinyliciousClient.ts +++ b/packages/service-clients/tinylicious-client/src/TinyliciousClient.ts @@ -29,8 +29,8 @@ import type { IFluidContainer, CompatibilityMode, } from "@fluidframework/fluid-static"; +import type { IStaticEntryPoint } from "@fluidframework/fluid-static/internal"; import { - type IRootDataObject, createDOProviderContainerRuntimeFactory, createFluidContainer, createServiceAudience, @@ -101,7 +101,7 @@ export class TinyliciousClient { }, }); - const rootDataObject = await this.getContainerEntryPoint(container); + const staticEntryPoint = await this.getContainerEntryPoint(container); /** * See {@link FluidContainer.attach} @@ -120,7 +120,7 @@ export class TinyliciousClient { const fluidContainer = createFluidContainer({ container, - rootDataObject, + staticEntryPoint, }); fluidContainer.attach = attach; @@ -145,10 +145,10 @@ export class TinyliciousClient { }> { const loaderProps = this.getLoaderProps(containerSchema, compatibilityMode); const container = await loadExistingContainer({ ...loaderProps, request: { url: id } }); - const rootDataObject = await this.getContainerEntryPoint(container); + const staticEntryPoint = await this.getContainerEntryPoint(container); const fluidContainer = createFluidContainer({ container, - rootDataObject, + staticEntryPoint, }); const services = this.getContainerServices(container); return { container: fluidContainer, services }; @@ -206,13 +206,13 @@ export class TinyliciousClient { return loaderProps; } - private async getContainerEntryPoint(container: IContainer): Promise { - const rootDataObject: FluidObject = await container.getEntryPoint(); + private async getContainerEntryPoint(container: IContainer): Promise { + const entryPoint: FluidObject = await container.getEntryPoint(); assert( - rootDataObject.IRootDataObject !== undefined, - 0x875 /* entryPoint must be of type IRootDataObject */, + entryPoint.IStaticEntryPoint !== undefined, + "entryPoint must be of type IStaticEntryPoint", ); - return rootDataObject.IRootDataObject; + return entryPoint.IStaticEntryPoint; } // #endregion } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b99d6a3a6858..2420e3dc624f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -11357,6 +11357,9 @@ importers: '@fluidframework/core-interfaces': specifier: workspace:~ version: link:../../common/core-interfaces + '@fluidframework/core-utils': + specifier: workspace:~ + version: link:../../common/core-utils '@fluidframework/datastore-definitions': specifier: workspace:~ version: link:../../runtime/datastore-definitions From 1c4be86fb5bcbc8f6c5e17919af86b4f463ea5d0 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 22 May 2025 11:05:13 -0700 Subject: [PATCH 12/21] style(client-container-runtime): restore implements ordering --- packages/runtime/container-runtime/src/containerRuntime.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index c42ac22e5bba..11ee00c6ed60 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -859,8 +859,8 @@ export class ContainerRuntime IContainerRuntimeInternal, // eslint-disable-next-line import/no-deprecated IContainerRuntimeBaseExperimental, - IGarbageCollectionRuntime, IRuntime, + IGarbageCollectionRuntime, ISummarizerRuntime, ISummarizerInternalsProvider, IFluidParentContext, From 98b0e923e74b8cbaf58a5baa9973c4347ba48d1a Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 22 May 2025 11:50:37 -0700 Subject: [PATCH 13/21] revert(client-container-runtime): remove dark pathBasedAddressing runtime option --- .changeset/wet-towns-ask.md | 8 --- .../container-runtime/src/compatUtils.ts | 3 - .../container-runtime/src/containerRuntime.ts | 59 +++---------------- .../src/test/containerRuntime.spec.ts | 1 - .../test-service-load/src/optionsMatrix.ts | 1 - 5 files changed, 9 insertions(+), 63 deletions(-) delete mode 100644 .changeset/wet-towns-ask.md diff --git a/.changeset/wet-towns-ask.md b/.changeset/wet-towns-ask.md deleted file mode 100644 index 774483e07e8c..000000000000 --- a/.changeset/wet-towns-ask.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -"@fluidframework/container-runtime": minor -"__section": feature ---- - -Introduce path based message routing - -Add ability for runtime to address messages with a `/` separated path scheme. `/runtime/` is reserved for runtime where `undefined` was previously used and data store messages are prefixed with `/channels/`. To enable general sending messages with this scheme, internal `IContainerRuntimeOptionsInternal.pathBasedAddressing` must be enabled. Internally `presence` requires this support under the `/ext/` path and thus should only be used once involved clients are using version 2.33 or later. diff --git a/packages/runtime/container-runtime/src/compatUtils.ts b/packages/runtime/container-runtime/src/compatUtils.ts index 19c745b59b6e..3c2af4af8ae7 100644 --- a/packages/runtime/container-runtime/src/compatUtils.ts +++ b/packages/runtime/container-runtime/src/compatUtils.ts @@ -100,9 +100,6 @@ export type RuntimeOptionsAffectingDocSchema = Omit< | "maxBatchSizeInBytes" | "loadSequenceNumberVerification" | "summaryOptions" - // This feature will eventually impact but it does not yet impact ops and should - // should never be enabled outside of experimentation. - | "pathBasedAddressing" >; /** diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 11ee00c6ed60..add9df661293 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -483,15 +483,6 @@ export interface ContainerRuntimeOptionsInternal extends ContainerRuntimeOptions * In that case, batched messages will be sent individually (but still all at the same time). */ readonly enableGroupedBatching: boolean; - - /** - * When this property is enabled, runtime will use flexible address encoding - * pattern to route messages to the correct channel or extensible location. - * Minimum client runtime version is 2.33. - * Client runtimes older than 2.33 will log errors receiving a signal with - * path-based address and ultimately ignore the signal. - */ - readonly pathBasedAddressing: true | undefined; } /** @@ -729,7 +720,7 @@ interface PathedAddressInfo { /** * Mostly undefined address parts extracted from an address not using pathed address format. */ -interface LegacyAddressInfo { +interface SingleAddressInfo { top: undefined; critical: undefined; subaddress: undefined; @@ -739,40 +730,10 @@ interface LegacyAddressInfo { /** * Union of the two address info types. */ -type NonContainerAddressInfo = PathedAddressInfo | LegacyAddressInfo; +type NonContainerAddressInfo = PathedAddressInfo | SingleAddressInfo; type UnsequencedSignalEnvelope = Omit; -function submitWithPathBasedSignalAddress( - submitUnsequencedSignalFn: ( - contents: UnsequencedSignalEnvelope, - targetClientId?: string, - ) => void, -): (contents: UnsequencedSignalEnvelope, targetClientId?: string) => void { - return (envelope: UnsequencedSignalEnvelope, targetClientId?: string) => { - if (envelope.address === undefined) { - envelope.address = "/runtime"; - } else if (!envelope.address.startsWith("/")) { - envelope.address = `/channels/${envelope.address}`; - } - submitUnsequencedSignalFn(envelope, targetClientId); - }; -} - -function submitAssertingLegacySignalAddressing( - submitUnsequencedSignalFn: ( - contents: UnsequencedSignalEnvelope, - targetClientId?: string, - ) => void, -): (envelope: UnsequencedSignalEnvelope, targetClientId?: string) => void { - return (envelope: UnsequencedSignalEnvelope, targetClientId?: string) => { - if (envelope.address?.startsWith("/")) { - throw new Error("Path based addressing is not enabled"); - } - submitUnsequencedSignalFn(envelope, targetClientId); - }; -} - /** * This object holds the parameters necessary for the {@link loadContainerRuntime} function. * @legacy @@ -948,7 +909,6 @@ export class ContainerRuntime loadSequenceNumberVerification: "close", maxBatchSizeInBytes: defaultMaxBatchSizeInBytes, chunkSizeInBytes: defaultChunkSizeInBytes, - pathBasedAddressing: undefined, }; const defaultConfigs = { @@ -974,7 +934,6 @@ export class ContainerRuntime ? disabledCompressionConfig : defaultConfigs.compressionOptions, createBlobPayloadPending = defaultConfigs.createBlobPayloadPending, - pathBasedAddressing = defaultConfigs.pathBasedAddressing, }: IContainerRuntimeOptionsInternal = runtimeOptions; // The logic for enableRuntimeIdCompressor is a bit different. Since `undefined` represents a logical state (off) @@ -1181,7 +1140,6 @@ export class ContainerRuntime enableGroupedBatching, explicitSchemaControl, createBlobPayloadPending, - pathBasedAddressing, }; const runtime = new containerRuntimeCtor( @@ -1612,11 +1570,12 @@ export class ContainerRuntime } submitSignalFn(envelope, targetClientId); }; - const submitPathAddressedSignal = - submitWithPathBasedSignalAddress(sequenceAndSubmitSignal); - this.submitSignalFn = runtimeOptions.pathBasedAddressing - ? submitPathAddressedSignal - : submitAssertingLegacySignalAddressing(sequenceAndSubmitSignal); + this.submitSignalFn = (envelope: UnsequencedSignalEnvelope, targetClientId?: string) => { + if (envelope.address?.startsWith("/")) { + throw new Error("General path based addressing is not implemented"); + } + sequenceAndSubmitSignal(envelope, targetClientId); + }; this.submitExtensionSignal = ( id: string, subaddress: string, @@ -1628,7 +1587,7 @@ export class ContainerRuntime message.type, message.content, ); - submitPathAddressedSignal(envelope, message.targetClientId); + sequenceAndSubmitSignal(envelope, message.targetClientId); }; // TODO: After IContainerContext.options is removed, we'll just create a new blank object {} here. diff --git a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts index d1a78c86d873..e23ea3e8bc0d 100644 --- a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts +++ b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts @@ -1599,7 +1599,6 @@ describe("Runtime", () => { enableGroupedBatching: true, // Redundant, but makes the JSON.stringify yield the same result as the logs explicitSchemaControl: false, createBlobPayloadPending: undefined, - pathBasedAddressing: undefined, } as const satisfies ContainerRuntimeOptionsInternal; const mergedRuntimeOptions = { ...defaultRuntimeOptions, ...runtimeOptions } as const; diff --git a/packages/test/test-service-load/src/optionsMatrix.ts b/packages/test/test-service-load/src/optionsMatrix.ts index 57315a2e1895..bb9506961565 100644 --- a/packages/test/test-service-load/src/optionsMatrix.ts +++ b/packages/test/test-service-load/src/optionsMatrix.ts @@ -118,7 +118,6 @@ export function generateRuntimeOptions( enableGroupedBatching: [true, false], createBlobPayloadPending: [true, undefined], explicitSchemaControl: [true, false], - pathBasedAddressing: [true, undefined], }; const pairwiseOptions = generatePairwiseOptions( From f1c3bea3f05f94d61535759dbfb1f4906e6049db Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 22 May 2025 12:04:06 -0700 Subject: [PATCH 14/21] docs(client-presence): `getPresence` now supported --- .changeset/public-houses-add.md | 7 +++++++ .../framework/presence/api-report/presence.alpha.api.md | 6 +++--- .../presence/src/datastorePresenceManagerFactory.ts | 3 +++ 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 .changeset/public-houses-add.md diff --git a/.changeset/public-houses-add.md b/.changeset/public-houses-add.md new file mode 100644 index 000000000000..71ee6eae7849 --- /dev/null +++ b/.changeset/public-houses-add.md @@ -0,0 +1,7 @@ +--- +"@fluidframework/presence": minor +"__section": feature +--- +"getPresence(container: IFluidContainer): Presence" now supported + +`getPresence` is now supported and may be used to directly acquire `Presence` instead of using `ExperimentalPresenceManager` in container schema and calling `getPresenceViaDataObject`. (Both of those are now deprecated.) diff --git a/packages/framework/presence/api-report/presence.alpha.api.md b/packages/framework/presence/api-report/presence.alpha.api.md index be5c3d878085..a4dafc4900fe 100644 --- a/packages/framework/presence/api-report/presence.alpha.api.md +++ b/packages/framework/presence/api-report/presence.alpha.api.md @@ -46,17 +46,17 @@ export interface BroadcastControlSettings { // @alpha export type ClientConnectionId = string; -// @alpha @sealed +// @alpha @sealed @deprecated export class ExperimentalPresenceDO { } -// @alpha +// @alpha @deprecated export const ExperimentalPresenceManager: SharedObjectKind; // @alpha export function getPresence(fluidContainer: IFluidContainer): Presence; -// @alpha +// @alpha @deprecated export function getPresenceViaDataObject(fluidLoadable: ExperimentalPresenceDO): Presence; // @alpha @system diff --git a/packages/framework/presence/src/datastorePresenceManagerFactory.ts b/packages/framework/presence/src/datastorePresenceManagerFactory.ts index 43beb7e5dfe9..279bd8054d21 100644 --- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts +++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts @@ -93,6 +93,7 @@ class PresenceManagerFactory { * @remarks * See {@link getPresenceViaDataObject} for example usage. * + * @deprecated Use {@link getPresence} instead. * @sealed * @alpha */ @@ -104,6 +105,7 @@ export declare class ExperimentalPresenceDO { * DataStore based Presence Manager that is used as fallback for preferred Container * Extension based version requires registration. Export SharedObjectKind for registration. * + * @deprecated Use {@link getPresence} instead. * @alpha */ export const ExperimentalPresenceManager = @@ -129,6 +131,7 @@ export const ExperimentalPresenceManager = * ); * ``` * + * @deprecated Use {@link getPresence} instead. * @alpha */ export function getPresenceViaDataObject(fluidLoadable: ExperimentalPresenceDO): Presence { From d92548a9d716d4c5436f1d119edd12e949ad4381 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 23 May 2025 01:59:02 -0700 Subject: [PATCH 15/21] style(client): renames and comments --- .../src/datastorePresenceManagerFactory.ts | 4 +-- .../presence/src/experimentalAccess.ts | 10 +++--- .../framework/presence/src/internalTypes.ts | 11 ++++--- .../src/containerExtension.ts | 33 ++++++++++++------- .../src/index.ts | 4 +-- .../container-runtime/src/containerRuntime.ts | 26 +++++++-------- 6 files changed, 48 insertions(+), 40 deletions(-) diff --git a/packages/framework/presence/src/datastorePresenceManagerFactory.ts b/packages/framework/presence/src/datastorePresenceManagerFactory.ts index 279bd8054d21..b0a12007b884 100644 --- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts +++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts @@ -9,7 +9,7 @@ import { createEmitter } from "@fluid-internal/client-utils"; import type { - ExtensionRuntimeEvents, + ExtensionHostEvents, RawInboundExtensionMessage, } from "@fluidframework/container-runtime-definitions/internal"; import type { IFluidLoadable } from "@fluidframework/core-interfaces"; @@ -50,7 +50,7 @@ class PresenceManagerDataObject extends LoadableFluidObject { // TODO: investigate if ContainerExtensionStore (path-based address routing for // Signals) is readily detectable here and use that presence manager directly. const runtime = this.runtime; - const events = createEmitter(); + const events = createEmitter(); runtime.on("connected", (clientId) => events.emit("connected", clientId)); runtime.on("disconnected", () => events.emit("disconnected")); diff --git a/packages/framework/presence/src/experimentalAccess.ts b/packages/framework/presence/src/experimentalAccess.ts index 87772db3de02..7c374d592bc9 100644 --- a/packages/framework/presence/src/experimentalAccess.ts +++ b/packages/framework/presence/src/experimentalAccess.ts @@ -11,7 +11,7 @@ import { assert } from "@fluidframework/core-utils/internal"; import type { IFluidContainer } from "@fluidframework/fluid-static"; import { isInternalFluidContainer } from "@fluidframework/fluid-static/internal"; -import type { ExtensionRuntime, ExtensionRuntimeProperties } from "./internalTypes.js"; +import type { ExtensionHost, ExtensionRuntimeProperties } from "./internalTypes.js"; import type { Presence } from "./presence.js"; import type { PresenceExtensionInterface } from "./presenceManager.js"; import { createPresenceManager } from "./presenceManager.js"; @@ -27,16 +27,16 @@ class ContainerPresenceManager public readonly extension = this; private readonly manager: PresenceExtensionInterface; - public constructor(runtime: ExtensionRuntime) { + public constructor(host: ExtensionHost) { this.interface = this.manager = createPresenceManager({ - ...runtime, + ...host, submitSignal: (message) => { - runtime.submitAddressedSignal("", message); + host.submitAddressedSignal("", message); }, }); } - public onNewContext(): void { + public onNewUse(): void { // No-op } diff --git a/packages/framework/presence/src/internalTypes.ts b/packages/framework/presence/src/internalTypes.ts index 820836dc7c41..57b59019c043 100644 --- a/packages/framework/presence/src/internalTypes.ts +++ b/packages/framework/presence/src/internalTypes.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import type { ExtensionRuntime as ContainerExtensionRuntime } from "@fluidframework/container-runtime-definitions/internal"; +import type { ExtensionHost as ContainerExtensionHost } from "@fluidframework/container-runtime-definitions/internal"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { AttendeeId, Attendee } from "./presence.js"; @@ -20,9 +20,10 @@ export interface ExtensionRuntimeProperties { SignalMessages: SignalMessages; } /** + * Presence specific ExtensionHost * @internal */ -export type ExtensionRuntime = ContainerExtensionRuntime; +export type ExtensionHost = ContainerExtensionHost; /** * @internal @@ -35,7 +36,7 @@ export interface ClientRecord & +export type IEphemeralRuntime = Omit & // Apart from tests, there is always a logger. So this could be promoted to required. - Partial> & { + Partial> & { /** * Submits the signal to be sent to other clients. * @param type - Type of the signal. diff --git a/packages/runtime/container-runtime-definitions/src/containerExtension.ts b/packages/runtime/container-runtime-definitions/src/containerExtension.ts index 4e4e35cd6ff2..14330a0068be 100644 --- a/packages/runtime/container-runtime-definitions/src/containerExtension.ts +++ b/packages/runtime/container-runtime-definitions/src/containerExtension.ts @@ -154,9 +154,14 @@ export interface ContainerExtension< /** * Notifies the extension of a new use context. * - * @param context - Context new reference to extension is acquired within + * @param useContext - Context new reference to extension is acquired within. + * + * @remarks + * This is called when a secondary reference to the extension is acquired. + * useContext is the array of arguments that would otherwise be passed to + * the factory during first acquisition request. */ - onNewContext(...context: TUseContext): void; + onNewUse(...useContext: TUseContext): void; /** * Callback for signal sent by this extension. @@ -174,27 +179,29 @@ export interface ContainerExtension< } /** - * Events emitted by the {@link ExtensionRuntime}. + * Events emitted by the {@link ExtensionHost}. * * @internal */ -export interface ExtensionRuntimeEvents { +export interface ExtensionHostEvents { "disconnected": () => void; "connected": (clientId: ClientConnectionId) => void; } /** - * Defines the runtime interface an extension may access. + * Defines the runtime aspects an extension may access. + * + * @remarks * In most cases this is a logical subset of {@link @fluidframework/container-runtime-definitions#IContainerRuntime}. * * @sealed * @internal */ -export interface ExtensionRuntime { +export interface ExtensionHost { readonly isConnected: () => boolean; readonly getClientId: () => ClientConnectionId | undefined; - readonly events: Listenable; + readonly events: Listenable; readonly logger: ITelemetryBaseLogger; @@ -216,6 +223,7 @@ export interface ExtensionRuntime IQuorumClients; + getAudience: () => IAudience; } @@ -227,10 +235,11 @@ export interface ExtensionRuntime = new ( - runtime: ExtensionRuntime, - ...context: TUseContext + host: ExtensionHost, + ...useContext: TUseContext ) => { readonly interface: T; readonly extension: ContainerExtension; diff --git a/packages/runtime/container-runtime-definitions/src/index.ts b/packages/runtime/container-runtime-definitions/src/index.ts index f523a6568be9..bf0dc6d1eaae 100644 --- a/packages/runtime/container-runtime-definitions/src/index.ts +++ b/packages/runtime/container-runtime-definitions/src/index.ts @@ -9,9 +9,9 @@ export type { ContainerExtensionId, ContainerExtensionStore, ContainerExtension, + ExtensionHost, + ExtensionHostEvents, ExtensionMessage, - ExtensionRuntime, - ExtensionRuntimeEvents, ExtensionRuntimeProperties, InboundExtensionMessage, OutboundExtensionMessage, diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index add9df661293..dc47a69b37a4 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -27,8 +27,8 @@ import { isIDeltaManagerFull } from "@fluidframework/container-definitions/inter import type { ContainerExtensionFactory, ContainerExtensionId, - ExtensionRuntime, - ExtensionRuntimeEvents, + ExtensionHost, + ExtensionHostEvents, ExtensionRuntimeProperties, IContainerRuntime, IContainerRuntimeEvents, @@ -5073,14 +5073,12 @@ export class ContainerRuntime // While internal, ContainerRuntime has not been converted to use the new events support. // Recreate the required events (new pattern) with injected, wrapper new emitter. // It is lazily create to avoid listeners (old events) that ultimately go nowhere. - private readonly lazyEventsForExtensions = new Lazy>( - () => { - const eventEmitter = createEmitter(); - this.on("connected", (clientId) => eventEmitter.emit("connected", clientId)); - this.on("disconnected", () => eventEmitter.emit("disconnected")); - return eventEmitter; - }, - ); + private readonly lazyEventsForExtensions = new Lazy>(() => { + const eventEmitter = createEmitter(); + this.on("connected", (clientId) => eventEmitter.emit("connected", clientId)); + this.on("disconnected", () => eventEmitter.emit("disconnected")); + return eventEmitter; + }); private readonly submitExtensionSignal: ( id: string, @@ -5095,7 +5093,7 @@ export class ContainerRuntime >( id: ContainerExtensionId, factory: ContainerExtensionFactory, - ...context: TUseContext + ...useContext: TUseContext ): T { let entry = this.extensions.get(id); if (entry === undefined) { @@ -5112,12 +5110,12 @@ export class ContainerRuntime }, getQuorum: this.getQuorum.bind(this), getAudience: this.getAudience.bind(this), - } satisfies ExtensionRuntime; - entry = new factory(runtime, ...context); + } satisfies ExtensionHost; + entry = new factory(runtime, ...useContext); this.extensions.set(id, entry); } else { assert(entry instanceof factory, "Extension entry is not of the expected type"); - entry.extension.onNewContext(...context); + entry.extension.onNewUse(...useContext); } return entry.interface as T; } From a6ad229595ad8a6d19b73df75ddcacf9abedb800 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 23 May 2025 02:00:32 -0700 Subject: [PATCH 16/21] refactor(client): generics ordering for explicit simplicity and correctness when no UseContext --- packages/framework/presence/src/experimentalAccess.ts | 7 ++++++- packages/framework/presence/src/presenceManager.ts | 2 +- .../src/containerExtension.ts | 10 +++++----- .../runtime/container-runtime/src/containerRuntime.ts | 6 +++--- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/framework/presence/src/experimentalAccess.ts b/packages/framework/presence/src/experimentalAccess.ts index 7c374d592bc9..870ffc85e304 100644 --- a/packages/framework/presence/src/experimentalAccess.ts +++ b/packages/framework/presence/src/experimentalAccess.ts @@ -5,6 +5,7 @@ import type { ContainerExtension, + ContainerExtensionFactory, InboundExtensionMessage, } from "@fluidframework/container-runtime-definitions/internal"; import { assert } from "@fluidframework/core-utils/internal"; @@ -21,10 +22,14 @@ import type { SignalMessages } from "./protocol.js"; * Common Presence manager for a container */ class ContainerPresenceManager - implements ContainerExtension + implements + ContainerExtension, + InstanceType> { + // ContainerExtensionFactory return elements public readonly interface: Presence; public readonly extension = this; + private readonly manager: PresenceExtensionInterface; public constructor(host: ExtensionHost) { diff --git a/packages/framework/presence/src/presenceManager.ts b/packages/framework/presence/src/presenceManager.ts index 080c3560f778..d972ba15e756 100644 --- a/packages/framework/presence/src/presenceManager.ts +++ b/packages/framework/presence/src/presenceManager.ts @@ -39,7 +39,7 @@ import type { * @internal */ export type PresenceExtensionInterface = Required< - Pick, "processSignal"> + Pick, "processSignal"> >; /** diff --git a/packages/runtime/container-runtime-definitions/src/containerExtension.ts b/packages/runtime/container-runtime-definitions/src/containerExtension.ts index 14330a0068be..e3a6761e002d 100644 --- a/packages/runtime/container-runtime-definitions/src/containerExtension.ts +++ b/packages/runtime/container-runtime-definitions/src/containerExtension.ts @@ -148,8 +148,8 @@ export interface ExtensionRuntimeProperties { * @internal */ export interface ContainerExtension< - TUseContext extends unknown[], TRuntimeProperties extends ExtensionRuntimeProperties, + TUseContext extends unknown[] = [], > { /** * Notifies the extension of a new use context. @@ -249,14 +249,14 @@ export interface ExtensionHost = new ( host: ExtensionHost, ...useContext: TUseContext ) => { readonly interface: T; - readonly extension: ContainerExtension; + readonly extension: ContainerExtension; }; /** @@ -291,11 +291,11 @@ export interface ContainerExtensionStore { */ acquireExtension< T, - TUseContext extends unknown[], TRuntimeProperties extends ExtensionRuntimeProperties, + TUseContext extends unknown[] = [], >( id: ContainerExtensionId, - factory: ContainerExtensionFactory, + factory: ContainerExtensionFactory, ...context: TUseContext ): T; } diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index dc47a69b37a4..19d7db8d9a16 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -299,7 +299,7 @@ import { Throttler, formExponentialFn } from "./throttler.js"; * A {@link ContainerExtension}'s factory function as stored in extension map. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any -- `any` required to allow typed factory to be assignable per ContainerExtension.processSignal -type ExtensionEntry = ContainerExtensionFactory extends new ( +type ExtensionEntry = ContainerExtensionFactory extends new ( ...args: any[] ) => infer T ? T @@ -5088,11 +5088,11 @@ export class ContainerRuntime public acquireExtension< T, - TUseContext extends unknown[], TRuntimeProperties extends ExtensionRuntimeProperties, + TUseContext extends unknown[], >( id: ContainerExtensionId, - factory: ContainerExtensionFactory, + factory: ContainerExtensionFactory, ...useContext: TUseContext ): T { let entry = this.extensions.get(id); From 5b1ed990e4b01bb4f9b8531ed592d0946e847ed5 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 23 May 2025 03:48:48 -0700 Subject: [PATCH 17/21] refactor(client-container-runtime): simplify addressed path support using just split by "/" --- .../src/datastorePresenceManagerFactory.ts | 2 +- .../presence/src/experimentalAccess.ts | 6 +- .../framework/presence/src/presenceManager.ts | 6 +- .../presence/src/test/eventing.spec.ts | 2 +- .../src/test/notificationsManager.spec.ts | 8 +- .../src/test/presenceDatastoreManager.spec.ts | 20 +-- .../presence/src/test/presenceManager.spec.ts | 4 +- .../framework/presence/src/test/testUtils.ts | 2 +- .../src/containerExtension.ts | 8 +- .../container-runtime/src/containerRuntime.ts | 115 ++++-------------- 10 files changed, 54 insertions(+), 119 deletions(-) diff --git a/packages/framework/presence/src/datastorePresenceManagerFactory.ts b/packages/framework/presence/src/datastorePresenceManagerFactory.ts index b0a12007b884..31043475c5d3 100644 --- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts +++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts @@ -65,7 +65,7 @@ class PresenceManagerDataObject extends LoadableFluidObject { }); this.runtime.on("signal", (message: IInboundSignalMessage, local: boolean) => { assertSignalMessageIsValid(message); - manager.processSignal("", message, local); + manager.processSignal([], message, local); }); this._presenceManager = manager; } diff --git a/packages/framework/presence/src/experimentalAccess.ts b/packages/framework/presence/src/experimentalAccess.ts index 870ffc85e304..48009ea1b9c4 100644 --- a/packages/framework/presence/src/experimentalAccess.ts +++ b/packages/framework/presence/src/experimentalAccess.ts @@ -36,7 +36,7 @@ class ContainerPresenceManager this.interface = this.manager = createPresenceManager({ ...host, submitSignal: (message) => { - host.submitAddressedSignal("", message); + host.submitAddressedSignal([], message); }, }); } @@ -48,11 +48,11 @@ class ContainerPresenceManager public static readonly extensionId = "dis:bb89f4c0-80fd-4f0c-8469-4f2848ee7f4a"; public processSignal( - address: string, + addressChain: string[], message: InboundExtensionMessage, local: boolean, ): void { - this.manager.processSignal(address, message, local); + this.manager.processSignal(addressChain, message, local); } } diff --git a/packages/framework/presence/src/presenceManager.ts b/packages/framework/presence/src/presenceManager.ts index d972ba15e756..3c4f105f923e 100644 --- a/packages/framework/presence/src/presenceManager.ts +++ b/packages/framework/presence/src/presenceManager.ts @@ -121,19 +121,19 @@ class PresenceManager implements Presence, PresenceExtensionInterface { /** * Check for Presence message and process it. * - * @param address - Address of the message + * @param addressChain - Address chain of the message * @param message - Unverified message to be processed * @param local - Whether the message originated locally (`true`) or remotely (`false`) */ public processSignal( - address: string, + addressChain: string[], message: InboundExtensionMessage, local: boolean, ): void { this.datastoreManager.processSignal( message, local, - /* optional */ address.startsWith("?"), + /* optional */ addressChain[0] === "?", ); } } diff --git a/packages/framework/presence/src/test/eventing.spec.ts b/packages/framework/presence/src/test/eventing.spec.ts index 8ae7d022864d..104dd24d2090 100644 --- a/packages/framework/presence/src/test/eventing.spec.ts +++ b/packages/framework/presence/src/test/eventing.spec.ts @@ -298,7 +298,7 @@ describe("Presence", () => { const updates = { "system:presence": attendeeUpdate, ...valueManagerUpdates }; presence.processSignal( - "", + [], { type: datastoreUpdateType, content: { diff --git a/packages/framework/presence/src/test/notificationsManager.spec.ts b/packages/framework/presence/src/test/notificationsManager.spec.ts index 177062cc4bd0..4d92ba58772a 100644 --- a/packages/framework/presence/src/test/notificationsManager.spec.ts +++ b/packages/framework/presence/src/test/notificationsManager.spec.ts @@ -250,7 +250,7 @@ describe("Presence", () => { // Processing this signal should trigger the testEvents.newId event listeners presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { @@ -328,7 +328,7 @@ describe("Presence", () => { // Processing this signal should trigger the testEvents.newId event listeners presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { @@ -394,7 +394,7 @@ describe("Presence", () => { // Processing this signal should trigger the testEvents.newId event listeners presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { @@ -466,7 +466,7 @@ describe("Presence", () => { // Act presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index df2283df75a9..7c1d0d1cfa9e 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -118,7 +118,7 @@ describe("Presence", () => { // Act presence.processSignal( - "", + [], { type: "Pres:ClientJoin", content: { @@ -142,7 +142,7 @@ describe("Presence", () => { // #region Part 1 (no response) // Act presence.processSignal( - "", + [], { type: "Pres:ClientJoin", content: { @@ -256,7 +256,7 @@ describe("Presence", () => { // Act presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { @@ -284,7 +284,7 @@ describe("Presence", () => { // Act presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { @@ -315,7 +315,7 @@ describe("Presence", () => { // Act presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { @@ -353,7 +353,7 @@ describe("Presence", () => { // Act presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { @@ -381,7 +381,7 @@ describe("Presence", () => { // Act presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { @@ -409,7 +409,7 @@ describe("Presence", () => { // Act presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { @@ -437,7 +437,7 @@ describe("Presence", () => { // Act presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { @@ -453,7 +453,7 @@ describe("Presence", () => { false, ); presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { diff --git a/packages/framework/presence/src/test/presenceManager.spec.ts b/packages/framework/presence/src/test/presenceManager.spec.ts index d65de5bb9de6..9dbbd945dd83 100644 --- a/packages/framework/presence/src/test/presenceManager.spec.ts +++ b/packages/framework/presence/src/test/presenceManager.spec.ts @@ -116,7 +116,7 @@ describe("Presence", () => { ); for (const signal of signals) { - presence.processSignal("", signal, false); + presence.processSignal([], signal, false); } cleanUpListener(); @@ -578,7 +578,7 @@ describe("Presence", () => { runtime.connect("client6"); clock.tick(15_000); presence.processSignal( - "", + [], { type: "Pres:DatastoreUpdate", content: { diff --git a/packages/framework/presence/src/test/testUtils.ts b/packages/framework/presence/src/test/testUtils.ts index eaa6895baa04..ab2cbc5271fe 100644 --- a/packages/framework/presence/src/test/testUtils.ts +++ b/packages/framework/presence/src/test/testUtils.ts @@ -169,7 +169,7 @@ export function prepareConnectedPresence( clock.tick(10); // Return the join signal - presence.processSignal("", { ...expectedClientJoin, clientId: clientConnectionId }, true); + presence.processSignal([], { ...expectedClientJoin, clientId: clientConnectionId }, true); return presence; } diff --git a/packages/runtime/container-runtime-definitions/src/containerExtension.ts b/packages/runtime/container-runtime-definitions/src/containerExtension.ts index e3a6761e002d..28f40e031937 100644 --- a/packages/runtime/container-runtime-definitions/src/containerExtension.ts +++ b/packages/runtime/container-runtime-definitions/src/containerExtension.ts @@ -166,13 +166,13 @@ export interface ContainerExtension< /** * Callback for signal sent by this extension. * - * @param address - Address of the signal + * @param addressChain - Address chain of the signal * @param signalMessage - Signal unverified content and metadata * @param local - True if signal was sent by this client * */ processSignal?: ( - address: string, + addressChain: string[], signalMessage: InboundExtensionMessage, local: boolean, ) => void; @@ -207,14 +207,14 @@ export interface ExtensionHost, ) => void; diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 19d7db8d9a16..e1ca395f32ef 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -695,43 +695,6 @@ export let getSingleUseLegacyLogCallback = (logger: ITelemetryLoggerExt, type: s }; }; -/** - * Address parts extracted from an address using the pathed address format. - */ -interface PathedAddressInfo { - /** - * First address in path - extracted from {@link PathedAddressInfo.fullAddress} - */ - top: string; - /** - * When true, it is considered an error if {@link PathedAddressInfo.top} is not found as a destination - */ - critical: boolean; - /** - * Address after {@link PathedAddressInfo.top} - extracted from {@link PathedAddressInfo.fullAddress} - */ - subaddress: string; - /** - * Full original address - */ - fullAddress: string; -} - -/** - * Mostly undefined address parts extracted from an address not using pathed address format. - */ -interface SingleAddressInfo { - top: undefined; - critical: undefined; - subaddress: undefined; - fullAddress: string; -} - -/** - * Union of the two address info types. - */ -type NonContainerAddressInfo = PathedAddressInfo | SingleAddressInfo; - type UnsequencedSignalEnvelope = Omit; /** @@ -1578,12 +1541,12 @@ export class ContainerRuntime }; this.submitExtensionSignal = ( id: string, - subaddress: string, + addressChain: string[], message: OutboundExtensionMessage, ): void => { this.verifyNotClosed(); const envelope = createNewSignalEnvelope( - `/ext/${id}/${subaddress}`, + `/ext/${id}/${addressChain.join("/")}`, message.type, message.content, ); @@ -3283,49 +3246,27 @@ export class ContainerRuntime } const fullAddress = envelope.address; - if (fullAddress === undefined || fullAddress === "/runtime/") { + if (fullAddress === undefined) { // No address indicates a container signal message. this.emit("signal", transformed, local); return; } - // Note that this pattern allows for ! prefix in top-address but - // does not give it any more special treatment than without it. - const topAddressAndSubaddress = fullAddress.match( - /^\/(?[!?]?)(?[^/]*)\/(?.*)$/, - ); - const { optional, top, subaddress } = topAddressAndSubaddress?.groups ?? { - optional: undefined, - top: undefined, - subaddress: undefined, - }; - this.routeNonContainerSignal( - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- cast required - { - top, - // Could just check !== "?", but would not correctly meet strict typing. - critical: optional === undefined ? undefined : optional !== "?", - subaddress, - fullAddress, - } as NonContainerAddressInfo, - transformed, - local, - ); + this.routeNonContainerSignal(fullAddress, transformed, local); } private routeNonContainerSignal( - address: NonContainerAddressInfo, + address: string, signalMessage: IInboundSignalMessage<{ type: string; content: JsonDeserialized }>, local: boolean, ): void { - // channelCollection signals are identified by no top address (use full address) or by the top address of "channels". - const isChannelAddress = address.top === undefined || address.top === "channels"; - if (isChannelAddress) { + // channelCollection signals are identified by no starting `/` in address. + if (!address.startsWith("/")) { // Due to a mismatch between different layers in terms of // what is the interface of passing signals, we need to adjust // the signal envelope before sending it to the datastores to be processed const envelope = { - address: address.subaddress ?? address.fullAddress, + address, contents: signalMessage.content, }; signalMessage.content = envelope; @@ -3334,29 +3275,23 @@ export class ContainerRuntime return; } - if (address.top === "ext") { - const idAndSubaddress = address.subaddress.match( - /^(?[^/]*:[^/]*)\/(?.*)$/, - ); - const { id, subaddress } = idAndSubaddress?.groups ?? {}; - if (id !== undefined && subaddress !== undefined) { - const entry = this.extensions.get(id as ContainerExtensionId); - if (entry !== undefined) { - entry.extension.processSignal?.(subaddress, signalMessage, local); - return; - } + const addresses = address.split("/"); + if (addresses.length > 2 && addresses[1] === "ext") { + const id = addresses[2] as ContainerExtensionId; + const entry = this.extensions.get(id); + if (entry !== undefined) { + entry.extension.processSignal?.(addresses.slice(3), signalMessage, local); + return; } } - if (address.critical) { - assert(!local, "No recipient found for critical local signal"); - this.mc.logger.sendTelemetryEvent({ - eventName: "SignalCriticalAddressNotFound", - ...tagCodeArtifacts({ - address: address.top, - }), - }); - } + assert(!local, "No recipient found for local signal"); + this.mc.logger.sendTelemetryEvent({ + eventName: "SignalAddressNotFound", + ...tagCodeArtifacts({ + address, + }), + }); } /** @@ -5082,7 +5017,7 @@ export class ContainerRuntime private readonly submitExtensionSignal: ( id: string, - subaddress: string, + addressChain: string[], message: OutboundExtensionMessage, ) => void; @@ -5103,10 +5038,10 @@ export class ContainerRuntime events: this.lazyEventsForExtensions.value, logger: this.baseLogger, submitAddressedSignal: ( - address: string, + addressChain: string[], message: OutboundExtensionMessage, ) => { - this.submitExtensionSignal(id, address, message); + this.submitExtensionSignal(id, addressChain, message); }, getQuorum: this.getQuorum.bind(this), getAudience: this.getAudience.bind(this), From 41cfcc627149074fca10892e2a81910a7e0b4a84 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 23 May 2025 03:51:50 -0700 Subject: [PATCH 18/21] build(client-presence): remove unused container-loader dep --- packages/framework/presence/package.json | 1 - pnpm-lock.yaml | 3 --- 2 files changed, 4 deletions(-) diff --git a/packages/framework/presence/package.json b/packages/framework/presence/package.json index 1fd3f1578069..4f16256d19e5 100644 --- a/packages/framework/presence/package.json +++ b/packages/framework/presence/package.json @@ -96,7 +96,6 @@ "dependencies": { "@fluid-internal/client-utils": "workspace:~", "@fluidframework/container-definitions": "workspace:~", - "@fluidframework/container-loader": "workspace:~", "@fluidframework/container-runtime-definitions": "workspace:~", "@fluidframework/core-interfaces": "workspace:~", "@fluidframework/core-utils": "workspace:~", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f11b880bf1ec..81412a2dc360 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -11527,9 +11527,6 @@ importers: '@fluidframework/container-definitions': specifier: workspace:~ version: link:../../common/container-definitions - '@fluidframework/container-loader': - specifier: workspace:~ - version: link:../../loader/container-loader '@fluidframework/container-runtime-definitions': specifier: workspace:~ version: link:../../runtime/container-runtime-definitions From ce572d13f972f7c706cdd8b6b0da9de8bb6b3c5b Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 23 May 2025 03:52:48 -0700 Subject: [PATCH 19/21] test(client-examples): cleanup deprecated presence init --- examples/apps/ai-collab/src/app/page.tsx | 4 ++-- examples/apps/ai-collab/src/types/sharedTreeAppSchema.ts | 6 ------ examples/apps/presence-tracker/src/app.ts | 9 ++++++--- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/examples/apps/ai-collab/src/app/page.tsx b/examples/apps/ai-collab/src/app/page.tsx index 7fe2e53e98fb..86222c590329 100644 --- a/examples/apps/ai-collab/src/app/page.tsx +++ b/examples/apps/ai-collab/src/app/page.tsx @@ -5,7 +5,7 @@ "use client"; -import { getPresenceViaDataObject } from "@fluidframework/presence/alpha"; +import { getPresence } from "@fluidframework/presence/alpha"; import { Box, Button, @@ -63,7 +63,7 @@ export default function TasksListPage(): JSX.Element { const _treeView = fluidContainer.initialObjects.appState.viewWith(TREE_CONFIGURATION); setTreeView(_treeView); - const presence = getPresenceViaDataObject(fluidContainer.initialObjects.presence); + const presence = getPresence(fluidContainer); setPresenceManagerContext(new PresenceManager(presence)); return { sharedTree: _treeView }; }, diff --git a/examples/apps/ai-collab/src/types/sharedTreeAppSchema.ts b/examples/apps/ai-collab/src/types/sharedTreeAppSchema.ts index 3b03f162dd6d..6e3465b7fce1 100644 --- a/examples/apps/ai-collab/src/types/sharedTreeAppSchema.ts +++ b/examples/apps/ai-collab/src/types/sharedTreeAppSchema.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. */ -import { ExperimentalPresenceManager } from "@fluidframework/presence/alpha"; import { Tree, type TreeNode, TreeViewConfiguration } from "@fluidframework/tree"; import { SchemaFactoryAlpha } from "@fluidframework/tree/alpha"; import { SharedTree } from "fluid-framework"; @@ -202,11 +201,6 @@ export const INITIAL_APP_STATE = { export const CONTAINER_SCHEMA = { initialObjects: { appState: SharedTree, - /** - * A Presence Manager object temporarily needs to be placed within container schema - * https://github.com/microsoft/FluidFramework/blob/main/packages/framework/presence/README.md#onboarding - * */ - presence: ExperimentalPresenceManager, }, }; diff --git a/examples/apps/presence-tracker/src/app.ts b/examples/apps/presence-tracker/src/app.ts index 4056117d604b..f0c976dd9a81 100644 --- a/examples/apps/presence-tracker/src/app.ts +++ b/examples/apps/presence-tracker/src/app.ts @@ -5,7 +5,9 @@ import { getPresence, + // eslint-disable-next-line import/no-deprecated getPresenceViaDataObject, + // eslint-disable-next-line import/no-deprecated ExperimentalPresenceManager, } from "@fluidframework/presence/alpha"; import { TinyliciousClient } from "@fluidframework/tinylicious-client"; @@ -21,11 +23,11 @@ import { renderControlPanel, renderFocusPresence, renderMousePresence } from "./ // But the old experimental presence data object is used to check that old path still works. // Besides initialObjects is not currently allowed to be empty. // That version of presence is compatible with all 2.x runtimes. Long-term support without -// data object requires 2.32 or later. +// data object requires 2.41 or later. const containerSchema = { initialObjects: { - // A Presence Manager object placed within container schema for experimental presence access - // https://github.com/microsoft/FluidFramework/blob/main/packages/framework/presence/README.md#onboarding + // Optional Presence Manager object placed within container schema for experimental presence access + // eslint-disable-next-line import/no-deprecated presence: ExperimentalPresenceManager, }, } satisfies ContainerSchema; @@ -64,6 +66,7 @@ async function start() { const useDataObject = new URLSearchParams(location.search).has("useDataObject"); const presence = useDataObject ? // Retrieve a reference to the presence APIs via the data object. + // eslint-disable-next-line import/no-deprecated getPresenceViaDataObject(container.initialObjects.presence) : getPresence(container); From 1d0fdddceecd90b02fae9a02e875575499b7fe21 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 23 May 2025 11:00:34 -0700 Subject: [PATCH 20/21] fix(client-fluid-static): fix breaks from merge --- packages/framework/fluid-static/src/index.ts | 1 - packages/framework/fluid-static/src/rootDataObject.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/framework/fluid-static/src/index.ts b/packages/framework/fluid-static/src/index.ts index e0ad5bd564fd..2890dc1ea907 100644 --- a/packages/framework/fluid-static/src/index.ts +++ b/packages/framework/fluid-static/src/index.ts @@ -27,7 +27,6 @@ export type { IMember, IServiceAudience, IServiceAudienceEvents, - IStaticEntryPoint, LoadableObjectRecord, MemberChangedListener, Myself, diff --git a/packages/framework/fluid-static/src/rootDataObject.ts b/packages/framework/fluid-static/src/rootDataObject.ts index ec703ff8e688..6c590ca5555f 100644 --- a/packages/framework/fluid-static/src/rootDataObject.ts +++ b/packages/framework/fluid-static/src/rootDataObject.ts @@ -186,7 +186,7 @@ const rootDataStoreId = "rootDOId"; /** * Creates an {@link @fluidframework/aqueduct#BaseContainerRuntimeFactory} which constructs containers - * with a {@link IStaticEntryPoint} (containing single {@link IRootDataObject}) as their entry point, + * with an entry point containing single IRootDataObject (entry point is opaque to caller), * where the root data object's registry and initial objects are configured based on the provided * schema (and optionally, data store registry). * From 7ec15649fc813e5c6a367a6195703c4ef1f16857 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 23 May 2025 13:19:37 -0700 Subject: [PATCH 21/21] fix(client-example): correct for fluid-static changes --- .../azure-client/external-controller/tests/index.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/examples/service-clients/azure-client/external-controller/tests/index.ts b/examples/service-clients/azure-client/external-controller/tests/index.ts index 8c9646158365..15d300958c7e 100644 --- a/examples/service-clients/azure-client/external-controller/tests/index.ts +++ b/examples/service-clients/azure-client/external-controller/tests/index.ts @@ -9,8 +9,11 @@ import { IRuntimeFactory, } from "@fluidframework/container-definitions/legacy"; import { Loader } from "@fluidframework/container-loader/legacy"; -// eslint-disable-next-line import/no-internal-modules -- #26986: `fluid-static` internal used in examples -import { createDOProviderContainerRuntimeFactory } from "@fluidframework/fluid-static/internal"; +import { + createDOProviderContainerRuntimeFactory, + createFluidContainer, + // eslint-disable-next-line import/no-internal-modules -- #26986: `fluid-static` internal used in examples +} from "@fluidframework/fluid-static/internal"; // eslint-disable-next-line import/no-internal-modules -- #26987: `local-driver` internal used in examples import { LocalSessionStorageDbFactory } from "@fluidframework/local-driver/internal"; import { @@ -119,8 +122,7 @@ async function createContainerAndRenderInElement( ); // Get the Default Object from the Container - const fluidContainer = - (await container.getEntryPoint()) as IFluidContainer; + const fluidContainer = await createFluidContainer({ container }); if (createNewFlag) { await initializeNewContainer(fluidContainer); await attach?.();