Skip to content

Commit c6c8ad1

Browse files
eredzikymc9
eredzik
and
ymc9
authored
fix(orm): unify zenstack v3 and prisma behavior on empty branches in OR filter (#2528)
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
1 parent 5a60fb2 commit c6c8ad1

File tree

5 files changed

+150
-16
lines changed

5 files changed

+150
-16
lines changed

‎packages/orm/src/client/crud/dialects/base-dialect.ts‎

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -260,14 +260,41 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
260260
key: (typeof LOGICAL_COMBINATORS)[number],
261261
payload: any,
262262
): Expression<SqlBool> {
263+
// Normalize payload: ensure array, remove empty objects and objects with
264+
// only undefined fields values
265+
const normalizedPayload = enumerate(payload).filter((el) => {
266+
if (typeof el === 'object' && el !== null && !Array.isArray(el)) {
267+
const entries = Object.entries(el);
268+
return entries.some(([, v]) => v !== undefined);
269+
} else {
270+
return true;
271+
}
272+
});
273+
274+
const normalizedFilters = normalizedPayload.map((el) => this.buildFilter(model, modelAlias, el));
275+
263276
return match(key)
264-
.with('AND', () =>
265-
this.and(...enumerate(payload).map((subPayload) => this.buildFilter(model, modelAlias, subPayload))),
266-
)
267-
.with('OR', () =>
268-
this.or(...enumerate(payload).map((subPayload) => this.buildFilter(model, modelAlias, subPayload))),
269-
)
270-
.with('NOT', () => this.eb.not(this.buildCompositeFilter(model, modelAlias, 'AND', payload)))
277+
.with('AND', () => {
278+
if (normalizedFilters.length === 0) {
279+
// AND of no conditions is a no-op, return true
280+
return this.true();
281+
}
282+
return this.and(...normalizedFilters);
283+
})
284+
.with('OR', () => {
285+
if (normalizedFilters.length === 0) {
286+
// OR of no conditions is always false, return false
287+
return this.false();
288+
}
289+
return this.or(...normalizedFilters);
290+
})
291+
.with('NOT', () => {
292+
if (normalizedFilters.length === 0) {
293+
// NOT of no conditions is a no-op, return true
294+
return this.true();
295+
}
296+
return this.not(...normalizedFilters);
297+
})
271298
.exhaustive();
272299
}
273300

@@ -800,6 +827,9 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
800827
if (excludeKeys.includes(op)) {
801828
continue;
802829
}
830+
if (value === undefined) {
831+
continue;
832+
}
803833
const rhs = Array.isArray(value) ? value.map(getRhs) : getRhs(value);
804834
const condition = match(op)
805835
.with('equals', () => (rhs === null ? this.eb(lhs, 'is', null) : this.eb(lhs, '=', rhs)))
@@ -881,6 +911,10 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
881911
continue;
882912
}
883913

914+
if (value === undefined) {
915+
continue;
916+
}
917+
884918
invariant(typeof value === 'string', `${key} value must be a string`);
885919

886920
const escapedValue = this.escapeLikePattern(value);

‎packages/orm/src/client/crud/operations/base.ts‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import * as uuid from 'uuid';
1818
import type { BuiltinType, Expression, FieldDef } from '../../../schema';
1919
import { ExpressionUtils, type GetModels, type ModelDef, type SchemaDef } from '../../../schema';
2020
import type { AnyKysely } from '../../../utils/kysely-utils';
21-
import { extractFields, fieldsToSelectObject } from '../../../utils/object-utils';
21+
import { extractFields, fieldsToSelectObject, isEmptyObject } from '../../../utils/object-utils';
2222
import { NUMERIC_FIELD_TYPES } from '../../constants';
2323
import { TransactionIsolationLevel, type ClientContract, type CRUD } from '../../contract';
2424
import type { FindArgs, SelectIncludeOmit, WhereInput } from '../../crud-types';
@@ -1152,7 +1152,7 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
11521152

11531153
const parentWhere = await this.buildUpdateParentRelationFilter(kysely, fromRelation);
11541154

