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

[v2] Port fix for credentials not refreshed in ds tree and other misc profile fixes #3349

Open
wants to merge 6 commits into
base: v2-lts
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,17 @@ import * as vscode from "vscode";
import { ZoweTreeNode } from "../../../src/tree/ZoweTreeNode";
import { IZoweTreeNode } from "../../../src/tree/IZoweTreeNode";
import { imperative } from "@zowe/cli";

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 @@ -46,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 @@ -78,4 +88,28 @@ describe("ZoweTreeNode", () => {
expect(node.getProfile()).toBeUndefined();
expect(node.getProfileName()).toBeUndefined();
});

it("setProfileToChoice should update properties on existing profile object", () => {
const node = makeNode("test", vscode.TreeItemCollapsibleState.None, undefined, undefined, {
...fakeProfile,
});
node.setProfileToChoice({ ...fakeProfile, profile: { host: "example.com", port: 443 } });
expect(node.getProfile().profile?.port).toBeDefined();
});

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 fsEntry = {
metadata: {
profile: node.getProfile(),
},
};
expect(node.getProfile().profile?.user).toBe("banana");
expect(nodeChild.getProfile().profile?.user).toBe("banana");
expect(fsEntry.metadata.profile.profile?.user).toBe("banana");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ describe("ZoweVsCodeExtension", () => {
mergeArgsForProfile: jest.fn().mockReturnValue({ knownArgs: [] }),
}),
refresh: jest.fn(),
updateProfilesArrays: jest.fn(),
};

beforeEach(() => {
Expand Down Expand Up @@ -375,7 +376,7 @@ describe("ZoweVsCodeExtension", () => {
expect(loginSpy).not.toHaveBeenCalled();
expect(testSpy).not.toHaveBeenCalled();
expect(testCache.updateBaseProfileFileLogin).toHaveBeenCalledWith(baseProfile, updProfile, false);
expect(testNode.setProfileToChoice).toHaveBeenCalled();
expect(testCache.updateProfilesArrays).toHaveBeenCalledWith(serviceProfile, testNode);
quickPickMock.mockRestore();
});
});
Expand Down
5 changes: 4 additions & 1 deletion packages/zowe-explorer-api/src/profiles/ProfilesCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { URL } from "url";

import * as zowe from "@zowe/cli";
import { ZoweExplorerApi } from "./ZoweExplorerApi";
import { IZoweNodeType } from "../tree";

