Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

refactor(knex): refactor knex query builder and fixes an issue where or queries were not mapped correctly #2034

Merged
merged 4 commits into from
Sep 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/releases.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ title: Release Notes
This file contains changes and migration steps for the Graphback project.
Please follow individual releases for more information.

# 1.0.0

### Bug Fixes

* 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

* 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
Expand Down
11 changes: 9 additions & 2 deletions packages/graphback-core/src/runtime/QueryFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T = any> = {
[P in keyof T | 'and' | 'or' | 'not']?: Maybe<IdInput | BooleanInput | StringInput | FloatInput | IntInput | GraphbackScalarInput | T | T[]>;
};
[P in keyof T]: IdInput | BooleanInput | StringInput | FloatInput | IntInput | GraphbackScalarInput | any;
} & RootQuerySelector<T>;

type RootQuerySelector<T = any> = {
and?: QueryFilter<T>[];
or?: QueryFilter<T>[];
not?: QueryFilter<T>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down Expand Up @@ -143,7 +143,7 @@ export function createInMemoryFilterPredicate<T = any>(filter: QueryFilter): (in

if (orFilter) {
const orPredicateResult = getOrPredicateResult<T>(orFilter, payload);
predicateResult = predicateResult || orPredicateResult;
predicateResult = predicateResult && orPredicateResult;
if (!predicateResult) {
return false;
}
Expand All @@ -164,22 +164,18 @@ export function createInMemoryFilterPredicate<T = any>(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<T>} payload - Subscription payload
*/
function getAndPredicateResult<T>(and: QueryFilter | QueryFilter[], payload: Partial<T>): boolean {
function getAndPredicateResult<T>(and: QueryFilter[], payload: Partial<T>): boolean {
let andResult = true;

if (Array.isArray(and)) {
for (const andItem of and) {
andResult = createInMemoryFilterPredicate<T>(andItem)(payload);
for (const andItem of and) {
andResult = createInMemoryFilterPredicate<T>(andItem)(payload);

if (!andResult) {
break;
}
if (!andResult) {
break;
}
} else {
andResult = createInMemoryFilterPredicate<T>(and)(payload);
}

return andResult;
Expand All @@ -188,21 +184,18 @@ function getAndPredicateResult<T>(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<T>} payload - Subscription payload
*/
function getOrPredicateResult<T>(or: any | any[], payload: Partial<T>): boolean {
function getOrPredicateResult<T>(or: QueryFilter[], payload: Partial<T>): boolean {
let orResult = true;
if (Array.isArray(or)) {
for (const orItem of or) {
orResult = createInMemoryFilterPredicate<T>(orItem)(payload);

if (orResult) {
break;
}
for (const orItem of or) {
orResult = createInMemoryFilterPredicate<T>(orItem)(payload);

if (orResult) {
break;
}
} else {
orResult = createInMemoryFilterPredicate<T>(or)(payload);
}

return orResult;
Expand Down
37 changes: 21 additions & 16 deletions packages/graphback-core/tests/createInMemoryFilterPredicateTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -871,14 +871,14 @@ describe('createInMemoryFilterPredicate', () => {
name: {
endsWith: 'Simpson'
},
and: {
and: [{
machi1990 marked this conversation as resolved.
Show resolved Hide resolved
age: {
gt: 10
},
verified: {
eq: true
}
}
}]
}

const filterSubscription = createInMemoryFilterPredicate<User>(filter)
Expand Down Expand Up @@ -929,42 +929,47 @@ describe('createInMemoryFilterPredicate', () => {
name: {
eq: 'Homer Simpson'
},
or: {
name: {
eq: 'Homer Thompson'
or: [
{
age: {
eq: 39
}
}
}
]
}

const filterSubscription = createInMemoryFilterPredicate<User>(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<UserSubscriptionFilter> = {
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<User>(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', () => {
Expand Down
12 changes: 7 additions & 5 deletions packages/graphback-runtime-knex/src/KnexDBDataProvider.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -18,9 +18,11 @@ export class KnexDBDataProvider<Type = any> implements GraphbackDataProvider<Typ
protected db: Knex;
protected tableName: string;
protected tableMap: ModelTableMap;
protected queryBuilder: CRUDKnexQueryMapper;

public constructor(model: ModelDefinition, db: Knex) {
this.db = db;
this.queryBuilder = createKnexQueryMapper(db);
this.tableMap = buildModelTableMap(model.graphqlType);
this.tableName = this.tableMap.tableName;
}
Expand Down Expand Up @@ -70,7 +72,7 @@ export class KnexDBDataProvider<Type = any> implements GraphbackDataProvider<Typ
}

public async findBy(args?: FindByArgs, selectedFields?: string[]): Promise<Type[]> {
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)
Expand All @@ -86,14 +88,14 @@ export class KnexDBDataProvider<Type = any> implements GraphbackDataProvider<Typ
}

public async count(filter?: QueryFilter): Promise<number> {
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<Type[][]> {
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) => {
Expand Down
Loading