pre.remove middleware of objects in array are never called (with 5.0.9)

[jmcollin78]

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

What is the current behavior?
I create an Embedded object named FileSpace into an array of object named Space. When removing FileSpace, pre remove middleware is never call (but pre validate middleware is called)

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

Here is a repro code:

'use strict';

var mongoose = require('mongoose'),
	Schema = mongoose.Schema;

mongoose.set('debug', true);


/**
 *  A file
 */
var FileSpaceSchema = new Schema({
	fileKey: {
		type: String,
		required: true
	}
});

// Normally called 
FileSpaceSchema.pre('validate', function (next) {
	console.log('Calling FileSpace.pre.validate me="%s"', this.fileKey);
	next();
});

// Never called !
FileSpaceSchema.pre('remove', function(next) {
	console.log(' !!! Calling FileSpace.pre.remove fileKey="%s"', this.fileKey);
	next();
});

let FileSpace = mongoose.model('FileSpace', FileSpaceSchema);

/**
 * A space containing an array of files
 */
var SpaceDocSchema = new Schema({
	label: {
		type: 'string',
		required: true
	},
	files: [FileSpaceSchema]
});

SpaceDocSchema.pre('validate', function (next) {
	console.log('Calling SpaceDocSchema.preValidate hook spaceDoc is "%s"', this.label);
	next();
});

SpaceDocSchema.pre('remove', function (next) {
	console.log('Calling Space.post.remove spaceDoc is "%s"', this.label);
	next();
});

let SpaceDoc = mongoose.model('SpaceDoc', SpaceDocSchema);

console.log('--> Starting');

console.log('--> Creating a space');
let space = new SpaceDoc({
	label: 'The SpaceDoc'
}),
	removedFile;

// connect to mongo
mongoose.connect('mongodb://mongodbsrv/clouderialTestDB?w=1&j=true');

mongoose.connection.on('open', () => {
	console.log('Connection to MongoDB is effective');
	
	space.save()
		.then((s) => {
			space = s;
			console.log('Created space is "%s"', s.label);
			
			console.log('--> Creating a FileSpace');
			return new FileSpace({fileKey : 'fileSpace', spaceLabel:'The space label'}).save();
		})
		.then((fs) => {
			console.log('Created FileSpace is "%s"', fs.fileKey);
			console.log('--> Add fileSpace into SpaceDoc.files');
			space.files.push(fs);
			space.markModified('files');
			return space.save();
		})
		.then((s) => {
			space = s;
			console.log('Updated space is "%s", nbFiles="%d"', space.label, space.files.length);
			console.log('--> Remove fileSpace from space');
			removedFile = space.files[0];
			space.files.splice(0, 1);
//			space.files = [];
			space.markModified('files');
			console.log('--> Update space without file');
			return space.save();
		})
		.then((s) => {
			space = s;
			console.log('Updated space is "%s", nbFiles="%d"', space.label, space.files.length);
			console.log('--> Remove fileSpace');
			return removedFile.remove();
		})
		.then(() => {
			console.log('--> Should see the call to pre.remove of FileSpace');
			console.log('--> Remove space');
			return space.remove();
		})
		.catch(console.error);
});

setTimeout(() => {
	console.log('--> Close MongoDB connection');
	mongoose.connection.close();
}, 3000);

The output is the following:

$ npm start

> test@0.0.1 start /datas/cld-apps/test
> NODE_PATH=/home/vagrant/cld-apps/node_modules:. TZ=Europe/Paris node test.js

--> Starting
--> Creating a space
Connection to MongoDB is effective
Calling SpaceDocSchema.preValidate hook spaceDoc is "The SpaceDoc"
Mongoose: spacedocs.insert({ label: 'The SpaceDoc', files: [], _id: ObjectId("5aa18e47f13311778fdc3beb"), __v: 0 })
Created space is "The SpaceDoc"
--> Creating a FileSpace
Calling FileSpace.pre.validate me="fileSpace"
Mongoose: filespaces.insert({ fileKey: 'fileSpace', _id: ObjectId("5aa18e47f13311778fdc3bec"), __v: 0 })
Created FileSpace is "fileSpace"
--> Add fileSpace into SpaceDoc.files
Calling SpaceDocSchema.preValidate hook spaceDoc is "The SpaceDoc"
Calling FileSpace.pre.validate me="fileSpace"
Mongoose: spacedocs.update({ _id: ObjectId("5aa18e47f13311778fdc3beb"), __v: 0 }, { '$set': { files: [ { fileKey: 'fileSpace', _id: ObjectId("5aa18e47f13311778fdc3bec"), __v: 0 } ] }, '$inc': { __v: 1 } })
Updated space is "The SpaceDoc", nbFiles="1"
--> Remove fileSpace from space
--> Update space without file
Calling SpaceDocSchema.preValidate hook spaceDoc is "The SpaceDoc"
Mongoose: spacedocs.update({ _id: ObjectId("5aa18e47f13311778fdc3beb"), __v: 1 }, { '$set': { files: [] }, '$inc': { __v: 1 } })
Updated space is "The SpaceDoc", nbFiles="0"
--> Remove fileSpace
--> Should see the call to pre.remove of FileSpace
--> Remove space
Calling Space.post.remove spaceDoc is "The SpaceDoc"
Mongoose: spacedocs.remove({ _id: ObjectId("5aa18e47f13311778fdc3beb") }, {})
--> Close MongoDB connection

What is the expected behavior?
We should see the log line:

!!! Calling FileSpace.pre.remove fileKey

Please mention your node.js, mongoose and MongoDB version.
Node 9.5.0, Mongoose 5.0.9, MongoDB 3.6.3, Mongo driver: 3.0.3

EDIT: try with 5.0.9.

Hi @jmcollin78 Thanks for the thorough repro script!

In your example you are calling .remove() on an embedded document. Which according to the docs is equivalent to calling pull on the subdocument. This does not fire a remove event. The subdocs .remove() middleware will only run when you call .remove() on the parent document. Here is an example:

'use strict'

const mongoose = require('mongoose')
mongoose.connect('mongodb://localhost/test')
const Schema = mongoose.Schema

const subSchema = new Schema({
  name: String
})

subSchema.pre('save', (next) => {
  console.log('subSchema.pre:save on', this)
  next()
})

subSchema.pre('remove', (next) => {
  console.log('subSchema.pre:remove!')
  next()
})

const schema = new Schema({
  name: String,
  subs: [subSchema]
})

schema.pre('save', (next) => {
  console.log('schema.pre:save!', this)
  next()
})

schema.pre('remove', (next) => {
  console.log('schema.pre:remove!', this)
  next()
})

const Test = mongoose.model('tests', schema)

const obj = {
  name: 'Outer',
  subs: [{
    name: 'Franklin'
  }]
}

const test1 = new Test(obj)
const test2 = new Test(obj)

test1.save((err, doc) => {
  if (err) { return console.error(err) }
  console.log('calling remove on subdoc:')
  doc.subs[0].remove()
  doc.save()
})

test2.save((err, doc) => {
  if (err) { return console.error(err) }
  console.log('calling remove on doc:')
  doc.remove()
  mongoose.connection.close()
})

output:

InspiredMacPro:6224 lineus$ node index.js
subSchema.pre:save on {}
subSchema.pre:save on {}
schema.pre:save! {}
schema.pre:save! {}
calling remove on subdoc:
schema.pre:save! {}
calling remove on doc:
subSchema.pre:remove!
schema.pre:remove! {}
InspiredMacPro:6224 lineus$

as you can see, the remove hooks only get run when you call remove on the parent document. but in both cases, remove and save, the subdoc's hooks get run first, then the parents.

[jmcollin78]

Many thanks for this !
I understand your repro case, and in fact you have confirm my observation. But you don't give an explanation but only "it is like that".

If I remove a doc or a subdoc, I guess it should be removed, and subsequently, remove hooks should be called.
I can't understand why doc.subs[0].remove() don't call the pre.remove hook. Do you think it's normal and if yes, what can be the reason ?

Thanks for your explanation.

@jmcollin78 I'm pretty new to mongoose and mongodb, rather than guess or make assumptions, I'll CC some folks that will have the answer.

@vkarpov15 @varunjayaraman What is the reasoning behind only hooking a subdocument's remove when the parent is removed and not when the remove call is made on just the subdoc?

I looked for existing open/closed bugs that reference this specific question, but if they exist I couldn't find them. Thanks!

[vkarpov15]

Because the alternative would be to make doc.subs[0].remove() call doc.save(), which is weird if you need to remove multiple docs. In general, the subdoc isn't removed until you call doc.remove() or doc.save(), because there's no way to remove a subdoc without updating the parent doc.

[jmcollin78]

If I add a subdoc and call doc.save() the pre('save') callback is called.
If I remove a subdoc, I suppose that pre('remove') will be called too.
There is something missing in my understanding. It's pretty confusing to have two different behaviours (that makes me think about a bug initially).

[pascallemoine]

Hi there,

I agree with @jmcollin78 : pre/post('remove') should be triggered on subdocuments.

Here is why, each child has a file attached like so :

const Child = new Schema({
  filename: String
})

Child.post('remove', () => {
  fs.unlink(this.filename)
})

const Parent = new Schema({
  children: [Child]
})

So now, if I call parent.remove() the children's post('remove') hook is triggered and delete the file.

But If I call parent.children.id(id).remove(), no hook is triggered and I have to delete the file manually, which is weird according to the code.

So in my opinion it could be an improvement of the hook feature.

What's your opinion @vkarpov15 ?

[jmcollin78]

Thx @pascallemoine, I feel so lonely with my problem....
When @vkarpov15 says "there's no way to remove a subdoc without updating the parent doc", I agree with that but updating the parent list (with a subdoc removed from the list) don't call the hook also.
This lead to confusion and makes some resources not cleaned after removal.

[georgehess]

@jmcollin78 did you ever get an adequate answer to your question? My project was using mongoose v4 and it supported post-remove middleware for subdocuments in the exact way you described. It wasn't until we upgraded to v5 that we lost the ability to run middleware whenever a subdocument was removed. Seems strange to me that they would remove that piece of functionality, but as far as I can tell, they have.

[georgehess]

For what it's worth, I noticed that mongoose uses kareem for middleware hooks which can be executed manually. There is a reference to kareem in the subdoc's schema (subdoc.schema.s.hooks). So we created our own plugin which executes a subdoc's pre/post middleware when it is removed. We use bluebird for our promise library and it has a helpful fromCallback function that resolves a promise when the middleware calls next() or rejects the promise if it is passed an arg next(new Error())

Bluebird.fromCallback(callback =>
	subdoc.schema.s.hooks.execPre("remove", subdoc, callback)
)
.then(() => {
	subdoc.remove();
	return doc.save();
})
.then(() =>
	Bluebird.fromCallback(callback =>
		subdoc.schema.s.hooks.execPost("remove", query, [subdoc], callback)
	)
)
.return(doc);

[vkarpov15]

@georgehess can you please open a new issue and follow the issue template?