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

fix(3.0.3): Prompt in editor for 401 error, fix profile propagation #3318

Merged
merged 9 commits into from
Nov 15, 2024
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
1 change: 1 addition & 0 deletions packages/zowe-explorer-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ All notable changes to the "zowe-explorer-api" extension will be documented in t
### Bug fixes

- Fixed an issue where the `responseTimeout` profile property was ignored for z/OSMF MVS and USS API calls. [#3225](https://github.com/zowe/zowe-explorer-vscode/issues/3225)
- Fixed an issue where the assignment of the `profile` property in `ZoweTreeNode.setProfileToChoice` caused references to that object to break elsewhere. [#3289](https://github.com/zowe/zowe-explorer-vscode/issues/3289)

## `3.0.2`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,17 @@ import * as vscode from "vscode";
import { ZoweTreeNode } from "../../../src/tree/ZoweTreeNode";
import { IZoweTreeNode } from "../../../src/tree/IZoweTreeNode";
import * as imperative from "@zowe/imperative";
import { BaseProvider } from "../../../src";

describe("ZoweTreeNode", () => {
const innerProfile = { user: "apple", password: "banana" };
const fakeProfile: imperative.IProfileLoaded = {
name: "amazingProfile",
profile: innerProfile,
message: "",
type: "zosmf",
failNotFound: true,
};

const makeNode = (
name: string,
collapseState: vscode.TreeItemCollapsibleState,
Expand Down Expand Up @@ -48,8 +56,8 @@ describe("ZoweTreeNode", () => {

it("getProfile should return profile of current node", () => {
const node = makeNode("test", vscode.TreeItemCollapsibleState.None, undefined);
node.setProfileToChoice("myProfile" as unknown as imperative.IProfileLoaded);
expect(node.getProfile()).toBe("myProfile");
node.setProfileToChoice(fakeProfile);
expect(node.getProfile().name).toBe("amazingProfile");
});

it("getProfile should return profile of parent node", () => {
Expand Down Expand Up @@ -83,49 +91,43 @@ describe("ZoweTreeNode", () => {

it("setProfileToChoice should update properties on existing profile object", () => {
const node = makeNode("test", vscode.TreeItemCollapsibleState.None, undefined, undefined, {
name: "oldProfile",
profile: { host: "example.com" },
...fakeProfile,
});
node.setProfileToChoice({ name: "newProfile", profile: { host: "example.com", port: 443 } } as unknown as imperative.IProfileLoaded);
// Profile name should not change but properties should
expect(node.getProfileName()).toBe("oldProfile");
node.setProfileToChoice({ ...fakeProfile, profile: { host: "example.com", port: 443 } });
expect(node.getProfile().profile?.port).toBeDefined();
});

it("setProfileToChoice should update profile for associated FSProvider entry", () => {
const node = makeNode("test", vscode.TreeItemCollapsibleState.None, undefined);
node.resourceUri = vscode.Uri.file(__dirname);
const prof = { ...fakeProfile, profile: { ...innerProfile } };
const fsEntry = {
metadata: {
profile: { name: "oldProfile" },
profile: prof,
},
};
node.setProfileToChoice(
{ name: "newProfile" } as unknown as imperative.IProfileLoaded,
{
lookup: jest.fn().mockReturnValue(fsEntry),
} as unknown as BaseProvider
);
expect(node.getProfileName()).toBe("newProfile");
expect(fsEntry.metadata.profile.name).toBe("newProfile");
prof.profile.user = "banana";
prof.profile.password = "apple";
node.setProfileToChoice(prof);
expect(node.getProfile().profile?.user).toBe("banana");
expect(node.getProfile().profile?.password).toBe("apple");
expect(fsEntry.metadata.profile.profile?.user).toBe("banana");
expect(fsEntry.metadata.profile.profile?.password).toBe("apple");
});

it("setProfileToChoice should update child nodes with the new profile", () => {
const node = makeNode("test", vscode.TreeItemCollapsibleState.Expanded, undefined);
node.setProfileToChoice({ ...fakeProfile, profile: { ...fakeProfile.profile, user: "banana" } });
const nodeChild = makeNode("child", vscode.TreeItemCollapsibleState.None, undefined);
nodeChild.setProfileToChoice(node.getProfile());
node.children = [nodeChild as any];
const setProfileToChoiceChildMock = jest.spyOn(nodeChild, "setProfileToChoice").mockImplementation();
const fsEntry = {
metadata: {
profile: { name: "oldProfile" },
profile: node.getProfile(),
},
};
const mockNewProfile = { name: "newProfile" } as unknown as imperative.IProfileLoaded;
const mockProvider = {
lookup: jest.fn().mockReturnValue(fsEntry),
} as unknown as BaseProvider;
node.setProfileToChoice(mockNewProfile, mockProvider);
expect(node.getProfileName()).toBe("newProfile");
expect(setProfileToChoiceChildMock).toHaveBeenCalledWith(mockNewProfile, mockProvider);
expect(node.getProfile().profile?.user).toBe("banana");
expect(nodeChild.getProfile().profile?.user).toBe("banana");
expect(fsEntry.metadata.profile.profile?.user).toBe("banana");
});
});
17 changes: 2 additions & 15 deletions packages/zowe-explorer-api/src/tree/ZoweTreeNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,21 +101,8 @@ export class ZoweTreeNode extends vscode.TreeItem {
* @param {imperative.IProfileLoaded} The profile you will set the node to use
*/
public setProfileToChoice(aProfile: imperative.IProfileLoaded, fsProvider?: BaseProvider): void {
if (this.profile == null) {
this.profile = aProfile;
} else {
// Don't reassign profile, we want to keep object reference shared across nodes
this.profile.profile = aProfile.profile;
}
if (this.resourceUri != null) {
const fsEntry = fsProvider?.lookup(this.resourceUri, true);
if (fsEntry != null) {
fsEntry.metadata.profile = aProfile;
}
}
for (const child of this.children) {
(child as unknown as ZoweTreeNode).setProfileToChoice(aProfile, fsProvider);
}
// Don't reassign profile if its already defined, as we want to keep the reference valid for other nodes and filesystems
this.profile = Object.assign(this.profile ?? {}, aProfile);
}
/**
* Sets the session for this node to the one chosen in parameters.
Expand Down
2 changes: 2 additions & 0 deletions packages/zowe-explorer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen
- Reduced the number of MVS API calls performed by `vscode.workspace.fs.readFile` when fetching the contents of a data set entry. [#3278](https://github.com/zowe/zowe-explorer-vscode/issues/3278)
- Fixed an issue to review inconsistent capitalization across translation strings. [#2935](https://github.com/zowe/zowe-explorer-vscode/issues/2935)
- Updated the test for the default credential manager for better compatibility with Cloud-based platforms such as Eclipse Che and Red Hat OpenShift Dev Spaces. [#3297](https://github.com/zowe/zowe-explorer-vscode/pull/3297)
- Fixed issue where users were not prompted to enter credentials if a 401 error was encountered when opening files, data sets or spools in the editor. [#3197](https://github.com/zowe/zowe-explorer-vscode/issues/3197)
- Fixed issue where profile credential updates or token changes were not reflected within the filesystem. [#3289](https://github.com/zowe/zowe-explorer-vscode/issues/3289)

## `3.0.2`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,7 @@ describe("Profiles Unit Tests - function checkCurrentProfile", () => {
jest.spyOn(AuthUtils, "isUsingTokenAuth").mockResolvedValueOnce(true);
environmentSetup(globalMocks);
setupProfilesCheck(globalMocks);
const ssoLoginSpy = jest.spyOn(Profiles.getInstance(), "ssoLogin").mockResolvedValueOnce();
const ssoLoginSpy = jest.spyOn(Profiles.getInstance(), "ssoLogin").mockResolvedValueOnce(true);
jest.spyOn(Profiles.getInstance(), "loadNamedProfile").mockReturnValueOnce(globalMocks.testProfile);
await expect(Profiles.getInstance().checkCurrentProfile(globalMocks.testProfile)).resolves.toEqual({ name: "sestest", status: "active" });
expect(ssoLoginSpy).toHaveBeenCalledTimes(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
*/

import { Disposable, FilePermission, FileType, Uri, window } from "vscode";
import { FsJobsUtils, FilterEntry, Gui, JobEntry, SpoolEntry, ZoweScheme } from "@zowe/zowe-explorer-api";
import { FsJobsUtils, FilterEntry, Gui, JobEntry, SpoolEntry, ZoweScheme, imperative } from "@zowe/zowe-explorer-api";
import { createIProfile } from "../../../__mocks__/mockCreators/shared";
import { createIJobFile, createIJobObject } from "../../../__mocks__/mockCreators/jobs";
import { ZoweExplorerApiRegister } from "../../../../src/extending/ZoweExplorerApiRegister";
import { JobFSProvider } from "../../../../src/trees/job/JobFSProvider";
import { MockedProperty } from "../../../__mocks__/mockUtils";
import { AuthUtils } from "../../../../src/utils/AuthUtils";

const testProfile = createIProfile();

Expand Down Expand Up @@ -222,6 +223,47 @@ describe("fetchSpoolAtUri", () => {
jesApiMock.mockRestore();
lookupAsFileMock.mockRestore();
});
it("fetches the spool contents for a given URI - getSpoolContentById", async () => {
const lookupAsFileMock = jest
.spyOn(JobFSProvider.instance as any, "_lookupAsFile")
.mockReturnValueOnce({ ...testEntries.spool, data: new Uint8Array() });
const lookupParentDirMock = jest.spyOn(JobFSProvider.instance as any, "_lookupParentDirectory").mockReturnValueOnce({ ...testEntries.job });
const mockJesApi = {
getSpoolContentById: jest.fn((opts) => {
return "spool contents";
}),
};
const jesApiMock = jest.spyOn(ZoweExplorerApiRegister, "getJesApi").mockReturnValueOnce(mockJesApi as any);
const entry = await JobFSProvider.instance.fetchSpoolAtUri(testUris.spool);
expect(lookupAsFileMock).toHaveBeenCalledWith(testUris.spool);
expect(lookupParentDirMock).toHaveBeenCalledWith(testUris.spool);
expect(mockJesApi.getSpoolContentById).toHaveBeenCalled();
expect(entry.data.toString()).toStrictEqual("spool contents");
jesApiMock.mockRestore();
lookupAsFileMock.mockRestore();
});

it("calls AuthUtils.promptForAuthError when an error occurs", async () => {
const lookupAsFileMock = jest
.spyOn(JobFSProvider.instance as any, "_lookupAsFile")
.mockReturnValueOnce({ ...testEntries.spool, data: new Uint8Array() });
const mockJesApi = {
downloadSingleSpool: jest.fn((opts) => {
throw new imperative.ImperativeError({
msg: "Failed to download spool",
errorCode: "401"
});
}),
};
const promptForAuthErrorMock = jest.spyOn(AuthUtils, "promptForAuthError").mockImplementation();
const jesApiMock = jest.spyOn(ZoweExplorerApiRegister, "getJesApi").mockReturnValueOnce(mockJesApi as any);
await expect(JobFSProvider.instance.fetchSpoolAtUri(testUris.spool)).rejects.toThrow();
expect(promptForAuthErrorMock).toHaveBeenCalled();
expect(lookupAsFileMock).toHaveBeenCalledWith(testUris.spool);
expect(mockJesApi.downloadSingleSpool).toHaveBeenCalled();
jesApiMock.mockRestore();
lookupAsFileMock.mockRestore();
});
});

describe("readFile", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,11 @@ describe("ZosJobsProvider - Function searchPrompt", () => {
const globalMocks = await createGlobalMocks();
jest.spyOn(globalMocks.testJobsProvider, "applySavedFavoritesSearchLabel").mockReturnValue(undefined);
const applySearchLabelToNode = jest.spyOn(globalMocks.testJobsProvider, "applySearchLabelToNode");
const jobSessionNode = new ZoweJobNode({ label: "sestest", collapsibleState: vscode.TreeItemCollapsibleState.Collapsed });
const jobSessionNode = new ZoweJobNode({
label: "sestest",
collapsibleState: vscode.TreeItemCollapsibleState.Collapsed,
profile: createIProfile(),
});
jobSessionNode.contextValue = Constants.JOBS_SESSION_CONTEXT + Constants.FAV_SUFFIX;
await globalMocks.testJobsProvider.searchPrompt(jobSessionNode);
expect(applySearchLabelToNode).toHaveBeenCalled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
*/

import { Disposable, FilePermission, FileSystemError, FileType, TextEditor, Uri, workspace } from "vscode";
import { BaseProvider, DirEntry, FileEntry, Gui, UssDirectory, UssFile, ZoweScheme } from "@zowe/zowe-explorer-api";
import { BaseProvider, DirEntry, FileEntry, Gui, imperative, UssDirectory, UssFile, ZoweScheme } from "@zowe/zowe-explorer-api";
import { Profiles } from "../../../../src/configuration/Profiles";
import { createIProfile } from "../../../__mocks__/mockCreators/shared";
import { ZoweExplorerApiRegister } from "../../../../src/extending/ZoweExplorerApiRegister";
import { UssFSProvider } from "../../../../src/trees/uss/UssFSProvider";
import { USSFileStructure } from "../../../../src/trees/uss/USSFileStructure";
import { AuthUtils } from "../../../../src/utils/AuthUtils";

const testProfile = createIProfile();

Expand Down Expand Up @@ -322,6 +323,28 @@ describe("fetchFileAtUri", () => {
expect(fileEntry.data?.byteLength).toBe(exampleData.length);
autoDetectEncodingMock.mockRestore();
});
it("returns early if it failed to fetch contents", async () => {
const fileEntry = { ...testEntries.file };
fileEntry.wasAccessed = false;
const _fireSoonSpy = jest.spyOn((UssFSProvider as any).prototype, "_fireSoon");
const lookupAsFileMock = jest.spyOn((UssFSProvider as any).prototype, "_lookupAsFile").mockReturnValueOnce(fileEntry);
const autoDetectEncodingMock = jest.spyOn(UssFSProvider.instance, "autoDetectEncoding").mockImplementation();
const ussApiMock = jest.spyOn(ZoweExplorerApiRegister, "getUssApi").mockReturnValueOnce({
getContents: jest.fn().mockRejectedValue(new imperative.ImperativeError({ msg: "Error fetching contents" })),
} as any);
const promptForAuthErrorMock = jest.spyOn(AuthUtils, "promptForAuthError").mockImplementation();

await expect(UssFSProvider.instance.fetchFileAtUri(testUris.file)).resolves.not.toThrow();

expect(lookupAsFileMock).toHaveBeenCalledWith(testUris.file);
expect(promptForAuthErrorMock).toHaveBeenCalled();
expect(autoDetectEncodingMock).toHaveBeenCalledWith(fileEntry);
expect(_fireSoonSpy).not.toHaveBeenCalled();
autoDetectEncodingMock.mockRestore();
promptForAuthErrorMock.mockRestore();
ussApiMock.mockRestore();
lookupAsFileMock.mockRestore();
});
it("calls getContents to get the data for a file entry with encoding", async () => {
const fileEntry = { ...testEntries.file };
const lookupAsFileMock = jest.spyOn((UssFSProvider as any).prototype, "_lookupAsFile").mockReturnValueOnce(fileEntry);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* This program and the accompanying materials are made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-v20.html
*
* SPDX-License-Identifier: EPL-2.0
*
* Copyright Contributors to the Zowe Project.
*
*/

import { imperative } from "@zowe/zowe-explorer-api";
import { AuthUtils } from "../../../src/utils/AuthUtils";

describe("AuthUtils", () => {
describe("promptForAuthError", () => {
it("should prompt for authentication", async () => {
const errorDetails = new imperative.ImperativeError({
errorCode: 401 as unknown as string,
msg: "All configured authentication methods failed",
});
const profile = { type: "zosmf" } as any;
const promptForAuthenticationMock = jest
.spyOn(AuthUtils, "promptForAuthentication")
.mockImplementation(async () => Promise.resolve(true));
AuthUtils.promptForAuthError(errorDetails, profile);
expect(promptForAuthenticationMock).toHaveBeenCalledWith(errorDetails, profile);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,6 @@ describe("ProfilesUtils unit tests", () => {
await AuthUtils.errorHandling(errorDetails, label, moreInfo);
expect(showErrorSpy).toHaveBeenCalledTimes(1);
expect(promptCredentialsSpy).not.toHaveBeenCalled();
expect(showMsgSpy).toHaveBeenCalledWith("Operation Cancelled");
showErrorSpy.mockClear();
showMsgSpy.mockClear();
promptCredentialsSpy.mockClear();
Expand Down
Loading
Loading