Add the separator to all the file operations

- Added a context to pass down the FileStorage and file separator - Cleaned up all the types and the tests
deephaven · Jul 5, 2024 · e69d64a · e69d64a
1 parent a3373aa
commit e69d64a
Show file tree

Hide file tree

Showing 27 changed files with 578 additions and 463 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/app-utils/src/storage/grpc/GrpcFileStorage.ts b/packages/app-utils/src/storage/grpc/GrpcFileStorage.ts
@@ -59,7 +59,7 @@ export class GrpcFileStorage implements FileStorage {
  type: 'directory',
  id: path,
  filename: path,
- basename: FileUtils.getBaseName(path),
+ basename: FileUtils.getBaseName(path, this.separator),
  };
  }
 
@@ -90,7 +90,7 @@ export class GrpcFileStorage implements FileStorage {
  const content = await fileContents.text();
  return {
  filename: name,
- basename: FileUtils.getBaseName(name),
+ basename: FileUtils.getBaseName(name, this.separator),
  content,
  };
  }
@@ -120,8 +120,8 @@ export class GrpcFileStorage implements FileStorage {
 
  async info(name: string): Promise<FileStorageItem> {
  const allItems = await this.storageService.listItems(
- this.addRoot(FileUtils.getPath(name)),
- FileUtils.getBaseName(name)
+ this.addRoot(FileUtils.getPath(name, this.separator)),
+ FileUtils.getBaseName(name, this.separator)
  );
  if (allItems.length === 0) {
  throw new FileNotFoundError();

diff --git a/packages/app-utils/src/storage/grpc/GrpcFileStorageTable.test.ts b/packages/app-utils/src/storage/grpc/GrpcFileStorageTable.test.ts
@@ -43,12 +43,8 @@ it('Does not get contents until a viewport is set', () => {
  expect(storageService.listItems).toHaveBeenCalled();
 });
 
-describe('directory expansion tests', () => {
- function makeFile(
- name: string,
- path = '',
- separator = '/'
- ): dh.storage.ItemDetails {
+describe.each(['/', `\\`])('directory expansion tests %s', separator => {
+ function makeFile(name: string, path = ''): dh.storage.ItemDetails {
  return {
  basename: name,
  filename: `${path}${separator}${name}`,
@@ -59,32 +55,27 @@ describe('directory expansion tests', () => {
  };
  }
 
- function makeDirectory(
- name: string,
- path = '',
- separator = '/'
- ): dh.storage.ItemDetails {
- const file = makeFile(name, path, separator);
+ function makeDirectory(name: string, path = ''): dh.storage.ItemDetails {
+ const file = makeFile(name, path);
  file.type = 'directory';
  return file;
  }
 
  function makeDirectoryContents(
- path = '/',
+ path = separator,
  numDirs = 3,
- numFiles = 2,
- separator = '/'
+ numFiles = 2
  ): Array<dh.storage.ItemDetails> {
  const results = [] as dh.storage.ItemDetails[];
 
  for (let i = 0; i < numDirs; i += 1) {
  const name = `dir${i}`;
- results.push(makeDirectory(name, path, separator));
+ results.push(makeDirectory(name, path));
  }
 
  for (let i = 0; i < numFiles; i += 1) {
  const name = `file${i}`;
- results.push(makeFile(name, path, separator));
+ results.push(makeFile(name, path));
  }
 
  return results;
@@ -100,128 +91,91 @@ describe('directory expansion tests', () => {
 
  beforeEach(() => {
  storageService.listItems = jest.fn(async path => {
- const depth = path.length > 0 ? FileUtils.getDepth(path) + 1 : 1;
+ const depth =
+ path.length > 0 ? FileUtils.getDepth(path, separator) + 1 : 1;
  const dirContents = makeDirectoryContents(path, 5 - depth, 10 - depth);
  return dirContents;
  });
  });
 
- it.each(['/'])(
- `expands multiple directories correctly with '%s' separator`,
- async separator => {
- const table = makeTable('', '', separator);
- const handleUpdate = jest.fn();
- table.onUpdate(handleUpdate);
- table.setViewport({ top: 0, bottom: 5 });
-
- jest.runAllTimers();
-
- await table.getViewportData();
- expect(handleUpdate).toHaveBeenCalledWith({
- offset: 0,
- items: [
- expectItem(makeDirectory('dir0', '', separator)),
- expectItem(makeDirectory('dir1', '', separator)),
- expectItem(makeDirectory('dir2', '', separator)),
- expectItem(makeDirectory('dir3', '', separator)),
- expectItem(makeFile('file0', '', separator)),
- ],
- });
- handleUpdate.mockReset();
-
- const dirPath = `${separator}dir1${separator}`;
- table.setExpanded(dirPath, true);
-
- jest.runAllTimers();
-
- await table.getViewportData();
- expect(handleUpdate).toHaveBeenCalledWith({
- offset: 0,
- items: [
- expectItem(makeDirectory('dir0', '', separator)),
- expectItem(makeDirectory('dir1', '', separator)),
- expectItem(
- makeDirectory(
- 'dir0',
- makeDirectory('dir1', '', separator).filename,
- separator
- )
- ),
- expectItem(
- makeDirectory(
- 'dir1',
- makeDirectory('dir1', '', separator).filename,
- separator
- )
- ),
- expectItem(
- makeDirectory(
- 'dir2',
- makeDirectory('dir1', '', separator).filename,
- separator
- )
- ),
- ],
- });
- handleUpdate.mockReset();
-
- table.setExpanded(`${separator}dir1${separator}dir1${separator}`, true);
-
- jest.runAllTimers();
-
- await table.getViewportData();
- expect(handleUpdate).toHaveBeenCalledWith({
- offset: 0,
- items: expect.arrayContaining([
- expectItem(makeDirectory('dir0', '', separator)),
- expectItem(makeDirectory('dir1', '', separator)),
- expectItem(
- makeDirectory(
- 'dir0',
- makeDirectory('dir1', '', separator).filename,
- separator
- )
- ),
- expectItem(
- makeDirectory(
- 'dir1',
- makeDirectory('dir1', '', separator).filename,
- separator
- )
- ),
- expectItem(
- makeDirectory(
- 'dir0',
- makeDirectory(
- 'dir1',
- makeDirectory('dir1', '', separator).filename,
- separator
- ).filename,
- separator
- )
- ),
- ]),
- });
- handleUpdate.mockReset();
-
- // Now collapse it all
- table.setExpanded(`${separator}dir1${separator}`, false);
-
- jest.runAllTimers();
-
- await table.getViewportData();
- expect(handleUpdate).toHaveBeenCalledWith({
- offset: 0,
- items: expect.arrayContaining([
- expectItem(makeDirectory('dir0', '', separator)),
- expectItem(makeDirectory('dir1', '', separator)),
- expectItem(makeDirectory('dir2', '', separator)),
- expectItem(makeDirectory('dir3', '', separator)),
- expectItem(makeFile('file0', '', separator)),
- ]),
- });
- table.collapseAll();
- handleUpdate.mockReset();
- }
- );
+ it(`expands multiple directories correctly`, async () => {
+ const table = makeTable('', '', separator);
+ const handleUpdate = jest.fn();
+ table.onUpdate(handleUpdate);
+ table.setViewport({ top: 0, bottom: 5 });
+
+ jest.runAllTimers();
+
+ await table.getViewportData();
+ expect(handleUpdate).toHaveBeenCalledWith({
+ offset: 0,
+ items: [
+ expectItem(makeDirectory('dir0', '')),
+ expectItem(makeDirectory('dir1', '')),
+ expectItem(makeDirectory('dir2', '')),
+ expectItem(makeDirectory('dir3', '')),
+ expectItem(makeFile('file0', '')),
+ ],
+ });
+ handleUpdate.mockReset();
+
+ const dirPath = `${separator}dir1${separator}`;
+ table.setExpanded(dirPath, true);
+
+ jest.runAllTimers();
+
+ await table.getViewportData();
+ expect(handleUpdate).toHaveBeenCalledWith({
+ offset: 0,
+ items: [
+ expectItem(makeDirectory('dir0', '')),
+ expectItem(makeDirectory('dir1', '')),
+ expectItem(makeDirectory('dir0', makeDirectory('dir1', '').filename)),
+ expectItem(makeDirectory('dir1', makeDirectory('dir1', '').filename)),
+ expectItem(makeDirectory('dir2', makeDirectory('dir1', '').filename)),
+ ],
+ });
+ handleUpdate.mockReset();
+
+ table.setExpanded(`${separator}dir1${separator}dir1${separator}`, true);
+
+ jest.runAllTimers();
+
+ await table.getViewportData();
+ expect(handleUpdate).toHaveBeenCalledWith({
+ offset: 0,
+ items: expect.arrayContaining([
+ expectItem(makeDirectory('dir0', '')),
+ expectItem(makeDirectory('dir1', '')),
+ expectItem(makeDirectory('dir0', makeDirectory('dir1', '').filename)),
+ expectItem(makeDirectory('dir1', makeDirectory('dir1', '').filename)),
+ expectItem(
+ makeDirectory(
+ 'dir0',
+ makeDirectory('dir1', makeDirectory('dir1', '').filename).filename
+ )
+ ),
+ ]),
+ });
+ handleUpdate.mockReset();
+
+ // Now collapse it all
+ table.setExpanded(`${separator}dir1${separator}`, false);
+
+ jest.runAllTimers();
+
+ await table.getViewportData();
+ expect(handleUpdate).toHaveBeenCalledWith({
+ offset: 0,
+ items: expect.arrayContaining([
+ expectItem(makeDirectory('dir0', '')),
+ expectItem(makeDirectory('dir1', '')),
+ expectItem(makeDirectory('dir2', '')),
+ expectItem(makeDirectory('dir3', '')),
+ expectItem(makeFile('file0', '')),
+ ]),
+ });
+ table.collapseAll();
+ handleUpdate.mockReset();
+ });
 });
diff --git a/packages/app-utils/src/storage/grpc/GrpcFileStorageTable.ts b/packages/app-utils/src/storage/grpc/GrpcFileStorageTable.ts
@@ -119,7 +119,8 @@ export class GrpcFileStorageTable implements FileStorageTable {
  const childTable = new GrpcFileStorageTable(
  this.storageService,
  this.baseRoot,
- `${this.root}${this.separator}${nextPath}`
+ `${this.root}${this.separator}${nextPath}`,
+ this.separator
  );
  this.childTables.set(nextPath, childTable);
  }
@@ -269,7 +270,8 @@ export class GrpcFileStorageTable implements FileStorageTable {
  const item = items[i];
  const { basename, filename } = item;
  if (
- filename === this.removeBaseRoot(`${this.root}/${basename}`) &&
+ filename ===
+ this.removeBaseRoot(`${this.root}${this.separator}${basename}`) &&
  item.type === 'directory'
  ) {
  const childTable = this.childTables.get(basename);

diff --git a/packages/code-studio/src/main/AppInit.tsx b/packages/code-studio/src/main/AppInit.tsx
@@ -13,6 +13,7 @@ import {
 } from '@deephaven/dashboard-core-plugins';
 import { useApi, useClient } from '@deephaven/jsapi-bootstrap';
 import { getSessionDetails, loadSessionWrapper } from '@deephaven/jsapi-utils';
+import { FileStorage, FileStorageContext } from '@deephaven/file-explorer';
 import Log from '@deephaven/log';
 import { PouchCommandHistoryStorage } from '@deephaven/pouch-storage';
 import {
@@ -28,6 +29,7 @@ import {
  setWorkspaceStorage,
  setServerConfigValues,
  setUser,
+ getFileStorage,
 } from '@deephaven/redux';
 import {
  LocalWorkspaceStorage,
@@ -54,6 +56,9 @@ function AppInit(): JSX.Element {
  const serverConfig = useServerConfig();
  const user = useUser();
  const workspace = useSelector<RootState>(getWorkspace);
+ const fileStorage = useSelector<RootState>(
+ getFileStorage
+ ) as FileStorage | null;
  const dispatch = useDispatch();
 
  // General error means the app is dead and is unlikely to recover
@@ -88,7 +93,7 @@ function AppInit(): JSX.Element {
  layoutRoot,
  fileSeparator
  );
- const fileStorage = new GrpcFileStorage(
+ const grpcFileStorage = new GrpcFileStorage(
  api,
  storageService,
  notebookRoot,
@@ -129,7 +134,7 @@ function AppInit(): JSX.Element {
  dispatch(setServerConfigValues(serverConfig));
  dispatch(setCommandHistoryStorage(commandHistoryStorage));
  dispatch(setDashboardData(DEFAULT_DASHBOARD_ID, dashboardData));
- dispatch(setFileStorage(fileStorage));
+ dispatch(setFileStorage(grpcFileStorage));
  dispatch(setLayoutStorage(layoutStorage));
  dispatch(setDashboardConnection(DEFAULT_DASHBOARD_ID, connection));
  if (sessionWrapper !== undefined) {
@@ -160,14 +165,14 @@ function AppInit(): JSX.Element {
  const errorMessage = error != null ? `${error}` : null;
 
  return (
- <>
+ <FileStorageContext.Provider value={fileStorage}>
  {isLoaded && <App />}
  <LoadingOverlay
  isLoading={isLoading && errorMessage == null}
  isLoaded={isLoaded}
  errorMessage={errorMessage}
  />
- </>
+ </FileStorageContext.Provider>
  );
 }