search

hwillson / meteor-stub-collections

Stub out Meteor collections with in-memory local collections.
MIT License
24 stars 17 forks source link

Restore produces error on client side with version 1.0.8 #29

Open kpala opened 6 years ago

kpala commented 6 years ago

Hi

After upgrading from 1.0.7 to 1.0.8, I get the following error when running restore on a client side collection:

I20180326-20:52:19.074(3)?   2) ... collection "after all" hook:
I20180326-20:52:19.074(3)?      Not permitted. Untrusted code may only remove documents by ID. [403]
I20180326-20:52:19.074(3)?   Error
I20180326-20:52:19.075(3)?       at throwIfSelectorIsNotId (packages/allow-deny.js?hash=17efd8167bebe1f2018f9c642d8000a549d1fe26:538:11)
I20180326-20:52:19.075(3)?       at FileCollection._callMutatorMethod (packages/allow-deny.js?hash=17efd8167bebe1f2018f9c642d8000a549d1fe26:462:7)
I20180326-20:52:19.075(3)?       at FileCollection.remove (packages/mongo.js?hash=7dae0d2bbf4992c92bbb005435b6ee27a23d13b2:698:19)
I20180326-20:52:19.075(3)?       at FileCollection.<anonymous> (packages/matb33_collection-hooks.js?hash=f1f87bc2e36c884177945d27780c5feac3f558c7:596:25)
I20180326-20:52:19.075(3)?       at FileCollection.collection.(anonymous function) [as remove] (packages/matb33_collection-hooks.js?hash=f1f87bc2e36c884177945d27780c5feac3f558c7:155:21)
I20180326-20:52:19.075(3)?       at Object.publicApi.restore (packages/hwillson_stub-collections.js?hash=78141e0344bdcbbb91197c8d81dee7964dd532d3:97:24)
I20180326-20:52:19.076(3)?       at Context.<anonymous> (app/app.js?hash=1f60b72a619b4be1e22017eff622999fe287c0fb:75355:21)
I20180326-20:52:19.076(3)?       at callFn (packages/practicalmeteor_mocha-core.js?hash=8beea7367d1e32321cbe94d562492c67d7ddc5d2:4359:21)
I20180326-20:52:19.076(3)?       at Hook.Runnable.run (packages/practicalmeteor_mocha-core.js?hash=8beea7367d1e32321cbe94d562492c67d7ddc5d2:4352:7)
I20180326-20:52:19.076(3)?       at next (packages/practicalmeteor_mocha-core.js?hash=8beea7367d1e32321cbe94d562492c67d7ddc5d2:4698:10)

I guess this is caused by localCollection.remove({}); (https://github.com/hwillson/meteor-stub-collections/blob/master/index.js#L35) which tries to clear the collection without providing identifiers.

hwillson commented 6 years ago

Thanks @kpala - I haven't had a chance to look into this yet. Any ideas @coagmano?

coagmano commented 6 years ago

@kpala which FilesCollection are you using?

I was under the impression that collections with a null connection didn't call _callMutatorMethod because they don't have a connection. In a standard Collection, all calls to that function are wrapped in an if (this._isRemoteCollection()) { block here, here and here

The only other call to throwIfSelectorIsNotValid is in _defineMutationMethods here, which shouldn't run because it's wrapped in an if block that checks the truthyness of the connection here

@hwillson Correct me if I'm wrong about allow/deny rules not being called for null/local collections on the client

I suspect the FileCollection is setting a connection in it's constructor, and we use that constructor to build the local collection to preserve custom behaviour. Any other ideas?

coagmano commented 6 years ago

So I went for a google for FileCollections and found that most implementations are not compatible with

  1. File-Collection: vsivsi:file-collection Doesn't pass through the options in the constructor. Also adds a string to the name, so it wouldn't actually stub the collection.

  2. CollectionFS: cfs:standard-packages Doesn't pass through options in constructor. Also adds a string to the name, so it wouldn't actually stub the collection.

  3. Meteor Files: ostrio:files Would need to stub FilesCollection.collection instead of FilesCollection directly. Otherwise should be okay

We could test if the name or connection is not null after initialising localCollection, and issue a warning that an actually local collection could not be created?

kpala commented 6 years ago

In this project I used vsivsi:file-collection.

Sorry should have noticed earlier that according to the logs it's indeed related to the FileCollection.

coagmano commented 6 years ago

In that case, it's likely that StubCollections was never actually stubbing the FileCollection in older versions

@hwillson What are your thoughts on how StubCollections should handle the case where an extended collection doesn't behave in a way that we can force it to use a LocalCollection?

If dealing with constructors like this is too hard, we could resolve the error by providing a method so the client can ask the server to remove from the collection (ie. how xolvio:cleaner does it on the client) on restore.