Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverted Follow up to add nav parameter #23873

Merged
merged 19 commits into from
Feb 19, 2025
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -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, containerPackageInfo?: IContainerPackageInfo | undefined);
constructor(shareLinkFetcherProps?: ShareLinkFetcherProps | undefined, logger?: ITelemetryBaseLogger, appName?: string | undefined, getContext?: ((resolvedUrl: IOdspResolvedUrl, dataStorePath: string) => Promise<string | undefined>) | 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;
Expand Down
4 changes: 1 addition & 3 deletions packages/drivers/odsp-driver/src/createFile/createFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,7 @@ export async function createNewFluidFile(

odspResolvedUrl.context = resolvedUrl?.context;
odspResolvedUrl.appName = resolvedUrl?.appName;
odspResolvedUrl.codeHint = odspResolvedUrl.codeHint?.containerPackageName
? odspResolvedUrl.codeHint
: resolvedUrl?.codeHint;
odspResolvedUrl.codeHint = resolvedUrl?.codeHint;

if (shareLinkInfo?.createLink?.link) {
let newWebUrl = shareLinkInfo.createLink.link.webUrl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from "@fluidframework/driver-definitions/internal";
import { ISharingLinkKind } from "@fluidframework/odsp-driver-definitions/internal";

import { buildOdspShareLinkReqParams } from "./odspUtils.js";
import { buildOdspShareLinkReqParams, getContainerPackageName } from "./odspUtils.js";

/**
* Create the request object with url and headers for creating a new file on OneDrive Sharepoint
Expand All @@ -37,7 +37,7 @@ export function createOdspCreateContainerRequest(
const createNewRequest: IRequest = {
url: `${siteUrl}?driveId=${encodeURIComponent(driveId)}&path=${encodeURIComponent(
filePath,
)}${shareLinkRequestParams ? `&${shareLinkRequestParams}` : ""}`,
)}${containerPackageInfo ? `&containerPackageName=${getContainerPackageName(containerPackageInfo)}` : ""}${shareLinkRequestParams ? `&${shareLinkRequestParams}` : ""}`,
headers: {
[DriverHeader.createNew]: {
fileName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ export class OdspDocumentServiceFactoryCore
!!this.hostPolicy.sessionOptions?.forceAccessTokenViaAuthorizationHeader,
odspResolvedUrl.isClpCompliantApp,
this.hostPolicy.enableSingleRequestForShareLinkWithCreate,
odspResolvedUrl,
)
: module.createNewContainerOnExistingFile(
getAuthHeader,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ 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,
Expand All @@ -83,7 +82,6 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
resolvedUrl: IOdspResolvedUrl,
dataStorePath: string,
) => Promise<string | undefined>,
private readonly containerPackageInfo?: IContainerPackageInfo | undefined,
) {
this.logger = createOdspLogger(logger);
if (shareLinkFetcherProps) {
Expand Down Expand Up @@ -157,10 +155,6 @@ 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
Expand Down Expand Up @@ -263,8 +257,7 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {

const containerPackageName: string | undefined =
getContainerPackageName(packageInfoSource) ??
odspResolvedUrl.codeHint?.containerPackageName ??
getContainerPackageName(this.containerPackageInfo);
odspResolvedUrl.codeHint?.containerPackageName;

odspResolvedUrl.appName = this.appName;

Expand Down
132 changes: 57 additions & 75 deletions packages/drivers/odsp-driver/src/test/createNewUtilsTests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,13 @@ import {
SharingLinkRole,
SharingLinkScope,
} from "@fluidframework/odsp-driver-definitions/internal";
import { createChildLogger, MockLogger } from "@fluidframework/telemetry-utils/internal";
import { createChildLogger } 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,
getOdspResolvedUrl,
} from "../odspUtils.js";
import { IExistingFileInfo, INewFileInfo, createCacheSnapshotKey } from "../odspUtils.js";

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

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

// Extract the Base64 encoded value of `nav`
const base64Value = odspResolvedUrl.shareLinkInfo.createLink.link.webUrl.match(
/nav=([^&]*)/,
)?.[1] as string;
const base64Value = mockSharingLinkData.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);

Expand Down Expand Up @@ -484,74 +471,69 @@ 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 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 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";
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",
};
const shareLinkResolver = new OdspDriverUrlResolverForShareLink(
sharingLinkFetcherProps,
logger,
"appName",
getContext,
{ name: "containerPackageName" },
);

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 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 finalResolverUrl = getOdspResolvedUrl(docService.resolvedUrl);
// '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";

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 = 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);
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);

// Compare the values to make sure that the nav parameter was added correctly
assert.equal(
decodedValue,
"s=%2FsiteUrl&d=driveId&f=itemId&c=%2F&fluid=1&a=appName&p=containerPackageName&x=context",
"s=%2FsiteUrl&d=driveId&f=mockItemId&c=%2F&fluid=1&a=mockAppName&p=mockContainerPackageName&x=mockContext",
);
// Reset the webUrl to the original value
mockSharingLinkData.webUrl = "https://mock.url";
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -457,4 +457,20 @@ 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",
);
});
});
Loading