From 0e768ff363f6636766423e7977a66ac446550dab Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Wed, 30 Mar 2022 07:04:46 +0200 Subject: [PATCH 01/29] Attachment Migration had wrong typed source --- server/migrations.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/migrations.js b/server/migrations.js index 8486de1f9..bf26c8f55 100644 --- a/server/migrations.js +++ b/server/migrations.js @@ -1221,7 +1221,7 @@ Migrations.add('migrate-attachments-collectionFS-to-ostrioFiles', () => { cardId: fileObj.cardId, listId: fileObj.listId, swimlaneId: fileObj.swimlaneId, - source: 'import,' + source: 'import' }, userId, size: fileSize, From 54cf71ce1b355d9f1360c4e3d233d9a20d1a1a30 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Thu, 31 Mar 2022 17:50:50 +0200 Subject: [PATCH 02/29] Activity at attachment deleting had an error (name is undefined) - introduced by commit: e702f17c7b4d2d4ffae85670b0c4e4998043f9d0 --- models/activities.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/models/activities.js b/models/activities.js index ff036a037..2b1406753 100644 --- a/models/activities.js +++ b/models/activities.js @@ -247,9 +247,8 @@ if (Meteor.isServer) { params.commentId = comment._id; } if (activity.attachmentId) { - const attachment = activity.attachment(); - params.attachment = attachment.name; - params.attachmentId = attachment._id; + params.attachment = activity.attachmentName; + params.attachmentId = activity.attachmentId; } if (activity.checklistId) { const checklist = activity.checklist(); From 8c7ed1855e6b18f63bca6eae502ad255a1c5085d Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 27 Mar 2022 23:58:41 +0200 Subject: [PATCH 03/29] Create Attachment Popup Menu --- client/components/cards/attachments.jade | 27 +++++++++------- client/components/cards/attachments.js | 39 ++++++++++++++++-------- i18n/en.i18n.json | 3 +- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/client/components/cards/attachments.jade b/client/components/cards/attachments.jade index 74a22f38d..c01195ea1 100644 --- a/client/components/cards/attachments.jade +++ b/client/components/cards/attachments.jade @@ -49,17 +49,7 @@ template(name="attachmentsGalery") if currentUser.isBoardMember unless currentUser.isCommentOnly unless currentUser.isWorker - if isImage - a(class="{{#if $eq ../coverId _id}}js-remove-cover{{else}}js-add-cover{{/if}}") - i.fa.fa-thumb-tack - if($eq ../coverId _id) - | {{_ 'remove-cover'}} - else - | {{_ 'add-cover'}} - if currentUser.isBoardAdmin - a.js-confirm-delete - i.fa.fa-close - | {{_ 'delete'}} + a.fa.fa-navicon.attachment-details-menu.js-open-attachment-menu(title="{{_ 'attachmentActionsPopup-title'}}") if currentUser.isBoardMember unless currentUser.isCommentOnly @@ -67,3 +57,18 @@ template(name="attachmentsGalery") //li.attachment-item.add-attachment a.js-add-attachment(title="{{_ 'add-attachment' }}") i.fa.fa-plus + +template(name="attachmentActionsPopup") + ul.pop-over-list + li + if isImage + a(class="{{#if isCover}}js-remove-cover{{else}}js-add-cover{{/if}}") + i.fa.fa-thumb-tack + if isCover + | {{_ 'remove-cover'}} + else + | {{_ 'add-cover'}} + if currentUser.isBoardAdmin + a.js-confirm-delete + i.fa.fa-close + | {{_ 'delete'}} diff --git a/client/components/cards/attachments.js b/client/components/cards/attachments.js index bfa345186..fb39dce79 100644 --- a/client/components/cards/attachments.js +++ b/client/components/cards/attachments.js @@ -1,23 +1,11 @@ Template.attachmentsGalery.events({ 'click .js-add-attachment': Popup.open('cardAttachments'), - 'click .js-confirm-delete': Popup.afterConfirm( - 'attachmentDelete', - function() { - Attachments.remove(this._id); - Popup.back(); - }, - ), // If we let this event bubble, FlowRouter will handle it and empty the page // content, see #101. 'click .js-download'(event) { event.stopPropagation(); }, - 'click .js-add-cover'() { - Cards.findOne(this.meta.cardId).setCover(this._id); - }, - 'click .js-remove-cover'() { - Cards.findOne(this.meta.cardId).unsetCover(); - }, + 'click .js-open-attachment-menu': Popup.open('attachmentActions'), }); Template.attachmentsGalery.helpers({ @@ -129,3 +117,28 @@ Template.previewClipboardImagePopup.events({ } }, }); + +BlazeComponent.extendComponent({ + isCover() { + const ret = Cards.findOne(this.data().meta.cardId).coverId == this.data()._id; + return ret; + }, + events() { + return [ + { + 'click .js-confirm-delete': Popup.afterConfirm('attachmentDelete', function() { + Attachments.remove(this._id); + Popup.back(2); + }), + 'click .js-add-cover'() { + Cards.findOne(this.data().meta.cardId).setCover(this.data()._id); + Popup.back(); + }, + 'click .js-remove-cover'() { + Cards.findOne(this.data().meta.cardId).unsetCover(); + Popup.back(); + }, + } + ] + } +}).register('attachmentActionsPopup'); diff --git a/i18n/en.i18n.json b/i18n/en.i18n.json index 374a78834..6ba37690e 100644 --- a/i18n/en.i18n.json +++ b/i18n/en.i18n.json @@ -1164,5 +1164,6 @@ "copyChecklist": "Copy Checklist", "copyChecklistPopup-title": "Copy Checklist", "card-show-lists": "Card Show Lists", - "subtaskActionsPopup-title": "Subtask Actions" + "subtaskActionsPopup-title": "Subtask Actions", + "attachmentActionsPopup-title": "Attachment Actions" } From 536fb00d610afe783f2a28260bea7633a3b9a1ff Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Tue, 29 Mar 2022 17:35:40 +0200 Subject: [PATCH 04/29] Attachment Details Action Menu, more space to previous item - better clickable, especially on mobile devices --- client/components/cards/attachments.styl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/client/components/cards/attachments.styl b/client/components/cards/attachments.styl index a8320adcd..02566b850 100644 --- a/client/components/cards/attachments.styl +++ b/client/components/cards/attachments.styl @@ -46,6 +46,9 @@ .attachment-details-actions a display: block + &.attachment-details-menu + padding-top: 10px + .attachment-image-preview max-width: 100px display: block From 44fd652b0593b3806a59b2e16ddc198149923aff Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Fri, 25 Mar 2022 14:08:37 +0100 Subject: [PATCH 05/29] Move Attachment to other storages now possible --- client/components/cards/attachments.jade | 13 + client/components/cards/attachments.js | 8 + i18n/en.i18n.json | 4 +- models/attachments.js | 88 ++++--- models/avatars.js | 19 +- models/lib/attachmentStoreStrategy.js | 246 ++++++++++++++++++ models/lib/fsHooks/createInterceptDownload.js | 6 +- models/lib/fsHooks/createOnAfterRemove.js | 21 +- models/lib/fsHooks/createOnAfterUpload.js | 77 +++--- 9 files changed, 383 insertions(+), 99 deletions(-) create mode 100644 models/lib/attachmentStoreStrategy.js diff --git a/client/components/cards/attachments.jade b/client/components/cards/attachments.jade index c01195ea1..975d21e84 100644 --- a/client/components/cards/attachments.jade +++ b/client/components/cards/attachments.jade @@ -72,3 +72,16 @@ template(name="attachmentActionsPopup") a.js-confirm-delete i.fa.fa-close | {{_ 'delete'}} + p.attachment-storage + | {{versions.original.storage}} + + if $neq versions.original.storage "fs" + a.js-move-storage-fs + i.fa.fa-arrow-right + | {{_ 'attachment-move-storage-fs'}} + + if $neq versions.original.storage "gridfs" + if versions.original.storage + a.js-move-storage-gridfs + i.fa.fa-arrow-right + | {{_ 'attachment-move-storage-gridfs'}} diff --git a/client/components/cards/attachments.js b/client/components/cards/attachments.js index fb39dce79..91ff81ae3 100644 --- a/client/components/cards/attachments.js +++ b/client/components/cards/attachments.js @@ -138,6 +138,14 @@ BlazeComponent.extendComponent({ Cards.findOne(this.data().meta.cardId).unsetCover(); Popup.back(); }, + 'click .js-move-storage-fs'() { + Meteor.call('moveToStorage', this.data()._id, "fs"); + Popup.back(); + }, + 'click .js-move-storage-gridfs'() { + Meteor.call('moveToStorage', this.data()._id, "gridfs"); + Popup.back(); + }, } ] } diff --git a/i18n/en.i18n.json b/i18n/en.i18n.json index 6ba37690e..cfdf89a5a 100644 --- a/i18n/en.i18n.json +++ b/i18n/en.i18n.json @@ -1165,5 +1165,7 @@ "copyChecklistPopup-title": "Copy Checklist", "card-show-lists": "Card Show Lists", "subtaskActionsPopup-title": "Subtask Actions", - "attachmentActionsPopup-title": "Attachment Actions" + "attachmentActionsPopup-title": "Attachment Actions", + "attachment-move-storage-fs": "Move attachment to filesystem", + "attachment-move-storage-gridfs": "Move attachment to GridFS" } diff --git a/models/attachments.js b/models/attachments.js index 6d2e95f0e..38c3566dd 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -2,30 +2,7 @@ import { Meteor } from 'meteor/meteor'; import { FilesCollection } from 'meteor/ostrio:files'; import fs from 'fs'; import path from 'path'; -import { createBucket } from './lib/grid/createBucket'; -import { createOnAfterUpload } from './lib/fsHooks/createOnAfterUpload'; -import { createInterceptDownload } from './lib/fsHooks/createInterceptDownload'; -import { createOnAfterRemove } from './lib/fsHooks/createOnAfterRemove'; - -let attachmentBucket; -if (Meteor.isServer) { - attachmentBucket = createBucket('attachments'); -} - -const insertActivity = (fileObj, activityType) => - Activities.insert({ - userId: fileObj.userId, - type: 'card', - activityType, - attachmentId: fileObj._id, - // this preserves the name so that notifications can be meaningful after - // this file is removed - attachmentName: fileObj.name, - boardId: fileObj.meta.boardId, - cardId: fileObj.meta.cardId, - listId: fileObj.meta.listId, - swimlaneId: fileObj.meta.swimlaneId, - }); +import AttachmentStoreStrategy from '/models/lib/attachmentStoreStrategy'; // XXX Enforce a schema for the Attachments FilesCollection // see: https://github.com/VeliovGroup/Meteor-Files/wiki/Schema @@ -40,20 +17,20 @@ Attachments = new FilesCollection({ } return path.normalize(`assets/app/uploads/${this.collectionName}`); }, - onAfterUpload: function onAfterUpload(fileRef) { - createOnAfterUpload(attachmentBucket).call(this, fileRef); - // If the attachment doesn't have a source field - // or its source is different than import - if (!fileRef.meta.source || fileRef.meta.source !== 'import') { - // Add activity about adding the attachment - insertActivity(fileRef, 'addAttachment'); - } + onAfterUpload(fileObj) { + Object.keys(fileObj.versions).forEach(versionName => { + AttachmentStoreStrategy.getFileStrategy(this, fileObj, versionName).onAfterUpload(); + }) }, - interceptDownload: createInterceptDownload(attachmentBucket), - onAfterRemove: function onAfterRemove(files) { - createOnAfterRemove(attachmentBucket).call(this, files); + interceptDownload(http, fileObj, versionName) { + const ret = AttachmentStoreStrategy.getFileStrategy(this, fileObj, versionName).interceptDownload(http); + return ret; + }, + onAfterRemove(files) { files.forEach(fileObj => { - insertActivity(fileObj, 'deleteAttachment'); + Object.keys(fileObj.versions).forEach(versionName => { + AttachmentStoreStrategy.getFileStrategy(this, fileObj, versionName).onAfterRemove(); + }); }); }, // We authorize the attachment download either: @@ -82,6 +59,45 @@ if (Meteor.isServer) { fetch: ['meta'], }); + Meteor.methods({ + moveToStorage(fileObjId, storageDestination) { + check(fileObjId, String); + check(storageDestination, String); + + const fileObj = Attachments.findOne({_id: fileObjId}); + + Object.keys(fileObj.versions).forEach(versionName => { + const strategyRead = AttachmentStoreStrategy.getFileStrategy(this, fileObj, versionName); + const strategyWrite = AttachmentStoreStrategy.getFileStrategy(this, fileObj, versionName, storageDestination); + + if (strategyRead.constructor.name != strategyWrite.constructor.name) { + const readStream = strategyRead.getReadStream(); + const writeStream = strategyWrite.getWriteStream(); + + writeStream.on('error', error => { + console.error('[writeStream error]: ', error, fileObjId); + }); + + readStream.on('error', error => { + console.error('[readStream error]: ', error, fileObjId); + }); + + writeStream.on('finish', Meteor.bindEnvironment((finishedData) => { + strategyWrite.writeStreamFinished(finishedData); + })); + + // https://forums.meteor.com/t/meteor-code-must-always-run-within-a-fiber-try-wrapping-callbacks-that-you-pass-to-non-meteor-libraries-with-meteor-bindenvironmen/40099/8 + readStream.on('end', Meteor.bindEnvironment(() => { + Attachments.update({ _id: fileObj._id }, { $set: { [`versions.${versionName}.storage`]: strategyWrite.getStorageName() } }); + strategyRead.unlink(); + })); + + readStream.pipe(writeStream); + } + }); + }, + }); + Meteor.startup(() => { Attachments.collection._ensureIndex({ 'meta.cardId': 1 }); const storagePath = Attachments.storagePath(); diff --git a/models/avatars.js b/models/avatars.js index ff47aa150..7e9fe0e05 100644 --- a/models/avatars.js +++ b/models/avatars.js @@ -28,9 +28,22 @@ Avatars = new FilesCollection({ } return 'avatar-too-big'; }, - onAfterUpload: createOnAfterUpload(avatarsBucket), - interceptDownload: createInterceptDownload(avatarsBucket), - onAfterRemove: createOnAfterRemove(avatarsBucket), + onAfterUpload(fileObj) { + Object.keys(fileObj.versions).forEach(versionName => { + createOnAfterUpload(this, avatarsBucket, fileObj, versionName); + }); + }, + interceptDownload(http, fileObj, versionName) { + const ret = createInterceptDownload(this, avatarsBucket, fileObj, http, versionName); + return ret; + }, + onAfterRemove(files) { + files.forEach(fileObj => { + Object.keys(fileObj.versions).forEach(versionName => { + createOnAfterRemove(this, avatarsBucket, fileObj, versionName); + }); + }); + }, }); function isOwner(userId, doc) { diff --git a/models/lib/attachmentStoreStrategy.js b/models/lib/attachmentStoreStrategy.js new file mode 100644 index 000000000..c86aa9e1e --- /dev/null +++ b/models/lib/attachmentStoreStrategy.js @@ -0,0 +1,246 @@ +import fs from 'fs'; +import { createBucket } from './grid/createBucket'; +import { createObjectId } from './grid/createObjectId'; +import { createOnAfterUpload } from './fsHooks/createOnAfterUpload'; +import { createInterceptDownload } from './fsHooks/createInterceptDownload'; +import { createOnAfterRemove } from './fsHooks/createOnAfterRemove'; + +const insertActivity = (fileObj, activityType) => + Activities.insert({ + userId: fileObj.userId, + type: 'card', + activityType, + attachmentId: fileObj._id, + attachmentName: fileObj.name, + boardId: fileObj.meta.boardId, + cardId: fileObj.meta.cardId, + listId: fileObj.meta.listId, + swimlaneId: fileObj.meta.swimlaneId, + }); + +let attachmentBucket; +if (Meteor.isServer) { + attachmentBucket = createBucket('attachments'); +} + +/** Strategy to store attachments */ +class AttachmentStoreStrategy { + + /** constructor + * @param filesCollection the current FilesCollection instance + * @param fileObj the current file object + * @param versionName the current version + */ + constructor(filesCollection, fileObj, versionName) { + this.filesCollection = filesCollection; + this.fileObj = fileObj; + this.versionName = versionName; + } + + /** after successfull upload */ + onAfterUpload() { + // If the attachment doesn't have a source field or its source is different than import + if (!this.fileObj.meta.source || this.fileObj.meta.source !== 'import') { + // Add activity about adding the attachment + insertActivity(this.fileObj, 'addAttachment'); + } + } + + /** download the file + * @param http the current http request + */ + interceptDownload(http) { + } + + /** after file remove */ + onAfterRemove() { + insertActivity(this.fileObj, 'deleteAttachment'); + } + + /** returns a read stream + * @return the read stream + */ + getReadStream() { + } + + /** returns a write stream + * @return the write stream + */ + getWriteStream() { + } + + /** writing finished + * @param finishedData the data of the write stream finish event + */ + writeStreamFinished(finishedData) { + } + + /** remove the file */ + unlink() { + } + + /** return the storage name + * @return the storage name + */ + getStorageName() { + } + + static getFileStrategy(filesCollection, fileObj, versionName, storage) { + if (!storage) { + storage = fileObj.versions[versionName].storage || "gridfs"; + } + let ret; + if (["fs", "gridfs"].includes(storage)) { + if (storage == "fs") { + ret = new AttachmentStoreStrategyFilesystem(filesCollection, fileObj, versionName); + } else if (storage == "gridfs") { + ret = new AttachmentStoreStrategyGridFs(filesCollection, fileObj, versionName); + } + } + console.log("getFileStrategy: ", ret.constructor.name); + return ret; + } +} + +/** Strategy to store attachments at GridFS (MongoDB) */ +class AttachmentStoreStrategyGridFs extends AttachmentStoreStrategy { + + /** constructor + * @param filesCollection the current FilesCollection instance + * @param fileObj the current file object + * @param versionName the current version + */ + constructor(filesCollection, fileObj, versionName) { + super(filesCollection, fileObj, versionName); + } + + /** after successfull upload */ + onAfterUpload() { + createOnAfterUpload(this.filesCollection, attachmentBucket, this.fileObj, this.versionName); + super.onAfterUpload(); + } + + /** download the file + * @param http the current http request + */ + interceptDownload(http) { + const ret = createInterceptDownload(this.filesCollection, attachmentBucket, this.fileObj, http, this.versionName); + return ret; + } + + /** after file remove */ + onAfterRemove() { + this.unlink(); + super.onAfterRemove(); + } + + /** returns a read stream + * @return the read stream + */ + getReadStream() { + const gridFsFileId = (this.fileObj.versions[this.versionName].meta || {}) + .gridFsFileId; + let ret; + if (gridFsFileId) { + const gfsId = createObjectId({ gridFsFileId }); + ret = attachmentBucket.openDownloadStream(gfsId); + } + return ret; + } + + /** returns a write stream + * @return the write stream + */ + getWriteStream() { + const fileObj = this.fileObj; + const versionName = this.versionName; + const metadata = { ...fileObj.meta, versionName, fileId: fileObj._id }; + const ret = attachmentBucket.openUploadStream(this.fileObj.name, { + contentType: fileObj.type || 'binary/octet-stream', + metadata, + }); + return ret; + } + + /** writing finished + * @param finishedData the data of the write stream finish event + */ + writeStreamFinished(finishedData) { + const gridFsFileIdName = this.getGridFsFileIdName(); + Attachments.update({ _id: this.fileObj._id }, { $set: { [gridFsFileIdName]: finishedData._id.toHexString(), } }); + } + + /** remove the file */ + unlink() { + createOnAfterRemove(this.filesCollection, attachmentBucket, this.fileObj, this.versionName); + const gridFsFileIdName = this.getGridFsFileIdName(); + Attachments.update({ _id: this.fileObj._id }, { $unset: { [gridFsFileIdName]: 1 } }); + } + + /** return the storage name + * @return the storage name + */ + getStorageName() { + return "gridfs"; + } + + /** returns the property name of gridFsFileId + * @return the property name of gridFsFileId + */ + getGridFsFileIdName() { + const ret = `versions.${this.versionName}.meta.gridFsFileId`; + return ret; + } +} + +/** Strategy to store attachments at filesystem */ +class AttachmentStoreStrategyFilesystem extends AttachmentStoreStrategy { + + /** constructor + * @param filesCollection the current FilesCollection instance + * @param fileObj the current file object + * @param versionName the current version + */ + constructor(filesCollection, fileObj, versionName) { + super(filesCollection, fileObj, versionName); + } + + /** returns a read stream + * @return the read stream + */ + getReadStream() { + const ret = fs.createReadStream(this.fileObj.versions[this.versionName].path) + return ret; + } + + /** returns a write stream + * @return the write stream + */ + getWriteStream() { + const newFileName = this.fileObj.name; + const filePath = this.fileObj.versions[this.versionName].path; + const ret = fs.createWriteStream(filePath); + return ret; + } + + /** writing finished + * @param finishedData the data of the write stream finish event + */ + writeStreamFinished(finishedData) { + } + + /** remove the file */ + unlink() { + const filePath = this.fileObj.versions[this.versionName].path; + fs.unlink(filePath, () => {}); + } + + /** return the storage name + * @return the storage name + */ + getStorageName() { + return "fs"; + } +} + +export default AttachmentStoreStrategy; diff --git a/models/lib/fsHooks/createInterceptDownload.js b/models/lib/fsHooks/createInterceptDownload.js index c375d03f8..1d094c3f9 100644 --- a/models/lib/fsHooks/createInterceptDownload.js +++ b/models/lib/fsHooks/createInterceptDownload.js @@ -1,7 +1,7 @@ import { createObjectId } from '../grid/createObjectId'; -export const createInterceptDownload = bucket => - function interceptDownload(http, file, versionName) { +export const createInterceptDownload = + function interceptDownload(filesCollection, bucket, file, http, versionName) { const { gridFsFileId } = file.versions[versionName].meta || {}; if (gridFsFileId) { // opens the download stream using a given gfs id @@ -24,7 +24,7 @@ export const createInterceptDownload = bucket => http.response.end('not found'); }); - http.response.setHeader('Cache-Control', this.cacheControl); + http.response.setHeader('Cache-Control', filesCollection.cacheControl); http.response.setHeader( 'Content-Disposition', getContentDisposition(file.name, http?.params?.query?.download), diff --git a/models/lib/fsHooks/createOnAfterRemove.js b/models/lib/fsHooks/createOnAfterRemove.js index f2e0a4ba7..446cf1f0b 100644 --- a/models/lib/fsHooks/createOnAfterRemove.js +++ b/models/lib/fsHooks/createOnAfterRemove.js @@ -1,17 +1,12 @@ import { createObjectId } from '../grid/createObjectId'; -export const createOnAfterRemove = bucket => - function onAfterRemove(files) { - files.forEach(file => { - Object.keys(file.versions).forEach(versionName => { - const gridFsFileId = (file.versions[versionName].meta || {}) - .gridFsFileId; - if (gridFsFileId) { - const gfsId = createObjectId({ gridFsFileId }); - bucket.delete(gfsId, err => { - // if (err) console.error(err); - }); - } +export const createOnAfterRemove = + function onAfterRemove(filesCollection, bucket, file, versionName) { + const gridFsFileId = (file.versions[versionName].meta || {}) + .gridFsFileId; + if (gridFsFileId) { + const gfsId = createObjectId({ gridFsFileId }); + bucket.delete(gfsId, err => { }); - }); + } }; diff --git a/models/lib/fsHooks/createOnAfterUpload.js b/models/lib/fsHooks/createOnAfterUpload.js index 3c6c54036..173d02f3c 100644 --- a/models/lib/fsHooks/createOnAfterUpload.js +++ b/models/lib/fsHooks/createOnAfterUpload.js @@ -1,51 +1,42 @@ import { Meteor } from 'meteor/meteor'; import fs from 'fs'; -export const createOnAfterUpload = bucket => - function onAfterUpload(file) { - const self = this; +export const createOnAfterUpload = function onAfterUpload(filesCollection, bucket, file, versionName) { + const self = filesCollection; + const metadata = { ...file.meta, versionName, fileId: file._id }; + fs.createReadStream(file.versions[versionName].path) - // here you could manipulate your file - // and create a new version, for example a scaled 'thumbnail' - // ... + // this is where we upload the binary to the bucket using bucket.openUploadStream + // see http://mongodb.github.io/node-mongodb-native/3.2/api/GridFSBucket.html#openUploadStream + .pipe( + bucket.openUploadStream(file.name, { + contentType: file.type || 'binary/octet-stream', + metadata, + }), + ) - // then we read all versions we have got so far - Object.keys(file.versions).forEach(versionName => { - const metadata = { ...file.meta, versionName, fileId: file._id }; - fs.createReadStream(file.versions[versionName].path) + // and we unlink the file from the fs on any error + // that occurred during the upload to prevent zombie files + .on('error', err => { + console.error("[createOnAfterUpload error]", err); + self.unlink(self.collection.findOne(file._id), versionName); // Unlink files from FS + }) - // this is where we upload the binary to the bucket using bucket.openUploadStream - // see http://mongodb.github.io/node-mongodb-native/3.2/api/GridFSBucket.html#openUploadStream - .pipe( - bucket.openUploadStream(file.name, { - contentType: file.type || 'binary/octet-stream', - metadata, - }), - ) + // once we are finished, we attach the gridFS Object id on the + // FilesCollection document's meta section and finally unlink the + // upload file from the filesystem + .on( + 'finish', + Meteor.bindEnvironment(ver => { + const property = `versions.${versionName}.meta.gridFsFileId`; - // and we unlink the file from the fs on any error - // that occurred during the upload to prevent zombie files - .on('error', err => { - console.error("[createOnAfterUpload error]", err); - self.unlink(this.collection.findOne(file._id), versionName); // Unlink files from FS - }) + self.collection.update(file._id, { + $set: { + [property]: ver._id.toHexString(), + }, + }); - // once we are finished, we attach the gridFS Object id on the - // FilesCollection document's meta section and finally unlink the - // upload file from the filesystem - .on( - 'finish', - Meteor.bindEnvironment(ver => { - const property = `versions.${versionName}.meta.gridFsFileId`; - - self.collection.update(file._id, { - $set: { - [property]: ver._id.toHexString(), - }, - }); - - self.unlink(this.collection.findOne(file._id), versionName); // Unlink files from FS - }), - ); - }); - }; + self.unlink(self.collection.findOne(file._id), versionName); // Unlink files from FS + }), + ); +}; From 0fcfd8e1682012d4171c61877473ce9d4177c666 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Fri, 1 Apr 2022 23:09:59 +0200 Subject: [PATCH 06/29] Attachment filename contains now the filename of the uploaded file --- client/components/cards/attachments.js | 30 ++++++++++++++++---------- models/attachments.js | 7 ++++++ 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/client/components/cards/attachments.js b/client/components/cards/attachments.js index 91ff81ae3..7a3245320 100644 --- a/client/components/cards/attachments.js +++ b/client/components/cards/attachments.js @@ -21,12 +21,16 @@ Template.cardAttachmentsPopup.events({ 'change .js-attach-file'(event) { const card = this; if (event.currentTarget.files && event.currentTarget.files[0]) { + const fileId = Random.id(); + const config = { + file: event.currentTarget.files[0], + fileId: fileId, + meta: Utils.getCommonAttachmentMetaFrom(card), + chunkSize: 'dynamic', + }; + config.meta.fileId = fileId; const uploader = Attachments.insert( - { - file: event.currentTarget.files[0], - meta: Utils.getCommonAttachmentMetaFrom(card), - chunkSize: 'dynamic', - }, + config, false, ); uploader.on('uploaded', (error, fileRef) => { @@ -92,13 +96,17 @@ Template.previewClipboardImagePopup.events({ if (pastedResults && pastedResults.file) { const file = pastedResults.file; window.oPasted = pastedResults; + const fileId = Random.id(); + const config = { + file, + fileId: fileId, + meta: Utils.getCommonAttachmentMetaFrom(card), + fileName: file.name || file.type.replace('image/', 'clipboard.'), + chunkSize: 'dynamic', + }; + config.meta.fileId = fileId; const uploader = Attachments.insert( - { - file, - meta: Utils.getCommonAttachmentMetaFrom(card), - fileName: file.name || file.type.replace('image/', 'clipboard.'), - chunkSize: 'dynamic', - }, + config, false, ); uploader.on('uploaded', (error, fileRef) => { diff --git a/models/attachments.js b/models/attachments.js index 38c3566dd..e8df5a333 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -11,6 +11,13 @@ Attachments = new FilesCollection({ debug: false, // Change to `true` for debugging collectionName: 'attachments', allowClientCode: true, + namingFunction(opts) { + const filenameWithoutExtension = opts.name.replace(/(.+)\..+/, "$1"); + const ret = opts.meta.fileId + "-" + filenameWithoutExtension; + // remove fileId from meta, it was only stored there to have this information here in the namingFunction function + delete opts.meta.fileId; + return ret; + }, storagePath() { if (process.env.WRITABLE_PATH) { return path.join(process.env.WRITABLE_PATH, 'uploads', 'attachments'); From dd0d5dbe9f054d4a6d14b43ff6c0c18e05cae887 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sat, 2 Apr 2022 16:27:37 +0200 Subject: [PATCH 07/29] Output error if bucket.delete fails --- models/lib/fsHooks/createOnAfterRemove.js | 1 + 1 file changed, 1 insertion(+) diff --git a/models/lib/fsHooks/createOnAfterRemove.js b/models/lib/fsHooks/createOnAfterRemove.js index 446cf1f0b..702eaa596 100644 --- a/models/lib/fsHooks/createOnAfterRemove.js +++ b/models/lib/fsHooks/createOnAfterRemove.js @@ -7,6 +7,7 @@ export const createOnAfterRemove = if (gridFsFileId) { const gfsId = createObjectId({ gridFsFileId }); bucket.delete(gfsId, err => { + console.error("error on gfs bucket.delete: ", err); }); } }; From 883beba9edc4f3b0fa78538814a7e5d2ff07b2c3 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Thu, 7 Apr 2022 22:49:13 +0200 Subject: [PATCH 08/29] Split AttachmentStoreStrategy classes to new FileStoreStrategy classes --- client/components/cards/attachments.js | 4 +- models/attachments.js | 50 ++--- models/lib/attachmentStoreStrategy.js | 204 ++---------------- models/lib/fileStoreStrategy.js | 277 +++++++++++++++++++++++++ 4 files changed, 310 insertions(+), 225 deletions(-) create mode 100644 models/lib/fileStoreStrategy.js diff --git a/client/components/cards/attachments.js b/client/components/cards/attachments.js index 7a3245320..8c78b363d 100644 --- a/client/components/cards/attachments.js +++ b/client/components/cards/attachments.js @@ -147,11 +147,11 @@ BlazeComponent.extendComponent({ Popup.back(); }, 'click .js-move-storage-fs'() { - Meteor.call('moveToStorage', this.data()._id, "fs"); + Meteor.call('moveAttachmentToStorage', this.data()._id, "fs"); Popup.back(); }, 'click .js-move-storage-gridfs'() { - Meteor.call('moveToStorage', this.data()._id, "gridfs"); + Meteor.call('moveAttachmentToStorage', this.data()._id, "gridfs"); Popup.back(); }, } diff --git a/models/attachments.js b/models/attachments.js index e8df5a333..1465092fe 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -1,8 +1,17 @@ import { Meteor } from 'meteor/meteor'; import { FilesCollection } from 'meteor/ostrio:files'; +import { createBucket } from './lib/grid/createBucket'; import fs from 'fs'; import path from 'path'; -import AttachmentStoreStrategy from '/models/lib/attachmentStoreStrategy'; +import { AttachmentStoreStrategyFilesystem, AttachmentStoreStrategyGridFs} from '/models/lib/attachmentStoreStrategy'; +import FileStoreStrategyFactory, {moveToStorage} from '/models/lib/fileStoreStrategy'; + +let attachmentBucket; +if (Meteor.isServer) { + attachmentBucket = createBucket('attachments'); +} + +const fileStoreStrategyFactory = new FileStoreStrategyFactory(AttachmentStoreStrategyFilesystem, AttachmentStoreStrategyGridFs, attachmentBucket); // XXX Enforce a schema for the Attachments FilesCollection // see: https://github.com/VeliovGroup/Meteor-Files/wiki/Schema @@ -26,17 +35,17 @@ Attachments = new FilesCollection({ }, onAfterUpload(fileObj) { Object.keys(fileObj.versions).forEach(versionName => { - AttachmentStoreStrategy.getFileStrategy(this, fileObj, versionName).onAfterUpload(); + fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).onAfterUpload(); }) }, interceptDownload(http, fileObj, versionName) { - const ret = AttachmentStoreStrategy.getFileStrategy(this, fileObj, versionName).interceptDownload(http); + const ret = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).interceptDownload(http); return ret; }, onAfterRemove(files) { files.forEach(fileObj => { Object.keys(fileObj.versions).forEach(versionName => { - AttachmentStoreStrategy.getFileStrategy(this, fileObj, versionName).onAfterRemove(); + fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).onAfterRemove(); }); }); }, @@ -67,41 +76,12 @@ if (Meteor.isServer) { }); Meteor.methods({ - moveToStorage(fileObjId, storageDestination) { + moveAttachmentToStorage(fileObjId, storageDestination) { check(fileObjId, String); check(storageDestination, String); const fileObj = Attachments.findOne({_id: fileObjId}); - - Object.keys(fileObj.versions).forEach(versionName => { - const strategyRead = AttachmentStoreStrategy.getFileStrategy(this, fileObj, versionName); - const strategyWrite = AttachmentStoreStrategy.getFileStrategy(this, fileObj, versionName, storageDestination); - - if (strategyRead.constructor.name != strategyWrite.constructor.name) { - const readStream = strategyRead.getReadStream(); - const writeStream = strategyWrite.getWriteStream(); - - writeStream.on('error', error => { - console.error('[writeStream error]: ', error, fileObjId); - }); - - readStream.on('error', error => { - console.error('[readStream error]: ', error, fileObjId); - }); - - writeStream.on('finish', Meteor.bindEnvironment((finishedData) => { - strategyWrite.writeStreamFinished(finishedData); - })); - - // https://forums.meteor.com/t/meteor-code-must-always-run-within-a-fiber-try-wrapping-callbacks-that-you-pass-to-non-meteor-libraries-with-meteor-bindenvironmen/40099/8 - readStream.on('end', Meteor.bindEnvironment(() => { - Attachments.update({ _id: fileObj._id }, { $set: { [`versions.${versionName}.storage`]: strategyWrite.getStorageName() } }); - strategyRead.unlink(); - })); - - readStream.pipe(writeStream); - } - }); + moveToStorage(fileObj, storageDestination, fileStoreStrategyFactory); }, }); diff --git a/models/lib/attachmentStoreStrategy.js b/models/lib/attachmentStoreStrategy.js index c86aa9e1e..e525647af 100644 --- a/models/lib/attachmentStoreStrategy.js +++ b/models/lib/attachmentStoreStrategy.js @@ -1,9 +1,5 @@ import fs from 'fs'; -import { createBucket } from './grid/createBucket'; -import { createObjectId } from './grid/createObjectId'; -import { createOnAfterUpload } from './fsHooks/createOnAfterUpload'; -import { createInterceptDownload } from './fsHooks/createInterceptDownload'; -import { createOnAfterRemove } from './fsHooks/createOnAfterRemove'; +import FileStoreStrategy, {FileStoreStrategyFilesystem, FileStoreStrategyGridFs} from './fileStoreStrategy' const insertActivity = (fileObj, activityType) => Activities.insert({ @@ -18,27 +14,22 @@ const insertActivity = (fileObj, activityType) => swimlaneId: fileObj.meta.swimlaneId, }); -let attachmentBucket; -if (Meteor.isServer) { - attachmentBucket = createBucket('attachments'); -} - -/** Strategy to store attachments */ -class AttachmentStoreStrategy { +/** Strategy to store attachments at GridFS (MongoDB) */ +export class AttachmentStoreStrategyGridFs extends FileStoreStrategyGridFs { /** constructor + * @param gridFsBucket use this GridFS Bucket * @param filesCollection the current FilesCollection instance * @param fileObj the current file object * @param versionName the current version */ - constructor(filesCollection, fileObj, versionName) { - this.filesCollection = filesCollection; - this.fileObj = fileObj; - this.versionName = versionName; + constructor(gridFsBucket, filesCollection, fileObj, versionName) { + super(gridFsBucket, filesCollection, fileObj, versionName); } /** after successfull upload */ onAfterUpload() { + super.onAfterUpload(); // If the attachment doesn't have a source field or its source is different than import if (!this.fileObj.meta.source || this.fileObj.meta.source !== 'import') { // Add activity about adding the attachment @@ -46,64 +37,15 @@ class AttachmentStoreStrategy { } } - /** download the file - * @param http the current http request - */ - interceptDownload(http) { - } - /** after file remove */ onAfterRemove() { + super.onAfterRemove(); insertActivity(this.fileObj, 'deleteAttachment'); } - - /** returns a read stream - * @return the read stream - */ - getReadStream() { - } - - /** returns a write stream - * @return the write stream - */ - getWriteStream() { - } - - /** writing finished - * @param finishedData the data of the write stream finish event - */ - writeStreamFinished(finishedData) { - } - - /** remove the file */ - unlink() { - } - - /** return the storage name - * @return the storage name - */ - getStorageName() { - } - - static getFileStrategy(filesCollection, fileObj, versionName, storage) { - if (!storage) { - storage = fileObj.versions[versionName].storage || "gridfs"; - } - let ret; - if (["fs", "gridfs"].includes(storage)) { - if (storage == "fs") { - ret = new AttachmentStoreStrategyFilesystem(filesCollection, fileObj, versionName); - } else if (storage == "gridfs") { - ret = new AttachmentStoreStrategyGridFs(filesCollection, fileObj, versionName); - } - } - console.log("getFileStrategy: ", ret.constructor.name); - return ret; - } } -/** Strategy to store attachments at GridFS (MongoDB) */ -class AttachmentStoreStrategyGridFs extends AttachmentStoreStrategy { +/** Strategy to store attachments at filesystem */ +export class AttachmentStoreStrategyFilesystem extends FileStoreStrategyFilesystem { /** constructor * @param filesCollection the current FilesCollection instance @@ -116,131 +58,17 @@ class AttachmentStoreStrategyGridFs extends AttachmentStoreStrategy { /** after successfull upload */ onAfterUpload() { - createOnAfterUpload(this.filesCollection, attachmentBucket, this.fileObj, this.versionName); super.onAfterUpload(); - } - - /** download the file - * @param http the current http request - */ - interceptDownload(http) { - const ret = createInterceptDownload(this.filesCollection, attachmentBucket, this.fileObj, http, this.versionName); - return ret; + // If the attachment doesn't have a source field or its source is different than import + if (!this.fileObj.meta.source || this.fileObj.meta.source !== 'import') { + // Add activity about adding the attachment + insertActivity(this.fileObj, 'addAttachment'); + } } /** after file remove */ onAfterRemove() { - this.unlink(); super.onAfterRemove(); - } - - /** returns a read stream - * @return the read stream - */ - getReadStream() { - const gridFsFileId = (this.fileObj.versions[this.versionName].meta || {}) - .gridFsFileId; - let ret; - if (gridFsFileId) { - const gfsId = createObjectId({ gridFsFileId }); - ret = attachmentBucket.openDownloadStream(gfsId); - } - return ret; - } - - /** returns a write stream - * @return the write stream - */ - getWriteStream() { - const fileObj = this.fileObj; - const versionName = this.versionName; - const metadata = { ...fileObj.meta, versionName, fileId: fileObj._id }; - const ret = attachmentBucket.openUploadStream(this.fileObj.name, { - contentType: fileObj.type || 'binary/octet-stream', - metadata, - }); - return ret; - } - - /** writing finished - * @param finishedData the data of the write stream finish event - */ - writeStreamFinished(finishedData) { - const gridFsFileIdName = this.getGridFsFileIdName(); - Attachments.update({ _id: this.fileObj._id }, { $set: { [gridFsFileIdName]: finishedData._id.toHexString(), } }); - } - - /** remove the file */ - unlink() { - createOnAfterRemove(this.filesCollection, attachmentBucket, this.fileObj, this.versionName); - const gridFsFileIdName = this.getGridFsFileIdName(); - Attachments.update({ _id: this.fileObj._id }, { $unset: { [gridFsFileIdName]: 1 } }); - } - - /** return the storage name - * @return the storage name - */ - getStorageName() { - return "gridfs"; - } - - /** returns the property name of gridFsFileId - * @return the property name of gridFsFileId - */ - getGridFsFileIdName() { - const ret = `versions.${this.versionName}.meta.gridFsFileId`; - return ret; + insertActivity(this.fileObj, 'deleteAttachment'); } } - -/** Strategy to store attachments at filesystem */ -class AttachmentStoreStrategyFilesystem extends AttachmentStoreStrategy { - - /** constructor - * @param filesCollection the current FilesCollection instance - * @param fileObj the current file object - * @param versionName the current version - */ - constructor(filesCollection, fileObj, versionName) { - super(filesCollection, fileObj, versionName); - } - - /** returns a read stream - * @return the read stream - */ - getReadStream() { - const ret = fs.createReadStream(this.fileObj.versions[this.versionName].path) - return ret; - } - - /** returns a write stream - * @return the write stream - */ - getWriteStream() { - const newFileName = this.fileObj.name; - const filePath = this.fileObj.versions[this.versionName].path; - const ret = fs.createWriteStream(filePath); - return ret; - } - - /** writing finished - * @param finishedData the data of the write stream finish event - */ - writeStreamFinished(finishedData) { - } - - /** remove the file */ - unlink() { - const filePath = this.fileObj.versions[this.versionName].path; - fs.unlink(filePath, () => {}); - } - - /** return the storage name - * @return the storage name - */ - getStorageName() { - return "fs"; - } -} - -export default AttachmentStoreStrategy; diff --git a/models/lib/fileStoreStrategy.js b/models/lib/fileStoreStrategy.js new file mode 100644 index 000000000..ff8cae7a2 --- /dev/null +++ b/models/lib/fileStoreStrategy.js @@ -0,0 +1,277 @@ +import fs from 'fs'; +import { createObjectId } from './grid/createObjectId'; +import { createOnAfterUpload } from './fsHooks/createOnAfterUpload'; +import { createInterceptDownload } from './fsHooks/createInterceptDownload'; +import { createOnAfterRemove } from './fsHooks/createOnAfterRemove'; + +/** Factory for FileStoreStrategy */ +export default class FileStoreStrategyFactory { + + /** constructor + * @param classFileStoreStrategyFilesystem use this strategy for filesystem storage + * @param classFileStoreStrategyGridFs use this strategy for GridFS storage + * @param gridFsBucket use this GridFS Bucket as GridFS Storage + */ + constructor(classFileStoreStrategyFilesystem, classFileStoreStrategyGridFs, gridFsBucket) { + this.classFileStoreStrategyFilesystem = classFileStoreStrategyFilesystem; + this.classFileStoreStrategyGridFs = classFileStoreStrategyGridFs; + this.gridFsBucket = gridFsBucket; + } + + /** returns the right FileStoreStrategy + * @param filesCollection the current FilesCollection instance + * @param fileObj the current file object + * @param versionName the current version + * @param use this storage, or if not set, get the storage from fileObj + */ + getFileStrategy(filesCollection, fileObj, versionName, storage) { + if (!storage) { + storage = fileObj.versions[versionName].storage || "gridfs"; + } + let ret; + if (["fs", "gridfs"].includes(storage)) { + if (storage == "fs") { + ret = new this.classFileStoreStrategyFilesystem(filesCollection, fileObj, versionName); + } else if (storage == "gridfs") { + ret = new this.classFileStoreStrategyGridFs(this.gridFsBucket, filesCollection, fileObj, versionName); + } + } + return ret; + } +} + +/** Strategy to store files */ +class FileStoreStrategy { + + /** constructor + * @param filesCollection the current FilesCollection instance + * @param fileObj the current file object + * @param versionName the current version + */ + constructor(filesCollection, fileObj, versionName) { + this.filesCollection = filesCollection; + this.fileObj = fileObj; + this.versionName = versionName; + } + + /** after successfull upload */ + onAfterUpload() { + } + + /** download the file + * @param http the current http request + */ + interceptDownload(http) { + } + + /** after file remove */ + onAfterRemove() { + } + + /** returns a read stream + * @return the read stream + */ + getReadStream() { + } + + /** returns a write stream + * @return the write stream + */ + getWriteStream() { + } + + /** writing finished + * @param finishedData the data of the write stream finish event + */ + writeStreamFinished(finishedData) { + } + + /** remove the file */ + unlink() { + } + + /** return the storage name + * @return the storage name + */ + getStorageName() { + } +} + +/** Strategy to store attachments at GridFS (MongoDB) */ +export class FileStoreStrategyGridFs extends FileStoreStrategy { + + /** constructor + * @param gridFsBucket use this GridFS Bucket + * @param filesCollection the current FilesCollection instance + * @param fileObj the current file object + * @param versionName the current version + */ + constructor(gridFsBucket, filesCollection, fileObj, versionName) { + super(filesCollection, fileObj, versionName); + this.gridFsBucket = gridFsBucket; + } + + /** after successfull upload */ + onAfterUpload() { + createOnAfterUpload(this.filesCollection, this.gridFsBucket, this.fileObj, this.versionName); + super.onAfterUpload(); + } + + /** download the file + * @param http the current http request + */ + interceptDownload(http) { + const ret = createInterceptDownload(this.filesCollection, this.gridFsBucket, this.fileObj, http, this.versionName); + return ret; + } + + /** after file remove */ + onAfterRemove() { + this.unlink(); + super.onAfterRemove(); + } + + /** returns a read stream + * @return the read stream + */ + getReadStream() { + const gridFsFileId = (this.fileObj.versions[this.versionName].meta || {}) + .gridFsFileId; + let ret; + if (gridFsFileId) { + const gfsId = createObjectId({ gridFsFileId }); + ret = this.gridFsBucket.openDownloadStream(gfsId); + } + return ret; + } + + /** returns a write stream + * @return the write stream + */ + getWriteStream() { + const fileObj = this.fileObj; + const versionName = this.versionName; + const metadata = { ...fileObj.meta, versionName, fileId: fileObj._id }; + const ret = this.gridFsBucket.openUploadStream(this.fileObj.name, { + contentType: fileObj.type || 'binary/octet-stream', + metadata, + }); + return ret; + } + + /** writing finished + * @param finishedData the data of the write stream finish event + */ + writeStreamFinished(finishedData) { + const gridFsFileIdName = this.getGridFsFileIdName(); + Attachments.update({ _id: this.fileObj._id }, { $set: { [gridFsFileIdName]: finishedData._id.toHexString(), } }); + } + + /** remove the file */ + unlink() { + createOnAfterRemove(this.filesCollection, this.gridFsBucket, this.fileObj, this.versionName); + const gridFsFileIdName = this.getGridFsFileIdName(); + Attachments.update({ _id: this.fileObj._id }, { $unset: { [gridFsFileIdName]: 1 } }); + } + + /** return the storage name + * @return the storage name + */ + getStorageName() { + return "gridfs"; + } + + /** returns the property name of gridFsFileId + * @return the property name of gridFsFileId + */ + getGridFsFileIdName() { + const ret = `versions.${this.versionName}.meta.gridFsFileId`; + return ret; + } +} + +/** Strategy to store attachments at filesystem */ +export class FileStoreStrategyFilesystem extends FileStoreStrategy { + + /** constructor + * @param filesCollection the current FilesCollection instance + * @param fileObj the current file object + * @param versionName the current version + */ + constructor(filesCollection, fileObj, versionName) { + super(filesCollection, fileObj, versionName); + } + + /** returns a read stream + * @return the read stream + */ + getReadStream() { + const ret = fs.createReadStream(this.fileObj.versions[this.versionName].path) + return ret; + } + + /** returns a write stream + * @return the write stream + */ + getWriteStream() { + const filePath = this.fileObj.versions[this.versionName].path; + const ret = fs.createWriteStream(filePath); + return ret; + } + + /** writing finished + * @param finishedData the data of the write stream finish event + */ + writeStreamFinished(finishedData) { + } + + /** remove the file */ + unlink() { + const filePath = this.fileObj.versions[this.versionName].path; + fs.unlink(filePath, () => {}); + } + + /** return the storage name + * @return the storage name + */ + getStorageName() { + return "fs"; + } +} + +/** move the fileObj to another storage + * @param fileObj move this fileObj to another storage + * @param storageDestination the storage destination (fs or gridfs) + * @param fileStoreStrategyFactory get FileStoreStrategy from this factory + */ +export const moveToStorage = function(fileObj, storageDestination, fileStoreStrategyFactory) { + Object.keys(fileObj.versions).forEach(versionName => { + const strategyRead = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName); + const strategyWrite = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName, storageDestination); + + if (strategyRead.constructor.name != strategyWrite.constructor.name) { + const readStream = strategyRead.getReadStream(); + const writeStream = strategyWrite.getWriteStream(); + + writeStream.on('error', error => { + console.error('[writeStream error]: ', error, fileObjId); + }); + + readStream.on('error', error => { + console.error('[readStream error]: ', error, fileObjId); + }); + + writeStream.on('finish', Meteor.bindEnvironment((finishedData) => { + strategyWrite.writeStreamFinished(finishedData); + })); + + // https://forums.meteor.com/t/meteor-code-must-always-run-within-a-fiber-try-wrapping-callbacks-that-you-pass-to-non-meteor-libraries-with-meteor-bindenvironmen/40099/8 + readStream.on('end', Meteor.bindEnvironment(() => { + Attachments.update({ _id: fileObj._id }, { $set: { [`versions.${versionName}.storage`]: strategyWrite.getStorageName() } }); + strategyRead.unlink(); + })); + + readStream.pipe(writeStream); + } + }); +}; From d63f1b740cfdea7f051ce1ef2316a2338d8b04a1 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 3 Apr 2022 15:09:20 +0200 Subject: [PATCH 09/29] Avatars are using the FileStoreStrategyFactory too --- models/avatars.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/models/avatars.js b/models/avatars.js index 7e9fe0e05..30c539792 100644 --- a/models/avatars.js +++ b/models/avatars.js @@ -1,17 +1,17 @@ import { Meteor } from 'meteor/meteor'; import { FilesCollection } from 'meteor/ostrio:files'; +import { createBucket } from './lib/grid/createBucket'; import fs from 'fs'; import path from 'path'; -import { createBucket } from './lib/grid/createBucket'; -import { createOnAfterUpload } from './lib/fsHooks/createOnAfterUpload'; -import { createInterceptDownload } from './lib/fsHooks/createInterceptDownload'; -import { createOnAfterRemove } from './lib/fsHooks/createOnAfterRemove'; +import FileStoreStrategyFactory, { FileStoreStrategyFilesystem, FileStoreStrategyGridFs} from '/models/lib/fileStoreStrategy'; let avatarsBucket; if (Meteor.isServer) { avatarsBucket = createBucket('avatars'); } +const fileStoreStrategyFactory = new FileStoreStrategyFactory(FileStoreStrategyFilesystem, FileStoreStrategyGridFs, avatarsBucket); + Avatars = new FilesCollection({ debug: false, // Change to `true` for debugging collectionName: 'avatars', @@ -30,17 +30,17 @@ Avatars = new FilesCollection({ }, onAfterUpload(fileObj) { Object.keys(fileObj.versions).forEach(versionName => { - createOnAfterUpload(this, avatarsBucket, fileObj, versionName); + fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).onAfterUpload(); }); }, interceptDownload(http, fileObj, versionName) { - const ret = createInterceptDownload(this, avatarsBucket, fileObj, http, versionName); + const ret = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).interceptDownload(http); return ret; }, onAfterRemove(files) { files.forEach(fileObj => { Object.keys(fileObj.versions).forEach(versionName => { - createOnAfterRemove(this, avatarsBucket, fileObj, versionName); + fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).onAfterRemove(); }); }); }, From 927f155078ad64a42ed8e37937616562c58fe32d Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 3 Apr 2022 15:14:38 +0200 Subject: [PATCH 10/29] WRITABLE_PATH is mandatory, so simplify the code logic --- models/attachments.js | 6 ++---- models/avatars.js | 6 ++---- server/migrations.js | 6 ++---- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/models/attachments.js b/models/attachments.js index 1465092fe..e3558d448 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -28,10 +28,8 @@ Attachments = new FilesCollection({ return ret; }, storagePath() { - if (process.env.WRITABLE_PATH) { - return path.join(process.env.WRITABLE_PATH, 'uploads', 'attachments'); - } - return path.normalize(`assets/app/uploads/${this.collectionName}`); + const ret = path.join(process.env.WRITABLE_PATH, 'attachments'); + return ret; }, onAfterUpload(fileObj) { Object.keys(fileObj.versions).forEach(versionName => { diff --git a/models/avatars.js b/models/avatars.js index 30c539792..49cb10bfb 100644 --- a/models/avatars.js +++ b/models/avatars.js @@ -17,10 +17,8 @@ Avatars = new FilesCollection({ collectionName: 'avatars', allowClientCode: true, storagePath() { - if (process.env.WRITABLE_PATH) { - return path.join(process.env.WRITABLE_PATH, 'uploads', 'avatars'); - } - return path.normalize(`assets/app/uploads/${this.collectionName}`);; + const ret = path.join(process.env.WRITABLE_PATH, 'avatars'); + return ret; }, onBeforeUpload(file) { if (file.size <= 72000 && file.type.startsWith('image/')) { diff --git a/server/migrations.js b/server/migrations.js index bf26c8f55..a5f348239 100644 --- a/server/migrations.js +++ b/server/migrations.js @@ -1178,8 +1178,7 @@ Migrations.add('add-card-details-show-lists', () => { }); Migrations.add('migrate-attachments-collectionFS-to-ostrioFiles', () => { - //const storagePath = Attachments.storagePath(); - const storagePath = process.env.WRITABLE_PATH || `./wekan-uploads`; + const storagePath = Attachments.storagePath(); if (!fs.existsSync(storagePath)) { console.log("create storagePath because it doesn't exist: " + storagePath); fs.mkdirSync(storagePath, { recursive: true }); @@ -1246,8 +1245,7 @@ Migrations.add('migrate-attachments-collectionFS-to-ostrioFiles', () => { }); Migrations.add('migrate-avatars-collectionFS-to-ostrioFiles', () => { - //const storagePath = Avatars.storagePath(); - const storagePath = process.env.WRITABLE_PATH || `./wekan-uploads`; + const storagePath = Avatars.storagePath(); if (!fs.existsSync(storagePath)) { console.log("create storagePath because it doesn't exist: " + storagePath); fs.mkdirSync(storagePath, { recursive: true }); From fe018225b41b6983d5bc7101d3efb37f3cd281bb Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 3 Apr 2022 23:53:18 +0200 Subject: [PATCH 11/29] Attachment property "meta.source" had wrong value --- server/migrations.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/server/migrations.js b/server/migrations.js index a5f348239..55f3eecb3 100644 --- a/server/migrations.js +++ b/server/migrations.js @@ -1332,3 +1332,12 @@ Migrations.add('migrate-attachment-drop-index-cardId', () => { } catch (error) { } }); + +Migrations.add('migrate-attachment-migration-fix-source-import', () => { + // there was an error at first versions, so source was import, instead of import + Attachments.update( + {"meta.source":"import,"}, + {$set:{"meta.source":"import"}}, + noValidateMulti + ); +}); From 9ef45a75aff179b04d590eb95720b3ad3e00444b Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 3 Apr 2022 23:44:02 +0200 Subject: [PATCH 12/29] removed createOnAfterUpload file and use existing code for initial file move to GridFS --- models/attachments.js | 7 ++-- models/avatars.js | 4 ++- models/lib/fileStoreStrategy.js | 18 +++++----- models/lib/fsHooks/createOnAfterUpload.js | 42 ----------------------- 4 files changed, 18 insertions(+), 53 deletions(-) delete mode 100644 models/lib/fsHooks/createOnAfterUpload.js diff --git a/models/attachments.js b/models/attachments.js index e3558d448..8fdc99d53 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -32,9 +32,12 @@ Attachments = new FilesCollection({ return ret; }, onAfterUpload(fileObj) { + // current storage is the filesystem, update object and database Object.keys(fileObj.versions).forEach(versionName => { - fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).onAfterUpload(); - }) + fileObj.versions[versionName].storage = "fs"; + }); + Attachments.update({ _id: fileObj._id }, { $set: { "versions" : fileObj.versions } }); + moveToStorage(fileObj, "gridfs", fileStoreStrategyFactory); }, interceptDownload(http, fileObj, versionName) { const ret = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).interceptDownload(http); diff --git a/models/avatars.js b/models/avatars.js index 49cb10bfb..e179da244 100644 --- a/models/avatars.js +++ b/models/avatars.js @@ -27,9 +27,11 @@ Avatars = new FilesCollection({ return 'avatar-too-big'; }, onAfterUpload(fileObj) { + // current storage is the filesystem, update object and database Object.keys(fileObj.versions).forEach(versionName => { - fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).onAfterUpload(); + fileObj.versions[versionName].storage = "fs"; }); + Avatars.update({ _id: fileObj._id }, { $set: { "versions" : fileObj.versions } }); }, interceptDownload(http, fileObj, versionName) { const ret = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).interceptDownload(http); diff --git a/models/lib/fileStoreStrategy.js b/models/lib/fileStoreStrategy.js index ff8cae7a2..c4d0cfe47 100644 --- a/models/lib/fileStoreStrategy.js +++ b/models/lib/fileStoreStrategy.js @@ -1,6 +1,5 @@ import fs from 'fs'; import { createObjectId } from './grid/createObjectId'; -import { createOnAfterUpload } from './fsHooks/createOnAfterUpload'; import { createInterceptDownload } from './fsHooks/createInterceptDownload'; import { createOnAfterRemove } from './fsHooks/createOnAfterRemove'; @@ -26,7 +25,16 @@ export default class FileStoreStrategyFactory { */ getFileStrategy(filesCollection, fileObj, versionName, storage) { if (!storage) { - storage = fileObj.versions[versionName].storage || "gridfs"; + storage = fileObj.versions[versionName].storage; + if (!storage) { + if (fileObj.meta.source == "import") { + // uploaded by import, so it's in GridFS (MongoDB) + storage = "gridfs"; + } else { + // newly uploaded, so it's at the filesystem + storage = "fs"; + } + } } let ret; if (["fs", "gridfs"].includes(storage)) { @@ -111,12 +119,6 @@ export class FileStoreStrategyGridFs extends FileStoreStrategy { this.gridFsBucket = gridFsBucket; } - /** after successfull upload */ - onAfterUpload() { - createOnAfterUpload(this.filesCollection, this.gridFsBucket, this.fileObj, this.versionName); - super.onAfterUpload(); - } - /** download the file * @param http the current http request */ diff --git a/models/lib/fsHooks/createOnAfterUpload.js b/models/lib/fsHooks/createOnAfterUpload.js deleted file mode 100644 index 173d02f3c..000000000 --- a/models/lib/fsHooks/createOnAfterUpload.js +++ /dev/null @@ -1,42 +0,0 @@ -import { Meteor } from 'meteor/meteor'; -import fs from 'fs'; - -export const createOnAfterUpload = function onAfterUpload(filesCollection, bucket, file, versionName) { - const self = filesCollection; - const metadata = { ...file.meta, versionName, fileId: file._id }; - fs.createReadStream(file.versions[versionName].path) - - // this is where we upload the binary to the bucket using bucket.openUploadStream - // see http://mongodb.github.io/node-mongodb-native/3.2/api/GridFSBucket.html#openUploadStream - .pipe( - bucket.openUploadStream(file.name, { - contentType: file.type || 'binary/octet-stream', - metadata, - }), - ) - - // and we unlink the file from the fs on any error - // that occurred during the upload to prevent zombie files - .on('error', err => { - console.error("[createOnAfterUpload error]", err); - self.unlink(self.collection.findOne(file._id), versionName); // Unlink files from FS - }) - - // once we are finished, we attach the gridFS Object id on the - // FilesCollection document's meta section and finally unlink the - // upload file from the filesystem - .on( - 'finish', - Meteor.bindEnvironment(ver => { - const property = `versions.${versionName}.meta.gridFsFileId`; - - self.collection.update(file._id, { - $set: { - [property]: ver._id.toHexString(), - }, - }); - - self.unlink(self.collection.findOne(file._id), versionName); // Unlink files from FS - }), - ); -}; From 99c37bd5219e477024f97757bf831c133e30f8a0 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 3 Apr 2022 23:52:36 +0200 Subject: [PATCH 13/29] removed createOnAfterRemove file and use existing code for remove file from GridFS --- models/lib/fileStoreStrategy.js | 30 ++++++++++++++++++----- models/lib/fsHooks/createOnAfterRemove.js | 13 ---------- 2 files changed, 24 insertions(+), 19 deletions(-) delete mode 100644 models/lib/fsHooks/createOnAfterRemove.js diff --git a/models/lib/fileStoreStrategy.js b/models/lib/fileStoreStrategy.js index c4d0cfe47..c15bd6cc2 100644 --- a/models/lib/fileStoreStrategy.js +++ b/models/lib/fileStoreStrategy.js @@ -1,7 +1,6 @@ import fs from 'fs'; import { createObjectId } from './grid/createObjectId'; import { createInterceptDownload } from './fsHooks/createInterceptDownload'; -import { createOnAfterRemove } from './fsHooks/createOnAfterRemove'; /** Factory for FileStoreStrategy */ export default class FileStoreStrategyFactory { @@ -137,11 +136,9 @@ export class FileStoreStrategyGridFs extends FileStoreStrategy { * @return the read stream */ getReadStream() { - const gridFsFileId = (this.fileObj.versions[this.versionName].meta || {}) - .gridFsFileId; + const gfsId = this.getGridFsObjectId(); let ret; - if (gridFsFileId) { - const gfsId = createObjectId({ gridFsFileId }); + if (gfsId) { ret = this.gridFsBucket.openDownloadStream(gfsId); } return ret; @@ -171,7 +168,15 @@ export class FileStoreStrategyGridFs extends FileStoreStrategy { /** remove the file */ unlink() { - createOnAfterRemove(this.filesCollection, this.gridFsBucket, this.fileObj, this.versionName); + const gfsId = this.getGridFsObjectId(); + if (gfsId) { + this.gridFsBucket.delete(gfsId, err => { + if (err) { + console.error("error on gfs bucket.delete: ", err); + } + }); + } + const gridFsFileIdName = this.getGridFsFileIdName(); Attachments.update({ _id: this.fileObj._id }, { $unset: { [gridFsFileIdName]: 1 } }); } @@ -183,6 +188,19 @@ export class FileStoreStrategyGridFs extends FileStoreStrategy { return "gridfs"; } + /** returns the GridFS Object-Id + * @return the GridFS Object-Id + */ + getGridFsObjectId() { + const gridFsFileId = (this.fileObj.versions[this.versionName].meta || {}) + .gridFsFileId; + let ret; + if (gridFsFileId) { + ret = createObjectId({ gridFsFileId }); + } + return ret; + } + /** returns the property name of gridFsFileId * @return the property name of gridFsFileId */ diff --git a/models/lib/fsHooks/createOnAfterRemove.js b/models/lib/fsHooks/createOnAfterRemove.js deleted file mode 100644 index 702eaa596..000000000 --- a/models/lib/fsHooks/createOnAfterRemove.js +++ /dev/null @@ -1,13 +0,0 @@ -import { createObjectId } from '../grid/createObjectId'; - -export const createOnAfterRemove = - function onAfterRemove(filesCollection, bucket, file, versionName) { - const gridFsFileId = (file.versions[versionName].meta || {}) - .gridFsFileId; - if (gridFsFileId) { - const gfsId = createObjectId({ gridFsFileId }); - bucket.delete(gfsId, err => { - console.error("error on gfs bucket.delete: ", err); - }); - } - }; From 72b8672e622e893b8e35a2d7e79aecc778203234 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Mon, 4 Apr 2022 00:08:07 +0200 Subject: [PATCH 14/29] Move storage names for filesystem and gridfs to constants --- models/attachments.js | 6 +++--- models/lib/fileStoreStrategy.js | 17 ++++++++++------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/models/attachments.js b/models/attachments.js index 8fdc99d53..2d1000b16 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -4,7 +4,7 @@ import { createBucket } from './lib/grid/createBucket'; import fs from 'fs'; import path from 'path'; import { AttachmentStoreStrategyFilesystem, AttachmentStoreStrategyGridFs} from '/models/lib/attachmentStoreStrategy'; -import FileStoreStrategyFactory, {moveToStorage} from '/models/lib/fileStoreStrategy'; +import FileStoreStrategyFactory, {moveToStorage, STORAGE_NAME_FILESYSTEM, STORAGE_NAME_GRIDFS} from '/models/lib/fileStoreStrategy'; let attachmentBucket; if (Meteor.isServer) { @@ -34,10 +34,10 @@ Attachments = new FilesCollection({ onAfterUpload(fileObj) { // current storage is the filesystem, update object and database Object.keys(fileObj.versions).forEach(versionName => { - fileObj.versions[versionName].storage = "fs"; + fileObj.versions[versionName].storage = STORAGE_NAME_FILESYSTEM; }); Attachments.update({ _id: fileObj._id }, { $set: { "versions" : fileObj.versions } }); - moveToStorage(fileObj, "gridfs", fileStoreStrategyFactory); + moveToStorage(fileObj, STORAGE_NAME_GRIDFS, fileStoreStrategyFactory); }, interceptDownload(http, fileObj, versionName) { const ret = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).interceptDownload(http); diff --git a/models/lib/fileStoreStrategy.js b/models/lib/fileStoreStrategy.js index c15bd6cc2..b49c894e3 100644 --- a/models/lib/fileStoreStrategy.js +++ b/models/lib/fileStoreStrategy.js @@ -2,6 +2,9 @@ import fs from 'fs'; import { createObjectId } from './grid/createObjectId'; import { createInterceptDownload } from './fsHooks/createInterceptDownload'; +export const STORAGE_NAME_FILESYSTEM = "fs"; +export const STORAGE_NAME_GRIDFS = "gridfs"; + /** Factory for FileStoreStrategy */ export default class FileStoreStrategyFactory { @@ -28,18 +31,18 @@ export default class FileStoreStrategyFactory { if (!storage) { if (fileObj.meta.source == "import") { // uploaded by import, so it's in GridFS (MongoDB) - storage = "gridfs"; + storage = STORAGE_NAME_GRIDFS; } else { // newly uploaded, so it's at the filesystem - storage = "fs"; + storage = STORAGE_NAME_FILESYSTEM; } } } let ret; - if (["fs", "gridfs"].includes(storage)) { - if (storage == "fs") { + if ([STORAGE_NAME_FILESYSTEM, STORAGE_NAME_GRIDFS].includes(storage)) { + if (storage == STORAGE_NAME_FILESYSTEM) { ret = new this.classFileStoreStrategyFilesystem(filesCollection, fileObj, versionName); - } else if (storage == "gridfs") { + } else if (storage == STORAGE_NAME_GRIDFS) { ret = new this.classFileStoreStrategyGridFs(this.gridFsBucket, filesCollection, fileObj, versionName); } } @@ -185,7 +188,7 @@ export class FileStoreStrategyGridFs extends FileStoreStrategy { * @return the storage name */ getStorageName() { - return "gridfs"; + return STORAGE_NAME_GRIDFS; } /** returns the GridFS Object-Id @@ -255,7 +258,7 @@ export class FileStoreStrategyFilesystem extends FileStoreStrategy { * @return the storage name */ getStorageName() { - return "fs"; + return STORAGE_NAME_FILESYSTEM; } } From cfb88baa7f8b044c6599dfe4011243333ee858ba Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Tue, 5 Apr 2022 16:31:53 +0200 Subject: [PATCH 15/29] removed createInterceptDownload file and use existing code for http download --- models/lib/fileStoreStrategy.js | 24 ++++++++-- models/lib/fsHooks/createInterceptDownload.js | 47 ------------------- models/lib/httpStream.js | 29 ++++++++++++ 3 files changed, 49 insertions(+), 51 deletions(-) delete mode 100644 models/lib/fsHooks/createInterceptDownload.js create mode 100644 models/lib/httpStream.js diff --git a/models/lib/fileStoreStrategy.js b/models/lib/fileStoreStrategy.js index b49c894e3..22c9cbf18 100644 --- a/models/lib/fileStoreStrategy.js +++ b/models/lib/fileStoreStrategy.js @@ -1,6 +1,6 @@ import fs from 'fs'; import { createObjectId } from './grid/createObjectId'; -import { createInterceptDownload } from './fsHooks/createInterceptDownload'; +import { httpStreamOutput } from './httpStream.js' export const STORAGE_NAME_FILESYSTEM = "fs"; export const STORAGE_NAME_GRIDFS = "gridfs"; @@ -125,7 +125,15 @@ export class FileStoreStrategyGridFs extends FileStoreStrategy { * @param http the current http request */ interceptDownload(http) { - const ret = createInterceptDownload(this.filesCollection, this.gridFsBucket, this.fileObj, http, this.versionName); + const readStream = this.getReadStream(); + const downloadFlag = http?.params?.query?.download; + + let ret = false; + if (readStream) { + ret = true; + httpStreamOutput(readStream, this.fileObj.name, http, downloadFlag, this.filesCollection.cacheControl); + } + return ret; } @@ -195,15 +203,23 @@ export class FileStoreStrategyGridFs extends FileStoreStrategy { * @return the GridFS Object-Id */ getGridFsObjectId() { - const gridFsFileId = (this.fileObj.versions[this.versionName].meta || {}) - .gridFsFileId; let ret; + const gridFsFileId = this.getGridFsFileId(); if (gridFsFileId) { ret = createObjectId({ gridFsFileId }); } return ret; } + /** returns the GridFS Object-Id + * @return the GridFS Object-Id + */ + getGridFsFileId() { + const ret = (this.fileObj.versions[this.versionName].meta || {}) + .gridFsFileId; + return ret; + } + /** returns the property name of gridFsFileId * @return the property name of gridFsFileId */ diff --git a/models/lib/fsHooks/createInterceptDownload.js b/models/lib/fsHooks/createInterceptDownload.js deleted file mode 100644 index 1d094c3f9..000000000 --- a/models/lib/fsHooks/createInterceptDownload.js +++ /dev/null @@ -1,47 +0,0 @@ -import { createObjectId } from '../grid/createObjectId'; - -export const createInterceptDownload = - function interceptDownload(filesCollection, bucket, file, http, versionName) { - const { gridFsFileId } = file.versions[versionName].meta || {}; - if (gridFsFileId) { - // opens the download stream using a given gfs id - // see: http://mongodb.github.io/node-mongodb-native/3.2/api/GridFSBucket.html#openDownloadStream - const gfsId = createObjectId({ gridFsFileId }); - const readStream = bucket.openDownloadStream(gfsId); - - readStream.on('data', data => { - http.response.write(data); - }); - - readStream.on('end', () => { - http.response.end(); // don't pass parameters to end() or it will be attached to the file's binary stream - }); - - readStream.on('error', () => { - // not found probably - // eslint-disable-next-line no-param-reassign - http.response.statusCode = 404; - http.response.end('not found'); - }); - - http.response.setHeader('Cache-Control', filesCollection.cacheControl); - http.response.setHeader( - 'Content-Disposition', - getContentDisposition(file.name, http?.params?.query?.download), - ); - } - return Boolean(gridFsFileId); // Serve file from either GridFS or FS if it wasn't uploaded yet - }; - -/** - * Will initiate download, if links are called with ?download="true" queryparam. - **/ -const getContentDisposition = (name, downloadFlag) => { - const dispositionType = downloadFlag === 'true' ? 'attachment;' : 'inline;'; - - const encodedName = encodeURIComponent(name); - const dispositionName = `filename="${encodedName}"; filename=*UTF-8"${encodedName}";`; - const dispositionEncoding = 'charset=utf-8'; - - return `${dispositionType} ${dispositionName} ${dispositionEncoding}`; -}; diff --git a/models/lib/httpStream.js b/models/lib/httpStream.js new file mode 100644 index 000000000..953d83134 --- /dev/null +++ b/models/lib/httpStream.js @@ -0,0 +1,29 @@ +export const httpStreamOutput = function(readStream, name, http, downloadFlag, cacheControl) { + readStream.on('data', data => { + http.response.write(data); + }); + + readStream.on('end', () => { + // don't pass parameters to end() or it will be attached to the file's binary stream + http.response.end(); + }); + + readStream.on('error', () => { + http.response.statusCode = 404; + http.response.end('not found'); + }); + + http.response.setHeader('Cache-Control', cacheControl); + http.response.setHeader('Content-Disposition', getContentDisposition(name, http?.params?.query?.download)); + }; + +/** will initiate download, if links are called with ?download="true" queryparam */ +const getContentDisposition = (name, downloadFlag) => { + const dispositionType = downloadFlag === 'true' ? 'attachment;' : 'inline;'; + + const encodedName = encodeURIComponent(name); + const dispositionName = `filename="${encodedName}"; filename=*UTF-8"${encodedName}";`; + const dispositionEncoding = 'charset=utf-8'; + + return `${dispositionType} ${dispositionName} ${dispositionEncoding}`; +}; From dace63d4dcfbf995338332c8fb09314d0bd581e7 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Thu, 7 Apr 2022 10:25:56 +0200 Subject: [PATCH 16/29] Files Report works now with Meteor-Files (needed migration to new code) --- client/components/settings/adminReports.jade | 12 +++----- client/components/settings/adminReports.js | 10 ++----- server/publications/attachments.js | 29 ++++++-------------- 3 files changed, 15 insertions(+), 36 deletions(-) diff --git a/client/components/settings/adminReports.jade b/client/components/settings/adminReports.jade index 7a1b8cb96..fded9505a 100644 --- a/client/components/settings/adminReports.jade +++ b/client/components/settings/adminReports.jade @@ -88,18 +88,14 @@ template(name="filesReport") th Filename th.right Size (kB) th MIME Type - th.center Usage - th MD5 Sum th ID each att in results tr - td {{ att.filename }} - td.right {{fileSize att.length }} - td {{ att.contentType }} - td.center {{usageCount att._id.toHexString }} - td {{ att.md5 }} - td {{ att._id.toHexString }} + td {{ att.name }} + td.right {{fileSize att.size }} + td {{ att.type }} + td {{ att._id }} else div {{_ 'no-results' }} diff --git a/client/components/settings/adminReports.js b/client/components/settings/adminReports.js index 6dcbb0fc4..ebeca8dae 100644 --- a/client/components/settings/adminReports.js +++ b/client/components/settings/adminReports.js @@ -1,4 +1,4 @@ -import { AttachmentStorage } from '/models/attachments'; +import Attachments from '/models/attachments'; import { CardSearchPagedComponent } from '/client/lib/cardSearch'; import SessionData from '/models/usersessiondata'; import { QueryParams } from '/config/query-classes'; @@ -103,8 +103,6 @@ class AdminReport extends BlazeComponent { results() { // eslint-disable-next-line no-console - // console.log('attachments:', AttachmentStorage.find()); - // console.log('attachments.count:', AttachmentStorage.find().count()); return this.collection.find(); } @@ -124,10 +122,6 @@ class AdminReport extends BlazeComponent { return Math.round(size / 1024); } - usageCount(key) { - return Attachments.find({ 'copies.attachments.key': key }).count(); - } - abbreviate(text) { if (text.length > 30) { return `${text.substr(0, 29)}...`; @@ -137,7 +131,7 @@ class AdminReport extends BlazeComponent { } (class extends AdminReport { - collection = AttachmentStorage; + collection = Attachments; }.register('filesReport')); (class extends AdminReport { diff --git a/server/publications/attachments.js b/server/publications/attachments.js index 18566f7da..33d4d53e2 100644 --- a/server/publications/attachments.js +++ b/server/publications/attachments.js @@ -1,35 +1,24 @@ -import Attachments, { AttachmentStorage } from '/models/attachments'; +import Attachments from '/models/attachments'; import { ObjectID } from 'bson'; Meteor.publish('attachmentsList', function() { - // eslint-disable-next-line no-console - // console.log('attachments:', AttachmentStorage.find()); - const files = AttachmentStorage.find( + const ret = Attachments.find( {}, { fields: { _id: 1, - filename: 1, - md5: 1, - length: 1, - contentType: 1, - metadata: 1, + name: 1, + size: 1, + type: 1, + meta: 1, }, sort: { - filename: 1, + name: 1, }, limit: 250, }, - ); - const attIds = []; - files.forEach(file => { - attIds.push(file._id._str); - }); - - return [ - files, - Attachments.find({ 'copies.attachments.key': { $in: attIds } }), - ]; + ).cursor; + return ret; }); Meteor.publish('orphanedAttachments', function() { From e75f423edd25b1b151feea70a4ffec4f84c9b58c Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Thu, 7 Apr 2022 11:14:03 +0200 Subject: [PATCH 17/29] Removed "Orphaned Files" from Admin-Reports - not necessary with Meteor-Files --- client/components/settings/adminReports.jade | 28 ----------------- client/components/settings/adminReports.js | 10 ------ i18n/en.i18n.json | 1 - server/publications/attachments.js | 32 -------------------- 4 files changed, 71 deletions(-) diff --git a/client/components/settings/adminReports.jade b/client/components/settings/adminReports.jade index fded9505a..452d2e1b9 100644 --- a/client/components/settings/adminReports.jade +++ b/client/components/settings/adminReports.jade @@ -11,11 +11,6 @@ template(name="adminReports") i.fa.fa-chain-broken | {{_ 'broken-cards'}} - li - a.js-report-files(data-id="report-orphaned-files") - i.fa.fa-paperclip - | {{_ 'orphanedFilesReportTitle'}} - li a.js-report-files(data-id="report-files") i.fa.fa-paperclip @@ -43,8 +38,6 @@ template(name="adminReports") +brokenCardsReport else if showFilesReport.get +filesReport - else if showOrphanedFilesReport.get - +orphanedFilesReport else if showRulesReport.get +rulesReport else if showBoardsReport.get @@ -99,27 +92,6 @@ template(name="filesReport") else div {{_ 'no-results' }} -template(name="orphanedFilesReport") - h1 {{_ 'orphanedFilesReportTitle'}} - if resultsCount - table.table - tr - th Filename - th.right Size (kB) - th MIME Type - th MD5 Sum - th ID - - each att in results - tr - td {{ att.filename }} - td.right {{fileSize att.length }} - td {{ att.contentType }} - td {{ att.md5 }} - td {{ att._id.toHexString }} - else - div {{_ 'no-results' }} - template(name="cardsReport") h1 {{_ 'cardsReportTitle'}} if resultsCount diff --git a/client/components/settings/adminReports.js b/client/components/settings/adminReports.js index ebeca8dae..c1c58692a 100644 --- a/client/components/settings/adminReports.js +++ b/client/components/settings/adminReports.js @@ -25,7 +25,6 @@ BlazeComponent.extendComponent({ { 'click a.js-report-broken': this.switchMenu, 'click a.js-report-files': this.switchMenu, - 'click a.js-report-orphaned-files': this.switchMenu, 'click a.js-report-rules': this.switchMenu, 'click a.js-report-cards': this.switchMenu, 'click a.js-report-boards': this.switchMenu, @@ -65,11 +64,6 @@ BlazeComponent.extendComponent({ this.subscription = Meteor.subscribe('attachmentsList', () => { this.loading.set(false); }); - } else if ('report-orphaned-files' === targetID) { - this.showOrphanedFilesReport.set(true); - this.subscription = Meteor.subscribe('orphanedAttachments', () => { - this.loading.set(false); - }); } else if ('report-rules' === targetID) { this.subscription = Meteor.subscribe('rulesReport', () => { this.showRulesReport.set(true); @@ -134,10 +128,6 @@ class AdminReport extends BlazeComponent { collection = Attachments; }.register('filesReport')); -(class extends AdminReport { - collection = AttachmentStorage; -}.register('orphanedFilesReport')); - (class extends AdminReport { collection = Rules; diff --git a/i18n/en.i18n.json b/i18n/en.i18n.json index cfdf89a5a..639667f6e 100644 --- a/i18n/en.i18n.json +++ b/i18n/en.i18n.json @@ -1086,7 +1086,6 @@ "custom-field-stringtemplate-item-placeholder": "Press enter to add more items", "creator": "Creator", "filesReportTitle": "Files Report", - "orphanedFilesReportTitle": "Orphaned Files Report", "reports": "Reports", "rulesReportTitle": "Rules Report", "boardsReportTitle": "Boards Report", diff --git a/server/publications/attachments.js b/server/publications/attachments.js index 33d4d53e2..fe89cd3c8 100644 --- a/server/publications/attachments.js +++ b/server/publications/attachments.js @@ -20,35 +20,3 @@ Meteor.publish('attachmentsList', function() { ).cursor; return ret; }); - -Meteor.publish('orphanedAttachments', function() { - let keys = []; - - if (Attachments.find({}, { fields: { copies: 1 } }) !== undefined) { - Attachments.find({}, { fields: { copies: 1 } }).forEach(att => { - keys.push(new ObjectID(att.copies.attachments.key)); - }); - keys.sort(); - keys = _.uniq(keys, true); - - return AttachmentStorage.find( - { _id: { $nin: keys } }, - { - fields: { - _id: 1, - filename: 1, - md5: 1, - length: 1, - contentType: 1, - metadata: 1, - }, - sort: { - filename: 1, - }, - limit: 250, - }, - ); - } else { - return []; - } -}); From 9d587e76abb68bef7d7c81e95b213d64e4905920 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Thu, 7 Apr 2022 23:06:16 +0200 Subject: [PATCH 18/29] Remove filesCollection from FileStoreStrategy classes --- models/attachments.js | 4 ++-- models/avatars.js | 4 ++-- models/lib/attachmentStoreStrategy.js | 10 ++++---- models/lib/fileStoreStrategy.js | 33 ++++++++++++--------------- models/lib/httpStream.js | 4 +++- 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/models/attachments.js b/models/attachments.js index 2d1000b16..ecdcdfbb8 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -40,13 +40,13 @@ Attachments = new FilesCollection({ moveToStorage(fileObj, STORAGE_NAME_GRIDFS, fileStoreStrategyFactory); }, interceptDownload(http, fileObj, versionName) { - const ret = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).interceptDownload(http); + const ret = fileStoreStrategyFactory.getFileStrategy(fileObj, versionName).interceptDownload(http, this.cacheControl); return ret; }, onAfterRemove(files) { files.forEach(fileObj => { Object.keys(fileObj.versions).forEach(versionName => { - fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).onAfterRemove(); + fileStoreStrategyFactory.getFileStrategy(fileObj, versionName).onAfterRemove(); }); }); }, diff --git a/models/avatars.js b/models/avatars.js index e179da244..310a41783 100644 --- a/models/avatars.js +++ b/models/avatars.js @@ -34,13 +34,13 @@ Avatars = new FilesCollection({ Avatars.update({ _id: fileObj._id }, { $set: { "versions" : fileObj.versions } }); }, interceptDownload(http, fileObj, versionName) { - const ret = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).interceptDownload(http); + const ret = fileStoreStrategyFactory.getFileStrategy(fileObj, versionName).interceptDownload(http, this.cacheControl); return ret; }, onAfterRemove(files) { files.forEach(fileObj => { Object.keys(fileObj.versions).forEach(versionName => { - fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName).onAfterRemove(); + fileStoreStrategyFactory.getFileStrategy(fileObj, versionName).onAfterRemove(); }); }); }, diff --git a/models/lib/attachmentStoreStrategy.js b/models/lib/attachmentStoreStrategy.js index e525647af..6e7ebb2c8 100644 --- a/models/lib/attachmentStoreStrategy.js +++ b/models/lib/attachmentStoreStrategy.js @@ -19,12 +19,11 @@ export class AttachmentStoreStrategyGridFs extends FileStoreStrategyGridFs { /** constructor * @param gridFsBucket use this GridFS Bucket - * @param filesCollection the current FilesCollection instance * @param fileObj the current file object * @param versionName the current version */ - constructor(gridFsBucket, filesCollection, fileObj, versionName) { - super(gridFsBucket, filesCollection, fileObj, versionName); + constructor(gridFsBucket, fileObj, versionName) { + super(gridFsBucket, fileObj, versionName); } /** after successfull upload */ @@ -48,12 +47,11 @@ export class AttachmentStoreStrategyGridFs extends FileStoreStrategyGridFs { export class AttachmentStoreStrategyFilesystem extends FileStoreStrategyFilesystem { /** constructor - * @param filesCollection the current FilesCollection instance * @param fileObj the current file object * @param versionName the current version */ - constructor(filesCollection, fileObj, versionName) { - super(filesCollection, fileObj, versionName); + constructor(fileObj, versionName) { + super(fileObj, versionName); } /** after successfull upload */ diff --git a/models/lib/fileStoreStrategy.js b/models/lib/fileStoreStrategy.js index 22c9cbf18..661d727b3 100644 --- a/models/lib/fileStoreStrategy.js +++ b/models/lib/fileStoreStrategy.js @@ -20,12 +20,11 @@ export default class FileStoreStrategyFactory { } /** returns the right FileStoreStrategy - * @param filesCollection the current FilesCollection instance * @param fileObj the current file object * @param versionName the current version * @param use this storage, or if not set, get the storage from fileObj */ - getFileStrategy(filesCollection, fileObj, versionName, storage) { + getFileStrategy(fileObj, versionName, storage) { if (!storage) { storage = fileObj.versions[versionName].storage; if (!storage) { @@ -41,9 +40,9 @@ export default class FileStoreStrategyFactory { let ret; if ([STORAGE_NAME_FILESYSTEM, STORAGE_NAME_GRIDFS].includes(storage)) { if (storage == STORAGE_NAME_FILESYSTEM) { - ret = new this.classFileStoreStrategyFilesystem(filesCollection, fileObj, versionName); + ret = new this.classFileStoreStrategyFilesystem(fileObj, versionName); } else if (storage == STORAGE_NAME_GRIDFS) { - ret = new this.classFileStoreStrategyGridFs(this.gridFsBucket, filesCollection, fileObj, versionName); + ret = new this.classFileStoreStrategyGridFs(this.gridFsBucket, fileObj, versionName); } } return ret; @@ -54,12 +53,10 @@ export default class FileStoreStrategyFactory { class FileStoreStrategy { /** constructor - * @param filesCollection the current FilesCollection instance * @param fileObj the current file object * @param versionName the current version */ - constructor(filesCollection, fileObj, versionName) { - this.filesCollection = filesCollection; + constructor(fileObj, versionName) { this.fileObj = fileObj; this.versionName = versionName; } @@ -70,8 +67,9 @@ class FileStoreStrategy { /** download the file * @param http the current http request + * @param cacheControl cacheControl of FilesCollection */ - interceptDownload(http) { + interceptDownload(http, cacheControl) { } /** after file remove */ @@ -112,26 +110,26 @@ export class FileStoreStrategyGridFs extends FileStoreStrategy { /** constructor * @param gridFsBucket use this GridFS Bucket - * @param filesCollection the current FilesCollection instance * @param fileObj the current file object * @param versionName the current version */ - constructor(gridFsBucket, filesCollection, fileObj, versionName) { - super(filesCollection, fileObj, versionName); + constructor(gridFsBucket, fileObj, versionName) { + super(fileObj, versionName); this.gridFsBucket = gridFsBucket; } /** download the file * @param http the current http request + * @param cacheControl cacheControl of FilesCollection */ - interceptDownload(http) { + interceptDownload(http, cacheControl) { const readStream = this.getReadStream(); const downloadFlag = http?.params?.query?.download; let ret = false; if (readStream) { ret = true; - httpStreamOutput(readStream, this.fileObj.name, http, downloadFlag, this.filesCollection.cacheControl); + httpStreamOutput(readStream, this.fileObj.name, http, downloadFlag, cacheControl); } return ret; @@ -233,12 +231,11 @@ export class FileStoreStrategyGridFs extends FileStoreStrategy { export class FileStoreStrategyFilesystem extends FileStoreStrategy { /** constructor - * @param filesCollection the current FilesCollection instance * @param fileObj the current file object * @param versionName the current version */ - constructor(filesCollection, fileObj, versionName) { - super(filesCollection, fileObj, versionName); + constructor(fileObj, versionName) { + super(fileObj, versionName); } /** returns a read stream @@ -285,8 +282,8 @@ export class FileStoreStrategyFilesystem extends FileStoreStrategy { */ export const moveToStorage = function(fileObj, storageDestination, fileStoreStrategyFactory) { Object.keys(fileObj.versions).forEach(versionName => { - const strategyRead = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName); - const strategyWrite = fileStoreStrategyFactory.getFileStrategy(this, fileObj, versionName, storageDestination); + const strategyRead = fileStoreStrategyFactory.getFileStrategy(fileObj, versionName); + const strategyWrite = fileStoreStrategyFactory.getFileStrategy(fileObj, versionName, storageDestination); if (strategyRead.constructor.name != strategyWrite.constructor.name) { const readStream = strategyRead.getReadStream(); diff --git a/models/lib/httpStream.js b/models/lib/httpStream.js index 953d83134..4b156a9c8 100644 --- a/models/lib/httpStream.js +++ b/models/lib/httpStream.js @@ -13,7 +13,9 @@ export const httpStreamOutput = function(readStream, name, http, downloadFlag, c http.response.end('not found'); }); - http.response.setHeader('Cache-Control', cacheControl); + if (cacheControl) { + http.response.setHeader('Cache-Control', cacheControl); + } http.response.setHeader('Content-Disposition', getContentDisposition(name, http?.params?.query?.download)); }; From a064e03fc76f191f8a89fce71e574fbeaf8b8832 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Fri, 8 Apr 2022 08:52:48 +0200 Subject: [PATCH 19/29] Store file on filesystem renew's the storage path --- models/attachments.js | 8 ++++--- models/avatars.js | 8 ++++--- models/lib/fileStoreStrategy.js | 39 +++++++++++++++++++++++++++------ 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/models/attachments.js b/models/attachments.js index ecdcdfbb8..b89fda85c 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -7,11 +7,13 @@ import { AttachmentStoreStrategyFilesystem, AttachmentStoreStrategyGridFs} from import FileStoreStrategyFactory, {moveToStorage, STORAGE_NAME_FILESYSTEM, STORAGE_NAME_GRIDFS} from '/models/lib/fileStoreStrategy'; let attachmentBucket; +let storagePath; if (Meteor.isServer) { attachmentBucket = createBucket('attachments'); + storagePath = path.join(process.env.WRITABLE_PATH, 'attachments'); } -const fileStoreStrategyFactory = new FileStoreStrategyFactory(AttachmentStoreStrategyFilesystem, AttachmentStoreStrategyGridFs, attachmentBucket); +const fileStoreStrategyFactory = new FileStoreStrategyFactory(AttachmentStoreStrategyFilesystem, storagePath, AttachmentStoreStrategyGridFs, attachmentBucket); // XXX Enforce a schema for the Attachments FilesCollection // see: https://github.com/VeliovGroup/Meteor-Files/wiki/Schema @@ -28,7 +30,7 @@ Attachments = new FilesCollection({ return ret; }, storagePath() { - const ret = path.join(process.env.WRITABLE_PATH, 'attachments'); + const ret = fileStoreStrategyFactory.storagePath; return ret; }, onAfterUpload(fileObj) { @@ -88,7 +90,7 @@ if (Meteor.isServer) { Meteor.startup(() => { Attachments.collection._ensureIndex({ 'meta.cardId': 1 }); - const storagePath = Attachments.storagePath(); + const storagePath = fileStoreStrategyFactory.storagePath; if (!fs.existsSync(storagePath)) { console.log("create storagePath because it doesn't exist: " + storagePath); fs.mkdirSync(storagePath, { recursive: true }); diff --git a/models/avatars.js b/models/avatars.js index 310a41783..49fd4c8af 100644 --- a/models/avatars.js +++ b/models/avatars.js @@ -6,18 +6,20 @@ import path from 'path'; import FileStoreStrategyFactory, { FileStoreStrategyFilesystem, FileStoreStrategyGridFs} from '/models/lib/fileStoreStrategy'; let avatarsBucket; +let storagePath; if (Meteor.isServer) { avatarsBucket = createBucket('avatars'); + storagePath = path.join(process.env.WRITABLE_PATH, 'avatars'); } -const fileStoreStrategyFactory = new FileStoreStrategyFactory(FileStoreStrategyFilesystem, FileStoreStrategyGridFs, avatarsBucket); +const fileStoreStrategyFactory = new FileStoreStrategyFactory(FileStoreStrategyFilesystem, storagePath, FileStoreStrategyGridFs, avatarsBucket); Avatars = new FilesCollection({ debug: false, // Change to `true` for debugging collectionName: 'avatars', allowClientCode: true, storagePath() { - const ret = path.join(process.env.WRITABLE_PATH, 'avatars'); + const ret = fileStoreStrategyFactory.storagePath; return ret; }, onBeforeUpload(file) { @@ -59,7 +61,7 @@ if (Meteor.isServer) { }); Meteor.startup(() => { - const storagePath = Avatars.storagePath(); + const storagePath = fileStoreStrategyFactory.storagePath; if (!fs.existsSync(storagePath)) { console.log("create storagePath because it doesn't exist: " + storagePath); fs.mkdirSync(storagePath, { recursive: true }); diff --git a/models/lib/fileStoreStrategy.js b/models/lib/fileStoreStrategy.js index 661d727b3..d8f0cc15b 100644 --- a/models/lib/fileStoreStrategy.js +++ b/models/lib/fileStoreStrategy.js @@ -1,4 +1,5 @@ import fs from 'fs'; +import path from 'path'; import { createObjectId } from './grid/createObjectId'; import { httpStreamOutput } from './httpStream.js' @@ -10,11 +11,13 @@ export default class FileStoreStrategyFactory { /** constructor * @param classFileStoreStrategyFilesystem use this strategy for filesystem storage + * @param storagePath file storage path * @param classFileStoreStrategyGridFs use this strategy for GridFS storage * @param gridFsBucket use this GridFS Bucket as GridFS Storage */ - constructor(classFileStoreStrategyFilesystem, classFileStoreStrategyGridFs, gridFsBucket) { + constructor(classFileStoreStrategyFilesystem, storagePath, classFileStoreStrategyGridFs, gridFsBucket) { this.classFileStoreStrategyFilesystem = classFileStoreStrategyFilesystem; + this.storagePath = storagePath; this.classFileStoreStrategyGridFs = classFileStoreStrategyGridFs; this.gridFsBucket = gridFsBucket; } @@ -83,9 +86,10 @@ class FileStoreStrategy { } /** returns a write stream + * @param filePath if set, use this path * @return the write stream */ - getWriteStream() { + getWriteStream(filePath) { } /** writing finished @@ -94,6 +98,18 @@ class FileStoreStrategy { writeStreamFinished(finishedData) { } + /** returns the new file path + * @param storagePath use this storage path + * @return the new file path + */ + getNewPath(storagePath, name) { + if (!_.isString(name)) { + name = this.fileObj.name; + } + const ret = path.join(storagePath, this.fileObj._id + "-" + this.versionName + "-" + name); + return ret; + } + /** remove the file */ unlink() { } @@ -154,9 +170,10 @@ export class FileStoreStrategyGridFs extends FileStoreStrategy { } /** returns a write stream + * @param filePath if set, use this path * @return the write stream */ - getWriteStream() { + getWriteStream(filePath) { const fileObj = this.fileObj; const versionName = this.versionName; const metadata = { ...fileObj.meta, versionName, fileId: fileObj._id }; @@ -247,10 +264,13 @@ export class FileStoreStrategyFilesystem extends FileStoreStrategy { } /** returns a write stream + * @param filePath if set, use this path * @return the write stream */ - getWriteStream() { - const filePath = this.fileObj.versions[this.versionName].path; + getWriteStream(filePath) { + if (!_.isString(filePath)) { + filePath = this.fileObj.versions[this.versionName].path; + } const ret = fs.createWriteStream(filePath); return ret; } @@ -287,7 +307,9 @@ export const moveToStorage = function(fileObj, storageDestination, fileStoreStra if (strategyRead.constructor.name != strategyWrite.constructor.name) { const readStream = strategyRead.getReadStream(); - const writeStream = strategyWrite.getWriteStream(); + + const filePath = strategyWrite.getNewPath(fileStoreStrategyFactory.storagePath); + const writeStream = strategyWrite.getWriteStream(filePath); writeStream.on('error', error => { console.error('[writeStream error]: ', error, fileObjId); @@ -303,7 +325,10 @@ export const moveToStorage = function(fileObj, storageDestination, fileStoreStra // https://forums.meteor.com/t/meteor-code-must-always-run-within-a-fiber-try-wrapping-callbacks-that-you-pass-to-non-meteor-libraries-with-meteor-bindenvironmen/40099/8 readStream.on('end', Meteor.bindEnvironment(() => { - Attachments.update({ _id: fileObj._id }, { $set: { [`versions.${versionName}.storage`]: strategyWrite.getStorageName() } }); + Attachments.update({ _id: fileObj._id }, { $set: { + [`versions.${versionName}.storage`]: strategyWrite.getStorageName(), + [`versions.${versionName}.path`]: filePath, + } }); strategyRead.unlink(); })); From b8d14abe0c7d2aac48ad70656f12640b2c177b60 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Fri, 8 Apr 2022 00:27:56 +0200 Subject: [PATCH 20/29] Attachment Management at Admin Console --- client/components/settings/attachments.jade | 81 ++++++++++++ client/components/settings/attachments.js | 123 ++++++++++++++++++ client/components/settings/settingHeader.jade | 4 + config/router.js | 24 ++++ i18n/en.i18n.json | 13 +- models/attachments.js | 2 +- server/publications/attachments.js | 6 +- 7 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 client/components/settings/attachments.jade create mode 100644 client/components/settings/attachments.js diff --git a/client/components/settings/attachments.jade b/client/components/settings/attachments.jade new file mode 100644 index 000000000..e9a3da328 --- /dev/null +++ b/client/components/settings/attachments.jade @@ -0,0 +1,81 @@ +template(name="attachments") + .setting-content.attachments-content + unless currentUser.isAdmin + | {{_ 'error-notAuthorized'}} + else + .content-body + .side-menu + ul + li + a.js-move-attachments(data-id="move-attachments") + i.fa.fa-arrow-right + | {{_ 'attachment-move'}} + + .main-body + if loading.get + +spinner + else if showMoveAttachments.get + +moveAttachments + +template(name="moveAttachments") + .js-move-attachment + button.js-move-all-attachments-to-fs {{_ 'move-all-attachments-to-fs'}} + .js-move-attachment + button.js-move-all-attachments-to-gridfs {{_ 'move-all-attachments-to-gridfs'}} + + each board in getBoardsWithAttachments + +moveBoardAttachments board + +template(name="moveBoardAttachments") + .board-description + table + tr + th {{_ 'board'}} ID + th {{_ 'board-title'}} + tr + td {{ _id }} + td {{ title }} + + .js-move-attachment + button.js-move-all-attachments-of-board-to-fs {{_ 'move-all-attachments-of-board-to-fs'}} + .js-move-attachment + button.js-move-all-attachments-of-board-to-gridfs {{_ 'move-all-attachments-of-board-to-gridfs'}} + + .board-attachments + table + tr + th {{_ 'card'}}-Id + th {{_ 'attachment'}}-Id + th {{_ 'name'}} + th {{_ 'path'}} + th {{_ 'version-name'}} + th {{_ 'size'}} (B) + th GridFsFileId + th {{_ 'storage'}} + th {{_ 'action'}} + + each attachment in attachments + +moveAttachment attachment + +template(name="moveAttachment") + each version in flatVersion + tr + td {{ meta.cardId }} + td {{ _id }} + td {{ name }} + td {{ version.path }} + td {{ version.versionName }} + td {{ version.size }} + td {{ version.meta.gridFsFileId }} + td {{ version.storageName }} + td + if $neq version.storageName "fs" + button.js-move-storage-fs + i.fa.fa-arrow-right + | {{_ 'attachment-move-storage-fs'}} + + if $neq version.storageName "gridfs" + if version.storageName + button.js-move-storage-gridfs + i.fa.fa-arrow-right + | {{_ 'attachment-move-storage-gridfs'}} diff --git a/client/components/settings/attachments.js b/client/components/settings/attachments.js new file mode 100644 index 000000000..c101ffcc7 --- /dev/null +++ b/client/components/settings/attachments.js @@ -0,0 +1,123 @@ +import Attachments, { fileStoreStrategyFactory } from '/models/attachments'; + +BlazeComponent.extendComponent({ + subscription: null, + showMoveAttachments: new ReactiveVar(false), + sessionId: null, + + onCreated() { + this.error = new ReactiveVar(''); + this.loading = new ReactiveVar(false); + }, + + events() { + return [ + { + 'click a.js-move-attachments': this.switchMenu, + }, + ]; + }, + + switchMenu(event) { + const target = $(event.target); + if (!target.hasClass('active')) { + this.loading.set(true); + this.showMoveAttachments.set(false); + if (this.subscription) { + this.subscription.stop(); + } + + $('.side-menu li.active').removeClass('active'); + target.parent().addClass('active'); + const targetID = target.data('id'); + + if ('move-attachments' === targetID) { + this.showMoveAttachments.set(true); + this.subscription = Meteor.subscribe('attachmentsList', () => { + this.loading.set(false); + }); + } + } + }, +}).register('attachments'); + +BlazeComponent.extendComponent({ + getBoardsWithAttachments() { + this.attachments = Attachments.find().get(); + this.attachmentsByBoardId = _.chain(this.attachments) + .groupBy(fileObj => fileObj.meta.boardId) + .value(); + + const ret = Object.keys(this.attachmentsByBoardId) + .map(boardId => { + const boardAttachments = this.attachmentsByBoardId[boardId]; + + _.each(boardAttachments, _attachment => { + _attachment.flatVersion = Object.keys(_attachment.versions) + .map(_versionName => { + const _version = Object.assign(_attachment.versions[_versionName], {"versionName": _versionName}); + _version.storageName = fileStoreStrategyFactory.getFileStrategy(_attachment, _versionName).getStorageName(); + return _version; + }); + }); + const board = Boards.findOne(boardId); + board.attachments = boardAttachments; + return board; + }) + return ret; + }, + getBoardData(boardid) { + const ret = Boards.findOne(boardId); + return ret; + }, + events() { + return [ + { + 'click button.js-move-all-attachments-to-fs'(event) { + this.attachments.forEach(_attachment => { + Meteor.call('moveAttachmentToStorage', _attachment._id, "fs"); + }); + }, + 'click button.js-move-all-attachments-to-gridfs'(event) { + this.attachments.forEach(_attachment => { + Meteor.call('moveAttachmentToStorage', _attachment._id, "gridfs"); + }); + }, + } + ] + } +}).register('moveAttachments'); + +BlazeComponent.extendComponent({ + events() { + return [ + { + 'click button.js-move-all-attachments-of-board-to-fs'(event) { + this.data().attachments.forEach(_attachment => { + Meteor.call('moveAttachmentToStorage', _attachment._id, "fs"); + }); + }, + 'click button.js-move-all-attachments-of-board-to-gridfs'(event) { + this.data().attachments.forEach(_attachment => { + Meteor.call('moveAttachmentToStorage', _attachment._id, "gridfs"); + }); + }, + } + ] + }, +}).register('moveBoardAttachments'); + +BlazeComponent.extendComponent({ + events() { + return [ + { + 'click button.js-move-storage-fs'(event) { + Meteor.call('moveAttachmentToStorage', this.data()._id, "fs"); + }, + 'click button.js-move-storage-gridfs'(event) { + Meteor.call('moveAttachmentToStorage', this.data()._id, "gridfs"); + }, + } + ] + }, +}).register('moveAttachment'); diff --git a/client/components/settings/settingHeader.jade b/client/components/settings/settingHeader.jade index 14de3ab1f..72ed611c8 100644 --- a/client/components/settings/settingHeader.jade +++ b/client/components/settings/settingHeader.jade @@ -16,6 +16,10 @@ template(name="settingHeaderBar") i.fa(class="fa-list") span {{_ 'reports'}} + a.setting-header-btn.informations(href="{{pathFor 'attachments'}}") + i.fa(class="fa-paperclip") + span {{_ 'attachments'}} + a.setting-header-btn.informations(href="{{pathFor 'information'}}") i.fa(class="fa-info-circle") span {{_ 'info'}} diff --git a/config/router.js b/config/router.js index 7e8f78660..43900da69 100644 --- a/config/router.js +++ b/config/router.js @@ -355,6 +355,30 @@ FlowRouter.route('/admin-reports', { }, }); +FlowRouter.route('/attachments', { + name: 'attachments', + triggersEnter: [ + AccountsTemplates.ensureSignedIn, + () => { + Session.set('currentBoard', null); + Session.set('currentList', null); + Session.set('currentCard', null); + Session.set('popupCardId', null); + Session.set('popupCardBoardId', null); + + Filter.reset(); + Session.set('sortBy', ''); + EscapeActions.executeAll(); + }, + ], + action() { + BlazeLayout.render('defaultLayout', { + headerBar: 'settingHeaderBar', + content: 'attachments', + }); + }, +}); + FlowRouter.notFound = { action() { BlazeLayout.render('defaultLayout', { content: 'notFound' }); diff --git a/i18n/en.i18n.json b/i18n/en.i18n.json index 639667f6e..a4f2d204c 100644 --- a/i18n/en.i18n.json +++ b/i18n/en.i18n.json @@ -1166,5 +1166,16 @@ "subtaskActionsPopup-title": "Subtask Actions", "attachmentActionsPopup-title": "Attachment Actions", "attachment-move-storage-fs": "Move attachment to filesystem", - "attachment-move-storage-gridfs": "Move attachment to GridFS" + "attachment-move-storage-gridfs": "Move attachment to GridFS", + "attachment-move": "Move Attachment", + "move-all-attachments-to-fs": "Move all attachments to filesystem", + "move-all-attachments-to-gridfs": "Move all attachments to GridFS", + "move-all-attachments-of-board-to-fs": "Move all attachments of board to filesystem", + "move-all-attachments-of-board-to-gridfs": "Move all attachments of board to GridFS", + "path": "Path", + "version-name": "Version-Name", + "size": "Size", + "storage": "Storage", + "action": "Action", + "board-title": "Board Title" } diff --git a/models/attachments.js b/models/attachments.js index b89fda85c..aaa4f370e 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -13,7 +13,7 @@ if (Meteor.isServer) { storagePath = path.join(process.env.WRITABLE_PATH, 'attachments'); } -const fileStoreStrategyFactory = new FileStoreStrategyFactory(AttachmentStoreStrategyFilesystem, storagePath, AttachmentStoreStrategyGridFs, attachmentBucket); +export const fileStoreStrategyFactory = new FileStoreStrategyFactory(AttachmentStoreStrategyFilesystem, storagePath, AttachmentStoreStrategyGridFs, attachmentBucket); // XXX Enforce a schema for the Attachments FilesCollection // see: https://github.com/VeliovGroup/Meteor-Files/wiki/Schema diff --git a/server/publications/attachments.js b/server/publications/attachments.js index fe89cd3c8..d6eab790c 100644 --- a/server/publications/attachments.js +++ b/server/publications/attachments.js @@ -1,7 +1,7 @@ import Attachments from '/models/attachments'; import { ObjectID } from 'bson'; -Meteor.publish('attachmentsList', function() { +Meteor.publish('attachmentsList', function(limit) { const ret = Attachments.find( {}, { @@ -11,11 +11,13 @@ Meteor.publish('attachmentsList', function() { size: 1, type: 1, meta: 1, + path: 1, + versions: 1, }, sort: { name: 1, }, - limit: 250, + limit: limit, }, ).cursor; return ret; From c6212c7d62484aa0b46b874dad1d91d5a983e470 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Fri, 8 Apr 2022 18:59:38 +0200 Subject: [PATCH 21/29] Initial attachment upload set's now "original" in filename too - later if the file is moving to filesystem, it has "original" in the filename too. --- models/attachments.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/attachments.js b/models/attachments.js index aaa4f370e..bbbf8e596 100644 --- a/models/attachments.js +++ b/models/attachments.js @@ -24,7 +24,7 @@ Attachments = new FilesCollection({ allowClientCode: true, namingFunction(opts) { const filenameWithoutExtension = opts.name.replace(/(.+)\..+/, "$1"); - const ret = opts.meta.fileId + "-" + filenameWithoutExtension; + const ret = opts.meta.fileId + "-original-" + filenameWithoutExtension; // remove fileId from meta, it was only stored there to have this information here in the namingFunction function delete opts.meta.fileId; return ret; From 5c890e4cc3a6c5ae96c8095474fd77c0ea6d82c0 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 10 Apr 2022 11:52:05 +0200 Subject: [PATCH 22/29] Flex-Box Layout of "Move Attachment" buttons --- client/components/settings/attachments.jade | 18 ++++++++++-------- client/components/settings/attachments.styl | 3 +++ 2 files changed, 13 insertions(+), 8 deletions(-) create mode 100644 client/components/settings/attachments.styl diff --git a/client/components/settings/attachments.jade b/client/components/settings/attachments.jade index e9a3da328..114c5666e 100644 --- a/client/components/settings/attachments.jade +++ b/client/components/settings/attachments.jade @@ -18,10 +18,11 @@ template(name="attachments") +moveAttachments template(name="moveAttachments") - .js-move-attachment - button.js-move-all-attachments-to-fs {{_ 'move-all-attachments-to-fs'}} - .js-move-attachment - button.js-move-all-attachments-to-gridfs {{_ 'move-all-attachments-to-gridfs'}} + .move-attachment-buttons + .js-move-attachment + button.js-move-all-attachments-to-fs {{_ 'move-all-attachments-to-fs'}} + .js-move-attachment + button.js-move-all-attachments-to-gridfs {{_ 'move-all-attachments-to-gridfs'}} each board in getBoardsWithAttachments +moveBoardAttachments board @@ -36,10 +37,11 @@ template(name="moveBoardAttachments") td {{ _id }} td {{ title }} - .js-move-attachment - button.js-move-all-attachments-of-board-to-fs {{_ 'move-all-attachments-of-board-to-fs'}} - .js-move-attachment - button.js-move-all-attachments-of-board-to-gridfs {{_ 'move-all-attachments-of-board-to-gridfs'}} + .move-attachment-buttons + .js-move-attachment + button.js-move-all-attachments-of-board-to-fs {{_ 'move-all-attachments-of-board-to-fs'}} + .js-move-attachment + button.js-move-all-attachments-of-board-to-gridfs {{_ 'move-all-attachments-of-board-to-gridfs'}} .board-attachments table diff --git a/client/components/settings/attachments.styl b/client/components/settings/attachments.styl new file mode 100644 index 000000000..39363c152 --- /dev/null +++ b/client/components/settings/attachments.styl @@ -0,0 +1,3 @@ +.move-attachment-buttons + display: flex + gap: 10px From 9367ef8c413c038f01d76da4b9beed0b80f6cd48 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 10 Apr 2022 12:35:24 +0200 Subject: [PATCH 23/29] Admin Panel, use flex "gap" instead of "margin" and "padding" --- client/components/settings/peopleBody.styl | 3 +-- client/components/settings/settingBody.styl | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/client/components/settings/peopleBody.styl b/client/components/settings/peopleBody.styl index 9ab1c1f4b..71fc7cbe8 100644 --- a/client/components/settings/peopleBody.styl +++ b/client/components/settings/peopleBody.styl @@ -22,14 +22,13 @@ table .ext-box-left display: flex; width: 100% + gap: 10px span vertical-align: center; line-height: 34px; - margin-right: 10px; input, button - margin: 0 10px 0 0; padding: 0; button diff --git a/client/components/settings/settingBody.styl b/client/components/settings/settingBody.styl index fed061b76..ad51de926 100644 --- a/client/components/settings/settingBody.styl +++ b/client/components/settings/settingBody.styl @@ -21,6 +21,7 @@ display flex padding-top 15px height 100% + gap: 10px; .side-menu background-color: #f7f7f7; @@ -54,7 +55,6 @@ margin-right: 20px .main-body - padding: 0.1em 1em -webkit-user-select: text // Safari 3.1+ -moz-user-select: text // Firefox 2+ -ms-user-select: text // IE 10+ From a167d8ff3f804420fd9e79633a80dee10b482dc6 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 10 Apr 2022 12:35:40 +0200 Subject: [PATCH 24/29] table width 100% isn't necessary at Admin Panel "People" - also it was set global to Wekan, not really a good practise ... --- client/components/settings/peopleBody.styl | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/components/settings/peopleBody.styl b/client/components/settings/peopleBody.styl index 71fc7cbe8..2d524b336 100644 --- a/client/components/settings/peopleBody.styl +++ b/client/components/settings/peopleBody.styl @@ -2,8 +2,6 @@ overflow: scroll; table - border-collapse: collapse; - width: 100%; color: #000; td, th From 5e567365f3b4e7cc40558105a0bd779efa17ac5b Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 10 Apr 2022 12:37:08 +0200 Subject: [PATCH 25/29] .gitignore, ignore all vim swap files --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index e30d6282f..e159c4fe2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ *~ -*.swp +*.sw* .meteor-spk *.sublime-workspace tmp/ From bd8eda235170a760c456f3447c242f993bdf6a41 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 10 Apr 2022 13:47:44 +0200 Subject: [PATCH 26/29] Admin Panel, right table scrollbar is now visible --- client/components/settings/settingBody.styl | 1 - 1 file changed, 1 deletion(-) diff --git a/client/components/settings/settingBody.styl b/client/components/settings/settingBody.styl index ad51de926..4380b14d7 100644 --- a/client/components/settings/settingBody.styl +++ b/client/components/settings/settingBody.styl @@ -7,7 +7,6 @@ display: flex .setting-content - padding 30px color: #727479 background: #dedede width 100% From d1efd1f493ff529cf14434dc18e5407130c76176 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 10 Apr 2022 18:23:26 +0200 Subject: [PATCH 27/29] Admin Panel, use full screen height --- client/components/settings/adminReports.styl | 3 --- client/components/settings/settingBody.styl | 1 - 2 files changed, 4 deletions(-) delete mode 100644 client/components/settings/adminReports.styl diff --git a/client/components/settings/adminReports.styl b/client/components/settings/adminReports.styl deleted file mode 100644 index 3a5234842..000000000 --- a/client/components/settings/adminReports.styl +++ /dev/null @@ -1,3 +0,0 @@ -.admin-reports-content - height: auto !important - diff --git a/client/components/settings/settingBody.styl b/client/components/settings/settingBody.styl index 4380b14d7..6905f6182 100644 --- a/client/components/settings/settingBody.styl +++ b/client/components/settings/settingBody.styl @@ -10,7 +10,6 @@ color: #727479 background: #dedede width 100% - height calc(100% - 80px) position: absolute; .content-title From f1a52c99c55d3959cb54f6142a50b681308a1338 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 10 Apr 2022 19:48:57 +0200 Subject: [PATCH 28/29] Admin Panel, add seperator line at attachments --- client/components/settings/attachments.jade | 1 + client/components/settings/attachments.styl | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/client/components/settings/attachments.jade b/client/components/settings/attachments.jade index 114c5666e..0c35fb0e4 100644 --- a/client/components/settings/attachments.jade +++ b/client/components/settings/attachments.jade @@ -28,6 +28,7 @@ template(name="moveAttachments") +moveBoardAttachments board template(name="moveBoardAttachments") + hr .board-description table tr diff --git a/client/components/settings/attachments.styl b/client/components/settings/attachments.styl index 39363c152..df3635995 100644 --- a/client/components/settings/attachments.styl +++ b/client/components/settings/attachments.styl @@ -1,3 +1,8 @@ .move-attachment-buttons display: flex gap: 10px + +.attachments-content + hr + height: 0px + border: 1px solid black From b7ab79efefc6a29339346519801ac5dd0b487f12 Mon Sep 17 00:00:00 2001 From: Martin Filser Date: Sun, 10 Apr 2022 19:50:29 +0200 Subject: [PATCH 29/29] Admin Reports, more detailed column header + add boardId and cardId to table --- client/components/settings/adminReports.jade | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/client/components/settings/adminReports.jade b/client/components/settings/adminReports.jade index 452d2e1b9..961980b46 100644 --- a/client/components/settings/adminReports.jade +++ b/client/components/settings/adminReports.jade @@ -57,7 +57,7 @@ template(name="brokenCardsReport") template(name="rulesReport") h1 {{_ 'rulesReportTitle'}} if resultsCount - table.table + table tr th Rule Title th Board Title @@ -76,12 +76,14 @@ template(name="rulesReport") template(name="filesReport") h1 {{_ 'filesReportTitle'}} if resultsCount - table.table + table tr th Filename th.right Size (kB) th MIME Type - th ID + th Attachment ID + th Board ID + th Card ID each att in results tr @@ -89,6 +91,8 @@ template(name="filesReport") td.right {{fileSize att.size }} td {{ att.type }} td {{ att._id }} + td {{ att.meta.boardId }} + td {{ att.meta.cardId }} else div {{_ 'no-results' }}