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

Potentially undocumented migration (breaking) change for populating virtuals with no matches, mongoose 5.x returns an empty array, mongoose 6.x returns undefined. #10816

Closed
barca-reddit opened this issue Sep 29, 2021 · 6 comments
Labels

Comments

@barca-reddit
Copy link

barca-reddit commented Sep 29, 2021

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

Possible bug or potentially undocumented breaking change.

What is the current behavior?

Apologies in advance because this seems like something minor. The current behavior is the following - on mongoose 5.x, or version 5.12.14 to be exact, when you are using the Model.populate({ path: '....' }) method, if there are no documents to be found, the query returns an empty array. On Mongoose@latest (6.x+) the same exact query returns undefined instead.

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

Matches.js

const mongoose = require('mongoose');
const mongooseLeanVirtuals = require('mongoose-lean-virtuals');
const Schema = mongoose.Schema;

const MatchSchema = new Schema(
    {
        _id: { type: Schema.Types.ObjectId, auto: true },
        timestamp: { type: Number, required: true, min: 1577836800000, max: 4102444800000 },
    },
    {
        toObject: {
            virtuals: true
        },
        toJSON: {
            virtuals: true
        }
    }
);

MatchSchema.virtual('predictions', {
    ref: 'predictions',
    localField: '_id',
    foreignField: 'match_id',
    justOne: false,
});

MatchSchema.plugin(mongooseLeanVirtuals);
const Matches = mongoose.model('matches', MatchSchema);

module.exports = Matches;

Predictions.js

const mongoose = require('mongoose');
const mongooseLeanVirtuals = require('mongoose-lean-virtuals');
const Schema = mongoose.Schema;

const PredictionSchema = new Schema(
    {
        _id: { type: Schema.Types.ObjectId, auto: true },
        match_id: { type: Schema.Types.ObjectId, required: true },
    },
    {
        toObject: {
            virtuals: true
        },
        toJSON: {
            virtuals: true
        }
    }
);

PredictionSchema.plugin(mongooseLeanVirtuals);
const Predictions = mongoose.model('predictions', PredictionSchema);

module.exports = Predictions;

Main.js

const mongoose = require('mongoose');
const mongoose_options = {
    autoIndex: true,
    connectTimeoutMS: 10000,
    socketTimeoutMS: 45000,
    family: 4,
    ssl: true
};

const Matches = require('./models/matches');
const Predictions = require('./models/predictions');

(async () => {
    try {
        await mongoose.connect(
            `mongodb+srv://${USER}:${PASS}@${SERVER_ADDR}/${DB_NAME}?retryWrites=true&w=majority`,
            mongoose_options,
        );

        await runTest();

    } catch (error) {
        console.log(error);
    }
})();

const runTest = async () => {
    await Matches.findOneAndUpdate(
        { timestamp: 1577836800000 },
        { timestamp: 1577836800000 },
        { upsert: true }
    );

    const matches = await Matches.find({})
        .populate({ path: 'predictions' })
        .sort({ timestamp: 'desc' })
        .lean({ virtuals: ['predictions'] });

    console.log(matches);
}

What is the expected behavior?

Suppose that there is a match document with a specific mongo _id, but there are no prediction documents to match the populate() query. The code above outputs the following:

// 5.12.14
[
    {
        _id: 6154f33f92736dfa92eda7c9,
        timestamp: 4102444100000,
        predictions: [] // <= this is version 5.x, it returns an empty array.
    },
]
// 6.0.8
[
    {
        _id: new ObjectId("6154f33f92736dfa92eda7c9"),
        timestamp: 4102444100000,
        predictions: undefined // <= this is version 6.x, and it returns undefined instead.
    },
]

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

Node v16.9.1
Mongoose 5.12.14 and 6.0.8


Anyway, my only small request here is to either document this in the migration guide, or if this is not intended behavior and it's a bug, then I suppose that I am reporting it now.

Thank you for your time!

@IslandRhythms IslandRhythms added the can't reproduce Mongoose devs have been unable to reproduce this issue. Close after 14 days of inactivity. label Sep 30, 2021
@IslandRhythms
Copy link
Collaborator

I only got an empty array from the script

const mongoose = require('mongoose');
const mongooseLeanVirtuals = require('mongoose-lean-virtuals');
const Schema = mongoose.Schema;

const MatchSchema = new Schema(
    {
        _id: { type: Schema.Types.ObjectId, auto: true },
        timestamp: { type: Number, required: true, min: 1577836800000, max: 4102444800000 },
    },
    {
        toObject: {
            virtuals: true
        },
        toJSON: {
            virtuals: true
        }
    }
);

MatchSchema.virtual('predictions', {
    ref: 'predictions',
    localField: '_id',
    foreignField: 'match_id',
    justOne: false,
});

MatchSchema.plugin(mongooseLeanVirtuals);
const Matches = mongoose.model('matches', MatchSchema);

const PredictionSchema = new Schema(
    {
        _id: { type: Schema.Types.ObjectId, auto: true },
        match_id: { type: Schema.Types.ObjectId, required: true },
    },
    {
        toObject: {
            virtuals: true
        },
        toJSON: {
            virtuals: true
        }
    }
);

