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

feat: Read file separator from server config #2118

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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 package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 15 additions & 6 deletions packages/app-utils/src/storage/grpc/GrpcFileStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export class GrpcFileStorage implements FileStorage {

private readonly root: string;

public readonly separator: string;

/**
* FileStorage implementation using gRPC
* @param storageService Storage service to use
Expand All @@ -33,11 +35,13 @@ export class GrpcFileStorage implements FileStorage {
constructor(
dh: typeof DhType,
storageService: DhType.storage.StorageService,
root = ''
root = '',
separator = '/'
) {
this.dh = dh;
this.storageService = storageService;
this.root = root;
this.separator = separator;
}

private removeRoot(filename: string): string {
Expand All @@ -55,12 +59,17 @@ export class GrpcFileStorage implements FileStorage {
type: 'directory',
id: path,
filename: path,
basename: FileUtils.getBaseName(path),
basename: FileUtils.getBaseName(path, this.separator),
};
}

async getTable(): Promise<FileStorageTable> {
const table = new GrpcFileStorageTable(this.storageService, this.root);
const table = new GrpcFileStorageTable(
this.storageService,
this.root,
this.root,
this.separator
);
this.tables.push(table);
return table;
}
Expand All @@ -81,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,
};
}
Expand Down Expand Up @@ -111,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();
Expand Down
75 changes: 43 additions & 32 deletions packages/app-utils/src/storage/grpc/GrpcFileStorageTable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@ import type { dh } from '@deephaven/jsapi-types';
import GrpcFileStorageTable from './GrpcFileStorageTable';

let storageService: dh.storage.StorageService;
function makeTable(baseRoot = '', root = ''): GrpcFileStorageTable {
return new GrpcFileStorageTable(storageService, baseRoot, root);
function makeTable(
baseRoot = '',
root = '',
separator = '/'
): GrpcFileStorageTable {
return new GrpcFileStorageTable(storageService, baseRoot, root, separator);
}

function makeStorageService(): dh.storage.StorageService {
Expand Down Expand Up @@ -39,11 +43,11 @@ it('Does not get contents until a viewport is set', () => {
expect(storageService.listItems).toHaveBeenCalled();
});

describe('directory expansion tests', () => {
describe.each(['/', `\\`])('directory expansion tests %s', separator => {
function makeFile(name: string, path = ''): dh.storage.ItemDetails {
return {
basename: name,
filename: `${path}/${name}`,
filename: `${path}${separator}${name}`,
dirname: path,
etag: '',
size: 1,
Expand All @@ -58,7 +62,7 @@ describe('directory expansion tests', () => {
}

function makeDirectoryContents(
path = '/',
path = separator,
numDirs = 3,
numFiles = 2
): Array<dh.storage.ItemDetails> {
Expand Down Expand Up @@ -87,15 +91,15 @@ 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);
// console.log('dirContents for ', path, dirContents);
return dirContents;
});
});

it('expands multiple directories correctly', async () => {
const table = makeTable();
it(`expands multiple directories correctly`, async () => {
const table = makeTable('', '', separator);
const handleUpdate = jest.fn();
table.onUpdate(handleUpdate);
table.setViewport({ top: 0, bottom: 5 });
Expand All @@ -106,65 +110,72 @@ describe('directory expansion tests', () => {
expect(handleUpdate).toHaveBeenCalledWith({
offset: 0,
items: [
expectItem(makeDirectory('dir0')),
expectItem(makeDirectory('dir1')),
expectItem(makeDirectory('dir2')),
expectItem(makeDirectory('dir3')),
expectItem(makeFile('file0')),
expectItem(makeDirectory('dir0', '')),
expectItem(makeDirectory('dir1', '')),
expectItem(makeDirectory('dir2', '')),
expectItem(makeDirectory('dir3', '')),
expectItem(makeFile('file0', '')),
],
});
handleUpdate.mockReset();

table.setExpanded('/dir1/', true);
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', '/dir1')),
expectItem(makeDirectory('dir1', '/dir1')),
expectItem(makeDirectory('dir2', '/dir1')),
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('/dir1/dir1/', true);
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', '/dir1')),
expectItem(makeDirectory('dir1', '/dir1')),
expectItem(makeDirectory('dir0', '/dir1/dir1')),
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('/dir1/', false);
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')),
expectItem(makeDirectory('dir0', '')),
expectItem(makeDirectory('dir1', '')),
expectItem(makeDirectory('dir2', '')),
expectItem(makeDirectory('dir3', '')),
expectItem(makeFile('file0', '')),
]),
});
table.collapseAll();
handleUpdate.mockReset();
});
});
19 changes: 13 additions & 6 deletions packages/app-utils/src/storage/grpc/GrpcFileStorageTable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export class GrpcFileStorageTable implements FileStorageTable {

private readonly root: string;

public readonly separator: string;

private currentSize = 0;

private currentViewport?: StorageTableViewport;
Expand All @@ -58,16 +60,19 @@ export class GrpcFileStorageTable implements FileStorageTable {
/**
* @param storageService The storage service to use
* @param baseRoot Base root for the service
* @param root The root path for this storage table
* @param root Root path for this storage table
* @param separator Separator used in the paths
*/
constructor(
storageService: dh.storage.StorageService,
baseRoot = '',
root = baseRoot
root = baseRoot,
separator = '/'
) {
this.storageService = storageService;
this.baseRoot = baseRoot;
this.root = root;
this.separator = separator;
if (root === baseRoot) {
this.doRefresh = debounce(
this.doRefresh.bind(this),
Expand Down Expand Up @@ -100,21 +105,22 @@ export class GrpcFileStorageTable implements FileStorageTable {
}

setExpanded(path: string, expanded: boolean): void {
const paths = path.split('/');
const paths = path.split(this.separator);
let nextPath = paths.shift();
if (nextPath === undefined || nextPath === '') {
nextPath = paths.shift();
}
if (nextPath === undefined || nextPath === '') {
throw new Error(`Invalid path: ${path}`);
}
const remainingPath = paths.join('/');
const remainingPath = paths.join(this.separator);
if (expanded) {
if (!this.childTables.has(nextPath)) {
const childTable = new GrpcFileStorageTable(
this.storageService,
this.baseRoot,
`${this.root}/${nextPath}`
`${this.root}${this.separator}${nextPath}`,
this.separator
);
this.childTables.set(nextPath, childTable);
}
Expand Down Expand Up @@ -264,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);
Expand Down
12 changes: 10 additions & 2 deletions packages/app-utils/src/storage/grpc/GrpcLayoutStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,22 @@ export class GrpcLayoutStorage implements LayoutStorage {

readonly root: string;

private readonly separator: string;

/**
*
* @param storageService The gRPC storage service to use
* @param root The root path where the layouts are stored
* @param separator The separator used in the paths
*/
constructor(storageService: dh.storage.StorageService, root = '') {
constructor(
storageService: dh.storage.StorageService,
root = '',
separator = '/'
) {
this.storageService = storageService;
this.root = root;
this.separator = separator;
}

async getLayouts(): Promise<string[]> {
Expand All @@ -24,7 +32,7 @@ export class GrpcLayoutStorage implements LayoutStorage {

async getLayout(name: string): Promise<ExportedLayout> {
const fileContents = await this.storageService.loadFile(
`${this.root}/${name}`
`${this.root}${this.separator}${name}`
);
const content = await fileContents.text();

Expand Down
5 changes: 0 additions & 5 deletions packages/code-studio/.env
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ VITE_CORE_API_NAME=dh-core.js
# Like the CORE_API_URL, we assume this is served up as a sibling of the code studio
VITE_MODULE_PLUGINS_URL=../js-plugins

# Path for notebooks and layouts storage on the gRPCStorageService
# Note these are not URLs, these are file system paths on the server in the gRPCStorageService
VITE_STORAGE_PATH_NOTEBOOKS=/notebooks
VITE_STORAGE_PATH_LAYOUTS=/layouts

# Any routes we define here
VITE_ROUTE_NOTEBOOKS=notebooks/

Expand Down
Loading
Loading