From f10fa632ca42141cf3132dfd0a900695d37b4864 Mon Sep 17 00:00:00 2001 From: ehhc Date: Tue, 15 May 2018 20:17:32 +0200 Subject: [PATCH] Move note with attachment to different storage Fix for #1788 --- browser/main/SideNav/StorageItem.js | 9 ++- .../main/lib/dataApi/attachmentManagement.js | 24 ++++++++ browser/main/lib/dataApi/copyImage.js | 38 ------------- browser/main/lib/dataApi/moveNote.js | 31 +++------- package.json | 2 + tests/dataApi/attachmentManagement.test.js | 57 +++++++++++++++++++ yarn.lock | 18 ++++++ 7 files changed, 115 insertions(+), 64 deletions(-) delete mode 100644 browser/main/lib/dataApi/copyImage.js diff --git a/browser/main/SideNav/StorageItem.js b/browser/main/SideNav/StorageItem.js index a4d74c30..93e9157f 100644 --- a/browser/main/SideNav/StorageItem.js +++ b/browser/main/SideNav/StorageItem.js @@ -14,6 +14,8 @@ import i18n from 'browser/lib/i18n' const { remote } = require('electron') const { Menu, dialog } = remote +const escapeStringRegexp = require('escape-string-regexp') +const path = require('path') class StorageItem extends React.Component { constructor (props) { @@ -201,7 +203,7 @@ class StorageItem extends React.Component { createdNoteData.forEach((newNote) => { dispatch({ type: 'MOVE_NOTE', - originNote: noteData.find((note) => note.content === newNote.content), + originNote: noteData.find((note) => note.content === newNote.oldContent), note: newNote }) }) @@ -223,7 +225,8 @@ class StorageItem extends React.Component { const { folderNoteMap, trashedSet } = data const SortableStorageItemChild = SortableElement(StorageItemChild) const folderList = storage.folders.map((folder, index) => { - const isActive = !!(location.pathname.match(new RegExp('\/storages\/' + storage.key + '\/folders\/' + folder.key))) + let folderRegex = new RegExp(escapeStringRegexp(path.sep) + 'storages' + escapeStringRegexp(path.sep) + storage.key + escapeStringRegexp(path.sep) + 'folders' + escapeStringRegexp(path.sep) + folder.key) + const isActive = !!(location.pathname.match(folderRegex)) const noteSet = folderNoteMap.get(storage.key + '-' + folder.key) let noteCount = 0 @@ -253,7 +256,7 @@ class StorageItem extends React.Component { ) }) - const isActive = location.pathname.match(new RegExp('\/storages\/' + storage.key + '$')) + const isActive = location.pathname.match(new RegExp(escapeStringRegexp(path.sep) + 'storages' + escapeStringRegexp(path.sep) + storage.key + '$')) return (
} an image path - */ -function copyImage (filePath, storageKey, rename = true) { - return new Promise((resolve, reject) => { - try { - const targetStorage = findStorage(storageKey) - - const inputImage = fs.createReadStream(filePath) - const imageExt = path.extname(filePath) - const imageName = rename ? Math.random().toString(36).slice(-16) : path.basename(filePath, imageExt) - const basename = `${imageName}${imageExt}` - const imageDir = path.join(targetStorage.path, 'images') - if (!fs.existsSync(imageDir)) fs.mkdirSync(imageDir) - const outputImage = fs.createWriteStream(path.join(imageDir, basename)) - outputImage.on('error', reject) - inputImage.on('error', reject) - inputImage.on('end', () => { - resolve(basename) - }) - inputImage.pipe(outputImage) - } catch (e) { - return reject(e) - } - }) -} - -module.exports = copyImage diff --git a/browser/main/lib/dataApi/moveNote.js b/browser/main/lib/dataApi/moveNote.js index fbfd375e..2d306cdf 100644 --- a/browser/main/lib/dataApi/moveNote.js +++ b/browser/main/lib/dataApi/moveNote.js @@ -6,6 +6,7 @@ const CSON = require('@rokt33r/season') const keygen = require('browser/lib/keygen') const sander = require('sander') const { findStorage } = require('browser/lib/findStorage') +const attachmentManagement = require('./attachmentManagement') function moveNote (storageKey, noteKey, newStorageKey, newFolderKey) { let oldStorage, newStorage @@ -63,36 +64,20 @@ function moveNote (storageKey, noteKey, newStorageKey, newFolderKey) { noteData.key = newNoteKey noteData.storage = newStorageKey noteData.updatedAt = new Date() + noteData.oldContent = noteData.content return noteData }) - .then(function moveImages (noteData) { - if (oldStorage.path === newStorage.path) return noteData - - const searchImagesRegex = /!\[.*\]\(:storage\/(.+)\)/gi - let match = searchImagesRegex.exec(noteData.content) - - const moveTasks = [] - while (match != null) { - const [, filename] = match - const oldPath = path.join(oldStorage.path, 'attachments', filename) - const newPath = path.join(newStorage.path, 'attachments', filename) - // TODO: ehhc: attachmentManagement - moveTasks.push( - sander.copyFile(oldPath).to(newPath) - .then(() => { - fs.unlinkSync(oldPath) - }) - ) - - // find next occurence - match = searchImagesRegex.exec(noteData.content) + .then(function moveAttachments (noteData) { + if (oldStorage.path === newStorage.path) { + return noteData } - return Promise.all(moveTasks).then(() => noteData) + noteData.content = attachmentManagement.moveAttachments(oldStorage.path, newStorage.path, noteKey, newNoteKey, noteData.content) + return noteData }) .then(function writeAndReturn (noteData) { - CSON.writeFileSync(path.join(newStorage.path, 'notes', noteData.key + '.cson'), _.omit(noteData, ['key', 'storage'])) + CSON.writeFileSync(path.join(newStorage.path, 'notes', noteData.key + '.cson'), _.omit(noteData, ['key', 'storage', 'oldContent'])) return noteData }) .then(function deleteOldNote (data) { diff --git a/package.json b/package.json index 82c90896..7ee171d5 100644 --- a/package.json +++ b/package.json @@ -57,9 +57,11 @@ "codemirror-mode-elixir": "^1.1.1", "electron-config": "^0.2.1", "electron-gh-releases": "^2.0.2", + "escape-string-regexp": "^1.0.5", "filenamify": "^2.0.0", "flowchart.js": "^1.6.5", "font-awesome": "^4.3.0", + "fs-extra": "^5.0.0", "i18n-2": "^0.7.2", "iconv-lite": "^0.4.19", "immutable": "^3.8.1", diff --git a/tests/dataApi/attachmentManagement.test.js b/tests/dataApi/attachmentManagement.test.js index 7efe4f21..9511f6d7 100644 --- a/tests/dataApi/attachmentManagement.test.js +++ b/tests/dataApi/attachmentManagement.test.js @@ -7,6 +7,7 @@ const findStorage = require('browser/lib/findStorage') jest.mock('unique-slug') const uniqueSlug = require('unique-slug') const mdurl = require('mdurl') +const fse = require('fs-extra') const systemUnderTest = require('browser/main/lib/dataApi/attachmentManagement') @@ -312,3 +313,59 @@ it('should test that deleteAttachmentsNotPresentInNote does not delete reference } expect(fsUnlinkCallArguments.includes(path.join(attachmentFolderPath, dummyFilesInFolder[0]))).toBe(false) }) + +it('should test that moveAttachments moves attachments only if the source folder existed', function () { + fse.existsSync = jest.fn(() => false) + fse.moveSync = jest.fn() + + const oldPath = 'oldPath' + const newPath = 'newPath' + const oldNoteKey = 'oldNoteKey' + const newNoteKey = 'newNoteKey' + const content = '' + + const expectedSource = path.join(oldPath, systemUnderTest.DESTINATION_FOLDER, oldNoteKey) + + systemUnderTest.moveAttachments(oldPath, newPath, oldNoteKey, newNoteKey, content) + expect(fse.existsSync).toHaveBeenCalledWith(expectedSource) + expect(fse.moveSync).not.toHaveBeenCalled() +}) + +it('should test that moveAttachments moves attachments to the right destination', function () { + fse.existsSync = jest.fn(() => true) + fse.moveSync = jest.fn() + + const oldPath = 'oldPath' + const newPath = 'newPath' + const oldNoteKey = 'oldNoteKey' + const newNoteKey = 'newNoteKey' + const content = '' + + const expectedSource = path.join(oldPath, systemUnderTest.DESTINATION_FOLDER, oldNoteKey) + const expectedDestination = path.join(newPath, systemUnderTest.DESTINATION_FOLDER, newNoteKey) + + systemUnderTest.moveAttachments(oldPath, newPath, oldNoteKey, newNoteKey, content) + expect(fse.existsSync).toHaveBeenCalledWith(expectedSource) + expect(fse.moveSync).toHaveBeenCalledWith(expectedSource, expectedDestination) +}) + +it('should test that moveAttachments returns a correct modified content version', function () { + fse.existsSync = jest.fn() + fse.moveSync = jest.fn() + + const oldPath = 'oldPath' + const newPath = 'newPath' + const oldNoteKey = 'oldNoteKey' + const newNoteKey = 'newNoteKey' + const testInput = + 'Test input' + + '![' + systemUnderTest.STORAGE_FOLDER_PLACEHOLDER + path.sep + oldNoteKey + path.sep + 'image.jpg](imageName}) \n' + + '[' + systemUnderTest.STORAGE_FOLDER_PLACEHOLDER + path.sep + oldNoteKey + path.sep + 'pdf.pdf](pdf})' + const expectedOutput = + 'Test input' + + '![' + systemUnderTest.STORAGE_FOLDER_PLACEHOLDER + path.sep + newNoteKey + path.sep + 'image.jpg](imageName}) \n' + + '[' + systemUnderTest.STORAGE_FOLDER_PLACEHOLDER + path.sep + newNoteKey + path.sep + 'pdf.pdf](pdf})' + + const actualContent = systemUnderTest.moveAttachments(oldPath, newPath, oldNoteKey, newNoteKey, testInput) + expect(actualContent).toBe(expectedOutput) +}) diff --git a/yarn.lock b/yarn.lock index a0e4d96f..fa748b66 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3637,6 +3637,14 @@ fs-extra@^1.0.0: jsonfile "^2.1.0" klaw "^1.0.0" +fs-extra@^5.0.0: + version "5.0.0" + resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-5.0.0.tgz#414d0110cdd06705734d055652c5411260c31abd" + dependencies: + graceful-fs "^4.1.2" + jsonfile "^4.0.0" + universalify "^0.1.0" + fs-plus@2.x: version "2.10.1" resolved "https://registry.yarnpkg.com/fs-plus/-/fs-plus-2.10.1.tgz#3204781d7840611e6364e7b6fb058c96327c5aa5" @@ -5309,6 +5317,12 @@ jsonfile@^2.0.0, jsonfile@^2.1.0: optionalDependencies: graceful-fs "^4.1.6" +jsonfile@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/jsonfile/-/jsonfile-4.0.0.tgz#8771aae0799b64076b76640fca058f9c10e33ecb" + optionalDependencies: + graceful-fs "^4.1.6" + jsonify@~0.0.0: version "0.0.0" resolved "https://registry.yarnpkg.com/jsonify/-/jsonify-0.0.0.tgz#2c74b6ee41d93ca51b7b5aaee8f503631d252a73" @@ -8734,6 +8748,10 @@ unique-temp-dir@^1.0.0: os-tmpdir "^1.0.1" uid2 "0.0.3" +universalify@^0.1.0: + version "0.1.1" + resolved "https://registry.yarnpkg.com/universalify/-/universalify-0.1.1.tgz#fa71badd4437af4c148841e3b3b165f9e9e590b7" + unpipe@~1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/unpipe/-/unpipe-1.0.0.tgz#b2bf4ee8514aae6165b4817829d21b2ef49904ec"