From 05731e50fc1d064d914acc7e4f8f0fbdbf809e41 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Mon, 4 Apr 2022 17:48:36 -0500 Subject: [PATCH] explorer: split archive saga from fileStorage This splits the fileStorage archive into two sagas to separate concerns. fileStorage now just gives a dump of the database and explorer deals with the user interaction and zipping. --- src/explorer/Explorer.test.tsx | 5 +- src/explorer/Explorer.tsx | 4 +- src/explorer/actions.ts | 23 +++++++++ src/explorer/sagas.test.ts | 70 ++++++++++++++++++++++++++ src/explorer/sagas.ts | 52 ++++++++++++++++++- src/fileStorage/actions.ts | 26 ++++++---- src/fileStorage/sagas.test.ts | 35 +++++++------ src/fileStorage/sagas.ts | 39 ++++++-------- src/notifications/i18n.ts | 2 +- src/notifications/sagas.test.ts | 6 +-- src/notifications/sagas.ts | 8 +-- src/notifications/translations/en.json | 6 +-- 12 files changed, 209 insertions(+), 67 deletions(-) diff --git a/src/explorer/Explorer.test.tsx b/src/explorer/Explorer.test.tsx index 378b1230..b96e88a7 100644 --- a/src/explorer/Explorer.test.tsx +++ b/src/explorer/Explorer.test.tsx @@ -6,9 +6,10 @@ import { cleanup } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { testRender, uuid } from '../../test'; -import { FileMetadata, fileStorageArchiveAllFiles } from '../fileStorage/actions'; +import { FileMetadata } from '../fileStorage/actions'; import Explorer from './Explorer'; import { + explorerArchiveAllFiles, explorerDeleteFile, explorerExportFile, explorerImportFiles, @@ -37,7 +38,7 @@ describe('archive button', () => { expect(button).toBeEnabled(); userEvent.click(button); - expect(dispatch).toHaveBeenCalledWith(fileStorageArchiveAllFiles()); + expect(dispatch).toHaveBeenCalledWith(explorerArchiveAllFiles()); }); it('should be disabled if there are no files', () => { diff --git a/src/explorer/Explorer.tsx b/src/explorer/Explorer.tsx index 930b5394..29379582 100644 --- a/src/explorer/Explorer.tsx +++ b/src/explorer/Explorer.tsx @@ -23,12 +23,12 @@ import { useTreeEnvironment, } from 'react-complex-tree'; import { useDispatch } from 'react-redux'; -import { fileStorageArchiveAllFiles } from '../fileStorage/actions'; import { useSelector } from '../reducers'; import { isMacOS } from '../utils/os'; import { preventBrowserNativeContextMenu } from '../utils/react'; import { TreeItemContext, TreeItemData, renderers } from '../utils/tree-renderer'; import { + explorerArchiveAllFiles, explorerDeleteFile, explorerExportFile, explorerImportFiles, @@ -141,7 +141,7 @@ const Header: React.VoidFunctionComponent = ({ i18n }) => { icon="archive" tooltip={i18n.translate(I18nId.HeaderExportAllTooltip)} disabled={files.length === 0} - onClick={() => dispatch(fileStorageArchiveAllFiles())} + onClick={() => dispatch(explorerArchiveAllFiles())} /> ({ + type: 'explorer.action.archiveAllFiles', +})); + +/** + * Indicates that {@link explorerArchiveAllFiles} succeeded. + */ +export const explorerDidArchiveAllFiles = createAction(() => ({ + type: 'explorer.action.didArchiveAllFiles', +})); + +/** + * Indicates that {@link explorerArchiveAllFiles} failed. + * @param error The error that was raised. + */ +export const explorerDidFailToArchiveAllFiles = createAction((error: Error) => ({ + type: 'explorer.action.didFailToArchiveAllFiles', + error, +})); + /** * Action that requests to import (upload) files into the app. */ diff --git a/src/explorer/sagas.test.ts b/src/explorer/sagas.test.ts index 3b95328f..b5bc2b57 100644 --- a/src/explorer/sagas.test.ts +++ b/src/explorer/sagas.test.ts @@ -6,11 +6,14 @@ import { FileWithHandle } from 'browser-fs-access'; import { mock } from 'jest-mock-extended'; import { AsyncSaga } from '../../test'; import { + fileStorageDidDumpAllFiles, + fileStorageDidFailToDumpAllFiles, fileStorageDidFailToReadFile, fileStorageDidFailToRenameFile, fileStorageDidReadFile, fileStorageDidRenameFile, fileStorageDidWriteFile, + fileStorageDumpAllFiles, fileStorageReadFile, fileStorageRenameFile, fileStorageWriteFile, @@ -18,9 +21,12 @@ import { import { pythonFileExtension } from '../pybricksMicropython/lib'; import { Hub, + explorerArchiveAllFiles, explorerCreateNewFile, + explorerDidArchiveAllFiles, explorerDidCreateNewFile, explorerDidExportFile, + explorerDidFailToArchiveAllFiles, explorerDidFailToExportFile, explorerDidFailToImportFiles, explorerDidFailToRenameFile, @@ -37,6 +43,70 @@ import { } from './renameFileDialog/actions'; import explorer from './sagas'; +jest.mock('browser-fs-access'); + +describe('handleExplorerArchiveAllFiles', () => { + let saga: AsyncSaga; + + beforeEach(async () => { + saga = new AsyncSaga(explorer); + }); + + describe('should call into fileStorage', () => { + beforeEach(async () => { + saga.put(explorerArchiveAllFiles()); + + await expect(saga.take()).resolves.toEqual(fileStorageDumpAllFiles()); + }); + + it('should propagate error when fileStorage fails', async () => { + const testError = new Error('test error'); + + saga.put(fileStorageDidFailToDumpAllFiles(testError)); + + await expect(saga.take()).resolves.toEqual( + explorerDidFailToArchiveAllFiles(testError), + ); + }); + + describe('should continue when fileStorage succeeds', () => { + beforeEach(async () => { + saga.put( + fileStorageDidDumpAllFiles([ + { path: 'test.file', contents: 'test file contents' }, + ]), + ); + }); + + it('should catch error', async () => { + const testError = new Error('test error'); + + jest.spyOn(browserFsAccess, 'fileSave').mockImplementation(() => { + throw testError; + }); + + await expect(saga.take()).resolves.toEqual( + explorerDidFailToArchiveAllFiles(testError), + ); + }); + + it('should archive file', async () => { + jest.spyOn(browserFsAccess, 'fileSave'); + + await expect(saga.take()).resolves.toEqual( + explorerDidArchiveAllFiles(), + ); + + expect(browserFsAccess.fileSave).toHaveBeenCalled(); + }); + }); + }); + + afterEach(async () => { + await saga.end(); + }); +}); + describe('handleExplorerImportFiles', () => { it('should write file to storage', async () => { const testFileName = 'test.py'; diff --git a/src/explorer/sagas.ts b/src/explorer/sagas.ts index 9d6e8a96..5bcac7e9 100644 --- a/src/explorer/sagas.ts +++ b/src/explorer/sagas.ts @@ -2,6 +2,7 @@ // Copyright (c) 2022 The Pybricks Authors import { fileOpen, fileSave } from 'browser-fs-access'; +import JSZip from 'jszip'; import { call, put, @@ -13,12 +14,15 @@ import { } from 'typed-redux-saga/macro'; import { getPybricksMicroPythonFileTemplate } from '../editor/pybricksMicroPython'; import { + fileStorageDidDumpAllFiles, + fileStorageDidFailToDumpAllFiles, fileStorageDidFailToReadFile, fileStorageDidFailToRenameFile, fileStorageDidFailToWriteFile, fileStorageDidReadFile, fileStorageDidRenameFile, fileStorageDidWriteFile, + fileStorageDumpAllFiles, fileStorageReadFile, fileStorageRenameFile, fileStorageWriteFile, @@ -31,11 +35,14 @@ import { validateFileName, } from '../pybricksMicropython/lib'; import { RootState } from '../reducers'; -import { defined, ensureError } from '../utils'; +import { defined, ensureError, timestamp } from '../utils'; import { + explorerArchiveAllFiles, explorerCreateNewFile, + explorerDidArchiveAllFiles, explorerDidCreateNewFile, explorerDidExportFile, + explorerDidFailToArchiveAllFiles, explorerDidFailToCreateNewFile, explorerDidFailToExportFile, explorerDidFailToImportFiles, @@ -52,6 +59,48 @@ import { renameFileDialogShow, } from './renameFileDialog/actions'; +function* handleExplorerArchiveAllFiles(): Generator { + try { + yield* put(fileStorageDumpAllFiles()); + + const { didDump, didFailToDump } = yield* race({ + didDump: take(fileStorageDidDumpAllFiles), + didFailToDump: take(fileStorageDidFailToDumpAllFiles), + }); + + if (didFailToDump) { + throw didFailToDump.error; + } + + defined(didDump); + + const zip = new JSZip(); + + for (const f of didDump.files) { + yield* call(() => zip.file(f.path, f.contents)); + } + + const zipData = yield* call(() => zip.generateAsync({ type: 'blob' })); + + const fileName = `pybricks-backup-${timestamp()}.zip`; + + yield* call(() => + fileSave(zipData, { + id: 'pybricksCodeFileStorageArchive', + fileName, + extensions: ['.zip'], + mimeTypes: ['application/zip'], + // TODO: translate description + description: 'Zip Files', + }), + ); + + yield* put(explorerDidArchiveAllFiles()); + } catch (err) { + yield* put(explorerDidFailToArchiveAllFiles(ensureError(err))); + } +} + function* handleExplorerImportFiles(): Generator { try { const selectedFiles = yield* call(() => @@ -221,6 +270,7 @@ function* handleExplorerExportFile( } export default function* (): Generator { + yield* takeEvery(explorerArchiveAllFiles, handleExplorerArchiveAllFiles); yield* takeEvery(explorerImportFiles, handleExplorerImportFiles); yield* takeEvery(explorerCreateNewFile, handleExplorerCreateNewFile); // takeLatest should ensure that if we trigger a new rename before the diff --git a/src/fileStorage/actions.ts b/src/fileStorage/actions.ts index cc994ead..7ad216bd 100644 --- a/src/fileStorage/actions.ts +++ b/src/fileStorage/actions.ts @@ -326,24 +326,28 @@ export const fileStorageDidFailToRenameFile = createAction( ); /** - * Request to archive (download) all files in the store. + * Requests file storage to dump all file paths and contents currently in storage. */ -export const fileStorageArchiveAllFiles = createAction(() => ({ - type: 'fileStorage.action.archiveAllFiles', +export const fileStorageDumpAllFiles = createAction(() => ({ + type: 'fileStorage.action.dumpAllFiles', })); /** - * Indicates that fileStorageArchiveAllFiles() succeeded. + * Indicates that {@link fileStorageDumpAllFiles} succeeded. + * @param files: An array of all file paths and contents. */ -export const fileStorageDidArchiveAllFiles = createAction(() => ({ - type: 'fileStorage.action.didArchiveAllFiles', -})); +export const fileStorageDidDumpAllFiles = createAction( + (files: ReadonlyArray>) => ({ + type: 'fileStorage.action.didDumpAllFiles', + files, + }), +); /** - * Indicates that fileStorageArchiveAllFiles() failed. - * @param error The error that was raised. + * Indicates that {@link fileStorageDumpAllFiles} succeeded. + * @param error The error. */ -export const fileStorageDidFailToArchiveAllFiles = createAction((error: Error) => ({ - type: 'fileStorage.action.didFailToArchiveAllFiles', +export const fileStorageDidFailToDumpAllFiles = createAction((error: Error) => ({ + type: 'fileStorage.action.didFailToDumpAllFiles', error, })); diff --git a/src/fileStorage/sagas.test.ts b/src/fileStorage/sagas.test.ts index a70b3c2d..767742c0 100644 --- a/src/fileStorage/sagas.test.ts +++ b/src/fileStorage/sagas.test.ts @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2022 The Pybricks Authors -import * as browserFsAccess from 'browser-fs-access'; import 'fake-indexeddb/auto'; import Dexie from 'dexie'; import 'dexie-observable'; @@ -11,16 +10,15 @@ import { FD, FileMetadata, FileOpenMode, - fileStorageArchiveAllFiles, fileStorageClose, fileStorageDeleteFile, fileStorageDidAddItem, - fileStorageDidArchiveAllFiles, fileStorageDidChangeItem, fileStorageDidClose, fileStorageDidDeleteFile, - fileStorageDidFailToArchiveAllFiles, + fileStorageDidDumpAllFiles, fileStorageDidFailToDeleteFile, + fileStorageDidFailToDumpAllFiles, fileStorageDidFailToInitialize, fileStorageDidFailToOpen, fileStorageDidFailToRead, @@ -36,6 +34,7 @@ import { fileStorageDidRenameFile, fileStorageDidWrite, fileStorageDidWriteFile, + fileStorageDumpAllFiles, fileStorageOpen, fileStorageRead, fileStorageReadFile, @@ -45,8 +44,6 @@ import { } from './actions'; import fileStorage from './sagas'; -jest.mock('browser-fs-access'); - /** SHA256 hash of '' */ const emptyFileSha256 = 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'; @@ -700,33 +697,39 @@ describe('renameFile', () => { }); }); -describe('archive', () => { +describe('dump all files', () => { let saga: AsyncSaga; + let testFile: FileMetadata; + let testFileContents: string; beforeEach(async () => { saga = new AsyncSaga(fileStorage); await expect(saga.take()).resolves.toEqual(fileStorageDidInitialize([])); - await setUpTestFile(saga); + [testFile, testFileContents] = await setUpTestFile(saga); }); it('should archive file', async () => { - jest.spyOn(browserFsAccess, 'fileSave'); + saga.put(fileStorageDumpAllFiles()); - saga.put(fileStorageArchiveAllFiles()); - - await expect(saga.take()).resolves.toEqual(fileStorageDidArchiveAllFiles()); - expect(browserFsAccess.fileSave).toHaveBeenCalled(); + await expect(saga.take()).resolves.toEqual( + fileStorageDidDumpAllFiles([ + { path: testFile.path, contents: testFileContents }, + ]), + ); }); it('should catch error', async () => { const testError = new Error('test error'); - jest.spyOn(browserFsAccess, 'fileSave').mockRejectedValue(testError); - saga.put(fileStorageArchiveAllFiles()); + jest.spyOn(Dexie.prototype, 'transaction').mockImplementation(() => { + throw testError; + }); + + saga.put(fileStorageDumpAllFiles()); await expect(saga.take()).resolves.toEqual( - fileStorageDidFailToArchiveAllFiles(testError), + fileStorageDidFailToDumpAllFiles(testError), ); }); diff --git a/src/fileStorage/sagas.ts b/src/fileStorage/sagas.ts index 9cd21aaa..14de30b1 100644 --- a/src/fileStorage/sagas.ts +++ b/src/fileStorage/sagas.ts @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2022 The Pybricks Authors -import { fileSave } from 'browser-fs-access'; import Dexie, { Table } from 'dexie'; import { ICreateChange, @@ -10,10 +9,9 @@ import { IUpdateChange, } from 'dexie-observable/api'; import 'dexie-observable'; -import JSZip from 'jszip'; import { eventChannel } from 'redux-saga'; import { call, fork, put, race, take, takeEvery } from 'typed-redux-saga/macro'; -import { defined, ensureError, timestamp } from '../utils'; +import { defined, ensureError } from '../utils'; import { sha256Digest } from '../utils/crypto'; import { createCountFunc } from '../utils/iter'; import { @@ -21,16 +19,15 @@ import { FileMetadata, FileOpenMode, UUID, - fileStorageArchiveAllFiles, fileStorageClose, fileStorageDeleteFile, fileStorageDidAddItem, - fileStorageDidArchiveAllFiles, fileStorageDidChangeItem, fileStorageDidClose, fileStorageDidDeleteFile, - fileStorageDidFailToArchiveAllFiles, + fileStorageDidDumpAllFiles, fileStorageDidFailToDeleteFile, + fileStorageDidFailToDumpAllFiles, fileStorageDidFailToInitialize, fileStorageDidFailToOpen, fileStorageDidFailToRead, @@ -46,6 +43,7 @@ import { fileStorageDidRenameFile, fileStorageDidWrite, fileStorageDidWriteFile, + fileStorageDumpAllFiles, fileStorageOpen, fileStorageRead, fileStorageReadFile, @@ -615,30 +613,23 @@ function* handleRenameFile( } } -function* handleArchiveAllFiles(db: FileStorageDb): Generator { +function* handleDumpAllFiles(db: FileStorageDb): Generator { try { - const zip = new JSZip(); + const dump = new Array<{ path: string; contents: string }>(); - yield* call(() => db._contents.each((f) => zip.file(f.path, f.contents))); - - const zipData = yield* call(() => zip.generateAsync({ type: 'blob' })); - - const fileName = `pybricks-backup-${timestamp()}.zip`; + // REVISIT: consider using dexie-export-import addon if we want to do + // a full backup instead of just the file contents + // https://www.npmjs.com/package/dexie-export-import yield* call(() => - fileSave(zipData, { - id: 'pybricksCodeFileStorageArchive', - fileName, - extensions: ['.zip'], - mimeTypes: ['application/zip'], - // TODO: translate description - description: 'Zip Files', - }), + db.transaction('r', db._contents, () => + db._contents.each((f) => dump.push(f)), + ), ); - yield* put(fileStorageDidArchiveAllFiles()); + yield* put(fileStorageDidDumpAllFiles(dump)); } catch (err) { - yield* put(fileStorageDidFailToArchiveAllFiles(ensureError(err))); + yield* put(fileStorageDidFailToDumpAllFiles(ensureError(err))); } } @@ -701,7 +692,7 @@ function* initialize(): Generator { yield* takeEvery(fileStorageWriteFile, handleWriteFile); yield* takeEvery(fileStorageDeleteFile, handleDeleteFile, db); yield* takeEvery(fileStorageRenameFile, handleRenameFile, db); - yield* takeEvery(fileStorageArchiveAllFiles, handleArchiveAllFiles, db); + yield* takeEvery(fileStorageDumpAllFiles, handleDumpAllFiles, db); const files = yield* call(() => db.metadata.toArray()); diff --git a/src/notifications/i18n.ts b/src/notifications/i18n.ts index 0ed18b27..234a3004 100644 --- a/src/notifications/i18n.ts +++ b/src/notifications/i18n.ts @@ -18,8 +18,8 @@ export enum I18nId { ExplorerFailedToImportFiles = 'explorer.failedToImportFiles', ExplorerFailedToCreate = 'explorer.failedToCreate', ExplorerFailedToExport = 'explorer.failedToExport', + ExplorerFailedToArchive = 'explorer.failedToArchive', FileStorageFailedToInitialize = 'fileStorage.failedToInitialize', - FileStorageFailedToArchive = 'fileStorage.failedToArchive', FlashFirmwareTimedOut = 'flashFirmware.timedOut', FlashFirmwareBleError = 'flashFirmware.bleError', FlashFirmwareDisconnected = 'flashFirmware.disconnected', diff --git a/src/notifications/sagas.test.ts b/src/notifications/sagas.test.ts index 1e9783ef..90688d7e 100644 --- a/src/notifications/sagas.test.ts +++ b/src/notifications/sagas.test.ts @@ -18,13 +18,13 @@ import { } from '../ble/actions'; import { explorerDeleteFile, + explorerDidFailToArchiveAllFiles, explorerDidFailToCreateNewFile, explorerDidFailToExportFile, explorerDidFailToImportFiles, } from '../explorer/actions'; import { fileStorageDeleteFile, - fileStorageDidFailToArchiveAllFiles, fileStorageDidFailToInitialize, fileStorageDidRemoveItem, } from '../fileStorage/actions'; @@ -112,7 +112,7 @@ test.each([ appDidCheckForUpdate(false), bleDIServiceDidReceiveFirmwareRevision('3.0.0'), fileStorageDidFailToInitialize(new Error('test error')), - fileStorageDidFailToArchiveAllFiles(new Error('test error')), + explorerDidFailToArchiveAllFiles(new Error('test error')), explorerDidFailToImportFiles(new Error('test error')), explorerDidFailToCreateNewFile(new Error('test error')), explorerDidFailToExportFile('test.file', new Error('test error')), @@ -135,7 +135,7 @@ test.each([ serviceWorkerDidSucceed(), appDidCheckForUpdate(true), bleDIServiceDidReceiveFirmwareRevision(firmwareVersion), - fileStorageDidFailToArchiveAllFiles(new DOMException('test message', 'AbortError')), + explorerDidFailToArchiveAllFiles(new DOMException('test message', 'AbortError')), explorerDidFailToImportFiles(new DOMException('test message', 'AbortError')), explorerDidFailToExportFile( 'test.file', diff --git a/src/notifications/sagas.ts b/src/notifications/sagas.ts index bd999ae9..1db475e5 100644 --- a/src/notifications/sagas.ts +++ b/src/notifications/sagas.ts @@ -19,13 +19,13 @@ import { } from '../ble/actions'; import { explorerDeleteFile, + explorerDidFailToArchiveAllFiles, explorerDidFailToCreateNewFile, explorerDidFailToExportFile, explorerDidFailToImportFiles, } from '../explorer/actions'; import { fileStorageDeleteFile, - fileStorageDidFailToArchiveAllFiles, fileStorageDidFailToInitialize, fileStorageDidRemoveItem, } from '../fileStorage/actions'; @@ -388,14 +388,14 @@ function* showFileStorageFailToInitialize( } function* showFileStorageFailToArchive( - action: ReturnType, + action: ReturnType, ): Generator { if (action.error.name === 'AbortError') { // user clicked cancel button - not an error return; } - yield* showUnexpectedError(I18nId.FileStorageFailedToArchive, action.error); + yield* showUnexpectedError(I18nId.ExplorerFailedToArchive, action.error); } function* showDeleteFileWarning(action: ReturnType) { @@ -472,7 +472,7 @@ export default function* (): Generator { yield* takeEvery(appDidCheckForUpdate, showNoUpdateInfo); yield* takeEvery(bleDIServiceDidReceiveFirmwareRevision, checkVersion); yield* takeEvery(fileStorageDidFailToInitialize, showFileStorageFailToInitialize); - yield* takeEvery(fileStorageDidFailToArchiveAllFiles, showFileStorageFailToArchive); + yield* takeEvery(explorerDidFailToArchiveAllFiles, showFileStorageFailToArchive); yield* takeEvery(explorerDeleteFile, showDeleteFileWarning); yield* takeEvery(explorerDidFailToImportFiles, showExplorerFailToImportFiles); yield* takeEvery(explorerDidFailToCreateNewFile, showExplorerFailToCreateFile); diff --git a/src/notifications/translations/en.json b/src/notifications/translations/en.json index 046b7673..8b889e98 100644 --- a/src/notifications/translations/en.json +++ b/src/notifications/translations/en.json @@ -21,11 +21,11 @@ }, "failedToImportFiles": "Failed to import file(s).", "failedToCreate": "Failed to create file.", - "failedToExport": "Failed to export file." + "failedToExport": "Failed to export file.", + "failedToArchive": "Failed to archive files.'" }, "fileStorage": { - "failedToInitialize": "Failed to initial file storage. Changes will not be automatically saved.", - "failedToArchive": "Failed to archive files.'" + "failedToInitialize": "Failed to initial file storage. Changes will not be automatically saved." }, "flashFirmware": { "timedOut": "The hub took too long to respond. Restart the hub and try again.",