From 583ae6bc70b7fb6aeceab5134d0faf7af523a4ce Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 30 Dec 2024 17:30:48 -0300 Subject: [PATCH 1/4] fix: Thumbnails are not deleted on room deletion --- apps/meteor/app/file-upload/server/lib/FileUpload.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/meteor/app/file-upload/server/lib/FileUpload.ts b/apps/meteor/app/file-upload/server/lib/FileUpload.ts index cffdfe6288f2..7d4dfff28927 100644 --- a/apps/meteor/app/file-upload/server/lib/FileUpload.ts +++ b/apps/meteor/app/file-upload/server/lib/FileUpload.ts @@ -638,20 +638,20 @@ export const FileUpload = { const cursor = Messages.find( { rid, - 'file._id': { + 'files._id': { $exists: true, }, }, { projection: { - 'file._id': 1, + 'files._id': 1, }, }, ); for await (const document of cursor) { - if (document.file) { - await FileUpload.getStore('Uploads').deleteById(document.file._id); + if (document.files) { + await Promise.all(document.files.map((file) => FileUpload.getStore('Uploads').deleteById(file._id))); } } }, From 36fa8952e2428ed07f2d3f4f3d7b717c20be9e9b Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 30 Dec 2024 17:31:05 -0300 Subject: [PATCH 2/4] tests: add unit tests --- apps/meteor/.mocharc.js | 2 +- .../file-upload/server/lib/FileUpload.spec.ts | 103 ++++++++++++++++++ apps/meteor/tests/mocks/data.ts | 9 ++ 3 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 apps/meteor/app/file-upload/server/lib/FileUpload.spec.ts diff --git a/apps/meteor/.mocharc.js b/apps/meteor/.mocharc.js index f11b315b8cef..c14c09c26959 100644 --- a/apps/meteor/.mocharc.js +++ b/apps/meteor/.mocharc.js @@ -36,6 +36,6 @@ module.exports = { 'tests/unit/lib/**/*.spec.ts', 'tests/unit/server/**/*.tests.ts', 'tests/unit/server/**/*.spec.ts', - 'app/api/**/*.spec.ts', + 'app/file-upload/server/**/*.spec.ts', ], }; diff --git a/apps/meteor/app/file-upload/server/lib/FileUpload.spec.ts b/apps/meteor/app/file-upload/server/lib/FileUpload.spec.ts new file mode 100644 index 000000000000..73249fea904a --- /dev/null +++ b/apps/meteor/app/file-upload/server/lib/FileUpload.spec.ts @@ -0,0 +1,103 @@ +import { expect } from 'chai'; +import { before, beforeEach, describe, it } from 'mocha'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +import { createFakeMessageWithAttachment } from '../../../../tests/mocks/data'; + +const fakeStorageModel = { findOneById: sinon.stub(), deleteFile: sinon.stub() }; +const settingsStub = { watch: sinon.stub(), get: sinon.stub() }; +const settingsGetMap = new Map(); +const messagesModelStub = { + find: sinon.stub(), +}; + +const { FileUpload, FileUploadClass } = proxyquire.noCallThru().load('./FileUpload', { + '@rocket.chat/models': { + Messages: messagesModelStub, + }, + 'meteor/check': sinon.stub(), + 'meteor/meteor': sinon.stub(), + 'meteor/ostrio:cookies': { Cookies: sinon.stub() }, + 'sharp': sinon.stub(), + 'stream-buffers': sinon.stub(), + './streamToBuffer': sinon.stub(), + '../../../../server/lib/i18n': sinon.stub(), + '../../../../server/lib/logger/system': sinon.stub(), + '../../../../server/lib/rooms/roomCoordinator': sinon.stub(), + '../../../../server/ufs': sinon.stub(), + '../../../../server/ufs/ufs-methods': sinon.stub(), + '../../../settings/server': { settings: settingsStub }, + '../../../utils/lib/mimeTypes': sinon.stub(), + '../../../utils/server/lib/JWTHelper': sinon.stub(), + '../../../utils/server/restrictions': sinon.stub(), +}); + +describe('FileUpload', () => { + before(() => { + new FileUploadClass({ name: 'fakeStorage:Uploads', model: fakeStorageModel, store: {} }); + settingsGetMap.set('FileUpload_Storage_Type', 'fakeStorage'); + settingsStub.get.callsFake((settingName) => settingsGetMap.get(settingName)); + }); + + beforeEach(() => { + messagesModelStub.find.reset(); + fakeStorageModel.findOneById.reset(); + fakeStorageModel.deleteFile.reset(); + }); + + it('should not remove any file if no room id is provided', async () => { + expect(await FileUpload.removeFilesByRoomId()).to.be.undefined; + + expect(messagesModelStub.find.called).to.be.false; + expect(fakeStorageModel.findOneById.called).to.be.false; + }); + + it('should not remove any file if an empty room id is provided', async () => { + expect(await FileUpload.removeFilesByRoomId('')).to.be.undefined; + + expect(messagesModelStub.find.called).to.be.false; + expect(fakeStorageModel.findOneById.called).to.be.false; + }); + + it('should not remove any file if an invalid room id is provided', async () => { + messagesModelStub.find.returns([]); + expect(await FileUpload.removeFilesByRoomId('invalid')).to.be.undefined; + + expect(messagesModelStub.find.called).to.be.true; + expect(fakeStorageModel.findOneById.called).to.be.false; + }); + + it('should delete file from storage if message contains a single file', async () => { + fakeStorageModel.findOneById.resolves({ _id: 'file-id', store: 'fakeStorage:Uploads' }); + + const fakeMessage = createFakeMessageWithAttachment(); + messagesModelStub.find.returns([fakeMessage]); + expect(await FileUpload.removeFilesByRoomId('invalid')).to.be.undefined; + + expect(messagesModelStub.find.called).to.be.true; + expect(fakeStorageModel.findOneById.calledOnceWith(fakeMessage.files?.[0]._id)).to.be.true; + expect(fakeStorageModel.deleteFile.calledOnceWith('file-id')).to.be.true; + }); + + it('should delete multiple files from storage if message contains many files (e.g. image and thumbnail)', async () => { + fakeStorageModel.findOneById.callsFake((_id) => ({ _id, store: 'fakeStorage:Uploads' })); + + const fakeMessage = createFakeMessageWithAttachment({ + files: [ + { _id: 'file-id', name: 'image', size: 100, type: 'image/png', format: 'png' }, + { _id: 'thumbnail-id', name: 'thumbnail-image', size: 25, type: 'image/png', format: 'png' }, + ], + }); + messagesModelStub.find.returns([fakeMessage]); + expect(await FileUpload.removeFilesByRoomId('invalid')).to.be.undefined; + + expect(messagesModelStub.find.called).to.be.true; + expect(fakeStorageModel.findOneById.calledTwice).to.be.true; + expect(fakeStorageModel.findOneById.calledWith('file-id')).to.be.true; + expect(fakeStorageModel.findOneById.calledWith('thumbnail-id')).to.be.true; + expect(fakeStorageModel.deleteFile.calledTwice).to.be.true; + expect(fakeStorageModel.deleteFile.calledWith('file-id')).to.be.true; + expect(fakeStorageModel.deleteFile.calledWith('thumbnail-id')).to.be.true; + }); +}); diff --git a/apps/meteor/tests/mocks/data.ts b/apps/meteor/tests/mocks/data.ts index 26dbdca25179..d059d941abd6 100644 --- a/apps/meteor/tests/mocks/data.ts +++ b/apps/meteor/tests/mocks/data.ts @@ -253,6 +253,15 @@ export function createFakeMessageWithAttachment(overrides?: Partial): title_link: `/file-upload/${fileId}/${fileName}`, }, ], + files: [ + { + _id: fileId, + name: fileName, + type: 'text/plain', + size: faker.number.int(), + format: faker.string.alpha(), + }, + ], ...overrides, }; } From 82108dac4356338df87ba9a78f3b2efbecca54bb Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 30 Dec 2024 17:32:39 -0300 Subject: [PATCH 3/4] re-add unrelated change --- apps/meteor/.mocharc.js | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/meteor/.mocharc.js b/apps/meteor/.mocharc.js index c14c09c26959..344cd772d49d 100644 --- a/apps/meteor/.mocharc.js +++ b/apps/meteor/.mocharc.js @@ -36,6 +36,7 @@ module.exports = { 'tests/unit/lib/**/*.spec.ts', 'tests/unit/server/**/*.tests.ts', 'tests/unit/server/**/*.spec.ts', + 'app/api/**/*.spec.ts', 'app/file-upload/server/**/*.spec.ts', ], }; From 8a72729a669ecb41c1ff7912e90596be7af2ab0c Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Mon, 30 Dec 2024 17:35:42 -0300 Subject: [PATCH 4/4] Create changeset --- .changeset/twelve-pumas-behave.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/twelve-pumas-behave.md diff --git a/.changeset/twelve-pumas-behave.md b/.changeset/twelve-pumas-behave.md new file mode 100644 index 000000000000..de651a34dbf2 --- /dev/null +++ b/.changeset/twelve-pumas-behave.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +Fixes thumbnails not being deleted from storage on room deletion