Mongoose: Updating document without $set or query defined

[mirazasx]

Feature request/ maybe bug, not sure about it:
After I updated to Mongoose 5.0.6 the function .update() requires to pass all new values as $set in order for mongoose history to work, it would be nice if it's not needed as it was in 4.x version. The mongoose history worked just fine with 4.x version of mongoose without $set parameter.
_Here is simple example, :_

testSchema.plugin(mongooseHistory, { indexes: [{'d._id': 1}] });
let testDB = mongoose.model('dummyTest', testSchema);

let doc = testDB.create({
  username : "test 1",
  sttring2 : "test 2"
}, function($err, $doc){
  // this throws works, but based on docs, it's not needed $set param when updating via document
  $doc.update({$set:{
    username: "It works"
  }}, function($err, $result){
    if ($err) console.log($err);
    console.log("updated with $set");
  });
  $doc.doSomeUpdate(function($err, $result){
    console.log("doSomeUpdate");
  });
  // this throws an error
  /*$doc.update({
    username: "this fails"
  }, function($err, $result){
    if ($err) console.log($err);
    console.log("updated");
  });*/
});

I would expect to set Internally $set variable on update by default behavior, so that mongoose history would work without explicitly defining $set.

My setup:
node -v v6.11.0
db.version 3.2.6
mongoose 5.0.6
mongoose-history 0.6.0

[lineus]

I couldn't find a package on npm or github called mongohistory. can you provide a url to the exact plugin you're using?

[tayfunyugruk]

@lineus i think it is mongoose-history not mongo-history ? https://www.npmjs.com/package/mongoose-history here version matches

[mirazasx]

Yes its mongoose-history, sorry for late answer.

[lineus]

Hi @mirazasx, let me say first, that I've never used the mongoose-history plugin before. In order to help sort this out I wrote the following minimal but complete example to illustrate the problem. The only meaningful differences between your example and mine are

1. i added a call to set debug to true on mongoose
2. a small package that prints relevant version info
3. the inclusion of the schema
4. a bit of logic that allows the script to include useMongoClient in mongoose 4.
5. exclusion of doSomeUpdate() I don't know what that is, so I left it out.
6. a silly film reference ( do I get bonus points for that? )

#!/usr/bin/env node
'use strict'

const mongoose = require('mongoose')
mongoose.set('debug', true)
mongoose.Promise = global.Promise
const Schema = mongoose.Schema
const mongooseHistory = require('mongoose-history')
const uri = 'mongodb://localhost/test'
require('get-mongoose-versions').get(uri)
  .then(versions => console.log(versions))

let mOpts = {}

if (mongoose.version[0] === '4') {
  mOpts.useMongoClient = true
}

const testSchema = new Schema({
  username: String,
  description: String
})

const options = { indexes: [{ 'd._id': 1 }] }
testSchema.plugin(mongooseHistory, options)

const Test = mongoose.model('deathwish', testSchema, 'deathwish')

const star = {
  username: 'cbronson',
  description: 'vigilante'
}

mongoose.connect(uri, mOpts, () => {
  console.log('connected.')
  Test.create(star, function (err, doc) {
    if (err) { return console.error(err) }
    console.log(doc)
    doc.update({ username: 'bwillis' }, function (err, result) {
      if (err) { return console.error(err) }
      console.log(result)
      Test.find({}).then((res) => {
        console.log(res[0])
        mongoose.connection.close()
      })
    })
  })
}).catch((err) => console.error(err))

when I run this example, here is my output:

InspiredMacPro:Help lineus$ ./mongoose4/6167/index.js 
connected.
{ mongooseVersion: '4.13.11',
  mongodbNativeVersion: '2.2.34',
  nodeVersion: 'v6.11.0',
  mongodbServerVersion: '3.6.2' }
^C

after commenting out the call to testSchema.plugin here is my output:

InspiredMacPro:Help lineus$ ./mongoose4/6167/index.js 
connected.
Mongoose: deathwish.insert({ username: 'cbronson', description: 'vigilante', _id: ObjectId("5a940e1b4d91b9be1d689367"), __v: 0 })
{ mongooseVersion: '4.13.11',
  mongodbNativeVersion: '2.2.34',
  nodeVersion: 'v6.11.0',
  mongodbServerVersion: '3.6.2' }
{ __v: 0,
  username: 'cbronson',
  description: 'vigilante',
  _id: 5a940e1b4d91b9be1d689367 }
Mongoose: deathwish.update({ _id: ObjectId("5a940e1b4d91b9be1d689367") }, { '$set': { username: 'bwillis' } }, {})
{ __v: 0,
  username: 'cbronson',
  description: 'vigilante',
  _id: 5a940e1b4d91b9be1d689367 }
{ n: 1, nModified: 1, ok: 1 }
Mongoose: deathwish.find({}, { fields: {} })
{ _id: 5a940aeffcecbdbcf41a6d30,
  username: 'bwillis',
  description: 'vigilante',
  __v: 0 }

can you see any problems with my implementation of the plugin that would cause nothing to be inserted or updated? I'll continue to debug on my end, but your insight would be welcomed.

[lineus]

I figured it out, I didn't have the mongoose-history npm package installed in mongoose4 directory. it was however installed in a parent of mongoose4, so somewhere in there it caused the problem. installing it in the mongoose4 dir and re-running with the plugin line uncommented I get the expected output. I'll continue on to testing on mongoose5.

[lineus]

I updated the promise err handling in my comment above to remove deprecation warnings.

mongoose4 output:

InspiredMacPro:Help lineus$ ./mongoose4/6167/index.js 
connected.
Mongoose: deathwish_history.insert({ t: new Date("Mon, 26 Feb 2018 14:11:58 GMT"), o: 'i', d: { _id: ObjectId("5a9415aee50b64c0a1868232"), description: 'vigilante', username: 'cbronson' }, _id: ObjectId("5a9415aee50b64c0a1868233") })
Mongoose: deathwish_history.ensureIndex({ 'd._id': 1 }, { background: true })
Mongoose: deathwish.insert({ username: 'cbronson', description: 'vigilante', _id: ObjectId("5a9415aee50b64c0a1868232"), __v: 0 })
{ mongooseVersion: '4.13.11',
  mongodbNativeVersion: '2.2.34',
  nodeVersion: 'v6.11.0',
  mongodbServerVersion: '3.6.2' }
{ __v: 0,
  username: 'cbronson',
  description: 'vigilante',
  _id: 5a9415aee50b64c0a1868232 }
Mongoose: deathwish_history.insert({ t: new Date("Mon, 26 Feb 2018 14:11:58 GMT"), o: 'u', d: { username: 'bwillis' }, _id: ObjectId("5a9415aee50b64c0a1868234") })
Mongoose: deathwish.update({ _id: ObjectId("5a9415aee50b64c0a1868232") }, { '$set': { username: 'bwillis', __v: undefined } }, {})
{ n: 1, nModified: 1, ok: 1 }
Mongoose: deathwish.find({}, { fields: {} })
{ _id: 5a9414bbf62979bfdd64f8b8,
  username: 'cbronson',
  description: 'vigilante',
  __v: 0 }
InspiredMacPro:Help lineus$ 

mongoose5 output:

InspiredMacPro:Help lineus$ ./mongoose5/6167/index.js 
{ mongooseVersion: '5.0.6',
  mongodbNativeVersion: '3.0.2',
  nodeVersion: 'v6.11.0',
  mongodbServerVersion: '3.6.2' }
connected.
Mongoose: deathwish_history.ensureIndex({ 'd._id': 1 }, { background: true })
Mongoose: deathwish_history.insert({ t: new Date("Mon, 26 Feb 2018 14:13:42 GMT"), o: 'i', d: { username: 'cbronson', description: 'vigilante', _id: ObjectId("5a94161627e7aec0bca9974a") }, _id: ObjectId("5a94161627e7aec0bca9974b") })
Mongoose: deathwish.insert({ username: 'cbronson', description: 'vigilante', _id: ObjectId("5a94161627e7aec0bca9974a"), __v: 0 })
{ username: 'cbronson',
  description: 'vigilante',
  _id: 5a94161627e7aec0bca9974a,
  __v: 0 }
