Skip to content

Commit

Permalink
fix(query-typeorm): name collision on deeply nested filters
Browse files Browse the repository at this point in the history
  • Loading branch information
Silas Rosenkranz committed May 26, 2023
1 parent d596a8d commit 327300e
Show file tree
Hide file tree
Showing 3 changed files with 200 additions and 23 deletions.
139 changes: 139 additions & 0 deletions examples/filters-deep/e2e/users.resolver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,145 @@ describe('UsersResolver (filters-deep - e2e)', () => {
)
})
})

it('should not fail if the same property-name occurs twice', async () => {
await request(app.getHttpServer())
.post('/graphql')
.send({
operationName: null,
variables: {},
query: `{
# "John Doe", "Jane Doe", "Post 1 Only", "Post 2 Only", "Post 3 Only", "Post 4 Only", "Post 6 Only"
users(
filter: {
# "John Doe", "Jane Doe", "Post 1 Only", "Post 2 Only", "Post 3 Only", "Post 4 Only", "Post 5 Only", "Post 6 Only"
firstName: {
iLike: "%%"
}
# "John Doe", "Jane Doe", "Post 1 Only", "Post 2 Only", "Post 3 Only", "Post 4 Only", "Post 5 Only", "Post 6 Only"
and: [
{
lastName: {
iLike: "%%"
}
},
{
firstName: {
iLike: "%%"
}
}
]
# "John Doe", "Jane Doe", "Post 1 Only", "Post 2 Only", "Post 3 Only", "Post 4 Only", "Post 6 Only"
# "Post 1", "Post 2", "Post 3", "Post 4", "Post 6"
posts: {
or: [
# "Post 1"
{
title: {
eq: "Post 1"
}
},
# "Post 1", "Post 2", "Post 6", "Post 3", "Post 4"
{
# "Sports", "Politics"
categories: {
# "Post 2"
posts: {
and: [
{
title: {
eq: "Post 2"
}
},
{
description: {
eq: "Post 2 Description"
}
}
]
}
}
# "John Doe", "Jane Doe", "Post 1 Only"
authors: {
# "Post 1"
posts: {
and: [
{
title: {
eq: "Post 1"
}
},
{
description: {
eq: "Post 1 Description"
}
}
]
}
}
}
]
}
}
) {
id
firstName
lastName
}
}`
})
.expect(200)
.then(({ body }) => {
const users = body.data.users

expect(users).toHaveLength(7)
expect(users).toEqual(
expect.arrayContaining([
expect.objectContaining({
firstName: 'John',
lastName: 'Doe'
}),

expect.objectContaining({
firstName: 'Jane',
lastName: 'Doe'
}),

expect.objectContaining({
firstName: 'Post 1',
lastName: 'Only'
}),

expect.objectContaining({
firstName: 'Post 2',
lastName: 'Only'
}),

expect.objectContaining({
firstName: 'Post 3',
lastName: 'Only'
}),

expect.objectContaining({
firstName: 'Post 4',
lastName: 'Only'
}),

expect.objectContaining({
firstName: 'Post 6',
lastName: 'Only'
})
])
)
})
})
})

