diff --git a/packages/orm/src/client/crud/dialects/base-dialect.ts b/packages/orm/src/client/crud/dialects/base-dialect.ts index 169213e62..c6b4fe72d 100644 --- a/packages/orm/src/client/crud/dialects/base-dialect.ts +++ b/packages/orm/src/client/crud/dialects/base-dialect.ts @@ -260,14 +260,41 @@ export abstract class BaseCrudDialect { key: (typeof LOGICAL_COMBINATORS)[number], payload: any, ): Expression { + // Normalize payload: ensure array, remove empty objects and objects with + // only undefined fields values + const normalizedPayload = enumerate(payload).filter((el) => { + if (typeof el === 'object' && el !== null && !Array.isArray(el)) { + const entries = Object.entries(el); + return entries.some(([, v]) => v !== undefined); + } else { + return true; + } + }); + + const normalizedFilters = normalizedPayload.map((el) => this.buildFilter(model, modelAlias, el)); + return match(key) - .with('AND', () => - this.and(...enumerate(payload).map((subPayload) => this.buildFilter(model, modelAlias, subPayload))), - ) - .with('OR', () => - this.or(...enumerate(payload).map((subPayload) => this.buildFilter(model, modelAlias, subPayload))), - ) - .with('NOT', () => this.eb.not(this.buildCompositeFilter(model, modelAlias, 'AND', payload))) + .with('AND', () => { + if (normalizedFilters.length === 0) { + // AND of no conditions is a no-op, return true + return this.true(); + } + return this.and(...normalizedFilters); + }) + .with('OR', () => { + if (normalizedFilters.length === 0) { + // OR of no conditions is always false, return false + return this.false(); + } + return this.or(...normalizedFilters); + }) + .with('NOT', () => { + if (normalizedFilters.length === 0) { + // NOT of no conditions is a no-op, return true + return this.true(); + } + return this.not(...normalizedFilters); + }) .exhaustive(); } @@ -800,6 +827,9 @@ export abstract class BaseCrudDialect { if (excludeKeys.includes(op)) { continue; } + if (value === undefined) { + continue; + } const rhs = Array.isArray(value) ? value.map(getRhs) : getRhs(value); const condition = match(op) .with('equals', () => (rhs === null ? this.eb(lhs, 'is', null) : this.eb(lhs, '=', rhs))) @@ -881,6 +911,10 @@ export abstract class BaseCrudDialect { continue; } + if (value === undefined) { + continue; + } + invariant(typeof value === 'string', `${key} value must be a string`); const escapedValue = this.escapeLikePattern(value); diff --git a/packages/orm/src/client/crud/operations/base.ts b/packages/orm/src/client/crud/operations/base.ts index 46306c034..94ac64ad3 100644 --- a/packages/orm/src/client/crud/operations/base.ts +++ b/packages/orm/src/client/crud/operations/base.ts @@ -18,7 +18,7 @@ import * as uuid from 'uuid'; import type { BuiltinType, Expression, FieldDef } from '../../../schema'; import { ExpressionUtils, type GetModels, type ModelDef, type SchemaDef } from '../../../schema'; import type { AnyKysely } from '../../../utils/kysely-utils'; -import { extractFields, fieldsToSelectObject } from '../../../utils/object-utils'; +import { extractFields, fieldsToSelectObject, isEmptyObject } from '../../../utils/object-utils'; import { NUMERIC_FIELD_TYPES } from '../../constants'; import { TransactionIsolationLevel, type ClientContract, type CRUD } from '../../contract'; import type { FindArgs, SelectIncludeOmit, WhereInput } from '../../crud-types'; @@ -1152,7 +1152,7 @@ export abstract class BaseOperationHandler { const parentWhere = await this.buildUpdateParentRelationFilter(kysely, fromRelation); - let combinedWhere: Record = where ?? {}; + let combinedWhere: Record = where ?? true; if (Object.keys(parentWhere).length > 0) { combinedWhere = Object.keys(combinedWhere).length > 0 ? { AND: [parentWhere, combinedWhere] } : parentWhere; } @@ -1574,7 +1574,7 @@ export abstract class BaseOperationHandler { } const parentWhere = await this.buildUpdateParentRelationFilter(kysely, fromRelation); - let combinedWhere: WhereInput, any, false> = where ?? {}; + let combinedWhere: WhereInput, any, false> = where ?? true; if (Object.keys(parentWhere).length > 0) { combinedWhere = Object.keys(combinedWhere).length > 0 ? { AND: [parentWhere, combinedWhere] } : parentWhere; } @@ -1890,12 +1890,12 @@ export abstract class BaseOperationHandler { } case 'delete': { - await this.deleteRelation(kysely, fieldModel, value, fromRelationContext, true); + await this.deleteRelation(kysely, fieldModel, value, fromRelationContext, true, true); break; } case 'deleteMany': { - await this.deleteRelation(kysely, fieldModel, value, fromRelationContext, false); + await this.deleteRelation(kysely, fieldModel, value, fromRelationContext, false, false); break; } @@ -2031,6 +2031,9 @@ export abstract class BaseOperationHandler { } else { disconnectConditions = [true]; } + } else if (isEmptyObject(data)) { + // empty object means true + disconnectConditions = [true]; } else { disconnectConditions = this.normalizeRelationManipulationInput(model, data); @@ -2235,15 +2238,24 @@ export abstract class BaseOperationHandler { model: GetModels, data: any, fromRelation: FromRelationContext, + uniqueDelete: boolean, throwForNotFound: boolean, ) { let deleteConditions: any[] = []; - let expectedDeleteCount: number; + let expectedDeleteCount = -1; // -1 means no expected delete count check if (typeof data === 'boolean') { if (data === false) { return; } else { deleteConditions = [true]; + if (uniqueDelete) { + expectedDeleteCount = 1; + } + } + } else if (isEmptyObject(data)) { + // empty object means true + deleteConditions = [true]; + if (uniqueDelete) { expectedDeleteCount = 1; } } else { @@ -2251,7 +2263,10 @@ export abstract class BaseOperationHandler { if (deleteConditions.length === 0) { return; } - expectedDeleteCount = deleteConditions.length; + if (uniqueDelete) { + // each delete condition is one unique match + expectedDeleteCount = deleteConditions.length; + } } let deleteResult: Awaited>; @@ -2319,7 +2334,7 @@ export abstract class BaseOperationHandler { } // validate result - if (throwForNotFound && expectedDeleteCount > (deleteResult.numAffectedRows ?? 0)) { + if (throwForNotFound && expectedDeleteCount >= 0 && expectedDeleteCount > (deleteResult.numAffectedRows ?? 0)) { // some entities were not deleted throw createNotFoundError(deleteFromModel); } diff --git a/packages/orm/src/utils/object-utils.ts b/packages/orm/src/utils/object-utils.ts index ba9a0886d..4214f0fbb 100644 --- a/packages/orm/src/utils/object-utils.ts +++ b/packages/orm/src/utils/object-utils.ts @@ -1,3 +1,5 @@ +import { isPlainObject } from '@zenstackhq/common-helpers'; + /** * Extract fields from an object. */ @@ -11,3 +13,10 @@ export function extractFields(obj: any, fields: readonly string[]) { export function fieldsToSelectObject(fields: readonly string[]): Record { return Object.fromEntries(fields.map((f) => [f, true])); } + +/** + * Checks if the given value is an empty plain object. + */ +export function isEmptyObject(x: unknown) { + return isPlainObject(x) && Object.keys(x as object).length === 0; +} diff --git a/tests/e2e/orm/client-api/filter.test.ts b/tests/e2e/orm/client-api/filter.test.ts index 36173ca2d..a6524da45 100644 --- a/tests/e2e/orm/client-api/filter.test.ts +++ b/tests/e2e/orm/client-api/filter.test.ts @@ -801,5 +801,81 @@ describe('Client filter tests ', () => { await expect(client.user.findMany({ where: { id: undefined } })).toResolveWithLength(1); }); + it('ignores undefined branch inside OR filters', async () => { + await createUser('u1@test.com', { + name: 'First', + role: 'ADMIN', + profile: { create: { id: 'p1', bio: 'bio1' } }, + }); + const user2 = await createUser('u2@test.com', { + name: 'Second', + role: 'USER', + profile: { create: { id: 'p2', bio: 'bio2' } }, + }); + + const baseline = await client.user.findFirst({ + where: { + OR: [{ id: user2.id }], + } as any, + orderBy: { createdAt: 'asc' }, + }); + + const withUndefinedBranch = await client.user.findFirst({ + where: { + OR: [{ id: undefined }, { id: user2.id }], + } as any, + orderBy: { createdAt: 'asc' }, + }); + + const onlyUndefinedBranch = await client.user.findFirst({ + where: { + OR: [{ id: undefined }], + } as any, + orderBy: { createdAt: 'asc' }, + }); + + expect(baseline?.email).toBe(user2.email); + expect(withUndefinedBranch?.email).toBe(baseline?.email); + expect(onlyUndefinedBranch).toBeNull(); + }); + + it('strips undefined filter operators inside OR branches', async () => { + await createUser('alice@test.com', { name: 'Alice', role: 'ADMIN' }); + await createUser('bob@test.com', { name: 'Bob', role: 'USER' }); + + const result = await client.user.findMany({ + where: { + OR: [{ name: { startsWith: 'A', contains: undefined } }], + } as any, + }); + + expect(result).toHaveLength(1); + expect(result[0]!.name).toBe('Alice'); + }); + + describe('AND/OR/NOT with no-op filters', () => { + beforeEach(async () => { + await createUser('u1@test.com', { name: 'Alice', role: 'ADMIN' }); + await createUser('u2@test.com', { name: 'Bob', role: 'USER' }); + }); + + it('AND is no-op for empty array, array with all-undefined object, and plain all-undefined object', async () => { + await expect(client.user.findMany({ where: { AND: [] } })).toResolveWithLength(2); + await expect(client.user.findMany({ where: { AND: [{ id: undefined }] } })).toResolveWithLength(2); + await expect(client.user.findMany({ where: { AND: { id: undefined } } as any })).toResolveWithLength(2); + }); + + it('OR returns no records for empty array, array with all-undefined object, and plain all-undefined object', async () => { + await expect(client.user.findMany({ where: { OR: [] } })).toResolveWithLength(0); + await expect(client.user.findMany({ where: { OR: [{ id: undefined }] } })).toResolveWithLength(0); + }); + + it('NOT is no-op for empty array, array with all-undefined object, and plain all-undefined object', async () => { + await expect(client.user.findMany({ where: { NOT: [] } })).toResolveWithLength(2); + await expect(client.user.findMany({ where: { NOT: [{ id: undefined }] } })).toResolveWithLength(2); + await expect(client.user.findMany({ where: { NOT: { id: undefined } } })).toResolveWithLength(2); + }); + }); + // TODO: filter for bigint, decimal, bytes }); diff --git a/tests/e2e/orm/client-api/find.test.ts b/tests/e2e/orm/client-api/find.test.ts index 75f684de3..1ea151ab1 100644 --- a/tests/e2e/orm/client-api/find.test.ts +++ b/tests/e2e/orm/client-api/find.test.ts @@ -479,7 +479,7 @@ describe('Client find tests ', () => { ).toResolveFalsy(); // NOT - await expect(client.user.findMany({ where: { NOT: [] } })).resolves.toHaveLength(0); + await expect(client.user.findMany({ where: { NOT: [] } })).resolves.toHaveLength(2); await expect( client.user.findFirst({ where: {