diff --git a/lerna.json b/lerna.json index db0a1d59fa..9488ebe3a0 100644 --- a/lerna.json +++ b/lerna.json @@ -22,4 +22,4 @@ "loadEnvFiles": false } } -} \ No newline at end of file +} diff --git a/packages/server/src/api/controllers/view/exporters.ts b/packages/server/src/api/controllers/view/exporters.ts index 3b5f951dca..9cf114f4e5 100644 --- a/packages/server/src/api/controllers/view/exporters.ts +++ b/packages/server/src/api/controllers/view/exporters.ts @@ -57,5 +57,5 @@ export function isFormat(format: any): format is Format { } export function parseCsvExport(value: string) { - return JSON.parse(value?.replace(/'/g, '"')) as T + return JSON.parse(value) as T } diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 5de788dc9c..aac43874a0 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -1,6 +1,6 @@ import { tableForDatasource } from "../../../tests/utilities/structures" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" -import { db as dbCore } from "@budibase/backend-core" +import { db as dbCore, utils } from "@budibase/backend-core" import * as setup from "./utilities" import { @@ -87,21 +87,67 @@ describe.each([ class SearchAssertion { constructor(private readonly query: RowSearchParams) {} - private popRow(expectedRow: any, foundRows: any[]) { - const row = foundRows.find(foundRow => _.isMatch(foundRow, expectedRow)) + // We originally used _.isMatch to compare rows, but found that when + // comparing arrays it would return true if the source array was a subset of + // the target array. This would sometimes create false matches. This + // function is a more strict version of _.isMatch that only returns true if + // the source array is an exact match of the target. + // + // _.isMatch("100", "1") also returns true which is not what we want. + private isMatch>(expected: T, found: T) { + if (!expected) { + throw new Error("Expected is undefined") + } + if (!found) { + return false + } + + for (const key of Object.keys(expected)) { + if (Array.isArray(expected[key])) { + if (!Array.isArray(found[key])) { + return false + } + if (expected[key].length !== found[key].length) { + return false + } + if (!_.isMatch(found[key], expected[key])) { + return false + } + } else if (typeof expected[key] === "object") { + if (!this.isMatch(expected[key], found[key])) { + return false + } + } else { + if (expected[key] !== found[key]) { + return false + } + } + } + return true + } + + // This function exists to ensure that the same row is not matched twice. + // When a row gets matched, we make sure to remove it from the list of rows + // we're matching against. + private popRow( + expectedRow: T, + foundRows: T[] + ): NonNullable { + const row = foundRows.find(row => this.isMatch(expectedRow, row)) if (!row) { const fields = Object.keys(expectedRow) // To make the error message more readable, we only include the fields // that are present in the expected row. const searchedObjects = foundRows.map(row => _.pick(row, fields)) throw new Error( - `Failed to find row: ${JSON.stringify( - expectedRow - )} in ${JSON.stringify(searchedObjects)}` + `Failed to find row:\n\n${JSON.stringify( + expectedRow, + null, + 2 + )}\n\nin\n\n${JSON.stringify(searchedObjects, null, 2)}` ) } - // Ensuring the same row is not matched twice foundRows.splice(foundRows.indexOf(row), 1) return row } @@ -1055,6 +1101,7 @@ describe.each([ describe("notEqual", () => { it("successfully finds a row", () => expectQuery({ notEqual: { time: T_1000 } }).toContainExactly([ + { timeid: NULL_TIME__ID }, { time: "10:45:00" }, { time: "12:00:00" }, { time: "15:30:00" }, @@ -1064,6 +1111,7 @@ describe.each([ it("return all when requesting non-existing", () => expectQuery({ notEqual: { time: UNEXISTING_TIME } }).toContainExactly( [ + { timeid: NULL_TIME__ID }, { time: "10:00:00" }, { time: "10:45:00" }, { time: "12:00:00" }, @@ -1530,14 +1578,169 @@ describe.each([ await createRows([{ "1:name": "bar" }, { "1:name": "foo" }]) }) + it("successfully finds a row", () => + expectQuery({ equal: { "1:1:name": "bar" } }).toContainExactly([ + { "1:name": "bar" }, + ])) + + it("fails to find nonexistent row", () => + expectQuery({ equal: { "1:1:name": "none" } }).toFindNothing()) + }) + + describe("user", () => { + let user1: User + let user2: User + + beforeAll(async () => { + user1 = await config.createUser({ _id: `us_${utils.newid()}` }) + user2 = await config.createUser({ _id: `us_${utils.newid()}` }) + + table = await createTable({ + user: { + name: "user", + type: FieldType.BB_REFERENCE_SINGLE, + subtype: BBReferenceFieldSubType.USER, + }, + }) + + await createRows([ + { user: JSON.stringify(user1) }, + { user: JSON.stringify(user2) }, + { user: null }, + ]) + }) + describe("equal", () => { it("successfully finds a row", () => - expectQuery({ equal: { "1:1:name": "bar" } }).toContainExactly([ - { "1:name": "bar" }, + expectQuery({ equal: { user: user1._id } }).toContainExactly([ + { user: { _id: user1._id } }, ])) it("fails to find nonexistent row", () => - expectQuery({ equal: { "1:1:name": "none" } }).toFindNothing()) + expectQuery({ equal: { user: "us_none" } }).toFindNothing()) + }) + + describe("notEqual", () => { + it("successfully finds a row", () => + expectQuery({ notEqual: { user: user1._id } }).toContainExactly([ + { user: { _id: user2._id } }, + {}, + ])) + + it("fails to find nonexistent row", () => + expectQuery({ notEqual: { user: "us_none" } }).toContainExactly([ + { user: { _id: user1._id } }, + { user: { _id: user2._id } }, + {}, + ])) + }) + + describe("oneOf", () => { + it("successfully finds a row", () => + expectQuery({ oneOf: { user: [user1._id] } }).toContainExactly([ + { user: { _id: user1._id } }, + ])) + + it("fails to find nonexistent row", () => + expectQuery({ oneOf: { user: ["us_none"] } }).toFindNothing()) + }) + + describe("empty", () => { + it("finds empty rows", () => + expectQuery({ empty: { user: null } }).toContainExactly([{}])) + }) + + describe("notEmpty", () => { + it("finds non-empty rows", () => + expectQuery({ notEmpty: { user: null } }).toContainExactly([ + { user: { _id: user1._id } }, + { user: { _id: user2._id } }, + ])) + }) + }) + + describe("multi user", () => { + let user1: User + let user2: User + + beforeAll(async () => { + user1 = await config.createUser({ _id: `us_${utils.newid()}` }) + user2 = await config.createUser({ _id: `us_${utils.newid()}` }) + + table = await createTable({ + users: { + name: "users", + type: FieldType.BB_REFERENCE, + subtype: BBReferenceFieldSubType.USER, + constraints: { type: "array" }, + }, + number: { + name: "number", + type: FieldType.NUMBER, + }, + }) + + await createRows([ + { number: 1, users: JSON.stringify([user1]) }, + { number: 2, users: JSON.stringify([user2]) }, + { number: 3, users: JSON.stringify([user1, user2]) }, + { number: 4, users: JSON.stringify([]) }, + ]) + }) + + describe("contains", () => { + it("successfully finds a row", () => + expectQuery({ contains: { users: [user1._id] } }).toContainExactly([ + { users: [{ _id: user1._id }] }, + { users: [{ _id: user1._id }, { _id: user2._id }] }, + ])) + + it("fails to find nonexistent row", () => + expectQuery({ contains: { users: ["us_none"] } }).toFindNothing()) + }) + + describe("notContains", () => { + it("successfully finds a row", () => + expectQuery({ notContains: { users: [user1._id] } }).toContainExactly([ + { users: [{ _id: user2._id }] }, + {}, + ])) + + it("fails to find nonexistent row", () => + expectQuery({ notContains: { users: ["us_none"] } }).toContainExactly([ + { users: [{ _id: user1._id }] }, + { users: [{ _id: user2._id }] }, + { users: [{ _id: user1._id }, { _id: user2._id }] }, + {}, + ])) + }) + + describe("containsAny", () => { + it("successfully finds rows", () => + expectQuery({ + containsAny: { users: [user1._id, user2._id] }, + }).toContainExactly([ + { users: [{ _id: user1._id }] }, + { users: [{ _id: user2._id }] }, + { users: [{ _id: user1._id }, { _id: user2._id }] }, + ])) + + it("fails to find nonexistent row", () => + expectQuery({ containsAny: { users: ["us_none"] } }).toFindNothing()) + }) + + describe("multi-column equals", () => { + it("successfully finds a row", () => + expectQuery({ + equal: { number: 1 }, + contains: { users: [user1._id] }, + }).toContainExactly([{ users: [{ _id: user1._id }], number: 1 }])) + + it("fails to find nonexistent row", () => + expectQuery({ + equal: { number: 2 }, + contains: { users: [user1._id] }, + }).toFindNothing()) }) }) diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index c3dca2a39c..43e5961947 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -231,8 +231,7 @@ class InternalBuilder { } const contains = (mode: object, any: boolean = false) => { - const fnc = allOr ? "orWhere" : "where" - const rawFnc = `${fnc}Raw` + const rawFnc = allOr ? "orWhereRaw" : "whereRaw" const not = mode === filters?.notContains ? "NOT " : "" function stringifyArray(value: Array, quoteStyle = '"'): string { for (let i in value) { @@ -245,24 +244,24 @@ class InternalBuilder { if (this.client === SqlClient.POSTGRES) { iterate(mode, (key: string, value: Array) => { const wrap = any ? "" : "'" - const containsOp = any ? "\\?| array" : "@>" + const op = any ? "\\?| array" : "@>" const fieldNames = key.split(/\./g) - const tableName = fieldNames[0] - const columnName = fieldNames[1] - // @ts-ignore + const table = fieldNames[0] + const col = fieldNames[1] query = query[rawFnc]( - `${not}"${tableName}"."${columnName}"::jsonb ${containsOp} ${wrap}${stringifyArray( + `${not}COALESCE("${table}"."${col}"::jsonb ${op} ${wrap}${stringifyArray( value, any ? "'" : '"' - )}${wrap}` + )}${wrap}, FALSE)` ) }) } else if (this.client === SqlClient.MY_SQL) { const jsonFnc = any ? "JSON_OVERLAPS" : "JSON_CONTAINS" iterate(mode, (key: string, value: Array) => { - // @ts-ignore query = query[rawFnc]( - `${not}${jsonFnc}(${key}, '${stringifyArray(value)}')` + `${not}COALESCE(${jsonFnc}(${key}, '${stringifyArray( + value + )}'), FALSE)` ) }) } else { @@ -277,7 +276,7 @@ class InternalBuilder { } statement += (statement ? andOr : "") + - `LOWER(${likeKey(this.client, key)}) LIKE ?` + `COALESCE(LOWER(${likeKey(this.client, key)}), '') LIKE ?` } if (statement === "") { @@ -342,14 +341,34 @@ class InternalBuilder { } if (filters.equal) { iterate(filters.equal, (key, value) => { - const fnc = allOr ? "orWhere" : "where" - query = query[fnc]({ [key]: value }) + const fnc = allOr ? "orWhereRaw" : "whereRaw" + if (this.client === SqlClient.MS_SQL) { + query = query[fnc]( + `CASE WHEN ${likeKey(this.client, key)} = ? THEN 1 ELSE 0 END = 1`, + [value] + ) + } else { + query = query[fnc]( + `COALESCE(${likeKey(this.client, key)} = ?, FALSE)`, + [value] + ) + } }) } if (filters.notEqual) { iterate(filters.notEqual, (key, value) => { - const fnc = allOr ? "orWhereNot" : "whereNot" - query = query[fnc]({ [key]: value }) + const fnc = allOr ? "orWhereRaw" : "whereRaw" + if (this.client === SqlClient.MS_SQL) { + query = query[fnc]( + `CASE WHEN ${likeKey(this.client, key)} = ? THEN 1 ELSE 0 END = 0`, + [value] + ) + } else { + query = query[fnc]( + `COALESCE(${likeKey(this.client, key)} != ?, TRUE)`, + [value] + ) + } }) } if (filters.empty) { diff --git a/packages/server/src/integrations/tests/sql.spec.ts b/packages/server/src/integrations/tests/sql.spec.ts index 9b84409e92..302b61fc74 100644 --- a/packages/server/src/integrations/tests/sql.spec.ts +++ b/packages/server/src/integrations/tests/sql.spec.ts @@ -189,7 +189,7 @@ describe("SQL query builder", () => { ) expect(query).toEqual({ bindings: ["%20%", "%25%", `%"john"%`, `%"mary"%`, limit], - sql: `select * from (select * from (select * from "test" where (LOWER("test"."age") LIKE :1 AND LOWER("test"."age") LIKE :2) and (LOWER("test"."name") LIKE :3 AND LOWER("test"."name") LIKE :4)) where rownum <= :5) "test"`, + sql: `select * from (select * from (select * from "test" where (COALESCE(LOWER("test"."age"), '') LIKE :1 AND COALESCE(LOWER("test"."age"), '') LIKE :2) and (COALESCE(LOWER("test"."name"), '') LIKE :3 AND COALESCE(LOWER("test"."name"), '') LIKE :4)) where rownum <= :5) "test"`, }) query = new Sql(SqlClient.ORACLE, limit)._query( diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 0de4d0a151..f907e1ab8e 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -77,7 +77,7 @@ describe("Captures of real examples", () => { "b"."completed" as "b.completed", "b"."qaid" as "b.qaid" from (select * from "products" as "a" order by "a"."productname" asc nulls first limit $1) as "a" left join "products_tasks" as "c" on "a"."productid" = "c"."productid" - left join "tasks" as "b" on "b"."taskid" = "c"."taskid" where "b"."taskname" = $2 + left join "tasks" as "b" on "b"."taskid" = "c"."taskid" where COALESCE("b"."taskname" = $2, FALSE) order by "a"."productname" asc nulls first limit $3`), }) }) @@ -137,12 +137,12 @@ describe("Captures of real examples", () => { "c"."city" as "c.city", "c"."lastname" as "c.lastname", "c"."year" as "c.year", "c"."firstname" as "c.firstname", "c"."personid" as "c.personid", "c"."address" as "c.address", "c"."age" as "c.age", "c"."type" as "c.type", "c"."city" as "c.city", "c"."lastname" as "c.lastname" - from (select * from "tasks" as "a" where not "a"."completed" = $1 + from (select * from "tasks" as "a" where COALESCE("a"."completed" != $1, TRUE) order by "a"."taskname" asc nulls first limit $2) as "a" left join "products_tasks" as "d" on "a"."taskid" = "d"."taskid" left join "products" as "b" on "b"."productid" = "d"."productid" left join "persons" as "c" on "a"."executorid" = "c"."personid" or "a"."qaid" = "c"."personid" - where "c"."year" between $3 and $4 and "b"."productname" = $5 order by "a"."taskname" asc nulls first limit $6`), + where "c"."year" between $3 and $4 and COALESCE("b"."productname" = $5, FALSE) order by "a"."taskname" asc nulls first limit $6`), }) }) }) @@ -154,7 +154,7 @@ describe("Captures of real examples", () => { expect(query).toEqual({ bindings: [1990, "C", "A Street", 34, "designer", "London", "B", 5], sql: multiline(`update "persons" as "a" set "year" = $1, "firstname" = $2, "address" = $3, "age" = $4, - "type" = $5, "city" = $6, "lastname" = $7 where "a"."personid" = $8 returning *`), + "type" = $5, "city" = $6, "lastname" = $7 where COALESCE("a"."personid" = $8, FALSE) returning *`), }) }) @@ -164,7 +164,7 @@ describe("Captures of real examples", () => { expect(query).toEqual({ bindings: [1990, "C", "A Street", 34, "designer", "London", "B", 5], sql: multiline(`update "persons" as "a" set "year" = $1, "firstname" = $2, "address" = $3, "age" = $4, - "type" = $5, "city" = $6, "lastname" = $7 where "a"."personid" = $8 returning *`), + "type" = $5, "city" = $6, "lastname" = $7 where COALESCE("a"."personid" = $8, FALSE) returning *`), }) }) }) @@ -175,8 +175,9 @@ describe("Captures of real examples", () => { let query = new Sql(SqlClient.POSTGRES, limit)._query(queryJson) expect(query).toEqual({ bindings: ["ddd", ""], - sql: multiline(`delete from "compositetable" as "a" where "a"."keypartone" = $1 and "a"."keyparttwo" = $2 - returning "a"."keyparttwo" as "a.keyparttwo", "a"."keypartone" as "a.keypartone", "a"."name" as "a.name"`), + sql: multiline(`delete from "compositetable" as "a" + where COALESCE("a"."keypartone" = $1, FALSE) and COALESCE("a"."keyparttwo" = $2, FALSE) + returning "a"."keyparttwo" as "a.keyparttwo", "a"."keypartone" as "a.keypartone", "a"."name" as "a.name"`), }) }) }) @@ -197,7 +198,7 @@ describe("Captures of real examples", () => { returningQuery = input }, queryJson) expect(returningQuery).toEqual({ - sql: "select * from (select top (@p0) * from [people] where [people].[name] = @p1 and [people].[age] = @p2 order by [people].[name] asc) as [people]", + sql: "select * from (select top (@p0) * from [people] where CASE WHEN [people].[name] = @p1 THEN 1 ELSE 0 END = 1 and CASE WHEN [people].[age] = @p2 THEN 1 ELSE 0 END = 1 order by [people].[name] asc) as [people]", bindings: [1, "Test", 22], }) }) diff --git a/packages/server/src/sdk/app/rows/search/sqs.ts b/packages/server/src/sdk/app/rows/search/sqs.ts index ec3ac08c2e..7e5ac94a66 100644 --- a/packages/server/src/sdk/app/rows/search/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/sqs.ts @@ -171,7 +171,8 @@ export async function search( sql = sql.replace(/`doc2`.`rowId`/g, "`doc2.rowId`") const db = context.getAppDB() - return await db.sql(sql, bindings) + const rows = await db.sql(sql, bindings) + return rows }) // process from the format of tableId.column to expected format