Mongoose: [Bug] Inconsistent Boolean Default Value on Save

[MikeShi42]

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

What is the current behavior?
Given a doc that was created before a new boolean schema field was added, if the boolean schema field is set to default: false, it will not be set to false. However, if it is default: true, it will be set to true. This is probably clearer looking at the example.

If the current behavior is a bug, please provide the steps to reproduce.

// index.js
const mongoose = require('mongoose');
// Import User Schema
require('./user');

mongoose.connect('mongodb://localhost:27017/mongo-default-bug');

const User = mongoose.model('User');

const newUser = new User({
  otherVal: 99,
  defaultValT: undefined,
  defaultValF: undefined,
});

// Save the new user doc
newUser.save().then(() => {
  // Remove defaultValT, defaultValF to simulate this is an old doc before
  // new schema with `defaultValT and `defaultValF` were added in as fields.
  User.collection.update(
    {}, {$unset: {defaultValT: true, defaultValF: true}},
    {multi: true, safe: true},
    () => {
      // Update the user doc
      User.findOne({}).then(user => {
        user.otherVal = 9999;
        user.save();
        console.log(user);
      })
    }
  );
});

// user.js
const mongoose = require('mongoose');
const { Schema } = mongoose;

/**
 * User Schema
 */
const UserSchema = new Schema({
  defaultValT: { type: Boolean, default: true },
  defaultValF: { type: Boolean, default: false },
  otherVal: 0,
});

mongoose.model('User', UserSchema);



md5-88eeb553a82c4964f49635f6d81d8868



{ defaultValT: true,
  defaultValF: false,
  _id: 5afc7908d5b4056e35502493,
  otherVal: 9999,
  __v: 0 }



md5-8aa2e5d9ff334e1a2c5a13cea2387903



{
    "_id" : ObjectId("5afc7908d5b4056e35502493"),
    "otherVal" : 9999,
    "__v" : 0,
    "defaultValT" : true
}



md5-c27666089fba9daa2434f86fdafec51b



node --version
v8.4.0
[email protected]
MongoDB server version: 3.6.2

[lineus]

@MikeShi42 Thanks for the clear explanation!

[lineus]

here's the repro script ( based on @MikeShi42's code ) I used to work on this:

6477.js

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

const assert = require('assert');
const mongoose = require('mongoose');
// mongoose.set('debug', true);

var opts = {};

if (mongoose.version.charAt(0) === '4') {
  mongoose.Promise = global.Promise;
  opts.useMongoClient = true;
}

mongoose.connect('mongodb://localhost/test', opts);
const conn = mongoose.connection;
const Schema = mongoose.Schema;

const schema = new Schema({
  first: String,
  last: String,
  f: {
    type: Boolean,
    default: false
  },
  t: {
    type: Boolean,
    default: true
  }
});

const Test = mongoose.model('test', schema);

const test = new Test({
  first: 'George'
});

async function run() {
  process.stdout.write(`${mongoose.version}: `);
  await conn.dropDatabase();
  let saved = await test.save();
  assert.strictEqual(saved.t, true, 't: new docs should have defaults set');
  assert.strictEqual(saved.f, false, 'f: new docs should have defaults set');

  await Test.collection.update({}, { $unset: { f: true, t: true } });
  let raw = await Test.collection.findOne();
  assert.strictEqual(raw.t, undefined, 't: update should have unset');
  assert.strictEqual(raw.f, undefined, 'f: update should have unset');

  let found = await Test.findOne();
  found.last = 'Boole';
  await found.save();

  let final = await Test.collection.findOne();
  assert.strictEqual(final.t, true, 't: failed to set t = true on save()');
  assert.strictEqual(final.f, false, 'f: failed to set f = false on save()');
  console.log();
  return conn.close();
}

run().catch(error);

function error(e) {
  console.error(e.message);
  return conn.close();
}

Output on 5.1.1:

issues: ./6477.js
5.1.1: f: failed to set f = false on save()
issues:

[MikeShi42]

@lineus I guess a question is why defaults are being set on save anyways? Doesn't the documentation say that they should only be set on insert? (I could be totally wrong, but that's been my interpretation).

Personally I'm fine with either behavior, just a bit perplexed :)

[lineus]

@MikeShi42 I agree, the docs and the behavior don't match. I was playing around with this some to determine how far back it goes:

6477.js

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

const assert = require('assert');
const mongoose = require('mongoose');
// mongoose.set('debug', true);

var opts = {};

if (mongoose.version.charAt(0) === '4') {
  mongoose.Promise = global.Promise;
  opts.useMongoClient = true;
}

mongoose.connect('mongodb://localhost/test', opts);
const conn = mongoose.connection;
const Schema = mongoose.Schema;

const schema = new Schema({
  first: String,
  last: {
    type: String,
    default: 'Boole'
  },
  t: {
    type: Boolean,
    default: true
  }
});

const Test = mongoose.model('test', schema);

async function run() {
  process.stdout.write(`${mongoose.version}: `);
  await conn.dropDatabase();
  await Test.collection.insert({});

  let found = await Test.findOne();
  found.first = 'George';
  await found.save();

  let final = await Test.collection.findOne();
  assert.strictEqual(final.t, true, 't: failed to set t = true on save()');
  assert.strictEqual(final.last, 'Boole', 't: failed to set t = true on save()');
  console.log('works!');
  return conn.close();
}

run().catch(error);

function error(e) {
  console.error(e.message);
  return conn.close();
}

Shell Output in a for loop of versions:

issues: describe nim
function nim # 'install a version of mongoose [ defaults to latest ]'
{
    ver=${1:-'@latest'}
    npm install mongoose${ver} >/dev/null 2>&1
}
issues: for ver in 4.8.0 4.13.12 5.0.0 5.1.1 ;do nim @$ver && ./6477.js;done
4.8.0: works!
4.13.12: works!
5.0.0: works!
5.1.1: works!
issues:

I plan to update the docs to reflect the actual behavior, and I think adding a schema option that turns off this behavior could be useful. Changing the current behavior to match the docs could backwards break stuff for folks, but it's worth a think for ver 6.

What do you think @vkarpov15 ?

[MikeShi42]

@lineus thanks for digging deeply and writing all these tests! really appreciated and good to know that the current behavior involving defaults will be the same moving forward :)

[vkarpov15]

Good catch @lineus , thanks for the detailed investigation :+1: