Aggregations with filters on ObjectIds fail in MongooseQueryService

[h4rm]

Version: 0.22

When a DTO has a reference which is specified as follows:

@ObjectType('Document')
export class DocumentDTO {
  @FilterableField(() => String, { nullable: true })
  userId?: Types.ObjectId;
}

and a schema

@Schema({ })
export class Document {
  @Prop({ type: SchemaTypes.ObjectId, ref: 'User', required: false })
  userId?: Types.ObjectId;
}

currently I cannot filter for the userId in an aggregation:

query countDocuments($filter: DocumentAggregateFilter) {
  documentAggregate(filter: $filter) {
    count {
      id
    }
  }
}

when I call countDocuments(filter: {userId:{eq: "123456"} }) because I get the following error:

TypeError: Cannot destructure property '_id' of 'undefined' as it is undefined.
    at Function.convertToAggregateResponse (/Users/ben/Documents/coding/dexter/server/node_modules/@nestjs-query/query-mongoose/dist/src/query/aggregate.builder.js:21:41)
    at DocumentService.aggregate (/Users/ben/Documents/coding/dexter/server/node_modules/@nestjs-query/query-mongoose/dist/src/services/mongoose-query.service.js:52:41)
    at processTicksAndRejections (node:internal/process/task_queues:94:5)
    at async target (/Users/ben/Documents/coding/dexter/server/node_modules/@nestjs/core/helpers/external-context-creator.js:76:28)
    at async /Users/ben/Documents/coding/dexter/server/node_modules/@nestjs/core/helpers/external-proxy.js:9:24

Basically the mongoose aggregate function doesn't return a valid result here. Also, in some cases where the was no result to an aggregate query, I also got this error and not the number 0 as expected.

Filtering for other properties works and I can also filter for a specific userId in a normal query.
I think this is a bug with some unhandled case, but maybe I am misusing something.

[h4rm]

This is probably due to the fact that aggregate expects the following:

document.aggregate([
  {
    $match: { userId: ObjectId('1234') }
  }
])

@doug-martin Any plans on extending the aggregate query builder to convert fields of type ID to the mongoose ObjectId?

[doug-martin]

@h4rm thank you for the bug report and tracking down what's causing it, that helps a lot! There will need to be some investigation on how to identify the fields that should be turned into an objectId. @smolinari do you know a way of doing this?

[h4rm]

So I dug a little bit deeper and overwrite the aggregate in my DocumentService.

Two things actually fix the problem:

Number 1

Replace all string Ids to by the same ObjectIds. I am doing it manually right now but I think it should be possible to gather that information from the schema somehow and do it automatically in the buildAggreateQuery which uses the default buildFilterQuery internally. Maybe I could get help on that.

Number 2

Call convertToAggregateRepsonse as followed to prevent the above mentioned error when the mongoose aggregate returns an empty array:

    return AggregateBuilder.convertToAggregateResponse({ _id: null, ...aggResult[0] });

This will result in the counts to return as null. Of course, the nicest thing would be to return 0 for all aggregates but at least the request doesn't crash in that case.

[smolinari]

@doug-martin - I'm not so deep into the inner workings of Mongoose. The person who might know better is @JohnMcInall.

Scott

[doug-martin]

@h4rm this has been fixed in v0.25.1!

[h4rm]

That's fantastic. Thanks for your amazing effort guys.

…

On Wed, Apr 7, 2021, 7:54 AM Doug Martin ***@***.***> wrote: @h4rm <https://github.com/h4rm> this has been fixed in v0.25.1! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#881 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAELBTXHSMUA3GRQ2HTPGRTTHPXRDANCNFSM4XWYFG3Q> .

[smolinari]

Yes. Thanks so much for covering this Doug.

Scott

[h4rm]

@doug-martin Is it possible that the same problem persists on the subscription part of the filters? I was secretly hoping it's using the same code underneath :-)