From c19924ddf83bed1a2e4c5acdc6c7b3ce3fe011d8 Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Thu, 10 Apr 2025 10:39:59 -0700 Subject: [PATCH 1/7] Add read support for placeholder handles --- .../common/core-interfaces/src/handles.ts | 16 ++++++ packages/common/core-interfaces/src/index.ts | 1 + .../dds/shared-object-base/src/serializer.ts | 2 +- .../src/test/serializer.spec.ts | 24 +++++++-- .../container-runtime.legacy.alpha.api.md | 1 + .../src/blobManager/blobManager.ts | 49 +++++++++++++++---- .../container-runtime/src/containerRuntime.ts | 22 ++++++++- .../src/summary/documentSchema.ts | 3 ++ .../src/test/blobManager.spec.ts | 9 +++- .../src/test/blobManager.stashed.spec.ts | 1 + .../src/test/containerRuntime.spec.ts | 1 + .../src/test/documentSchema.spec.ts | 25 ++++++++-- packages/runtime/runtime-utils/src/handles.ts | 38 ++++++++++++-- packages/runtime/runtime-utils/src/index.ts | 1 + .../src/remoteFluidObjectHandle.ts | 6 ++- packages/runtime/runtime-utils/src/utils.ts | 4 ++ .../test-service-load/src/optionsMatrix.ts | 1 + 17 files changed, 177 insertions(+), 27 deletions(-) diff --git a/packages/common/core-interfaces/src/handles.ts b/packages/common/core-interfaces/src/handles.ts index b3bf3361a9af..411e03a66aea 100644 --- a/packages/common/core-interfaces/src/handles.ts +++ b/packages/common/core-interfaces/src/handles.ts @@ -101,6 +101,22 @@ export interface IFluidHandleInternal< bind(handle: IFluidHandleInternal): void; } +/** + * @privateRemarks + * To be merged onto IFluidHandleInternal in accordance with breaking change policy + * @internal + */ +export interface IFluidHandleInternalPlaceholder< + // REVIEW: Constrain `T` to something? How do we support dds and datastores safely? + out T = unknown, // FluidObject & IFluidLoadable, +> extends IFluidHandleInternal { + /** + * Whether the handle is a placeholder, meaning that it may exist before its payload is retrievable. + * For instance, the BlobManager can generate handles before completing the blob upload/attach. + */ + readonly placeholder: boolean; +} + /** * Symbol which must only be used on an {@link (IFluidHandle:interface)}, and is used to identify such objects. * diff --git a/packages/common/core-interfaces/src/index.ts b/packages/common/core-interfaces/src/index.ts index 06796dab9223..bdef0d2b7ce5 100644 --- a/packages/common/core-interfaces/src/index.ts +++ b/packages/common/core-interfaces/src/index.ts @@ -31,6 +31,7 @@ export type { IProvideFluidHandleContext, IProvideFluidHandle, IFluidHandleInternal, + IFluidHandleInternalPlaceholder, IFluidHandleErased, } from "./handles.js"; export { IFluidHandleContext, IFluidHandle, fluidHandleSymbol } from "./handles.js"; diff --git a/packages/dds/shared-object-base/src/serializer.ts b/packages/dds/shared-object-base/src/serializer.ts index ac527f5bcc5b..fce2e05f0932 100644 --- a/packages/dds/shared-object-base/src/serializer.ts +++ b/packages/dds/shared-object-base/src/serializer.ts @@ -153,7 +153,7 @@ export class FluidSerializer implements IFluidSerializer { ? value.url : generateHandleContextPath(value.url, this.context); - return new RemoteFluidObjectHandle(absolutePath, this.root); + return new RemoteFluidObjectHandle(absolutePath, this.root, value.placeholder === true); } else { return value; } diff --git a/packages/dds/shared-object-base/src/test/serializer.spec.ts b/packages/dds/shared-object-base/src/test/serializer.spec.ts index 9b9021f93c5b..6dbf1c81b826 100644 --- a/packages/dds/shared-object-base/src/test/serializer.spec.ts +++ b/packages/dds/shared-object-base/src/test/serializer.spec.ts @@ -41,7 +41,11 @@ describe("FluidSerializer", () => { describe("vanilla JSON", () => { const context = new MockHandleContext(); const serializer = new FluidSerializer(context); - const handle = new RemoteFluidObjectHandle("/root", context); + const handle = new RemoteFluidObjectHandle( + "/root", + context, + false, // placeholder + ); // Start with the various JSON-serializable types. A mix of "truthy" and "falsy" values // are of particular interest. @@ -167,7 +171,11 @@ describe("FluidSerializer", () => { describe("JSON w/embedded handles", () => { const context = new MockHandleContext(); const serializer = new FluidSerializer(context); - const handle = new RemoteFluidObjectHandle("/root", context); + const handle = new RemoteFluidObjectHandle( + "/root", + context, + false, // placeholder + ); const serializedHandle = { type: "__fluid_handle__", url: "/root", @@ -329,8 +337,16 @@ describe("FluidSerializer", () => { describe("Utils", () => { const serializer = new FluidSerializer(new MockHandleContext()); it("makeSerializable is idempotent", () => { - const bind = new RemoteFluidObjectHandle("/", new MockHandleContext()); - const handle = new RemoteFluidObjectHandle("/okay", new MockHandleContext()); + const bind = new RemoteFluidObjectHandle( + "/", + new MockHandleContext(), + false, // placeholder + ); + const handle = new RemoteFluidObjectHandle( + "/okay", + new MockHandleContext(), + false, // placeholder + ); const input = { x: handle, y: 123 }; const serializedOnce = makeHandlesSerializable(input, serializer, bind) as { x: { type: "__fluid_handle__" }; diff --git a/packages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md b/packages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md index d79734e3584f..f315fdaef2f9 100644 --- a/packages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md +++ b/packages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md @@ -101,6 +101,7 @@ export interface ICompressionRuntimeOptions { export interface IContainerRuntimeOptions { readonly chunkSizeInBytes?: number; readonly compressionOptions?: ICompressionRuntimeOptions; + readonly createBlobPlaceholders?: boolean; // @deprecated readonly enableGroupedBatching?: boolean; readonly enableRuntimeIdCompressor?: IdCompressorMode; diff --git a/packages/runtime/container-runtime/src/blobManager/blobManager.ts b/packages/runtime/container-runtime/src/blobManager/blobManager.ts index cc980b17a159..8822fcffff7f 100644 --- a/packages/runtime/container-runtime/src/blobManager/blobManager.ts +++ b/packages/runtime/container-runtime/src/blobManager/blobManager.ts @@ -18,6 +18,7 @@ import type { IEventProvider, IFluidHandleContext, IFluidHandleInternal, + IFluidHandleInternalPlaceholder, } from "@fluidframework/core-interfaces/internal"; import { assert, Deferred } from "@fluidframework/core-utils/internal"; import { @@ -62,7 +63,10 @@ import { * DataObject.request() recognizes requests in the form of `/blobs/` * and loads blob. */ -export class BlobHandle extends FluidHandleBase { +export class BlobHandle + extends FluidHandleBase + implements IFluidHandleInternalPlaceholder +{ private attached: boolean = false; public get isAttached(): boolean { @@ -75,6 +79,7 @@ export class BlobHandle extends FluidHandleBase { public readonly path: string, public readonly routeContext: IFluidHandleContext, public get: () => Promise, + public readonly placeholder: boolean, private readonly onAttachGraph?: () => void, ) { super(); @@ -133,7 +138,8 @@ export interface IBlobManagerEvents extends IEvent { } interface IBlobManagerInternalEvents extends IEvent { - (event: "blobAttached", listener: (pending: PendingBlob) => void); + (event: "handleAttached", listener: (pending: PendingBlob) => void); + (event: "processedBlobAttach", listener: (localId: string, storageId: string) => void); } const stashedPendingBlobOverrides: Pick< @@ -195,7 +201,7 @@ export class BlobManager { new Map(); public readonly stashedBlobsUploadP: Promise<(void | ICreateBlobResponse)[]>; - constructor(props: { + public constructor(props: { readonly routeContext: IFluidHandleContext; blobManagerLoadInfo: IBlobManagerLoadInfo; @@ -220,6 +226,7 @@ export class BlobManager { readonly runtime: IBlobManagerRuntime; stashedBlobs: IPendingBlobs | undefined; readonly localBlobIdGenerator?: (() => string) | undefined; + readonly createBlobPlaceholders: boolean; }) { const { routeContext, @@ -351,7 +358,11 @@ export class BlobManager { return [...this.pendingBlobs.values()].some((e) => e.stashedUpload === true); } - public async getBlob(blobId: string): Promise { + public hasBlob(blobId: string): boolean { + return this.redirectTable.get(blobId) !== undefined; + } + + public async getBlob(blobId: string, placeholder: boolean): Promise { // Verify that the blob is not deleted, i.e., it has not been garbage collected. If it is, this will throw // an error, failing the call. this.verifyBlobNotDeleted(blobId); @@ -374,8 +385,24 @@ export class BlobManager { storageId = blobId; } else { const attachedStorageId = this.redirectTable.get(blobId); - assert(!!attachedStorageId, 0x11f /* "requesting unknown blobs" */); - storageId = attachedStorageId; + if (!placeholder) { + assert(!!attachedStorageId, 0x11f /* "requesting unknown blobs" */); + } + // If we didn't find it in the redirectTable, assume the attach op is coming eventually and wait. + // We do this even if the local client doesn't have the blob placeholder flag enabled, in case a + // remote client does have it enabled. This wait may be infinite if the uploading client failed + // the upload and doesn't exist anymore. + storageId = + attachedStorageId ?? + (await new Promise((resolve) => { + const onProcessBlobAttach = (localId: string, _storageId: string): void => { + if (localId === blobId) { + this.internalEvents.off("processedBlobAttach", onProcessBlobAttach); + resolve(_storageId); + } + }; + this.internalEvents.on("processedBlobAttach", onProcessBlobAttach); + })); } return PerformanceEvent.timedExecAsync( @@ -406,14 +433,15 @@ export class BlobManager { ? () => { pending.attached = true; // Notify listeners (e.g. serialization process) that blob has been attached - this.internalEvents.emit("blobAttached", pending); + this.internalEvents.emit("handleAttached", pending); this.deletePendingBlobMaybe(localId); } : undefined; return new BlobHandle( getGCNodePathFromBlobId(localId), this.routeContext, - async () => this.getBlob(localId), + async () => this.getBlob(localId, false), + false, // placeholder callback, ); } @@ -677,6 +705,7 @@ export class BlobManager { this.deletePendingBlobMaybe(localId); } } + this.internalEvents.emit("processedBlobAttach", localId, blobId); } public summarize(telemetryContext?: ITelemetryContext): ISummaryTreeWithStats { @@ -870,14 +899,14 @@ export class BlobManager { ); const onBlobAttached = (attachedEntry: PendingBlob): void => { if (attachedEntry === entry) { - this.internalEvents.off("blobAttached", onBlobAttached); + this.internalEvents.off("handleAttached", onBlobAttached); resolve(); } }; if (entry.attached) { resolve(); } else { - this.internalEvents.on("blobAttached", onBlobAttached); + this.internalEvents.on("handleAttached", onBlobAttached); } }), ); diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 5c048ec2aac3..1bc52ec0dfe7 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -99,6 +99,7 @@ import { import { GCDataBuilder, RequestParser, + RuntimeHeaders, TelemetryContext, addBlobToSummary, addSummarizeResultToSummary, @@ -397,6 +398,12 @@ export interface IContainerRuntimeOptions { * are engaged as they become available, without giving legacy clients any chance to fail predictably. */ readonly explicitSchemaControl?: boolean; + + /** + * Create blob placeholders when calling createBlob (default is false). + * When enabled, createBlob will return a handle before the blob upload completes. + */ + readonly createBlobPlaceholders?: boolean; } /** @@ -790,6 +797,7 @@ export class ContainerRuntime chunkSizeInBytes = defaultChunkSizeInBytes, enableGroupedBatching = true, explicitSchemaControl = false, + createBlobPlaceholders = false, }: IContainerRuntimeOptionsInternal = runtimeOptions; const registry = new FluidDataStoreRegistry(registryEntries); @@ -966,6 +974,7 @@ export class ContainerRuntime compressionLz4, idCompressorMode, opGroupingEnabled: enableGroupedBatching, + createBlobPlaceholders, disallowedVersions: [], }, (schema) => { @@ -992,6 +1001,7 @@ export class ContainerRuntime enableRuntimeIdCompressor: enableRuntimeIdCompressor as "on" | "delayed", enableGroupedBatching, explicitSchemaControl, + createBlobPlaceholders, }; const runtime = new containerRuntimeCtor( @@ -1739,6 +1749,7 @@ export class ContainerRuntime isBlobDeleted: (blobPath: string) => this.garbageCollector.isNodeDeleted(blobPath), runtime: this, stashedBlobs: pendingRuntimeState?.pendingAttachmentBlobs, + createBlobPlaceholders: this.sessionSchema.createBlobPlaceholders === true, }); this.deltaScheduler = new DeltaScheduler( @@ -2257,7 +2268,16 @@ export class ContainerRuntime } if (id === blobManagerBasePath && requestParser.isLeaf(2)) { - const blob = await this.blobManager.getBlob(requestParser.pathParts[1]); + const localId = requestParser.pathParts[1]; + const placeholder = requestParser.headers?.[RuntimeHeaders.placeholder] === true; + if ( + !this.blobManager.hasBlob(localId) && + requestParser.headers?.[RuntimeHeaders.wait] === false + ) { + return create404Response(request); + } + + const blob = await this.blobManager.getBlob(localId, placeholder); return { status: 200, mimeType: "fluid/object", diff --git a/packages/runtime/container-runtime/src/summary/documentSchema.ts b/packages/runtime/container-runtime/src/summary/documentSchema.ts index 472f4b7d61c7..bb993ee74182 100644 --- a/packages/runtime/container-runtime/src/summary/documentSchema.ts +++ b/packages/runtime/container-runtime/src/summary/documentSchema.ts @@ -96,6 +96,7 @@ export interface IDocumentSchemaFeatures { compressionLz4: boolean; idCompressorMode: IdCompressorMode; opGroupingEnabled: boolean; + createBlobPlaceholders: boolean; /** * List of disallowed versions of the runtime. @@ -227,6 +228,7 @@ const documentSchemaSupportedConfigs = { idCompressorMode: new IdCompressorProperty(["delayed", "on"]), opGroupingEnabled: new TrueOrUndefined(), compressionLz4: new TrueOrUndefined(), + createBlobPlaceholders: new TrueOrUndefined(), disallowedVersions: new CheckVersions(), }; @@ -482,6 +484,7 @@ export class DocumentsSchemaController { compressionLz4: boolToProp(features.compressionLz4), idCompressorMode: features.idCompressorMode, opGroupingEnabled: boolToProp(features.opGroupingEnabled), + createBlobPlaceholders: boolToProp(features.createBlobPlaceholders), disallowedVersions: arrayToProp(features.disallowedVersions), }, }; diff --git a/packages/runtime/container-runtime/src/test/blobManager.spec.ts b/packages/runtime/container-runtime/src/test/blobManager.spec.ts index fbc2fd73b9a6..8ada034a647a 100644 --- a/packages/runtime/container-runtime/src/test/blobManager.spec.ts +++ b/packages/runtime/container-runtime/src/test/blobManager.spec.ts @@ -27,6 +27,7 @@ import { Deferred } from "@fluidframework/core-utils/internal"; import { IClientDetails, SummaryType } from "@fluidframework/driver-definitions"; import { IDocumentStorageService } from "@fluidframework/driver-definitions/internal"; import type { ISequencedMessageEnvelope } from "@fluidframework/runtime-definitions/internal"; +import { isFluidHandleInternalPlaceholder } from "@fluidframework/runtime-utils/internal"; import { LoggingError, MockLogger, @@ -104,6 +105,7 @@ export class MockRuntime isBlobDeleted: (blobPath: string) => this.isBlobDeleted(blobPath), runtime: this, stashedBlobs: stashed[1] as IPendingBlobs | undefined, + createBlobPlaceholders: false, }); } @@ -160,7 +162,10 @@ export class MockRuntime ): Promise { const pathParts = blobHandle.absolutePath.split("/"); const blobId = pathParts[2]; - return this.blobManager.getBlob(blobId); + const placeholder = isFluidHandleInternalPlaceholder(blobHandle) + ? blobHandle.placeholder + : false; + return this.blobManager.getBlob(blobId, placeholder); } public async getPendingLocalState(): Promise<(unknown[] | IPendingBlobs | undefined)[]> { @@ -743,7 +748,7 @@ describe("BlobManager", () => { }); await assert.rejects( - async () => runtime.blobManager.getBlob(someId), + async () => runtime.blobManager.getBlob(someId, false), (e: Error) => e.message === "BOOM!", "Expected getBlob to throw with test error message", ); diff --git a/packages/runtime/container-runtime/src/test/blobManager.stashed.spec.ts b/packages/runtime/container-runtime/src/test/blobManager.stashed.spec.ts index 57bd52e367c1..2fa3b1b9d8be 100644 --- a/packages/runtime/container-runtime/src/test/blobManager.stashed.spec.ts +++ b/packages/runtime/container-runtime/src/test/blobManager.stashed.spec.ts @@ -52,6 +52,7 @@ function createBlobManager(overrides?: Partial { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, // Redundant, but makes the JSON.stringify yield the same result as the logs explicitSchemaControl: false, + createBlobPlaceholders: false, // Redundant, but makes the JSON.stringify yield the same result as the logs }; const mergedRuntimeOptions = { ...defaultRuntimeOptions, ...runtimeOptions }; diff --git a/packages/runtime/container-runtime/src/test/documentSchema.spec.ts b/packages/runtime/container-runtime/src/test/documentSchema.spec.ts index 8f8c43677bf1..bfde8befa959 100644 --- a/packages/runtime/container-runtime/src/test/documentSchema.spec.ts +++ b/packages/runtime/container-runtime/src/test/documentSchema.spec.ts @@ -29,6 +29,7 @@ describe("Runtime", () => { compressionLz4: true, idCompressorMode: "delayed", // opGroupingEnabled: undefined, + createBlobPlaceholders: true, }, }; @@ -37,6 +38,7 @@ describe("Runtime", () => { compressionLz4: true, opGroupingEnabled: false, idCompressorMode: "delayed", + createBlobPlaceholders: true, disallowedVersions: [], }; @@ -301,6 +303,7 @@ describe("Runtime", () => { compressionLz4: boolToProp(featuresModified.compressionLz4), idCompressorMode: featuresModified.idCompressorMode, opGroupingEnabled: boolToProp(featuresModified.opGroupingEnabled), + createBlobPlaceholders: boolToProp(featuresModified.createBlobPlaceholders), disallowedVersions: arrayToProp(featuresModified.disallowedVersions), }, }; @@ -457,7 +460,12 @@ describe("Runtime", () => { true, // existing, 0, // snapshotSequenceNumber undefined, // old schema, - { ...features, idCompressorMode: undefined, compressionLz4: false }, + { + ...features, + idCompressorMode: undefined, + compressionLz4: false, + createBlobPlaceholders: false, + }, () => { assert(false, "no changes!"); }, // onSchemaChange @@ -473,7 +481,12 @@ describe("Runtime", () => { true, // existing, 0, // snapshotSequenceNumber newSchema, // old schema, - { ...features, idCompressorMode: undefined, compressionLz4: false }, + { + ...features, + idCompressorMode: undefined, + compressionLz4: false, + createBlobPlaceholders: false, + }, () => { assert(false, "no changes!"); }, // onSchemaChange @@ -495,7 +508,12 @@ describe("Runtime", () => { true, // existing, 0, // snapshotSequenceNumber newSchema, // old schema, - { ...features, idCompressorMode: "on", compressionLz4: false }, + { + ...features, + idCompressorMode: "on", + compressionLz4: false, + createBlobPlaceholders: false, + }, () => { schemaChanged = true; }, // onSchemaChange @@ -534,6 +552,7 @@ describe("Runtime", () => { idCompressorMode: undefined, compressionLz4: false, opGroupingEnabled: true, + createBlobPlaceholders: false, }, () => (schemaChanged = true), // onSchemaChange ); diff --git a/packages/runtime/runtime-utils/src/handles.ts b/packages/runtime/runtime-utils/src/handles.ts index 82b1abaa4f14..41aa4ece3f90 100644 --- a/packages/runtime/runtime-utils/src/handles.ts +++ b/packages/runtime/runtime-utils/src/handles.ts @@ -5,7 +5,10 @@ import type { IFluidHandleErased } from "@fluidframework/core-interfaces"; import { IFluidHandle, fluidHandleSymbol } from "@fluidframework/core-interfaces"; -import type { IFluidHandleInternal } from "@fluidframework/core-interfaces/internal"; +import type { + IFluidHandleInternal, + IFluidHandleInternalPlaceholder, +} from "@fluidframework/core-interfaces/internal"; /** * JSON serialized form of an IFluidHandle @@ -17,6 +20,17 @@ export interface ISerializedHandle { // URL to the object. Relative URLs are relative to the handle context passed to the stringify. url: string; + + /** + * The handle may be a placeholder, as determined by and resolvable by the subsystem that the handle + * relates to. For instance, the BlobManager uses this to distinguish blob handles which may + * not yet have an attached blob yet. + * + * @remarks + * Will only exist if the handle is a placeholder, will be omitted entirely from the serialized format + * if the handle is not a placeholder. + */ + readonly placeholder?: true; } /** @@ -26,6 +40,14 @@ export interface ISerializedHandle { export const isSerializedHandle = (value: any): value is ISerializedHandle => value?.type === "__fluid_handle__"; +/** + * @internal + */ +export const isFluidHandleInternalPlaceholder = ( + fluidHandleInternal: IFluidHandleInternal, +): fluidHandleInternal is IFluidHandleInternalPlaceholder => + "placeholder" in fluidHandleInternal && fluidHandleInternal.placeholder === true; + /** * Encodes the given IFluidHandle into a JSON-serializable form, * @param handle - The IFluidHandle to serialize. @@ -34,10 +56,16 @@ export const isSerializedHandle = (value: any): value is ISerializedHandle => * @internal */ export function encodeHandleForSerialization(handle: IFluidHandleInternal): ISerializedHandle { - return { - type: "__fluid_handle__", - url: handle.absolutePath, - }; + return isFluidHandleInternalPlaceholder(handle) + ? { + type: "__fluid_handle__", + url: handle.absolutePath, + placeholder: true, + } + : { + type: "__fluid_handle__", + url: handle.absolutePath, + }; } /** diff --git a/packages/runtime/runtime-utils/src/index.ts b/packages/runtime/runtime-utils/src/index.ts index d39f495a956b..b45af132c354 100644 --- a/packages/runtime/runtime-utils/src/index.ts +++ b/packages/runtime/runtime-utils/src/index.ts @@ -17,6 +17,7 @@ export { ISerializedHandle, isSerializedHandle, isFluidHandle, + isFluidHandleInternalPlaceholder, toFluidHandleErased, toFluidHandleInternal, FluidHandleBase, diff --git a/packages/runtime/runtime-utils/src/remoteFluidObjectHandle.ts b/packages/runtime/runtime-utils/src/remoteFluidObjectHandle.ts index 13e72aedaa8c..d32c7d5577f2 100644 --- a/packages/runtime/runtime-utils/src/remoteFluidObjectHandle.ts +++ b/packages/runtime/runtime-utils/src/remoteFluidObjectHandle.ts @@ -35,6 +35,7 @@ export class RemoteFluidObjectHandle extends FluidHandleBase { constructor( public readonly absolutePath: string, public readonly routeContext: IFluidHandleContext, + public readonly placeholder: boolean, ) { super(); assert( @@ -48,7 +49,10 @@ export class RemoteFluidObjectHandle extends FluidHandleBase { // Add `viaHandle` header to distinguish from requests from non-handle paths. const request: IRequest = { url: this.absolutePath, - headers: { [RuntimeHeaders.viaHandle]: true }, + headers: { + [RuntimeHeaders.viaHandle]: true, + [RuntimeHeaders.placeholder]: this.placeholder, + }, }; this.objectP = this.routeContext.resolveHandle(request).then((response) => { if (response.mimeType === "fluid/object") { diff --git a/packages/runtime/runtime-utils/src/utils.ts b/packages/runtime/runtime-utils/src/utils.ts index 15b2304b4ce7..d5c086af3c31 100644 --- a/packages/runtime/runtime-utils/src/utils.ts +++ b/packages/runtime/runtime-utils/src/utils.ts @@ -22,6 +22,10 @@ export enum RuntimeHeaders { * True if the request is coming from an IFluidHandle. */ viaHandle = "viaHandle", + /** + * True if the request is coming from a placeholder handle. + */ + placeholder = "placeholder", } /** diff --git a/packages/test/test-service-load/src/optionsMatrix.ts b/packages/test/test-service-load/src/optionsMatrix.ts index 855473cc4e1f..6027ff4cc81a 100644 --- a/packages/test/test-service-load/src/optionsMatrix.ts +++ b/packages/test/test-service-load/src/optionsMatrix.ts @@ -111,6 +111,7 @@ export function generateRuntimeOptions( chunkSizeInBytes: [204800], enableRuntimeIdCompressor: ["on", undefined, "delayed"], enableGroupedBatching: [true, false], + createBlobPlaceholders: [true, false], explicitSchemaControl: [true, false], }; From 9f97f4be84a7d7d2c0af28d5ddc2c666d737bac0 Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Fri, 11 Apr 2025 13:41:52 -0700 Subject: [PATCH 2/7] Revert some of documentSchema.spec.ts, default to off --- .../src/test/documentSchema.spec.ts | 26 ++++--------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/packages/runtime/container-runtime/src/test/documentSchema.spec.ts b/packages/runtime/container-runtime/src/test/documentSchema.spec.ts index bfde8befa959..6e5833e41ff8 100644 --- a/packages/runtime/container-runtime/src/test/documentSchema.spec.ts +++ b/packages/runtime/container-runtime/src/test/documentSchema.spec.ts @@ -29,7 +29,7 @@ describe("Runtime", () => { compressionLz4: true, idCompressorMode: "delayed", // opGroupingEnabled: undefined, - createBlobPlaceholders: true, + // createBlobPlaceholders: true, }, }; @@ -38,7 +38,7 @@ describe("Runtime", () => { compressionLz4: true, opGroupingEnabled: false, idCompressorMode: "delayed", - createBlobPlaceholders: true, + createBlobPlaceholders: false, disallowedVersions: [], }; @@ -460,12 +460,7 @@ describe("Runtime", () => { true, // existing, 0, // snapshotSequenceNumber undefined, // old schema, - { - ...features, - idCompressorMode: undefined, - compressionLz4: false, - createBlobPlaceholders: false, - }, + { ...features, idCompressorMode: undefined, compressionLz4: false }, () => { assert(false, "no changes!"); }, // onSchemaChange @@ -481,12 +476,7 @@ describe("Runtime", () => { true, // existing, 0, // snapshotSequenceNumber newSchema, // old schema, - { - ...features, - idCompressorMode: undefined, - compressionLz4: false, - createBlobPlaceholders: false, - }, + { ...features, idCompressorMode: undefined, compressionLz4: false }, () => { assert(false, "no changes!"); }, // onSchemaChange @@ -508,12 +498,7 @@ describe("Runtime", () => { true, // existing, 0, // snapshotSequenceNumber newSchema, // old schema, - { - ...features, - idCompressorMode: "on", - compressionLz4: false, - createBlobPlaceholders: false, - }, + { ...features, idCompressorMode: "on", compressionLz4: false }, () => { schemaChanged = true; }, // onSchemaChange @@ -552,7 +537,6 @@ describe("Runtime", () => { idCompressorMode: undefined, compressionLz4: false, opGroupingEnabled: true, - createBlobPlaceholders: false, }, () => (schemaChanged = true), // onSchemaChange ); From 55d13d7efa7f4f0e5d502c5288de4ced82ba7702 Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Fri, 11 Apr 2025 14:21:24 -0700 Subject: [PATCH 3/7] add getBlob placeholder test --- .../src/test/blobManager.spec.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/runtime/container-runtime/src/test/blobManager.spec.ts b/packages/runtime/container-runtime/src/test/blobManager.spec.ts index 8ada034a647a..f4361af395ad 100644 --- a/packages/runtime/container-runtime/src/test/blobManager.spec.ts +++ b/packages/runtime/container-runtime/src/test/blobManager.spec.ts @@ -765,6 +765,29 @@ describe("BlobManager", () => { ); }); + it("waits for placeholder blobs without error", async () => { + await runtime.attach(); + + // Part of remoteUpload, but stop short of processing the message + const response = await runtime.storage.createBlob(IsoBuffer.from("blob", "utf8")); + const op = { metadata: { localId: uuid(), blobId: response.id } }; + + await assert.rejects( + runtime.blobManager.getBlob(op.metadata.localId, false), + "Rejects when attempting to get non-existent, non-placeholder blobs", + ); + + // Try to get the blob that we haven't processed the attach op for yet, as a placeholder + // Simulating having found this ID in a placeholder handle that the remote client would have sent + const blobP = runtime.blobManager.getBlob(op.metadata.localId, true); + + // Process the op as if it were arriving from the remote client, which should cause the blobP promise to resolve + runtime.blobManager.processBlobAttachMessage(op as ISequencedMessageEnvelope, false); + + // Await the promise to confirm it settles and does not reject + await blobP; + }); + describe("Abort Signal", () => { it("abort before upload", async () => { await runtime.attach(); From 42f64db9a28587eb66ecfc70042d97ccd3f18939 Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Tue, 22 Apr 2025 11:31:36 -0700 Subject: [PATCH 4/7] Add config to runtimeOptionsAffectingDocSchemaConfigMap --- packages/runtime/container-runtime/src/compatUtils.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/runtime/container-runtime/src/compatUtils.ts b/packages/runtime/container-runtime/src/compatUtils.ts index e1577e86ab59..c20e4097121e 100644 --- a/packages/runtime/container-runtime/src/compatUtils.ts +++ b/packages/runtime/container-runtime/src/compatUtils.ts @@ -150,6 +150,9 @@ const runtimeOptionsAffectingDocSchemaConfigMap = { // Although sweep is supported in 2.x, it is disabled by default until compatibilityVersion>=3.0.0 to be extra safe. "3.0.0": { enableGCSweep: true }, } as const, + createBlobPlaceholders: { + "1.0.0": false, + }, } as const satisfies ConfigMap; /** From 3b413474cff85bb97f241d7e0b8ea46bedb53a10 Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Tue, 22 Apr 2025 12:54:10 -0700 Subject: [PATCH 5/7] Update test --- .../src/test/containerRuntime.spec.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts index ac53221e1af7..059b5cf3222a 100644 --- a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts +++ b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts @@ -3653,6 +3653,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, explicitSchemaControl: false, + createBlobPlaceholders: false, }; logger.assertMatchAny([ @@ -3690,6 +3691,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: false, explicitSchemaControl: false, + createBlobPlaceholders: false, }; logger.assertMatchAny([ @@ -3727,6 +3729,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, explicitSchemaControl: false, + createBlobPlaceholders: false, }; logger.assertMatchAny([ @@ -3764,6 +3767,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, explicitSchemaControl: true, + createBlobPlaceholders: false, }; logger.assertMatchAny([ @@ -3801,6 +3805,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, explicitSchemaControl: true, + createBlobPlaceholders: false, }; logger.assertMatchAny([ @@ -3828,6 +3833,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: "on", enableGroupedBatching: false, // By turning off batching, we will also disable compression automatically explicitSchemaControl: true, + createBlobPlaceholders: false, }, provideEntryPoint: mockProvideEntryPoint, }); @@ -3846,6 +3852,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: "on", enableGroupedBatching: false, explicitSchemaControl: true, + createBlobPlaceholders: false, }; logger.assertMatchAny([ @@ -3876,6 +3883,7 @@ describe("Runtime", () => { enableGroupedBatching: undefined, compressionOptions: undefined, explicitSchemaControl: undefined, + createBlobPlaceholders: undefined, }, }, ]) @@ -3903,6 +3911,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, // idCompressor is undefined, since that represents a logical state (off) enableGroupedBatching: true, explicitSchemaControl: false, + createBlobPlaceholders: false, }; logger.assertMatchAny([ @@ -3939,6 +3948,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, explicitSchemaControl: true, + createBlobPlaceholders: false, }; logger.assertMatchAny([ From ac5d93e28126f8d40343ac83a7eba10adc57bc90 Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Thu, 24 Apr 2025 08:25:34 -0700 Subject: [PATCH 6/7] Scott's feedback --- packages/runtime/container-runtime/src/compatUtils.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/runtime/container-runtime/src/compatUtils.ts b/packages/runtime/container-runtime/src/compatUtils.ts index c20e4097121e..1f619ee12c67 100644 --- a/packages/runtime/container-runtime/src/compatUtils.ts +++ b/packages/runtime/container-runtime/src/compatUtils.ts @@ -151,8 +151,11 @@ const runtimeOptionsAffectingDocSchemaConfigMap = { "3.0.0": { enableGCSweep: true }, } as const, createBlobPlaceholders: { + // This feature is new and disabled by default. In the future we will enable it by default, but we have not + // closed on the version where that will happen yet. Probably a .10 release since blob functionality is not + // exposed on the public API surface. "1.0.0": false, - }, + } as const, } as const satisfies ConfigMap; /** From 3a3e785208348a33f930a4e075606d182261ffcc Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Thu, 24 Apr 2025 09:14:47 -0700 Subject: [PATCH 7/7] Rename placeholder -> payloadPending --- .../common/core-interfaces/src/handles.ts | 6 ++--- packages/common/core-interfaces/src/index.ts | 2 +- .../dds/shared-object-base/src/serializer.ts | 6 ++++- .../src/test/serializer.spec.ts | 8 +++---- .../container-runtime.legacy.alpha.api.md | 2 +- .../src/blobManager/blobManager.ts | 16 ++++++------- .../container-runtime/src/compatUtils.ts | 2 +- .../container-runtime/src/containerRuntime.ts | 16 ++++++------- .../src/summary/documentSchema.ts | 6 ++--- .../src/test/blobManager.spec.ts | 18 +++++++------- .../src/test/containerRuntime.spec.ts | 22 ++++++++--------- .../src/test/documentSchema.spec.ts | 6 ++--- packages/runtime/runtime-utils/src/handles.ts | 24 +++++++++---------- packages/runtime/runtime-utils/src/index.ts | 2 +- .../src/remoteFluidObjectHandle.ts | 5 ++-- packages/runtime/runtime-utils/src/utils.ts | 4 ++-- .../test-service-load/src/optionsMatrix.ts | 2 +- 17 files changed, 76 insertions(+), 71 deletions(-) diff --git a/packages/common/core-interfaces/src/handles.ts b/packages/common/core-interfaces/src/handles.ts index 411e03a66aea..998bd53e6190 100644 --- a/packages/common/core-interfaces/src/handles.ts +++ b/packages/common/core-interfaces/src/handles.ts @@ -106,15 +106,15 @@ export interface IFluidHandleInternal< * To be merged onto IFluidHandleInternal in accordance with breaking change policy * @internal */ -export interface IFluidHandleInternalPlaceholder< +export interface IFluidHandleInternalPayloadPending< // REVIEW: Constrain `T` to something? How do we support dds and datastores safely? out T = unknown, // FluidObject & IFluidLoadable, > extends IFluidHandleInternal { /** - * Whether the handle is a placeholder, meaning that it may exist before its payload is retrievable. + * Whether the handle has a pending payload, meaning that it may exist before its payload is retrievable. * For instance, the BlobManager can generate handles before completing the blob upload/attach. */ - readonly placeholder: boolean; + readonly payloadPending: boolean; } /** diff --git a/packages/common/core-interfaces/src/index.ts b/packages/common/core-interfaces/src/index.ts index bdef0d2b7ce5..0d6e8f70f55c 100644 --- a/packages/common/core-interfaces/src/index.ts +++ b/packages/common/core-interfaces/src/index.ts @@ -31,7 +31,7 @@ export type { IProvideFluidHandleContext, IProvideFluidHandle, IFluidHandleInternal, - IFluidHandleInternalPlaceholder, + IFluidHandleInternalPayloadPending, IFluidHandleErased, } from "./handles.js"; export { IFluidHandleContext, IFluidHandle, fluidHandleSymbol } from "./handles.js"; diff --git a/packages/dds/shared-object-base/src/serializer.ts b/packages/dds/shared-object-base/src/serializer.ts index fce2e05f0932..077d7cb6602f 100644 --- a/packages/dds/shared-object-base/src/serializer.ts +++ b/packages/dds/shared-object-base/src/serializer.ts @@ -153,7 +153,11 @@ export class FluidSerializer implements IFluidSerializer { ? value.url : generateHandleContextPath(value.url, this.context); - return new RemoteFluidObjectHandle(absolutePath, this.root, value.placeholder === true); + return new RemoteFluidObjectHandle( + absolutePath, + this.root, + value.payloadPending === true, + ); } else { return value; } diff --git a/packages/dds/shared-object-base/src/test/serializer.spec.ts b/packages/dds/shared-object-base/src/test/serializer.spec.ts index 6dbf1c81b826..56df90a4516b 100644 --- a/packages/dds/shared-object-base/src/test/serializer.spec.ts +++ b/packages/dds/shared-object-base/src/test/serializer.spec.ts @@ -44,7 +44,7 @@ describe("FluidSerializer", () => { const handle = new RemoteFluidObjectHandle( "/root", context, - false, // placeholder + false, // payloadPending ); // Start with the various JSON-serializable types. A mix of "truthy" and "falsy" values @@ -174,7 +174,7 @@ describe("FluidSerializer", () => { const handle = new RemoteFluidObjectHandle( "/root", context, - false, // placeholder + false, // payloadPending ); const serializedHandle = { type: "__fluid_handle__", @@ -340,12 +340,12 @@ describe("FluidSerializer", () => { const bind = new RemoteFluidObjectHandle( "/", new MockHandleContext(), - false, // placeholder + false, // payloadPending ); const handle = new RemoteFluidObjectHandle( "/okay", new MockHandleContext(), - false, // placeholder + false, // payloadPending ); const input = { x: handle, y: 123 }; const serializedOnce = makeHandlesSerializable(input, serializer, bind) as { diff --git a/packages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md b/packages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md index 1947ee96637c..2b8b7125f480 100644 --- a/packages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md +++ b/packages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md @@ -36,7 +36,7 @@ export enum ContainerMessageType { export interface ContainerRuntimeOptions { readonly chunkSizeInBytes: number; readonly compressionOptions: ICompressionRuntimeOptions; - readonly createBlobPlaceholders: boolean; + readonly createBlobPayloadPending: boolean; // @deprecated readonly enableGroupedBatching: boolean; readonly enableRuntimeIdCompressor: IdCompressorMode; diff --git a/packages/runtime/container-runtime/src/blobManager/blobManager.ts b/packages/runtime/container-runtime/src/blobManager/blobManager.ts index 0680345c2cc7..a741ebc616c5 100644 --- a/packages/runtime/container-runtime/src/blobManager/blobManager.ts +++ b/packages/runtime/container-runtime/src/blobManager/blobManager.ts @@ -18,7 +18,7 @@ import type { IEventProvider, IFluidHandleContext, IFluidHandleInternal, - IFluidHandleInternalPlaceholder, + IFluidHandleInternalPayloadPending, } from "@fluidframework/core-interfaces/internal"; import { assert, Deferred } from "@fluidframework/core-utils/internal"; import { @@ -65,7 +65,7 @@ import { */ export class BlobHandle extends FluidHandleBase - implements IFluidHandleInternalPlaceholder + implements IFluidHandleInternalPayloadPending { private attached: boolean = false; @@ -79,7 +79,7 @@ export class BlobHandle public readonly path: string, public readonly routeContext: IFluidHandleContext, public get: () => Promise, - public readonly placeholder: boolean, + public readonly payloadPending: boolean, private readonly onAttachGraph?: () => void, ) { super(); @@ -226,7 +226,7 @@ export class BlobManager { readonly runtime: IBlobManagerRuntime; stashedBlobs: IPendingBlobs | undefined; readonly localBlobIdGenerator?: (() => string) | undefined; - readonly createBlobPlaceholders: boolean; + readonly createBlobPayloadPending: boolean; }) { const { routeContext, @@ -362,7 +362,7 @@ export class BlobManager { return this.redirectTable.get(blobId) !== undefined; } - public async getBlob(blobId: string, placeholder: boolean): Promise { + public async getBlob(blobId: string, payloadPending: boolean): Promise { // Verify that the blob is not deleted, i.e., it has not been garbage collected. If it is, this will throw // an error, failing the call. this.verifyBlobNotDeleted(blobId); @@ -385,11 +385,11 @@ export class BlobManager { storageId = blobId; } else { const attachedStorageId = this.redirectTable.get(blobId); - if (!placeholder) { + if (!payloadPending) { assert(!!attachedStorageId, 0x11f /* "requesting unknown blobs" */); } // If we didn't find it in the redirectTable, assume the attach op is coming eventually and wait. - // We do this even if the local client doesn't have the blob placeholder flag enabled, in case a + // We do this even if the local client doesn't have the blob payloadPending flag enabled, in case a // remote client does have it enabled. This wait may be infinite if the uploading client failed // the upload and doesn't exist anymore. storageId = @@ -441,7 +441,7 @@ export class BlobManager { getGCNodePathFromBlobId(localId), this.routeContext, async () => this.getBlob(localId, false), - false, // placeholder + false, // payloadPending callback, ); } diff --git a/packages/runtime/container-runtime/src/compatUtils.ts b/packages/runtime/container-runtime/src/compatUtils.ts index c5185980e9e7..d8bd0abbc016 100644 --- a/packages/runtime/container-runtime/src/compatUtils.ts +++ b/packages/runtime/container-runtime/src/compatUtils.ts @@ -147,7 +147,7 @@ const runtimeOptionsAffectingDocSchemaConfigMap = { // Although sweep is supported in 2.x, it is disabled by default until compatibilityVersion>=3.0.0 to be extra safe. "3.0.0": { enableGCSweep: true }, } as const, - createBlobPlaceholders: { + createBlobPayloadPending: { // This feature is new and disabled by default. In the future we will enable it by default, but we have not // closed on the version where that will happen yet. Probably a .10 release since blob functionality is not // exposed on the public API surface. diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 99294a4a9798..05a137b03195 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -409,10 +409,10 @@ export interface ContainerRuntimeOptions { readonly explicitSchemaControl: boolean; /** - * Create blob placeholders when calling createBlob (default is false). + * Create blob handles with pending payloads when calling createBlob (default is false). * When enabled, createBlob will return a handle before the blob upload completes. */ - readonly createBlobPlaceholders: boolean; + readonly createBlobPayloadPending: boolean; } /** @@ -836,7 +836,7 @@ export class ContainerRuntime compressionOptions = enableGroupedBatching === false ? disabledCompressionConfig : defaultConfigs.compressionOptions, - createBlobPlaceholders = defaultConfigs.createBlobPlaceholders, + createBlobPayloadPending = defaultConfigs.createBlobPayloadPending, }: IContainerRuntimeOptionsInternal = runtimeOptions; // The logic for enableRuntimeIdCompressor is a bit different. Since `undefined` represents a logical state (off) @@ -1020,7 +1020,7 @@ export class ContainerRuntime compressionLz4, idCompressorMode, opGroupingEnabled: enableGroupedBatching, - createBlobPlaceholders, + createBlobPayloadPending, disallowedVersions: [], }, (schema) => { @@ -1046,7 +1046,7 @@ export class ContainerRuntime enableRuntimeIdCompressor, enableGroupedBatching, explicitSchemaControl, - createBlobPlaceholders, + createBlobPayloadPending, }; const runtime = new containerRuntimeCtor( @@ -1781,7 +1781,7 @@ export class ContainerRuntime isBlobDeleted: (blobPath: string) => this.garbageCollector.isNodeDeleted(blobPath), runtime: this, stashedBlobs: pendingRuntimeState?.pendingAttachmentBlobs, - createBlobPlaceholders: this.sessionSchema.createBlobPlaceholders === true, + createBlobPayloadPending: this.sessionSchema.createBlobPayloadPending === true, }); this.deltaScheduler = new DeltaScheduler( @@ -2331,7 +2331,7 @@ export class ContainerRuntime if (id === blobManagerBasePath && requestParser.isLeaf(2)) { const localId = requestParser.pathParts[1]; - const placeholder = requestParser.headers?.[RuntimeHeaders.placeholder] === true; + const payloadPending = requestParser.headers?.[RuntimeHeaders.payloadPending] === true; if ( !this.blobManager.hasBlob(localId) && requestParser.headers?.[RuntimeHeaders.wait] === false @@ -2339,7 +2339,7 @@ export class ContainerRuntime return create404Response(request); } - const blob = await this.blobManager.getBlob(localId, placeholder); + const blob = await this.blobManager.getBlob(localId, payloadPending); return { status: 200, mimeType: "fluid/object", diff --git a/packages/runtime/container-runtime/src/summary/documentSchema.ts b/packages/runtime/container-runtime/src/summary/documentSchema.ts index bb993ee74182..82e1d07cf091 100644 --- a/packages/runtime/container-runtime/src/summary/documentSchema.ts +++ b/packages/runtime/container-runtime/src/summary/documentSchema.ts @@ -96,7 +96,7 @@ export interface IDocumentSchemaFeatures { compressionLz4: boolean; idCompressorMode: IdCompressorMode; opGroupingEnabled: boolean; - createBlobPlaceholders: boolean; + createBlobPayloadPending: boolean; /** * List of disallowed versions of the runtime. @@ -228,7 +228,7 @@ const documentSchemaSupportedConfigs = { idCompressorMode: new IdCompressorProperty(["delayed", "on"]), opGroupingEnabled: new TrueOrUndefined(), compressionLz4: new TrueOrUndefined(), - createBlobPlaceholders: new TrueOrUndefined(), + createBlobPayloadPending: new TrueOrUndefined(), disallowedVersions: new CheckVersions(), }; @@ -484,7 +484,7 @@ export class DocumentsSchemaController { compressionLz4: boolToProp(features.compressionLz4), idCompressorMode: features.idCompressorMode, opGroupingEnabled: boolToProp(features.opGroupingEnabled), - createBlobPlaceholders: boolToProp(features.createBlobPlaceholders), + createBlobPayloadPending: boolToProp(features.createBlobPayloadPending), disallowedVersions: arrayToProp(features.disallowedVersions), }, }; diff --git a/packages/runtime/container-runtime/src/test/blobManager.spec.ts b/packages/runtime/container-runtime/src/test/blobManager.spec.ts index f4361af395ad..22a74d7a2422 100644 --- a/packages/runtime/container-runtime/src/test/blobManager.spec.ts +++ b/packages/runtime/container-runtime/src/test/blobManager.spec.ts @@ -27,7 +27,7 @@ import { Deferred } from "@fluidframework/core-utils/internal"; import { IClientDetails, SummaryType } from "@fluidframework/driver-definitions"; import { IDocumentStorageService } from "@fluidframework/driver-definitions/internal"; import type { ISequencedMessageEnvelope } from "@fluidframework/runtime-definitions/internal"; -import { isFluidHandleInternalPlaceholder } from "@fluidframework/runtime-utils/internal"; +import { isFluidHandleInternalPayloadPending } from "@fluidframework/runtime-utils/internal"; import { LoggingError, MockLogger, @@ -105,7 +105,7 @@ export class MockRuntime isBlobDeleted: (blobPath: string) => this.isBlobDeleted(blobPath), runtime: this, stashedBlobs: stashed[1] as IPendingBlobs | undefined, - createBlobPlaceholders: false, + createBlobPayloadPending: false, }); } @@ -162,10 +162,10 @@ export class MockRuntime ): Promise { const pathParts = blobHandle.absolutePath.split("/"); const blobId = pathParts[2]; - const placeholder = isFluidHandleInternalPlaceholder(blobHandle) - ? blobHandle.placeholder + const payloadPending = isFluidHandleInternalPayloadPending(blobHandle) + ? blobHandle.payloadPending : false; - return this.blobManager.getBlob(blobId, placeholder); + return this.blobManager.getBlob(blobId, payloadPending); } public async getPendingLocalState(): Promise<(unknown[] | IPendingBlobs | undefined)[]> { @@ -765,7 +765,7 @@ describe("BlobManager", () => { ); }); - it("waits for placeholder blobs without error", async () => { + it("waits for blobs from handles with pending payloads without error", async () => { await runtime.attach(); // Part of remoteUpload, but stop short of processing the message @@ -774,11 +774,11 @@ describe("BlobManager", () => { await assert.rejects( runtime.blobManager.getBlob(op.metadata.localId, false), - "Rejects when attempting to get non-existent, non-placeholder blobs", + "Rejects when attempting to get non-existent, shared-payload blobs", ); - // Try to get the blob that we haven't processed the attach op for yet, as a placeholder - // Simulating having found this ID in a placeholder handle that the remote client would have sent + // Try to get the blob that we haven't processed the attach op for yet. + // This simulates having found this ID in a handle with a pending payload that the remote client would have sent const blobP = runtime.blobManager.getBlob(op.metadata.localId, true); // Process the op as if it were arriving from the remote client, which should cause the blobP promise to resolve diff --git a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts index 059b5cf3222a..620ad691adec 100644 --- a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts +++ b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts @@ -1588,7 +1588,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, // Redundant, but makes the JSON.stringify yield the same result as the logs explicitSchemaControl: false, - createBlobPlaceholders: false, // Redundant, but makes the JSON.stringify yield the same result as the logs + createBlobPayloadPending: false, // Redundant, but makes the JSON.stringify yield the same result as the logs }; const mergedRuntimeOptions = { ...defaultRuntimeOptions, ...runtimeOptions }; @@ -3653,7 +3653,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, explicitSchemaControl: false, - createBlobPlaceholders: false, + createBlobPayloadPending: false, }; logger.assertMatchAny([ @@ -3691,7 +3691,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: false, explicitSchemaControl: false, - createBlobPlaceholders: false, + createBlobPayloadPending: false, }; logger.assertMatchAny([ @@ -3729,7 +3729,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, explicitSchemaControl: false, - createBlobPlaceholders: false, + createBlobPayloadPending: false, }; logger.assertMatchAny([ @@ -3767,7 +3767,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, explicitSchemaControl: true, - createBlobPlaceholders: false, + createBlobPayloadPending: false, }; logger.assertMatchAny([ @@ -3805,7 +3805,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, explicitSchemaControl: true, - createBlobPlaceholders: false, + createBlobPayloadPending: false, }; logger.assertMatchAny([ @@ -3833,7 +3833,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: "on", enableGroupedBatching: false, // By turning off batching, we will also disable compression automatically explicitSchemaControl: true, - createBlobPlaceholders: false, + createBlobPayloadPending: false, }, provideEntryPoint: mockProvideEntryPoint, }); @@ -3852,7 +3852,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: "on", enableGroupedBatching: false, explicitSchemaControl: true, - createBlobPlaceholders: false, + createBlobPayloadPending: false, }; logger.assertMatchAny([ @@ -3883,7 +3883,7 @@ describe("Runtime", () => { enableGroupedBatching: undefined, compressionOptions: undefined, explicitSchemaControl: undefined, - createBlobPlaceholders: undefined, + createBlobPayloadPending: undefined, }, }, ]) @@ -3911,7 +3911,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, // idCompressor is undefined, since that represents a logical state (off) enableGroupedBatching: true, explicitSchemaControl: false, - createBlobPlaceholders: false, + createBlobPayloadPending: false, }; logger.assertMatchAny([ @@ -3948,7 +3948,7 @@ describe("Runtime", () => { enableRuntimeIdCompressor: undefined, enableGroupedBatching: true, explicitSchemaControl: true, - createBlobPlaceholders: false, + createBlobPayloadPending: false, }; logger.assertMatchAny([ diff --git a/packages/runtime/container-runtime/src/test/documentSchema.spec.ts b/packages/runtime/container-runtime/src/test/documentSchema.spec.ts index 6e5833e41ff8..5454ff59df41 100644 --- a/packages/runtime/container-runtime/src/test/documentSchema.spec.ts +++ b/packages/runtime/container-runtime/src/test/documentSchema.spec.ts @@ -29,7 +29,7 @@ describe("Runtime", () => { compressionLz4: true, idCompressorMode: "delayed", // opGroupingEnabled: undefined, - // createBlobPlaceholders: true, + // createBlobPayloadPending: true, }, }; @@ -38,7 +38,7 @@ describe("Runtime", () => { compressionLz4: true, opGroupingEnabled: false, idCompressorMode: "delayed", - createBlobPlaceholders: false, + createBlobPayloadPending: false, disallowedVersions: [], }; @@ -303,7 +303,7 @@ describe("Runtime", () => { compressionLz4: boolToProp(featuresModified.compressionLz4), idCompressorMode: featuresModified.idCompressorMode, opGroupingEnabled: boolToProp(featuresModified.opGroupingEnabled), - createBlobPlaceholders: boolToProp(featuresModified.createBlobPlaceholders), + createBlobPayloadPending: boolToProp(featuresModified.createBlobPayloadPending), disallowedVersions: arrayToProp(featuresModified.disallowedVersions), }, }; diff --git a/packages/runtime/runtime-utils/src/handles.ts b/packages/runtime/runtime-utils/src/handles.ts index 41aa4ece3f90..3c5eeb7d1e06 100644 --- a/packages/runtime/runtime-utils/src/handles.ts +++ b/packages/runtime/runtime-utils/src/handles.ts @@ -7,7 +7,7 @@ import type { IFluidHandleErased } from "@fluidframework/core-interfaces"; import { IFluidHandle, fluidHandleSymbol } from "@fluidframework/core-interfaces"; import type { IFluidHandleInternal, - IFluidHandleInternalPlaceholder, + IFluidHandleInternalPayloadPending, } from "@fluidframework/core-interfaces/internal"; /** @@ -22,15 +22,15 @@ export interface ISerializedHandle { url: string; /** - * The handle may be a placeholder, as determined by and resolvable by the subsystem that the handle - * relates to. For instance, the BlobManager uses this to distinguish blob handles which may - * not yet have an attached blob yet. + * The handle may have a pending payload, as determined by and resolvable by the subsystem that + * the handle relates to. For instance, the BlobManager uses this to distinguish blob handles + * which may not yet have an attached blob yet. * * @remarks - * Will only exist if the handle is a placeholder, will be omitted entirely from the serialized format - * if the handle is not a placeholder. + * Will only exist if the handle was created with a pending payload, will be omitted entirely from + * the serialized format if the handle was created with an already-shared payload. */ - readonly placeholder?: true; + readonly payloadPending?: true; } /** @@ -43,10 +43,10 @@ export const isSerializedHandle = (value: any): value is ISerializedHandle => /** * @internal */ -export const isFluidHandleInternalPlaceholder = ( +export const isFluidHandleInternalPayloadPending = ( fluidHandleInternal: IFluidHandleInternal, -): fluidHandleInternal is IFluidHandleInternalPlaceholder => - "placeholder" in fluidHandleInternal && fluidHandleInternal.placeholder === true; +): fluidHandleInternal is IFluidHandleInternalPayloadPending => + "payloadPending" in fluidHandleInternal && fluidHandleInternal.payloadPending === true; /** * Encodes the given IFluidHandle into a JSON-serializable form, @@ -56,11 +56,11 @@ export const isFluidHandleInternalPlaceholder = ( * @internal */ export function encodeHandleForSerialization(handle: IFluidHandleInternal): ISerializedHandle { - return isFluidHandleInternalPlaceholder(handle) + return isFluidHandleInternalPayloadPending(handle) ? { type: "__fluid_handle__", url: handle.absolutePath, - placeholder: true, + payloadPending: true, } : { type: "__fluid_handle__", diff --git a/packages/runtime/runtime-utils/src/index.ts b/packages/runtime/runtime-utils/src/index.ts index 113840538dee..62424a16139b 100644 --- a/packages/runtime/runtime-utils/src/index.ts +++ b/packages/runtime/runtime-utils/src/index.ts @@ -15,7 +15,7 @@ export { ISerializedHandle, isSerializedHandle, isFluidHandle, - isFluidHandleInternalPlaceholder, + isFluidHandleInternalPayloadPending, toFluidHandleErased, toFluidHandleInternal, FluidHandleBase, diff --git a/packages/runtime/runtime-utils/src/remoteFluidObjectHandle.ts b/packages/runtime/runtime-utils/src/remoteFluidObjectHandle.ts index d32c7d5577f2..a662a1b1ee16 100644 --- a/packages/runtime/runtime-utils/src/remoteFluidObjectHandle.ts +++ b/packages/runtime/runtime-utils/src/remoteFluidObjectHandle.ts @@ -31,11 +31,12 @@ export class RemoteFluidObjectHandle extends FluidHandleBase { * Creates a new RemoteFluidObjectHandle when parsing an IFluidHandle. * @param absolutePath - The absolute path to the handle from the container runtime. * @param routeContext - The root IFluidHandleContext that has a route to this handle. + * @param payloadPending - Whether the handle may have a pending payload that is not yet available. */ constructor( public readonly absolutePath: string, public readonly routeContext: IFluidHandleContext, - public readonly placeholder: boolean, + public readonly payloadPending: boolean, ) { super(); assert( @@ -51,7 +52,7 @@ export class RemoteFluidObjectHandle extends FluidHandleBase { url: this.absolutePath, headers: { [RuntimeHeaders.viaHandle]: true, - [RuntimeHeaders.placeholder]: this.placeholder, + [RuntimeHeaders.payloadPending]: this.payloadPending, }, }; this.objectP = this.routeContext.resolveHandle(request).then((response) => { diff --git a/packages/runtime/runtime-utils/src/utils.ts b/packages/runtime/runtime-utils/src/utils.ts index d5c086af3c31..6ffed8e077bf 100644 --- a/packages/runtime/runtime-utils/src/utils.ts +++ b/packages/runtime/runtime-utils/src/utils.ts @@ -23,9 +23,9 @@ export enum RuntimeHeaders { */ viaHandle = "viaHandle", /** - * True if the request is coming from a placeholder handle. + * True if the request is coming from a handle with a pending payload. */ - placeholder = "placeholder", + payloadPending = "payloadPending", } /** diff --git a/packages/test/test-service-load/src/optionsMatrix.ts b/packages/test/test-service-load/src/optionsMatrix.ts index 43a812d9efa6..bbd38d51b119 100644 --- a/packages/test/test-service-load/src/optionsMatrix.ts +++ b/packages/test/test-service-load/src/optionsMatrix.ts @@ -116,7 +116,7 @@ export function generateRuntimeOptions( chunkSizeInBytes: [204800], enableRuntimeIdCompressor: ["on", undefined, "delayed"], enableGroupedBatching: [true, false], - createBlobPlaceholders: [true, false], + createBlobPayloadPending: [true, false], explicitSchemaControl: [true, false], };