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

Saving modified subdocuments in multidimensional arrays throws an error #8926

Closed
beaulac opened this issue May 3, 2020 · 3 comments
Closed
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@beaulac
Copy link

beaulac commented May 3, 2020

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

What is the current behavior?
Unable to save modified subdocuments if they are in a multidimensional array. Saving such a document throws an error.

The 'modified path' is missing one of the indices in the subdocument's path, and so saving throws an error because it is trying to set a property on an array which is unsupported in Mongo.

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

const SubchildSchema = new mongoose.Schema({ title: String });

const ChildSchema = new mongoose.Schema({
	subchilds: [[SubchildSchema]],
});

const ParentModel = mongoose.model(
	'Parent',
	{ children: [ChildSchema] },
);

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

	const { _id } = await ParentModel.create({
		children: [
			{
				subchilds: [
					[{}],
				],
			},
		],
	});

	const parent = await ParentModel.findById(_id);
	parent.children[0].subchilds[0][0].title = 'foo';
	return parent.save();
}

repro()
	.catch(console.error)
	.finally(mongoose.disconnect);

MongoError: Cannot create field 'title' in element {0: [ { _id: ObjectId('5eaf31d48ba42b1eddc904b8') } ]}

Seems the path calculated in _markModified in core_array.js is missing the index of the nested array. The path is
'parent.children.0.subchilds.0.title' but should be 'parent.children.0.subchilds.0.0.title'.

What is the expected behavior?

Able to save this subdocument update.

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

@AbdelrahmanHafez
Copy link
Collaborator

AbdelrahmanHafez commented May 3, 2020

Welcome @beaulac
Please check the question below, does this help clarify the issue?
https://mongoosejs.com/docs/faq.html#array-changes-not-saved

@beaulac
Copy link
Author

beaulac commented May 3, 2020

Sorry, I should have been clearer with the title; the issue isn't that the changes are not saved, but rather that it throws an error because it is trying to save an invalid path.

@AbdelrahmanHafez
In the link provided:

This only affects when you set an array index directly. If you set a path on a document array element, you do not need to use markModified().

In the repro script provided, the only modification is a path on a document array. The issue seems to be that the automatic markModified() performed by the setter is giving the wrong path.

I created a branch with a test and a hacky fix (only works for 2-dimensional arrays). I'm not familiar with mongoose internals so I'm really not sure this function is even the right place to try to address it – perhaps it should be addressed by making sure the parent is correctly determined for the 2nd-level array.

@beaulac beaulac changed the title Can't save modified subdocuments in multidimensional arrays Saving modified subdocuments in multidimensional arrays throws an error May 3, 2020
@AbdelrahmanHafez
Copy link
Collaborator

Oh, okay. I can confirm that this is a bug.

I played with the script you provided a little bit, and can confirm that the issue is we're sending the wrong index in the update operation.

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

run().catch(console.error);

async function run () {
  await mongoose.connect('mongodb://localhost:27017/test', {
    useNewUrlParser: true,
    useUnifiedTopology: true
  });

  await mongoose.connection.dropDatabase();

  const bookSchema = new Schema({ title: String });

  const aisleSchema = new Schema({
    shelves: [[bookSchema]]
  });

  const librarySchema = new Schema({ aisles: [aisleSchema] });

  const Library = mongoose.model('Library', librarySchema);

  await Library.create({
    aisles: [{ shelves: [[{ title: 'Clean Code' }]] }]
  });

  const library = await Library.findOne();
  library.aisles[0].shelves[0][0].title = 'Refactoring';

  await library.save();

  const foundLibrary = await Library.findOne();

  assert.equal(foundLibrary.aisles[0].shelves[0][0].title, 'Refactoring');
  console.log('All assertions passed.');
}

Current output:

{ '$set': { 'aisles.0.shelves.0.title': [ 'Refactoring' ] } }

Expected output:

{ '$set': { 'aisles.0.shelves.0.0.title': [ 'Refactoring' ] } }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Projects
None yet
Development

No branches or pull requests

3 participants