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

…3318) * fix: profile change propagation, prompt on auth error Signed-off-by: Trae Yelovich <[email protected]> * tests: profile propagation, tree node cases Signed-off-by: Trae Yelovich <[email protected]> * chore: changelogs Signed-off-by: Trae Yelovich <[email protected]> * test(jobs): add patch coverage Signed-off-by: Trae Yelovich <[email protected]> * tests: AuthUtils.promptForAuthError Signed-off-by: Trae Yelovich <[email protected]> * tests: UssFSProvider.fetchFileAtUri Signed-off-by: Trae Yelovich <[email protected]> * fix: reset wasAccessed flag if ImperativeError caught Signed-off-by: Trae Yelovich <[email protected]> * tests: resolve failing USS tests Signed-off-by: Trae Yelovich <[email protected]> * fix failing jobs test Signed-off-by: Trae Yelovich <[email protected]> --------- Signed-off-by: Trae Yelovich <[email protected]>
zowe · Nov 15, 2024 · a1f44ac · a1f44ac
1 parent 98a4d51
commit a1f44ac
Show file tree

Hide file tree

Showing 16 changed files with 286 additions and 145 deletions.
diff --git a/packages/zowe-explorer-api/CHANGELOG.md b/packages/zowe-explorer-api/CHANGELOG.md
@@ -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`
 

diff --git a/packages/zowe-explorer-api/__tests__/__unit__/tree/ZoweTreeNode.unit.test.ts b/packages/zowe-explorer-api/__tests__/__unit__/tree/ZoweTreeNode.unit.test.ts
@@ -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,
@@ -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", () => {
@@ -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");
     });
 });
diff --git a/packages/zowe-explorer-api/src/tree/ZoweTreeNode.ts b/packages/zowe-explorer-api/src/tree/ZoweTreeNode.ts
@@ -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.

diff --git a/packages/zowe-explorer/CHANGELOG.md b/packages/zowe-explorer/CHANGELOG.md
@@ -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`
 

diff --git a/packages/zowe-explorer/__tests__/__unit__/configuration/Profiles.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/configuration/Profiles.unit.test.ts
@@ -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);

diff --git a/packages/zowe-explorer/__tests__/__unit__/trees/job/JobFSProvider.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/trees/job/JobFSProvider.unit.test.ts
@@ -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();
 
@@ -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", () => {

diff --git a/packages/zowe-explorer/__tests__/__unit__/trees/job/ZoweJobNode.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/trees/job/ZoweJobNode.unit.test.ts
@@ -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();

diff --git a/packages/zowe-explorer/__tests__/__unit__/trees/uss/UssFSProvider.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/trees/uss/UssFSProvider.unit.test.ts
@@ -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();
 
@@ -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);

diff --git a/packages/zowe-explorer/__tests__/__unit__/utils/AuthUtils.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/utils/AuthUtils.unit.test.ts
@@ -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);
+        });
+    });
+});
diff --git a/packages/zowe-explorer/__tests__/__unit__/utils/ProfilesUtils.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/utils/ProfilesUtils.unit.test.ts
@@ -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();