From 3d1252b2f3b7621720c27120b546a661d6a5b3ea Mon Sep 17 00:00:00 2001 From: Jatin Garg Date: Mon, 22 Jan 2024 00:42:55 -0800 Subject: [PATCH 1/9] Initial Loader and driver changes for Data Virtualization --- .../api-report/container-definitions.api.md | 2 + .../container-definitions/src/runtime.ts | 7 ++- .../api-report/driver-definitions.api.md | 33 ++++++++++ .../common/driver-definitions/src/index.ts | 3 + .../common/driver-definitions/src/storage.ts | 51 ++++++++++++++++ .../file-driver/api-report/file-driver.api.md | 4 ++ .../odsp-driver/api-report/odsp-driver.api.md | 2 +- .../src/odspDocumentStorageManager.ts | 17 +++++- .../src/odspDocumentStorageServiceBase.ts | 11 +++- .../odsp-driver/src/odspPublicUtils.ts | 1 + .../src/retryErrorsStorageAdapter.ts | 16 +++++ .../loader/container-loader/src/container.ts | 60 +++++++++++++++---- .../container-loader/src/containerContext.ts | 3 +- .../src/containerStorageAdapter.ts | 14 +++++ .../src/protocolTreeDocumentStorageService.ts | 1 + .../src/retriableDocumentStorageService.ts | 19 ++++++ .../api-report/driver-utils.api.md | 7 +++ packages/loader/driver-utils/package.json | 9 ++- .../src/documentStorageServiceProxy.ts | 14 +++++ packages/loader/driver-utils/src/index.ts | 1 + .../loader/driver-utils/src/storageUtils.ts | 18 ++++++ .../validateDriverUtilsPrevious.generated.ts | 2 + .../src/faultInjectionDriver.ts | 10 ++++ 23 files changed, 289 insertions(+), 16 deletions(-) create mode 100644 packages/loader/driver-utils/src/storageUtils.ts diff --git a/packages/common/container-definitions/api-report/container-definitions.api.md b/packages/common/container-definitions/api-report/container-definitions.api.md index 90dc8157c59a..f5f800aaf8ba 100644 --- a/packages/common/container-definitions/api-report/container-definitions.api.md +++ b/packages/common/container-definitions/api-report/container-definitions.api.md @@ -24,6 +24,7 @@ import { IResolvedUrl } from '@fluidframework/driver-definitions'; import { ISequencedDocumentMessage } from '@fluidframework/protocol-definitions'; import { ISequencedProposal } from '@fluidframework/protocol-definitions'; import { ISignalMessage } from '@fluidframework/protocol-definitions'; +import { ISnapshot } from '@fluidframework/driver-definitions'; import { ISnapshotTree } from '@fluidframework/protocol-definitions'; import { ISummaryContent } from '@fluidframework/protocol-definitions'; import { ISummaryTree } from '@fluidframework/protocol-definitions'; @@ -176,6 +177,7 @@ export interface IContainerContext { // (undocumented) readonly quorum: IQuorumClients; readonly scope: FluidObject; + readonly snapshot?: ISnapshot; // (undocumented) readonly storage: IDocumentStorageService; // (undocumented) diff --git a/packages/common/container-definitions/src/runtime.ts b/packages/common/container-definitions/src/runtime.ts index aaa48b88e4de..99ad4444c24d 100644 --- a/packages/common/container-definitions/src/runtime.ts +++ b/packages/common/container-definitions/src/runtime.ts @@ -5,7 +5,7 @@ import { ITelemetryBaseLogger, IDisposable, FluidObject } from "@fluidframework/core-interfaces"; -import { IDocumentStorageService } from "@fluidframework/driver-definitions"; +import { IDocumentStorageService, ISnapshot } from "@fluidframework/driver-definitions"; import { IClientDetails, ISequencedDocumentMessage, @@ -198,6 +198,11 @@ export interface IContainerContext { * @privateremarks Tracking in AB#5714 */ readonly id: string; + + /** + * This snapshot contains other artifacts too like blobContents, ops etc. + */ + readonly snapshot?: ISnapshot; } /** diff --git a/packages/common/driver-definitions/api-report/driver-definitions.api.md b/packages/common/driver-definitions/api-report/driver-definitions.api.md index f62a6525ce75..fbc21d45dd56 100644 --- a/packages/common/driver-definitions/api-report/driver-definitions.api.md +++ b/packages/common/driver-definitions/api-report/driver-definitions.api.md @@ -78,6 +78,14 @@ export enum FetchSource { // @alpha (undocumented) export type FiveDaysMs = 432000000; +// @alpha +export enum GetSnapshotOptions { + // (undocumented) + cacheSnapshot = "cacheSnapshot", + // (undocumented) + scenarioName = "scenarioName" +} + // @public export interface IAnyDriverError extends Omit { // (undocumented) @@ -176,12 +184,15 @@ export interface IDocumentServiceFactory { export interface IDocumentServicePolicies { readonly storageOnly?: boolean; readonly summarizeProtocolTree?: boolean; + readonly supportNewSnapshotFormat?: boolean; } // @alpha export interface IDocumentStorageService extends Partial { createBlob(file: ArrayBufferLike): Promise; downloadSummary(handle: ISummaryHandle): Promise; + // (undocumented) + getSnapshot?(version?: IVersion, snapshotFetchOptions?: ISnapshotFetchOptions): Promise; getSnapshotTree(version?: IVersion, scenarioName?: string): Promise; getVersions(versionId: string | null, count: number, scenarioName?: string, fetchSource?: FetchSource): Promise; readonly policies?: IDocumentStorageServicePolicies; @@ -255,6 +266,28 @@ export interface IResolvedUrl { url: string; } +// @alpha (undocumented) +export interface ISnapshot { + // (undocumented) + blobContents: Map; + latestSequenceNumber: number | undefined; + // (undocumented) + ops: ISequencedDocumentMessage[]; + sequenceNumber: number | undefined; + // (undocumented) + snapshotInNewFormat: true; + // (undocumented) + snapshotTree: ISnapshotTree; +} + +// @alpha (undocumented) +export interface ISnapshotFetchOptions { + // (undocumented) + [GetSnapshotOptions.cacheSnapshot]: boolean; + // (undocumented) + [GetSnapshotOptions.scenarioName]: string; +} + // @alpha export interface IStream { // (undocumented) diff --git a/packages/common/driver-definitions/src/index.ts b/packages/common/driver-definitions/src/index.ts index 9a9b6f28dcc5..9ab3b85c4568 100644 --- a/packages/common/driver-definitions/src/index.ts +++ b/packages/common/driver-definitions/src/index.ts @@ -15,6 +15,7 @@ export { IThrottlingWarning, } from "./driverError"; export { + GetSnapshotOptions, FetchSource, FiveDaysMs, IDeltasFetchResult, @@ -28,6 +29,8 @@ export { IDocumentServicePolicies, IDocumentStorageService, IDocumentStorageServicePolicies, + ISnapshot, + ISnapshotFetchOptions, IStream, IStreamResult, ISummaryContext, diff --git a/packages/common/driver-definitions/src/storage.ts b/packages/common/driver-definitions/src/storage.ts index eda933d791f0..167537465a6f 100644 --- a/packages/common/driver-definitions/src/storage.ts +++ b/packages/common/driver-definitions/src/storage.ts @@ -161,6 +161,11 @@ export interface IDocumentStorageService extends Partial { // eslint-disable-next-line @rushstack/no-new-null getSnapshotTree(version?: IVersion, scenarioName?: string): Promise; + getSnapshot?( + version?: IVersion, + snapshotFetchOptions?: ISnapshotFetchOptions, + ): Promise; + /** * Retrieves all versions of the document starting at the specified versionId - or null if from the head * @param versionId - Version id of the requested version. @@ -341,6 +346,12 @@ export interface IDocumentServicePolicies { * Summarizer uploads the protocol tree too when summarizing. */ readonly summarizeProtocolTree?: boolean; + + /** + * Whether the driver supports fetching of snapshot in new format which container all + * contents along with the snapshot tree. + */ + readonly supportNewSnapshotFormat?: boolean; } /** @@ -450,3 +461,43 @@ export enum FetchSource { default = "default", noCache = "noCache", } + +/** + * @alpha + */ +export interface ISnapshot { + snapshotTree: ISnapshotTree; + blobContents: Map; + ops: ISequencedDocumentMessage[]; + + /** + * Sequence number of the snapshot + */ + sequenceNumber: number | undefined; + + /** + * Sequence number for the latest op/snapshot for the file in ODSP + */ + latestSequenceNumber: number | undefined; + + snapshotInNewFormat: true; +} + +/** + * Additional key in the loader request header + * @alpha + */ +export enum GetSnapshotOptions { + // Indicate scenario in which the snapshot is fetched + scenarioName = "scenarioName", + // Tell driver to cache the fetched snapshot. + cacheSnapshot = "cacheSnapshot", +} + +/** + * @alpha + */ +export interface ISnapshotFetchOptions { + [GetSnapshotOptions.scenarioName]: string; + [GetSnapshotOptions.cacheSnapshot]: boolean; +} diff --git a/packages/drivers/file-driver/api-report/file-driver.api.md b/packages/drivers/file-driver/api-report/file-driver.api.md index c896479fc308..3155945769d1 100644 --- a/packages/drivers/file-driver/api-report/file-driver.api.md +++ b/packages/drivers/file-driver/api-report/file-driver.api.md @@ -22,6 +22,8 @@ import { IResolvedUrl } from '@fluidframework/driver-definitions'; import { ISequencedDocumentMessage } from '@fluidframework/protocol-definitions'; import { ISignalClient } from '@fluidframework/protocol-definitions'; import { ISignalMessage } from '@fluidframework/protocol-definitions'; +import { ISnapshot } from '@fluidframework/driver-definitions'; +import { ISnapshotFetchOptions } from '@fluidframework/driver-definitions'; import { IStream } from '@fluidframework/driver-definitions'; import { ISummaryContext } from '@fluidframework/driver-definitions'; import { ISummaryTree } from '@fluidframework/protocol-definitions'; @@ -63,6 +65,7 @@ export const FileSnapshotWriterClassFactory: (B buildTree(snapshotTree: api.ISnapshotTree): Promise; repositoryUrl: string; readonly policies?: IDocumentStorageServicePolicies | undefined; + getSnapshot?(version?: api.IVersion | undefined, snapshotFetchOptions?: ISnapshotFetchOptions | undefined): Promise; createBlob(file: ArrayBufferLike): Promise; downloadSummary(handle: api.ISummaryHandle): Promise; readonly disposed?: boolean | undefined; @@ -99,6 +102,7 @@ export const FluidFetchReaderFileSnapshotWriter: { buildTree(snapshotTree: api.ISnapshotTree): Promise; repositoryUrl: string; readonly policies?: IDocumentStorageServicePolicies | undefined; + getSnapshot?(version?: api.IVersion | undefined, snapshotFetchOptions?: ISnapshotFetchOptions | undefined): Promise; createBlob(file: ArrayBufferLike): Promise; downloadSummary(handle: api.ISummaryHandle): Promise; readonly disposed?: boolean | undefined; diff --git a/packages/drivers/odsp-driver/api-report/odsp-driver.api.md b/packages/drivers/odsp-driver/api-report/odsp-driver.api.md index 8f0ed6fd4c92..b2f5fba21e05 100644 --- a/packages/drivers/odsp-driver/api-report/odsp-driver.api.md +++ b/packages/drivers/odsp-driver/api-report/odsp-driver.api.md @@ -172,7 +172,7 @@ export interface ISharingLinkHeader { [SharingLinkHeader.isSharingLinkToRedeem]: boolean; } -// @alpha (undocumented) +// @alpha @deprecated (undocumented) export interface ISnapshotContents { // (undocumented) blobs: Map; diff --git a/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts b/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts index c5e13ddb88de..cc7c66c93b16 100644 --- a/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts +++ b/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts @@ -16,7 +16,12 @@ import { assert, delay } from "@fluidframework/core-utils"; import { LogLevel } from "@fluidframework/core-interfaces"; import * as api from "@fluidframework/protocol-definitions"; import { promiseRaceWithWinner } from "@fluidframework/driver-base"; -import { ISummaryContext, FetchSource } from "@fluidframework/driver-definitions"; +import { + ISummaryContext, + FetchSource, + ISnapshot, + ISnapshotFetchOptions, +} from "@fluidframework/driver-definitions"; import { RateLimiter, NonRetryableError } from "@fluidframework/driver-utils"; import { IOdspResolvedUrl, @@ -204,6 +209,16 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { return super.getSnapshotTree(version, scenarioName); } + public async getSnapshot( + version?: api.IVersion, + snapshotFetchOptions?: ISnapshotFetchOptions, + ): Promise { + if (!this.snapshotUrl) { + return undefined; + } + return super.getSnapshot(version, snapshotFetchOptions); + } + public async getVersions( // eslint-disable-next-line @rushstack/no-new-null blobid: string | null, diff --git a/packages/drivers/odsp-driver/src/odspDocumentStorageServiceBase.ts b/packages/drivers/odsp-driver/src/odspDocumentStorageServiceBase.ts index 18bda6e86b64..a554a3241711 100644 --- a/packages/drivers/odsp-driver/src/odspDocumentStorageServiceBase.ts +++ b/packages/drivers/odsp-driver/src/odspDocumentStorageServiceBase.ts @@ -11,9 +11,11 @@ import { LoaderCachingPolicy, FiveDaysMs, FetchSource, + ISnapshot, + ISnapshotFetchOptions, } from "@fluidframework/driver-definitions"; import * as api from "@fluidframework/protocol-definitions"; -import { IConfigProvider } from "@fluidframework/telemetry-utils"; +import { IConfigProvider, UsageError } from "@fluidframework/telemetry-utils"; import { ISnapshotContents } from "./odspPublicUtils"; const maximumCacheDurationMs: FiveDaysMs = 432000000; // 5 * 24 * 60 * 60 * 1000 = 5 days in ms @@ -208,6 +210,13 @@ export abstract class OdspDocumentStorageServiceBase implements IDocumentStorage return this.combineProtocolAndAppSnapshotTree(appTree, protocolTree); } + public async getSnapshot( + version?: api.IVersion, + snapshotFetchOptions?: ISnapshotFetchOptions, + ): Promise { + throw new UsageError("Not implemented yet!"); + } + public abstract getVersions( // eslint-disable-next-line @rushstack/no-new-null blobid: string | null, diff --git a/packages/drivers/odsp-driver/src/odspPublicUtils.ts b/packages/drivers/odsp-driver/src/odspPublicUtils.ts index 93bae1a15564..f3f8a678c418 100644 --- a/packages/drivers/odsp-driver/src/odspPublicUtils.ts +++ b/packages/drivers/odsp-driver/src/odspPublicUtils.ts @@ -16,6 +16,7 @@ export async function getHashedDocumentId(driveId: string, itemId: string): Prom /** * @alpha + * @deprecated - This is deprecated. */ export interface ISnapshotContents { snapshotTree: ISnapshotTree; diff --git a/packages/drivers/odsp-driver/src/retryErrorsStorageAdapter.ts b/packages/drivers/odsp-driver/src/retryErrorsStorageAdapter.ts index dbf4a06890de..9f4ae81d8a21 100644 --- a/packages/drivers/odsp-driver/src/retryErrorsStorageAdapter.ts +++ b/packages/drivers/odsp-driver/src/retryErrorsStorageAdapter.ts @@ -3,11 +3,14 @@ * Licensed under the MIT License. */ +import { assert } from "@fluidframework/core-utils"; import { LoggingError, ITelemetryLoggerExt } from "@fluidframework/telemetry-utils"; import { FetchSource, IDocumentStorageService, IDocumentStorageServicePolicies, + ISnapshot, + ISnapshotFetchOptions, ISummaryContext, } from "@fluidframework/driver-definitions"; import { @@ -49,6 +52,19 @@ export class RetryErrorsStorageAdapter implements IDocumentStorageService, IDisp ); } + public async getSnapshot( + version?: IVersion, + snapshotFetchOptions?: ISnapshotFetchOptions, + ): Promise { + return this.runWithRetry(async () => { + assert( + this.internalStorageService.getSnapshot !== undefined, + "getSnapshot should exist in storage adapter in ODSP driver", + ); + return this.internalStorageService.getSnapshot(version, snapshotFetchOptions); + }, "storage_getSnapshot"); + } + public async readBlob(id: string): Promise { return this.runWithRetry( async () => this.internalStorageService.readBlob(id), diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index 45c42cef4c4a..2dc0bf05d737 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -41,6 +41,7 @@ import { IDocumentServiceFactory, IDocumentStorageService, IResolvedUrl, + ISnapshot, IThrottlingWarning, IUrlResolver, } from "@fluidframework/driver-definitions"; @@ -51,6 +52,7 @@ import { runWithRetry, isCombinedAppAndProtocolSummary, MessageType2, + isInstanceOfISnapshot, } from "@fluidframework/driver-utils"; import { IQuorumSnapshot } from "@fluidframework/protocol-base"; import { @@ -1622,19 +1624,22 @@ export class Container // Fetch specified snapshot. const { snapshot, versionId } = pendingLocalState === undefined - ? await this.fetchSnapshotTree(specifiedVersion) + ? await this.fetchSnapshot(specifiedVersion) : { snapshot: pendingLocalState.baseSnapshot, versionId: undefined }; + const snapshotTree: ISnapshotTree | undefined = isInstanceOfISnapshot(snapshot) + ? snapshot.snapshotTree + : snapshot; if (pendingLocalState) { this.baseSnapshot = pendingLocalState.baseSnapshot; this.baseSnapshotBlobs = pendingLocalState.snapshotBlobs; } else { - assert(snapshot !== undefined, 0x237 /* "Snapshot should exist" */); + assert(snapshotTree !== undefined, 0x237 /* "Snapshot should exist" */); if (this.offlineLoadEnabled) { - this.baseSnapshot = snapshot; + this.baseSnapshot = snapshotTree; // Save contents of snapshot now, otherwise closeAndGetPendingLocalState() must be async this.baseSnapshotBlobs = await getBlobContentsFromTree( - snapshot, + snapshotTree, this.storageAdapter, ); } @@ -1642,7 +1647,7 @@ export class Container const attributes: IDocumentAttributes = await this.getDocumentAttributes( this.storageAdapter, - snapshot, + snapshotTree, ); // If we saved ops, we will replay them and don't need DeltaManager to fetch them @@ -1729,15 +1734,20 @@ export class Container // ...load in the existing quorum // Initialize the protocol handler - await this.initializeProtocolStateFromSnapshot(attributes, this.storageAdapter, snapshot); + await this.initializeProtocolStateFromSnapshot( + attributes, + this.storageAdapter, + snapshotTree, + ); timings.phase3 = performance.now(); const codeDetails = this.getCodeDetailsFromQuorum(); await this.instantiateRuntime( codeDetails, - snapshot, + snapshotTree, // give runtime a dummy value so it knows we're loading from a stash blob pendingLocalState ? pendingLocalState?.pendingRuntimeState ?? {} : undefined, + isInstanceOfISnapshot(snapshot) ? snapshot : undefined, ); // replay saved ops @@ -2432,10 +2442,39 @@ export class Container return { snapshot, versionId: version?.id }; } + private async fetchSnapshot( + specifiedVersion: string | undefined, + ): Promise<{ snapshot?: ISnapshot | ISnapshotTree; versionId?: string }> { + if ( + this.mc.config.getBoolean("Fluid.Container.FetchSnapshotInNewFormat") === true && + this.service?.policies?.supportNewSnapshotFormat === true + ) { + const snapshot = await this.storageAdapter.getSnapshot({ + id: specifiedVersion ?? "", + treeId: specifiedVersion ?? "", + }); + const version: IVersion = { + id: snapshot?.snapshotTree.id ?? "", + treeId: snapshot?.snapshotTree.id ?? "", + }; + this._loadedFromVersion = version; + + if (snapshot === undefined && specifiedVersion !== undefined) { + this.mc.logger.sendErrorEvent({ + eventName: "getSnapshotTreeFailed", + id: version.id, + }); + } + return { snapshot, versionId: version?.id }; + } + return this.fetchSnapshotTree(specifiedVersion); + } + private async instantiateRuntime( codeDetails: IFluidCodeDetails, - snapshot: ISnapshotTree | undefined, + snapshotTree: ISnapshotTree | undefined, pendingLocalState?: unknown, + snapshot?: ISnapshot, ) { assert(this._runtime?.disposed !== false, 0x0dd /* "Existing runtime not disposed" */); @@ -2469,12 +2508,12 @@ export class Container (this.protocolHandler.quorum.get("code") ?? this.protocolHandler.quorum.get("code2")) as IFluidCodeDetails | undefined; - const existing = snapshot !== undefined; + const existing = snapshotTree !== undefined; const context = new ContainerContext( this.options, this.scope, - snapshot, + snapshotTree, this._loadedFromVersion, this._deltaManager, this.storageAdapter, @@ -2501,6 +2540,7 @@ export class Container existing, this.subLogger, pendingLocalState, + snapshot, ); this._runtime = await PerformanceEvent.timedExecAsync( diff --git a/packages/loader/container-loader/src/containerContext.ts b/packages/loader/container-loader/src/containerContext.ts index 16927ad4145a..028a557004d6 100644 --- a/packages/loader/container-loader/src/containerContext.ts +++ b/packages/loader/container-loader/src/containerContext.ts @@ -16,7 +16,7 @@ import { IBatchMessage, } from "@fluidframework/container-definitions"; import { FluidObject } from "@fluidframework/core-interfaces"; -import { IDocumentStorageService } from "@fluidframework/driver-definitions"; +import { IDocumentStorageService, ISnapshot } from "@fluidframework/driver-definitions"; import { IClientDetails, IDocumentMessage, @@ -99,6 +99,7 @@ export class ContainerContext implements IContainerContext { public readonly existing: boolean, public readonly taggedLogger: ITelemetryLoggerExt, public readonly pendingLocalState?: unknown, + public readonly snapshot?: ISnapshot, ) {} public getLoadedFromVersion(): IVersion | undefined { diff --git a/packages/loader/container-loader/src/containerStorageAdapter.ts b/packages/loader/container-loader/src/containerStorageAdapter.ts index 6947b1f4a3fb..d93080e01277 100644 --- a/packages/loader/container-loader/src/containerStorageAdapter.ts +++ b/packages/loader/container-loader/src/containerStorageAdapter.ts @@ -13,6 +13,8 @@ import { IDocumentService, IDocumentStorageService, IDocumentStorageServicePolicies, + ISnapshot, + ISnapshotFetchOptions, ISummaryContext, } from "@fluidframework/driver-definitions"; import { UsageError } from "@fluidframework/driver-utils"; @@ -127,6 +129,17 @@ export class ContainerStorageAdapter implements IDocumentStorageService, IDispos return this._storageService.getSnapshotTree(version, scenarioName); } + public async getSnapshot( + version?: IVersion, + snapshotFetchOptions?: ISnapshotFetchOptions, + ): Promise { + assert( + this._storageService.getSnapshot !== undefined, + "getSnapshot api should exist in ContainerStorageAdapter", + ); + return this._storageService.getSnapshot(version, snapshotFetchOptions); + } + public async readBlob(id: string): Promise { const maybeBlob = this.blobContents[id]; if (maybeBlob !== undefined) { @@ -199,6 +212,7 @@ class BlobOnlyStorage implements IDocumentStorageService { /* eslint-disable @typescript-eslint/unbound-method */ public getSnapshotTree: () => Promise = this.notCalled; + public getSnapshot: () => Promise = this.notCalled; public getVersions: () => Promise = this.notCalled; public write: () => Promise = this.notCalled; public uploadSummaryWithContext: () => Promise = this.notCalled; diff --git a/packages/loader/container-loader/src/protocolTreeDocumentStorageService.ts b/packages/loader/container-loader/src/protocolTreeDocumentStorageService.ts index 533addad27d2..33987861ddfe 100644 --- a/packages/loader/container-loader/src/protocolTreeDocumentStorageService.ts +++ b/packages/loader/container-loader/src/protocolTreeDocumentStorageService.ts @@ -27,6 +27,7 @@ export class ProtocolTreeStorageService implements IDocumentStorageService, IDis } getSnapshotTree = this.internalStorageService.getSnapshotTree.bind(this.internalStorageService); + getSnapshot = this.internalStorageService.getSnapshot?.bind(this.internalStorageService); getVersions = this.internalStorageService.getVersions.bind(this.internalStorageService); createBlob = this.internalStorageService.createBlob.bind(this.internalStorageService); readBlob = this.internalStorageService.readBlob.bind(this.internalStorageService); diff --git a/packages/loader/container-loader/src/retriableDocumentStorageService.ts b/packages/loader/container-loader/src/retriableDocumentStorageService.ts index 69eed9f363a1..9d05fe8c5ba5 100644 --- a/packages/loader/container-loader/src/retriableDocumentStorageService.ts +++ b/packages/loader/container-loader/src/retriableDocumentStorageService.ts @@ -8,6 +8,8 @@ import { FetchSource, IDocumentStorageService, IDocumentStorageServicePolicies, + ISnapshot, + ISnapshotFetchOptions, ISummaryContext, } from "@fluidframework/driver-definitions"; import { @@ -64,6 +66,23 @@ export class RetriableDocumentStorageService implements IDocumentStorageService, ); } + public async getSnapshot( + version?: IVersion, + snapshotFetchOptions?: ISnapshotFetchOptions, + ): Promise { + return this.runWithRetry( + async () => + this.internalStorageServiceP.then(async (s) => { + assert( + s.getSnapshot !== undefined, + "getSnapshot api should exist in RetriableDocStorageService", + ); + return s.getSnapshot(version, snapshotFetchOptions); + }), + "storage_getSnapshot", + ); + } + public async readBlob(id: string): Promise { return this.runWithRetry( async () => this.internalStorageServiceP.then(async (s) => s.readBlob(id)), diff --git a/packages/loader/driver-utils/api-report/driver-utils.api.md b/packages/loader/driver-utils/api-report/driver-utils.api.md index b14bffe8f3e7..f999d50c7858 100644 --- a/packages/loader/driver-utils/api-report/driver-utils.api.md +++ b/packages/loader/driver-utils/api-report/driver-utils.api.md @@ -22,6 +22,8 @@ import { ILocationRedirectionError } from '@fluidframework/driver-definitions'; import { IRequest } from '@fluidframework/core-interfaces'; import { IResolvedUrl } from '@fluidframework/driver-definitions'; import { ISequencedDocumentMessage } from '@fluidframework/protocol-definitions'; +import { ISnapshot } from '@fluidframework/driver-definitions'; +import { ISnapshotFetchOptions } from '@fluidframework/driver-definitions'; import { ISnapshotTree } from '@fluidframework/protocol-definitions'; import { IStream } from '@fluidframework/driver-definitions'; import { IStreamResult } from '@fluidframework/driver-definitions'; @@ -140,6 +142,8 @@ export class DocumentStorageServiceProxy implements IDocumentStorageService { // (undocumented) downloadSummary(handle: ISummaryHandle): Promise; // (undocumented) + getSnapshot(version?: IVersion, snapshotFetchOptions?: ISnapshotFetchOptions): Promise; + // (undocumented) getSnapshotTree(version?: IVersion, scenarioName?: string): Promise; // (undocumented) getVersions(versionId: string | null, count: number, scenarioName?: string, fetchSource?: FetchSource): Promise; @@ -222,6 +226,9 @@ export interface IProgress { // @internal export function isCombinedAppAndProtocolSummary(summary: ISummaryTree | undefined, ...optionalRootTrees: string[]): summary is CombinedAppAndProtocolSummary; +// @internal +export function isInstanceOfISnapshot(obj: ISnapshotTree | ISnapshot | undefined): obj is ISnapshot; + // @internal export function isOnline(): OnlineStatus; diff --git a/packages/loader/driver-utils/package.json b/packages/loader/driver-utils/package.json index 3f194fa0e4b2..227fdd059c61 100644 --- a/packages/loader/driver-utils/package.json +++ b/packages/loader/driver-utils/package.json @@ -117,6 +117,13 @@ } }, "typeValidation": { - "broken": {} + "broken": { + "ClassDeclaration_DocumentStorageServiceProxy": { + "forwardCompat": false + }, + "ClassDeclaration_PrefetchDocumentStorageService": { + "forwardCompat": false + } + } } } diff --git a/packages/loader/driver-utils/src/documentStorageServiceProxy.ts b/packages/loader/driver-utils/src/documentStorageServiceProxy.ts index 617f3ffa84fb..f36bd3e9e615 100644 --- a/packages/loader/driver-utils/src/documentStorageServiceProxy.ts +++ b/packages/loader/driver-utils/src/documentStorageServiceProxy.ts @@ -3,10 +3,13 @@ * Licensed under the MIT License. */ +import { assert } from "@fluidframework/core-utils"; import { FetchSource, IDocumentStorageService, IDocumentStorageServicePolicies, + ISnapshot, + ISnapshotFetchOptions, ISummaryContext, } from "@fluidframework/driver-definitions"; import { @@ -44,6 +47,17 @@ export class DocumentStorageServiceProxy implements IDocumentStorageService { return this.internalStorageService.getSnapshotTree(version, scenarioName); } + public async getSnapshot( + version?: IVersion, + snapshotFetchOptions?: ISnapshotFetchOptions, + ): Promise { + assert( + this.internalStorageService.getSnapshot !== undefined, + "getSnapshot api should exist on documentStorageServiceProxy class", + ); + return this.internalStorageService.getSnapshot(version, snapshotFetchOptions); + } + public async getVersions( versionId: string | null, count: number, diff --git a/packages/loader/driver-utils/src/index.ts b/packages/loader/driver-utils/src/index.ts index 2175d603480d..a44fe3feb97e 100644 --- a/packages/loader/driver-utils/src/index.ts +++ b/packages/loader/driver-utils/src/index.ts @@ -54,3 +54,4 @@ export { SummaryCompressionAlgorithm, blobHeadersBlobName, } from "./adapters"; +export { isInstanceOfISnapshot } from "./storageUtils"; diff --git a/packages/loader/driver-utils/src/storageUtils.ts b/packages/loader/driver-utils/src/storageUtils.ts new file mode 100644 index 000000000000..a2879e86fda6 --- /dev/null +++ b/packages/loader/driver-utils/src/storageUtils.ts @@ -0,0 +1,18 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { ISnapshot } from "@fluidframework/driver-definitions"; +import { ISnapshotTree } from "@fluidframework/protocol-definitions"; + +/** + * Utility API to check if the type of snapshot contents is `ISnapshot`. + * @internal + * @param obj - obj whose type needs to be identified. + */ +export function isInstanceOfISnapshot( + obj: ISnapshotTree | ISnapshot | undefined, +): obj is ISnapshot { + return obj !== undefined && "snapshotInNewFormat" in obj && obj.snapshotInNewFormat === true; +} diff --git a/packages/loader/driver-utils/src/test/types/validateDriverUtilsPrevious.generated.ts b/packages/loader/driver-utils/src/test/types/validateDriverUtilsPrevious.generated.ts index 0124fda82795..b2fa55644651 100644 --- a/packages/loader/driver-utils/src/test/types/validateDriverUtilsPrevious.generated.ts +++ b/packages/loader/driver-utils/src/test/types/validateDriverUtilsPrevious.generated.ts @@ -151,6 +151,7 @@ declare function get_old_ClassDeclaration_DocumentStorageServiceProxy(): declare function use_current_ClassDeclaration_DocumentStorageServiceProxy( use: TypeOnly): void; use_current_ClassDeclaration_DocumentStorageServiceProxy( + // @ts-expect-error compatibility expected to be broken get_old_ClassDeclaration_DocumentStorageServiceProxy()); /* @@ -463,6 +464,7 @@ declare function get_old_ClassDeclaration_PrefetchDocumentStorageService(): declare function use_current_ClassDeclaration_PrefetchDocumentStorageService( use: TypeOnly): void; use_current_ClassDeclaration_PrefetchDocumentStorageService( + // @ts-expect-error compatibility expected to be broken get_old_ClassDeclaration_PrefetchDocumentStorageService()); /* diff --git a/packages/test/test-service-load/src/faultInjectionDriver.ts b/packages/test/test-service-load/src/faultInjectionDriver.ts index 7af642839907..a5e6eb4b7b06 100644 --- a/packages/test/test-service-load/src/faultInjectionDriver.ts +++ b/packages/test/test-service-load/src/faultInjectionDriver.ts @@ -16,6 +16,7 @@ import { IDocumentServiceFactory, IDocumentStorageService, IResolvedUrl, + ISnapshotFetchOptions, } from "@fluidframework/driver-definitions"; import { IClient, @@ -352,6 +353,15 @@ export class FaultInjectionDocumentStorageService implements IDocumentStorageSer return this.internal.getSnapshotTree(version, scenarioName); } + public async getSnapshot(version, snapshotFetchOptions?: ISnapshotFetchOptions) { + this.throwIfOffline(); + assert( + this.internal.getSnapshot !== undefined, + "getSnapshot api should exist in faultInjectionStorageService", + ); + return this.internal.getSnapshot(version, snapshotFetchOptions); + } + public async getVersions(versionId, count, scenarioName, fetchSource) { this.throwIfOffline(); return this.internal.getVersions(versionId, count, scenarioName, fetchSource); From 4c354d0082a07cda658e448fb987467594926219 Mon Sep 17 00:00:00 2001 From: Jatin Garg Date: Mon, 29 Jan 2024 19:25:20 -0800 Subject: [PATCH 2/9] pr sugg --- .../api-report/container-definitions.api.md | 2 +- .../container-definitions/src/runtime.ts | 4 +- .../api-report/driver-definitions.api.md | 22 +++------ .../common/driver-definitions/src/index.ts | 1 - .../common/driver-definitions/src/storage.ts | 49 +++++++++++-------- .../file-driver/api-report/file-driver.api.md | 4 +- .../loader/container-loader/src/container.ts | 20 ++++---- .../container-loader/src/containerContext.ts | 2 +- .../src/containerStorageAdapter.ts | 16 +++--- .../src/retriableDocumentStorageService.ts | 16 +++--- .../api-report/driver-utils.api.md | 2 +- .../src/documentStorageServiceProxy.ts | 16 +++--- .../src/faultInjectionDriver.ts | 13 +++-- 13 files changed, 78 insertions(+), 89 deletions(-) diff --git a/packages/common/container-definitions/api-report/container-definitions.api.md b/packages/common/container-definitions/api-report/container-definitions.api.md index f5f800aaf8ba..15a6f8b9093c 100644 --- a/packages/common/container-definitions/api-report/container-definitions.api.md +++ b/packages/common/container-definitions/api-report/container-definitions.api.md @@ -177,7 +177,7 @@ export interface IContainerContext { // (undocumented) readonly quorum: IQuorumClients; readonly scope: FluidObject; - readonly snapshot?: ISnapshot; + readonly snapshotWithContents?: ISnapshot; // (undocumented) readonly storage: IDocumentStorageService; // (undocumented) diff --git a/packages/common/container-definitions/src/runtime.ts b/packages/common/container-definitions/src/runtime.ts index 99ad4444c24d..2f8943618e80 100644 --- a/packages/common/container-definitions/src/runtime.ts +++ b/packages/common/container-definitions/src/runtime.ts @@ -200,9 +200,9 @@ export interface IContainerContext { readonly id: string; /** - * This snapshot contains other artifacts too like blobContents, ops etc. + * This contains all parts of a snapshot like blobContents, ops etc. */ - readonly snapshot?: ISnapshot; + readonly snapshotWithContents?: ISnapshot; } /** diff --git a/packages/common/driver-definitions/api-report/driver-definitions.api.md b/packages/common/driver-definitions/api-report/driver-definitions.api.md index fbc21d45dd56..c1c4d699e3dc 100644 --- a/packages/common/driver-definitions/api-report/driver-definitions.api.md +++ b/packages/common/driver-definitions/api-report/driver-definitions.api.md @@ -78,14 +78,6 @@ export enum FetchSource { // @alpha (undocumented) export type FiveDaysMs = 432000000; -// @alpha -export enum GetSnapshotOptions { - // (undocumented) - cacheSnapshot = "cacheSnapshot", - // (undocumented) - scenarioName = "scenarioName" -} - // @public export interface IAnyDriverError extends Omit { // (undocumented) @@ -184,15 +176,14 @@ export interface IDocumentServiceFactory { export interface IDocumentServicePolicies { readonly storageOnly?: boolean; readonly summarizeProtocolTree?: boolean; - readonly supportNewSnapshotFormat?: boolean; + readonly supportGetSnapshotApi?: boolean; } // @alpha export interface IDocumentStorageService extends Partial { createBlob(file: ArrayBufferLike): Promise; downloadSummary(handle: ISummaryHandle): Promise; - // (undocumented) - getSnapshot?(version?: IVersion, snapshotFetchOptions?: ISnapshotFetchOptions): Promise; + getSnapshot?(snapshotFetchOptions?: ISnapshotFetchOptions): Promise; getSnapshotTree(version?: IVersion, scenarioName?: string): Promise; getVersions(versionId: string | null, count: number, scenarioName?: string, fetchSource?: FetchSource): Promise; readonly policies?: IDocumentStorageServicePolicies; @@ -280,12 +271,11 @@ export interface ISnapshot { snapshotTree: ISnapshotTree; } -// @alpha (undocumented) +// @alpha export interface ISnapshotFetchOptions { - // (undocumented) - [GetSnapshotOptions.cacheSnapshot]: boolean; - // (undocumented) - [GetSnapshotOptions.scenarioName]: string; + cacheSnapshot?: boolean; + scenarioName?: string; + versionId?: string; } // @alpha diff --git a/packages/common/driver-definitions/src/index.ts b/packages/common/driver-definitions/src/index.ts index 9ab3b85c4568..9b35494685c5 100644 --- a/packages/common/driver-definitions/src/index.ts +++ b/packages/common/driver-definitions/src/index.ts @@ -15,7 +15,6 @@ export { IThrottlingWarning, } from "./driverError"; export { - GetSnapshotOptions, FetchSource, FiveDaysMs, IDeltasFetchResult, diff --git a/packages/common/driver-definitions/src/storage.ts b/packages/common/driver-definitions/src/storage.ts index 167537465a6f..615646264a39 100644 --- a/packages/common/driver-definitions/src/storage.ts +++ b/packages/common/driver-definitions/src/storage.ts @@ -161,10 +161,13 @@ export interface IDocumentStorageService extends Partial { // eslint-disable-next-line @rushstack/no-new-null getSnapshotTree(version?: IVersion, scenarioName?: string): Promise; - getSnapshot?( - version?: IVersion, - snapshotFetchOptions?: ISnapshotFetchOptions, - ): Promise; + /** + * Returns the snapshot which can contain other artifacts too like blob contents, ops etc. It is different from + * `getSnapshotTree` api in that, that API only returns the snapshot tree from the snapshot. + * @param snapshotFetchOptions - Options specified by the caller to specify and want certain behavior from the + * driver when fetching the snapshot. + */ + getSnapshot?(snapshotFetchOptions?: ISnapshotFetchOptions): Promise; /** * Retrieves all versions of the document starting at the specified versionId - or null if from the head @@ -348,10 +351,11 @@ export interface IDocumentServicePolicies { readonly summarizeProtocolTree?: boolean; /** - * Whether the driver supports fetching of snapshot in new format which container all - * contents along with the snapshot tree. + * Whether the driver supports the new getSnapshot api which returns snapshot which + * contains all contents along with the snapshot tree. Enable this by default when the + * driver can fully support the api. */ - readonly supportNewSnapshotFormat?: boolean; + readonly supportGetSnapshotApi?: boolean; } /** @@ -484,20 +488,25 @@ export interface ISnapshot { } /** - * Additional key in the loader request header - * @alpha - */ -export enum GetSnapshotOptions { - // Indicate scenario in which the snapshot is fetched - scenarioName = "scenarioName", - // Tell driver to cache the fetched snapshot. - cacheSnapshot = "cacheSnapshot", -} - -/** + * Snapshot fetch options which are used to communicate different things to the driver + * when fetching the snapshot. * @alpha */ export interface ISnapshotFetchOptions { - [GetSnapshotOptions.scenarioName]: string; - [GetSnapshotOptions.cacheSnapshot]: boolean; + /** + * Indicates scenario in which the snapshot is fetched. It is a free form string mostly + * used for telemetry purposes. + */ + scenarioName?: string; + /** + * Tell driver to cache the fetched snapshot. Driver is supposed to cache the fetched snapshot if this is + * set to true. If undefined, then it is upto the driver, to cache it or not. + */ + cacheSnapshot?: boolean; + + /** + * Version of the snapshot to be fetched. Certain storage services just keep 1 snapshot for the + * container, so specifying version is not necessary for storage services. + */ + versionId?: string; } diff --git a/packages/drivers/file-driver/api-report/file-driver.api.md b/packages/drivers/file-driver/api-report/file-driver.api.md index 3155945769d1..a833db4091c8 100644 --- a/packages/drivers/file-driver/api-report/file-driver.api.md +++ b/packages/drivers/file-driver/api-report/file-driver.api.md @@ -65,7 +65,7 @@ export const FileSnapshotWriterClassFactory: (B buildTree(snapshotTree: api.ISnapshotTree): Promise; repositoryUrl: string; readonly policies?: IDocumentStorageServicePolicies | undefined; - getSnapshot?(version?: api.IVersion | undefined, snapshotFetchOptions?: ISnapshotFetchOptions | undefined): Promise; + getSnapshot?(snapshotFetchOptions?: ISnapshotFetchOptions | undefined): Promise; createBlob(file: ArrayBufferLike): Promise; downloadSummary(handle: api.ISummaryHandle): Promise; readonly disposed?: boolean | undefined; @@ -102,7 +102,7 @@ export const FluidFetchReaderFileSnapshotWriter: { buildTree(snapshotTree: api.ISnapshotTree): Promise; repositoryUrl: string; readonly policies?: IDocumentStorageServicePolicies | undefined; - getSnapshot?(version?: api.IVersion | undefined, snapshotFetchOptions?: ISnapshotFetchOptions | undefined): Promise; + getSnapshot?(snapshotFetchOptions?: ISnapshotFetchOptions | undefined): Promise; createBlob(file: ArrayBufferLike): Promise; downloadSummary(handle: api.ISummaryHandle): Promise; readonly disposed?: boolean | undefined; diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index 2dc0bf05d737..1fcda21cebb9 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -2446,16 +2446,16 @@ export class Container specifiedVersion: string | undefined, ): Promise<{ snapshot?: ISnapshot | ISnapshotTree; versionId?: string }> { if ( - this.mc.config.getBoolean("Fluid.Container.FetchSnapshotInNewFormat") === true && - this.service?.policies?.supportNewSnapshotFormat === true + this.mc.config.getBoolean("Fluid.Container.FetchSnapshotUsingGetSnapshotApi") === + true && + this.service?.policies?.supportGetSnapshotApi === true ) { const snapshot = await this.storageAdapter.getSnapshot({ - id: specifiedVersion ?? "", - treeId: specifiedVersion ?? "", + versionId: specifiedVersion, }); const version: IVersion = { - id: snapshot?.snapshotTree.id ?? "", - treeId: snapshot?.snapshotTree.id ?? "", + id: snapshot.snapshotTree.id ?? "", + treeId: snapshot.snapshotTree.id ?? "", }; this._loadedFromVersion = version; @@ -2465,7 +2465,7 @@ export class Container id: version.id, }); } - return { snapshot, versionId: version?.id }; + return { snapshot, versionId: version.id }; } return this.fetchSnapshotTree(specifiedVersion); } @@ -2508,8 +2508,6 @@ export class Container (this.protocolHandler.quorum.get("code") ?? this.protocolHandler.quorum.get("code2")) as IFluidCodeDetails | undefined; - const existing = snapshotTree !== undefined; - const context = new ContainerContext( this.options, this.scope, @@ -2537,7 +2535,7 @@ export class Container () => this.connected, getSpecifiedCodeDetails, this._deltaManager.clientDetails, - existing, + true, // existing this.subLogger, pendingLocalState, snapshot, @@ -2546,7 +2544,7 @@ export class Container this._runtime = await PerformanceEvent.timedExecAsync( this.subLogger, { eventName: "InstantiateRuntime" }, - async () => runtimeFactory.instantiateRuntime(context, existing), + async () => runtimeFactory.instantiateRuntime(context, true /* existing */), ); this._lifecycleEvents.emit("runtimeInstantiated"); diff --git a/packages/loader/container-loader/src/containerContext.ts b/packages/loader/container-loader/src/containerContext.ts index 028a557004d6..28d8f7abbfdb 100644 --- a/packages/loader/container-loader/src/containerContext.ts +++ b/packages/loader/container-loader/src/containerContext.ts @@ -99,7 +99,7 @@ export class ContainerContext implements IContainerContext { public readonly existing: boolean, public readonly taggedLogger: ITelemetryLoggerExt, public readonly pendingLocalState?: unknown, - public readonly snapshot?: ISnapshot, + public readonly snapshotWithContents?: ISnapshot, ) {} public getLoadedFromVersion(): IVersion | undefined { diff --git a/packages/loader/container-loader/src/containerStorageAdapter.ts b/packages/loader/container-loader/src/containerStorageAdapter.ts index d93080e01277..bcd562f9e1db 100644 --- a/packages/loader/container-loader/src/containerStorageAdapter.ts +++ b/packages/loader/container-loader/src/containerStorageAdapter.ts @@ -129,15 +129,13 @@ export class ContainerStorageAdapter implements IDocumentStorageService, IDispos return this._storageService.getSnapshotTree(version, scenarioName); } - public async getSnapshot( - version?: IVersion, - snapshotFetchOptions?: ISnapshotFetchOptions, - ): Promise { - assert( - this._storageService.getSnapshot !== undefined, - "getSnapshot api should exist in ContainerStorageAdapter", + public async getSnapshot(snapshotFetchOptions?: ISnapshotFetchOptions): Promise { + if (this._storageService.getSnapshot !== undefined) { + return this._storageService.getSnapshot(snapshotFetchOptions); + } + throw new UsageError( + "getSnapshot api should exist in internal storage in ContainerStorageAdapter", ); - return this._storageService.getSnapshot(version, snapshotFetchOptions); } public async readBlob(id: string): Promise { @@ -212,7 +210,7 @@ class BlobOnlyStorage implements IDocumentStorageService { /* eslint-disable @typescript-eslint/unbound-method */ public getSnapshotTree: () => Promise = this.notCalled; - public getSnapshot: () => Promise = this.notCalled; + public getSnapshot: () => Promise = this.notCalled; public getVersions: () => Promise = this.notCalled; public write: () => Promise = this.notCalled; public uploadSummaryWithContext: () => Promise = this.notCalled; diff --git a/packages/loader/container-loader/src/retriableDocumentStorageService.ts b/packages/loader/container-loader/src/retriableDocumentStorageService.ts index 9d05fe8c5ba5..8ea48a49d70b 100644 --- a/packages/loader/container-loader/src/retriableDocumentStorageService.ts +++ b/packages/loader/container-loader/src/retriableDocumentStorageService.ts @@ -20,7 +20,7 @@ import { IVersion, } from "@fluidframework/protocol-definitions"; import { IDisposable } from "@fluidframework/core-interfaces"; -import { GenericError, ITelemetryLoggerExt } from "@fluidframework/telemetry-utils"; +import { GenericError, ITelemetryLoggerExt, UsageError } from "@fluidframework/telemetry-utils"; import { runWithRetry } from "@fluidframework/driver-utils"; export class RetriableDocumentStorageService implements IDocumentStorageService, IDisposable { @@ -66,18 +66,16 @@ export class RetriableDocumentStorageService implements IDocumentStorageService, ); } - public async getSnapshot( - version?: IVersion, - snapshotFetchOptions?: ISnapshotFetchOptions, - ): Promise { + public async getSnapshot(snapshotFetchOptions?: ISnapshotFetchOptions): Promise { return this.runWithRetry( async () => this.internalStorageServiceP.then(async (s) => { - assert( - s.getSnapshot !== undefined, - "getSnapshot api should exist in RetriableDocStorageService", + if (s.getSnapshot !== undefined) { + return s.getSnapshot(snapshotFetchOptions); + } + throw new UsageError( + "getSnapshot api should exist on internal storage in RetriableDocStorageService class", ); - return s.getSnapshot(version, snapshotFetchOptions); }), "storage_getSnapshot", ); diff --git a/packages/loader/driver-utils/api-report/driver-utils.api.md b/packages/loader/driver-utils/api-report/driver-utils.api.md index f999d50c7858..77301fd4e607 100644 --- a/packages/loader/driver-utils/api-report/driver-utils.api.md +++ b/packages/loader/driver-utils/api-report/driver-utils.api.md @@ -142,7 +142,7 @@ export class DocumentStorageServiceProxy implements IDocumentStorageService { // (undocumented) downloadSummary(handle: ISummaryHandle): Promise; // (undocumented) - getSnapshot(version?: IVersion, snapshotFetchOptions?: ISnapshotFetchOptions): Promise; + getSnapshot(snapshotFetchOptions?: ISnapshotFetchOptions): Promise; // (undocumented) getSnapshotTree(version?: IVersion, scenarioName?: string): Promise; // (undocumented) diff --git a/packages/loader/driver-utils/src/documentStorageServiceProxy.ts b/packages/loader/driver-utils/src/documentStorageServiceProxy.ts index f36bd3e9e615..3864ba55cc1f 100644 --- a/packages/loader/driver-utils/src/documentStorageServiceProxy.ts +++ b/packages/loader/driver-utils/src/documentStorageServiceProxy.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. */ -import { assert } from "@fluidframework/core-utils"; import { FetchSource, IDocumentStorageService, @@ -19,6 +18,7 @@ import { ISummaryTree, IVersion, } from "@fluidframework/protocol-definitions"; +import { UsageError } from "."; /** * @internal @@ -47,15 +47,13 @@ export class DocumentStorageServiceProxy implements IDocumentStorageService { return this.internalStorageService.getSnapshotTree(version, scenarioName); } - public async getSnapshot( - version?: IVersion, - snapshotFetchOptions?: ISnapshotFetchOptions, - ): Promise { - assert( - this.internalStorageService.getSnapshot !== undefined, - "getSnapshot api should exist on documentStorageServiceProxy class", + public async getSnapshot(snapshotFetchOptions?: ISnapshotFetchOptions): Promise { + if (this.internalStorageService.getSnapshot !== undefined) { + return this.internalStorageService.getSnapshot(snapshotFetchOptions); + } + throw new UsageError( + "getSnapshot api should exist on internal storage in documentStorageServiceProxy class", ); - return this.internalStorageService.getSnapshot(version, snapshotFetchOptions); } public async getVersions( diff --git a/packages/test/test-service-load/src/faultInjectionDriver.ts b/packages/test/test-service-load/src/faultInjectionDriver.ts index a5e6eb4b7b06..7c3dca044d69 100644 --- a/packages/test/test-service-load/src/faultInjectionDriver.ts +++ b/packages/test/test-service-load/src/faultInjectionDriver.ts @@ -25,7 +25,7 @@ import { INack, NackErrorType, } from "@fluidframework/protocol-definitions"; -import { LoggingError, wrapError } from "@fluidframework/telemetry-utils"; +import { LoggingError, UsageError, wrapError } from "@fluidframework/telemetry-utils"; export class FaultInjectionDocumentServiceFactory implements IDocumentServiceFactory { private readonly _documentServices = new Map(); @@ -353,13 +353,12 @@ export class FaultInjectionDocumentStorageService implements IDocumentStorageSer return this.internal.getSnapshotTree(version, scenarioName); } - public async getSnapshot(version, snapshotFetchOptions?: ISnapshotFetchOptions) { + public async getSnapshot(snapshotFetchOptions?: ISnapshotFetchOptions) { this.throwIfOffline(); - assert( - this.internal.getSnapshot !== undefined, - "getSnapshot api should exist in faultInjectionStorageService", - ); - return this.internal.getSnapshot(version, snapshotFetchOptions); + if (this.internal.getSnapshot !== undefined) { + return this.internal.getSnapshot(snapshotFetchOptions); + } + throw new UsageError("GetSnapshotApi not present"); } public async getVersions(versionId, count, scenarioName, fetchSource) { From cc764101d1a4d991b9827d22cb1e2417af137e4f Mon Sep 17 00:00:00 2001 From: Jatin Garg Date: Mon, 29 Jan 2024 19:47:25 -0800 Subject: [PATCH 3/9] Remove ISnapshotContents --- .../odsp-driver/api-report/odsp-driver.api.md | 5 +- packages/drivers/odsp-driver/package.json | 11 +++- .../odsp-driver/src/compactSnapshotParser.ts | 19 +++--- .../odsp-driver/src/compactSnapshotWriter.ts | 6 +- packages/drivers/odsp-driver/src/contracts.ts | 12 ++++ .../drivers/odsp-driver/src/createFile.ts | 4 +- .../src/createNewContainerOnExistingFile.ts | 4 +- .../drivers/odsp-driver/src/createNewUtils.ts | 13 ++-- .../drivers/odsp-driver/src/fetchSnapshot.ts | 22 +++---- .../localOdspDocumentStorageManager.ts | 13 +++- packages/drivers/odsp-driver/src/odspCache.ts | 4 +- .../odsp-driver/src/odspDocumentService.ts | 1 + .../src/odspDocumentStorageManager.ts | 63 ++++++++++++------- .../src/odspDocumentStorageServiceBase.ts | 18 ++---- .../odsp-driver/src/odspSnapshotParser.ts | 11 ++-- packages/drivers/odsp-driver/src/odspUtils.ts | 14 ++++- .../src/retryErrorsStorageAdapter.ts | 17 ++--- .../src/test/createNewUtilsTests.spec.ts | 11 ++-- .../src/test/fetchSnapshot.spec.ts | 8 ++- .../odsp-driver/src/test/getVersions.spec.ts | 13 ++-- .../src/test/jsonSnapshotFormatTests.spec.ts | 2 +- .../src/test/prefetchSnapshotTests.spec.ts | 9 +-- .../src/test/snapshotFormatTests.spec.ts | 18 +++--- .../validateOdspDriverPrevious.generated.ts | 4 ++ 24 files changed, 181 insertions(+), 121 deletions(-) diff --git a/packages/drivers/odsp-driver/api-report/odsp-driver.api.md b/packages/drivers/odsp-driver/api-report/odsp-driver.api.md index b2f5fba21e05..006d6298e9bd 100644 --- a/packages/drivers/odsp-driver/api-report/odsp-driver.api.md +++ b/packages/drivers/odsp-driver/api-report/odsp-driver.api.md @@ -20,6 +20,7 @@ import { IRequest } from '@fluidframework/core-interfaces'; import { IResolvedUrl } from '@fluidframework/driver-definitions'; import { ISequencedDocumentMessage } from '@fluidframework/protocol-definitions'; import { ISharingLinkKind } from '@fluidframework/odsp-driver-definitions'; +import { ISnapshot } from '@fluidframework/driver-definitions'; import { ISnapshotOptions } from '@fluidframework/odsp-driver-definitions'; import { ISnapshotTree } from '@fluidframework/protocol-definitions'; import { ISocketStorageDiscovery } from '@fluidframework/odsp-driver-definitions'; @@ -159,7 +160,7 @@ export interface IPersistedFileCache { } // @alpha (undocumented) -export interface IPrefetchSnapshotContents extends ISnapshotContents { +export interface IPrefetchSnapshotContents extends ISnapshot { // (undocumented) fluidEpoch: string; // (undocumented) @@ -185,7 +186,7 @@ export interface ISnapshotContents { } // @internal -export interface ISnapshotContentsWithProps extends ISnapshotContents { +export interface ISnapshotContentsWithProps extends ISnapshot { // (undocumented) telemetryProps: Record; } diff --git a/packages/drivers/odsp-driver/package.json b/packages/drivers/odsp-driver/package.json index 7aa5a6c51af3..d39b400db60d 100644 --- a/packages/drivers/odsp-driver/package.json +++ b/packages/drivers/odsp-driver/package.json @@ -120,6 +120,15 @@ } }, "typeValidation": { - "broken": {} + "broken": { + "InterfaceDeclaration_ISnapshotContentsWithProps": { + "backCompat": false, + "forwardCompat": false + }, + "InterfaceDeclaration_IPrefetchSnapshotContents": { + "backCompat": false, + "forwardCompat": false + } + } } } diff --git a/packages/drivers/odsp-driver/src/compactSnapshotParser.ts b/packages/drivers/odsp-driver/src/compactSnapshotParser.ts index c274ad0592df..2a4ed4ae0c2d 100644 --- a/packages/drivers/odsp-driver/src/compactSnapshotParser.ts +++ b/packages/drivers/odsp-driver/src/compactSnapshotParser.ts @@ -6,7 +6,7 @@ import { assert } from "@fluidframework/core-utils"; import { ISequencedDocumentMessage, ISnapshotTree } from "@fluidframework/protocol-definitions"; import { ITelemetryLoggerExt } from "@fluidframework/telemetry-utils"; -import { ISnapshotContents } from "./odspPublicUtils"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { ReadBuffer } from "./ReadBufferUtils"; import { assertBlobCoreInstance, @@ -29,7 +29,7 @@ export const currentReadVersion = "1.0"; * represents how many times slower parsing path is executed. This will be then logged into telemetry. * @internal */ -export interface ISnapshotContentsWithProps extends ISnapshotContents { +export interface ISnapshotContentsWithProps extends ISnapshot { telemetryProps: Record; } @@ -40,7 +40,7 @@ export interface ISnapshotContentsWithProps extends ISnapshotContents { function readBlobSection(node: NodeTypes) { assertNodeCoreInstance(node, "TreeBlobs should be of type NodeCore"); let slowBlobStructureCount = 0; - const blobs: Map = new Map(); + const blobContents: Map = new Map(); for (const blob of node) { assertNodeCoreInstance(blob, "blob should be node"); @@ -56,7 +56,7 @@ function readBlobSection(node: NodeTypes) { ) { // "id": // "data": - blobs.set(blob.getString(1), blob.getBlob(3).arrayBuffer); + blobContents.set(blob.getString(1), blob.getBlob(3).arrayBuffer); continue; } @@ -67,9 +67,9 @@ function readBlobSection(node: NodeTypes) { const records = getNodeProps(blob); assertBlobCoreInstance(records.data, "data should be of BlobCore type"); const id = getStringInstance(records.id, "blob id should be string"); - blobs.set(id, records.data.arrayBuffer); + blobContents.set(id, records.data.arrayBuffer); } - return { blobs, slowBlobStructureCount }; + return { blobContents, slowBlobStructureCount }; } /** @@ -249,19 +249,20 @@ export function parseCompactSnapshotResponse( ); const [snapshot, durationSnapshotTree] = measure(() => readSnapshotSection(records.snapshot)); - const [blobs, durationBlobs] = measure(() => readBlobSection(records.blobs)); + const [blobContents, durationBlobs] = measure(() => readBlobSection(records.blobs)); return { ...snapshot, - ...blobs, + ...blobContents, ops: records.deltas !== undefined ? readOpsSection(records.deltas) : [], latestSequenceNumber: records.lsn, + snapshotInNewFormat: true, telemetryProps: { ...telemetryProps, durationSnapshotTree, durationBlobs, slowTreeStructureCount: snapshot.slowTreeStructureCount, - slowBlobStructureCount: blobs.slowBlobStructureCount, + slowBlobStructureCount: blobContents.slowBlobStructureCount, }, }; } diff --git a/packages/drivers/odsp-driver/src/compactSnapshotWriter.ts b/packages/drivers/odsp-driver/src/compactSnapshotWriter.ts index 7ace2f08d7f6..5e40bfe3d512 100644 --- a/packages/drivers/odsp-driver/src/compactSnapshotWriter.ts +++ b/packages/drivers/odsp-driver/src/compactSnapshotWriter.ts @@ -10,8 +10,8 @@ import { ISequencedDocumentMessage, ISnapshotTree, } from "@fluidframework/protocol-definitions"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { snapshotMinReadVersion } from "./compactSnapshotParser"; -import { ISnapshotContents } from "./odspPublicUtils"; import { TreeBuilderSerializer } from "./WriteBufferUtils"; import { addBoolProperty, @@ -144,7 +144,7 @@ function writeOpsSection(rootNode: NodeCore, ops: ISequencedDocumentMessage[]) { * @param snapshotContents - snapshot tree contents to serialize * @returns ReadBuffer - binary representation of the data. */ -export function convertToCompactSnapshot(snapshotContents: ISnapshotContents): Uint8Array { +export function convertToCompactSnapshot(snapshotContents: ISnapshot): Uint8Array { const builder = new TreeBuilderSerializer(); // Create the root node. const rootNode = builder.addNode(); @@ -166,7 +166,7 @@ export function convertToCompactSnapshot(snapshotContents: ISnapshotContents): U writeSnapshotSection(rootNode, snapshotContents.snapshotTree, snapshotContents.sequenceNumber); // Add Blobs - writeBlobsSection(rootNode, snapshotContents.blobs); + writeBlobsSection(rootNode, snapshotContents.blobContents); // Then write the ops node. writeOpsSection(rootNode, snapshotContents.ops); diff --git a/packages/drivers/odsp-driver/src/contracts.ts b/packages/drivers/odsp-driver/src/contracts.ts index fb5e38e8a929..4206affa76a7 100644 --- a/packages/drivers/odsp-driver/src/contracts.ts +++ b/packages/drivers/odsp-driver/src/contracts.ts @@ -5,6 +5,7 @@ import * as api from "@fluidframework/protocol-definitions"; import { HostStoragePolicy } from "@fluidframework/odsp-driver-definitions"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { ISnapshotContents } from "./odspPublicUtils"; /** @@ -165,6 +166,8 @@ export interface IOdspSnapshot { */ export interface HostStoragePolicyInternal extends HostStoragePolicy { summarizerClient?: boolean; + + supportGetSnapshotApi?: boolean; } export interface ICreateFileResponse { @@ -207,11 +210,20 @@ export interface IFlushOpsResponse { /** * Represents the cached snapshot value. + * @deprecated - This will be replaced with ISnapshotCachedEntry2 which wraps the new ISnapshot interface. + * For now, to support back compat from cache, we need to keep it for now. */ export interface ISnapshotCachedEntry extends ISnapshotContents { cacheEntryTime: number; } +/** + * Represents the cached snapshot value. + */ +export interface ISnapshotCachedEntry2 extends ISnapshot { + cacheEntryTime: number; +} + /** * Represents the type of signal containing the sensitivity policy labels for the container. */ diff --git a/packages/drivers/odsp-driver/src/createFile.ts b/packages/drivers/odsp-driver/src/createFile.ts index 28a516934efd..898f658599c0 100644 --- a/packages/drivers/odsp-driver/src/createFile.ts +++ b/packages/drivers/odsp-driver/src/createFile.ts @@ -14,6 +14,7 @@ import { ShareLinkInfoType, IFileEntry, } from "@fluidframework/odsp-driver-definitions"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { ICreateFileResponse } from "./contracts"; import { getUrlAndHeadersWithAuth } from "./getUrlAndHeadersWithAuth"; import { @@ -23,7 +24,6 @@ import { INewFileInfo, getOrigin, } from "./odspUtils"; -import { ISnapshotContents } from "./odspPublicUtils"; import { createOdspUrl } from "./createOdspUrl"; import { getApiRoot } from "./odspUrlHelper"; import { EpochTracker } from "./epochTracker"; @@ -108,7 +108,7 @@ export async function createNewFluidFile( if (createNewSummary !== undefined && createNewCaching) { assert(summaryHandle !== undefined, 0x203 /* "Summary handle is undefined" */); // converting summary and getting sequence number - const snapshot: ISnapshotContents = convertCreateNewSummaryTreeToTreeAndBlobs( + const snapshot: ISnapshot = convertCreateNewSummaryTreeToTreeAndBlobs( createNewSummary, summaryHandle, ); diff --git a/packages/drivers/odsp-driver/src/createNewContainerOnExistingFile.ts b/packages/drivers/odsp-driver/src/createNewContainerOnExistingFile.ts index 8c55c0a75b3f..b60d74d0378d 100644 --- a/packages/drivers/odsp-driver/src/createNewContainerOnExistingFile.ts +++ b/packages/drivers/odsp-driver/src/createNewContainerOnExistingFile.ts @@ -6,6 +6,7 @@ import { ISummaryTree } from "@fluidframework/protocol-definitions"; import { ITelemetryLoggerExt } from "@fluidframework/telemetry-utils"; import { UsageError } from "@fluidframework/driver-utils"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { IFileEntry, InstrumentedStorageTokenFetcher, @@ -13,7 +14,6 @@ import { } from "@fluidframework/odsp-driver-definitions"; import { IWriteSummaryResponse } from "./contracts"; import { createCacheSnapshotKey, getOrigin, IExistingFileInfo } from "./odspUtils"; -import { ISnapshotContents } from "./odspPublicUtils"; import { createOdspUrl } from "./createOdspUrl"; import { getApiRoot } from "./odspUrlHelper"; import { EpochTracker } from "./epochTracker"; @@ -81,7 +81,7 @@ export async function createNewContainerOnExistingFile( if (createNewCaching) { // converting summary and getting sequence number - const snapshot: ISnapshotContents = convertCreateNewSummaryTreeToTreeAndBlobs( + const snapshot: ISnapshot = convertCreateNewSummaryTreeToTreeAndBlobs( createNewSummary, summaryHandle, ); diff --git a/packages/drivers/odsp-driver/src/createNewUtils.ts b/packages/drivers/odsp-driver/src/createNewUtils.ts index 4ad60f88db4b..a0f0c05cab08 100644 --- a/packages/drivers/odsp-driver/src/createNewUtils.ts +++ b/packages/drivers/odsp-driver/src/createNewUtils.ts @@ -19,6 +19,7 @@ import { unreachableCase } from "@fluidframework/core-utils"; import { getGitType } from "@fluidframework/protocol-base"; import { ITelemetryLoggerExt, PerformanceEvent } from "@fluidframework/telemetry-utils"; import { InstrumentedStorageTokenFetcher } from "@fluidframework/odsp-driver-definitions"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { IOdspSummaryPayload, IOdspSummaryTree, @@ -26,7 +27,6 @@ import { OdspSummaryTreeValue, } from "./contracts"; import { getWithRetryForTokenRefresh, maxUmpPostBodySize } from "./odspUtils"; -import { ISnapshotContents } from "./odspPublicUtils"; import { EpochTracker, FetchType } from "./epochTracker"; import { getUrlAndHeadersWithAuth } from "./getUrlAndHeadersWithAuth"; import { runWithRetry } from "./retryUtils"; @@ -37,19 +37,20 @@ import { runWithRetry } from "./retryUtils"; export function convertCreateNewSummaryTreeToTreeAndBlobs( summary: ISummaryTree, treeId: string, -): ISnapshotContents { +): ISnapshot { const protocolSummary = summary.tree[".protocol"] as ISummaryTree; const documentAttributes = getDocAttributesFromProtocolSummary(protocolSummary); const sequenceNumber = documentAttributes.sequenceNumber; - const blobs = new Map(); - const snapshotTree = convertCreateNewSummaryTreeToTreeAndBlobsCore(summary, blobs); + const blobContents = new Map(); + const snapshotTree = convertCreateNewSummaryTreeToTreeAndBlobsCore(summary, blobContents); snapshotTree.id = treeId; - const snapshotTreeValue: ISnapshotContents = { + const snapshotTreeValue: ISnapshot = { snapshotTree, - blobs, + blobContents, ops: [], sequenceNumber, latestSequenceNumber: sequenceNumber, + snapshotInNewFormat: true, }; return snapshotTreeValue; diff --git a/packages/drivers/odsp-driver/src/fetchSnapshot.ts b/packages/drivers/odsp-driver/src/fetchSnapshot.ts index a1101f1d87da..5c3c9f4650ae 100644 --- a/packages/drivers/odsp-driver/src/fetchSnapshot.ts +++ b/packages/drivers/odsp-driver/src/fetchSnapshot.ts @@ -13,6 +13,7 @@ import { import { fromUtf8ToBase64 } from "@fluid-internal/client-utils"; import { assert } from "@fluidframework/core-utils"; import { getW3CData } from "@fluidframework/driver-base"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { IOdspResolvedUrl, ISnapshotOptions, @@ -28,7 +29,7 @@ import { import { fetchIncorrectResponse, throwOdspNetworkError } from "@fluidframework/odsp-doclib-utils"; import { IOdspSnapshot, - ISnapshotCachedEntry, + ISnapshotCachedEntry2, IVersionedValueWithEpoch, persistedCacheValueVersion, } from "./contracts"; @@ -43,7 +44,6 @@ import { measure, measureP, } from "./odspUtils"; -import { ISnapshotContents } from "./odspPublicUtils"; import { convertOdspSnapshotToSnapshotTreeAndBlobs } from "./odspSnapshotParser"; import { currentReadVersion, @@ -85,7 +85,7 @@ export async function fetchSnapshot( url: string, fetchOptions: { [index: string]: any }, ) => Promise>, -): Promise { +): Promise { const path = `/trees/${versionId}`; let queryParams: ISnapshotOptions = {}; @@ -125,7 +125,7 @@ export async function fetchSnapshotWithRedeem( putInCache: (valueWithEpoch: IVersionedValueWithEpoch) => Promise, removeEntries: () => Promise, enableRedeemFallback?: boolean, -): Promise { +): Promise { // back-compat: This block to be removed with #8784 when we only consume/consider odsp resolvers that are >= 0.51 const sharingLinkToRedeem = (odspResolvedUrl as any).sharingLinkToRedeem; if (sharingLinkToRedeem) { @@ -248,7 +248,7 @@ async function fetchLatestSnapshotCore( ) => Promise, putInCache: (valueWithEpoch: IVersionedValueWithEpoch) => Promise, enableRedeemFallback?: boolean, -): Promise { +): Promise { return getWithRetryForTokenRefresh(async (tokenFetchOptions) => { const storageToken = await storageTokenFetcher(tokenFetchOptions, "TreesLatest", true); assert(storageToken !== null, 0x1e5 /* "Storage token should not be null" */); @@ -327,7 +327,7 @@ async function fetchLatestSnapshotCore( let content: IOdspSnapshot; [content, parseTime] = measure(() => JSON.parse(text) as IOdspSnapshot); validateBlobsAndTrees(content); - const snapshotContents: ISnapshotContents = + const snapshotContents: ISnapshot = convertOdspSnapshotToSnapshotTreeAndBlobs(content); parsedSnapshotContents = { ...odspResponse, @@ -436,7 +436,7 @@ async function fetchLatestSnapshotCore( fluidEpoch !== undefined, 0x1e6 /* "Epoch should be present in response" */, ); - const value: ISnapshotCachedEntry = { + const value: ISnapshotCachedEntry2 = { ...snapshot, cacheEntryTime: Date.now(), }; @@ -451,7 +451,7 @@ async function fetchLatestSnapshotCore( event.end({ trees, - blobs: snapshot.blobs?.size ?? 0, + blobs: snapshot.blobContents?.size ?? 0, leafNodes: numBlobs, encodedBlobsSize, sequenceNumber, @@ -535,11 +535,11 @@ function getFormBodyAndHeaders( return { body: postBody, headers: header }; } -export function evalBlobsAndTrees(snapshot: ISnapshotContents) { +export function evalBlobsAndTrees(snapshot: ISnapshot) { const trees = countTreesInSnapshotTree(snapshot.snapshotTree); - const numBlobs = snapshot.blobs.size; + const numBlobs = snapshot.blobContents.size; let encodedBlobsSize = 0; - for (const [_, blobContent] of snapshot.blobs) { + for (const [_, blobContent] of snapshot.blobContents) { encodedBlobsSize += blobContent.byteLength; } return { trees, numBlobs, encodedBlobsSize }; diff --git a/packages/drivers/odsp-driver/src/localOdspDriver/localOdspDocumentStorageManager.ts b/packages/drivers/odsp-driver/src/localOdspDriver/localOdspDocumentStorageManager.ts index 64ae3e6d676c..dc19de98d3c2 100644 --- a/packages/drivers/odsp-driver/src/localOdspDriver/localOdspDocumentStorageManager.ts +++ b/packages/drivers/odsp-driver/src/localOdspDriver/localOdspDocumentStorageManager.ts @@ -5,11 +5,14 @@ import { assert } from "@fluidframework/core-utils"; import { ITelemetryLoggerExt, loggerToMonitoringContext } from "@fluidframework/telemetry-utils"; -import { ISummaryContext } from "@fluidframework/driver-definitions"; +import { + ISnapshot, + ISnapshotFetchOptions, + ISummaryContext, +} from "@fluidframework/driver-definitions"; import { UsageError } from "@fluidframework/driver-utils"; import * as api from "@fluidframework/protocol-definitions"; import { OdspDocumentStorageServiceBase } from "../odspDocumentStorageServiceBase"; -import { ISnapshotContents } from "../odspPublicUtils"; import { IOdspSnapshot } from "../contracts"; import { convertOdspSnapshotToSnapshotTreeAndBlobs } from "../odspSnapshotParser"; import { parseCompactSnapshotResponse } from "../compactSnapshotParser"; @@ -45,7 +48,7 @@ export class LocalOdspDocumentStorageService extends OdspDocumentStorageServiceB } this.calledGetVersions = true; - let snapshotContents: ISnapshotContents; + let snapshotContents: ISnapshot; if (typeof this.localSnapshot === "string") { const content: IOdspSnapshot = JSON.parse(this.localSnapshot); @@ -58,6 +61,10 @@ export class LocalOdspDocumentStorageService extends OdspDocumentStorageServiceB return this.getSnapshotVersion(); } + public async getSnapshot(snapshotFetchOptions?: ISnapshotFetchOptions): Promise { + this.throwUsageError("getSnapshot"); + } + private getSnapshotVersion(): api.IVersion[] { return this.snapshotTreeId ? [{ id: this.snapshotTreeId, treeId: undefined! }] : []; } diff --git a/packages/drivers/odsp-driver/src/odspCache.ts b/packages/drivers/odsp-driver/src/odspCache.ts index 6056a03aebd8..7b76c62486df 100644 --- a/packages/drivers/odsp-driver/src/odspCache.ts +++ b/packages/drivers/odsp-driver/src/odspCache.ts @@ -12,7 +12,7 @@ import { ISocketStorageDiscovery, getKeyForCacheEntry, } from "@fluidframework/odsp-driver-definitions"; -import { ISnapshotContents } from "./odspPublicUtils"; +import { ISnapshot } from "@fluidframework/driver-definitions"; /** * Similar to IPersistedCache, but exposes cache interface for single file @@ -142,7 +142,7 @@ export class NonPersistentCache implements INonPersistentCache { /** * @alpha */ -export interface IPrefetchSnapshotContents extends ISnapshotContents { +export interface IPrefetchSnapshotContents extends ISnapshot { fluidEpoch: string; prefetchStartTime: number; } diff --git a/packages/drivers/odsp-driver/src/odspDocumentService.ts b/packages/drivers/odsp-driver/src/odspDocumentService.ts index b6d30527a86a..424b0352d87d 100644 --- a/packages/drivers/odsp-driver/src/odspDocumentService.ts +++ b/packages/drivers/odsp-driver/src/odspDocumentService.ts @@ -147,6 +147,7 @@ export class OdspDocumentService }); this.hostPolicy = hostPolicy; + this.hostPolicy.supportGetSnapshotApi = this._policies.supportGetSnapshotApi; if (this.clientIsSummarizer) { this.hostPolicy = { ...this.hostPolicy, summarizerClient: true }; } diff --git a/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts b/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts index cc7c66c93b16..bd116d7fc855 100644 --- a/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts +++ b/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts @@ -35,6 +35,7 @@ import { HostStoragePolicyInternal, IVersionedValueWithEpoch, ISnapshotCachedEntry, + ISnapshotCachedEntry2, } from "./contracts"; import { downloadSnapshot, @@ -45,8 +46,11 @@ import { } from "./fetchSnapshot"; import { getUrlAndHeadersWithAuth } from "./getUrlAndHeadersWithAuth"; import { IOdspCache, IPrefetchSnapshotContents } from "./odspCache"; -import { createCacheSnapshotKey, getWithRetryForTokenRefresh } from "./odspUtils"; -import { ISnapshotContents } from "./odspPublicUtils"; +import { + createCacheSnapshotKey, + getWithRetryForTokenRefresh, + isInstanceOfISnapshot, +} from "./odspUtils"; import { EpochTracker } from "./epochTracker"; import type { OdspSummaryUploadManager } from "./odspSummaryUploadManager"; import { FlushResult } from "./odspDocumentDeltaConnection"; @@ -209,14 +213,13 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { return super.getSnapshotTree(version, scenarioName); } - public async getSnapshot( - version?: api.IVersion, - snapshotFetchOptions?: ISnapshotFetchOptions, - ): Promise { - if (!this.snapshotUrl) { - return undefined; - } - return super.getSnapshot(version, snapshotFetchOptions); + public async getSnapshot(snapshotFetchOptions?: ISnapshotFetchOptions): Promise { + assert(this.hostPolicy.supportGetSnapshotApi !== true, "api should not be called yet"); + // This is just temporary as this api is not yet enabled in service policies. + return this.fetchSnapshot( + this.hostPolicy.snapshotOptions, + snapshotFetchOptions?.scenarioName, + ); } public async getVersions( @@ -248,16 +251,13 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { // If count is one, we can use the trees/latest API, which returns the latest version and trees in a single request for better performance if (count === 1 && (blobid === null || blobid === this.documentId)) { const hostSnapshotOptions = this.hostPolicy.snapshotOptions; - const odspSnapshotCacheValue: ISnapshotContents = await PerformanceEvent.timedExecAsync( + const odspSnapshotCacheValue: ISnapshot = await PerformanceEvent.timedExecAsync( this.logger, { eventName: "ObtainSnapshot", fetchSource }, async (event: PerformanceEvent) => { const props: GetVersionsTelemetryProps = {}; let cacheLookupTimeInSerialFetch = 0; - let retrievedSnapshot: - | ISnapshotContents - | IPrefetchSnapshotContents - | undefined; + let retrievedSnapshot: ISnapshot | IPrefetchSnapshotContents | undefined; let method: string; let prefetchWaitStartTime: number = performance.now(); @@ -270,10 +270,14 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { } else { // Here's the logic to grab the persistent cache snapshot implemented by the host // Epoch tracker is responsible for communicating with the persistent cache, handling epochs and cache versions - const cachedSnapshotP: Promise = - this.epochTracker - .get(createCacheSnapshotKey(this.odspResolvedUrl)) - .then(async (snapshotCachedEntry: ISnapshotCachedEntry) => { + const cachedSnapshotP: Promise = this.epochTracker + .get(createCacheSnapshotKey(this.odspResolvedUrl)) + .then( + async ( + snapshotCachedEntry: + | ISnapshotCachedEntry + | ISnapshotCachedEntry2, + ) => { if (snapshotCachedEntry !== undefined) { // If the cached entry does not contain the entry time, then assign it a default of 30 days old. const age = @@ -298,8 +302,19 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { props.cacheEntryAge = age; } - return snapshotCachedEntry; - }); + // Snapshot from cache could be in older format, so transform that before returning. + if (isInstanceOfISnapshot(snapshotCachedEntry)) { + return snapshotCachedEntry; + } else { + const snapshot: ISnapshot = { + ...snapshotCachedEntry, + blobContents: snapshotCachedEntry.blobs, + snapshotInNewFormat: true, + }; + return snapshot; + } + }, + ); // Based on the concurrentSnapshotFetch policy: // Either retrieve both the network and cache snapshots concurrently and pick the first to return, // or grab the cache value and then the network value if the cache value returns undefined. @@ -482,7 +497,7 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { private async fetchSnapshotCore( hostSnapshotOptions: ISnapshotOptions | undefined, scenarioName?: string, - ): Promise { + ): Promise { // Don't look into cache, if the host specifically tells us so. if (!this.hostPolicy.avoidPrefetchSnapshotCache) { const prefetchCacheKey = getKeyForCacheEntry( @@ -761,8 +776,8 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { treeId = snapshot.snapshotTree.id; this.setRootTree(treeId, snapshot.snapshotTree); } - if (snapshot.blobs) { - this.initBlobsCache(snapshot.blobs); + if (snapshot.blobContents) { + this.initBlobsCache(snapshot.blobContents); } // If the version id doesn't match with the id of the tree, then use the id of first tree which in that case // will be the actual id of tree to be fetched. diff --git a/packages/drivers/odsp-driver/src/odspDocumentStorageServiceBase.ts b/packages/drivers/odsp-driver/src/odspDocumentStorageServiceBase.ts index a554a3241711..cf80d0dcd29f 100644 --- a/packages/drivers/odsp-driver/src/odspDocumentStorageServiceBase.ts +++ b/packages/drivers/odsp-driver/src/odspDocumentStorageServiceBase.ts @@ -15,8 +15,7 @@ import { ISnapshotFetchOptions, } from "@fluidframework/driver-definitions"; import * as api from "@fluidframework/protocol-definitions"; -import { IConfigProvider, UsageError } from "@fluidframework/telemetry-utils"; -import { ISnapshotContents } from "./odspPublicUtils"; +import { IConfigProvider } from "@fluidframework/telemetry-utils"; const maximumCacheDurationMs: FiveDaysMs = 432000000; // 5 * 24 * 60 * 60 * 1000 = 5 days in ms @@ -210,12 +209,7 @@ export abstract class OdspDocumentStorageServiceBase implements IDocumentStorage return this.combineProtocolAndAppSnapshotTree(appTree, protocolTree); } - public async getSnapshot( - version?: api.IVersion, - snapshotFetchOptions?: ISnapshotFetchOptions, - ): Promise { - throw new UsageError("Not implemented yet!"); - } + public abstract getSnapshot(snapshotFetchOptions?: ISnapshotFetchOptions): Promise; public abstract getVersions( // eslint-disable-next-line @rushstack/no-new-null @@ -276,11 +270,11 @@ export abstract class OdspDocumentStorageServiceBase implements IDocumentStorage } protected initializeFromSnapshot( - odspSnapshotCacheValue: ISnapshotContents, + odspSnapshotCacheValue: ISnapshot, cacheOps: boolean = true, ): string | undefined { this._snapshotSequenceNumber = odspSnapshotCacheValue.sequenceNumber; - const { snapshotTree, blobs, ops } = odspSnapshotCacheValue; + const { snapshotTree, blobContents, ops } = odspSnapshotCacheValue; // id should be undefined in case of just ops in snapshot. let id: string | undefined; @@ -290,8 +284,8 @@ export abstract class OdspDocumentStorageServiceBase implements IDocumentStorage this.setRootTree(id, snapshotTree); } - if (blobs) { - this.initBlobsCache(blobs); + if (blobContents) { + this.initBlobsCache(blobContents); } if (cacheOps) { diff --git a/packages/drivers/odsp-driver/src/odspSnapshotParser.ts b/packages/drivers/odsp-driver/src/odspSnapshotParser.ts index 9a8b09557fbc..56df1f799842 100644 --- a/packages/drivers/odsp-driver/src/odspSnapshotParser.ts +++ b/packages/drivers/odsp-driver/src/odspSnapshotParser.ts @@ -6,8 +6,8 @@ import { stringToBuffer } from "@fluid-internal/client-utils"; import { assert } from "@fluidframework/core-utils"; import * as api from "@fluidframework/protocol-definitions"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { IOdspSnapshot, IOdspSnapshotCommit } from "./contracts"; -import { ISnapshotContents } from "./odspPublicUtils"; /** * Build a tree hierarchy base on a flat tree @@ -51,9 +51,7 @@ function buildHierarchy(flatTree: IOdspSnapshotCommit): api.ISnapshotTree { * Converts existing IOdspSnapshot to snapshot tree, blob array and ops * @param odspSnapshot - snapshot */ -export function convertOdspSnapshotToSnapshotTreeAndBlobs( - odspSnapshot: IOdspSnapshot, -): ISnapshotContents { +export function convertOdspSnapshotToSnapshotTreeAndBlobs(odspSnapshot: IOdspSnapshot): ISnapshot { const blobsWithBufferContent = new Map(); if (odspSnapshot.blobs) { odspSnapshot.blobs.forEach((blob) => { @@ -70,8 +68,8 @@ export function convertOdspSnapshotToSnapshotTreeAndBlobs( const sequenceNumber = odspSnapshot?.trees[0].sequenceNumber; - const val: ISnapshotContents = { - blobs: blobsWithBufferContent, + const val: ISnapshot = { + blobContents: blobsWithBufferContent, ops: odspSnapshot.ops?.map((op) => op.op) ?? [], sequenceNumber, snapshotTree: buildHierarchy(odspSnapshot.trees[0]), @@ -79,6 +77,7 @@ export function convertOdspSnapshotToSnapshotTreeAndBlobs( odspSnapshot.ops && odspSnapshot.ops.length > 0 ? odspSnapshot.ops[odspSnapshot.ops.length - 1].sequenceNumber : sequenceNumber, + snapshotInNewFormat: true, }; return val; } diff --git a/packages/drivers/odsp-driver/src/odspUtils.ts b/packages/drivers/odsp-driver/src/odspUtils.ts index 24b6019310fc..ad5342a16564 100644 --- a/packages/drivers/odsp-driver/src/odspUtils.ts +++ b/packages/drivers/odsp-driver/src/odspUtils.ts @@ -4,7 +4,7 @@ */ import { ITelemetryProperties, ITelemetryBaseLogger } from "@fluidframework/core-interfaces"; -import { IResolvedUrl } from "@fluidframework/driver-definitions"; +import { IResolvedUrl, ISnapshot } from "@fluidframework/driver-definitions"; import { isOnline, OnlineStatus, @@ -43,6 +43,7 @@ import { import { fetch } from "./fetch"; import { pkgVersion as driverVersion } from "./packageVersion"; import { IOdspSnapshot } from "./contracts"; +import { ISnapshotContents } from "./odspPublicUtils"; export const getWithRetryForTokenRefreshRepeat = "getWithRetryForTokenRefreshRepeat"; @@ -473,3 +474,14 @@ export async function measureP(callback: () => Promise): Promise<[T, numbe export function getJoinSessionCacheKey(odspResolvedUrl: IOdspResolvedUrl) { return `${odspResolvedUrl.hashedDocumentId}/joinsession`; } + +/** + * Utility API to check if the type of snapshot contents is `ISnapshot`. + * @internal + * @param obj - obj whose type needs to be identified. + */ +export function isInstanceOfISnapshot( + obj: ISnapshotContents | ISnapshot | undefined, +): obj is ISnapshot { + return obj !== undefined && "snapshotInNewFormat" in obj && obj.snapshotInNewFormat === true; +} diff --git a/packages/drivers/odsp-driver/src/retryErrorsStorageAdapter.ts b/packages/drivers/odsp-driver/src/retryErrorsStorageAdapter.ts index 9f4ae81d8a21..01bf51b9ef33 100644 --- a/packages/drivers/odsp-driver/src/retryErrorsStorageAdapter.ts +++ b/packages/drivers/odsp-driver/src/retryErrorsStorageAdapter.ts @@ -3,8 +3,7 @@ * Licensed under the MIT License. */ -import { assert } from "@fluidframework/core-utils"; -import { LoggingError, ITelemetryLoggerExt } from "@fluidframework/telemetry-utils"; +import { LoggingError, ITelemetryLoggerExt, UsageError } from "@fluidframework/telemetry-utils"; import { FetchSource, IDocumentStorageService, @@ -52,16 +51,12 @@ export class RetryErrorsStorageAdapter implements IDocumentStorageService, IDisp ); } - public async getSnapshot( - version?: IVersion, - snapshotFetchOptions?: ISnapshotFetchOptions, - ): Promise { + public async getSnapshot(snapshotFetchOptions?: ISnapshotFetchOptions): Promise { return this.runWithRetry(async () => { - assert( - this.internalStorageService.getSnapshot !== undefined, - "getSnapshot should exist in storage adapter in ODSP driver", - ); - return this.internalStorageService.getSnapshot(version, snapshotFetchOptions); + if (this.internalStorageService.getSnapshot !== undefined) { + return this.internalStorageService.getSnapshot(snapshotFetchOptions); + } + throw new UsageError("getSnapshot should exist in storage adapter in ODSP driver"); }, "storage_getSnapshot"); } diff --git a/packages/drivers/odsp-driver/src/test/createNewUtilsTests.spec.ts b/packages/drivers/odsp-driver/src/test/createNewUtilsTests.spec.ts index 174990750cc1..cc2cf4b87e36 100644 --- a/packages/drivers/odsp-driver/src/test/createNewUtilsTests.spec.ts +++ b/packages/drivers/odsp-driver/src/test/createNewUtilsTests.spec.ts @@ -4,6 +4,7 @@ */ import { strict as assert } from "assert"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import * as api from "@fluidframework/protocol-definitions"; import { bufferToString } from "@fluid-internal/client-utils"; import { @@ -18,7 +19,7 @@ import { convertCreateNewSummaryTreeToTreeAndBlobs } from "../createNewUtils"; import { createNewFluidFile } from "../createFile"; import { createNewContainerOnExistingFile } from "../createNewContainerOnExistingFile"; import { EpochTracker } from "../epochTracker"; -import { getHashedDocumentId, ISnapshotContents } from "../odspPublicUtils"; +import { getHashedDocumentId } from "../odspPublicUtils"; import { INewFileInfo, createCacheSnapshotKey, IExistingFileInfo } from "../odspUtils"; import { LocalPersistentCache } from "../odspCache"; import { mockFetchOk } from "./mockFetch"; @@ -106,14 +107,14 @@ describe("Create New Utils Tests", () => { await epochTracker.removeEntries().catch(() => {}); }); - const test = (snapshot: ISnapshotContents) => { + const test = (snapshot: ISnapshot) => { const snapshotTree = snapshot.snapshotTree; assert.strictEqual( Object.entries(snapshotTree.trees).length, 2, "app and protocol should be there", ); - assert.strictEqual(snapshot.blobs.size, 2, "2 blobs should be there"); + assert.strictEqual(snapshot.blobContents.size, 2, "2 blobs should be there"); const appTree = snapshotTree.trees[".app"]; const protocolTree = snapshotTree.trees[".protocol"]; @@ -121,13 +122,13 @@ describe("Create New Utils Tests", () => { assert(protocolTree !== undefined, "Protocol tree should be there"); const appTreeBlobId = appTree.blobs.attributes; - const appTreeBlobValBuffer = snapshot.blobs.get(appTreeBlobId); + const appTreeBlobValBuffer = snapshot.blobContents.get(appTreeBlobId); assert(appTreeBlobValBuffer !== undefined, "app blob value should exist"); const appTreeBlobVal = bufferToString(appTreeBlobValBuffer, "utf8"); assert(appTreeBlobVal === blobContent, "Blob content should match"); const docAttributesBlobId = protocolTree.blobs.attributes; - const docAttributesBuffer = snapshot.blobs.get(docAttributesBlobId); + const docAttributesBuffer = snapshot.blobContents.get(docAttributesBlobId); assert(docAttributesBuffer !== undefined, "protocol attributes blob value should exist"); const docAttributesBlobValue = bufferToString(docAttributesBuffer, "utf8"); assert( diff --git a/packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts b/packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts index 00c313ff1a1b..20b96a2c5a0d 100644 --- a/packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts +++ b/packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts @@ -5,6 +5,7 @@ import { strict as assert } from "assert"; import { stub } from "sinon"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { OdspErrorTypes, IOdspResolvedUrl } from "@fluidframework/odsp-driver-definitions"; import { createChildLogger } from "@fluidframework/telemetry-utils"; import { EpochTracker } from "../epochTracker"; @@ -13,7 +14,7 @@ import * as fetchSnapshotImport from "../fetchSnapshot"; import { LocalPersistentCache, NonPersistentCache } from "../odspCache"; import { INewFileInfo, IOdspResponse } from "../odspUtils"; import { createOdspUrl } from "../createOdspUrl"; -import { getHashedDocumentId, ISnapshotContents } from "../odspPublicUtils"; +import { getHashedDocumentId } from "../odspPublicUtils"; import { OdspDriverUrlResolver } from "../odspDriverUrlResolver"; import { ISnapshotRequestAndResponseOptions } from "../fetchSnapshot"; import { OdspDocumentStorageService } from "../odspDocumentStorageManager"; @@ -58,16 +59,17 @@ describe("Tests for snapshot fetch", () => { const logger = createChildLogger(); const odspUrl = createOdspUrl({ ...newFileParams, itemId, dataStorePath: "/" }); - const content: ISnapshotContents = { + const content: ISnapshot = { snapshotTree: { id: "id", blobs: {}, trees: {}, }, - blobs: new Map(), + blobContents: new Map(), ops: [], sequenceNumber: 0, latestSequenceNumber: 0, + snapshotInNewFormat: true, }; before(async () => { hashedDocumentId = await getHashedDocumentId(driveId, itemId); diff --git a/packages/drivers/odsp-driver/src/test/getVersions.spec.ts b/packages/drivers/odsp-driver/src/test/getVersions.spec.ts index f05704ad790e..f41f02fd363e 100644 --- a/packages/drivers/odsp-driver/src/test/getVersions.spec.ts +++ b/packages/drivers/odsp-driver/src/test/getVersions.spec.ts @@ -4,6 +4,7 @@ */ import { strict as assert } from "assert"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { IOdspResolvedUrl, ICacheEntry } from "@fluidframework/odsp-driver-definitions"; import { createChildLogger } from "@fluidframework/telemetry-utils"; import { delay } from "@fluidframework/core-utils"; @@ -17,7 +18,7 @@ import { import { LocalPersistentCache, NonPersistentCache } from "../odspCache"; import { INewFileInfo } from "../odspUtils"; import { createOdspUrl } from "../createOdspUrl"; -import { getHashedDocumentId, ISnapshotContents } from "../odspPublicUtils"; +import { getHashedDocumentId } from "../odspPublicUtils"; import { OdspDriverUrlResolver } from "../odspDriverUrlResolver"; import { OdspDocumentStorageService, @@ -80,16 +81,17 @@ describe("Tests for snapshot fetch", () => { blobs: [], }; - const content: ISnapshotContents = { + const content: ISnapshot = { snapshotTree: { id: "id", blobs: {}, trees: {}, }, - blobs: new Map(), + blobContents: new Map(), ops: [], sequenceNumber: 0, latestSequenceNumber: 0, + snapshotInNewFormat: true, }; const value: IVersionedValueWithEpoch = { @@ -167,16 +169,17 @@ describe("Tests for snapshot fetch", () => { }); it("should not fetch from cache with the same snapshot", async () => { - const latestContent: ISnapshotContents = { + const latestContent: ISnapshot = { snapshotTree: { id: "WrongId", blobs: {}, trees: {}, }, - blobs: new Map(), + blobContents: new Map(), ops: [], sequenceNumber: 0, latestSequenceNumber: 0, + snapshotInNewFormat: true, }; const latestValue: IVersionedValueWithEpoch = { diff --git a/packages/drivers/odsp-driver/src/test/jsonSnapshotFormatTests.spec.ts b/packages/drivers/odsp-driver/src/test/jsonSnapshotFormatTests.spec.ts index 7d94e9a00fb2..d15eafda3b04 100644 --- a/packages/drivers/odsp-driver/src/test/jsonSnapshotFormatTests.spec.ts +++ b/packages/drivers/odsp-driver/src/test/jsonSnapshotFormatTests.spec.ts @@ -100,7 +100,7 @@ describe("JSON Snapshot Format Conversion Tests", () => { assert(result.latestSequenceNumber === 3, "Latest sequence number should match"); assert((result.snapshotTree.id = snapshotTree.id), "Snapshot id should match"); assert(result.ops.length === 2, "2 ops should be there"); - assert(result.blobs.size === 2, "2 blobs should be there"); + assert(result.blobContents.size === 2, "2 blobs should be there"); assert(Object.keys(result.snapshotTree.trees).length === 2, "2 trees should be there"); const shouldBeEmptyTree = result.snapshotTree.trees[".app"]?.trees[".channels"]?.trees[ diff --git a/packages/drivers/odsp-driver/src/test/prefetchSnapshotTests.spec.ts b/packages/drivers/odsp-driver/src/test/prefetchSnapshotTests.spec.ts index 35a62ac2f5d5..f8dde1f54445 100644 --- a/packages/drivers/odsp-driver/src/test/prefetchSnapshotTests.spec.ts +++ b/packages/drivers/odsp-driver/src/test/prefetchSnapshotTests.spec.ts @@ -12,7 +12,7 @@ import { getKeyForCacheEntry, } from "@fluidframework/odsp-driver-definitions"; import { MockLogger } from "@fluidframework/telemetry-utils"; -import { FetchSource } from "@fluidframework/driver-definitions"; +import { FetchSource, ISnapshot } from "@fluidframework/driver-definitions"; import { IOdspSnapshot, HostStoragePolicyInternal, @@ -22,7 +22,7 @@ import { import { IPrefetchSnapshotContents, LocalPersistentCache } from "../odspCache"; import { createCacheSnapshotKey, INewFileInfo } from "../odspUtils"; import { createOdspUrl } from "../createOdspUrl"; -import { getHashedDocumentId, ISnapshotContents } from "../odspPublicUtils"; +import { getHashedDocumentId } from "../odspPublicUtils"; import { OdspDriverUrlResolver } from "../odspDriverUrlResolver"; import { OdspDocumentStorageService } from "../odspDocumentStorageManager"; import { prefetchLatestSnapshot } from "../prefetchLatestSnapshot"; @@ -79,16 +79,17 @@ describe("Tests for prefetching snapshot", () => { blobs: [], }; - const content: ISnapshotContents = { + const content: ISnapshot = { snapshotTree: { id: "id", blobs: {}, trees: {}, }, - blobs: new Map(), + blobContents: new Map(), ops: [], sequenceNumber: 0, latestSequenceNumber: 0, + snapshotInNewFormat: true, }; const value: IVersionedValueWithEpoch = { diff --git a/packages/drivers/odsp-driver/src/test/snapshotFormatTests.spec.ts b/packages/drivers/odsp-driver/src/test/snapshotFormatTests.spec.ts index bcc414ac3f28..6ef778354923 100644 --- a/packages/drivers/odsp-driver/src/test/snapshotFormatTests.spec.ts +++ b/packages/drivers/odsp-driver/src/test/snapshotFormatTests.spec.ts @@ -6,10 +6,10 @@ import { strict as assert } from "assert"; import { ISequencedDocumentMessage, ISnapshotTree } from "@fluidframework/protocol-definitions"; import { stringToBuffer } from "@fluid-internal/client-utils"; +import { ISnapshot } from "@fluidframework/driver-definitions"; import { MockLogger } from "@fluidframework/telemetry-utils"; import { parseCompactSnapshotResponse } from "../compactSnapshotParser"; import { convertToCompactSnapshot } from "../compactSnapshotWriter"; -import { ISnapshotContents } from "../odspPublicUtils"; const snapshotTree: ISnapshotTree = { id: "SnapshotId", @@ -53,7 +53,7 @@ const snapshotTree: ISnapshotTree = { }, }; -const blobs = new Map([ +const blobContents = new Map([ [ "bARADgIe4qmDjJl2l2zz12IM3", stringToBuffer( @@ -120,18 +120,19 @@ const ops: ISequencedDocumentMessage[] = [ describe("Snapshot Format Conversion Tests", () => { it("Conversion test", async () => { - const snapshotContents: ISnapshotContents = { + const snapshotContents: ISnapshot = { snapshotTree, - blobs, + blobContents, ops, sequenceNumber: 0, latestSequenceNumber: 2, + snapshotInNewFormat: true, }; const logger = new MockLogger(); const compactSnapshot = convertToCompactSnapshot(snapshotContents); const result = parseCompactSnapshotResponse(compactSnapshot, logger.toTelemetryLogger()); assert.deepStrictEqual(result.snapshotTree, snapshotTree, "Tree structure should match"); - assert.deepStrictEqual(result.blobs, blobs, "Blobs content should match"); + assert.deepStrictEqual(result.blobContents, blobContents, "Blobs content should match"); assert.deepStrictEqual(result.ops, ops, "Ops should match"); assert(result.sequenceNumber === 0, "Seq number should match"); assert(result.latestSequenceNumber === 2, "Latest sequence number should match"); @@ -150,18 +151,19 @@ describe("Snapshot Format Conversion Tests", () => { }); it("Conversion test with empty ops", async () => { - const snapshotContents: ISnapshotContents = { + const snapshotContents: ISnapshot = { snapshotTree, - blobs, + blobContents, ops: [], sequenceNumber: 0, latestSequenceNumber: 2, + snapshotInNewFormat: true, }; const logger = new MockLogger(); const compactSnapshot = convertToCompactSnapshot(snapshotContents); const result = parseCompactSnapshotResponse(compactSnapshot, logger.toTelemetryLogger()); assert.deepStrictEqual(result.snapshotTree, snapshotTree, "Tree structure should match"); - assert.deepStrictEqual(result.blobs, blobs, "Blobs content should match"); + assert.deepStrictEqual(result.blobContents, blobContents, "Blobs content should match"); assert.deepStrictEqual(result.ops, [], "Ops should match"); assert(result.sequenceNumber === 0, "Seq number should match"); assert(result.latestSequenceNumber === 2, "Latest sequence number should match"); diff --git a/packages/drivers/odsp-driver/src/test/types/validateOdspDriverPrevious.generated.ts b/packages/drivers/odsp-driver/src/test/types/validateOdspDriverPrevious.generated.ts index c29d3b04b32b..5d47e856f91c 100644 --- a/packages/drivers/odsp-driver/src/test/types/validateOdspDriverPrevious.generated.ts +++ b/packages/drivers/odsp-driver/src/test/types/validateOdspDriverPrevious.generated.ts @@ -271,6 +271,7 @@ declare function get_old_InterfaceDeclaration_IPrefetchSnapshotContents(): declare function use_current_InterfaceDeclaration_IPrefetchSnapshotContents( use: TypeOnly): void; use_current_InterfaceDeclaration_IPrefetchSnapshotContents( + // @ts-expect-error compatibility expected to be broken get_old_InterfaceDeclaration_IPrefetchSnapshotContents()); /* @@ -283,6 +284,7 @@ declare function get_current_InterfaceDeclaration_IPrefetchSnapshotContents(): declare function use_old_InterfaceDeclaration_IPrefetchSnapshotContents( use: TypeOnly): void; use_old_InterfaceDeclaration_IPrefetchSnapshotContents( + // @ts-expect-error compatibility expected to be broken get_current_InterfaceDeclaration_IPrefetchSnapshotContents()); /* @@ -343,6 +345,7 @@ declare function get_old_InterfaceDeclaration_ISnapshotContentsWithProps(): declare function use_current_InterfaceDeclaration_ISnapshotContentsWithProps( use: TypeOnly): void; use_current_InterfaceDeclaration_ISnapshotContentsWithProps( + // @ts-expect-error compatibility expected to be broken get_old_InterfaceDeclaration_ISnapshotContentsWithProps()); /* @@ -355,6 +358,7 @@ declare function get_current_InterfaceDeclaration_ISnapshotContentsWithProps(): declare function use_old_InterfaceDeclaration_ISnapshotContentsWithProps( use: TypeOnly): void; use_old_InterfaceDeclaration_ISnapshotContentsWithProps( + // @ts-expect-error compatibility expected to be broken get_current_InterfaceDeclaration_ISnapshotContentsWithProps()); /* From 53c7131d05eb5493e0c71bca82e8268d6c25ec82 Mon Sep 17 00:00:00 2001 From: Jatin Garg Date: Mon, 29 Jan 2024 20:04:05 -0800 Subject: [PATCH 4/9] Remove ISnapshotContents --- .../src/odspDocumentStorageManager.ts | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts b/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts index bd116d7fc855..f77848e76e7b 100644 --- a/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts +++ b/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts @@ -300,18 +300,21 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { // Record the cache age props.cacheEntryAge = age; - } - - // Snapshot from cache could be in older format, so transform that before returning. - if (isInstanceOfISnapshot(snapshotCachedEntry)) { - return snapshotCachedEntry; - } else { - const snapshot: ISnapshot = { - ...snapshotCachedEntry, - blobContents: snapshotCachedEntry.blobs, - snapshotInNewFormat: true, - }; - return snapshot; + // Snapshot from cache could be in older format, so transform that before returning. + if (isInstanceOfISnapshot(snapshotCachedEntry)) { + return snapshotCachedEntry; + } else { + const snapshot: ISnapshot = { + snapshotTree: snapshotCachedEntry.snapshotTree, + blobContents: snapshotCachedEntry.blobs, + ops: snapshotCachedEntry.ops, + latestSequenceNumber: + snapshotCachedEntry.latestSequenceNumber, + sequenceNumber: snapshotCachedEntry.sequenceNumber, + snapshotInNewFormat: true, + }; + return snapshot; + } } }, ); From 73af91a75698f7499eb5b12f4ae443e1ade6ca4f Mon Sep 17 00:00:00 2001 From: Jatin Garg Date: Mon, 29 Jan 2024 20:27:59 -0800 Subject: [PATCH 5/9] Remove ISnapshotContents --- packages/loader/driver-utils/src/documentStorageServiceProxy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/loader/driver-utils/src/documentStorageServiceProxy.ts b/packages/loader/driver-utils/src/documentStorageServiceProxy.ts index 3864ba55cc1f..5325525ad338 100644 --- a/packages/loader/driver-utils/src/documentStorageServiceProxy.ts +++ b/packages/loader/driver-utils/src/documentStorageServiceProxy.ts @@ -18,7 +18,7 @@ import { ISummaryTree, IVersion, } from "@fluidframework/protocol-definitions"; -import { UsageError } from "."; +import { UsageError } from "@fluidframework/telemetry-utils"; /** * @internal From 007ec01fd3e5a38a2a3e37d1c9a0f2f80635ccb1 Mon Sep 17 00:00:00 2001 From: Jatin Garg Date: Mon, 29 Jan 2024 20:43:01 -0800 Subject: [PATCH 6/9] add changeset --- .changeset/four-tables-smell.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/four-tables-smell.md diff --git a/.changeset/four-tables-smell.md b/.changeset/four-tables-smell.md new file mode 100644 index 000000000000..30f408c839c1 --- /dev/null +++ b/.changeset/four-tables-smell.md @@ -0,0 +1,12 @@ +--- +"@fluidframework/container-definitions": minor +"@fluidframework/container-loader": minor +"@fluidframework/driver-definitions": minor +"@fluidframework/driver-utils": minor +"@fluidframework/file-driver": minor +"@fluidframework/odsp-driver": minor +--- + +Deprecate `ISnapshotContents` + +`ISnapshotContents` is deprecated. It has been replaced with `ISnapshot`. From 1141962b26c27f21f11a8c36d57e351bb28170dd Mon Sep 17 00:00:00 2001 From: Jatin Garg Date: Mon, 29 Jan 2024 20:47:56 -0800 Subject: [PATCH 7/9] change version --- .../driver-definitions/api-report/driver-definitions.api.md | 2 +- packages/common/driver-definitions/src/storage.ts | 2 +- packages/drivers/odsp-driver/src/compactSnapshotParser.ts | 2 +- packages/drivers/odsp-driver/src/createNewUtils.ts | 2 +- .../drivers/odsp-driver/src/odspDocumentStorageManager.ts | 2 +- packages/drivers/odsp-driver/src/odspSnapshotParser.ts | 2 +- packages/drivers/odsp-driver/src/odspUtils.ts | 2 +- packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts | 2 +- packages/drivers/odsp-driver/src/test/getVersions.spec.ts | 4 ++-- .../odsp-driver/src/test/prefetchSnapshotTests.spec.ts | 2 +- .../drivers/odsp-driver/src/test/snapshotFormatTests.spec.ts | 4 ++-- packages/loader/driver-utils/src/storageUtils.ts | 2 +- 12 files changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/common/driver-definitions/api-report/driver-definitions.api.md b/packages/common/driver-definitions/api-report/driver-definitions.api.md index c1c4d699e3dc..071baa48caac 100644 --- a/packages/common/driver-definitions/api-report/driver-definitions.api.md +++ b/packages/common/driver-definitions/api-report/driver-definitions.api.md @@ -266,7 +266,7 @@ export interface ISnapshot { ops: ISequencedDocumentMessage[]; sequenceNumber: number | undefined; // (undocumented) - snapshotInNewFormat: true; + snapshotFormatV: 1; // (undocumented) snapshotTree: ISnapshotTree; } diff --git a/packages/common/driver-definitions/src/storage.ts b/packages/common/driver-definitions/src/storage.ts index 615646264a39..86432a62e675 100644 --- a/packages/common/driver-definitions/src/storage.ts +++ b/packages/common/driver-definitions/src/storage.ts @@ -484,7 +484,7 @@ export interface ISnapshot { */ latestSequenceNumber: number | undefined; - snapshotInNewFormat: true; + snapshotFormatV: 1; } /** diff --git a/packages/drivers/odsp-driver/src/compactSnapshotParser.ts b/packages/drivers/odsp-driver/src/compactSnapshotParser.ts index 2a4ed4ae0c2d..6dccbcbbeb70 100644 --- a/packages/drivers/odsp-driver/src/compactSnapshotParser.ts +++ b/packages/drivers/odsp-driver/src/compactSnapshotParser.ts @@ -256,7 +256,7 @@ export function parseCompactSnapshotResponse( ...blobContents, ops: records.deltas !== undefined ? readOpsSection(records.deltas) : [], latestSequenceNumber: records.lsn, - snapshotInNewFormat: true, + snapshotFormatV: 1, telemetryProps: { ...telemetryProps, durationSnapshotTree, diff --git a/packages/drivers/odsp-driver/src/createNewUtils.ts b/packages/drivers/odsp-driver/src/createNewUtils.ts index a0f0c05cab08..8070bc2966c0 100644 --- a/packages/drivers/odsp-driver/src/createNewUtils.ts +++ b/packages/drivers/odsp-driver/src/createNewUtils.ts @@ -50,7 +50,7 @@ export function convertCreateNewSummaryTreeToTreeAndBlobs( ops: [], sequenceNumber, latestSequenceNumber: sequenceNumber, - snapshotInNewFormat: true, + snapshotFormatV: 1, }; return snapshotTreeValue; diff --git a/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts b/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts index f77848e76e7b..2167c8fba8e1 100644 --- a/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts +++ b/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts @@ -311,7 +311,7 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { latestSequenceNumber: snapshotCachedEntry.latestSequenceNumber, sequenceNumber: snapshotCachedEntry.sequenceNumber, - snapshotInNewFormat: true, + snapshotFormatV: 1, }; return snapshot; } diff --git a/packages/drivers/odsp-driver/src/odspSnapshotParser.ts b/packages/drivers/odsp-driver/src/odspSnapshotParser.ts index 56df1f799842..cfa866345c06 100644 --- a/packages/drivers/odsp-driver/src/odspSnapshotParser.ts +++ b/packages/drivers/odsp-driver/src/odspSnapshotParser.ts @@ -77,7 +77,7 @@ export function convertOdspSnapshotToSnapshotTreeAndBlobs(odspSnapshot: IOdspSna odspSnapshot.ops && odspSnapshot.ops.length > 0 ? odspSnapshot.ops[odspSnapshot.ops.length - 1].sequenceNumber : sequenceNumber, - snapshotInNewFormat: true, + snapshotFormatV: 1, }; return val; } diff --git a/packages/drivers/odsp-driver/src/odspUtils.ts b/packages/drivers/odsp-driver/src/odspUtils.ts index ad5342a16564..5f22464e1043 100644 --- a/packages/drivers/odsp-driver/src/odspUtils.ts +++ b/packages/drivers/odsp-driver/src/odspUtils.ts @@ -483,5 +483,5 @@ export function getJoinSessionCacheKey(odspResolvedUrl: IOdspResolvedUrl) { export function isInstanceOfISnapshot( obj: ISnapshotContents | ISnapshot | undefined, ): obj is ISnapshot { - return obj !== undefined && "snapshotInNewFormat" in obj && obj.snapshotInNewFormat === true; + return obj !== undefined && "snapshotFormatV" in obj && obj.snapshotFormatV === 1; } diff --git a/packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts b/packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts index 20b96a2c5a0d..30d5bfdb5046 100644 --- a/packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts +++ b/packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts @@ -69,7 +69,7 @@ describe("Tests for snapshot fetch", () => { ops: [], sequenceNumber: 0, latestSequenceNumber: 0, - snapshotInNewFormat: true, + snapshotFormatV: 1, }; before(async () => { hashedDocumentId = await getHashedDocumentId(driveId, itemId); diff --git a/packages/drivers/odsp-driver/src/test/getVersions.spec.ts b/packages/drivers/odsp-driver/src/test/getVersions.spec.ts index f41f02fd363e..bd0c1291dcb9 100644 --- a/packages/drivers/odsp-driver/src/test/getVersions.spec.ts +++ b/packages/drivers/odsp-driver/src/test/getVersions.spec.ts @@ -91,7 +91,7 @@ describe("Tests for snapshot fetch", () => { ops: [], sequenceNumber: 0, latestSequenceNumber: 0, - snapshotInNewFormat: true, + snapshotFormatV: 1, }; const value: IVersionedValueWithEpoch = { @@ -179,7 +179,7 @@ describe("Tests for snapshot fetch", () => { ops: [], sequenceNumber: 0, latestSequenceNumber: 0, - snapshotInNewFormat: true, + snapshotFormatV: 1, }; const latestValue: IVersionedValueWithEpoch = { diff --git a/packages/drivers/odsp-driver/src/test/prefetchSnapshotTests.spec.ts b/packages/drivers/odsp-driver/src/test/prefetchSnapshotTests.spec.ts index f8dde1f54445..de47406fbe33 100644 --- a/packages/drivers/odsp-driver/src/test/prefetchSnapshotTests.spec.ts +++ b/packages/drivers/odsp-driver/src/test/prefetchSnapshotTests.spec.ts @@ -89,7 +89,7 @@ describe("Tests for prefetching snapshot", () => { ops: [], sequenceNumber: 0, latestSequenceNumber: 0, - snapshotInNewFormat: true, + snapshotFormatV: 1, }; const value: IVersionedValueWithEpoch = { diff --git a/packages/drivers/odsp-driver/src/test/snapshotFormatTests.spec.ts b/packages/drivers/odsp-driver/src/test/snapshotFormatTests.spec.ts index 6ef778354923..f7b3d7888521 100644 --- a/packages/drivers/odsp-driver/src/test/snapshotFormatTests.spec.ts +++ b/packages/drivers/odsp-driver/src/test/snapshotFormatTests.spec.ts @@ -126,7 +126,7 @@ describe("Snapshot Format Conversion Tests", () => { ops, sequenceNumber: 0, latestSequenceNumber: 2, - snapshotInNewFormat: true, + snapshotFormatV: 1, }; const logger = new MockLogger(); const compactSnapshot = convertToCompactSnapshot(snapshotContents); @@ -157,7 +157,7 @@ describe("Snapshot Format Conversion Tests", () => { ops: [], sequenceNumber: 0, latestSequenceNumber: 2, - snapshotInNewFormat: true, + snapshotFormatV: 1, }; const logger = new MockLogger(); const compactSnapshot = convertToCompactSnapshot(snapshotContents); diff --git a/packages/loader/driver-utils/src/storageUtils.ts b/packages/loader/driver-utils/src/storageUtils.ts index a2879e86fda6..e4b23db366db 100644 --- a/packages/loader/driver-utils/src/storageUtils.ts +++ b/packages/loader/driver-utils/src/storageUtils.ts @@ -14,5 +14,5 @@ import { ISnapshotTree } from "@fluidframework/protocol-definitions"; export function isInstanceOfISnapshot( obj: ISnapshotTree | ISnapshot | undefined, ): obj is ISnapshot { - return obj !== undefined && "snapshotInNewFormat" in obj && obj.snapshotInNewFormat === true; + return obj !== undefined && "snapshotFormatV" in obj && obj.snapshotFormatV === 1; } From 9ec182452528fc3d0666fa2a4612754578dfae5f Mon Sep 17 00:00:00 2001 From: Jatin Garg Date: Mon, 29 Jan 2024 22:20:52 -0800 Subject: [PATCH 8/9] fix --- packages/loader/container-loader/src/container.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index 1fcda21cebb9..150e16a892d5 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -2508,6 +2508,8 @@ export class Container (this.protocolHandler.quorum.get("code") ?? this.protocolHandler.quorum.get("code2")) as IFluidCodeDetails | undefined; + const existing = snapshot !== undefined; + const context = new ContainerContext( this.options, this.scope, @@ -2535,7 +2537,7 @@ export class Container () => this.connected, getSpecifiedCodeDetails, this._deltaManager.clientDetails, - true, // existing + existing, this.subLogger, pendingLocalState, snapshot, @@ -2544,7 +2546,7 @@ export class Container this._runtime = await PerformanceEvent.timedExecAsync( this.subLogger, { eventName: "InstantiateRuntime" }, - async () => runtimeFactory.instantiateRuntime(context, true /* existing */), + async () => runtimeFactory.instantiateRuntime(context, existing), ); this._lifecycleEvents.emit("runtimeInstantiated"); From 9d7f7afc99b1fb8f954d2bc4371587a1d68683d2 Mon Sep 17 00:00:00 2001 From: Jatin Garg Date: Mon, 29 Jan 2024 22:25:37 -0800 Subject: [PATCH 9/9] fix --- packages/loader/container-loader/src/container.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index 150e16a892d5..d57f9b17d3ed 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -2508,7 +2508,7 @@ export class Container (this.protocolHandler.quorum.get("code") ?? this.protocolHandler.quorum.get("code2")) as IFluidCodeDetails | undefined; - const existing = snapshot !== undefined; + const existing = snapshotTree !== undefined; const context = new ContainerContext( this.options,