Mongoose: Mongoose Update should update nested object fields separately if arguments provided.

[vadimshvetsov]

I've tried to found something related to my case, but it wasn't obvious, because it's not nested Schema below. If I'm doing wrong will be glad to know that.

Do you want to request a feature or report a bug?

I would like to have a feature with Model.find*Update methods. I can't seek proper way to update nested object properties separately without nested Schema.

I've created Issue at StackOverflow already.

Let's say User Schema presented as below:

var userSchema = new Schema({
  name: {
    first: { type: String, required: true },
    last: { type: String, required: true },
  },
  email: { type: String, required: true, unique: true, lowercase: true },
  password: { type: String, required: true },
  avatar: { type: String },
  isAdmin: { type: Boolean, default: false },
  createdAt: { type: Date, default: Date.now },
});

What is the current behavior?

If I provide in arguments only one property of the name, the second property becomes undefined. So nested name object is fully overwritten during update operation.

describe('User model', function () {
  before(function (done) {
    mongoose.createConnection(db, done);
  });

  var newUser = {
    name: {
      first: 'Foo',
      last: 'Bar',
    },
    email: '[email protected]',
    password: 'foobar',
  };

  var updateUserArgs = {
    name: {
      first: 'Baz',
    },
    email: '[email protected]',
  };

  describe('CRUD', function () {
    it('should save a new user', function (done) {
      new User(newUser).save((err, user) => {
        assert.isOk(user._id);
        if (err) done(err);
        done();
      });
    });

    it('should update user first name and email without removing last name', function (done) {
      User.findOneAndUpdate(
        { email: '[email protected]' }, 
        updateUserArgs,
        { new: true },
        function (err, updatedUser) {
          if (err) done(err);
          assert.equal(updateUserArgs.email, user.email, 'User email should be the same as in update args');
          assert.equal(updateUserArgs.name.first, user.name.first, 'User first name should be the same as in update args');
          assert.equal(newUser.name.last, user.name.last, 'User last name shouldn\'t be affected with updating only first name');
          done();
        });
    });
  });
});

So the current behavior is that it's failing last assertion with following error:

1 failing

  1) User model CRUD should update created user first name and email:
     Uncaught AssertionError: User last name shouldn't be affected with updating only first name: expected 'Bar' to equal undefined

What is the expected behavior?

During update operation the first name must be overwritten with new first name and the last name shouldn't be affected. The third assertion must provide success.

Please mention your node.js, mongoose and MongoDB version.
Node.js - v7.9.0
Mongoose - v4.9.7
MongoDB - v3.2.10

[varunjayaraman]

I can label this a feature request, but i'm pretty sure deep diffing is extremely difficult if not impossible with an update operation. @vkarpov15 thoughts?

[vadimshvetsov]

@varunjayaraman I'm sure that it's not impossible.

Here is a Gist for one nested level object updates.

I think we can do it in while loop for all nested levels.

[varunjayaraman]

I didn't mean that it was impossible to deep diff (obviously libraries like lodash have implemented that). You can manually create the diff that way if you want. What I meant was more:

an update operation doesn't retrieve the entry before sending off the update, and mongodb won't deep diff for you, so the only solution would be to manually deep diff in the way you did it in the gist. The problem is that is a) backwards breaking and b) not necessarily always what a user is going for. It seems like you should manually deep diff and create your $sets when you need it and default to overwriting nested objects. And if you find that cumbersome, a .save() might be more what you're looking for

[vadimshvetsov]

@varunjayaraman sometimes .save() is the best way for models with meaningful pre hooks (password encrypting and so on and so forth). Totally agreed. But anyway update with deep merge is more convenient in my mind than without.

[vkarpov15]

I might be missing something but why can't you just use the below code?

  var updateUserArgs = {
    'name.first': 'Baz',
    email: '[email protected]',
  };

[vadimshvetsov]

@vkarpov15 Sometimes it's not convenient to hardcode all props. If I will extend for example name object it's error prone or I have too much to sort out. I have complex models, it's really cool to rely on framework, for example in update operation.

The second con in this way that every time I need prepare my object to dot notation, and it's overwhelming too. Would be really easy if update function make it for me.

I suppose that would be perfectly for users if it can be handled with some condition on the framework side. For example if object have nested object it can be converted to the dot.notation by mongoose otherwise convert function can be omitted and then processed update operation.

[varunjayaraman]

@vadimshvetsov so how would it handle the cases when people do explicitly want to overwrite the objects instead of deep diffing?

[vadimshvetsov]

@varunjayaraman what the purpose of using update without deep merging? If you have nested object you need to provide every time all fields instead only altered props or it will be overwritten with new object probably even without required props for example. As option there can be a flag to don't use deep merge.

[vkarpov15]

This behavior is enforced at the mongodb layer, not mongoose, so mongoose would have to convert from name: { first: 'Baz' } to 'name.first': 'Baz' for you, and I'm not sure how that API would work. It isn't too difficult to convert all nested paths in an obj into dot notation though

[vadimshvetsov]

