update, remove and patch many

[daffl]

Sometimes you want to update multiple resources e.g. via REST sending a PUT /todos or DELETE /todos. Supporting this is fairly easy, the question is what method signatures to use. Currently I see two options.

Option 1: *Many service methods

We could add updateMany, patchMany and removeMany (and possibly createMany although it would just be passing an array to create).

var myService = {
  find: function(params, callback) {},
  get: function(id, params, callback) {},
  create: function(data, params, callback) {},
  update: function(id, data, params, callback) {},
  updateMany: function(data, params, callback) {},
  patch: function(id, data, params, callback) {},
  patchMany: function(data, params, callback) {},
  remove: function(id, params, callback) {},
  removeMany: function(params, callback) {},
  setup: function(app) {}
}

• + Backwards compatible
• - Cluttered service interface

Option 2: null values for id

The other option is to pass a null value as an id when operating on multiple resources.

app.use('/todos', {
  remove: function(id, params, callback) {
    if(id === null) {
    }
  }
}

• + Keeps service interface the same
• - Could break backwards compatibility in some cases (although you would most likely just return a notFound error)

[daffl]

Personally I'm currently in favour of option #2 and shipping it with Feathers 2.0.

[marshallswain]

Definitely option 2, and we do need createMany support.

[daffl]

Doesn't createMany just mean that you send an array to create? Which also would be consistent with option 2.

[marshallswain]

Oh, Yes. To be clear, I meant createMany behavior, not that specific name in the API.

[ekryski]

I agree. This should definitely be in core. I had done custom services and hooks to make this happen in previous projects.

[marshallswain]

What would a batch update look like? PUT an array of objects with their ids intact? Same for PATCH?

And for DELETE, an array of ids [ 1, 2, 3 ] and/or an array of objects with ids?

[marshallswain]

What’s needed to make this happen?

• Modify each service adapter to handle creating, updating, patching, & deleting many.
• Modify the callback to send an event for each item in the array.
• Document the adapter.
• Update documentation on creating adapters. Do we even have a published adapter spec?

[daffl]

I was thinking of things like like DELETE /todos?complete=true which would delete all completed todos. But because of our common querying mechanism something like .remove({ id: { $in: [ 2, 3, 5 ] }) should also just work.

Batch update and patch behaviour is more interesting. Probably if you do something like
service.patch(null, { complete: false }, { query: { complete: true } }) it would set all incomplete todos to completed. Maybe, due to how update works, batch updates shouldn't be possible. I don't see a common use case where you'd completely want to batch replace your existing resources with a single one.

[marshallswain]

That sounds good to me. I can't think of a case where batch update would be useful. Patch, sure.

[daffl]

Good point. I didn't even think about sending an event for each item in the array but that's the way to go. So the core changes I think would be neccessary:

1. Update the REST adapter at https://github.com/feathersjs/feathers/blob/master/lib/providers/rest/index.js#L51 to also allow POST, PATCH, PUT and DELETE on baseRoute. This would simply apply the matching service method with the id set to null.
2. Update the event mixin at https://github.com/feathersjs/feathers/blob/master/lib/mixins/event.js#L44 to handle arrays (so if an array comes in, send an event for each item in the array).

I am currently working on making https://github.com/feathersjs/feathers-memory a reference adapter with the supported functionality and documentation so that we can update the other ones accordingly. I think - although core supports it - the officially supported adapters shouldn't support batch update, only batch patch.

[MichaelPlan]

Thanks for that, but there is still no possibility to send an array in body in the delete-Request, to remove more than one document. So "/ DELETE / -> service.remove(null, data, params)" not really works, you just get the same response like with id. Only with null for id.

I guess, you have to change

// Returns the leading parameters for a `get` or `remove` request (the id)
function reqId (req) {
  return [ req.params.id || null  ];
}

to

// Returns the leading parameters for a `get` or `remove` request (the id)
function reqId (req) {
  return [ req.params.id || null, req.body  ];
}

in your wappers.js

And in your feathers-commons/arguments.js some changes in the " getOrRemove(name)"-function are necessary to fix it. What do you think?

[daffl]

I know that the spec doesn't explicitly state that you can or can't send a body for DELETE requests but what does the data you want to send look like in your case?

Our thinking for delete-many was being able to do things like DELETE /todos?complete=true which will set params.query.complete and delete all completed todos. The new database adapters already support this functionality.

[MichaelPlan]

I understand your thinking and in many cases it's completely sufficient, but if you want delete many documents by id, which can't be found by a query, it doesn't work. In this case you need the possibility to send an array with this id's in body. For example, if the user select the documents to delete in a list. Of course, you could fire for every document a delete-Request or write your own delete-route-Function directly with the express router, but it would be more efficient to handle it all in feathers with just one DELETE-Request.

[marshallswain]

You should be able to use {id: {$in: [1,2,3]}} to pass the array of ids you want deleted.

[MichaelPlan]

@marshallswain for an mongoDB/ Mongoose query I would use it for sure, but this doesn't solve the described problem. To use the id's in this case, you have to get them first. I prefer to get the id's in array in the body of a DELETE-Request, but this is currently not possible in feathers. Alternative you could use an own POST-Request with the data in body for deleting more then one document, but it would be better and easier to handle it in a DELETE-Request.

[marshallswain]

All of the feathers adapters will allow you to use that syntax, not just
the mongo-based ones. We have created a common syntax across the adapters.

I prefer to get the id's in array in the body of a DELETE-Request, but
this is currently not possible in feathers.

How would you get the ids in the body of the delete request? What would
that look like?

Is the problem the response? Maybe DELETE should return an array of deleted
ids?
On Wed, Dec 23, 2015 at 8:39 AM MichaelPlan notifications@github.com
wrote:

@marshallswain https://github.com/marshallswain for an mongoDB/
Mongoose query I would use it for sure, but this doesn't solve the
described problem. To use the id's in this case, you have to get them
first. I prefer to get the id's in array in the body of a DELETE-Request,
but this is currently not possible in feathers. Alternative you could use
an own POST-Request with the data in body for deleting more then one
document, but it would be better and easier to handle it in a
DELETE-Request.

—
Reply to this email directly or view it on GitHub
#144 (comment)
.

[MichaelPlan]

How would you get the ids in the body of the delete request?

like a POST-Request. In short: A payload (Body) with the id's in it in a DELETE-Request would be perfect.

That's what I understood in your code with

// DELETE / -> service.remove(null, data, params)

(https://github.com/feathersjs/feathers/blob/master/lib/providers/rest/index.js#L59)

where 'data' would be the payload. Otherwise, it should be

// DELETE / -> service.remove(null, params) 

The response wouldn't be a problem. Thanks for your help.

[daffl]

You're right, that's definitely a copy & paste mistake in the comment. Thanks for pointing that out, I just fixed it. I'm still not sure if we're only talking about the difference of making a request like

DELETE /todos?id[$in]=1&id[$in]=2&id[$in]=3

And sending

DELETE /todos
{id: {$in: [1,2,3]}}

in the body? If you would rather send it in the body you could map it to the parameters in a middleware like this:

app.use('/todos', function(req, res, next) {
  if(req.method === 'DELETE') {
    req.feathers.body = req.body;
  }
  next();
}, {
  remove: function(id, params, callback) {
    if(id === null) {
      doSomething(params.body);
    }
  }
});

[daffl]

A little nicer would be to map the query to the body in a similar middleware like this:

app.use('/todos', function(req, res, next) {
  if(req.method === 'DELETE') {
    req.feathers.query = Object.assign(req.feathers.query || {}, req.body);
  }
  next();
}, {
  remove: function(id, params, callback) {
    if(id === null) {
      doSomething(params.query);
    }
  }
});

Which allows to send any DELETE query also as the body.

[ekryski]

Ya I was/am a bit confused as well. Sorry I'm a bit slow...

If I understand correctly @daffl's solution should work fine for your use case. I'm not entirely sure why you wouldn't want to just send the ids in the query string like so:

DELETE /todos?id[$in]=1&id[$in]=2&id[$in]=3

Are there a lot of ids and the query string is too long? Readability an issue?

Not trying to be a dick, just genuinely trying to understand the problem.

[marshallswain]

And I'm realizing that I was completely ignorant that DELETE requests weren't already in the body. I just assumed they were.

[ekryski]

@daffl your last update, that might be a pretty negligible change in core that would support this. I just want to get a better handle on the use case first. I've never had to send data in the body as part of a delete request.

[MichaelPlan]

@daffl Thanks a lot, that's it. This solution works perfect for me. ;)
@ekryski Yes, I have a lot of ids and every id is very long, so I would have a problem with this long query string in the DELETE-request. So I searched for a better solution. First of all, I thought it was already implemented, because of the comment in the file. But with the solution of @daffl I get the body in my DELETE-Request. Thanks.
@marshallswain no problem :)

Interesting Article about Url-Length:
http://www.boutell.com/newfaq/misc/urllength.html

[ekryski]

@MichaelPlan sounds good. Glad that solved your problem. Thanks for sharing that link. We are aware of the query string limitations but so far have never really run into that problem. (Although we had talked about it in the past)

@daffl Since it isn't explicit in the spec, what are your thoughts on moving that into core? Quickly looking at our core code, I'm inclined to just document how to patch it like above (just not sure where these one off examples should go). Maybe FAQ for now?

[daffl]

I'm glad that worked! I think we can just put it in the FAQ for now.

Also, if anybody is running into other querystring limitations, feathers-batch might be useful because it lets you POST all your methods parameters.

[lock]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue with a link to this issue for related bugs.