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

defaults option for queries #7287

Closed
mhombach opened this issue Nov 28, 2018 · 12 comments · Fixed by #10245
Closed

defaults option for queries #7287

mhombach opened this issue Nov 28, 2018 · 12 comments · Fixed by #10245
Assignees
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature
Milestone

Comments

@mhombach
Copy link

mhombach commented Nov 28, 2018

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

Possible bug or unexpected behaviour

What is the current behavior?

.find() overwrites fields that are undefined but have a default value with that exact value, making it inconsistent and not possible to see if such that value is set in the database or not.
See #7269 for reference.

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

const mongoose = require("mongoose")
const { Schema } = mongoose;
const PersonSchema = new Schema({
  name: String,
  age: { type: Number, default: 0 }
})
const Person = mongoose.model("Person", PersonSchema)
mongoose.connect('mongodb://localhost:27017/test');

async function runTest() {
  // creating document with manual set value for "age"
  await Person.create({ name: "martin", age: 30 });

  // get document
  let entry = await Person.findOne();
  console.log(entry); // entry.age == 30

  // deleting the key "age"
  await Person.collection.update({ _id: entry._id }, { $unset: { age: 1 }});
  // attribute "age" is now not existing on that document in the database-storage

  let newResult = await Person.findOne();
  console.log(newResult); // attribute "age" is now existing again on the runtime object and has the default value of 0
}
runTest();

What is the expected behavior?
The default value of an attribute should be used on creation or maybe even on update
(https://mongoosejs.com/docs/defaults.html#the-setdefaultsoninsert-option) but NOT on pure reading a document from the database.

Please mention your node.js, mongoose and MongoDB version.
node v8.11.3, mongoose 5.3.13, mongoDB latest version

@vkarpov15
Copy link
Collaborator

This is something worth discussing more. Currently, this is by design. The spirit of this behavior is to apply defaults where possible, because setDefaultsOnInsert isn't always on. This way, you can use defaults to avoid an extra != null check if you want to use String#startsWith() or similar.

You can always use Document#$isDefault() to check whether age is set to the default value or not.

@vkarpov15 vkarpov15 added the discussion If you have any thoughts or comments on this issue, please share them! label Dec 6, 2018
@mhombach
Copy link
Author

mhombach commented Dec 6, 2018

@vkarpov15 i kinda agree that it's more writing when you need to null-check attributes and can't rely on it beeing an empty string or so.
But the thing that made me create this issue is the pure fact, that "reading" a mongoose-document should, if no custom hooks are modifying it, return the object "as it is" from the database. The intent on pulling some document from the database is to know the values that are stored in the database. If there is an attribute that is undefined there will always be a reason why that value is saved as undefined into the database. In my opinion one should start rethinking there and not letting the database set "some value the nest time the data is pulled from the db".
Scenario:

  • A field "user" gets a default of the user who is logged into the GUI and creating the document.
  • at some point old data need to be imported manualle into the DB directoy to mongoDB, skipping mongoose. All fields are valid in mongoose but the "user" field is undefined because at the time these entried were created first, there was no username as default.
  • You now cann tell, that if the value of username is undefined then it was imported and not created the normal way.
    But if you now find a document and the correct undefined gets replaced by the currently logged in user, then the document incorrectly states, that user xyz has created that document, which is simply not true.
    I am not saying that this behaviour MUST not be used, but i too would like to discuss this and maybe we can work out some option/setting for that behaviour like setting forceDefault: true/false or forceDefaultOnEmpty: true/false in the schema :)

@GirayEryilmaz
Copy link

Any updates on this issue? We sometimes deliberately put empty objects to the database and count on them to be empty when we fetch them. But they are populated with the default values.

@vkarpov15
Copy link
Collaborator

@GirayEryilmaz you can use Document#$isDefault() to check if a value is set using a default.

We'll add a new option to disable defaults on queries, since we already have a defaults option for documents in 9609e87 and #8271

@vkarpov15 vkarpov15 changed the title Inconsistency in .find() and default values defaults option for queries May 12, 2021
@vkarpov15 vkarpov15 added new feature This change adds new functionality, like a new method or class and removed discussion If you have any thoughts or comments on this issue, please share them! labels May 12, 2021
@vkarpov15 vkarpov15 added this to the 5.12.9 milestone May 12, 2021
@vkarpov15
Copy link
Collaborator

TLDR; Model.findOne().setOptions({ defaults: false }) should skip applying defaults to the result document. See the changes from 9609e87 to learn about the defaults option for the Model() constructor.

@vkarpov15 vkarpov15 modified the milestones: 5.12.9, 5.12.10 May 13, 2021
@vkarpov15 vkarpov15 linked a pull request May 18, 2021 that will close this issue
@vkarpov15 vkarpov15 added enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature and removed new feature This change adds new functionality, like a new method or class labels May 18, 2021
@hugomallet
Copy link

Hello,

The default behaviour may be helpful for values like booleans.
But for other kind of values like creation date, ids etc., it's not a desired behaviour.
Especially if the default option is "setDefaultsOnInsert" only.

I think the default behaviour should be reversed. And we should be able to fill in defaults on demand when reading.
If this change involves an annoying breaking change, i suggest adding an option on schema like :

uuid: {
    type: String,
    default: () => uuid(),
    setDefaultOnRead: false,
},

@vkarpov15
Copy link
Collaborator

@hugomallet I don't understand your issue, can you please clarify?

@hugomallet
Copy link

@vkarpov15, my case was :

I added a new field scheduledAt to an existing model. This fields defaults to the current date at the time of the insert if not defined.
Something like this :

scheduledAt: {
    type: Date,
    default: () => new Date(),
},

My issue is that the previous objects have an undefined scheduledAt field. But when i read, scheduledAt equals to the current date (read time) which is an unwanted value because it changes at every read. I expected to have undefined instead of an incorrect value.

@vkarpov15
Copy link
Collaborator

@hugomallet you can create a middleware that sets defaults to false for all find and findOne operations:

schema.pre(['find', 'findOne'], function disableDefaults() {
  this.setOptions({ defaults: false });
});

Would that help?

@IYCI
Copy link

IYCI commented Jul 7, 2021

Hi, @vkarpov15 Thanks for adding defaults as a model constructor option!

In addition to Model.findOne().setOptions({ defaults: false })
Is it possible to also have defaultsOnRead: false added as a schema options property?

@junjenfong
Copy link

junjenfong commented Mar 16, 2022

@hugomallet you can create a middleware that sets defaults to false for all find and findOne operations:

schema.pre(['find', 'findOne'], function disableDefaults() {
  this.setOptions({ defaults: false });
});

Would that help?

Doesn't seems to work on me. Thoughts? I have the exact same issue as hugomallet

@vkarpov15
Copy link
Collaborator

@junjenfong please open a new issue and follow the issue template. "Doesn't work" isn't an actionable bug report.

@Automattic Automattic locked and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants