Merge pull request #88 from bigalorm/unexpected-empty-where

Throw error if query has an empty or statement
bigalorm · Oct 4, 2023 · 97c89e7 · 97c89e7
2 parents 95f1634 + c332fa4
commit 97c89e7
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 7 deletions.
diff --git a/src/SqlHelper.ts b/src/SqlHelper.ts
@@ -637,14 +637,18 @@ export function buildWhereStatement<T extends Entity>({
   params: unknown[];
 } {
   let whereStatement;
-  if (_.isObject(where)) {
+  if (_.isObject(where) && Object.keys(where).length) {
     whereStatement = buildWhere({
       repositoriesByModelNameLowered,
       model,
       comparer: 'and',
       value: where,
       params,
     });
+
+    if (!whereStatement) {
+      throw new QueryError(`WHERE statement is unexpectedly empty.`, model, where);
+    }
   }
 
   if (whereStatement) {
@@ -1043,7 +1047,11 @@ function buildWhere<T extends Entity>({
           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<T extends Entity>({
       params,
     });
 
-    orClauses.push(`(${orClause})`);
+    if (orClause) {
+      orClauses.push(`(${orClause})`);
+    }
   }
 
   if (orClauses.length === 1) {
@@ -1121,7 +1131,11 @@ function buildOrOperatorStatement<T extends Entity>({
     return orClauses.join(' AND ');
   }
 
-  return `(${orClauses.join(' OR ')})`;
+  if (orClauses.length) {
+    return `(${orClauses.join(' OR ')})`;
+  }
+
+  return '';
 }
 
 interface ComparisonOperatorStatementParams<T extends Entity> {
@@ -1192,7 +1206,11 @@ function buildLikeOperatorStatement<T extends Entity>({ 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
@@ -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
@@ -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<Product>,
+          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<Product>,
+        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<Product>,
+        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();