From 76d0107d4da9ae180b3b9f030330cbe82a0d3a57 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 10:10:15 +0200 Subject: [PATCH 01/41] Handle empty relationships --- packages/backend-core/src/sql/sql.ts | 81 +++++++++++++++++++++------- 1 file changed, 61 insertions(+), 20 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index b415a6f1b7..b35b2b5ec8 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -23,12 +23,14 @@ import { InternalSearchFilterOperator, JsonFieldMetadata, JsonTypes, + LogicalOperator, Operation, prefixed, QueryJson, QueryOptions, RangeOperator, RelationshipsJson, + SearchFilterKey, SearchFilters, SortOrder, SqlClient, @@ -96,6 +98,22 @@ function isSqs(table: Table): boolean { ) } +const allowEmptyRelationships: Record = { + [BasicOperator.EQUAL]: false, + [BasicOperator.NOT_EQUAL]: true, + [BasicOperator.EMPTY]: false, + [BasicOperator.NOT_EMPTY]: true, + [BasicOperator.FUZZY]: false, + [BasicOperator.STRING]: false, + [RangeOperator.RANGE]: false, + [ArrayOperator.CONTAINS]: false, + [ArrayOperator.NOT_CONTAINS]: true, + [ArrayOperator.CONTAINS_ANY]: false, + [ArrayOperator.ONE_OF]: false, + [LogicalOperator.AND]: false, + [LogicalOperator.OR]: false, +} + class InternalBuilder { private readonly client: SqlClient private readonly query: QueryJson @@ -405,6 +423,7 @@ class InternalBuilder { addRelationshipForFilter( query: Knex.QueryBuilder, + allowEmptyRelationships: boolean, filterKey: string, whereCb: (query: Knex.QueryBuilder) => Knex.QueryBuilder ): Knex.QueryBuilder { @@ -426,9 +445,10 @@ class InternalBuilder { relationship.to && relationship.tableName ) { - let subQuery = mainKnex + const joinTable = mainKnex .select(mainKnex.raw(1)) .from({ [toAlias]: relatedTableName }) + let subQuery = joinTable.clone() const manyToMany = validateManyToMany(relationship) if (manyToMany) { const throughAlias = @@ -440,7 +460,6 @@ class InternalBuilder { subQuery = subQuery // add a join through the junction table .innerJoin(throughTable, function () { - // @ts-ignore this.on( `${toAlias}.${manyToMany.toPrimary}`, "=", @@ -460,18 +479,33 @@ class InternalBuilder { if (this.client === SqlClient.SQL_LITE) { subQuery = this.addJoinFieldCheck(subQuery, manyToMany) } + + query = query.whereExists(whereCb(subQuery)) + if (allowEmptyRelationships) { + query = query.orWhereNotExists( + joinTable.clone().innerJoin(throughTable, function () { + this.on( + `${fromAlias}.${manyToMany.fromPrimary}`, + "=", + `${throughAlias}.${manyToMany.from}` + ) + }) + ) + } } else { + const foreignKey = `${fromAlias}.${relationship.from}` // "join" to the main table, making sure the ID matches that of the main subQuery = subQuery.where( `${toAlias}.${relationship.to}`, "=", - mainKnex.raw( - this.quotedIdentifier(`${fromAlias}.${relationship.from}`) - ) + mainKnex.raw(this.quotedIdentifier(foreignKey)) ) + + query = query.whereExists(whereCb(subQuery)) + if (allowEmptyRelationships) { + query = query.orWhereNull(foreignKey) + } } - query = query.whereExists(whereCb(subQuery)) - break } } return query @@ -502,6 +536,7 @@ class InternalBuilder { } function iterate( structure: AnySearchFilter, + operation: SearchFilterKey, fn: ( query: Knex.QueryBuilder, key: string, @@ -558,9 +593,14 @@ class InternalBuilder { if (allOr) { query = query.or } - query = builder.addRelationshipForFilter(query, updatedKey, q => { - return handleRelationship(q, updatedKey, value) - }) + query = builder.addRelationshipForFilter( + query, + allowEmptyRelationships[operation], + updatedKey, + q => { + return handleRelationship(q, updatedKey, value) + } + ) } } } @@ -592,7 +632,7 @@ class InternalBuilder { return `[${value.join(",")}]` } if (this.client === SqlClient.POSTGRES) { - iterate(mode, (q, key, value) => { + iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => { const wrap = any ? "" : "'" const op = any ? "\\?| array" : "@>" const fieldNames = key.split(/\./g) @@ -610,7 +650,7 @@ class InternalBuilder { this.client === SqlClient.MARIADB ) { const jsonFnc = any ? "JSON_OVERLAPS" : "JSON_CONTAINS" - iterate(mode, (q, key, value) => { + iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => { return q[rawFnc]( `${not}COALESCE(${jsonFnc}(${key}, '${stringifyArray( value @@ -619,7 +659,7 @@ class InternalBuilder { }) } else { const andOr = mode === filters?.containsAny ? " OR " : " AND " - iterate(mode, (q, key, value) => { + iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => { let statement = "" const identifier = this.quotedIdentifier(key) for (let i in value) { @@ -673,6 +713,7 @@ class InternalBuilder { const fnc = allOr ? "orWhereIn" : "whereIn" iterate( filters.oneOf, + ArrayOperator.ONE_OF, (q, key: string, array) => { if (this.client === SqlClient.ORACLE) { key = this.convertClobs(key) @@ -697,7 +738,7 @@ class InternalBuilder { ) } if (filters.string) { - iterate(filters.string, (q, key, value) => { + iterate(filters.string, BasicOperator.STRING, (q, key, value) => { const fnc = allOr ? "orWhere" : "where" // postgres supports ilike, nothing else does if (this.client === SqlClient.POSTGRES) { @@ -712,10 +753,10 @@ class InternalBuilder { }) } if (filters.fuzzy) { - iterate(filters.fuzzy, like) + iterate(filters.fuzzy, BasicOperator.FUZZY, like) } if (filters.range) { - iterate(filters.range, (q, key, value) => { + iterate(filters.range, RangeOperator.RANGE, (q, key, value) => { const isEmptyObject = (val: any) => { return ( val && @@ -781,7 +822,7 @@ class InternalBuilder { }) } if (filters.equal) { - iterate(filters.equal, (q, key, value) => { + iterate(filters.equal, BasicOperator.EQUAL, (q, key, value) => { const fnc = allOr ? "orWhereRaw" : "whereRaw" if (this.client === SqlClient.MS_SQL) { return q[fnc]( @@ -801,7 +842,7 @@ class InternalBuilder { }) } if (filters.notEqual) { - iterate(filters.notEqual, (q, key, value) => { + iterate(filters.notEqual, BasicOperator.NOT_EQUAL, (q, key, value) => { const fnc = allOr ? "orWhereRaw" : "whereRaw" if (this.client === SqlClient.MS_SQL) { return q[fnc]( @@ -822,13 +863,13 @@ class InternalBuilder { }) } if (filters.empty) { - iterate(filters.empty, (q, key) => { + iterate(filters.empty, BasicOperator.EMPTY, (q, key) => { const fnc = allOr ? "orWhereNull" : "whereNull" return q[fnc](key) }) } if (filters.notEmpty) { - iterate(filters.notEmpty, (q, key) => { + iterate(filters.notEmpty, BasicOperator.NOT_EMPTY, (q, key) => { const fnc = allOr ? "orWhereNotNull" : "whereNotNull" return q[fnc](key) }) From 57da952f6944b5874fe20e4539ce33b5ac2c3055 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 11:34:35 +0200 Subject: [PATCH 02/41] Fix "parenthesis" --- packages/backend-core/src/sql/sql.ts | 30 +++++++++++++--------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index b35b2b5ec8..3e6032fc99 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -480,18 +480,20 @@ class InternalBuilder { subQuery = this.addJoinFieldCheck(subQuery, manyToMany) } - query = query.whereExists(whereCb(subQuery)) - if (allowEmptyRelationships) { - query = query.orWhereNotExists( - joinTable.clone().innerJoin(throughTable, function () { - this.on( - `${fromAlias}.${manyToMany.fromPrimary}`, - "=", - `${throughAlias}.${manyToMany.from}` - ) - }) - ) - } + query = query.where(q => { + q.whereExists(whereCb(subQuery)) + if (allowEmptyRelationships) { + q.orWhereNotExists( + joinTable.clone().innerJoin(throughTable, function () { + this.on( + `${fromAlias}.${manyToMany.fromPrimary}`, + "=", + `${throughAlias}.${manyToMany.from}` + ) + }) + ) + } + }) } else { const foreignKey = `${fromAlias}.${relationship.from}` // "join" to the main table, making sure the ID matches that of the main @@ -1265,12 +1267,10 @@ class InternalBuilder { }) : undefined if (!throughTable) { - // @ts-ignore query = query.leftJoin(toTableWithSchema, function () { for (let relationship of columns) { const from = relationship.from, to = relationship.to - // @ts-ignore this.orOn(`${fromAlias}.${from}`, "=", `${toAlias}.${to}`) } }) @@ -1281,7 +1281,6 @@ class InternalBuilder { for (let relationship of columns) { const fromPrimary = relationship.fromPrimary const from = relationship.from - // @ts-ignore this.orOn( `${fromAlias}.${fromPrimary}`, "=", @@ -1293,7 +1292,6 @@ class InternalBuilder { for (let relationship of columns) { const toPrimary = relationship.toPrimary const to = relationship.to - // @ts-ignore this.orOn(`${toAlias}.${toPrimary}`, `${throughAlias}.${to}`) } }) From 5782b20509e3f3ea209e692d15ce2446940d7bef Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 11:36:29 +0200 Subject: [PATCH 03/41] Update test --- packages/server/src/api/routes/tests/search.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 3ab35c9294..23fb5a056f 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2458,7 +2458,7 @@ describe.each([ }).toContainExactly([ { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, { name: "bar", productCat: [{ _id: productCatRows[1]._id }] }, - // { name: "baz", productCat: undefined }, // TODO + { name: "baz", productCat: undefined }, ]) }) @@ -2506,7 +2506,7 @@ describe.each([ }).toContainExactly([ { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, { name: "bar", productCat: [{ _id: productCatRows[1]._id }] }, - // { name: "baz", productCat: undefined }, // TODO + { name: "baz", productCat: undefined }, ]) } ) @@ -2530,7 +2530,7 @@ describe.each([ }).toContainExactly([ { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, { name: "bar", productCat: [{ _id: productCatRows[1]._id }] }, - // { name: "baz", productCat: undefined }, // TODO + { name: "baz", productCat: undefined }, ]) }) }) From 975945a23a0b9d30a2b448fc9af73ec75f1f56ae Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 12:03:23 +0200 Subject: [PATCH 04/41] Improve tests --- .../src/api/routes/tests/search.spec.ts | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 23fb5a056f..798157b122 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2278,12 +2278,16 @@ describe.each([ // It also can't work for in-memory searching because the related table name // isn't available. !isInMemory && - describe("relations", () => { + describe.each([ + RelationshipType.ONE_TO_MANY, + RelationshipType.MANY_TO_ONE, + RelationshipType.MANY_TO_MANY, + ])("relations", relationshipType => { let productCategoryTable: Table, productCatRows: Row[] beforeAll(async () => { const { relatedTable, tableId } = await basicRelationshipTables( - RelationshipType.ONE_TO_MANY + relationshipType ) tableOrViewId = tableId productCategoryTable = relatedTable @@ -2380,7 +2384,10 @@ describe.each([ ], }, }).toContainExactly([ - { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, + { + name: "foo", + productCat: [{ _id: productCatRows[0]._id }], + }, ]) } ) @@ -2480,7 +2487,10 @@ describe.each([ ], }, }).toContainExactly([ - { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, + { + name: "foo", + productCat: [{ _id: productCatRows[0]._id }], + }, ]) } ) @@ -2504,8 +2514,14 @@ describe.each([ ], }, }).toContainExactly([ - { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, - { name: "bar", productCat: [{ _id: productCatRows[1]._id }] }, + { + name: "foo", + productCat: [{ _id: productCatRows[0]._id }], + }, + { + name: "bar", + productCat: [{ _id: productCatRows[1]._id }], + }, { name: "baz", productCat: undefined }, ]) } From 41405ea39db26b03fbde5664add1cb116cc74467 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 14:15:45 +0200 Subject: [PATCH 05/41] Add extra rows --- .../src/api/routes/tests/search.spec.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 3ab35c9294..ece84435bf 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2609,6 +2609,21 @@ describe.each([ related1: [relatedRows[2]._id!], related2: [relatedRows[3]._id!], }), + config.api.row.save(tableOrViewId, { + name: "test3", + related1: [], + related2: [], + }), + config.api.row.save(tableOrViewId, { + name: "test4", + related1: [relatedRows[1]._id!], + related2: [], + }), + config.api.row.save(tableOrViewId, { + name: "test5", + related1: [], + related2: [relatedRows[1]._id!], + }), ]) }) @@ -2626,6 +2641,17 @@ describe.each([ related1: [{ _id: relatedRows[2]._id }], related2: [{ _id: relatedRows[3]._id }], }, + { + name: "test3", + }, + { + name: "test4", + related1: [{ _id: relatedRows[1]._id }], + }, + { + name: "test5", + related2: [{ _id: relatedRows[1]._id! }], + }, ]) }) From 4920ab30a55dfe80c751afbf772dd4b2002be049 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 15 Oct 2024 13:43:48 +0000 Subject: [PATCH 06/41] Bump version to 2.32.18 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 13530e9aee..0c246a4968 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.32.17", + "version": "2.32.18", "npmClient": "yarn", "packages": [ "packages/*", From c150918773f4c2e58983869095e43fdac97a8e27 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 15 Oct 2024 13:46:12 +0000 Subject: [PATCH 07/41] Bump version to 2.33.0 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 0c246a4968..5432f60e9e 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.32.18", + "version": "2.33.0", "npmClient": "yarn", "packages": [ "packages/*", From 6688ccf1ab29ef958075d17ccc7df1b46d467a20 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 15:50:52 +0200 Subject: [PATCH 08/41] Ensure we replace only on when starting with --- packages/server/src/sdk/app/rows/search/filters.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/filters.ts b/packages/server/src/sdk/app/rows/search/filters.ts index 4049fc5352..64656b1a37 100644 --- a/packages/server/src/sdk/app/rows/search/filters.ts +++ b/packages/server/src/sdk/app/rows/search/filters.ts @@ -53,8 +53,12 @@ export function updateFilterKeys( ) if (possibleKey && possibleKey.original !== possibleKey.updated) { // only replace the first, not replaceAll - filter[key.replace(possibleKey.original, possibleKey.updated)] = - filter[key] + filter[ + key.replace( + new RegExp(`^${possibleKey.original}\\.`), + `${possibleKey.updated}.` + ) + ] = filter[key] delete filter[key] } } From 225d062fccce09ba68e9c431dbfaa04fd6ff6b8b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 16:30:10 +0200 Subject: [PATCH 09/41] Fix --- packages/server/src/sdk/app/rows/search/filters.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/filters.ts b/packages/server/src/sdk/app/rows/search/filters.ts index 64656b1a37..9fab045554 100644 --- a/packages/server/src/sdk/app/rows/search/filters.ts +++ b/packages/server/src/sdk/app/rows/search/filters.ts @@ -44,10 +44,11 @@ export function updateFilterKeys( if (!isPlainObject(filter)) { continue } - for (let [key, keyFilter] of Object.entries(filter)) { + for (const [key, keyFilter] of Object.entries(filter)) { if (keyFilter === "") { delete filter[key] } + const possibleKey = updates.find(({ original }) => key.match(makeFilterKeyRegex(original)) ) @@ -55,8 +56,8 @@ export function updateFilterKeys( // only replace the first, not replaceAll filter[ key.replace( - new RegExp(`^${possibleKey.original}\\.`), - `${possibleKey.updated}.` + new RegExp(`^(\\d+:)?${possibleKey.original}\\.`), + `$1${possibleKey.updated}.` ) ] = filter[key] delete filter[key] From 86e5718705df0c3fb8c3eaa3aa5577435280eb6f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 17:40:03 +0200 Subject: [PATCH 10/41] Fix tests --- packages/server/src/integrations/tests/sqlAlias.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 890c8c4663..ddc84ae1ce 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -78,8 +78,7 @@ describe("Captures of real examples", () => { bindings: ["assembling", primaryLimit, relationshipLimit], sql: expect.stringContaining( multiline( - `where exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" - and (COALESCE("b"."taskname" = $1, FALSE))` + `where (exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" and (COALESCE("b"."taskname" = $1, FALSE)))` ) ), }) @@ -133,6 +132,8 @@ describe("Captures of real examples", () => { expect(query).toEqual({ bindings: [ + rangeValue.low, + rangeValue.high, rangeValue.low, rangeValue.high, equalValue, @@ -144,7 +145,7 @@ describe("Captures of real examples", () => { ], sql: expect.stringContaining( multiline( - `where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2))` + `where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2)) and exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $3 and $4)) and (exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $5, FALSE)))` ) ), }) From 8a6dbef24938d99f448e1fb3aa9e0be6224f036c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 18:50:58 +0200 Subject: [PATCH 11/41] Fix sqs --- packages/backend-core/src/sql/sql.ts | 34 ++++++++++++++----- .../src/sdk/app/rows/search/internal/sqs.ts | 33 ++---------------- 2 files changed, 28 insertions(+), 39 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index b415a6f1b7..83a4fd0d59 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -406,23 +406,26 @@ class InternalBuilder { addRelationshipForFilter( query: Knex.QueryBuilder, filterKey: string, - whereCb: (query: Knex.QueryBuilder) => Knex.QueryBuilder + whereCb: (filterKey: string, query: Knex.QueryBuilder) => Knex.QueryBuilder ): Knex.QueryBuilder { const mainKnex = this.knex const { relationships, endpoint, tableAliases: aliases } = this.query const tableName = endpoint.entityId const fromAlias = aliases?.[tableName] || tableName - const matches = (possibleTable: string) => - filterKey.startsWith(`${possibleTable}`) + const matches = (value: string) => filterKey.startsWith(`${value}`) if (!relationships) { return query } for (const relationship of relationships) { const relatedTableName = relationship.tableName const toAlias = aliases?.[relatedTableName] || relatedTableName + + const matchesTableName = matches(relatedTableName) || matches(toAlias) + const matchesRelationName = matches(relationship.column) + // this is the relationship which is being filtered if ( - (matches(relatedTableName) || matches(toAlias)) && + (matchesTableName || matchesRelationName) && relationship.to && relationship.tableName ) { @@ -430,6 +433,17 @@ class InternalBuilder { .select(mainKnex.raw(1)) .from({ [toAlias]: relatedTableName }) const manyToMany = validateManyToMany(relationship) + let updatedKey + + if (matchesRelationName) { + updatedKey = filterKey.replace( + new RegExp(`^${relationship.column}.`), + `${aliases![relationship.tableName]}.` + ) + } else { + updatedKey = filterKey + } + if (manyToMany) { const throughAlias = aliases?.[manyToMany.through] || relationship.through @@ -470,7 +484,7 @@ class InternalBuilder { ) ) } - query = query.whereExists(whereCb(subQuery)) + query = query.whereExists(whereCb(updatedKey, subQuery)) break } } @@ -558,9 +572,13 @@ class InternalBuilder { if (allOr) { query = query.or } - query = builder.addRelationshipForFilter(query, updatedKey, q => { - return handleRelationship(q, updatedKey, value) - }) + query = builder.addRelationshipForFilter( + query, + updatedKey, + (updatedKey, q) => { + return handleRelationship(q, updatedKey, value) + } + ) } } } 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 916e20957b..e3bbca271e 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -39,11 +39,6 @@ import AliasTables from "../../sqlAlias" import { outputProcessing } from "../../../../../utilities/rowProcessor" import pick from "lodash/pick" import { processRowCountResponse } from "../../utils" -import { - getRelationshipColumns, - getTableIDList, - updateFilterKeys, -} from "../filters" import { dataFilters, helpers, @@ -133,31 +128,7 @@ async function buildInternalFieldList( return [...new Set(fieldList)] } -function cleanupFilters( - filters: SearchFilters, - table: Table, - allTables: Table[] -) { - // get a list of all relationship columns in the table for updating - const relationshipColumns = getRelationshipColumns(table) - // get table names to ID map for relationships - const tableNameToID = getTableIDList(allTables) - // all should be applied at once - filters = updateFilterKeys( - filters, - relationshipColumns - .map(({ name, definition }) => ({ - original: name, - updated: definition.tableId, - })) - .concat( - tableNameToID.map(({ name, id }) => ({ - original: name, - updated: id, - })) - ) - ) - +function cleanupFilters(filters: SearchFilters, allTables: Table[]) { // generate a map of all possible column names (these can be duplicated across tables // the map of them will always be the same const userColumnMap: Record = {} @@ -356,7 +327,7 @@ export async function search( const relationships = buildInternalRelationships(table, allTables) const searchFilters: SearchFilters = { - ...cleanupFilters(query, table, allTables), + ...cleanupFilters(query, allTables), documentType: DocumentType.ROW, } From b01564c934922c0acabfd5bea53861f511163fb5 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 10:21:17 +0200 Subject: [PATCH 12/41] Fix multiple relations to same table for external --- packages/backend-core/src/sql/sql.ts | 6 +- .../api/controllers/row/ExternalRequest.ts | 12 ---- packages/server/src/sdk/app/rows/index.ts | 2 - .../server/src/sdk/app/rows/search/filters.ts | 70 ------------------- 4 files changed, 3 insertions(+), 87 deletions(-) delete mode 100644 packages/server/src/sdk/app/rows/search/filters.ts diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 83a4fd0d59..c47eabca57 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -412,7 +412,8 @@ class InternalBuilder { const { relationships, endpoint, tableAliases: aliases } = this.query const tableName = endpoint.entityId const fromAlias = aliases?.[tableName] || tableName - const matches = (value: string) => filterKey.startsWith(`${value}`) + const matches = (value: string) => + filterKey.match(new RegExp(`^${value}\\.`)) if (!relationships) { return query } @@ -435,7 +436,7 @@ class InternalBuilder { const manyToMany = validateManyToMany(relationship) let updatedKey - if (matchesRelationName) { + if (!matchesTableName) { updatedKey = filterKey.replace( new RegExp(`^${relationship.column}.`), `${aliases![relationship.tableName]}.` @@ -485,7 +486,6 @@ class InternalBuilder { ) } query = query.whereExists(whereCb(updatedKey, subQuery)) - break } } return query diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 56522acb33..7ac13d3200 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -204,18 +204,6 @@ export class ExternalRequest { filters: SearchFilters, table: Table ): SearchFilters { - // replace any relationship columns initially, table names and relationship column names are acceptable - const relationshipColumns = sdk.rows.filters.getRelationshipColumns(table) - filters = sdk.rows.filters.updateFilterKeys( - filters, - relationshipColumns.map(({ name, definition }) => { - const { tableName } = breakExternalTableId(definition.tableId) - return { - original: name, - updated: tableName, - } - }) - ) const primary = table.primary // if passed in array need to copy for shifting etc let idCopy: undefined | string | any[] = cloneDeep(id) diff --git a/packages/server/src/sdk/app/rows/index.ts b/packages/server/src/sdk/app/rows/index.ts index fb077509a9..c117941419 100644 --- a/packages/server/src/sdk/app/rows/index.ts +++ b/packages/server/src/sdk/app/rows/index.ts @@ -3,14 +3,12 @@ import * as rows from "./rows" import * as search from "./search" import * as utils from "./utils" import * as external from "./external" -import * as filters from "./search/filters" import AliasTables from "./sqlAlias" export default { ...attachments, ...rows, ...search, - filters, utils, external, AliasTables, diff --git a/packages/server/src/sdk/app/rows/search/filters.ts b/packages/server/src/sdk/app/rows/search/filters.ts deleted file mode 100644 index 9fab045554..0000000000 --- a/packages/server/src/sdk/app/rows/search/filters.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { - FieldType, - RelationshipFieldMetadata, - SearchFilters, - Table, -} from "@budibase/types" -import { isPlainObject } from "lodash" -import { dataFilters } from "@budibase/shared-core" - -export function getRelationshipColumns(table: Table): { - name: string - definition: RelationshipFieldMetadata -}[] { - // performing this with a for loop rather than an array filter improves - // type guarding, as no casts are required - const linkEntries: [string, RelationshipFieldMetadata][] = [] - for (let entry of Object.entries(table.schema)) { - if (entry[1].type === FieldType.LINK) { - const linkColumn: RelationshipFieldMetadata = entry[1] - linkEntries.push([entry[0], linkColumn]) - } - } - return linkEntries.map(entry => ({ - name: entry[0], - definition: entry[1], - })) -} - -export function getTableIDList( - tables: Table[] -): { name: string; id: string }[] { - return tables - .filter(table => table.originalName && table._id) - .map(table => ({ id: table._id!, name: table.originalName! })) -} - -export function updateFilterKeys( - filters: SearchFilters, - updates: { original: string; updated: string }[] -): SearchFilters { - const makeFilterKeyRegex = (str: string) => - new RegExp(`^${str}\\.|:${str}\\.`) - for (let filter of Object.values(filters)) { - if (!isPlainObject(filter)) { - continue - } - for (const [key, keyFilter] of Object.entries(filter)) { - if (keyFilter === "") { - delete filter[key] - } - - const possibleKey = updates.find(({ original }) => - key.match(makeFilterKeyRegex(original)) - ) - if (possibleKey && possibleKey.original !== possibleKey.updated) { - // only replace the first, not replaceAll - filter[ - key.replace( - new RegExp(`^(\\d+:)?${possibleKey.original}\\.`), - `$1${possibleKey.updated}.` - ) - ] = filter[key] - delete filter[key] - } - } - } - return dataFilters.recurseLogicalOperators(filters, (f: SearchFilters) => { - return updateFilterKeys(f, updates) - }) -} From 2d460f19557c866b1087634d850229e91835747b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 12:26:13 +0200 Subject: [PATCH 13/41] Fix supporting filter using table names --- .../server/src/api/controllers/row/index.ts | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 680a7671d5..9122dc5ff3 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -15,13 +15,16 @@ import { ExportRowsResponse, FieldType, GetRowResponse, + isRelationshipField, PatchRowRequest, PatchRowResponse, Row, RowAttachment, RowSearchParams, + SearchFilters, SearchRowRequest, SearchRowResponse, + Table, UserCtx, ValidateResponse, } from "@budibase/types" @@ -33,6 +36,8 @@ import sdk from "../../../sdk" import * as exporters from "../view/exporters" import { Format } from "../view/exporters" import { apiFileReturn } from "../../../utilities/fileSystem" +import { dataFilters } from "@budibase/shared-core" +import { isPlainObject } from "lodash" export * as views from "./views" @@ -211,13 +216,16 @@ export async function search(ctx: Ctx) { await context.ensureSnippetContext(true) - const enrichedQuery = await utils.enrichSearchContext( + let enrichedQuery: SearchFilters = await utils.enrichSearchContext( { ...ctx.request.body.query }, { user: sdk.users.getUserContextBindings(ctx.user), } ) + const allTables = await sdk.tables.getAllTables() + enrichedQuery = replaceTableNamesInFilters(tableId, enrichedQuery, allTables) + const searchParams: RowSearchParams = { ...ctx.request.body, query: enrichedQuery, @@ -229,6 +237,50 @@ export async function search(ctx: Ctx) { ctx.body = await sdk.rows.search(searchParams) } +function replaceTableNamesInFilters( + tableId: string, + filters: SearchFilters, + allTables: Table[] +): SearchFilters { + for (const filter of Object.values(filters)) { + if (!isPlainObject(filter)) { + continue + } + for (const key of Object.keys(filter)) { + const matches = key.match(`^(?.+)\\.(?.+)`) + + const relation = matches?.groups?.["relation"] + const field = matches?.groups?.["field"] + + if (!relation || !field) { + continue + } + + const table = allTables.find(r => r._id === tableId)! + if (Object.values(table.schema).some(f => f.name === relation)) { + continue + } + + const matchedTable = allTables.find(t => t.name === relation) + const relationship = Object.values(table.schema).find( + f => isRelationshipField(f) && f.tableId === matchedTable?._id + ) + if (!relationship) { + continue + } + + const updatedField = `${relationship.name}.${field}` + if (updatedField && updatedField !== key) { + filter[updatedField] = filter[key] + delete filter[key] + } + } + } + return dataFilters.recurseLogicalOperators(filters, (f: SearchFilters) => { + return replaceTableNamesInFilters(tableId, f, allTables) + }) +} + export async function validate(ctx: Ctx) { const source = await utils.getSource(ctx) const table = await utils.getTableFromSource(source) From 83ffee31947284cf2b0bf7e7df2500ef26ce506c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 12:33:19 +0200 Subject: [PATCH 14/41] Fix tests --- .../src/api/routes/tests/search.spec.ts | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index ece84435bf..5d778ed6d2 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2614,16 +2614,6 @@ describe.each([ related1: [], related2: [], }), - config.api.row.save(tableOrViewId, { - name: "test4", - related1: [relatedRows[1]._id!], - related2: [], - }), - config.api.row.save(tableOrViewId, { - name: "test5", - related1: [], - related2: [relatedRows[1]._id!], - }), ]) }) @@ -2644,19 +2634,11 @@ describe.each([ { name: "test3", }, - { - name: "test4", - related1: [{ _id: relatedRows[1]._id }], - }, - { - name: "test5", - related2: [{ _id: relatedRows[1]._id! }], - }, ]) }) isSqs && - it("should be able to filter down to second row with equal", async () => { + it("should be able to filter via the first relation field with equal", async () => { await expectSearch({ query: { equal: { @@ -2672,11 +2654,11 @@ describe.each([ }) isSqs && - it("should be able to filter down to first row with not equal", async () => { + it("should be able to filter via the second relation field with not equal", async () => { await expectSearch({ query: { notEqual: { - ["1:related2.name"]: "bar", + ["1:related2.name"]: "foo", ["2:related2.name"]: "baz", ["3:related2.name"]: "boo", }, @@ -2684,7 +2666,6 @@ describe.each([ }).toContainExactly([ { name: "test", - related1: [{ _id: relatedRows[0]._id }], }, ]) }) From 7266fb3c1bd0cf7b9d3f344002e46602bd324677 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 12:38:38 +0200 Subject: [PATCH 15/41] Run tests for all --- .../server/src/api/controllers/row/index.ts | 16 ++--- .../src/api/routes/tests/search.spec.ts | 59 +++++++++---------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 9122dc5ff3..51bdb12ca2 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -216,15 +216,15 @@ export async function search(ctx: Ctx) { await context.ensureSnippetContext(true) - let enrichedQuery: SearchFilters = await utils.enrichSearchContext( - { ...ctx.request.body.query }, - { - user: sdk.users.getUserContextBindings(ctx.user), - } - ) + let { query } = ctx.request.body + if (!isPlainObject(query)) { + const allTables = await sdk.tables.getAllTables() + query = replaceTableNamesInFilters(tableId, query, allTables) + } - const allTables = await sdk.tables.getAllTables() - enrichedQuery = replaceTableNamesInFilters(tableId, enrichedQuery, allTables) + let enrichedQuery: SearchFilters = await utils.enrichSearchContext(query, { + user: sdk.users.getUserContextBindings(ctx.user), + }) const searchParams: RowSearchParams = { ...ctx.request.body, diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 5d778ed6d2..7132115d94 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2567,7 +2567,8 @@ describe.each([ expect(response.rows[0].productCat).toBeArrayOfSize(11) }) }) - ;(isSqs || isLucene) && + + isSql && describe("relations to same table", () => { let relatedTable: string, relatedRows: Row[] @@ -2637,38 +2638,36 @@ describe.each([ ]) }) - isSqs && - it("should be able to filter via the first relation field with equal", async () => { - await expectSearch({ - query: { - equal: { - ["related1.name"]: "baz", - }, + it("should be able to filter via the first relation field with equal", async () => { + await expectSearch({ + query: { + equal: { + ["related1.name"]: "baz", }, - }).toContainExactly([ - { - name: "test2", - related1: [{ _id: relatedRows[2]._id }], - }, - ]) - }) + }, + }).toContainExactly([ + { + name: "test2", + related1: [{ _id: relatedRows[2]._id }], + }, + ]) + }) - isSqs && - it("should be able to filter via the second relation field with not equal", async () => { - await expectSearch({ - query: { - notEqual: { - ["1:related2.name"]: "foo", - ["2:related2.name"]: "baz", - ["3:related2.name"]: "boo", - }, + it("should be able to filter via the second relation field with not equal", async () => { + await expectSearch({ + query: { + notEqual: { + ["1:related2.name"]: "foo", + ["2:related2.name"]: "baz", + ["3:related2.name"]: "boo", }, - }).toContainExactly([ - { - name: "test", - }, - ]) - }) + }, + }).toContainExactly([ + { + name: "test", + }, + ]) + }) }) isInternal && From 88d4ebc725f3a87cb3ad4aa2b34655cab82b1879 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 12:44:25 +0200 Subject: [PATCH 16/41] Add more tests --- .../src/api/routes/tests/search.spec.ts | 21 +++++++++++++++++-- 1 file changed, 19 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 7132115d94..da0fd4fbba 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2612,8 +2612,8 @@ describe.each([ }), config.api.row.save(tableOrViewId, { name: "test3", - related1: [], - related2: [], + related1: [relatedRows[1]._id], + related2: [relatedRows[2]._id!], }), ]) }) @@ -2634,6 +2634,8 @@ describe.each([ }, { name: "test3", + related1: [{ _id: relatedRows[1]._id }], + related2: [{ _id: relatedRows[2]._id }], }, ]) }) @@ -2668,6 +2670,21 @@ describe.each([ }, ]) }) + + it("should be able to filter on both fields", async () => { + await expectSearch({ + query: { + notEqual: { + ["related1.name"]: "foo", + ["related2.name"]: "baz", + }, + }, + }).toContainExactly([ + { + name: "test2", + }, + ]) + }) }) isInternal && From 830ed0cda25c914cca5364a1c5de88b5c7258be6 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 12:48:07 +0200 Subject: [PATCH 17/41] Fix undefineds --- packages/server/src/api/controllers/row/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 51bdb12ca2..40b41f410f 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -217,7 +217,7 @@ export async function search(ctx: Ctx) { await context.ensureSnippetContext(true) let { query } = ctx.request.body - if (!isPlainObject(query)) { + if (query && !isPlainObject(query)) { const allTables = await sdk.tables.getAllTables() query = replaceTableNamesInFilters(tableId, query, allTables) } From 0a1912e42c2119fe5e2eea122729bcfda5d92579 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Wed, 16 Oct 2024 11:56:52 +0100 Subject: [PATCH 18/41] Don't extend dataprovider queries when no extensions are in use --- packages/client/src/components/app/DataProvider.svelte | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/client/src/components/app/DataProvider.svelte b/packages/client/src/components/app/DataProvider.svelte index 7b5187ffb6..054264a9e1 100644 --- a/packages/client/src/components/app/DataProvider.svelte +++ b/packages/client/src/components/app/DataProvider.svelte @@ -126,6 +126,9 @@ } const extendQuery = (defaultQuery, extensions) => { + if (!Object.keys(extensions).length) { + return + } const extended = { [LogicalOperator.AND]: { conditions: [ From ba80880ab65d76d938e9746f896513528c4119a4 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 13:02:53 +0200 Subject: [PATCH 19/41] Fix --- packages/server/src/api/controllers/row/index.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 40b41f410f..60775ce628 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -37,7 +37,6 @@ import * as exporters from "../view/exporters" import { Format } from "../view/exporters" import { apiFileReturn } from "../../../utilities/fileSystem" import { dataFilters } from "@budibase/shared-core" -import { isPlainObject } from "lodash" export * as views from "./views" @@ -217,7 +216,7 @@ export async function search(ctx: Ctx) { await context.ensureSnippetContext(true) let { query } = ctx.request.body - if (query && !isPlainObject(query)) { + if (query) { const allTables = await sdk.tables.getAllTables() query = replaceTableNamesInFilters(tableId, query, allTables) } @@ -243,9 +242,6 @@ function replaceTableNamesInFilters( allTables: Table[] ): SearchFilters { for (const filter of Object.values(filters)) { - if (!isPlainObject(filter)) { - continue - } for (const key of Object.keys(filter)) { const matches = key.match(`^(?.+)\\.(?.+)`) From 51c95058ee8b3096d13d479d8032d913fe4b1cc2 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Wed, 16 Oct 2024 12:04:19 +0100 Subject: [PATCH 20/41] Fix the drawerShow event not being broadcast, preventing component popovers from hiding properly when opening drawers --- .../design/settings/controls/FilterEditor/FilterEditor.svelte | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/builder/src/components/design/settings/controls/FilterEditor/FilterEditor.svelte b/packages/builder/src/components/design/settings/controls/FilterEditor/FilterEditor.svelte index d6f1732e64..34c317e865 100644 --- a/packages/builder/src/components/design/settings/controls/FilterEditor/FilterEditor.svelte +++ b/packages/builder/src/components/design/settings/controls/FilterEditor/FilterEditor.svelte @@ -59,6 +59,7 @@ bind:this={drawer} title="Filtering" on:drawerHide + on:drawerShow on:drawerShow={() => { // Reset to the currently available value. localFilters = Helpers.cloneDeep(value) From 57ce8b7e855126c3f4bd905e7121137dd22d25ab Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 13:17:07 +0200 Subject: [PATCH 21/41] Fix test --- packages/server/src/integrations/tests/sqlAlias.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 890c8c4663..81013b328c 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -78,8 +78,7 @@ describe("Captures of real examples", () => { bindings: ["assembling", primaryLimit, relationshipLimit], sql: expect.stringContaining( multiline( - `where exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" - and (COALESCE("b"."taskname" = $1, FALSE))` + `where exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" and (COALESCE("b"."taskname" = $1, FALSE))` ) ), }) @@ -133,6 +132,8 @@ describe("Captures of real examples", () => { expect(query).toEqual({ bindings: [ + rangeValue.low, + rangeValue.high, rangeValue.low, rangeValue.high, equalValue, @@ -144,7 +145,7 @@ describe("Captures of real examples", () => { ], sql: expect.stringContaining( multiline( - `where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2))` + `where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2)) and exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $3 and $4)) and exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $5, FALSE))` ) ), }) From 86c105e1c8983b732f9bef6c90e88b47ddcbc994 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 16 Oct 2024 12:20:41 +0100 Subject: [PATCH 22/41] Ensure external tables always have a sourceId. --- packages/server/src/api/controllers/row/ExternalRequest.ts | 4 ++-- packages/server/src/sdk/app/tables/getters.ts | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 56522acb33..9e68845a45 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -173,9 +173,9 @@ export class ExternalRequest { if (!opts.datasource) { if (sdk.views.isView(source)) { const table = await sdk.views.getTable(source.id) - opts.datasource = await sdk.datasources.get(table.sourceId!) + opts.datasource = await sdk.datasources.get(table.sourceId) } else { - opts.datasource = await sdk.datasources.get(source.sourceId!) + opts.datasource = await sdk.datasources.get(source.sourceId) } } diff --git a/packages/server/src/sdk/app/tables/getters.ts b/packages/server/src/sdk/app/tables/getters.ts index 280fb5378a..49944bce85 100644 --- a/packages/server/src/sdk/app/tables/getters.ts +++ b/packages/server/src/sdk/app/tables/getters.ts @@ -90,7 +90,11 @@ export async function getExternalTable( if (!entities[tableName]) { throw new Error(`Unable to find table named "${tableName}"`) } - return processTable(entities[tableName]) + const table = await processTable(entities[tableName]) + if (!table.sourceId) { + table.sourceId = datasourceId + } + return table } export async function getTable(tableId: string): Promise { From 5f1cd4eb9f562f3bf4e179fbf7a5caaa8d877977 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 13:47:09 +0200 Subject: [PATCH 23/41] Test multiple relationship types --- .../server/src/api/routes/tests/search.spec.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index da0fd4fbba..0f5797f8de 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2278,12 +2278,16 @@ describe.each([ // It also can't work for in-memory searching because the related table name // isn't available. !isInMemory && - describe("relations", () => { + describe.each([ + RelationshipType.ONE_TO_MANY, + RelationshipType.MANY_TO_ONE, + RelationshipType.MANY_TO_MANY, + ])("relations (%s)", relationshipType => { let productCategoryTable: Table, productCatRows: Row[] beforeAll(async () => { const { relatedTable, tableId } = await basicRelationshipTables( - RelationshipType.ONE_TO_MANY + relationshipType ) tableOrViewId = tableId productCategoryTable = relatedTable @@ -2538,10 +2542,13 @@ describe.each([ }) isSql && - describe("big relations", () => { + describe.each([ + RelationshipType.MANY_TO_ONE, + RelationshipType.MANY_TO_MANY, + ])("big relations (%s)", relationshipType => { beforeAll(async () => { const { relatedTable, tableId } = await basicRelationshipTables( - RelationshipType.MANY_TO_ONE + relationshipType ) tableOrViewId = tableId const mainRow = await config.api.row.save(tableOrViewId, { From 3ea8e240e4716361850ddbb8c027d1fae968f531 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 14:05:48 +0200 Subject: [PATCH 24/41] Fix one-to-many --- packages/backend-core/src/sql/sql.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index b7b499b92c..662558415e 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -518,10 +518,12 @@ class InternalBuilder { mainKnex.raw(this.quotedIdentifier(foreignKey)) ) - query = query.whereExists(whereCb(updatedKey, subQuery)) - if (allowEmptyRelationships) { - query = query.orWhereNull(foreignKey) - } + query = query.where(q => { + q.whereExists(whereCb(updatedKey, subQuery)) + if (allowEmptyRelationships) { + q.orWhereNull(foreignKey) + } + }) } } } From 3fbe937bf43443647ce6709942bd32e3717045b2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 14:23:58 +0200 Subject: [PATCH 25/41] Fix sqlalias test --- packages/server/src/integrations/tests/sqlAlias.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index ddc84ae1ce..739d3a4aee 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -145,7 +145,7 @@ describe("Captures of real examples", () => { ], sql: expect.stringContaining( multiline( - `where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2)) and exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $3 and $4)) and (exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $5, FALSE)))` + `where (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2))) and (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $3 and $4))) and (exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $5, FALSE))))` ) ), }) From dac963a2ff1e01c28e63f69e8110fdcd17ff758d Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Wed, 16 Oct 2024 13:43:34 +0000 Subject: [PATCH 26/41] Bump version to 2.33.1 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 5432f60e9e..caaa07683a 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.33.0", + "version": "2.33.1", "npmClient": "yarn", "packages": [ "packages/*", From 46b9877455e638d29071dd763a8e6947d738a8a1 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Wed, 16 Oct 2024 15:15:23 +0100 Subject: [PATCH 27/41] Return default query --- packages/client/src/components/app/DataProvider.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/src/components/app/DataProvider.svelte b/packages/client/src/components/app/DataProvider.svelte index 054264a9e1..e6629aa3f3 100644 --- a/packages/client/src/components/app/DataProvider.svelte +++ b/packages/client/src/components/app/DataProvider.svelte @@ -127,7 +127,7 @@ const extendQuery = (defaultQuery, extensions) => { if (!Object.keys(extensions).length) { - return + return defaultQuery } const extended = { [LogicalOperator.AND]: { From b6874f52f6d8e5228b57842a9844eba28f72e9e6 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 16:16:39 +0200 Subject: [PATCH 28/41] Fix many-to-one --- packages/backend-core/src/sql/sql.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 662558415e..949d7edf1b 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -510,18 +510,19 @@ class InternalBuilder { } }) } else { + const toKey = `${toAlias}.${relationship.to}` const foreignKey = `${fromAlias}.${relationship.from}` // "join" to the main table, making sure the ID matches that of the main subQuery = subQuery.where( - `${toAlias}.${relationship.to}`, + toKey, "=", mainKnex.raw(this.quotedIdentifier(foreignKey)) ) query = query.where(q => { - q.whereExists(whereCb(updatedKey, subQuery)) + q.whereExists(whereCb(updatedKey, subQuery.clone())) if (allowEmptyRelationships) { - q.orWhereNull(foreignKey) + q.orWhereNotExists(subQuery) } }) } From f4428d60766d4b03427d3e8c29fa7c6d543e8186 Mon Sep 17 00:00:00 2001 From: Dean Date: Wed, 16 Oct 2024 15:26:33 +0100 Subject: [PATCH 29/41] Bump pro module --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 6139154961..fc4c7f4925 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 61391549614b5ac153f267633d0aaea9b07f05c5 +Subproject commit fc4c7f4925139af078480217965c3d6338dc0a7f From 38a57faee4de5278a0fba9432b57d3eadfb89eab Mon Sep 17 00:00:00 2001 From: Dean Date: Wed, 16 Oct 2024 15:37:12 +0100 Subject: [PATCH 30/41] Bump pro correctly --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index fc4c7f4925..1a749caba9 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit fc4c7f4925139af078480217965c3d6338dc0a7f +Subproject commit 1a749caba9c85aab2645e5d00db479eb53d3f80f From 8b9bb784c49d56aeb1d04b67f6a8bc048fbf430e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 17:06:15 +0200 Subject: [PATCH 31/41] Initial appMetadata sdk usage --- .../server/src/api/controllers/application.ts | 19 ++++++++----------- .../server/src/sdk/app/appMetadata/index.ts | 5 +++++ .../src/sdk/app/appMetadata/metadata.ts | 8 ++++++++ packages/server/src/sdk/index.ts | 2 ++ 4 files changed, 23 insertions(+), 11 deletions(-) create mode 100644 packages/server/src/sdk/app/appMetadata/index.ts create mode 100644 packages/server/src/sdk/app/appMetadata/metadata.ts diff --git a/packages/server/src/api/controllers/application.ts b/packages/server/src/api/controllers/application.ts index 830acc55bf..660d35c29d 100644 --- a/packages/server/src/api/controllers/application.ts +++ b/packages/server/src/api/controllers/application.ts @@ -208,9 +208,8 @@ export async function fetchAppDefinition( export async function fetchAppPackage( ctx: UserCtx ) { - const db = context.getAppDB() const appId = context.getAppId() - let application = await db.get(DocumentType.APP_METADATA) + const application = await sdk.appMetadata.get() const layouts = await getLayouts() let screens = await getScreens() const license = await licensing.cache.getCachedLicense() @@ -315,7 +314,7 @@ async function performAppCreate(ctx: UserCtx) { // If we used a template or imported an app there will be an existing doc. // Fetch and migrate some metadata from the existing app. try { - const existing: App = await db.get(DocumentType.APP_METADATA) + const existing = await sdk.appMetadata.get() const keys: (keyof App)[] = [ "_rev", "navigation", @@ -489,8 +488,7 @@ export async function update( export async function updateClient(ctx: UserCtx) { // Get current app version - const db = context.getAppDB() - const application = await db.get(DocumentType.APP_METADATA) + const application = await sdk.appMetadata.get() const currentVersion = application.version let manifest @@ -518,8 +516,7 @@ export async function updateClient(ctx: UserCtx) { export async function revertClient(ctx: UserCtx) { // Check app can be reverted - const db = context.getAppDB() - const application = await db.get(DocumentType.APP_METADATA) + const application = await sdk.appMetadata.get() if (!application.revertableVersion) { ctx.throw(400, "There is no version to revert to") } @@ -577,7 +574,7 @@ async function destroyApp(ctx: UserCtx) { const db = dbCore.getDB(devAppId) // standard app deletion flow - const app = await db.get(DocumentType.APP_METADATA) + const app = await sdk.appMetadata.get() const result = await db.destroy() await quotas.removeApp() await events.app.deleted(app) @@ -728,7 +725,7 @@ export async function updateAppPackage( ) { return context.doInAppContext(appId, async () => { const db = context.getAppDB() - const application = await db.get(DocumentType.APP_METADATA) + const application = await sdk.appMetadata.get() const newAppPackage: App = { ...application, ...appPackage } if (appPackage._rev !== application._rev) { @@ -754,7 +751,7 @@ export async function setRevertableVersion( return } const db = context.getAppDB() - const app = await db.get(DocumentType.APP_METADATA) + const app = await sdk.appMetadata.get() app.revertableVersion = ctx.request.body.revertableVersion await db.put(app) @@ -763,7 +760,7 @@ export async function setRevertableVersion( async function migrateAppNavigation() { const db = context.getAppDB() - const existing: App = await db.get(DocumentType.APP_METADATA) + const existing = await sdk.appMetadata.get() const layouts: Layout[] = await getLayouts() const screens: Screen[] = await getScreens() diff --git a/packages/server/src/sdk/app/appMetadata/index.ts b/packages/server/src/sdk/app/appMetadata/index.ts new file mode 100644 index 0000000000..267899a214 --- /dev/null +++ b/packages/server/src/sdk/app/appMetadata/index.ts @@ -0,0 +1,5 @@ +import * as metadata from "./metadata" + +export default { + ...metadata, +} diff --git a/packages/server/src/sdk/app/appMetadata/metadata.ts b/packages/server/src/sdk/app/appMetadata/metadata.ts new file mode 100644 index 0000000000..78d2a040b9 --- /dev/null +++ b/packages/server/src/sdk/app/appMetadata/metadata.ts @@ -0,0 +1,8 @@ +import { context, DocumentType } from "@budibase/backend-core" +import { App } from "@budibase/types" + +export async function get() { + const db = context.getAppDB() + const application = await db.get(DocumentType.APP_METADATA) + return application +} diff --git a/packages/server/src/sdk/index.ts b/packages/server/src/sdk/index.ts index a871546b60..44df8b50fb 100644 --- a/packages/server/src/sdk/index.ts +++ b/packages/server/src/sdk/index.ts @@ -11,6 +11,7 @@ import { default as plugins } from "./plugins" import * as views from "./app/views" import * as permissions from "./app/permissions" import * as rowActions from "./app/rowActions" +import { default as appMetadata } from "./app/appMetadata" const sdk = { backups, @@ -26,6 +27,7 @@ const sdk = { permissions, links, rowActions, + appMetadata, } // default export for TS From 42086f36a2194c0d2ce038d5e81ee36c27d4ca16 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 17 Oct 2024 07:30:11 +0000 Subject: [PATCH 32/41] Bump version to 2.33.2 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index caaa07683a..df50828b30 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.33.1", + "version": "2.33.2", "npmClient": "yarn", "packages": [ "packages/*", From ab517bf86da556af6f0c5ca9cd8dabb0e5d87dc8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 10:32:17 +0200 Subject: [PATCH 33/41] Persist created version --- .../server/src/api/controllers/application.ts | 24 +++++++++++-------- .../src/sdk/app/appMetadata/metadata.ts | 10 ++++++++ packages/types/src/documents/app/app.ts | 1 + 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/packages/server/src/api/controllers/application.ts b/packages/server/src/api/controllers/application.ts index 660d35c29d..b8f523c331 100644 --- a/packages/server/src/api/controllers/application.ts +++ b/packages/server/src/api/controllers/application.ts @@ -271,6 +271,7 @@ async function performAppCreate(ctx: UserCtx) { path: ctx.request.body.file?.path, } } + const tenantId = tenancy.isMultiTenant() ? tenancy.getTenantId() : null const appId = generateDevAppID(generateAppID(tenantId)) @@ -278,7 +279,7 @@ async function performAppCreate(ctx: UserCtx) { const instance = await createInstance(appId, instanceConfig) const db = context.getAppDB() - let newApplication: App = { + const newApplication: App = { _id: DocumentType.APP_METADATA, _rev: undefined, appId, @@ -309,12 +310,18 @@ async function performAppCreate(ctx: UserCtx) { disableUserMetadata: true, skeletonLoader: true, }, + creationVersion: undefined, } + const isImport = !!instanceConfig.file + if (!isImport) { + newApplication.creationVersion = envCore.VERSION + } + + const existing = await sdk.appMetadata.tryGet() // If we used a template or imported an app there will be an existing doc. // Fetch and migrate some metadata from the existing app. - try { - const existing = await sdk.appMetadata.get() + if (existing) { const keys: (keyof App)[] = [ "_rev", "navigation", @@ -322,6 +329,7 @@ async function performAppCreate(ctx: UserCtx) { "customTheme", "icon", "snippets", + "creationVersion", ] keys.forEach(key => { if (existing[key]) { @@ -339,14 +347,10 @@ async function performAppCreate(ctx: UserCtx) { } // Migrate navigation settings and screens if required - if (existing) { - const navigation = await migrateAppNavigation() - if (navigation) { - newApplication.navigation = navigation - } + const navigation = await migrateAppNavigation() + if (navigation) { + newApplication.navigation = navigation } - } catch (err) { - // Nothing to do } const response = await db.put(newApplication, { force: true }) diff --git a/packages/server/src/sdk/app/appMetadata/metadata.ts b/packages/server/src/sdk/app/appMetadata/metadata.ts index 78d2a040b9..bbda55c085 100644 --- a/packages/server/src/sdk/app/appMetadata/metadata.ts +++ b/packages/server/src/sdk/app/appMetadata/metadata.ts @@ -1,8 +1,18 @@ import { context, DocumentType } from "@budibase/backend-core" import { App } from "@budibase/types" +/** + * @deprecated the plan is to get everything using `tryGet` instead, then rename + * `tryGet` to `get`. + */ export async function get() { const db = context.getAppDB() const application = await db.get(DocumentType.APP_METADATA) return application } + +export async function tryGet() { + const db = context.getAppDB() + const application = await db.tryGet(DocumentType.APP_METADATA) + return application +} diff --git a/packages/types/src/documents/app/app.ts b/packages/types/src/documents/app/app.ts index 69bf3489e1..06fca8307c 100644 --- a/packages/types/src/documents/app/app.ts +++ b/packages/types/src/documents/app/app.ts @@ -27,6 +27,7 @@ export interface App extends Document { usedPlugins?: Plugin[] upgradableVersion?: string snippets?: Snippet[] + creationVersion?: string } export interface AppInstance { From 8cdc5be38e27cb8555d8c0aae14eacdcfa867d9a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 11:36:02 +0200 Subject: [PATCH 34/41] Store proper version even on local --- packages/backend-core/src/environment.ts | 50 ++++++++++++++++-------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index 2ab8c550cc..87dd2c5e87 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -54,30 +54,46 @@ function getPackageJsonFields(): { VERSION: string SERVICE_NAME: string } { - function findFileInAncestors( - fileName: string, - currentDir: string - ): string | null { - const filePath = `${currentDir}/${fileName}` - if (existsSync(filePath)) { - return filePath + function getParentFile(file: string) { + function findFileInAncestors( + fileName: string, + currentDir: string + ): string | null { + const filePath = `${currentDir}/${fileName}` + if (existsSync(filePath)) { + return filePath + } + + const parentDir = `${currentDir}/..` + if (parentDir === currentDir) { + // reached root directory + return null + } + + return findFileInAncestors(fileName, parentDir) } - const parentDir = `${currentDir}/..` - if (parentDir === currentDir) { - // reached root directory - return null - } + const packageJsonFile = findFileInAncestors(file, process.cwd()) + const content = readFileSync(packageJsonFile!, "utf-8") + const parsedContent = JSON.parse(content) + return parsedContent + } - return findFileInAncestors(fileName, parentDir) + let localVersion: string | undefined + if (isDev()) { + try { + const lerna = getParentFile("lerna.json") + localVersion = lerna.version + } catch { + // + } } try { - const packageJsonFile = findFileInAncestors("package.json", process.cwd()) - const content = readFileSync(packageJsonFile!, "utf-8") - const parsedContent = JSON.parse(content) + const parsedContent = getParentFile("package.json") return { - VERSION: process.env.BUDIBASE_VERSION || parsedContent.version, + VERSION: + localVersion || process.env.BUDIBASE_VERSION || parsedContent.version, SERVICE_NAME: parsedContent.name, } } catch { From 15bb730c591f4b8879485ee20e36cc74c8414bc1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 11:37:41 +0200 Subject: [PATCH 35/41] Remove power role for apps created at >= 3.0.0 --- packages/backend-core/src/security/roles.ts | 49 ++++++++++++++++----- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index fa2d114d7d..ea4cb1b38a 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -1,3 +1,4 @@ +import semver from "semver" import { BuiltinPermissionID, PermissionLevel } from "./permissions" import { prefixRoleID, @@ -7,7 +8,13 @@ import { doWithDB, } from "../db" import { getAppDB } from "../context" -import { Screen, Role as RoleDoc, RoleUIMetadata } from "@budibase/types" +import { + Screen, + Role as RoleDoc, + RoleUIMetadata, + Database, + App, +} from "@budibase/types" import cloneDeep from "lodash/fp/cloneDeep" import { RoleColor } from "@budibase/shared-core" @@ -23,14 +30,6 @@ const BUILTIN_IDS = { BUILDER: "BUILDER", } -// exclude internal roles like builder -const EXTERNAL_BUILTIN_ROLE_IDS = [ - BUILTIN_IDS.ADMIN, - BUILTIN_IDS.POWER, - BUILTIN_IDS.BASIC, - BUILTIN_IDS.PUBLIC, -] - export const RoleIDVersion = { // original version, with a UUID based ID UUID: undefined, @@ -319,7 +318,7 @@ export async function getAllRoles(appId?: string): Promise { } return internal(appDB) } - async function internal(db: any) { + async function internal(db: Database | undefined) { let roles: RoleDoc[] = [] if (db) { const body = await db.allDocs( @@ -334,8 +333,26 @@ export async function getAllRoles(appId?: string): Promise { } const builtinRoles = getBuiltinRoles() + // exclude internal roles like builder + let externalBuiltinRoles = [] + + if (db && !(await shouldIncludePowerRole(db))) { + externalBuiltinRoles = [ + BUILTIN_IDS.ADMIN, + BUILTIN_IDS.POWER, + BUILTIN_IDS.BASIC, + BUILTIN_IDS.PUBLIC, + ] + } else { + externalBuiltinRoles = [ + BUILTIN_IDS.ADMIN, + BUILTIN_IDS.BASIC, + BUILTIN_IDS.PUBLIC, + ] + } + // need to combine builtin with any DB record of them (for sake of permissions) - for (let builtinRoleId of EXTERNAL_BUILTIN_ROLE_IDS) { + for (let builtinRoleId of externalBuiltinRoles) { const builtinRole = builtinRoles[builtinRoleId] const dbBuiltin = roles.filter( dbRole => @@ -366,6 +383,16 @@ export async function getAllRoles(appId?: string): Promise { } } +async function shouldIncludePowerRole(db: Database) { + const app = await db.get(DocumentType.APP_METADATA) + const { creationVersion } = app + if (semver.gte(creationVersion || "", "3.0.0")) { + return true + } + + return false +} + export class AccessController { userHierarchies: { [key: string]: string[] } constructor() { From 1155be45304ca24eb24850a3341b6d33729c5fe9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 11:52:03 +0200 Subject: [PATCH 36/41] Fix --- packages/backend-core/src/security/roles.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index ea4cb1b38a..cd9217d600 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -336,7 +336,7 @@ export async function getAllRoles(appId?: string): Promise { // exclude internal roles like builder let externalBuiltinRoles = [] - if (db && !(await shouldIncludePowerRole(db))) { + if (!db || (await shouldIncludePowerRole(db))) { externalBuiltinRoles = [ BUILTIN_IDS.ADMIN, BUILTIN_IDS.POWER, @@ -386,11 +386,13 @@ export async function getAllRoles(appId?: string): Promise { async function shouldIncludePowerRole(db: Database) { const app = await db.get(DocumentType.APP_METADATA) const { creationVersion } = app - if (semver.gte(creationVersion || "", "3.0.0")) { + if (!creationVersion) { + // Old apps don't have creationVersion, so we should include it for backward compatibility return true } - return false + const isGreaterThan3x = semver.gte(creationVersion, "3.0.0") + return !isGreaterThan3x } export class AccessController { From 8008d2ced1fa11b0e77b99bd98ddf7c27f801e37 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 11:53:20 +0200 Subject: [PATCH 37/41] Fix all references --- packages/backend-core/src/security/roles.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index cd9217d600..2fcde768bc 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -384,8 +384,8 @@ export async function getAllRoles(appId?: string): Promise { } async function shouldIncludePowerRole(db: Database) { - const app = await db.get(DocumentType.APP_METADATA) - const { creationVersion } = app + const app = await db.tryGet(DocumentType.APP_METADATA) + const creationVersion = app?.creationVersion if (!creationVersion) { // Old apps don't have creationVersion, so we should include it for backward compatibility return true From 7bb69d7ffdb61353c12006d7356c87e82e7b09d6 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 12:17:01 +0200 Subject: [PATCH 38/41] Add tests --- packages/backend-core/src/security/roles.ts | 2 +- .../src/api/routes/global/tests/roles.spec.ts | 57 ++++++++++++++++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 2fcde768bc..fad5f7cb74 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -386,7 +386,7 @@ export async function getAllRoles(appId?: string): Promise { async function shouldIncludePowerRole(db: Database) { const app = await db.tryGet(DocumentType.APP_METADATA) const creationVersion = app?.creationVersion - if (!creationVersion) { + if (!creationVersion || !semver.valid(creationVersion)) { // Old apps don't have creationVersion, so we should include it for backward compatibility return true } diff --git a/packages/worker/src/api/routes/global/tests/roles.spec.ts b/packages/worker/src/api/routes/global/tests/roles.spec.ts index 11de06328e..1ca9ff7e29 100644 --- a/packages/worker/src/api/routes/global/tests/roles.spec.ts +++ b/packages/worker/src/api/routes/global/tests/roles.spec.ts @@ -1,6 +1,6 @@ import { structures, TestConfiguration } from "../../../../tests" import { context, db, permissions, roles } from "@budibase/backend-core" -import { Database } from "@budibase/types" +import { App, Database } from "@budibase/types" jest.mock("@budibase/backend-core", () => { const core = jest.requireActual("@budibase/backend-core") @@ -30,6 +30,14 @@ async function addAppMetadata() { }) } +async function updateAppMetadata(update: Partial>) { + const app = await appDb.get("app_metadata") + await appDb.put({ + ...app, + ...update, + }) +} + describe("/api/global/roles", () => { const config = new TestConfiguration() @@ -69,6 +77,53 @@ describe("/api/global/roles", () => { expect(res.body[appId].roles.length).toEqual(5) expect(res.body[appId].roles.map((r: any) => r._id)).toContain(ROLE_NAME) }) + + it.each(["3.0.0", "3.0.1", "3.1.0"])( + "exclude POWER roles after v3 (%s)", + async creationVersion => { + await updateAppMetadata({ creationVersion }) + const res = await config.api.roles.get() + expect(res.body).toBeDefined() + expect(res.body[appId].roles.map((r: any) => r._id)).toEqual([ + ROLE_NAME, + roles.BUILTIN_ROLE_IDS.ADMIN, + roles.BUILTIN_ROLE_IDS.BASIC, + roles.BUILTIN_ROLE_IDS.PUBLIC, + ]) + } + ) + + it.each(["2.9.0", "1.0.0"])( + "include POWER roles before v3 (%s)", + async creationVersion => { + await updateAppMetadata({ creationVersion }) + const res = await config.api.roles.get() + expect(res.body).toBeDefined() + expect(res.body[appId].roles.map((r: any) => r._id)).toEqual([ + ROLE_NAME, + roles.BUILTIN_ROLE_IDS.ADMIN, + roles.BUILTIN_ROLE_IDS.POWER, + roles.BUILTIN_ROLE_IDS.BASIC, + roles.BUILTIN_ROLE_IDS.PUBLIC, + ]) + } + ) + + it.each(["invalid", ""])( + "include POWER roles when the version is corrupted (%s)", + async creationVersion => { + await updateAppMetadata({ creationVersion }) + const res = await config.api.roles.get() + + expect(res.body[appId].roles.map((r: any) => r._id)).toEqual([ + ROLE_NAME, + roles.BUILTIN_ROLE_IDS.ADMIN, + roles.BUILTIN_ROLE_IDS.POWER, + roles.BUILTIN_ROLE_IDS.BASIC, + roles.BUILTIN_ROLE_IDS.PUBLIC, + ]) + } + ) }) describe("GET api/global/roles/:appId", () => { From c1128ffe2a953f3f4f1e1056083541eb8787d2cc Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 12:20:17 +0200 Subject: [PATCH 39/41] Fix test --- packages/backend-core/src/environment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index 87dd2c5e87..4e93e8d9ee 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -80,7 +80,7 @@ function getPackageJsonFields(): { } let localVersion: string | undefined - if (isDev()) { + if (isDev() && !isTest()) { try { const lerna = getParentFile("lerna.json") localVersion = lerna.version From ca974cf2f509f4497f571ef8723de994da29261a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 16:30:22 +0200 Subject: [PATCH 40/41] Renames --- .../server/src/api/controllers/application.ts | 16 ++++++++-------- packages/server/src/sdk/app/appMetadata/index.ts | 5 ----- .../server/src/sdk/app/applications/index.ts | 2 ++ .../{appMetadata => applications}/metadata.ts | 0 packages/server/src/sdk/index.ts | 2 -- 5 files changed, 10 insertions(+), 15 deletions(-) delete mode 100644 packages/server/src/sdk/app/appMetadata/index.ts rename packages/server/src/sdk/app/{appMetadata => applications}/metadata.ts (100%) diff --git a/packages/server/src/api/controllers/application.ts b/packages/server/src/api/controllers/application.ts index b8f523c331..59f67540fe 100644 --- a/packages/server/src/api/controllers/application.ts +++ b/packages/server/src/api/controllers/application.ts @@ -209,7 +209,7 @@ export async function fetchAppPackage( ctx: UserCtx ) { const appId = context.getAppId() - const application = await sdk.appMetadata.get() + const application = await sdk.applications.metadata.get() const layouts = await getLayouts() let screens = await getScreens() const license = await licensing.cache.getCachedLicense() @@ -318,7 +318,7 @@ async function performAppCreate(ctx: UserCtx) { newApplication.creationVersion = envCore.VERSION } - const existing = await sdk.appMetadata.tryGet() + const existing = await sdk.applications.metadata.tryGet() // If we used a template or imported an app there will be an existing doc. // Fetch and migrate some metadata from the existing app. if (existing) { @@ -492,7 +492,7 @@ export async function update( export async function updateClient(ctx: UserCtx) { // Get current app version - const application = await sdk.appMetadata.get() + const application = await sdk.applications.metadata.get() const currentVersion = application.version let manifest @@ -520,7 +520,7 @@ export async function updateClient(ctx: UserCtx) { export async function revertClient(ctx: UserCtx) { // Check app can be reverted - const application = await sdk.appMetadata.get() + const application = await sdk.applications.metadata.get() if (!application.revertableVersion) { ctx.throw(400, "There is no version to revert to") } @@ -578,7 +578,7 @@ async function destroyApp(ctx: UserCtx) { const db = dbCore.getDB(devAppId) // standard app deletion flow - const app = await sdk.appMetadata.get() + const app = await sdk.applications.metadata.get() const result = await db.destroy() await quotas.removeApp() await events.app.deleted(app) @@ -729,7 +729,7 @@ export async function updateAppPackage( ) { return context.doInAppContext(appId, async () => { const db = context.getAppDB() - const application = await sdk.appMetadata.get() + const application = await sdk.applications.metadata.get() const newAppPackage: App = { ...application, ...appPackage } if (appPackage._rev !== application._rev) { @@ -755,7 +755,7 @@ export async function setRevertableVersion( return } const db = context.getAppDB() - const app = await sdk.appMetadata.get() + const app = await sdk.applications.metadata.get() app.revertableVersion = ctx.request.body.revertableVersion await db.put(app) @@ -764,7 +764,7 @@ export async function setRevertableVersion( async function migrateAppNavigation() { const db = context.getAppDB() - const existing = await sdk.appMetadata.get() + const existing = await sdk.applications.metadata.get() const layouts: Layout[] = await getLayouts() const screens: Screen[] = await getScreens() diff --git a/packages/server/src/sdk/app/appMetadata/index.ts b/packages/server/src/sdk/app/appMetadata/index.ts deleted file mode 100644 index 267899a214..0000000000 --- a/packages/server/src/sdk/app/appMetadata/index.ts +++ /dev/null @@ -1,5 +0,0 @@ -import * as metadata from "./metadata" - -export default { - ...metadata, -} diff --git a/packages/server/src/sdk/app/applications/index.ts b/packages/server/src/sdk/app/applications/index.ts index 04ed3b2919..f315e84896 100644 --- a/packages/server/src/sdk/app/applications/index.ts +++ b/packages/server/src/sdk/app/applications/index.ts @@ -2,10 +2,12 @@ import * as sync from "./sync" import * as utils from "./utils" import * as applications from "./applications" import * as imports from "./import" +import * as metadata from "./metadata" export default { ...sync, ...utils, ...applications, ...imports, + metadata, } diff --git a/packages/server/src/sdk/app/appMetadata/metadata.ts b/packages/server/src/sdk/app/applications/metadata.ts similarity index 100% rename from packages/server/src/sdk/app/appMetadata/metadata.ts rename to packages/server/src/sdk/app/applications/metadata.ts diff --git a/packages/server/src/sdk/index.ts b/packages/server/src/sdk/index.ts index 44df8b50fb..a871546b60 100644 --- a/packages/server/src/sdk/index.ts +++ b/packages/server/src/sdk/index.ts @@ -11,7 +11,6 @@ import { default as plugins } from "./plugins" import * as views from "./app/views" import * as permissions from "./app/permissions" import * as rowActions from "./app/rowActions" -import { default as appMetadata } from "./app/appMetadata" const sdk = { backups, @@ -27,7 +26,6 @@ const sdk = { permissions, links, rowActions, - appMetadata, } // default export for TS From 4c688b9734a95b19110b79419b5208928fa19a8d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 16:40:26 +0200 Subject: [PATCH 41/41] Add more tests --- packages/worker/src/api/routes/global/tests/roles.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/roles.spec.ts b/packages/worker/src/api/routes/global/tests/roles.spec.ts index 1ca9ff7e29..28977d8521 100644 --- a/packages/worker/src/api/routes/global/tests/roles.spec.ts +++ b/packages/worker/src/api/routes/global/tests/roles.spec.ts @@ -78,7 +78,7 @@ describe("/api/global/roles", () => { expect(res.body[appId].roles.map((r: any) => r._id)).toContain(ROLE_NAME) }) - it.each(["3.0.0", "3.0.1", "3.1.0"])( + it.each(["3.0.0", "3.0.1", "3.1.0", "3.0.0+2146.b125a7c"])( "exclude POWER roles after v3 (%s)", async creationVersion => { await updateAppMetadata({ creationVersion }) @@ -93,7 +93,7 @@ describe("/api/global/roles", () => { } ) - it.each(["2.9.0", "1.0.0"])( + it.each(["2.9.0", "1.0.0", "0.0.0", "2.32.17+2146.b125a7c"])( "include POWER roles before v3 (%s)", async creationVersion => { await updateAppMetadata({ creationVersion })