From 16451904c98ec28c94cb566295a26282a9b200f9 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 12 Oct 2023 17:12:49 +0100 Subject: [PATCH] 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 }