Vscode-extension-for-zowe: Research refresh functionality

Created on 20 Oct 2020  ·  10Comments  ·  Source: zowe/vscode-extension-for-zowe

The purpose of this issue is to track research on refresh in Zowe Explorer.

  • Gather issues related to refresh in Zowe Explorer
  • Examine the different types of refresh in Zowe Explorer
  • Outside-in view/use case scenarios
  • Look for areas of improvement

issues related to refresh in Zowe Explorer
Issues that are directly refresh

Issues that are functions calling on refresh
(These filepaths are based on the current monorepo-switch branch.

Issues I'm not sure about (i.e. Cannot recreate issue or unsure if it is still an issue)

Different types of refresh in Zowe Explorer
In the zowe-explorer-api package

  • src/Utils.ts: (This file no longer exists.)

    • refreshTree(IZoweTreeNode): Refresh profile and session (sessNode, which is type IZoweTreeNode)

  • src/ProfilesCache.ts:

    • refresh(apiRegister: ZoweExplorerApi.IApiRegisterClient):

    • Gets base profiles

    • Gets extender profiles

    • Get an array of all profiles (Profiles.allProfiles)

    • Get an array of all profile types (Profiles.allTypes)

    • Remove all profiles from profilesForValidation array (profiles.profilesForValidation)

  • src/IZoweTree.ts (Declarations only):

    • refresh(): Refreshes Tree

    • refreshElement(node: IZoweNodeType): Refreshes tree element, i.e. a tree node

    • refreshPS(node: IZoweNodeType): Refreshes the passed node with current mainframe data. Seems to be used (defined) by the Data Sets view only (in the zowe-explorer package)

  • src/IZoweTreeNode.ts (Declarations only. The below functions are specific to IZoweUssTreeNode only):

    • refreshUSS?(): Refreshes the passed node with current mainframe data. (Function definition is in ZoweUSSNode.ts.)

    • refreshAndReopen?(hasClosedInstance?: boolean): Refreshes node and reopens it. (Function definition is in ZoweUSSNode.ts)

In the zowe-explorer package

  • src/abstract/ZoweTreeProvider:

    • refreshElement(element: IZoweDatasetTreeNode): Called whenever the tree (tree element?) needs to be refreshed, and fires the data change event

    • Marks element as dirty, then calls mOnDidChangeTreeData on the element in the tree provider.

    • refresh(): Called whenever the tree needs to be refreshed, and fires the data change event

    • Calls mOnDidChangeTreeData on the tree provider.

  • src/dataset/actions:

    • refreshAll(datasetProvider: IZoweTree<IZoweDatasetTreeNode>): Refreshes treeview

    • refreshPS(node: IZoweDatasetTreeNode): Refreshes the passed node with current mainframe data, where "node" represents the data set.

  • src/uss/actions:

    • refreshAllUSS(ussFileProvider: IZoweTree<IZoweUSSTreeNode>): Refreshes treeview

    • refreshUSSInTree(node: IZoweUSSTreeNode, ussFileProvider: IZoweTree<IZoweUSSTreeNode>)

    • Seems to just call refreshElement on the node in the tree.

  • src/uss/ZoweUssNode:

    • refreshUSS(): Refreshes the passed node with current mainframe data

    • refreshAndReopen(hasClosedInstance = false): Refreshes node and reopens it

  • src/job/actions:

    • refreshAllJobs(jobsProvider: IZoweTree<IZoweJobTreeNode>): Refresh all jobs in the job tree.

    • refreshJobsServer(node: IZoweJobTreeNode, jobsProvider: IZoweTree<IZoweJobTreeNode>): Refresh a node in the job tree

  • src/shared/utils.ts:

    • refreshTree(sessNode: IZoweTreeNode): Refresh Profile and Session

    • labelRefresh(node: vscode.TreeItem): Not actually a tree refresh, but just a silent label rename to force a node to repaint.

  • src/utils/ProfileUtils.ts:

    • refreshTree(sessNode: IZoweTreeNode): Refresh Profile and Session

Outside-in view/use case scenarios
(See comment below.)

Areas of potential improvement

  • Name refresh functions more descriptively to better differentiate them and what they do.
  • Consider breaking the refresh in Profiles.ts down into smaller functions.
picture lauren-li
👍2

Most helpful comment

Working on a fork for the refresh refactoring https://github.com/JillieBeanSim/vscode-extension-for-zowe/tree/refresh-refactoring

So far

  • refreshAll/refreshAllUSS/refreshAllJobs in actions.ts file (datasets, uss, and jobs) have been removed and all calls are using refreshAll() in the new /shared/refresh.ts file.
  • dsNodeActions.ts has been removed since all it had was another refreshAll() method
  • unused file and folder mvs/mvsNodeActions.ts removed
picture JillieBeanSim on 10 Dec 2020
❤2

All 10 comments

Outside-in view/use case scenarios
General principles (Updated with suggestions from meeting with @IlyaKreynin on 11/3):

  • Folders that the user has expanded should be kept expanded where possible.
  • Refresh should be consistent between views (Data Sets, USS, Jobs).
  • Refresh at the lowest level possible.
  • Minimum impediment to user action.
  • Minimize number of refreshes

Profiles

  • Add, hide, or delete profile: Entire tree refresh

All trees:

  • Add, upload,rename, or delete file/folder: Parent node should refresh

    • This includes pasting in a copied file/folder

    • Need to refresh corresponding favorite, if applicable

    • Or, if the action is being done in Favorites, need to refresh corresponding profile node

  • Favorites: Remove from favorites
  • Click refresh button: Entire tree should refresh, including favorites
  • Enable/disable validation:

    • Global setting: Entire tree refresh

    • Profile-specific: Entire tree refresh

Data Sets:

  • Migrate/Recall: Parent profile node refresh

USS:

  • _Toggle Binary/Text: Parent node of the file refreshes (currently, all expanded folders are also refreshed)_

Jobs:

  • Maybe when a job is submitted via the Data Sets tree?

Inconsistencies in the current refresh UX

  • Data Sets refresh preserves expanded folders, whereas USS does not. (Issue #650 USS tree collapses after folder rename.)
lauren-li picture lauren-li on 27 Oct 2020

Thanks for your hard work on putting this together, @lauren-li !

The only thing I could add under USS tree is the Toggle Binary/Text command. Currently, when I select this option, all the expanded folders are also refreshed. Ideally, only the parent node of the file is refreshed.

crawr picture crawr on 6 Nov 2020
❤1 👍1

src/Profiles.ts:
refresh():
Gets base profiles
Gets extender profiles
Get an array of all profiles (Profiles.allProfiles)
Get an array of all profile types (Profiles.allTypes)
Remove all profiles from profilesForValidation array (profiles.profilesForValidation)

Additional to this, call getCombinedProfiles because once it is refreshed the allProfiles array doesn't contain the combined profiles. (I added this in refreshTree())

Also, do you think the trees's RefreshAll should be part of the ZE api? I'm not sure, I'm just thinking that if an external extension wants to refresh the Dataset Tree, for example, how are they going to do it?

jellypuno picture jellypuno on 10 Nov 2020

Potential issue (currently investigating):
Refresh button does not seem to be reloading extenders' profiles anymore.

Edit: Created issue #1078 for this (Tree Refresh button does not pick up extenders' profile updates)

lauren-li picture lauren-li on 17 Nov 2020

Thanks everyone for your input on this so far!

I've updated the issue description with changes that were made to the monorepo, and updated the outside-in use cases with @craw's comment.

@jellypuno I'm a little confused about getCombinedProfiles. Are you saying it is currently called as part of the refresh function in src/Profiles.ts (or apparently src/ProfilesCache.ts now)? Or are you saying we should add it in there? I looked in master branch, but only see it being called in each tree's addSingleSession function. I do not see it in either of the refreshTree functions, either.

  • Regarding each tree's refreshAll function, the API seems to already have a refresh function that refreshes a tree, and it appears to be defined in zowe-explorer/src/abstract/ZoweTreeProvider. It seems a bit more general than the refreshAll functions, though. I think the answer to your question may depend on whether we want to merge the functionality of these two implementations of tree refresh functions.
lauren-li picture lauren-li on 17 Nov 2020

EDIT 01/13/2021: The issue below is now outdated since PR #1064 was merged in (Fix: Initialize Error when using base profiles).

~
In refreshTree of packages/zowe-explorer/src/shared/utils, the following line appears to always evaluate to True, even if no changes were made to the profile (tested with z/OSMF profiles):
if (sessNode.getSession().ISession !== SessionProfile)

It appears that the two sides of the equation return different objects. This means that this function is always going to update Zowe Explorer's profile from its yaml file, even if nothing has been changed. It is not necessarily harmful during the refresh, but it is just an extra check and extra step.

  • sessNode.getSession().ISession returned an object containing values for the following keys:

    1. base64EncodedAuth

    2. basePath

    3. hostname

    4. password

    5. port

    6. protocol

    7. rejectUnauthorized

    8. secureProtocol

    9. strictSSL

    10. type

    11. user

  • SessionProfile returned an object containing values for the following keys:

    1. basePath

    2. encoding

    3. host

    4. password

    5. port

    6. rejectUnauthorized

    7. responseTimeout

    8. user

lauren-li picture lauren-li on 19 Nov 2020

@zdmullen I've created a low-hanging fruit graph to map out the value/effort for the issues we currently have listed as related to refresh. It is rather general (imprecise) since the text boxes take up quite a bit of space, but I interpreted the left edges of the text boxes to be approximately where they sit in terms of effort:

Screen Shot 2020-12-03 at 10 39 38 AM

Here is the word document in case anyone wants to play with moving issues around on the graph:
ze-refresh.docx

As for the issues that did not make it into the graph, I have categorized them as follows:

  • Validating/Done:

    • #1078 Tree Refresh button does not pick up extenders' profile updates

  • Possibly related to refresh:

    • #471 Refresh of DS is taking long time (This may be related to the way we pull things from mainframe, or it may be related to calls we make within the refresh function.)

  • Not related to VS Code UI refresh (i.e. not planned to be addressed as part of the Refresh Refactor):

    • #755 eTag error encountered when saving file in root of USS tree
    • #988/#1074 Enable update of Job logs/spool files (This is more related to pulling from mainframe)
    • #832 Refresh DS members without having to individually select them (This is more related to pulling from mainframe)
    • #1033 Clear all settings and default profile in meta yaml to null if profiles are manually deleted (This should be fixed in the code to add session/add single session.)
lauren-li picture lauren-li on 3 Dec 2020
👍1

Suggestions/considerations for refresh functions:

  • refreshAll/refreshAllUSS/refreshAllJobs in actions.ts file (datasets, uss, and jobs): Perhaps take this function and put it in a shared file (maybe src/shared/utils.ts?), as the code appears to be identical between all 3 trees.

    • There are also two refreshAll functions defined: One in dataset/actions.ts, and one in dsNodeActions.ts. Perhaps one of those can be removed.

    • This function skips refreshing favorites (if (contextually.isSessionNotFav(sessNode))) – should we take this snippet out so refreshAll refreshes favorites, as well?

  • refreshUSSInTree: Could we refactor this out, and just use ussFileProvider.refreshElement(node) directly?

  • ~refreshJobsServer: Is this being used anywhere? Can we delete this function?~ (Edit: This is used to manually refresh jobs for a single profile.)

  • refreshPS and refreshUSS: Consider renaming these, as they do not involve UI refresh. (However, they are “refresh” in the sense of “refreshing a file/data set with the latest content from the mainframe”.)

  • refreshTree:

    • There is a refreshTree defined in src/utils/ProfileUtils.ts, and one defined in src/shared/utils.ts. Is there a difference in purpose between these two functions? The one in ProfileUtils looks kind of the same as the one in shared/utils, except not updated for base profiles. It is just used in getDatasets, getJobs, getChildren (USS).
    • (The refreshTree in shared/utils is just used by the refreshAll- functions.)
    • This function does not do anything directly related to refresh. Perhaps we could rename it to, getUpdatedProfiles). (Actually, I think refreshTree is a better function name for what the refreshAll functions currently do…)

Suggestions/questions for functions that use refresh:

  • Why is refreshTree called during the error during for getDatasets, getJobs, getChildren (USS)?
  • For development:

    • Theoretically, if we want to refresh just a specific node or its parent, we should be able to do it via refreshElement(element), where element is the node or its parent.

    • To refresh an entire view (data sets, USS, or Jobs), we would use refreshAll(treeProvider)

lauren-li picture lauren-li on 8 Dec 2020
👍1

Tables to categorize user actions into different types of refresh.

Category explanations:

  • UI Refresh level:

    • View: Refresh the entire view, i.e. Data Sets (MVS), USS, or Jobs
    • Node: This is any node inside a view, whether it is a session node, parent node, file/folder, etc.

  • Info flow: In which direction is information being sent?

    • Client>Host: Client telling mainframe the "truth"
    • Host>Client: Mainframe telling client the "truth"
    • (None): No information is passed between client and host

  • Automatic/Manual: Automatic versus manual refresh

  • Notes: Additional notes

Non-profile methods

| Method/Command | UI Refresh level | Info flow | Automatic/Manual | Notes |
| ------------------------------ | ---------------- | ----------- | ------------------- | ----------------------------------- |
| Create/Upload (MVS, USS) | Node | Client>Host | Automatic | |
| Rename (MVS, USS) | Node | Client>Host | Automatic | Renaming folder changes child paths |
| Delete (MVS, USS, Jobs) | Node | Client>Host | Automatic | Currently manual for jobs |
| Pull from mainframe (MVS, USS, Jobs files) | (None) | Host>Client | Automatic (ideally) | Currently manual |
| Migrate/Recall (MVS) | Node | Client>Host | Automatic | Currently manual |
| Allocate like (MVS) | Node | Client>Host | Automatic | |
| Paste (MVS) | Node | Client>Host | Automatic | |
| Toggle binary (USS) | Node | (None) | Automatic | |
| Add favorite (MVS, USS, Jobs) | Node | (None) | Automatic | |

Profile methods (via Zowe Explorer)

| Method/Command | UI Refresh level | Info flow | Automatic/Manual | Notes |
| ------------------------- | ---------------- | ----------- | ---------------- | ----- |
| Create new | View | Client>Host | Automatic | |
| Add pre-existing | View | (None) | Automatic | |
| Delete | View | Client>Host | Automatic | |
| Hide | View | (None) | Automatic | |
| Enable/Disable Validation | View | (None) | Automatic | |
| Add favorite | Node | (None) | Automatic | |

Profile methods (via Zowe CLI)

(Refresh categories still apply to Zowe Explorer.)
All of these currently require a manual refresh in Zowe Explorer.

| Method/Command | UI Refresh level | Info flow | Automatic/Manual | Notes |
| -------------- | ---------------- | ----------- | ------------------- | ----- |
| Create | View | Client>Host | Automatic (ideally) | |
| Update | View | Client>Host | Automatic (ideally) | |
| Delete | View | Client>Host | Automatic (ideally) | |

lauren-li picture lauren-li on 10 Dec 2020

Working on a fork for the refresh refactoring https://github.com/JillieBeanSim/vscode-extension-for-zowe/tree/refresh-refactoring

So far

  • refreshAll/refreshAllUSS/refreshAllJobs in actions.ts file (datasets, uss, and jobs) have been removed and all calls are using refreshAll() in the new /shared/refresh.ts file.
  • dsNodeActions.ts has been removed since all it had was another refreshAll() method
  • unused file and folder mvs/mvsNodeActions.ts removed
JillieBeanSim picture JillieBeanSim on 10 Dec 2020
❤2
Was this page helpful?
0 / 5 - 0 ratings

Related issues

jelaplan picture jelaplan  ·  4Comments
travatine picture travatine  ·  4Comments
ghost picture ghost  ·  4Comments
gplambert picture gplambert  ·  3Comments
t1m0thyj picture t1m0thyj  ·  3Comments