Async support

[fracz]

Meteor 2.8+ is rushing to get rid of fiber and replace everything with *Async. Docs: https://guide.meteor.com/2.8-migration.html

Do you plan to support it in the FilesCollection, too? I.e. writeAsync, findOneAsync, forEachAsync.

I don't really understand the consequences of using the current non-async FilesCollection methods in a meteor async method. Nevertheless, it seems to work.

[dr-dimitru]

@fracz this library mostly relies on classic hooks and callbacks approach. Most of the changes will be done in the package internally with a goal to minimize changes within the integration guidelines

[make-github-pseudonymous-again]

@dr-dimitru Not sure I understand this correctly. Do you mean you will not change the current API, but the underlying implementation will change?

@fracz You can always try FilesCollection.collection if you need to use the *Async methods.

[dr-dimitru]

@make-github-pseudonymous-again yes, I don't see many changes in library's API

[bratelefant]

@dr-dimitru Not sure I understand this correctly. Do you mean you will not change the current API, but the underlying implementation will change?

@fracz You can always try FilesCollection.collection if you need to use the *Async methods.

FilesCollection.collection.async[...] is working fine on the server side. However, if I FilesCollection.collection.insertAsync on the client side, permissions aren't as expected and calling FilesCollection.insert from the client yields server side calls on the corresponding __pre_... collection.

[dr-dimitru]

@bratelefant

FilesCollection.collection.insertAsync on the client side, permissions aren't as expected and calling

No changes need on the client, keep using callback API it would remain working as expected

[dr-dimitru]

@bratelefant also there's no .insert() on the server, uploads are meant to work from Client to Server only

[bratelefant]

@bratelefant also there's no .insert() on the server, uploads are meant to work from Client to Server only

That is true, sorry for that confusion. However, using .insert() on the client side yields async warnings when WARN_WHEN_USING_OLD_API=true:

Calling method __pre_MyFilesCollection.insert from old API on server.
   This method will be removed, from the server, in version 3.
   Trace is below:
[object Object]
    at warnUsingOldApi (packages/mongo/collection.js:24:12)
    at ns.Collection.insert (packages/mongo/collection.js:569:5)
    at ns.Collection.Mongo.Collection.<computed> [as insert] (packages/aldeed:collection2/collection2.js:217:19)
    at MethodInvocation.FilesCollection._methods.<computed> (packages/ostrio:files/server.js:868:31)
    at MethodInvocation.methodMap.<computed> (packages/montiapm:agent/lib/hijack/wrap_session.js:190:30)
    at maybeAuditArgumentChecks (packages/ddp-server/livedata_server.js:1902:12)
    at getCurrentMethodInvocationResult (packages/ddp-server/livedata_server.js:772:38)
    at packages/meteor.js:365:18
    at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1389:31)
    at packages/ddp-server/livedata_server.js:791:46
    at new Promise (<anonymous>)
    at Session.method (packages/ddp-server/livedata_server.js:739:23)
    at packages/montiapm:agent/lib/hijack/wrap_session.js:74:38
    at packages/meteor.js:365:18
    at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1389:31)
    at Session.sessionProto.protocol_handlers.method (packages/montiapm:agent/lib/hijack/wrap_session.js:73:44)

Edit: Also, if one calls .addFile on the server side, this seems to call this.collection.insert in a fiber.

[dr-dimitru]

@bratelefant just warnings for now, we will update codebase after fibers phased out completely

[bratelefant]

Also, it would be really nice if in the FilesCollection the value of config.protected could be a function that returns a promise resolving to true or false.

[jankapunkt]

we will update codebase after fibers phased out completely

@bratelefant @dr-dimitru would that imply there is no way to prepare migration until the full 3.0 is out? I think we have already some very sophisticated Alpha Releases (as of today the latest is 3.0.0-alpha.15) and I would like to slowly start migrating Files as well, especially since we do lots of stuff after upload (ffmpeg etc.)

[bratelefant]

I agree, I think it would be really nice to get things going, since there'll be some time needed to do some proper testing files in a staging env, especially if you're using s3 or similar things.

we will update codebase after fibers phased out completely

@bratelefant @dr-dimitru would that imply there is no way to prepare migration until the full 3.0 is out? I think we have already some very sophisticated Alpha Releases (as of today the latest is 3.0.0-alpha.15) and I would like to slowly start migrating Files as well, especially since we do lots of stuff after upload (ffmpeg etc.)

[jankapunkt]

@dr-dimitru you accepting pull requests for this one?

@bratelefant interested in collabroating on a PR for this?

[dr-dimitru]

@jankapunkt sure, if you or someone else ready for this. Willing to help and assist as well

[bratelefant]

@jankapunkt sounds fun ;) I personally always considered meteor-files rather complicated, maybe since I didn't really get it as a beginner... this could be a good opportunity to better understand this package.

[dr-dimitru]

@bratelefant or we go together working on its successor in a simpler implementation 😅

[tylercapx]

@dr-dimitru , i'm running across an async issue using the interceptDownload method when trying to migrate to Meteor 3.0 async calls.

Using async/await below doesn't wait for the return true. Is it possible to add support for async in preparation for Meteor. 3.0.

async interceptDownload(http, fileRef, version) {
  const displayName = await Pictures.findOneAsync(fileRef._id);
   if (fileRef && displayName) {
    // get file from S3 and serve file to client business logic
    return true;
  }
  return false;
}

[dallman2]

@dr-dimitru @jankapunkt @bratelefant is there a PR in the works for this? We're currently migrating to Meteor 3, and file uploads and downloads are core to the product.

[dr-dimitru]

@dallman2 we are working on it today, sorry for the delay and thank you for the ping

[bratelefant]

@dr-dimitru @jankapunkt @bratelefant is there a PR in the works for this? We're currently migrating to Meteor 3, and file uploads and downloads are core to the product.

Sorry, not yet; it's on my list though

[chmelevskij]

Dunno if it's worth own issue or better putting it here.

Found interesting problem with the *Async methods which are using FilesCollection in them.

So we are using react-router-dom v6 loaders and load all the data in there with mdg:validated-method and noticed interesting thing. If we have a even a simple method like this and no publications from that collection whatsoever:

export const getImages = new ValidatedMethod({
  name: 'assets.getImages',
  validate: () => {},
  run() {
    console.log(AssetsCollection.find().fetch())
    return AssetsCollection.find().fetch();
  },
});

Calling console.log returns [] which is expected.

Then if method is called like getImages.call on the client side in the loader, same result empty array.

BUT if the same methods is called like await getImages.callAsync() it returns literally all of the contents of collection.

Tried doing the same with regular collections and it works as expected, no publish, no data out. So suspecting this might be something to do with how files collection is implemented.

This feels like potentially a massive issue once meteor 3 rolls out fully 🫠🫣

[make-github-pseudonymous-again]

@chmelevskij I am not sure I understand your example. If this is a method, then it should return the contents of the collection independently of any publication/subscription. Are you talking about the method simulation on the client or the execution of the method on the server?

[chmelevskij]

Riiiiiiggghhhht.... Ok, nvm me here 🫣 Had fundamental gap in my knowledge here, thanks for pointing it out.

[bratelefant]

I'm working on a fork, added async functions to core.js and cursor.js, currently working on server.js. I'd also like to add protectedAsync to the config, since often you do async mongo queries to check perms or so.

Any hints on testing or linting?

Ok, guess I just give it a shot and come up with PR, for work in progress and as a starting point for discussions. I also added some mocha testing and updated eslint.