Automattic / mongoose

MongoDB object modeling designed to work in an asynchronous environment.
https://mongoosejs.com
MIT License
26.92k stars 3.84k forks source link

Nested, empty objects are not saved on type Mixed or Object #8505

Open jsdevtom opened 4 years ago

jsdevtom commented 4 years ago

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

What is the current behavior? Attempts to save nested, empty objects in a field of type Mixed are futile; The empty object is not saved.

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

const mongoose = require('mongoose');
const util = require('util');

mongoose.connect('mongodb://localhost/test', {useNewUrlParser: true});

const db = mongoose.connection;

db.on('error', console.error.bind(console, 'connection error:'));

db.once('open', function() {
    const mainSchema = new mongoose.Schema({
        nested: {
            type: mongoose.Schema.Types.Mixed,
            required: true,
        },
    }, {strict: false});

    const Main = mongoose.model('Main', mainSchema);

    const mainInstance = new Main({
        nested: {
            a: [
                1,
                2,
                {},
                {
                    a: {
                        anyOtherProp: '', // <-- Try commenting this line out
                        b: {},
                    },
                },
            ],
        },
    });

    console.debug('mainInstance: ', util.inspect(mainInstance, false, null, true ));
    /* ^^Result:
    {
        _id: 5e1d6959890dec46d573dab2,
        nested: {
            a: [
                1,
                2,
                {},
                {
                    a: {
                        anyOtherProp: ''
                    }
                }
            ]
        }
    }
    */

    mainInstance.save(function (err, saved) {
        if (err) return console.error(err);
        console.debug('saved: ', util.inspect(saved, false, null, true ));
        /* ^^Result:
        {
            _id: 5e1d6959890dec46d573dab2,
            nested: {
                a: [
                    1,
                    2,
                    {},
                    {
                        a: {
                            anyOtherProp: ''
                        }
                    }
                ]
            },
            __v: 0
        }
        */

        Main.find(function (err, instances) {
            if (err) return console.error(err);
            console.debug('instances: ', util.inspect(instances, false, null, true )) // Result  is same as previous
        })
    });
});

Please notice that the nested property b is missing in the example above. In my case, the object I am trying to save is dynamic and so it is not possible to define a recursive schema.

What is the expected behavior? I expect the all nested objects, be they empty or filled, to be recursively saved on type Mixed or equivalent types.

~I have had limited success when recursively iterating all nested objects and calling the markModified method. However, if the field with the type Mixed has the required property set to true, the required validation is also applied to all nested fields, and so fields with type null are marked as invalid.~

Even recursively marking all properties as modified doesn't result in the empty, nested objects being saved.

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

"mongo: "4.2.2",
"mongoose": "5.8.7",
"node": "10.16.0"
jsdevtom commented 4 years ago

The solution was to add minimize: false to the schema. I strongly suggest changing this setting to false by default. Changing the object asked to be saved in order to save memory should never be the default behaviour in my opinion

vkarpov15 commented 4 years ago

minimize isn't about saving memory, it's more about avoiding saving empty nested objects. If you have a schema like this:

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

When you create a new empty document, typeof doc.nested === 'object', even though doc.nested looks undefined.

We'll look into whether we can skip minimize for mixed types.

jsdevtom commented 4 years ago

Thanks for your reply. Got it :+1: Would it be possible/desirable to change the behaviour of minify to save the empty object if one was provided and not to do so, if one wasn't? This would satisfy both your and my use case I believe.

vkarpov15 commented 4 years ago

It may be possible. However, it isn't as easy as you might think, because doc.nested !== undefined even though console.log(doc.nested) prints undefined. We'll investigate that for a future release.

Marchelune commented 2 years ago

I'm trying to upgrade from Mongoose 5.12.10 to 6.2.8, and there seem to be a breaking changes with this beahviour - but I am not sure whether it lies within Mongoose or the underlying Mongo client.

Similar to @jsdevtom we have a subdocument with only optional fields, so the resulting document subdocument can be empty. From a code point of view however, it should always be defined.

Here are sample models - we're using nestjs with mongoose, but this is just a syntactic sugar to help with schema definition from typescript files:

import { Prop, Schema, SchemaFactory } from '@nestjs/mongoose';
import { Document } from 'mongoose';

@Schema({ _id: false })
class Bar {
    @Prop()
    optionalFieldOne?: string;

    @Prop()
    optionalFieldTwo?: string;
}

// In vanilla JS we would do mongoose.schema(...)
const BarSchema = SchemaFactory.createForClass(Bar);

@Schema({ collection: 'foo' })
export class Foo {
    @Prop({ required: true })
    someProperty!: Date;

    @Prop({ required: true, type: BarSchema })
    barProperty!: Bar;
}

export const FooSchema = SchemaFactory.createForClass(Foo);
export type FooDocument = Foo & Document;

Since the 6.2.8 upgrade, if we don't specify minimize: false, then this may throw an exception:

// Imagine we retrieved a foo document from the collection somehow
const foo: Foo;
// uh oh, TypeError: Cannot read properties of undefined (reading 'optionalFieldOne')
if (foo.barProperty.optionalFieldOne) { 
    // do something
}

Setting minimize: false solves the issue for us, but I could not find it in the breaking changes note

vkarpov15 commented 2 years ago

@Marchelune the below script executes successfully with Mongoose 6.3.1 and nestjs/mongoose@9.0.3. Can you please modify the below script to demonstrate your issue?

import { Prop, Schema, SchemaFactory } from '@nestjs/mongoose';
import { connection, model, connect, Document } from 'mongoose';

@Schema({ _id: false })
class Bar {
    @Prop({ type: String })
    optionalFieldOne?: string;

    @Prop({ type: String })
    optionalFieldTwo?: string;
}

// In vanilla JS we would do mongoose.schema(...)
const BarSchema = SchemaFactory.createForClass(Bar);

@Schema({ collection: 'foo' })
class Foo {
    @Prop({ required: true, type: Date })
    someProperty!: Date;

    @Prop({ required: true, type: BarSchema })
    barProperty!: Bar;
}

const FooSchema = SchemaFactory.createForClass(Foo);

async function run() {
  await connect('mongodb://localhost:27017/test');
  await connection.dropDatabase();
  const Foo = model('Foo', FooSchema);

  await Foo.create({});
  const foo = await Foo.findOne({});
  // @ts-ignore
  foo.barProperty.optionalFieldOne; 

  console.log('Done');
}
devesh-anand commented 4 days ago

using mongoose version 8.2.2. The bug still persists for save() and .insert().

Was very annoying to debug. minimize: false should ideally be a default property.