Skip to content

Commit dad284d

Browse files
Reverted Follow up to add nav parameter (#23873)
## Description This change needs to be reverted for now and will me merged for next release --------- Co-authored-by: Jatin Garg <[email protected]>
1 parent ffab747 commit dad284d

7 files changed

+78
-90
lines changed

packages/drivers/odsp-driver/api-report/odsp-driver.legacy.alpha.api.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ export class OdspDriverUrlResolver implements IUrlResolver {
174174

175175
// @alpha
176176
export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
177-
constructor(shareLinkFetcherProps?: ShareLinkFetcherProps | undefined, logger?: ITelemetryBaseLogger, appName?: string | undefined, getContext?: ((resolvedUrl: IOdspResolvedUrl, dataStorePath: string) => Promise<string | undefined>) | undefined, containerPackageInfo?: IContainerPackageInfo | undefined);
177+
constructor(shareLinkFetcherProps?: ShareLinkFetcherProps | undefined, logger?: ITelemetryBaseLogger, appName?: string | undefined, getContext?: ((resolvedUrl: IOdspResolvedUrl, dataStorePath: string) => Promise<string | undefined>) | undefined);
178178
appendDataStorePath(requestUrl: URL, pathToAppend: string): string | undefined;
179179
appendLocatorParams(baseUrl: string, resolvedUrl: IResolvedUrl, dataStorePath: string, packageInfoSource?: IContainerPackageInfo): Promise<string>;
180180
static createDocumentUrl(baseUrl: string, driverInfo: OdspFluidDataStoreLocator): string;

packages/drivers/odsp-driver/src/createFile/createFile.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,7 @@ export async function createNewFluidFile(
115115

116116
odspResolvedUrl.context = resolvedUrl?.context;
117117
odspResolvedUrl.appName = resolvedUrl?.appName;
118-
odspResolvedUrl.codeHint = odspResolvedUrl.codeHint?.containerPackageName
119-
? odspResolvedUrl.codeHint
120-
: resolvedUrl?.codeHint;
118+
odspResolvedUrl.codeHint = resolvedUrl?.codeHint;
121119

122120
if (shareLinkInfo?.createLink?.link) {
123121
let newWebUrl = shareLinkInfo.createLink.link.webUrl;

packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
} from "@fluidframework/driver-definitions/internal";
1111
import { ISharingLinkKind } from "@fluidframework/odsp-driver-definitions/internal";
1212

13-
import { buildOdspShareLinkReqParams } from "./odspUtils.js";
13+
import { buildOdspShareLinkReqParams, getContainerPackageName } from "./odspUtils.js";
1414

1515
/**
1616
* Create the request object with url and headers for creating a new file on OneDrive Sharepoint
@@ -37,7 +37,7 @@ export function createOdspCreateContainerRequest(
3737
const createNewRequest: IRequest = {
3838
url: `${siteUrl}?driveId=${encodeURIComponent(driveId)}&path=${encodeURIComponent(
3939
filePath,
40-
)}${shareLinkRequestParams ? `&${shareLinkRequestParams}` : ""}`,
40+
)}${containerPackageInfo ? `&containerPackageName=${getContainerPackageName(containerPackageInfo)}` : ""}${shareLinkRequestParams ? `&${shareLinkRequestParams}` : ""}`,
4141
headers: {
4242
[DriverHeader.createNew]: {
4343
fileName,

packages/drivers/odsp-driver/src/odspDocumentServiceFactoryCore.ts

-1
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,6 @@ export class OdspDocumentServiceFactoryCore
187187
!!this.hostPolicy.sessionOptions?.forceAccessTokenViaAuthorizationHeader,
188188
odspResolvedUrl.isClpCompliantApp,
189189
this.hostPolicy.enableSingleRequestForShareLinkWithCreate,
190-
odspResolvedUrl,
191190
)
192191
: module.createNewContainerOnExistingFile(
193192
getAuthHeader,

packages/drivers/odsp-driver/src/odspDriverUrlResolverForShareLink.ts

+1-8
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
7373
* navigates directly to the link.
7474
* @param getContext - callback function which is used to get context for given resolved url. If context
7575
* is returned then it will be embedded into url returned by getAbsoluteUrl() method.
76-
* @param containerPackageInfo - container package information which will be used to extract the container package name.
7776
*/
7877
public constructor(
7978
shareLinkFetcherProps?: ShareLinkFetcherProps | undefined,
@@ -83,7 +82,6 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
8382
resolvedUrl: IOdspResolvedUrl,
8483
dataStorePath: string,
8584
) => Promise<string | undefined>,
86-
private readonly containerPackageInfo?: IContainerPackageInfo | undefined,
8785
) {
8886
this.logger = createOdspLogger(logger);
8987
if (shareLinkFetcherProps) {
@@ -157,10 +155,6 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
157155

158156
odspResolvedUrl.appName = this.appName;
159157

160-
odspResolvedUrl.codeHint = odspResolvedUrl.codeHint?.containerPackageName
161-
? odspResolvedUrl.codeHint
162-
: { containerPackageName: getContainerPackageName(this.containerPackageInfo) };
163-
164158
if (isSharingLinkToRedeem) {
165159
// We need to remove the nav param if set by host when setting the sharelink as otherwise the shareLinkId
166160
// when redeeming the share link during the redeem fallback for trees latest call becomes greater than
@@ -263,8 +257,7 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
263257

264258
const containerPackageName: string | undefined =
265259
getContainerPackageName(packageInfoSource) ??
266-
odspResolvedUrl.codeHint?.containerPackageName ??
267-
getContainerPackageName(this.containerPackageInfo);
260+
odspResolvedUrl.codeHint?.containerPackageName;
268261

269262
odspResolvedUrl.appName = this.appName;
270263

packages/drivers/odsp-driver/src/test/createNewUtilsTests.spec.ts

+57-75
Original file line numberDiff line numberDiff line change
@@ -15,24 +15,13 @@ import {
1515
SharingLinkRole,
1616
SharingLinkScope,
1717
} from "@fluidframework/odsp-driver-definitions/internal";
18-
import { createChildLogger, MockLogger } from "@fluidframework/telemetry-utils/internal";
18+
import { createChildLogger } from "@fluidframework/telemetry-utils/internal";
1919

2020
import { useCreateNewModule } from "../createFile/index.js";
21-
import { createOdspCreateContainerRequest } from "../createOdspCreateContainerRequest.js";
2221
import { EpochTracker } from "../epochTracker.js";
2322
import { LocalPersistentCache } from "../odspCache.js";
24-
import { OdspDocumentServiceFactory } from "../odspDocumentServiceFactory.js";
25-
import {
26-
OdspDriverUrlResolverForShareLink,
27-
type ShareLinkFetcherProps,
28-
} from "../odspDriverUrlResolverForShareLink.js";
2923
import { getHashedDocumentId } from "../odspPublicUtils.js";
30-
import {
31-
IExistingFileInfo,
32-
INewFileInfo,
33-
createCacheSnapshotKey,
34-
getOdspResolvedUrl,
35-
} from "../odspUtils.js";
24+
import { IExistingFileInfo, INewFileInfo, createCacheSnapshotKey } from "../odspUtils.js";
3625

3726
import { mockFetchOk } from "./mockFetch.js";
3827

@@ -335,9 +324,7 @@ describe("Create New Utils Tests", () => {
335324
});
336325

337326
// Extract the Base64 encoded value of `nav`
338-
const base64Value = odspResolvedUrl.shareLinkInfo.createLink.link.webUrl.match(
339-
/nav=([^&]*)/,
340-
)?.[1] as string;
327+
const base64Value = mockSharingLinkData.webUrl.match(/nav=([^&]*)/)?.[1] as string;
341328
// Decode the Base64 value to UTF-8, \r�� is being stored at the end of the string so we slice it off
342329
const decodedValue = fromBase64ToUtf8(base64Value).slice(0, -3);
343330

@@ -484,74 +471,69 @@ describe("Create New Utils Tests", () => {
484471
assert(!odspResolvedUrl2.isClpCompliantApp, "isClpCompliantApp should be falsy");
485472
await epochTracker.removeEntries().catch(() => {});
486473
});
487-
488-
it("Should set the appropriate nav param info when a request is made", async () => {
489-
const odspDocumentServiceFactory = new OdspDocumentServiceFactory(
490-
async (_options) => "token",
491-
async (_options) => "token",
492-
new LocalPersistentCache(2000),
493-
{ snapshotOptions: { timeout: 2000 }, enableSingleRequestForShareLinkWithCreate: true },
494-
);
495-
496-
const expectedResponse = {
497-
context: "http://sp.devinstall/_api/v2.1/$metadata#",
498-
sequenceNumber: 1,
499-
sha: "shaxxshaxx",
500-
itemUrl: `http://fake.microsoft.com/_api/v2.1/drives/${driveId}/items/${itemId}`,
501-
driveId,
502-
itemId,
503-
id: "Summary Handle",
504-
sharing: mockSharingData,
505-
dataStorePath: "/path",
506-
};
507-
508-
const fileName = "fileName";
509-
const request = createOdspCreateContainerRequest(siteUrl, driveId, filePath, fileName);
510-
const logger = new MockLogger();
511-
512-
const sharingLinkFetcherProps: ShareLinkFetcherProps = {
513-
tokenFetcher: async () => "token",
514-
identityType: "Enterprise",
515-
};
516-
517-
const getContext = async (
518-
url: IOdspResolvedUrl,
519-
dataStorePath: string,
520-
): Promise<string> => {
521-
if (dataStorePath === "") {
522-
return "context";
523-
}
524-
return url.dataStorePath ?? "context";
474+
it("Should set the appropriate nav param info when a resolved url is sent", async () => {
475+
const mockOdspResolvedUrl: IOdspResolvedUrl = {
476+
...resolvedUrl,
477+
odspResolvedUrl: true,
478+
summarizer: true,
479+
dataStorePath: "/dataStorePath",
480+
codeHint: {
481+
containerPackageName: "mockContainerPackageName",
482+
},
483+
fileVersion: "mockFileVersion",
484+
context: "mockContext",
485+
appName: "mockAppName",
525486
};
526-
const shareLinkResolver = new OdspDriverUrlResolverForShareLink(
527-
sharingLinkFetcherProps,
528-
logger,
529-
"appName",
530-
getContext,
531-
{ name: "containerPackageName" },
532-
);
533-
534-
const resolved = await shareLinkResolver.resolve(request);
535-
const summary = createSummary();
536-
const docService = await mockFetchOk(
537-
async () => odspDocumentServiceFactory.createContainer(summary, resolved, logger),
538-
expectedResponse,
539-
{ "x-fluid-epoch": "epoch1" },
487+
const odspResolvedUrl = await useCreateNewModule(createChildLogger(), async (module) =>
488+
mockFetchOk(
489+
async () =>
490+
module.createNewFluidFile(
491+
async (_options) => "token",
492+
newFileParams,
493+
createChildLogger(),
494+
createSummary(),
495+
epochTracker,
496+
fileEntry,
497+
false /* createNewCaching */,
498+
false /* forceAccessTokenViaAuthorizationHeader */,
499+
undefined /* isClpCompliantApp */,
500+
true /* enableSingleRequestForShareLinkWithCreate */,
501+
mockOdspResolvedUrl,
502+
),
503+
{
504+
itemId: "mockItemId",
505+
id: "mockId",
506+
sharing: mockSharingData,
507+
sharingLinkErrorReason: undefined,
508+
},
509+
{ "x-fluid-epoch": "epoch1" },
510+
),
540511
);
541512

542-
const finalResolverUrl = getOdspResolvedUrl(docService.resolvedUrl);
513+
// 's=%2FsiteUrl&d=driveId&f=mockItemId&c=%2F&fluid=1&a=mockAppName&p=mockContainerPackageName&x=mockContext'
514+
// Update the webUrl to the version that has the nav parameter that was supposed to be added
515+
mockSharingLinkData.webUrl =
516+
"https://mock.url/?nav=cz0lMkZzaXRlVXJsJmQ9ZHJpdmVJZCZmPW1vY2tJdGVtSWQmYz0lMkYmZmx1aWQ9MSZhPW1vY2tBcHBOYW1lJnA9bW9ja0NvbnRhaW5lclBhY2thZ2VOYW1lJng9bW9ja0NvbnRleHQ%3D";
543517

518+
assert.deepStrictEqual(odspResolvedUrl.shareLinkInfo?.createLink, {
519+
shareId: mockSharingData.shareId,
520+
link: {
521+
role: mockSharingData.sharingLink.type,
522+
...mockSharingData.sharingLink,
523+
},
524+
error: undefined,
525+
});
544526
// Extract the Base64 encoded value of `nav`
545-
const base64Value = finalResolverUrl.shareLinkInfo?.createLink?.link?.webUrl.match(
546-
/nav=([^&]*)/,
547-
)?.[1] as string;
548-
// Decode the Base64 value to UTF-8, \r�� is being stored at the end of the string so we slice it off
549-
const decodedValue = fromBase64ToUtf8(base64Value).slice(0, -3);
527+
const base64Value = mockSharingLinkData.webUrl.match(/nav=([^&]*)/)?.[1] as string;
528+
// Decode the Base64 value to UTF-8, need to slice off last unnecessary character
529+
const decodedValue = fromBase64ToUtf8(base64Value).slice(0, -1);
550530

551531
// Compare the values to make sure that the nav parameter was added correctly
552532
assert.equal(
553533
decodedValue,
554-
"s=%2FsiteUrl&d=driveId&f=itemId&c=%2F&fluid=1&a=appName&p=containerPackageName&x=context",
534+
"s=%2FsiteUrl&d=driveId&f=mockItemId&c=%2F&fluid=1&a=mockAppName&p=mockContainerPackageName&x=mockContext",
555535
);
536+
// Reset the webUrl to the original value
537+
mockSharingLinkData.webUrl = "https://mock.url";
556538
});
557539
});

packages/drivers/odsp-driver/src/test/odspDriverResolverTest.spec.ts

+16
Original file line numberDiff line numberDiff line change
@@ -457,4 +457,20 @@ describe("Odsp Driver Resolver", () => {
457457
`https://placeholder/placeholder/${resolvedUrl.hashedDocumentId}/` + `${testFilePath}`;
458458
assert.strictEqual(resolvedUrl.url, expectedResolvedUrl, "resolved url is wrong");
459459
});
460+
it("Should create request with containerPackageName and resolve it", async () => {
461+
request = createOdspCreateContainerRequest(
462+
siteUrl,
463+
driveId,
464+
filePath,
465+
fileName,
466+
undefined,
467+
{ name: "testContainerPackageName" }, // Container package info variable,
468+
);
469+
const resolvedUrl = await resolver.resolve(request);
470+
assert.strictEqual(
471+
resolvedUrl.codeHint?.containerPackageName,
472+
"testContainerPackageName",
473+
"containerPackageName should match",
474+
);
475+
});
460476
});

0 commit comments

Comments
 (0)