Skip to content

Commit

Permalink
fix(mongoose,typegoose,#881): Allow string objectId filters
Browse files Browse the repository at this point in the history
  • Loading branch information
doug-martin committed Apr 7, 2021
1 parent d0981c1 commit 99658ec
Show file tree
Hide file tree
Showing 17 changed files with 257 additions and 101 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,6 @@ type FilterFieldComparisonType<FieldType, IsKeys extends true | false> = FieldTy
? StringFieldComparisons // eslint-disable-next-line @typescript-eslint/ban-types
: FieldType extends boolean | Boolean
? BooleanFieldComparisons
: FieldType extends null | undefined | never
? BooleanFieldComparisons // eslint-disable-next-line @typescript-eslint/no-explicit-any
: FieldType extends number | Date | RegExp | bigint | BuiltInTypes[] | symbol
? CommonFieldComparisonType<FieldType>
: FieldType extends Array<infer U>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export class TestEntity extends Document {
dateType!: Date;

@Prop({ type: SchemaTypes.ObjectId, ref: 'TestReference' })
testReference?: Types.ObjectId;
testReference?: Types.ObjectId | string;

@Prop([{ type: SchemaTypes.ObjectId, ref: 'TestReference' }])
testReferences?: Types.ObjectId[];
Expand Down
59 changes: 56 additions & 3 deletions packages/query-mongoose/__tests__/query/comparison.builder.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { CommonFieldComparisonBetweenType } from '@nestjs-query/core';
import { TestEntity } from '../__fixtures__/test.entity';
import { model, Types } from 'mongoose';
import { TestEntity, TestEntitySchema } from '../__fixtures__/test.entity';
import { ComparisonBuilder } from '../../src/query';

describe('ComparisonBuilder', (): void => {
const createComparisonBuilder = () => new ComparisonBuilder<TestEntity>();
const createComparisonBuilder = () => new ComparisonBuilder<TestEntity>(model('TestEntity', TestEntitySchema));

it('should throw an error for an invalid comparison type', () => {
// @ts-ignore
Expand All @@ -18,6 +19,14 @@ describe('ComparisonBuilder', (): void => {
},
});
});

it('should convert convert query fields to objectIds if the field is an objectId', (): void => {
expect(createComparisonBuilder().build('testReference', 'eq', '5f74af112fae2b251510e3ad')).toEqual({
testReference: {
$eq: Types.ObjectId('5f74af112fae2b251510e3ad'),
},
});
});
});

it('should build neq sql fragment', (): void => {
Expand Down Expand Up @@ -153,6 +162,14 @@ describe('ComparisonBuilder', (): void => {
},
});
});

it('should convert convert query fields to objectIds if the field is an objectId', (): void => {
expect(createComparisonBuilder().build('testReference', 'in', ['5f74af112fae2b251510e3ad'])).toEqual({
testReference: {
$in: [Types.ObjectId('5f74af112fae2b251510e3ad')],
},
});
});
});

describe('notIn comparisons', () => {
Expand All @@ -164,6 +181,14 @@ describe('ComparisonBuilder', (): void => {
},
});
});

it('should convert convert query fields to objectIds if the field is an objectId', (): void => {
expect(createComparisonBuilder().build('testReference', 'notIn', ['5f74af112fae2b251510e3ad'])).toEqual({
testReference: {
$nin: [Types.ObjectId('5f74af112fae2b251510e3ad')],
},
});
});
});

describe('between comparisons', () => {
Expand All @@ -174,6 +199,20 @@ describe('ComparisonBuilder', (): void => {
});
});

it('should convert convert query fields to objectIds if the field is an objectId', (): void => {
expect(
createComparisonBuilder().build('testReference', 'between', {
lower: '5f74af112fae2b251510e3ad',
upper: '5f74af112fae2b251510e3ad',
}),
).toEqual({
testReference: {
$gte: Types.ObjectId('5f74af112fae2b251510e3ad'),
$lte: Types.ObjectId('5f74af112fae2b251510e3ad'),
},
});
});