PredictionSchema.plugin(mongooseLeanVirtuals);
const Predictions = mongoose.model('predictions', PredictionSchema);


(async () => {
    try {
        await mongoose.connect(
            `mongodb://localhost:27017/`
        );

        await runTest();

    } catch (error) {
        console.log(error);
    }
})();

const runTest = async () => {
    const matches = await Matches.find({})
        .populate({ path: 'predictions' })
        .sort({ timestamp: 'desc' })
        .lean({ virtuals: ['predictions'] });

    console.log(matches);
}

@barca-reddit
Copy link
Author

barca-reddit commented Sep 30, 2021

Hey @IslandRhythms, thanks for reaching out. You are getting an empty array but that's just the result of the whole Matches.find() query, or in other words there are no results to be found so it's an empty array.

This is my bad, for some reason I assumed someone would actually insert a test match document before trying this, so apologies for that. Please modify the runTest function, and you will be able to reproduce, will also edit my original message.

const runTest = async () => {
    await Matches.findOneAndUpdate(
        { timestamp: 1577836800000 },
        { timestamp: 1577836800000 },
        { upsert: true }
    );

    const matches = await Matches.find({})
        .populate({ path: 'predictions' })
        .sort({ timestamp: 'desc' })
        .lean({ virtuals: ['predictions'] });

    console.log(matches);
}

This results in the following:

[
  {
    _id: new ObjectId("6155f7676f93f16f4ee07c31"),
    timestamp: 1577836800000,
    __v: 0,
    predictions: undefined
  }
]

You can then run npm i [email protected] and npm i, then run the script again and you should get the following:

[
  {
    _id: 6155f7676f93f16f4ee07c31,
    timestamp: 1577836800000,
    __v: 0,
    predictions: []
  }
]

@vkarpov15 vkarpov15 added has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue and removed can't reproduce Mongoose devs have been unable to reproduce this issue. Close after 14 days of inactivity. labels Oct 1, 2021
@vkarpov15 vkarpov15 added this to the 6.0.12 milestone Oct 1, 2021
@IslandRhythms
Copy link
Collaborator

Some deprecation warnings pop up on 5.12.14 so I don't know if thats related to this at all or that was the state of mongoose 5.12.14

const mongoose = require('mongoose');
const mongooseLeanVirtuals = require('mongoose-lean-virtuals');
const Schema = mongoose.Schema;

const MatchSchema = new Schema(
    {
        _id: { type: Schema.Types.ObjectId, auto: true },
        timestamp: { type: Number, required: true, min: 1577836800000, max: 4102444800000 },
    },
    {
        toObject: {
            virtuals: true
        },
        toJSON: {
            virtuals: true
        }
    }
);

MatchSchema.virtual('predictions', {
    ref: 'predictions',
    localField: '_id',
    foreignField: 'match_id',
    justOne: false,
});

MatchSchema.plugin(mongooseLeanVirtuals);
const Matches = mongoose.model('matches', MatchSchema);

const PredictionSchema = new Schema(
    {
        _id: { type: Schema.Types.ObjectId, auto: true },
        match_id: { type: Schema.Types.ObjectId, required: true },
    },
    {
        toObject: {
            virtuals: true
        },
        toJSON: {
            virtuals: true
        }
    }
);

PredictionSchema.plugin(mongooseLeanVirtuals);
const Predictions = mongoose.model('predictions', PredictionSchema);


(async () => {
    try {
        await mongoose.connect(
            `mongodb://localhost:27017/`
        );

        await runTest();

    } catch (error) {
        console.log(error);
    }
})();

const runTest = async () => {
    await Matches.findOneAndUpdate(
        { timestamp: 1577836800000 },
        { timestamp: 1577836800000 },
        { upsert: true }
    );
    const matches = await Matches.find({})
        .populate({ path: 'predictions' })
        .sort({ timestamp: 'desc' })
        .lean({ virtuals: ['predictions'] });

    console.log(matches);
}

@IslandRhythms IslandRhythms added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. and removed has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue labels Oct 4, 2021
@vkarpov15 vkarpov15 modified the milestones: 6.0.12, 6.0.11 Oct 12, 2021
@vkarpov15 vkarpov15 removed this from the 6.0.11 milestone Oct 14, 2021
@vkarpov15 vkarpov15 added plugin and removed confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. labels Oct 14, 2021
@vkarpov15
Copy link
Collaborator

This is an issue with mongoose-lean-virtuals, or rather an issue with mongoose-lean-virtuals that a bug in Mongoose 5.x was hiding. This worked in Mongoose 5 because Mongoose 5 would still set [] on the populated virtual even though lean() was set.

We fixed this in [email protected].

@brentmjohnson
Copy link

We ran into this issue as well during our 5 -> 6 migration. In our case we were not using the mongoose-lean-virtuals package and instead statically setting toJSON: { virtuals: true }, toObject: { virtuals: true } at the parent schema level.

The virtual was defined with the default of justOne: false

Can you confirm it is expected behavior that using lean() with populate virtuals (but without the mongoose-lean-virtuals package) will now return undefined rather than an empty array?

@vkarpov15
Copy link
Collaborator

@brentmjohnson that is correct, lean() without this package will return undefined rather than empty array if there's no results.

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

No branches or pull requests

4 participants