1155-
let combinedWhere: Record<string, unknown> = where ?? {};
1155+
let combinedWhere: Record<string, unknown> = where ?? true;
11561156
if (Object.keys(parentWhere).length > 0) {
11571157
combinedWhere = Object.keys(combinedWhere).length > 0 ? { AND: [parentWhere, combinedWhere] } : parentWhere;
11581158
}
@@ -1574,7 +1574,7 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
15741574
}
15751575

15761576
const parentWhere = await this.buildUpdateParentRelationFilter(kysely, fromRelation);
1577-
let combinedWhere: WhereInput<Schema, GetModels<Schema>, any, false> = where ?? {};
1577+
let combinedWhere: WhereInput<Schema, GetModels<Schema>, any, false> = where ?? true;
15781578
if (Object.keys(parentWhere).length > 0) {
15791579
combinedWhere = Object.keys(combinedWhere).length > 0 ? { AND: [parentWhere, combinedWhere] } : parentWhere;
15801580
}
@@ -1890,12 +1890,12 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
18901890
}
18911891

18921892
case 'delete': {
1893-
await this.deleteRelation(kysely, fieldModel, value, fromRelationContext, true);
1893+
await this.deleteRelation(kysely, fieldModel, value, fromRelationContext, true, true);
18941894
break;
18951895
}
18961896

18971897
case 'deleteMany': {
1898-
await this.deleteRelation(kysely, fieldModel, value, fromRelationContext, false);
1898+
await this.deleteRelation(kysely, fieldModel, value, fromRelationContext, false, false);
18991899
break;
19001900
}
19011901

@@ -2031,6 +2031,9 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
20312031
} else {
20322032
disconnectConditions = [true];
20332033
}
2034+
} else if (isEmptyObject(data)) {
2035+
// empty object means true
2036+
disconnectConditions = [true];
20342037
} else {
20352038
disconnectConditions = this.normalizeRelationManipulationInput(model, data);
20362039

@@ -2235,23 +2238,35 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
22352238
model: GetModels<Schema>,
22362239
data: any,
22372240
fromRelation: FromRelationContext,
2241+
uniqueDelete: boolean,
22382242
throwForNotFound: boolean,
22392243
) {
22402244
let deleteConditions: any[] = [];
2241-
let expectedDeleteCount: number;
2245+
let expectedDeleteCount = -1; // -1 means no expected delete count check
22422246
if (typeof data === 'boolean') {
22432247
if (data === false) {
22442248
return;
22452249
} else {
22462250
deleteConditions = [true];
2251+
if (uniqueDelete) {
2252+
expectedDeleteCount = 1;
2253+
}
2254+
}
2255+
} else if (isEmptyObject(data)) {
2256+
// empty object means true
2257+
deleteConditions = [true];
2258+
if (uniqueDelete) {
22472259
expectedDeleteCount = 1;
22482260
}
22492261
} else {
22502262
deleteConditions = this.normalizeRelationManipulationInput(model, data);
22512263
if (deleteConditions.length === 0) {
22522264
return;
22532265
}
2254-
expectedDeleteCount = deleteConditions.length;
2266+
if (uniqueDelete) {
2267+
// each delete condition is one unique match
2268+
expectedDeleteCount = deleteConditions.length;
2269+
}
22552270
}
22562271

22572272
let deleteResult: Awaited<ReturnType<typeof this.delete>>;
@@ -2319,7 +2334,7 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
23192334
}
23202335

