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

Field values of {} are being discarded, but pass Schema on insert #14058

Closed
2 tasks done
hardcodet opened this issue Nov 7, 2023 · 5 comments
Closed
2 tasks done

Field values of {} are being discarded, but pass Schema on insert #14058

hardcodet opened this issue Nov 7, 2023 · 5 comments
Milestone

Comments

@hardcodet
Copy link

hardcodet commented Nov 7, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Mongoose version

7.6.2

Node.js version

20.x

MongoDB server version

Atlas

Typescript version (if applicable)

4.8.4

Description

Hi all

I wonder whether this is a newer issue, because we just had multiple cases of this in the past weeks, but never before:

Here's my schema:

  • I have a Foo entity, with a bar sub document, which is required
  • Bar only has optional fields
const BarSchema = new mongoose.Schema({
    a: { type: String, required: false },
    b: { type: String, required: false },
}, { _id: false });

export const FooSchema = new mongoose.Schema({
    name: { type: String, required: true },
    bar: { type: BarSchema, required: true },
    ...
});

Next, I create an entity instance like this and create a document in the database:

const foo: Foo = {
    name: "hello",
    bar: {}
}

Now, this insert passes validation, because bar is not null or undefined. However, upon inserting the document into the DB, this field is ignored, and the bar field is missing in the created document.
Accordingly, if I then fetch that document, it looks like this:

{
    name: "hello",
}

This means that subsequent updates of that document will no longer pass validation. So this fails with a runtime error:

   const myFoo = await this.getFoo(id); // looks like this: { name: "hello" }
   myFoo.name = "update";
   await this.repository.update(myFoo); // validation error: Path 'bar' is required

Steps to Reproduce

See above: Create the an entry with a required field, where the value is {}. You'll see that this field is not persisted in a new document, but will pass validation.

Expected Behavior

The empty object should be persisted on MongoDB. If I store an object that looks like this:

{
   name: "hello",
   bar: {}
}

...then fetching it should give me the same result, which requires that bar is not stripped upon inserting the document into the DB.

@IslandRhythms IslandRhythms added the confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. label Nov 8, 2023
@IslandRhythms
Copy link
Collaborator

const mongoose = require('mongoose');

const BarSchema = new mongoose.Schema({
  a: { type: String, required: false },
  b: { type: String, required: false },
}, { _id: false });

const FooSchema = new mongoose.Schema({
  name: { type: String, required: true },
  bar: { type: BarSchema, required: true },
});


const Foo = mongoose.model('Foo', FooSchema);

async function run() {
  await mongoose.connect('mongodb://localhost:27017');
  await mongoose.connection.dropDatabase();

  await Foo.create({
    name: 'hello',
    bar: {}
  });

  console.log(await Foo.findOne());

  const doc = await Foo.findOne();
  doc.name = 'Test Testerson';
  await doc.save();

}

run();

@vkarpov15 vkarpov15 added this to the 7.6.5 milestone Nov 9, 2023
@vkarpov15
Copy link
Collaborator

Stripping out empty objects is expected behavior with the minimize option. For example, with the following change, @IslandRhythms 's script succeeds:

const FooSchema = new mongoose.Schema({
  name: { type: String, required: true },
  bar: { type: BarSchema, required: true },
}, { minimize: false });

That being said, I agree that bar passing the required validator but then not ending up in the database is surprising and problematic. We will look into fixing this.

As a workaround, I recommend adding a default value to bar that's an empty object as follows:

const FooSchema = new mongoose.Schema({
  name: { type: String, required: true },
  bar: { type: BarSchema, required: true, default: () => ({}) },
});

@vkarpov15
Copy link
Collaborator

Upon further inspection, this is expected behavior that was introduced in 6.2.2 with #11247, and not something we can change without a backwards breaking change. Please add default: () => ({}) to the affected subdocument paths, or use the following to add a default value of {} to all your subdocument paths by default.

mongoose.Schema.Types.Subdocument.set('default', () => ({}));

@vkarpov15 vkarpov15 removed this from the 7.6.5 milestone Nov 14, 2023
@vkarpov15 vkarpov15 added won't fix and removed confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. labels Nov 14, 2023
@vkarpov15 vkarpov15 reopened this Nov 21, 2023
@vkarpov15 vkarpov15 added this to the 7.6.7 milestone Nov 21, 2023
@vkarpov15
Copy link
Collaborator

Will take another look in 7.6.7. This issue doesn't sit well with me, we'll put in some more cycles to see if we can work around it.

@hardcodet
Copy link
Author

Thanks, really appreciated. While the global default is probably nothing I'd want to do, I could live a specific default where it could happen.

However, my gut feeling though is that introducing a breaking change may be worth it. TBH, if somebody injected {} into their DB, and then their code broke if they didn't get undefined back, that's probably more on them than on mongoose ;)

vkarpov15 added a commit that referenced this issue Dec 5, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
fix: avoid minimizing single nested subdocs if they are required
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

3 participants