pass meta to onBeforeUpload

[veliovgroup]

pass meta to onBeforeUpload

[dr-dimitru]

Hi @menelike

1. Have you checked function's context?
2. It's about Server, Client or both?

[menelike]

Thanks for the quick response @dr-dimitru

I use this on the server only where context does not contain the meta object. If the meta data does not validate (based on collection references) I want to permit the upload.

[dr-dimitru]

Implemented in dev branch (see: a265fcf). Could you test on your end?

[menelike]

The solution looks great, the implementation looks valid, now we can predict the document much better.

But apart of that, onBeforeUpload is fired twice (tested on master and dev) and to my surprise, only the second time the meta object exists.

Current master output:

=> Meteor server restarted
I20160519-23:40:47.735(2)? { size: 2308,
I20160519-23:40:47.738(2)?   type: 'text/csv',
I20160519-23:40:47.739(2)?   name: 'foo.csv',
I20160519-23:40:47.739(2)?   ext: 'csv',
I20160519-23:40:47.739(2)?   extension: 'csv',
I20160519-23:40:47.740(2)?   extensionWithDot: '.csv',
I20160519-23:40:47.740(2)?   mime: 'text/csv',
I20160519-23:40:47.740(2)?   'mime-type': 'text/csv' }
I20160519-23:40:47.775(2)? { size: 2308,
I20160519-23:40:47.776(2)?   type: 'text/csv',
I20160519-23:40:47.776(2)?   name: 'foo.csv',
I20160519-23:40:47.777(2)?   ext: 'csv',
I20160519-23:40:47.777(2)?   extension: 'csv',
I20160519-23:40:47.777(2)?   extensionWithDot: '.csv',
I20160519-23:40:47.778(2)?   mime: 'text/csv',
I20160519-23:40:47.778(2)?   'mime-type': 'text/csv' }

Current dev output:

ostrio:files  upgraded from 1.5.3 to 1.5.4

=> Meteor server restarted
I20160519-23:43:58.797(2)? { name: 'foo.csv',
I20160519-23:43:58.804(2)?   extension: 'csv',
I20160519-23:43:58.805(2)?   path: 'assets/app/uploads/FooFiles/WcdLuRfcWEkrKCxWg.csv',
I20160519-23:43:58.805(2)?   meta: {},
I20160519-23:43:58.808(2)?   type: 'text/csv',
I20160519-23:43:58.808(2)?   size: 2308,
I20160519-23:43:58.808(2)?   versions: 
I20160519-23:43:58.808(2)?    { original: 
I20160519-23:43:58.808(2)?       { path: 'assets/app/uploads/FooFiles/WcdLuRfcWEkrKCxWg.csv',
I20160519-23:43:58.809(2)?         size: 2308,
I20160519-23:43:58.809(2)?         type: 'text/csv',
I20160519-23:43:58.809(2)?         extension: 'csv' } },
I20160519-23:43:58.809(2)?   isVideo: false,
I20160519-23:43:58.809(2)?   isAudio: false,
I20160519-23:43:58.809(2)?   isImage: false,
I20160519-23:43:58.810(2)?   isText: true,
I20160519-23:43:58.810(2)?   isJSON: false,
I20160519-23:43:58.810(2)?   _prefix: '0dcd4313d1d69716d073070e28230beb5fbaa875a08b2d4436844bed5283d3f2',
I20160519-23:43:58.810(2)?   _storagePath: 'assets/app/uploads/FooFiles',
I20160519-23:43:58.810(2)?   _downloadRoute: '/cdn/storage',
I20160519-23:43:58.811(2)?   _collectionName: 'FooFiles',
I20160519-23:43:58.811(2)?   _id: 'WcdLuRfcWEkrKCxWg',
I20160519-23:43:58.811(2)?   userId: 'kSc4u7h5a3fjcfukw' }
I20160519-23:43:58.835(2)? { name: 'foo.csv',
I20160519-23:43:58.836(2)?   extension: 'csv',
I20160519-23:43:58.836(2)?   path: 'assets/app/uploads/FooFiles/WcdLuRfcWEkrKCxWg.csv',
I20160519-23:43:58.837(2)?   meta: { fooId: 'pB9fYnRKkhuBEeCP8', username: 'foo' },
I20160519-23:43:58.837(2)?   type: 'text/csv',
I20160519-23:43:58.838(2)?   size: 2308,
I20160519-23:43:58.838(2)?   versions: 
I20160519-23:43:58.838(2)?    { original: 
I20160519-23:43:58.838(2)?       { path: 'assets/app/uploads/FooFiles/WcdLuRfcWEkrKCxWg.csv',
I20160519-23:43:58.839(2)?         size: 2308,
I20160519-23:43:58.839(2)?         type: 'text/csv',
I20160519-23:43:58.839(2)?         extension: 'csv' } },
I20160519-23:43:58.839(2)?   isVideo: false,
I20160519-23:43:58.839(2)?   isAudio: false,
I20160519-23:43:58.839(2)?   isImage: false,
I20160519-23:43:58.840(2)?   isText: true,
I20160519-23:43:58.840(2)?   isJSON: false,
I20160519-23:43:58.840(2)?   _prefix: '0dcd4313d1d69716d073070e28230beb5fbaa875a08b2d4436844bed5283d3f2',
I20160519-23:43:58.840(2)?   _storagePath: 'assets/app/uploads/FooFiles',
I20160519-23:43:58.841(2)?   _downloadRoute: '/cdn/storage',
I20160519-23:43:58.841(2)?   _collectionName: 'FooFiles',
I20160519-23:43:58.841(2)?   _id: 'WcdLuRfcWEkrKCxWg',
I20160519-23:43:58.842(2)?   userId: 'kSc4u7h5a3fjcfukw' }

