From c332fa4d5c26b9f65a3e383486ab80474bff25ee Mon Sep 17 00:00:00 2001 From: Jim Geurts Date: Wed, 4 Oct 2023 16:11:30 -0500 Subject: [PATCH] Throw error if query has an empty or statement --- src/SqlHelper.ts | 28 +++++++++++++--- tests/models/Product.ts | 2 +- tests/sqlHelper.tests.ts | 71 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 94 insertions(+), 7 deletions(-) diff --git a/src/SqlHelper.ts b/src/SqlHelper.ts index 678c60f..2705bbd 100644 --- a/src/SqlHelper.ts +++ b/src/SqlHelper.ts @@ -637,7 +637,7 @@ export function buildWhereStatement({ params: unknown[]; } { let whereStatement; - if (_.isObject(where)) { + if (_.isObject(where) && Object.keys(where).length) { whereStatement = buildWhere({ repositoriesByModelNameLowered, model, @@ -645,6 +645,10 @@ export function buildWhereStatement({ value: where, params, }); + + if (!whereStatement) { + throw new QueryError(`WHERE statement is unexpectedly empty.`, model, where); + } } if (whereStatement) { @@ -1043,7 +1047,11 @@ function buildWhere({ return orConstraints.join(' AND '); } - return `(${orConstraints.join(' OR ')})`; + if (orConstraints.length) { + return `(${orConstraints.join(' OR ')})`; + } + + return ''; } if (_.isObject(value) && !_.isDate(value)) { @@ -1110,7 +1118,9 @@ function buildOrOperatorStatement({ params, }); - orClauses.push(`(${orClause})`); + if (orClause) { + orClauses.push(`(${orClause})`); + } } if (orClauses.length === 1) { @@ -1121,7 +1131,11 @@ function buildOrOperatorStatement({ return orClauses.join(' AND '); } - return `(${orClauses.join(' OR ')})`; + if (orClauses.length) { + return `(${orClauses.join(' OR ')})`; + } + + return ''; } interface ComparisonOperatorStatementParams { @@ -1192,7 +1206,11 @@ function buildLikeOperatorStatement({ model, propertyName, isN return orConstraints.join(' AND '); } - return `(${orConstraints.join(' OR ')})`; + if (orConstraints.length) { + return `(${orConstraints.join(' OR ')})`; + } + + return ''; } // eslint-disable-next-line no-param-reassign diff --git a/tests/models/Product.ts b/tests/models/Product.ts index 791aa3c..b158ed9 100644 --- a/tests/models/Product.ts +++ b/tests/models/Product.ts @@ -33,7 +33,7 @@ export class Product extends ModelBase { defaultsTo: [], name: 'alias_names', }) - public aliases?: string[]; + public aliases!: string[]; @column({ model: 'store', // NOTE: Lower case to test that case doesn't matter diff --git a/tests/sqlHelper.tests.ts b/tests/sqlHelper.tests.ts index 8aaf65b..efafbb4 100644 --- a/tests/sqlHelper.tests.ts +++ b/tests/sqlHelper.tests.ts @@ -7,6 +7,7 @@ import { mock } from 'ts-mockito'; import { initialize } from '../src'; import type { IReadonlyRepository, IRepository, Entity, ModelMetadata } from '../src'; +import { QueryError } from '../src/errors'; import * as sqlHelper from '../src/SqlHelper'; import { @@ -1179,7 +1180,7 @@ describe('sqlHelper', () => { store: undefined, }, }); - }).should.throw(Error, `Attempting to query with an undefined value. store on ${repositoriesByModelNameLowered.product.model.name}`); + }).should.throw(QueryError, `Attempting to query with an undefined value. store on ${repositoriesByModelNameLowered.product.model.name}`); }); it('should use column name if defined', () => { const storeId = faker.number.int(); @@ -1659,6 +1660,74 @@ describe('sqlHelper', () => { whereStatement.should.equal('WHERE (("name"=$1) OR ("name"<>$2 AND "store_id"=$3))'); params.should.deep.equal([name, name, store]); }); + it('should throw for empty or', () => { + ((): void => { + sqlHelper.buildWhereStatement({ + repositoriesByModelNameLowered, + model: repositoriesByModelNameLowered.product.model as ModelMetadata, + where: { + or: [], + }, + }); + }).should.throw(QueryError, `WHERE statement is unexpectedly empty.`); + }); + it('should handle or with a int field and a string array field', () => { + const alias1 = faker.string.uuid(); + const alias2 = faker.string.uuid(); + const alias3 = faker.string.uuid(); + const store = faker.number.int(); + const store2 = faker.number.int(); + const { whereStatement, params } = sqlHelper.buildWhereStatement({ + repositoriesByModelNameLowered, + model: repositoriesByModelNameLowered.product.model as ModelMetadata, + where: { + or: [ + { + store, + aliases: alias1, + }, + { + store, + aliases: alias2, + }, + { + store: store2, + aliases: alias3, + }, + ], + }, + }); + + assert(whereStatement); + whereStatement.should.equal('WHERE (("store_id"=$1 AND $2=ANY("alias_names")) OR ("store_id"=$3 AND $4=ANY("alias_names")) OR ("store_id"=$5 AND $6=ANY("alias_names")))'); + params.should.deep.equal([store, alias1, store, alias2, store2, alias3]); + }); + it('should handle or with two string fields', () => { + const name = faker.string.uuid(); + const name1 = faker.string.uuid(); + const location1 = faker.string.uuid(); + const location2 = faker.string.uuid(); + const { whereStatement, params } = sqlHelper.buildWhereStatement({ + repositoriesByModelNameLowered, + model: repositoriesByModelNameLowered.product.model as ModelMetadata, + where: { + or: [ + { + name, + location: location1, + }, + { + name: name1, + location: location2, + }, + ], + }, + }); + + assert(whereStatement); + whereStatement.should.equal('WHERE (("name"=$1 AND "location"=$2) OR ("name"=$3 AND "location"=$4))'); + params.should.deep.equal([name, location1, name1, location2]); + }); it('should handle mixed or/and constraints', () => { const id = faker.number.int(); const name = faker.string.uuid();