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 f932e09 commit 11098c4
Show file tree
Hide file tree
Showing 17 changed files with 257 additions and 101 deletions.
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
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
@@ -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 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 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 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 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 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
@@ -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
@@ -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
@@ -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
@@ -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
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
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();
}
}
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

0 comments on commit 11098c4

Please sign in to comment.