Skip to content

Commit e17e752

Browse files
traeokzFernand0
andauthored
refactor: remove dependency on tmp (#3805)
* refactor: remove dependency on tmp in FTP extension Signed-off-by: Trae Yelovich <[email protected]> * chore: update changelog Signed-off-by: Trae Yelovich <[email protected]> * fix linter errors Signed-off-by: Trae Yelovich <[email protected]> * tests: refactor upload USS directory test Signed-off-by: Trae Yelovich <[email protected]> * fix(ftp/UssApi): path building in uploadDirectory Signed-off-by: Trae Yelovich <[email protected]> * chore: add bug fix to changelog Signed-off-by: Trae Yelovich <[email protected]> * fix: do not replace double backslashes in local path Signed-off-by: Trae Yelovich <[email protected]> * chore: add second bugfix to changelog Signed-off-by: Trae Yelovich <[email protected]> * tests: add case to make sure slashes are preserved Signed-off-by: Trae Yelovich <[email protected]> * Update tmp version to avoid vuln Signed-off-by: Trae Yelovich <[email protected]> * test: Mock imperative.IO.isDir as the dir doesn't always exist Signed-off-by: Trae Yelovich <[email protected]> * refactor: remove dep on tmp in downloadLocalizations.js Signed-off-by: Trae Yelovich <[email protected]> * fix: add back regex for replacing back slashes Signed-off-by: Trae Yelovich <[email protected]> * chore: remove entry from changelog Signed-off-by: Trae Yelovich <[email protected]> * refactor: only rmSync tmpDir Signed-off-by: Trae Yelovich <[email protected]> --------- Signed-off-by: Trae Yelovich <[email protected]> Co-authored-by: Fernando Rijo Cedeno <[email protected]>
1 parent 32f52be commit e17e752

File tree

9 files changed

+139
-72
lines changed

9 files changed

+139
-72
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@ All notable changes to the "zowe-explorer-ftp-extension" extension will be docum
55
### New features and enhancements
66

77
- Changed the response field names for the `FtpMvsApi.allMembers` function from "created" and "changed" to "c4date" and "m4date" to be consistent with the z/OSMF API response format for a PDS member list. [#3751](https://github.com/zowe/zowe-explorer-vscode/issues/3751)
8+
- Replaced use of `tmp` dependency with Node.js functions for temporary file creation when downloading data sets and USS file contents. [#3805](https://github.com/zowe/zowe-explorer-vscode/pull/3805)
89

910
### Bug fixes
1011

1112
- Fixed an issue where USS directories could not be loaded via FTP as virtual workspaces. [#3763](https://github.com/zowe/zowe-explorer-vscode/pull/3763)
13+
- Fixed an issue where the `UssApi.uploadDirectory` function did not properly parse the file name from the input file paths, causing incorrect file paths to be used for the USS destination path. [#3805](https://github.com/zowe/zowe-explorer-vscode/pull/3805)
1214

1315
## `3.2.2`
1416

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

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
import { FtpMvsApi } from "../../../src/ZoweExplorerFtpMvsApi";
1717
import { DataSetUtils } from "@zowe/zos-ftp-for-zowe-cli";
1818
import TestUtils from "../utils/TestUtils";
19-
import * as tmp from "tmp";
19+
import * as path from "path";
20+
import * as os from "os";
21+
import * as crypto from "crypto";
2022
import { Gui, imperative } from "@zowe/zowe-explorer-api";
2123
import * as globals from "../../../src/globals";
2224
import { ZoweFtpExtensionError } from "../../../src/ZoweFtpExtensionError";
@@ -35,6 +37,12 @@ const fs = require("fs");
3537

3638
fs.createReadStream = jest.fn().mockReturnValue(readableStream);
3739

40+
// Helper function to create temporary file names using Node.js built-ins
41+
function createTempFileName(): string {
42+
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "zowe-test-mvs-"));
43+
return path.join(tmpDir, `temp-${crypto.randomUUID()}.dat`);
44+
}
45+
3846
describe("FtpMvsApi", () => {
3947
let MvsApi: FtpMvsApi;
4048
beforeEach(() => {
@@ -112,7 +120,7 @@ describe("FtpMvsApi", () => {
112120
});
113121

114122
it("should view dataset content.", async () => {
115-
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
123+
const localFile = createTempFileName();
116124
const response = TestUtils.getSingleLineStream();
117125
DataSetUtils.downloadDataSet = jest.fn().mockReturnValue(response);
118126

@@ -134,8 +142,8 @@ describe("FtpMvsApi", () => {
134142
});
135143

136144
it("should upload content to dataset - sequential data set", async () => {
137-
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
138-
const tmpNameSyncSpy = jest.spyOn(tmp, "tmpNameSync");
145+
const localFile = createTempFileName();
146+
const mkdtempSyncSpy = jest.spyOn(fs, "mkdtempSync");
139147
const rmSyncSpy = jest.spyOn(fs, "rmSync");
140148

141149
fs.writeFileSync(localFile, "hello");
@@ -158,14 +166,14 @@ describe("FtpMvsApi", () => {
158166
expect(DataSetUtils.listDataSets).toHaveBeenCalledTimes(1);
159167
expect(DataSetUtils.uploadDataSet).toHaveBeenCalledTimes(1);
160168
expect(MvsApi.releaseConnection).toHaveBeenCalled();
161-
// check that correct function is called from node-tmp
162-
expect(tmpNameSyncSpy).toHaveBeenCalled();
169+
// check that correct Node.js built-in functions are called
170+
expect(mkdtempSyncSpy).toHaveBeenCalled();
163171
expect(rmSyncSpy).toHaveBeenCalled();
164172
});
165173

166174
it("should generate a member name for PDS upload if one wasn't provided", async () => {
167-
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
168-
const tmpNameSyncSpy = jest.spyOn(tmp, "tmpNameSync");
175+
const localFile = createTempFileName();
176+
const mkdtempSyncSpy = jest.spyOn(fs, "mkdtempSync");
169177
const rmSyncSpy = jest.spyOn(fs, "rmSync");
170178

171179
fs.writeFileSync(localFile, "helloPdsMember");
@@ -190,13 +198,13 @@ describe("FtpMvsApi", () => {
190198
expect(dataSetMock).toHaveBeenCalledTimes(1);
191199
expect(uploadDataSetMock).toHaveBeenCalledTimes(1);
192200
expect(MvsApi.releaseConnection).toHaveBeenCalled();
193-
// check that correct function is called from node-tmp
194-
expect(tmpNameSyncSpy).toHaveBeenCalled();
201+
// check that correct Node.js built-in functions are called
202+
expect(mkdtempSyncSpy).toHaveBeenCalled();
195203
expect(rmSyncSpy).toHaveBeenCalled();
196204
});
197205

198206
it("should upload single space to dataset when secureFtp is true and contents are empty", async () => {
199-
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
207+
const localFile = createTempFileName();
200208

201209
fs.writeFileSync(localFile, "");
202210
const response = TestUtils.getSingleLineStream();
@@ -233,7 +241,7 @@ describe("FtpMvsApi", () => {
233241
});
234242

235243
it("should upload single space to dataset when secureFtp is true and contents are empty", async () => {
236-
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
244+
const localFile = createTempFileName();
237245

238246
fs.writeFileSync(localFile, "");
239247
const response = TestUtils.getSingleLineStream();
@@ -433,7 +441,7 @@ describe("FtpMvsApi", () => {
433441
throw new Error("Upload dataset failed.");
434442
})
435443
);
436-
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
444+
const localFile = createTempFileName();
437445
const mockParams = {
438446
inputFilePath: localFile,
439447
dataSetName: "IBMUSER.DS2",
@@ -514,7 +522,7 @@ describe("FtpMvsApi", () => {
514522
return {
515523
processNewlinesSpy: jest.spyOn(imperative.IO, "processNewlines"),
516524
putContents: jest.spyOn(MvsApi, "putContents").mockImplementation(),
517-
tmpFileSyncMock: jest.spyOn(tmp, "fileSync").mockReturnValueOnce({ fd: 12345 } as any),
525+
mkdtempSyncMock: jest.spyOn(fs, "mkdtempSync").mockReturnValueOnce("/tmp/zowe-test-mvs-12345"),
518526
writeSyncMock: jest.spyOn(fs, "writeSync").mockImplementation(),
519527
};
520528
}

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

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import TestUtils from "../utils/TestUtils";
1919
import * as zosfiles from "@zowe/zos-files-for-zowe-sdk";
2020
import * as globals from "../../../src/globals";
2121
import { ZoweFtpExtensionError } from "../../../src/ZoweFtpExtensionError";
22-
import * as tmp from "tmp";
22+
import * as path from "path";
23+
import * as os from "os";
24+
import * as crypto from "crypto";
2325
import { imperative } from "@zowe/zowe-explorer-api";
2426
import { mocked } from "../../../__mocks__/mockUtils";
2527

@@ -34,6 +36,12 @@ const fs = require("fs");
3436

3537
fs.createReadStream = jest.fn().mockReturnValue(readableStream);
3638

39+
// Helper function to create temporary file names using Node.js built-ins
40+
function createTempFileName(): string {
41+
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "zowe-test-uss-"));
42+
return path.join(tmpDir, `temp-${crypto.randomUUID()}.dat`);
43+
}
44+
3745
describe("FtpUssApi", () => {
3846
let UssApi: FtpUssApi;
3947
beforeEach(() => {
@@ -67,7 +75,7 @@ describe("FtpUssApi", () => {
6775
});
6876

6977
it("should view uss files.", async () => {
70-
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
78+
const localFile = createTempFileName();
7179
const response = TestUtils.getSingleLineStream();
7280
UssUtils.downloadFile = jest.fn().mockReturnValue(response);
7381

@@ -98,10 +106,10 @@ describe("FtpUssApi", () => {
98106
});
99107

100108
it("should upload uss files.", async () => {
101-
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
109+
const localFile = createTempFileName();
102110
const response = TestUtils.getSingleLineStream();
103111
UssUtils.uploadFile = jest.fn().mockReturnValue(response);
104-
const tmpNameSyncSpy = jest.spyOn(tmp, "tmpNameSync");
112+
const mkdtempSyncSpy = jest.spyOn(fs, "mkdtempSync");
105113
const rmSyncSpy = jest.spyOn(fs, "rmSync");
106114
jest.spyOn(UssApi, "getContents").mockResolvedValue({ apiResponse: { etag: "test" } } as any);
107115
const mockParams = {
@@ -122,26 +130,61 @@ describe("FtpUssApi", () => {
122130
expect(UssUtils.downloadFile).toHaveBeenCalledTimes(1);
123131
expect(UssUtils.uploadFile).toHaveBeenCalledTimes(1);
124132
expect(UssApi.releaseConnection).toHaveBeenCalled();
125-
// check that correct function is called from node-tmp
126-
expect(tmpNameSyncSpy).toHaveBeenCalled();
133+
// check that correct Node.js built-in functions are called
134+
expect(mkdtempSyncSpy).toHaveBeenCalled();
127135
expect(rmSyncSpy).toHaveBeenCalled();
128136
});
129137

130138
it("should upload uss directory.", async () => {
131139
const localpath = "/tmp";
132-
const files = ["file1", "file2"];
133-
zosfiles.ZosFilesUtils.getFileListFromPath = jest.fn().mockReturnValue(files);
140+
const files = ["/tmp/file1", "/tmp/file2"];
141+
const isDirMock = jest.spyOn(imperative.IO, "isDir").mockReturnValue(true);
142+
const getFileListFromPathMock = jest.spyOn(zosfiles.ZosFilesUtils, "getFileListFromPath").mockReturnValue(files);
134143
const mockParams = {
135144
inputDirectoryPath: localpath,
136145
ussDirectoryPath: "/a/b/c",
137146
options: {},
138147
};
139148
const response = {};
140-
jest.spyOn(UssApi, "putContent").mockResolvedValue(response as any);
149+
const putContentMock = jest
150+
.spyOn(UssApi, "putContent")
151+
.mockReset()
152+
.mockResolvedValue(response as any);
141153
await UssApi.uploadDirectory(mockParams.inputDirectoryPath, mockParams.ussDirectoryPath, mockParams.options);
142-
expect(UssApi.putContent).toHaveBeenCalledTimes(3);
154+
// putContent is called for each file in the directory
155+
expect(putContentMock).toHaveBeenCalledWith("/tmp/file1", "/a/b/c/file1");
156+
expect(putContentMock).toHaveBeenCalledWith("/tmp/file2", "/a/b/c/file2");
157+
expect(getFileListFromPathMock).toHaveBeenCalledTimes(1);
158+
expect(isDirMock).toHaveBeenCalledTimes(1);
159+
expect(isDirMock).toHaveBeenCalledWith(localpath);
160+
isDirMock.mockRestore();
143161
});
144162

163+
if (os.platform() === "win32") {
164+
it("should upload uss directory on Windows with proper input file paths.", async () => {
165+
const localpath = "C:\\Windows\\Temp";
166+
const files = ["C:\\Windows\\Temp\\file1", "C:\\Windows\\Temp\\file2"];
167+
// Mock out the isDir result in case the drive letter is not present on the system where the test is running
168+
jest.spyOn(imperative.IO, "isDir").mockReturnValueOnce(true);
169+
const getFileListFromPathMock = jest.spyOn(zosfiles.ZosFilesUtils, "getFileListFromPath").mockReturnValue(files);
170+
const mockParams = {
171+
inputDirectoryPath: localpath,
172+
ussDirectoryPath: "/a/b/c",
173+
options: {},
174+
};
175+
const response = {};
176+
const putContentMock = jest
177+
.spyOn(UssApi, "putContent")
178+
.mockReset()
179+
.mockResolvedValue(response as any);
180+
await UssApi.uploadDirectory(mockParams.inputDirectoryPath, mockParams.ussDirectoryPath, mockParams.options);
181+
// putContent is called for each file in the directory
182+
expect(putContentMock).toHaveBeenCalledWith("C:\\Windows\\Temp\\file1", "/a/b/c/file1");
183+
expect(putContentMock).toHaveBeenCalledWith("C:\\Windows\\Temp\\file2", "/a/b/c/file2");
184+
expect(getFileListFromPathMock).toHaveBeenCalledTimes(1);
185+
});
186+
}
187+
145188
it("should create uss directory.", async () => {
146189
UssUtils.makeDirectory = jest.fn();
147190
UssUtils.uploadFile = jest.fn();
@@ -229,7 +272,7 @@ describe("FtpUssApi", () => {
229272
jest.spyOn(UssUtils, "downloadFile").mockImplementationOnce(() => {
230273
throw new Error("Download file failed.");
231274
});
232-
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
275+
const localFile = createTempFileName();
233276
const mockParams = {
234277
ussFilePath: "/a/b/d.txt",
235278
options: {
@@ -313,7 +356,7 @@ describe("FtpUssApi", () => {
313356
return {
314357
processNewlinesSpy: jest.spyOn(imperative.IO, "processNewlines"),
315358
putContent: jest.spyOn(UssApi, "putContent").mockImplementation(),
316-
tmpFileSyncMock: jest.spyOn(tmp, "fileSync").mockReturnValueOnce({ fd: 12345 } as any),
359+
mkdtempSyncMock: jest.spyOn(fs, "mkdtempSync").mockReturnValueOnce("/tmp/zowe-test-uss-12345"),
317360
writeSyncMock: jest.spyOn(fs, "writeSync").mockImplementation(),
318361
};
319362
}

packages/zowe-explorer-ftp-extension/package.json

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,7 @@
5555
"@zowe/zos-files-for-zowe-sdk": "^8.24.1",
5656
"@zowe/zos-ftp-for-zowe-cli": "^3.0.0",
5757
"@zowe/zos-jobs-for-zowe-sdk": "^8.24.1",
58-
"@zowe/zowe-explorer-api": "3.3.0-SNAPSHOT",
59-
"tmp": "0.2.3"
60-
},
61-
"devDependencies": {
62-
"@types/tmp": "0.2.6"
58+
"@zowe/zowe-explorer-api": "3.3.0-SNAPSHOT"
6359
},
6460
"jest": {
6561
"moduleFileExtensions": [

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

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,14 @@
1111

1212
import * as fs from "fs";
1313
import * as crypto from "crypto";
14-
import * as tmp from "tmp";
14+
import * as path from "path";
15+
import * as os from "os";
1516

1617
import * as zosfiles from "@zowe/zos-files-for-zowe-sdk";
1718
import { BufferBuilder, Gui, imperative, MainframeInteraction, MessageSeverity } from "@zowe/zowe-explorer-api";
1819
import { CoreUtils, DataSetUtils } from "@zowe/zos-ftp-for-zowe-cli";
1920
import { AbstractFtpApi } from "./ZoweExplorerAbstractFtpApi";
20-
import * as globals from "./globals";
21+
import { LOGGER } from "./globals";
2122
import { ZoweFtpExtensionError } from "./ZoweFtpExtensionError";
2223
// The Zowe FTP CLI plugin is written and uses mostly JavaScript, so relax the rules here.
2324
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
@@ -117,7 +118,7 @@ export class FtpMvsApi extends AbstractFtpApi implements MainframeInteraction.IM
117118
try {
118119
connection = await this.ftpClient(this.checkedProfile());
119120
if (!connection || !fileOrStreamSpecified) {
120-
globals.LOGGER.logImperativeMessage(result.commandResponse, MessageSeverity.ERROR);
121+
LOGGER.logImperativeMessage(result.commandResponse, MessageSeverity.ERROR);
121122
throw new Error(result.commandResponse);
122123
}
123124
if (options.file) {
@@ -181,7 +182,7 @@ export class FtpMvsApi extends AbstractFtpApi implements MainframeInteraction.IM
181182
try {
182183
connection = await this.ftpClient(profile);
183184
if (!connection) {
184-
globals.LOGGER.logImperativeMessage(result.commandResponse, MessageSeverity.ERROR);
185+
LOGGER.logImperativeMessage(result.commandResponse, MessageSeverity.ERROR);
185186
throw new Error(result.commandResponse);
186187
}
187188
const lrecl: number = dsAtrribute.apiResponse.items[0].lrecl;
@@ -404,14 +405,27 @@ export class FtpMvsApi extends AbstractFtpApi implements MainframeInteraction.IM
404405
return loadResult.apiResponse.etag as string;
405406
}
406407

407-
const tmpFileName = tmp.tmpNameSync();
408-
const options: zosfiles.IDownloadOptions = {
409-
binary: false,
410-
file: tmpFileName,
411-
};
412-
const loadResult = await this.getContents(dataSetName, options);
413-
fs.rmSync(tmpFileName, { force: true });
414-
return loadResult.apiResponse.etag as string;
408+
// Create a temporary directory and unique filename
409+
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "zowe-ftp-mvs-"));
410+
const tmpFileName = path.join(tmpDir, `temp-${crypto.randomUUID()}.dat`);
411+
412+
try {
413+
const options: zosfiles.IDownloadOptions = {
414+
binary: false,
415+
file: tmpFileName,
416+
};
417+
const loadResult = await this.getContents(dataSetName, options);
418+
return loadResult.apiResponse.etag as string;
419+
} finally {
420+
// Clean up temporary file and directory
421+
try {
422+
fs.rmSync(tmpDir, { force: true, recursive: true });
423+
} catch (cleanupError) {
424+
if (cleanupError instanceof Error) {
425+
LOGGER.logImperativeMessage(`Failed to clean up temporary files: ${cleanupError.message}`, MessageSeverity.WARN);
426+
}
427+
}
428+
}
415429
}
416430
private getDefaultResponse(): zosfiles.IZosFilesResponse {
417431
return {

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

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import * as fs from "fs";
1313
import * as path from "path";
1414
import * as crypto from "crypto";
15-
import * as tmp from "tmp";
15+
import * as os from "os";
1616
import * as zosfiles from "@zowe/zos-files-for-zowe-sdk";
1717

1818
import { CoreUtils, UssUtils, TransferMode } from "@zowe/zos-ftp-for-zowe-cli";
@@ -181,7 +181,7 @@ export class FtpUssApi extends AbstractFtpApi implements MainframeInteraction.IU
181181
await this.putContent(inputDirectoryPath, ussDirectoryPath);
182182

183183
// getting list of files from directory
184-
const files = zosfiles.ZosFilesUtils.getFileListFromPath(inputDirectoryPath, false);
184+
const files = zosfiles.ZosFilesUtils.getFileListFromPath(inputDirectoryPath, true);
185185
// TODO: this solution will not perform very well; rewrite this and putContent methods
186186
for (const file of files) {
187187
const relativePath = path.relative(inputDirectoryPath, file).replace(/\\/g, "/");
@@ -285,14 +285,27 @@ export class FtpUssApi extends AbstractFtpApi implements MainframeInteraction.IU
285285
return loadResult.apiResponse.etag as string;
286286
}
287287

288-
const tmpFileName = tmp.tmpNameSync();
289-
const options: zosfiles.IDownloadOptions = {
290-
binary: false,
291-
file: tmpFileName,
292-
};
293-
const loadResult = await this.getContents(ussFilePath, options);
294-
fs.rmSync(tmpFileName, { force: true });
295-
return loadResult.apiResponse.etag as string;
288+
// Create a temporary directory and unique filename
289+
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "zowe-ftp-uss-"));
290+
const tmpFileName = path.join(tmpDir, `temp-${crypto.randomUUID()}.dat`);
291+
292+
try {
293+
const options: zosfiles.IDownloadOptions = {
294+
binary: false,
295+
file: tmpFileName,
296+
};
297+
const loadResult = await this.getContents(ussFilePath, options);
298+
return loadResult.apiResponse.etag as string;
299+
} finally {
300+
// Clean up temporary file and directory
301+
try {
302+
fs.rmSync(tmpDir, { force: true, recursive: true });
303+
} catch (cleanupError) {
304+
if (cleanupError instanceof Error) {
305+
LOGGER.logImperativeMessage(`Failed to clean up temporary files: ${cleanupError.message}`, MessageSeverity.WARN);
306+
}
307+
}
308+
}
296309
}
297310

298311
private getDefaultResponse(): zosfiles.IZosFilesResponse {

packages/zowe-explorer/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2154,7 +2154,6 @@
21542154
"log4js": "^6.4.6",
21552155
"markdownlint-cli": "^0.33.0",
21562156
"sinon": "^19.0.2",
2157-
"tmp": "^0.2.3",
21582157
"ts-node": "^10.9.2",
21592158
"wdio-vscode-service": "^6.1.3",
21602159
"wdio-wait-for": "^3.0.11",

0 commit comments

Comments
 (0)