From 1515e5ae4f9738fec192873bac8dbae22a1e1546 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 1 Apr 2022 13:34:46 -0500 Subject: [PATCH] fileStorage: add file handles This changes how files work. There is now an open function that translates a path to a file handle (id). Then this handle is used to perform other actions on the file. The file contents are moved to a separate table so that the actual file storage is independent of the database (e.g. in the future, we may use File Access API). We store a hash of the file contents in the metadata file so we can detect file changes without having to compare file contents. We also separate the change and add actions. --- package.json | 4 +- src/editor/sagas.ts | 32 +++-- src/explorer/sagas.test.ts | 60 +++++---- src/explorer/sagas.ts | 33 +++-- src/fileStorage/actions.ts | 216 ++++++++++++++++++++----------- src/fileStorage/reducers.test.ts | 9 +- src/fileStorage/reducers.ts | 13 +- src/fileStorage/sagas.test.ts | 117 +++++++++-------- src/fileStorage/sagas.ts | 170 +++++++++++++++--------- src/notifications/sagas.ts | 2 +- src/setupTests.ts | 5 + src/utils/crypto.test.ts | 10 ++ src/utils/crypto.ts | 11 ++ test/index.tsx | 13 ++ yarn.lock | 22 ++-- 15 files changed, 462 insertions(+), 255 deletions(-) create mode 100644 src/utils/crypto.test.ts create mode 100644 src/utils/crypto.ts diff --git a/package.json b/package.json index 63247969..cd496034 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "@testing-library/user-event": "^13.5.0", "@types/file-saver": "^2.0.5", "@types/jest": "^27.4.1", - "@types/node": "^12.20.47", + "@types/node": "^16.11.7", "@types/react": "^16.14.24", "@types/react-dom": "^16.9.14", "@types/react-redux": "^7.1.23", @@ -36,7 +36,7 @@ "canvas": "^2.9.0", "copy-webpack-plugin": "^6.4.1", "dexie": "^3.2.1", - "dexie-observable": "^3.0.0-beta.11", + "dexie-observable": "^4.0.0-beta.13", "fake-indexeddb": "^3.1.7", "jszip": "^3.7.1", "license-webpack-plugin": "^3.0.0", diff --git a/src/editor/sagas.ts b/src/editor/sagas.ts index 40a1dc48..5afde736 100644 --- a/src/editor/sagas.ts +++ b/src/editor/sagas.ts @@ -14,12 +14,16 @@ import { takeEvery, } from 'typed-redux-saga/macro'; import { + fileStorageDidFailToOpenFile, fileStorageDidFailToReadFile, fileStorageDidInitialize, + fileStorageDidOpenFile, fileStorageDidReadFile, + fileStorageOpenFile, fileStorageReadFile, } from '../fileStorage/actions'; import { RootState } from '../reducers'; +import { defined } from '../utils'; import { editorDidCreate, editorGetValueRequest, @@ -69,19 +73,31 @@ function* handleDidCreateEditor(editor: monaco.editor.ICodeEditor): Generator { // then we can load the most recently used file // REVISIT: should this be here or elsewhere? - yield* put(fileStorageReadFile('main.py')); + yield* put(fileStorageOpenFile('main.py')); - const { succeeded } = yield* race({ - succeeded: take(fileStorageDidReadFile.when((a) => a.fileName === 'main.py')), - failed: take( - fileStorageDidFailToReadFile.when((a) => a.fileName === 'main.py'), - ), + const didOpen = yield* race({ + succeeded: take(fileStorageDidOpenFile.when((a) => a.path === 'main.py')), + failed: take(fileStorageDidFailToOpenFile.when((a) => a.path === 'main.py')), }); // TODO: what to do in case of failure? + if (didOpen.succeeded) { + defined(didOpen.succeeded); - if (succeeded) { - editor.setValue(succeeded.fileContents); + const { id } = didOpen.succeeded; + + yield* put(fileStorageReadFile(id)); + + const didRead = yield* race({ + succeeded: take(fileStorageDidReadFile.when((a) => a.id === id)), + failed: take(fileStorageDidFailToReadFile.when((a) => a.id === id)), + }); + + // TODO: what to do in case of failure? + if (didRead.succeeded) { + const { contents } = didRead.succeeded; + editor.setValue(contents); + } } yield* takeEvery(editorGetValueRequest, handleEditorGetValueRequest, editor); diff --git a/src/explorer/sagas.test.ts b/src/explorer/sagas.test.ts index 898ae83d..d4dfb323 100644 --- a/src/explorer/sagas.test.ts +++ b/src/explorer/sagas.test.ts @@ -4,10 +4,13 @@ import * as browserFsAccess from 'browser-fs-access'; import { FileWithHandle } from 'browser-fs-access'; import { mock } from 'jest-mock-extended'; -import { AsyncSaga } from '../../test'; +import { AsyncSaga, uuid } from '../../test'; import { + fileStorageDidFailToOpenFile, fileStorageDidFailToRenameFile, + fileStorageDidOpenFile, fileStorageDidRenameFile, + fileStorageOpenFile, fileStorageRenameFile, fileStorageWriteFile, } from '../fileStorage/actions'; @@ -79,7 +82,7 @@ describe('handleExplorerCreateNewFile', () => { const action = await saga.take(); expect(action).toMatchInlineSnapshot(` Object { - "fileContents": "from pybricks.hubs import TechnicHub + "contents": "from pybricks.hubs import TechnicHub from pybricks.pupdevices import Motor from pybricks.parameters import Button, Color, Direction, Port, Stop from pybricks.robotics import DriveBase @@ -88,7 +91,7 @@ describe('handleExplorerCreateNewFile', () => { hub = TechnicHub() ", - "fileName": "test.py", + "id": "test.py", "type": "fileStorage.action.writeFile", } `); @@ -105,43 +108,52 @@ describe('handleExplorerRenameFile', () => { saga.put(explorerRenameFile('old.file')); - const action = await saga.take(); - expect(action).toEqual(renameFileDialogShow('old.file')); + await expect(saga.take()).resolves.toEqual(renameFileDialogShow('old.file')); }); it('should dispatch action if canceled', async () => { saga.put(renameFileDialogDidCancel()); - const action = await saga.take(); - expect(action).toEqual(explorerDidFailToRenameFile()); + await expect(saga.take()).resolves.toEqual(explorerDidFailToRenameFile()); }); - describe('should dispatch fileStorage action if accepted', () => { + describe('should dispatch fileStorageOpenFile action if accepted', () => { beforeEach(async () => { saga.put(renameFileDialogDidAccept('old.file', 'new.file')); - const action = await saga.take(); - expect(action).toEqual(fileStorageRenameFile('old.file', 'new.file')); + await expect(saga.take()).resolves.toEqual(fileStorageOpenFile('old.file')); }); - it('should dispatch action on fileStorage failure', async () => { - saga.put( - fileStorageDidFailToRenameFile( - 'old.file', - 'new.file', - new Error('test error'), - ), - ); + it('should dispatch action on fileStorageOpenFile failure', async () => { + saga.put(fileStorageDidFailToOpenFile('old.file', new Error('test error'))); - const action = await saga.take(); - expect(action).toEqual(explorerDidFailToRenameFile()); + await expect(saga.take()).resolves.toEqual(explorerDidFailToRenameFile()); }); - it('should dispatch action on fileStorage success', async () => { - saga.put(fileStorageDidRenameFile('old.file', 'new.file')); + describe('should dispatch fileStorageRenameFile action on fileStorageOpenFile success', () => { + beforeEach(async () => { + saga.put(fileStorageDidOpenFile('old.file', uuid(0))); - const action = await saga.take(); - expect(action).toEqual(explorerDidRenameFile()); + 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()); + }); }); }); diff --git a/src/explorer/sagas.ts b/src/explorer/sagas.ts index 980d6b67..a8a90608 100644 --- a/src/explorer/sagas.ts +++ b/src/explorer/sagas.ts @@ -13,8 +13,11 @@ import { } from 'typed-redux-saga/macro'; import { getPybricksMicroPythonFileTemplate } from '../editor/pybricksMicroPython'; import { + fileStorageDidFailToOpenFile, fileStorageDidFailToRenameFile, + fileStorageDidOpenFile, fileStorageDidRenameFile, + fileStorageOpenFile, fileStorageRenameFile, fileStorageWriteFile, } from '../fileStorage/actions'; @@ -121,22 +124,34 @@ function* handleExplorerRenameFile( defined(accepted); - yield* put(fileStorageRenameFile(accepted.oldName, accepted.newName)); + yield* put(fileStorageOpenFile(accepted.oldName)); - const { failed } = yield* race({ + const didOpen = yield* race({ succeeded: take( - fileStorageDidRenameFile.when( - (a) => a.oldName === accepted.oldName && a.newName === accepted.newName, - ), + fileStorageDidOpenFile.when((a) => a.path === accepted.oldName), ), failed: take( - fileStorageDidFailToRenameFile.when( - (a) => a.oldName === accepted.oldName && a.newName === accepted.newName, - ), + fileStorageDidFailToOpenFile.when((a) => a.path === accepted.oldName), ), }); - if (failed) { + if (didOpen.failed) { + yield* put(explorerDidFailToRenameFile()); + return; + } + + defined(didOpen.succeeded); + + const { id } = didOpen.succeeded; + + yield* put(fileStorageRenameFile(id, accepted.newName)); + + const didRename = yield* race({ + succeeded: take(fileStorageDidRenameFile.when((a) => a.id === id)), + failed: take(fileStorageDidFailToRenameFile.when((a) => a.id === id)), + }); + + if (didRename.failed) { yield* put(explorerDidFailToRenameFile()); return; } diff --git a/src/fileStorage/actions.ts b/src/fileStorage/actions.ts index c0adc8e9..2bc0e3eb 100644 --- a/src/fileStorage/actions.ts +++ b/src/fileStorage/actions.ts @@ -12,171 +12,231 @@ export const fileStorageDidInitialize = createAction((fileNames: string[]) => ({ fileNames, })); -/** Action that indicates that the storage backend failed to initialize. */ +/** + * Action that indicates that the storage backend failed to initialize. + * @param error The error. + */ export const fileStorageDidFailToInitialize = createAction((error: Error) => ({ type: 'fileStorage.action.didFailToInitialize', error, })); -/** Action that indicates that an item in the storage was created or changed by us or in another tab. */ -export const fileStorageDidChangeItem = createAction((fileName: string) => ({ +/** + * Action that indicates that an item in the storage was created by us or in another tab. + * @param id The file handle UUID. + */ +export const fileStorageDidAddItem = createAction((id: string) => ({ + type: 'fileStorage.action.didAddItem', + id, +})); + +/** + * Action that indicates that an item in the storage was changed by us or in another tab. + * @param id The file handle UUID. + */ +export const fileStorageDidChangeItem = createAction((id: string) => ({ type: 'fileStorage.action.didChangeItem', - fileName, + id, })); -/** Action that indicates that an item in the storage was removed by us or in another tab. */ -export const fileStorageDidRemoveItem = createAction((fileName: string) => ({ +/** + * Action that indicates that an item in the storage was removed by us or in another tab. + * @param id The file handle UUID. + */ +export const fileStorageDidRemoveItem = createAction((id: string) => ({ type: 'fileStorage.action.didRemoveItem', - fileName, + id, })); -/** Requests to read a file from storage. */ -export const fileStorageReadFile = createAction((fileName: string) => ({ - type: 'fileStorage.action.readFile', - fileName, +/** + * Action that requests to open a file in storage. + * @param path The file path. + */ +export const fileStorageOpenFile = createAction((path: string) => ({ + type: 'fileStorage.action.Open', + path, })); -/** Response to read file request indicating success. */ -export const fileStorageDidReadFile = createAction( - (fileName: string, fileContents: string) => ({ - type: 'fileStorage.action.didReadFile', - fileName, - fileContents, +/** + * Action that indicates that {@link fileStorageOpenFile} succeeded. + * @param path The file path. + * @param id The file handle UUID. + */ +export const fileStorageDidOpenFile = createAction((path: string, id: string) => ({ + type: 'fileStorage.action.DidOpen', + path, + id, +})); + +/** + * Action that indicates that {@link fileStorageOpenFile} failed. + * @param path The file path. + * @param error The error. + */ +export const fileStorageDidFailToOpenFile = createAction( + (path: string, error: Error) => ({ + type: 'fileStorage.action.DidFailToOpen', + path, + error, }), ); +/** + * Requests to read a file from storage. + * @param id The file handle UUID. + */ +export const fileStorageReadFile = createAction((id: string) => ({ + type: 'fileStorage.action.readFile', + id, +})); -/** Response to read file request indicating failure. */ +/** + * Response to read file request indicating success. + * @param id The file handle UUID. + * @param contents The contents of the file. + */ +export const fileStorageDidReadFile = createAction((id: string, contents: string) => ({ + type: 'fileStorage.action.didReadFile', + id, + contents, +})); + +/** + * Response to read file request indicating failure. + * @param id The file handle UUID. + * @param error The error. + */ export const fileStorageDidFailToReadFile = createAction( - (fileName: string, error: Error) => ({ + (id: string, error: Error) => ({ type: 'fileStorage.action.didFailToReadFile', - fileName, + id, error, }), ); -/** Requests to write a file to storage. */ -export const fileStorageWriteFile = createAction( - (fileName: string, fileContents: string) => ({ - type: 'fileStorage.action.writeFile', - fileName, - fileContents, - }), -); - -/** Response to write file request indicating success. */ -export const fileStorageDidWriteFile = createAction((fileName: string) => ({ - type: 'fileStorage.action.didWriteFile', - fileName, +/** + * 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) => ({ + type: 'fileStorage.action.writeFile', + id, + contents, })); -/** Response to write file request indicating failure. */ +/** + * Response to write file request indicating success. + * @param id The file handle UUID. + */ +export const fileStorageDidWriteFile = createAction((id: string) => ({ + type: 'fileStorage.action.didWriteFile', + id, +})); + +/** + * Response to write file request indicating failure. + * @param id The file handle UUID. + * @param error The error. + */ export const fileStorageDidFailToWriteFile = createAction( - (fileName: string, error: Error) => ({ + (id: string, error: Error) => ({ type: 'fileStorage.action.didFailToWriteFile', - fileName, + id, error, }), ); /** * Request to delete a file from storage. - * @param fileName The name of the file to delete. + * @param id The file handle UUID. */ -export const fileStorageDeleteFile = createAction((fileName: string) => ({ +export const fileStorageDeleteFile = createAction((id: string) => ({ type: 'fileStorage.action.deleteFile', - fileName, + id, })); /** - * Indicates that fileStorageDeleteFile(fileName) succeeded. - * @param fileName The name of the file that was deleted. + * Indicates that {@link fileStorageDeleteFile} succeeded. + * @param id The file handle UUID. */ -export const fileStorageDidDeleteFile = createAction((fileName: string) => ({ +export const fileStorageDidDeleteFile = createAction((id: string) => ({ type: 'fileStorage.action.didDeleteFile', - fileName, + id, })); /** - * Indicates that fileStorageDeleteFile(fileName) failed. - * @param fileName The name of the file that should have been deleted. + * Indicates that {@link fileStorageDeleteFile} failed. + * @param id The file handle UUID. * @param error The error. */ export const fileStorageDidFailToDeleteFile = createAction( - (fileName: string, error: Error) => ({ + (id: string, error: Error) => ({ type: 'fileStorage.action.didFailToDeleteFile', - fileName, + id, error, }), ); /** * Requests for a file to be renamed. - * @param oldName The name of a file that exists in storage. + * @param id The file handle UUID. * @param newName The new name for the file. */ -export const fileStorageRenameFile = createAction( - (oldName: string, newName: string) => ({ - type: 'fileStorage.action.renameFile', - oldName, - newName, - }), -); +export const fileStorageRenameFile = createAction((id: string, newName: string) => ({ + type: 'fileStorage.action.renameFile', + id, + newName, +})); /** * Indicates that fileStorageRenameFile(oldName, newName) succeeded. - * @param oldName The previous file name. - * @param newName The current file name. + * @param id The file handle UUID. */ -export const fileStorageDidRenameFile = createAction( - (oldName: string, newName: string) => ({ - type: 'fileStorage.action.didRenameFile', - oldName, - newName, - }), -); +export const fileStorageDidRenameFile = createAction((id: string) => ({ + type: 'fileStorage.action.didRenameFile', + id, +})); /** * Indicates that fileStorageRenameFile(oldName, newName) failed. - * @param oldName The current file name. - * @param newName The requested new file name. + * @param id The file handle UUID. * @param error The error. */ export const fileStorageDidFailToRenameFile = createAction( - (oldName: string, newName: string, error: Error) => ({ + (id: string, error: Error) => ({ type: 'fileStorage.action.didFailToRenameFile', - oldName, - newName, + id, error, }), ); /** * Request to export (download) a file. - * @param fileName The name of the file. + * @param id The file handle UUID. */ -export const fileStorageExportFile = createAction((fileName: string) => ({ +export const fileStorageExportFile = createAction((id: string) => ({ type: 'fileStorage.action.exportFile', - fileName, + id, })); /** * Indicates that fileStorageExportFile(fileName) succeeded. - * @param fileName The name of the file. + * @param id The file handle UUID. */ -export const fileStorageDidExportFile = createAction((fileName: string) => ({ +export const fileStorageDidExportFile = createAction((id: string) => ({ type: 'fileStorage.action.didExportFile', - fileName, + id, })); /** * Indicates that fileStorageExportFile(fileName) failed. - * @param fileName The name of the file. + * @param id The file handle UUID. * @param error The error that was raised. */ export const fileStorageDidFailToExportFile = createAction( - (fileName: string, error: Error) => ({ + (id: string, error: Error) => ({ type: 'fileStorage.action.didFailToExportFile', - fileName, + id, error, }), ); diff --git a/src/fileStorage/reducers.test.ts b/src/fileStorage/reducers.test.ts index 402d3b88..99cd12a2 100644 --- a/src/fileStorage/reducers.test.ts +++ b/src/fileStorage/reducers.test.ts @@ -3,6 +3,7 @@ import { AnyAction } from 'redux'; import { + fileStorageDidAddItem, fileStorageDidChangeItem, fileStorageDidInitialize, fileStorageDidRemoveItem, @@ -38,15 +39,15 @@ test('fileNames', () => { ).fileNames, ).toEqual([testFileName]); - // if item is not in set, add it + // adding appends an item expect( reducers( { fileNames: [] as ReadonlyArray } as State, - fileStorageDidChangeItem(testFileName), + fileStorageDidAddItem(testFileName), ).fileNames, ).toEqual([testFileName]); - // if item is already in set, there should not be duplicates + // changing does nothing expect( reducers( { fileNames: [testFileName] as ReadonlyArray } as State, @@ -54,7 +55,7 @@ test('fileNames', () => { ).fileNames, ).toEqual([testFileName]); - // if item is in set, it should be removed + // removing deletes an item expect( reducers( { fileNames: [testFileName] as ReadonlyArray } as State, diff --git a/src/fileStorage/reducers.ts b/src/fileStorage/reducers.ts index ff1a73b4..b6b30172 100644 --- a/src/fileStorage/reducers.ts +++ b/src/fileStorage/reducers.ts @@ -3,6 +3,7 @@ import { Reducer, combineReducers } from 'redux'; import { + fileStorageDidAddItem, fileStorageDidChangeItem, fileStorageDidInitialize, fileStorageDidRemoveItem, @@ -21,16 +22,16 @@ const fileNames: Reducer> = (state = [], action) => { return [...action.fileNames]; } - if (fileStorageDidChangeItem.matches(action)) { - if (state.includes(action.fileName)) { - return state; - } + if (fileStorageDidAddItem.matches(action)) { + return [...state, action.id]; + } - return [...state, action.fileName]; + if (fileStorageDidChangeItem.matches(action)) { + return state; } if (fileStorageDidRemoveItem.matches(action)) { - return [...state].filter((value) => value !== action.fileName); + return [...state].filter((value) => value !== action.id); } return state; diff --git a/src/fileStorage/sagas.test.ts b/src/fileStorage/sagas.test.ts index 64d6f615..990aa4ac 100644 --- a/src/fileStorage/sagas.test.ts +++ b/src/fileStorage/sagas.test.ts @@ -3,10 +3,14 @@ import * as browserFsAccess from 'browser-fs-access'; import 'fake-indexeddb/auto'; -import { AsyncSaga } from '../../test'; +import Dexie from 'dexie'; +import 'dexie-observable'; +import { AsyncSaga, uuid } from '../../test'; +import { createCountFunc } from '../utils/iter'; import { fileStorageArchiveAllFiles, fileStorageDeleteFile, + fileStorageDidAddItem, fileStorageDidArchiveAllFiles, fileStorageDidChangeItem, fileStorageDidDeleteFile, @@ -15,11 +19,13 @@ import { fileStorageDidFailToExportFile, fileStorageDidFailToReadFile, fileStorageDidInitialize, + fileStorageDidOpenFile, fileStorageDidReadFile, fileStorageDidRemoveItem, fileStorageDidRenameFile, fileStorageDidWriteFile, fileStorageExportFile, + fileStorageOpenFile, fileStorageReadFile, fileStorageRenameFile, fileStorageWriteFile, @@ -28,6 +34,12 @@ import fileStorage from './sagas'; jest.mock('browser-fs-access'); +beforeEach(() => { + // deterministic UUID generator for repeatable tests + const nextId = createCountFunc(); + Dexie.Observable.createUUID = () => uuid(nextId()); +}); + afterEach(async () => { jest.clearAllMocks(); @@ -43,21 +55,28 @@ afterEach(async () => { /** * helper function that writes test file to storage for later use in a test * @param saga The saga. - * @returns The test file name and test file contents. + * @returns The test file id and test file contents. */ async function setUpTestFile(saga: AsyncSaga): Promise<[string, string]> { - const testFileName = 'test.file'; + const testFilePath = 'test.file'; + const testFileId = uuid(0); const testFileContents = 'test file contents'; await expect(saga.take()).resolves.toEqual(fileStorageDidInitialize([])); - saga.put(fileStorageWriteFile(testFileName, testFileContents)); + saga.put(fileStorageOpenFile(testFilePath)); - await expect(saga.take()).resolves.toEqual(fileStorageDidWriteFile(testFileName)); + await expect(saga.take()).resolves.toEqual( + fileStorageDidOpenFile(testFilePath, testFileId), + ); + await expect(saga.take()).resolves.toEqual(fileStorageDidAddItem(testFileId)); - await expect(saga.take()).resolves.toEqual(fileStorageDidChangeItem(testFileName)); + saga.put(fileStorageWriteFile(testFileId, testFileContents)); - return [testFileName, testFileContents]; + await expect(saga.take()).resolves.toEqual(fileStorageDidWriteFile(testFileId)); + await expect(saga.take()).resolves.toEqual(fileStorageDidChangeItem(testFileId)); + + return [testFileId, testFileContents]; } it('should migrate old program from local storage during initialization', async () => { @@ -83,23 +102,31 @@ it('should read and write files', async () => { await expect(saga.take()).resolves.toEqual(fileStorageDidInitialize([])); - const testFileName = 'test.file'; + const testFile = 'test.file'; + const testId = uuid(0); const testFileContents = 'test file contents'; - // test writing a file - saga.put(fileStorageWriteFile(testFileName, testFileContents)); - - // writing file triggers response - await expect(saga.take()).resolves.toEqual(fileStorageDidWriteFile(testFileName)); - - // and as a side-effect, triggers item change as well - await expect(saga.take()).resolves.toEqual(fileStorageDidChangeItem(testFileName)); - - // test reading the same file back - saga.put(fileStorageReadFile(testFileName)); + saga.put(fileStorageOpenFile(testFile)); await expect(saga.take()).resolves.toEqual( - fileStorageDidReadFile(testFileName, testFileContents), + fileStorageDidOpenFile(testFile, testId), + ); + await expect(saga.take()).resolves.toEqual(fileStorageDidAddItem(testId)); + + // test writing a file + saga.put(fileStorageWriteFile(testId, testFileContents)); + + // writing file triggers response + await expect(saga.take()).resolves.toEqual(fileStorageDidWriteFile(testId)); + + // and as a side-effect, triggers item change as well + await expect(saga.take()).resolves.toEqual(fileStorageDidChangeItem(testId)); + + // test reading the same file back + saga.put(fileStorageReadFile(testId)); + + await expect(saga.take()).resolves.toEqual( + fileStorageDidReadFile(testId, testFileContents), ); await saga.end(); @@ -110,9 +137,9 @@ it('should dispatch fail action if file does not exist', async () => { await expect(saga.take()).resolves.toEqual(fileStorageDidInitialize([])); - const testFileName = 'test.file'; + const testFile = 'test.file'; - saga.put(fileStorageReadFile(testFileName)); + saga.put(fileStorageReadFile(testFile)); await expect(saga.take()).resolves.toEqual( fileStorageDidFailToReadFile('test.file', new Error('file does not exist')), @@ -124,13 +151,13 @@ it('should dispatch fail action if file does not exist', async () => { it('should delete files', async () => { const saga = new AsyncSaga(fileStorage); - const [testFileName] = await setUpTestFile(saga); + const [testFile] = await setUpTestFile(saga); - saga.put(fileStorageDeleteFile(testFileName)); + saga.put(fileStorageDeleteFile(testFile)); - await expect(saga.take()).resolves.toEqual(fileStorageDidDeleteFile(testFileName)); + await expect(saga.take()).resolves.toEqual(fileStorageDidDeleteFile(testFile)); - await expect(saga.take()).resolves.toEqual(fileStorageDidRemoveItem(testFileName)); + await expect(saga.take()).resolves.toEqual(fileStorageDidRemoveItem(testFile)); await saga.end(); }); @@ -141,19 +168,12 @@ describe('rename', () => { const saga = new AsyncSaga(fileStorage); - const [testFileName] = await setUpTestFile(saga); + const [testFile] = await setUpTestFile(saga); - saga.put(fileStorageRenameFile(testFileName, newName)); + saga.put(fileStorageRenameFile(testFile, newName)); - await expect(saga.take()).resolves.toEqual( - fileStorageDidRenameFile(testFileName, newName), - ); - - await expect(saga.take()).resolves.toEqual(fileStorageDidChangeItem(newName)); - - await expect(saga.take()).resolves.toEqual( - fileStorageDidRemoveItem(testFileName), - ); + await expect(saga.take()).resolves.toEqual(fileStorageDidRenameFile(testFile)); + await expect(saga.take()).resolves.toEqual(fileStorageDidChangeItem(testFile)); await saga.end(); }); @@ -161,19 +181,16 @@ describe('rename', () => { describe('export', () => { it('should fail if file does not exist', async () => { - const testFileName = 'test.file'; + const testFile = 'test.file'; const saga = new AsyncSaga(fileStorage); await expect(saga.take()).resolves.toEqual(fileStorageDidInitialize([])); - saga.put(fileStorageExportFile(testFileName)); + saga.put(fileStorageExportFile(testFile)); await expect(saga.take()).resolves.toEqual( - fileStorageDidFailToExportFile( - testFileName, - new Error('file does not exist'), - ), + fileStorageDidFailToExportFile(testFile, new Error('file does not exist')), ); await saga.end(); @@ -182,15 +199,13 @@ describe('export', () => { it('should export file', async () => { const saga = new AsyncSaga(fileStorage); - const [testFileName] = await setUpTestFile(saga); + const [testFile] = await setUpTestFile(saga); jest.spyOn(browserFsAccess, 'fileSave'); - saga.put(fileStorageExportFile(testFileName)); + saga.put(fileStorageExportFile(testFile)); - await expect(saga.take()).resolves.toEqual( - fileStorageDidExportFile(testFileName), - ); + await expect(saga.take()).resolves.toEqual(fileStorageDidExportFile(testFile)); expect(browserFsAccess.fileSave).toHaveBeenCalled(); await saga.end(); @@ -199,15 +214,15 @@ describe('export', () => { it('should catch error', async () => { const saga = new AsyncSaga(fileStorage); - const [testFileName] = await setUpTestFile(saga); + const [testFile] = await setUpTestFile(saga); const testError = new Error('test error'); jest.spyOn(browserFsAccess, 'fileSave').mockRejectedValue(testError); - saga.put(fileStorageExportFile(testFileName)); + saga.put(fileStorageExportFile(testFile)); await expect(saga.take()).resolves.toEqual( - fileStorageDidFailToExportFile(testFileName, testError), + fileStorageDidFailToExportFile(testFile, testError), ); await saga.end(); diff --git a/src/fileStorage/sagas.ts b/src/fileStorage/sagas.ts index 377d85c9..ca9a180e 100644 --- a/src/fileStorage/sagas.ts +++ b/src/fileStorage/sagas.ts @@ -15,9 +15,11 @@ import { eventChannel } from 'redux-saga'; import { call, fork, put, take, takeEvery } from 'typed-redux-saga/macro'; import { pythonFileExtension, pythonFileMimeType } from '../pybricksMicropython/lib'; import { ensureError, timestamp } from '../utils'; +import { sha256Digest } from '../utils/crypto'; import { fileStorageArchiveAllFiles, fileStorageDeleteFile, + fileStorageDidAddItem, fileStorageDidArchiveAllFiles, fileStorageDidChangeItem, fileStorageDidDeleteFile, @@ -26,15 +28,18 @@ import { fileStorageDidFailToDeleteFile, fileStorageDidFailToExportFile, fileStorageDidFailToInitialize, + fileStorageDidFailToOpenFile, fileStorageDidFailToReadFile, fileStorageDidFailToRenameFile, fileStorageDidFailToWriteFile, fileStorageDidInitialize, + fileStorageDidOpenFile, fileStorageDidReadFile, fileStorageDidRemoveItem, fileStorageDidRenameFile, fileStorageDidWriteFile, fileStorageExportFile, + fileStorageOpenFile, fileStorageReadFile, fileStorageRenameFile, fileStorageWriteFile, @@ -96,9 +101,11 @@ function isFileMetaDataDeleteChange(change: IDeleteChange): change is Omit< /** Database metadata table data type. */ type FileMetadata = { /** A globally unique identifier that serves a a file handle. */ - uuid?: string; + 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. */ @@ -134,24 +141,64 @@ function* handleFileStorageDidChange(changes: IDatabaseChange[]): Generator { for (const change of changes) { if (isCreateChange(change)) { if (isFileMetadataCreateChange(change)) { - yield* put(fileStorageDidChangeItem(change.obj.path)); + yield* put(fileStorageDidAddItem(change.obj.uuid)); } } else if (isUpdateChange(change)) { if (isFileMetadataUpdateChange(change)) { - if (change.oldObj.path !== change.obj.path) { - // TODO: need to introduce a DidCreate action - yield* put(fileStorageDidChangeItem(change.obj.path)); - yield* put(fileStorageDidRemoveItem(change.oldObj.path)); - } + yield* put(fileStorageDidChangeItem(change.obj.uuid)); } } else if (isDeleteChange(change)) { if (isFileMetaDataDeleteChange(change)) { - yield* put(fileStorageDidRemoveItem(change.oldObj.path)); + yield* put(fileStorageDidRemoveItem(change.oldObj.uuid)); } } } } +/** + * Handles requests to open a file. + * @param db The database instance. + * @param action The requested action. + */ +function* handleOpenFile( + db: FileStorageDb, + action: ReturnType, +): Generator { + try { + // NB: can't await non-db functions inside of transaction, so we have + // to do this before even if it is not used + const contents = ''; + const sha256 = yield* call(() => sha256Digest(contents)); + + const uuid = yield* call(() => + db.transaction('rw', db.metadata, db._contents, async () => { + const metadata = await db.metadata + .where('path') + .equals(action.path) + .first(); + + // if the file exists, return the existing uuid + if (metadata) { + return metadata.uuid; + } + + // otherwise create a new empty file + const key = await db.metadata.add((>{ + path: action.path, + sha256, + }) as FileMetadata); + + await db._contents.put({ path: action.path, contents }); + + return key; + }), + ); + yield* put(fileStorageDidOpenFile(action.path, uuid)); + } catch (err) { + yield* put(fileStorageDidFailToOpenFile(action.path, ensureError(err))); + } +} + /** * Handles requests to read a file. * @param db The database instance. @@ -164,13 +211,13 @@ function* handleReadFile( try { const file = yield* call(() => db.transaction('r', db.metadata, db._contents, async () => { - const metadata = await db.metadata.get(action.fileName); + const metadata = await db.metadata.get(action.id); if (!metadata) { return undefined; } - return db._contents.get(metadata.path); + return await db._contents.get(metadata.path); }), ); @@ -178,9 +225,9 @@ function* handleReadFile( throw new Error('file does not exist'); } - yield* put(fileStorageDidReadFile(action.fileName, file.contents)); + yield* put(fileStorageDidReadFile(action.id, file.contents)); } catch (err) { - yield* put(fileStorageDidFailToReadFile(action.fileName, ensureError(err))); + yield* put(fileStorageDidFailToReadFile(action.id, ensureError(err))); } } @@ -194,21 +241,26 @@ function* handleWriteFile( action: ReturnType, ) { try { + const sha256 = yield* call(() => sha256Digest(action.contents)); + yield* call(() => db.transaction('rw', db.metadata, db._contents, async () => { - await db.metadata.put({ - uuid: action.fileName, - path: action.fileName, - }); + const metadata = await db.metadata.get(action.id); + + if (!metadata) { + throw new Error(`file handle '${action.id}' does not exist`); + } + + await db.metadata.put({ ...metadata, sha256 }); await db._contents.put({ - path: action.fileName, - contents: action.fileContents, + path: metadata.path, + contents: action.contents, }); }), ); - yield* put(fileStorageDidWriteFile(action.fileName)); + yield* put(fileStorageDidWriteFile(action.id)); } catch (err) { - yield* put(fileStorageDidFailToWriteFile(action.fileName, ensureError(err))); + yield* put(fileStorageDidFailToWriteFile(action.id, ensureError(err))); } } @@ -218,22 +270,19 @@ function* handleExportFile( ): Generator { const file = yield* call(() => db.transaction('r', db.metadata, db._contents, async () => { - const metadata = await db.metadata.get(action.fileName); + const metadata = await db.metadata.get(action.id); if (!metadata) { return undefined; } - return db._contents.get(metadata.path); + return await db._contents.get(metadata.path); }), ); if (!file) { yield* put( - fileStorageDidFailToExportFile( - action.fileName, - new Error('file does not exist'), - ), + fileStorageDidFailToExportFile(action.id, new Error('file does not exist')), ); return; } @@ -244,7 +293,7 @@ function* handleExportFile( yield* call(() => fileSave(blob, { id: 'pybricksCodeFileStorageExport', - fileName: action.fileName, + fileName: file.path, extensions: [pythonFileExtension], mimeTypes: [pythonFileMimeType], // TODO: translate description @@ -252,9 +301,9 @@ function* handleExportFile( }), ); - yield* put(fileStorageDidExportFile(action.fileName)); + yield* put(fileStorageDidExportFile(action.id)); } catch (err) { - yield* put(fileStorageDidFailToExportFile(action.fileName, ensureError(err))); + yield* put(fileStorageDidFailToExportFile(action.id, ensureError(err))); } } @@ -270,21 +319,19 @@ function* handleDeleteFile( try { yield* call(() => db.transaction('rw', db.metadata, db._contents, async () => { - const metadata = await db.metadata.get(action.fileName); + const metadata = await db.metadata.get(action.id); if (!metadata) { - throw new Error( - `cannot rename: file '${action.fileName}' does not exist in db`, - ); + throw new Error(`file handle '${action.id}' does not exist`); } - await db.metadata.delete(action.fileName); + await db.metadata.delete(action.id); await db._contents.delete(metadata.path); }), ); - yield* put(fileStorageDidDeleteFile(action.fileName)); + yield* put(fileStorageDidDeleteFile(action.id)); } catch (err) { - yield* put(fileStorageDidFailToDeleteFile(action.fileName, ensureError(err))); + yield* put(fileStorageDidFailToDeleteFile(action.id, ensureError(err))); } } @@ -300,20 +347,18 @@ function* handleRenameFile( try { yield* call(() => db.transaction('rw', db.metadata, db._contents, async () => { - const metadata = await db.metadata.get(action.oldName); + const metadata = await db.metadata.get(action.id); if (!metadata) { - throw new Error( - `cannot rename: file '${action.oldName}' does not exist in db`, - ); + throw new Error(`file handle '${action.id}' does not exist`); } - const oldFile = await db._contents.get(metadata.path); + const oldName = metadata.path; + + const oldFile = await db._contents.get(oldName); if (!oldFile) { - throw new Error( - `cannot rename: file '${action.oldName}' does not exist in storage`, - ); + throw new Error(`file '${oldName}' does not exist in storage`); } const newFile = await db._contents.get(action.newName); @@ -324,22 +369,16 @@ function* handleRenameFile( ); } - await db._contents.delete(action.oldName); + await db._contents.delete(oldName); await db._contents.add({ ...oldFile, path: action.newName }); await db.metadata.put({ ...metadata, path: action.newName }); }), ); - yield* put(fileStorageDidRenameFile(action.oldName, action.newName)); + yield* put(fileStorageDidRenameFile(action.id)); } catch (err) { - yield* put( - fileStorageDidFailToRenameFile( - action.oldName, - action.newName, - ensureError(err), - ), - ); + yield* put(fileStorageDidFailToRenameFile(action.id, ensureError(err))); } } @@ -381,19 +420,27 @@ function* initialize(): Generator { // migrate from old storage - // NB: this is a one-shot event, so we don't need to unsubscribe - db.on('ready', async () => { - // Previous versions of pybricks code used local storage to save a single program. - const oldProgram = localStorage.getItem('program'); + const oldProgram = localStorage.getItem('program'); - if (oldProgram !== null) { + if (oldProgram !== null) { + // NB: Dexie only allows Promise and DexiePromise to be awaited + // inside of 'ready' callback so we have to do digest here + const sha256 = yield* call(() => sha256Digest(oldProgram)); + + // NB: this is a one-shot event, so we don't need to unsubscribe + db.on('ready', async () => { await db.transaction('rw', db.metadata, db._contents, async () => { - await db.metadata.add({ uuid: 'main.py', path: 'main.py' }); + await db.metadata.add((>{ + path: 'main.py', + sha256, + }) as FileMetadata); + await db._contents.add({ path: 'main.py', contents: oldProgram }); }); + localStorage.removeItem('program'); - } - }); + }); + } yield* call(() => db.open()); defer.push(() => db.close()); @@ -410,6 +457,7 @@ function* initialize(): Generator { // subscribe to events yield* takeEvery(changesChan, handleFileStorageDidChange); + yield* takeEvery(fileStorageOpenFile, handleOpenFile, db); yield* takeEvery(fileStorageReadFile, handleReadFile, db); yield* takeEvery(fileStorageWriteFile, handleWriteFile, db); yield* takeEvery(fileStorageDeleteFile, handleDeleteFile, db); diff --git a/src/notifications/sagas.ts b/src/notifications/sagas.ts index 50398c39..ba2dc150 100644 --- a/src/notifications/sagas.ts +++ b/src/notifications/sagas.ts @@ -445,7 +445,7 @@ function* showDeleteFileWarning(action: ReturnType) { const { didRemoveFile } = yield* race({ userActionEvent: take(ch), didRemoveFile: take( - fileStorageDidRemoveItem.when((a) => a.fileName === action.fileName), + fileStorageDidRemoveItem.when((a) => a.id === action.fileName), ), }); diff --git a/src/setupTests.ts b/src/setupTests.ts index 772cd438..15b39a06 100644 --- a/src/setupTests.ts +++ b/src/setupTests.ts @@ -6,6 +6,7 @@ // expect(element).toHaveTextContent(/react/i) // learn more: https://github.com/testing-library/jest-dom import '@testing-library/jest-dom/extend-expect'; +import crypto from 'crypto'; import { KeyCodes, Modifiers, @@ -71,3 +72,7 @@ function addWhichToKeyboardEvent(e: KeyboardEvent) { document.addEventListener('keydown', addWhichToKeyboardEvent); document.addEventListener('keypress', addWhichToKeyboardEvent); document.addEventListener('keyup', addWhichToKeyboardEvent); + +Object.defineProperty(global.self, 'crypto', { + value: crypto.webcrypto, +}); diff --git a/src/utils/crypto.test.ts b/src/utils/crypto.test.ts new file mode 100644 index 00000000..d6205df4 --- /dev/null +++ b/src/utils/crypto.test.ts @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { sha256Digest } from './crypto'; + +test('sha256Digest', async () => { + await expect(sha256Digest('test')).resolves.toBe( + '9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08', + ); +}); diff --git a/src/utils/crypto.ts b/src/utils/crypto.ts new file mode 100644 index 00000000..2a8566c2 --- /dev/null +++ b/src/utils/crypto.ts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +const encoder = new TextEncoder(); + +export async function sha256Digest(data: string): Promise { + const hash = await window.crypto.subtle.digest('SHA-256', encoder.encode(data)); + return Array.from(new Uint8Array(hash)) + .map((n) => n.toString(16).padStart(2, '0')) + .join(''); +} diff --git a/test/index.tsx b/test/index.tsx index 48bf9568..65e34034 100644 --- a/test/index.tsx +++ b/test/index.tsx @@ -149,3 +149,16 @@ export const testRender = ( return [result, dispatch]; }; + +/** + * Formats a number as a UUID string. + * + * The UUID will look like `XXXXXXXX-0000-0000-0000-00000000`. + * + * This allows for deterministic UUIDs for testing. + * + * @param id A unique identifier. + */ +export function uuid(id: number): string { + return `${id.toString().padStart(8, '0')}-0000-0000-0000-000000000000`; +} diff --git a/yarn.lock b/yarn.lock index ea6efd96..a6f1d45f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2115,7 +2115,7 @@ __metadata: "@testing-library/user-event": ^13.5.0 "@types/file-saver": ^2.0.5 "@types/jest": ^27.4.1 - "@types/node": ^12.20.47 + "@types/node": ^16.11.7 "@types/react": ^16.14.24 "@types/react-dom": ^16.9.14 "@types/react-redux": ^7.1.23 @@ -2132,7 +2132,7 @@ __metadata: canvas: ^2.9.0 copy-webpack-plugin: ^6.4.1 dexie: ^3.2.1 - dexie-observable: ^3.0.0-beta.11 + dexie-observable: ^4.0.0-beta.13 eslint: ^7.31.0 eslint-config-prettier: ^7.2.0 eslint-config-typed-fp: ^1.6.0 @@ -2814,10 +2814,10 @@ __metadata: languageName: node linkType: hard -"@types/node@npm:^12.20.47": - version: 12.20.47 - resolution: "@types/node@npm:12.20.47" - checksum: 97487af02fada4342e1bd47f9c9ebf601c12b85cbf17056ba73a315339ae996490403f2d4d1d799d8078cfe43a59e33370b4a5ae2d4fb79dd02359f4691934b4 +"@types/node@npm:^16.11.7": + version: 16.11.26 + resolution: "@types/node@npm:16.11.26" + checksum: 57757caaba3f0d95de82198cb276a1002c49b710108c932a1d02d7c91ff2fa57cfe2dd19fde60853b6dd90b0964b3cf35557981d2628e20aed6a909057aedfe6 languageName: node linkType: hard @@ -6293,12 +6293,12 @@ __metadata: languageName: node linkType: hard -"dexie-observable@npm:^3.0.0-beta.11": - version: 3.0.0-beta.11 - resolution: "dexie-observable@npm:3.0.0-beta.11" +"dexie-observable@npm:^4.0.0-beta.13": + version: 4.0.0-beta.13 + resolution: "dexie-observable@npm:4.0.0-beta.13" peerDependencies: - dexie: ^3.0.2 - checksum: af154708ca5a47d3c35a78fdf5b1a0935d1f8a394dbaaf09576344289188f02855f77776428bf68deb1d2489b3f69d9ca8690f76d06b50826ca471fefd5610a1 + dexie: ^3.0.2 || ^4.0.0-alpha.1 + checksum: 1f80f6308cebde83d63681babeccc110d170be2a6715ff7129695c949e808fec0ef8bee851ffa27c59f0888baf79fc82fd9ae74e02e1b2d2b8a6bcd247703084 languageName: node linkType: hard