From 46a16cfcab757d6abd58d3df0aba44dc73170b06 Mon Sep 17 00:00:00 2001 From: fratzinger <22286818+fratzinger@users.noreply.github.com> Date: Sun, 28 Jun 2026 09:06:59 +0200 Subject: [PATCH 1/2] fix: mergeQuery flattenAndBranches --- src/utils/merge-query/flatten-and-branches.ts | 56 +++++++++++++++++++ src/utils/merge-query/merge-query-bodies.ts | 20 ++++++- .../merge-query/merge-query.util.test.ts | 13 +++++ 3 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 src/utils/merge-query/flatten-and-branches.ts diff --git a/src/utils/merge-query/flatten-and-branches.ts b/src/utils/merge-query/flatten-and-branches.ts new file mode 100644 index 0000000..634dc9f --- /dev/null +++ b/src/utils/merge-query/flatten-and-branches.ts @@ -0,0 +1,56 @@ +type QueryRecord = Record + +/** + * Recursively hoists nested `$and` conjuncts so that an `$and` never contains + * another `$and` branch. `$and` is associative, so a branch like + * `{ $or: [...], $and: [n] }` is split into the two conjuncts `{ $or: [...] }` + * and `n`. A pure `{ $and: [...] }` branch contributes only its inner branches. + * Internal helper for {@link mergeQuery} (intersect). + */ +export function flattenAndBranches(branches: QueryRecord[]): QueryRecord[] { + const result: QueryRecord[] = [] + for (const branch of branches) { + if (Array.isArray(branch.$and)) { + const { $and, ...rest } = branch + if (Object.keys(rest).length > 0) { + result.push(rest) + } + result.push(...flattenAndBranches($and)) + } else { + result.push(branch) + } + } + return result +} + +if (import.meta.vitest) { + const { describe, it, expect } = import.meta.vitest + + describe('flattenAndBranches', () => { + it('leaves plain branches untouched', () => { + expect(flattenAndBranches([{ id: 1 }, { id: 2 }])).toEqual([ + { id: 1 }, + { id: 2 }, + ]) + }) + + it('hoists a nested $and and keeps the remaining keys as a branch', () => { + expect( + flattenAndBranches([{ $or: ['c'], $and: [{ $nor: ['n'] }] }]), + ).toEqual([{ $or: ['c'] }, { $nor: ['n'] }]) + }) + + it('drops the remainder when a branch is a pure $and', () => { + expect(flattenAndBranches([{ $and: [{ id: 1 }, { id: 2 }] }])).toEqual([ + { id: 1 }, + { id: 2 }, + ]) + }) + + it('flattens recursively', () => { + expect( + flattenAndBranches([{ $and: [{ $and: [{ id: 1 }] }, { id: 2 }] }]), + ).toEqual([{ id: 1 }, { id: 2 }]) + }) + }) +} diff --git a/src/utils/merge-query/merge-query-bodies.ts b/src/utils/merge-query/merge-query-bodies.ts index 016a5ec..f0f4a4c 100644 --- a/src/utils/merge-query/merge-query-bodies.ts +++ b/src/utils/merge-query/merge-query-bodies.ts @@ -2,6 +2,7 @@ import type { MergeQueryMode } from './merge-query.util.js' import { isEmptyObject } from '../../common/is-empty-object.js' import { logicalBranches } from './logical-branches.js' import { dedupeBranches } from './dedupe-branches.js' +import { flattenAndBranches } from './flatten-and-branches.js' import { hasConflict } from './has-conflict.js' type QueryRecord = Record @@ -54,10 +55,15 @@ export function mergeQueryBodies( return { ...target, ...source } } - const branches = dedupeBranches([ + const collected = [ ...(targetBranches ?? [target]), ...(sourceBranches ?? [source]), - ]) + ] + + // under `$and`, hoist any nested `$and` so the result never nests `$and` in `$and` + const branches = dedupeBranches( + op === '$and' ? flattenAndBranches(collected) : collected, + ) if (branches.length === 0) { return {} @@ -132,5 +138,15 @@ if (import.meta.vitest) { ), ).toEqual({ $and: [{ id: 1 }, { id: 2 }, { id: 3 }] }) }) + + it('intersect hoists a nested $and instead of nesting it', () => { + expect( + mergeQueryBodies( + { $or: ['u'] }, + { $or: ['c'], $and: [{ $nor: ['n'] }] }, + 'intersect', + ), + ).toEqual({ $and: [{ $or: ['u'] }, { $or: ['c'] }, { $nor: ['n'] }] }) + }) }) } diff --git a/src/utils/merge-query/merge-query.util.test.ts b/src/utils/merge-query/merge-query.util.test.ts index fbb3129..ec2aada 100644 --- a/src/utils/merge-query/merge-query.util.test.ts +++ b/src/utils/merge-query/merge-query.util.test.ts @@ -229,6 +229,19 @@ describe('mergeQuery', () => { options: { mode: 'intersect' }, expected: { $and: [{ price: { $gt: 5 } }, { price: { $gt: 8 } }] }, }, + // e.g. a user $or merged with a CASL rule that carries its own $and ($nor) + 'hoists the source $and instead of nesting it': { + target: { $or: [{ a: 1 }] }, + source: { $or: [{ b: 2 }], $and: [{ $nor: [{ c: 3 }] }] }, + options: { mode: 'intersect' }, + expected: { + $and: [ + { $or: [{ a: 1 }] }, + { $or: [{ b: 2 }] }, + { $nor: [{ c: 3 }] }, + ], + }, + }, }) }) From b54e954ae2d3cc611776c32dbe025b4d4505dcfe Mon Sep 17 00:00:00 2001 From: fratzinger <22286818+fratzinger@users.noreply.github.com> Date: Mon, 29 Jun 2026 04:50:58 +0200 Subject: [PATCH 2/2] feat: new util simplifyQuery --- .../merge-query => common}/dedupe-branches.ts | 6 +- .../flatten-and-branches.ts | 2 +- src/common/flatten-or-branches.ts | 52 +++++ src/common/index.ts | 3 + src/utils/index.ts | 1 + src/utils/merge-query/merge-query-bodies.ts | 4 +- src/utils/merge-query/merge-query.util.md | 1 + .../merge-query/merge-query.util.test.ts | 69 ++++++- src/utils/merge-query/merge-query.util.ts | 7 +- .../replace-data/replace-data.util.test.ts | 50 ++--- src/utils/replace-data/replace-data.util.ts | 8 +- .../replace-result.util.test.ts | 74 +++---- .../replace-result/replace-result.util.ts | 36 ++-- .../simplify-query/merge-and-branches-up.ts | 86 ++++++++ .../simplify-query/merge-or-branch-up.ts | 74 +++++++ .../simplify-query/simplify-query.util.md | 8 + .../simplify-query.util.test.ts | 191 ++++++++++++++++++ .../simplify-query/simplify-query.util.ts | 98 +++++++++ test/index.test.ts | 1 + 19 files changed, 670 insertions(+), 101 deletions(-) rename src/{utils/merge-query => common}/dedupe-branches.ts (82%) rename src/{utils/merge-query => common}/flatten-and-branches.ts (96%) create mode 100644 src/common/flatten-or-branches.ts create mode 100644 src/utils/simplify-query/merge-and-branches-up.ts create mode 100644 src/utils/simplify-query/merge-or-branch-up.ts create mode 100644 src/utils/simplify-query/simplify-query.util.md create mode 100644 src/utils/simplify-query/simplify-query.util.test.ts create mode 100644 src/utils/simplify-query/simplify-query.util.ts diff --git a/src/utils/merge-query/dedupe-branches.ts b/src/common/dedupe-branches.ts similarity index 82% rename from src/utils/merge-query/dedupe-branches.ts rename to src/common/dedupe-branches.ts index c2f2933..d8eeaf1 100644 --- a/src/utils/merge-query/dedupe-branches.ts +++ b/src/common/dedupe-branches.ts @@ -1,11 +1,11 @@ import { dequal as deepEqual } from 'dequal' -import { isEmptyObject } from '../../common/is-empty-object.js' +import { isEmptyObject } from './is-empty-object.js' type QueryRecord = Record /** - * Removes empty (`{}`) and deep-equal duplicate branches, preserving order. - * Internal helper for {@link mergeQuery}. + * Removes empty (`{}`) and deep-equal duplicate branches from a logical + * (`$and`/`$or`) branch array, preserving order. Internal query-AST helper. */ export function dedupeBranches(branches: QueryRecord[]): QueryRecord[] { const result: QueryRecord[] = [] diff --git a/src/utils/merge-query/flatten-and-branches.ts b/src/common/flatten-and-branches.ts similarity index 96% rename from src/utils/merge-query/flatten-and-branches.ts rename to src/common/flatten-and-branches.ts index 634dc9f..4fe7d1b 100644 --- a/src/utils/merge-query/flatten-and-branches.ts +++ b/src/common/flatten-and-branches.ts @@ -5,7 +5,7 @@ type QueryRecord = Record * another `$and` branch. `$and` is associative, so a branch like * `{ $or: [...], $and: [n] }` is split into the two conjuncts `{ $or: [...] }` * and `n`. A pure `{ $and: [...] }` branch contributes only its inner branches. - * Internal helper for {@link mergeQuery} (intersect). + * Internal query-AST helper. */ export function flattenAndBranches(branches: QueryRecord[]): QueryRecord[] { const result: QueryRecord[] = [] diff --git a/src/common/flatten-or-branches.ts b/src/common/flatten-or-branches.ts new file mode 100644 index 0000000..7339aee --- /dev/null +++ b/src/common/flatten-or-branches.ts @@ -0,0 +1,52 @@ +type QueryRecord = Record + +/** + * Recursively hoists nested *pure* `$or` branches so that an `$or` never contains + * another `{ $or: [...] }`-only branch. `$or` is associative, so a branch that is + * exactly `{ $or: [a, b] }` is replaced by `a` and `b`. A branch that mixes `$or` + * with other keys is left intact (its other keys are AND-ed and cannot be hoisted). + * Internal query-AST helper. + */ +export function flattenOrBranches(branches: QueryRecord[]): QueryRecord[] { + const result: QueryRecord[] = [] + for (const branch of branches) { + const keys = Object.keys(branch) + if (keys.length === 1 && keys[0] === '$or' && Array.isArray(branch.$or)) { + result.push(...flattenOrBranches(branch.$or)) + } else { + result.push(branch) + } + } + return result +} + +if (import.meta.vitest) { + const { describe, it, expect } = import.meta.vitest + + describe('flattenOrBranches', () => { + it('leaves plain branches untouched', () => { + expect(flattenOrBranches([{ a: 1 }, { b: 2 }])).toEqual([ + { a: 1 }, + { b: 2 }, + ]) + }) + + it('hoists a pure $or branch', () => { + expect( + flattenOrBranches([{ a: 1 }, { $or: [{ b: 2 }, { c: 3 }] }]), + ).toEqual([{ a: 1 }, { b: 2 }, { c: 3 }]) + }) + + it('does not hoist a $or branch mixed with other keys', () => { + expect(flattenOrBranches([{ $or: [{ b: 2 }], x: 1 }])).toEqual([ + { $or: [{ b: 2 }], x: 1 }, + ]) + }) + + it('flattens recursively', () => { + expect( + flattenOrBranches([{ $or: [{ $or: [{ a: 1 }] }, { b: 2 }] }]), + ).toEqual([{ a: 1 }, { b: 2 }]) + }) + }) +} diff --git a/src/common/index.ts b/src/common/index.ts index 2d981d7..4b439ee 100644 --- a/src/common/index.ts +++ b/src/common/index.ts @@ -16,3 +16,6 @@ export function isPromise(p: any): p is Promise { export { traverse } from './traverse.js' export { clone } from './clone.js' export { isEmptyObject } from './is-empty-object.js' +export { dedupeBranches } from './dedupe-branches.js' +export { flattenAndBranches } from './flatten-and-branches.js' +export { flattenOrBranches } from './flatten-or-branches.js' diff --git a/src/utils/index.ts b/src/utils/index.ts index f42521a..738977f 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -17,6 +17,7 @@ export * from './query-defaults/query-defaults.util.js' export * from './query-has-property/query-has-property.util.js' export * from './replace-data/replace-data.util.js' export * from './replace-result/replace-result.util.js' +export * from './simplify-query/simplify-query.util.js' export * from './skip-result/skip-result.util.js' export * from './sort-query-properties/sort-query-properties.util.js' export * from './to-paginated/to-paginated.util.js' diff --git a/src/utils/merge-query/merge-query-bodies.ts b/src/utils/merge-query/merge-query-bodies.ts index f0f4a4c..5b59e3e 100644 --- a/src/utils/merge-query/merge-query-bodies.ts +++ b/src/utils/merge-query/merge-query-bodies.ts @@ -1,8 +1,8 @@ import type { MergeQueryMode } from './merge-query.util.js' import { isEmptyObject } from '../../common/is-empty-object.js' +import { dedupeBranches } from '../../common/dedupe-branches.js' +import { flattenAndBranches } from '../../common/flatten-and-branches.js' import { logicalBranches } from './logical-branches.js' -import { dedupeBranches } from './dedupe-branches.js' -import { flattenAndBranches } from './flatten-and-branches.js' import { hasConflict } from './has-conflict.js' type QueryRecord = Record diff --git a/src/utils/merge-query/merge-query.util.md b/src/utils/merge-query/merge-query.util.md index 5023ed5..7ee4ad0 100644 --- a/src/utils/merge-query/merge-query.util.md +++ b/src/utils/merge-query/merge-query.util.md @@ -5,4 +5,5 @@ see: - utils/add-to-query - utils/query-defaults - utils/walk-query + - utils/simplify-query --- diff --git a/src/utils/merge-query/merge-query.util.test.ts b/src/utils/merge-query/merge-query.util.test.ts index ec2aada..656652a 100644 --- a/src/utils/merge-query/merge-query.util.test.ts +++ b/src/utils/merge-query/merge-query.util.test.ts @@ -132,10 +132,13 @@ describe('mergeQuery', () => { }, '$and bodies become branches of an $or': { target: { $and: [{ id: 1 }, { id: 2 }] }, - source: { $and: [{ id: 3 }] }, + source: { $and: [{ id: 3 }, { id: 4 }] }, options: { mode: 'combine' }, expected: { - $or: [{ $and: [{ id: 1 }, { id: 2 }] }, { $and: [{ id: 3 }] }], + $or: [ + { $and: [{ id: 1 }, { id: 2 }] }, + { $and: [{ id: 3 }, { id: 4 }] }, + ], }, }, 'empty $or branches collapse to {}': { @@ -211,10 +214,13 @@ describe('mergeQuery', () => { }, '$or bodies become branches of an $and': { target: { $or: [{ id: 1 }, { id: 2 }] }, - source: { $or: [{ id: 3 }] }, + source: { $or: [{ id: 3 }, { id: 4 }] }, options: { mode: 'intersect' }, expected: { - $and: [{ $or: [{ id: 1 }, { id: 2 }] }, { $or: [{ id: 3 }] }], + $and: [ + { $or: [{ id: 1 }, { id: 2 }] }, + { $or: [{ id: 3 }, { id: 4 }] }, + ], }, }, 'subset (source ⊆ target) merges flat': { @@ -229,22 +235,67 @@ describe('mergeQuery', () => { options: { mode: 'intersect' }, expected: { $and: [{ price: { $gt: 5 } }, { price: { $gt: 8 } }] }, }, - // e.g. a user $or merged with a CASL rule that carries its own $and ($nor) - 'hoists the source $and instead of nesting it': { + // a user $or merged with a CASL rule that carries its own $and ($nor): + // single-branch wrappers collapse, the rest merges flat (no nesting) + 'merges a user $or with a CASL rule, flat and without nesting': { target: { $or: [{ a: 1 }] }, source: { $or: [{ b: 2 }], $and: [{ $nor: [{ c: 3 }] }] }, options: { mode: 'intersect' }, + expected: { a: 1, b: 2, $nor: [{ c: 3 }] }, + }, + // when both sides keep multiple $or branches the $nor stays a separate + // (still flat) conjunct alongside them + 'merges multi-branch $or rules without nesting the $and': { + target: { $or: [{ a: 1 }, { a: 2 }] }, + source: { $or: [{ b: 1 }, { b: 2 }], $and: [{ $nor: [{ c: 3 }] }] }, + options: { mode: 'intersect' }, expected: { $and: [ - { $or: [{ a: 1 }] }, - { $or: [{ b: 2 }] }, - { $nor: [{ c: 3 }] }, + { $or: [{ a: 1 }, { a: 2 }] }, + { $or: [{ b: 1 }, { b: 2 }], $nor: [{ c: 3 }] }, ], }, }, }) }) + // a source with a single-branch $or collapses to its child (an $or of one is + // that child) and then merges correctly — i.e. `replaceOr` is not needed here + describe('source with a single-branch $or', () => { + run({ + 'intersect, conflicting child': { + target: { id: 1 }, + source: { $or: [{ id: 2 }] }, + options: { mode: 'intersect' }, + expected: { $and: [{ id: 1 }, { id: 2 }] }, + }, + 'intersect, disjoint child': { + target: { status: 'active' }, + source: { $or: [{ role: 'admin' }] }, + options: { mode: 'intersect' }, + expected: { status: 'active', role: 'admin' }, + }, + 'intersect, child with multiple keys': { + target: { a: 1 }, + source: { $or: [{ b: 2, c: 3 }] }, + options: { mode: 'intersect' }, + expected: { a: 1, b: 2, c: 3 }, + }, + 'combine, disjoint child': { + target: { a: 1 }, + source: { $or: [{ b: 2 }] }, + options: { mode: 'combine' }, + expected: { $or: [{ a: 1 }, { b: 2 }] }, + }, + 'combine, into an existing target $or': { + target: { $or: [{ a: 1 }, { a: 2 }] }, + source: { $or: [{ b: 1 }] }, + options: { mode: 'combine' }, + expected: { $or: [{ a: 1 }, { a: 2 }, { b: 1 }] }, + }, + }) + }) + describe('filters', () => { run({ '$limit: -1 is preserved': { diff --git a/src/utils/merge-query/merge-query.util.ts b/src/utils/merge-query/merge-query.util.ts index 8cc9606..da52cf1 100644 --- a/src/utils/merge-query/merge-query.util.ts +++ b/src/utils/merge-query/merge-query.util.ts @@ -1,4 +1,5 @@ import type { Query } from '@feathersjs/feathers' +import { simplifyQuery } from '../simplify-query/simplify-query.util.js' import { extractQueryFilters } from './extract-query-filters.js' import { mergeQueryBodies } from './merge-query-bodies.js' import { mergeSelect } from './merge-select.js' @@ -64,8 +65,10 @@ export function mergeQuery( ): Query { const mode = options?.mode ?? 'combine' - const targetFilters = extractQueryFilters(target) - const sourceFilters = extractQueryFilters(source) + // normalize inputs first: drop empty/duplicate/redundant logical wrappers and + // hoist nested operators, so the merge works on clean, canonical queries + const targetFilters = extractQueryFilters(simplifyQuery(target)) + const sourceFilters = extractQueryFilters(simplifyQuery(source)) const result: Query = mergeQueryBodies( targetFilters.query, diff --git a/src/utils/replace-data/replace-data.util.test.ts b/src/utils/replace-data/replace-data.util.test.ts index 5bc7a53..1b70076 100644 --- a/src/utils/replace-data/replace-data.util.test.ts +++ b/src/utils/replace-data/replace-data.util.test.ts @@ -1,31 +1,31 @@ -import { describe, it, expect } from "vitest"; -import type { HookContext } from "@feathersjs/feathers"; -import { replaceData } from "./replace-data.util.js"; +import { describe, it, expect } from 'vitest' +import type { HookContext } from '@feathersjs/feathers' +import { replaceData } from './replace-data.util.js' const ctx = (data: any): HookContext => - ({ type: "before", method: "create", data }) as any; + ({ type: 'before', method: 'create', data }) as any -describe("replaceData", () => { - it("replaces a single data item, preserving the single shape", () => { - const context = ctx({ name: "a" }); - replaceData(context, [{ name: "b" }]); - expect(context.data).toEqual({ name: "b" }); - }); +describe('replaceData', () => { + it('replaces a single data item, preserving the single shape', () => { + const context = ctx({ name: 'a' }) + replaceData(context, [{ name: 'b' }]) + expect(context.data).toEqual({ name: 'b' }) + }) - it("replaces array data, preserving the array shape", () => { - const context = ctx([{ n: 1 }, { n: 2 }]); - replaceData(context, [{ n: 10 }]); - expect(context.data).toEqual([{ n: 10 }]); - }); + it('replaces array data, preserving the array shape', () => { + const context = ctx([{ n: 1 }, { n: 2 }]) + replaceData(context, [{ n: 10 }]) + expect(context.data).toEqual([{ n: 10 }]) + }) - it("returns the context", () => { - const context = ctx({ a: 1 }); - expect(replaceData(context, [{ a: 2 }])).toBe(context); - }); + it('returns the context', () => { + const context = ctx({ a: 1 }) + expect(replaceData(context, [{ a: 2 }])).toBe(context) + }) - it("round-trips with getDataIsArray shape", () => { - const context = ctx([{ n: 1 }, { n: 2 }]); - replaceData(context, [{ n: 1 }, { n: 2 }, { n: 3 }]); - expect(context.data).toEqual([{ n: 1 }, { n: 2 }, { n: 3 }]); - }); -}); + it('round-trips with getDataIsArray shape', () => { + const context = ctx([{ n: 1 }, { n: 2 }]) + replaceData(context, [{ n: 1 }, { n: 2 }, { n: 3 }]) + expect(context.data).toEqual([{ n: 1 }, { n: 2 }, { n: 3 }]) + }) +}) diff --git a/src/utils/replace-data/replace-data.util.ts b/src/utils/replace-data/replace-data.util.ts index 87418e9..8d12a0c 100644 --- a/src/utils/replace-data/replace-data.util.ts +++ b/src/utils/replace-data/replace-data.util.ts @@ -1,5 +1,5 @@ -import type { HookContext } from "@feathersjs/feathers"; -import type { DataSingleHookContext } from "../../utility-types/hook-context.js"; +import type { HookContext } from '@feathersjs/feathers' +import type { DataSingleHookContext } from '../../utility-types/hook-context.js' /** * Replaces `context.data` wholesale with the given items, preserving the original @@ -21,6 +21,6 @@ export function replaceData( context: H, data: DataSingleHookContext[], ): H { - context.data = Array.isArray(context.data) ? data : data[0]; - return context; + context.data = Array.isArray(context.data) ? data : data[0] + return context } diff --git a/src/utils/replace-result/replace-result.util.test.ts b/src/utils/replace-result/replace-result.util.test.ts index de17d66..a656db1 100644 --- a/src/utils/replace-result/replace-result.util.test.ts +++ b/src/utils/replace-result/replace-result.util.test.ts @@ -1,55 +1,55 @@ -import { describe, it, expect } from "vitest"; -import type { HookContext } from "@feathersjs/feathers"; -import { replaceResult } from "./replace-result.util.js"; +import { describe, it, expect } from 'vitest' +import type { HookContext } from '@feathersjs/feathers' +import { replaceResult } from './replace-result.util.js' const ctx = (method: string, result: any, dispatch?: any): HookContext => - ({ type: "after", method, result, dispatch }) as any; + ({ type: 'after', method, result, dispatch }) as any -describe("replaceResult", () => { - it("replaces a single (get) result", () => { - const context = ctx("get", { id: 1, name: "a" }); - replaceResult(context, [{ id: 1, name: "b" }]); - expect(context.result).toEqual({ id: 1, name: "b" }); - }); +describe('replaceResult', () => { + it('replaces a single (get) result', () => { + const context = ctx('get', { id: 1, name: 'a' }) + replaceResult(context, [{ id: 1, name: 'b' }]) + expect(context.result).toEqual({ id: 1, name: 'b' }) + }) - it("replaces an array (find, non-paginated) result", () => { - const context = ctx("find", [{ n: 1 }, { n: 2 }]); - replaceResult(context, [{ n: 9 }]); - expect(context.result).toEqual([{ n: 9 }]); - }); + it('replaces an array (find, non-paginated) result', () => { + const context = ctx('find', [{ n: 1 }, { n: 2 }]) + replaceResult(context, [{ n: 9 }]) + expect(context.result).toEqual([{ n: 9 }]) + }) - it("replaces a paginated result.data, preserving the envelope", () => { - const context = ctx("find", { + it('replaces a paginated result.data, preserving the envelope', () => { + const context = ctx('find', { total: 2, limit: 10, skip: 0, data: [{ n: 1 }, { n: 2 }], - }); - replaceResult(context, [{ n: 1 }]); + }) + replaceResult(context, [{ n: 1 }]) expect(context.result).toEqual({ total: 2, limit: 10, skip: 0, data: [{ n: 1 }], - }); - }); + }) + }) - it("writes to context.dispatch when dispatch is true, leaving result intact", () => { - const context = ctx("get", { id: 1, secret: "x", name: "a" }); - replaceResult(context, [{ id: 1, name: "a" }], { dispatch: true }); - expect(context.dispatch).toEqual({ id: 1, name: "a" }); - expect(context.result).toEqual({ id: 1, secret: "x", name: "a" }); - }); + it('writes to context.dispatch when dispatch is true, leaving result intact', () => { + const context = ctx('get', { id: 1, secret: 'x', name: 'a' }) + replaceResult(context, [{ id: 1, name: 'a' }], { dispatch: true }) + expect(context.dispatch).toEqual({ id: 1, name: 'a' }) + expect(context.result).toEqual({ id: 1, secret: 'x', name: 'a' }) + }) it('writes both result and dispatch with dispatch: "both"', () => { - const context = ctx("get", { id: 1, name: "a" }); - replaceResult(context, [{ id: 1, name: "b" }], { dispatch: "both" }); - expect(context.result).toEqual({ id: 1, name: "b" }); - expect(context.dispatch).toEqual({ id: 1, name: "b" }); - }); + const context = ctx('get', { id: 1, name: 'a' }) + replaceResult(context, [{ id: 1, name: 'b' }], { dispatch: 'both' }) + expect(context.result).toEqual({ id: 1, name: 'b' }) + expect(context.dispatch).toEqual({ id: 1, name: 'b' }) + }) - it("returns the context", () => { - const context = ctx("get", { a: 1 }); - expect(replaceResult(context, [{ a: 2 }])).toBe(context); - }); -}); + it('returns the context', () => { + const context = ctx('get', { a: 1 }) + expect(replaceResult(context, [{ a: 2 }])).toBe(context) + }) +}) diff --git a/src/utils/replace-result/replace-result.util.ts b/src/utils/replace-result/replace-result.util.ts index 64c15d6..a0aa888 100644 --- a/src/utils/replace-result/replace-result.util.ts +++ b/src/utils/replace-result/replace-result.util.ts @@ -1,8 +1,8 @@ -import type { HookContext } from "@feathersjs/feathers"; -import { copy } from "fast-copy"; -import { getResultIsArray } from "../get-result-is-array/get-result-is-array.util.js"; -import type { DispatchOption } from "../../types.js"; -import type { ResultSingleHookContext } from "../../utility-types/hook-context.js"; +import type { HookContext } from '@feathersjs/feathers' +import { copy } from 'fast-copy' +import { getResultIsArray } from '../get-result-is-array/get-result-is-array.util.js' +import type { DispatchOption } from '../../types.js' +import type { ResultSingleHookContext } from '../../utility-types/hook-context.js' export type ReplaceResultOptions = { /** @@ -10,8 +10,8 @@ export type ReplaceResultOptions = { * writes both `result` and `dispatch`. When dispatch is requested and not yet * present, it is seeded from a clone of `context.result`. */ - dispatch?: DispatchOption; -}; + dispatch?: DispatchOption +} /** * Replaces `context.result` (and/or `context.dispatch`) wholesale with the given @@ -34,27 +34,27 @@ export function replaceResult( options?: ReplaceResultOptions, ): H { if (!!options?.dispatch && !context.dispatch) { - context.dispatch = copy(context.result); + context.dispatch = copy(context.result) } const write = (dispatch: boolean) => { - const { isArray, key } = getResultIsArray(context, { dispatch }); + const { isArray, key } = getResultIsArray(context, { dispatch }) if (!isArray) { - context[key] = result[0]; + context[key] = result[0] } else if (!Array.isArray(context[key]) && context[key]?.data) { - context[key].data = result; + context[key].data = result } else { - context[key] = result; + context[key] = result } - }; + } - if (options?.dispatch === "both") { - write(true); - write(false); + if (options?.dispatch === 'both') { + write(true) + write(false) } else { - write(!!options?.dispatch); + write(!!options?.dispatch) } - return context; + return context } diff --git a/src/utils/simplify-query/merge-and-branches-up.ts b/src/utils/simplify-query/merge-and-branches-up.ts new file mode 100644 index 0000000..4b92703 --- /dev/null +++ b/src/utils/simplify-query/merge-and-branches-up.ts @@ -0,0 +1,86 @@ +import { dequal as deepEqual } from 'dequal' + +type QueryRecord = Record + +/** + * `$and` is an implicit AND with the rest of the query, so ALL of its branches can + * be merged up into the parent at once — as long as no key would be set to two + * different values (across the branches or the existing keys). On any such collision + * the `$and` is kept intact. Internal helper for {@link simplifyQuery}. + */ +export function mergeAndBranchesUp( + result: QueryRecord, + enabled: boolean, +): QueryRecord { + if (!enabled || !Array.isArray(result.$and) || result.$and.length === 0) { + return result + } + const rest = { ...result } + delete rest.$and + const seen = new Map(Object.entries(rest)) + for (const branch of result.$and) { + for (const [key, value] of Object.entries(branch)) { + if (seen.has(key) && !deepEqual(seen.get(key), value)) { + return result + } + seen.set(key, value) + } + } + return Object.assign(rest, ...result.$and) +} + +if (import.meta.vitest) { + const { describe, it, expect } = import.meta.vitest + + describe('mergeAndBranchesUp', () => { + it('merges all branches up when keys do not collide', () => { + expect(mergeAndBranchesUp({ $and: [{ a: 1 }, { b: 2 }] }, true)).toEqual({ + a: 1, + b: 2, + }) + }) + + it('merges branches up alongside existing root keys', () => { + expect( + mergeAndBranchesUp({ c: 3, $and: [{ a: 1 }, { b: 2 }] }, true), + ).toEqual({ a: 1, b: 2, c: 3 }) + }) + + it('keeps the $and on a value collision between branches', () => { + expect(mergeAndBranchesUp({ $and: [{ a: 1 }, { a: 2 }] }, true)).toEqual({ + $and: [{ a: 1 }, { a: 2 }], + }) + }) + + it('keeps the $and on a collision with a root key', () => { + expect(mergeAndBranchesUp({ a: 1, $and: [{ a: 2 }] }, true)).toEqual({ + a: 1, + $and: [{ a: 2 }], + }) + }) + + it('allows a branch key equal to an existing root value', () => { + expect( + mergeAndBranchesUp({ a: 1, $and: [{ a: 1 }, { b: 2 }] }, true), + ).toEqual({ a: 1, b: 2 }) + }) + + it('does nothing when disabled', () => { + expect(mergeAndBranchesUp({ $and: [{ a: 1 }] }, false)).toEqual({ + $and: [{ a: 1 }], + }) + }) + + it('ignores a missing or empty $and', () => { + expect(mergeAndBranchesUp({ a: 1 }, true)).toEqual({ a: 1 }) + expect(mergeAndBranchesUp({ $and: [] }, true)).toEqual({ $and: [] }) + }) + + it('does not mutate the input', () => { + const query = { c: 3, $and: [{ a: 1 }, { b: 2 }] } + const snapshot = structuredClone(query) + mergeAndBranchesUp(query, true) + expect(query).toEqual(snapshot) + }) + }) +} diff --git a/src/utils/simplify-query/merge-or-branch-up.ts b/src/utils/simplify-query/merge-or-branch-up.ts new file mode 100644 index 0000000..cb8c446 --- /dev/null +++ b/src/utils/simplify-query/merge-or-branch-up.ts @@ -0,0 +1,74 @@ +import { dequal as deepEqual } from 'dequal' + +type QueryRecord = Record + +/** + * `$or` is a disjunction, so only a *single* branch can be merged up (an `$or` of + * one is just that branch) — and only when no key would collide. This is the key + * asymmetry with `$and` ({@link mergeAndBranchesUp}). Internal helper for + * {@link simplifyQuery}. + */ +export function mergeOrBranchUp( + result: QueryRecord, + enabled: boolean, +): QueryRecord { + if (!enabled || !Array.isArray(result.$or) || result.$or.length !== 1) { + return result + } + const branch = result.$or[0] + const rest = { ...result } + delete rest.$or + for (const key of Object.keys(branch)) { + if (key in rest && !deepEqual(rest[key], branch[key])) { + return result + } + } + return { ...rest, ...branch } +} + +if (import.meta.vitest) { + const { describe, it, expect } = import.meta.vitest + + describe('mergeOrBranchUp', () => { + it('merges a single branch up when keys do not collide', () => { + expect(mergeOrBranchUp({ a: 1, $or: [{ b: 2 }] }, true)).toEqual({ + a: 1, + b: 2, + }) + }) + + it('collapses a sole single-branch $or to that branch', () => { + expect(mergeOrBranchUp({ $or: [{ a: 1 }] }, true)).toEqual({ a: 1 }) + }) + + it('keeps the $or on a collision with a root key', () => { + expect(mergeOrBranchUp({ a: 1, $or: [{ a: 2 }] }, true)).toEqual({ + a: 1, + $or: [{ a: 2 }], + }) + }) + + it('does NOT merge a multi-branch $or', () => { + expect(mergeOrBranchUp({ $or: [{ a: 1 }, { b: 2 }] }, true)).toEqual({ + $or: [{ a: 1 }, { b: 2 }], + }) + }) + + it('does nothing when disabled', () => { + expect(mergeOrBranchUp({ $or: [{ a: 1 }] }, false)).toEqual({ + $or: [{ a: 1 }], + }) + }) + + it('ignores a missing $or', () => { + expect(mergeOrBranchUp({ a: 1 }, true)).toEqual({ a: 1 }) + }) + + it('does not mutate the input', () => { + const query = { a: 1, $or: [{ b: 2 }] } + const snapshot = structuredClone(query) + mergeOrBranchUp(query, true) + expect(query).toEqual(snapshot) + }) + }) +} diff --git a/src/utils/simplify-query/simplify-query.util.md b/src/utils/simplify-query/simplify-query.util.md new file mode 100644 index 0000000..e9863e2 --- /dev/null +++ b/src/utils/simplify-query/simplify-query.util.md @@ -0,0 +1,8 @@ +--- +title: simplifyQuery +category: utils +see: + - utils/merge-query + - utils/add-to-query + - utils/walk-query +--- diff --git a/src/utils/simplify-query/simplify-query.util.test.ts b/src/utils/simplify-query/simplify-query.util.test.ts new file mode 100644 index 0000000..b500cf0 --- /dev/null +++ b/src/utils/simplify-query/simplify-query.util.test.ts @@ -0,0 +1,191 @@ +import { describe, it, expect } from 'vitest' +import { simplifyQuery } from './simplify-query.util.js' + +describe('simplifyQuery', () => { + it('returns falsy queries as-is', () => { + expect(simplifyQuery(null)).toBe(null) + expect(simplifyQuery(undefined)).toBe(undefined) + }) + + it('returns a query without $and/$or unchanged (same reference)', () => { + const query = { id: 1, status: 'active' } + expect(simplifyQuery(query)).toBe(query) + }) + + it('drops an empty $and', () => { + expect(simplifyQuery({ id: 1, $and: [] })).toEqual({ id: 1 }) + }) + + it('drops an empty $or', () => { + expect(simplifyQuery({ id: 1, $or: [] })).toEqual({ id: 1 }) + }) + + it('drops a $or that contains an (always-true) empty branch', () => { + expect(simplifyQuery({ id: 1, $or: [{}, { a: 1 }] })).toEqual({ id: 1 }) + }) + + it('collapses a single-branch $and to that branch', () => { + expect(simplifyQuery({ $and: [{ id: 1 }] })).toEqual({ id: 1 }) + }) + + it('collapses a single-branch $or to that branch', () => { + expect(simplifyQuery({ $or: [{ id: 1 }] })).toEqual({ id: 1 }) + }) + + it('merges a single-branch $and up when keys do not collide', () => { + expect(simplifyQuery({ status: 'a', $and: [{ id: 1 }] })).toEqual({ + status: 'a', + id: 1, + }) + }) + + it('keeps the wrapper when merging a single branch up would collide', () => { + expect(simplifyQuery({ id: 2, $and: [{ id: 1 }] })).toEqual({ + id: 2, + $and: [{ id: 1 }], + }) + }) + + it('merges a single $and branch that carries an $or up', () => { + expect( + simplifyQuery({ status: 'a', $and: [{ $or: [{ x: 1 }, { y: 2 }] }] }), + ).toEqual({ status: 'a', $or: [{ x: 1 }, { y: 2 }] }) + }) + + it('respects replaceAnd: false / replaceOr: false at the top level', () => { + expect(simplifyQuery({ $and: [{ id: 1 }] }, { replaceAnd: false })).toEqual( + { + $and: [{ id: 1 }], + }, + ) + expect(simplifyQuery({ $or: [{ id: 1 }] }, { replaceOr: false })).toEqual({ + $or: [{ id: 1 }], + }) + }) + + it('dedupes $and and $or branches', () => { + expect(simplifyQuery({ $and: [{ id: 1 }, { id: 1 }, { id: 2 }] })).toEqual({ + $and: [{ id: 1 }, { id: 2 }], + }) + expect(simplifyQuery({ $or: [{ id: 1 }, { id: 1 }, { id: 2 }] })).toEqual({ + $or: [{ id: 1 }, { id: 2 }], + }) + }) + + it('merges all $and branches up when keys do not collide', () => { + expect(simplifyQuery({ $and: [{ a: 1 }, { b: 2 }] })).toEqual({ + a: 1, + b: 2, + }) + }) + + it('keeps a multi-branch $and when a key collides (e.g. a split range)', () => { + expect( + simplifyQuery({ $and: [{ c: { $gt: 1 } }, { c: { $lt: 9 } }] }), + ).toEqual({ $and: [{ c: { $gt: 1 } }, { c: { $lt: 9 } }] }) + }) + + it('does NOT merge a multi-branch $or up (disjunction)', () => { + expect(simplifyQuery({ status: 'a', $or: [{ x: 1 }, { y: 2 }] })).toEqual({ + status: 'a', + $or: [{ x: 1 }, { y: 2 }], + }) + }) + + it('hoists a nested pure $or (multi-branch)', () => { + expect( + simplifyQuery({ $or: [{ a: 1 }, { $or: [{ b: 2 }, { c: 3 }] }] }), + ).toEqual({ $or: [{ a: 1 }, { b: 2 }, { c: 3 }] }) + }) + + it('simplifies recursively and merges up', () => { + expect( + simplifyQuery({ $and: [{ $or: [{ id: 1 }] }, { $and: [{ a: 2 }] }] }), + ).toEqual({ id: 1, a: 2 }) + }) + + it('preserves filters and merges a (deduped) single-branch $and up beside them', () => { + expect( + simplifyQuery({ + $limit: 10, + $sort: { id: 1 }, + $and: [{ id: 1 }, { id: 1 }], + }), + ).toEqual({ $limit: 10, $sort: { id: 1 }, id: 1 }) + }) + + it('removes an empty branch from $and and merges the rest up', () => { + expect(simplifyQuery({ $and: [{}, { a: 1 }, { b: 2 }] })).toEqual({ + a: 1, + b: 2, + }) + }) + + it('drops a $or whose only branch is empty', () => { + expect(simplifyQuery({ $or: [{}] })).toEqual({}) + }) + + it('handles $and and $or present together', () => { + expect( + simplifyQuery({ $and: [{ a: 1 }], $or: [{ b: 2 }, { c: 3 }] }), + ).toEqual({ a: 1, $or: [{ b: 2 }, { c: 3 }] }) + }) + + it('simplifies deeply nested structures', () => { + expect(simplifyQuery({ $and: [{ $and: [{ $or: [{ a: 1 }] }] }] })).toEqual({ + a: 1, + }) + }) + + it('keeps the $and (carrying an $or) when it collides with the root $or', () => { + const query = { + $or: [{ a: 1 }, { a: 2 }], + $and: [{ $or: [{ b: 1 }, { b: 2 }] }], + } + expect(simplifyQuery(query)).toEqual({ + $or: [{ a: 1 }, { a: 2 }], + $and: [{ $or: [{ b: 1 }, { b: 2 }] }], + }) + }) + + it('still simplifies nested levels even when replaceAnd is false', () => { + expect( + simplifyQuery( + { $and: [{ $or: [{ a: 1 }, { a: 1 }] }] }, + { replaceAnd: false }, + ), + ).toEqual({ $and: [{ a: 1 }] }) + }) + + it('does not recurse into $nor (left untouched)', () => { + const query = { $nor: [{ $or: [{ a: 1 }, { a: 1 }] }] } + expect(simplifyQuery(query)).toEqual(query) + }) + + it('does not touch non-logical operators like $in', () => { + expect(simplifyQuery({ a: { $in: [1, 1, 2] }, $and: [{ b: 2 }] })).toEqual({ + a: { $in: [1, 1, 2] }, + b: 2, + }) + }) + + it('is idempotent', () => { + const query = { + $and: [ + { a: 1 }, + { $or: [{ b: 2 }, { b: 2 }] }, + { $and: [{ c: 3 }, { d: 4 }] }, + ], + $or: [{ e: 5 }], + } + const once = simplifyQuery(query) + expect(simplifyQuery(once)).toEqual(once) + }) + + it('does not mutate the input', () => { + const query = { $and: [{ id: 1 }, { id: 1 }], $or: [{ a: 1 }] } + const snapshot = structuredClone(query) + simplifyQuery(query) + expect(query).toEqual(snapshot) + }) +}) diff --git a/src/utils/simplify-query/simplify-query.util.ts b/src/utils/simplify-query/simplify-query.util.ts new file mode 100644 index 0000000..35914c7 --- /dev/null +++ b/src/utils/simplify-query/simplify-query.util.ts @@ -0,0 +1,98 @@ +import type { Query } from '@feathersjs/feathers' +import { isEmptyObject } from '../../common/is-empty-object.js' +import { dedupeBranches } from '../../common/dedupe-branches.js' +import { flattenAndBranches } from '../../common/flatten-and-branches.js' +import { flattenOrBranches } from '../../common/flatten-or-branches.js' +import { mergeAndBranchesUp } from './merge-and-branches-up.js' +import { mergeOrBranchUp } from './merge-or-branch-up.js' + +export interface SimplifyQueryOptions { + /** + * Dissolve a top-level single-branch `$and` by merging its branch up into the + * query (only when no key would collide). Nested levels are always dissolved. + * Default `true`. + */ + replaceAnd?: boolean + /** + * Dissolve a top-level single-branch `$or` by merging its branch up into the + * query (only when no key would collide). Nested levels are always dissolved. + * Default `true`. + */ + replaceOr?: boolean +} + +/** + * Normalizes the logical structure of a Feathers query without changing what it + * matches: empty `$and`/`$or` are dropped, duplicate branches removed, nested + * same-operator branches hoisted (`$and`-in-`$and`, pure `$or`-in-`$or`), and + * branches merged up into the parent where it is safe — all of an `$and` when no + * key collides, a single-branch `$or`. Runs recursively. Inputs are not mutated; + * a query with nothing to simplify is returned unchanged. + * + * @param query the query to simplify (a falsy query is returned as-is) + * @param options + * @returns the simplified query + * + * @example + * ```ts + * import { simplifyQuery } from 'feathers-utils/utils' + * + * // non-colliding $and branches (here also a hoisted nested $and) merge up + * simplifyQuery({ $and: [{ id: 1 }, { $and: [{ status: 'a' }] }] }) + * // => { id: 1, status: 'a' } + * + * simplifyQuery({ $or: [{ id: 1 }] }) + * // => { id: 1 } + * + * // a colliding key keeps the $and intact + * simplifyQuery({ $and: [{ price: { $gt: 1 } }, { price: { $lt: 9 } }] }) + * // => { $and: [{ price: { $gt: 1 } }, { price: { $lt: 9 } }] } + * ``` + * + * @see https://utils.feathersjs.com/utils/simplify-query.html + */ +export function simplifyQuery( + query: Q, + options: SimplifyQueryOptions = {}, +): Q { + const { replaceAnd = true, replaceOr = true } = options + return simplify(query, replaceAnd, replaceOr) +} + +function simplify(query: any, replaceAnd: boolean, replaceOr: boolean): any { + if (!query || typeof query !== 'object' || Array.isArray(query)) { + return query + } + + const hasAnd = Array.isArray(query.$and) + const hasOr = Array.isArray(query.$or) + if (!hasAnd && !hasOr) { + return query + } + + const { $and, $or, ...rest } = query + const result: Record = { ...rest } + + if (hasAnd) { + // an empty `{}` branch is the AND identity → dropped by dedupeBranches + const branches = dedupeBranches( + flattenAndBranches($and.map((b: any) => simplify(b, true, true))), + ) + if (branches.length > 0) { + result.$and = branches + } + } + + if (hasOr) { + const simplified = $or.map((b: any) => simplify(b, true, true)) + // an empty `{}` branch makes the whole `$or` match-all → drop the `$or` + if (!simplified.some(isEmptyObject)) { + const branches = dedupeBranches(flattenOrBranches(simplified)) + if (branches.length > 0) { + result.$or = branches + } + } + } + + return mergeOrBranchUp(mergeAndBranchesUp(result, replaceAnd), replaceOr) +} diff --git a/test/index.test.ts b/test/index.test.ts index afd77c6..7f880e5 100755 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -65,6 +65,7 @@ const utils = [ 'queryHasProperty', 'replaceData', 'replaceResult', + 'simplifyQuery', 'skipResult', 'sortQueryProperties', 'toPaginated',