Merge pull request #2316 from zowe/fix/etag-mismatch

Fix Etag Mismatch auto saving

packages/zowe-explorer-ftp-extension/CHANGELOG.md

@@ -6,6 +6,8 @@ All notable changes to the "zowe-explorer-ftp-extension" extension will be docum
 
 ### Bug fixes
 
+- Fixed an issue with mismatch etag, correcting error message sent to Zowe Explorer to trigger diff editor. [#2277](https://github.com/zowe/vscode-extension-for-zowe/issues/2277)
+
 ## `2.8.1`
 
 ### Bug fixes

packages/zowe-explorer-ftp-extension/src/ZoweExplorerFtpMvsApi.ts

@@ -138,11 +138,9 @@ export class FtpMvsApi extends AbstractFtpApi implements ZoweExplorerApi.IMvs {
             if (options.returnEtag && options.etag) {
                 const contentsTag = await this.getContentsTag(dataSetName);
                 if (contentsTag && contentsTag !== options.etag) {
-                    // TODO: extension.ts should not check for zosmf errors.
-                    await Gui.errorMessage("Save conflict. Please pull the latest content from mainframe first.", {
-                        logger: ZoweLogger,
-                    });
-                    throw new Error();
+                    result.success = false;
+                    result.commandResponse = "Rest API failure with HTTP(S) status 412 Save conflict.";
+                    return result;
                 }
             }
             const lrecl: number = dsAtrribute.apiResponse.items[0].lrecl;

packages/zowe-explorer-ftp-extension/src/ZoweExplorerFtpUssApi.ts

@@ -130,10 +130,7 @@ export class FtpUssApi extends AbstractFtpApi implements ZoweExplorerApi.IUss {
             if (returnEtag && etag) {
                 const contentsTag = await this.getContentsTag(ussFilePath);
                 if (contentsTag && contentsTag !== etag) {
-                    await Gui.errorMessage("Save conflict. Please pull the latest content from mainframe first.", {
-                        logger: ZoweLogger,
-                    });
-                    throw new Error();
+                    throw new Error("Rest API failure with HTTP(S) status 412 Save conflict.");
                 }
             }
             await UssUtils.uploadFile(connection, ussFilePath, transferOptions);

packages/zowe-explorer/CHANGELOG.md

@@ -17,6 +17,13 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen
 - Fixed issue where user was not able to view job spool file with the same DD name in different steps because of duplicated local file name. [#2279](https://github.com/zowe/vscode-extension-for-zowe/issues/2279)
 - Fixed issue where user was not able to view job spool file from jobs with duplicated step names because of duplicated local file name. [#2315](https://github.com/zowe/vscode-extension-for-zowe/issues/2315)
 - Fixed issue with Windows path when uploading file to data set. [#2323](https://github.com/zowe/vscode-extension-for-zowe/issues/2323)
+- Fixed an issue with mismatch etag error returned not triggering the diff editor and possible loss of data due to the issue. [#2277](https://github.com/zowe/vscode-extension-for-zowe/issues/2277)
+
+## `2.8.2`
+
+### Bug fixes
+
+- Fixed `zowe.settings.version` being added to settings.json in workspaces. [#2312](https://github.com/zowe/vscode-extension-for-zowe/issues/2312)
 
 ## `2.8.1`
 

packages/zowe-explorer/__tests__/__unit__/dataset/actions.unit.test.ts

@@ -38,9 +38,11 @@ import * as fs from "fs";
 import * as sharedUtils from "../../../src/shared/utils";
 import { Profiles } from "../../../src/Profiles";
 import * as utils from "../../../src/utils/ProfilesUtils";
-
+import * as wsUtils from "../../../src/utils/workspace";
 import { getNodeLabels } from "../../../src/dataset/utils";
 import { ZoweLogger } from "../../../src/utils/LoggerUtils";
+import * as context from "../../../src/shared/context";
+import { ZoweExplorerApiRegister } from "../../../src/ZoweExplorerApiRegister";
 
 // Missing the definition of path module, because I need the original logic for tests
 jest.mock("fs");
@@ -1393,30 +1395,39 @@ describe("Dataset Actions Unit Tests - Function saveFile", () => {
             commandResponse: "Rest API failure with HTTP(S) status 412",
             apiResponse: [],
         });
-        mocked(zowe.Download.dataSet).mockResolvedValueOnce({
-            success: true,
-            commandResponse: "",
-            apiResponse: {
-                etag: "",
-            },
-        });
+
         mocked(vscode.window.withProgress).mockImplementation((progLocation, callback) => {
             return callback();
         });
         const profile = blockMocks.imperativeProfile;
-        const mainframeCodePage = 1047;
-        profile.profile.encoding = mainframeCodePage;
+        profile.profile.encoding = 1047;
         blockMocks.profileInstance.loadNamedProfile.mockReturnValueOnce(blockMocks.imperativeProfile);
         mocked(Profiles.getInstance).mockReturnValue(blockMocks.profileInstance);
+        Object.defineProperty(wsUtils, "markDocumentUnsaved", {
+            value: jest.fn(),
+            configurable: true,
+        });
+        Object.defineProperty(context, "isTypeUssTreeNode", {
+            value: jest.fn().mockReturnValueOnce(false),
+            configurable: true,
+        });
+        Object.defineProperty(ZoweExplorerApiRegister.getMvsApi, "getContents", {
+            value: jest.fn(),
+            configurable: true,
+        });
+
         const testDocument = createTextDocument("HLQ.TEST.AFILE", blockMocks.datasetSessionNode);
         (testDocument as any).fileName = path.join(globals.DS_DIR, testDocument.fileName);
+        const logSpy = jest.spyOn(ZoweLogger, "warn");
+        const commandSpy = jest.spyOn(vscode.commands, "executeCommand");
 
         await dsActions.saveFile(testDocument, blockMocks.testDatasetTree);
 
-        expect(mocked(Gui.warningMessage)).toBeCalledWith(
-            "Remote file has been modified in the meantime.\nSelect 'Compare' to resolve the conflict."
-        );
+        expect(logSpy).toBeCalledWith("Remote file has changed. Presenting with way to resolve file.");
         expect(mocked(sharedUtils.concatChildNodes)).toBeCalled();
+        expect(commandSpy).toBeCalledWith("workbench.files.action.compareWithSaved");
+        logSpy.mockClear();
+        commandSpy.mockClear();
     });
 });
 

packages/zowe-explorer/__tests__/__unit__/uss/actions.unit.test.ts

@@ -37,6 +37,8 @@ import * as fs from "fs";
 import { createUssApi, bindUssApi } from "../../../__mocks__/mockCreators/api";
 import * as refreshActions from "../../../src/shared/refresh";
 import { ZoweLogger } from "../../../src/utils/LoggerUtils";
+import * as wsUtils from "../../../src/utils/workspace";
+import * as context from "../../../src/shared/context";
 
 function createGlobalMocks() {
     const globalMocks = {
@@ -511,15 +513,26 @@ describe("USS Action Unit Tests - Function saveUSSFile", () => {
 
         globalMocks.withProgress.mockRejectedValueOnce(Error("Rest API failure with HTTP(S) status 412"));
         globalMocks.ussFile.mockResolvedValueOnce(downloadResponse);
+        Object.defineProperty(wsUtils, "markDocumentUnsaved", {
+            value: jest.fn(),
+            configurable: true,
+        });
+        Object.defineProperty(context, "isTypeUssTreeNode", {
+            value: jest.fn().mockReturnValueOnce(true),
+            configurable: true,
+        });
+        const logSpy = jest.spyOn(ZoweLogger, "warn");
+        const commandSpy = jest.spyOn(vscode.commands, "executeCommand");
 
         try {
             await ussNodeActions.saveUSSFile(blockMocks.testDoc, blockMocks.testUSSTree);
         } catch (e) {
             expect(e.message).toBe("vscode.Position is not a constructor");
         }
-        expect(globalMocks.showWarningMessage.mock.calls[0][0]).toBe(
-            "Remote file has been modified in the meantime.\nSelect 'Compare' to resolve the conflict."
-        );
+        expect(logSpy).toBeCalledWith("Remote file has changed. Presenting with way to resolve file.");
+        expect(commandSpy).toBeCalledWith("workbench.files.action.compareWithSaved");
+        logSpy.mockClear();
+        commandSpy.mockClear();
     });
 });
 

packages/zowe-explorer/__tests__/__unit__/utils/SettingsConfig.unit.test.ts

@@ -109,36 +109,56 @@ describe("SettingsConfig Unit Tests - function standardizeSettings", () => {
             value: ["test"],
             configurable: true,
         });
-        jest.spyOn(SettingsConfig as any, "currentVersionNumber", "get").mockReturnValueOnce("vtest");
+        jest.spyOn(SettingsConfig as any, "currentVersionNumber", "get").mockReturnValueOnce("v2");
         jest.spyOn(SettingsConfig as any, "zoweOldConfigurations", "get").mockReturnValue(["zowe.settings.test"]);
         jest.spyOn(vscode.workspace, "getConfiguration").mockReturnValue({
             update: jest.fn(),
         } as any);
     });
 
+    afterEach(() => {
+        jest.clearAllMocks();
+    });
+
     it("should standardize workspace settings if not migrated and workspace is open", async () => {
         jest.spyOn(SettingsConfig as any, "configurations", "get").mockReturnValue({
             inspect: () => ({
-                globalValue: "vtest",
-                workspaceValue: "",
+                globalValue: "v2",
+                workspaceValue: "v1",
             }),
         });
+        const setDirectValueSpy = jest.spyOn(SettingsConfig as any, "setDirectValue").mockImplementation();
         const standardizeWorkspaceSettingsSpy = jest.spyOn(SettingsConfig as any, "standardizeWorkspaceSettings").mockImplementation();
         await expect(SettingsConfig.standardizeSettings()).resolves.not.toThrow();
+        expect(setDirectValueSpy).toHaveBeenCalledTimes(1);
         expect(standardizeWorkspaceSettingsSpy).toHaveBeenCalledTimes(1);
     });
 
     it("should standardize global settings if not migrated", async () => {
         jest.spyOn(SettingsConfig as any, "configurations", "get").mockReturnValue({
             inspect: () => ({
-                globalValue: "",
-                workspaceValue: "vtest",
+                globalValue: "v1",
+                workspaceValue: "v2",
             }),
         });
+        const setDirectValueSpy = jest.spyOn(SettingsConfig as any, "setDirectValue").mockImplementation();
         const standardizeGlobalSettingsSpy = jest.spyOn(SettingsConfig as any, "standardizeGlobalSettings").mockImplementation();
         await expect(SettingsConfig.standardizeSettings()).resolves.not.toThrow();
+        expect(setDirectValueSpy).toHaveBeenCalledTimes(1);
         expect(standardizeGlobalSettingsSpy).toHaveBeenCalledTimes(1);
     });
+
+    it("should not update settings if global is migrated and workspace is undefined", async () => {
+        jest.spyOn(SettingsConfig as any, "configurations", "get").mockReturnValue({
+            inspect: () => ({
+                globalValue: "v2",
+                workspaceValue: undefined,
+            }),
+        });
+        const setDirectValueSpy = jest.spyOn(SettingsConfig as any, "setDirectValue").mockImplementation();
+        await expect(SettingsConfig.standardizeSettings()).resolves.not.toThrow();
+        expect(setDirectValueSpy).toHaveBeenCalledTimes(0);
+    });
 });
 
 describe("SettingsConfig Unit Tests - function currentVersionNumber", () => {

packages/zowe-explorer/i18n/sample/src/dataset/actions.i18n.json

@@ -79,8 +79,6 @@
   "saveFile.saving": "Saving file {0}",
   "saveFile.saveFailed.error": "Data set failed to save. Data set may have been deleted or renamed on mainframe.",
   "saveFile.progress.title": "Saving data set...",
-  "saveFile.etagMismatch.log.warning": "Remote file has changed. Presented with way to resolve file.",
-  "saveFile.etagMismatch.warning": "Remote file has been modified in the meantime.\nSelect 'Compare' to resolve the conflict.",
   "pasteDataSetMembers.paste.error": "Invalid paste. Copy data set(s) first.",
   "downloadDs.invalidNode.error": "Cannot download, item invalid.",
   "copySequentialDatasets.notSupported.error": "Copying data sets is not supported.",

packages/zowe-explorer/i18n/sample/src/shared/utils.i18n.json

@@ -12,5 +12,6 @@
   "saveFile.overwriteConfirmation.no": "No",
   "uploadContent.cancelled": "Upload cancelled.",
   "searchJobs.owner.invalid": "Invalid job owner",
-  "searchJobs.prefix.invalid": "Invalid job prefix"
+  "searchJobs.prefix.invalid": "Invalid job prefix",
+  "saveFile.etagMismatch.log.warning": "Remote file has changed. Presenting with way to resolve file."
 }

packages/zowe-explorer/i18n/sample/src/uss/actions.i18n.json

@@ -8,7 +8,6 @@
   "copyPath.infoMessage": "Copy Path is not yet supported in Theia.",
   "saveUSSFile.log.debug.saveRequest": "save requested for USS file ",
   "saveUSSFile.response.title": "Saving file...",
-  "saveFile.error.etagMismatch": "Remote file has been modified in the meantime.\nSelect 'Compare' to resolve the conflict.",
   "deleteUssPrompt.confirmation.delete": "Delete",
   "deleteUssPrompt.confirmation.message": "Are you sure you want to delete the following item?\nThis will permanently remove the following file or folder from your system.\n\n{0}",
   "deleteUssPrompt.confirmation.cancel.log.debug": "Delete action was canceled.",

packages/zowe-explorer/src/dataset/actions.ts

@@ -21,11 +21,11 @@ import {
     getDocumentFilePath,
     concatChildNodes,
     checkForAddedSuffix,
-    willForceUpload,
     getSelectedNodeList,
     JobSubmitDialogOpts,
     JOB_SUBMIT_DIALOG_OPTS,
     getDefaultUri,
+    compareFileContent,
 } from "../shared/utils";
 import { ZoweExplorerApiRegister } from "../ZoweExplorerApiRegister";
 import { Profiles } from "../Profiles";
@@ -1609,43 +1609,7 @@ export async function saveFile(doc: vscode.TextDocument, datasetProvider: api.IZ
                 setFileSaved(true);
             }
         } else if (!uploadResponse.success && uploadResponse.commandResponse.includes("Rest API failure with HTTP(S) status 412")) {
-            if (globals.ISTHEIA) {
-                willForceUpload(node, doc, label, node ? node.getProfile() : profile);
-            } else {
-                const oldDoc = doc;
-                const oldDocText = oldDoc.getText();
-                const prof = node ? node.getProfile() : profile;
-                const downloadResponse = await ZoweExplorerApiRegister.getMvsApi(prof).getContents(label, {
-                    file: doc.fileName,
-                    returnEtag: true,
-                    encoding: prof.profile?.encoding,
-                    responseTimeout: prof.profile?.responseTimeout,
-                });
-                // re-assign etag, so that it can be used with subsequent requests
-                const downloadEtag = downloadResponse.apiResponse.etag;
-                if (node && downloadEtag !== node.getEtag()) {
-                    node.setEtag(downloadEtag);
-                }
-                ZoweLogger.warn(localize("saveFile.etagMismatch.log.warning", "Remote file has changed. Presented with way to resolve file."));
-                api.Gui.warningMessage(
-                    localize(
-                        "saveFile.etagMismatch.warning",
-                        "Remote file has been modified in the meantime.\nSelect 'Compare' to resolve the conflict."
-                    )
-                );
-                if (vscode.window.activeTextEditor) {
-                    // Store document in a separate variable, to be used on merge conflict
-                    const startPosition = new vscode.Position(0, 0);
-                    const endPosition = new vscode.Position(oldDoc.lineCount, 0);
-                    const deleteRange = new vscode.Range(startPosition, endPosition);
-                    await vscode.window.activeTextEditor.edit((editBuilder) => {
-                        // re-write the old content in the editor view
-                        editBuilder.delete(deleteRange);
-                        editBuilder.insert(startPosition, oldDocText);
-                    });
-                    await vscode.window.activeTextEditor.document.save();
-                }
-            }
+            await compareFileContent(doc, node, label, null, profile);
         } else {
             await markDocumentUnsaved(doc);
             api.Gui.errorMessage(uploadResponse.commandResponse);

packages/zowe-explorer/src/shared/context.ts

@@ -11,7 +11,7 @@
 
 import * as globals from "../globals";
 import { TreeItem } from "vscode";
-import { IZoweTreeNode } from "@zowe/zowe-explorer-api";
+import { IZoweTreeNode, IZoweUSSTreeNode } from "@zowe/zowe-explorer-api";
 
 /**
  *
@@ -456,3 +456,12 @@ export function isValidationEnabled(node: TreeItem): boolean {
 export function isJobsSession(node: TreeItem): boolean {
     return new RegExp("^(" + globals.JOBS_SESSION_CONTEXT + ")").test(node.contextValue);
 }
+
+/**
+ * Helper function which identifies if the node is part of the USS tree view
+ * @param node
+ * @return true if part of the USS tree, false otherwise
+ */
+export function isTypeUssTreeNode(node): node is IZoweUSSTreeNode {
+    return (node as IZoweUSSTreeNode).getUSSDocumentFilePath !== undefined;
+}

packages/zowe-explorer/src/shared/utils.ts

@@ -21,6 +21,8 @@ import * as nls from "vscode-nls";
 import { IZosFilesResponse, imperative } from "@zowe/cli";
 import { IUploadOptions } from "@zowe/zos-files-for-zowe-sdk";
 import { ZoweLogger } from "../utils/LoggerUtils";
+import { isTypeUssTreeNode } from "./context";
+import { markDocumentUnsaved } from "../utils/workspace";
 
 // Set up localization
 nls.config({
@@ -341,3 +343,48 @@ export function jobStringValidator(text: string, localizedParam: "owner" | "pref
 export function getDefaultUri(): vscode.Uri {
     return vscode.workspace.workspaceFolders?.[0]?.uri ?? vscode.Uri.file(os.homedir());
 }
+
+/**
+ * Function that triggers compare of the old and new document in the active editor
+ * @param {vscode.TextDocument} doc - document to update and compare with previous content
+ * @param {IZoweDatasetTreeNode | IZoweUSSTreeNode} node - IZoweTreeNode
+ * @param {string} label - {optional} used by IZoweDatasetTreeNode to getContents of file
+ * @param {boolean} binary - {optional} used by IZoweUSSTreeNode to getContents of file
+ * @param {imperative.IProfileLoaded} profile - {optional}
+ * @returns {Promise<void>}
+ */
+export async function compareFileContent(
+    doc: vscode.TextDocument,
+    node: IZoweDatasetTreeNode | IZoweUSSTreeNode,
+    label?: string,
+    binary?: boolean,
+    profile?: imperative.IProfileLoaded
+): Promise<void> {
+    await markDocumentUnsaved(doc);
+    const prof = node ? node.getProfile() : profile;
+    let downloadResponse;
+
+    if (isTypeUssTreeNode(node)) {
+        downloadResponse = await ZoweExplorerApiRegister.getUssApi(prof).getContents(node.fullPath, {
+            file: node.getUSSDocumentFilePath(),
+            binary,
+            returnEtag: true,
+            encoding: prof.profile?.encoding,
+            responseTimeout: prof.profile?.responseTimeout,
+        });
+    } else {
+        downloadResponse = await ZoweExplorerApiRegister.getMvsApi(prof).getContents(label, {
+            file: doc.fileName,
+            returnEtag: true,
+            encoding: prof.profile?.encoding,
+            responseTimeout: prof.profile?.responseTimeout,
+        });
+    }
+    ZoweLogger.warn(localize("saveFile.etagMismatch.log.warning", "Remote file has changed. Presenting with way to resolve file."));
+    vscode.commands.executeCommand("workbench.files.action.compareWithSaved");
+    // re-assign etag, so that it can be used with subsequent requests
+    const downloadEtag = downloadResponse?.apiResponse?.etag;
+    if (node && downloadEtag !== node.getEtag()) {
+        node.setEtag(downloadEtag);
+    }
+}