Skip to content

Commit

Permalink
fix: call Mutex.waitForUnlock instead of locking profile before reque…
Browse files Browse the repository at this point in the history
…st (#3409)

* fix: call Mutex.waitForUnlock instead of locking profile before request

Signed-off-by: Trae Yelovich <[email protected]>

* fix: AuthHandler.waitForUnlock

Signed-off-by: Trae Yelovich <[email protected]>

* tests: AuthHandler.waitForUnlock

Signed-off-by: Trae Yelovich <[email protected]>

* fix: add true opt-in support for profile locks w/ extender types

Signed-off-by: Trae Yelovich <[email protected]>

---------

Signed-off-by: Trae Yelovich <[email protected]>
  • Loading branch information
traeok authored Jan 23, 2025
1 parent b95e1a1 commit 4fefdf9
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
65 changes: 53 additions & 12 deletions packages/zowe-explorer-api/src/profiles/AuthHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,23 @@ export type AuthPromptParams = {
export type ProfileLike = string | imperative.IProfileLoaded;
export class AuthHandler {
private static profileLocks: Map<string, Mutex> = new Map();
private static enabledProfileTypes: Set<string> = 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
Expand All @@ -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()) {
Expand Down Expand Up @@ -90,7 +107,7 @@ export class AuthHandler {
* @returns {boolean} Whether authentication was successful
*/
public static async promptForAuthentication(profile: ProfileLike, params: AuthPromptParams): Promise<boolean> {
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) {
Expand Down Expand Up @@ -137,33 +154,57 @@ export class AuthHandler {
* @returns Whether the profile was successfully locked
*/
public static async lockProfile(profile: ProfileLike, authOpts?: AuthPromptParams): Promise<boolean> {
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<void> {
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;
}
Expand Down
6 changes: 2 additions & 4 deletions packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,8 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
private async fetchEntriesForDataset(entry: PdsEntry, uri: vscode.Uri, uriInfo: UriFsInfo): Promise<void> {
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;
Expand Down Expand Up @@ -382,15 +381,14 @@ 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,
responseTimeout: metadata.profile.profile?.responseTimeout,
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) {
Expand Down
3 changes: 1 addition & 2 deletions packages/zowe-explorer/src/trees/job/JobFSProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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();
Expand Down
6 changes: 2 additions & 4 deletions packages/zowe-explorer/src/trees/uss/UssFSProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,14 @@ 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,
responseTimeout: metadata.profile.profile?.responseTimeout,
returnEtag: true,
stream: bufBuilder,
});
AuthHandler.unlockProfile(metadata.profile);
} catch (err) {
if (err instanceof Error) {
ZoweLogger.error(err.message);
Expand Down Expand Up @@ -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);
Expand All @@ -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<ZosEncoding> {
Expand Down

0 comments on commit 4fefdf9

Please sign in to comment.