Follow up to add nav parameter (#23845)

## Description This task is a follow up to [#20323](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/20323) . When loop tried to implement the code some errors came up, the expected information was not being added. Acceptance criteria The link works properly when loop tries to implement and use it. Execution plan Update the methods we are using to receive the appName and the containerPackageName. Also update IODSPResolvedURL to be able to have the containerPackageName. ## Reviewer Guidance Please let me know if there's something that is missing or that I should keep in mind. Thanks! Fixes: [AB#29956](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/29956) --------- Co-authored-by: Jatin Garg <[email protected]>
microsoft · Feb 18, 2025 · 3c2d546 · 3c2d546
1 parent 2f0b9ff
commit 3c2d546
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 79 deletions.
diff --git a/packages/drivers/odsp-driver/api-report/odsp-driver.legacy.alpha.api.md b/packages/drivers/odsp-driver/api-report/odsp-driver.legacy.alpha.api.md
@@ -174,7 +174,7 @@ export class OdspDriverUrlResolver implements IUrlResolver {
 
 // @alpha
 export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
-    constructor(shareLinkFetcherProps?: ShareLinkFetcherProps | undefined, logger?: ITelemetryBaseLogger, appName?: string | undefined, getContext?: ((resolvedUrl: IOdspResolvedUrl, dataStorePath: string) => Promise<string | undefined>) | undefined);
+    constructor(shareLinkFetcherProps?: ShareLinkFetcherProps | undefined, logger?: ITelemetryBaseLogger, appName?: string | undefined, getContext?: ((resolvedUrl: IOdspResolvedUrl, dataStorePath: string) => Promise<string | undefined>) | undefined, containerPackageInfo?: IContainerPackageInfo | undefined);
     appendDataStorePath(requestUrl: URL, pathToAppend: string): string | undefined;
     appendLocatorParams(baseUrl: string, resolvedUrl: IResolvedUrl, dataStorePath: string, packageInfoSource?: IContainerPackageInfo): Promise<string>;
     static createDocumentUrl(baseUrl: string, driverInfo: OdspFluidDataStoreLocator): string;

diff --git a/packages/drivers/odsp-driver/src/createFile/createFile.ts b/packages/drivers/odsp-driver/src/createFile/createFile.ts
@@ -115,7 +115,9 @@ export async function createNewFluidFile(
 
 	odspResolvedUrl.context = resolvedUrl?.context;
 	odspResolvedUrl.appName = resolvedUrl?.appName;
-	odspResolvedUrl.codeHint = resolvedUrl?.codeHint;
+	odspResolvedUrl.codeHint = odspResolvedUrl.codeHint?.containerPackageName
+		? odspResolvedUrl.codeHint
+		: resolvedUrl?.codeHint;
 
 	if (shareLinkInfo?.createLink?.link) {
 		let newWebUrl = shareLinkInfo.createLink.link.webUrl;

diff --git a/packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts b/packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts
@@ -10,7 +10,7 @@ import {
 } from "@fluidframework/driver-definitions/internal";
 import { ISharingLinkKind } from "@fluidframework/odsp-driver-definitions/internal";
 
-import { buildOdspShareLinkReqParams, getContainerPackageName } from "./odspUtils.js";
+import { buildOdspShareLinkReqParams } from "./odspUtils.js";
 
 /**
  * Create the request object with url and headers for creating a new file on OneDrive Sharepoint
@@ -37,7 +37,7 @@ export function createOdspCreateContainerRequest(
 	const createNewRequest: IRequest = {
 		url: `${siteUrl}?driveId=${encodeURIComponent(driveId)}&path=${encodeURIComponent(
 			filePath,
-		)}${containerPackageInfo ? `&containerPackageName=${getContainerPackageName(containerPackageInfo)}` : ""}${shareLinkRequestParams ? `&${shareLinkRequestParams}` : ""}`,
+		)}${shareLinkRequestParams ? `&${shareLinkRequestParams}` : ""}`,
 		headers: {
 			[DriverHeader.createNew]: {
 				fileName,

diff --git a/packages/drivers/odsp-driver/src/odspDocumentServiceFactoryCore.ts b/packages/drivers/odsp-driver/src/odspDocumentServiceFactoryCore.ts
@@ -187,6 +187,7 @@ export class OdspDocumentServiceFactoryCore
 								!!this.hostPolicy.sessionOptions?.forceAccessTokenViaAuthorizationHeader,
 								odspResolvedUrl.isClpCompliantApp,
 								this.hostPolicy.enableSingleRequestForShareLinkWithCreate,
+								odspResolvedUrl,
 							)
 						: module.createNewContainerOnExistingFile(
 								getAuthHeader,

diff --git a/packages/drivers/odsp-driver/src/odspDriverUrlResolverForShareLink.ts b/packages/drivers/odsp-driver/src/odspDriverUrlResolverForShareLink.ts
@@ -73,6 +73,7 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
 	 * navigates directly to the link.
 	 * @param getContext - callback function which is used to get context for given resolved url. If context
 	 * is returned then it will be embedded into url returned by getAbsoluteUrl() method.
+	 * @param containerPackageInfo - container package information which will be used to extract the container package name.
 	 */
 	public constructor(
 		shareLinkFetcherProps?: ShareLinkFetcherProps | undefined,
@@ -82,6 +83,7 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
 			resolvedUrl: IOdspResolvedUrl,
 			dataStorePath: string,
 		) => Promise<string | undefined>,
+		private readonly containerPackageInfo?: IContainerPackageInfo | undefined,
 	) {
 		this.logger = createOdspLogger(logger);
 		if (shareLinkFetcherProps) {
@@ -155,6 +157,10 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
 
 		odspResolvedUrl.appName = this.appName;
 
+		odspResolvedUrl.codeHint = odspResolvedUrl.codeHint?.containerPackageName
+			? odspResolvedUrl.codeHint
+			: { containerPackageName: getContainerPackageName(this.containerPackageInfo) };
+
 		if (isSharingLinkToRedeem) {
 			// We need to remove the nav param if set by host when setting the sharelink as otherwise the shareLinkId
 			// when redeeming the share link during the redeem fallback for trees latest call becomes greater than
@@ -257,7 +263,8 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
 
 		const containerPackageName: string | undefined =
 			getContainerPackageName(packageInfoSource) ??
-			odspResolvedUrl.codeHint?.containerPackageName;
+			odspResolvedUrl.codeHint?.containerPackageName ??
+			getContainerPackageName(this.containerPackageInfo);
 
 		odspResolvedUrl.appName = this.appName;
 

diff --git a/packages/drivers/odsp-driver/src/test/createNewUtilsTests.spec.ts b/packages/drivers/odsp-driver/src/test/createNewUtilsTests.spec.ts
@@ -15,13 +15,24 @@ import {
 	SharingLinkRole,
 	SharingLinkScope,
 } from "@fluidframework/odsp-driver-definitions/internal";
-import { createChildLogger } from "@fluidframework/telemetry-utils/internal";
+import { createChildLogger, MockLogger } from "@fluidframework/telemetry-utils/internal";
 
 import { useCreateNewModule } from "../createFile/index.js";
+import { createOdspCreateContainerRequest } from "../createOdspCreateContainerRequest.js";
 import { EpochTracker } from "../epochTracker.js";
 import { LocalPersistentCache } from "../odspCache.js";
+import { OdspDocumentServiceFactory } from "../odspDocumentServiceFactory.js";
+import {
+	OdspDriverUrlResolverForShareLink,
+	type ShareLinkFetcherProps,
+} from "../odspDriverUrlResolverForShareLink.js";
 import { getHashedDocumentId } from "../odspPublicUtils.js";
-import { IExistingFileInfo, INewFileInfo, createCacheSnapshotKey } from "../odspUtils.js";
+import {
+	IExistingFileInfo,
+	INewFileInfo,
+	createCacheSnapshotKey,
+	getOdspResolvedUrl,
+} from "../odspUtils.js";
 
 import { mockFetchOk } from "./mockFetch.js";
 
@@ -324,7 +335,9 @@ describe("Create New Utils Tests", () => {
 		});
 
 		// Extract the Base64 encoded value of `nav`
-		const base64Value = mockSharingLinkData.webUrl.match(/nav=([^&]*)/)?.[1] as string;
+		const base64Value = odspResolvedUrl.shareLinkInfo.createLink.link.webUrl.match(
+			/nav=([^&]*)/,
+		)?.[1] as string;
 		// Decode the Base64 value to UTF-8, \r�� is being stored at the end of the string so we slice it off
 		const decodedValue = fromBase64ToUtf8(base64Value).slice(0, -3);
 
@@ -471,70 +484,74 @@ describe("Create New Utils Tests", () => {
 		assert(!odspResolvedUrl2.isClpCompliantApp, "isClpCompliantApp should be falsy");
 		await epochTracker.removeEntries().catch(() => {});
 	});
-	it("Should set the appropriate nav param info when a resolved url is sent", async () => {
-		const mockOdspResolvedUrl: IOdspResolvedUrl = {
-			...resolvedUrl,
-			odspResolvedUrl: true,
-			summarizer: true,
-			dataStorePath: "/dataStorePath",
-			codeHint: {
-				containerPackageName: "mockContainerPackageName",
-			},
-			fileVersion: "mockFileVersion",
-			context: "mockContext",
-			appName: "mockAppName",
+
+	it("Should set the appropriate nav param info when a request is made", async () => {
+		const odspDocumentServiceFactory = new OdspDocumentServiceFactory(
+			async (_options) => "token",
+			async (_options) => "token",
+			new LocalPersistentCache(2000),
+			{ snapshotOptions: { timeout: 2000 }, enableSingleRequestForShareLinkWithCreate: true },
+		);
+
+		const expectedResponse = {
+			context: "http://sp.devinstall/_api/v2.1/$metadata#",
+			sequenceNumber: 1,
+			sha: "shaxxshaxx",
+			itemUrl: `http://fake.microsoft.com/_api/v2.1/drives/${driveId}/items/${itemId}`,
+			driveId,
+			itemId,
+			id: "Summary Handle",
+			sharing: mockSharingData,
+			dataStorePath: "/path",
 		};
-		const odspResolvedUrl = await useCreateNewModule(createChildLogger(), async (module) =>
-			mockFetchOk(
-				async () =>
-					module.createNewFluidFile(
-						async (_options) => "token",
-						newFileParams,
-						createChildLogger(),
-						createSummary(),
-						epochTracker,
-						fileEntry,
-						false /* createNewCaching */,
-						false /* forceAccessTokenViaAuthorizationHeader */,
-						undefined /* isClpCompliantApp */,
-						true /* enableSingleRequestForShareLinkWithCreate */,
-						mockOdspResolvedUrl,
-					),
-				{
-					itemId: "mockItemId",
-					id: "mockId",
-					sharing: mockSharingData,
-					sharingLinkErrorReason: undefined,
-				},
-				{ "x-fluid-epoch": "epoch1" },
-			),
+
+		const fileName = "fileName";
+		const request = createOdspCreateContainerRequest(siteUrl, driveId, filePath, fileName);
+		const logger = new MockLogger();
+
+		const sharingLinkFetcherProps: ShareLinkFetcherProps = {
+			tokenFetcher: async () => "token",
+			identityType: "Enterprise",
+		};
+
+		const getContext = async (
+			url: IOdspResolvedUrl,
+			dataStorePath: string,
+		): Promise<string> => {
+			if (dataStorePath === "") {
+				return "context";
+			}
+			return url.dataStorePath ?? "context";
+		};
+		const shareLinkResolver = new OdspDriverUrlResolverForShareLink(
+			sharingLinkFetcherProps,
+			logger,
+			"appName",
+			getContext,
+			{ name: "containerPackageName" },
 		);
 
-		// 's=%2FsiteUrl&d=driveId&f=mockItemId&c=%2F&fluid=1&a=mockAppName&p=mockContainerPackageName&x=mockContext'
-		// Update the webUrl to the version that has the nav parameter that was supposed to be added
-		mockSharingLinkData.webUrl =
-			"https://mock.url/?nav=cz0lMkZzaXRlVXJsJmQ9ZHJpdmVJZCZmPW1vY2tJdGVtSWQmYz0lMkYmZmx1aWQ9MSZhPW1vY2tBcHBOYW1lJnA9bW9ja0NvbnRhaW5lclBhY2thZ2VOYW1lJng9bW9ja0NvbnRleHQ%3D";
+		const resolved = await shareLinkResolver.resolve(request);
+		const summary = createSummary();
+		const docService = await mockFetchOk(
+			async () => odspDocumentServiceFactory.createContainer(summary, resolved, logger),
+			expectedResponse,
+			{ "x-fluid-epoch": "epoch1" },
+		);
+
+		const finalResolverUrl = getOdspResolvedUrl(docService.resolvedUrl);
 
-		assert.deepStrictEqual(odspResolvedUrl.shareLinkInfo?.createLink, {
-			shareId: mockSharingData.shareId,
-			link: {
-				role: mockSharingData.sharingLink.type,
-				...mockSharingData.sharingLink,
-			},
-			error: undefined,
-		});
 		// Extract the Base64 encoded value of `nav`
-		const base64Value = mockSharingLinkData.webUrl.match(/nav=([^&]*)/)?.[1] as string;
-		// Decode the Base64 value to UTF-8, need to slice off last unnecessary character
-		const decodedValue = fromBase64ToUtf8(base64Value).slice(0, -1);
+		const base64Value = finalResolverUrl.shareLinkInfo?.createLink?.link?.webUrl.match(
+			/nav=([^&]*)/,
+		)?.[1] as string;
+		// Decode the Base64 value to UTF-8, \r�� is being stored at the end of the string so we slice it off
+		const decodedValue = fromBase64ToUtf8(base64Value).slice(0, -3);
 
 		// Compare the values to make sure that the nav parameter was added correctly
 		assert.equal(
 			decodedValue,
-			"s=%2FsiteUrl&d=driveId&f=mockItemId&c=%2F&fluid=1&a=mockAppName&p=mockContainerPackageName&x=mockContext",
+			"s=%2FsiteUrl&d=driveId&f=itemId&c=%2F&fluid=1&a=appName&p=containerPackageName&x=context",
 		);
-
-		// Reset the webUrl to the original value
-		mockSharingLinkData.webUrl = "https://mock.url";
 	});
 });
diff --git a/packages/drivers/odsp-driver/src/test/odspDriverResolverTest.spec.ts b/packages/drivers/odsp-driver/src/test/odspDriverResolverTest.spec.ts
@@ -457,20 +457,4 @@ describe("Odsp Driver Resolver", () => {
 			`https://placeholder/placeholder/${resolvedUrl.hashedDocumentId}/` + `${testFilePath}`;
 		assert.strictEqual(resolvedUrl.url, expectedResolvedUrl, "resolved url is wrong");
 	});
-	it("Should create request with containerPackageName and resolve it", async () => {
-		request = createOdspCreateContainerRequest(
-			siteUrl,
-			driveId,
-			filePath,
-			fileName,
-			undefined,
-			{ name: "testContainerPackageName" }, // Container package info variable,
-		);
-		const resolvedUrl = await resolver.resolve(request);
-		assert.strictEqual(
-			resolvedUrl.codeHint?.containerPackageName,
-			"testContainerPackageName",
-			"containerPackageName should match",
-		);
-	});
 });