_id field for subdocuments array items no longer being set on retrieval

[ajwootto]

Prerequisites

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

Mongoose version

6.5.2

Node.js version

16.x

MongoDB server version

5.x

Description

In the version of Mongoose I was using before ( 6.0.11 ), if you had data inserted in Mongo that contained a subdocument array containing items without _id fields, Mongoose would fill in those fields with new _ids when the document was retrieved.

On the latest version of Mongoose, that seems to no longer be the case. I'm not sure if this is a deliberate change of behaviour or a regression. See the sample code below:

Steps to Reproduce

import { Schema, model, connect, Types } from 'mongoose'
import * as mongoose from 'mongoose'

interface NestedChild {
    name: string
    _id: Types.ObjectId
}
const nestedChildSchema: Schema = new Schema({ name: String })

interface Parent {
    nestedChild: Types.DocumentArray<NestedChild>
    name?: string
}

const ParentModel = model<Parent>('Parent', new Schema({
    nestedChild: [nestedChildSchema],
    name: String
}))

export async function run() {
    console.log('Running test')
    await connect('mongodb://localhost:27017/')
    const dbConnection = mongoose.connections[0]
    const { insertedId } = await dbConnection.collection('parents').insertOne({
        nestedChild: [{ name: 'foo' }],
        name: 'Parent'
    })

    const parentRaw = await dbConnection.collection('parents').findOne({ _id: insertedId })
    const parentFound = await ParentModel.findOne({ _id: insertedId })

    // this is always undefined
    console.log('raw data', parentRaw?.nestedChild[0]._id)
    // on mongoose 6.0.11 this is an object id
    // on mongoose 6.5.2 this is undefined
    console.log('after retrieval', parentFound?.nestedChild[0]._id)
}

run()

Expected Behavior

Unless this was deliberate, I would expect this behaviour to be unchanged and for mongoose to fill in the _id fields of the subdocuments upon retrieval

[AbdelrahmanHafez]

I can confirm that this is an issue. This was introduced on 6.2.4, specifically in 4c85c70 as a performance improvement to prevent memory spikes re #10400.

I agree that this is a breaking change and would've preferred if we did not ship it in a patch version, but I think the performance benefit is generally worth it.

What's your use-case here? Do you have another backend connected to your MongoDB server and inserting data there without the ids?

Would an option to opt-in in getting ids when they do not exist on the MongoDB server work for you?
@vkarpov15 FYI

[AbdelrahmanHafez]

JS repro script

12264.js

'use strict';
const mongoose = require('mongoose');
const { Schema } = mongoose;
const assert = require('node:assert');

const nestedChildSchema = new Schema({ name: String });


const ParentModel = mongoose.model('Parent', new Schema({
  nestedChild: [nestedChildSchema],
  name: String
}));

run().catch(console.error);

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

  const { insertedId } = await ParentModel.collection.insertOne({
    nestedChild: [{ name: 'foo' }],
    name: 'Parent'
  });

  const parentFound = await ParentModel.findOne({ _id: insertedId });

  assert.ok(parentFound.nestedChild[0]._id);
  console.log('Assertions passed')
  console.log(mongoose.version)
}

Output

$ node 12264.js
AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert.ok(parentFound.nestedChild[0]._id)

    at run (E:\dev\mongoose\test.js:26:10)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: undefined,
  expected: true,
  operator: '=='
}

[ajwootto]

@AbdelrahmanHafez our use case for this is actually strictly for testing

I only noticed it because after updating, our tests which relied on seeding raw test data directly into the database (not through mongoose) ended up failing because mongoose no longer applied the _ids to the documents. An opt-in setting would work fine for this case though.

It does feel like a notable exception to the standard behaviour of casting everything from the database into something that is valid according to the schema, though.

[AbdelrahmanHafez]

Glad that this did not affect your production environment.
I agree that this goes against how we generally do things for non-lean documents.

Perhaps for now if we introduce a new option, it should be opt-out instead of opt-in to avoid potentially breaking other people's code, then in v7 we can change the default behavior to NOT add the _id by default, and only add it for those who want.

What do you think? @vkarpov15