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 6 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 @@ -16,6 +16,7 @@ import { createIJobFile, createIJobObject } from "../../../__mocks__/mockCreator
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,44 @@ 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 Error("Failed to download spool");
}),
};
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 @@ -16,6 +16,7 @@ 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,27 @@ 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 };
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 Error("error retrieving 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,19 @@
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
54 changes: 27 additions & 27 deletions packages/zowe-explorer/l10n/bundle.l10n.json
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@
"Enter the path to the certificate key for authenticating the connection.": "Enter the path to the certificate key for authenticating the connection.",
"Certificate Keys": "Certificate Keys",
"Select Certificate Key": "Select Certificate Key",
"Required parameter 'host' must not be blank.": "Required parameter 'host' must not be blank.",
"Invalid Credentials for profile '{0}'. Please ensure the username and password are valid or this may lead to a lock-out./Label": {
"message": "Invalid Credentials for profile '{0}'. Please ensure the username and password are valid or this may lead to a lock-out.",
"comment": [
Expand All @@ -172,6 +171,7 @@
]
},
"Update Credentials": "Update Credentials",
"Required parameter 'host' must not be blank.": "Required parameter 'host' must not be blank.",
"Profile Name {0} is inactive. Please check if your Zowe server is active or if the URL and port in your profile is correct./Profile name": {
"message": "Profile Name {0} is inactive. Please check if your Zowe server is active or if the URL and port in your profile is correct.",
"comment": [
Expand Down Expand Up @@ -208,32 +208,6 @@
"Profile auth error": "Profile auth error",
"Profile is not authenticated, please log in to continue": "Profile is not authenticated, please log in to continue",
"Retrieving response from USS list API": "Retrieving response from USS list API",
"The 'move' function is not implemented for this USS API.": "The 'move' function is not implemented for this USS API.",
"Could not list USS files: Empty path provided in URI": "Could not list USS files: Empty path provided in URI",
"Profile does not exist for this file.": "Profile does not exist for this file.",
"Saving USS file...": "Saving USS file...",
"Renaming {0} failed due to API error: {1}/File pathError message": {
"message": "Renaming {0} failed due to API error: {1}",
"comment": [
"File path",
"Error message"
]
},
"Deleting {0} failed due to API error: {1}/File nameError message": {
"message": "Deleting {0} failed due to API error: {1}",
"comment": [
"File name",
"Error message"
]
},
"No error details given": "No error details given",
"Error fetching destination {0} for paste action: {1}/USS pathError message": {
"message": "Error fetching destination {0} for paste action: {1}",
"comment": [
"USS path",
"Error message"
]
},
"Downloaded: {0}/Download time": {
"message": "Downloaded: {0}",
"comment": [
Expand Down Expand Up @@ -304,6 +278,32 @@
"initializeUSSFavorites.error.buttonRemove": "initializeUSSFavorites.error.buttonRemove",
"File does not exist. It may have been deleted.": "File does not exist. It may have been deleted.",
"Pulling from Mainframe...": "Pulling from Mainframe...",
"The 'move' function is not implemented for this USS API.": "The 'move' function is not implemented for this USS API.",
"Could not list USS files: Empty path provided in URI": "Could not list USS files: Empty path provided in URI",
"Profile does not exist for this file.": "Profile does not exist for this file.",
"Saving USS file...": "Saving USS file...",
"Renaming {0} failed due to API error: {1}/File pathError message": {
"message": "Renaming {0} failed due to API error: {1}",
"comment": [
"File path",
"Error message"
]
},
"Deleting {0} failed due to API error: {1}/File nameError message": {
"message": "Deleting {0} failed due to API error: {1}",
"comment": [
"File name",
"Error message"
]
},
"No error details given": "No error details given",
"Error fetching destination {0} for paste action: {1}/USS pathError message": {
"message": "Error fetching destination {0} for paste action: {1}",
"comment": [
"USS path",
"Error message"
]
},
"{0} location/Node type": {
"message": "{0} location",
"comment": [
Expand Down
Loading
Loading