From 6e1962e6ea58dbdb20ed401f231a80d740559dfb Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 12 Oct 2023 12:07:13 +0100 Subject: [PATCH 01/15] Plumb Google Sheets table fetching error through to buildSchemaFromDb endpoint. --- .../server/src/api/controllers/datasource.ts | 12 +++++++++- packages/server/src/constants/index.ts | 1 + .../server/src/integrations/googlesheets.ts | 24 ++++++++++++++++--- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 399d5f1d0c..76c29be538 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -79,7 +79,6 @@ async function buildSchemaHelper(datasource: Datasource) { let error = null if (errors && Object.keys(errors).length > 0) { const noKey = getErrorTables(errors, BuildSchemaErrors.NO_KEY) - const invalidCol = getErrorTables(errors, BuildSchemaErrors.INVALID_COLUMN) if (noKey.length) { error = updateError( error, @@ -87,6 +86,8 @@ async function buildSchemaHelper(datasource: Datasource) { noKey ) } + + const invalidCol = getErrorTables(errors, BuildSchemaErrors.INVALID_COLUMN) if (invalidCol.length) { const invalidCols = Object.values(InvalidColumns).join(", ") error = updateError( @@ -95,6 +96,15 @@ async function buildSchemaHelper(datasource: Datasource) { invalidCol ) } + + const noHeader = getErrorTables(errors, BuildSchemaErrors.NO_HEADER_ROW) + if (noHeader.length) { + error = updateError( + error, + `No header row found in the following:`, + noHeader + ) + } } return { tables: connector.tables, error } } diff --git a/packages/server/src/constants/index.ts b/packages/server/src/constants/index.ts index 326389996d..ccd4aaf485 100644 --- a/packages/server/src/constants/index.ts +++ b/packages/server/src/constants/index.ts @@ -162,6 +162,7 @@ export enum InvalidColumns { export enum BuildSchemaErrors { NO_KEY = "no_key", INVALID_COLUMN = "invalid_column", + NO_HEADER_ROW = "no_header_row", } export enum AutomationErrors { diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index 5360d6b319..0ee3df12df 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -21,7 +21,7 @@ import { GoogleSpreadsheet, GoogleSpreadsheetRow } from "google-spreadsheet" import fetch from "node-fetch" import { cache, configs, context, HTTPError } from "@budibase/backend-core" import { dataFilters, utils } from "@budibase/shared-core" -import { GOOGLE_SHEETS_PRIMARY_KEY } from "../constants" +import { BuildSchemaErrors, GOOGLE_SHEETS_PRIMARY_KEY } from "../constants" interface GoogleSheetsConfig { spreadsheetId: string @@ -289,11 +289,29 @@ class GoogleSheetsIntegration implements DatasourcePlus { await this.connect() const sheets = this.client.sheetsByIndex const tables: Record = {} + const errors: Record = {} await utils.parallelForeach( sheets, async sheet => { // must fetch rows to determine schema - await sheet.getRows() + try { + await sheet.getRows() + } catch (err) { + // We expect this to always be an Error so if it's not, rethrow it to + // make sure we don't fail quietly. + if (!(err instanceof Error)) { + throw err; + } + + if (err.message.startsWith("No values in the header row")) { + errors[sheet.title] = BuildSchemaErrors.NO_HEADER_ROW + } else { + // If we get an error we don't have a BuildSchemaErrors enum variant + // for, rethrow to avoid failing quietly. + throw err; + } + return + } const id = buildExternalTableId(datasourceId, sheet.title) tables[sheet.title] = this.getTableSchema( @@ -307,7 +325,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { ) const final = finaliseExternalTables(tables, entities) this.tables = final.tables - this.schemaErrors = final.errors + this.schemaErrors = { ...final.errors, ...errors } } async query(json: QueryJson) { From 85b3af2971b60841b86426d9168397693dd06805 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 12 Oct 2023 16:27:18 +0100 Subject: [PATCH 02/15] Refactor DatasourcePlus's buildSchema to return a value, rather than rely on member variables. --- .../builder/src/stores/backend/datasources.js | 15 +---- .../server/src/api/controllers/datasource.ts | 64 ++++--------------- .../server/src/integrations/googlesheets.ts | 17 ++--- .../src/integrations/microsoftSqlServer.ts | 11 ++-- packages/server/src/integrations/mysql.ts | 11 ++-- packages/server/src/integrations/oracle.ts | 9 ++- packages/server/src/integrations/postgres.ts | 11 ++-- packages/server/src/integrations/utils.ts | 48 ++++---------- packages/types/src/sdk/datasources.ts | 8 +-- 9 files changed, 56 insertions(+), 138 deletions(-) diff --git a/packages/builder/src/stores/backend/datasources.js b/packages/builder/src/stores/backend/datasources.js index 7d2db44d6a..ddfef20336 100644 --- a/packages/builder/src/stores/backend/datasources.js +++ b/packages/builder/src/stores/backend/datasources.js @@ -25,7 +25,6 @@ export function createDatasourcesStore() { const store = writable({ list: [], selectedDatasourceId: null, - schemaError: null, }) const derivedStore = derived([store, tables], ([$store, $tables]) => { @@ -75,18 +74,13 @@ export function createDatasourcesStore() { store.update(state => ({ ...state, selectedDatasourceId: id, - // Remove any possible schema error - schemaError: null, })) } const updateDatasource = response => { const { datasource, error } = response if (error) { - store.update(state => ({ - ...state, - schemaError: error, - })) + store.update(state => ({ ...state })) } replaceDatasource(datasource._id, datasource) select(datasource._id) @@ -171,12 +165,6 @@ export function createDatasourcesStore() { replaceDatasource(datasource._id, null) } - const removeSchemaError = () => { - store.update(state => { - return { ...state, schemaError: null } - }) - } - const replaceDatasource = (datasourceId, datasource) => { if (!datasourceId) { return @@ -229,7 +217,6 @@ export function createDatasourcesStore() { create, update, delete: deleteDatasource, - removeSchemaError, replaceDatasource, getTableNames, } diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 76c29be538..cfbca8eff3 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -18,6 +18,7 @@ import { FetchDatasourceInfoResponse, IntegrationBase, SourceName, + Table, UpdateDatasourceResponse, UserCtx, VerifyDatasourceRequest, @@ -71,46 +72,18 @@ async function getAndMergeDatasource(datasource: Datasource) { return await sdk.datasources.enrich(enrichedDatasource) } -async function buildSchemaHelper(datasource: Datasource) { +async function buildSchemaHelper( + datasource: Datasource +): Promise> { const connector = (await getConnector(datasource)) as DatasourcePlus - await connector.buildSchema(datasource._id!, datasource.entities!) - - const errors = connector.schemaErrors - let error = null - if (errors && Object.keys(errors).length > 0) { - const noKey = getErrorTables(errors, BuildSchemaErrors.NO_KEY) - if (noKey.length) { - error = updateError( - error, - "No primary key constraint found for the following:", - noKey - ) - } - - const invalidCol = getErrorTables(errors, BuildSchemaErrors.INVALID_COLUMN) - if (invalidCol.length) { - const invalidCols = Object.values(InvalidColumns).join(", ") - error = updateError( - error, - `Cannot use columns ${invalidCols} found in following:`, - invalidCol - ) - } - - const noHeader = getErrorTables(errors, BuildSchemaErrors.NO_HEADER_ROW) - if (noHeader.length) { - error = updateError( - error, - `No header row found in the following:`, - noHeader - ) - } - } - return { tables: connector.tables, error } + return await connector.buildSchema(datasource._id!, datasource.entities!) } -async function buildFilteredSchema(datasource: Datasource, filter?: string[]) { - let { tables, error } = await buildSchemaHelper(datasource) +async function buildFilteredSchema( + datasource: Datasource, + filter?: string[] +): Promise<{ tables: Record }> { + let tables = await buildSchemaHelper(datasource) let finalTables = tables if (filter) { finalTables = {} @@ -122,7 +95,7 @@ async function buildFilteredSchema(datasource: Datasource, filter?: string[]) { } } } - return { tables: finalTables, error } + return { tables: finalTables } } export async function fetch(ctx: UserCtx) { @@ -166,7 +139,7 @@ export async function buildSchemaFromDb(ctx: UserCtx) { const tablesFilter = ctx.request.body.tablesFilter const datasource = await sdk.datasources.get(ctx.params.datasourceId) - const { tables, error } = await buildFilteredSchema(datasource, tablesFilter) + const { tables } = await buildFilteredSchema(datasource, tablesFilter) datasource.entities = tables setDefaultDisplayColumns(datasource) @@ -177,9 +150,6 @@ export async function buildSchemaFromDb(ctx: UserCtx) { const cleanedDatasource = await sdk.datasources.removeSecretSingle(datasource) const res: any = { datasource: cleanedDatasource } - if (error) { - res.error = error - } ctx.body = res } @@ -308,13 +278,8 @@ export async function save( type: plus ? DocumentType.DATASOURCE_PLUS : DocumentType.DATASOURCE, } - let schemaError = null if (fetchSchema) { - const { tables, error } = await buildFilteredSchema( - datasource, - tablesFilter - ) - schemaError = error + const { tables } = await buildFilteredSchema(datasource, tablesFilter) datasource.entities = tables setDefaultDisplayColumns(datasource) } @@ -340,9 +305,6 @@ export async function save( const response: CreateDatasourceResponse = { datasource: await sdk.datasources.removeSecretSingle(datasource), } - if (schemaError) { - response.error = schemaError - } ctx.body = response builderSocket?.emitDatasourceUpdate(ctx, datasource) } diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index 0ee3df12df..c744a8f207 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -14,6 +14,7 @@ import { SortJson, ExternalTable, TableRequest, + Table, } from "@budibase/types" import { OAuth2Client } from "google-auth-library" import { buildExternalTableId, finaliseExternalTables } from "./utils" @@ -138,8 +139,6 @@ const SCHEMA: Integration = { class GoogleSheetsIntegration implements DatasourcePlus { private readonly config: GoogleSheetsConfig private client: GoogleSpreadsheet - public tables: Record = {} - public schemaErrors: Record = {} constructor(config: GoogleSheetsConfig) { this.config = config @@ -280,11 +279,11 @@ class GoogleSheetsIntegration implements DatasourcePlus { async buildSchema( datasourceId: string, - entities: Record - ) { + entities: Record + ): Promise> { // not fully configured yet if (!this.config.auth) { - return + return {} } await this.connect() const sheets = this.client.sheetsByIndex @@ -300,7 +299,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { // We expect this to always be an Error so if it's not, rethrow it to // make sure we don't fail quietly. if (!(err instanceof Error)) { - throw err; + throw err } if (err.message.startsWith("No values in the header row")) { @@ -308,7 +307,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { } else { // If we get an error we don't have a BuildSchemaErrors enum variant // for, rethrow to avoid failing quietly. - throw err; + throw err } return } @@ -323,9 +322,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { }, 10 ) - const final = finaliseExternalTables(tables, entities) - this.tables = final.tables - this.schemaErrors = { ...final.errors, ...errors } + return finaliseExternalTables(tables, entities) } async query(json: QueryJson) { diff --git a/packages/server/src/integrations/microsoftSqlServer.ts b/packages/server/src/integrations/microsoftSqlServer.ts index cd62e590d8..783096a7dd 100644 --- a/packages/server/src/integrations/microsoftSqlServer.ts +++ b/packages/server/src/integrations/microsoftSqlServer.ts @@ -11,6 +11,7 @@ import { DatasourceFeature, ConnectionInfo, SourceName, + Table, } from "@budibase/types" import { getSqlQuery, @@ -380,8 +381,8 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { */ async buildSchema( datasourceId: string, - entities: Record - ) { + entities: Record + ): Promise> { await this.connect() let tableInfo: MSSQLTablesResponse[] = await this.runSQL(this.TABLES_SQL) if (tableInfo == null || !Array.isArray(tableInfo)) { @@ -394,7 +395,7 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { .map((record: any) => record.TABLE_NAME) .filter((name: string) => this.MASTER_TABLES.indexOf(name) === -1) - const tables: Record = {} + const tables: Record = {} for (let tableName of tableNames) { // get the column definition (type) const definition = await this.runSQL( @@ -445,9 +446,7 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { schema, } } - const final = finaliseExternalTables(tables, entities) - this.tables = final.tables - this.schemaErrors = final.errors + return finaliseExternalTables(tables, entities) } async queryTableNames() { diff --git a/packages/server/src/integrations/mysql.ts b/packages/server/src/integrations/mysql.ts index 8a688c5f3b..90d0cd256c 100644 --- a/packages/server/src/integrations/mysql.ts +++ b/packages/server/src/integrations/mysql.ts @@ -10,6 +10,7 @@ import { DatasourceFeature, ConnectionInfo, SourceName, + Table, } from "@budibase/types" import { getSqlQuery, @@ -278,9 +279,9 @@ class MySQLIntegration extends Sql implements DatasourcePlus { async buildSchema( datasourceId: string, - entities: Record - ) { - const tables: { [key: string]: ExternalTable } = {} + entities: Record + ): Promise> { + const tables: { [key: string]: Table } = {} await this.connect() try { @@ -328,9 +329,7 @@ class MySQLIntegration extends Sql implements DatasourcePlus { } finally { await this.disconnect() } - const final = finaliseExternalTables(tables, entities) - this.tables = final.tables - this.schemaErrors = final.errors + return finaliseExternalTables(tables, entities) } async queryTableNames() { diff --git a/packages/server/src/integrations/oracle.ts b/packages/server/src/integrations/oracle.ts index 38f0a9d5ac..fc4cd45cf6 100644 --- a/packages/server/src/integrations/oracle.ts +++ b/packages/server/src/integrations/oracle.ts @@ -9,6 +9,7 @@ import { DatasourcePlus, DatasourceFeature, ConnectionInfo, + Table, } from "@budibase/types" import { buildExternalTableId, @@ -264,8 +265,8 @@ class OracleIntegration extends Sql implements DatasourcePlus { */ async buildSchema( datasourceId: string, - entities: Record - ) { + entities: Record + ): Promise> { const columnsResponse = await this.internalQuery({ sql: this.COLUMNS_SQL, }) @@ -326,9 +327,7 @@ class OracleIntegration extends Sql implements DatasourcePlus { }) }) - const final = finaliseExternalTables(tables, entities) - this.tables = final.tables - this.schemaErrors = final.errors + return finaliseExternalTables(tables, entities) } async getTableNames() { diff --git a/packages/server/src/integrations/postgres.ts b/packages/server/src/integrations/postgres.ts index c4b7c2bb65..76487f7b65 100644 --- a/packages/server/src/integrations/postgres.ts +++ b/packages/server/src/integrations/postgres.ts @@ -10,6 +10,7 @@ import { DatasourceFeature, ConnectionInfo, SourceName, + Table, } from "@budibase/types" import { getSqlQuery, @@ -145,8 +146,6 @@ class PostgresIntegration extends Sql implements DatasourcePlus { private readonly config: PostgresConfig private index: number = 1 private open: boolean - public tables: Record = {} - public schemaErrors: Record = {} COLUMNS_SQL!: string @@ -273,8 +272,8 @@ class PostgresIntegration extends Sql implements DatasourcePlus { */ async buildSchema( datasourceId: string, - entities: Record - ) { + entities: Record + ): Promise> { let tableKeys: { [key: string]: string[] } = {} await this.openConnection() try { @@ -342,9 +341,7 @@ class PostgresIntegration extends Sql implements DatasourcePlus { } } - const final = finaliseExternalTables(tables, entities) - this.tables = final.tables - this.schemaErrors = final.errors + return finaliseExternalTables(tables, entities) } catch (err) { // @ts-ignore throw new Error(err) diff --git a/packages/server/src/integrations/utils.ts b/packages/server/src/integrations/utils.ts index db562473e3..6525bd07e2 100644 --- a/packages/server/src/integrations/utils.ts +++ b/packages/server/src/integrations/utils.ts @@ -4,13 +4,10 @@ import { SearchFilters, Datasource, FieldType, + ExternalTable, } from "@budibase/types" import { DocumentType, SEPARATOR } from "../db/utils" -import { - BuildSchemaErrors, - InvalidColumns, - NoEmptyFilterStrings, -} from "../constants" +import { NoEmptyFilterStrings } from "../constants" import { helpers } from "@budibase/shared-core" const DOUBLE_SEPARATOR = `${SEPARATOR}${SEPARATOR}` @@ -266,9 +263,9 @@ export function shouldCopySpecialColumn( function copyExistingPropsOver( tableName: string, table: Table, - entities: { [key: string]: any }, - tableIds: [string] -) { + entities: Record, + tableIds: string[] +): Table { if (entities && entities[tableName]) { if (entities[tableName]?.primaryDisplay) { table.primaryDisplay = entities[tableName].primaryDisplay @@ -295,42 +292,23 @@ function copyExistingPropsOver( /** * Look through the final table definitions to see if anything needs to be - * copied over from the old and if any errors have occurred mark them so - * that the user can be made aware. + * copied over from the old. * @param tables The list of tables that have been retrieved from the external database. * @param entities The old list of tables, if there was any to look for definitions in. */ export function finaliseExternalTables( - tables: { [key: string]: any }, - entities: { [key: string]: any } -) { - const invalidColumns = Object.values(InvalidColumns) - let finalTables: { [key: string]: any } = {} - const errors: { [key: string]: string } = {} - // @ts-ignore - const tableIds: [string] = Object.values(tables).map(table => table._id) + tables: Record, + entities: Record +): Record { + let finalTables: Record = {} + const tableIds = Object.values(tables).map(table => table._id!) for (let [name, table] of Object.entries(tables)) { - const schemaFields = Object.keys(table.schema) - // make sure every table has a key - if (table.primary == null || table.primary.length === 0) { - errors[name] = BuildSchemaErrors.NO_KEY - continue - } else if ( - schemaFields.find(field => - invalidColumns.includes(field as InvalidColumns) - ) - ) { - errors[name] = BuildSchemaErrors.INVALID_COLUMN - continue - } - // make sure all previous props have been added back finalTables[name] = copyExistingPropsOver(name, table, entities, tableIds) } - // sort the tables by name - finalTables = Object.entries(finalTables) + // sort the tables by name (TODO(samwho): why?) + return Object.entries(finalTables) .sort(([a], [b]) => a.localeCompare(b)) .reduce((r, [k, v]) => ({ ...r, [k]: v }), {}) - return { tables: finalTables, errors } } /** diff --git a/packages/types/src/sdk/datasources.ts b/packages/types/src/sdk/datasources.ts index d6a0d4a7c8..2896bc62f5 100644 --- a/packages/types/src/sdk/datasources.ts +++ b/packages/types/src/sdk/datasources.ts @@ -175,13 +175,13 @@ export interface IntegrationBase { } export interface DatasourcePlus extends IntegrationBase { - tables: Record - schemaErrors: Record - // if the datasource supports the use of bindings directly (to protect against SQL injection) // this returns the format of the identifier getBindingIdentifier(): string getStringConcat(parts: string[]): string - buildSchema(datasourceId: string, entities: Record): any + buildSchema( + datasourceId: string, + entities: Record + ): Promise> getTableNames(): Promise } From 1faf920c676a96deafe9e3189aa1872344500a1f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 12 Oct 2023 16:38:15 +0100 Subject: [PATCH 03/15] DatasourcePlus deals exclusively in ExternalTables, reflect that in the types. --- packages/server/src/api/controllers/datasource.ts | 8 ++++++-- packages/server/src/integrations/googlesheets.ts | 5 ++--- packages/server/src/integrations/microsoftSqlServer.ts | 7 +++---- packages/server/src/integrations/mysql.ts | 7 +++---- packages/server/src/integrations/oracle.ts | 5 ++--- packages/server/src/integrations/postgres.ts | 5 ++--- packages/server/src/integrations/utils.ts | 6 +++--- packages/types/src/sdk/datasources.ts | 6 +++--- 8 files changed, 24 insertions(+), 25 deletions(-) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index cfbca8eff3..ad0ed06b83 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -14,6 +14,7 @@ import { CreateDatasourceResponse, Datasource, DatasourcePlus, + ExternalTable, FetchDatasourceInfoRequest, FetchDatasourceInfoResponse, IntegrationBase, @@ -74,9 +75,12 @@ async function getAndMergeDatasource(datasource: Datasource) { async function buildSchemaHelper( datasource: Datasource -): Promise> { +): Promise> { const connector = (await getConnector(datasource)) as DatasourcePlus - return await connector.buildSchema(datasource._id!, datasource.entities!) + return await connector.buildSchema( + datasource._id!, + datasource.entities! as Record + ) } async function buildFilteredSchema( diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index c744a8f207..b895f68e78 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -14,7 +14,6 @@ import { SortJson, ExternalTable, TableRequest, - Table, } from "@budibase/types" import { OAuth2Client } from "google-auth-library" import { buildExternalTableId, finaliseExternalTables } from "./utils" @@ -279,8 +278,8 @@ class GoogleSheetsIntegration implements DatasourcePlus { async buildSchema( datasourceId: string, - entities: Record - ): Promise> { + entities: Record + ): Promise> { // not fully configured yet if (!this.config.auth) { return {} diff --git a/packages/server/src/integrations/microsoftSqlServer.ts b/packages/server/src/integrations/microsoftSqlServer.ts index 783096a7dd..c64d8a36fe 100644 --- a/packages/server/src/integrations/microsoftSqlServer.ts +++ b/packages/server/src/integrations/microsoftSqlServer.ts @@ -11,7 +11,6 @@ import { DatasourceFeature, ConnectionInfo, SourceName, - Table, } from "@budibase/types" import { getSqlQuery, @@ -381,8 +380,8 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { */ async buildSchema( datasourceId: string, - entities: Record - ): Promise> { + entities: Record + ): Promise> { await this.connect() let tableInfo: MSSQLTablesResponse[] = await this.runSQL(this.TABLES_SQL) if (tableInfo == null || !Array.isArray(tableInfo)) { @@ -395,7 +394,7 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { .map((record: any) => record.TABLE_NAME) .filter((name: string) => this.MASTER_TABLES.indexOf(name) === -1) - const tables: Record = {} + const tables: Record = {} for (let tableName of tableNames) { // get the column definition (type) const definition = await this.runSQL( diff --git a/packages/server/src/integrations/mysql.ts b/packages/server/src/integrations/mysql.ts index 90d0cd256c..1e5d3f54c3 100644 --- a/packages/server/src/integrations/mysql.ts +++ b/packages/server/src/integrations/mysql.ts @@ -10,7 +10,6 @@ import { DatasourceFeature, ConnectionInfo, SourceName, - Table, } from "@budibase/types" import { getSqlQuery, @@ -279,9 +278,9 @@ class MySQLIntegration extends Sql implements DatasourcePlus { async buildSchema( datasourceId: string, - entities: Record - ): Promise> { - const tables: { [key: string]: Table } = {} + entities: Record + ): Promise> { + const tables: { [key: string]: ExternalTable } = {} await this.connect() try { diff --git a/packages/server/src/integrations/oracle.ts b/packages/server/src/integrations/oracle.ts index fc4cd45cf6..6c1271bf0e 100644 --- a/packages/server/src/integrations/oracle.ts +++ b/packages/server/src/integrations/oracle.ts @@ -9,7 +9,6 @@ import { DatasourcePlus, DatasourceFeature, ConnectionInfo, - Table, } from "@budibase/types" import { buildExternalTableId, @@ -265,8 +264,8 @@ class OracleIntegration extends Sql implements DatasourcePlus { */ async buildSchema( datasourceId: string, - entities: Record - ): Promise> { + entities: Record + ): Promise> { const columnsResponse = await this.internalQuery({ sql: this.COLUMNS_SQL, }) diff --git a/packages/server/src/integrations/postgres.ts b/packages/server/src/integrations/postgres.ts index 76487f7b65..3340b666e3 100644 --- a/packages/server/src/integrations/postgres.ts +++ b/packages/server/src/integrations/postgres.ts @@ -10,7 +10,6 @@ import { DatasourceFeature, ConnectionInfo, SourceName, - Table, } from "@budibase/types" import { getSqlQuery, @@ -272,8 +271,8 @@ class PostgresIntegration extends Sql implements DatasourcePlus { */ async buildSchema( datasourceId: string, - entities: Record - ): Promise> { + entities: Record + ): Promise> { let tableKeys: { [key: string]: string[] } = {} await this.openConnection() try { diff --git a/packages/server/src/integrations/utils.ts b/packages/server/src/integrations/utils.ts index 6525bd07e2..2f594a7e87 100644 --- a/packages/server/src/integrations/utils.ts +++ b/packages/server/src/integrations/utils.ts @@ -297,9 +297,9 @@ function copyExistingPropsOver( * @param entities The old list of tables, if there was any to look for definitions in. */ export function finaliseExternalTables( - tables: Record, - entities: Record -): Record { + tables: Record, + entities: Record +): Record { let finalTables: Record = {} const tableIds = Object.values(tables).map(table => table._id!) for (let [name, table] of Object.entries(tables)) { diff --git a/packages/types/src/sdk/datasources.ts b/packages/types/src/sdk/datasources.ts index 2896bc62f5..a8ffd74b7a 100644 --- a/packages/types/src/sdk/datasources.ts +++ b/packages/types/src/sdk/datasources.ts @@ -1,4 +1,4 @@ -import { Table } from "../documents" +import { ExternalTable, Table } from "../documents" export const PASSWORD_REPLACEMENT = "--secret-value--" @@ -181,7 +181,7 @@ export interface DatasourcePlus extends IntegrationBase { getStringConcat(parts: string[]): string buildSchema( datasourceId: string, - entities: Record - ): Promise> + entities: Record + ): Promise> getTableNames(): Promise } From 16451904c98ec28c94cb566295a26282a9b200f9 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 12 Oct 2023 17:12:49 +0100 Subject: [PATCH 04/15] Expose an errors object via the buildSchemaFromDb endpoint. --- .../server/src/api/controllers/datasource.ts | 65 ++++++++----------- packages/server/src/constants/index.ts | 6 -- .../server/src/integrations/googlesheets.ts | 22 ++++--- .../src/integrations/microsoftSqlServer.ts | 11 +++- packages/server/src/integrations/mysql.ts | 11 ++-- packages/server/src/integrations/oracle.ts | 8 ++- packages/server/src/integrations/postgres.ts | 8 ++- packages/server/src/integrations/utils.ts | 20 +++++- packages/types/src/api/web/app/datasource.ts | 2 +- packages/types/src/sdk/datasources.ts | 7 +- 10 files changed, 93 insertions(+), 67 deletions(-) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index ad0ed06b83..433271b495 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -5,7 +5,6 @@ import { getTableParams, } from "../../db/utils" import { destroy as tableDestroy } from "./table/internal" -import { BuildSchemaErrors, InvalidColumns } from "../../constants" import { getIntegration } from "../../integrations" import { invalidateDynamicVariables } from "../../threads/utils" import { context, db as dbCore, events } from "@budibase/backend-core" @@ -18,6 +17,7 @@ import { FetchDatasourceInfoRequest, FetchDatasourceInfoResponse, IntegrationBase, + Schema, SourceName, Table, UpdateDatasourceResponse, @@ -29,23 +29,6 @@ import sdk from "../../sdk" import { builderSocket } from "../../websockets" import { setupCreationAuth as googleSetupCreationAuth } from "../../integrations/googlesheets" -function getErrorTables(errors: any, errorType: string) { - return Object.entries(errors) - .filter(entry => entry[1] === errorType) - .map(([name]) => name) -} - -function updateError(error: any, newError: any, tables: string[]) { - if (!error) { - error = "" - } - if (error.length > 0) { - error += "\n" - } - error += `${newError} ${tables.join(", ")}` - return error -} - async function getConnector( datasource: Datasource ): Promise { @@ -73,9 +56,7 @@ async function getAndMergeDatasource(datasource: Datasource) { return await sdk.datasources.enrich(enrichedDatasource) } -async function buildSchemaHelper( - datasource: Datasource -): Promise> { +async function buildSchemaHelper(datasource: Datasource): Promise { const connector = (await getConnector(datasource)) as DatasourcePlus return await connector.buildSchema( datasource._id!, @@ -86,20 +67,23 @@ async function buildSchemaHelper( async function buildFilteredSchema( datasource: Datasource, filter?: string[] -): Promise<{ tables: Record }> { - let tables = await buildSchemaHelper(datasource) - let finalTables = tables +): Promise { + let schema = await buildSchemaHelper(datasource) + let filteredSchema: Schema = { tables: {}, errors: {} } if (filter) { - finalTables = {} - for (let key in tables) { - if ( - filter.some((filter: any) => filter.toLowerCase() === key.toLowerCase()) - ) { - finalTables[key] = tables[key] + for (let key in schema.tables) { + if (filter.some(filter => filter.toLowerCase() === key.toLowerCase())) { + filteredSchema.tables[key] = schema.tables[key] + } + } + + for (let key in schema.errors) { + if (filter.some(filter => filter.toLowerCase() === key.toLowerCase())) { + filteredSchema.errors[key] = schema.errors[key] } } } - return { tables: finalTables } + return filteredSchema } export async function fetch(ctx: UserCtx) { @@ -143,7 +127,7 @@ export async function buildSchemaFromDb(ctx: UserCtx) { const tablesFilter = ctx.request.body.tablesFilter const datasource = await sdk.datasources.get(ctx.params.datasourceId) - const { tables } = await buildFilteredSchema(datasource, tablesFilter) + const { tables, errors } = await buildFilteredSchema(datasource, tablesFilter) datasource.entities = tables setDefaultDisplayColumns(datasource) @@ -151,10 +135,11 @@ export async function buildSchemaFromDb(ctx: UserCtx) { sdk.tables.populateExternalTableSchemas(datasource) ) datasource._rev = dbResp.rev - const cleanedDatasource = await sdk.datasources.removeSecretSingle(datasource) - const res: any = { datasource: cleanedDatasource } - ctx.body = res + ctx.body = { + datasource: await sdk.datasources.removeSecretSingle(datasource), + errors, + } } /** @@ -282,10 +267,12 @@ export async function save( type: plus ? DocumentType.DATASOURCE_PLUS : DocumentType.DATASOURCE, } + let errors: Record = {} if (fetchSchema) { - const { tables } = await buildFilteredSchema(datasource, tablesFilter) - datasource.entities = tables + const schema = await buildFilteredSchema(datasource, tablesFilter) + datasource.entities = schema.tables setDefaultDisplayColumns(datasource) + errors = schema.errors } if (preSaveAction[datasource.source]) { @@ -306,10 +293,10 @@ export async function save( } } - const response: CreateDatasourceResponse = { + ctx.body = { datasource: await sdk.datasources.removeSecretSingle(datasource), + errors, } - ctx.body = response builderSocket?.emitDatasourceUpdate(ctx, datasource) } diff --git a/packages/server/src/constants/index.ts b/packages/server/src/constants/index.ts index ccd4aaf485..b37a4b36c1 100644 --- a/packages/server/src/constants/index.ts +++ b/packages/server/src/constants/index.ts @@ -159,12 +159,6 @@ export enum InvalidColumns { TABLE_ID = "tableId", } -export enum BuildSchemaErrors { - NO_KEY = "no_key", - INVALID_COLUMN = "invalid_column", - NO_HEADER_ROW = "no_header_row", -} - export enum AutomationErrors { INCORRECT_TYPE = "INCORRECT_TYPE", MAX_ITERATIONS = "MAX_ITERATIONS_REACHED", diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index b895f68e78..04582b6eff 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -14,14 +14,15 @@ import { SortJson, ExternalTable, TableRequest, + Schema, } from "@budibase/types" import { OAuth2Client } from "google-auth-library" -import { buildExternalTableId, finaliseExternalTables } from "./utils" +import { buildExternalTableId, checkExternalTables, finaliseExternalTables } from "./utils" import { GoogleSpreadsheet, GoogleSpreadsheetRow } from "google-spreadsheet" import fetch from "node-fetch" import { cache, configs, context, HTTPError } from "@budibase/backend-core" import { dataFilters, utils } from "@budibase/shared-core" -import { BuildSchemaErrors, GOOGLE_SHEETS_PRIMARY_KEY } from "../constants" +import { GOOGLE_SHEETS_PRIMARY_KEY } from "../constants" interface GoogleSheetsConfig { spreadsheetId: string @@ -279,15 +280,16 @@ class GoogleSheetsIntegration implements DatasourcePlus { async buildSchema( datasourceId: string, entities: Record - ): Promise> { + ): Promise { // not fully configured yet if (!this.config.auth) { - return {} + // TODO(samwho): is this the correct behaviour? + return { tables: {}, errors: {} } } await this.connect() const sheets = this.client.sheetsByIndex const tables: Record = {} - const errors: Record = {} + let errors: Record = {} await utils.parallelForeach( sheets, async sheet => { @@ -302,10 +304,10 @@ class GoogleSheetsIntegration implements DatasourcePlus { } if (err.message.startsWith("No values in the header row")) { - errors[sheet.title] = BuildSchemaErrors.NO_HEADER_ROW + errors[sheet.title] = err.message } else { - // If we get an error we don't have a BuildSchemaErrors enum variant - // for, rethrow to avoid failing quietly. + // If we get an error we don't expect, rethrow to avoid failing + // quietly. throw err } return @@ -321,7 +323,9 @@ class GoogleSheetsIntegration implements DatasourcePlus { }, 10 ) - return finaliseExternalTables(tables, entities) + let externalTables = finaliseExternalTables(tables, entities) + errors = { ...errors, ...checkExternalTables(externalTables) } + return { tables: externalTables, errors } } async query(json: QueryJson) { diff --git a/packages/server/src/integrations/microsoftSqlServer.ts b/packages/server/src/integrations/microsoftSqlServer.ts index c64d8a36fe..113c1da878 100644 --- a/packages/server/src/integrations/microsoftSqlServer.ts +++ b/packages/server/src/integrations/microsoftSqlServer.ts @@ -11,6 +11,7 @@ import { DatasourceFeature, ConnectionInfo, SourceName, + Schema, } from "@budibase/types" import { getSqlQuery, @@ -18,6 +19,7 @@ import { convertSqlType, finaliseExternalTables, SqlClient, + checkExternalTables, } from "./utils" import Sql from "./base/sql" import { MSSQLTablesResponse, MSSQLColumn } from "./base/types" @@ -381,7 +383,7 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { async buildSchema( datasourceId: string, entities: Record - ): Promise> { + ): Promise { await this.connect() let tableInfo: MSSQLTablesResponse[] = await this.runSQL(this.TABLES_SQL) if (tableInfo == null || !Array.isArray(tableInfo)) { @@ -445,7 +447,12 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { schema, } } - return finaliseExternalTables(tables, entities) + let externalTables = finaliseExternalTables(tables, entities) + let errors = checkExternalTables(externalTables) + return { + tables: externalTables, + errors, + } } async queryTableNames() { diff --git a/packages/server/src/integrations/mysql.ts b/packages/server/src/integrations/mysql.ts index 1e5d3f54c3..0cde4c00b2 100644 --- a/packages/server/src/integrations/mysql.ts +++ b/packages/server/src/integrations/mysql.ts @@ -10,6 +10,7 @@ import { DatasourceFeature, ConnectionInfo, SourceName, + Schema, } from "@budibase/types" import { getSqlQuery, @@ -17,6 +18,7 @@ import { buildExternalTableId, convertSqlType, finaliseExternalTables, + checkExternalTables, } from "./utils" import dayjs from "dayjs" import { NUMBER_REGEX } from "../utilities" @@ -140,8 +142,6 @@ export function bindingTypeCoerce(bindings: any[]) { class MySQLIntegration extends Sql implements DatasourcePlus { private config: MySQLConfig private client?: mysql.Connection - public tables: Record = {} - public schemaErrors: Record = {} constructor(config: MySQLConfig) { super(SqlClient.MY_SQL) @@ -279,7 +279,7 @@ class MySQLIntegration extends Sql implements DatasourcePlus { async buildSchema( datasourceId: string, entities: Record - ): Promise> { + ): Promise { const tables: { [key: string]: ExternalTable } = {} await this.connect() @@ -328,7 +328,10 @@ class MySQLIntegration extends Sql implements DatasourcePlus { } finally { await this.disconnect() } - return finaliseExternalTables(tables, entities) + + let externalTables = finaliseExternalTables(tables, entities) + let errors = checkExternalTables(tables) + return { tables: externalTables, errors } } async queryTableNames() { diff --git a/packages/server/src/integrations/oracle.ts b/packages/server/src/integrations/oracle.ts index 6c1271bf0e..97d0318954 100644 --- a/packages/server/src/integrations/oracle.ts +++ b/packages/server/src/integrations/oracle.ts @@ -9,9 +9,11 @@ import { DatasourcePlus, DatasourceFeature, ConnectionInfo, + Schema, } from "@budibase/types" import { buildExternalTableId, + checkExternalTables, convertSqlType, finaliseExternalTables, getSqlQuery, @@ -265,7 +267,7 @@ class OracleIntegration extends Sql implements DatasourcePlus { async buildSchema( datasourceId: string, entities: Record - ): Promise> { + ): Promise { const columnsResponse = await this.internalQuery({ sql: this.COLUMNS_SQL, }) @@ -326,7 +328,9 @@ class OracleIntegration extends Sql implements DatasourcePlus { }) }) - return finaliseExternalTables(tables, entities) + let externalTables = finaliseExternalTables(tables, entities) + let errors = checkExternalTables(externalTables) + return { tables: externalTables, errors } } async getTableNames() { diff --git a/packages/server/src/integrations/postgres.ts b/packages/server/src/integrations/postgres.ts index 3340b666e3..ef63f39d87 100644 --- a/packages/server/src/integrations/postgres.ts +++ b/packages/server/src/integrations/postgres.ts @@ -10,6 +10,7 @@ import { DatasourceFeature, ConnectionInfo, SourceName, + Schema, } from "@budibase/types" import { getSqlQuery, @@ -17,6 +18,7 @@ import { convertSqlType, finaliseExternalTables, SqlClient, + checkExternalTables, } from "./utils" import Sql from "./base/sql" import { PostgresColumn } from "./base/types" @@ -272,7 +274,7 @@ class PostgresIntegration extends Sql implements DatasourcePlus { async buildSchema( datasourceId: string, entities: Record - ): Promise> { + ): Promise { let tableKeys: { [key: string]: string[] } = {} await this.openConnection() try { @@ -340,7 +342,9 @@ class PostgresIntegration extends Sql implements DatasourcePlus { } } - return finaliseExternalTables(tables, entities) + let finalizedTables = finaliseExternalTables(tables, entities) + let errors = checkExternalTables(finalizedTables) + return { tables: finalizedTables, errors } } catch (err) { // @ts-ignore throw new Error(err) diff --git a/packages/server/src/integrations/utils.ts b/packages/server/src/integrations/utils.ts index 2f594a7e87..0ce067e967 100644 --- a/packages/server/src/integrations/utils.ts +++ b/packages/server/src/integrations/utils.ts @@ -7,7 +7,7 @@ import { ExternalTable, } from "@budibase/types" import { DocumentType, SEPARATOR } from "../db/utils" -import { NoEmptyFilterStrings } from "../constants" +import { InvalidColumns, NoEmptyFilterStrings } from "../constants" import { helpers } from "@budibase/shared-core" const DOUBLE_SEPARATOR = `${SEPARATOR}${SEPARATOR}` @@ -311,6 +311,24 @@ export function finaliseExternalTables( .reduce((r, [k, v]) => ({ ...r, [k]: v }), {}) } +export function checkExternalTables( + tables: Record +): Record { + const invalidColumns = Object.values(InvalidColumns) as string[] + const errors: Record = {} + for (let [name, table] of Object.entries(tables)) { + if (!table.primary || table.primary.length === 0) { + errors[name] = "Table must have a primary key." + } + + const schemaFields = Object.keys(table.schema) + if (schemaFields.find(f => invalidColumns.includes(f))) { + errors[name] = "Table contains invalid columns." + } + } + return errors +} + /** * Checks if the provided input is an object, but specifically not a date type object. * Used during coercion of types and relationship handling, dates are considered valid diff --git a/packages/types/src/api/web/app/datasource.ts b/packages/types/src/api/web/app/datasource.ts index d0688a24d3..9cd3c8f4bb 100644 --- a/packages/types/src/api/web/app/datasource.ts +++ b/packages/types/src/api/web/app/datasource.ts @@ -2,7 +2,7 @@ import { Datasource } from "../../../documents" export interface CreateDatasourceResponse { datasource: Datasource - error?: any + errors: Record } export interface UpdateDatasourceResponse { diff --git a/packages/types/src/sdk/datasources.ts b/packages/types/src/sdk/datasources.ts index a8ffd74b7a..0b143ad788 100644 --- a/packages/types/src/sdk/datasources.ts +++ b/packages/types/src/sdk/datasources.ts @@ -174,6 +174,11 @@ export interface IntegrationBase { }): void } +export interface Schema { + tables: Record + errors: Record +} + export interface DatasourcePlus extends IntegrationBase { // if the datasource supports the use of bindings directly (to protect against SQL injection) // this returns the format of the identifier @@ -182,6 +187,6 @@ export interface DatasourcePlus extends IntegrationBase { buildSchema( datasourceId: string, entities: Record - ): Promise> + ): Promise getTableNames(): Promise } From f4fa542e8695c59b606a19196e63f9b5ccf5c978 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 12 Oct 2023 17:34:48 +0100 Subject: [PATCH 05/15] Remove some unused fields, fix a broken spec. --- packages/server/src/api/routes/tests/datasource.spec.ts | 2 +- packages/server/src/integrations/microsoftSqlServer.ts | 2 -- packages/server/src/integrations/oracle.ts | 3 --- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index 5019073db4..3c1d7839e8 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -37,7 +37,7 @@ describe("/datasources", () => { .expect(200) expect(res.body.datasource.name).toEqual("Test") - expect(res.body.errors).toBeUndefined() + expect(res.body.errors).toEqual({}) expect(events.datasource.created).toBeCalledTimes(1) }) }) diff --git a/packages/server/src/integrations/microsoftSqlServer.ts b/packages/server/src/integrations/microsoftSqlServer.ts index 113c1da878..06ffaf955d 100644 --- a/packages/server/src/integrations/microsoftSqlServer.ts +++ b/packages/server/src/integrations/microsoftSqlServer.ts @@ -192,8 +192,6 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { private readonly config: MSSQLConfig private index: number = 0 private client?: sqlServer.ConnectionPool - public tables: Record = {} - public schemaErrors: Record = {} MASTER_TABLES = [ "spt_fallback_db", diff --git a/packages/server/src/integrations/oracle.ts b/packages/server/src/integrations/oracle.ts index 97d0318954..28d8fdd84d 100644 --- a/packages/server/src/integrations/oracle.ts +++ b/packages/server/src/integrations/oracle.ts @@ -110,9 +110,6 @@ class OracleIntegration extends Sql implements DatasourcePlus { private readonly config: OracleConfig private index: number = 1 - public tables: Record = {} - public schemaErrors: Record = {} - private readonly COLUMNS_SQL = ` SELECT tabs.table_name, From 6af05500e9170f3ff8ee65870475da6aacddfd2f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 12 Oct 2023 17:59:02 +0100 Subject: [PATCH 06/15] Fix lint warnings. --- packages/server/src/integrations/googlesheets.ts | 6 +++++- packages/server/src/integrations/mysql.ts | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index 04582b6eff..5416f07987 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -17,7 +17,11 @@ import { Schema, } from "@budibase/types" import { OAuth2Client } from "google-auth-library" -import { buildExternalTableId, checkExternalTables, finaliseExternalTables } from "./utils" +import { + buildExternalTableId, + checkExternalTables, + finaliseExternalTables, +} from "./utils" import { GoogleSpreadsheet, GoogleSpreadsheetRow } from "google-spreadsheet" import fetch from "node-fetch" import { cache, configs, context, HTTPError } from "@budibase/backend-core" diff --git a/packages/server/src/integrations/mysql.ts b/packages/server/src/integrations/mysql.ts index 0cde4c00b2..3a954da9bd 100644 --- a/packages/server/src/integrations/mysql.ts +++ b/packages/server/src/integrations/mysql.ts @@ -328,7 +328,7 @@ class MySQLIntegration extends Sql implements DatasourcePlus { } finally { await this.disconnect() } - + let externalTables = finaliseExternalTables(tables, entities) let errors = checkExternalTables(tables) return { tables: externalTables, errors } From e1af1a5be3681f617ad3ed4b17ead600ea4edb37 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 13 Oct 2023 15:29:59 +0100 Subject: [PATCH 07/15] Introduce integration tests for `POST /api/datasources/:datasourceId/schema` --- .../server/src/api/controllers/datasource.ts | 24 +++++---- .../src/integration-test/postgres.spec.ts | 50 +++++++++++++++++++ 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 433271b495..8e6a0620da 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -69,18 +69,20 @@ async function buildFilteredSchema( filter?: string[] ): Promise { let schema = await buildSchemaHelper(datasource) - let filteredSchema: Schema = { tables: {}, errors: {} } - if (filter) { - for (let key in schema.tables) { - if (filter.some(filter => filter.toLowerCase() === key.toLowerCase())) { - filteredSchema.tables[key] = schema.tables[key] - } - } + if (!filter) { + return schema + } - for (let key in schema.errors) { - if (filter.some(filter => filter.toLowerCase() === key.toLowerCase())) { - filteredSchema.errors[key] = schema.errors[key] - } + let filteredSchema: Schema = { tables: {}, errors: {} } + for (let key in schema.tables) { + if (filter.some(filter => filter.toLowerCase() === key.toLowerCase())) { + filteredSchema.tables[key] = schema.tables[key] + } + } + + for (let key in schema.errors) { + if (filter.some(filter => filter.toLowerCase() === key.toLowerCase())) { + filteredSchema.errors[key] = schema.errors[key] } } return filteredSchema diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index 84c19f8bbc..64f29c1a2b 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -18,6 +18,7 @@ import _ from "lodash" import { generator } from "@budibase/backend-core/tests" import { utils } from "@budibase/backend-core" import { databaseTestProviders } from "../integrations/tests/utils" +import { Client } from "pg" const config = setup.getConfig()! @@ -1055,4 +1056,53 @@ describe("postgres integrations", () => { expect(response.body.tableNames.indexOf(primaryName)).not.toBe(-1) }) }) + + describe("POST /api/datasources/:datasourceId/schema", () => { + let client: Client + + beforeAll(async () => { + client = new Client( + (await databaseTestProviders.postgres.getDsConfig()).config! + ) + await client.connect() + }) + + afterAll(async () => { + await client.end() + }) + + it("recognises when a table has no primary key", async () => { + await client.query(` + CREATE TABLE table_without_primary_key ( + id SERIAL + ) + `) + + const response = await makeRequest( + "post", + `/api/datasources/${postgresDatasource._id}/schema` + ) + + expect(response.body.errors).toMatchObject({ + table_without_primary_key: "Table must have a primary key.", + }) + }) + + it("recognises when a table is using a reserved column name", async () => { + await client.query(` + CREATE TABLE table_with_reserved_column_name ( + _id SERIAL + ) + `) + + const response = await makeRequest( + "post", + `/api/datasources/${postgresDatasource._id}/schema` + ) + + expect(response.body.errors).toMatchObject({ + table_without_primary_key: "Table must have a primary key.", + }) + }) + }) }) From 0b8c829ed188da55472f54474f5073879917dff1 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 13 Oct 2023 15:59:07 +0100 Subject: [PATCH 08/15] Clean up correctly after Postgres integration tests. --- .../src/integration-test/postgres.spec.ts | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index 64f29c1a2b..90f0fc9f2c 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -1060,48 +1060,41 @@ describe("postgres integrations", () => { describe("POST /api/datasources/:datasourceId/schema", () => { let client: Client - beforeAll(async () => { + beforeEach(async () => { client = new Client( (await databaseTestProviders.postgres.getDsConfig()).config! ) await client.connect() }) - afterAll(async () => { + afterEach(async () => { + await client.query(`DROP TABLE IF EXISTS "table"`) await client.end() }) it("recognises when a table has no primary key", async () => { - await client.query(` - CREATE TABLE table_without_primary_key ( - id SERIAL - ) - `) + await client.query(`CREATE TABLE "table" (id SERIAL)`) const response = await makeRequest( "post", `/api/datasources/${postgresDatasource._id}/schema` ) - expect(response.body.errors).toMatchObject({ - table_without_primary_key: "Table must have a primary key.", + expect(response.body.errors).toEqual({ + table: "Table must have a primary key.", }) }) it("recognises when a table is using a reserved column name", async () => { - await client.query(` - CREATE TABLE table_with_reserved_column_name ( - _id SERIAL - ) - `) + await client.query(`CREATE TABLE "table" (_id SERIAL PRIMARY KEY) `) const response = await makeRequest( "post", `/api/datasources/${postgresDatasource._id}/schema` ) - expect(response.body.errors).toMatchObject({ - table_without_primary_key: "Table must have a primary key.", + expect(response.body.errors).toEqual({ + table: "Table contains invalid columns.", }) }) }) From 89e64d18a5ccb311b0bd6de454081d6b69f9aca3 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 13 Oct 2023 16:11:56 +0100 Subject: [PATCH 09/15] Remove TODOs. --- packages/server/src/integrations/googlesheets.ts | 1 - packages/server/src/integrations/utils.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index 5416f07987..57b6682cc8 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -287,7 +287,6 @@ class GoogleSheetsIntegration implements DatasourcePlus { ): Promise { // not fully configured yet if (!this.config.auth) { - // TODO(samwho): is this the correct behaviour? return { tables: {}, errors: {} } } await this.connect() diff --git a/packages/server/src/integrations/utils.ts b/packages/server/src/integrations/utils.ts index 0ce067e967..79b18e767c 100644 --- a/packages/server/src/integrations/utils.ts +++ b/packages/server/src/integrations/utils.ts @@ -305,7 +305,7 @@ export function finaliseExternalTables( for (let [name, table] of Object.entries(tables)) { finalTables[name] = copyExistingPropsOver(name, table, entities, tableIds) } - // sort the tables by name (TODO(samwho): why?) + // sort the tables by name, this is for the UI to display them in alphabetical order return Object.entries(finalTables) .sort(([a], [b]) => a.localeCompare(b)) .reduce((r, [k, v]) => ({ ...r, [k]: v }), {}) From b7ff1d60f1fb3688bd416b8165ed23e8eb38c0b8 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 16 Oct 2023 16:36:42 +0100 Subject: [PATCH 10/15] Wire up the frontend to show the new datasource errors. --- .../tableSelectionStore.js | 9 +--- .../builder/src/stores/backend/datasources.js | 49 +++++++++---------- packages/builder/src/stores/backend/index.js | 2 +- 3 files changed, 27 insertions(+), 33 deletions(-) diff --git a/packages/builder/src/components/backend/Datasources/TableImportSelection/tableSelectionStore.js b/packages/builder/src/components/backend/Datasources/TableImportSelection/tableSelectionStore.js index 6235ea397a..5c2ea9767c 100644 --- a/packages/builder/src/components/backend/Datasources/TableImportSelection/tableSelectionStore.js +++ b/packages/builder/src/components/backend/Datasources/TableImportSelection/tableSelectionStore.js @@ -1,6 +1,6 @@ import { derived, writable, get } from "svelte/store" import { keepOpen, notifications } from "@budibase/bbui" -import { datasources, ImportTableError, tables } from "stores/backend" +import { datasources, tables } from "stores/backend" export const createTableSelectionStore = (integration, datasource) => { const tableNamesStore = writable([]) @@ -30,12 +30,7 @@ export const createTableSelectionStore = (integration, datasource) => { notifications.success(`Tables fetched successfully.`) await onComplete() } catch (err) { - if (err instanceof ImportTableError) { - errorStore.set(err) - } else { - notifications.error("Error fetching tables.") - } - + errorStore.set(err) return keepOpen } } diff --git a/packages/builder/src/stores/backend/datasources.js b/packages/builder/src/stores/backend/datasources.js index cec2f4eac2..9265f34b96 100644 --- a/packages/builder/src/stores/backend/datasources.js +++ b/packages/builder/src/stores/backend/datasources.js @@ -9,15 +9,23 @@ import { API } from "api" import { DatasourceFeature } from "@budibase/types" import { TableNames } from "constants" -export class ImportTableError extends Error { - constructor(message) { - super(message) - const [title, description] = message.split(" - ") +class TableImportError extends Error { + constructor(errors) { + super(`Error importing tables: ${Object.keys(errors).join(", ")}`) + this.name = "TableImportError" + this.errors = errors + } - this.name = "TableSelectionError" - // Capitalize the first character of both the title and description - this.title = title[0].toUpperCase() + title.substr(1) - this.description = description[0].toUpperCase() + description.substr(1) + get title() { + return `Error fetching tables` + } + + get description() { + let message = "" + for (const key in this.errors) { + message += `${key}: ${this.errors[key]}\n` + } + return message } } @@ -78,9 +86,9 @@ export function createDatasourcesStore() { } const updateDatasource = response => { - const { datasource, error } = response - if (error) { - store.update(state => ({ ...state })) + const { datasource, errors } = response + if (errors && Object.keys(errors).length > 0) { + throw new TableImportError(errors) } replaceDatasource(datasource._id, datasource) select(datasource._id) @@ -88,20 +96,11 @@ export function createDatasourcesStore() { } const updateSchema = async (datasource, tablesFilter) => { - try { - const response = await API.buildDatasourceSchema({ - datasourceId: datasource?._id, - tablesFilter, - }) - updateDatasource(response) - } catch (e) { - // buildDatasourceSchema call returns user presentable errors with two parts divided with a " - ". - if (e.message.split(" - ").length === 2) { - throw new ImportTableError(e.message) - } else { - throw e - } - } + const response = await API.buildDatasourceSchema({ + datasourceId: datasource?._id, + tablesFilter, + }) + updateDatasource(response) } const sourceCount = source => { diff --git a/packages/builder/src/stores/backend/index.js b/packages/builder/src/stores/backend/index.js index 278e43c1ed..3781e2ab92 100644 --- a/packages/builder/src/stores/backend/index.js +++ b/packages/builder/src/stores/backend/index.js @@ -4,7 +4,7 @@ export { views } from "./views" export { viewsV2 } from "./viewsV2" export { permissions } from "./permissions" export { roles } from "./roles" -export { datasources, ImportTableError } from "./datasources" +export { datasources } from "./datasources" export { integrations } from "./integrations" export { sortedIntegrations } from "./sortedIntegrations" export { queries } from "./queries" From 39292e88a87a222d1ee37be84229bb609f5ac88f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 16 Oct 2023 17:11:51 +0100 Subject: [PATCH 11/15] Correct error message to say sheets when it's a Google sheet. --- .../backend/Datasources/TableImportSelection/index.svelte | 2 +- packages/builder/src/stores/backend/datasources.js | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/builder/src/components/backend/Datasources/TableImportSelection/index.svelte b/packages/builder/src/components/backend/Datasources/TableImportSelection/index.svelte index 1fc83d4978..3bc2457c99 100644 --- a/packages/builder/src/components/backend/Datasources/TableImportSelection/index.svelte +++ b/packages/builder/src/components/backend/Datasources/TableImportSelection/index.svelte @@ -57,7 +57,7 @@ {#if $store.error} {/if} diff --git a/packages/builder/src/stores/backend/datasources.js b/packages/builder/src/stores/backend/datasources.js index 9265f34b96..11184f2caa 100644 --- a/packages/builder/src/stores/backend/datasources.js +++ b/packages/builder/src/stores/backend/datasources.js @@ -11,15 +11,11 @@ import { TableNames } from "constants" class TableImportError extends Error { constructor(errors) { - super(`Error importing tables: ${Object.keys(errors).join(", ")}`) + super() this.name = "TableImportError" this.errors = errors } - get title() { - return `Error fetching tables` - } - get description() { let message = "" for (const key in this.errors) { From 7dcd6767f993b8a026e37e9405cbf13fa0378e9a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 16 Oct 2023 18:37:56 +0100 Subject: [PATCH 12/15] Fixes for adding autocolumns, it was not possible to select auto column type and fixing icon, as well as display of column when editing it - this corrects a few issues in this area as auto columns did not have a correct fieldId and also did not have a valid field definition. --- .../DataTable/modals/CreateEditColumn.svelte | 18 ++++++--- .../builder/src/constants/backend/index.js | 38 +++++++++++-------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte index ba61ede746..7b51e6c839 100644 --- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte @@ -36,7 +36,7 @@ import { FieldType, FieldSubtype, SourceName } from "@budibase/types" import RelationshipSelector from "components/common/RelationshipSelector.svelte" - const AUTO_TYPE = "auto" + const AUTO_TYPE = FIELDS.AUTO.type const FORMULA_TYPE = FIELDS.FORMULA.type const LINK_TYPE = FIELDS.LINK.type const STRING_TYPE = FIELDS.STRING.type @@ -60,8 +60,13 @@ {} ) - function makeFieldId(type, subtype) { - return `${type}${subtype || ""}`.toUpperCase() + function makeFieldId(type, subtype, autocolumn) { + // don't make field IDs for auto types + if (type === AUTO_TYPE || autocolumn) { + return type.toUpperCase() + } else { + return `${type}${subtype || ""}`.toUpperCase() + } } let originalName @@ -183,7 +188,8 @@ if (!savingColumn) { editableColumn.fieldId = makeFieldId( editableColumn.type, - editableColumn.subtype + editableColumn.subtype, + editableColumn.autocolumn ) allowedTypes = getAllowedTypes().map(t => ({ @@ -419,7 +425,7 @@ FIELDS.FORMULA, FIELDS.JSON, isUsers ? FIELDS.USERS : FIELDS.USER, - { name: "Auto Column", type: AUTO_TYPE }, + FIELDS.AUTO, ] } else { let fields = [ @@ -538,7 +544,7 @@ getOptionValue={field => field.fieldId} getOptionIcon={field => field.icon} isOptionEnabled={option => { - if (option.type == AUTO_TYPE) { + if (option.type === AUTO_TYPE) { return availableAutoColumnKeys?.length > 0 } return true diff --git a/packages/builder/src/constants/backend/index.js b/packages/builder/src/constants/backend/index.js index a81b33c2d3..ac4079b69e 100644 --- a/packages/builder/src/constants/backend/index.js +++ b/packages/builder/src/constants/backend/index.js @@ -1,5 +1,21 @@ import { FieldType, FieldSubtype } from "@budibase/types" +export const AUTO_COLUMN_SUB_TYPES = { + AUTO_ID: "autoID", + CREATED_BY: "createdBy", + CREATED_AT: "createdAt", + UPDATED_BY: "updatedBy", + UPDATED_AT: "updatedAt", +} + +export const AUTO_COLUMN_DISPLAY_NAMES = { + AUTO_ID: "Auto ID", + CREATED_BY: "Created By", + CREATED_AT: "Created At", + UPDATED_BY: "Updated By", + UPDATED_AT: "Updated At", +} + export const FIELDS = { STRING: { name: "Text", @@ -107,6 +123,12 @@ export const FIELDS = { presence: false, }, }, + AUTO: { + name: "Auto Column", + type: FieldType.AUTO, + icon: "MagicWand", + constraints: {}, + }, FORMULA: { name: "Formula", type: FieldType.FORMULA, @@ -139,22 +161,6 @@ export const FIELDS = { }, } -export const AUTO_COLUMN_SUB_TYPES = { - AUTO_ID: "autoID", - CREATED_BY: "createdBy", - CREATED_AT: "createdAt", - UPDATED_BY: "updatedBy", - UPDATED_AT: "updatedAt", -} - -export const AUTO_COLUMN_DISPLAY_NAMES = { - AUTO_ID: "Auto ID", - CREATED_BY: "Created By", - CREATED_AT: "Created At", - UPDATED_BY: "Updated By", - UPDATED_AT: "Updated At", -} - export const FILE_TYPES = { IMAGE: ["png", "tiff", "gif", "raw", "jpg", "jpeg"], CODE: ["js", "rs", "py", "java", "rb", "hs", "yml"], From ea20217fbc0bea5f7b2623f02a128c27b42c925c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Oct 2023 14:31:08 +0200 Subject: [PATCH 13/15] Use build --- .github/workflows/release-singleimage.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release-singleimage.yml b/.github/workflows/release-singleimage.yml index 61ab9a4eb2..f7f87f6e4c 100644 --- a/.github/workflows/release-singleimage.yml +++ b/.github/workflows/release-singleimage.yml @@ -20,8 +20,8 @@ jobs: with: root-reserve-mb: 30000 swap-size-mb: 1024 - remove-android: 'true' - remove-dotnet: 'true' + remove-android: "true" + remove-dotnet: "true" - name: Fail if not a tag run: | if [[ $GITHUB_REF != refs/tags/* ]]; then @@ -48,7 +48,7 @@ jobs: - name: Update versions run: ./scripts/updateVersions.sh - name: Run Yarn Build - run: yarn build:docker:pre + run: yarn build - name: Login to Docker Hub uses: docker/login-action@v2 with: From 697e733f820c5b3ad83c50e08ba9a0273920cc44 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Oct 2023 15:24:29 +0200 Subject: [PATCH 14/15] Fix timeout --- hosting/single/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hosting/single/Dockerfile b/hosting/single/Dockerfile index 95e383edb0..c7b90dbdc4 100644 --- a/hosting/single/Dockerfile +++ b/hosting/single/Dockerfile @@ -12,14 +12,14 @@ RUN chmod +x /cleanup.sh WORKDIR /app ADD packages/server . COPY yarn.lock . -RUN yarn install --production=true --network-timeout 100000 +RUN yarn install --production=true --network-timeout 1000000 RUN /cleanup.sh # build worker WORKDIR /worker ADD packages/worker . COPY yarn.lock . -RUN yarn install --production=true --network-timeout 100000 +RUN yarn install --production=true --network-timeout 1000000 RUN /cleanup.sh FROM budibase/couchdb From bbda776c49c664404f26a01b6ab88b598199e3a5 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 17 Oct 2023 14:39:22 +0000 Subject: [PATCH 15/15] Bump version to 2.11.36 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 62a762ec71..34faefc099 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.11.35", + "version": "2.11.36", "npmClient": "yarn", "packages": [ "packages/*"