Skip to content

Commit

Permalink
Merge pull request #3093 from zowe/fix/listing-items-api-calls
Browse files Browse the repository at this point in the history 
fix(list): Do not fetch directory URIs in `stat`
  • Loading branch information
@JillieBeanSim
JillieBeanSim authored Sep 9, 2024
2 parents 6bc3179 + 3c5c3a5 commit c28d75b
Show file tree
Hide file tree
Showing 5 changed files with 270 additions and 18 deletions.
1 change: 1 addition & 0 deletions packages/zowe-explorer/CHANGELOG.md
View file
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
87 changes: 87 additions & 0 deletions packages/zowe-explorer/__tests__/__unit__/trees/dataset/DatasetFSProvider.unit.test.ts
View file
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
101 changes: 98 additions & 3 deletions packages/zowe-explorer/__tests__/__unit__/trees/uss/UssFSProvider.unit.test.ts
View file
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
View file
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;
} 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;
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

0 comments on commit c28d75b

Please sign in to comment.