Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/service-clients/odsp-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
"dependencies": {
"@fluidframework/container-definitions": "workspace:~",
"@fluidframework/container-loader": "workspace:~",
"@fluidframework/container-runtime-definitions": "workspace:~",
"@fluidframework/core-interfaces": "workspace:~",
"@fluidframework/core-utils": "workspace:~",
"@fluidframework/driver-definitions": "workspace:~",
Expand All @@ -114,6 +115,7 @@
"@fluidframework/odsp-doclib-utils": "workspace:~",
"@fluidframework/odsp-driver": "workspace:~",
"@fluidframework/odsp-driver-definitions": "workspace:~",
"@fluidframework/runtime-utils": "workspace:~",
"@fluidframework/telemetry-utils": "workspace:~",
"uuid": "^11.1.0"
},
Expand Down
35 changes: 34 additions & 1 deletion packages/service-clients/odsp-client/src/odspClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ import {
loadExistingContainer,
type ILoaderProps,
} from "@fluidframework/container-loader/internal";
import type { IContainerRuntimeInternal } from "@fluidframework/container-runtime-definitions/internal";
import type {
FluidObject,
IConfigProviderBase,
IRequest,
ITelemetryBaseLogger,
Expand Down Expand Up @@ -232,6 +234,37 @@ export class OdspClient {
}

private async getContainerServices(container: IContainer): Promise<IOdspContainerServices> {
return new OdspContainerServices(container);
const runtimeInternal = await this.getRuntimeInternal(container);
// Get the resolved URL for ODSP-specific URL building
const resolvedUrl = container.resolvedUrl;
const odspResolvedUrl =
resolvedUrl && isOdspResolvedUrl(resolvedUrl) ? resolvedUrl : undefined;
return new OdspContainerServices(container, odspResolvedUrl, runtimeInternal);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you have everything you need to create a partially-applied function lookupTemporaryBlobURL(handle) and hand that function directly to the OdspContainerServices rather than making OdspContainerServices construct it itself. This will let you avoid passing large objects around.

}

private async getRuntimeInternal(
container: IContainer,
): Promise<IContainerRuntimeInternal | undefined> {
const entryPoint = await container.getEntryPoint();
if (
entryPoint !== undefined &&
typeof (entryPoint as IMaybeFluidObjectWithContainerRuntime).IStaticEntryPoint
?.extensionStore === "function"
) {
// If the container has a static entry point with an extension store, use that to get the runtime
return (entryPoint as IMaybeFluidObjectWithContainerRuntime).IStaticEntryPoint
.extensionStore;
}
}
}

/**
* Unclear if this is the best way to go about accessing the runtime internal from the entry point.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead I think I'd expect to plumb a lookupTemporaryBlobStorageId onto IRootDataObject, in general IRootDataObject is the chokepoint for exposing stuff in the runtime to the outside world.

* We need it for IContainerRuntimeInternal.lookupTemporaryBlobStorageId,
* and fluid-static guarantees this exists on the container's entry point, but it is not exposed publicly.
*/
interface IMaybeFluidObjectWithContainerRuntime extends FluidObject {
IStaticEntryPoint: {
extensionStore: IContainerRuntimeInternal;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,94 @@
*/

import type { IContainer } from "@fluidframework/container-definitions/internal";
import type { IContainerRuntimeInternal } from "@fluidframework/container-runtime-definitions/internal";
import type { IFluidHandle } from "@fluidframework/core-interfaces";
import { createServiceAudience } from "@fluidframework/fluid-static/internal";
import type { IOdspResolvedUrl } from "@fluidframework/odsp-driver-definitions/internal";
import { lookupTemporaryBlobStorageId } from "@fluidframework/runtime-utils/internal";

import type {
IOdspAudience,
OdspContainerServices as IOdspContainerServices,
} from "./interfaces.js";
import { createOdspAudienceMember } from "./odspAudience.js";

/**
* Helper function to build a blob URL from a storage ID using ODSP-specific logic
* @param storageId - The storage ID of the blob
* @param resolvedUrl - The ODSP resolved URL containing endpoint information
* @returns The blob URL if it can be built, undefined otherwise
*/
function buildOdspBlobUrl(
storageId: string,
resolvedUrl: IOdspResolvedUrl,
): string | undefined {
try {
const attachmentGETUrl = resolvedUrl.endpoints.attachmentGETStorageUrl;
if (!attachmentGETUrl) {
return undefined;
}
return `${attachmentGETUrl}/${encodeURIComponent(storageId)}/content`;
} catch {
return undefined;
}
}

/**
* Helper function for ODSPClient to lookup blob URLs
* @param runtimeInternal - The container runtime internal interface
* @param handle - The blob handle to lookup the URL for
* @param resolvedUrl - The ODSP resolved URL containing endpoint information
* @returns The blob URL if found and the blob is not pending, undefined otherwise
*/
function lookupOdspBlobURL(
runtimeInternal: IContainerRuntimeInternal,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runtime-utils lookupTemporaryBobStorageId takes an IContainerRuntime, so this should just be an IContainerRuntime.

handle: IFluidHandle,
resolvedUrl: IOdspResolvedUrl,
): string | undefined {
try {
if (
runtimeInternal !== undefined &&
typeof (runtimeInternal as { lookupTemporaryBlobStorageId?: unknown })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't need this guard since odsp-client brings along a modern version of the runtime (via fluid-static) and therefore knows it's going to have a version with this function.

That said, would be nice to add a more explicit check and throw a more helpful error if it's missing in runtime-utils rather than a plain cast.

.lookupTemporaryBlobStorageId === "function"
) {
// Get the storage ID from the runtime
const storageId = lookupTemporaryBlobStorageId(runtimeInternal, handle);
if (storageId === undefined) {
return undefined;
}

// Build the URL using ODSP-specific logic
return buildOdspBlobUrl(storageId, resolvedUrl);
}
return undefined;
} catch {
return undefined;
}
}

/**
* @internal
*/
export class OdspContainerServices implements IOdspContainerServices {
public readonly audience: IOdspAudience;

public constructor(container: IContainer) {
public constructor(
container: IContainer,
private readonly odspResolvedUrl?: IOdspResolvedUrl,
private readonly containerRuntimeInternal?: IContainerRuntimeInternal,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here also prefer to pass around an IContainerRuntime. In general my preference is to defer casting to the internal type until just when it's needed.

) {
this.audience = createServiceAudience({
container,
createServiceMember: createOdspAudienceMember,
});
}

public lookupTemporaryBlobURL(handle: IFluidHandle): string | undefined {
if (!this.odspResolvedUrl || this.containerRuntimeInternal === undefined) {
// Can't build URLs without ODSP resolved URL information
return undefined;
}
return lookupOdspBlobURL(this.containerRuntimeInternal, handle, this.odspResolvedUrl);
}
}
Loading
Loading