FileCollection.remove() on the Client side triggers Meteor find() selector bug when combined with packages that transform Collection documents in certain ways

[RaniereSouza]

Hello, @vsivsi !

Thanks for your hard work, that's a really great package and I'm starting to use it (and I intend to apply it in Production environment).

I'm having a little problem, maybe someone also had the same problem, maybe you've already answered someone about it (couldn't find it in the Issues stack though)... I'm doing .remove() operations on the Client side (by file's _id). The thing is - even with the correct _id (ObjectID), the operation returns the number 0, and despite that, it removes ALL the files from the FileCollection in the database (verified on mongo shell). The Allow/Deny rules are all open (all Allow rules return true, no Deny rules), just for the sake of testing it in Development environment. I'm using Meteor 1.4.1.3 (MongoDB 3.2), and the version 1.3.6 of this package. There's nothing special on this .remove() operation, it's just a simple and plain .remove() operation on the Client side, and yet this error occurs. I'm not understanding it, maybe you could help me understand this problem.

Thanks in advance

[vsivsi]

Hi, this sounds very concerning. I have not received any reports of such behavior in the more than two years this package has been available.

Notably, the sample app uses exactly this method (client side remove) and it does not do what you describe here. See code: https://github.com/vsivsi/meteor-file-sample-app/blob/master/sample.coffee#L102

If you have indeed found a way to easily and accidentally delete all files remotely, that is very serious. Please provide a link to a working minimal reproduction code base and I will look into it quickly once i receive it.

[RaniereSouza]

Thanks for the lightning-fast response! I'll try to make a minimalistic reproduction of the problem right now (maybe I can even find out in the process if that was an actual problem or some logical/structural mistake that I made). I'll keep in touch

[RaniereSouza]

Ok @vsivsi , sorry for taking so long! The error reproduction is in here: https://github.com/RaniereSouza/meteor-fc-cli-rmv-problem (I even added you as collaborator, just in case)

I really hope it's just me making some silly mistake, but I still can't understand what am I doing wrong

[vsivsi]

Updated to fix typos in the reproduction

@RaniereSouza Hi, Okay, I've dug into this a bit and have been able to reproduce what you are describing using the project you provided. Thanks for setting that up for me.

TL;DR Either stop using dburles:collection-helpers or report the bug I describe below to them and wait for a fix...

Gory details:

I've traced the problem to this line in the source code for FileCollection:
https://github.com/vsivsi/meteor-file-collection/blob/master/src/gridFS_server.coffee#L317

In the sample app (and every other project I've encountered) that line of code works as expected, but in your project it does not. For a client-side remove (which must have an _id value), that selector is the whole validated file document. It should only ever return one result (because it has an _id which must be unique.) However, in your project, it returns all of the documents, which of course leads to them all being removed. The rub here is that FileCollection does not implement .find(), Meteor does, via its Mongo package. So it appears that in your app, Mongo.Collection.find() is somehow broken, which FileCollection depends upon, and so it also breaks.

But this only happens in your app, so something must be special. And it is. Your app is not really "minimal" as far as Collections are concerned, because you are using the aldeed:collection2 and dburles:collection-helpers packages, among others. These packages modify (monkey patch/replace) parts of Meteor's Mongo Collection implementation, which FileCollection is partially built on top of. So there can be side-effects.

Just to understand this rather catastrophic bug, I spent another couple of hours trying to narrow down the root cause and it appears that there is some bug in dburles:collection-helpers that causes it to make .find() malfunction when operating on documents structured to use the gridFS schema. Note that this does not appear to have anything to do with FileCollection per se, just the structure of the gridFS .files collection documents that it uses.

Here is the evidence:

Load up your reproduction app and create three new "events" with images using the client UI.

Now, in two other console windows run meteor mongo in one, and meteor shell in the other (each from the application's project root dir).

In the "mongo" window try this:

> db.images.files.find().count()
3
> db.images.files.find(db.images.files.findOne()).count()
1

Now in the "shell" window try this:

> Images.find().count()
3
> Images.find(Images.findOne()).count()
3

Uh oh! Not good!

Let's see if we can repro that on another FileCollection:

// Still in the "shell" console
> testFC = new FileCollection("testFC")
[... object stuff ...]
// Create some empty files
> testFC.insert()
[... object ID ...]
> testFC.insert()
[... object ID ...]
> testFC.insert()
[... object ID ...]
// Now test the behavior of find()
> testFC.find().count()
3
> testFC.find(Images.findOne()).count()
1
// Hmm, works on this FileCollection!
// But wait, what if we define a helper?
> testFC.helpers({ url: () => "http://woot.com"})
// Now what will happen?
> testFC.find().count()
3
> testFC.find(testFC.findOne()).count()
3
// BINGO, something in dburles:collection-helpers is breaking .find() 

I haven't dug into what is going on in that other package, but I will say that all of the nastiest bugs I've encountered in Meteor development have resulted from strange interactions with Atmosphere packages that monkey patch or replace the functionality of Meteor systems packages. And this appears to be another example.

Just to confirm that this doesn't actually require a FileCollection to break (a normal Mongo Collection with the wrong type of document will suffice):

// Keep going in your "shell" console
> testOC = new Mongo.Collection("testOC")
[... object stuff ...]
// Insert the gridFS file documents from testFC
> testFC.find().forEach((d) => testOC.insert(d))
// Now test the behavior of find()
> testOC.find().count()
3
> testOC.find(testOC.findOne()).count()
1
// So far so good, now add a helper to this vanilla Mongo Collection
> testOC.helpers({ url: () => "http://woot.com"})
> testOC.find().count()
3
> testOC.find(testOC.findOne()).count()
3
// And FAIL! FileCollection is not required. 
// dburles:collection-helpers is breaking .find() even for vanilla Mongo Collections!

[vsivsi]

@RaniereSouza Note that there were a bunch of copy-paste typos in the code blocks above, but everything should be good now...

[RaniereSouza]

Oh, I see... WOW, this is really unexpected. I'll look if there's some Issue in the dburles:collection-helpers repository related to this particular problem, and if there's not I'm going to create it.
I actually don't need to use a collection helper in the FileCollection (as in my example app, the collection helper that really comes to be used is the one on the other collection (Events, that holds a foreign key to the respective Image), which returns the URL string to serve the Image on the browser).

Either way, many thanks for the careful look at my particular case! I'll close this topic, as it is a problem that isn't related to vsivsi:file-collection itself, but to it's interaction with another package. As a solution for now, I'll just stop using the collection helper on the FileCollection.

[vsivsi]

Just for completeness, a minimal reproduction of this bug in dburles:collection-helpers without using FileCollection at all is here:

https://github.com/vsivsi/helper-bug-repro

[vsivsi]

It has been confirmed that this is occurring due to a bug in Meteor core. The next version of file-collection will check for and guard against this situation.

Meteor issue: meteor/meteor#8329