I'm not sure if this is a bug from my higher logic. From my point of view this should fire only once. Can you reproduce this?

If a new issue is needed here let me know.

[dr-dimitru]

This is expected behaviour, form the docs:

Callback, triggered right before upload is started on client and right after receiving a chunk on server

So, on server it will be triggered as many chunks it received.

only the second time the meta object exists.

This is also expected behaviour, as metadata sent only after last chunk of the file is received. We can pass metadata along with each chunk, but this will cause extra bytes sent over the wire - this is not what we want.

[menelike]

@dr-dimitru sorry for that, totally missed that in the docs. I agree, passing metadata along every chunk isn't the choice in terms of performance.
Is it possible to transfer metadata on the initial request? I need to abort the transfer directly, not after all chunks have been transferred but before. But again, from a developers perspective a single validation run on start would be the best. Maybe onBeforeUpload does not fit here at all. If metadata is missing on all times > 1 the validation becomes inconsistent. On the first run it returns true because it validates true, on every run after it validates false as the metadata got lost.
A function like onInitialUpload (server only) which is fired only once would fit well in this scenario.

[dr-dimitru]

@menelike unfortunately not as we currently can't define "first chunk" as any of them may be first - upload is asynchronous.

---

Please see: 5e77d6a
Now meta is sent with chunk labeled as #1, but after running some tests - I can tell you what in 30% this chunk is not coming first to server, but 100% between chunks 4*streams.

[menelike]

Understood. Hard to solve. I'll try to patch the source and let you know my results within this day. Thanks for your great support!

[dr-dimitru]

@menelike have a good coding! Let me know your results then, maybe we can workaround this somehow, or improve the package.

[menelike]

@dr-dimitru

This is a though one. I exposed Meteor.Collection to subclass insert and invalidate the DB insert.
To be honest, exposing the Collection might be a great way to transform the doc, but this is not the proper way to add validation logic (for example the userId isn't reachable and could be only derived from the doc).

Therefore I'm not sure if a PR is welcome. What do you think?

https://github.com/risetechnologies/Meteor-Files/commit/8296de3df67b35953a3d12c439ddb83b10f403a3 adds CollectionClass as a parameter to FilesCollection.

A simple example to get the idea:

class FooFilesCollection extends Mongo.Collection {
  insert(doc, callback) {
    const newDoc = doc;
    ... // set invalid
     if (invalid) {
      FooFiles.unlink(doc); // this is dirty *circular dependency*
      return callback(new Meteor.Error('fooFiles.insert', 'fooError'));
    }
    return super.insert(newDoc, callback);
  }
}

export const FooFiles = new FilesCollection({
  collectionName: 'FooFiles',
  CollectionClass: FooFilesCollection,
});

As a result, data is always transferred over the wire. If the insert throws an error, the doc won't be saved to the database but isn't deleted from the storage, you would need to manually call unlink too.

Without huge additional efforts I do not see how this could be easily solved. The easiest way would be indeed to add the metadata which might impact performance. a265fcf adds more data than needed, but since I do not see any reliable usability I'd revert that commit.

Maybe a flag which controls that meta is always(true)/never(false) accessible from onBeforeUpload would be the simplest and best solution.

[dr-dimitru]

I believe receiving and checking chunk labeled as #1 (and the last one of course) is enough to validate current upload.

Your suggested approach make it more complex, while I'm trying to keep this package with a little moving parts as possible to leave less weak spots which can be broken.

I agree with add an ability to pass your own Mongo.Collection instance.

Waiting for your point.

[menelike]

@dr-dimitru I fully agree with your approach.

Your implementation resulted in always sending the meta data. In addition the chunkId needs to be accessible on the receiving server part. See PR.

[dr-dimitru]

chunkId is always transferred between Client and Server back and forward, so we can show percentage and catch EOF

[dr-dimitru]

Done in v1.5.4
Thank you @menelike