From 4fefdf934c10a50d65149b5d8a8770b4ee5e63fd Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Thu, 23 Jan 2025 15:37:37 -0500 Subject: [PATCH] fix: call Mutex.waitForUnlock instead of locking profile before request (#3409) * fix: call Mutex.waitForUnlock instead of locking profile before request Signed-off-by: Trae Yelovich * fix: AuthHandler.waitForUnlock Signed-off-by: Trae Yelovich * tests: AuthHandler.waitForUnlock Signed-off-by: Trae Yelovich * fix: add true opt-in support for profile locks w/ extender types Signed-off-by: Trae Yelovich --------- Signed-off-by: Trae Yelovich --- .../profiles/AuthHandler.unit.test.ts | 44 +++++++++++++ .../src/profiles/AuthHandler.ts | 65 +++++++++++++++---- .../src/trees/dataset/DatasetFSProvider.ts | 6 +- .../src/trees/job/JobFSProvider.ts | 3 +- .../src/trees/uss/UssFSProvider.ts | 6 +- 5 files changed, 102 insertions(+), 22 deletions(-) diff --git a/packages/zowe-explorer-api/__tests__/__unit__/profiles/AuthHandler.unit.test.ts b/packages/zowe-explorer-api/__tests__/__unit__/profiles/AuthHandler.unit.test.ts index 1df9768d0..857dc62ec 100644 --- a/packages/zowe-explorer-api/__tests__/__unit__/profiles/AuthHandler.unit.test.ts +++ b/packages/zowe-explorer-api/__tests__/__unit__/profiles/AuthHandler.unit.test.ts @@ -17,6 +17,39 @@ import { AuthPromptParams } from "@zowe/zowe-explorer-api"; const TEST_PROFILE_NAME = "lpar.zosmf"; +describe("AuthHandler.disableLocksForType", () => { + it("removes the profile type from the list of profile types w/ locks enabled", () => { + AuthHandler.disableLocksForType("zosmf"); + expect((AuthHandler as any).enabledProfileTypes.has("zosmf")).toBe(false); + }); +}); + +describe("AuthHandler.enableLocksForType", () => { + it("adds the profile type to the list of profile types w/ locks enabled", () => { + AuthHandler.enableLocksForType("sample-type"); + expect((AuthHandler as any).enabledProfileTypes.has("sample-type")).toBe(true); + + // cleanup for other tests + (AuthHandler as any).enabledProfileTypes.delete("sample-type"); + }); +}); + +describe("AuthHandler.waitForUnlock", () => { + it("calls Mutex.waitForUnlock if the profile lock is present", async () => { + const mutex = new Mutex(); + const waitForUnlockMock = jest.spyOn(mutex, "waitForUnlock"); + (AuthHandler as any).profileLocks.set(TEST_PROFILE_NAME, mutex); + await AuthHandler.waitForUnlock(TEST_PROFILE_NAME); + expect(waitForUnlockMock).toHaveBeenCalled(); + (AuthHandler as any).profileLocks.clear(); + }); + it("does nothing if the profile lock is not in the profileLocks map", async () => { + const waitForUnlockMock = jest.spyOn(Mutex.prototype, "waitForUnlock"); + await AuthHandler.waitForUnlock(TEST_PROFILE_NAME); + expect(waitForUnlockMock).not.toHaveBeenCalled(); + }); +}); + describe("AuthHandler.isProfileLocked", () => { it("returns true if the profile is locked", async () => { await AuthHandler.lockProfile(TEST_PROFILE_NAME); @@ -34,6 +67,17 @@ describe("AuthHandler.isProfileLocked", () => { }); describe("AuthHandler.lockProfile", () => { + it("does not acquire a Mutex if the profile type doesn't have locks enabled", async () => { + const acquireMutex = jest.spyOn(Mutex.prototype, "acquire"); + await AuthHandler.lockProfile({ + profile: {}, + type: "sample-type", + message: "", + failNotFound: false, + }); + expect(acquireMutex).not.toHaveBeenCalled(); + }); + it("assigns and acquires a Mutex to the profile in the profile map", async () => { await AuthHandler.lockProfile(TEST_PROFILE_NAME); expect((AuthHandler as any).profileLocks.has(TEST_PROFILE_NAME)).toBe(true); diff --git a/packages/zowe-explorer-api/src/profiles/AuthHandler.ts b/packages/zowe-explorer-api/src/profiles/AuthHandler.ts index 28ef53cab..4fc2e0742 100644 --- a/packages/zowe-explorer-api/src/profiles/AuthHandler.ts +++ b/packages/zowe-explorer-api/src/profiles/AuthHandler.ts @@ -40,6 +40,23 @@ export type AuthPromptParams = { export type ProfileLike = string | imperative.IProfileLoaded; export class AuthHandler { private static profileLocks: Map = new Map(); + private static enabledProfileTypes: Set = new Set(["zosmf"]); + + /** + * Enables profile locks for the given type. + * @param type The profile type to enable locks for. + */ + public static enableLocksForType(type: string): void { + this.enabledProfileTypes.add(type); + } + + /** + * Disables profile locks for the given type. + * @param type The profile type to disable locks for. + */ + public static disableLocksForType(type: string): void { + this.enabledProfileTypes.delete(type); + } /** * Function that checks whether a profile is using token based authentication @@ -61,7 +78,7 @@ export class AuthHandler { * @param refreshResources {boolean} Whether to refresh high-priority resources (active editor & virtual workspace) after unlocking */ public static unlockProfile(profile: ProfileLike, refreshResources?: boolean): void { - const profileName = typeof profile === "string" ? profile : profile.name; + const profileName = AuthHandler.getProfileName(profile); const mutex = this.profileLocks.get(profileName); // If a mutex doesn't exist for this profile or the mutex is no longer locked, return if (mutex == null || !mutex.isLocked()) { @@ -90,7 +107,7 @@ export class AuthHandler { * @returns {boolean} Whether authentication was successful */ public static async promptForAuthentication(profile: ProfileLike, params: AuthPromptParams): Promise { - const profileName = typeof profile === "string" ? profile : profile.name; + const profileName = AuthHandler.getProfileName(profile); if (params.imperativeError.mDetails.additionalDetails) { const tokenError: string = params.imperativeError.mDetails.additionalDetails; if (tokenError.includes("Token is not valid or expired.") || params.isUsingTokenAuth) { @@ -137,33 +154,57 @@ export class AuthHandler { * @returns Whether the profile was successfully locked */ public static async lockProfile(profile: ProfileLike, authOpts?: AuthPromptParams): Promise { - const profileName = typeof profile === "string" ? profile : profile.name; + const profileName = AuthHandler.getProfileName(profile); - // If the mutex does not exist, make one for the profile and acquire the lock - if (!this.profileLocks.has(profileName)) { - this.profileLocks.set(profileName, new Mutex()); - } + // Only create a lock for the profile when we can determine the profile type and the profile type has locks enabled + if (!AuthHandler.isProfileLoaded(profile) || this.enabledProfileTypes.has(profile.type)) { + // If the mutex does not exist, make one for the profile and acquire the lock + if (!this.profileLocks.has(profileName)) { + this.profileLocks.set(profileName, new Mutex()); + } - // Attempt to acquire the lock - const mutex = this.profileLocks.get(profileName); - await mutex.acquire(); + // Attempt to acquire the lock - only lock the mutex if the profile type has locks enabled + const mutex = this.profileLocks.get(profileName); + await mutex.acquire(); + } // Prompt the user to re-authenticate if an error and options were provided if (authOpts) { await AuthHandler.promptForAuthentication(profile, authOpts); - mutex.release(); + this.profileLocks.get(profileName)?.release(); } return true; } + private static isProfileLoaded(profile: ProfileLike): profile is imperative.IProfileLoaded { + return typeof profile !== "string"; + } + + private static getProfileName(profile: ProfileLike): string { + return typeof profile === "string" ? profile : profile.name; + } + + /** + * Waits for the profile to be unlocked (ONLY if the profile was locked after an authentication error) + * @param profile The profile name or object that may be locked + */ + public static async waitForUnlock(profile: ProfileLike): Promise { + const profileName = AuthHandler.getProfileName(profile); + if (!this.profileLocks.has(profileName)) { + return; + } + + return this.profileLocks.get(profileName)?.waitForUnlock(); + } + /** * Checks whether the given profile has its lock acquired. * @param profile The profile to check * @returns {boolean} `true` if the given profile is locked, `false` otherwise */ public static isProfileLocked(profile: ProfileLike): boolean { - const mutex = this.profileLocks.get(typeof profile === "string" ? profile : profile.name); + const mutex = this.profileLocks.get(AuthHandler.getProfileName(profile)); if (mutex == null) { return false; } diff --git a/packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts b/packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts index 36fb164dd..02922a5b5 100644 --- a/packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts +++ b/packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts @@ -193,9 +193,8 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem private async fetchEntriesForDataset(entry: PdsEntry, uri: vscode.Uri, uriInfo: UriFsInfo): Promise { let members: IZosFilesResponse; try { - await AuthHandler.lockProfile(uriInfo.profile); + await AuthHandler.waitForUnlock(entry.metadata.profile); members = await ZoweExplorerApiRegister.getMvsApi(uriInfo.profile).allMembers(path.posix.basename(uri.path)); - AuthHandler.unlockProfile(uriInfo.profile); } catch (err) { await AuthUtils.handleProfileAuthOnError(err, uriInfo.profile); throw err; @@ -382,7 +381,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem const metadata = dsEntry?.metadata ?? this._getInfoFromUri(uri); const profileEncoding = dsEntry?.encoding ? null : dsEntry?.metadata.profile.profile?.encoding; try { - await AuthHandler.lockProfile(metadata.profile); + await AuthHandler.waitForUnlock(metadata.profile); const resp = await ZoweExplorerApiRegister.getMvsApi(metadata.profile).getContents(metadata.dsName, { binary: dsEntry?.encoding?.kind === "binary", encoding: dsEntry?.encoding?.kind === "other" ? dsEntry?.encoding.codepage : profileEncoding, @@ -390,7 +389,6 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem returnEtag: true, stream: bufBuilder, }); - AuthHandler.unlockProfile(metadata.profile); const data: Uint8Array = bufBuilder.read() ?? new Uint8Array(); //if an entry does not exist for the dataset, create it if (!dsEntry) { diff --git a/packages/zowe-explorer/src/trees/job/JobFSProvider.ts b/packages/zowe-explorer/src/trees/job/JobFSProvider.ts index 0b0c47613..db367d5e9 100644 --- a/packages/zowe-explorer/src/trees/job/JobFSProvider.ts +++ b/packages/zowe-explorer/src/trees/job/JobFSProvider.ts @@ -208,7 +208,7 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv const bufBuilder = new BufferBuilder(); const jesApi = ZoweExplorerApiRegister.getJesApi(spoolEntry.metadata.profile); - await AuthHandler.lockProfile(spoolEntry.metadata.profile); + await AuthHandler.waitForUnlock(spoolEntry.metadata.profile); try { if (jesApi.downloadSingleSpool) { const spoolDownloadObject: IDownloadSpoolContentParms = { @@ -232,7 +232,6 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv await AuthUtils.handleProfileAuthOnError(err, spoolEntry.metadata.profile); throw err; } - AuthHandler.unlockProfile(spoolEntry.metadata.profile); this._fireSoon({ type: vscode.FileChangeType.Changed, uri }); spoolEntry.data = bufBuilder.read() ?? new Uint8Array(); diff --git a/packages/zowe-explorer/src/trees/uss/UssFSProvider.ts b/packages/zowe-explorer/src/trees/uss/UssFSProvider.ts index 795d8110a..389e5ab08 100644 --- a/packages/zowe-explorer/src/trees/uss/UssFSProvider.ts +++ b/packages/zowe-explorer/src/trees/uss/UssFSProvider.ts @@ -280,7 +280,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv try { await this.autoDetectEncoding(file as UssFile); const profileEncoding = file.encoding ? null : file.metadata.profile.profile?.encoding; - await AuthHandler.lockProfile(metadata.profile); + await AuthHandler.waitForUnlock(file.metadata.profile); resp = await ZoweExplorerApiRegister.getUssApi(metadata.profile).getContents(filePath, { binary: file.encoding?.kind === "binary", encoding: file.encoding?.kind === "other" ? file.encoding.codepage : profileEncoding, @@ -288,7 +288,6 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv returnEtag: true, stream: bufBuilder, }); - AuthHandler.unlockProfile(metadata.profile); } catch (err) { if (err instanceof Error) { ZoweLogger.error(err.message); @@ -324,7 +323,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv if (entry.encoding !== undefined) { return; } - await AuthHandler.lockProfile(entry.metadata.profile); + await AuthHandler.waitForUnlock(entry.metadata.profile); const ussApi = ZoweExplorerApiRegister.getUssApi(entry.metadata.profile); if (ussApi.getTag != null) { const taggedEncoding = await ussApi.getTag(entry.metadata.path); @@ -337,7 +336,6 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv const isBinary = await ussApi.isFileTagBinOrAscii(entry.metadata.path); entry.encoding = isBinary ? { kind: "binary" } : undefined; } - AuthHandler.unlockProfile(entry.metadata.profile); } public async fetchEncodingForUri(uri: vscode.Uri): Promise {