Automattic / mongoose

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

hook is not of type Aggregate #14903

Closed dragontaek-lee closed 1 month ago

dragontaek-lee commented 1 month ago

Prerequisites

Mongoose version

8.6.1

Node.js version

20.10.0

MongoDB server version

7.0.9

Typescript version (if applicable)

No response

Description

this context in hooks are not of type Aggregate.

This issue arises when custom static methods overwrite built-in aggregate functions. (as seen in the implementation in mongoose-delete)

This issue is similar to the past one (https://github.com/Automattic/mongoose/issues/7790), where the context in hooks was not of type Query. I noticed that it has been fixed in this PR (https://github.com/Automattic/mongoose/commit/f848b54d33cc5f61be3b1f0539370f9c43d9d4e2), which filters custom static methods that overwrite existing query middleware in applyStaticHooks.js.

Steps to Reproduce

I created a minimum reproduction repository. (https://github.com/dragontaek-lee/mongoose-staticHooks-overwrite-support-more-cases)

I created some test cases to reproduce this issue in the reproduction repository. The code can be found in index.js, test/aggregate.middleware.test.js, or test/query.middleware.test.js. Tests and scripts can also be run using the npm commands mentioned in the README.

The summarized reproduction code is as follows:

schema1.statics.aggregate = function(pipeline) {
    let match = { $match: { deleted: { '$ne': false } } };

    if (pipeline.length && pipeline[0].$match) {
        pipeline[0].$match.deleted = { '$ne': false };
    } else {
        pipeline.unshift(match);
    }

    const query = Model.aggregate.apply(this, [pipeline]);
    return query;
};

Assume that the static method overwrites the built-in Mongoose aggregate function.

schema1.pre('aggregate', function(next) {
    console.log(this);
    next();
});

schema1.post('aggregate', function() {
    console.log(this);
});

In this situation, also assume that there are pre and post hooks for aggregate.

Test 1

Test 2

Expected Behavior

Since the issues with query middleware were fixed in this PR (https://github.com/Automattic/mongoose/commit/f848b54d33cc5f61be3b1f0539370f9c43d9d4e2), it is expected that if a custom static method overwrites an existing aggregate middleware, the middleware should not be applied by default, just as it works with query middleware.

I’m going to submit the PR right away for this.

dragontaek-lee commented 1 month ago

Fix PR: https://github.com/Automattic/mongoose/pull/14904