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

Mongoose is creating subdocuments (and setting schema defaults) for fields that are undefined #10660

Closed
thaoula opened this issue Sep 2, 2021 · 9 comments
Assignees
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@thaoula
Copy link

thaoula commented Sep 2, 2021

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

What is the current behavior?
Mongoose appears to be creating sub documents during update operations for fields that are undefined.

I am not sure if this is a version 6.x issue or latest version 5.x issue.

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

  1. Create a schema with a property that is an an embedded custom schema.
  2. Construct and update with with this property set to undefined (actually not set)
  3. The record will be created in the database and the property with the undefined value will be created with any defaults (defined in the schema set)
  4. If you repeat the steps and set the field specifically = null then this issue does not happen.

What is the expected behavior?
The expected behaviour is that undefined is treated like null like in the past. There are now 1000's of places where we have to check for undefined values to ensure that invalid subdocuments are not automatically created.

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.
Mongoose 6.0.4

@thaoula
Copy link
Author

thaoula commented Sep 2, 2021

Current Behaviour - Job and Site fields were undefined and are now populated with the default values on the schema
image

Previous Behaviour - Job was undefined and Site was not set and both are correctly null.
image

The underlying code to update the record has not changed just a change of mongoose from latest 5x version -> 6.0.4

This is creating major bugs in our system now.

@thaoula
Copy link
Author

thaoula commented Sep 2, 2021

Is this issue #10505 talking about making my current issue the default behaviour. The author states that the issue he discusses went away in 5.13.4

@thaoula
Copy link
Author

thaoula commented Sep 2, 2021

This is a massive change in behaviour and people may not be aware that data is now being stored differently and their applications will start failing all of a sudden because of this change.

As you can see from the picture below .. appointments with information were created using older version of Mongoose and appointments that are blank are using 6.0.3 and now 6.0.4 (same issue).

image

@thaoula
Copy link
Author

thaoula commented Sep 2, 2021

It seem that using mongoose.set('setDefaultsOnInsert', false) stops the issue from occurring.

However, I still believe the current implementation is confusing.

Below I have a cut down schema representing Entity (sub document) and Appointment (owns the collection) ... As you can can see the Entity schema has a default for _id.

The appointment schema is using the Entity schema for the property overhead but does not have a default value set on that property definition.

When I read about breaking change setDefaultsOnInsert being true by default, I thought it would not affect us because the default was on the _id property of Entity and i was not setting the overhead property on appointment so we should get null in the database.

If i had set overhead to some object then i would expect the default value to kick in on save because the overhead to some real value.

// Embedded Sub Document
  const EntitySchema = new Schema({
      _id: { type: String, default: uuid.v4 },
      name: String,
      color: String,
  })
  
// Main Schema
  const AppointmentSchema = new Schema({
    date: Date,
    overhead: {type: EntitySchema} 
  });

@IslandRhythms
Copy link
Collaborator

with 6.0, setDefaultsOnInsert is set to true by default, which is why when you manually set it to false your issue is solved.

@IslandRhythms IslandRhythms added the help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary label Sep 2, 2021
@thaoula
Copy link
Author

thaoula commented Sep 3, 2021

Hi @IslandRhythms,

I understand that I can use that setting but I think the behaviour is dangerous which is why I did not close this issue myself.

In my example schema above - overhead: {type: EntitySchema} does not have default set .. so i do not expect it be populated when the value for appointment.overhead = undefined.

If I had provided a default like I did for - _id: { type: String, default: uuid.v4 } in the overhead schema, then i would have expected Mongoose to create a default value. In this case mongoose is ignoring the fact that no default was specified on overhead property ...

It just went .. hey overhead is undefined and setDefaultsOnInsert = true so i should created the subdocument and take the default on the subdocument.

A subdocument is not the same as number, string, boolean etc... it is mostly some sort of reference type and the developer has to make a decision to populate it and cannot exist as a value of _id and some other defaults.

I honestly believe this setting is going to create insidious bugs for developers as its impact may not be known immediately.

Also I wonder how many like me would have interpreted setDefaultsOnInsert to use the defaults on the Appointment schema and not subdocument schema.

Regards,
Tarek

@vkarpov15 vkarpov15 added needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary labels Sep 5, 2021
@vkarpov15 vkarpov15 modified the milestones: 6.0.6, 6.0.7 Sep 5, 2021
@vkarpov15 vkarpov15 added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. and removed needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue labels Sep 18, 2021
@vkarpov15
Copy link
Collaborator

You're right that setDefaultsOnInsert() should ignore defaults underneath single nested paths. Fix will be in v6.0.7.

vkarpov15 added a commit that referenced this issue Sep 18, 2021
@seantcanavan
Copy link

seantcanavan commented Jan 20, 2022

I'm still seeing this issue on a super important sub-document in my user code. Every user I save to the database ends up with the same empty schema:

    "physicalAddress" : {
        "rawAddress" : ""
    },

Even if I force remove the variable delete physicalAddress as soon as I call userModel.save() with the document the subdocument returns. Both the subdocument itself is marked required:false and every member of the subdocument is set to required:false.

...
    physicalAddress: { type: addressSchema, required: false, default: undefined },
...
export const addressSchema = new mongoose.Schema(
    {
        city: { type: String, default: undefined, required: false },
        country: { type: String, default: undefined, required: false },
        county: { type: String, default: undefined, required: false },
        formattedAddress: { type: String, default: undefined, required: false },
        lat: { type: Number, default: undefined, required: false },
        lng: { type: Number, default: undefined, required: false },
        rawAddress: { type: String, default: undefined, required: false },
        state: { type: String, default: undefined, required: false },
        street: { type: String, default: undefined, required: false }, // atomic value for street name only
        streetAndNumber: { type: String, default: undefined, required: false }, // non-atomic value for user-entered street and number
        streetNumber: { type: String, default: undefined, required: false }, // atomic value for street number only
        unit: { type: String, default: undefined, required: false },
        zip: { type: String, default: undefined, required: false },
        zipSuffix: { type: String, default: undefined, required: false }
    },
    { _id: false }
);

This is with mongoose 6.0.13 and @nestjs/mongoose 8.0.1

@vkarpov15
Copy link
Collaborator

@seantcanavan please open a new issue and follow the issue template

@Automattic Automattic locked and limited conversation to collaborators Jan 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Projects
None yet
Development

No branches or pull requests

4 participants