From 0b58cadbd0b1335c70b7ce954934f80dd78eaa63 Mon Sep 17 00:00:00 2001 From: dswbx Date: Sun, 26 Oct 2025 21:05:11 +0100 Subject: [PATCH] feat: implement mergeFilters function and enhance query object merging - Added mergeFilters function to combine filter objects with priority handling. - Introduced comprehensive tests for mergeFilters in permissions.spec.ts. - Created query.spec.ts to validate query structure and expression handling. - Enhanced error messages in query.ts for better debugging and clarity. --- .../auth/authorize/permissions.spec.ts | 43 ++++++- app/src/auth/authorize/Guard.ts | 22 +++- app/src/core/object/query/query.spec.ts | 111 ++++++++++++++++++ app/src/core/object/query/query.ts | 27 ++++- 4 files changed, 196 insertions(+), 7 deletions(-) create mode 100644 app/src/core/object/query/query.spec.ts diff --git a/app/__test__/auth/authorize/permissions.spec.ts b/app/__test__/auth/authorize/permissions.spec.ts index 14a01ee..411ad20 100644 --- a/app/__test__/auth/authorize/permissions.spec.ts +++ b/app/__test__/auth/authorize/permissions.spec.ts @@ -5,9 +5,10 @@ import { Policy } from "auth/authorize/Policy"; import { Hono } from "hono"; import { getPermissionRoutes, permission } from "auth/middlewares/permission.middleware"; import { auth } from "auth/middlewares/auth.middleware"; -import { Guard, type GuardConfig } from "auth/authorize/Guard"; +import { Guard, mergeFilters, type GuardConfig } from "auth/authorize/Guard"; import { Role, RolePermission } from "auth/authorize/Role"; import { Exception } from "bknd"; +import { convert } from "core/object/query/object-query"; describe("Permission", () => { it("works with minimal schema", () => { @@ -177,6 +178,46 @@ describe("Guard", () => { // hence it can be found expect(guard.filters(p, {}, { a: 1 }).filter).toEqual({ foo: "bar" }); }); + + it("merges filters correctly", () => { + expect(mergeFilters({ foo: "bar" }, { baz: "qux" })).toEqual({ + foo: { $eq: "bar" }, + baz: { $eq: "qux" }, + }); + expect(mergeFilters({ foo: "bar" }, { baz: { $eq: "qux" } })).toEqual({ + foo: { $eq: "bar" }, + baz: { $eq: "qux" }, + }); + expect(mergeFilters({ foo: "bar" }, { foo: "baz" })).toEqual({ foo: { $eq: "baz" } }); + + expect(mergeFilters({ foo: "bar" }, { foo: { $lt: 1 } })).toEqual({ + foo: { $eq: "bar", $lt: 1 }, + }); + + // overwrite base $or with priority + expect(mergeFilters({ $or: { foo: "one" } }, { foo: "bar" })).toEqual({ + $or: { + foo: { + $eq: "bar", + }, + }, + foo: { + $eq: "bar", + }, + }); + + // ignore base $or if priority has different key + expect(mergeFilters({ $or: { other: "one" } }, { foo: "bar" })).toEqual({ + $or: { + other: { + $eq: "one", + }, + }, + foo: { + $eq: "bar", + }, + }); + }); }); describe("permission middleware", () => { diff --git a/app/src/auth/authorize/Guard.ts b/app/src/auth/authorize/Guard.ts index 6336a59..a8f91e3 100644 --- a/app/src/auth/authorize/Guard.ts +++ b/app/src/auth/authorize/Guard.ts @@ -6,6 +6,7 @@ import type { ServerEnv } from "modules/Controller"; import type { Role } from "./Role"; import { HttpStatus } from "bknd/utils"; import type { Policy, PolicySchema } from "./Policy"; +import { convert, type ObjectQuery } from "core/object/query/object-query"; export type GuardUserContext = { role?: string | null; @@ -294,7 +295,7 @@ export class Guard { filter, policies, merge: (givenFilter: object | undefined) => { - return mergeObject(givenFilter ?? {}, filter ?? {}); + return mergeFilters(givenFilter ?? {}, filter ?? {}); }, matches: (subject: object | object[], opts?: { throwOnError?: boolean }) => { const subjects = Array.isArray(subject) ? subject : [subject]; @@ -319,3 +320,22 @@ export class Guard { }; } } + +export function mergeFilters(base: ObjectQuery, priority: ObjectQuery) { + const base_converted = convert(base); + const priority_converted = convert(priority); + const merged = mergeObject(base_converted, priority_converted); + + // in case priority filter is also contained in base's $and, merge priority in + if ("$or" in base_converted && base_converted.$or) { + const $ors = base_converted.$or as ObjectQuery; + const priority_keys = Object.keys(priority_converted); + for (const key of priority_keys) { + if (key in $ors) { + merged.$or[key] = mergeObject($ors[key], priority_converted[key]); + } + } + } + + return merged; +} diff --git a/app/src/core/object/query/query.spec.ts b/app/src/core/object/query/query.spec.ts new file mode 100644 index 0000000..27e247c --- /dev/null +++ b/app/src/core/object/query/query.spec.ts @@ -0,0 +1,111 @@ +import { describe, expect, test } from "bun:test"; +import { makeValidator, exp, Expression, isPrimitive, type Primitive } from "./query"; + +describe("query", () => { + test("isPrimitive", () => { + expect(isPrimitive(1)).toBe(true); + expect(isPrimitive("1")).toBe(true); + expect(isPrimitive(true)).toBe(true); + expect(isPrimitive(false)).toBe(true); + + // not primitives + expect(isPrimitive(null)).toBe(false); + expect(isPrimitive(undefined)).toBe(false); + expect(isPrimitive([])).toBe(false); + expect(isPrimitive({})).toBe(false); + expect(isPrimitive(Symbol("test"))).toBe(false); + expect(isPrimitive(new Date())).toBe(false); + expect(isPrimitive(new Error())).toBe(false); + expect(isPrimitive(new Set())).toBe(false); + expect(isPrimitive(new Map())).toBe(false); + }); + + test("strict expression creation", () => { + // @ts-expect-error + expect(() => exp()).toThrow(); + // @ts-expect-error + expect(() => exp("")).toThrow(); + // @ts-expect-error + expect(() => exp("invalid")).toThrow(); + // @ts-expect-error + expect(() => exp("$eq")).toThrow(); + // @ts-expect-error + expect(() => exp("$eq", 1)).toThrow(); + // @ts-expect-error + expect(() => exp("$eq", () => null)).toThrow(); + // @ts-expect-error + expect(() => exp("$eq", () => null, 1)).toThrow(); + expect( + exp( + "$eq", + () => true, + () => null, + ), + ).toBeInstanceOf(Expression); + }); + + test("$eq is required", () => { + expect(() => makeValidator([])).toThrow(); + expect(() => + makeValidator([ + exp( + "$valid", + () => true, + () => null, + ), + ]), + ).toThrow(); + expect( + makeValidator([ + exp( + "$eq", + () => true, + () => null, + ), + ]), + ).toBeDefined(); + }); + + test("validates filter structure", () => { + const validator = makeValidator([ + exp( + "$eq", + (v: Primitive) => isPrimitive(v), + (e, a) => e === a, + ), + exp( + "$like", + (v: string) => typeof v === "string", + (e, a) => e === a, + ), + ]); + + // @ts-expect-error intentionally typed as union of given expression keys + expect(validator.expressionKeys).toEqual(["$eq", "$like"]); + + // @ts-expect-error "$and" is not allowed + expect(() => validator.convert({ $and: {} })).toThrow(); + + // @ts-expect-error "$or" must be an object + expect(() => validator.convert({ $or: [] })).toThrow(); + + // @ts-expect-error "invalid" is not a valid expression key + expect(() => validator.convert({ foo: { invalid: "bar" } })).toThrow(); + + // @ts-expect-error "invalid" is not a valid expression key + expect(() => validator.convert({ foo: { $invalid: "bar" } })).toThrow(); + + // @ts-expect-error "null" is not a valid value + expect(() => validator.convert({ foo: null })).toThrow(); + + // @ts-expect-error only primitives are allowed for $eq + expect(() => validator.convert({ foo: { $eq: [] } })).toThrow(); + + // @ts-expect-error only strings are allowed for $like + expect(() => validator.convert({ foo: { $like: 1 } })).toThrow(); + + expect(validator.convert({ foo: "bar" })).toEqual({ foo: { $eq: "bar" } }); + expect(validator.convert({ foo: { $eq: "bar" } })).toEqual({ foo: { $eq: "bar" } }); + expect(validator.convert({ foo: { $like: "bar" } })).toEqual({ foo: { $like: "bar" } }); + }); +}); diff --git a/app/src/core/object/query/query.ts b/app/src/core/object/query/query.ts index 24352c4..58a50cc 100644 --- a/app/src/core/object/query/query.ts +++ b/app/src/core/object/query/query.ts @@ -1,5 +1,5 @@ import type { PrimaryFieldType } from "core/config"; -import { getPath, invariant } from "bknd/utils"; +import { getPath, invariant, isPlainObject } from "bknd/utils"; export type Primitive = PrimaryFieldType | string | number | boolean; export function isPrimitive(value: any): value is Primitive { @@ -26,6 +26,10 @@ export function exp( valid: (v: Expect) => boolean, validate: (e: Expect, a: unknown, ctx: CTX) => any, ): Expression { + invariant(typeof key === "string", "key must be a string"); + invariant(key[0] === "$", "key must start with '$'"); + invariant(typeof valid === "function", "valid must be a function"); + invariant(typeof validate === "function", "validate must be a function"); return new Expression(key, valid, validate); } @@ -85,13 +89,16 @@ function _convert( function validate(key: string, value: any, path: string[] = []) { const exp = getExpression(expressions, key as any); if (exp.valid(value) === false) { - throw new Error(`Invalid value at "${[...path, key].join(".")}": ${value}`); + throw new Error( + `Given value at "${[...path, key].join(".")}" is invalid, got "${JSON.stringify(value)}"`, + ); } } for (const [key, value] of Object.entries($query)) { // if $or, convert each value if (key === "$or") { + invariant(isPlainObject(value), "$or must be an object"); newQuery.$or = _convert(value, expressions, [...path, key]); // if primitive, assume $eq @@ -100,7 +107,7 @@ function _convert( newQuery[key] = { $eq: value }; // if object, check for expressions - } else if (typeof value === "object") { + } else if (isPlainObject(value)) { // when object is given, check if all keys are expressions const invalid = Object.keys(value).filter( (f) => !ExpressionConditionKeys.includes(f as any), @@ -114,9 +121,13 @@ function _convert( } } else { throw new Error( - `Invalid key(s) at "${key}": ${invalid.join(", ")}. Expected expressions.`, + `Invalid key(s) at "${key}": ${invalid.join(", ")}. Expected expression key: ${ExpressionConditionKeys.join(", ")}.`, ); } + } else { + throw new Error( + `Invalid value at "${[...path, key].join(".")}", got "${JSON.stringify(value)}"`, + ); } } @@ -151,7 +162,9 @@ function _build( throw new Error(`Expression does not exist: "${$op}"`); } if (!exp.valid(expected)) { - throw new Error(`Invalid expected value at "${[...path, $op].join(".")}": ${expected}`); + throw new Error( + `Invalid value at "${[...path, $op].join(".")}", got "${JSON.stringify(expected)}"`, + ); } return exp.validate(expected, actual, options.exp_ctx); } @@ -191,6 +204,10 @@ function _validate(results: ValidationResults): boolean { } export function makeValidator(expressions: Exps) { + if (!expressions.some((e) => e.key === "$eq")) { + throw new Error("'$eq' expression is required"); + } + return { convert: (query: FilterQuery) => _convert(query, expressions), build: (query: FilterQuery, options: BuildOptions) =>