diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index bafaef40e4..bb3133f852 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -463,6 +463,24 @@ class InternalBuilder { } } + if (filters.$and) { + const { $and } = filters + query = query.where(x => { + for (const condition of $and.conditions) { + x = this.addFilters(x, condition, opts) + } + }) + } + + if (filters.$or) { + const { $or } = filters + query = query.where(x => { + for (const condition of $or.conditions) { + x = this.addFilters(x, { ...condition, allOr: true }, opts) + } + }) + } + if (filters.oneOf) { const fnc = allOr ? "orWhereIn" : "whereIn" iterate( diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index 12e76155bc..c38a415aa2 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -6,11 +6,13 @@ import { RequiredKeys, RowSearchParams, SearchFilterKey, + LogicalOperator, } from "@budibase/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../../sdk" import { db, context } from "@budibase/backend-core" import { enrichSearchContext } from "./utils" +import { isExternalTableID } from "../../../integrations/utils" export async function searchView( ctx: UserCtx @@ -35,25 +37,33 @@ export async function searchView( // that could let users find rows they should not be allowed to access. let query = dataFilters.buildQuery(view.query || []) if (body.query) { - // Extract existing fields - const existingFields = - view.query - ?.filter(filter => filter.field) - .map(filter => db.removeKeyNumbering(filter.field)) || [] - // Delete extraneous search params that cannot be overridden delete body.query.allOr delete body.query.onEmptyFilter - // Carry over filters for unused fields - Object.keys(body.query).forEach(key => { - const operator = key as SearchFilterKey - Object.keys(body.query[operator] || {}).forEach(field => { - if (!existingFields.includes(db.removeKeyNumbering(field))) { - query[operator]![field] = body.query[operator]![field] - } + if (!isExternalTableID(view.tableId) && !db.isSqsEnabledForTenant()) { + // Extract existing fields + const existingFields = + view.query + ?.filter(filter => filter.field) + .map(filter => db.removeKeyNumbering(filter.field)) || [] + + // Carry over filters for unused fields + Object.keys(body.query).forEach(key => { + const operator = key as Exclude + Object.keys(body.query[operator] || {}).forEach(field => { + if (!existingFields.includes(db.removeKeyNumbering(field))) { + query[operator]![field] = body.query[operator]![field] + } + }) }) - }) + } else { + query = { + $and: { + conditions: [query, body.query], + }, + } + } } await context.ensureSnippetContext(true) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 7d7fa7b1e0..9f0a9e6269 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2696,4 +2696,239 @@ describe.each([ ) }) }) + + !isLucene && + describe("$and", () => { + beforeAll(async () => { + table = await createTable({ + age: { name: "age", type: FieldType.NUMBER }, + name: { name: "name", type: FieldType.STRING }, + }) + await createRows([ + { age: 1, name: "Jane" }, + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("successfully finds a row for one level condition", async () => { + await expectQuery({ + $and: { + conditions: [{ equal: { age: 10 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([{ age: 10, name: "Jack" }]) + }) + + it("successfully finds a row for one level with multiple conditions", async () => { + await expectQuery({ + $and: { + conditions: [{ equal: { age: 10 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([{ age: 10, name: "Jack" }]) + }) + + it("successfully finds multiple rows for one level with multiple conditions", async () => { + await expectQuery({ + $and: { + conditions: [ + { range: { age: { low: 1, high: 9 } } }, + { string: { name: "Ja" } }, + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("successfully finds rows for nested filters", async () => { + await expectQuery({ + $and: { + conditions: [ + { + $and: { + conditions: [ + { + range: { age: { low: 1, high: 10 } }, + }, + { string: { name: "Ja" } }, + ], + }, + equal: { name: "Jane" }, + }, + ], + }, + }).toContainExactly([{ age: 1, name: "Jane" }]) + }) + + it("returns nothing when filtering out all data", async () => { + await expectQuery({ + $and: { + conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], + }, + }).toFindNothing() + }) + + !isInMemory && + it("validates conditions that are not objects", async () => { + await expect( + expectQuery({ + $and: { + conditions: [{ equal: { age: 10 } }, "invalidCondition" as any], + }, + }).toFindNothing() + ).rejects.toThrow( + 'Invalid body - "query.$and.conditions[1]" must be of type object' + ) + }) + + !isInMemory && + it("validates $and without conditions", async () => { + await expect( + expectQuery({ + $and: { + conditions: [ + { equal: { age: 10 } }, + { + $and: { + conditions: undefined as any, + }, + }, + ], + }, + }).toFindNothing() + ).rejects.toThrow( + 'Invalid body - "query.$and.conditions[1].$and.conditions" is required' + ) + }) + }) + + !isLucene && + describe("$or", () => { + beforeAll(async () => { + table = await createTable({ + age: { name: "age", type: FieldType.NUMBER }, + name: { name: "name", type: FieldType.STRING }, + }) + await createRows([ + { age: 1, name: "Jane" }, + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("successfully finds a row for one level condition", async () => { + await expectQuery({ + $or: { + conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([ + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + ]) + }) + + it("successfully finds a row for one level with multiple conditions", async () => { + await expectQuery({ + $or: { + conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([ + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + ]) + }) + + it("successfully finds multiple rows for one level with multiple conditions", async () => { + await expectQuery({ + $or: { + conditions: [ + { range: { age: { low: 1, high: 9 } } }, + { string: { name: "Jan" } }, + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("successfully finds rows for nested filters", async () => { + await expectQuery({ + $or: { + conditions: [ + { + $or: { + conditions: [ + { + range: { age: { low: 1, high: 7 } }, + }, + { string: { name: "Jan" } }, + ], + }, + equal: { name: "Jane" }, + }, + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("returns nothing when filtering out all data", async () => { + await expectQuery({ + $or: { + conditions: [{ equal: { age: 6 } }, { equal: { name: "John" } }], + }, + }).toFindNothing() + }) + + it("can nest $and under $or filters", async () => { + await expectQuery({ + $or: { + conditions: [ + { + $and: { + conditions: [ + { + range: { age: { low: 1, high: 8 } }, + }, + { equal: { name: "Jan" } }, + ], + }, + equal: { name: "Jane" }, + }, + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("can nest $or under $and filters", async () => { + await expectQuery({ + $and: { + conditions: [ + { + $or: { + conditions: [ + { + range: { age: { low: 1, high: 8 } }, + }, + { equal: { name: "Jan" } }, + ], + }, + equal: { name: "Jane" }, + }, + ], + }, + }).toContainExactly([{ age: 1, name: "Jane" }]) + }) + }) }) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 8c0bc39234..ea6aedbe3c 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1485,6 +1485,119 @@ describe.each([ } ) }) + + isLucene && + it("in lucene, cannot override a view filter", async () => { + await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + const two = await config.api.row.save(table._id!, { + one: "foo2", + two: "bar2", + }) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + query: [ + { + operator: BasicOperator.EQUAL, + field: "two", + value: "bar2", + }, + ], + schema: { + id: { visible: true }, + one: { visible: false }, + two: { visible: true }, + }, + }) + + const response = await config.api.viewV2.search(view.id, { + query: { + equal: { + two: "bar", + }, + }, + }) + expect(response.rows).toHaveLength(1) + expect(response.rows).toEqual([ + expect.objectContaining({ _id: two._id }), + ]) + }) + + !isLucene && + it("can filter a view without a view filter", async () => { + const one = await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + await config.api.row.save(table._id!, { + one: "foo2", + two: "bar2", + }) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + id: { visible: true }, + one: { visible: false }, + two: { visible: true }, + }, + }) + + const response = await config.api.viewV2.search(view.id, { + query: { + equal: { + two: "bar", + }, + }, + }) + expect(response.rows).toHaveLength(1) + expect(response.rows).toEqual([ + expect.objectContaining({ _id: one._id }), + ]) + }) + + !isLucene && + it("cannot bypass a view filter", async () => { + await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + await config.api.row.save(table._id!, { + one: "foo2", + two: "bar2", + }) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + query: [ + { + operator: BasicOperator.EQUAL, + field: "two", + value: "bar2", + }, + ], + schema: { + id: { visible: true }, + one: { visible: false }, + two: { visible: true }, + }, + }) + + const response = await config.api.viewV2.search(view.id, { + query: { + equal: { + two: "bar", + }, + }, + }) + expect(response.rows).toHaveLength(0) + }) }) describe("permissions", () => { diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 671ce95038..9aa112cf4d 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -1,6 +1,11 @@ import { auth, permissions } from "@budibase/backend-core" import { DataSourceOperation } from "../../../constants" -import { Table, WebhookActionType } from "@budibase/types" +import { + EmptyFilterOption, + SearchFilters, + Table, + WebhookActionType, +} from "@budibase/types" import Joi, { CustomValidator } from "joi" import { ValidSnippetNameRegex, helpers } from "@budibase/shared-core" import sdk from "../../../sdk" @@ -84,7 +89,12 @@ export function datasourceValidator() { } function filterObject() { - return Joi.object({ + const conditionalFilteringObject = () => + Joi.object({ + conditions: Joi.array().items(Joi.link("#schema")).required(), + }) + + const filtersValidators: Record = { string: Joi.object().optional(), fuzzy: Joi.object().optional(), range: Joi.object().optional(), @@ -95,8 +105,17 @@ function filterObject() { oneOf: Joi.object().optional(), contains: Joi.object().optional(), notContains: Joi.object().optional(), + containsAny: Joi.object().optional(), allOr: Joi.boolean().optional(), - }).unknown(true) + onEmptyFilter: Joi.string() + .optional() + .valid(...Object.values(EmptyFilterOption)), + $and: conditionalFilteringObject(), + $or: conditionalFilteringObject(), + fuzzyOr: Joi.forbidden(), + documentType: Joi.forbidden(), + } + return Joi.object(filtersValidators).unknown(true).id("schema") } export function internalSearchValidator() { diff --git a/packages/server/src/automations/steps/queryRows.ts b/packages/server/src/automations/steps/queryRows.ts index 172bbf7f55..526e994c1f 100644 --- a/packages/server/src/automations/steps/queryRows.ts +++ b/packages/server/src/automations/steps/queryRows.ts @@ -11,13 +11,10 @@ import { AutomationStepSchema, AutomationStepType, EmptyFilterOption, - SearchFilters, - Table, SortOrder, QueryRowsStepInputs, QueryRowsStepOutputs, } from "@budibase/types" -import { db as dbCore } from "@budibase/backend-core" const SortOrderPretty = { [SortOrder.ASCENDING]: "Ascending", @@ -95,38 +92,6 @@ async function getTable(appId: string, tableId: string) { return ctx.body } -function typeCoercion(filters: SearchFilters, table: Table) { - if (!filters || !table) { - return filters - } - for (let key of Object.keys(filters)) { - const searchParam = filters[key as keyof SearchFilters] - if (typeof searchParam === "object") { - for (let [property, value] of Object.entries(searchParam)) { - // We need to strip numerical prefixes here, so that we can look up - // the correct field name in the schema - const columnName = dbCore.removeKeyNumbering(property) - const column = table.schema[columnName] - - // convert string inputs - if (!column || typeof value !== "string") { - continue - } - if (column.type === FieldType.NUMBER) { - if (key === "oneOf") { - searchParam[property] = value - .split(",") - .map(item => parseFloat(item)) - } else { - searchParam[property] = parseFloat(value) - } - } - } - } - } - return filters -} - function hasNullFilters(filters: any[]) { return ( filters.length === 0 || @@ -157,7 +122,7 @@ export async function run({ sortType = fieldType === FieldType.NUMBER ? FieldType.NUMBER : FieldType.STRING } - const ctx: any = buildCtx(appId, null, { + const ctx = buildCtx(appId, null, { params: { tableId, }, @@ -165,7 +130,7 @@ export async function run({ sortType, limit, sort: sortColumn, - query: typeCoercion(filters || {}, table), + query: filters || {}, // default to ascending, like data tab sortOrder: sortOrder || SortOrder.ASCENDING, }, diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 321ffbd9af..66ec905c61 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -2,6 +2,7 @@ import { Datasource, DocumentType, FieldType, + isLogicalSearchOperator, Operation, QueryJson, RelationshipFieldMetadata, @@ -137,20 +138,33 @@ function cleanupFilters( allTables.some(table => table.schema[key]) const splitter = new dataFilters.ColumnSplitter(allTables) - for (const filter of Object.values(filters)) { - for (const key of Object.keys(filter)) { - const { numberPrefix, relationshipPrefix, column } = splitter.run(key) - if (keyInAnyTable(column)) { - filter[ - `${numberPrefix || ""}${relationshipPrefix || ""}${mapToUserColumn( - column - )}` - ] = filter[key] - delete filter[key] + + const prefixFilters = (filters: SearchFilters) => { + for (const filterKey of Object.keys(filters) as (keyof SearchFilters)[]) { + if (isLogicalSearchOperator(filterKey)) { + for (const condition of filters[filterKey]!.conditions) { + prefixFilters(condition) + } + } else { + const filter = filters[filterKey]! + if (typeof filter !== "object") { + continue + } + for (const key of Object.keys(filter)) { + const { numberPrefix, relationshipPrefix, column } = splitter.run(key) + if (keyInAnyTable(column)) { + filter[ + `${numberPrefix || ""}${ + relationshipPrefix || "" + }${mapToUserColumn(column)}` + ] = filter[key] + delete filter[key] + } + } } } } - + prefixFilters(filters) return filters } diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index d30f591abc..e1a783175d 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -17,6 +17,8 @@ import { Table, BasicOperator, RangeOperator, + LogicalOperator, + isLogicalSearchOperator, } from "@budibase/types" import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" @@ -358,6 +360,8 @@ export const buildQuery = (filter: SearchFilter[]) => { high: value, } } + } else if (isLogicalSearchOperator(queryOperator)) { + // TODO } else if (query[queryOperator] && operator !== "onEmptyFilter") { if (type === "boolean") { // Transform boolean filters to cope with null. @@ -458,14 +462,17 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) => (doc: Record) => { for (const [key, testValue] of Object.entries(query[type] || {})) { - const result = test(deepGet(doc, removeKeyNumbering(key)), testValue) + const valueToCheck = isLogicalSearchOperator(type) + ? doc + : deepGet(doc, removeKeyNumbering(key)) + const result = test(valueToCheck, testValue) if (query.allOr && result) { return true } else if (!query.allOr && !result) { return false } } - return true + return !query.allOr } const stringMatch = match( @@ -666,8 +673,45 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) const containsAny = match(ArrayOperator.CONTAINS_ANY, _contains("some")) + const and = match( + LogicalOperator.AND, + (docValue: Record, conditions: SearchFilters[]) => { + if (!conditions.length) { + return false + } + for (const condition of conditions) { + const matchesCondition = runQuery([docValue], condition) + if (!matchesCondition.length) { + return false + } + } + return true + } + ) + const or = match( + LogicalOperator.OR, + (docValue: Record, conditions: SearchFilters[]) => { + if (!conditions.length) { + return false + } + for (const condition of conditions) { + const matchesCondition = runQuery([docValue], { + ...condition, + allOr: true, + }) + if (matchesCondition.length) { + return true + } + } + return false + } + ) + const docMatch = (doc: Record) => { - const filterFunctions = { + const filterFunctions: Record< + SearchFilterOperator, + (doc: Record) => boolean + > = { string: stringMatch, fuzzy: fuzzyMatch, range: rangeMatch, @@ -679,6 +723,8 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { contains: contains, containsAny: containsAny, notContains: notContains, + [LogicalOperator.AND]: and, + [LogicalOperator.OR]: or, } const results = Object.entries(query || {}) diff --git a/packages/shared-core/tsconfig.build.json b/packages/shared-core/tsconfig.build.json index 13e298d71c..c1d5bc96e8 100644 --- a/packages/shared-core/tsconfig.build.json +++ b/packages/shared-core/tsconfig.build.json @@ -18,6 +18,6 @@ }, "tsBuildInfoFile": "dist/tsconfig.tsbuildinfo" }, - "include": ["src/**/*"], + "include": ["src/**/*.ts"], "exclude": ["**/*.spec.ts", "**/*.spec.js", "__mocks__", "src/tests"] } diff --git a/packages/shared-core/tsconfig.json b/packages/shared-core/tsconfig.json index d0c5134f1c..41dcee87c9 100644 --- a/packages/shared-core/tsconfig.json +++ b/packages/shared-core/tsconfig.json @@ -1,9 +1,6 @@ { "extends": "./tsconfig.build.json", "compilerOptions": { - "baseUrl": "..", - "rootDir": "src", - "composite": true, "types": ["node", "jest"] }, "exclude": ["node_modules", "dist"] diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 5607efece8..6feea40766 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -23,7 +23,22 @@ export enum RangeOperator { RANGE = "range", } -export type SearchFilterOperator = BasicOperator | ArrayOperator | RangeOperator +export enum LogicalOperator { + AND = "$and", + OR = "$or", +} + +export function isLogicalSearchOperator( + value: string +): value is LogicalOperator { + return value === LogicalOperator.AND || value === LogicalOperator.OR +} + +export type SearchFilterOperator = + | BasicOperator + | ArrayOperator + | RangeOperator + | LogicalOperator export enum InternalSearchFilterOperator { COMPLEX_ID_OPERATOR = "_complexIdOperator", @@ -75,6 +90,13 @@ export interface SearchFilters { // to make sure the documents returned are always filtered down to a // specific document type (such as just rows) documentType?: DocumentType + + [LogicalOperator.AND]?: { + conditions: SearchFilters[] + } + [LogicalOperator.OR]?: { + conditions: SearchFilters[] + } } export type SearchFilterKey = keyof Omit<