From ab4f5278b0222eed121492ec3e181900f81cee73 Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Tue, 16 Mar 2021 19:02:35 +0300 Subject: [PATCH 01/20] Added a migration --- .../index.js | 26 ++++++++++++++++ ...6-populate-mimetype-on-attachments.spec.js | 30 +++++++++++++++++++ .../specs/fixtures.js | 3 ++ 3 files changed, 59 insertions(+) create mode 100644 app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js create mode 100644 app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js create mode 100644 app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js new file mode 100644 index 0000000000..0aeb7b0a6b --- /dev/null +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -0,0 +1,26 @@ +import request from 'shared/JSONRequest'; + +export default { + delta: 36, + + name: 'populate-mimetype-to-attachment', + + description: 'Populates mimetype of an attachment from a url ', + + async up(db) { + process.stdout.write(`${this.name}...\r\n`); + const cursor = await db.collection('files').find(); + + // eslint-disable-next-line no-await-in-loop + while (await cursor.hasNext()) { + const file = cursor.next(); + if (file.url && !file.mimetype) { + // eslint-disable-next-line no-await-in-loop + const response = await request.head(file.url); + const mimetype = response.headers.get('content-type') || undefined; + + db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); + } + } + }, +}; diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js new file mode 100644 index 0000000000..8e0bdbc73f --- /dev/null +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -0,0 +1,30 @@ +import testingDB from 'api/utils/testing_db'; +import request from 'shared/JSONRequest'; +import migration from '../index.js'; +import fixtures from './fixtures.js'; + +describe('migration populate-mimetype-on-attachments', () => { + let headRequestMock; + beforeEach(async () => { + spyOn(process.stdout, 'write'); + headRequestMock = spyOn(request, 'head'); + await testingDB.clearAllAndLoad(fixtures); + }); + + afterAll(async () => { + headRequestMock.mockRestore(); + await testingDB.disconnect(); + }); + + it('should have a delta number', () => { + expect(migration.delta).toBe(36); + }); + + it('should populate mimetype with Content-Type', async () => { + headRequestMock.mockReturnValue( + Promise.resolve({ headers: { 'Content-Type': 'application/pdf' } }) + ); + await migration.up(testingDB.mongodb); + expect(request.head).toHaveBeenCalled(); + }); +}); diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js new file mode 100644 index 0000000000..64aaf47ee6 --- /dev/null +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js @@ -0,0 +1,3 @@ +export default { + files: [{ url: 'some/file/path.jpg' }], +}; From 2cf7bd105537538234fb27788533c0cf214a95ac Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Wed, 17 Mar 2021 16:55:43 +0300 Subject: [PATCH 02/20] Added tests to the migration --- .../index.js | 5 ++- ...6-populate-mimetype-on-attachments.spec.js | 41 +++++++++++++++++-- .../specs/fixtures.js | 2 +- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js index 0aeb7b0a6b..039dc2c13e 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -9,11 +9,12 @@ export default { async up(db) { process.stdout.write(`${this.name}...\r\n`); - const cursor = await db.collection('files').find(); + const cursor = await db.collection('files').find({}); // eslint-disable-next-line no-await-in-loop while (await cursor.hasNext()) { - const file = cursor.next(); + // eslint-disable-next-line no-await-in-loop + const file = await cursor.next(); if (file.url && !file.mimetype) { // eslint-disable-next-line no-await-in-loop const response = await request.head(file.url); diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index 8e0bdbc73f..382418f06b 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -21,10 +21,45 @@ describe('migration populate-mimetype-on-attachments', () => { }); it('should populate mimetype with Content-Type', async () => { - headRequestMock.mockReturnValue( - Promise.resolve({ headers: { 'Content-Type': 'application/pdf' } }) + await testingDB.clearAllAndLoad(fixtures); + const headers = { + get: jest + .fn() + .mockReturnValueOnce('application/pdf') + .mockReturnValueOnce('mimetype2'), + }; + headRequestMock.and.returnValue( + Promise.resolve({ + headers, + }) ); await migration.up(testingDB.mongodb); - expect(request.head).toHaveBeenCalled(); + expect(request.head).toHaveBeenCalledWith(fixtures.files[0].url); + expect(request.head).toHaveBeenCalledWith(fixtures.files[1].url); + expect(headers.get).toHaveBeenCalledWith('content-type'); + + const files = await testingDB.mongodb + .collection('files') + .find({}) + .toArray(); + + expect(files[0].mimetype).toEqual('application/pdf'); + expect(files[1].mimetype).toEqual('mimetype2'); + }); + + it('should not change the value of mimetype if it already exists', async () => { + const fixturesWithMimetype = { + files: [ + { + url: 'some/url/item.jpg', + mimetype: 'application/pdf', + }, + ], + }; + await testingDB.clearAllAndLoad(fixturesWithMimetype); + + const file = await testingDB.mongodb.collection('files').findOne({}); + + expect(file.mimetype).toEqual(fixturesWithMimetype.files[0].mimetype); }); }); diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js index 64aaf47ee6..ddb3e522b1 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js @@ -1,3 +1,3 @@ export default { - files: [{ url: 'some/file/path.jpg' }], + files: [{ url: 'some/file/path.jpg' }, { url: 'some/other/path.jpg' }], }; From 4fb9c8630aa6f3a9150f912d357c8c3a50d8ef5b Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Thu, 18 Mar 2021 09:53:38 +0300 Subject: [PATCH 03/20] Added a case where the attachment is local --- .../index.js | 9 +++- ...6-populate-mimetype-on-attachments.spec.js | 54 ++++++++++++++++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js index 039dc2c13e..0510918bc6 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -1,4 +1,6 @@ import request from 'shared/JSONRequest'; +import { attachmentsPath } from 'api/files/filesystem'; +import { execSync } from 'child_process'; export default { delta: 36, @@ -19,7 +21,12 @@ export default { // eslint-disable-next-line no-await-in-loop const response = await request.head(file.url); const mimetype = response.headers.get('content-type') || undefined; - + db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); + } + if (file.filename && file.type === 'attachment' && !file.mimetype) { + const mimetype = execSync(`file --mime-type -b "${attachmentsPath(file.filename)}"`) + .toString() + .trim(); db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); } } diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index 382418f06b..c1a8f78ee5 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -1,18 +1,27 @@ import testingDB from 'api/utils/testing_db'; import request from 'shared/JSONRequest'; +import * as attachmentMethods from 'api/files/filesystem'; +import childProcess from 'child_process'; import migration from '../index.js'; import fixtures from './fixtures.js'; describe('migration populate-mimetype-on-attachments', () => { let headRequestMock; + let attachmentPathMock; + let execSyncMock; + beforeEach(async () => { spyOn(process.stdout, 'write'); headRequestMock = spyOn(request, 'head'); + attachmentPathMock = spyOn(attachmentMethods, 'attachmentsPath'); + execSyncMock = spyOn(childProcess, 'execSync'); await testingDB.clearAllAndLoad(fixtures); }); afterAll(async () => { headRequestMock.mockRestore(); + attachmentPathMock.mockRestore(); + execSyncMock.mockRestore(); await testingDB.disconnect(); }); @@ -47,7 +56,7 @@ describe('migration populate-mimetype-on-attachments', () => { expect(files[1].mimetype).toEqual('mimetype2'); }); - it('should not change the value of mimetype if it already exists', async () => { + it('should not change the value of mimetype if it already exists in external attachments', async () => { const fixturesWithMimetype = { files: [ { @@ -62,4 +71,47 @@ describe('migration populate-mimetype-on-attachments', () => { expect(file.mimetype).toEqual(fixturesWithMimetype.files[0].mimetype); }); + + it('should not change if value of mimetype already exists in internal attachments', async () => { + const fixturesWithFilenames = { + files: [ + { + filename: 'somename.pdf', + mimetype: 'application/pdf', + type: 'attachment', + }, + ], + }; + await testingDB.clearAllAndLoad(fixturesWithFilenames); + attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); + execSyncMock.and.returnValue('application/pdf'); + await migration.up(testingDB.mongodb); + + const file = await testingDB.mongodb.collection('files').findOne({}); + expect(file.mimetype).toEqual(fixturesWithFilenames.files[0].mimetype); + }); + + it('should update mimetype if filename exists in internal attachments', async () => { + const fixturesWithFilenames = { + files: [ + { + filename: 'somename.pdf', + type: 'attachment', + }, + ], + }; + await testingDB.clearAllAndLoad(fixturesWithFilenames); + execSyncMock.and.returnValue('application/pdf'); + attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); + await migration.up(testingDB.mongodb); + + const file = await testingDB.mongodb.collection('files').findOne({}); + expect(file.mimetype).toEqual('application/pdf'); + expect(attachmentMethods.attachmentsPath).toHaveBeenCalledWith( + fixturesWithFilenames.files[0].filename + ); + expect(childProcess.execSync).toHaveBeenCalledWith( + 'file --mime-type -b "/some/path/to/file.pdf"' + ); + }); }); From eee3b43dfffc501de4932114732f3cc3a023a977 Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Thu, 18 Mar 2021 09:57:24 +0300 Subject: [PATCH 04/20] Addeda test making sure mimetype is not updated if type is not attachment --- .../36-populate-mimetype-on-attachments.spec.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index c1a8f78ee5..bae0607f3c 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -114,4 +114,21 @@ describe('migration populate-mimetype-on-attachments', () => { 'file --mime-type -b "/some/path/to/file.pdf"' ); }); + it('should not update mimetype if type is not attachment in internal attachments', async () => { + const fixturesWithFilenames = { + files: [ + { + filename: 'somename.pdf', + type: 'document', + }, + ], + }; + await testingDB.clearAllAndLoad(fixturesWithFilenames); + execSyncMock.and.returnValue('application/pdf'); + attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); + await migration.up(testingDB.mongodb); + + expect(attachmentMethods.attachmentsPath).not.toHaveBeenCalledWith(); + expect(childProcess.execSync).not.toHaveBeenCalledWith(); + }); }); From 6aec884aa3edd11488085d8b0ed313071aaff9de Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Fri, 19 Mar 2021 11:09:27 +0300 Subject: [PATCH 05/20] Updated tests --- .../specs/36-populate-mimetype-on-attachments.spec.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index bae0607f3c..5b36525019 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -128,7 +128,9 @@ describe('migration populate-mimetype-on-attachments', () => { attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); await migration.up(testingDB.mongodb); - expect(attachmentMethods.attachmentsPath).not.toHaveBeenCalledWith(); - expect(childProcess.execSync).not.toHaveBeenCalledWith(); + expect(attachmentMethods.attachmentsPath).not.toHaveBeenCalledWith( + fixturesWithFilenames.files[0].filename + ); + expect(childProcess.execSync).not.toHaveBeenCalledWith('/some/path/to/file.pdf'); }); }); From 1763aa49d32588421fb2c903140557117d4e862c Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Mon, 22 Mar 2021 15:29:43 +0300 Subject: [PATCH 06/20] Removed fixtures file --- .../specs/36-populate-mimetype-on-attachments.spec.js | 5 +++-- .../36-populate-mimetype-on-attachments/specs/fixtures.js | 3 --- 2 files changed, 3 insertions(+), 5 deletions(-) delete mode 100644 app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index 5b36525019..80670e790d 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -3,7 +3,6 @@ import request from 'shared/JSONRequest'; import * as attachmentMethods from 'api/files/filesystem'; import childProcess from 'child_process'; import migration from '../index.js'; -import fixtures from './fixtures.js'; describe('migration populate-mimetype-on-attachments', () => { let headRequestMock; @@ -15,7 +14,6 @@ describe('migration populate-mimetype-on-attachments', () => { headRequestMock = spyOn(request, 'head'); attachmentPathMock = spyOn(attachmentMethods, 'attachmentsPath'); execSyncMock = spyOn(childProcess, 'execSync'); - await testingDB.clearAllAndLoad(fixtures); }); afterAll(async () => { @@ -30,6 +28,9 @@ describe('migration populate-mimetype-on-attachments', () => { }); it('should populate mimetype with Content-Type', async () => { + const fixtures = { + files: [{ url: 'some/file/path.jpg' }, { url: 'some/other/path.jpg' }], + } await testingDB.clearAllAndLoad(fixtures); const headers = { get: jest diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js deleted file mode 100644 index ddb3e522b1..0000000000 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js +++ /dev/null @@ -1,3 +0,0 @@ -export default { - files: [{ url: 'some/file/path.jpg' }, { url: 'some/other/path.jpg' }], -}; From f2b39cbf6220f4bc7ac4bcb3bc26a43bcc5f6b7f Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Mon, 22 Mar 2021 15:47:10 +0300 Subject: [PATCH 07/20] Added an else for readability --- .../migrations/36-populate-mimetype-on-attachments/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js index 0510918bc6..10926a95dd 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -22,8 +22,7 @@ export default { const response = await request.head(file.url); const mimetype = response.headers.get('content-type') || undefined; db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); - } - if (file.filename && file.type === 'attachment' && !file.mimetype) { + } else if (file.filename && file.type === 'attachment' && !file.mimetype) { const mimetype = execSync(`file --mime-type -b "${attachmentsPath(file.filename)}"`) .toString() .trim(); From 2413b95f58e2c1b10e087e522d09c3012c94da1d Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Tue, 16 Mar 2021 19:02:35 +0300 Subject: [PATCH 08/20] Added a migration --- .../index.js | 26 ++++++++++++++++ ...6-populate-mimetype-on-attachments.spec.js | 30 +++++++++++++++++++ .../specs/fixtures.js | 3 ++ 3 files changed, 59 insertions(+) create mode 100644 app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js create mode 100644 app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js create mode 100644 app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js new file mode 100644 index 0000000000..0aeb7b0a6b --- /dev/null +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -0,0 +1,26 @@ +import request from 'shared/JSONRequest'; + +export default { + delta: 36, + + name: 'populate-mimetype-to-attachment', + + description: 'Populates mimetype of an attachment from a url ', + + async up(db) { + process.stdout.write(`${this.name}...\r\n`); + const cursor = await db.collection('files').find(); + + // eslint-disable-next-line no-await-in-loop + while (await cursor.hasNext()) { + const file = cursor.next(); + if (file.url && !file.mimetype) { + // eslint-disable-next-line no-await-in-loop + const response = await request.head(file.url); + const mimetype = response.headers.get('content-type') || undefined; + + db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); + } + } + }, +}; diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js new file mode 100644 index 0000000000..8e0bdbc73f --- /dev/null +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -0,0 +1,30 @@ +import testingDB from 'api/utils/testing_db'; +import request from 'shared/JSONRequest'; +import migration from '../index.js'; +import fixtures from './fixtures.js'; + +describe('migration populate-mimetype-on-attachments', () => { + let headRequestMock; + beforeEach(async () => { + spyOn(process.stdout, 'write'); + headRequestMock = spyOn(request, 'head'); + await testingDB.clearAllAndLoad(fixtures); + }); + + afterAll(async () => { + headRequestMock.mockRestore(); + await testingDB.disconnect(); + }); + + it('should have a delta number', () => { + expect(migration.delta).toBe(36); + }); + + it('should populate mimetype with Content-Type', async () => { + headRequestMock.mockReturnValue( + Promise.resolve({ headers: { 'Content-Type': 'application/pdf' } }) + ); + await migration.up(testingDB.mongodb); + expect(request.head).toHaveBeenCalled(); + }); +}); diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js new file mode 100644 index 0000000000..64aaf47ee6 --- /dev/null +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js @@ -0,0 +1,3 @@ +export default { + files: [{ url: 'some/file/path.jpg' }], +}; From a470aa32566545ccd62efb4facd44c4b48f307bd Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Wed, 17 Mar 2021 16:55:43 +0300 Subject: [PATCH 09/20] Added tests to the migration --- .../index.js | 5 ++- ...6-populate-mimetype-on-attachments.spec.js | 41 +++++++++++++++++-- .../specs/fixtures.js | 2 +- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js index 0aeb7b0a6b..039dc2c13e 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -9,11 +9,12 @@ export default { async up(db) { process.stdout.write(`${this.name}...\r\n`); - const cursor = await db.collection('files').find(); + const cursor = await db.collection('files').find({}); // eslint-disable-next-line no-await-in-loop while (await cursor.hasNext()) { - const file = cursor.next(); + // eslint-disable-next-line no-await-in-loop + const file = await cursor.next(); if (file.url && !file.mimetype) { // eslint-disable-next-line no-await-in-loop const response = await request.head(file.url); diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index 8e0bdbc73f..382418f06b 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -21,10 +21,45 @@ describe('migration populate-mimetype-on-attachments', () => { }); it('should populate mimetype with Content-Type', async () => { - headRequestMock.mockReturnValue( - Promise.resolve({ headers: { 'Content-Type': 'application/pdf' } }) + await testingDB.clearAllAndLoad(fixtures); + const headers = { + get: jest + .fn() + .mockReturnValueOnce('application/pdf') + .mockReturnValueOnce('mimetype2'), + }; + headRequestMock.and.returnValue( + Promise.resolve({ + headers, + }) ); await migration.up(testingDB.mongodb); - expect(request.head).toHaveBeenCalled(); + expect(request.head).toHaveBeenCalledWith(fixtures.files[0].url); + expect(request.head).toHaveBeenCalledWith(fixtures.files[1].url); + expect(headers.get).toHaveBeenCalledWith('content-type'); + + const files = await testingDB.mongodb + .collection('files') + .find({}) + .toArray(); + + expect(files[0].mimetype).toEqual('application/pdf'); + expect(files[1].mimetype).toEqual('mimetype2'); + }); + + it('should not change the value of mimetype if it already exists', async () => { + const fixturesWithMimetype = { + files: [ + { + url: 'some/url/item.jpg', + mimetype: 'application/pdf', + }, + ], + }; + await testingDB.clearAllAndLoad(fixturesWithMimetype); + + const file = await testingDB.mongodb.collection('files').findOne({}); + + expect(file.mimetype).toEqual(fixturesWithMimetype.files[0].mimetype); }); }); diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js index 64aaf47ee6..ddb3e522b1 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js @@ -1,3 +1,3 @@ export default { - files: [{ url: 'some/file/path.jpg' }], + files: [{ url: 'some/file/path.jpg' }, { url: 'some/other/path.jpg' }], }; From cee74b270f505860a959aa2ba38e8ee392c0c260 Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Thu, 18 Mar 2021 09:53:38 +0300 Subject: [PATCH 10/20] Added a case where the attachment is local --- .../index.js | 9 +++- ...6-populate-mimetype-on-attachments.spec.js | 54 ++++++++++++++++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js index 039dc2c13e..0510918bc6 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -1,4 +1,6 @@ import request from 'shared/JSONRequest'; +import { attachmentsPath } from 'api/files/filesystem'; +import { execSync } from 'child_process'; export default { delta: 36, @@ -19,7 +21,12 @@ export default { // eslint-disable-next-line no-await-in-loop const response = await request.head(file.url); const mimetype = response.headers.get('content-type') || undefined; - + db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); + } + if (file.filename && file.type === 'attachment' && !file.mimetype) { + const mimetype = execSync(`file --mime-type -b "${attachmentsPath(file.filename)}"`) + .toString() + .trim(); db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); } } diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index 382418f06b..c1a8f78ee5 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -1,18 +1,27 @@ import testingDB from 'api/utils/testing_db'; import request from 'shared/JSONRequest'; +import * as attachmentMethods from 'api/files/filesystem'; +import childProcess from 'child_process'; import migration from '../index.js'; import fixtures from './fixtures.js'; describe('migration populate-mimetype-on-attachments', () => { let headRequestMock; + let attachmentPathMock; + let execSyncMock; + beforeEach(async () => { spyOn(process.stdout, 'write'); headRequestMock = spyOn(request, 'head'); + attachmentPathMock = spyOn(attachmentMethods, 'attachmentsPath'); + execSyncMock = spyOn(childProcess, 'execSync'); await testingDB.clearAllAndLoad(fixtures); }); afterAll(async () => { headRequestMock.mockRestore(); + attachmentPathMock.mockRestore(); + execSyncMock.mockRestore(); await testingDB.disconnect(); }); @@ -47,7 +56,7 @@ describe('migration populate-mimetype-on-attachments', () => { expect(files[1].mimetype).toEqual('mimetype2'); }); - it('should not change the value of mimetype if it already exists', async () => { + it('should not change the value of mimetype if it already exists in external attachments', async () => { const fixturesWithMimetype = { files: [ { @@ -62,4 +71,47 @@ describe('migration populate-mimetype-on-attachments', () => { expect(file.mimetype).toEqual(fixturesWithMimetype.files[0].mimetype); }); + + it('should not change if value of mimetype already exists in internal attachments', async () => { + const fixturesWithFilenames = { + files: [ + { + filename: 'somename.pdf', + mimetype: 'application/pdf', + type: 'attachment', + }, + ], + }; + await testingDB.clearAllAndLoad(fixturesWithFilenames); + attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); + execSyncMock.and.returnValue('application/pdf'); + await migration.up(testingDB.mongodb); + + const file = await testingDB.mongodb.collection('files').findOne({}); + expect(file.mimetype).toEqual(fixturesWithFilenames.files[0].mimetype); + }); + + it('should update mimetype if filename exists in internal attachments', async () => { + const fixturesWithFilenames = { + files: [ + { + filename: 'somename.pdf', + type: 'attachment', + }, + ], + }; + await testingDB.clearAllAndLoad(fixturesWithFilenames); + execSyncMock.and.returnValue('application/pdf'); + attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); + await migration.up(testingDB.mongodb); + + const file = await testingDB.mongodb.collection('files').findOne({}); + expect(file.mimetype).toEqual('application/pdf'); + expect(attachmentMethods.attachmentsPath).toHaveBeenCalledWith( + fixturesWithFilenames.files[0].filename + ); + expect(childProcess.execSync).toHaveBeenCalledWith( + 'file --mime-type -b "/some/path/to/file.pdf"' + ); + }); }); From 7f573c0f4a4d22a894ee1c065360e9e58160611e Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Thu, 18 Mar 2021 09:57:24 +0300 Subject: [PATCH 11/20] Addeda test making sure mimetype is not updated if type is not attachment --- .../36-populate-mimetype-on-attachments.spec.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index c1a8f78ee5..bae0607f3c 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -114,4 +114,21 @@ describe('migration populate-mimetype-on-attachments', () => { 'file --mime-type -b "/some/path/to/file.pdf"' ); }); + it('should not update mimetype if type is not attachment in internal attachments', async () => { + const fixturesWithFilenames = { + files: [ + { + filename: 'somename.pdf', + type: 'document', + }, + ], + }; + await testingDB.clearAllAndLoad(fixturesWithFilenames); + execSyncMock.and.returnValue('application/pdf'); + attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); + await migration.up(testingDB.mongodb); + + expect(attachmentMethods.attachmentsPath).not.toHaveBeenCalledWith(); + expect(childProcess.execSync).not.toHaveBeenCalledWith(); + }); }); From fc2386cc6f29fce998933f920a6cd05ce728d7ae Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Fri, 19 Mar 2021 11:09:27 +0300 Subject: [PATCH 12/20] Updated tests --- .../specs/36-populate-mimetype-on-attachments.spec.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index bae0607f3c..5b36525019 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -128,7 +128,9 @@ describe('migration populate-mimetype-on-attachments', () => { attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); await migration.up(testingDB.mongodb); - expect(attachmentMethods.attachmentsPath).not.toHaveBeenCalledWith(); - expect(childProcess.execSync).not.toHaveBeenCalledWith(); + expect(attachmentMethods.attachmentsPath).not.toHaveBeenCalledWith( + fixturesWithFilenames.files[0].filename + ); + expect(childProcess.execSync).not.toHaveBeenCalledWith('/some/path/to/file.pdf'); }); }); From 385127fbc46cd22ae4c4fd82d18ffc2c4ded2589 Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Mon, 22 Mar 2021 15:29:43 +0300 Subject: [PATCH 13/20] Removed fixtures file --- .../specs/36-populate-mimetype-on-attachments.spec.js | 5 +++-- .../36-populate-mimetype-on-attachments/specs/fixtures.js | 3 --- 2 files changed, 3 insertions(+), 5 deletions(-) delete mode 100644 app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index 5b36525019..80670e790d 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -3,7 +3,6 @@ import request from 'shared/JSONRequest'; import * as attachmentMethods from 'api/files/filesystem'; import childProcess from 'child_process'; import migration from '../index.js'; -import fixtures from './fixtures.js'; describe('migration populate-mimetype-on-attachments', () => { let headRequestMock; @@ -15,7 +14,6 @@ describe('migration populate-mimetype-on-attachments', () => { headRequestMock = spyOn(request, 'head'); attachmentPathMock = spyOn(attachmentMethods, 'attachmentsPath'); execSyncMock = spyOn(childProcess, 'execSync'); - await testingDB.clearAllAndLoad(fixtures); }); afterAll(async () => { @@ -30,6 +28,9 @@ describe('migration populate-mimetype-on-attachments', () => { }); it('should populate mimetype with Content-Type', async () => { + const fixtures = { + files: [{ url: 'some/file/path.jpg' }, { url: 'some/other/path.jpg' }], + } await testingDB.clearAllAndLoad(fixtures); const headers = { get: jest diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js deleted file mode 100644 index ddb3e522b1..0000000000 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/fixtures.js +++ /dev/null @@ -1,3 +0,0 @@ -export default { - files: [{ url: 'some/file/path.jpg' }, { url: 'some/other/path.jpg' }], -}; From 25e42deb77078e409525dc7f6899e6a49e148201 Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Mon, 22 Mar 2021 15:47:10 +0300 Subject: [PATCH 14/20] Added an else for readability --- .../migrations/36-populate-mimetype-on-attachments/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js index 0510918bc6..10926a95dd 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -22,8 +22,7 @@ export default { const response = await request.head(file.url); const mimetype = response.headers.get('content-type') || undefined; db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); - } - if (file.filename && file.type === 'attachment' && !file.mimetype) { + } else if (file.filename && file.type === 'attachment' && !file.mimetype) { const mimetype = execSync(`file --mime-type -b "${attachmentsPath(file.filename)}"`) .toString() .trim(); From 7adaaa7c7cc2005de9f448a1eaa870f5d17688b6 Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Mon, 22 Mar 2021 16:05:21 +0300 Subject: [PATCH 15/20] Added a mixed test --- ...6-populate-mimetype-on-attachments.spec.js | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index 80670e790d..86d1d6062d 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -134,4 +134,34 @@ describe('migration populate-mimetype-on-attachments', () => { ); expect(childProcess.execSync).not.toHaveBeenCalledWith('/some/path/to/file.pdf'); }); + it('should not use local attachment if url field is present', async () => { + const mixedFixtures = { + files: [ + { + filename: 'somename.pdf', + type: 'attachment', + url: '/some/url/to/file.something', + }, + ], + }; + await testingDB.clearAllAndLoad(mixedFixtures); + const headers = { + get: jest + .fn() + .mockReturnValueOnce('application/pdf') + .mockReturnValueOnce('mimetype2'), + }; + headRequestMock.and.returnValue( + Promise.resolve({ + headers, + }) + ); + execSyncMock.and.returnValue('application/pdf'); + attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); + await migration.up(testingDB.mongodb); + + expect(attachmentMethods.attachmentsPath).not.toHaveBeenCalled(); + expect(childProcess.execSync).not.toHaveBeenCalled(); + expect(request.head).toHaveBeenCalledWith(mixedFixtures.files[0].url); + }); }); From e159e2a8d355f2097f677656f395abafbfa0b09d Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Mon, 22 Mar 2021 16:06:56 +0300 Subject: [PATCH 16/20] Fixed syntax error --- .../specs/36-populate-mimetype-on-attachments.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index 86d1d6062d..dc2dac7a35 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -30,7 +30,7 @@ describe('migration populate-mimetype-on-attachments', () => { it('should populate mimetype with Content-Type', async () => { const fixtures = { files: [{ url: 'some/file/path.jpg' }, { url: 'some/other/path.jpg' }], - } + }; await testingDB.clearAllAndLoad(fixtures); const headers = { get: jest From 21e94367e213071470e56161f90f79001fac781a Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Tue, 23 Mar 2021 12:40:58 +0300 Subject: [PATCH 17/20] Added mime-type lib to get the mimetype of files --- .../index.js | 9 ++++---- ...6-populate-mimetype-on-attachments.spec.js | 22 +++++++++---------- package.json | 1 + yarn.lock | 12 ++++++++++ 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js index 10926a95dd..e08c30f56a 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -1,13 +1,14 @@ import request from 'shared/JSONRequest'; import { attachmentsPath } from 'api/files/filesystem'; -import { execSync } from 'child_process'; +// import { execSync } from 'child_process'; +import mime from 'mime-types'; export default { delta: 36, name: 'populate-mimetype-to-attachment', - description: 'Populates mimetype of an attachment from a url ', + description: 'Populates mimetype of an attachment from a url', async up(db) { process.stdout.write(`${this.name}...\r\n`); @@ -23,9 +24,7 @@ export default { const mimetype = response.headers.get('content-type') || undefined; db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); } else if (file.filename && file.type === 'attachment' && !file.mimetype) { - const mimetype = execSync(`file --mime-type -b "${attachmentsPath(file.filename)}"`) - .toString() - .trim(); + const mimetype = mime.lookup(attachmentsPath(file.filename)); db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); } } diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index dc2dac7a35..27fb21a758 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -2,24 +2,26 @@ import testingDB from 'api/utils/testing_db'; import request from 'shared/JSONRequest'; import * as attachmentMethods from 'api/files/filesystem'; import childProcess from 'child_process'; +import mime from 'mime-types'; import migration from '../index.js'; describe('migration populate-mimetype-on-attachments', () => { let headRequestMock; let attachmentPathMock; let execSyncMock; + let mimeMock; beforeEach(async () => { spyOn(process.stdout, 'write'); headRequestMock = spyOn(request, 'head'); attachmentPathMock = spyOn(attachmentMethods, 'attachmentsPath'); - execSyncMock = spyOn(childProcess, 'execSync'); + mimeMock = spyOn(mime, 'lookup'); }); afterAll(async () => { headRequestMock.mockRestore(); attachmentPathMock.mockRestore(); - execSyncMock.mockRestore(); + mimeMock.mockRestore(); await testingDB.disconnect(); }); @@ -85,7 +87,7 @@ describe('migration populate-mimetype-on-attachments', () => { }; await testingDB.clearAllAndLoad(fixturesWithFilenames); attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); - execSyncMock.and.returnValue('application/pdf'); + mimeMock.and.returnValue('application/pdf'); await migration.up(testingDB.mongodb); const file = await testingDB.mongodb.collection('files').findOne({}); @@ -102,7 +104,7 @@ describe('migration populate-mimetype-on-attachments', () => { ], }; await testingDB.clearAllAndLoad(fixturesWithFilenames); - execSyncMock.and.returnValue('application/pdf'); + mimeMock.and.returnValue('application/pdf'); attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); await migration.up(testingDB.mongodb); @@ -111,9 +113,7 @@ describe('migration populate-mimetype-on-attachments', () => { expect(attachmentMethods.attachmentsPath).toHaveBeenCalledWith( fixturesWithFilenames.files[0].filename ); - expect(childProcess.execSync).toHaveBeenCalledWith( - 'file --mime-type -b "/some/path/to/file.pdf"' - ); + expect(mime.lookup).toHaveBeenCalledWith('/some/path/to/file.pdf'); }); it('should not update mimetype if type is not attachment in internal attachments', async () => { const fixturesWithFilenames = { @@ -125,14 +125,14 @@ describe('migration populate-mimetype-on-attachments', () => { ], }; await testingDB.clearAllAndLoad(fixturesWithFilenames); - execSyncMock.and.returnValue('application/pdf'); + mimeMock.and.returnValue('application/pdf'); attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); await migration.up(testingDB.mongodb); expect(attachmentMethods.attachmentsPath).not.toHaveBeenCalledWith( fixturesWithFilenames.files[0].filename ); - expect(childProcess.execSync).not.toHaveBeenCalledWith('/some/path/to/file.pdf'); + expect(mime.lookup).not.toHaveBeenCalledWith('/some/path/to/file.pdf'); }); it('should not use local attachment if url field is present', async () => { const mixedFixtures = { @@ -156,12 +156,12 @@ describe('migration populate-mimetype-on-attachments', () => { headers, }) ); - execSyncMock.and.returnValue('application/pdf'); + mimeMock.and.returnValue('application/pdf'); attachmentPathMock.and.returnValue('/some/path/to/file.pdf'); await migration.up(testingDB.mongodb); expect(attachmentMethods.attachmentsPath).not.toHaveBeenCalled(); - expect(childProcess.execSync).not.toHaveBeenCalled(); + expect(mime.lookup).not.toHaveBeenCalled(); expect(request.head).toHaveBeenCalledWith(mixedFixtures.files[0].url); }); }); diff --git a/package.json b/package.json index 2136c743f7..dfae563e7f 100644 --- a/package.json +++ b/package.json @@ -112,6 +112,7 @@ "mark.js": "^8.11.1", "markdown-it": "11.0.0", "markdown-it-container": "3.0.0", + "mime-types": "^2.1.29", "moment": "2.27.0", "moment-timezone": "0.5.31", "mongodb": "^3.6.2", diff --git a/yarn.lock b/yarn.lock index 20b664de43..e466648f41 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9428,6 +9428,11 @@ mime-db@1.45.0: resolved "https://registry.yarnpkg.com/mime-db/-/mime-db-1.45.0.tgz#cceeda21ccd7c3a745eba2decd55d4b73e7879ea" integrity sha512-CkqLUxUk15hofLoLyljJSrukZi8mAtgd+yE5uO4tqRZsdsAJKv0O+rFMhVDRJgozy+yG6md5KwuXhD4ocIoP+w== +mime-db@1.46.0: + version "1.46.0" + resolved "https://registry.yarnpkg.com/mime-db/-/mime-db-1.46.0.tgz#6267748a7f799594de3cbc8cde91def349661cee" + integrity sha512-svXaP8UQRZ5K7or+ZmfNhg2xX3yKDMUzqadsSqi4NCH/KomcH75MAMYAGVlvXn4+b/xOPhS3I2uHKRUzvjY7BQ== + mime-db@~1.33.0: version "1.33.0" resolved "https://registry.yarnpkg.com/mime-db/-/mime-db-1.33.0.tgz#a3492050a5cb9b63450541e39d9788d2272783db" @@ -9439,6 +9444,13 @@ mime-types@^2.1.12, mime-types@~2.1.19: dependencies: mime-db "1.45.0" +mime-types@^2.1.29: + version "2.1.29" + resolved "https://registry.yarnpkg.com/mime-types/-/mime-types-2.1.29.tgz#1d4ab77da64b91f5f72489df29236563754bb1b2" + integrity sha512-Y/jMt/S5sR9OaqteJtslsFZKWOIIqMACsJSiHghlCAyhf7jfVYjKBmLiX8OgpWeW+fjJ2b+Az69aPFPkUOY6xQ== + dependencies: + mime-db "1.46.0" + mime-types@~2.1.17, mime-types@~2.1.18: version "2.1.18" resolved "https://registry.yarnpkg.com/mime-types/-/mime-types-2.1.18.tgz#6f323f60a83d11146f831ff11fd66e2fe5503bb8" From e06be466a275d81a87a988963fb3c97479200633 Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Tue, 23 Mar 2021 12:50:35 +0300 Subject: [PATCH 18/20] Removed comments and unused variables --- .../migrations/36-populate-mimetype-on-attachments/index.js | 1 - .../specs/36-populate-mimetype-on-attachments.spec.js | 2 -- 2 files changed, 3 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js index e08c30f56a..d6d9402908 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -1,6 +1,5 @@ import request from 'shared/JSONRequest'; import { attachmentsPath } from 'api/files/filesystem'; -// import { execSync } from 'child_process'; import mime from 'mime-types'; export default { diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js index 27fb21a758..a9c9cb97ca 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/specs/36-populate-mimetype-on-attachments.spec.js @@ -1,14 +1,12 @@ import testingDB from 'api/utils/testing_db'; import request from 'shared/JSONRequest'; import * as attachmentMethods from 'api/files/filesystem'; -import childProcess from 'child_process'; import mime from 'mime-types'; import migration from '../index.js'; describe('migration populate-mimetype-on-attachments', () => { let headRequestMock; let attachmentPathMock; - let execSyncMock; let mimeMock; beforeEach(async () => { From 9a64551d2478b73d469ec86198fa241a045e43a8 Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Wed, 24 Mar 2021 16:34:52 +0300 Subject: [PATCH 19/20] Added undefined as default mimetype --- .../migrations/36-populate-mimetype-on-attachments/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js index d6d9402908..c0cb887e03 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -23,7 +23,7 @@ export default { const mimetype = response.headers.get('content-type') || undefined; db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); } else if (file.filename && file.type === 'attachment' && !file.mimetype) { - const mimetype = mime.lookup(attachmentsPath(file.filename)); + const mimetype = mime.lookup(attachmentsPath(file.filename)) || undefined; db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); } } From c1fdb4d6a75ec5cbf82c7a3140d6d5a09886af31 Mon Sep 17 00:00:00 2001 From: Kevin Nderitu Date: Thu, 25 Mar 2021 15:38:19 +0300 Subject: [PATCH 20/20] Update now happens if mimetype is not undefined --- .../36-populate-mimetype-on-attachments/index.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js index c0cb887e03..db2e54dd8a 100644 --- a/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js +++ b/app/api/migrations/migrations/36-populate-mimetype-on-attachments/index.js @@ -21,11 +21,18 @@ export default { // eslint-disable-next-line no-await-in-loop const response = await request.head(file.url); const mimetype = response.headers.get('content-type') || undefined; - db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); + // eslint-disable-next-line no-await-in-loop + await this.updateFile(db, file, mimetype); } else if (file.filename && file.type === 'attachment' && !file.mimetype) { const mimetype = mime.lookup(attachmentsPath(file.filename)) || undefined; - db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); + // eslint-disable-next-line no-await-in-loop + await this.updateFile(db, file, mimetype); } } }, + async updateFile(db, file, mimetype) { + if (mimetype) { + await db.collection('files').updateOne({ _id: file._id }, { $set: { mimetype } }); + } + }, };