it('should throw an error if the comparison is not a between comparison', (): void => {
const between = [1, 10];
expect(() => createComparisonBuilder().build('numberType', 'between', between)).toThrow(
Expand All @@ -190,10 +229,24 @@ describe('ComparisonBuilder', (): void => {
});
});

it('should convert convert query fields to objectIds if the field is an objectId', (): void => {
expect(
createComparisonBuilder().build('testReference', 'notBetween', {
lower: '5f74af112fae2b251510e3ad',
upper: '5f74af112fae2b251510e3ad',
}),
).toEqual({
testReference: {
$lt: Types.ObjectId('5f74af112fae2b251510e3ad'),
$gt: Types.ObjectId('5f74af112fae2b251510e3ad'),
},
});
});

it('should throw an error if the comparison is not a between comparison', (): void => {
const between = [1, 10];
expect(() => createComparisonBuilder().build('numberType', 'notBetween', between)).toThrow(
'Invalid value for not between expected {lower: val, upper: val} got [1,10]',
'Invalid value for notbetween expected {lower: val, upper: val} got [1,10]',
);
});
});
Expand Down
6 changes: 3 additions & 3 deletions packages/query-mongoose/__tests__/query/where.builder.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { Filter } from '@nestjs-query/core';
import { FilterQuery } from 'mongoose';
import { TestEntity } from '../__fixtures__/test.entity';
import { FilterQuery, model } from 'mongoose';
import { TestEntity, TestEntitySchema } from '../__fixtures__/test.entity';
import { WhereBuilder } from '../../src/query';

