Meteor-Community-Packages / Meteor-CollectionFS

Reactive file manager for Meteor
MIT License
1.05k stars 237 forks source link

Error on Insert using meteor shark branch and devel-merge #146

Closed ixdi closed 10 years ago

ixdi commented 10 years ago

I'm new to FS.Collection (devel-merge branch). When I try to implement the example code in Readme (create an Image collection and upload file to filesystem) I get the error (fome Chrome console):

Uncaught Error: can only transform documents with _id wrap_transform.js:18 (anonymous function) wraptransform.js:18 (anonymous function) collection-hooks.js:70 (anonymous function) common.js:155 (anonymous function) insert.js:12 .each._.forEach underscore.js:105 (anonymous function) insert.js:11 (anonymous function) collection-hooks.js:64 doInsert api.common.js:20 FS.Collection.insert api.common.js:39

I'm using last shark branch in Meteor.

What's wrong ? Thanks

copleykj commented 10 years ago

I have this too with the most recent updates to shark.. Looks like its due to changes concerning oplog tailing.

raix commented 10 years ago

Bugger, It works fine on 7.0.1 - fails on shark branch but works on the devel branch.

copleykj commented 10 years ago

Yeah this is very recent.. Last pull on my meteor checkout before this was 3 days ago and it was working fine then. On Feb 2, 2014 3:15 AM, "Morten N.O. Nørgaard Henriksen" < notifications@github.com> wrote:

Bugger, It works fine on 7.0.1 - fails on shark branch but works on the devel branch.

Reply to this email directly or view it on GitHubhttps://github.com/CollectionFS/Meteor-CollectionFS/issues/146#issuecomment-33894935 .

raix commented 10 years ago

@aldeed is it possible to deprecate the collection hooks package, We only use this for validation checks? (before insert)

ixdi commented 10 years ago

For now I've avoided this bug creating an _id inside doInsert(), typing:

fileObj._id = ("0000" + (Math.random()*Math.pow(36,4) << 0).toString(36)).substr(-4); (on line 16, 4 char long random _id)

I'm not sure about this as a good way to proceed but I think the problem is with new oplog in minimongo, where transform doesn't accept empty _id.

raix commented 10 years ago

I've tried to set breakpoints - seems this is triggered after files.insert but the collection insert function is not called (transform is called before insert, this part does not make sense)

aldeed commented 10 years ago

We can drop the collection-hooks dependency if we use a deny function instead. That's what collection2 does. I can make that change.

avital commented 10 years ago

It looks like you're setting a transform on a collection with document that don't have _id's. We didn't anticipate this happening. What type of collection is this? (For example, I know that documents in an oplog collection don't have _ids)

raix commented 10 years ago

@avital we use a regular meteor collection and have a transform function mounted. Not sure why, but it seems to transform data from the meteor collection just before it's actually inserted. Not sure how this is triggered.

aldeed commented 10 years ago

I think it might be collection-hooks doing that. Doesn't it pass a transformed doc to before insert hooks? As I mentioned, we could stop using collection-hooks and use a deny function instead.

Someone should try to reproduce with just a collection-hooks before insert hook and not using collectionFS.

aldeed commented 10 years ago

@raix, I removed the collection-hooks dependency. @avital, if it helps, we were calling this.transform() inside a collection-hooks before.insert, so I'm guessing the transform was being applied on inserts before the _id had actually been generated. I'm thinking this should probably be considered an issue with the collection-hooks package, but maybe mongo-livedata or whatever is throwing this error should support transformations without _id if there isn't a good reason not to.

I didn't test, but I'm guessing this error will be gone now.

raix commented 10 years ago

@aldeed Thanks Eric, I'll pull and merge/test

avital commented 10 years ago

We added that check intentionally. Since observe only sees transformed documents, unless we have an _id in the untransformed document there's no way to identify the objects that come out of the transform function. The change was specifically motivated by the implementation of {{#each}} in Blaze (formerly known as the Meteor UI rendering engine/shark) but is generally helpful for other cases.

aldeed commented 10 years ago

That makes sense to me. I'll submit a collection-hooks issue.

aldeed commented 10 years ago

Assuming the original issue here is fixed now?

copleykj commented 10 years ago

Working good on my end.