vsivsi / meteor-file-collection

Extends Meteor Collections to handle file data using MongoDB gridFS.
http://atmospherejs.com/vsivsi/file-collection
Other
159 stars 37 forks source link

Argument Check Error Throw on /fs.files/remove #115

Closed brucejo75 closed 8 years ago

brucejo75 commented 8 years ago

It is recommended practice to check arguments across client/server calls, so I check all arguments across that boundary. To do this I include the following packages in my main application:

audit-argument-checks
check

Repro

Include the following packages:

audit-argument-checks
check

Expected

All Meteor cross client/server arguments to be checked.

Observed

When I include these packages I am getting an argument check error on file removes see Console Output.

The call at this line, seems to cause it: https://github.com/vsivsi/meteor-file-collection/blob/master/src/gridFS_server.coffee#L315

Console Output

 Exception while invoking method '/fs.files/remove' Error: Did not check() all arguments during call to '/fs.files/remove'
    at [object Object]._.extend.throwUnlessAllArgumentsHaveBeenChecked (meteor://💻app/packages/check/match.js:432:1)
    at Object.exports.Match._failIfArgumentsAreNotAllChecked (meteor://💻app/packages/check/match.js:110:1)
    at maybeAuditArgumentChecks (meteor://💻app/packages/ddp-server/livedata_server.js:1701:18)
    at meteor://💻app/packages/ddp-server/livedata_server.js:711:19
    at [object Object]._.extend.withValue (meteor://💻app/packages/meteor/dynamics_nodejs.js:56:1)
    at meteor://💻app/packages/ddp-server/livedata_server.js:709:40
    at [object Object]._.extend.withValue (meteor://💻app/packages/meteor/dynamics_nodejs.js:56:1)
    at meteor://💻app/packages/ddp-server/livedata_server.js:707:46
    at tryCallTwo (C:\Users\admin\AppData\Local\.meteor\packages\promise\0.6.7\npm\node_modules\meteor-promise\node_modules\promise\lib\core.js:45:5)
    at doResolve (C:\Users\admin\AppData\Local\.meteor\packages\promise\0.6.7\npm\node_modules\meteor-promise\node_modules\promise\lib\core.js:200:13)
vsivsi commented 8 years ago

Just updated dev branch at 937e14715ae3c5cc2aa9418eeba3bbb9efb7becc. Please verify this passes your audit test.

brucejo75 commented 8 years ago

Hi @vsivsi,

I would be happy to check this. I am new to git / github, etc. Can you give me instructions on how to build the package local and install it into my meteor app? I have never built a package before.

I have done the following:

  1. git clone https://github.com/vsivsi/meteor-file-collection
  2. git checkout dev
  3. git pull https://github.com/vsivsi/meteor-file-collection dev

So I have a local copy of the dev branch.

Thanks!

vsivsi commented 8 years ago

Within the meteor-file-collection directory, you can run the unit tests (just to validate your install) by running:

meteor test-packages ./

To run the dev branch within your app, you need to move (or re-clone) the package into a directory called packages under your application's root.

So, something like:

cd myAppDirectory
mkdir packages 
cd packages
git clone https://github.com/vsivsi/meteor-file-collection
cd meteor-file-collection
git checkout dev
# the git pull shouldn't be necessary
cd ../..
meteor  # Or whatever you run to start your app / test harness
brucejo75 commented 8 years ago

When I clone meteor-file-collection, the resumable directory is unpopulated. Missing something? I see the resumable code on your repo.

meteor test-packages ./

=> Started proxy.
=> Started MongoDB.
vsivsi:file-collection: updating npm dependencies -- mongodb, gridfs-locking-stream, gridfs-locks, dicer,
async, express, cookie-parser, through2...
=> Errors prevented startup:

   While building package vsivsi:file-collection:
   error: File not found: resumable/resumable.js
vsivsi commented 8 years ago

RIght, sorry, you need to run:

git clone --recursive https://github.com/vsivsi/meteor-file-collection

brucejo75 commented 8 years ago

Works as advertised, please release!

Thanks for walking me through this. Helps me to understand packages a little bit better, would have been much slower without your help.

Steps

Here are the steps that made it work for me. Note 1: the git pull was necessary, I checked the file and verified the code was not updated without it see output below. Either way it does not hurt. Note 2: leave the package in the .meteor/packages file, the build knows to grab the local version.

cd myAppDirectory
mkdir packages 
cd packages
git clone --recursive https://github.com/vsivsi/meteor-file-collection
cd meteor-file-collection
git checkout dev
git pull https://github.com/vsivsi/meteor-file-collection dev
cd ../..
meteor 

-- Output from git pull ...

From https://github.com/vsivsi/meteor-file-collection
 * branch            dev        -> FETCH_HEAD
Updating 1678a79..c480315
Fast-forward
 .travis.yml                       |  1 +
 HISTORY.md                        |  9 ++++---
 package.js                        |  4 +--
 src/gridFS_server.coffee          |  3 +++
 src/http_access_server.coffee     | 11 ++++++++
 test/file_collection_tests.coffee | 57 ++++++++++++++++++++++++++++++++-------
 6 files changed, 70 insertions(+), 15 deletions(-)
vsivsi commented 8 years ago

Just published in v1.3.5