Skip to content

Commit

Permalink
Remove duplicate implementation from toInstrumentedOdspTokenFetcher (…
Browse files Browse the repository at this point in the history 
…toInstrumentedTokenFetcher) (#9375)

* Remove duplicate toInstrumentedTokenFetcher implementation (duplicate from toInstrumentedOdspTokenFetcher )
  • Loading branch information
@NicholasCouri
NicholasCouri committed Mar 8, 2022
1 parent 592f870 commit 1f8c2bb
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 41 deletions.
29 changes: 23 additions & 6 deletions packages/drivers/odsp-driver/src/getFileLink.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ import {
OdspResourceTokenFetchOptions,
IdentityType,
TokenFetcher,
tokenFromResponse,
} from "@fluidframework/odsp-driver-definitions";
import { getUrlAndHeadersWithAuth } from "./getUrlAndHeadersWithAuth";
import { fetchHelper, getWithRetryForTokenRefresh } from "./odspUtils";
import { fetchHelper, getWithRetryForTokenRefresh, toInstrumentedOdspTokenFetcher } from "./odspUtils";
import { pkgVersion as driverVersion } from "./packageVersion";

// Store cached responses for the lifetime of web session as file link remains the same for given file item
Expand Down Expand Up @@ -101,12 +100,21 @@ async function getFileLinkCore(
let additionalProps;
const fileLink = await getWithRetryForTokenRefresh(async (options) => {
attempts++;
const token = await getToken({ ...options, ... odspUrlParts });
const storageTokenFetcher = toInstrumentedOdspTokenFetcher(
logger,
odspUrlParts,
getToken,
true /* throwOnNullToken */,
);
const storageToken = await storageTokenFetcher(options,"GetFileLinkCore");
assert(storageToken !== null,
"Instrumented token fetcher with throwOnNullToken = true should never return null");

const { url, headers } = getUrlAndHeadersWithAuth(
`${odspUrlParts.siteUrl}/_api/web/GetFileByUrl(@a1)/ListItemAllFields/GetSharingInformation?@a1=${
encodeURIComponent(`'${fileItem.webDavUrl}'`)
}`,
tokenFromResponse(token),
storageToken,
false,
);
const requestInit = {
Expand Down Expand Up @@ -167,10 +175,19 @@ async function getFileItemLite(
const fileItem = await getWithRetryForTokenRefresh(async (options) => {
attempts++;
const {siteUrl, driveId, itemId} = odspUrlParts;
const token = await getToken({ ...options, siteUrl, driveId, itemId});
const storageTokenFetcher = toInstrumentedOdspTokenFetcher(
logger,
odspUrlParts,
getToken,
true /* throwOnNullToken */,
);
const storageToken = await storageTokenFetcher(options,"GetFileItemLite");
assert(storageToken !== null,
"Instrumented token fetcher with throwOnNullToken =true should never return null");

const { url, headers } = getUrlAndHeadersWithAuth(
`${siteUrl}/_api/v2.0/drives/${driveId}/items/${itemId}?select=webUrl,webDavUrl`,
tokenFromResponse(token),
storageToken,
forceAccessTokenViaAuthorizationHeader,
);
const requestInit = { method: "GET", headers };
Expand Down
17 changes: 14 additions & 3 deletions packages/drivers/odsp-driver/src/odspDocumentServiceFactoryCore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
IPersistedCache,
HostStoragePolicy,
IFileEntry,
IOdspUrlParts,
} from "@fluidframework/odsp-driver-definitions";
import { v4 as uuid } from "uuid";
import {
Expand Down Expand Up @@ -57,6 +58,11 @@ export class OdspDocumentServiceFactoryCore implements IDocumentServiceFactory {
ensureFluidResolvedUrl(createNewResolvedUrl);

let odspResolvedUrl = getOdspResolvedUrl(createNewResolvedUrl);
const resolvedUrlData: IOdspUrlParts = {
siteUrl: odspResolvedUrl.siteUrl,
driveId: odspResolvedUrl.driveId,
itemId: odspResolvedUrl.itemId,
};
const [, queryString] = odspResolvedUrl.url.split("?");

const searchParams = new URLSearchParams(queryString);
Expand Down Expand Up @@ -94,7 +100,7 @@ export class OdspDocumentServiceFactoryCore implements IDocumentServiceFactory {
odspResolvedUrl = await createNewFluidFile(
toInstrumentedOdspTokenFetcher(
odspLogger,
odspResolvedUrl,
resolvedUrlData,
this.getStorageToken,
true /* throwOnNullToken */,
),
Expand Down Expand Up @@ -150,6 +156,11 @@ export class OdspDocumentServiceFactoryCore implements IDocumentServiceFactory {
cacheAndTrackerArg?: ICacheAndTracker,
): Promise<IDocumentService> {
const odspResolvedUrl = getOdspResolvedUrl(resolvedUrl);
const resolvedUrlData: IOdspUrlParts = {
siteUrl: odspResolvedUrl.siteUrl,
driveId: odspResolvedUrl.driveId,
itemId: odspResolvedUrl.itemId,
};
const cacheAndTracker = cacheAndTrackerArg ?? createOdspCacheAndTracker(
this.persistedCache,
this.nonPersistentCache,
Expand All @@ -158,7 +169,7 @@ export class OdspDocumentServiceFactoryCore implements IDocumentServiceFactory {

const storageTokenFetcher = toInstrumentedOdspTokenFetcher(
odspLogger,
odspResolvedUrl,
resolvedUrlData,
this.getStorageToken,
true /* throwOnNullToken */,
);
Expand All @@ -167,7 +178,7 @@ export class OdspDocumentServiceFactoryCore implements IDocumentServiceFactory {
? undefined
: async (options: TokenFetchOptions) => toInstrumentedOdspTokenFetcher(
odspLogger,
odspResolvedUrl,
resolvedUrlData,
this.getWebsocketToken!,
false /* throwOnNullToken */,
)(options, "GetWebsocketToken");
Expand Down
28 changes: 1 addition & 27 deletions packages/drivers/odsp-driver/src/odspDriverUrlResolverForShareLink.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ import {
IUrlResolver,
} from "@fluidframework/driver-definitions";
import { ITelemetryBaseLogger, ITelemetryLogger } from "@fluidframework/common-definitions";
import { NonRetryableError } from "@fluidframework/driver-utils";
import { PerformanceEvent } from "@fluidframework/telemetry-utils";
import {
IOdspResolvedUrl,
IdentityType,
isTokenFromCache,
OdspResourceTokenFetchOptions,
TokenFetcher,
OdspErrorType,
} from "@fluidframework/odsp-driver-definitions";
import {
getLocatorFromOdspUrl,
Expand All @@ -32,7 +28,6 @@ import { createOdspUrl } from "./createOdspUrl";
import { OdspDriverUrlResolver } from "./odspDriverUrlResolver";
import { getOdspResolvedUrl, createOdspLogger } from "./odspUtils";
import { getFileLink } from "./getFileLink";
import { pkgVersion as driverVersion } from "./packageVersion";

/**
* Properties passed to the code responsible for fetching share link for a file.
Expand Down Expand Up @@ -77,7 +72,7 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
if (shareLinkFetcherProps) {
this.shareLinkFetcherProps = {
...shareLinkFetcherProps,
tokenFetcher: this.toInstrumentedTokenFetcher(this.logger, shareLinkFetcherProps.tokenFetcher),
tokenFetcher: shareLinkFetcherProps.tokenFetcher,
};
}
}
Expand Down Expand Up @@ -157,27 +152,6 @@ export class OdspDriverUrlResolverForShareLink implements IUrlResolver {
return url.href;
}

private toInstrumentedTokenFetcher(
logger: ITelemetryLogger,
tokenFetcher: TokenFetcher<OdspResourceTokenFetchOptions>,
): TokenFetcher<OdspResourceTokenFetchOptions> {
return async (options: OdspResourceTokenFetchOptions) => {
return PerformanceEvent.timedExecAsync(
logger,
{ eventName: "GetSharingLinkToken" },
async (event) => tokenFetcher(options).then((tokenResponse) => {
if (tokenResponse === null) {
throw new NonRetryableError(
"Token callback returned null for share link",
OdspErrorType.fetchTokenError,
{ driverVersion });
}
event.end({ fromCache: isTokenFromCache(tokenResponse) });
return tokenResponse;
}));
};
}

private async getShareLinkPromise(resolvedUrl: IOdspResolvedUrl): Promise<string> {
if (this.shareLinkFetcherProps === undefined) {
throw new Error("Failed to get share link because share link fetcher props are missing");
Expand Down
7 changes: 3 additions & 4 deletions packages/drivers/odsp-driver/src/odspUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
ICacheEntry,
snapshotKey,
InstrumentedStorageTokenFetcher,
IOdspUrlParts,
} from "@fluidframework/odsp-driver-definitions";
import { fetch } from "./fetch";
import { pkgVersion as driverVersion } from "./packageVersion";
Expand Down Expand Up @@ -271,7 +272,7 @@ export function evalBlobsAndTrees(snapshot: IOdspSnapshot) {

export function toInstrumentedOdspTokenFetcher(
logger: ITelemetryLogger,
resolvedUrl: IOdspResolvedUrl,
resolvedUrlParts: IOdspUrlParts,
tokenFetcher: TokenFetcher<OdspResourceTokenFetchOptions>,
throwOnNullToken: boolean,
): InstrumentedStorageTokenFetcher {
Expand All @@ -290,9 +291,7 @@ export function toInstrumentedOdspTokenFetcher(
},
async (event) => tokenFetcher({
...options,
siteUrl: resolvedUrl.siteUrl,
driveId: resolvedUrl.driveId,
itemId: resolvedUrl.itemId,
...resolvedUrlParts,
}).then((tokenResponse) => {
const token = tokenFromResponse(tokenResponse);
// This event alone generates so many events that is materially impacts cost of telemetry
Expand Down
8 changes: 7 additions & 1 deletion packages/drivers/odsp-driver/src/prefetchLatestSnapshot.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
ISnapshotOptions,
OdspResourceTokenFetchOptions,
TokenFetcher,
IOdspUrlParts,
} from "@fluidframework/odsp-driver-definitions";
import { ChildLogger, PerformanceEvent } from "@fluidframework/telemetry-utils";
import {
Expand Down Expand Up @@ -52,9 +53,14 @@ export async function prefetchLatestSnapshot(
const odspLogger = createOdspLogger(ChildLogger.create(logger, "PrefetchSnapshot"));
const odspResolvedUrl = getOdspResolvedUrl(resolvedUrl);

const resolvedUrlData: IOdspUrlParts = {
siteUrl: odspResolvedUrl.siteUrl,
driveId: odspResolvedUrl.driveId,
itemId: odspResolvedUrl.itemId,
};
const storageTokenFetcher = toInstrumentedOdspTokenFetcher(
odspLogger,
odspResolvedUrl,
resolvedUrlData,
getStorageToken,
true /* throwOnNullToken */,
);
Expand Down

0 comments on commit 1f8c2bb

Please sign in to comment.