afterAll(async () => {
Expand Down
24 changes: 18 additions & 6 deletions packages/query-typeorm/src/query/filter-query.builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,17 @@ export class FilterQueryBuilder<Entity> {
*/
public select(query: Query<Entity>): SelectQueryBuilder<Entity> {
let qb = this.createQueryBuilder()

qb = this.applyRelationJoinsRecursive(
qb,
this.getReferencedRelationsRecursive(this.repo.metadata, query.filter, query.relations),
query.relations
)

qb = this.applyFilter(qb, query.filter, qb.alias)
qb = this.applySorting(qb, query.sorting, qb.alias)
qb = this.applyPaging(qb, query.paging, this.shouldUseSkipTake(query.filter))

return qb
}

Expand Down Expand Up @@ -245,32 +248,41 @@ export class FilterQueryBuilder<Entity> {
qb: SelectQueryBuilder<Entity>,
relationsMap?: NestedRecord,
selectRelations?: SelectRelation<Entity>[],
alias?: string
alias?: string,
counters = new Map<string, number>()
): SelectQueryBuilder<Entity> {
if (!relationsMap) {
return qb
}

const referencedRelations = Object.keys(relationsMap)

// TODO:: If relation is not nullable use inner join?
return referencedRelations.reduce((rqb, relation) => {
return referencedRelations.reduce((rqb, relation, i) => {
// TODO:: Change to find and also apply the query for the relation
const selectRelation = selectRelations && selectRelations.find(({ name }) => name === relation)
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const count = (counters.get(relation) ?? -1) + 1
const relationAlias = count === 0 ? relation : `${relation}_${count}`

counters.set(relation, count)

if (selectRelation) {
return this.applyRelationJoinsRecursive(
rqb.leftJoinAndSelect(`${alias ?? rqb.alias}.${relation}`, relation),
rqb.leftJoinAndSelect(`${alias ?? rqb.alias}.${relation}`, relationAlias),
relationsMap[relation],
selectRelation.query.relations,
relation
relationAlias,
counters
)
}

return this.applyRelationJoinsRecursive(
rqb.leftJoin(`${alias ?? rqb.alias}.${relation}`, relation),
rqb.leftJoin(`${alias ?? rqb.alias}.${relation}`, relationAlias),
relationsMap[relation],
[],
relation
relationAlias,
counters
)
}, qb)
}
Expand Down
60 changes: 43 additions & 17 deletions packages/query-typeorm/src/query/where.builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,20 @@ export class WhereBuilder<Entity> {
where: Where,
filter: Filter<Entity>,
relationNames: NestedRecord,
alias?: string
alias?: string,
counters = new Map<string, number>()
): Where {
const { and, or } = filter

if (and && and.length) {
this.filterAnd(where, and, relationNames, alias)
this.filterAnd(where, and, relationNames, alias, counters)
}

if (or && or.length) {
this.filterOr(where, or, relationNames, alias)
this.filterOr(where, or, relationNames, alias, counters)
}
return this.filterFields(where, filter, relationNames, alias)

return this.filterFields(where, filter, relationNames, alias, counters)
}

/**
Expand All @@ -49,10 +53,11 @@ export class WhereBuilder<Entity> {
where: Where,
filters: Filter<Entity>[],
relationNames: NestedRecord,
alias?: string
alias: string | undefined,
counters: Map<string, number>
): Where {
return where.andWhere(
new Brackets((qb) => filters.reduce((w, f) => qb.andWhere(this.createBrackets(f, relationNames, alias)), qb))
new Brackets((qb) => filters.reduce((w, f) => qb.andWhere(this.createBrackets(f, relationNames, alias, counters)), qb))
)
}

Expand All @@ -68,10 +73,11 @@ export class WhereBuilder<Entity> {
where: Where,
filter: Filter<Entity>[],
relationNames: NestedRecord,
alias?: string
alias: string | undefined,
counters: Map<string, number>
): Where {
return where.andWhere(
new Brackets((qb) => filter.reduce((w, f) => qb.orWhere(this.createBrackets(f, relationNames, alias)), qb))
new Brackets((qb) => filter.reduce((w, f) => qb.orWhere(this.createBrackets(f, relationNames, alias, counters)), qb))
)
}

Expand All @@ -85,8 +91,13 @@ export class WhereBuilder<Entity> {
* @param relationNames - the relations tree.
* @param alias - optional alias to use to qualify an identifier
*/
private createBrackets(filter: Filter<Entity>, relationNames: NestedRecord, alias?: string): Brackets {
return new Brackets((qb) => this.build(qb, filter, relationNames, alias))
private createBrackets(
filter: Filter<Entity>,
relationNames: NestedRecord,
alias: string | undefined,
counters: Map<string, number>
): Brackets {
return new Brackets((qb) => this.build(qb, filter, relationNames, alias, counters))
}

/**
Expand All @@ -100,7 +111,8 @@ export class WhereBuilder<Entity> {
where: Where,
filter: Filter<Entity>,
relationNames: NestedRecord,
alias?: string
alias: string | undefined,
counters: Map<string, number>
): Where {
return Object.keys(filter).reduce((w, field) => {
if (field !== 'and' && field !== 'or') {
Expand All @@ -109,7 +121,8 @@ export class WhereBuilder<Entity> {
field as keyof Entity,
this.getField(filter, field as keyof Entity),
relationNames,
alias
alias,
counters
)
}
return w
Expand All @@ -128,17 +141,24 @@ export class WhereBuilder<Entity> {
field: T,
cmp: FilterFieldComparison<Entity[T]>,
relationNames: NestedRecord,
alias?: string
alias: string | undefined,
counters: Map<string, number>
): Where {
if (relationNames[field as string]) {
return this.withRelationFilter(where, field, cmp as Filter<Entity[T]>, relationNames[field as string])
return this.withRelationFilter(where, field, cmp as Filter<Entity[T]>, relationNames[field as string], counters)
}

return where.andWhere(
new Brackets((qb) => {
const opts = Object.keys(cmp) as (keyof FilterFieldComparison<Entity[T]>)[]

const count = counters.get(alias) ?? 0
const $alias = count === 0 ? alias : `${alias}_${count}`

const sqlComparisons = opts.map((cmpType) =>
this.sqlComparisonBuilder.build(field, cmpType, cmp[cmpType] as EntityComparisonField<Entity, T>, alias)
this.sqlComparisonBuilder.build(field, cmpType, cmp[cmpType] as EntityComparisonField<Entity, T>, $alias)
)

sqlComparisons.map(({ sql, params }) => qb.orWhere(sql, params))
})
)
Expand All @@ -148,13 +168,19 @@ export class WhereBuilder<Entity> {
where: Where,
field: T,
cmp: Filter<Entity[T]>,
relationNames: NestedRecord
relationNames: NestedRecord,
counters: Map<string, number>
): Where {
return where.andWhere(
new Brackets((qb) => {
const relationWhere = new WhereBuilder<Entity[T]>()
return relationWhere.build(qb, cmp, relationNames, field as string)
const count = (counters.get(field as string) ?? -1) + 1
if (field as string) counters.set(field as string, count)

return relationWhere.build(qb, cmp, relationNames, field as string, counters)
})
)
}

private branchCount = -1
}

0 comments on commit 327300e

Please sign in to comment.