From 5fabe14f6261e2ef7c75129ffa2b34941eae4305 Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Mon, 4 Mar 2024 12:20:27 +0000 Subject: [PATCH] Revert "Rebuild table schema when adding new column to get externalType (#13165)" (#13184) This reverts commit a59647e1580932f6ca278ed933e93c84582a52e5. --- packages/account-portal | 2 +- packages/builder/src/stores/builder/tables.js | 6 - .../src/api/controllers/table/external.ts | 3 +- .../server/src/integration-test/mysql.spec.ts | 309 ------------------ .../src/sdk/app/tables/external/index.ts | 14 +- .../types/src/documents/app/table/table.ts | 3 +- packages/types/src/sdk/search.ts | 4 - 7 files changed, 5 insertions(+), 336 deletions(-) delete mode 100644 packages/server/src/integration-test/mysql.spec.ts diff --git a/packages/account-portal b/packages/account-portal index 806b6fd5c1..19f7a5829f 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 806b6fd5c11c284ebf4a01627d75db939f0f8152 +Subproject commit 19f7a5829f4d23cbc694136e45d94482a59a475a diff --git a/packages/builder/src/stores/builder/tables.js b/packages/builder/src/stores/builder/tables.js index f86b37ab85..51b8416eda 100644 --- a/packages/builder/src/stores/builder/tables.js +++ b/packages/builder/src/stores/builder/tables.js @@ -147,12 +147,6 @@ 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/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index f3478af83b..f035822068 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -28,7 +28,6 @@ function getDatasourceId(table: Table) { export async function save(ctx: UserCtx) { 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 @@ -41,7 +40,7 @@ export async function save(ctx: UserCtx) { const { datasource, table } = await sdk.tables.external.save( datasourceId!, inputs, - { tableId, renaming, adding } + { tableId, renaming } ) builderSocket?.emitDatasourceUpdate(ctx, datasource) return table diff --git a/packages/server/src/integration-test/mysql.spec.ts b/packages/server/src/integration-test/mysql.spec.ts deleted file mode 100644 index f7d0388c62..0000000000 --- a/packages/server/src/integration-test/mysql.spec.ts +++ /dev/null @@ -1,309 +0,0 @@ -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 () => { - const response = await config.api.datasource.verify({ - datasource: await databaseTestProviders.mysql.datasource(), - }) - expect(response.status).toBe(200) - expect(response.body.connected).toBe(true) - }) - - it("should state an invalid datasource cannot connect", async () => { - const dbConfig = await databaseTestProviders.mysql.datasource() - const response = await config.api.datasource.verify({ - datasource: { - ...dbConfig, - config: { - ...dbConfig.config, - password: "wrongpassword", - }, - }, - }) - - expect(response.status).toBe(200) - expect(response.body.connected).toBe(false) - expect(response.body.error).toBeDefined() - }) - }) - - 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! - ) - }) - - afterEach(async () => { - await client.end() - }) - - it("will emit the datasource entity schema with externalType to the front-end when adding a new column", async () => { - mysqlDatasource = ( - await makeRequest( - "post", - `/api/datasources/${mysqlDatasource._id}/schema` - ) - ).body.datasource - - 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) - }) - }) -}) diff --git a/packages/server/src/sdk/app/tables/external/index.ts b/packages/server/src/sdk/app/tables/external/index.ts index 0ace19d00e..9a2bed0da2 100644 --- a/packages/server/src/sdk/app/tables/external/index.ts +++ b/packages/server/src/sdk/app/tables/external/index.ts @@ -3,7 +3,6 @@ import { Operation, RelationshipType, RenameColumn, - AddColumn, Table, TableRequest, ViewV2, @@ -33,7 +32,7 @@ import * as viewSdk from "../../views" export async function save( datasourceId: string, update: Table, - opts?: { tableId?: string; renaming?: RenameColumn; adding?: AddColumn } + opts?: { tableId?: string; renaming?: RenameColumn } ) { let tableToSave: TableRequest = { ...update, @@ -166,17 +165,8 @@ 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 3b419dd811..f3b8e6df8d 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 { AddColumn, RenameColumn } from "../../../sdk" +import { RenameColumn } from "../../../sdk" import { TableSchema } from "./schema" export const INTERNAL_TABLE_SOURCE_ID = "bb_internal" @@ -29,6 +29,5 @@ 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 7a0ddaed66..67c344d845 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -60,10 +60,6 @@ export interface RenameColumn { updated: string } -export interface AddColumn { - name: string -} - export interface RelationshipsJson { through?: string from?: string