improvement(client): Cleanup presence manager interface (#22553)

to distinguish internal needs and external uses. IPresenceManager is removed (was accidentally duplicated) and became three distinct parts: - IPresence (public user interface) - PresenceManagerInternal (needs of internal presence sub-components) - PresenceExtensionInterface (for future container hostable) - Push `on("signal"` up to temp DataObject and use `processSignal` for both paths. - `submitSignal` still needs managed. Relocate IEphemeralRuntime to internalTypes.ts to help avoid circular dependencies.
2024-09-21 11:34:05 -07:00 · 2024-09-21 11:34:05 -07:00 · e7a2f48d73
--- a/packages/framework/presence/src/container-definitions/containerExtensions.ts
+++ b/packages/framework/presence/src/container-definitions/containerExtensions.ts
@ -113,16 +113,16 @@ export interface IExtensionRuntime {
 * @param runtime - Runtime for extension to work against
 * @param context - Custom context for extension.
 * @returns Record providing:
- * `extension` instance (type `T`) that is provided to caller of
+ * `interface` instance (type `T`) that is provided to caller of
 * {@link ContainerExtensionStore.acquireExtension} and
- * `interface` store/runtime uses to interact with extension.
+ * `extension` store/runtime uses to interact with extension.
 *
 * @internal
 */
 export type ContainerExtensionFactory<T, TContext extends unknown[]> = new (
 	runtime: IExtensionRuntime,
 	...context: TContext
-) => { readonly extension: T; readonly interface: IContainerExtension<TContext> };
+) => { readonly interface: T; readonly extension: IContainerExtension<TContext> };

 /**
 * Unique identifier for extension
--- a/packages/framework/presence/src/datastorePresenceManagerFactory.ts
+++ b/packages/framework/presence/src/datastorePresenceManagerFactory.ts
@ -8,12 +8,24 @@
 */

 import type { IFluidLoadable } from "@fluidframework/core-interfaces";
+import { assert } from "@fluidframework/core-utils/internal";
+import type { IInboundSignalMessage } from "@fluidframework/runtime-definitions/internal";
 import type { SharedObjectKind } from "@fluidframework/shared-object-base";

 import { BasicDataStoreFactory, LoadableFluidObject } from "./datastoreSupport.js";
 import type { IPresence } from "./presence.js";
 import { createPresenceManager } from "./presenceManager.js";

+import type { IExtensionMessage } from "@fluid-experimental/presence/internal/container-definitions/internal";
+
+function assertSignalMessageIsValid(
+	message: IInboundSignalMessage | IExtensionMessage,
+): asserts message is IExtensionMessage {
+	assert(message.clientId !== null, "Signal must have a client ID");
+	// The other difference between messages is that `content` for
+	// IExtensionMessage is JsonDeserialized and we are fine assuming that.
+}
+
 /**
 * Simple FluidObject holding Presence Manager.
 */
@ -26,7 +38,12 @@ 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.
-			this._presenceManager = createPresenceManager(this.runtime);
+			const manager = createPresenceManager(this.runtime);
+			this.runtime.on("signal", (message: IInboundSignalMessage, local: boolean) => {
+				assertSignalMessageIsValid(message);
+				manager.processSignal("", message, local);
+			});
+			this._presenceManager = manager;
 		}
 		return this._presenceManager;
 	}
