From 1e453f870e80883422600da12b8246d7fe9c05a9 Mon Sep 17 00:00:00 2001 From: Alexander Onnikov Date: Mon, 20 Nov 2023 15:10:51 +0700 Subject: [PATCH] UBERF-4321 Refactor mergeQueries (#4012) Signed-off-by: Alexander Onnikov --- packages/core/src/__tests__/memdb.test.ts | 50 +----- packages/core/src/__tests__/utils.test.ts | 185 +++++++++++++++++++++ packages/core/src/utils.ts | 187 +++++++++++++++++----- 3 files changed, 332 insertions(+), 90 deletions(-) create mode 100644 packages/core/src/__tests__/utils.test.ts diff --git a/packages/core/src/__tests__/memdb.test.ts b/packages/core/src/__tests__/memdb.test.ts index 0bdf71cff0..1ca542a524 100644 --- a/packages/core/src/__tests__/memdb.test.ts +++ b/packages/core/src/__tests__/memdb.test.ts @@ -13,7 +13,7 @@ // limitations under the License. // -import { Client, generateId, mergeQueries } from '..' +import { Client } from '..' import type { Class, Doc, Obj, Ref } from '../classes' import core from '../component' import { Hierarchy } from '../hierarchy' @@ -408,52 +408,4 @@ describe('memdb', () => { expect(e).toEqual(new Error('createDoc cannot be used for objects inherited from AttachedDoc')) } }) - - it('mergeQueries', () => { - const id1 = generateId() - const id2 = generateId() - const q1 = { - space: id1, - unique: 'item', - age: { $gt: 10 } - } as any - const q2 = { - space: { $in: [id1, id2] }, - age: 30 - } as any - const resCompare = { - space: id1, - unique: 'item', - age: { $gt: 30 } - } as any - expect(mergeQueries(q1, q2)).toEqual(resCompare) - expect(mergeQueries(q2, q1)).toEqual(resCompare) - - const q3 = { - space: { $nin: [id1] }, - age: 20 - } as any - const resCompare2 = { - space: { $ne: id1 }, - age: [] - } as any - expect(mergeQueries(q2, q3)).toEqual(resCompare2) - expect(mergeQueries(q3, q2)).toEqual(resCompare2) - - expect(mergeQueries({ age: { $lt: 20 } } as any, { age: { $gt: 25 } } as any)).toEqual({ age: { $gt: 25 } }) - expect(mergeQueries({ age: { $gt: 25 } } as any, { age: { $lt: 20 } } as any)).toEqual({ age: { $lt: 20 } }) - - const query4 = { - space: { $in: [id1] } - } as any - const query5 = { - space: { $in: [id2, id1] } - } as any - const resQuery45 = { - space: id1 - } as any - - expect(mergeQueries(query4, query5)).toEqual(resQuery45) - expect(mergeQueries(query5, query4)).toEqual(resQuery45) - }) }) diff --git a/packages/core/src/__tests__/utils.test.ts b/packages/core/src/__tests__/utils.test.ts new file mode 100644 index 0000000000..0171bba093 --- /dev/null +++ b/packages/core/src/__tests__/utils.test.ts @@ -0,0 +1,185 @@ +// +// Copyright © 2023 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import { mergeQueries } from '..' + +describe('mergeQueries', () => { + it('merges query with empty query', () => { + const q1 = { name: 'john', age: { $gt: 42 } } + const q2 = {} + const res = { name: 'john', age: { $gt: 42 } } + + expect(mergeQueries(q1, q2)).toEqual(res) + expect(mergeQueries(q2, q1)).toEqual(res) + }) + + it('merges query with different fields', () => { + const q1 = { name: 'john' } + const q2 = { age: { $gt: 42 } } + const res = { name: 'john', age: { $gt: 42 } } + + expect(mergeQueries(q1, q2)).toEqual(res) + expect(mergeQueries(q2, q1)).toEqual(res) + }) + + it('merges equal field values', () => { + expect(mergeQueries({ value: 42 }, { value: 42 })).toEqual({ value: 42 }) + }) + + it('does not merge different field values', () => { + const q1 = { value: 42 } + const q2 = { value: 'true' } + const res = { value: { $in: [] } } + expect(mergeQueries(q1, q2)).toEqual(res) + expect(mergeQueries(q2, q1)).toEqual(res) + }) + + it('merges predicate with predicate', () => { + expect(mergeQueries({ age: { $in: [41, 42] } }, { age: { $in: [42, 43] } })).toEqual({ age: { $in: [42] } }) + expect(mergeQueries({ age: { $in: [42, 43] } }, { age: { $in: [41, 42] } })).toEqual({ age: { $in: [42] } }) + + expect(mergeQueries({ age: { $nin: [42] } }, { age: { $nin: [42] } })).toEqual({ age: { $nin: [42] } }) + + expect(mergeQueries({ age: { $lt: 45 } }, { age: { $lt: 42 } })).toEqual({ age: { $lt: 42 } }) + expect(mergeQueries({ age: { $lt: 42 } }, { age: { $lt: 45 } })).toEqual({ age: { $lt: 42 } }) + + expect(mergeQueries({ age: { $gt: 40 } }, { age: { $gt: 42 } })).toEqual({ age: { $gt: 42 } }) + expect(mergeQueries({ age: { $gt: 42 } }, { age: { $gt: 40 } })).toEqual({ age: { $gt: 42 } }) + + expect(mergeQueries({ age: { $lte: 45 } }, { age: { $lte: 42 } })).toEqual({ age: { $lte: 42 } }) + expect(mergeQueries({ age: { $lte: 42 } }, { age: { $lte: 45 } })).toEqual({ age: { $lte: 42 } }) + + expect(mergeQueries({ age: { $gte: 40 } }, { age: { $gte: 42 } })).toEqual({ age: { $gte: 42 } }) + expect(mergeQueries({ age: { $gte: 42 } }, { age: { $gte: 40 } })).toEqual({ age: { $gte: 42 } }) + + expect(mergeQueries({ age: { $ne: 42 } }, { age: { $ne: 42 } })).toEqual({ age: { $ne: 42 } }) + }) + + it('merges predicate with value', () => { + // positive + expect(mergeQueries({ age: { $in: [41, 42, 43] } }, { age: 42 })).toEqual({ age: 42 }) + expect(mergeQueries({ age: 42 }, { age: { $in: [41, 42, 43] } })).toEqual({ age: 42 }) + + expect(mergeQueries({ age: { $nin: [41, 43] } }, { age: 42 })).toEqual({ age: 42 }) + expect(mergeQueries({ age: 42 }, { age: { $nin: [41, 43] } })).toEqual({ age: 42 }) + + expect(mergeQueries({ age: { $lt: 45 } }, { age: 42 })).toEqual({ age: 42 }) + expect(mergeQueries({ age: 42 }, { age: { $lt: 45 } })).toEqual({ age: 42 }) + + expect(mergeQueries({ age: { $gt: 40 } }, { age: 42 })).toEqual({ age: 42 }) + expect(mergeQueries({ age: 42 }, { age: { $gt: 40 } })).toEqual({ age: 42 }) + + expect(mergeQueries({ age: { $lte: 42 } }, { age: 42 })).toEqual({ age: 42 }) + expect(mergeQueries({ age: 42 }, { age: { $lte: 42 } })).toEqual({ age: 42 }) + + expect(mergeQueries({ age: { $gte: 42 } }, { age: 42 })).toEqual({ age: 42 }) + expect(mergeQueries({ age: 42 }, { age: { $gte: 42 } })).toEqual({ age: 42 }) + + expect(mergeQueries({ age: { $ne: 40 } }, { age: 42 })).toEqual({ age: 42 }) + expect(mergeQueries({ age: 42 }, { age: { $ne: 40 } })).toEqual({ age: 42 }) + + // negative + expect(mergeQueries({ age: { $in: [31, 32, 33] } }, { age: 42 })).toEqual({ age: { $in: [] } }) + expect(mergeQueries({ age: 42 }, { age: { $in: [31, 32, 33] } })).toEqual({ age: { $in: [] } }) + + expect(mergeQueries({ age: { $nin: [41, 42, 43] } }, { age: 42 })).toEqual({ age: { $in: [] } }) + expect(mergeQueries({ age: 42 }, { age: { $nin: [41, 42, 43] } })).toEqual({ age: { $in: [] } }) + + expect(mergeQueries({ age: { $lt: 42 } }, { age: 42 })).toEqual({ age: { $in: [] } }) + expect(mergeQueries({ age: 42 }, { age: { $lt: 42 } })).toEqual({ age: { $in: [] } }) + + expect(mergeQueries({ age: { $gt: 42 } }, { age: 42 })).toEqual({ age: { $in: [] } }) + expect(mergeQueries({ age: 42 }, { age: { $gt: 42 } })).toEqual({ age: { $in: [] } }) + + expect(mergeQueries({ age: { $lte: 40 } }, { age: 42 })).toEqual({ age: { $in: [] } }) + expect(mergeQueries({ age: 42 }, { age: { $lte: 40 } })).toEqual({ age: { $in: [] } }) + + expect(mergeQueries({ age: { $gte: 43 } }, { age: 42 })).toEqual({ age: { $in: [] } }) + expect(mergeQueries({ age: 42 }, { age: { $gte: 43 } })).toEqual({ age: { $in: [] } }) + + expect(mergeQueries({ age: { $ne: 42 } }, { age: 42 })).toEqual({ age: { $in: [] } }) + expect(mergeQueries({ age: 42 }, { age: { $ne: 42 } })).toEqual({ age: { $in: [] } }) + }) + + it('$in merge', () => { + expect(mergeQueries({ value: { $in: [1, 2] } }, { value: { $in: [2, 3] } })).toEqual({ value: { $in: [2] } }) + expect(mergeQueries({ value: { $in: [2, 3] } }, { value: { $in: [1, 2] } })).toEqual({ value: { $in: [2] } }) + + expect(mergeQueries({ value: { $in: [1, 2] } }, { value: { $in: [3, 4] } })).toEqual({ value: { $in: [] } }) + expect(mergeQueries({ value: { $in: [3, 4] } }, { value: { $in: [1, 2] } })).toEqual({ value: { $in: [] } }) + + expect(mergeQueries({ value: { $in: [] } }, { value: { $in: [] } })).toEqual({ value: { $in: [] } }) + expect(mergeQueries({ value: { $in: [42] } }, { value: { $in: [42] } })).toEqual({ value: { $in: [42] } }) + }) + + it('$nin merge', () => { + expect(mergeQueries({ value: { $nin: [] } }, { value: { $nin: [] } })).toEqual({}) + expect(mergeQueries({ value: { $nin: [42] } }, { value: { $nin: [42] } })).toEqual({ value: { $nin: [42] } }) + }) + + it('$in $nin $ne merge', () => { + // $in and $nin + expect(mergeQueries({ value: { $in: [1, 2] } }, { value: { $nin: [1] } })).toEqual({ value: { $in: [2] } }) + expect(mergeQueries({ value: { $nin: [1] } }, { value: { $in: [1, 2] } })).toEqual({ value: { $in: [2] } }) + + expect(mergeQueries({ value: { $in: ['a', 'b'] } }, { value: { $nin: ['a'] } })).toEqual({ value: { $in: ['b'] } }) + expect(mergeQueries({ value: { $nin: ['a'] } }, { value: { $in: ['a', 'b'] } })).toEqual({ value: { $in: ['b'] } }) + + expect(mergeQueries({ value: { $in: [1, 2] } }, { value: { $nin: [1, 2, 3] } })).toEqual({ value: { $in: [] } }) + expect(mergeQueries({ value: { $nin: [1, 2, 3] } }, { value: { $in: [1, 2] } })).toEqual({ value: { $in: [] } }) + + expect(mergeQueries({ value: { $in: [1, 2] } }, { value: { $nin: [1, 2] } })).toEqual({ value: { $in: [] } }) + + expect(mergeQueries({ value: { $in: [] } }, { value: { $nin: [42] } })).toEqual({ value: { $in: [] } }) + expect(mergeQueries({ value: { $nin: [42] } }, { value: { $in: [] } })).toEqual({ value: { $in: [] } }) + + // $in and $ne + expect(mergeQueries({ value: { $in: [1, 2] } }, { value: { $ne: 1 } })).toEqual({ value: { $in: [2] } }) + expect(mergeQueries({ value: { $ne: 1 } }, { value: { $in: [1, 2] } })).toEqual({ value: { $in: [2] } }) + + expect(mergeQueries({ value: { $in: [1] } }, { value: { $ne: 1 } })).toEqual({ value: { $in: [] } }) + expect(mergeQueries({ value: { $ne: 1 } }, { value: { $in: [1] } })).toEqual({ value: { $in: [] } }) + + expect(mergeQueries({ value: { $in: [] } }, { value: { $ne: 42 } })).toEqual({ value: { $in: [] } }) + expect(mergeQueries({ value: { $ne: 42 } }, { value: { $in: [] } })).toEqual({ value: { $in: [] } }) + }) + + it('$lt and $gt', () => { + expect(mergeQueries({ age: { $lt: 25 } }, { age: { $gt: 20 } })).toEqual({ age: { $lt: 25, $gt: 20 } }) + expect(mergeQueries({ age: { $gt: 20 } }, { age: { $lt: 25 } })).toEqual({ age: { $lt: 25, $gt: 20 } }) + + expect(mergeQueries({ age: { $lt: 20 } }, { age: { $gt: 25 } })).toEqual({ age: { $lt: 20, $gt: 25 } }) + expect(mergeQueries({ age: { $gt: 25 } }, { age: { $lt: 20 } })).toEqual({ age: { $lt: 20, $gt: 25 } }) + }) + + it('complex', () => { + const q1 = { + space: 1, + unique: 'item', + age: { $gt: 10 } + } as any + const q2 = { + space: { $in: [1, 2] }, + age: 30 + } as any + const res = { + space: 1, + unique: 'item', + age: 30 + } as any + expect(mergeQueries(q1, q2)).toEqual(res) + expect(mergeQueries(q2, q1)).toEqual(res) + }) +}) diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index 617ac33a77..103477aeee 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -13,6 +13,7 @@ // limitations under the License. // +import { deepEqual } from 'fast-equals' import { Account, AnyAttribute, Class, Doc, DocData, DocIndexState, IndexKind, Obj, Ref, Space } from './classes' import core from './component' import { Hierarchy } from './hierarchy' @@ -342,62 +343,166 @@ export class RateLimitter { } export function mergeQueries (query1: DocumentQuery, query2: DocumentQuery): DocumentQuery { - const q = Object.assign({}, query1) - for (const k in query2) { - if (!Object.keys(query1).includes(k)) { - Object.assign(q, { [k]: query2[k] }) - continue + const keys1 = Object.keys(query1) + const keys2 = Object.keys(query2) + + const query = {} + + for (const key of keys1) { + if (!keys2.includes(key)) { + Object.assign(query, { [key]: query1[key] }) } - Object.assign(q, { [k]: getInNiN(query1[k], query2[k]) }) - if (isPredicate(query2[k]) || isPredicate(query1[k])) { - const toIterate = isPredicate(query2[k]) ? query2[k] : query1[k] - for (const x in toIterate) { - if (['$lt', '$gt'].includes(x)) { - const val1 = isPredicate(query1[k]) ? query1[k][x] : query1[k] - const val2 = isPredicate(query2[k]) ? query2[k][x] : query2[k] - if (x === '$lt') { - Object.assign(q, { [k]: { $lt: val1 < val2 ? val1 : val2 } }) - continue - } - if (x === '$gt') { - Object.assign(q, { [k]: { $gt: val1 > val2 ? val1 : val2 } }) - } - } + } + + for (const key of keys2) { + if (!keys1.includes(key)) { + Object.assign(query, { [key]: query2[key] }) + } else { + const value = mergeField(query1[key], query2[key]) + if (value !== undefined) { + Object.assign(query, { [key]: value }) } } } - return q + + return query +} + +function mergeField (field1: any, field2: any): Object | undefined { + // this is a special predicate that causes query never return any docs + // it is used in cases when queries intersection is empty + const never = { $in: [] } + // list of ignored predicates, handled separately + const ignored = ['$in', '$nin', '$ne'] + + const isPredicate1 = isPredicate(field1) + const isPredicate2 = isPredicate(field2) + + if (isPredicate1 && isPredicate2) { + // $in, $nin, $eq are related fields so handle them separately here + const result = getInNiN(field1, field2) + + const keys1 = Object.keys(field1) + const keys2 = Object.keys(field2) + + for (const key of keys1) { + if (ignored.includes(key)) continue + + if (!keys2.includes(key)) { + Object.assign(result, { [key]: field1[key] }) + } else { + const value = mergePredicateWithPredicate(key, field1[key], field2[key]) + if (value !== undefined) { + Object.assign(result, { [key]: value }) + } + } + } + + for (const key of keys2) { + if (ignored.includes(key)) continue + + if (!keys1.includes(key)) { + Object.assign(result, { [key]: field2[key] }) + } + } + + return Object.keys(result).length > 0 ? result : undefined + } else if (isPredicate1 || isPredicate2) { + // when one field is a predicate and the other is a simple value + // we need to ensure that the value matches predicate + const predicate = isPredicate1 ? field1 : field2 + const value = isPredicate1 ? field2 : field1 + + for (const x in predicate) { + const result = mergePredicateWithValue(x, predicate[x], value) + if (result !== undefined) { + return result + } + } + + // if we reached here, the value does not match the predicate + return never + } else { + // both are not predicates, can filter only when values are equal + return deepEqual(field1, field2) ? field1 : never + } +} + +function mergePredicateWithPredicate (predicate: string, val1: any, val2: any): any | undefined { + if (val1 === undefined) return val2 + if (val2 === undefined) return val1 + + switch (predicate) { + case '$lt': + return val1 < val2 ? val1 : val2 + case '$lte': + return val1 <= val2 ? val1 : val2 + case '$gt': + return val1 > val2 ? val1 : val2 + case '$gte': + return val1 >= val2 ? val1 : val2 + } + + // TODO we should properly support all available predicates here + // until then, fallback to the first predicate value + + return val1 +} + +function mergePredicateWithValue (predicate: string, val1: any, val2: any): any | undefined { + switch (predicate) { + case '$in': + return Array.isArray(val1) && val1.includes(val2) ? val2 : undefined + case '$nin': + return Array.isArray(val1) && !val1.includes(val2) ? val2 : undefined + case '$lt': + return val2 < val1 ? val2 : undefined + case '$lte': + return val2 <= val1 ? val2 : undefined + case '$gt': + return val2 > val1 ? val2 : undefined + case '$gte': + return val2 >= val1 ? val2 : undefined + case '$ne': + return val1 !== val2 ? val2 : undefined + } + + // TODO we should properly support all available predicates here + // until then, fallback to the non-predicate value + + return val2 } function getInNiN (query1: any, query2: any): Object { - const aIn = - (typeof query1 === 'object' && '$in' in query1 ? query1.$in : undefined) ?? - (typeof query1 !== 'object' && query1 !== undefined ? [query1] : []) + const aIn = typeof query1 === 'object' && '$in' in query1 ? query1.$in : undefined + const bIn = typeof query2 === 'object' && '$in' in query2 ? query2.$in : undefined const aNIn = (typeof query1 === 'object' && '$nin' in query1 ? query1.$nin : undefined) ?? (typeof query1 === 'object' && query1.$ne !== undefined ? [query1.$ne] : []) - const bIn = - (typeof query2 === 'object' && '$in' in query2 ? query2.$in : undefined) ?? - (typeof query2 !== 'object' && query2 !== undefined ? [query2] : []) const bNIn = (typeof query2 === 'object' && '$nin' in query2 ? query2.$nin : undefined) ?? - (typeof query2 === 'object' && query2.$ne !== undefined ? [query2.$ne] : []) - const finalIn = - aIn.length - bIn.length < 0 ? bIn.filter((c: any) => aIn.includes(c)) : aIn.filter((c: any) => bIn.includes(c)) + (typeof query1 === 'object' && query2.$ne !== undefined ? [query2.$ne] : []) + const finalNin = Array.from(new Set([...aNIn, ...bNIn])) - if (finalIn.length === 1 && finalNin.length === 0) { - return finalIn[0] + + // we must keep $in if it was in the original query + if (aIn !== undefined || bIn !== undefined) { + const finalIn = + aIn !== undefined && bIn !== undefined + ? aIn.length - bIn.length < 0 + ? bIn.filter((c: any) => aIn.includes(c)) + : aIn.filter((c: any) => bIn.includes(c)) + : aIn ?? bIn + return { $in: finalIn.filter((p: any) => !finalNin.includes(p)) } } - if (finalIn.length === 0 && finalNin.length === 1) { - return { $ne: finalNin[0] } - } - const res: any = {} - if (finalIn.length > 0) { - res.$in = finalIn + // try to preserve original $ne instead of $nin + if ((typeof query1 === 'object' && '$ne' in query1) || (typeof query2 === 'object' && '$ne' in query2)) { + if (finalNin.length === 1) { + return { $ne: finalNin[0] } + } } if (finalNin.length > 0) { - res.$nin = finalNin + return { $nin: finalNin } } - if (aIn.length === 1 && bIn.length === 1) return [] - return res + return {} }