Merge pull request #2526 from zowe/fix/ftp/temp-files

fix(FTP): One active FTP connection per operation; remove temp. files once finished

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

@@ -11,6 +11,8 @@ All notable changes to the "zowe-explorer-ftp-extension" extension will be docum
 ### Bug fixes
 
 - Fixed ECONNRESET error when trying to upload or create an empty data set member. [#2350](https://github.com/zowe/vscode-extension-for-zowe/issues/2350)
+- Fixed issue where temporary files for e-tag comparison were not deleted after use.
+- Fixed issue where another connection attempt was made inside `putContents` (in `getContentsTag`) even though a connection was already active.
 
 ## `2.11.2`
 

packages/zowe-explorer-ftp-extension/__tests__/__unit__/Mvs/ZoweExplorerFtpMvsApi.unit.test.ts

@@ -100,6 +100,8 @@ describe("FtpMvsApi", () => {
 
     it("should upload content to dataset.", async () => {
         const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
+        const tmpNameSyncSpy = jest.spyOn(tmp, "tmpNameSync");
+        const rmSyncSpy = jest.spyOn(fs, "rmSync");
 
         fs.writeFileSync(localFile, "hello");
         const response = TestUtils.getSingleLineStream();
@@ -113,14 +115,17 @@ describe("FtpMvsApi", () => {
             dataSetName: "   (IBMUSER).DS2",
             options: { encoding: "", returnEtag: true, etag: "utf8" },
         };
-        jest.spyOn(MvsApi as any, "getContentsTag").mockReturnValue(undefined);
+        jest.spyOn(MvsApi as any, "getContents").mockResolvedValueOnce({ apiResponse: { etag: "utf8" } });
         jest.spyOn(fs, "readFileSync").mockReturnValue("test");
         jest.spyOn(Gui, "warningMessage").mockImplementation();
         const result = await MvsApi.putContents(mockParams.inputFilePath, mockParams.dataSetName, mockParams.options);
         expect(result.commandResponse).toContain("Data set uploaded successfully.");
         expect(DataSetUtils.listDataSets).toBeCalledTimes(1);
         expect(DataSetUtils.uploadDataSet).toBeCalledTimes(1);
         expect(MvsApi.releaseConnection).toBeCalled();
+        // check that correct function is called from node-tmp
+        expect(tmpNameSyncSpy).toHaveBeenCalled();
+        expect(rmSyncSpy).toHaveBeenCalled();
     });
 
     it("should upload single space to dataset when secureFtp is true and contents are empty", async () => {

packages/zowe-explorer-ftp-extension/__tests__/__unit__/Uss/ZoweExplorerFtpUssApi.unit.test.ts

@@ -96,6 +96,8 @@ describe("FtpUssApi", () => {
         const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
         const response = TestUtils.getSingleLineStream();
         UssUtils.uploadFile = jest.fn().mockReturnValue(response);
+        const tmpNameSyncSpy = jest.spyOn(tmp, "tmpNameSync");
+        const rmSyncSpy = jest.spyOn(fs, "rmSync");
         jest.spyOn(UssApi, "getContents").mockResolvedValue({ apiResponse: { etag: "test" } } as any);
         const mockParams = {
             inputFilePath: localFile,
@@ -107,11 +109,14 @@ describe("FtpUssApi", () => {
             },
         };
         const result = await UssApi.putContents(mockParams.inputFilePath, mockParams.ussFilePath, undefined, undefined, "test", true);
-        jest.spyOn(UssApi as any, "getContentsTag").mockReturnValue("test");
+        jest.spyOn(UssApi as any, "getContents").mockResolvedValueOnce({ apiResponse: { etag: "test" } });
         expect(result.commandResponse).toContain("File uploaded successfully.");
         expect(UssUtils.downloadFile).toBeCalledTimes(1);
         expect(UssUtils.uploadFile).toBeCalledTimes(1);
         expect(UssApi.releaseConnection).toBeCalled();
+        // check that correct function is called from node-tmp
+        expect(tmpNameSyncSpy).toHaveBeenCalled();
+        expect(rmSyncSpy).toHaveBeenCalled();
     });
 
     it("should call putContents when calling putContent", async () => {

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

@@ -131,22 +131,24 @@ export class FtpMvsApi extends AbstractFtpApi implements ZoweExplorerApi.IMvs {
         }
         const result = this.getDefaultResponse();
         const profile = this.checkedProfile();
+
+        // Save-Save with FTP requires loading the file first
+        // (moved this block above connection request so only one connection is active at a time)
+        if (options.returnEtag && options.etag) {
+            const contentsTag = await this.getContentsTag(dataSetName);
+            if (contentsTag && contentsTag !== options.etag) {
+                result.success = false;
+                result.commandResponse = "Rest API failure with HTTP(S) status 412 Save conflict.";
+                return result;
+            }
+        }
         let connection;
         try {
             connection = await this.ftpClient(profile);
             if (!connection) {
                 ZoweLogger.logImperativeMessage(result.commandResponse, MessageSeverity.ERROR);
                 throw new Error(result.commandResponse);
             }
-            // Save-Save with FTP requires loading the file first
-            if (options.returnEtag && options.etag) {
-                const contentsTag = await this.getContentsTag(dataSetName);
-                if (contentsTag && contentsTag !== options.etag) {
-                    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;
             const data = fs.readFileSync(inputFilePath, { encoding: "utf8" });
             const transferOptions: Record<string, any> = {
@@ -178,6 +180,9 @@ export class FtpMvsApi extends AbstractFtpApi implements ZoweExplorerApi.IMvs {
             await DataSetUtils.uploadDataSet(connection, targetDataset, transferOptions);
             result.success = true;
             if (options.returnEtag) {
+                // release this connection instance because a new one will be made with getContentsTag
+                this.releaseConnection(connection);
+                connection = null;
                 const contentsTag = await this.getContentsTag(dataSetName);
                 result.apiResponse = [
                     {
@@ -367,6 +372,7 @@ export class FtpMvsApi extends AbstractFtpApi implements ZoweExplorerApi.IMvs {
         };
         const loadResult = await this.getContents(dataSetName, options);
         const etag: string = loadResult.apiResponse.etag;
+        fs.rmSync(tmpFileName, { force: true });
         return etag;
     }
     private getDefaultResponse(): zowe.IZosFilesResponse {

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

@@ -126,23 +126,27 @@ export class FtpUssApi extends AbstractFtpApi implements ZoweExplorerApi.IUss {
             localFile: inputFilePath,
         };
         const result = this.getDefaultResponse();
+        // Save-Save with FTP requires loading the file first
+        // (moved this block above connection request so only one connection is active at a time)
+        if (returnEtag && etag) {
+            const contentsTag = await this.getContentsTag(ussFilePath);
+            if (contentsTag && contentsTag !== etag) {
+                throw new Error("Rest API failure with HTTP(S) status 412 Save conflict.");
+            }
+        }
         let connection;
         try {
             connection = await this.ftpClient(this.checkedProfile());
             if (!connection) {
                 throw new Error(result.commandResponse);
             }
-            // Save-Save with FTP requires loading the file first
-            if (returnEtag && etag) {
-                const contentsTag = await this.getContentsTag(ussFilePath);
-                if (contentsTag && contentsTag !== etag) {
-                    throw new Error("Rest API failure with HTTP(S) status 412 Save conflict.");
-                }
-            }
             await UssUtils.uploadFile(connection, ussFilePath, transferOptions);
 
             result.success = true;
             if (returnEtag) {
+                // release this connection instance because a new one will be made with getContentsTag
+                this.releaseConnection(connection);
+                connection = null;
                 const contentsTag = await this.getContentsTag(ussFilePath);
                 result.apiResponse.etag = contentsTag;
             }
@@ -274,6 +278,7 @@ export class FtpUssApi extends AbstractFtpApi implements ZoweExplorerApi.IUss {
         };
         const loadResult = await this.getContents(ussFilePath, options);
         const etag: string = loadResult.apiResponse.etag;
+        fs.rmSync(tmpFileName, { force: true });
         return etag;
     }