Skip to content

Commit

Permalink
Remove dictionary from ILoaderOptions (microsoft#19306)
Browse files Browse the repository at this point in the history
  • Loading branch information
ChumpChief authored and alexvy86 committed Feb 8, 2024
1 parent 40a201f commit 8550935
Show file tree
Hide file tree
Showing 24 changed files with 107 additions and 90 deletions.
16 changes: 16 additions & 0 deletions .changeset/slick-kings-wish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@fluidframework/container-definitions": major
"@fluidframework/container-loader": major
"@fluidframework/container-runtime": major
"@fluidframework/container-runtime-definitions": major
"@fluidframework/datastore": major
"@fluidframework/datastore-definitions": major
"@fluidframework/runtime-definitions": major
"@fluidframework/sequence": major
"@fluid-private/test-end-to-end-tests": major
"@fluidframework/test-runtime-utils": major
---

ILoaderOptions no longer accepts arbitrary key/value pairs

ILoaderOptions has been narrowed to the specific set of supported loader options, and may no longer be used to pass arbitrary key/value pairs through to the runtime.
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,7 @@ export interface IContainerContext {
readonly id: string;
// (undocumented)
readonly loader: ILoader;
// (undocumented)
readonly options: ILoaderOptions;
readonly options: Record<string | number, any>;
// (undocumented)
pendingLocalState?: unknown;
// (undocumented)
Expand Down Expand Up @@ -403,11 +402,11 @@ export interface ILoaderHeader {
[LoaderHeader.version]: string | undefined;
}

// @public (undocumented)
// @alpha
export type ILoaderOptions = {
[key in string | number]: any;
} & {
cache?: boolean;
client?: IClient;
enableOfflineLoad?: boolean;
provideScopeLoader?: boolean;
maxClientLeaveWaitTime?: number;
};
Expand Down
19 changes: 14 additions & 5 deletions packages/common/container-definitions/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import { IRequest, FluidObject, IEvent, IEventProvider } from "@fluidframework/core-interfaces";
import {
IClient,
IClientDetails,
IDocumentMessage,
IQuorumClients,
Expand Down Expand Up @@ -303,7 +304,6 @@ export type ConnectionState =
* The Host's view of a Container and its connection to storage
* @alpha
*/
// eslint-disable-next-line import/no-deprecated
export interface IContainer extends IEventProvider<IContainerEvents> {
/**
* The Delta Manager supporting the op stream for this Container
Expand Down Expand Up @@ -531,12 +531,11 @@ export interface IHostLoader extends ILoader {
}

/**
* @public
* Options to configure various behaviors of the ILoader.
* @alpha
*/
// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
export type ILoaderOptions = {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key in string | number]: any;
} & {
/**
* @deprecated This option has been deprecated and will be removed in a future release
* Set caching behavior for the loader. If true, we will load a container from cache if one
Expand All @@ -548,6 +547,16 @@ export type ILoaderOptions = {
*/
cache?: boolean;

/**
* @deprecated Do not use.
*/
client?: IClient;

/**
* @deprecated Do not use.
*/
enableOfflineLoad?: boolean;

/**
* Provide the current Loader through the scope object when creating Containers. It is added
* as the `ILoader` property, and will overwrite an existing property of the same name on the
Expand Down
11 changes: 9 additions & 2 deletions packages/common/container-definitions/src/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
import { IAudience } from "./audience";
import { IDeltaManager } from "./deltas";
import { ICriticalContainerError } from "./error";
import { ILoader, ILoaderOptions } from "./loader";
import { ILoader } from "./loader";
import { IFluidCodeDetails } from "./fluidPackage";

/**
Expand Down Expand Up @@ -123,7 +123,14 @@ export interface IBatchMessage {
* @alpha
*/
export interface IContainerContext {
readonly options: ILoaderOptions;
/**
* Not recommended for general use, is used in some cases to control various runtime behaviors.
*
* @remarks
* Used to be ILoaderOptions, this is staging for eventual removal.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
readonly options: Record<string | number, any>;
readonly clientId: string | undefined;
readonly clientDetails: IClientDetails;
readonly storage: IDocumentStorageService;
Expand Down
6 changes: 3 additions & 3 deletions packages/dds/sequence/src/sequence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ export abstract class SharedSegmentSequence<T extends ISegment>
.catch((error) => {
this.loadFinished(error);
});
if (this.dataStoreRuntime.options?.sequenceInitializeFromHeaderOnly !== true) {
if (this.dataStoreRuntime.options.sequenceInitializeFromHeaderOnly !== true) {
// if we not doing partial load, await the catch up ops,
// and the finalization of the load
await loadCatchUpOps;
Expand Down Expand Up @@ -807,15 +807,15 @@ export abstract class SharedSegmentSequence<T extends ISegment>
}
const needsTransformation = message.referenceSequenceNumber !== message.sequenceNumber - 1;
let stashMessage: Readonly<ISequencedDocumentMessage> = message;
if (this.runtime.options?.newMergeTreeSnapshotFormat !== true) {
if (this.runtime.options.newMergeTreeSnapshotFormat !== true) {
if (needsTransformation) {
this.on("sequenceDelta", transformOps);
}
}

this.client.applyMsg(message, local);

if (this.runtime.options?.newMergeTreeSnapshotFormat !== true) {
if (this.runtime.options.newMergeTreeSnapshotFormat !== true) {
if (needsTransformation) {
this.removeListener("sequenceDelta", transformOps);
// shallow clone the message as we only overwrite top level properties,
Expand Down
26 changes: 14 additions & 12 deletions packages/loader/container-loader/src/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,7 @@ export class Container
private readonly connectionTransitionTimes: number[] = [];
private _loadedFromVersion: IVersion | undefined;
private _dirtyContainer = false;
private readonly offlineLoadEnabled: boolean;
private readonly savedOps: ISequencedDocumentMessage[] = [];
private attachmentData: AttachmentData = { state: AttachState.Detached };
private readonly _containerId: string;
Expand Down Expand Up @@ -676,12 +677,8 @@ export class Container
return this._clientId;
}

private get offlineLoadEnabled(): boolean {
const enabled =
this.mc.config.getBoolean("Fluid.Container.enableOfflineLoad") ??
this.options?.enableOfflineLoad === true;
// summarizer will not have any pending state we want to save
return enabled && this.deltaManager.clientDetails.capabilities.interactive;
private get isInteractiveClient(): boolean {
return this.deltaManager.clientDetails.capabilities.interactive;
}

/**
Expand Down Expand Up @@ -825,7 +822,7 @@ export class Container

this.client = Container.setupClient(
this._containerId,
this.options,
options.client,
this.clientDetailsOverride,
);

Expand Down Expand Up @@ -942,6 +939,10 @@ export class Container
pendingLocalState?.clientId,
);

this.offlineLoadEnabled =
this.mc.config.getBoolean("Fluid.Container.enableOfflineLoad") ??
options.enableOfflineLoad === true;

this.on(savedContainerEvent, () => {
this.connectionStateHandler.containerSaved();
});
Expand Down Expand Up @@ -1616,7 +1617,8 @@ export class Container
};
} else {
assert(snapshotTree !== undefined, 0x237 /* "Snapshot should exist" */);
if (this.offlineLoadEnabled) {
// non-interactive clients will not have any pending state we want to save
if (this.offlineLoadEnabled && this.isInteractiveClient) {
const blobs = await getBlobContentsFromTree(snapshotTree, this.storageAdapter);
this.attachmentData = {
state: AttachState.Attached,
Expand Down Expand Up @@ -2022,13 +2024,12 @@ export class Container

private static setupClient(
containerId: string,
options?: ILoaderOptions,
loaderOptionsClient?: IClient,
clientDetailsOverride?: IClientDetails,
): IClient {
const loaderOptionsClient = structuredClone(options?.client);
const client: IClient =
loaderOptionsClient !== undefined
? (loaderOptionsClient as IClient)
? structuredClone(loaderOptionsClient)
: {
details: {
capabilities: { interactive: true },
Expand Down Expand Up @@ -2333,7 +2334,8 @@ export class Container
}

private processRemoteMessage(message: ISequencedDocumentMessage) {
if (this.offlineLoadEnabled) {
// non-interactive clients will not have any pending state we want to save
if (this.offlineLoadEnabled && this.isInteractiveClient) {
this.savedOps.push(message);
}
const local = this.clientId === message.clientId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { IEventProvider } from '@fluidframework/core-interfaces';
import { IFluidDataStoreContextDetached } from '@fluidframework/runtime-definitions';
import { IFluidHandle } from '@fluidframework/core-interfaces';
import { IFluidHandleContext } from '@fluidframework/core-interfaces';
import { ILoaderOptions } from '@fluidframework/container-definitions';
import { IProvideFluidDataStoreRegistry } from '@fluidframework/runtime-definitions';
import { IRequest } from '@fluidframework/core-interfaces';
import { IResponse } from '@fluidframework/core-interfaces';
Expand All @@ -41,7 +40,7 @@ export interface IContainerRuntime extends IProvideFluidDataStoreRegistry, ICont
getAliasedDataStoreEntryPoint(alias: string): Promise<IFluidHandle<FluidObject> | undefined>;
readonly isDirty: boolean;
// (undocumented)
readonly options: ILoaderOptions;
readonly options: Record<string | number, any>;
// (undocumented)
readonly scope: FluidObject;
// (undocumented)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License.
*/

import { AttachState, IDeltaManager, ILoaderOptions } from "@fluidframework/container-definitions";
import { AttachState, IDeltaManager } from "@fluidframework/container-definitions";
import {
IEventProvider,
IRequest,
Expand Down Expand Up @@ -57,7 +57,8 @@ export type IContainerRuntimeBaseWithCombinedEvents = IContainerRuntimeBase &
export interface IContainerRuntime
extends IProvideFluidDataStoreRegistry,
IContainerRuntimeBaseWithCombinedEvents {
readonly options: ILoaderOptions;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
readonly options: Record<string | number, any>;
readonly clientId: string | undefined;
readonly clientDetails: IClientDetails;
readonly connected: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import { IGarbageCollectionData } from '@fluidframework/runtime-definitions';
import { IGetPendingLocalStateProps } from '@fluidframework/container-definitions';
import type { IIdCompressor } from '@fluidframework/id-compressor';
import type { IIdCompressorCore } from '@fluidframework/id-compressor';
import { ILoaderOptions } from '@fluidframework/container-definitions';
import { IProvideFluidHandleContext } from '@fluidframework/core-interfaces';
import { IQuorumClients } from '@fluidframework/protocol-definitions';
import { IRequest } from '@fluidframework/core-interfaces';
Expand Down Expand Up @@ -180,7 +179,7 @@ export class ContainerRuntime extends TypedEventEmitter<IContainerRuntimeEvents
// (undocumented)
notifyOpReplay(message: ISequencedDocumentMessage): Promise<void>;
// (undocumented)
readonly options: ILoaderOptions;
readonly options: Record<string | number, any>;
// (undocumented)
orderSequentially<T>(callback: () => T): T;
// (undocumented)
Expand Down
7 changes: 4 additions & 3 deletions packages/runtime/container-runtime/src/containerRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
IRuntime,
ICriticalContainerError,
AttachState,
ILoaderOptions,
ILoader,
LoaderHeader,
IGetPendingLocalStateProps,
Expand Down Expand Up @@ -934,7 +933,7 @@ export class ContainerRuntime
return runtime;
}

public readonly options: ILoaderOptions;
public readonly options: Record<string | number, any>;
private imminentClosure: boolean = false;

private readonly _getClientId: () => string | undefined;
Expand Down Expand Up @@ -1245,7 +1244,9 @@ export class ContainerRuntime
this.submitSummaryFn = submitSummaryFn;
this.submitSignalFn = submitSignalFn;

this.options = options;
// 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.
this.options = options ?? {};
this.clientDetails = clientDetails;
this.isSummarizerClient = this.clientDetails.type === summarizerClientType;
this.loadedFromVersionId = context.getLoadedFromVersion()?.id;
Expand Down
10 changes: 3 additions & 7 deletions packages/runtime/container-runtime/src/dataStoreContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,7 @@ import {
IFluidHandle,
ITelemetryProperties,
} from "@fluidframework/core-interfaces";
import {
IAudience,
IDeltaManager,
AttachState,
ILoaderOptions,
} from "@fluidframework/container-definitions";
import { IAudience, IDeltaManager, AttachState } from "@fluidframework/container-definitions";
import { TypedEventEmitter } from "@fluid-internal/client-utils";
import { assert, Deferred, LazyPromise } from "@fluidframework/core-utils";
import { IDocumentStorageService } from "@fluidframework/driver-definitions";
Expand Down Expand Up @@ -152,7 +147,8 @@ export abstract class FluidDataStoreContext
return this.pkg;
}

public get options(): ILoaderOptions {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
public get options(): Record<string | number, any> {
return this._containerRuntime.options;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { IFluidLoadable } from '@fluidframework/core-interfaces';
import { IGarbageCollectionData } from '@fluidframework/runtime-definitions';
import { IIdCompressor } from '@fluidframework/id-compressor';
import { IInboundSignalMessage } from '@fluidframework/runtime-definitions';
import { ILoaderOptions } from '@fluidframework/container-definitions';
import { IQuorumClients } from '@fluidframework/protocol-definitions';
import { ISequencedDocumentMessage } from '@fluidframework/protocol-definitions';
import { ISummaryTreeWithStats } from '@fluidframework/runtime-definitions';
Expand Down Expand Up @@ -117,7 +116,7 @@ export interface IFluidDataStoreRuntime extends IEventProvider<IFluidDataStoreRu
// (undocumented)
readonly objectsRoutingContext: IFluidHandleContext;
// (undocumented)
readonly options: ILoaderOptions;
readonly options: Record<string | number, any>;
// (undocumented)
readonly rootRoutingContext: IFluidHandleContext;
submitSignal(type: string, content: any, targetClientId?: string): void;
Expand Down
10 changes: 3 additions & 7 deletions packages/runtime/datastore-definitions/src/dataStoreRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@ import {
IFluidHandle,
FluidObject,
} from "@fluidframework/core-interfaces";
import {
IAudience,
IDeltaManager,
AttachState,
ILoaderOptions,
} from "@fluidframework/container-definitions";
import { IAudience, IDeltaManager, AttachState } from "@fluidframework/container-definitions";
import {
IDocumentMessage,
IQuorumClients,
Expand Down Expand Up @@ -53,7 +48,8 @@ export interface IFluidDataStoreRuntime
readonly channelsRoutingContext: IFluidHandleContext;
readonly objectsRoutingContext: IFluidHandleContext;

readonly options: ILoaderOptions;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
readonly options: Record<string | number, any>;

readonly deltaManager: IDeltaManager<ISequencedDocumentMessage, IDocumentMessage>;

Expand Down
3 changes: 1 addition & 2 deletions packages/runtime/datastore/api-report/datastore.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { IFluidHandleContext } from '@fluidframework/core-interfaces';
import { IGarbageCollectionData } from '@fluidframework/runtime-definitions';
import { IIdCompressor } from '@fluidframework/id-compressor';
import { IInboundSignalMessage } from '@fluidframework/runtime-definitions';
import { ILoaderOptions } from '@fluidframework/container-definitions';
import { IQuorumClients } from '@fluidframework/protocol-definitions';
import { IRequest } from '@fluidframework/core-interfaces';
import { IResponse } from '@fluidframework/core-interfaces';
Expand Down Expand Up @@ -96,7 +95,7 @@ export class FluidDataStoreRuntime extends TypedEventEmitter<IFluidDataStoreRunt
// (undocumented)
get objectsRoutingContext(): this;
// (undocumented)
readonly options: ILoaderOptions;
readonly options: Record<string | number, any>;
// (undocumented)
process(message: ISequencedDocumentMessage, local: boolean, localOpMetadata: unknown): void;
// (undocumented)
Expand Down
Loading

0 comments on commit 8550935

Please sign in to comment.