From 7b7d10d1ff35fff240726cc401d57b8ed3c802cb Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 4 Mar 2024 15:47:04 +0000 Subject: [PATCH 1/9] Fixing an issue with returning statements in MS-SQL and MySQL, when creating, updating or deleting rows the response is not retrieved correctly when aliased. --- .../server/src/integrations/tests/sqlQueryJson/createSimple.json | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 packages/server/src/integrations/tests/sqlQueryJson/createSimple.json diff --git a/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json b/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json new file mode 100644 index 0000000000..e69de29bb2 From 02e3b36cd80575acb9c408ab70a64d5b542e3433 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 4 Mar 2024 15:47:27 +0000 Subject: [PATCH 2/9] Adding missing files. --- .../server/src/api/controllers/row/alias.ts | 99 ++++++++++--------- packages/server/src/integrations/base/sql.ts | 34 ++++++- .../src/integrations/tests/sqlAlias.spec.ts | 23 +++++ .../tests/sqlQueryJson/createSimple.json | 65 ++++++++++++ .../types/src/documents/app/table/table.ts | 1 + 5 files changed, 172 insertions(+), 50 deletions(-) diff --git a/packages/server/src/api/controllers/row/alias.ts b/packages/server/src/api/controllers/row/alias.ts index 9658a0d638..4bd1e0a2ff 100644 --- a/packages/server/src/api/controllers/row/alias.ts +++ b/packages/server/src/api/controllers/row/alias.ts @@ -107,57 +107,62 @@ export default class AliasTables { } async queryWithAliasing(json: QueryJson): DatasourcePlusQueryResponse { - json = cloneDeep(json) - const aliasTable = (table: Table) => ({ - ...table, - name: this.getAlias(table.name), - }) - // run through the query json to update anywhere a table may be used - if (json.resource?.fields) { - json.resource.fields = json.resource.fields.map(field => - this.aliasField(field) - ) - } - if (json.filters) { - for (let [filterKey, filter] of Object.entries(json.filters)) { - if (typeof filter !== "object") { - continue - } - const aliasedFilters: typeof filter = {} - for (let key of Object.keys(filter)) { - aliasedFilters[this.aliasField(key)] = filter[key] - } - json.filters[filterKey as keyof SearchFilters] = aliasedFilters + const fieldLength = json.resource?.fields?.length + const aliasingEnabled = fieldLength && fieldLength > 0 + if (aliasingEnabled) { + json = cloneDeep(json) + const aliasTable = (table: Table) => ({ + ...table, + name: this.getAlias(table.name), + originalName: table.name, + }) + // run through the query json to update anywhere a table may be used + if (json.resource?.fields) { + json.resource.fields = json.resource.fields.map(field => + this.aliasField(field) + ) } - } - if (json.relationships) { - json.relationships = json.relationships.map(relationship => ({ - ...relationship, - aliases: this.aliasMap([ - relationship.through, - relationship.tableName, - json.endpoint.entityId, - ]), - })) - } - if (json.meta?.table) { - json.meta.table = aliasTable(json.meta.table) - } - if (json.meta?.tables) { - const aliasedTables: Record = {} - for (let [tableName, table] of Object.entries(json.meta.tables)) { - aliasedTables[this.getAlias(tableName)] = aliasTable(table) + if (json.filters) { + for (let [filterKey, filter] of Object.entries(json.filters)) { + if (typeof filter !== "object") { + continue + } + const aliasedFilters: typeof filter = {} + for (let key of Object.keys(filter)) { + aliasedFilters[this.aliasField(key)] = filter[key] + } + json.filters[filterKey as keyof SearchFilters] = aliasedFilters + } } - json.meta.tables = aliasedTables + if (json.relationships) { + json.relationships = json.relationships.map(relationship => ({ + ...relationship, + aliases: this.aliasMap([ + relationship.through, + relationship.tableName, + json.endpoint.entityId, + ]), + })) + } + if (json.meta?.table) { + json.meta.table = aliasTable(json.meta.table) + } + if (json.meta?.tables) { + const aliasedTables: Record = {} + for (let [tableName, table] of Object.entries(json.meta.tables)) { + aliasedTables[this.getAlias(tableName)] = aliasTable(table) + } + json.meta.tables = aliasedTables + } + // invert and return + const invertedTableAliases: Record = {} + for (let [key, value] of Object.entries(this.tableAliases)) { + invertedTableAliases[value] = key + } + json.tableAliases = invertedTableAliases } - // invert and return - const invertedTableAliases: Record = {} - for (let [key, value] of Object.entries(this.tableAliases)) { - invertedTableAliases[value] = key - } - json.tableAliases = invertedTableAliases const response = await getDatasourceAndQuery(json) - if (Array.isArray(response)) { + if (Array.isArray(response) && aliasingEnabled) { return this.reverse(response) } else { return response diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index 6605052598..5467232924 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -9,9 +9,12 @@ import { RelationshipsJson, SearchFilters, SortDirection, + Table, } from "@budibase/types" import environment from "../../environment" +type QueryFunction = (query: Knex.SqlNative, operation: Operation) => any + const envLimit = environment.SQL_MAX_ROWS ? parseInt(environment.SQL_MAX_ROWS) : null @@ -117,6 +120,29 @@ function generateSelectStatement( }) } +function disableAliasing(json: QueryJson) { + if (json.tableAliases) { + json.tableAliases = undefined + } + const removeTableAlias = (table: Table) => { + if (table.originalName) { + table.name = table.originalName + } + return table + } + if (json.meta?.table) { + json.meta.table = removeTableAlias(json.meta.table) + } + if (json.meta?.tables) { + for (let tableName of Object.keys(json.meta.tables)) { + json.meta.tables[tableName] = removeTableAlias( + json.meta.tables[tableName] + ) + } + } + return json +} + class InternalBuilder { private readonly client: string @@ -605,10 +631,12 @@ class SqlQueryBuilder extends SqlTableQueryBuilder { return query.toSQL().toNative() } - async getReturningRow(queryFn: Function, json: QueryJson) { + async getReturningRow(queryFn: QueryFunction, json: QueryJson) { if (!json.extra || !json.extra.idFilter) { return {} } + // disable aliasing if it is enabled + json = disableAliasing(json) const input = this._query({ endpoint: { ...json.endpoint, @@ -617,7 +645,7 @@ class SqlQueryBuilder extends SqlTableQueryBuilder { resource: { fields: [], }, - filters: json.extra.idFilter, + filters: json.extra?.idFilter, paginate: { limit: 1, }, @@ -646,7 +674,7 @@ class SqlQueryBuilder extends SqlTableQueryBuilder { // this function recreates the returning functionality of postgres async queryWithReturning( json: QueryJson, - queryFn: Function, + queryFn: QueryFunction, processFn: Function = (result: any) => result ) { const sqlClient = this.getSqlClient() diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 9b3f6a1b38..c93e3916ce 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -4,6 +4,7 @@ import Sql from "../base/sql" import { SqlClient } from "../utils" import AliasTables from "../../api/controllers/row/alias" import { generator } from "@budibase/backend-core/tests" +import { Knex } from "knex" function multiline(sql: string) { return sql.replace(/\n/g, "").replace(/ +/g, " ") @@ -160,6 +161,28 @@ describe("Captures of real examples", () => { }) }) + describe("returning (everything bar Postgres)", () => { + it("should be able to handle row returning", () => { + const queryJson = getJson("createSimple.json") + const SQL = new Sql(SqlClient.MS_SQL, limit) + let query = SQL._query(queryJson, { disableReturning: true }) + expect(query).toEqual({ + sql: "insert into [people] ([age], [name]) values (@p0, @p1)", + bindings: [222, "awfawf"], + }) + + // now check returning + let returningQuery: Knex.SqlNative = { sql: "", bindings: [] } + SQL.getReturningRow((input: Knex.SqlNative) => { + 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]", + bindings: [1, "awfawf", 222], + }) + }) + }) + describe("check max character aliasing", () => { it("should handle over 'z' max character alias", () => { const tableNames = [] diff --git a/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json b/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json index e69de29bb2..42fae2325e 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json @@ -0,0 +1,65 @@ +{ + "endpoint": { + "datasourceId": "datasource_plus_0ed5835e5552496285df546030f7c4ae", + "entityId": "people", + "operation": "CREATE" + }, + "resource": { + "fields": [ + "a.name", + "a.age" + ] + }, + "filters": {}, + "relationships": [], + "body": { + "name": "awfawf", + "age": 222 + }, + "extra": { + "idFilter": { + "equal": { + "name": "awfawf", + "age": 222 + } + } + }, + "meta": { + "table": { + "_id": "datasource_plus_0ed5835e5552496285df546030f7c4ae__people", + "type": "table", + "sourceId": "datasource_plus_0ed5835e5552496285df546030f7c4ae", + "sourceType": "external", + "primary": [ + "name", + "age" + ], + "name": "a", + "schema": { + "name": { + "type": "string", + "externalType": "varchar", + "autocolumn": false, + "name": "name", + "constraints": { + "presence": true + } + }, + "age": { + "type": "number", + "externalType": "int", + "autocolumn": false, + "name": "age", + "constraints": { + "presence": false + } + } + }, + "primaryDisplay": "name", + "originalName": "people" + } + }, + "tableAliases": { + "people": "a" + } +} \ No newline at end of file diff --git a/packages/types/src/documents/app/table/table.ts b/packages/types/src/documents/app/table/table.ts index 3b419dd811..b284e9a840 100644 --- a/packages/types/src/documents/app/table/table.ts +++ b/packages/types/src/documents/app/table/table.ts @@ -15,6 +15,7 @@ export interface Table extends Document { sourceType: TableSourceType views?: { [key: string]: View | ViewV2 } name: string + originalName?: string sourceId: string primary?: string[] schema: TableSchema From a7fcd7cca16ff9d143ed9babf3b44ed34fe6c4ae Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 4 Mar 2024 16:55:10 +0000 Subject: [PATCH 3/9] Attempting to fix issue with table name needing to be aliased. --- .../scripts/integrations/mysql/init.sql | 6 ++++ .../server/src/api/controllers/row/alias.ts | 29 ++++++++--------- packages/server/src/integrations/base/sql.ts | 32 +++---------------- .../basicFetchWithRelationships.json | 2 +- .../tests/sqlQueryJson/createSimple.json | 5 ++- .../sqlQueryJson/createWithRelationships.json | 2 +- .../tests/sqlQueryJson/deleteSimple.json | 2 +- .../sqlQueryJson/enrichRelationship.json | 2 +- .../tests/sqlQueryJson/fetchManyToMany.json | 2 +- .../sqlQueryJson/filterByRelationship.json | 2 +- .../sqlQueryJson/manyRelationshipFilters.json | 2 +- .../sqlQueryJson/updateRelationship.json | 2 +- .../tests/sqlQueryJson/updateSimple.json | 2 +- 13 files changed, 35 insertions(+), 55 deletions(-) diff --git a/packages/server/scripts/integrations/mysql/init.sql b/packages/server/scripts/integrations/mysql/init.sql index ae5cd07788..e687c7c3b1 100644 --- a/packages/server/scripts/integrations/mysql/init.sql +++ b/packages/server/scripts/integrations/mysql/init.sql @@ -10,6 +10,11 @@ CREATE TABLE Persons ( City varchar(255), PRIMARY KEY (PersonID) ); +CREATE TABLE Person ( + PersonID int NOT NULL AUTO_INCREMENT, + Name varchar(255), + PRIMARY KEY (PersonID) +); CREATE TABLE Tasks ( TaskID int NOT NULL AUTO_INCREMENT, PersonID INT, @@ -27,6 +32,7 @@ CREATE TABLE Products ( ); INSERT INTO Persons (FirstName, LastName, Age, Address, City, CreatedAt) VALUES ('Mike', 'Hughes', 28.2, '123 Fake Street', 'Belfast', '2021-01-19 03:14:07'); INSERT INTO Persons (FirstName, LastName, Age, Address, City, CreatedAt) VALUES ('Dave', 'Johnson', 29, '124 Fake Street', 'Belfast', '2022-04-01 00:11:11'); +INSERT INTO Person (Name) VALUES ('Elf'); INSERT INTO Tasks (PersonID, TaskName, CreatedAt) VALUES (1, 'assembling', '2020-01-01'); INSERT INTO Tasks (PersonID, TaskName, CreatedAt) VALUES (2, 'processing', '2019-12-31'); INSERT INTO Products (name, updated) VALUES ('Meat', '11:00:22'), ('Fruit', '10:00:00'); diff --git a/packages/server/src/api/controllers/row/alias.ts b/packages/server/src/api/controllers/row/alias.ts index 4bd1e0a2ff..46b090bb97 100644 --- a/packages/server/src/api/controllers/row/alias.ts +++ b/packages/server/src/api/controllers/row/alias.ts @@ -62,7 +62,11 @@ export default class AliasTables { if (idx === -1 || idx > 1) { return } - return Math.abs(tableName.length - name.length) <= 2 + // this might look a bit mad, but the idea is if the field is wrapped, say in "", `` or [] + // then the idx of the table name will be 1, and we should allow for it ending in a closing + // character - otherwise it should be the full length if the index is zero + const allowedCharacterDiff = idx * 2 + return Math.abs(tableName.length - name.length) <= allowedCharacterDiff }) if (foundTableName) { const aliasedTableName = tableName.replace( @@ -111,11 +115,6 @@ export default class AliasTables { const aliasingEnabled = fieldLength && fieldLength > 0 if (aliasingEnabled) { json = cloneDeep(json) - const aliasTable = (table: Table) => ({ - ...table, - name: this.getAlias(table.name), - originalName: table.name, - }) // run through the query json to update anywhere a table may be used if (json.resource?.fields) { json.resource.fields = json.resource.fields.map(field => @@ -134,6 +133,14 @@ export default class AliasTables { json.filters[filterKey as keyof SearchFilters] = aliasedFilters } } + if (json.meta?.table) { + this.getAlias(json.meta.table.name) + } + if (json.meta?.tables) { + Object.keys(json.meta.tables).forEach(tableName => + this.getAlias(tableName) + ) + } if (json.relationships) { json.relationships = json.relationships.map(relationship => ({ ...relationship, @@ -144,16 +151,6 @@ export default class AliasTables { ]), })) } - if (json.meta?.table) { - json.meta.table = aliasTable(json.meta.table) - } - if (json.meta?.tables) { - const aliasedTables: Record = {} - for (let [tableName, table] of Object.entries(json.meta.tables)) { - aliasedTables[this.getAlias(tableName)] = aliasTable(table) - } - json.meta.tables = aliasedTables - } // invert and return const invertedTableAliases: Record = {} for (let [key, value] of Object.entries(this.tableAliases)) { diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index 5467232924..8c5066533e 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -120,29 +120,6 @@ function generateSelectStatement( }) } -function disableAliasing(json: QueryJson) { - if (json.tableAliases) { - json.tableAliases = undefined - } - const removeTableAlias = (table: Table) => { - if (table.originalName) { - table.name = table.originalName - } - return table - } - if (json.meta?.table) { - json.meta.table = removeTableAlias(json.meta.table) - } - if (json.meta?.tables) { - for (let tableName of Object.keys(json.meta.tables)) { - json.meta.tables[tableName] = removeTableAlias( - json.meta.tables[tableName] - ) - } - } - return json -} - class InternalBuilder { private readonly client: string @@ -348,15 +325,18 @@ class InternalBuilder { addSorting(query: Knex.QueryBuilder, json: QueryJson): Knex.QueryBuilder { let { sort, paginate } = json const table = json.meta?.table + const aliases = json.tableAliases + const aliased = + table?.name && aliases?.[table.name] ? aliases[table.name] : table?.name if (sort && Object.keys(sort || {}).length > 0) { for (let [key, value] of Object.entries(sort)) { const direction = value.direction === SortDirection.ASCENDING ? "asc" : "desc" - query = query.orderBy(`${table?.name}.${key}`, direction) + query = query.orderBy(`${aliased}.${key}`, direction) } } else if (this.client === SqlClient.MS_SQL && paginate?.limit) { // @ts-ignore - query = query.orderBy(`${table?.name}.${table?.primary[0]}`) + query = query.orderBy(`${aliased}.${table?.primary[0]}`) } return query } @@ -635,8 +615,6 @@ class SqlQueryBuilder extends SqlTableQueryBuilder { if (!json.extra || !json.extra.idFilter) { return {} } - // disable aliasing if it is enabled - json = disableAliasing(json) const input = this._query({ endpoint: { ...json.endpoint, diff --git a/packages/server/src/integrations/tests/sqlQueryJson/basicFetchWithRelationships.json b/packages/server/src/integrations/tests/sqlQueryJson/basicFetchWithRelationships.json index 3445f5fe67..ba7fa4ef9b 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/basicFetchWithRelationships.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/basicFetchWithRelationships.json @@ -68,7 +68,7 @@ "primary": [ "personid" ], - "name": "a", + "name": "persons", "schema": { "year": { "type": "number", diff --git a/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json b/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json index 42fae2325e..5c69f270dd 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json @@ -34,7 +34,7 @@ "name", "age" ], - "name": "a", + "name": "people", "schema": { "name": { "type": "string", @@ -55,8 +55,7 @@ } } }, - "primaryDisplay": "name", - "originalName": "people" + "primaryDisplay": "name" } }, "tableAliases": { diff --git a/packages/server/src/integrations/tests/sqlQueryJson/createWithRelationships.json b/packages/server/src/integrations/tests/sqlQueryJson/createWithRelationships.json index 20331b949a..82d85c417b 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/createWithRelationships.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/createWithRelationships.json @@ -58,7 +58,7 @@ "primary": [ "personid" ], - "name": "a", + "name": "persons", "schema": { "year": { "type": "number", diff --git a/packages/server/src/integrations/tests/sqlQueryJson/deleteSimple.json b/packages/server/src/integrations/tests/sqlQueryJson/deleteSimple.json index 2266b8c8be..d6e099c4b6 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/deleteSimple.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/deleteSimple.json @@ -34,7 +34,7 @@ "keypartone", "keyparttwo" ], - "name": "a", + "name": "compositetable", "schema": { "keyparttwo": { "type": "string", diff --git a/packages/server/src/integrations/tests/sqlQueryJson/enrichRelationship.json b/packages/server/src/integrations/tests/sqlQueryJson/enrichRelationship.json index ee658aed18..d71f0552c6 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/enrichRelationship.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/enrichRelationship.json @@ -49,7 +49,7 @@ "primary": [ "taskid" ], - "name": "a", + "name": "tasks", "schema": { "executorid": { "type": "number", diff --git a/packages/server/src/integrations/tests/sqlQueryJson/fetchManyToMany.json b/packages/server/src/integrations/tests/sqlQueryJson/fetchManyToMany.json index 682ebaab2d..cec2fdb025 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/fetchManyToMany.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/fetchManyToMany.json @@ -63,7 +63,7 @@ "primary": [ "productid" ], - "name": "a", + "name": "products", "schema": { "productname": { "type": "string", diff --git a/packages/server/src/integrations/tests/sqlQueryJson/filterByRelationship.json b/packages/server/src/integrations/tests/sqlQueryJson/filterByRelationship.json index eb1025f382..399cb0f4d2 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/filterByRelationship.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/filterByRelationship.json @@ -53,7 +53,7 @@ "primary": [ "productid" ], - "name": "a", + "name": "products", "schema": { "productname": { "type": "string", diff --git a/packages/server/src/integrations/tests/sqlQueryJson/manyRelationshipFilters.json b/packages/server/src/integrations/tests/sqlQueryJson/manyRelationshipFilters.json index afa0889450..2b5d156546 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/manyRelationshipFilters.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/manyRelationshipFilters.json @@ -109,7 +109,7 @@ "primary": [ "taskid" ], - "name": "a", + "name": "tasks", "schema": { "executorid": { "type": "number", diff --git a/packages/server/src/integrations/tests/sqlQueryJson/updateRelationship.json b/packages/server/src/integrations/tests/sqlQueryJson/updateRelationship.json index 01e795bd6c..42c2a44335 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/updateRelationship.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/updateRelationship.json @@ -66,7 +66,7 @@ "primary": [ "personid" ], - "name": "a", + "name": "persons", "schema": { "year": { "type": "number", diff --git a/packages/server/src/integrations/tests/sqlQueryJson/updateSimple.json b/packages/server/src/integrations/tests/sqlQueryJson/updateSimple.json index 01e795bd6c..42c2a44335 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/updateSimple.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/updateSimple.json @@ -66,7 +66,7 @@ "primary": [ "personid" ], - "name": "a", + "name": "persons", "schema": { "year": { "type": "number", From 86aefcfe1a2d4a4b237b62ececd08ae434ea0e63 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 4 Mar 2024 17:00:34 +0000 Subject: [PATCH 4/9] Updating test values. --- packages/server/src/integrations/tests/sqlAlias.spec.ts | 4 ++-- .../src/integrations/tests/sqlQueryJson/createSimple.json | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index c93e3916ce..fe9798aaa0 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -168,7 +168,7 @@ describe("Captures of real examples", () => { let query = SQL._query(queryJson, { disableReturning: true }) expect(query).toEqual({ sql: "insert into [people] ([age], [name]) values (@p0, @p1)", - bindings: [222, "awfawf"], + bindings: [22, "Test"], }) // now check returning @@ -178,7 +178,7 @@ describe("Captures of real examples", () => { }, 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]", - bindings: [1, "awfawf", 222], + bindings: [1, "Test", 22], }) }) }) diff --git a/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json b/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json index 5c69f270dd..33a88d30e1 100644 --- a/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json +++ b/packages/server/src/integrations/tests/sqlQueryJson/createSimple.json @@ -13,14 +13,14 @@ "filters": {}, "relationships": [], "body": { - "name": "awfawf", - "age": 222 + "name": "Test", + "age": 22 }, "extra": { "idFilter": { "equal": { - "name": "awfawf", - "age": 222 + "name": "Test", + "age": 22 } } }, From 45702ce91882c6c7f3005f3327ed312953f031ed Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 4 Mar 2024 17:02:52 +0000 Subject: [PATCH 5/9] Fixing typing. --- packages/server/src/integrations/base/sql.ts | 1 - packages/types/src/documents/app/table/table.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index 8c5066533e..c8acb606b3 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -9,7 +9,6 @@ import { RelationshipsJson, SearchFilters, SortDirection, - Table, } from "@budibase/types" import environment from "../../environment" diff --git a/packages/types/src/documents/app/table/table.ts b/packages/types/src/documents/app/table/table.ts index b284e9a840..3b419dd811 100644 --- a/packages/types/src/documents/app/table/table.ts +++ b/packages/types/src/documents/app/table/table.ts @@ -15,7 +15,6 @@ export interface Table extends Document { sourceType: TableSourceType views?: { [key: string]: View | ViewV2 } name: string - originalName?: string sourceId: string primary?: string[] schema: TableSchema From de0cdd25d11d539a18b3aa34947281bdd4a4667c Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Mon, 4 Mar 2024 17:37:22 +0000 Subject: [PATCH 6/9] Bump version to 2.21.1 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index c06173fe04..dc4485f1de 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.21.0", + "version": "2.21.1", "npmClient": "yarn", "packages": [ "packages/*", From 8694b8d772d0d6ac855a2167080400bc16dda3fb Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Tue, 5 Mar 2024 09:03:19 +0000 Subject: [PATCH 7/9] Relationship picker incorrectly renders selections (#13175) * Ensure _id is decoded for external search * Fetch initial value for 'Update' type forms * test didn't run locally - might run on github workflow * Tested and appears to be as before * Null-pointer fix * undo type change * update modules * add test * update modules --- .../app/forms/RelationshipField.svelte | 27 +++- .../src/sdk/app/rows/search/external.ts | 18 ++- .../app/rows/search/tests/external.spec.ts | 115 ++++++++++-------- .../server/src/sdk/app/rows/search/utils.ts | 1 - 4 files changed, 102 insertions(+), 59 deletions(-) diff --git a/packages/client/src/components/app/forms/RelationshipField.svelte b/packages/client/src/components/app/forms/RelationshipField.svelte index 7cd571f6d9..1fbd0df522 100644 --- a/packages/client/src/components/app/forms/RelationshipField.svelte +++ b/packages/client/src/components/app/forms/RelationshipField.svelte @@ -1,7 +1,7 @@ { + const ids = breakRowIdField(row) + return ids[0] + }) + } + try { const table = await sdk.tables.getTable(tableId) options = searchInputMapping(table, options) @@ -119,9 +131,7 @@ export async function exportRows( requestQuery = { oneOf: { _id: rowIds.map((row: string) => { - const ids = JSON.parse( - decodeURI(row).replace(/'/g, `"`).replace(/%2C/g, ",") - ) + const ids = breakRowIdField(row) if (ids.length > 1) { throw new HTTPError( "Export data does not support composite keys.", diff --git a/packages/server/src/sdk/app/rows/search/tests/external.spec.ts b/packages/server/src/sdk/app/rows/search/tests/external.spec.ts index 1aaea8e258..bae84592ca 100644 --- a/packages/server/src/sdk/app/rows/search/tests/external.spec.ts +++ b/packages/server/src/sdk/app/rows/search/tests/external.spec.ts @@ -21,10 +21,11 @@ jest.unmock("mysql2/promise") jest.setTimeout(30000) -describe.skip("external", () => { +describe("external search", () => { const config = new TestConfiguration() let externalDatasource: Datasource, tableData: Table + const rows: Row[] = [] beforeAll(async () => { const container = await new GenericContainer("mysql") @@ -89,67 +90,81 @@ describe.skip("external", () => { }, }, } + + const table = await config.createExternalTable({ + ...tableData, + sourceId: externalDatasource._id, + }) + for (let i = 0; i < 10; i++) { + rows.push( + await config.createRow({ + tableId: table._id, + name: generator.first(), + surname: generator.last(), + age: generator.age(), + address: generator.address(), + }) + ) + } }) - describe("search", () => { - const rows: Row[] = [] - beforeAll(async () => { - const table = await config.createExternalTable({ - ...tableData, - sourceId: externalDatasource._id, - }) - for (let i = 0; i < 10; i++) { - rows.push( - await config.createRow({ - tableId: table._id, - name: generator.first(), - surname: generator.last(), - age: generator.age(), - address: generator.address(), - }) - ) + it("default search returns all the data", async () => { + await config.doInContext(config.appId, async () => { + const tableId = config.table!._id! + + const searchParams: SearchParams = { + tableId, + query: {}, } + const result = await search(searchParams) + + expect(result.rows).toHaveLength(10) + expect(result.rows).toEqual( + expect.arrayContaining(rows.map(r => expect.objectContaining(r))) + ) }) + }) - it("default search returns all the data", async () => { - await config.doInContext(config.appId, async () => { - const tableId = config.table!._id! + it("querying by fields will always return data attribute columns", async () => { + await config.doInContext(config.appId, async () => { + const tableId = config.table!._id! - const searchParams: SearchParams = { - tableId, - query: {}, - } - const result = await search(searchParams) + const searchParams: SearchParams = { + tableId, + query: {}, + fields: ["name", "age"], + } + const result = await search(searchParams) - expect(result.rows).toHaveLength(10) - expect(result.rows).toEqual( - expect.arrayContaining(rows.map(r => expect.objectContaining(r))) + expect(result.rows).toHaveLength(10) + expect(result.rows).toEqual( + expect.arrayContaining( + rows.map(r => ({ + ...expectAnyExternalColsAttributes, + name: r.name, + age: r.age, + })) ) - }) + ) }) + }) - it("querying by fields will always return data attribute columns", async () => { - await config.doInContext(config.appId, async () => { - const tableId = config.table!._id! + it("will decode _id in oneOf query", async () => { + await config.doInContext(config.appId, async () => { + const tableId = config.table!._id! - const searchParams: SearchParams = { - tableId, - query: {}, - fields: ["name", "age"], - } - const result = await search(searchParams) + const searchParams: SearchParams = { + tableId, + query: { + oneOf: { + _id: ["%5B1%5D", "%5B4%5D", "%5B8%5D"], + }, + }, + } + const result = await search(searchParams) - expect(result.rows).toHaveLength(10) - expect(result.rows).toEqual( - expect.arrayContaining( - rows.map(r => ({ - ...expectAnyExternalColsAttributes, - name: r.name, - age: r.age, - })) - ) - ) - }) + expect(result.rows).toHaveLength(3) + expect(result.rows.map(row => row.id)).toEqual([1, 4, 8]) }) }) }) diff --git a/packages/server/src/sdk/app/rows/search/utils.ts b/packages/server/src/sdk/app/rows/search/utils.ts index 4eee3cea41..5d93dcaca2 100644 --- a/packages/server/src/sdk/app/rows/search/utils.ts +++ b/packages/server/src/sdk/app/rows/search/utils.ts @@ -1,6 +1,5 @@ import { FieldType, - FieldTypeSubtypes, SearchParams, Table, DocumentType, From bc8fdeea6af82e8537751f3c785dbebf6e580211 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 5 Mar 2024 09:04:26 +0000 Subject: [PATCH 8/9] Bump version to 2.21.2 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index dc4485f1de..6fb032ac77 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.21.1", + "version": "2.21.2", "npmClient": "yarn", "packages": [ "packages/*", From fced2f369649410d9e8ea7e4ca381fef3d0afb12 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 5 Mar 2024 09:23:48 +0000 Subject: [PATCH 9/9] Respond to PR feedback. --- packages/backend-core/src/security/roles.ts | 2 +- packages/server/src/api/controllers/role.ts | 27 +++++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 01473ad991..213c65e18e 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -184,7 +184,7 @@ export async function getRole( return cloneDeep(BUILTIN_ROLES.PUBLIC) } // only throw an error if there is no role at all - if (!role || Object.keys(role).length === 0) { + if (Object.keys(role || {}).length === 0) { throw err } } diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index ffc1d74209..b3eb61a255 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -35,20 +35,21 @@ async function updateRolesOnUserTable( ) { const table = await sdk.tables.getTable(InternalTables.USER_METADATA) const constraints = table.schema.roleId?.constraints - if (constraints) { - const updatedRoleId = - roleVersion === roles.RoleIDVersion.NAME - ? roles.getExternalRoleID(roleId, roleVersion) - : roleId - const indexOfRoleId = constraints.inclusion!.indexOf(updatedRoleId) - const remove = updateOption === UpdateRolesOptions.REMOVED - if (remove && indexOfRoleId !== -1) { - constraints.inclusion!.splice(indexOfRoleId, 1) - } else if (!remove && indexOfRoleId === -1) { - constraints.inclusion!.push(updatedRoleId) - } - await db.put(table) + if (!constraints) { + return } + const updatedRoleId = + roleVersion === roles.RoleIDVersion.NAME + ? roles.getExternalRoleID(roleId, roleVersion) + : roleId + const indexOfRoleId = constraints.inclusion!.indexOf(updatedRoleId) + const remove = updateOption === UpdateRolesOptions.REMOVED + if (remove && indexOfRoleId !== -1) { + constraints.inclusion!.splice(indexOfRoleId, 1) + } else if (!remove && indexOfRoleId === -1) { + constraints.inclusion!.push(updatedRoleId) + } + await db.put(table) } export async function fetch(ctx: UserCtx) {