fileStorage: untangle file id/name

We were using file UUID and path interchangeable. This adds a new UUID
type for static checking and fixes a bunch of issues found.
This commit is contained in:
David Lechner
2022-04-01 17:24:00 -05:00
parent daa6c07910
commit 1709591afc
16 changed files with 420 additions and 278 deletions
+2 -2
View File
@@ -16,7 +16,7 @@ import MonacoEditor, {
import { useDispatch } from 'react-redux';
import { useTernaryDarkMode } from 'usehooks-ts';
import { IDisposable } from 'xterm';
import { fileStorageWriteFile } from '../fileStorage/actions';
import { UUID, fileStorageWriteFile } from '../fileStorage/actions';
import { compile } from '../mpy/actions';
import { useSettingIsShowDocsEnabled } from '../settings/hooks';
import { isMacOS } from '../utils/os';
@@ -268,7 +268,7 @@ const Editor: React.VFC = () => {
const handleChange = useCallback<ChangeHandler>(
// REVISIT: need to ensure we have exclusive access to file
(v) => dispatch(fileStorageWriteFile('main.py', v)),
(v) => dispatch(fileStorageWriteFile('main.py' as UUID, v)),
[dispatch],
);
+16 -9
View File
@@ -5,8 +5,9 @@ import { getByLabelText, waitFor } from '@testing-library/dom';
import { cleanup } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react';
import { testRender } from '../../test';
import { testRender, uuid } from '../../test';
import {
FileMetadata,
fileStorageArchiveAllFiles,
fileStorageExportFile,
} from '../fileStorage/actions';
@@ -19,10 +20,16 @@ afterEach(async () => {
localStorage.clear();
});
const testFile: FileMetadata = {
uuid: uuid(0),
path: 'test.file',
sha256: '',
};
describe('archive button', () => {
it('should be enabled if there are files', () => {
const [explorer, dispatch] = testRender(<Explorer />, {
fileStorage: { fileNames: ['test.file'] },
fileStorage: { files: [testFile] },
});
const button = explorer.getByTitle('Backup all files');
@@ -34,7 +41,7 @@ describe('archive button', () => {
it('should be disabled if there are no files', () => {
const [explorer, dispatch] = testRender(<Explorer />, {
fileStorage: { fileNames: [] },
fileStorage: { files: [] },
});
const button = explorer.getByTitle('Backup all files');
@@ -75,7 +82,7 @@ describe('new file button', () => {
describe('tree item', () => {
it('should dispatch action when button is clicked', async () => {
const [explorer, dispatch] = testRender(<Explorer />, {
fileStorage: { fileNames: ['test.file'] },
fileStorage: { files: [testFile] },
});
expect(
@@ -93,7 +100,7 @@ describe('tree item', () => {
it('should dispatch action when key is pressed', async () => {
const [explorer, dispatch] = testRender(<Explorer />, {
fileStorage: { fileNames: ['test.file'] },
fileStorage: { files: [testFile] },
});
expect(
@@ -110,7 +117,7 @@ describe('tree item', () => {
it('should dispatch delete action when button is clicked', async () => {
const [explorer, dispatch] = testRender(<Explorer />, {
fileStorage: { fileNames: ['test.file'] },
fileStorage: { files: [testFile] },
});
// NB: this button is intentionally not accessible (by role) since
@@ -124,7 +131,7 @@ describe('tree item', () => {
it('should dispatch delete action when key is pressed', async () => {
const [explorer, dispatch] = testRender(<Explorer />, {
fileStorage: { fileNames: ['test.file'] },
fileStorage: { files: [testFile] },
});
const treeItem = explorer.getByRole('treeitem', { name: 'test.file' });
@@ -137,7 +144,7 @@ describe('tree item', () => {
it('should dispatch export action when button is clicked', async () => {
const [explorer, dispatch] = testRender(<Explorer />, {
fileStorage: { fileNames: ['test.file'] },
fileStorage: { files: [testFile] },
});
// NB: this button is intentionally not accessible (by role) since
@@ -151,7 +158,7 @@ describe('tree item', () => {
it('should dispatch export action when key is pressed', async () => {
const [explorer, dispatch] = testRender(<Explorer />, {
fileStorage: { fileNames: ['test.file'] },
fileStorage: { files: [testFile] },
});
const treeItem = explorer.getByRole('treeitem', { name: 'test.file' });
+13 -12
View File
@@ -23,7 +23,6 @@ import {
useTreeEnvironment,
} from 'react-complex-tree';
import { useDispatch } from 'react-redux';
import { useDebounce } from 'usehooks-ts';
import {
fileStorageArchiveAllFiles,
fileStorageExportFile,
@@ -131,7 +130,7 @@ type HeaderProps = {
const Header: React.VoidFunctionComponent<HeaderProps> = ({ i18n }) => {
const [isNewFileWizardOpen, setIsNewFileWizardOpen] = useState(false);
const dispatch = useDispatch();
const fileNames = useSelector((s) => s.fileStorage.fileNames);
const files = useSelector((s) => s.fileStorage.files);
return (
<div style={{ display: 'flex', justifyContent: 'flex-end' }}>
@@ -139,7 +138,7 @@ const Header: React.VoidFunctionComponent<HeaderProps> = ({ i18n }) => {
<ActionButton
icon="archive"
tooltip={i18n.translate(I18nId.HeaderExportAllTooltip)}
disabled={fileNames.length === 0}
disabled={files.length === 0}
onClick={() => dispatch(fileStorageArchiveAllFiles())}
/>
<ActionButton
@@ -280,22 +279,21 @@ type FileTreeProps = {
const FileTree: React.VoidFunctionComponent<FileTreeProps> = ({ i18n }) => {
const [focusedItem, setFocusedItem] = useState<TreeItemIndex>();
const fileNames = useSelector((s) => s.fileStorage.fileNames);
const debouncedFileNames = useDebounce(fileNames);
const files = useSelector((s) => s.fileStorage.files);
const liveDescriptors = useLiveDescriptors(i18n);
const rootItemIndex = '/';
const rootItemIndex = 'root';
const treeItems = useMemo(
() =>
debouncedFileNames.reduce(
(obj, fileName) => {
const index = `/${fileName}`;
files.reduce(
(obj, file) => {
const index = file.uuid;
obj[index] = {
index,
data: {
fileName,
fileName: file.path,
icon: 'document',
secondaryLabel: (
<TreeItemContext.Consumer>
@@ -317,11 +315,14 @@ const FileTree: React.VoidFunctionComponent<FileTreeProps> = ({ i18n }) => {
index: rootItemIndex,
data: { fileName: '/' },
hasChildren: true,
children: debouncedFileNames.map((n) => `/${n}`),
children: [...files]
// REVISIT: consider using Intl.Collator() for i18n.locale
.sort((a, b) => a.path.localeCompare(b.path))
.map((n) => n.uuid),
},
} as Record<TreeItemIndex, FileTreeItem>,
),
[debouncedFileNames],
[files],
);
const getItemTitle = useCallback((item: FileTreeItem) => item.data.fileName, []);
+16
View File
@@ -61,6 +61,22 @@ export const explorerCreateNewFile = createAction(
}),
);
/**
* Action that indicates that {@link explorerCreateNewFile} succeeded.
*/
export const explorerDidCreateNewFile = createAction(() => ({
type: 'explorer.action.didCreateNewFile',
}));
/**
* Action that indicates that {@link explorerCreateNewFile} failed.
* @param error The error.
*/
export const explorerDidFailToCreateNewFile = createAction((error: Error) => ({
type: 'explorer.action.didFailToCreateNewFile',
error,
}));
/**
* Action that requests to rename a file.
* @param fileName The file name.
+2 -2
View File
@@ -41,11 +41,11 @@ const NewFileWizard: React.VoidFunctionComponent<NewFileWizardProps> = ({
const dispatch = useDispatch();
const [fileName, setFileName] = useState('');
const fileNames = useSelector((s) => s.fileStorage.fileNames);
const files = useSelector((s) => s.fileStorage.files);
const fileNameValidation = validateFileName(
fileName,
pythonFileExtension,
fileNames,
files.map((f) => f.path),
);
const [hubType, setHubType] = useState(defaultHub);
@@ -24,8 +24,12 @@ const RenameFileDialog: React.VFC = () => {
const [baseName, extension] = oldName.split(/(\.\w+)$/);
const [newName, setNewName] = useState(baseName);
const fileNames = useSelector((s) => s.fileStorage.fileNames);
const result = validateFileName(newName, extension, fileNames);
const files = useSelector((s) => s.fileStorage.files);
const result = validateFileName(
newName,
extension,
files.map((f) => f.path),
);
const inputRef = useRef<HTMLInputElement>(null);
+32 -34
View File
@@ -6,10 +6,10 @@ import { FileWithHandle } from 'browser-fs-access';
import { mock } from 'jest-mock-extended';
import { AsyncSaga, uuid } from '../../test';
import {
fileStorageDidFailToOpenFile,
fileStorageDidFailToRenameFile,
fileStorageDidOpenFile,
fileStorageDidRenameFile,
fileStorageDidWriteFile,
fileStorageOpenFile,
fileStorageRenameFile,
fileStorageWriteFile,
@@ -18,6 +18,7 @@ import { pythonFileExtension } from '../pybricksMicropython/lib';
import {
Hub,
explorerCreateNewFile,
explorerDidCreateNewFile,
explorerDidFailToImportFiles,
explorerDidFailToRenameFile,
explorerDidImportFiles,
@@ -48,11 +49,17 @@ describe('handleExplorerImportFiles', () => {
saga.put(explorerImportFiles());
const action = await saga.take();
expect(action).toEqual(fileStorageWriteFile(testFileName, testFileContents));
await expect(saga.take()).resolves.toEqual(fileStorageOpenFile(testFileName));
const action2 = await saga.take();
expect(action2).toEqual(explorerDidImportFiles());
saga.put(fileStorageDidOpenFile(testFileName, uuid(0)));
await expect(saga.take()).resolves.toEqual(
fileStorageWriteFile(uuid(0), testFileContents),
);
saga.put(fileStorageDidWriteFile(uuid(0)));
await expect(saga.take()).resolves.toEqual(explorerDidImportFiles());
await saga.end();
});
@@ -79,8 +86,11 @@ describe('handleExplorerCreateNewFile', () => {
saga.put(explorerCreateNewFile('test', pythonFileExtension, Hub.Technic));
const action = await saga.take();
expect(action).toMatchInlineSnapshot(`
await expect(saga.take()).resolves.toEqual(fileStorageOpenFile('test.py'));
saga.put(fileStorageDidOpenFile('test.py', uuid(0)));
await expect(saga.take()).resolves.toMatchInlineSnapshot(`
Object {
"contents": "from pybricks.hubs import TechnicHub
from pybricks.pupdevices import Motor
@@ -91,11 +101,15 @@ describe('handleExplorerCreateNewFile', () => {
hub = TechnicHub()
",
"id": "test.py",
"id": "00000000-0000-0000-0000-000000000000",
"type": "fileStorage.action.writeFile",
}
`);
saga.put(fileStorageDidWriteFile(uuid(0)));
await expect(saga.take()).resolves.toEqual(explorerDidCreateNewFile());
await saga.end();
});
});
@@ -121,39 +135,23 @@ describe('handleExplorerRenameFile', () => {
beforeEach(async () => {
saga.put(renameFileDialogDidAccept('old.file', 'new.file'));
await expect(saga.take()).resolves.toEqual(fileStorageOpenFile('old.file'));
await expect(saga.take()).resolves.toEqual(
fileStorageRenameFile('old.file', 'new.file'),
);
});
it('should dispatch action on fileStorageOpenFile failure', async () => {
saga.put(fileStorageDidFailToOpenFile('old.file', new Error('test error')));
it('should dispatch action on fileStorageRenameFile failure', async () => {
saga.put(
fileStorageDidFailToRenameFile('old.file', new Error('test error')),
);
await expect(saga.take()).resolves.toEqual(explorerDidFailToRenameFile());
});
describe('should dispatch fileStorageRenameFile action on fileStorageOpenFile success', () => {
beforeEach(async () => {
saga.put(fileStorageDidOpenFile('old.file', uuid(0)));
it('should dispatch action on fileStorageRenameFile success', async () => {
saga.put(fileStorageDidRenameFile('old.file'));
await expect(saga.take()).resolves.toEqual(
fileStorageRenameFile(uuid(0), 'new.file'),
);
});
it('should dispatch action on fileStorageRenameFile failure', async () => {
saga.put(
fileStorageDidFailToRenameFile(uuid(0), new Error('test error')),
);
await expect(saga.take()).resolves.toEqual(
explorerDidFailToRenameFile(),
);
});
it('should dispatch action on fileStorageRenameFile success', async () => {
saga.put(fileStorageDidRenameFile(uuid(0)));
await expect(saga.take()).resolves.toEqual(explorerDidRenameFile());
});
await expect(saga.take()).resolves.toEqual(explorerDidRenameFile());
});
});
+83 -35
View File
@@ -15,8 +15,10 @@ import { getPybricksMicroPythonFileTemplate } from '../editor/pybricksMicroPytho
import {
fileStorageDidFailToOpenFile,
fileStorageDidFailToRenameFile,
fileStorageDidFailToWriteFile,
fileStorageDidOpenFile,
fileStorageDidRenameFile,
fileStorageDidWriteFile,
fileStorageOpenFile,
fileStorageRenameFile,
fileStorageWriteFile,
@@ -32,6 +34,8 @@ import { RootState } from '../reducers';
import { defined, ensureError } from '../utils';
import {
explorerCreateNewFile,
explorerDidCreateNewFile,
explorerDidFailToCreateNewFile,
explorerDidFailToImportFiles,
explorerDidFailToRenameFile,
explorerDidImportFiles,
@@ -64,14 +68,12 @@ function* handleExplorerImportFiles(): Generator {
const text = yield* call(() => file.text());
const [baseName] = file.name.split(pythonFileExtensionRegex);
const existingFiles = yield* select(
(s: RootState) => s.fileStorage.fileNames,
);
const existingFiles = yield* select((s: RootState) => s.fileStorage.files);
const result = validateFileName(
baseName,
pythonFileExtension,
existingFiles,
existingFiles.map((f) => f.path),
);
if (result != FileNameValidationResult.IsOk) {
@@ -84,7 +86,37 @@ function* handleExplorerImportFiles(): Generator {
continue;
}
yield* put(fileStorageWriteFile(`${baseName}${pythonFileExtension}`, text));
const fileName = `${baseName}${pythonFileExtension}`;
yield* put(fileStorageOpenFile(fileName));
const { didOpen, didFailToOpen } = yield* race({
didOpen: take(fileStorageDidOpenFile.when((a) => a.path === fileName)),
didFailToOpen: take(
fileStorageDidFailToOpenFile.when((a) => a.path === fileName),
),
});
if (didFailToOpen) {
throw didFailToOpen.error;
}
defined(didOpen);
yield* put(fileStorageWriteFile(didOpen.id, text));
const { didFailToWrite } = yield* race({
didWrite: take(
fileStorageDidWriteFile.when((a) => a.id === didOpen.id),
),
didFailToWrite: take(
fileStorageDidFailToWriteFile.when((a) => a.id === didOpen.id),
),
});
if (didFailToWrite) {
throw didFailToWrite.error;
}
}
yield* put(explorerDidImportFiles());
@@ -96,14 +128,46 @@ function* handleExplorerImportFiles(): Generator {
function* handleExplorerCreateNewFile(
action: ReturnType<typeof explorerCreateNewFile>,
): Generator {
const fileName = `${action.fileName}${action.fileExtension}`;
try {
const fileName = `${action.fileName}${action.fileExtension}`;
yield* put(
fileStorageWriteFile(
fileName,
getPybricksMicroPythonFileTemplate(action.hub) || '',
),
);
yield* put(fileStorageOpenFile(fileName));
const { didOpen, didFailToOpen } = yield* race({
didOpen: take(fileStorageDidOpenFile.when((a) => a.path === fileName)),
didFailToOpen: take(
fileStorageDidFailToOpenFile.when((a) => a.path === fileName),
),
});
if (didFailToOpen) {
throw didFailToOpen.error;
}
defined(didOpen);
yield* put(
fileStorageWriteFile(
didOpen.id,
getPybricksMicroPythonFileTemplate(action.hub) || '',
),
);
const { didFailToWrite } = yield* race({
didWrite: take(fileStorageDidWriteFile.when((a) => a.id === didOpen.id)),
didFailToWrite: take(
fileStorageDidFailToWriteFile.when((a) => a.id === didOpen.id),
),
});
if (didFailToWrite) {
throw didFailToWrite.error;
}
yield* put(explorerDidCreateNewFile());
} catch (err) {
yield* put(explorerDidFailToCreateNewFile(ensureError(err)));
}
}
/** Connects user initiate rename file actions to the rename file dialog. */
@@ -124,31 +188,15 @@ function* handleExplorerRenameFile(
defined(accepted);
yield* put(fileStorageOpenFile(accepted.oldName));
const didOpen = yield* race({
succeeded: take(
fileStorageDidOpenFile.when((a) => a.path === accepted.oldName),
),
failed: take(
fileStorageDidFailToOpenFile.when((a) => a.path === accepted.oldName),
),
});
if (didOpen.failed) {
yield* put(explorerDidFailToRenameFile());
return;
}
defined(didOpen.succeeded);
const { id } = didOpen.succeeded;
yield* put(fileStorageRenameFile(id, accepted.newName));
yield* put(fileStorageRenameFile(action.fileName, accepted.newName));
const didRename = yield* race({
succeeded: take(fileStorageDidRenameFile.when((a) => a.id === id)),
failed: take(fileStorageDidFailToRenameFile.when((a) => a.id === id)),
succeeded: take(
fileStorageDidRenameFile.when((a) => a.fileName === action.fileName),
),
failed: take(
fileStorageDidFailToRenameFile.when((a) => a.fileName === action.fileName),
),
});
if (didRename.failed) {
+84 -63
View File
@@ -3,14 +3,33 @@
import { createAction } from '../actions';
/** Type to avoid mixing UUID with regular string. */
export type UUID = string & { _uuidBrand: undefined };
/**
* Database metadata table data type.
*
* IMPORTANT: if this type is changed, we need to modify the database schema to match
*/
export type FileMetadata = Readonly<{
/** A globally unique identifier that serves a a file handle. */
uuid: UUID;
/** The path of the file in storage. */
path: string;
/** The SHA256 hash of the file contents. */
sha256: string;
}>;
/**
* Action that indicates that the storage backend is ready to use.
* @param fileNames List of all files currently in storage.
* @param files List of all files currently in storage.
*/
export const fileStorageDidInitialize = createAction((fileNames: string[]) => ({
type: 'fileStorage.action.didInitialize',
fileNames,
}));
export const fileStorageDidInitialize = createAction(
(files: readonly FileMetadata[]) => ({
type: 'fileStorage.action.didInitialize',
files,
}),
);
/**
* Action that indicates that the storage backend failed to initialize.
@@ -23,29 +42,33 @@ export const fileStorageDidFailToInitialize = createAction((error: Error) => ({
/**
* Action that indicates that an item in the storage was created by us or in another tab.
* @param id The file handle UUID.
* @param file The file metadata.
*/
export const fileStorageDidAddItem = createAction((id: string) => ({
export const fileStorageDidAddItem = createAction((file: FileMetadata) => ({
type: 'fileStorage.action.didAddItem',
id,
file,
}));
/**
* Action that indicates that an item in the storage was changed by us or in another tab.
* @param id The file handle UUID.
* @param file The old file metadata.
* @param file The file metadata.
*/
export const fileStorageDidChangeItem = createAction((id: string) => ({
type: 'fileStorage.action.didChangeItem',
id,
}));
export const fileStorageDidChangeItem = createAction(
(oldFile: FileMetadata, file: FileMetadata) => ({
type: 'fileStorage.action.didChangeItem',
oldFile,
file,
}),
);
/**
* Action that indicates that an item in the storage was removed by us or in another tab.
* @param id The file handle UUID.
* @param file The file metadata.
*/
export const fileStorageDidRemoveItem = createAction((id: string) => ({
export const fileStorageDidRemoveItem = createAction((file: FileMetadata) => ({
type: 'fileStorage.action.didRemoveItem',
id,
file,
}));
/**
@@ -62,7 +85,7 @@ export const fileStorageOpenFile = createAction((path: string) => ({
* @param path The file path.
* @param id The file handle UUID.
*/
export const fileStorageDidOpenFile = createAction((path: string, id: string) => ({
export const fileStorageDidOpenFile = createAction((path: string, id: UUID) => ({
type: 'fileStorage.action.DidOpen',
path,
id,
@@ -84,7 +107,7 @@ export const fileStorageDidFailToOpenFile = createAction(
* Requests to read a file from storage.
* @param id The file handle UUID.
*/
export const fileStorageReadFile = createAction((id: string) => ({
export const fileStorageReadFile = createAction((id: UUID) => ({
type: 'fileStorage.action.readFile',
id,
}));
@@ -94,7 +117,7 @@ export const fileStorageReadFile = createAction((id: string) => ({
* @param id The file handle UUID.
* @param contents The contents of the file.
*/
export const fileStorageDidReadFile = createAction((id: string, contents: string) => ({
export const fileStorageDidReadFile = createAction((id: UUID, contents: string) => ({
type: 'fileStorage.action.didReadFile',
id,
contents,
@@ -105,20 +128,18 @@ export const fileStorageDidReadFile = createAction((id: string, contents: string
* @param id The file handle UUID.
* @param error The error.
*/
export const fileStorageDidFailToReadFile = createAction(
(id: string, error: Error) => ({
type: 'fileStorage.action.didFailToReadFile',
id,
error,
}),
);
export const fileStorageDidFailToReadFile = createAction((id: UUID, error: Error) => ({
type: 'fileStorage.action.didFailToReadFile',
id,
error,
}));
/**
* Requests to write a file to storage.
* @param id The file handle UUID.
* @param contents The contents of the file.
*/
export const fileStorageWriteFile = createAction((id: string, contents: string) => ({
export const fileStorageWriteFile = createAction((id: UUID, contents: string) => ({
type: 'fileStorage.action.writeFile',
id,
contents,
@@ -128,7 +149,7 @@ export const fileStorageWriteFile = createAction((id: string, contents: string)
* Response to write file request indicating success.
* @param id The file handle UUID.
*/
export const fileStorageDidWriteFile = createAction((id: string) => ({
export const fileStorageDidWriteFile = createAction((id: UUID) => ({
type: 'fileStorage.action.didWriteFile',
id,
}));
@@ -138,74 +159,74 @@ export const fileStorageDidWriteFile = createAction((id: string) => ({
* @param id The file handle UUID.
* @param error The error.
*/
export const fileStorageDidFailToWriteFile = createAction(
(id: string, error: Error) => ({
type: 'fileStorage.action.didFailToWriteFile',
id,
error,
}),
);
export const fileStorageDidFailToWriteFile = createAction((id: UUID, error: Error) => ({
type: 'fileStorage.action.didFailToWriteFile',
id,
error,
}));
/**
* Request to delete a file from storage.
* @param id The file handle UUID.
*/
export const fileStorageDeleteFile = createAction((id: string) => ({
export const fileStorageDeleteFile = createAction((fileName: string) => ({
type: 'fileStorage.action.deleteFile',
id,
fileName,
}));
/**
* Indicates that {@link fileStorageDeleteFile} succeeded.
* @param id The file handle UUID.
* @param fileName The file handle UUID.
*/
export const fileStorageDidDeleteFile = createAction((id: string) => ({
export const fileStorageDidDeleteFile = createAction((fileName: string) => ({
type: 'fileStorage.action.didDeleteFile',
id,
fileName,
}));
/**
* Indicates that {@link fileStorageDeleteFile} failed.
* @param id The file handle UUID.
* @param fileName The file handle UUID.
* @param error The error.
*/
export const fileStorageDidFailToDeleteFile = createAction(
(id: string, error: Error) => ({
(fileName: string, error: Error) => ({
type: 'fileStorage.action.didFailToDeleteFile',
id,
fileName,
error,
}),
);
/**
* Requests for a file to be renamed.
* @param id The file handle UUID.
* @param fileName The file handle UUID.
* @param newName The new name for the file.
*/
export const fileStorageRenameFile = createAction((id: string, newName: string) => ({
type: 'fileStorage.action.renameFile',
id,
newName,
}));
export const fileStorageRenameFile = createAction(
(fileName: string, newName: string) => ({
type: 'fileStorage.action.renameFile',
fileName,
newName,
}),
);
/**
* Indicates that fileStorageRenameFile(oldName, newName) succeeded.
* @param id The file handle UUID.
* @param fileName The file handle UUID.
*/
export const fileStorageDidRenameFile = createAction((id: string) => ({
export const fileStorageDidRenameFile = createAction((fileName: string) => ({
type: 'fileStorage.action.didRenameFile',
id,
fileName,
}));
/**
* Indicates that fileStorageRenameFile(oldName, newName) failed.
* @param id The file handle UUID.
* @param fileName The file handle UUID.
* @param error The error.
*/
export const fileStorageDidFailToRenameFile = createAction(
(id: string, error: Error) => ({
(fileName: string, error: Error) => ({
type: 'fileStorage.action.didFailToRenameFile',
id,
fileName,
error,
}),
);
@@ -214,29 +235,29 @@ export const fileStorageDidFailToRenameFile = createAction(
* Request to export (download) a file.
* @param id The file handle UUID.
*/
export const fileStorageExportFile = createAction((id: string) => ({
export const fileStorageExportFile = createAction((fileName: string) => ({
type: 'fileStorage.action.exportFile',
id,
fileName,
}));
/**
* Indicates that fileStorageExportFile(fileName) succeeded.
* @param id The file handle UUID.
* @param fileName The file handle UUID.
*/
export const fileStorageDidExportFile = createAction((id: string) => ({
export const fileStorageDidExportFile = createAction((fileName: string) => ({
type: 'fileStorage.action.didExportFile',
id,
fileName,
}));
/**
* Indicates that fileStorageExportFile(fileName) failed.
* @param id The file handle UUID.
* @param fileName The file name.
* @param error The error that was raised.
*/
export const fileStorageDidFailToExportFile = createAction(
(id: string, error: Error) => ({
(fileName: string, error: Error) => ({
type: 'fileStorage.action.didFailToExportFile',
id,
fileName,
error,
}),
);
+29 -19
View File
@@ -2,7 +2,9 @@
// Copyright (c) 2022 The Pybricks Authors
import { AnyAction } from 'redux';
import { uuid } from '../../test';
import {
FileMetadata,
fileStorageDidAddItem,
fileStorageDidChangeItem,
fileStorageDidInitialize,
@@ -15,7 +17,7 @@ type State = ReturnType<typeof reducers>;
test('initial state', () => {
expect(reducers(undefined, {} as AnyAction)).toMatchInlineSnapshot(`
Object {
"fileNames": Array [],
"files": Array [],
"isInitialized": false,
}
`);
@@ -29,37 +31,45 @@ test('isInitialized', () => {
});
test('fileNames', () => {
const testFileName = 'test.file';
const testFile: FileMetadata = {
uuid: uuid(0),
path: 'test.file',
sha256: '',
};
const modifiedFile: FileMetadata = { ...testFile, path: 'modified.file' };
expect(testFile).not.toEqual(modifiedFile);
// initialization populates file list
expect(
reducers(
{ fileNames: [] as ReadonlyArray<string> } as State,
fileStorageDidInitialize([testFileName]),
).fileNames,
).toEqual([testFileName]);
{ files: [] as readonly FileMetadata[] } as State,
fileStorageDidInitialize([testFile]),
).files,
).toEqual([testFile]);
// adding appends an item
expect(
reducers(
{ fileNames: [] as ReadonlyArray<string> } as State,
fileStorageDidAddItem(testFileName),
).fileNames,
).toEqual([testFileName]);
{ files: [] as readonly FileMetadata[] } as State,
fileStorageDidAddItem(testFile),
).files,
).toEqual([testFile]);
// changing does nothing
// changing replaces an item
expect(
reducers(
{ fileNames: [testFileName] as ReadonlyArray<string> } as State,
fileStorageDidChangeItem(testFileName),
).fileNames,
).toEqual([testFileName]);
{ files: [testFile] as readonly FileMetadata[] } as State,
fileStorageDidChangeItem(testFile, modifiedFile),
).files,
).toEqual([modifiedFile]);
// removing deletes an item
expect(
reducers(
{ fileNames: [testFileName] as ReadonlyArray<string> } as State,
fileStorageDidRemoveItem(testFileName),
).fileNames,
).not.toContain(testFileName);
{ files: [testFile] as readonly FileMetadata[] } as State,
fileStorageDidRemoveItem(testFile),
).files,
).not.toContain(testFile);
});
+7 -6
View File
@@ -3,6 +3,7 @@
import { Reducer, combineReducers } from 'redux';
import {
FileMetadata,
fileStorageDidAddItem,
fileStorageDidChangeItem,
fileStorageDidInitialize,
@@ -17,24 +18,24 @@ const isInitialized: Reducer<boolean> = (state = false, action) => {
return state;
};
const fileNames: Reducer<ReadonlyArray<string>> = (state = [], action) => {
const files: Reducer<readonly FileMetadata[]> = (state = [], action) => {
if (fileStorageDidInitialize.matches(action)) {
return [...action.fileNames];
return [...action.files];
}
if (fileStorageDidAddItem.matches(action)) {
return [...state, action.id];
return [...state, action.file];
}
if (fileStorageDidChangeItem.matches(action)) {
return state;
return [...state].map((f) => (f.uuid === action.file.uuid ? action.file : f));
}
if (fileStorageDidRemoveItem.matches(action)) {
return [...state].filter((value) => value !== action.id);
return [...state].filter((value) => value.uuid !== action.file.uuid);
}
return state;
};
export default combineReducers({ isInitialized, fileNames });
export default combineReducers({ isInitialized, files });
+71 -28
View File
@@ -8,6 +8,7 @@ import 'dexie-observable';
import { AsyncSaga, uuid } from '../../test';
import { createCountFunc } from '../utils/iter';
import {
FileMetadata,
fileStorageArchiveAllFiles,
fileStorageDeleteFile,
fileStorageDidAddItem,
@@ -55,12 +56,24 @@ afterEach(async () => {
/**
* helper function that writes test file to storage for later use in a test
* @param saga The saga.
* @returns The test file id and test file contents.
* @returns The test file metadata and test file contents.
*/
async function setUpTestFile(saga: AsyncSaga): Promise<[string, string]> {
async function setUpTestFile(saga: AsyncSaga): Promise<[FileMetadata, string]> {
const testFilePath = 'test.file';
const testFileId = uuid(0);
const testFileContents = 'test file contents';
const testFileContentsSha256 =
'c4fa968a745586faaa030054f51fb1cafd5e9ae25fa6b137ac6477715fdc81b1';
const testFile: FileMetadata = {
uuid: testFileId,
path: testFilePath,
sha256: testFileContentsSha256,
};
const emptyFileSha256 =
'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855';
const emptyFile: FileMetadata = { ...testFile, sha256: emptyFileSha256 };
await expect(saga.take()).resolves.toEqual(fileStorageDidInitialize([]));
@@ -69,19 +82,23 @@ async function setUpTestFile(saga: AsyncSaga): Promise<[string, string]> {
await expect(saga.take()).resolves.toEqual(
fileStorageDidOpenFile(testFilePath, testFileId),
);
await expect(saga.take()).resolves.toEqual(fileStorageDidAddItem(testFileId));
await expect(saga.take()).resolves.toEqual(fileStorageDidAddItem(emptyFile));
saga.put(fileStorageWriteFile(testFileId, testFileContents));
await expect(saga.take()).resolves.toEqual(fileStorageDidWriteFile(testFileId));
await expect(saga.take()).resolves.toEqual(fileStorageDidChangeItem(testFileId));
await expect(saga.take()).resolves.toEqual(
fileStorageDidChangeItem(emptyFile, testFile),
);
return [testFileId, testFileContents];
return [testFile, testFileContents];
}
it('should migrate old program from local storage during initialization', async () => {
const oldProgramKey = 'program';
const oldProgramContents = '# test program';
const oldProgramContentsSha256 =
'31c21eb39c9276341d9364f6d4bcac46a4aa3768bc2626f8aa742c46e3e0fdd6';
// add item to localStorage to simulate an existing program
localStorage.setItem(oldProgramKey, oldProgramContents);
@@ -91,7 +108,11 @@ it('should migrate old program from local storage during initialization', async
// initialization should remove the localStorage entry and add add it to
// new storage backend
await expect(saga.take()).resolves.toEqual(fileStorageDidInitialize(['main.py']));
await expect(saga.take()).resolves.toEqual(
fileStorageDidInitialize([
{ uuid: uuid(0), path: 'main.py', sha256: oldProgramContentsSha256 },
]),
);
expect(localStorage.getItem(oldProgramKey)).toBeNull();
await saga.end();
@@ -102,31 +123,45 @@ it('should read and write files', async () => {
await expect(saga.take()).resolves.toEqual(fileStorageDidInitialize([]));
const testFile = 'test.file';
const testId = uuid(0);
const testFilePath = 'test.file';
const testFileId = uuid(0);
const testFileContents = 'test file contents';
const testFileContentsSha256 =
'c4fa968a745586faaa030054f51fb1cafd5e9ae25fa6b137ac6477715fdc81b1';
saga.put(fileStorageOpenFile(testFile));
const testFile: FileMetadata = {
uuid: testFileId,
path: testFilePath,
sha256: testFileContentsSha256,
};
const emptyFileSha256 =
'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855';
const emptyFile: FileMetadata = { ...testFile, sha256: emptyFileSha256 };
saga.put(fileStorageOpenFile(testFilePath));
await expect(saga.take()).resolves.toEqual(
fileStorageDidOpenFile(testFile, testId),
fileStorageDidOpenFile(testFilePath, testFileId),
);
await expect(saga.take()).resolves.toEqual(fileStorageDidAddItem(testId));
await expect(saga.take()).resolves.toEqual(fileStorageDidAddItem(emptyFile));
// test writing a file
saga.put(fileStorageWriteFile(testId, testFileContents));
saga.put(fileStorageWriteFile(testFileId, testFileContents));
// writing file triggers response
await expect(saga.take()).resolves.toEqual(fileStorageDidWriteFile(testId));
await expect(saga.take()).resolves.toEqual(fileStorageDidWriteFile(testFileId));
// and as a side-effect, triggers item change as well
await expect(saga.take()).resolves.toEqual(fileStorageDidChangeItem(testId));
await expect(saga.take()).resolves.toEqual(
fileStorageDidChangeItem(emptyFile, testFile),
);
// test reading the same file back
saga.put(fileStorageReadFile(testId));
saga.put(fileStorageReadFile(testFileId));
await expect(saga.take()).resolves.toEqual(
fileStorageDidReadFile(testId, testFileContents),
fileStorageDidReadFile(testFileId, testFileContents),
);
await saga.end();
@@ -137,12 +172,12 @@ it('should dispatch fail action if file does not exist', async () => {
await expect(saga.take()).resolves.toEqual(fileStorageDidInitialize([]));
const testFile = 'test.file';
const testFileId = uuid(0);
saga.put(fileStorageReadFile(testFile));
saga.put(fileStorageReadFile(testFileId));
await expect(saga.take()).resolves.toEqual(
fileStorageDidFailToReadFile('test.file', new Error('file does not exist')),
fileStorageDidFailToReadFile(testFileId, new Error('file does not exist')),
);
await saga.end();
@@ -153,9 +188,9 @@ it('should delete files', async () => {
const [testFile] = await setUpTestFile(saga);
saga.put(fileStorageDeleteFile(testFile));
saga.put(fileStorageDeleteFile(testFile.path));
await expect(saga.take()).resolves.toEqual(fileStorageDidDeleteFile(testFile));
await expect(saga.take()).resolves.toEqual(fileStorageDidDeleteFile(testFile.path));
await expect(saga.take()).resolves.toEqual(fileStorageDidRemoveItem(testFile));
@@ -170,10 +205,16 @@ describe('rename', () => {
const [testFile] = await setUpTestFile(saga);
saga.put(fileStorageRenameFile(testFile, newName));
saga.put(fileStorageRenameFile(testFile.path, newName));
await expect(saga.take()).resolves.toEqual(fileStorageDidRenameFile(testFile));
await expect(saga.take()).resolves.toEqual(fileStorageDidChangeItem(testFile));
const newMetadata: FileMetadata = { ...testFile, path: newName };
await expect(saga.take()).resolves.toEqual(
fileStorageDidRenameFile(testFile.path),
);
await expect(saga.take()).resolves.toEqual(
fileStorageDidChangeItem(testFile, newMetadata),
);
await saga.end();
});
@@ -203,9 +244,11 @@ describe('export', () => {
jest.spyOn(browserFsAccess, 'fileSave');
saga.put(fileStorageExportFile(testFile));
saga.put(fileStorageExportFile(testFile.path));
await expect(saga.take()).resolves.toEqual(fileStorageDidExportFile(testFile));
await expect(saga.take()).resolves.toEqual(
fileStorageDidExportFile(testFile.path),
);
expect(browserFsAccess.fileSave).toHaveBeenCalled();
await saga.end();
@@ -219,10 +262,10 @@ describe('export', () => {
const testError = new Error('test error');
jest.spyOn(browserFsAccess, 'fileSave').mockRejectedValue(testError);
saga.put(fileStorageExportFile(testFile));
saga.put(fileStorageExportFile(testFile.path));
await expect(saga.take()).resolves.toEqual(
fileStorageDidFailToExportFile(testFile, testError),
fileStorageDidFailToExportFile(testFile.path, testError),
);
await saga.end();
+33 -29
View File
@@ -17,6 +17,8 @@ import { pythonFileExtension, pythonFileMimeType } from '../pybricksMicropython/
import { ensureError, timestamp } from '../utils';
import { sha256Digest } from '../utils/crypto';
import {
FileMetadata,
UUID,
fileStorageArchiveAllFiles,
fileStorageDeleteFile,
fileStorageDidAddItem,
@@ -98,16 +100,6 @@ function isFileMetaDataDeleteChange(change: IDeleteChange): change is Omit<
return change.table === 'metadata';
}
/** Database metadata table data type. */
type FileMetadata = {
/** A globally unique identifier that serves a a file handle. */
uuid: string;
/** The path of the file in storage. */
path: string;
/** The SHA256 hash of the file contents. */
sha256: string;
};
/** Database contents table data type. */
type FileContents = {
/** The path of the file in storage. */
@@ -117,7 +109,7 @@ type FileContents = {
};
class FileStorageDb extends Dexie {
metadata!: Table<FileMetadata, string>;
metadata!: Table<FileMetadata, UUID>;
// NB: This table starts with an underscore to hide it from Dexie observable.
// In the future we may change this to use File Access API or some other
// storage, so we don't want to rely on the file contents being included
@@ -127,7 +119,7 @@ class FileStorageDb extends Dexie {
constructor() {
super('pybricks.fileStorage');
this.version(1).stores({
metadata: '$$uuid, &path',
metadata: '$$uuid, &path, sha256',
_contents: 'path, contents',
});
}
@@ -141,15 +133,15 @@ function* handleFileStorageDidChange(changes: IDatabaseChange[]): Generator {
for (const change of changes) {
if (isCreateChange(change)) {
if (isFileMetadataCreateChange(change)) {
yield* put(fileStorageDidAddItem(change.obj.uuid));
yield* put(fileStorageDidAddItem(change.obj));
}
} else if (isUpdateChange(change)) {
if (isFileMetadataUpdateChange(change)) {
yield* put(fileStorageDidChangeItem(change.obj.uuid));
yield* put(fileStorageDidChangeItem(change.oldObj, change.obj));
}
} else if (isDeleteChange(change)) {
if (isFileMetaDataDeleteChange(change)) {
yield* put(fileStorageDidRemoveItem(change.oldObj.uuid));
yield* put(fileStorageDidRemoveItem(change.oldObj));
}
}
}
@@ -270,7 +262,10 @@ function* handleExportFile(
): Generator {
const file = yield* call(() =>
db.transaction('r', db.metadata, db._contents, async () => {
const metadata = await db.metadata.get(action.id);
const metadata = await db.metadata
.where('path')
.equals(action.fileName)
.first();
if (!metadata) {
return undefined;
@@ -282,7 +277,10 @@ function* handleExportFile(
if (!file) {
yield* put(
fileStorageDidFailToExportFile(action.id, new Error('file does not exist')),
fileStorageDidFailToExportFile(
action.fileName,
new Error('file does not exist'),
),
);
return;
}
@@ -301,9 +299,9 @@ function* handleExportFile(
}),
);
yield* put(fileStorageDidExportFile(action.id));
yield* put(fileStorageDidExportFile(action.fileName));
} catch (err) {
yield* put(fileStorageDidFailToExportFile(action.id, ensureError(err)));
yield* put(fileStorageDidFailToExportFile(action.fileName, ensureError(err)));
}
}
@@ -319,19 +317,22 @@ function* handleDeleteFile(
try {
yield* call(() =>
db.transaction('rw', db.metadata, db._contents, async () => {
const metadata = await db.metadata.get(action.id);
const metadata = await db.metadata
.where('path')
.equals(action.fileName)
.first();
if (!metadata) {
throw new Error(`file handle '${action.id}' does not exist`);
throw new Error(`file '${action.fileName}' does not exist`);
}
await db.metadata.delete(action.id);
await db.metadata.delete(metadata.uuid);
await db._contents.delete(metadata.path);
}),
);
yield* put(fileStorageDidDeleteFile(action.id));
yield* put(fileStorageDidDeleteFile(action.fileName));
} catch (err) {
yield* put(fileStorageDidFailToDeleteFile(action.id, ensureError(err)));
yield* put(fileStorageDidFailToDeleteFile(action.fileName, ensureError(err)));
}
}
@@ -347,10 +348,13 @@ function* handleRenameFile(
try {
yield* call(() =>
db.transaction('rw', db.metadata, db._contents, async () => {
const metadata = await db.metadata.get(action.id);
const metadata = await db.metadata
.where('path')
.equals(action.fileName)
.first();
if (!metadata) {
throw new Error(`file handle '${action.id}' does not exist`);
throw new Error(`file '${action.fileName}' does not exist`);
}
const oldName = metadata.path;
@@ -376,9 +380,9 @@ function* handleRenameFile(
}),
);
yield* put(fileStorageDidRenameFile(action.id));
yield* put(fileStorageDidRenameFile(action.fileName));
} catch (err) {
yield* put(fileStorageDidFailToRenameFile(action.id, ensureError(err)));
yield* put(fileStorageDidFailToRenameFile(action.fileName, ensureError(err)));
}
}
@@ -467,7 +471,7 @@ function* initialize(): Generator {
const files = yield* call(() => db.metadata.toArray());
yield* put(fileStorageDidInitialize(files.map((f) => f.path)));
yield* put(fileStorageDidInitialize(files));
// this blocks "forever" until canceled so that the finally
// clause will run cleanup code at the appropriate time
+10 -9
View File
@@ -9,22 +9,23 @@ import {
} from '@pybricks/firmware';
import { I18nManager } from '@shopify/react-i18n';
import { AnyAction } from 'redux';
import { AsyncSaga } from '../../test';
import { AsyncSaga, uuid } from '../../test';
import { appDidCheckForUpdate } from '../app/actions';
import { bleDIServiceDidReceiveFirmwareRevision } from '../ble-device-info-service/actions';
import {
BleDeviceFailToConnectReasonType,
didFailToConnect as bleDidFailToConnect,
} from '../ble/actions';
import { explorerDeleteFile, explorerDidFailToImportFiles } from '../explorer/actions';
import {
explorerDeleteFile,
explorerDidFailToCreateNewFile,
explorerDidFailToImportFiles,
} from '../explorer/actions';
import {
fileStorageDeleteFile,
fileStorageDidFailToArchiveAllFiles,
fileStorageDidFailToDeleteFile,
fileStorageDidFailToExportFile,
fileStorageDidFailToInitialize,
fileStorageDidFailToReadFile,
fileStorageDidFailToWriteFile,
fileStorageDidRemoveItem,
} from '../fileStorage/actions';
import {
@@ -111,12 +112,10 @@ test.each([
appDidCheckForUpdate(false),
bleDIServiceDidReceiveFirmwareRevision('3.0.0'),
fileStorageDidFailToInitialize(new Error('test error')),
fileStorageDidFailToReadFile('test.file', new Error('test error')),
fileStorageDidFailToWriteFile('test.file', new Error('test error')),
fileStorageDidFailToDeleteFile('test.file', new Error('test error')),
fileStorageDidFailToExportFile('test.file', new Error('test error')),
fileStorageDidFailToArchiveAllFiles(new Error('test error')),
explorerDidFailToImportFiles(new Error('test error')),
explorerDidFailToCreateNewFile(new Error('test error')),
])('actions that should show notification: %o', async (action: AnyAction) => {
const { toaster, saga } = createTestToasterSaga();
@@ -214,7 +213,9 @@ describe('delete file saga', () => {
toaster.getToasts().find((t) => t.key === I18nId.ExplorerDeleteFileMessage),
).toBeDefined();
saga.put(fileStorageDidRemoveItem('test.file'));
saga.put(
fileStorageDidRemoveItem({ uuid: uuid(0), path: 'test.file', sha256: '' }),
);
expect(
toaster.getToasts().find((t) => t.key === I18nId.ExplorerDeleteFileMessage),
+13 -26
View File
@@ -17,15 +17,16 @@ import {
BleDeviceFailToConnectReasonType,
didFailToConnect as bleDeviceDidFailToConnect,
} from '../ble/actions';
import { explorerDeleteFile, explorerDidFailToImportFiles } from '../explorer/actions';
import {
explorerDeleteFile,
explorerDidFailToCreateNewFile,
explorerDidFailToImportFiles,
} from '../explorer/actions';
import {
fileStorageDeleteFile,
fileStorageDidFailToArchiveAllFiles,
fileStorageDidFailToDeleteFile,
fileStorageDidFailToExportFile,
fileStorageDidFailToInitialize,
fileStorageDidFailToReadFile,
fileStorageDidFailToWriteFile,
fileStorageDidRemoveItem,
} from '../fileStorage/actions';
import { FailToFinishReasonType, didFailToFinish } from '../firmware/actions';
@@ -386,24 +387,6 @@ function* showFileStorageFailToInitialize(
yield* showUnexpectedError(I18nId.FileStorageFailedToInitialize, action.error);
}
function* showFileStorageFailToRead(
action: ReturnType<typeof fileStorageDidFailToReadFile>,
): Generator {
yield* showUnexpectedError(I18nId.FileStorageFailedToRead, action.error);
}
function* showFileStorageFailToWrite(
action: ReturnType<typeof fileStorageDidFailToWriteFile>,
): Generator {
yield* showUnexpectedError(I18nId.FileStorageFailedToWrite, action.error);
}
function* showFileStorageFailToDelete(
action: ReturnType<typeof fileStorageDidFailToDeleteFile>,
): Generator {
yield* showUnexpectedError(I18nId.FileStorageFailedToDelete, action.error);
}
function* showFileStorageFailToExport(
action: ReturnType<typeof fileStorageDidFailToExportFile>,
): Generator {
@@ -445,7 +428,7 @@ function* showDeleteFileWarning(action: ReturnType<typeof explorerDeleteFile>) {
const { didRemoveFile } = yield* race({
userActionEvent: take(ch),
didRemoveFile: take(
fileStorageDidRemoveItem.when((a) => a.id === action.fileName),
fileStorageDidRemoveItem.when((a) => a.file.path === action.fileName),
),
});
@@ -472,6 +455,12 @@ function* showExplorerFailToImportFiles(
yield* showUnexpectedError(I18nId.ExplorerFailedToImportFiles, action.error);
}
function* showExplorerFailToCreateFile(
action: ReturnType<typeof explorerDidFailToCreateNewFile>,
): Generator {
yield* showUnexpectedError(I18nId.FileStorageFailedToDelete, action.error);
}
export default function* (): Generator {
yield* takeEvery(bleDeviceDidFailToConnect, showBleDeviceDidFailToConnectError);
yield* takeEvery(bootloaderDidFailToConnect, showBootloaderDidFailToConnectError);
@@ -483,11 +472,9 @@ export default function* (): Generator {
yield* takeEvery(appDidCheckForUpdate, showNoUpdateInfo);
yield* takeEvery(bleDIServiceDidReceiveFirmwareRevision, checkVersion);
yield* takeEvery(fileStorageDidFailToInitialize, showFileStorageFailToInitialize);
yield* takeEvery(fileStorageDidFailToReadFile, showFileStorageFailToRead);
yield* takeEvery(fileStorageDidFailToWriteFile, showFileStorageFailToWrite);
yield* takeEvery(fileStorageDidFailToDeleteFile, showFileStorageFailToDelete);
yield* takeEvery(fileStorageDidFailToExportFile, showFileStorageFailToExport);
yield* takeEvery(fileStorageDidFailToArchiveAllFiles, showFileStorageFailToArchive);
yield* takeEvery(explorerDeleteFile, showDeleteFileWarning);
yield* takeEvery(explorerDidFailToImportFiles, showExplorerFailToImportFiles);
yield* takeEvery(explorerDidFailToCreateNewFile, showExplorerFailToCreateFile);
}