Odsp resolver code refactoring (#4152)

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

@@ -19,7 +19,6 @@ import {
     ICreateFileResponse,
 } from "./contracts";
 import { getUrlAndHeadersWithAuth } from "./getUrlAndHeadersWithAuth";
-import { OdspDriverUrlResolver2 } from "./odspDriverUrlResolver2";
 import {
     getWithRetryForTokenRefresh,
     INewFileInfo,
@@ -47,7 +46,6 @@ export async function createNewFluidFile(
     logger: ITelemetryLogger,
     createNewSummary: ISummaryTree,
     epochTracker: EpochTracker,
-    getSharingLinkToken?: (options: TokenFetchOptions, isForFileDefaultUrl: boolean) => Promise<string | null>,
 ): Promise<IOdspResolvedUrl> {
     // Check for valid filename before the request to create file is actually made.
     if (isInvalidFileName(newFileInfo.filename)) {
@@ -93,8 +91,7 @@ export async function createNewFluidFile(
     });
 
     const odspUrl = createOdspUrl(newFileInfo.siteUrl, newFileInfo.driveId, itemId, "/");
-    const resolver = getSharingLinkToken ?
-        new OdspDriverUrlResolver2(getSharingLinkToken) : new OdspDriverUrlResolver();
+    const resolver = new OdspDriverUrlResolver();
     return resolver.resolve({ url: odspUrl });
 }
 

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

@@ -7,7 +7,7 @@ import { ITelemetryLogger, ITelemetryProperties } from "@fluidframework/common-d
 import { PromiseCache } from "@fluidframework/common-utils";
 import { authorizedFetchWithRetry } from "./authorizedFetchWithRetry";
 import { RetryPolicy } from "./fetchWithRetry";
-import { IdentityType, TokenFetchOptions } from "./tokenFetch";
+import { IdentityType, SharingLinkScopeFor, TokenFetchOptions } from "./tokenFetch";
 
 /**
  * This represents a lite version of GraphItem containing only the name, webUrl and webDavUrl properties
@@ -33,7 +33,8 @@ export interface GraphItemLite {
  * @returns Promise which resolves to share link url when successful; otherwise, undefined.
  */
 export async function getShareLink(
-    getShareLinkToken: (options: TokenFetchOptions, isForFileDefaultUrl: boolean) => Promise<string | null>,
+    getShareLinkToken:
+        (options: TokenFetchOptions, scopeFor: SharingLinkScopeFor, siteUrl: string) => Promise<string | null>,
     siteUrl: string,
     driveId: string,
     itemId: string,
@@ -48,6 +49,7 @@ export async function getShareLink(
 
     const createShareLinkResponse = await graphFetch(
         getShareLinkToken,
+        siteUrl,
         `drives/${driveId}/items/${itemId}/createLink`,
         `GetShareLink_${scope}_${type}`,
         logger,
@@ -69,6 +71,7 @@ export async function getShareLink(
 /**
  * Issues a graph fetch request
  * @param getShareLinkToken - Token provider than can supply Graph tokens
+ * @param siteUrl - SiteUrl of the site that contains the file
  * @param graphUrl - Url to fetch. Can either be a full URL (e.g. https://graph.microsoft.com/v1.0/me/people)
  *  or a partial url (e.g. me/people)
  * @param nameForLogging - Name used for logging
@@ -78,7 +81,9 @@ export async function getShareLink(
  * @param timeoutMs - Timeout value to be passed to fetchWithRetry
  */
 export async function graphFetch(
-    getShareLinkToken: (options: TokenFetchOptions, isForFileDefaultUrl: boolean) => Promise<string | null>,
+    getShareLinkToken:
+        (options: TokenFetchOptions, scopeFor: SharingLinkScopeFor, siteUrl: string) => Promise<string | null>,
+    siteUrl: string,
     graphUrl: string,
     nameForLogging: string,
     logger?: ITelemetryLogger,
@@ -89,7 +94,8 @@ export async function graphFetch(
     const getToken = async (options: TokenFetchOptions) =>
         getShareLinkToken(
             options,
-            false,
+            SharingLinkScopeFor.nonFileDefaultUrl,
+            siteUrl,
         );
     const url = graphUrl.startsWith("http") ? graphUrl : `https://graph.microsoft.com/v1.0/${graphUrl}`;
     return (
@@ -143,14 +149,15 @@ const getSPOAndGraphRequestIdsFromResponse = async (response: Response) => {
  * @returns Promise which resolves to file url when successful; otherwise, undefined.
  */
 async function getFileDefaultUrl(
-    getShareLinkToken: (options: TokenFetchOptions, isForFileDefaultUrl: boolean) => Promise<string | null>,
+    getShareLinkToken:
+        (options: TokenFetchOptions, scopeFor: SharingLinkScopeFor, siteUrl: string) => Promise<string | null>,
     siteUrl: string,
     driveId: string,
     itemId: string,
     identityType: IdentityType,
     logger?: ITelemetryLogger,
 ): Promise<string | undefined> {
-    const graphItem = await getGraphItemLite(getShareLinkToken, driveId, itemId, logger);
+    const graphItem = await getGraphItemLite(getShareLinkToken, siteUrl, driveId, itemId, logger);
     if (!graphItem) {
         return undefined;
     }
@@ -163,7 +170,7 @@ async function getFileDefaultUrl(
     // ODSP link requires extra call to return link that is resistant to file being renamed or moved to different folder
     const fetchResponse = await authorizedFetchWithRetry({
         getToken: async (options) =>
-            getShareLinkToken(options, true),
+            getShareLinkToken(options, SharingLinkScopeFor.fileDefaultUrl, siteUrl),
         url: `${siteUrl}/_api/web/GetFileByUrl(@a1)/ListItemAllFields/GetSharingInformation?@a1=${encodeURIComponent(
             `'${graphItem.webDavUrl}'`,
         )}`,
@@ -194,6 +201,7 @@ const graphItemLiteCache = new PromiseCache<string, GraphItemLite | undefined>()
  * This API gets only few properties representing the GraphItem - hence 'Lite'.
  * Scope needed: files.read.all
  * @param getShareLinkToken - used to fetch access token needed to execute operation
+ * @param siteUrl - SiteUrl of the site that contains the file
  * @param driveId - ID for the drive that contains the file
  * @param itemId - ID of the file
  * @param logger - used to log results of operation, including any error
@@ -205,7 +213,9 @@ const graphItemLiteCache = new PromiseCache<string, GraphItemLite | undefined>()
  * powering MRU and Share functionality.
  */
 export async function getGraphItemLite(
-    getShareLinkToken: (options: TokenFetchOptions, isForFileDefaultUrl: boolean) => Promise<string | null>,
+    getShareLinkToken:
+        (options: TokenFetchOptions, scopeFor: SharingLinkScopeFor, siteUrl: string) => Promise<string | null>,
+    siteUrl: string,
     driveId: string,
     itemId: string,
     logger?: ITelemetryLogger,
@@ -217,7 +227,7 @@ export async function getGraphItemLite(
 
             let response: Response | undefined;
             try {
-                response = await graphFetch(getShareLinkToken, partialUrl, "GetGraphItemLite", logger);
+                response = await graphFetch(getShareLinkToken, siteUrl, partialUrl, "GetGraphItemLite", logger);
             } catch { }
 
             // Cache only if we got a response and the response was a 200 (success) or 404 (NotFound)

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

@@ -8,7 +8,7 @@ import { HostStoragePolicy } from "./contracts";
 import { IPersistedCache } from "./odspCache";
 import { OdspDocumentServiceFactoryCore } from "./odspDocumentServiceFactoryCore";
 import { getSocketIo } from "./getSocketIo";
-import { StorageTokenFetcher, PushTokenFetcher, SharingLinkTokenFetcher } from "./tokenFetch";
+import { StorageTokenFetcher, PushTokenFetcher } from "./tokenFetch";
 
 /**
  * Factory for creating the sharepoint document service. Use this if you want to
@@ -22,15 +22,13 @@ export class OdspDocumentServiceFactory
         getWebsocketToken: PushTokenFetcher,
         persistedCache?: IPersistedCache,
         hostPolicy?: HostStoragePolicy,
-        getSharingLinkToken?: SharingLinkTokenFetcher,
     ) {
         super(
             getStorageToken,
             getWebsocketToken,
             async () => getSocketIo(),
             persistedCache,
             hostPolicy,
-            getSharingLinkToken,
         );
     }
 }

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

@@ -32,7 +32,6 @@ import {
     TokenFetchOptions,
     isTokenFromCache,
     tokenFromResponse,
-    SharingLinkTokenFetcher,
 } from "./tokenFetch";
 import { EpochTracker } from "./epochTracker";
 
@@ -85,8 +84,7 @@ export class OdspDocumentServiceFactoryCore implements IDocumentServiceFactory {
                     logger2,
                     createNewSummary,
                     epochTracker,
-                    this.getSharingLinkToken ?
-                        this.toInstrumentedSharingLinkTokenFetcher(logger2, this.getSharingLinkToken) : undefined);
+                );
                 const docService = this.createDocumentService(odspResolvedUrl, logger, epochTracker);
                 event.end({
                     docId: odspResolvedUrl.hashedDocumentId,
@@ -103,15 +101,13 @@ export class OdspDocumentServiceFactoryCore implements IDocumentServiceFactory {
    * @param storageFetchWrapper - if not provided FetchWrapper will be used
    * @param deltasFetchWrapper - if not provided FetchWrapper will be used
    * @param persistedCache - PersistedCache provided by host for use in this session.
-   * @param getSharingLinkToken - function that can provide token used to fetch share link for a container.
    */
     constructor(
         private readonly getStorageToken: StorageTokenFetcher,
         private readonly getWebsocketToken: PushTokenFetcher,
         private readonly getSocketIOClient: () => Promise<SocketIOClientStatic>,
         protected persistedCache: IPersistedCache = new LocalPersistentCache(),
         private readonly hostPolicy: HostStoragePolicy = {},
-        private readonly getSharingLinkToken?: SharingLinkTokenFetcher,
     ) {
     }
 
@@ -178,20 +174,4 @@ export class OdspDocumentServiceFactoryCore implements IDocumentServiceFactory {
                 }));
         };
     }
-
-    private toInstrumentedSharingLinkTokenFetcher(
-        logger: ITelemetryLogger,
-        tokenFetcher: SharingLinkTokenFetcher,
-    ): (options: TokenFetchOptions, isForFileDefaultUrl: boolean) => Promise<string | null> {
-        return async (options: TokenFetchOptions, isForFileDefaultUrl: boolean) => {
-            return PerformanceEvent.timedExecAsync(
-                logger,
-                { eventName: "GetSharingLinkToken" },
-                async (event) => tokenFetcher(isForFileDefaultUrl, options.refresh, options.claims)
-                .then((tokenResponse) => {
-                    event.end({ fromCache: isTokenFromCache(tokenResponse) });
-                    return tokenFromResponse(tokenResponse);
-                }));
-        };
-    }
 }

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

@@ -7,7 +7,7 @@ import { IDocumentServiceFactory } from "@fluidframework/driver-definitions";
 import { IPersistedCache } from "./odspCache";
 import { OdspDocumentServiceFactoryCore } from "./odspDocumentServiceFactoryCore";
 import { HostStoragePolicy } from "./contracts";
-import { StorageTokenFetcher, PushTokenFetcher, SharingLinkTokenFetcher } from "./tokenFetch";
+import { StorageTokenFetcher, PushTokenFetcher } from "./tokenFetch";
 
 export class OdspDocumentServiceFactoryWithCodeSplit
     extends OdspDocumentServiceFactoryCore
@@ -17,15 +17,13 @@ export class OdspDocumentServiceFactoryWithCodeSplit
         getWebsocketToken: PushTokenFetcher,
         persistedCache?: IPersistedCache,
         hostPolicy?: HostStoragePolicy,
-        getSharingLinkToken?: SharingLinkTokenFetcher,
     ) {
         super(
             getStorageToken,
             getWebsocketToken,
             async () => import("./getSocketIo").then((m) => m.getSocketIo()),
             persistedCache,
             hostPolicy,
-            getSharingLinkToken,
         );
     }
 }

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

@@ -50,7 +50,73 @@ export class OdspDriverUrlResolver implements IUrlResolver {
     constructor() { }
 
     public async resolve(request: IRequest): Promise<IOdspResolvedUrl> {
-        return resolveRequest(request);
+        if (request.headers && request.headers[DriverHeader.createNew]) {
+            const [siteURL, queryString] = request.url.split("?");
+
+            const searchParams = new URLSearchParams(queryString);
+            const fileName = request.headers[DriverHeader.createNew].fileName;
+            const driveID = searchParams.get("driveId");
+            const filePath = searchParams.get("path");
+            const packageName = searchParams.get("containerPackageName");
+            if (!(fileName && siteURL && driveID && filePath !== null && filePath !== undefined)) {
+                throw new Error("Proper new file params should be there!!");
+            }
+            return {
+                endpoints: {
+                    snapshotStorageUrl: "",
+                    attachmentGETStorageUrl: "",
+                    attachmentPOSTStorageUrl: "",
+                },
+                tokens: {},
+                type: "fluid",
+                url: `fluid-odsp://${siteURL}?${queryString}&version=null`,
+                siteUrl: siteURL,
+                hashedDocumentId: "",
+                driveId: driveID,
+                itemId: "",
+                fileName,
+                summarizer: false,
+                codeHint: {
+                    containerPackageName: packageName ? packageName : undefined,
+                },
+            };
+        }
+        const { siteUrl, driveId, itemId, path, containerPackageName } = decodeOdspUrl(request.url);
+        const hashedDocumentId = getHashedDocumentId(driveId, itemId);
+        assert.ok(!hashedDocumentId.includes("/"), "Docid should not contain slashes!!");
+
+        let documentUrl = `fluid-odsp://placeholder/placeholder/${hashedDocumentId}/${removeBeginningSlash(path)}`;
+
+        if (request.url.length > 0) {
+            // In case of any additional parameters add them back to the url
+            const requestURL = new URL(request.url);
+            const searchParams = requestURL.search;
+            if (searchParams) {
+                documentUrl += searchParams;
+            }
+        }
+
+        const summarizer = request.headers?.[DriverHeader.summarizingClient];
+
+        return {
+            type: "fluid",
+            endpoints: {
+                snapshotStorageUrl: getSnapshotUrl(siteUrl, driveId, itemId),
+                attachmentPOSTStorageUrl: getAttachmentPOSTUrl(siteUrl, driveId, itemId),
+                attachmentGETStorageUrl: getAttachmentGETUrl(siteUrl, driveId, itemId),
+            },
+            tokens: {},
+            url: documentUrl,
+            hashedDocumentId,
+            siteUrl,
+            driveId,
+            itemId,
+            fileName: "",
+            summarizer,
+            codeHint: {
+                containerPackageName,
+            },
+        };
     }
 
     public async getAbsoluteUrl(
@@ -120,73 +186,3 @@ function decodeOdspUrl(url: string): {
         containerPackageName: containerPackageName ? decodeURIComponent(containerPackageName) : undefined,
     };
 }
-
-export async function resolveRequest(request: IRequest): Promise<IOdspResolvedUrl> {
-    if (request.headers && request.headers[DriverHeader.createNew]) {
-        const [siteURL, queryString] = request.url.split("?");
-
-        const searchParams = new URLSearchParams(queryString);
-        const fileName = request.headers[DriverHeader.createNew].fileName;
-        const driveID = searchParams.get("driveId");
-        const filePath = searchParams.get("path");
-        const packageName = searchParams.get("containerPackageName");
-        if (!(fileName && siteURL && driveID && filePath !== null && filePath !== undefined)) {
-            throw new Error("Proper new file params should be there!!");
-        }
-        return {
-            endpoints: {
-                snapshotStorageUrl: "",
-                attachmentGETStorageUrl: "",
-                attachmentPOSTStorageUrl: "",
-            },
-            tokens: {},
-            type: "fluid",
-            url: `fluid-odsp://${siteURL}?${queryString}&version=null`,
-            siteUrl: siteURL,
-            hashedDocumentId: "",
-            driveId: driveID,
-            itemId: "",
-            fileName,
-            summarizer: false,
-            codeHint: {
-                containerPackageName: packageName ? packageName : undefined,
-            },
-        };
-    }
-    const { siteUrl, driveId, itemId, path, containerPackageName } = decodeOdspUrl(request.url);
-    const hashedDocumentId = getHashedDocumentId(driveId, itemId);
-    assert.ok(!hashedDocumentId.includes("/"), "Docid should not contain slashes!!");
-
-    let documentUrl = `fluid-odsp://placeholder/placeholder/${hashedDocumentId}/${removeBeginningSlash(path)}`;
-
-    if (request.url.length > 0) {
-        // In case of any additional parameters add them back to the url
-        const requestURL = new URL(request.url);
-        const searchParams = requestURL.search;
-        if (searchParams) {
-            documentUrl += searchParams;
-        }
-    }
-
-    const summarizer = request.headers?.[DriverHeader.summarizingClient];
-
-    return {
-        type: "fluid",
-        endpoints: {
-            snapshotStorageUrl: getSnapshotUrl(siteUrl, driveId, itemId),
-            attachmentPOSTStorageUrl: getAttachmentPOSTUrl(siteUrl, driveId, itemId),
-            attachmentGETStorageUrl: getAttachmentGETUrl(siteUrl, driveId, itemId),
-        },
-        tokens: {},
-        url: documentUrl,
-        hashedDocumentId,
-        siteUrl,
-        driveId,
-        itemId,
-        fileName: "",
-        summarizer,
-        codeHint: {
-            containerPackageName,
-        },
-    };
-}