Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ftp): Generate member name if missing in putContents #3313

Merged
merged 7 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/zowe-explorer-ftp-extension/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ All notable changes to the "zowe-explorer-ftp-extension" extension will be docum

### Bug fixes

- Fixed issue where the MVS API `putContents` function did not support PDS members when the member was not specified in the data set name. [#3305](https://github.com/zowe/zowe-explorer-vscode/issues/3305)

## `3.0.2`

## `3.0.1`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { Gui, imperative } from "@zowe/zowe-explorer-api";
import * as globals from "../../../src/globals";
import { ZoweFtpExtensionError } from "../../../src/ZoweFtpExtensionError";
import { mocked } from "../../../__mocks__/mockUtils";
import { ZosFilesUtils } from "@zowe/zos-files-for-zowe-sdk";

// two methods to mock modules: create a __mocks__ file for zowe-explorer-api.ts and direct mock for extension.ts
jest.mock("../../../__mocks__/@zowe/zowe-explorer-api.ts");
Expand Down Expand Up @@ -100,7 +101,7 @@ describe("FtpMvsApi", () => {
expect((response._readableState.buffer.head?.data ?? response._readableState.buffer).toString()).toContain("Hello world");
});

it("should upload content to dataset.", async () => {
it("should upload content to dataset - sequential data set", async () => {
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
const tmpNameSyncSpy = jest.spyOn(tmp, "tmpNameSync");
const rmSyncSpy = jest.spyOn(fs, "rmSync");
Expand All @@ -114,7 +115,7 @@ describe("FtpMvsApi", () => {

const mockParams = {
inputFilePath: localFile,
dataSetName: " (IBMUSER).DS2",
dataSetName: "IBMUSER.DS2",
options: { encoding: "", returnEtag: true, etag: "utf8" },
};
jest.spyOn(MvsApi as any, "getContents").mockResolvedValueOnce({ apiResponse: { etag: "utf8" } });
Expand All @@ -130,6 +131,38 @@ describe("FtpMvsApi", () => {
expect(rmSyncSpy).toHaveBeenCalled();
});

it("should generate a member name for PDS upload if one wasn't provided", async () => {
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });
const tmpNameSyncSpy = jest.spyOn(tmp, "tmpNameSync");
const rmSyncSpy = jest.spyOn(fs, "rmSync");

fs.writeFileSync(localFile, "helloPdsMember");
const response = TestUtils.getSingleLineStream();
const response2 = { success: true, commandResponse: "", apiResponse: { items: [{ dsname: "IBMUSER.PDS", dsorg: "PO", lrecl: 255 }] } };
const dataSetMock = jest.spyOn(MvsApi, "dataSet").mockResolvedValue(response2 as any);
const uploadDataSetMock = jest.spyOn(DataSetUtils, "uploadDataSet").mockResolvedValue(response);
jest.spyOn(MvsApi, "getContents").mockResolvedValue({ apiResponse: { etag: "123" } } as any);

const mockParams = {
inputFilePath: localFile,
dataSetName: "IBMUSER.PDS",
options: { encoding: "", returnEtag: true, etag: "utf8" },
};
const generateMemberNameSpy = jest.spyOn(ZosFilesUtils, "generateMemberName");
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(generateMemberNameSpy).toHaveBeenCalledWith(localFile);
expect(result.commandResponse).toContain("Data set uploaded successfully.");
expect(dataSetMock).toHaveBeenCalledTimes(1);
expect(uploadDataSetMock).toHaveBeenCalledTimes(1);
expect(MvsApi.releaseConnection).toHaveBeenCalled();
// 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 () => {
const localFile = tmp.tmpNameSync({ tmpdir: "/tmp" });

Expand Down
20 changes: 17 additions & 3 deletions packages/zowe-explorer-ftp-extension/src/ZoweExplorerFtpMvsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,26 @@ export class FtpMvsApi extends AbstractFtpApi implements MainframeInteraction.IM
const result = this.getDefaultResponse();
const profile = this.checkedProfile();

const dsorg = dsAtrribute.apiResponse.items[0]?.dsorg;
const isPds = dsorg === "PO" || dsorg === "PO-E";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if there are a few other kind of PDS dsorgs that we may not be considering. Also, not sure if FTP does support them (like PO-L I think)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question - we use this check in most places in the codebase, but I think we should try to cover all of the possible PDS dsorg values moving forward 😅


/**
* Determine the data set name for uploading.
*
* For PDS: When the input is a file path and the provided data set name doesn't include the member name,
* we'll need to generate a member name.
*/
const uploadName =
isPds && openParens == -1 && typeof input === "string"
? `${dataSetName}(${zosfiles.ZosFilesUtils.generateMemberName(input)})`
: dataSetName;

const inputIsBuffer = input instanceof Buffer;

// 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, inputIsBuffer);
const contentsTag = await this.getContentsTag(uploadName, inputIsBuffer);
if (contentsTag && contentsTag !== options.etag) {
throw Error("Rest API failure with HTTP(S) status 412: Save conflict");
}
Expand Down Expand Up @@ -174,13 +188,13 @@ export class FtpMvsApi extends AbstractFtpApi implements MainframeInteraction.IM
return result;
}
}
await DataSetUtils.uploadDataSet(connection, dataSetName, transferOptions);
await DataSetUtils.uploadDataSet(connection, uploadName, 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 etag = await this.getContentsTag(dataSetName, inputIsBuffer);
const etag = await this.getContentsTag(uploadName, inputIsBuffer);
result.apiResponse = {
etag,
};
Expand Down
Loading