--- a/packages/framework/presence/src/experimentalAccess.ts
+++ b/packages/framework/presence/src/experimentalAccess.ts
@ -9,8 +9,9 @@ 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 { IPresence } from "./presence.js";
-import type { IEphemeralRuntime } from "./presenceDatastoreManager.js";
+import type { PresenceExtensionInterface } from "./presenceManager.js";
 import { createPresenceManager } from "./presenceManager.js";

 import type {
@ -26,23 +27,19 @@ function isContainerExtensionStore(
 	return (manager as ContainerExtensionStore).acquireExtension !== undefined;
 }

-/**
- * @internal
- */
-export interface IPresenceManager
-	extends IPresence,
-		Pick<Required<IContainerExtension<[]>>, "processSignal"> {}
-
 /**
 * Common Presence manager for a container
 */
 class ContainerPresenceManager implements IContainerExtension<never> {
-	public readonly extension: IPresenceManager;
-	public readonly interface = this;
+	public readonly interface: IPresence;
+	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.extension = createPresenceManager(runtime as unknown as IEphemeralRuntime);
+		this.interface = this.manager = createPresenceManager(
+			runtime as unknown as IEphemeralRuntime,
+		);
 	}

 	public onNewContext(): void {
@ -52,7 +49,7 @@ class ContainerPresenceManager implements IContainerExtension<never> {
 	public static readonly extensionId = "dis:bb89f4c0-80fd-4f0c-8469-4f2848ee7f4a";

 	public processSignal(address: string, message: IExtensionMessage, local: boolean): void {
-		this.extension.processSignal(address, message, local);
+		this.manager.processSignal(address, message, local);
 	}
 }

@ -75,9 +72,9 @@ export function acquirePresence(fluidContainer: IFluidContainer): IPresence {
 		0xa39 /* Container does not support extensions. Use acquirePresenceViaDataObject. */,
 	);

-	const pm = innerContainer.acquireExtension(
+	const presence = innerContainer.acquireExtension(
 		ContainerPresenceManager.extensionId,
 		ContainerPresenceManager,
 	);
-	return pm;
+	return presence;
 }
--- a/packages/framework/presence/src/internalTypes.ts
+++ b/packages/framework/presence/src/internalTypes.ts
@ -3,8 +3,13 @@
 * Licensed under the MIT License.
 */

+import type { IContainerRuntime } from "@fluidframework/container-runtime-definitions/internal";
+import type { IFluidDataStoreRuntime } from "@fluidframework/datastore-definitions/internal";
+
 import type { InternalTypes } from "./exposedInternalTypes.js";
-import type { ClientSessionId, ISessionClient } from "./presence.js";
+import type { ClientSessionId, IPresence, ISessionClient } from "./presence.js";
+
+import type { IRuntimeInternal } from "@fluid-experimental/presence/internal/container-definitions/internal";

 /**
 * @internal
@ -27,6 +32,27 @@ export const brandedObjectEntries = Object.entries as <K extends string, T>(
 	o: Record<K, T>,
 ) => [K, T][];

+/**
+ * This interface is a subset of (IContainerRuntime & IRuntimeInternal) and
+ * (IFluidDataStoreRuntime) that is needed by the Presence States.
+ *
+ * @privateRemarks
+ * Replace with non-DataStore based interface.
+ *
+ * @internal
+ */
+export type IEphemeralRuntime = Pick<
+	(IContainerRuntime & IRuntimeInternal) | IFluidDataStoreRuntime,
+	"clientId" | "connected" | "getQuorum" | "off" | "on" | "submitSignal"
+>;
+
+/**
+ * Collection of utilities provided by PresenceManager that are used by presence sub-components.
+ *
+ * @internal
+ */
+export type PresenceManagerInternal = Pick<IPresence, "getAttendee">;
+
 /**
 * @internal
 */
--- a/packages/framework/presence/src/presenceDatastoreManager.ts
+++ b/packages/framework/presence/src/presenceDatastoreManager.ts
@ -3,14 +3,13 @@
 * Licensed under the MIT License.
 */

-import type { IContainerRuntime } from "@fluidframework/container-runtime-definitions/internal";
 import { assert } from "@fluidframework/core-utils/internal";
-import type { IFluidDataStoreRuntime } from "@fluidframework/datastore-definitions/internal";
 import type { IInboundSignalMessage } from "@fluidframework/runtime-definitions/internal";

 import type { ClientConnectionId } from "./baseTypes.js";
 import type { InternalTypes } from "./exposedInternalTypes.js";
-import type { ClientSessionId, IPresence } from "./presence.js";
+import type { IEphemeralRuntime, PresenceManagerInternal } from "./internalTypes.js";
+import type { ClientSessionId } from "./presence.js";
 import type {
 	ClientUpdateEntry,
 	PresenceStatesInternal,
@ -19,7 +18,7 @@ import type {
 import { createPresenceStates, mergeUntrackedDatastore } from "./presenceStates.js";
 import type { PresenceStates, PresenceStatesSchema } from "./types.js";

-import type { IRuntimeInternal } from "@fluid-experimental/presence/internal/container-definitions/internal";
+import type { IExtensionMessage } from "@fluid-experimental/presence/internal/container-definitions/internal";

 interface PresenceStatesEntry<TSchema extends PresenceStatesSchema> {
 	public: PresenceStates<TSchema>;
@ -78,19 +77,6 @@ function isPresenceMessage(
 	return message.type.startsWith("Pres:");
 }

-/**
- * This interface is a subset of (IContainerRuntime & IRuntimeInternal) and (IFluidDataStoreRuntime) that is needed by the PresenceStates.
- *
- * @privateRemarks
- * Replace with non-DataStore based interface.
- *
- * @internal
- */
-export type IEphemeralRuntime = Pick<
-	(IContainerRuntime & IRuntimeInternal) | IFluidDataStoreRuntime,
-	"clientId" | "connected" | "getQuorum" | "off" | "on" | "submitSignal"
->;
-
 /**
 * @internal
 */
@ -99,7 +85,7 @@ export interface PresenceDatastoreManager {
 		internalWorkspaceAddress: string,
 		requestedContent: TSchema,
 	): PresenceStates<TSchema>;
-	processSignal(message: IInboundSignalMessage, local: boolean): void;
+	processSignal(message: IExtensionMessage, local: boolean): void;
 }

 /**
@ -118,10 +104,9 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager {
 	public constructor(
 		private readonly clientSessionId: ClientSessionId,
 		private readonly runtime: IEphemeralRuntime,
-		private readonly presence: IPresence,
+		private readonly presence: PresenceManagerInternal,
 	) {
 		runtime.on("connected", this.onConnect.bind(this));
-		runtime.on("signal", this.processSignal.bind(this));

 		// Check if already connected at the time of construction.
 		// If constructed during data store load, the runtime may already be connected
@ -232,6 +217,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
+		// issues with `ClientSessionId` keys and really unknown value content.
+		// IExtensionMessage is a subset of IInboundSignalMessage so this is safe.
+		// Change types of DatastoreUpdateMessage | ClientJoinMessage to
+		// IExtensionMessage<> derivatives to see the issues.
 		message: IInboundSignalMessage | DatastoreUpdateMessage | ClientJoinMessage,
 		local: boolean,
 	): void {
--- a/packages/framework/presence/src/presenceManager.ts
+++ b/packages/framework/presence/src/presenceManager.ts
@ -6,16 +6,14 @@
 import { createSessionId } from "@fluidframework/id-compressor/internal";

 import type { ClientConnectionId } from "./baseTypes.js";
+import type { IEphemeralRuntime, PresenceManagerInternal } from "./internalTypes.js";
 import type {
 	ClientSessionId,
 	IPresence,
 	ISessionClient,
 	PresenceEvents,
 } from "./presence.js";
-import type {
-	IEphemeralRuntime,
-	PresenceDatastoreManager,
-} from "./presenceDatastoreManager.js";
+import type { PresenceDatastoreManager } from "./presenceDatastoreManager.js";
 import { PresenceDatastoreManagerImpl } from "./presenceDatastoreManager.js";
 import type {
 	PresenceStates,
@ -30,16 +28,20 @@ import type {
 import { createEmitter } from "@fluid-experimental/presence/internal/events";

 /**
+ * Portion of the container extension requirements ({@link IContainerExtension}) that are delegated to presence manager.
+ *
 * @internal
 */
-export interface IPresenceManager
-	extends IPresence,
-		Pick<Required<IContainerExtension<[]>>, "processSignal"> {}
+export type PresenceExtensionInterface = Required<
+	Pick<IContainerExtension<never>, "processSignal">
+>;

 /**
 * The Presence manager
 */
-class PresenceManager implements IPresenceManager {
+class PresenceManager
+	implements IPresence, PresenceExtensionInterface, PresenceManagerInternal
+{
 	private readonly datastoreManager: PresenceDatastoreManager;
 	private readonly selfAttendee: ISessionClient = {
 		sessionId: createSessionId() as ClientSessionId,
@ -133,82 +135,8 @@ class PresenceManager implements IPresenceManager {
 *
 * @internal
 */
-export function createPresenceManager(runtime: IEphemeralRuntime): IPresenceManager {
+export function createPresenceManager(
+	runtime: IEphemeralRuntime,
+): IPresence & PresenceExtensionInterface {
 	return new PresenceManager(runtime);
 }
-
-// ============================================================================
-// This demonstrates pattern where PresenceStates creation uses a ctor and allows
-// instanceof verification for new requests.
-//
-// /**
-//  * @internal
-//  */
-// export type PresenceStatesFactory<TSchema, T> = new (
-// 	containerRuntime: IContainerRuntime & IRuntimeInternal,
-// 	initialContent: TSchema,
-// ) => PresenceStatesEntry<TSchema, T>;
-
-// class PresenceStatesEntry<TSchema extends PresenceStatesSchema>
-// 	implements InstanceType<PresenceStatesFactory<TSchema, PresenceStates<TSchema>>>
-// {
-// 	public readonly map: PresenceStates<TSchema>;
-// 	public readonly processSignal: (signal: IInboundSignalMessage, local: boolean) => void;
-// 	public readonly ensureContent: (content: TSchema) => void;
-
-// 	public constructor(
-// 		runtime: IEphemeralRuntime,
-// 		initialContent: TSchema,
-// 	) {
-// 		const { public, internal } = createPresenceStates(
-// 			this,
-// 			runtime,
-// 			initialContent,
-// 		);
-// 		this.map = public;
-// 		this.processSignal = internal.processSignal.bind(internal);
-// 		this.ensureContent = internal.ensureContent.bind(internal);
-// 	}
-// }
-
-// export class PresenceManager implements IContainerExtension<never> {
-// 	public readonly extension: IPresenceManager = this;
-// 	public readonly interface = this;
-
-// 	public constructor(private readonly runtime: IExtensionRuntime) {}
-
-// 	public onNewContext(): void {
-// 		// No-op
-// 	}
-
-// 	static readonly extensionId = "dis:bb89f4c0-80fd-4f0c-8469-4f2848ee7f4a";
-// 	private readonly maps = new Map<string, PresenceStatesEntry<unknown, unknown>>();
-
-// 	/**
-// 	 * Acquires an Presence Workspace from store or adds new one.
-// 	 *
-// 	 * @param mapAddress - Address of the requested Presence Workspace
-// 	 * @param factory - Factory to create the Presence Workspace if not found
-// 	 * @returns The Presence Workspace
-// 	 */
-// 	public acquirePresenceStates<
-// 		T extends PresenceStatesFacade<unknown>,
-// 		TSchema = T extends PresenceStatesFacade<infer _TSchema> ? _TSchema : never,
-// 	>(
-// 		containerRuntime: IContainerRuntime & IRuntimeInternal,
-// 		mapAddress: PresenceWorkspaceAddress,
-// 		requestedContent: TSchema,
-// 		factoryFacade: PresenceStatesFactoryFacade<T>,
-// 	): T {
-// 		const factory = factoryFacade as unknown as PresenceStatesFactory<TSchema, T>;
-// 		let existing = this.maps.get(mapAddress);
-// 		if (existing) {
-// 			assert(existing instanceof factory, "Existing PresenceStates is not of the expected type");
-// 			return existing.ensureContent(requestedContent);
-// 		}
-//		// TODO create the appropriate ephemeral runtime (map address must be in submitSignal, etc.)
-// 		const entry = new factory(containerRuntime, requestedContent);
-// 		this.maps.set(mapAddress, entry);
-// 		return entry.public;
-// 	}
-// }