From 4799e0c2c4d10dd90d431c33a585f3a7fb704e95 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 10:29:36 +0200 Subject: [PATCH 01/33] Add extra typings --- packages/types/src/sdk/search.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 5607efece8..7f4c1ca977 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -75,6 +75,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 + + $and?: { + conditions: SearchFilters[] + } + $or?: { + conditions: SearchFilters[] + } } export type SearchFilterKey = keyof Omit< From ff0bee59748c380bcda00addfc6ebf66ecfab75c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 10:55:56 +0200 Subject: [PATCH 02/33] Add tests --- .../src/api/routes/tests/search.spec.ts | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index bc3cdccf18..401715e198 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2692,4 +2692,49 @@ describe.each([ ) }) }) + + 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" }, + ]) + }) + }) }) From 47de3f0c5348a4927b3d9d372bb445fe24157515 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 11:07:37 +0200 Subject: [PATCH 03/33] Add or tests --- .../src/api/routes/tests/search.spec.ts | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 401715e198..3a54ec9035 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2736,5 +2736,117 @@ describe.each([ { 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() + }) + }) + + 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: "Ja" } }, + ], + }, + }).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() + }) }) }) From 2e23a0e4ceb60de68bb9f054554211219299e1ba Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 12:33:44 +0200 Subject: [PATCH 04/33] Implement SQL and/or --- packages/backend-core/src/sql/sql.ts | 19 +++++++++++++++++++ .../src/api/routes/tests/search.spec.ts | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 76c86b2b62..f58ddbac6a 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -369,6 +369,25 @@ class InternalBuilder { } } + if (filters.$and) { + iterate(filters.$and, (key: string, conditions: any[]) => { + query = query.where(x => { + for (const condition of conditions) { + x = this.addFilters(x, condition, table, opts) + } + }) + }) + } + + if (filters.$or) { + const { $or } = filters + query = query.where(x => { + for (const condition of $or.conditions) { + x = this.addFilters(x, { ...condition, allOr: true }, table, opts) + } + }) + } + if (filters.oneOf) { const fnc = allOr ? "orWhereIn" : "whereIn" iterate( diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 3a54ec9035..9243ec6b6e 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2807,7 +2807,7 @@ describe.each([ $or: { conditions: [ { range: { age: { low: 1, high: 9 } } }, - { string: { name: "Ja" } }, + { string: { name: "Jan" } }, ], }, }).toContainExactly([ From 940a080e1825667dced94be466e56f47839ffbc0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 12:35:09 +0200 Subject: [PATCH 05/33] Run only for external --- .../src/api/routes/tests/search.spec.ts | 290 +++++++++--------- 1 file changed, 146 insertions(+), 144 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 9243ec6b6e..bcffc8a868 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2693,160 +2693,162 @@ describe.each([ }) }) - 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" }, - ]) + !isInternal && + 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 rows for nested filters", async () => { - await expectQuery({ - $and: { - conditions: [ - { - $and: { - conditions: [ - { - range: { age: { low: 1, high: 10 } }, - }, - { string: { name: "Ja" } }, - ], + 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" }, }, - 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() - }) - }) - - describe("$or", () => { - beforeAll(async () => { - table = await createTable({ - age: { name: "age", type: FieldType.NUMBER }, - name: { name: "name", type: FieldType.STRING }, + ], + }, + }).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() }) - 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" }, - ]) - }) + !isInternal && + 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 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 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 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 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 rows for nested filters", async () => { - await expectQuery({ - $or: { - conditions: [ - { - $or: { - conditions: [ - { - range: { age: { low: 1, high: 7 } }, - }, - { string: { name: "Jan" } }, - ], + 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" }, }, - equal: { name: "Jane" }, - }, - ], - }, - }).toContainExactly([ - { age: 1, name: "Jane" }, - { age: 7, name: "Hanna" }, - { age: 8, name: "Jan" }, - ]) - }) + ], + }, + }).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("returns nothing when filtering out all data", async () => { + await expectQuery({ + $or: { + conditions: [{ equal: { age: 6 } }, { equal: { name: "John" } }], + }, + }).toFindNothing() + }) }) }) From ebca381e9bfe74ffa1c8eea413348f06ee25644d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 15:09:33 +0200 Subject: [PATCH 06/33] Nested $and's and $or's test --- packages/backend-core/src/sql/sql.ts | 11 +++-- .../src/api/routes/tests/search.spec.ts | 43 +++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index f58ddbac6a..714d68067d 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -370,12 +370,11 @@ class InternalBuilder { } if (filters.$and) { - iterate(filters.$and, (key: string, conditions: any[]) => { - query = query.where(x => { - for (const condition of conditions) { - x = this.addFilters(x, condition, table, opts) - } - }) + const { $and } = filters + query = query.where(x => { + for (const condition of $and.conditions) { + x = this.addFilters(x, condition, table, opts) + } }) } diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index bcffc8a868..517664c5ca 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2850,5 +2850,48 @@ describe.each([ }, }).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" }]) + }) }) }) From 0b5eb9f21cac4d0cc81156462e767b1fdcb406a0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 17:19:14 +0200 Subject: [PATCH 07/33] Run tests for all sql --- packages/server/src/api/routes/tests/search.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 517664c5ca..6c114ecf5b 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2693,7 +2693,7 @@ describe.each([ }) }) - !isInternal && + isSql && describe("$and", () => { beforeAll(async () => { table = await createTable({ @@ -2767,7 +2767,7 @@ describe.each([ }) }) - !isInternal && + isSql && describe("$or", () => { beforeAll(async () => { table = await createTable({ From 1cd33472626cac98ee9c5f8d0a5b98431a4e8013 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 17:21:10 +0200 Subject: [PATCH 08/33] Types --- .../src/sdk/app/rows/search/internal/sqs.ts | 1 + packages/types/src/sdk/search.ts | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) 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 bc46e8d4c1..8cc200545a 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, + LogicalOperator, Operation, QueryJson, RelationshipFieldMetadata, diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 7f4c1ca977..11bfb12b12 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -23,7 +23,16 @@ export enum RangeOperator { RANGE = "range", } -export type SearchFilterOperator = BasicOperator | ArrayOperator | RangeOperator +export enum LogicalOperator { + AND = "$and", + OR = "$or", +} + +export type SearchFilterOperator = + | BasicOperator + | ArrayOperator + | RangeOperator + | LogicalOperator export enum InternalSearchFilterOperator { COMPLEX_ID_OPERATOR = "_complexIdOperator", @@ -76,10 +85,10 @@ export interface SearchFilters { // specific document type (such as just rows) documentType?: DocumentType - $and?: { + [LogicalOperator.AND]?: { conditions: SearchFilters[] } - $or?: { + [LogicalOperator.OR]?: { conditions: SearchFilters[] } } From c845db966f98fbe2b2c9fc3bad273a34d2cbecb2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 17:25:17 +0200 Subject: [PATCH 09/33] Fix prefixes for SQS --- .../src/sdk/app/rows/search/internal/sqs.ts | 38 +++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) 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 8cc200545a..b72e854ef2 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -135,20 +135,36 @@ 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 ( + filterKey === LogicalOperator.AND || + filterKey === LogicalOperator.OR + ) { + 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 } From 58162410eafeb0d460f15c39f258ba478faad64d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 17:32:40 +0200 Subject: [PATCH 10/33] Fix merge conflicts --- packages/backend-core/src/sql/sql.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index d9379773ff..bb3133f852 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -467,7 +467,7 @@ class InternalBuilder { const { $and } = filters query = query.where(x => { for (const condition of $and.conditions) { - x = this.addFilters(x, condition, table, opts) + x = this.addFilters(x, condition, opts) } }) } @@ -476,7 +476,7 @@ class InternalBuilder { const { $or } = filters query = query.where(x => { for (const condition of $or.conditions) { - x = this.addFilters(x, { ...condition, allOr: true }, table, opts) + x = this.addFilters(x, { ...condition, allOr: true }, opts) } }) } From 98d9f52f66cdfc44f58d84342e24a12d701d5e33 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 08:09:20 +0200 Subject: [PATCH 11/33] Fix shared-core build --- packages/shared-core/src/filters.ts | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index d30f591abc..f4e40f50cb 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -17,6 +17,7 @@ import { Table, BasicOperator, RangeOperator, + LogicalOperator, } from "@budibase/types" import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" @@ -358,6 +359,11 @@ export const buildQuery = (filter: SearchFilter[]) => { high: value, } } + } else if ( + queryOperator === LogicalOperator.AND || + queryOperator === LogicalOperator.OR + ) { + // TODO } else if (query[queryOperator] && operator !== "onEmptyFilter") { if (type === "boolean") { // Transform boolean filters to cope with null. @@ -666,8 +672,26 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) const containsAny = match(ArrayOperator.CONTAINS_ANY, _contains("some")) + const and = match( + LogicalOperator.AND, + (_docValue: Record, _testValue: any) => { + // TODO + return false + } + ) + const or = match( + LogicalOperator.AND, + (_docValue: Record, _testValue: any) => { + // TODO + return false + } + ) + const docMatch = (doc: Record) => { - const filterFunctions = { + const filterFunctions: Record< + SearchFilterOperator, + (doc: Record) => boolean + > = { string: stringMatch, fuzzy: fuzzyMatch, range: rangeMatch, @@ -679,6 +703,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 || {}) From 1e6bb7ebd760ee8e8a951392581cc2e79a4c4fd1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 08:15:30 +0200 Subject: [PATCH 12/33] Fix build on view search --- packages/server/src/api/controllers/row/views.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index 12e76155bc..ad35e86de8 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -6,6 +6,7 @@ import { RequiredKeys, RowSearchParams, SearchFilterKey, + LogicalOperator, } from "@budibase/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../../sdk" @@ -48,9 +49,17 @@ export async function searchView( // 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 ( + operator === LogicalOperator.AND || + operator === LogicalOperator.OR + ) { + // TODO + } else { + query[operator]![field] = body.query[operator]![field] + } } }) }) From b9c1aa05b078367f9acfbfbfabfe2dedeaaa7f83 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 09:10:41 +0200 Subject: [PATCH 13/33] Add todo for types --- packages/server/src/automations/steps/queryRows.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/src/automations/steps/queryRows.ts b/packages/server/src/automations/steps/queryRows.ts index 172bbf7f55..abcbd4e359 100644 --- a/packages/server/src/automations/steps/queryRows.ts +++ b/packages/server/src/automations/steps/queryRows.ts @@ -114,10 +114,12 @@ function typeCoercion(filters: SearchFilters, table: Table) { } if (column.type === FieldType.NUMBER) { if (key === "oneOf") { + // @ts-ignore TODO searchParam[property] = value .split(",") .map(item => parseFloat(item)) } else { + // @ts-ignore TODO searchParam[property] = parseFloat(value) } } From 185a3462b53c8c863caac749e03b1b01c5574edb Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 09:11:34 +0200 Subject: [PATCH 14/33] Fix tsconfig paths --- packages/shared-core/tsconfig.build.json | 2 +- packages/shared-core/tsconfig.json | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) 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"] From 3b40db5db02852e3838a3f6618746e4a7e27a725 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 10:30:19 +0200 Subject: [PATCH 15/33] Run tests for in-memory --- packages/server/src/api/routes/tests/search.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 6ed9245ee6..e3f17c36cb 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2697,7 +2697,7 @@ describe.each([ }) }) - isSql && + !isLucene && describe("$and", () => { beforeAll(async () => { table = await createTable({ @@ -2771,7 +2771,7 @@ describe.each([ }) }) - isSql && + !isLucene && describe("$or", () => { beforeAll(async () => { table = await createTable({ From 5caf20635aaedc40cd8afae7c3eaaddcce9c951d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 17:34:40 +0200 Subject: [PATCH 16/33] Implement in memory filter --- packages/shared-core/src/filters.ts | 39 +++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index f4e40f50cb..bad6d36cba 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -464,14 +464,18 @@ 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 = + type === LogicalOperator.AND || type === LogicalOperator.OR + ? 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( @@ -674,15 +678,34 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { const and = match( LogicalOperator.AND, - (_docValue: Record, _testValue: any) => { - // TODO - return false + (docValue: Record, conditions: any) => { + 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.AND, - (_docValue: Record, _testValue: any) => { - // TODO + LogicalOperator.OR, + (docValue: Record, conditions: any) => { + if (!conditions.length) { + return false + } + for (const condition of conditions) { + const matchesCondition = runQuery([docValue], { + ...condition, + allOr: true, + }) + if (matchesCondition.length) { + return true + } + } return false } ) From 0667449463161e37a81080d3a03596ffd40948ce Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 18:25:35 +0200 Subject: [PATCH 17/33] Types --- packages/shared-core/src/filters.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index bad6d36cba..e16e27fd30 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -678,7 +678,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { const and = match( LogicalOperator.AND, - (docValue: Record, conditions: any) => { + (docValue: Record, conditions: SearchFilters[]) => { if (!conditions.length) { return false } @@ -693,7 +693,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) const or = match( LogicalOperator.OR, - (docValue: Record, conditions: any) => { + (docValue: Record, conditions: SearchFilters[]) => { if (!conditions.length) { return false } From 24463bd38775951122e6b31fc95f372b57264b8e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 10:51:40 +0200 Subject: [PATCH 18/33] Add helper --- packages/types/src/sdk/search.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 11bfb12b12..6feea40766 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -28,6 +28,12 @@ export enum LogicalOperator { OR = "$or", } +export function isLogicalSearchOperator( + value: string +): value is LogicalOperator { + return value === LogicalOperator.AND || value === LogicalOperator.OR +} + export type SearchFilterOperator = | BasicOperator | ArrayOperator From c5f504d724d41c315e0ac52e1b41087d09564b3a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 10:54:19 +0200 Subject: [PATCH 19/33] Use helper --- .../server/src/sdk/app/rows/search/internal/sqs.ts | 7 ++----- packages/shared-core/src/filters.ts | 13 +++++-------- 2 files changed, 7 insertions(+), 13 deletions(-) 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 bbcbf2c7cd..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,7 +2,7 @@ import { Datasource, DocumentType, FieldType, - LogicalOperator, + isLogicalSearchOperator, Operation, QueryJson, RelationshipFieldMetadata, @@ -141,10 +141,7 @@ function cleanupFilters( const prefixFilters = (filters: SearchFilters) => { for (const filterKey of Object.keys(filters) as (keyof SearchFilters)[]) { - if ( - filterKey === LogicalOperator.AND || - filterKey === LogicalOperator.OR - ) { + if (isLogicalSearchOperator(filterKey)) { for (const condition of filters[filterKey]!.conditions) { prefixFilters(condition) } diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index e16e27fd30..e1a783175d 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -18,6 +18,7 @@ import { BasicOperator, RangeOperator, LogicalOperator, + isLogicalSearchOperator, } from "@budibase/types" import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" @@ -359,10 +360,7 @@ export const buildQuery = (filter: SearchFilter[]) => { high: value, } } - } else if ( - queryOperator === LogicalOperator.AND || - queryOperator === LogicalOperator.OR - ) { + } else if (isLogicalSearchOperator(queryOperator)) { // TODO } else if (query[queryOperator] && operator !== "onEmptyFilter") { if (type === "boolean") { @@ -464,10 +462,9 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) => (doc: Record) => { for (const [key, testValue] of Object.entries(query[type] || {})) { - const valueToCheck = - type === LogicalOperator.AND || type === LogicalOperator.OR - ? doc - : deepGet(doc, removeKeyNumbering(key)) + const valueToCheck = isLogicalSearchOperator(type) + ? doc + : deepGet(doc, removeKeyNumbering(key)) const result = test(valueToCheck, testValue) if (query.allOr && result) { return true From 8191552352875a020582f018da734eee5a55f53d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 12:36:51 +0200 Subject: [PATCH 20/33] Bypass view --- .../src/api/routes/tests/viewV2.spec.ts | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 8c0bc39234..b2ac580f6c 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1485,6 +1485,43 @@ describe.each([ } ) }) + + 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", () => { From 42b6b6e9190c8bf601b94b59ecb165d1bf44afc8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 12:37:37 +0200 Subject: [PATCH 21/33] Fix --- .../server/src/api/controllers/row/views.ts | 32 ++++--------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index ad35e86de8..643b95df6f 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -5,12 +5,10 @@ import { SearchViewRowRequest, 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 { context } from "@budibase/backend-core" import { enrichSearchContext } from "./utils" export async function searchView( @@ -36,33 +34,15 @@ 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))) { - if ( - operator === LogicalOperator.AND || - operator === LogicalOperator.OR - ) { - // TODO - } else { - query[operator]![field] = body.query[operator]![field] - } - } - }) - }) + query = { + $and: { + conditions: [query, body.query], + }, + } } await context.ensureSnippetContext(true) From 94b6737bdcbe4bb05a42629bd401e3c3d7caf7f8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 12:40:03 +0200 Subject: [PATCH 22/33] Add extra tests --- .../src/api/routes/tests/viewV2.spec.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index b2ac580f6c..6edecde266 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1486,6 +1486,39 @@ describe.each([ ) }) + 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 }), + ]) + }) + it("cannot bypass a view filter", async () => { await config.api.row.save(table._id!, { one: "foo", From 28f11a5765a3173e92cf324ad4cc1fcbc6f61a1d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 13:15:30 +0200 Subject: [PATCH 23/33] Don't add breaking changes --- .../server/src/api/controllers/row/views.ts | 31 ++++++++++--- .../src/api/routes/tests/viewV2.spec.ts | 43 +++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index 643b95df6f..c38a415aa2 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -5,11 +5,14 @@ import { SearchViewRowRequest, RequiredKeys, RowSearchParams, + SearchFilterKey, + LogicalOperator, } from "@budibase/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../../sdk" -import { context } from "@budibase/backend-core" +import { db, context } from "@budibase/backend-core" import { enrichSearchContext } from "./utils" +import { isExternalTableID } from "../../../integrations/utils" export async function searchView( ctx: UserCtx @@ -38,10 +41,28 @@ export async function searchView( delete body.query.allOr delete body.query.onEmptyFilter - query = { - $and: { - conditions: [query, body.query], - }, + 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], + }, + } } } diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 6edecde266..02dbb70666 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1486,6 +1486,48 @@ 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", @@ -1519,6 +1561,7 @@ describe.each([ ]) }) + !isLucene && it("cannot bypass a view filter", async () => { await config.api.row.save(table._id!, { one: "foo", From 00cf4e48e3247cea8b68b73ca2e6ae7a49bcdc7e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 13:21:28 +0200 Subject: [PATCH 24/33] Lint --- .../src/api/routes/tests/viewV2.spec.ts | 120 +++++++++--------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 02dbb70666..ea6aedbe3c 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1528,76 +1528,76 @@ describe.each([ }) !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", - }) + 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", + 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 }), + ]) }) - 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", - }) + 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", + 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 }, }, - ], - schema: { - id: { visible: true }, - one: { visible: false }, - two: { visible: true }, - }, - }) + }) - const response = await config.api.viewV2.search(view.id, { - query: { - equal: { - two: "bar", + const response = await config.api.viewV2.search(view.id, { + query: { + equal: { + two: "bar", + }, }, - }, + }) + expect(response.rows).toHaveLength(0) }) - expect(response.rows).toHaveLength(0) - }) }) describe("permissions", () => { From d6e1bcb3829fafba06a95b9f66230021524363f0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 13:56:25 +0200 Subject: [PATCH 25/33] Type search validators --- packages/server/src/api/routes/utils/validators.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 671ce95038..0322120ecf 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -1,6 +1,6 @@ import { auth, permissions } from "@budibase/backend-core" import { DataSourceOperation } from "../../../constants" -import { Table, WebhookActionType } from "@budibase/types" +import { SearchFilters, Table, WebhookActionType } from "@budibase/types" import Joi, { CustomValidator } from "joi" import { ValidSnippetNameRegex, helpers } from "@budibase/shared-core" import sdk from "../../../sdk" @@ -84,7 +84,7 @@ export function datasourceValidator() { } function filterObject() { - return Joi.object({ + const filtersValidators: Record = { string: Joi.object().optional(), fuzzy: Joi.object().optional(), range: Joi.object().optional(), @@ -96,7 +96,8 @@ function filterObject() { contains: Joi.object().optional(), notContains: Joi.object().optional(), allOr: Joi.boolean().optional(), - }).unknown(true) + } + return Joi.object(filtersValidators).unknown(true) } export function internalSearchValidator() { From 8d1c658c7c744f0a3a7d984600b8830c91d6075d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 13:57:34 +0200 Subject: [PATCH 26/33] Add containsAny validator --- packages/server/src/api/routes/utils/validators.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 0322120ecf..9b9a763e2c 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -95,6 +95,7 @@ function filterObject() { oneOf: Joi.object().optional(), contains: Joi.object().optional(), notContains: Joi.object().optional(), + containsAny: Joi.object().optional(), allOr: Joi.boolean().optional(), } return Joi.object(filtersValidators).unknown(true) From c6f7f0133dcb52aa77ee0e5c906a5ce2d0b19695 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 13:57:54 +0200 Subject: [PATCH 27/33] Disallow fuzzyOr and documentType --- packages/server/src/api/routes/utils/validators.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 9b9a763e2c..ddd98f3858 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -97,6 +97,8 @@ function filterObject() { notContains: Joi.object().optional(), containsAny: Joi.object().optional(), allOr: Joi.boolean().optional(), + fuzzyOr: Joi.disallow(), + documentType: Joi.disallow(), } return Joi.object(filtersValidators).unknown(true) } From 22d9b930fc280c7ca129f05aae1d5b6232ee1f8b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 14:02:09 +0200 Subject: [PATCH 28/33] Validate onEmptyFilter --- packages/server/src/api/routes/utils/validators.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index ddd98f3858..72c97396db 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 { SearchFilters, 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" @@ -97,6 +102,9 @@ function filterObject() { notContains: Joi.object().optional(), containsAny: Joi.object().optional(), allOr: Joi.boolean().optional(), + onEmptyFilter: Joi.string() + .optional() + .valid(...Object.values(EmptyFilterOption)), fuzzyOr: Joi.disallow(), documentType: Joi.disallow(), } From 00c12b9686dd07d6a466d099ae6b90ccc49e62a8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 14:32:42 +0200 Subject: [PATCH 29/33] Add tests --- .../src/api/routes/tests/search.spec.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index e3f17c36cb..05f9e1676e 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2769,6 +2769,37 @@ describe.each([ }, }).toFindNothing() }) + + 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' + ) + }) + + 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 && From bc7ab264b0500410f18a93f2a9e0bddd0893f125 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 14:32:59 +0200 Subject: [PATCH 30/33] Add validations --- packages/server/src/api/routes/utils/validators.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 72c97396db..9aa112cf4d 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -89,6 +89,11 @@ export function datasourceValidator() { } function filterObject() { + const conditionalFilteringObject = () => + Joi.object({ + conditions: Joi.array().items(Joi.link("#schema")).required(), + }) + const filtersValidators: Record = { string: Joi.object().optional(), fuzzy: Joi.object().optional(), @@ -105,10 +110,12 @@ function filterObject() { onEmptyFilter: Joi.string() .optional() .valid(...Object.values(EmptyFilterOption)), - fuzzyOr: Joi.disallow(), - documentType: Joi.disallow(), + $and: conditionalFilteringObject(), + $or: conditionalFilteringObject(), + fuzzyOr: Joi.forbidden(), + documentType: Joi.forbidden(), } - return Joi.object(filtersValidators).unknown(true) + return Joi.object(filtersValidators).unknown(true).id("schema") } export function internalSearchValidator() { From 32702f2e9db4f7b53df67b9775a6733eee2a1353 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 14:39:05 +0200 Subject: [PATCH 31/33] Don't validate for in-memory --- packages/server/src/api/routes/tests/search.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 05f9e1676e..a92e2d3164 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2770,6 +2770,7 @@ describe.each([ }).toFindNothing() }) + !isInMemory && it("validates conditions that are not objects", async () => { await expect( expectQuery({ @@ -2782,6 +2783,7 @@ describe.each([ ) }) + !isInMemory && it("validates $and without conditions", async () => { await expect( expectQuery({ From 44a053ee0812e974d10aea4d7fd844aace9529bb Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 14:40:23 +0200 Subject: [PATCH 32/33] Lint --- .../src/api/routes/tests/search.spec.ts | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index a92e2d3164..9f0a9e6269 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2771,37 +2771,37 @@ describe.each([ }) !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' - ) - }) + 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, + 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' - ) - }) + ], + }, + }).toFindNothing() + ).rejects.toThrow( + 'Invalid body - "query.$and.conditions[1].$and.conditions" is required' + ) + }) }) !isLucene && From 80ff96308241d330fb24e9a997a8823a5dd50c61 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 17:01:59 +0200 Subject: [PATCH 33/33] Remove unnecessary coercion --- .../server/src/automations/steps/queryRows.ts | 41 +------------------ 1 file changed, 2 insertions(+), 39 deletions(-) diff --git a/packages/server/src/automations/steps/queryRows.ts b/packages/server/src/automations/steps/queryRows.ts index abcbd4e359..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,40 +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") { - // @ts-ignore TODO - searchParam[property] = value - .split(",") - .map(item => parseFloat(item)) - } else { - // @ts-ignore TODO - searchParam[property] = parseFloat(value) - } - } - } - } - } - return filters -} - function hasNullFilters(filters: any[]) { return ( filters.length === 0 || @@ -159,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, }, @@ -167,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, },