From 73ba8b8b13534174494a03e8243033206a32297b Mon Sep 17 00:00:00 2001 From: ehhc Date: Thu, 10 May 2018 21:36:58 +0200 Subject: [PATCH 1/6] Deleting a note should also delete the attachments -> fixes #1900 --- browser/main/lib/dataApi/attachmentManagement.js | 13 +++++++++++++ browser/main/lib/dataApi/deleteNote.js | 5 +++++ tests/dataApi/attachmentManagement.test.js | 14 ++++++++++++++ tests/dataApi/deleteNote-test.js | 10 ++++++++++ 4 files changed, 42 insertions(+) diff --git a/browser/main/lib/dataApi/attachmentManagement.js b/browser/main/lib/dataApi/attachmentManagement.js index c2e7f6d6..ace33464 100644 --- a/browser/main/lib/dataApi/attachmentManagement.js +++ b/browser/main/lib/dataApi/attachmentManagement.js @@ -4,6 +4,7 @@ const path = require('path') const findStorage = require('browser/lib/findStorage') const mdurl = require('mdurl') const escapeStringRegexp = require('escape-string-regexp') +const sander = require('sander') const STORAGE_FOLDER_PLACEHOLDER = ':storage' const DESTINATION_FOLDER = 'attachments' @@ -190,6 +191,17 @@ function removeStorageAndNoteReferences (input, noteKey) { return input.replace(new RegExp(mdurl.encode(path.sep), 'g'), path.sep).replace(new RegExp(STORAGE_FOLDER_PLACEHOLDER + escapeStringRegexp(path.sep) + noteKey, 'g'), DESTINATION_FOLDER) } +/** + * @description Deletes the attachment folder specified by the given storageKey and noteKey + * @param storageKey Key of the storage of the note to be deleted + * @param noteKey Key of the note to be deleted + */ +function deleteAttachmentFolder (storageKey, noteKey) { + const storagePath = findStorage.findStorage(storageKey) + const noteAttachmentPath = path.join(storagePath.path, DESTINATION_FOLDER, noteKey) + sander.rimraf(noteAttachmentPath) +} + module.exports = { copyAttachment, fixLocalURLS, @@ -199,6 +211,7 @@ module.exports = { getAttachmentsInContent, getAbsolutePathsOfAttachmentsInContent, removeStorageAndNoteReferences, + deleteAttachmentFolder, STORAGE_FOLDER_PLACEHOLDER, DESTINATION_FOLDER } diff --git a/browser/main/lib/dataApi/deleteNote.js b/browser/main/lib/dataApi/deleteNote.js index 49498a30..46ec2b55 100644 --- a/browser/main/lib/dataApi/deleteNote.js +++ b/browser/main/lib/dataApi/deleteNote.js @@ -1,6 +1,7 @@ const resolveStorageData = require('./resolveStorageData') const path = require('path') const sander = require('sander') +const attachmentManagement = require('./attachmentManagement') const { findStorage } = require('browser/lib/findStorage') function deleteNote (storageKey, noteKey) { @@ -25,6 +26,10 @@ function deleteNote (storageKey, noteKey) { storageKey } }) + .then(function deleteAttachments (storageInfo) { + attachmentManagement.deleteAttachmentFolder(storageInfo.storageKey, storageInfo.noteKey) + return storageInfo + }) } module.exports = deleteNote diff --git a/tests/dataApi/attachmentManagement.test.js b/tests/dataApi/attachmentManagement.test.js index d58a8eb8..148f6958 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 sander = require('sander') const systemUnderTest = require('browser/main/lib/dataApi/attachmentManagement') @@ -260,3 +261,16 @@ it('should remove the all ":storage" and noteKey references', function () { const actual = systemUnderTest.removeStorageAndNoteReferences(testInput, noteKey) expect(actual).toEqual(expectedOutput) }) + +it('should delete the correct attachment folder if a note is deleted', function () { + const dummyStorage = {path: 'dummyStoragePath'} + const storageKey = 'storageKey' + const noteKey = 'noteKey' + findStorage.findStorage = jest.fn(() => dummyStorage) + sander.rimraf = jest.fn() + + const expectedPathToBeDeleted = path.join(dummyStorage.path, systemUnderTest.DESTINATION_FOLDER, noteKey) + systemUnderTest.deleteAttachmentFolder(storageKey, noteKey) + expect(findStorage.findStorage).toHaveBeenCalledWith(storageKey) + expect(sander.rimraf).toHaveBeenCalledWith(expectedPathToBeDeleted) +}) diff --git a/tests/dataApi/deleteNote-test.js b/tests/dataApi/deleteNote-test.js index 611022de..ed37854d 100644 --- a/tests/dataApi/deleteNote-test.js +++ b/tests/dataApi/deleteNote-test.js @@ -14,6 +14,8 @@ const sander = require('sander') const os = require('os') const CSON = require('@rokt33r/season') const faker = require('faker') +const fs = require('fs') +const attachmentManagement = require('browser/main/lib/dataApi/attachmentManagement') const storagePath = path.join(os.tmpdir(), 'test/delete-note') @@ -42,6 +44,10 @@ test.serial('Delete a note', (t) => { return Promise.resolve() .then(function doTest () { return createNote(storageKey, input1) + .then(function createAttachmentFolder (data) { + fs.mkdirSync(path.join(storagePath, attachmentManagement.DESTINATION_FOLDER, data.noteKey)) + return data + }) .then(function (data) { return deleteNote(storageKey, data.key) }) @@ -54,6 +60,10 @@ test.serial('Delete a note', (t) => { t.is(err.code, 'ENOENT') } }) + .then(function assertAttachmentFolderDeleted (data) { + const attachmentFolderPath = path.join(storagePath, attachmentManagement.DESTINATION_FOLDER, data.noteKey) + t.assert(fs.existsSync(attachmentFolderPath) === false, 'Attachment folder was not deleted') + }) }) test.after(function after () { From ff59af6b51e6f79d885d1392b8cfead4fedc48a5 Mon Sep 17 00:00:00 2001 From: ehhc Date: Thu, 10 May 2018 21:42:23 +0200 Subject: [PATCH 2/6] Fix for the broken test --- tests/dataApi/deleteNote-test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/dataApi/deleteNote-test.js b/tests/dataApi/deleteNote-test.js index ed37854d..c9a33145 100644 --- a/tests/dataApi/deleteNote-test.js +++ b/tests/dataApi/deleteNote-test.js @@ -45,7 +45,7 @@ test.serial('Delete a note', (t) => { .then(function doTest () { return createNote(storageKey, input1) .then(function createAttachmentFolder (data) { - fs.mkdirSync(path.join(storagePath, attachmentManagement.DESTINATION_FOLDER, data.noteKey)) + fs.mkdirSync(path.join(storagePath.path, attachmentManagement.DESTINATION_FOLDER, data.noteKey)) return data }) .then(function (data) { @@ -54,14 +54,14 @@ test.serial('Delete a note', (t) => { }) .then(function assert (data) { try { - CSON.readFileSync(path.join(storagePath, 'notes', data.noteKey + '.cson')) + CSON.readFileSync(path.join(storagePath.path, 'notes', data.noteKey + '.cson')) t.fail('note cson must be deleted.') } catch (err) { t.is(err.code, 'ENOENT') } }) .then(function assertAttachmentFolderDeleted (data) { - const attachmentFolderPath = path.join(storagePath, attachmentManagement.DESTINATION_FOLDER, data.noteKey) + const attachmentFolderPath = path.join(storagePath.path, attachmentManagement.DESTINATION_FOLDER, data.noteKey) t.assert(fs.existsSync(attachmentFolderPath) === false, 'Attachment folder was not deleted') }) }) From 03fd1e29e37a84dacaea3bce8cdad7cda55599a4 Mon Sep 17 00:00:00 2001 From: ehhc Date: Thu, 10 May 2018 22:30:13 +0200 Subject: [PATCH 3/6] Fix for the broken test --- browser/main/lib/dataApi/attachmentManagement.js | 2 +- tests/dataApi/deleteNote-test.js | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/browser/main/lib/dataApi/attachmentManagement.js b/browser/main/lib/dataApi/attachmentManagement.js index ace33464..a1030aee 100644 --- a/browser/main/lib/dataApi/attachmentManagement.js +++ b/browser/main/lib/dataApi/attachmentManagement.js @@ -199,7 +199,7 @@ function removeStorageAndNoteReferences (input, noteKey) { function deleteAttachmentFolder (storageKey, noteKey) { const storagePath = findStorage.findStorage(storageKey) const noteAttachmentPath = path.join(storagePath.path, DESTINATION_FOLDER, noteKey) - sander.rimraf(noteAttachmentPath) + sander.rimrafSync(noteAttachmentPath) } module.exports = { diff --git a/tests/dataApi/deleteNote-test.js b/tests/dataApi/deleteNote-test.js index c9a33145..9c809dcf 100644 --- a/tests/dataApi/deleteNote-test.js +++ b/tests/dataApi/deleteNote-test.js @@ -45,7 +45,8 @@ test.serial('Delete a note', (t) => { .then(function doTest () { return createNote(storageKey, input1) .then(function createAttachmentFolder (data) { - fs.mkdirSync(path.join(storagePath.path, attachmentManagement.DESTINATION_FOLDER, data.noteKey)) + fs.mkdirSync(path.join(storagePath, attachmentManagement.DESTINATION_FOLDER)) + fs.mkdirSync(path.join(storagePath, attachmentManagement.DESTINATION_FOLDER, data.key)) return data }) .then(function (data) { @@ -54,15 +55,16 @@ test.serial('Delete a note', (t) => { }) .then(function assert (data) { try { - CSON.readFileSync(path.join(storagePath.path, 'notes', data.noteKey + '.cson')) + CSON.readFileSync(path.join(storagePath, 'notes', data.noteKey + '.cson')) t.fail('note cson must be deleted.') } catch (err) { t.is(err.code, 'ENOENT') + return data } }) .then(function assertAttachmentFolderDeleted (data) { - const attachmentFolderPath = path.join(storagePath.path, attachmentManagement.DESTINATION_FOLDER, data.noteKey) - t.assert(fs.existsSync(attachmentFolderPath) === false, 'Attachment folder was not deleted') + const attachmentFolderPath = path.join(storagePath, attachmentManagement.DESTINATION_FOLDER, data.noteKey) + t.is(fs.existsSync(attachmentFolderPath), false) }) }) From e9218d10889cd89b4dc13a7ea772d84a032d1bef Mon Sep 17 00:00:00 2001 From: ehhc Date: Thu, 10 May 2018 22:39:00 +0200 Subject: [PATCH 4/6] Fix for the broken test --- tests/dataApi/attachmentManagement.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/dataApi/attachmentManagement.test.js b/tests/dataApi/attachmentManagement.test.js index 148f6958..734aa3b9 100644 --- a/tests/dataApi/attachmentManagement.test.js +++ b/tests/dataApi/attachmentManagement.test.js @@ -267,10 +267,10 @@ it('should delete the correct attachment folder if a note is deleted', function const storageKey = 'storageKey' const noteKey = 'noteKey' findStorage.findStorage = jest.fn(() => dummyStorage) - sander.rimraf = jest.fn() + sander.rimrafSync = jest.fn() const expectedPathToBeDeleted = path.join(dummyStorage.path, systemUnderTest.DESTINATION_FOLDER, noteKey) systemUnderTest.deleteAttachmentFolder(storageKey, noteKey) expect(findStorage.findStorage).toHaveBeenCalledWith(storageKey) - expect(sander.rimraf).toHaveBeenCalledWith(expectedPathToBeDeleted) + expect(sander.rimrafSync).toHaveBeenCalledWith(expectedPathToBeDeleted) }) From 6f52744b0fba0b4fb2af1618fe6fbea2e4c57d91 Mon Sep 17 00:00:00 2001 From: ehhc Date: Tue, 15 May 2018 20:32:35 +0200 Subject: [PATCH 5/6] Fix eslinter issue From 905d6860fca8d4724b63c81aed9ce863e5da2ea2 Mon Sep 17 00:00:00 2001 From: ehhc Date: Tue, 15 May 2018 20:39:16 +0200 Subject: [PATCH 6/6] really fix the eslinter issue... -.- --- tests/dataApi/attachmentManagement.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/dataApi/attachmentManagement.test.js b/tests/dataApi/attachmentManagement.test.js index e625005f..91cac64b 100644 --- a/tests/dataApi/attachmentManagement.test.js +++ b/tests/dataApi/attachmentManagement.test.js @@ -274,7 +274,7 @@ it('should delete the correct attachment folder if a note is deleted', function expect(findStorage.findStorage).toHaveBeenCalledWith(storageKey) expect(sander.rimrafSync).toHaveBeenCalledWith(expectedPathToBeDeleted) }) - + it('should test that deleteAttachmentsNotPresentInNote deletes all unreferenced attachments ', function () { const dummyStorage = {path: 'dummyStoragePath'} const noteKey = 'noteKey'