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(list): Do not fetch directory URIs in stat #3093

Merged
merged 16 commits into from
Sep 9, 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
1 change: 1 addition & 0 deletions packages/zowe-explorer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen
- Resolved an issue where extender event callbacks were not always fired when the team configuration file was created, updated or deleted. [#3078](https://github.com/zowe/zowe-explorer-vscode/issues/3078)
- Update Zowe SDKs to `8.0.0-next.202408291544` for technical currency. [#3057](https://github.com/zowe/zowe-explorer-vscode/pull/3057)
- Fix issue with UnixCommand prompting for credentials. [#2762](https://github.com/zowe/zowe-explorer-vscode/issues/2762)
- Fixed issue where listing data sets or USS files would cause a drastic increase in API calls, causing delays or a complete halt in Zowe Explorer. [#3093](https://github.com/zowe/zowe-explorer-vscode/pull/3093)

## `3.0.0-next.202404242037`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,44 @@ describe("readFile", () => {
_lookupAsFileMock.mockRestore();
});

it("throws an error if the entry does not exist and the URI is actually a directory", async () => {
const _lookupAsFileMock = jest.spyOn(DatasetFSProvider.instance as any, "_lookupAsFile").mockImplementationOnce((uri) => {
throw FileSystemError.FileNotFound(uri as Uri);
});
const lookupParentDir = jest.spyOn(DatasetFSProvider.instance as any, "_lookupParentDirectory").mockReturnValueOnce(null);
const remoteLookupForResourceMock = jest.spyOn(DatasetFSProvider.instance, "remoteLookupForResource").mockResolvedValue(testEntries.pds);

let err;
try {
await DatasetFSProvider.instance.readFile(testUris.ps);
} catch (error) {
err = error;
expect(err.code).toBe("FileIsADirectory");
}
expect(err).toBeDefined();
expect(_lookupAsFileMock).toHaveBeenCalledWith(testUris.ps);
_lookupAsFileMock.mockRestore();
lookupParentDir.mockRestore();
remoteLookupForResourceMock.mockRestore();
});

it("throws an error if the entry does not exist and the error is not FileNotFound", async () => {
const _lookupAsFileMock = jest.spyOn(DatasetFSProvider.instance as any, "_lookupAsFile").mockImplementationOnce((uri) => {
throw FileSystemError.FileIsADirectory(uri as Uri);
});

let err;
try {
await DatasetFSProvider.instance.readFile(testUris.ps);
} catch (error) {
err = error;
expect(err.code).toBe("FileIsADirectory");
}
expect(err).toBeDefined();
expect(_lookupAsFileMock).toHaveBeenCalledWith(testUris.ps);
_lookupAsFileMock.mockRestore();
});

it("calls fetchDatasetAtUri if the entry has not yet been accessed", async () => {
const _lookupAsFileMock = jest
.spyOn(DatasetFSProvider.instance as any, "_lookupAsFile")
Expand All @@ -312,6 +350,55 @@ describe("readFile", () => {
_getInfoFromUriMock.mockRestore();
});

it("checks if parent dir exists when lookup fails & calls remoteLookupForResource if parent dir doesn't exist", async () => {
const _lookupAsFileMock = jest
.spyOn(DatasetFSProvider.instance as any, "_lookupAsFile")
.mockImplementationOnce(() => {
throw FileSystemError.FileNotFound(testUris.pdsMember);
})
.mockReturnValue(testEntries.pdsMember);

const fetchDatasetAtUriMock = jest.spyOn(DatasetFSProvider.instance, "fetchDatasetAtUri").mockImplementation();
const _lookupParentDirectoryMock = jest.spyOn(DatasetFSProvider.instance as any, "_lookupParentDirectory").mockReturnValueOnce(null);
const _getInfoFromUriMock = jest.spyOn(DatasetFSProvider.instance as any, "_getInfoFromUri").mockReturnValueOnce({
profile: testProfile,
path: "/USER.DATA.PS",
});
const remoteLookupForResourceMock = jest
.spyOn(DatasetFSProvider.instance, "remoteLookupForResource")
.mockResolvedValue(testEntries.pdsMember);

await DatasetFSProvider.instance.readFile(testUris.pdsMember);
expect(_lookupAsFileMock).toHaveBeenCalledWith(testUris.pdsMember);
expect(_lookupParentDirectoryMock).toHaveBeenCalledWith(testUris.pdsMember, true);
expect(remoteLookupForResourceMock).toHaveBeenCalledWith(testUris.pdsMember);
expect(fetchDatasetAtUriMock).toHaveBeenCalledWith(testUris.pdsMember, { isConflict: false });
_getInfoFromUriMock.mockRestore();
});

it("throws error if parent exists and file cannot be found", async () => {
const _lookupAsFileMock = jest.spyOn(DatasetFSProvider.instance as any, "_lookupAsFile").mockImplementationOnce(() => {
throw FileSystemError.FileNotFound(testUris.pdsMember);
});
const _lookupParentDirectoryMock = jest
.spyOn(DatasetFSProvider.instance as any, "_lookupParentDirectory")
.mockReturnValueOnce(testEntries.pds);
const _getInfoFromUriMock = jest.spyOn(DatasetFSProvider.instance as any, "_getInfoFromUri").mockReturnValueOnce({
profile: testProfile,
path: "/USER.DATA.PS",
});
const remoteLookupForResourceMock = jest
.spyOn(DatasetFSProvider.instance, "remoteLookupForResource")
.mockReset()
.mockResolvedValue(testEntries.pdsMember);

await expect(DatasetFSProvider.instance.readFile(testUris.pdsMember)).rejects.toThrow();
expect(_lookupAsFileMock).toHaveBeenCalledWith(testUris.pdsMember);
expect(_lookupParentDirectoryMock).toHaveBeenCalledWith(testUris.pdsMember, true);
expect(remoteLookupForResourceMock).not.toHaveBeenCalledWith(testUris.pdsMember);
_getInfoFromUriMock.mockRestore();
});

it("returns the data for an entry", async () => {
const fakePs = { ...testEntries.ps, wasAccessed: true, data: new Uint8Array([1, 2, 3]) };
const _lookupAsFileMock = jest.spyOn(DatasetFSProvider.instance as any, "_lookupAsFile").mockReturnValueOnce(fakePs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*
*/

import { Disposable, FilePermission, FileType, TextEditor, Uri } from "vscode";
import { Disposable, FilePermission, FileSystemError, FileType, TextEditor, Uri, workspace } from "vscode";
import { BaseProvider, DirEntry, FileEntry, Gui, UssDirectory, UssFile, ZoweScheme } from "@zowe/zowe-explorer-api";
import { Profiles } from "../../../../src/configuration/Profiles";
import { createIProfile } from "../../../__mocks__/mockCreators/shared";
Expand All @@ -25,6 +25,7 @@ const testUris: TestUris = {
conflictFile: Uri.from({ scheme: ZoweScheme.USS, path: "/sestest/aFile.txt", query: "conflict=true" }),
file: Uri.from({ scheme: ZoweScheme.USS, path: "/sestest/aFile.txt" }),
folder: Uri.from({ scheme: ZoweScheme.USS, path: "/sestest/aFolder" }),
innerFile: Uri.from({ scheme: ZoweScheme.USS, path: "/sestest/aFolder/innerFile.txt" }),
session: Uri.from({ scheme: ZoweScheme.USS, path: "/sestest" }),
};

Expand All @@ -46,6 +47,7 @@ const testEntries = {
type: FileType.File,
wasAccessed: true,
} as FileEntry,
innerFile: new UssFile("innerFile.txt"),
folder: {
name: "aFolder",
entries: new Map(),
Expand Down Expand Up @@ -158,15 +160,20 @@ describe("listFiles", () => {
success: true,
commandResponse: "",
apiResponse: {
items: [{ name: "." }, { name: ".." }, { name: "..." }, { name: "test.txt" }],
items: [
{ name: ".", mode: "drwxrwxrwx" },
{ name: "..", mode: "drwxrwxrwx" },
{ name: "...", mode: "drwxrwxrwx" },
{ name: "test.txt", mode: "-rwxrwxrwx" },
],
},
}),
} as any);
expect(await UssFSProvider.instance.listFiles(testProfile, testUris.folder)).toStrictEqual({
success: true,
commandResponse: "",
apiResponse: {
items: [{ name: "test.txt" }],
items: [{ name: "test.txt", mode: "-rwxrwxrwx" }],
},
});
});
Expand Down Expand Up @@ -208,6 +215,38 @@ describe("fetchEntries", () => {
existsMock.mockRestore();
lookupMock.mockRestore();
});
it("non-existent URI", async () => {
const existsMock = jest.spyOn(UssFSProvider.instance, "exists").mockReturnValueOnce(false);
const lookupMock = jest.spyOn(UssFSProvider.instance as any, "lookup").mockReturnValueOnce(null);
const listFilesMock = jest.spyOn(UssFSProvider.instance, "listFiles").mockResolvedValue({
success: true,
apiResponse: {
items: [{ name: testEntries.innerFile.name, mode: "-rwxrwxrwx" }],
},
commandResponse: "",
});
const lookupParentDirMock = jest
.spyOn(UssFSProvider.instance as any, "_lookupParentDirectory")
.mockReturnValueOnce(null)
.mockReturnValueOnce({ ...testEntries.folder, entries: new Map() });
const createDirMock = jest.spyOn(workspace.fs, "createDirectory").mockImplementation();
await expect(
(UssFSProvider.instance as any).fetchEntries(testUris.innerFile, {
isRoot: false,
slashAfterProfilePos: testUris.innerFile.path.indexOf("/", 1),
profile: testProfile,
profileName: testProfile.name,
})
).resolves.not.toThrow();
expect(existsMock).toHaveBeenCalledWith(testUris.innerFile);
expect(lookupMock).toHaveBeenCalledWith(testUris.innerFile, true);
expect(listFilesMock).toHaveBeenCalled();
existsMock.mockRestore();
lookupMock.mockRestore();
listFilesMock.mockRestore();
lookupParentDirMock.mockRestore();
createDirMock.mockRestore();
});
});
describe("folder", () => {
it("existing URI", async () => {
Expand Down Expand Up @@ -453,6 +492,62 @@ describe("readFile", () => {
expect(err).toBeDefined();
});

it("throws an error if an error was encountered during lookup and the code is not FileNotFound", async () => {
const lookupAsFileMock = jest.spyOn(UssFSProvider.instance as any, "_lookupAsFile").mockImplementationOnce((uri) => {
throw FileSystemError.FileIsADirectory(uri as Uri);
});

let err;
try {
await UssFSProvider.instance.readFile(testUris.file);
} catch (error) {
err = error;
expect(err.code).toBe("FileIsADirectory");
}
expect(err).toBeDefined();
lookupAsFileMock.mockRestore();
});

it("throws an error if an error was encountered during lookup and parent dir exists", async () => {
const lookupAsFileMock = jest.spyOn(UssFSProvider.instance as any, "_lookupAsFile").mockImplementationOnce((uri) => {
throw FileSystemError.FileNotFound(uri as Uri);
});
const lookupParentDirMock = jest.spyOn(UssFSProvider.instance as any, "_lookupParentDirectory").mockReturnValueOnce(testEntries.folder);

let err;
try {
await UssFSProvider.instance.readFile(testUris.innerFile);
} catch (error) {
err = error;
expect(err.code).toBe("FileNotFound");
}
expect(err).toBeDefined();
lookupAsFileMock.mockRestore();
lookupParentDirMock.mockRestore();
});

it("throws an error if an error was encountered during lookup and parent dir doesn't exist, but URI is a directory", async () => {
const lookupAsFileMock = jest.spyOn(UssFSProvider.instance as any, "_lookupAsFile").mockImplementationOnce((uri) => {
throw FileSystemError.FileNotFound(uri as Uri);
});
const lookupParentDirMock = jest.spyOn(UssFSProvider.instance as any, "_lookupParentDirectory").mockReturnValueOnce(null);
const remoteLookupForResource = jest
.spyOn(UssFSProvider.instance, "remoteLookupForResource")
.mockResolvedValueOnce(testEntries.folder as any);

let err;
try {
await UssFSProvider.instance.readFile(testUris.innerFile);
} catch (error) {
err = error;
expect(err.code).toBe("FileIsADirectory");
}
expect(err).toBeDefined();
lookupAsFileMock.mockRestore();
lookupParentDirMock.mockRestore();
remoteLookupForResource.mockRestore();
});

it("returns data for a file", async () => {
const lookupAsFileMock = jest.spyOn(UssFSProvider.instance as any, "_lookupAsFile");
lookupAsFileMock.mockReturnValue(testEntries.file);
Expand Down
50 changes: 40 additions & 10 deletions packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,13 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem

const uriInfo = FsAbstractUtils.getInfoForUri(uri, Profiles.getInstance());
const entry = isFetching ? await this.remoteLookupForResource(uri) : this.lookup(uri, false);
// Return the entry for profiles as there is no remote info to fetch
if (uriInfo.isRoot) {
// Do not perform remote lookup for profile or directory URIs; the code below is for change detection on PS or PDS members only
if (uriInfo.isRoot || FsAbstractUtils.isDirectoryEntry(entry)) {
return entry;
}

ZoweLogger.trace(`[DatasetFSProvider] stat is locating resource ${uri.toString()}`);

// Locate the resource using the profile in the given URI.
let resp;
const isPdsMember = !FsDatasetsUtils.isPdsEntry(entry) && (entry as DsEntry).isMember;
Expand Down Expand Up @@ -218,11 +220,13 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem

const entryExists = entry != null;
let entryIsDir = entry != null ? entry.type === vscode.FileType.Directory : false;
const uriPath = uri.path.substring(uriInfo.slashAfterProfilePos + 1).split("/");
const pdsMember = uriPath.length === 2;
if (!entryExists) {
const resp = await ZoweExplorerApiRegister.getMvsApi(uriInfo.profile).dataSet(path.posix.basename(uri.path), { attributes: true });
const resp = await ZoweExplorerApiRegister.getMvsApi(uriInfo.profile).dataSet(uriPath[0], { attributes: true });
if (resp.success) {
const dsorg: string = resp.apiResponse?.items?.[0]?.dsorg;
entryIsDir = dsorg?.startsWith("PO") ?? false;
entryIsDir = pdsMember ? false : dsorg?.startsWith("PO") ?? false;
zFernand0 marked this conversation as resolved.
Show resolved Hide resolved
} else {
throw vscode.FileSystemError.FileNotFound(uri);
}
Expand All @@ -235,8 +239,13 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
}
await this.fetchEntriesForDataset(entry as PdsEntry, uri, uriInfo);
} else if (!entryExists) {
await this.writeFile(uri, new Uint8Array(), { create: true, overwrite: false });
entry = this._lookupAsFile(uri) as DsEntry;
this.createDirectory(uri.with({ path: path.posix.join(uri.path, "..") }));
const parentDir = this._lookupParentDirectory(uri);
const dsname = uriPath[Number(pdsMember)];
const ds = new DsEntry(dsname, pdsMember);
ds.metadata = new DsEntryMetadata({ path: path.posix.join(parentDir.metadata.path, dsname), profile: parentDir.metadata.profile });
parentDir.entries.set(dsname, ds);
entry = parentDir.entries.get(dsname) as DsEntry;
}

return entry;
Expand Down Expand Up @@ -380,7 +389,28 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
* @returns The data set's contents as an array of bytes
*/
public async readFile(uri: vscode.Uri): Promise<Uint8Array> {
const file = this._lookupAsFile(uri);
let ds: DsEntry | DirEntry;
traeok marked this conversation as resolved.
Show resolved Hide resolved
try {
ds = this._lookupAsFile(uri) as DsEntry;
} catch (err) {
if (!(err instanceof vscode.FileSystemError) || err.code !== "FileNotFound") {
throw err;
}

// check if parent directory exists; if not, do a remote lookup
const parent = this._lookupParentDirectory(uri, true);
if (parent == null) {
ds = await this.remoteLookupForResource(uri);
}
}

if (ds == null) {
throw vscode.FileSystemError.FileNotFound(uri);
}
if (FsAbstractUtils.isDirectoryEntry(ds)) {
throw vscode.FileSystemError.FileIsADirectory(uri);
}

const profInfo = this._getInfoFromUri(uri);

if (profInfo.profile == null) {
Expand All @@ -391,14 +421,14 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
const isConflict = urlQuery.has("conflict");

// we need to fetch the contents from the mainframe if the file hasn't been accessed yet
if ((!file.wasAccessed && !urlQuery.has("inDiff")) || isConflict) {
if ((!ds.wasAccessed && !urlQuery.has("inDiff")) || isConflict) {
await this.fetchDatasetAtUri(uri, { isConflict });
if (!isConflict) {
file.wasAccessed = true;
ds.wasAccessed = true;
}
}

return isConflict ? file.conflictData.contents : file.data;
return isConflict ? ds.conflictData.contents : ds.data;
}

public makeEmptyDsWithEncoding(uri: vscode.Uri, encoding: ZosEncoding, isMember?: boolean): void {
Expand Down
Loading
Loading