TypeError: Cannot set property '__v' of undefined
    at createHistoryDoc (/Users/lineus/dev/Help/mongoose5/node_modules/mongoose-history/lib/mongoose-history.js:104:11)
    at Query.<anonymous> (/Users/lineus/dev/Help/mongoose5/node_modules/mongoose-history/lib/mongoose-history.js:90:22)
    at callMiddlewareFunction (/Users/lineus/dev/Help/mongoose5/node_modules/kareem/index.js:395:23)
    at next (/Users/lineus/dev/Help/mongoose5/node_modules/kareem/index.js:58:7)
    at Kareem.execPre (/Users/lineus/dev/Help/mongoose5/node_modules/kareem/index.js:86:8)
    at Kareem.wrap (/Users/lineus/dev/Help/mongoose5/node_modules/kareem/index.js:264:8)
    at Query._execUpdate (/Users/lineus/dev/Help/mongoose5/node_modules/kareem/index.js:310:11)
    at _update (/Users/lineus/dev/Help/mongoose5/node_modules/mongoose/lib/query.js:2907:13)
    at Query.update (/Users/lineus/dev/Help/mongoose5/node_modules/mongoose/lib/query.js:2702:10)
    at _update (/Users/lineus/dev/Help/mongoose5/node_modules/mongoose/lib/model.js:2624:16)
    at Function.update (/Users/lineus/dev/Help/mongoose5/node_modules/mongoose/lib/model.js:2528:10)
    at model.update (/Users/lineus/dev/Help/mongoose5/node_modules/mongoose/lib/document.js:503:34)
    at /Users/lineus/dev/Help/mongoose5/6167/index.js:39:9
    at Function.<anonymous> (/Users/lineus/dev/Help/mongoose5/node_modules/mongoose/lib/model.js:3928:16)
    at parallel (/Users/lineus/dev/Help/mongoose5/node_modules/mongoose/lib/model.js:2078:12)
    at /Users/lineus/dev/Help/mongoose5/node_modules/async/internal/parallel.js:35:9
^C
InspiredMacPro:Help lineus$ 

[mirazasx]

It's my first time reporting some kind of bug/error, so not sure how to contribute. But you got the same error I get when I run it. On Mogoose4 it worked just fine, and __v error roots to internal $set variable being null on update.
Regarding #5 doSomeUpdate: I missed it when I copied from my code, it was mongoose.method with this pointer, showing exactly the same. If needed I can find it and update, but I gues it's irelevant now, since you managed to reproduce error/bug.

[lineus]

the middleware mongoose-history is relying on the internal state of _update, which changed in 5.x.
from mongoose-history.js line 89:

    let d = this._update.$set; // undefined in 5.x
    let historyDoc = createHistoryDoc(d, 'u');

in mongoose version 5 the value of _update is _update: { username: 'bwillis' }
in mongoose version 4 the value of _update is _update: { '$set': { username: 'bwillis' } }

I believe this is addressed here in the migration guide. I think it might be more reasonable since this is by design in mongoose 5, to file an issue with the maintainer of mongoose-history.

@vkarpov15 thoughts?

[lineus]

@mirazasx I'm still new to mongoose and js in general :) so I'm just volunteering time here to learn and grow as a dev, plus it's fun! My contributions thus far have been limited to documentation. Your bug report was just fine. One thing I've noticed is that the closer you get to making it possible for your example to be copied and pasted ( and subsequently run ) by anyone, the lower the total time to closure for the issue will be. That seems to be true whether it's an actual bug in the repo, or just an implementation issue on the part of the requester. In this case, just including your entire schema including virtuals, statics, etc would have been the only thing that would have made an improvement.

[vkarpov15]

@lineus thanks for digging into mongoose-history's internals and finding this. You're right, that's the relevant change in the migration guide. We'll see if we can put in a PR to mongoose-history to fix it

[mirazasx]

@lineus thanks for help/debug on this issue and tips. I will try to post better reproduction code next time.

[vkarpov15]

I put in a PR to mongoose-history to fix this issue. Once that's merged and released, we'll close this one out

[lineus]

@vkarpov15's PR has been merged in mongoose-history. Closing this now, Thanks Val!