23212336
// validate result
2322-
if (throwForNotFound && expectedDeleteCount > (deleteResult.numAffectedRows ?? 0)) {
2337+
if (throwForNotFound && expectedDeleteCount >= 0 && expectedDeleteCount > (deleteResult.numAffectedRows ?? 0)) {
23232338
// some entities were not deleted
23242339
throw createNotFoundError(deleteFromModel);
23252340
}

‎packages/orm/src/utils/object-utils.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { isPlainObject } from '@zenstackhq/common-helpers';
2+
13
/**
24
* Extract fields from an object.
35
*/
@@ -11,3 +13,10 @@ export function extractFields(obj: any, fields: readonly string[]) {
1113
export function fieldsToSelectObject(fields: readonly string[]): Record<string, boolean> {
1214
return Object.fromEntries(fields.map((f) => [f, true]));
1315
}
16+
17+
/**
18+
* Checks if the given value is an empty plain object.
19+
*/
20+
export function isEmptyObject(x: unknown) {
21+
return isPlainObject(x) && Object.keys(x as object).length === 0;
22+
}

‎tests/e2e/orm/client-api/filter.test.ts‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,5 +801,81 @@ describe('Client filter tests ', () => {
801801
await expect(client.user.findMany({ where: { id: undefined } })).toResolveWithLength(1);
802802
});
803803

804+
it('ignores undefined branch inside OR filters', async () => {
805+
await createUser('u1@test.com', {
806+
name: 'First',
807+
role: 'ADMIN',
808+
profile: { create: { id: 'p1', bio: 'bio1' } },
809+
});
810+
const user2 = await createUser('u2@test.com', {
811+
name: 'Second',
812+
role: 'USER',
813+
profile: { create: { id: 'p2', bio: 'bio2' } },
814+
});
815+
816+
const baseline = await client.user.findFirst({
817+
where: {
818+
OR: [{ id: user2.id }],
819+
} as any,
820+
orderBy: { createdAt: 'asc' },
821+
});
822+
823+
const withUndefinedBranch = await client.user.findFirst({
824+
where: {
825+
OR: [{ id: undefined }, { id: user2.id }],
826+
} as any,
827+
orderBy: { createdAt: 'asc' },
828+
});
829+
830+
const onlyUndefinedBranch = await client.user.findFirst({
831+
where: {
832+
OR: [{ id: undefined }],
833+
} as any,
834+
orderBy: { createdAt: 'asc' },
835+
});
836+
837+
expect(baseline?.email).toBe(user2.email);
838+
expect(withUndefinedBranch?.email).toBe(baseline?.email);
839+
expect(onlyUndefinedBranch).toBeNull();
840+
});
841+
842+
it('strips undefined filter operators inside OR branches', async () => {
843+
await createUser('alice@test.com', { name: 'Alice', role: 'ADMIN' });
844+
await createUser('bob@test.com', { name: 'Bob', role: 'USER' });
845+
846+
const result = await client.user.findMany({
847+
where: {
848+
OR: [{ name: { startsWith: 'A', contains: undefined } }],
849+
} as any,
850+
});
851+
852+
expect(result).toHaveLength(1);
853+
expect(result[0]!.name).toBe('Alice');
854+
});
855+
856+
describe('AND/OR/NOT with no-op filters', () => {
857+
beforeEach(async () => {
858+
await createUser('u1@test.com', { name: 'Alice', role: 'ADMIN' });
859+
await createUser('u2@test.com', { name: 'Bob', role: 'USER' });
860+
});
861+
862+
it('AND is no-op for empty array, array with all-undefined object, and plain all-undefined object', async () => {
863+
await expect(client.user.findMany({ where: { AND: [] } })).toResolveWithLength(2);
864+
await expect(client.user.findMany({ where: { AND: [{ id: undefined }] } })).toResolveWithLength(2);
865+
await expect(client.user.findMany({ where: { AND: { id: undefined } } as any })).toResolveWithLength(2);
866+
});
867+
868+
it('OR returns no records for empty array, array with all-undefined object, and plain all-undefined object', async () => {
869+
await expect(client.user.findMany({ where: { OR: [] } })).toResolveWithLength(0);
870+
await expect(client.user.findMany({ where: { OR: [{ id: undefined }] } })).toResolveWithLength(0);
871+
});
872+
873+
it('NOT is no-op for empty array, array with all-undefined object, and plain all-undefined object', async () => {
874+
await expect(client.user.findMany({ where: { NOT: [] } })).toResolveWithLength(2);
875+
await expect(client.user.findMany({ where: { NOT: [{ id: undefined }] } })).toResolveWithLength(2);
876+
await expect(client.user.findMany({ where: { NOT: { id: undefined } } })).toResolveWithLength(2);
877+
});
878+
});
879+
804880
// TODO: filter for bigint, decimal, bytes
805881
});

‎tests/e2e/orm/client-api/find.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ describe('Client find tests ', () => {
479479
).toResolveFalsy();
480480

481481
// NOT
482-
await expect(client.user.findMany({ where: { NOT: [] } })).resolves.toHaveLength(0);
482+
await expect(client.user.findMany({ where: { NOT: [] } })).resolves.toHaveLength(2);
483483
await expect(
484484
client.user.findFirst({
485485
where: {

0 commit comments

Comments
 (0)