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

fix: set default for dotted path projection #11293

Merged

Conversation

noseworthy
Copy link
Contributor

Summary

When a schema included a subdocument that had a default value of empty
object and that subdocument itself included some defaults, the
subdocument was always null if a query was run with a dotted path
projection to the subdocument's default values.

Check not only that the curent path being evaluated to have defaults
applied, but also check if the current path is part of the included
children in the projection.

Examples

A test has been provided that fails before the change, but assume the following Schemas:

const mongoose = require('mongoose');

const childSchema = new mongoose.Schema({
  name: {
    type: mongoose.Schema.Types.String,
    default: () => 'child',
  },
});

const parentSchema = new mongoose.Schema({
  name: mongoose.Schema.Types.String,
  child: {
    type: childSchema,
    default: () => ({}),
  },
});

If there was a document in the database that didn't include the child field and a query was made that included a projection that had a dotted path to the child's default field, the returned document would have a value of null for the child field.:

/* Assume the database contains:
{
  _id: ObjectId(...),
  name: 'Parent'
}
*/

const  parentDoc = await ParentModel.findOne({name: 'Parent'}, {'child.name': 1});

parentDoc.child === null;

This doesn't happen when using nested paths.

@noseworthy noseworthy force-pushed the add-defaults-for-subdoc-projections branch 2 times, most recently from 9586b10 to a1f50f1 Compare January 30, 2022 18:50
When a schema included a subdocument that had a default value of empty
object and that subdocument itself included some defaults, the
subdocument was always null if a query was run with a dotted path
projection to the subdocument's default values.

Check not only that the curent path being evaluated to have defaults
applied, but also check if the current path is part of the included
children in the projection.
@noseworthy noseworthy force-pushed the add-defaults-for-subdoc-projections branch from a1f50f1 to 24e4d80 Compare January 31, 2022 18:37
@noseworthy
Copy link
Contributor Author

Hey @vkarpov15, thanks for kicking off the actions workflow on a previous version of this PR. Sorry that I missed a couple of test cases when I was verifying this locally and the tests failed last time. I've since fixed them and re-run all the tests locally again. I think this version of the PR should pass all the tests. Would it be possible to get another workflow approval? This bug is preventing us from using Typegoose and mongoose-graphql-compose effectively together. Thanks for taking the time to look at this, and thanks for your work on mongoose!

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@vkarpov15 vkarpov15 added this to the 6.2.1 milestone Feb 3, 2022
Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need to make a quick fix for the tests in prod, need to skip these tests on MongoDB server 4.0

@vkarpov15 vkarpov15 merged commit 486e4f7 into Automattic:master Feb 3, 2022
vkarpov15 added a commit that referenced this pull request Feb 3, 2022
@noseworthy noseworthy deleted the add-defaults-for-subdoc-projections branch February 3, 2022 18:58
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

Successfully merging this pull request may close these issues.

None yet

2 participants