Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weird behaviour when saving documents with undefined properties which have default values in the model #3981

Closed
darkbasic opened this issue Mar 15, 2016 · 10 comments
Milestone

Comments

@darkbasic
Copy link

mongoose.Schema({
    name: {
        type: String,
        required: true,
        validate: nameValidator
    },
    completed: {type: Boolean, default: false}
})

The model has a 'completed' property with a default value (false). It is obviously not marked as required because I want it to default to false if I don't specify it.
In my Express request handler if I create a document with {name: req.body.name, completed: req.body.completed} and req.body doesn't have the 'completed' property set it doesn't return me any validation error (shouldn't the type: Boolean property trigger an error?) and it also doesn't create the 'completed' property in db for the just created document despite the default value (shouldn't it create a 'completed' property with the default (false) value?). Beware that id DOES create a new document, it just doesn't have the 'completed' property.
Even if the 'completed' property doesn't exist in my document If I query the db with {id: 'myID'} it returns the default value (false) for 'completed', BUT if I also query for the 'completed' property ({id: 'myID', completed: false}) it returns an empty set!

@vkarpov15
Copy link
Collaborator

Can you clarify which version of mongoose you're using?

@vkarpov15 vkarpov15 added this to the 4.4.8 milestone Mar 17, 2016
@darkbasic
Copy link
Author

Sorry, I'm using 4.4.6 (I missed the 4.4.7 update a few days ago).

@vkarpov15 vkarpov15 modified the milestones: 4.4.9, 4.4.8 Mar 18, 2016
@seriousManual
Copy link
Contributor

I created a little test to reproduce the described behaviour:

    var testSchema = new mongoose.Schema({
        completed: {type: Boolean, default: false}
    });
    var TestModel = mongoose.model('test', testSchema);

    var a = new TestModel();
    var b = new TestModel({completed: true});
    var c = new TestModel({completed: false});

    var d = new TestModel({completed: 0});
    var e = new TestModel({completed: 1});
    var f1 = new TestModel({completed: ''});
    var f = new TestModel({completed: 'bert'});
    var g = new TestModel({completed: null});
    var h = new TestModel({completed: undefined});

    async.parallel([
        cb => a.save(cb),
        cb => b.save(cb),
        cb => c.save(cb),
        cb => d.save(cb),
        cb => e.save(cb),
        cb => f1.save(cb),
        cb => f.save(cb),
        cb => g.save(cb),
        cb => h.save(cb)
    ], (error, profiles) => {
        TestModel.find({}, (error, profiles) => console.log(profiles))
    })

I also patched the file 'model.js', line 127 with a console.log to see what actually is written to the database which looks like this:

inserting: { _id: 56eebc2ffa855ac0286fabac, completed: false, __v: 0 }
inserting: { _id: 56eebc2ffa855ac0286fabad, completed: true, __v: 0 }
inserting: { _id: 56eebc2ffa855ac0286fabae, completed: false, __v: 0 }
inserting: { _id: 56eebc2ffa855ac0286fabaf, completed: false, __v: 0 }
inserting: { _id: 56eebc2ffa855ac0286fabb0, completed: true, __v: 0 }
inserting: { _id: 56eebc2ffa855ac0286fabb1, completed: false, __v: 0 }
inserting: { _id: 56eebc2ffa855ac0286fabb2, completed: true, __v: 0 }
inserting: { _id: 56eebc2ffa855ac0286fabb3, completed: null, __v: 0 }
inserting: { _id: 56eebc2ffa855ac0286fabb4, __v: 0 }

The result of the above code looks like this:

[ { completed: false, __v: 0, _id: 56eebc2ffa855ac0286fabac },
  { completed: true, __v: 0, _id: 56eebc2ffa855ac0286fabad },
  { completed: false, __v: 0, _id: 56eebc2ffa855ac0286fabae },
  { completed: false, __v: 0, _id: 56eebc2ffa855ac0286fabaf },
  { completed: true, __v: 0, _id: 56eebc2ffa855ac0286fabb0 },
  { completed: false, __v: 0, _id: 56eebc2ffa855ac0286fabb1 },
  { completed: true, __v: 0, _id: 56eebc2ffa855ac0286fabb2 },
  { completed: null, __v: 0, _id: 56eebc2ffa855ac0286fabb3 },
  { completed: false, __v: 0, _id: 56eebc2ffa855ac0286fabb4 } ]

While all actual truthy values (like 1, 'foo', true) and all actual falsy values (0, '', false) are casted to Boolean, this does not happen for null and undefined.
While null is actually written to the database as null, undefined is not handled at all.
Furthermore, when reading from the database the null value is used 'as-is' while in the case of the non-existant value (previously undefined) the default from the schema is used.

@vkarpov15 can you clearify if the uncasted null/undefined values are supposed to be handled this way or if this is a bug?
I'd be happy to look into this issue if necessary!

@vkarpov15
Copy link
Collaborator

Not sure, try changing the behavior and seeing if the tests break :)

@vkarpov15 vkarpov15 modified the milestones: 4.4.10, 4.4.9 Mar 23, 2016
vkarpov15 added a commit that referenced this issue Mar 24, 2016
@vkarpov15
Copy link
Collaborator

I took a closer look and this is intentional behavior. The current implementation works like this:

  1. Build the document and apply defaults
  2. Take the object you passed to new TestModel() and set() all the fields.

The above commits make it so that new TestModel({completed: undefined}); doesn't overwrite the default. However, new TestModel({completed: null}); will overwrite the default, because null represents the intentional absence of a value according to the ES2015 spec. If you want to change this behavior, you can either use a required validator or a custom setter:

mongoose.Schema({
    name: {
        type: String,
        required: true,
        validate: nameValidator
    },
    completed: {
      type: Boolean,
      set: v => (v === null || v === undefined) ? false : v
    }
})

@darkbasic
Copy link
Author

May I suggest another possible behaviour?
{completed: null} will still overwrite the default as you said, but if you specify both "required" and "default" for the field then {completed: null} or {completed: ""} will not overwrite the default anymore. Actually specifying both "required" and "default" doesn't make much sense, because "default" is simply not taken into account because of "required".

@vkarpov15
Copy link
Collaborator

IMO required and default makes sense, and I don't think seeing default should preclude you from being able to intentionally set the field to null. That's what required is for

@darkbasic
Copy link
Author

How can "default" set its default value if you always have to provide your own value because of "required"? Do I miss something?

@vkarpov15
Copy link
Collaborator

A default will kick in if the value isn't defined when the object is created, but it won't prevent you from setting the field to null, which is by design since null represents the intentional absence of a value in JavaScript. required is there to prevent that.

@taxilian
Copy link
Contributor

As an FYI, this "fix" breaks other behavior that may be needed; specifically, I can no longer unset an array if I want to. I have a document which has a sparse unique index on an array; if the array is defined but empty I can only have one in the database at a time. In that case, previously I could just set the field to undefined and it wouldn't get inserted into the database; after the change I get a failure:

E11000 duplicate key error collection: databasename.collection index: field_id_1 dup key: { : undefined }

I'm trying to find a workaround; I will likely open a new ticket to address this specifically

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants