From 218663f447d5e73b80abd56cc9e4f70f2301fa6d Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 5 Jun 2026 11:48:46 -0700 Subject: [PATCH] improvement(client-ordered-collection): ConsensusQueueFactory generic - Remove unused `IConsensusOrderedCollectionFactory` - Make `ConsensusQueueFactory` generic (assumes `unknown`) - Get specific on `create` & `load` return types - Remove most intended internal class use in test --- .../src/consensusOrderedCollectionFactory.ts | 44 ++++++++++++++----- packages/dds/ordered-collection/src/index.ts | 1 - .../dds/ordered-collection/src/interfaces.ts | 24 ---------- .../test/consensusOrderedCollection.spec.ts | 42 +++++++++++------- .../ordered-collection/src/test/fuzzUtils.ts | 4 +- 5 files changed, 60 insertions(+), 55 deletions(-) diff --git a/packages/dds/ordered-collection/src/consensusOrderedCollectionFactory.ts b/packages/dds/ordered-collection/src/consensusOrderedCollectionFactory.ts index 0dc83875a1fc..6c3fa133e60a 100644 --- a/packages/dds/ordered-collection/src/consensusOrderedCollectionFactory.ts +++ b/packages/dds/ordered-collection/src/consensusOrderedCollectionFactory.ts @@ -5,16 +5,18 @@ import type { IChannelAttributes, - IFluidDataStoreRuntime, + IChannelFactory, IChannelServices, + IFluidDataStoreRuntime, } from "@fluidframework/datastore-definitions/internal"; +import type { SharedObjectCore } from "@fluidframework/shared-object-base/internal"; import { createSharedObjectKind } from "@fluidframework/shared-object-base/internal"; // eslint-disable-next-line import-x/no-deprecated -- Internal usage of deprecated class in factory import { ConsensusQueueClass } from "./consensusQueue.js"; import type { IConsensusOrderedCollection, - IConsensusOrderedCollectionFactory, + IConsensusOrderedCollectionEvents, } from "./interfaces.js"; import { pkgVersion } from "./packageVersion.js"; @@ -23,7 +25,9 @@ import { pkgVersion } from "./packageVersion.js"; * * @internal */ -export class ConsensusQueueFactory implements IConsensusOrderedCollectionFactory { +export class ConsensusQueueFactory + implements IChannelFactory> +{ // New type string, to be activated once the migration has been fully shipped dark and is safe to flip. // See LegacyTypeAwareRegistry in packages/runtime/datastore/src/dataStoreRuntime.ts. // public static Type = "consensus-queue"; @@ -43,24 +47,26 @@ export class ConsensusQueueFactory implements IConsensusOrderedCollectionFactory return ConsensusQueueFactory.Attributes; } - /** - * {@inheritDoc @fluidframework/datastore-definitions#IChannelFactory.load} - */ public async load( runtime: IFluidDataStoreRuntime, id: string, services: IChannelServices, attributes: IChannelAttributes, - ): Promise { + ): Promise< + IConsensusOrderedCollection & SharedObjectCore> + > { // eslint-disable-next-line import-x/no-deprecated -- Internal usage of deprecated class - const collection = new ConsensusQueueClass(id, runtime, attributes); + const collection = new ConsensusQueueClass(id, runtime, attributes); await collection.load(services); return collection; } - public create(document: IFluidDataStoreRuntime, id: string): IConsensusOrderedCollection { + public create( + document: IFluidDataStoreRuntime, + id: string, + ): IConsensusOrderedCollection & SharedObjectCore> { // eslint-disable-next-line import-x/no-deprecated -- Internal usage of deprecated class - const collection = new ConsensusQueueClass(id, document, this.attributes); + const collection = new ConsensusQueueClass(id, document, this.attributes); collection.initializeLocal(); return collection; } @@ -70,7 +76,23 @@ export class ConsensusQueueFactory implements IConsensusOrderedCollectionFactory * {@inheritDoc ConsensusQueueClass} * @legacy @beta */ -export const ConsensusQueue = createSharedObjectKind(ConsensusQueueFactory); +export const ConsensusQueue = createSharedObjectKind( + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- TODO: #22835 Use unknown instead of any (breaking change) + ConsensusQueueFactory, +); + +// For #22835, consider exposing a helper to produce type specific ConsensusQueue. +// One problem, with doing so is that nothing inside ConsensusQueue validates the shape of T. +// A DDS load might find content that is not the same, expected type. +// But that is not any worse than the `any` that exists currently. +// /** +// * Factory for shared object kind for queue-shaped ordered collections of specific type. +// * @legacy @beta +// */ +// export function TypedConsensusQueue(): ISharedObjectKind> & +// SharedObjectKind> { +// return ConsensusQueue; +// } /** * {@inheritDoc ConsensusQueueClass} diff --git a/packages/dds/ordered-collection/src/index.ts b/packages/dds/ordered-collection/src/index.ts index a9928471bd51..655f8917de19 100644 --- a/packages/dds/ordered-collection/src/index.ts +++ b/packages/dds/ordered-collection/src/index.ts @@ -8,7 +8,6 @@ export { ConsensusResult, type IConsensusOrderedCollection, type IConsensusOrderedCollectionEvents, - type IConsensusOrderedCollectionFactory, type IOrderedCollection, type ISnapshotable, } from "./interfaces.js"; diff --git a/packages/dds/ordered-collection/src/interfaces.ts b/packages/dds/ordered-collection/src/interfaces.ts index 904f66869771..30a83f272a90 100644 --- a/packages/dds/ordered-collection/src/interfaces.ts +++ b/packages/dds/ordered-collection/src/interfaces.ts @@ -3,12 +3,6 @@ * Licensed under the MIT License. */ -import type { - IChannelAttributes, - IChannelFactory, - IFluidDataStoreRuntime, - IChannelServices, -} from "@fluidframework/datastore-definitions/internal"; import type { ISharedObject, ISharedObjectEvents, @@ -29,24 +23,6 @@ export enum ConsensusResult { */ export type ConsensusCallback = (value: T) => Promise; -/** - * Consensus Ordered Collection channel factory interface - * - * Extends the base IChannelFactory to return a more definite type of IConsensusOrderedCollection - * Use for the runtime to create and load distributed data structure by type name of each channel - * @internal - */ -export interface IConsensusOrderedCollectionFactory extends IChannelFactory { - load( - document: IFluidDataStoreRuntime, - id: string, - services: IChannelServices, - attributes: IChannelAttributes, - ): Promise; - - create(document: IFluidDataStoreRuntime, id: string): IConsensusOrderedCollection; -} - /** * Events notifying about addition, acquisition, release and completion of items * @legacy @beta diff --git a/packages/dds/ordered-collection/src/test/consensusOrderedCollection.spec.ts b/packages/dds/ordered-collection/src/test/consensusOrderedCollection.spec.ts index f8fc6d08b455..27c34f94ae44 100644 --- a/packages/dds/ordered-collection/src/test/consensusOrderedCollection.spec.ts +++ b/packages/dds/ordered-collection/src/test/consensusOrderedCollection.spec.ts @@ -6,23 +6,25 @@ import { strict as assert } from "node:assert"; import { type IGCTestProvider, runGCTests } from "@fluid-private/test-dds-utils"; -import type { IFluidHandleInternal } from "@fluidframework/core-interfaces/internal"; -import type { IChannelServices } from "@fluidframework/datastore-definitions/internal"; +import type { IFluidHandleInternal } from "@fluidframework/core-interfaces/legacy"; +import type { IChannelServices } from "@fluidframework/datastore-definitions/legacy"; +import type { SharedObjectCore } from "@fluidframework/shared-object-base/legacy"; import { MockContainerRuntimeFactory, MockContainerRuntimeFactoryForReconnection, type MockContainerRuntimeForReconnection, MockFluidDataStoreRuntime, MockStorage, -} from "@fluidframework/test-runtime-utils/internal"; +} from "@fluidframework/test-runtime-utils/legacy"; -import { ConsensusOrderedCollection } from "../consensusOrderedCollection.js"; -import { - ConsensusQueueFactory, - type ConsensusQueue, -} from "../consensusOrderedCollectionFactory.js"; +import type { + IConsensusOrderedCollection, + IConsensusOrderedCollectionEvents, +} from "@fluidframework/ordered-collection/legacy"; +import { ConsensusResult } from "@fluidframework/ordered-collection/legacy"; + +import { ConsensusQueueFactory } from "../consensusOrderedCollectionFactory.js"; import { ConsensusQueueClass } from "../consensusQueue.js"; -import { ConsensusResult, type IConsensusOrderedCollection } from "../interfaces.js"; import { acquireAndComplete, acquireAndRelease, @@ -32,7 +34,7 @@ import { /** * Test class that exposes protected applyStashedOp method for testing */ -class TestConsensusQueue extends ConsensusQueueClass { +class TestConsensusQueue extends ConsensusQueueClass { public testApplyStashedOp(content: unknown): void { this.applyStashedOp(content); } @@ -41,7 +43,8 @@ class TestConsensusQueue extends ConsensusQueueClass { function createConnectedCollection( id: string, runtimeFactory: MockContainerRuntimeFactory, -): ConsensusQueue { +): IConsensusOrderedCollection & + SharedObjectCore> { const dataStoreRuntime = new MockFluidDataStoreRuntime(); runtimeFactory.createContainerRuntime(dataStoreRuntime); const services: IChannelServices = { @@ -52,12 +55,15 @@ function createConnectedCollection( const factory = new ConsensusQueueFactory(); const testCollection = factory.create(dataStoreRuntime, id); testCollection.connect(services); - return testCollection as ConsensusQueue; + return testCollection; } -function createLocalCollection(id: string): ConsensusQueue { +function createLocalCollection( + id: string, +): IConsensusOrderedCollection & + SharedObjectCore> { const factory = new ConsensusQueueFactory(); - return factory.create(new MockFluidDataStoreRuntime(), id) as ConsensusQueue; + return factory.create(new MockFluidDataStoreRuntime(), id); } function createCollectionForReconnection( @@ -104,10 +110,12 @@ describe("ConsensusOrderedCollection", () => { function generate( input: unknown[], output: unknown[], - creator: () => ConsensusOrderedCollection, + creator: () => IConsensusOrderedCollection & + SharedObjectCore>, processMessages: () => void, ): void { - let testCollection: ConsensusOrderedCollection; + let testCollection: IConsensusOrderedCollection & + SharedObjectCore>; async function removeItem(): Promise { const resP = acquireAndComplete(testCollection); @@ -155,7 +163,7 @@ describe("ConsensusOrderedCollection", () => { const acquiredValue = (await removeItem()) as IFluidHandleInternal; assert.strictEqual(acquiredValue.absolutePath, handle.absolutePath); - const dataStore = (await handle.get()) as ConsensusQueue; + const dataStore = (await handle.get()) as ConsensusQueueClass; assert.strictEqual(dataStore.handle.absolutePath, testCollection.handle.absolutePath); assert.strictEqual(await removeItem(), undefined); diff --git a/packages/dds/ordered-collection/src/test/fuzzUtils.ts b/packages/dds/ordered-collection/src/test/fuzzUtils.ts index 53652a80e098..f7c4f7eb75bc 100644 --- a/packages/dds/ordered-collection/src/test/fuzzUtils.ts +++ b/packages/dds/ordered-collection/src/test/fuzzUtils.ts @@ -60,7 +60,7 @@ export const defaultOptions: Partial = { saveFailures: { directory: path.join(_dirname, "../../src/test/results") }, }; -type FuzzTestState = DDSFuzzTestState; +type FuzzTestState = DDSFuzzTestState>; export interface AddOperation { type: "add"; @@ -180,7 +180,7 @@ function assertEqualConsensusOrderedCollections( * Base fuzz model for ConsensusOrderedCollection */ export const baseConsensusOrderedCollectionModel: DDSFuzzModel< - ConsensusQueueFactory, + ConsensusQueueFactory, ConsensusOrderedCollectionOperation, FuzzTestState > = {