PUT/PATCH don't support mongoose update operations

[smirea]

This should work:

curl -XPUT /api/v1/User -d {"$addToSet": {"friends": "BoB"}}

but because we use document.set instead of document.update we loose the ability to use any of the mongodb update operators

currently the way that request is handled it results in doing the following operation: doc.save({'$addToSet.friends': 'BoB'})

instead of the desired: doc.update({'$addToSet': {'friends': 'BoB}})

What do you think of this proposed solution? not tested yet, but seems to work on my use-cases

[Zertz]

TIL: I honestly was not aware of this method!

Could you post a PR to run our tests?

[smirea]

welp this is a late reply. I haven't had the chance to put that PR up. my patchwork broke a lot of tests since it changes part of the core functionality and I don't have the time atm to look into it. It was more a proof of concept to see how hard/easy would it be to get the functionality

So far I'm working around it by pushing to the list on the client side and sending the new list, but yeah it's not ideal

[Zertz]

By using update, we open the door to a bunch of useful methods, but it becomes rather complex to properly filter fields because we'd have to dig into a bunch of operators such as $addToSet, $push and so on.

While i agree that using save is not ideal, it is far simpler. That said, if someone is willing to commit time into this, you are more than welcome to! Just keep in mind that backward compatibility is important here.

[norpan]

If you decide to change this, please give consideration to the different semantics of PUT and PATCH. PUT is meant to replace a whole document. PATCH on the other hand is meant for just this use case.

There is a very good reason to separate PUT from PATCH. PUT is idempotent, which means that a client can assume that applying a PUT operation twice will give the same result as applying it once. So, while $addToSet happens to be idempotent, operations like $push are not.

[wesratcliff]

Curious if there are any thoughts on what the query/body will look like for nested array updates -- would it adhere to Mongoose operators or something like RFC6902.

[dortzur]

Any update on this?

[b-gran]

This is by no means a complete solution (doesn't handle access-protected routes or routes filtered by any other criteria), but here's what I'm using right now to solve this problem locally:

import _ from 'lodash';
import mongoose from 'mongoose';

function applyUpdateUsingRequestBody (model, getDocumentId) {
    if (!isModel(model)) {
        throw new Error(`Expected model to be a mongoose Model`);
    }

    if (!_.isFunction(getDocumentId) || getDocumentId.length !== 1) {
        throw new Error(`getDocumentId() must be a function of arity 1`);
    }

    return function (req, res, next) {
        const body = req.body;

        if (typeof body !== 'object') {
            return res.status(400).json({ message: 'The request body must be an object' });
        }

        const documentId = getDocumentId(req);

        if (_.isNil(documentId)) {
            return res.status(400).json({ message: 'No document id specified' });
        }

        return model.findOneAndUpdate({ _id: documentId }, body, { new: true }).then(result => {
            return res.status(200).json(result);
        }).catch(err => {
            return next(err);
        });
    }
}

function isModel (model) {
    return Object.getPrototypeOf(model) === mongoose.Model;
}

This function just takes the request body and passes it directly to Model#findOneAndUpdate(). It's very easy to use Document#update() instead, so we still get context filtering.

Currently, I mount this function at PUT /api/v1/model-name/:id/update, i.e.

const router = express.Router()
router.put(
  '/api/v1/model-name/:id/update',
  applyUpdateUsingRequestBody(model, req => req.params.id)
)
restify(router, model, { ... })

To incorporate this type of solution (passing request body to update()) into ERM, we would need to:

1. Decide where to integrate it.
   • Do we change PUT /api/v1/model/:id to pass the request body to Document#update()?
   • Do we create a new endpoint (e.g. /api/v1/model/:id/update) that passes the request to Document#update()?
   • Do we add a configuration parameter (something like passRequestBodyToUpdate: Boolean) that modifies PUT or PATCH /api/v1/model/:id to use Document#update()?
2. Apply access filtering
   • This would involve recursively searching the request body, looking for MongoDB update operators and removing the ones that try to modify properties disallowed according to req.access
3. Find a way to handle population

What do you all think about this? There's also the option of json-patch, and there's been some progress made in #323 .

[Zertz]

I'm not too fond of adding deep endpoints, I think it should be left to the user to create routes that go beyond /model/:id, perhaps in the form of plugins. I know we do it already with /model/:id/shallow, but I don't think it should be in core either. Since we're mostly using plain old middleware, it should be feasible to create separate modules that fill this purpose, especially with all the decoupling work you've done in #314.

That said, I do agree that having a way to use operators such as $addToSet would definitely be useful.

[b-gran]

That's a good point about deep endpoints.

For me, the key feature in opening up all of the MongoDB update operators is actually field deletion, because it's not currently possible to remove a field from a document.

To perform PUT/PATCH ops, we're using

context.findOneAndUpdate(
  {},
  { $set: body },
  { ... }
)

If we update PUT to completely replace the document, we would at least get field deletion via PUT, but we still won't have deletion via PATCH (which will still need to use $set)