@vkarpov15 it would be very handy to have this within framework logic in my opinion. I can try to implement this if feature request would be approved.

[vkarpov15]

You could pretty easily implement this in a plugin, just attach pre('update') and pre('findOneAndUpdate') hooks to do this conversion. I'm hesitant to put this functionality into the core framework unless there's overwhelming demand for it because this is a fundamental mongodb behavior and overwriting it will cause confusion.

[vadimshvetsov]

It sounds like a really good idea if it wouldn't go to the core. Thanks!

[markagrover]

I am Building an app that Models Vendors. Vendor has a nested data Schema. Its an object describing the address details.

address: {
houseNumber: 1,
street: "easy st",
state: "Ma"
}

My solution to this task is posted below with comments in the code... happy coding

exports.updateVendor = function(req, res) {

    let updatedAddressValues;

    const keys = Reflect.ownKeys(req.body);

    const updateValues = keys.reduce((accu, current) => {   // build up an object of the data

        if(req.body[current]){
            accu[current] = req.body[current];
        }
        return accu;
    },{});

    if(updateValues.address){   // check to see if the nested data is present

        const addressKeys = Reflect.ownKeys(updateValues.address);

        updatedAddressValues = addressKeys.reduce((accu, currentKey) => {
            if(updateValues.address[currentKey]){
                accu[currentKey] = updateValues.address[currentKey];
            }
            return accu;
        },{});

       delete updateValues.address;// remove key from object
    }


    db.Vendor.findOne( {_id: req.params.vendorId}, function(err, vendor){    // find Document

        if(err){
            console.error(err);
        } else {
            const keys = Reflect.ownKeys(updateValues);    // set data on object

            keys.forEach((key) => {
                vendor[key] = updateValues[key];
            });

            /*  vendor = Object.assign(vendor, updateValues); this works too*/

            if(updatedAddressValues){
                const addressKeys = Reflect.ownKeys(updatedAddressValues);  // set data on object

                // addressKeys.forEach((key) => {
                //     vendor.address[key] = updatedAddressValues[key];
                // }); // if nested data over write data

                vendor.address = Object.assign(vendor.address, updatedAddressValues);  // see I proved it!
            }
            vendor.save(function(err, updatedRecord){// save record
                if(err){
                    console.error(err);
                } else {
                    res.json(updatedRecord);
                }
            })

        }
    });

[vbrail]

@vadimshvetsov
so finally how did you resolved it??
did you used dot notation for every field
or unsolved
or any new function/trick/method

[vkarpov15]

@vbrail you need to use dot notation for every field, one way or another

[vadimshvetsov]

@vbrail Yep. I've implemented function for deep update with dot notation fields after parsing original object.

[nicky-lenaers]

@vadimshvetsov mind sharing your function for deep update with dot notation?

[nicky-lenaers]

@vadimshvetsov nevermind, found a clean solution based on other examples:

/**
 * Converts an object to a dotified object.
 *
 * @param obj         Object
 * @returns           Dotified Object
 */
export function dotify(obj: object) {

  const res = {};

  function recurse(obj: object, current?: string) {
    for (const key in obj) {
      const value = obj[key];
      const newKey = (current ? current + '.' + key : key);
      if (value && typeof value === 'object') {
        recurse(value, newKey);
      } else {
        res[newKey] = value;
      }
    }
  }

  recurse(obj);
  return res;
}

Hope this can still help people struggling with this issue!

[vadimshvetsov]

@nicky-lenaers Yep, mine looks almost the same

[st3h3n]

@vadimshvetsov nevermind, found a clean solution based on other examples:

/**
 * Converts an object to a dotified object.
 *
 * @param obj         Object
 * @returns           Dotified Object
 */
export function dotify(obj: object) {

  const res = {};

  function recurse(obj: object, current?: string) {
    for (const key in obj) {
      const value = obj[key];
      const newKey = (current ? current + '.' + key : key);
      if (value && typeof value === 'object') {
        recurse(value, newKey);
      } else {
        res[newKey] = value;
      }
    }
  }

  recurse(obj);
  return res;
}

Hope this can still help people struggling with this issue!

Careful! This will just skip non primitive types that don't offer keys, including Date().

[huy-ta]

@st3h3n I think using dot-object can solve that problem.

const dot = require('dot-object');

const studentObject = {
  name: {
    firstName: 'Huy',
    lastName: 'Ta'
  },
  phone: {
    self: {
      sim1: '123456789',
      sim2: '987654321'
    },
    parent: '123654987'
  },
  lastUpdated: new Date()
};

const dotifiedStudentObject = dot.dot(studentObject);

console.log(dotifiedStudentObject);
/* 
Result:
{ 
  'name.firstName': 'Huy',
  'name.lastName': 'Ta',
  'phone.self.sim1': '123456789',
  'phone.self.sim2': '987654321',
  'phone.parent': '123654987',
  lastUpdated: 2018-11-16T12:22:57.254Z 
}
*/

[st3h3n]

@quochuytlbk indeed, my team actually ended up using dot-object

[mayconmesquita]

@huy-ta Thank you! It solves here!