// TODO: find a home for constants
export const CONTEXT_PREFIX = "_";
Expand Down Expand Up @@ -129,10 +130,11 @@ export class ProfilesCache {
* Updates profile in allProfiles array and if default updates defaultProfileByType
*
* @param {string} profileLoaded
* @param {IZoweNodeType} profileNode
*
* @returns {void}
*/
public updateProfilesArrays(profileLoaded: zowe.imperative.IProfileLoaded): void {
public updateProfilesArrays(profileLoaded: zowe.imperative.IProfileLoaded, profileNode?: IZoweNodeType): void {
// update allProfiles array
const promptedTypeIndex = this.allProfiles.findIndex(
(profile) => profile?.type === profileLoaded?.type && profile?.name === profileLoaded?.name
Expand All @@ -143,6 +145,7 @@ export class ProfilesCache {
if (defaultProf?.name === profileLoaded?.name) {
this.defaultProfileByType.set(profileLoaded?.type, profileLoaded);
}
profileNode?.setProfileToChoice(profileLoaded);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/zowe-explorer-api/src/tree/ZoweTreeNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ export class ZoweTreeNode extends vscode.TreeItem {
* @param {imperative.IProfileLoaded} The profile you will set the node to use
*/
public setProfileToChoice(aProfile: imperative.IProfileLoaded): void {
this.profile = aProfile;
// Don't reassign profile if its already defined, as we want to keep the reference valid for other nodes
this.profile = Object.assign(this.profile ?? {}, aProfile);
}
/**
* Sets the session for this node to the one chosen in parameters.
Expand Down
28 changes: 7 additions & 21 deletions packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class ZoweVsCodeExtension {
options: IPromptCredentialsOptions,
apiRegister: ZoweExplorerApi.IApiRegisterClient
): Promise<imperative.IProfileLoaded> {
const cache = ZoweVsCodeExtension.profilesCache;
const cache = options.zeProfiles ?? ZoweVsCodeExtension.profilesCache;
const profInfo = await cache.getProfileInfo();
const setSecure = options.secure ?? profInfo.isSecured();

Expand Down Expand Up @@ -228,23 +228,11 @@ export class ZoweVsCodeExtension {
// If base profile already has a token type stored, then we check whether or not the connection details are the same
(serviceProfile.profile.host === baseProfile.profile.host && serviceProfile.profile.port === baseProfile.profile.port);
// If the connection details do not match, then we MUST forcefully store the token in the service profile
let profileToUpdate: imperative.IProfileLoaded;
if (connOk) {
profileToUpdate = baseProfile;
} else {
profileToUpdate = serviceProfile;
}
const profileToUpdate = connOk ? baseProfile : serviceProfile;

await cache.updateBaseProfileFileLogin(profileToUpdate, updBaseProfile, !connOk);
const baseIndex = cache.allProfiles.findIndex((profile) => profile.name === profileToUpdate.name);
cache.allProfiles[baseIndex] = { ...profileToUpdate, profile: { ...profileToUpdate.profile, ...updBaseProfile } };

if (node) {
node.setProfileToChoice({
...node.getProfile(),
profile: { ...node.getProfile().profile, ...updBaseProfile },
});
}
serviceProfile.profile = { ...serviceProfile.profile, ...updBaseProfile };
cache.updateProfilesArrays(serviceProfile, node);
return true;
}

Expand Down Expand Up @@ -283,11 +271,9 @@ export class ZoweVsCodeExtension {
await (zeRegister?.getCommonApi(serviceProfile).logout ?? Logout.apimlLogout)(updSession);

const connOk = serviceProfile.profile.host === baseProfile.profile.host && serviceProfile.profile.port === baseProfile.profile.port;
if (connOk) {
await cache.updateBaseProfileFileLogout(baseProfile);
} else {
await cache.updateBaseProfileFileLogout(serviceProfile);
}
await cache.updateBaseProfileFileLogout(connOk ? baseProfile : serviceProfile);
serviceProfile.profile = { ...serviceProfile.profile, tokenType: undefined, tokenValue: undefined };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed serviceProfile.profile is reassigned as a new object - since this is in v2, I don't think this has any major side effects. However, it does make me wonder if we have similar logic in v3 and whether that logic works well with other parts of Zowe Explorer that keep profile references (such as tree nodes, filesystem) 🤔

No request for changes here, it's merely an observation 😋

cache.updateProfilesArrays(serviceProfile);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import { imperative } from "@zowe/cli";
import { InputBoxOptions, OpenDialogOptions } from "vscode";
import { ProfilesCache } from "../../profiles";

export interface IPromptCredentialsCommonOptions {
rePrompt?: boolean;
Expand All @@ -23,6 +24,7 @@ export interface IPromptCredentialsOptions extends IPromptCredentialsCommonOptio
sessionName?: string;
sessionType?: string;
secure?: boolean;
zeProfiles?: ProfilesCache;
}

export interface IPromptUserPassOptions extends IPromptCredentialsCommonOptions {
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
- Fixed an issue where a migrated data set is unusable after it is recalled through Zowe Explorer. [#3294](https://github.com/zowe/zowe-explorer-vscode/issues/3294)
- Fixed an issue where a recalled PDS is expandable after it is migrated through Zowe Explorer. [#3294](https://github.com/zowe/zowe-explorer-vscode/issues/3294)
- Fixed an issue where data set nodes did not update if migrated or recalled outside of Zowe Explorer. [#3294](https://github.com/zowe/zowe-explorer-vscode/issues/3294)
- Fixed issue where Search operation did not prompt for credentials if profile contains expired token. [#2259](https://github.com/zowe/zowe-explorer-vscode/issues/2259)
- Fixed issue where inactive status was not displayed for profiles loaded from Global Config. [#3134](https://github.com/zowe/zowe-explorer-vscode/issues/3134)
Comment on lines +20 to +21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: We'll probably want to move these entries to the bottom of this section as the existing entries are sorted chronologically

- Fixed an issue where clicking on a file in the Unix System Services tree caused the tree to abruptly change focus to the selected item. [#2486](https://github.com/zowe/zowe-explorer-vscode/issues/2486)
- Fixed an issue where binary USS files were not fetched using the "Pull from Mainframe" context menu option. [#3355](https://github.com/zowe/zowe-explorer-vscode/issues/3355)
- Fixed an issue with Auto Save where a failed UNIX file or data set save operation caused an infinite loop of save requests. [#2406](https://github.com/zowe/zowe-explorer-vscode/issues/2406), [#2627](https://github.com/zowe/zowe-explorer-vscode/issues/2627)
Expand Down
25 changes: 25 additions & 0 deletions packages/zowe-explorer/__mocks__/vscode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,31 @@
*
*/

/**
* A location in the editor at which progress information can be shown. It depends on the
* location how progress is visually represented.
*/
export enum ProgressLocation {
/**
* Show progress for the source control viewlet, as overlay for the icon and as progress bar
* inside the viewlet (when visible). Neither supports cancellation nor discrete progress nor
* a label to describe the operation.
*/
SourceControl = 1,

/**
* Show progress in the status bar of the editor. Neither supports cancellation nor discrete progress.
* Supports rendering of {@link ThemeIcon theme icons} via the `$(<name>)`-syntax in the progress label.
*/
Window = 10,

/**
* Show progress as notification with an optional cancel button. Supports to show infinite and discrete
* progress but does not support rendering of icons.
*/
Notification = 15,
}

export enum ViewColumn {
/**
* A *symbolic* editor column representing the currently active column. This value
Expand Down
Loading
Loading