From 1ebe7e9bc8d3a61f0b3ef65b588881d16b7ae63f Mon Sep 17 00:00:00 2001 From: Enda Phelan Date: Tue, 8 Sep 2020 11:30:29 +0100 Subject: [PATCH 1/4] fix: remove invalid OR mapping --- .../src/queryBuilder.ts | 41 +--- .../tests/MongoDataProviderTest.ts | 211 ++++++++++++++---- .../tests/queryBuilderTest.ts | 156 ++++--------- 3 files changed, 217 insertions(+), 191 deletions(-) diff --git a/packages/graphback-runtime-mongodb/src/queryBuilder.ts b/packages/graphback-runtime-mongodb/src/queryBuilder.ts index ddb7408a07..1e1cf5b5c8 100644 --- a/packages/graphback-runtime-mongodb/src/queryBuilder.ts +++ b/packages/graphback-runtime-mongodb/src/queryBuilder.ts @@ -83,7 +83,7 @@ function isPrimitive(test: any): boolean { return ((test instanceof RegExp) || (test !== Object(test))); }; -function isOperator(key: string): key is QueryOperator { +function isQueryOperator(key: string): key is QueryOperator { return operators.includes(key as QueryOperator); } @@ -101,7 +101,7 @@ function mapQueryFilterToMongoFilterQuery(filter: any): FilterQuery { return Object.keys(filter).reduce((obj: any, key: string) => { const propKey = operatorMap[key] || key; let propVal: any; - if (isOperator(propKey)) { + if (isQueryOperator(propKey)) { propVal = filter[key]; } else { propVal = mapQueryFilterToMongoFilterQuery(filter[key]); @@ -126,46 +126,11 @@ function mapQueryFilterToMongoFilterQuery(filter: any): FilterQuery { return filter; } -/** - * Map or conditions into $or operator - * in MongoDB friendly-structure. - * - * @param {QueryFilter} filter - */ -export function mapOrConditions(filter: FilterQuery) { - const fields = Object.keys(filter); - - if (fields.includes('$or')) { - filter.$or = filter.$or || []; - if (!Array.isArray(filter.$or)) { - filter.$or = [filter.$or]; - } - const or = {}; - for (const field of fields) { - if (field !== '$or') { - or[field] = filter[field]; - // eslint-disable-next-line @typescript-eslint/tslint/config - delete filter[field] - } - } - if (Object.keys(or).length) { - filter.$or.push(or); - } - filter.$or.forEach(mapOrConditions) - } -} - /** * Build a MongoDB query from a Graphback filter * * @param {QueryFilter} filter */ export function buildQuery(filter: QueryFilter): FilterQuery { - const query = mapQueryFilterToMongoFilterQuery(filter); - - if (query) { - mapOrConditions(query); - } - - return query; + return mapQueryFilterToMongoFilterQuery(filter); } diff --git a/packages/graphback-runtime-mongodb/tests/MongoDataProviderTest.ts b/packages/graphback-runtime-mongodb/tests/MongoDataProviderTest.ts index 6dc9d55218..a674722985 100644 --- a/packages/graphback-runtime-mongodb/tests/MongoDataProviderTest.ts +++ b/packages/graphback-runtime-mongodb/tests/MongoDataProviderTest.ts @@ -397,7 +397,7 @@ describe('MongoDBDataProvider Basic CRUD', () => { expect(oldTodos).toEqual(allTodos); // assert that we did not retrieve the newly added todo item }); - it('a || b || c starting at root of query', async () => { + it('a && (b || c)', async () => { context = await createTestingContext(` scalar GraphbackObjectID @@ -419,37 +419,36 @@ describe('MongoDBDataProvider Basic CRUD', () => { c: 8 }, { - a: 9, + a: 1, b: 2, c: 10 }, { - a: 9, - b: 2, + a: 1, + b: 5, c: 3 }, { a: 6, b: 6, - c: 6 + c: 3 } ] } - }); + }) const filter: QueryFilter = { a: { eq: 1 }, or: [ - { - b: { - eq: 2 - } - }, { c: { - eq: 3 + eq: 6 + } + }, { + b: { + eq: 5 } } ] @@ -457,10 +456,10 @@ describe('MongoDBDataProvider Basic CRUD', () => { const items = await context.providers.Todo.findBy({ filter }); - expect(items).toHaveLength(3); - }) + expect(items).toHaveLength(2); + }); - it('a || b || c starting at first $or of query', async () => { + it('a && (b || c) starting at first $or', async () => { context = await createTestingContext(` scalar GraphbackObjectID @@ -482,41 +481,104 @@ describe('MongoDBDataProvider Basic CRUD', () => { c: 8 }, { - a: 9, + a: 1, b: 2, c: 10 }, { - a: 9, - b: 2, + a: 1, + b: 5, c: 3 }, { a: 6, b: 6, - c: 6 + c: 3 } ] } - }); + }) const filter: QueryFilter = { - $or: [ + or: [ { a: { eq: 1 }, - }, - { or: [ { + c: { + eq: 6 + } + }, { b: { - eq: 2 + eq: 5 } - }, + } + ] + } + ] + } + + const items = await context.providers.Todo.findBy({ filter }); + + expect(items).toHaveLength(2); + }); + + it('a && (c || b) from nested $or', async () => { + context = await createTestingContext(` + scalar GraphbackObjectID + + """ + @model + """ + type Todo { + _id: GraphbackObjectID + a: Int + b: Int + c: Int + } + `, { + seedData: { + Todo: [ + { + a: 1, + b: 1, + c: 8 + }, + { + a: 9, + b: 2, + c: 10 + }, + { + a: 1, + b: 5, + c: 3 + }, + { + a: 1, + b: 6, + c: 6 + } + ] + } + }); + + const filter: QueryFilter = { + or: [ + { + a: { + eq: 1 + }, + or: [ { c: { - eq: 3 + eq: 6 + } + }, { + b: { + eq: 5 } } ] @@ -526,10 +588,10 @@ describe('MongoDBDataProvider Basic CRUD', () => { const items = await context.providers.Todo.findBy({ filter }); - expect(items).toHaveLength(3); - }) + expect(items).toHaveLength(2); + }); - it('(a && b) || c', async () => { + it('a || a || a', async () => { context = await createTestingContext(` scalar GraphbackObjectID @@ -551,19 +613,19 @@ describe('MongoDBDataProvider Basic CRUD', () => { c: 8 }, { - a: 9, + a: 2, b: 2, c: 10 }, { - a: 9, - b: 2, + a: 3, + b: 5, c: 3 }, { a: 6, b: 6, - c: 6 + c: 3 } ] } @@ -574,21 +636,90 @@ describe('MongoDBDataProvider Basic CRUD', () => { { a: { eq: 1 + } + }, + { + a: { + eq: 2 + } + }, + { + a: { + eq: 3 + } + } + ] + } + + const items = await context.providers.Todo.findBy({ filter }); + + expect(items).toHaveLength(3) + }) + + it('a || (a && b)', async () => { + context = await createTestingContext(` + scalar GraphbackObjectID + + """ + @model + """ + type Todo { + _id: GraphbackObjectID + a: Int + b: Int + c: Int + } + `, { + seedData: { + Todo: [ + { + a: 1, + b: 5, + c: 8 }, - b: { - eq: 5 + { + a: 2, + b: 3, + c: 10 + }, + { + a: 2, + b: 3, + c: 3 + }, + { + a: 6, + b: 6, + c: 3 + } + ] + } + }); + + const filter: QueryFilter = { + or: [ + { + a: { + eq: 1 }, - or: [{ - c: { - eq: 6 + }, + { + or: [ + { + a: { + eq: 2 + }, + b: { + eq: 3 + } } - }] + ] } ] } const items = await context.providers.Todo.findBy({ filter }); - expect(items).toHaveLength(2); + expect(items).toHaveLength(3) }) }); diff --git a/packages/graphback-runtime-mongodb/tests/queryBuilderTest.ts b/packages/graphback-runtime-mongodb/tests/queryBuilderTest.ts index 0f6c855abf..19d757d4a2 100644 --- a/packages/graphback-runtime-mongodb/tests/queryBuilderTest.ts +++ b/packages/graphback-runtime-mongodb/tests/queryBuilderTest.ts @@ -37,7 +37,7 @@ describe('MongoDBDataProvider Advanced Filtering', () => { //Create a new database before each tests so that //all tests can run parallel - afterEach(() => context.server.stop()); + afterEach(() => context?.server?.stop()); it('can filter ObjectID', async () => { context = await createTestingContext(postSchema); @@ -86,10 +86,14 @@ describe('MongoDBDataProvider Advanced Filtering', () => { const posts: Post[] = await context.providers.Post.findBy({ filter: { - text: { eq: 'post2' }, - or: [{ - likes: { eq: 300 } - }] + or: [ + { + likes: { eq: 300 } + }, + { + text: { eq: 'post2' }, + } + ] } }); expect(posts.length).toEqual(2); @@ -107,8 +111,10 @@ describe('MongoDBDataProvider Advanced Filtering', () => { const posts: Post[] = await context.providers.Post.findBy({ filter: { - text: { eq: 'post2' }, or: [ + { + text: { eq: 'post2' }, + }, { likes: { eq: 300 }, }, @@ -362,7 +368,7 @@ describe('queryBuilder scalar filtering', () => { expect(newPosts.map((post: any) => post.text)).toEqual(["not yet", "bye guys"]); }); - it('a || b || c starting at root of query', () => { + it('a && (b || c)', () => { const inputQuery: QueryFilter = { a: { eq: 1 @@ -383,6 +389,9 @@ describe('queryBuilder scalar filtering', () => { const outputQuery = buildQuery(inputQuery); const expected = { + a: { + $eq: 1 + }, $or: [ { b: { @@ -393,11 +402,6 @@ describe('queryBuilder scalar filtering', () => { c: { $eq: 3 } - }, - { - a: { - $eq: 1 - } } ] }; @@ -449,7 +453,7 @@ describe('queryBuilder scalar filtering', () => { expect(Object.keys(outputQuery)).toEqual(['$or']); }); - it('should group multiple or conditions into single $or array', () => { + it('(a && b) && (c || c)', () => { const inputQuery: QueryFilter = { or: [ { @@ -462,12 +466,12 @@ describe('queryBuilder scalar filtering', () => { or: [ { c: { - eq: 2 + eq: 3 } }, { c: { - eq: 3 + eq: 2 } } ] @@ -479,58 +483,13 @@ describe('queryBuilder scalar filtering', () => { const expected: FilterQuery = { $or: [ - { - $or: [ - { - c: { - $eq: 2 - } - }, - { - c: { - $eq: 3 - } - }, - { - a: { - $eq: 1 - }, - b: { - $eq: 2 - } - } - ] - } - ] - } - - expect(outputQuery).toEqual(expected); - }); - - it('(a && b) || c from first $or', () => { - const inputQuery: QueryFilter = { - or: [ { a: { - eq: 1 + $eq: 1 }, b: { - eq: 2 + $eq: 2 }, - or: [{ - c: { - eq: 3 - } - }] - } - ] - } - - const outputQuery = buildQuery(inputQuery); - - const expected: FilterQuery = { - $or: [ - { $or: [ { c: { @@ -538,79 +497,55 @@ describe('queryBuilder scalar filtering', () => { } }, { - a: { - $eq: 1 - }, - b: { + c: { $eq: 2 } } ] - } + }, ] } expect(outputQuery).toEqual(expected); }) - it('(a && b) || c from query root', () => { + it('a && b && (c || b) from query root (explicit AND)', () => { const inputQuery: QueryFilter = { a: { eq: 1 }, - b: { - eq: 2 - }, - or: [{ - c: { - eq: 3 + and: [{ + b: { + eq: 2 } - }] - } - - const outputQuery = buildQuery(inputQuery); - - const expected: FilterQuery = { - $or: [ + }], + or: [ { c: { - $eq: 3 + eq: 3 } }, { - a: { - $eq: 1 - }, b: { - $eq: 2 + eq: 4 } } ] } - expect(outputQuery).toEqual(expected); - }) + const outputQuery = buildQuery(inputQuery); - it('(a && b) || c from query root (explicit AND)', () => { - const inputQuery: QueryFilter = { + const expected: FilterQuery = { a: { - eq: 1 + $eq: 1, }, - and: [{ - b: { - eq: 2 - } - }], - or: [{ - c: { - eq: 3 + $and: [ + { + b: { + $eq: 2 + } } - }] - } - - const outputQuery = buildQuery(inputQuery); - - const expected: FilterQuery = { + ], $or: [ { c: { @@ -618,14 +553,9 @@ describe('queryBuilder scalar filtering', () => { } }, { - a: { - $eq: 1 - }, - $and: [{ - b: { - $eq: 2 - } - }] + b: { + $eq: 4 + } } ] } From 61fc7bae0a8d5d4f744fd78311597f269053ec3d Mon Sep 17 00:00:00 2001 From: Enda Phelan Date: Wed, 9 Sep 2020 14:47:25 +0100 Subject: [PATCH 2/4] refactor(knex): rebuild knex query mapper implementation BREAKING --- .../src/KnexDBDataProvider.ts | 12 +- .../src/knexQueryMapper.ts | 241 +++++---- .../tests/data/KnexDbDataProviderTest.ts | 476 ++++++++++++++++-- .../tests/knexQueryMapperTest.ts | 474 +++++++++++++++++ 4 files changed, 1037 insertions(+), 166 deletions(-) create mode 100644 packages/graphback-runtime-knex/tests/knexQueryMapperTest.ts diff --git a/packages/graphback-runtime-knex/src/KnexDBDataProvider.ts b/packages/graphback-runtime-knex/src/KnexDBDataProvider.ts index 7cc346a0df..1c0fde315c 100644 --- a/packages/graphback-runtime-knex/src/KnexDBDataProvider.ts +++ b/packages/graphback-runtime-knex/src/KnexDBDataProvider.ts @@ -1,6 +1,6 @@ -import { buildModelTableMap, getDatabaseArguments, ModelTableMap, GraphbackContext, GraphbackDataProvider, GraphbackOrderBy, GraphbackPage, NoDataError, QueryFilter, ModelDefinition, FindByArgs } from '@graphback/core'; +import { buildModelTableMap, getDatabaseArguments, ModelTableMap, GraphbackDataProvider, NoDataError, QueryFilter, ModelDefinition, FindByArgs, GraphbackPage } from '@graphback/core'; import * as Knex from 'knex'; -import { buildQuery } from './knexQueryMapper'; +import { CRUDKnexQueryMapper, createKnexQueryMapper } from './knexQueryMapper'; /** * Knex.js database data provider exposing basic CRUD operations that works with all databases that knex supports. @@ -18,9 +18,11 @@ export class KnexDBDataProvider implements GraphbackDataProvider implements GraphbackDataProvider { - let query = buildQuery(this.db, args?.filter).select(this.getSelectedFields(selectedFields)).from(this.tableName) + let query = this.queryBuilder.buildQuery(args?.filter).select(this.getSelectedFields(selectedFields)).from(this.tableName) if (args?.orderBy) { query = query.orderBy(args.orderBy.field, args.orderBy.order) @@ -86,14 +88,14 @@ export class KnexDBDataProvider implements GraphbackDataProvider { - const dbResult = await buildQuery(this.db, filter).from(this.tableName).count(); + const dbResult = await this.queryBuilder.buildQuery(filter).from(this.tableName).count(); const count: any = Object.values(dbResult[0])[0]; return parseInt(count, 10); } public async batchRead(relationField: string, ids: string[], filter?: QueryFilter, selectedFields?: string[]): Promise { - const dbResult = await buildQuery(this.db, filter).select(this.getSelectedFields(selectedFields)).from(this.tableName).whereIn(relationField, ids); + const dbResult = await this.queryBuilder.buildQuery(filter).select(this.getSelectedFields(selectedFields)).from(this.tableName).whereIn(relationField, ids); if (dbResult) { const resultsById = ids.map((id: string) => dbResult.filter((data: any) => { diff --git a/packages/graphback-runtime-knex/src/knexQueryMapper.ts b/packages/graphback-runtime-knex/src/knexQueryMapper.ts index 2c91baa03f..a1ada00ac3 100644 --- a/packages/graphback-runtime-knex/src/knexQueryMapper.ts +++ b/packages/graphback-runtime-knex/src/knexQueryMapper.ts @@ -1,140 +1,169 @@ import Knex from 'knex'; +import { QueryFilter, QueryFilterOperator } from '@graphback/core'; -interface OperatorMap { - ne: '<>', - eq: '=', - le: '<=', - lt: '<', - ge: '>=', - gt: '>', - contains: 'like', - startswith: 'like', - endswith: 'like', -} +const knexOperators = ['=', '<>', '<=', '<', '>', '>=', 'like', 'between', 'in'] as const; +const knexMethods = ['where', 'orWhere', 'orWhereNot', 'whereNot'] as const; -const methodMapping = { - in: 'whereIn', - between: 'whereBetween', - default: 'where' -} +type KnexQueryOperator = typeof knexOperators[number]; +type KnexMethod = typeof knexMethods[number]; +type KnexWhereConditionMap = [KnexQueryOperator, any]; +type RootQuerySelector = 'and' | 'or' | 'not'; -const notMethodMap = { - where: 'whereNot', - whereNull: 'whereNotNull', - between: 'whereNotBetween', - in: 'whereNotIn' -} +type KnexRootQuerySelectorBuilderFn = (builder: Knex.QueryBuilder, filter: QueryFilter | QueryFilter[], rootSelectorContext: RootQuerySelectorContext) => Knex.QueryBuilder; +type KnexQueryBuilderMapFn = (builder: Knex.QueryBuilder, filter: QueryFilter | QueryFilter[]) => Knex.QueryBuilder; -const AND_FIELD = 'and'; -const OR_FIELD = 'or'; -const NOT_FIELD = 'not'; - -function mapOperator(operator: any) { - const operatorMap: OperatorMap = { - ne: '<>', - eq: '=', - le: '<=', - lt: '<', - ge: '>=', - gt: '>', - contains: 'like', - startswith: 'like', - endswith: 'like' - } +interface RootQuerySelectorContext { + and?: boolean + or?: boolean + not?: boolean +} - const oprLower = operator.toLowerCase() +export interface CRUDKnexQueryMapper { + /** + * Maps a Graphback QueryFilter to a Knex query + * + * @param {QueryFilter} [filter] - input filter + */ + buildQuery(filter?: QueryFilter): Knex.QueryBuilder; +} - if (!Object.keys(operatorMap).includes(oprLower)) { - throw Error(`Not supported operator: ${operator}`) +const mapQueryFilterOperatorToKnexWhereCondition = (operator: QueryFilterOperator, value: any): KnexWhereConditionMap => { + const operatorToWhereConditionMap: { [key in QueryFilterOperator]: KnexWhereConditionMap } = { + eq: ['=', value], + ne: ['<>', value], + lt: ['<', value], + le: ['<=', value], + ge: ['>=', value], + gt: ['>', value], + contains: ['like', `%${value}%`], + startsWith: ['like', `${value}%`], + endsWith: ['like', `%${value}`], + in: ['in', value], + between: ['between', value] } - return operatorMap[oprLower] + return operatorToWhereConditionMap[operator]; } -function builderMethod(method: string, or?: boolean, not?: boolean) { - if (!not && methodMapping[method]) { - method = methodMapping[method] - } else if (not && notMethodMap[method]) { - method = notMethodMap[method] +function buildKnexMethod(rootSelectorContext: RootQuerySelectorContext): KnexMethod { + let method = 'where'; + if (rootSelectorContext?.not) { + method = `${method}Not` } - if (or) { - return `or${method.charAt(0).toUpperCase()}${method.slice(1)}` + if (rootSelectorContext?.or) { + method = `or${method.charAt(0).toUpperCase()}${method.slice(1)}`; } - return method + return method as KnexMethod } -function where(builder: Knex.QueryBuilder, filter: any, or: boolean = false, not: boolean = false) { - if (!filter) { - return builder +const rootSelectorMapper: { [key in RootQuerySelector]: KnexRootQuerySelectorBuilderFn } = { + and: (builder: Knex.QueryBuilder, filters: QueryFilter[], rootSelectorContext: RootQuerySelectorContext): Knex.QueryBuilder => { + builder = builder.where((b: Knex.QueryBuilder) => { + filters.forEach((f: QueryFilter) => mapQuery(b, f, { ...rootSelectorContext, and: true })); + }); + + return builder; + }, + or: (builder: Knex.QueryBuilder, filters: QueryFilter[], rootSelectorContext: RootQuerySelectorContext): Knex.QueryBuilder => { + builder = builder.where((b: Knex.QueryBuilder) => { + filters.forEach((f: QueryFilter) => mapQuery(b, f, { ...rootSelectorContext, or: true })); + }); + + return builder; + }, + not: (builder: Knex.QueryBuilder, filter: QueryFilter, rootSelectorContext: RootQuerySelectorContext): Knex.QueryBuilder => { + const builderMethod = buildKnexMethod(rootSelectorContext); + builder = builder[builderMethod]((b: Knex.QueryBuilder) => { + builder = mapQuery(b, filter, { ...rootSelectorContext, not: true }); + }); + + return builder; + }, +} + +/** + * Wraps Kne methods and pipe the QueryFilter conditions into a final Knex condition + */ +const methodBuilderMapper: { [key in KnexMethod | 'finally']: KnexQueryBuilderMapFn } = { + where: (builder: Knex.QueryBuilder, filter: QueryFilter): Knex.QueryBuilder => { + return builder.where((b: Knex.QueryBuilder) => b = methodBuilderMapper.finally(b, filter)); + }, + orWhere: (builder: Knex.QueryBuilder, filter: QueryFilter): Knex.QueryBuilder => { + return builder.orWhere((b: Knex.QueryBuilder) => b = methodBuilderMapper.finally(b, filter)); + }, + whereNot: (builder: Knex.QueryBuilder, filter: QueryFilter): Knex.QueryBuilder => { + return builder.whereNot((b: Knex.QueryBuilder) => b = methodBuilderMapper.finally(b, filter)); + }, + orWhereNot: (builder: Knex.QueryBuilder, filter: QueryFilter): Knex.QueryBuilder => { + return builder.orWhereNot((b: Knex.QueryBuilder) => b = methodBuilderMapper.finally(b, filter)); + }, + finally: (builder: Knex.QueryBuilder, filter: QueryFilter): Knex.QueryBuilder => { + Object.entries(filter).forEach(([col, expr]: [string, any]) => { + Object.entries(expr).forEach(([operator, val]: [any, any]) => { + const [mappedOperator, transformedValue] = mapQueryFilterOperatorToKnexWhereCondition(operator, val); + builder = builder.where(col, mappedOperator, transformedValue); + }) + }); + + return builder; } +} - const andQueries = [] - const orQueries = [] - const notQueries = [] - for (const entry of Object.entries(filter)) { - const col = entry[0] - const expr = entry[1] as any - - // collect all AND condition filters - if (col === AND_FIELD) { - const andExpressions = Array.isArray(expr) ? expr : [expr] - andQueries.push(...andExpressions) - continue - } +function mapQuery(builder: Knex.QueryBuilder, filter: QueryFilter, rootSelectorContext: RootQuerySelectorContext = {}): Knex.QueryBuilder { + if (filter === undefined) { return builder }; - // collect all OR condition filters - if (col === OR_FIELD) { - const orExpressions = Array.isArray(expr) ? expr : [expr] - orQueries.push(...orExpressions) - continue - } + const mappedQuery = { + rootFieldQueries: {} as QueryFilter, + and: [] as QueryFilter[], + or: [] as QueryFilter[], + not: {} as QueryFilter + } - // collect all NOT condition filters - if (col === NOT_FIELD) { - notQueries.push(expr) - continue - } + for (const key of Object.keys(filter)) { + const rootSelector = rootSelectorMapper[key]; - const exprEntry = Object.entries(expr)[0] - - // eslint-disable-next-line no-null/no-null - if (exprEntry[1] === null) { - builder = builder[builderMethod('whereNull', or, exprEntry[0] === 'ne')](col) - } else if (Object.keys(methodMapping).includes(exprEntry[0])) { - builder = builder[builderMethod(exprEntry[0], or, not)](col, exprEntry[1]) - } else if (exprEntry[0] === 'contains') { - builder = builder[builderMethod('where', or, not)](col, mapOperator(exprEntry[0]), `%${exprEntry[1]}%`) - } else if (exprEntry[0] === 'startsWith') { - builder = builder[builderMethod('where', or, not)](col, mapOperator(exprEntry[0]), `${exprEntry[1]}%`) - } else if (exprEntry[0] === 'endsWith') { - builder = builder[builderMethod('where', or, not)](col, mapOperator(exprEntry[0]), `%${exprEntry[1]}`) + if (rootSelector) { + mappedQuery[key] = filter[key]; } else { - builder = builder[builderMethod('where', or, not)](col, mapOperator(exprEntry[0]), exprEntry[1]) + mappedQuery.rootFieldQueries[key] = filter[key]; } } - // build AND queries - for (const andFilter of andQueries) { - builder = where(builder, andFilter, false, not) - } + // build conditions for root fields + if (Object.keys(mappedQuery.rootFieldQueries).length) { + const knexMethodName = buildKnexMethod(rootSelectorContext); - // build NOT queries - for (const notFilter of notQueries) { - builder = where(builder, notFilter, or, true) + const wrappedKnexMethod = methodBuilderMapper[knexMethodName]; + + if (wrappedKnexMethod) { + builder = wrappedKnexMethod(builder, mappedQuery.rootFieldQueries); + } } - // build OR queries - for (const orFilter of orQueries) { - builder = where(builder, orFilter, true, not) + if (mappedQuery.and.length) { + builder = rootSelectorMapper.and(builder, mappedQuery.and, rootSelectorContext); + } + if (mappedQuery.or.length) { + builder = rootSelectorMapper.or(builder, mappedQuery.or, rootSelectorContext); + } + if (Object.keys(mappedQuery.not).length) { + builder = rootSelectorMapper.not(builder, mappedQuery.not, rootSelectorContext); } - return builder + return builder; } -export function buildQuery(knex: Knex, filter: any): Knex.QueryBuilder { - const builder = where(knex.queryBuilder(), filter) - - return builder +/** + * Create an instance of a CRUD => Knex query mapper + * * + * @param {Knex} knex - The current Knex connection instance + */ +export function createKnexQueryMapper(knex: Knex): CRUDKnexQueryMapper { + return { + buildQuery: (filter: QueryFilter): Knex.QueryBuilder => { + return mapQuery(knex.queryBuilder(), filter); + } + } } diff --git a/packages/graphback-runtime-knex/tests/data/KnexDbDataProviderTest.ts b/packages/graphback-runtime-knex/tests/data/KnexDbDataProviderTest.ts index 12c9142f81..4101021ed4 100644 --- a/packages/graphback-runtime-knex/tests/data/KnexDbDataProviderTest.ts +++ b/packages/graphback-runtime-knex/tests/data/KnexDbDataProviderTest.ts @@ -4,19 +4,20 @@ import { unlinkSync, existsSync } from 'fs'; import { buildSchema } from 'graphql'; import * as Knex from 'knex'; -import { GraphbackDataProvider, GraphbackCoreMetadata } from '@graphback/core'; +import { GraphbackDataProvider, GraphbackCoreMetadata, QueryFilter } from '@graphback/core'; import { SQLiteKnexDBDataProvider } from '../../src/SQLiteKnexDBDataProvider'; import { migrateDB, removeNonSafeOperationsFilter } from '../../../graphql-migrations/src'; const dbPath = `${__dirname}/db.sqlite`; -afterEach(() => { +beforeEach(() => { if (existsSync(dbPath)) { unlinkSync(dbPath) } }) const setup = async (schemaStr: string, config: { seedData?: { [tableName: string]: any[] | any } } = {}) => { + const schema = buildSchema(schemaStr); const defautCrudConfig = { "create": true, @@ -314,18 +315,18 @@ type Todo { const todos = await providers.Todo.findBy({ filter: { - "title": { - "eq": "one" + title: { + eq: "three" }, - "or": { - "title": { - "eq": "three" + or: [{ + description: { + eq: "three description" } - } + }] } }) - expect(todos).toHaveLength(2) + expect(todos).toHaveLength(1) }); test('or clause as array', async () => { @@ -345,46 +346,32 @@ type Todo { description: String }`, { seedData: { todo: seedData } }) - const todos = await providers.Todo.findBy({ - filter: { - "title": { - "eq": "one" - }, - "or": [ - { - "title": { - "eq": "three" - } - }, - { - "description": { - "eq": "" - } - } - ] - } - }) - - expect(todos).toHaveLength(3) - - // count - const count = await providers.Todo.count({ - "title": { - "eq": "one" - }, - "or": [ + const filter: QueryFilter = { + or: [ { - "title": { - "eq": "three" + title: { + eq: "one" } }, { - "description": { - "eq": "" + title: { + eq: "three" + }, + }, + { + description: { + eq: "" } } ] - }); + }; + + const todos = await providers.Todo.findBy({ filter }) + + expect(todos).toHaveLength(3) + + // count + const count = await providers.Todo.count(filter); expect(count).toEqual(3); }); @@ -408,28 +395,31 @@ type Todo { const todos = await providers.Todo.findBy({ filter: { - "title": { - "eq": "two" + title: { + eq: "two" }, - "and": { - "description": { - "eq": "" + and: [ + { + description: { + eq: "" + } } - } + ] } }) expect(todos).toHaveLength(1) const count = await providers.Todo.count({ - "title": { - "eq": "two" + title: { + eq: "two" }, - "and": { - "description": { - "eq": "" - } - } + and: [ + { + description: { + eq: "" + } + }] }) expect(count).toEqual(1) @@ -736,3 +726,379 @@ type Todo { expect(deletedTodo.id).toEqual(3); expect(deletedTodo.text).toEqual("updated todo"); }); + +describe('or', () => { + it('a && (b || c)', async () => { + const { providers: { Todo } } = await setup(` + + + """ + @model + """ + type Todo { + id: ID + a: Int + b: Int + c: Int + } + `, { + seedData: { + todo: [ + { + a: 1, + b: 5, + c: 8 + }, + { + a: 1, + b: 2, + c: 10 + }, + { + a: 1, + b: 5, + c: 3 + }, + { + a: 6, + b: 6, + c: 3 + } + ] + } + }) + + const filter: QueryFilter = { + a: { + eq: 1 + }, + or: [ + { + c: { + eq: 6 + } + }, { + b: { + eq: 5 + } + } + ] + } + + const items = await Todo.findBy({ filter }); + + expect(items).toHaveLength(2); + }); + + it('a && (b || c) starting at first $or', async () => { + const { providers: { Todo } } = await setup(` + + + """ + @model + """ + type Todo { + id: ID + a: Int + b: Int + c: Int + } + `, { + seedData: { + todo: [ + { + a: 1, + b: 5, + c: 8 + }, + { + a: 1, + b: 2, + c: 10 + }, + { + a: 1, + b: 5, + c: 3 + }, + { + a: 6, + b: 6, + c: 3 + } + ] + } + }) + + const filter: QueryFilter = { + or: [ + { + a: { + eq: 1 + }, + or: [ + { + c: { + eq: 6 + } + }, { + b: { + eq: 5 + } + } + ] + } + ] + } + + const items = await Todo.findBy({ filter }); + + expect(items).toHaveLength(2); + }); + + it('a && (c || b) from nested $or', async () => { + const { providers: { Todo } } = await setup(` + + + """ + @model + """ + type Todo { + id: ID + a: Int + b: Int + c: Int + } + `, { + seedData: { + todo: [ + { + a: 1, + b: 1, + c: 8 + }, + { + a: 9, + b: 2, + c: 10 + }, + { + a: 1, + b: 5, + c: 3 + }, + { + a: 1, + b: 6, + c: 6 + } + ] + } + }); + + const filter: QueryFilter = { + or: [ + { + a: { + eq: 1 + }, + or: [ + { + c: { + eq: 6 + } + }, { + b: { + eq: 5 + } + } + ] + } + ] + } + + const items = await Todo.findBy({ filter }); + + expect(items).toHaveLength(2); + }); + + it('a || a || a', async () => { + const { providers: { Todo } } = await setup(` + + + """ + @model + """ + type Todo { + id: ID + a: Int + b: Int + c: Int + } + `, { + seedData: { + todo: [ + { + a: 1, + b: 5, + c: 8 + }, + { + a: 2, + b: 2, + c: 10 + }, + { + a: 3, + b: 5, + c: 3 + }, + { + a: 6, + b: 6, + c: 3 + } + ] + } + }); + + const filter: QueryFilter = { + or: [ + { + a: { + eq: 1 + } + }, + { + a: { + eq: 2 + } + }, + { + a: { + eq: 3 + } + } + ] + } + + const items = await Todo.findBy({ filter }); + + expect(items).toHaveLength(3) + }) + + it('a || (a && b)', async () => { + const { providers: { Todo } } = await setup(` + """ + @model + """ + type Todo { + id: ID + a: Int + b: Int + c: Int + } + `, { + seedData: { + todo: [ + { + a: 1, + b: 5, + c: 8 + }, + { + a: 2, + b: 3, + c: 10 + }, + { + a: 2, + b: 3, + c: 3 + }, + { + a: 6, + b: 6, + c: 3 + } + ] + } + }); + + const filter: QueryFilter = { + or: [ + { + a: { + eq: 1 + }, + }, + { + a: { + eq: 2 + }, + b: { + eq: 3 + } + } + ] + } + + const items = await Todo.findBy({ filter }); + + expect(items).toHaveLength(3) + }) +}) + +test('or > not', async () => { + + const { providers: { Todo } } = await setup(` + """ + @model + """ + type Todo { + id: ID + a: Int + } + `, { + seedData: { + todo: [ + { + a: 1, + }, + { + a: 2, + }, + { + a: 2, + }, + { + a: 3, + }, + { + a: 11 + } + ] + } + }); + + const filter: QueryFilter = { + or: [ + { + a: { + in: [1, 11] + } + }, + { + not: { + a: { + eq: 2 + } + } + } + ] + } + + const items = await Todo.findBy({ filter }); + + expect(items).toHaveLength(3) +}) diff --git a/packages/graphback-runtime-knex/tests/knexQueryMapperTest.ts b/packages/graphback-runtime-knex/tests/knexQueryMapperTest.ts new file mode 100644 index 0000000000..0a65c7b394 --- /dev/null +++ b/packages/graphback-runtime-knex/tests/knexQueryMapperTest.ts @@ -0,0 +1,474 @@ +/* eslint-disable max-lines */ +import { QueryFilter } from "@graphback/core" +import * as Knex from 'knex'; +import { createKnexQueryMapper, CRUDKnexQueryMapper } from "../src/knexQueryMapper" + +describe('knexQueryMapper', () => { + let queryBuilder: CRUDKnexQueryMapper; + let db: Knex; + beforeAll(() => { + db = Knex({ + client: 'sqlite3', + connection: ':memory:', + useNullAsDefault: true + }); + + queryBuilder = createKnexQueryMapper(db); + }); + + test('select *', () => { + expect(queryBuilder.buildQuery(undefined).toQuery()).toEqual("select *") + expect(queryBuilder.buildQuery({}).toQuery()).toEqual("select *") + }); + + test('where name = ?', () => { + const filter: QueryFilter = { + name: { + eq: 'Enda' + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`name` = 'Enda')") + }); + + test('where name <> ?', () => { + const filter: QueryFilter = { + name: { + ne: 'Enda' + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`name` <> 'Enda')") + }); + + test('where age < ?', () => { + const filter: QueryFilter = { + age: { + lt: 25 + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`age` < 25)") + }); + + test('where age <= ?', () => { + const filter: QueryFilter = { + age: { + le: 25 + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`age` <= 25)") + }); + + test('where age > ?', () => { + const filter: QueryFilter = { + age: { + gt: 25 + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`age` > 25)") + }); + + test('where age >= ?', () => { + const filter: QueryFilter = { + age: { + ge: 25 + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`age` >= 25)") + }); + + test('where name = ? and id = ?', () => { + const filter: QueryFilter = { + name: { + eq: 'Enda' + }, + surname: { + eq: 'Stevens' + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`name` = 'Enda' and `surname` = 'Stevens')") + }); + + test('where age in [...]', () => { + const filter: QueryFilter = { + age: { + in: [26, 30, 45] + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual('select * where (`age` in (26, 30, 45))'); + }); + + test('where age between [...]', () => { + const filter: QueryFilter = { + age: { + between: [26, 30] + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual('select * where (`age` between 26 and 30)'); + }); + + describe('and', () => { + test("select * where (`name` = ?) and (`surname` = ?)", () => { + const filter: QueryFilter = { + and: [ + { + name: { + eq: 'Jerry' + } + }, + { + surname: { + eq: 'Seinfeld' + } + } + ] + }; + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where ((`name` = 'Jerry') and (`surname` = 'Seinfeld'))") + }); + + test("select * where (`name` = 'Jerry') and ((`profession` = ?) and ((`surname` = ?) or (`surname` = ?)))", () => { + const filter: QueryFilter = { + name: { + eq: 'Jerry' + }, + and: [ + { + profession: { + eq: 'Actor' + }, + or: [ + { + surname: { + eq: 'Seinfeld' + } + }, + { + surname: { + eq: 'Stiller' + } + } + ] + } + ] + }; + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`name` = 'Jerry') and ((`profession` = 'Actor') and ((`surname` = 'Seinfeld') or (`surname` = 'Stiller')))") + }) + }) + + describe('not', () => { + test('where not name = ?', () => { + const filter: QueryFilter = { + not: { + name: { + eq: 'Enda' + } + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (not (`name` = 'Enda'))") + }); + }) + + describe('or', () => { + test('where select * from name = ? or name = ?', () => { + const filter: QueryFilter = { + or: [ + { + name: { + eq: 'Enda' + }, + }, + { + name: { + eq: 'Matthew' + } + } + ] + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where ((`name` = 'Enda') or (`name` = 'Matthew'))") + }) + + test('where select * from name = ? or (name = ? and surname = ?)', () => { + const filter: QueryFilter = { + or: [ + { + name: { + eq: 'Enda' + }, + }, + { + name: { + eq: 'Lewis' + }, + surname: { + eq: 'Conlon' + } + } + ] + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where ((`name` = 'Enda') or (`name` = 'Lewis' and `surname` = 'Conlon'))") + }); + + test('where select * from users where name = ? and (lname = ? or lname = ?)', () => { + const filter: QueryFilter = { + name: { + eq: 'James' + }, + or: [ + { + surname: { + eq: 'McMorrow' + } + }, + { + surname: { + eq: 'McLean' + } + } + ] + }; + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`name` = 'James') and ((`surname` = 'McMorrow') or (`surname` = 'McLean'))"); + }); + + test('where name = ? and (surname = ? or (surname = ? and id = ?))', () => { + const filter: QueryFilter = { + name: { + eq: 'Sarah' + }, + or: [ + { + surname: { + eq: 'Smith' + } + }, + { + surname: { + eq: 'Simone' + }, + id: { + eq: 1 + } + } + ] + }; + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`name` = 'Sarah') and ((`surname` = 'Smith') or (`surname` = 'Simone' and `id` = 1))") + }) + + test('where name = ? and ((id > ? or id < ?) or (id > ? and lname = ?))', () => { + const filter: QueryFilter = { + name: { + eq: 'Jimi' + }, + or: [ + { + id: { + gt: 99 + } + }, + { + id: { + lt: 49 + } + }, + { + id: { + gt: 2000, + lt: 2500 + }, + surname: { + eq: 'Hendrix' + } + } + ] + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (`name` = 'Jimi') and ((`id` > 99) or (`id` < 49) or (`id` > 2000 and `id` < 2500 and `surname` = 'Hendrix'))") + }) + + test('where ((id > ? or id < ?) and (id > ? or id > ?))', () => { + const filter: QueryFilter = { + or: [ + { + or: [ + { + id: { + gt: 99 + } + }, + { + id: { + lt: 49 + } + } + ] + }, + { + or: [ + { + id: { + gt: 2000, + lt: 2500 + }, + surname: { + eq: 'Hendrix' + } + } + ] + } + ] + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where (((`id` > 99) or (`id` < 49)) and ((`id` > 2000 and `id` < 2500 and `surname` = 'Hendrix')))") + }); + + describe('not', () => { + test('where (not (id = 5 or not id = 2)', () => { + const filter: QueryFilter = { + not: { + or: [ + { + id: { + eq: 5 + } + }, + { + id: { + eq: 2 + } + } + ] + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual("select * where ((not (`id` = 5) or not (`id` = 2)))") + }); + }) + }); + + describe('nested root conditions', () => { + test('or > not', () => { + const filter: QueryFilter = { + or: [ + { + a: { + in: [1, 11] + } + }, + { + not: { + a: { + eq: 2 + } + } + } + ] + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual('select * where ((`a` in (1, 11)) or (not (`a` = 2)))') + }) + + test('not > or', () => { + const filter: QueryFilter = { + not: { + or: [ + { + a: { + eq: 1 + } + }, + { + a: { + eq: 2 + } + } + ] + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual('select * where ((not (`a` = 1) or not (`a` = 2)))') + }) + + test('not > and', () => { + const filter: QueryFilter = { + not: { + and: [ + { + a: { + eq: 1 + } + }, + { + a: { + eq: 2 + } + } + ] + } + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual('select * where ((not (`a` = 1) and not (`a` = 2)))') + }) + + test('and > or', () => { + const filter: QueryFilter = { + and: [ + { + c: { + eq: 1 + } + }, + { + or: [ + { + a: { + eq: 1 + } + }, + { + a: { + eq: 2 + } + } + ] + } + ] + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual('select * where ((`c` = 1) and ((`a` = 1) or (`a` = 2)))') + }) + + test('and > not', () => { + const filter: QueryFilter = { + and: [ + { + a: { + eq: 1 + } + }, + { + not: { + a: { + eq: 2 + } + } + } + ] + } + + expect(queryBuilder.buildQuery(filter).toQuery()).toEqual('select * where ((`a` = 1) and (not (`a` = 2)))') + }) + }) +}); \ No newline at end of file From 38fe0924e7d822179e9ef2784b2cb0544fc3998c Mon Sep 17 00:00:00 2001 From: Enda Phelan Date: Mon, 14 Sep 2020 10:24:28 +0100 Subject: [PATCH 3/4] refactor(core): add stronger typings to QueryFilter --- docs/releases.md | 11 +++++++++++ packages/graphback-core/src/runtime/QueryFilter.ts | 11 +++++++++-- .../tests/createInMemoryFilterPredicateTest.ts | 8 ++++---- .../graphback-runtime-knex/src/knexQueryMapper.ts | 2 +- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/docs/releases.md b/docs/releases.md index ce9b8da1bb..822f1285a8 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -8,6 +8,17 @@ title: Release Notes This file contains changes and migration steps for the Graphback project. Please follow individual releases for more information. +# 0.17.0 + +### Bug Fixes + +* Logical `or` filter selector was not mapped correctly in both `graphback-runtime-mongo` ([#2034](https://github.com/aerogear/graphback/pull/2034), fixed by [1ebe7e9](https://github.com/aerogear/graphback/pull/2034/commits/1ebe7e9bc8d3a61f0b3ef65b588881d16b7ae63f)) +* Logical `or` filter selector was not mapped correctly in both `graphback-runtime-knex` ([#2034](https://github.com/aerogear/graphback/pull/2034), fixed by [6d43f28](https://github.com/aerogear/graphback/commit/6d43f288865a2c8c0d441e486a156301ca6cc42a)) + +### Breaking Changes + +* Refactored the Knex Query Mapper ([#2034](https://github.com/aerogear/graphback/pull/2034), fixed by [6d43f28](https://github.com/aerogear/graphback/commit/6d43f288865a2c8c0d441e486a156301ca6cc42a)) + # 0.16.2 ### Bug Fixes diff --git a/packages/graphback-core/src/runtime/QueryFilter.ts b/packages/graphback-core/src/runtime/QueryFilter.ts index 3846f63641..01055b5539 100644 --- a/packages/graphback-core/src/runtime/QueryFilter.ts +++ b/packages/graphback-core/src/runtime/QueryFilter.ts @@ -145,9 +145,16 @@ export type GraphbackTimestampInput = { type GraphbackScalarInput = GraphbackDateInput | GraphbackDateTimeInput | GraphbackObjectIdInput | GraphbackTimeInput | GraphbackTimestampInput; +export type QueryFilterOperator = keyof IdInput | keyof BooleanInput | keyof StringInput | keyof FloatInput | keyof IntInput | keyof GraphbackScalarInput; /** * Query filter used in Graphback services and data providers */ export type QueryFilter = { - [P in keyof T | 'and' | 'or' | 'not']?: Maybe; -}; + [P in keyof T]: IdInput | BooleanInput | StringInput | FloatInput | IntInput | GraphbackScalarInput | any; +} & RootQuerySelector; + +type RootQuerySelector = { + and?: QueryFilter[]; + or?: QueryFilter[]; + not?: QueryFilter; +} \ No newline at end of file diff --git a/packages/graphback-core/tests/createInMemoryFilterPredicateTest.ts b/packages/graphback-core/tests/createInMemoryFilterPredicateTest.ts index 609ff6ce40..c8c01695f7 100644 --- a/packages/graphback-core/tests/createInMemoryFilterPredicateTest.ts +++ b/packages/graphback-core/tests/createInMemoryFilterPredicateTest.ts @@ -871,14 +871,14 @@ describe('createInMemoryFilterPredicate', () => { name: { endsWith: 'Simpson' }, - and: { + and: [{ age: { gt: 10 }, verified: { eq: true } - } + }] } const filterSubscription = createInMemoryFilterPredicate(filter) @@ -929,11 +929,11 @@ describe('createInMemoryFilterPredicate', () => { name: { eq: 'Homer Simpson' }, - or: { + or: [{ name: { eq: 'Homer Thompson' } - } + }] } const filterSubscription = createInMemoryFilterPredicate(filter) diff --git a/packages/graphback-runtime-knex/src/knexQueryMapper.ts b/packages/graphback-runtime-knex/src/knexQueryMapper.ts index a1ada00ac3..2d4d8df380 100644 --- a/packages/graphback-runtime-knex/src/knexQueryMapper.ts +++ b/packages/graphback-runtime-knex/src/knexQueryMapper.ts @@ -84,7 +84,7 @@ const rootSelectorMapper: { [key in RootQuerySelector]: KnexRootQuerySelectorBui } /** - * Wraps Kne methods and pipe the QueryFilter conditions into a final Knex condition + * Wraps Knex methods and pipe the QueryFilter conditions into a final Knex condition */ const methodBuilderMapper: { [key in KnexMethod | 'finally']: KnexQueryBuilderMapFn } = { where: (builder: Knex.QueryBuilder, filter: QueryFilter): Knex.QueryBuilder => { From 230a6dff40dcbe0bf7c76ce6a0e97008306cb98c Mon Sep 17 00:00:00 2001 From: Enda Phelan Date: Mon, 14 Sep 2020 14:13:38 +0100 Subject: [PATCH 4/4] fix: or condition in subscription filter --- docs/releases.md | 7 ++-- .../runtime/createInMemoryFilterPredicate.ts | 37 ++++++++----------- .../createInMemoryFilterPredicateTest.ts | 33 ++++++++++------- 3 files changed, 38 insertions(+), 39 deletions(-) diff --git a/docs/releases.md b/docs/releases.md index 822f1285a8..358875f4c0 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -8,12 +8,13 @@ title: Release Notes This file contains changes and migration steps for the Graphback project. Please follow individual releases for more information. -# 0.17.0 +# 1.0.0 ### Bug Fixes -* Logical `or` filter selector was not mapped correctly in both `graphback-runtime-mongo` ([#2034](https://github.com/aerogear/graphback/pull/2034), fixed by [1ebe7e9](https://github.com/aerogear/graphback/pull/2034/commits/1ebe7e9bc8d3a61f0b3ef65b588881d16b7ae63f)) -* Logical `or` filter selector was not mapped correctly in both `graphback-runtime-knex` ([#2034](https://github.com/aerogear/graphback/pull/2034), fixed by [6d43f28](https://github.com/aerogear/graphback/commit/6d43f288865a2c8c0d441e486a156301ca6cc42a)) +* Logical `or` filter selector was not mapped correctly in `graphback-runtime-mongo` ([#2034](https://github.com/aerogear/graphback/pull/2034), fixed by [1ebe7e9](https://github.com/aerogear/graphback/pull/2034/commits/1ebe7e9bc8d3a61f0b3ef65b588881d16b7ae63f)) +* Logical `or` filter selector was not mapped correctly in `graphback-runtime-knex` ([#2034](https://github.com/aerogear/graphback/pull/2034), fixed by [6d43f28](https://github.com/aerogear/graphback/commit/6d43f288865a2c8c0d441e486a156301ca6cc42a)) +* Logical `or` predicate was not applied correctly in `createInMemoryPredicateFilter` ([#2034](https://github.com/aerogear/graphback/pull/2034), fixed by [01f9912](https://github.com/aerogear/graphback/commit/01f99121a9462e5a277657359094ab131e6f809c)) ### Breaking Changes diff --git a/packages/graphback-core/src/runtime/createInMemoryFilterPredicate.ts b/packages/graphback-core/src/runtime/createInMemoryFilterPredicate.ts index 221458935f..99fcd756fd 100644 --- a/packages/graphback-core/src/runtime/createInMemoryFilterPredicate.ts +++ b/packages/graphback-core/src/runtime/createInMemoryFilterPredicate.ts @@ -84,7 +84,7 @@ const predicateMap: IPredicate = { } // if values are of MongoDb ObjectID, convert to timestamp before comparison - if (isObjectID(fromVal) || isObjectID(toVal)|| isObjectID(fieldValue)) { + if (isObjectID(fromVal) || isObjectID(toVal) || isObjectID(fieldValue)) { const toValTimestamp = getObjectIDTimestamp(parseObjectID(toVal.toString())); const fromValTimestamp = getObjectIDTimestamp(parseObjectID(fromVal.toString())); const fieldValTimestamp = getObjectIDTimestamp(parseObjectID(fieldValue.toString())); @@ -143,7 +143,7 @@ export function createInMemoryFilterPredicate(filter: QueryFilter): (in if (orFilter) { const orPredicateResult = getOrPredicateResult(orFilter, payload); - predicateResult = predicateResult || orPredicateResult; + predicateResult = predicateResult && orPredicateResult; if (!predicateResult) { return false; } @@ -164,22 +164,18 @@ export function createInMemoryFilterPredicate(filter: QueryFilter): (in /** * Get the predicate result of an `and` filter expression * - * @param {QueryFilter|QueryFilter[]} and - And filter + * @param {QueryFilter[]} and - And filter * @param {Partial} payload - Subscription payload */ -function getAndPredicateResult(and: QueryFilter | QueryFilter[], payload: Partial): boolean { +function getAndPredicateResult(and: QueryFilter[], payload: Partial): boolean { let andResult = true; - if (Array.isArray(and)) { - for (const andItem of and) { - andResult = createInMemoryFilterPredicate(andItem)(payload); + for (const andItem of and) { + andResult = createInMemoryFilterPredicate(andItem)(payload); - if (!andResult) { - break; - } + if (!andResult) { + break; } - } else { - andResult = createInMemoryFilterPredicate(and)(payload); } return andResult; @@ -188,21 +184,18 @@ function getAndPredicateResult(and: QueryFilter | QueryFilter[], payload: Par /** * Get the boolean result of an `or` filter expression * - * @param {QueryFilter|QueryFilter[]} or - Or query filter + * @param {QueryFilter[]} or - Or query filter * @param {Partial} payload - Subscription payload */ -function getOrPredicateResult(or: any | any[], payload: Partial): boolean { +function getOrPredicateResult(or: QueryFilter[], payload: Partial): boolean { let orResult = true; - if (Array.isArray(or)) { - for (const orItem of or) { - orResult = createInMemoryFilterPredicate(orItem)(payload); - if (orResult) { - break; - } + for (const orItem of or) { + orResult = createInMemoryFilterPredicate(orItem)(payload); + + if (orResult) { + break; } - } else { - orResult = createInMemoryFilterPredicate(or)(payload); } return orResult; diff --git a/packages/graphback-core/tests/createInMemoryFilterPredicateTest.ts b/packages/graphback-core/tests/createInMemoryFilterPredicateTest.ts index c8c01695f7..49d1098ba8 100644 --- a/packages/graphback-core/tests/createInMemoryFilterPredicateTest.ts +++ b/packages/graphback-core/tests/createInMemoryFilterPredicateTest.ts @@ -929,42 +929,47 @@ describe('createInMemoryFilterPredicate', () => { name: { eq: 'Homer Simpson' }, - or: [{ - name: { - eq: 'Homer Thompson' + or: [ + { + age: { + eq: 39 + } } - }] + ] } const filterSubscription = createInMemoryFilterPredicate(filter) - expect(filterSubscription({ name: 'Homer Simpson' })).toEqual(true) - expect(filterSubscription({ name: 'Homer Thompson' })).toEqual(true) - expect(filterSubscription({ name: 'Bart Simpson' })).toEqual(false) + expect(filterSubscription({ name: 'Homer Simpson', age: 39 })).toEqual(true) + expect(filterSubscription({ name: 'Homer Simpson', age: 38 })).toEqual(false) + expect(filterSubscription({ name: 'Homer Thompson', age: 39 })).toEqual(false) }); test('or multiple', () => { const filter: QueryFilter = { - name: { - eq: 'Homer J Simpson' + age: { + eq: 39 }, or: [ { name: { - eq: 'Homer Simpson' + eq: 'Homer Thompson' } }, { name: { - eq: 'Homer Thompson' + eq: 'Homer Simpson' } } ] } const filterSubscription = createInMemoryFilterPredicate(filter) - expect(filterSubscription({ name: 'Homer Simpson' })).toEqual(true) - expect(filterSubscription({ name: 'Homer Thompson' })).toEqual(true) - expect(filterSubscription({ name: 'Bart Simpson' })).toEqual(false) + expect(filterSubscription({ name: 'Homer Simpson' })).toEqual(false) + expect(filterSubscription({ name: 'Homer Simpson', age: 39 })).toEqual(true) + expect(filterSubscription({ name: 'Homer Thompson', age: 39 })).toEqual(true) + expect(filterSubscription({ name: 'Homer Simpson', age: 38 })).toEqual(false) + expect(filterSubscription({ name: 'Homer Thompson', age: 38 })).toEqual(false) + expect(filterSubscription({ name: 'Homer J Simpson', age: 39 })).toEqual(false) }); describe('empty or undefined filter', () => {