From 1e184605a1809310a355fcff9e9251252c99c334 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 4 Mar 2024 13:47:53 +0000 Subject: [PATCH 01/13] Migrate users of switchToSelfHosted to config.withEnv --- .../src/api/routes/tests/apikeys.spec.js | 4 ++-- .../src/api/routes/tests/application.spec.ts | 9 +++++++++ .../api/routes/tests/queries/query.seq.spec.ts | 2 +- .../server/src/api/routes/tests/row.spec.ts | 3 +-- .../src/api/routes/tests/utilities/index.ts | 18 ------------------ 5 files changed, 13 insertions(+), 23 deletions(-) diff --git a/packages/server/src/api/routes/tests/apikeys.spec.js b/packages/server/src/api/routes/tests/apikeys.spec.js index eb6933af7d..678da38f28 100644 --- a/packages/server/src/api/routes/tests/apikeys.spec.js +++ b/packages/server/src/api/routes/tests/apikeys.spec.js @@ -13,7 +13,7 @@ describe("/api/keys", () => { describe("fetch", () => { it("should allow fetching", async () => { - await setup.switchToSelfHosted(async () => { + await config.withEnv({ SELF_HOSTED: "true" }, async () => { const res = await request .get(`/api/keys`) .set(config.defaultHeaders()) @@ -34,7 +34,7 @@ describe("/api/keys", () => { describe("update", () => { it("should allow updating a value", async () => { - await setup.switchToSelfHosted(async () => { + await config.withEnv({ SELF_HOSTED: "true" }, async () => { const res = await request .put(`/api/keys/TEST`) .send({ diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index dc235dbd01..3e4ad693db 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -248,4 +248,13 @@ describe("/applications", () => { expect(devLogs.data.length).toBe(0) }) }) + + describe("permissions", () => { + it("should only return apps a user has access to", async () => { + const user = await config.createUser() + + const apps = await config.api.application.fetch() + expect(apps.length).toBeGreaterThan(0) + }) + }) }) diff --git a/packages/server/src/api/routes/tests/queries/query.seq.spec.ts b/packages/server/src/api/routes/tests/queries/query.seq.spec.ts index c5cb188cbc..4347ed9044 100644 --- a/packages/server/src/api/routes/tests/queries/query.seq.spec.ts +++ b/packages/server/src/api/routes/tests/queries/query.seq.spec.ts @@ -157,7 +157,7 @@ describe("/queries", () => { }) it("should find a query in cloud", async () => { - await setup.switchToSelfHosted(async () => { + await config.withEnv({ SELF_HOSTED: "true" }, async () => { const query = await config.createQuery() const res = await request .get(`/api/queries/${query._id}`) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index c02159bb42..de411f5397 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -882,8 +882,7 @@ describe.each([ ], tableId: table._id, }) - // the environment needs configured for this - await setup.switchToSelfHosted(async () => { + await config.withEnv({ SELF_HOSTED: "true" }, async () => { return context.doInAppContext(config.getAppId(), async () => { const enriched = await outputProcessing(table, [row]) expect((enriched as Row[])[0].attachment[0].url).toBe( diff --git a/packages/server/src/api/routes/tests/utilities/index.ts b/packages/server/src/api/routes/tests/utilities/index.ts index 27c178fc38..915ff5d970 100644 --- a/packages/server/src/api/routes/tests/utilities/index.ts +++ b/packages/server/src/api/routes/tests/utilities/index.ts @@ -77,21 +77,3 @@ export function getConfig() { } return config! } - -export async function switchToSelfHosted(func: any) { - // self hosted stops any attempts to Dynamo - env._set("NODE_ENV", "production") - env._set("SELF_HOSTED", true) - let error - try { - await func() - } catch (err) { - error = err - } - env._set("NODE_ENV", "jest") - env._set("SELF_HOSTED", false) - // don't throw error until after reset - if (error) { - throw error - } -} From 7b7d10d1ff35fff240726cc401d57b8ed3c802cb Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 4 Mar 2024 15:47:04 +0000 Subject: [PATCH 02/13] 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 03/13] 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 ee0f0abad25d9e1bcfc499eb2e8b7d4ed3d7b38f Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Mon, 4 Mar 2024 15:55:45 +0000 Subject: [PATCH 04/13] Fix/rename mysql column (#13186) * Rebuild table schema when adding new column to get externalType * Added MySQL integration test suite * Add test for emitting datasource on save new column * Update packages/server/src/integration-test/mysql.spec.ts Co-authored-by: Sam Rose * remove duplicate tests * Use UUID * update account portal * Remove _add for internal save * Internal DB add column unit test * rename column test * update modules * fix tests --------- Co-authored-by: Sam Rose --- packages/account-portal | 2 +- packages/builder/src/stores/builder/tables.js | 6 + packages/pro | 2 +- .../src/api/controllers/table/external.ts | 10 +- .../server/src/api/controllers/table/index.ts | 9 +- .../src/api/controllers/table/internal.ts | 14 +- .../server/src/api/routes/tests/table.spec.ts | 30 ++ .../server/src/integration-test/mysql.spec.ts | 363 ++++++++++++++++++ .../src/sdk/app/tables/external/index.ts | 14 +- .../types/src/documents/app/table/table.ts | 3 +- packages/types/src/sdk/search.ts | 4 + 11 files changed, 440 insertions(+), 17 deletions(-) create mode 100644 packages/server/src/integration-test/mysql.spec.ts diff --git a/packages/account-portal b/packages/account-portal index 19f7a5829f..0c050591c2 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 19f7a5829f4d23cbc694136e45d94482a59a475a +Subproject commit 0c050591c21d3b67dc0c9225d60cc9e2324c8dac diff --git a/packages/builder/src/stores/builder/tables.js b/packages/builder/src/stores/builder/tables.js index 51b8416eda..f86b37ab85 100644 --- a/packages/builder/src/stores/builder/tables.js +++ b/packages/builder/src/stores/builder/tables.js @@ -147,6 +147,12 @@ export function createTablesStore() { if (indexes) { draft.indexes = indexes } + // Add object to indicate if column is being added + if (draft.schema[field.name] === undefined) { + draft._add = { + name: field.name, + } + } draft.schema = { ...draft.schema, [field.name]: cloneDeep(field), diff --git a/packages/pro b/packages/pro index 183b35d3ac..22a278da72 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 183b35d3acd42433dcb2d32bcd89a36abe13afec +Subproject commit 22a278da720d92991dabdcd4cb6c96e7abe29781 diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index f035822068..c85b46a95c 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -6,6 +6,7 @@ import { BulkImportRequest, BulkImportResponse, Operation, + RenameColumn, SaveTableRequest, SaveTableResponse, Table, @@ -25,9 +26,12 @@ function getDatasourceId(table: Table) { return breakExternalTableId(table._id).datasourceId } -export async function save(ctx: UserCtx) { +export async function save( + ctx: UserCtx, + renaming?: RenameColumn +) { const inputs = ctx.request.body - const renaming = inputs?._rename + const adding = inputs?._add // can't do this right now delete inputs.rows const tableId = ctx.request.body._id @@ -40,7 +44,7 @@ export async function save(ctx: UserCtx) { const { datasource, table } = await sdk.tables.external.save( datasourceId!, inputs, - { tableId, renaming } + { tableId, renaming, adding } ) builderSocket?.emitDatasourceUpdate(ctx, datasource) return table diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 55a896373f..69305c461e 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -74,8 +74,15 @@ export async function save(ctx: UserCtx) { const appId = ctx.appId const table = ctx.request.body const isImport = table.rows + const renaming = ctx.request.body._rename - let savedTable = await pickApi({ table }).save(ctx) + const api = pickApi({ table }) + // do not pass _rename or _add if saving to CouchDB + if (api === internal) { + delete ctx.request.body._add + delete ctx.request.body._rename + } + let savedTable = await api.save(ctx, renaming) if (!table._id) { await events.table.created(savedTable) savedTable = sdk.tables.enrichViewSchemas(savedTable) diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index 8e90007d88..eb5e4b6c41 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -12,11 +12,12 @@ import { } from "@budibase/types" import sdk from "../../../sdk" -export async function save(ctx: UserCtx) { +export async function save( + ctx: UserCtx, + renaming?: RenameColumn +) { const { rows, ...rest } = ctx.request.body - let tableToSave: Table & { - _rename?: RenameColumn - } = { + let tableToSave: Table = { _id: generateTableID(), ...rest, // Ensure these fields are populated, even if not sent in the request @@ -28,15 +29,12 @@ export async function save(ctx: UserCtx) { tableToSave.views = {} } - const renaming = tableToSave._rename - delete tableToSave._rename - try { const { table } = await sdk.tables.internal.save(tableToSave, { user: ctx.user, rowsToImport: rows, tableId: ctx.request.body._id, - renaming: renaming, + renaming, }) return table diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 29465145a9..77704a0408 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -26,6 +26,7 @@ import { TableToBuild } from "../../../tests/utilities/TestConfiguration" tk.freeze(mocks.date.MOCK_DATE) const { basicTable } = setup.structures +const ISO_REGEX_PATTERN = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/ describe("/tables", () => { let request = setup.getRequest() @@ -285,6 +286,35 @@ describe("/tables", () => { expect(res.body.schema.roleId).toBeDefined() }) }) + + it("should add a new column for an internal DB table", async () => { + const saveTableRequest: SaveTableRequest = { + _add: { + name: "NEW_COLUMN", + }, + ...basicTable(), + } + + const response = await request + .post(`/api/tables`) + .send(saveTableRequest) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + + const expectedResponse = { + ...saveTableRequest, + _rev: expect.stringMatching(/^\d-.+/), + _id: expect.stringMatching(/^ta_.+/), + createdAt: expect.stringMatching(ISO_REGEX_PATTERN), + updatedAt: expect.stringMatching(ISO_REGEX_PATTERN), + views: {}, + } + delete expectedResponse._add + + expect(response.status).toBe(200) + expect(response.body).toEqual(expectedResponse) + }) }) describe("import", () => { diff --git a/packages/server/src/integration-test/mysql.spec.ts b/packages/server/src/integration-test/mysql.spec.ts new file mode 100644 index 0000000000..fac2bfcfeb --- /dev/null +++ b/packages/server/src/integration-test/mysql.spec.ts @@ -0,0 +1,363 @@ +import fetch from "node-fetch" +import { + generateMakeRequest, + MakeRequestResponse, +} from "../api/routes/public/tests/utils" +import { v4 as uuidv4 } from "uuid" +import * as setup from "../api/routes/tests/utilities" +import { + Datasource, + FieldType, + Table, + TableRequest, + TableSourceType, +} from "@budibase/types" +import _ from "lodash" +import { databaseTestProviders } from "../integrations/tests/utils" +import mysql from "mysql2/promise" +import { builderSocket } from "../websockets" +// @ts-ignore +fetch.mockSearch() + +const config = setup.getConfig()! + +jest.unmock("mysql2/promise") +jest.mock("../websockets", () => ({ + clientAppSocket: jest.fn(), + gridAppSocket: jest.fn(), + initialise: jest.fn(), + builderSocket: { + emitTableUpdate: jest.fn(), + emitTableDeletion: jest.fn(), + emitDatasourceUpdate: jest.fn(), + emitDatasourceDeletion: jest.fn(), + emitScreenUpdate: jest.fn(), + emitAppMetadataUpdate: jest.fn(), + emitAppPublish: jest.fn(), + }, +})) + +describe("mysql integrations", () => { + let makeRequest: MakeRequestResponse, + mysqlDatasource: Datasource, + primaryMySqlTable: Table + + beforeAll(async () => { + await config.init() + const apiKey = await config.generateApiKey() + + makeRequest = generateMakeRequest(apiKey, true) + + mysqlDatasource = await config.api.datasource.create( + await databaseTestProviders.mysql.datasource() + ) + }) + + afterAll(async () => { + await databaseTestProviders.mysql.stop() + }) + + beforeEach(async () => { + primaryMySqlTable = await config.createTable({ + name: uuidv4(), + type: "table", + primary: ["id"], + schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + }, + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + value: { + name: "value", + type: FieldType.NUMBER, + }, + }, + sourceId: mysqlDatasource._id, + sourceType: TableSourceType.EXTERNAL, + }) + }) + + afterAll(config.end) + + it("validate table schema", async () => { + const res = await makeRequest( + "get", + `/api/datasources/${mysqlDatasource._id}` + ) + + expect(res.status).toBe(200) + expect(res.body).toEqual({ + config: { + database: "mysql", + host: mysqlDatasource.config!.host, + password: "--secret-value--", + port: mysqlDatasource.config!.port, + user: "root", + }, + plus: true, + source: "MYSQL", + type: "datasource_plus", + _id: expect.any(String), + _rev: expect.any(String), + createdAt: expect.any(String), + updatedAt: expect.any(String), + entities: expect.any(Object), + }) + }) + + describe("POST /api/datasources/verify", () => { + it("should be able to verify the connection", async () => { + await config.api.datasource.verify( + { + datasource: await databaseTestProviders.mysql.datasource(), + }, + { + body: { + connected: true, + }, + } + ) + }) + + it("should state an invalid datasource cannot connect", async () => { + const dbConfig = await databaseTestProviders.mysql.datasource() + await config.api.datasource.verify( + { + datasource: { + ...dbConfig, + config: { + ...dbConfig.config, + password: "wrongpassword", + }, + }, + }, + { + body: { + connected: false, + error: + "Access denied for the specified user. User does not have the necessary privileges or the provided credentials are incorrect. Please verify the credentials, and ensure that the user has appropriate permissions.", + }, + } + ) + }) + }) + + describe("POST /api/datasources/info", () => { + it("should fetch information about mysql datasource", async () => { + const primaryName = primaryMySqlTable.name + const response = await makeRequest("post", "/api/datasources/info", { + datasource: mysqlDatasource, + }) + expect(response.status).toBe(200) + expect(response.body.tableNames).toBeDefined() + expect(response.body.tableNames.indexOf(primaryName)).not.toBe(-1) + }) + }) + + describe("Integration compatibility with mysql search_path", () => { + let client: mysql.Connection, pathDatasource: Datasource + const database = "test1" + const database2 = "test-2" + + beforeAll(async () => { + const dsConfig = await databaseTestProviders.mysql.datasource() + const dbConfig = dsConfig.config! + + client = await mysql.createConnection(dbConfig) + await client.query(`CREATE DATABASE \`${database}\`;`) + await client.query(`CREATE DATABASE \`${database2}\`;`) + + const pathConfig: any = { + ...dsConfig, + config: { + ...dbConfig, + database, + }, + } + pathDatasource = await config.api.datasource.create(pathConfig) + }) + + afterAll(async () => { + await client.query(`DROP DATABASE \`${database}\`;`) + await client.query(`DROP DATABASE \`${database2}\`;`) + await client.end() + }) + + it("discovers tables from any schema in search path", async () => { + await client.query( + `CREATE TABLE \`${database}\`.table1 (id1 SERIAL PRIMARY KEY);` + ) + const response = await makeRequest("post", "/api/datasources/info", { + datasource: pathDatasource, + }) + expect(response.status).toBe(200) + expect(response.body.tableNames).toBeDefined() + expect(response.body.tableNames).toEqual( + expect.arrayContaining(["table1"]) + ) + }) + + it("does not mix columns from different tables", async () => { + const repeated_table_name = "table_same_name" + await client.query( + `CREATE TABLE \`${database}\`.${repeated_table_name} (id SERIAL PRIMARY KEY, val1 TEXT);` + ) + await client.query( + `CREATE TABLE \`${database2}\`.${repeated_table_name} (id2 SERIAL PRIMARY KEY, val2 TEXT);` + ) + const response = await makeRequest( + "post", + `/api/datasources/${pathDatasource._id}/schema`, + { + tablesFilter: [repeated_table_name], + } + ) + expect(response.status).toBe(200) + expect( + response.body.datasource.entities[repeated_table_name].schema + ).toBeDefined() + const schema = + response.body.datasource.entities[repeated_table_name].schema + expect(Object.keys(schema).sort()).toEqual(["id", "val1"]) + }) + }) + + describe("POST /api/tables/", () => { + let client: mysql.Connection + const emitDatasourceUpdateMock = jest.fn() + + beforeEach(async () => { + client = await mysql.createConnection( + ( + await databaseTestProviders.mysql.datasource() + ).config! + ) + mysqlDatasource = await config.api.datasource.create( + await databaseTestProviders.mysql.datasource() + ) + }) + + afterEach(async () => { + await client.end() + }) + + it("will emit the datasource entity schema with externalType to the front-end when adding a new column", async () => { + const addColumnToTable: TableRequest = { + type: "table", + sourceType: TableSourceType.EXTERNAL, + name: "table", + sourceId: mysqlDatasource._id!, + primary: ["id"], + schema: { + id: { + type: FieldType.AUTO, + name: "id", + autocolumn: true, + }, + new_column: { + type: FieldType.NUMBER, + name: "new_column", + }, + }, + _add: { + name: "new_column", + }, + } + + jest + .spyOn(builderSocket!, "emitDatasourceUpdate") + .mockImplementation(emitDatasourceUpdateMock) + + await makeRequest("post", "/api/tables/", addColumnToTable) + + const expectedTable: TableRequest = { + ...addColumnToTable, + schema: { + id: { + type: FieldType.NUMBER, + name: "id", + autocolumn: true, + constraints: { + presence: false, + }, + externalType: "int unsigned", + }, + new_column: { + type: FieldType.NUMBER, + name: "new_column", + autocolumn: false, + constraints: { + presence: false, + }, + externalType: "float(8,2)", + }, + }, + created: true, + _id: `${mysqlDatasource._id}__table`, + } + delete expectedTable._add + + expect(emitDatasourceUpdateMock).toBeCalledTimes(1) + const emittedDatasource: Datasource = + emitDatasourceUpdateMock.mock.calls[0][1] + expect(emittedDatasource.entities!["table"]).toEqual(expectedTable) + }) + + it("will rename a column", async () => { + await makeRequest("post", "/api/tables/", primaryMySqlTable) + + let renameColumnOnTable: TableRequest = { + ...primaryMySqlTable, + schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + externalType: "unsigned integer", + }, + name: { + name: "name", + type: FieldType.STRING, + externalType: "text", + }, + description: { + name: "description", + type: FieldType.STRING, + externalType: "text", + }, + age: { + name: "age", + type: FieldType.NUMBER, + externalType: "float(8,2)", + }, + }, + } + + const response = await makeRequest( + "post", + "/api/tables/", + renameColumnOnTable + ) + mysqlDatasource = ( + await makeRequest( + "post", + `/api/datasources/${mysqlDatasource._id}/schema` + ) + ).body.datasource + + expect(response.status).toEqual(200) + expect( + Object.keys(mysqlDatasource.entities![primaryMySqlTable.name].schema) + ).toEqual(["id", "name", "description", "age"]) + }) + }) +}) diff --git a/packages/server/src/sdk/app/tables/external/index.ts b/packages/server/src/sdk/app/tables/external/index.ts index 9a2bed0da2..0ace19d00e 100644 --- a/packages/server/src/sdk/app/tables/external/index.ts +++ b/packages/server/src/sdk/app/tables/external/index.ts @@ -3,6 +3,7 @@ import { Operation, RelationshipType, RenameColumn, + AddColumn, Table, TableRequest, ViewV2, @@ -32,7 +33,7 @@ import * as viewSdk from "../../views" export async function save( datasourceId: string, update: Table, - opts?: { tableId?: string; renaming?: RenameColumn } + opts?: { tableId?: string; renaming?: RenameColumn; adding?: AddColumn } ) { let tableToSave: TableRequest = { ...update, @@ -165,8 +166,17 @@ export async function save( // remove the rename prop delete tableToSave._rename + + // if adding a new column, we need to rebuild the schema for that table to get the 'externalType' of the column + if (opts?.adding) { + datasource.entities[tableToSave.name] = ( + await datasourceSdk.buildFilteredSchema(datasource, [tableToSave.name]) + ).tables[tableToSave.name] + } else { + datasource.entities[tableToSave.name] = tableToSave + } + // store it into couch now for budibase reference - datasource.entities[tableToSave.name] = tableToSave await db.put(populateExternalTableSchemas(datasource)) // Since tables are stored inside datasources, we need to notify clients diff --git a/packages/types/src/documents/app/table/table.ts b/packages/types/src/documents/app/table/table.ts index f3b8e6df8d..3b419dd811 100644 --- a/packages/types/src/documents/app/table/table.ts +++ b/packages/types/src/documents/app/table/table.ts @@ -1,6 +1,6 @@ import { Document } from "../../document" import { View, ViewV2 } from "../view" -import { RenameColumn } from "../../../sdk" +import { AddColumn, RenameColumn } from "../../../sdk" import { TableSchema } from "./schema" export const INTERNAL_TABLE_SOURCE_ID = "bb_internal" @@ -29,5 +29,6 @@ export interface Table extends Document { export interface TableRequest extends Table { _rename?: RenameColumn + _add?: AddColumn created?: boolean } diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 67c344d845..7a0ddaed66 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -60,6 +60,10 @@ export interface RenameColumn { updated: string } +export interface AddColumn { + name: string +} + export interface RelationshipsJson { through?: string from?: string From 9d8c18337dc8d438da07a90307ceaff9d9a8951b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 4 Mar 2024 16:42:41 +0000 Subject: [PATCH 05/13] Type role controller. --- packages/backend-core/src/security/roles.ts | 57 +++++++++--------- packages/pro | 2 +- packages/server/src/api/controllers/role.ts | 58 ++++++++++--------- .../server/src/api/controllers/routing.ts | 2 +- .../src/api/routes/tests/application.spec.ts | 11 +++- .../src/api/routes/tests/utilities/index.ts | 7 +-- .../src/tests/utilities/TestConfiguration.ts | 10 ++++ packages/types/src/api/web/index.ts | 1 + packages/types/src/api/web/role.ts | 22 +++++++ 9 files changed, 102 insertions(+), 68 deletions(-) create mode 100644 packages/types/src/api/web/role.ts diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 4f048c0a11..01473ad991 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -84,16 +84,18 @@ export function getBuiltinRoles(): { [key: string]: RoleDoc } { return cloneDeep(BUILTIN_ROLES) } -export const BUILTIN_ROLE_ID_ARRAY = Object.values(BUILTIN_ROLES).map( - role => role._id -) +export function isBuiltin(role: string) { + return getBuiltinRole(role) !== undefined +} -export const BUILTIN_ROLE_NAME_ARRAY = Object.values(BUILTIN_ROLES).map( - role => role.name -) - -export function isBuiltin(role?: string) { - return BUILTIN_ROLE_ID_ARRAY.some(builtin => role?.includes(builtin)) +export function getBuiltinRole(roleId: string): Role | undefined { + const role = Object.values(BUILTIN_ROLES).find(role => + roleId.includes(role._id) + ) + if (!role) { + return undefined + } + return cloneDeep(role) } /** @@ -123,7 +125,7 @@ export function builtinRoleToNumber(id?: string) { /** * Converts any role to a number, but has to be async to get the roles from db. */ -export async function roleToNumber(id?: string) { +export async function roleToNumber(id: string) { if (isBuiltin(id)) { return builtinRoleToNumber(id) } @@ -131,7 +133,7 @@ export async function roleToNumber(id?: string) { defaultPublic: true, })) as RoleDoc[] for (let role of hierarchy) { - if (isBuiltin(role?.inherits)) { + if (role?.inherits && isBuiltin(role.inherits)) { return builtinRoleToNumber(role.inherits) + 1 } } @@ -161,35 +163,28 @@ export function lowerBuiltinRoleID(roleId1?: string, roleId2?: string): string { * @returns The role object, which may contain an "inherits" property. */ export async function getRole( - roleId?: string, + roleId: string, opts?: { defaultPublic?: boolean } -): Promise { - if (!roleId) { - return undefined - } - let role: any = {} +): Promise { // built in roles mostly come from the in-code implementation, // but can be extended by a doc stored about them (e.g. permissions) - if (isBuiltin(roleId)) { - role = cloneDeep( - Object.values(BUILTIN_ROLES).find(role => role._id === roleId) - ) - } else { + let role: RoleDoc | undefined = getBuiltinRole(roleId) + if (!role) { // make sure has the prefix (if it has it then it won't be added) roleId = prefixRoleID(roleId) } try { const db = getAppDB() - const dbRole = await db.get(getDBRoleID(roleId)) - role = Object.assign(role, dbRole) + const dbRole = await db.get(getDBRoleID(roleId)) + role = Object.assign(role || {}, dbRole) // finalise the ID - role._id = getExternalRoleID(role._id, role.version) + role._id = getExternalRoleID(role._id!, role.version) } catch (err) { if (!isBuiltin(roleId) && opts?.defaultPublic) { return cloneDeep(BUILTIN_ROLES.PUBLIC) } // only throw an error if there is no role at all - if (Object.keys(role).length === 0) { + if (!role || Object.keys(role).length === 0) { throw err } } @@ -200,7 +195,7 @@ export async function getRole( * Simple function to get all the roles based on the top level user role ID. */ async function getAllUserRoles( - userRoleId?: string, + userRoleId: string, opts?: { defaultPublic?: boolean } ): Promise { // admins have access to all roles @@ -226,7 +221,7 @@ async function getAllUserRoles( } export async function getUserRoleIdHierarchy( - userRoleId?: string + userRoleId: string ): Promise { const roles = await getUserRoleHierarchy(userRoleId) return roles.map(role => role._id!) @@ -241,7 +236,7 @@ export async function getUserRoleIdHierarchy( * highest level of access and the last being the lowest level. */ export async function getUserRoleHierarchy( - userRoleId?: string, + userRoleId: string, opts?: { defaultPublic?: boolean } ) { // special case, if they don't have a role then they are a public user @@ -265,9 +260,9 @@ export function checkForRoleResourceArray( return rolePerms } -export async function getAllRoleIds(appId?: string) { +export async function getAllRoleIds(appId: string): Promise { const roles = await getAllRoles(appId) - return roles.map(role => role._id) + return roles.map(role => role._id!) } /** diff --git a/packages/pro b/packages/pro index 183b35d3ac..22a278da72 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 183b35d3acd42433dcb2d32bcd89a36abe13afec +Subproject commit 22a278da720d92991dabdcd4cb6c96e7abe29781 diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index ae6b89e6d4..ffc1d74209 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -7,8 +7,14 @@ import { } from "@budibase/backend-core" import { getUserMetadataParams, InternalTables } from "../../db/utils" import { + AccessibleRolesResponse, Database, + DestroyRoleResponse, + FetchRolesResponse, + FindRoleResponse, Role, + SaveRoleRequest, + SaveRoleResponse, UserCtx, UserMetadata, UserRoles, @@ -25,43 +31,35 @@ async function updateRolesOnUserTable( db: Database, roleId: string, updateOption: string, - roleVersion: string | undefined + roleVersion?: string ) { const table = await sdk.tables.getTable(InternalTables.USER_METADATA) - const schema = table.schema - const remove = updateOption === UpdateRolesOptions.REMOVED - let updated = false - for (let prop of Object.keys(schema)) { - if (prop === "roleId") { - updated = true - const constraints = schema[prop].constraints! - const updatedRoleId = - roleVersion === roles.RoleIDVersion.NAME - ? roles.getExternalRoleID(roleId, roleVersion) - : roleId - const indexOfRoleId = constraints.inclusion!.indexOf(updatedRoleId) - if (remove && indexOfRoleId !== -1) { - constraints.inclusion!.splice(indexOfRoleId, 1) - } else if (!remove && indexOfRoleId === -1) { - constraints.inclusion!.push(updatedRoleId) - } - break + 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) } - } - if (updated) { await db.put(table) } } -export async function fetch(ctx: UserCtx) { +export async function fetch(ctx: UserCtx) { ctx.body = await roles.getAllRoles() } -export async function find(ctx: UserCtx) { +export async function find(ctx: UserCtx) { ctx.body = await roles.getRole(ctx.params.roleId) } -export async function save(ctx: UserCtx) { +export async function save(ctx: UserCtx) { const db = context.getAppDB() let { _id, name, inherits, permissionId, version } = ctx.request.body let isCreate = false @@ -109,9 +107,9 @@ export async function save(ctx: UserCtx) { ctx.body = role } -export async function destroy(ctx: UserCtx) { +export async function destroy(ctx: UserCtx) { const db = context.getAppDB() - let roleId = ctx.params.roleId + let roleId = ctx.params.roleId as string if (roles.isBuiltin(roleId)) { ctx.throw(400, "Cannot delete builtin role.") } else { @@ -144,14 +142,18 @@ export async function destroy(ctx: UserCtx) { ctx.status = 200 } -export async function accessible(ctx: UserCtx) { +export async function accessible(ctx: UserCtx) { let roleId = ctx.user?.roleId if (!roleId) { roleId = roles.BUILTIN_ROLE_IDS.PUBLIC } if (ctx.user && sharedSdk.users.isAdminOrBuilder(ctx.user)) { const appId = context.getAppId() - ctx.body = await roles.getAllRoleIds(appId) + if (!appId) { + ctx.body = [] + } else { + ctx.body = await roles.getAllRoleIds(appId) + } } else { ctx.body = await roles.getUserRoleIdHierarchy(roleId!) } diff --git a/packages/server/src/api/controllers/routing.ts b/packages/server/src/api/controllers/routing.ts index 4154c6b597..040cda4dd0 100644 --- a/packages/server/src/api/controllers/routing.ts +++ b/packages/server/src/api/controllers/routing.ts @@ -63,7 +63,7 @@ export async function fetch(ctx: UserCtx) { export async function clientFetch(ctx: UserCtx) { const routing = await getRoutingStructure() let roleId = ctx.user?.role?._id - const roleIds = await roles.getUserRoleIdHierarchy(roleId) + const roleIds = roleId ? await roles.getUserRoleIdHierarchy(roleId) : [] for (let topLevel of Object.values(routing.routes) as any) { for (let subpathKey of Object.keys(topLevel.subpaths)) { let found = false diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index 3e4ad693db..5a3be462e8 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -251,10 +251,15 @@ describe("/applications", () => { describe("permissions", () => { it("should only return apps a user has access to", async () => { - const user = await config.createUser() + const user = await config.createUser({ + builder: { global: false }, + admin: { global: false }, + }) - const apps = await config.api.application.fetch() - expect(apps.length).toBeGreaterThan(0) + await config.withUser(user, async () => { + const apps = await config.api.application.fetch() + expect(apps).toHaveLength(0) + }) }) }) }) diff --git a/packages/server/src/api/routes/tests/utilities/index.ts b/packages/server/src/api/routes/tests/utilities/index.ts index 915ff5d970..dcb8ccd6c0 100644 --- a/packages/server/src/api/routes/tests/utilities/index.ts +++ b/packages/server/src/api/routes/tests/utilities/index.ts @@ -1,5 +1,4 @@ -import TestConfig from "../../../../tests/utilities/TestConfiguration" -import env from "../../../../environment" +import TestConfiguration from "../../../../tests/utilities/TestConfiguration" import supertest from "supertest" export * as structures from "../../../../tests/utilities/structures" @@ -47,10 +46,10 @@ export function delay(ms: number) { } let request: supertest.SuperTest | undefined | null, - config: TestConfig | null + config: TestConfiguration | null export function beforeAll() { - config = new TestConfig() + config = new TestConfiguration() request = config.getRequest() } diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 35ca2982c0..2127e9d1cd 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -299,6 +299,16 @@ export default class TestConfiguration { } } + withUser(user: User, f: () => Promise) { + const oldUser = this.user + this.user = user + try { + return f() + } finally { + this.user = oldUser + } + } + // UTILS _req | void, Res>( diff --git a/packages/types/src/api/web/index.ts b/packages/types/src/api/web/index.ts index 9a688a17a5..8a091afdba 100644 --- a/packages/types/src/api/web/index.ts +++ b/packages/types/src/api/web/index.ts @@ -14,3 +14,4 @@ export * from "./cookies" export * from "./automation" export * from "./layout" export * from "./query" +export * from "./role" diff --git a/packages/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts new file mode 100644 index 0000000000..c37dee60e0 --- /dev/null +++ b/packages/types/src/api/web/role.ts @@ -0,0 +1,22 @@ +import { Role } from "../../documents" + +export interface SaveRoleRequest { + _id?: string + _rev?: string + name: string + inherits: string + permissionId: string + version: string +} + +export interface SaveRoleResponse extends Role {} + +export interface FindRoleResponse extends Role {} + +export type FetchRolesResponse = Role[] + +export interface DestroyRoleResponse { + message: string +} + +export type AccessibleRolesResponse = string[] From a7fcd7cca16ff9d143ed9babf3b44ed34fe6c4ae Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 4 Mar 2024 16:55:10 +0000 Subject: [PATCH 06/13] 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 07/13] 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 08/13] 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 09/13] 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 10/13] 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 11/13] 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 12/13] 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) { From 488cfea1f432c1dad91680d71afafeacdee5229b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 5 Mar 2024 14:40:29 +0000 Subject: [PATCH 13/13] Fix typing. --- packages/backend-core/src/security/roles.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 213c65e18e..01473ad991 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 (Object.keys(role || {}).length === 0) { + if (!role || Object.keys(role).length === 0) { throw err } }