Change remove-all syntax.

[marshallswain]

https://github.com/feathersjs/feathers-mongoose/blob/master/src/service.js#L191

The user should be required to pass both null and {} like service.remove(null, {}) in order to remove all records. Since the default query is already set to {}, if an id in the app accidentally gets set to null and the user tries service.remove(id) they just wiped out the entire collection in the database. If you guys agree, I will make the changes.

We should check if the id is null and also check if at least a query object was passed in order to remove.

[ekryski]

@marshallswain makes sense. I would actually prefer that we didn't require people to pass null to denote multiple deletions. I personally feel it is esoteric and not immediately clear. That being said we're not going to change that across all adapters right now so feathers-mongoose should behave the same as all the others.

Can you post a PR with what you think the fix should be?

[beeplin]

yes eric, personally I agree with you that setting id to null to make multiple change is not an ideal design. I prefer what meteor did in minimingo: when the first parameter is a string or objectid, it makes single modification; when the first parameter is an object, it would be taken as a query object and makes multiple modification.

I also agree that since this is related to all adapters, it is not hurry to make decision to change tje design.

[marshallswain]

Yes! That API sounds much better. When do we want to target updating the adapters? I think the sooner the better.

[beeplin]

Since you guys are interested in this, I would like to further suggest separating query from params. From my point of view, query object in find, update, patch and remove is a natural, universal part of database operations, no matter it's in a feathers app or not, but params are customized specifically for feathers app.
So I think it might be better to keep params where it is right now, and move query object out of params, making it a replacement of id in find, update, patch and remove. So the API may look like this:

find(id/query, params [, callback])
get(id, params [, callback]) 
create(data, params [, callback])
update(id/query, data, params [, callback]) 
patch(id/query, data, params [, callback])
remove(id/query, params [, callback]) 

This design would be more compatible with the traditional mongo syntax, and at the same time allows users to specify particular params for feathers, and also be more backward-compatible with our current syntax (by allowing put query into params just like what we do now).

[ekryski]

The @feathersjs/core-team discussed this internally and for the near future it's not feasible. It would introduce breaking changes across a bunch of repos. Although the current syntax is a bit weird, the amount of effort for the little gain is just not worth it right now. We might revisit in 6-12 months time, at which point we'll open an issue in the main repo.