describe('WhereBuilder', (): void => {
const createWhereBuilder = () => new WhereBuilder<TestEntity>();
const createWhereBuilder = () => new WhereBuilder<TestEntity>(model('TestEntity', TestEntitySchema));

const assertFilterQuery = (filter: Filter<TestEntity>, expectedFilterQuery: FilterQuery<TestEntity>): void => {
const actual = createWhereBuilder().build(filter);
Expand Down
64 changes: 42 additions & 22 deletions packages/query-mongoose/src/query/comparison.builder.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { CommonFieldComparisonBetweenType, FilterComparisonOperators } from '@nestjs-query/core';
import escapeRegExp from 'lodash.escaperegexp';
import { FilterQuery, Document } from 'mongoose';
import { Model as MongooseModel, FilterQuery, Document, Types, Schema } from 'mongoose';
import { QuerySelector } from 'mongodb';
import { BadRequestException } from '@nestjs/common';
import { getSchemaKey } from './helpers';

/**
Expand Down Expand Up @@ -33,7 +34,10 @@ export class ComparisonBuilder<Entity extends Document> {
isnot: '$ne',
};

constructor(readonly comparisonMap: Record<string, string> = ComparisonBuilder.DEFAULT_COMPARISON_MAP) {}
constructor(
readonly Model: MongooseModel<Entity>,
readonly comparisonMap: Record<string, string> = ComparisonBuilder.DEFAULT_COMPARISON_MAP,
) {}

/**
* Creates a valid SQL fragment with parameters.
Expand All @@ -52,39 +56,32 @@ export class ComparisonBuilder<Entity extends Document> {
let querySelector: QuerySelector<Entity[F]> | undefined;
if (this.comparisonMap[normalizedCmp]) {
// comparison operator (e.b. =, !=, >, <)
querySelector = { [this.comparisonMap[normalizedCmp]]: val };
querySelector = { [this.comparisonMap[normalizedCmp]]: this.convertQueryValue(field, val as Entity[F]) };
}
if (normalizedCmp.includes('like')) {
querySelector = (this.likeComparison(normalizedCmp, val) as unknown) as QuerySelector<Entity[F]>;
}
if (normalizedCmp === 'between') {
// between comparison (field BETWEEN x AND y)
querySelector = this.betweenComparison(val);
}
if (normalizedCmp === 'notbetween') {
// notBetween comparison (field NOT BETWEEN x AND y)
querySelector = this.notBetweenComparison(val);
if (normalizedCmp.includes('between')) {
querySelector = this.betweenComparison(normalizedCmp, field, val);
}
if (!querySelector) {
throw new Error(`unknown operator ${JSON.stringify(cmp)}`);
throw new BadRequestException(`unknown operator ${JSON.stringify(cmp)}`);
}
return { [schemaKey]: querySelector } as FilterQuery<Entity>;
}

private betweenComparison<F extends keyof Entity>(val: EntityComparisonField<Entity, F>): QuerySelector<Entity[F]> {
if (this.isBetweenVal(val)) {
return { $gte: val.lower, $lte: val.upper };
}
throw new Error(`Invalid value for between expected {lower: val, upper: val} got ${JSON.stringify(val)}`);
}

private notBetweenComparison<F extends keyof Entity>(
private betweenComparison<F extends keyof Entity>(
cmp: string,
field: F,
val: EntityComparisonField<Entity, F>,
): QuerySelector<Entity[F]> {
if (this.isBetweenVal(val)) {
return { $lt: val.lower, $gt: val.upper };
if (!this.isBetweenVal(val)) {
throw new Error(`Invalid value for ${cmp} expected {lower: val, upper: val} got ${JSON.stringify(val)}`);
}
throw new Error(`Invalid value for not between expected {lower: val, upper: val} got ${JSON.stringify(val)}`);
if (cmp === 'notbetween') {
return { $lt: this.convertQueryValue(field, val.lower), $gt: this.convertQueryValue(field, val.upper) };
}
return { $gte: this.convertQueryValue(field, val.lower), $lte: this.convertQueryValue(field, val.upper) };
}

private isBetweenVal<F extends keyof Entity>(
Expand All @@ -104,4 +101,27 @@ export class ComparisonBuilder<Entity extends Document> {
}
return { $regex: regExp };
}

private convertQueryValue<F extends keyof Entity>(field: F, val: Entity[F]): Entity[F] {
const schemaType = this.Model.schema.path(getSchemaKey(field as string));
if (!schemaType) {
throw new BadRequestException(`unknown comparison field ${String(field)}`);
}
if (schemaType instanceof Schema.Types.ObjectId) {
return this.convertToObjectId(val) as Entity[F];
}
return val;
}

private convertToObjectId(val: unknown): unknown {
if (Array.isArray(val)) {
return val.map((v) => this.convertToObjectId(v));
}
if (typeof val === 'string' || typeof val === 'number') {
if (Types.ObjectId.isValid(val)) {
return Types.ObjectId(val);
}
}
return val;
}
}
5 changes: 3 additions & 2 deletions packages/query-mongoose/src/query/filter-query.builder.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { AggregateQuery, Filter, Query, SortDirection, SortField } from '@nestjs-query/core';
import { FilterQuery, Document } from 'mongoose';
import { FilterQuery, Document, Model as MongooseModel } from 'mongoose';
import { AggregateBuilder, MongooseGroupAndAggregate } from './aggregate.builder';
import { getSchemaKey } from './helpers';
import { WhereBuilder } from './where.builder';
Expand All @@ -25,7 +25,8 @@ type MongooseAggregateQuery<Entity extends Document> = MongooseQuery<Entity> & {
*/
export class FilterQueryBuilder<Entity extends Document> {
constructor(
readonly whereBuilder: WhereBuilder<Entity> = new WhereBuilder<Entity>(),
readonly Model: MongooseModel<Entity>,
readonly whereBuilder: WhereBuilder<Entity> = new WhereBuilder<Entity>(Model),
readonly aggregateBuilder: AggregateBuilder<Entity> = new AggregateBuilder<Entity>(),
) {}

Expand Down
7 changes: 5 additions & 2 deletions packages/query-mongoose/src/query/where.builder.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import { Filter, FilterComparisons, FilterFieldComparison } from '@nestjs-query/core';
import { FilterQuery, Document } from 'mongoose';
import { FilterQuery, Document, Model as MongooseModel } from 'mongoose';
import { EntityComparisonField, ComparisonBuilder } from './comparison.builder';

/**
* @internal
* Builds a WHERE clause from a Filter.
*/
export class WhereBuilder<Entity extends Document> {
constructor(readonly comparisonBuilder: ComparisonBuilder<Entity> = new ComparisonBuilder<Entity>()) {}
constructor(
readonly Model: MongooseModel<Entity>,
readonly comparisonBuilder: ComparisonBuilder<Entity> = new ComparisonBuilder(Model),
) {}

/**
* Builds a WHERE clause from a Filter.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ type MongoDBDeletedOutput = {
export class MongooseQueryService<Entity extends Document>
extends ReferenceQueryService<Entity>
implements QueryService<Entity, DeepPartial<Entity>, DeepPartial<Entity>> {
readonly filterQueryBuilder: FilterQueryBuilder<Entity> = new FilterQueryBuilder();

constructor(readonly Model: MongooseModel<Entity>) {
constructor(
readonly Model: MongooseModel<Entity>,
readonly filterQueryBuilder: FilterQueryBuilder<Entity> = new FilterQueryBuilder(Model),
) {
super();
}

Expand Down
14 changes: 7 additions & 7 deletions packages/query-mongoose/src/services/reference-query.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export abstract class ReferenceQueryService<Entity extends Document> {
): Promise<AggregateResponse<Relation>[] | Map<Entity, AggregateResponse<Relation>[]>> {
this.checkForReference('AggregateRelations', relationName);
const relationModel = this.getReferenceModel(relationName);
const referenceQueryBuilder = ReferenceQueryService.getReferenceQueryBuilder();
const referenceQueryBuilder = this.getReferenceQueryBuilder(relationName);
if (Array.isArray(dto)) {
return dto.reduce(async (mapPromise, entity) => {
const map = await mapPromise;
Expand Down Expand Up @@ -108,7 +108,7 @@ export abstract class ReferenceQueryService<Entity extends Document> {
}
const assembler = AssemblerFactory.getAssembler(RelationClass, Document);
const relationModel = this.getReferenceModel(relationName);
const referenceQueryBuilder = ReferenceQueryService.getReferenceQueryBuilder();
const referenceQueryBuilder = this.getReferenceQueryBuilder(relationName);
const refFilter = this.getReferenceFilter(relationName, dto, assembler.convertQuery({ filter }).filter);
if (!refFilter) {
return 0;
Expand All @@ -135,7 +135,7 @@ export abstract class ReferenceQueryService<Entity extends Document> {
opts?: FindRelationOptions<Relation>,
): Promise<(Relation | undefined) | Map<Entity, Relation | undefined>> {
this.checkForReference('FindRelation', relationName);
const referenceQueryBuilder = ReferenceQueryService.getReferenceQueryBuilder();
const referenceQueryBuilder = this.getReferenceQueryBuilder(relationName);
if (Array.isArray(dto)) {
return dto.reduce(async (prev, curr) => {
const map = await prev;
Expand Down Expand Up @@ -173,7 +173,7 @@ export abstract class ReferenceQueryService<Entity extends Document> {
query: Query<Relation>,
): Promise<Relation[] | Map<Entity, Relation[]>> {
this.checkForReference('QueryRelations', relationName);
const referenceQueryBuilder = ReferenceQueryService.getReferenceQueryBuilder();
const referenceQueryBuilder = this.getReferenceQueryBuilder(relationName);
if (Array.isArray(dto)) {
return dto.reduce(async (mapPromise, entity) => {
const map = await mapPromise;
Expand Down Expand Up @@ -291,8 +291,8 @@ export abstract class ReferenceQueryService<Entity extends Document> {
return !!this.Model.schema.virtualpath(refName);
}

static getReferenceQueryBuilder<Ref extends Document>(): FilterQueryBuilder<Ref> {
return new FilterQueryBuilder<Ref>();
private getReferenceQueryBuilder<Ref extends Document>(refName: string): FilterQueryBuilder<Ref> {
return new FilterQueryBuilder<Ref>(this.getReferenceModel(refName));
}

private getReferenceModel<Ref extends Document>(refName: string): MongooseModel<Ref> {
Expand Down Expand Up @@ -364,7 +364,7 @@ export abstract class ReferenceQueryService<Entity extends Document> {
filter?: Filter<Relation>,
): Promise<number> {
const referenceModel = this.getReferenceModel<Relation>(relationName);
const referenceQueryBuilder = ReferenceQueryService.getReferenceQueryBuilder<Relation>();
const referenceQueryBuilder = this.getReferenceQueryBuilder<Relation>(relationName);
return referenceModel.count(referenceQueryBuilder.buildIdFilterQuery(relationIds, filter)).exec();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class TestEntity {
dateType!: Date;

@prop({ ref: TestReference, required: false })
testReference?: Ref<TestReference>;
testReference?: Ref<TestReference> | string;

@prop({ ref: TestReference, required: false })
testReferences?: Ref<TestReference>[];
Expand Down
Loading

0 comments on commit 99658ec

Please sign in to comment.