diff --git a/packages/zowe-explorer/CHANGELOG.md b/packages/zowe-explorer/CHANGELOG.md index a813b72aae..8e9441547c 100644 --- a/packages/zowe-explorer/CHANGELOG.md +++ b/packages/zowe-explorer/CHANGELOG.md @@ -19,6 +19,7 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen - 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 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) ## `2.18.0` diff --git a/packages/zowe-explorer/__tests__/__unit__/abstract/ZoweSaveQueue.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/abstract/ZoweSaveQueue.unit.test.ts index d50db952a2..830b110d38 100644 --- a/packages/zowe-explorer/__tests__/__unit__/abstract/ZoweSaveQueue.unit.test.ts +++ b/packages/zowe-explorer/__tests__/__unit__/abstract/ZoweSaveQueue.unit.test.ts @@ -14,7 +14,6 @@ import { createUSSTree } from "../../../__mocks__/mockCreators/uss"; import { saveUSSFile } from "../../../src/uss/actions"; import * as workspaceUtils from "../../../src/utils/workspace"; import { Gui } from "@zowe/zowe-explorer-api"; -import * as globals from "../../../src/globals"; import * as vscode from "vscode"; import { ZoweLogger } from "../../../src/utils/LoggerUtils"; @@ -24,6 +23,7 @@ describe("ZoweSaveQueue - unit tests", () => { const globalMocks = { errorMessageSpy: jest.spyOn(Gui, "errorMessage"), markDocumentUnsavedSpy: jest.spyOn(workspaceUtils, "markDocumentUnsaved"), + handleAutoSaveOnErrorMock: jest.spyOn(workspaceUtils, "handleAutoSaveOnError").mockImplementation(), processNextSpy: jest.spyOn(ZoweSaveQueue as any, "processNext"), allSpy: jest.spyOn(ZoweSaveQueue, "all"), trees: { @@ -118,4 +118,46 @@ describe("ZoweSaveQueue - unit tests", () => { ); } }); + + describe("markAllUnsaved", () => { + function getBlockMocks(): Record { + return { + markDocumentUnsaved: jest.spyOn(workspaceUtils, "markDocumentUnsaved").mockClear().mockImplementation(), + }; + } + + it("marks all documents as unsaved in the queue", async () => { + const blockMocks = getBlockMocks(); + (ZoweSaveQueue as any).savingQueue = [ + { + savedFile: { fileName: "some.jcl" }, + } as any, + { + savedFile: { fileName: "SOME.C.DS(MEM).c" } as any, + } as any, + ]; + await ZoweSaveQueue.markAllUnsaved(); + expect(blockMocks.markDocumentUnsaved).toHaveBeenCalledTimes(2); + }); + + it("clears the queue if clearQueue is true", async () => { + (ZoweSaveQueue as any).savingQueue = [ + { + savedFile: { fileName: "some.jcl" }, + } as any, + ]; + await ZoweSaveQueue.markAllUnsaved(); + expect((ZoweSaveQueue as any).savingQueue.length).toBe(0); + }); + + it("does not clear the queue if clearQueue is false", async () => { + (ZoweSaveQueue as any).savingQueue = [ + { + savedFile: { fileName: "some.jcl" }, + } as any, + ]; + await ZoweSaveQueue.markAllUnsaved(false); + expect((ZoweSaveQueue as any).savingQueue.length).toBe(1); + }); + }); }); diff --git a/packages/zowe-explorer/__tests__/__unit__/dataset/actions.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/dataset/actions.unit.test.ts index 638f417153..7b019df14b 100644 --- a/packages/zowe-explorer/__tests__/__unit__/dataset/actions.unit.test.ts +++ b/packages/zowe-explorer/__tests__/__unit__/dataset/actions.unit.test.ts @@ -43,6 +43,7 @@ import { getNodeLabels } from "../../../src/dataset/utils"; import { ZoweLogger } from "../../../src/utils/LoggerUtils"; import { mocked } from "../../../__mocks__/mockUtils"; import { LocalFileManagement } from "../../../src/utils/LocalFileManagement"; +import * as workspaceUtils from "../../../src/utils/workspace"; // Missing the definition of path module, because I need the original logic for tests jest.mock("fs"); @@ -1073,6 +1074,7 @@ describe("Dataset Actions Unit Tests - Function saveFile", () => { profileInstance, testDatasetTree, uploadContentSpy, + handleAutoSaveOnError: jest.spyOn(workspaceUtils, "handleAutoSaveOnError").mockImplementation(), }; } @@ -1238,6 +1240,7 @@ describe("Dataset Actions Unit Tests - Function saveFile", () => { (testDocument as any).fileName = path.join(globals.DS_DIR, testDocument.fileName); await dsActions.saveFile(testDocument, blockMocks.testDatasetTree); + expect(blockMocks.handleAutoSaveOnError).toHaveBeenCalled(); expect(globalMocks.concatChildNodes).toBeCalled(); expect(mocked(Gui.errorMessage)).toBeCalledWith("failed"); diff --git a/packages/zowe-explorer/__tests__/__unit__/uss/actions.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/uss/actions.unit.test.ts index 4501c269f2..8451f82002 100644 --- a/packages/zowe-explorer/__tests__/__unit__/uss/actions.unit.test.ts +++ b/packages/zowe-explorer/__tests__/__unit__/uss/actions.unit.test.ts @@ -32,6 +32,7 @@ import * as vscode from "vscode"; import * as path from "path"; import * as globals from "../../../src/globals"; import * as sharedUtils from "../../../src/shared/utils"; +import * as workspaceUtils from "../../../src/utils/workspace"; import { ZoweUSSNode } from "../../../src/uss/ZoweUSSNode"; import * as isbinaryfile from "isbinaryfile"; import * as fs from "fs"; @@ -41,6 +42,7 @@ import { ZoweLogger } from "../../../src/utils/LoggerUtils"; import { AttributeView } from "../../../src/uss/AttributeView"; import { mocked } from "../../../__mocks__/mockUtils"; import { LocalFileManagement } from "../../../src/utils/LocalFileManagement"; +import { SettingsConfig } from "../../../src/utils/SettingsConfig"; function createGlobalMocks() { const globalMocks = { @@ -461,6 +463,9 @@ describe("USS Action Unit Tests - Function saveUSSFile", () => { ussNode: createUSSNode(globalMocks.testSession, globalMocks.testProfile), ussFavoriteNode: createFavoriteUSSNode(globalMocks.testSession, globalMocks.testProfile), putUSSPayload: jest.fn().mockResolvedValue(`{"stdout":[""]}`), + handleAutoSaveOnError: jest.spyOn(workspaceUtils, "handleAutoSaveOnError"), + // Disable auto save for most of the test cases + getDirectValue: jest.spyOn(SettingsConfig, "getDirectValue").mockReturnValueOnce("off"), }; newMocks.node = new ZoweUSSNode({ @@ -544,6 +549,21 @@ describe("USS Action Unit Tests - Function saveUSSFile", () => { ); }); + it("calls handleAutoSaveOnError in the event of an error", async () => { + const globalMocks = createGlobalMocks(); + const blockMocks = await createBlockMocks(globalMocks); + + globalMocks.withProgress.mockImplementation((progLocation, callback) => callback()); + globalMocks.fileToUSSFile.mockResolvedValue(blockMocks.testResponse); + blockMocks.testResponse.success = false; + blockMocks.testResponse.commandResponse = "Save failed"; + + globalMocks.withProgress.mockReturnValueOnce(blockMocks.testResponse); + + await ussNodeActions.saveUSSFile(blockMocks.testDoc, blockMocks.testUSSTree); + expect(blockMocks.handleAutoSaveOnError).toHaveBeenCalled(); + }); + it("Tests that saveUSSFile fails when save fails", async () => { const globalMocks = createGlobalMocks(); const blockMocks = await createBlockMocks(globalMocks); @@ -570,6 +590,7 @@ describe("USS Action Unit Tests - Function saveUSSFile", () => { globalMocks.withProgress.mockRejectedValueOnce(Error("Test Error")); await ussNodeActions.saveUSSFile(blockMocks.testDoc, blockMocks.testUSSTree); + expect(blockMocks.handleAutoSaveOnError).toHaveBeenCalled(); expect(globalMocks.showErrorMessage.mock.calls.length).toBe(1); expect(globalMocks.showErrorMessage.mock.calls[0][0]).toBe("Error: Test Error"); expect(mocked(vscode.workspace.applyEdit)).toHaveBeenCalledTimes(2); diff --git a/packages/zowe-explorer/__tests__/__unit__/utils/workspace.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/utils/workspace.unit.test.ts index 467ca10dd8..810a013887 100644 --- a/packages/zowe-explorer/__tests__/__unit__/utils/workspace.unit.test.ts +++ b/packages/zowe-explorer/__tests__/__unit__/utils/workspace.unit.test.ts @@ -12,6 +12,9 @@ import * as vscode from "vscode"; import * as workspaceUtils from "../../../src/utils/workspace"; import { workspaceUtilMaxEmptyWindowsInTheRow } from "../../../src/config/constants"; +import { SettingsConfig } from "../../../src/utils/SettingsConfig"; +import { ZoweSaveQueue } from "../../../src/abstract/ZoweSaveQueue"; +import { Gui } from "@zowe/zowe-explorer-api"; function createGlobalMocks() { const activeTextEditor = jest.fn(); @@ -216,3 +219,70 @@ describe("Workspace Utils Unit Tests - function awaitForDocumentBeingSaved", () await expect(workspaceUtils.awaitForDocumentBeingSaved()).resolves.not.toThrow(); }); }); + +describe("Workspace Utils Unit Tests - function handleAutoSaveOnError", () => { + function getBlockMocks(autoSaveEnabled: boolean = true, userResponse?: string): Record { + const executeCommand = jest.spyOn(vscode.commands, "executeCommand").mockClear(); + const getDirectValue = jest.spyOn(SettingsConfig, "getDirectValue"); + if (autoSaveEnabled) { + executeCommand.mockResolvedValueOnce(undefined); + getDirectValue.mockReturnValueOnce("afterDelay"); + } else { + getDirectValue.mockReturnValueOnce("off"); + } + + if (userResponse === "Enable Auto Save") { + executeCommand.mockResolvedValueOnce(undefined); + } + + return { + errorMessage: jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce(userResponse), + executeCommand, + getDirectValue, + markAllUnsaved: jest.spyOn(ZoweSaveQueue, "markAllUnsaved"), + }; + } + + it("returns early if Auto Save is disabled", async () => { + const blockMocks = getBlockMocks(false); + await workspaceUtils.handleAutoSaveOnError(); + expect(blockMocks.getDirectValue).toHaveBeenCalledWith("files.autoSave"); + expect(blockMocks.executeCommand).not.toHaveBeenCalled(); + }); + + it("toggles off auto save if enabled", async () => { + const blockMocks = getBlockMocks(true); + await workspaceUtils.handleAutoSaveOnError(); + expect(blockMocks.executeCommand).toHaveBeenCalledWith("workbench.action.toggleAutoSave"); + }); + + it("calls ZoweSaveQueue.markAllUnsaved to mark documents unsaved and clear queue", async () => { + const blockMocks = getBlockMocks(true); + await workspaceUtils.handleAutoSaveOnError(); + expect(blockMocks.markAllUnsaved).toHaveBeenCalled(); + }); + + it("prompts the user to reactivate Auto Save in event of save error", async () => { + const blockMocks = getBlockMocks(true); + await workspaceUtils.handleAutoSaveOnError(); + expect(blockMocks.errorMessage).toHaveBeenCalledWith( + "Zowe Explorer encountered a save error and has disabled Auto Save. Once the issue is addressed, enable Auto Save and try again.", + { items: ["Enable Auto Save"] } + ); + }); + + it("does nothing if 'Enable Auto Save' clicked and Auto Save is already on", async () => { + const blockMocks = getBlockMocks(true, "Enable Auto Save"); + await workspaceUtils.handleAutoSaveOnError(); + expect(blockMocks.executeCommand).toHaveBeenCalledWith("workbench.action.toggleAutoSave"); + expect(blockMocks.executeCommand).toHaveBeenCalledTimes(1); + }); + + it("reactivates Auto Save if 'Enable Auto Save' clicked", async () => { + const blockMocks = getBlockMocks(true, "Enable Auto Save"); + blockMocks.getDirectValue.mockReturnValueOnce("off"); + await workspaceUtils.handleAutoSaveOnError(); + expect(blockMocks.executeCommand).toHaveBeenCalledTimes(2); + expect(blockMocks.executeCommand).toHaveBeenCalledWith("workbench.action.toggleAutoSave"); + }); +}); diff --git a/packages/zowe-explorer/i18n/sample/src/uss/USSTree.i18n.json b/packages/zowe-explorer/i18n/sample/src/uss/USSTree.i18n.json index fbc2871233..558c110803 100644 --- a/packages/zowe-explorer/i18n/sample/src/uss/USSTree.i18n.json +++ b/packages/zowe-explorer/i18n/sample/src/uss/USSTree.i18n.json @@ -1,6 +1,5 @@ { "Favorites": "Favorites", - "renameUSS.unsavedWork": "Unable to rename {0} because you have unsaved changes in this {1}. Please save your work before renaming the {1}.", "renameUSS.enterName": "Enter a new name for the {0}", "renameUSS.error": "Unable to rename node:", "renameUSS.duplicateName": "A {0} already exists with this name. Please choose a different name.", diff --git a/packages/zowe-explorer/i18n/sample/src/utils/TreeViewUtils.i18n.json b/packages/zowe-explorer/i18n/sample/src/utils/TreeViewUtils.i18n.json new file mode 100644 index 0000000000..ec20543536 --- /dev/null +++ b/packages/zowe-explorer/i18n/sample/src/utils/TreeViewUtils.i18n.json @@ -0,0 +1,4 @@ +{ + "uss.renameNode": "Rename", + "unsavedChanges.errorMsg": "Unable to {0} {1} because you have unsaved changes in this {2}. Please save your work and try again." +} diff --git a/packages/zowe-explorer/i18n/sample/src/utils/workspace.i18n.json b/packages/zowe-explorer/i18n/sample/src/utils/workspace.i18n.json new file mode 100644 index 0000000000..8e862a65c2 --- /dev/null +++ b/packages/zowe-explorer/i18n/sample/src/utils/workspace.i18n.json @@ -0,0 +1,4 @@ +{ + "zowe.autoSaveToggled": "Zowe Explorer encountered a save error and has disabled Auto Save. Once the issue is addressed, enable Auto Save and try again.", + "zowe.enableAutoSave": "Enable Auto Save" +} diff --git a/packages/zowe-explorer/src/abstract/ZoweSaveQueue.ts b/packages/zowe-explorer/src/abstract/ZoweSaveQueue.ts index 0a50c638f2..099ad1091e 100644 --- a/packages/zowe-explorer/src/abstract/ZoweSaveQueue.ts +++ b/packages/zowe-explorer/src/abstract/ZoweSaveQueue.ts @@ -12,7 +12,7 @@ import * as path from "path"; import * as vscode from "vscode"; import { Gui, IZoweTree, IZoweTreeNode } from "@zowe/zowe-explorer-api"; -import { markDocumentUnsaved } from "../utils/workspace"; +import { handleAutoSaveOnError, markDocumentUnsaved } from "../utils/workspace"; import { ZoweLogger } from "../utils/LoggerUtils"; // Set up localization @@ -56,6 +56,20 @@ export class ZoweSaveQueue { private static ongoingSave = Promise.resolve(); private static savingQueue: SaveRequest[] = []; + /** + * Marks all documents in the save queue as unsaved. + * @param clearQueue Whether to clear the queue after marking the documents as unsaved (default: `true`) + */ + public static async markAllUnsaved(clearQueue: boolean = true): Promise { + for (const { savedFile } of this.savingQueue) { + await markDocumentUnsaved(savedFile); + } + + if (clearQueue) { + this.savingQueue = []; + } + } + /** * Iterate over the queue and process next item until it is empty. */ @@ -71,6 +85,7 @@ export class ZoweSaveQueue { } catch (err) { ZoweLogger.error(err); await markDocumentUnsaved(nextRequest.savedFile); + await handleAutoSaveOnError(); await Gui.errorMessage( localize( "processNext.error.uploadFailed", diff --git a/packages/zowe-explorer/src/dataset/actions.ts b/packages/zowe-explorer/src/dataset/actions.ts index 86fc74d87e..4ac59097cd 100644 --- a/packages/zowe-explorer/src/dataset/actions.ts +++ b/packages/zowe-explorer/src/dataset/actions.ts @@ -33,7 +33,7 @@ import { Profiles } from "../Profiles"; import { getIconByNode } from "../generators/icons"; import { ZoweDatasetNode } from "./ZoweDatasetNode"; import * as contextually from "../shared/context"; -import { markDocumentUnsaved, setFileSaved } from "../utils/workspace"; +import { handleAutoSaveOnError, markDocumentUnsaved, setFileSaved } from "../utils/workspace"; import { IUploadOptions } from "@zowe/zos-files-for-zowe-sdk"; import { ZoweLogger } from "../utils/LoggerUtils"; import { ProfileManagement } from "../utils/ProfileManagement"; @@ -1566,6 +1566,7 @@ export async function saveFile(doc: vscode.TextDocument, datasetProvider: api.IZ return; } } catch (err) { + await handleAutoSaveOnError(); await markDocumentUnsaved(doc); await errorHandling(err, sesName); return; @@ -1610,10 +1611,12 @@ export async function saveFile(doc: vscode.TextDocument, datasetProvider: api.IZ } else if (!uploadResponse.success && uploadResponse.commandResponse.includes("Rest API failure with HTTP(S) status 412")) { resolveFileConflict(node, prof, doc, label); } else { + await handleAutoSaveOnError(); await markDocumentUnsaved(doc); api.Gui.errorMessage(uploadResponse.commandResponse); } } catch (err) { + await handleAutoSaveOnError(); await markDocumentUnsaved(doc); await errorHandling(err, sesName); } diff --git a/packages/zowe-explorer/src/uss/actions.ts b/packages/zowe-explorer/src/uss/actions.ts index 09f1281b04..d4785f0889 100644 --- a/packages/zowe-explorer/src/uss/actions.ts +++ b/packages/zowe-explorer/src/uss/actions.ts @@ -21,7 +21,7 @@ import { Profiles } from "../Profiles"; import { ZoweExplorerApiRegister } from "../ZoweExplorerApiRegister"; import { isBinaryFileSync } from "isbinaryfile"; import * as contextually from "../shared/context"; -import { markDocumentUnsaved, setFileSaved } from "../utils/workspace"; +import { handleAutoSaveOnError, markDocumentUnsaved, setFileSaved } from "../utils/workspace"; import * as nls from "vscode-nls"; import { refreshAll } from "../shared/refresh"; import { autoDetectEncoding, fileExistsCaseSensitiveSync } from "./utils"; @@ -319,6 +319,7 @@ export async function saveUSSFile(doc: vscode.TextDocument, ussFileProvider: IZo LocalFileManagement.removeRecoveredFile(doc); // this part never runs! zowe.Upload.fileToUSSFile doesn't return success: false, it just throws the error which is caught below!!!!! } else { + await handleAutoSaveOnError(); await markDocumentUnsaved(doc); Gui.errorMessage(uploadResponse.commandResponse); } @@ -328,6 +329,7 @@ export async function saveUSSFile(doc: vscode.TextDocument, ussFileProvider: IZo if (errorMessage.includes("Rest API failure with HTTP(S) status 412")) { resolveFileConflict(node, prof, doc, remote); } else { + await handleAutoSaveOnError(); await markDocumentUnsaved(doc); await errorHandling(err, sesName); } diff --git a/packages/zowe-explorer/src/utils/workspace.ts b/packages/zowe-explorer/src/utils/workspace.ts index c572cf62ae..ac9f4d4f57 100644 --- a/packages/zowe-explorer/src/utils/workspace.ts +++ b/packages/zowe-explorer/src/utils/workspace.ts @@ -16,6 +16,17 @@ import { workspaceUtilFileSaveInterval, workspaceUtilFileSaveMaxIterationCount, } from "../config/constants"; +import { SettingsConfig } from "./SettingsConfig"; +import { Gui } from "@zowe/zowe-explorer-api"; + +// Set up localization +import * as nls from "vscode-nls"; +import { ZoweSaveQueue } from "../abstract/ZoweSaveQueue"; +nls.config({ + messageFormat: nls.MessageFormat.bundle, + bundleFormat: nls.BundleFormat.standalone, +})(); +const localize: nls.LocalizeFunc = nls.loadMessageBundle(); interface IExtTextEditor extends vscode.TextEditor { id: string; @@ -143,3 +154,43 @@ export async function markDocumentUnsaved(document: vscode.TextDocument): Promis edits2.delete(document.uri, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 1))); await vscode.workspace.applyEdit(edits2); } + +/** + * Call in the event of a save error. This function checks if `Auto Save` is enabled: + * - If enabled, it is toggled off to prevent an error loop and an error message is shown to inform the user. + * - If disabled, the function returns. + * + * The user can either dismiss the message or click the "Enable Auto Save" button to reactivate the `Auto Save` feature. + */ +export async function handleAutoSaveOnError(): Promise { + // If auto save is disabled, return. Otherwise, toggle it off + if (SettingsConfig.getDirectValue("files.autoSave") === "off") { + return; + } + await vscode.commands.executeCommand("workbench.action.toggleAutoSave"); + + // Mark remaining documents in queue as unsaved and clear the queue + await ZoweSaveQueue.markAllUnsaved(); + + // Inform the user that `Auto Save` was disabled and offer a button to re-enable it + Gui.errorMessage( + localize( + "zowe.autoSaveToggled", + "Zowe Explorer encountered a save error and has disabled Auto Save. Once the issue is addressed, enable Auto Save and try again." + ), + { + items: [localize("zowe.enableAutoSave", "Enable Auto Save")], + } + ).then(async (value) => { + if (!value) { + // User dismissed the message + return; + } + + // Toggle auto save back on IFF it wasn't already reactivated + // (the prompt can be answered or dismissed at any point after its shown) + if (SettingsConfig.getDirectValue("files.autoSave") === "off") { + await vscode.commands.executeCommand("workbench.action.toggleAutoSave"); + } + }); +}