From 732ebb4f87e6bffa88eba775e43186d8e6659172 Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Thu, 18 May 2023 22:09:44 +0100 Subject: [PATCH 01/13] Adding function to fetch table names. --- .../src/integrations/microsoftSqlServer.ts | 15 ++++++++- packages/server/src/integrations/mysql.ts | 33 +++++++++++++------ packages/server/src/integrations/oracle.ts | 11 +++++++ packages/server/src/integrations/postgres.ts | 11 +++++++ packages/types/src/sdk/datasources.ts | 1 + 5 files changed, 60 insertions(+), 11 deletions(-) diff --git a/packages/server/src/integrations/microsoftSqlServer.ts b/packages/server/src/integrations/microsoftSqlServer.ts index 47f36f60e9..ab0a3e14e8 100644 --- a/packages/server/src/integrations/microsoftSqlServer.ts +++ b/packages/server/src/integrations/microsoftSqlServer.ts @@ -20,7 +20,6 @@ import { } from "./utils" import Sql from "./base/sql" import { MSSQLTablesResponse, MSSQLColumn } from "./base/types" - const sqlServer = require("mssql") const DEFAULT_SCHEMA = "dbo" @@ -284,6 +283,20 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { this.schemaErrors = final.errors } + async queryTableNames() { + let tableInfo: MSSQLTablesResponse[] = await this.runSQL(this.TABLES_SQL) + const schema = this.config.schema || DEFAULT_SCHEMA + return tableInfo + .filter((record: any) => record.TABLE_SCHEMA === schema) + .map((record: any) => record.TABLE_NAME) + .filter((name: string) => this.MASTER_TABLES.indexOf(name) === -1) + } + + async getTableNames() { + await this.connect() + return this.queryTableNames() + } + async read(query: SqlQuery | string) { await this.connect() const response = await this.internalQuery(getSqlQuery(query)) diff --git a/packages/server/src/integrations/mysql.ts b/packages/server/src/integrations/mysql.ts index eb721a6e0f..ca0c962d25 100644 --- a/packages/server/src/integrations/mysql.ts +++ b/packages/server/src/integrations/mysql.ts @@ -214,20 +214,11 @@ class MySQLIntegration extends Sql implements DatasourcePlus { async buildSchema(datasourceId: string, entities: Record) { const tables: { [key: string]: Table } = {} - const database = this.config.database await this.connect() try { // get the tables first - const tablesResp: Record[] = await this.internalQuery( - { sql: "SHOW TABLES;" }, - { connect: false } - ) - const tableNames: string[] = tablesResp.map( - (obj: any) => - obj[`Tables_in_${database}`] || - obj[`Tables_in_${database.toLowerCase()}`] - ) + const tableNames = await this.queryTableNames() for (let tableName of tableNames) { const primaryKeys = [] const schema: TableSchema = {} @@ -274,6 +265,28 @@ class MySQLIntegration extends Sql implements DatasourcePlus { this.schemaErrors = final.errors } + async queryTableNames() { + const database = this.config.database + const tablesResp: Record[] = await this.internalQuery( + { sql: "SHOW TABLES;" }, + { connect: false } + ) + return tablesResp.map( + (obj: any) => + obj[`Tables_in_${database}`] || + obj[`Tables_in_${database.toLowerCase()}`] + ) + } + + async getTableNames() { + await this.connect() + try { + return this.queryTableNames() + } finally { + await this.disconnect() + } + } + async create(query: SqlQuery | string) { const results = await this.internalQuery(getSqlQuery(query)) return results.length ? results : [{ created: true }] diff --git a/packages/server/src/integrations/oracle.ts b/packages/server/src/integrations/oracle.ts index f8ec6e8bae..53e88752cd 100644 --- a/packages/server/src/integrations/oracle.ts +++ b/packages/server/src/integrations/oracle.ts @@ -323,6 +323,17 @@ class OracleIntegration extends Sql implements DatasourcePlus { this.schemaErrors = final.errors } + async getTableNames() { + const columnsResponse = await this.internalQuery({ + sql: this.COLUMNS_SQL, + }) + if (!columnsResponse.rows) { + return [] + } else { + return columnsResponse.rows.map(row => row.TABLE_NAME) + } + } + async testConnection() { const response: ConnectionInfo = { connected: false, diff --git a/packages/server/src/integrations/postgres.ts b/packages/server/src/integrations/postgres.ts index bf77ec08c6..0954f94ecb 100644 --- a/packages/server/src/integrations/postgres.ts +++ b/packages/server/src/integrations/postgres.ts @@ -311,6 +311,17 @@ class PostgresIntegration extends Sql implements DatasourcePlus { } } + async getTableNames() { + try { + await this.openConnection() + const columnsResponse: { rows: PostgresColumn[] } = + await this.client.query(this.COLUMNS_SQL) + return columnsResponse.rows.map(row => row.table_name) + } finally { + await this.closeConnection() + } + } + async create(query: SqlQuery | string) { const response = await this.internalQuery(getSqlQuery(query)) return response.rows.length ? response.rows : [{ created: true }] diff --git a/packages/types/src/sdk/datasources.ts b/packages/types/src/sdk/datasources.ts index 9df9670877..4631a091b4 100644 --- a/packages/types/src/sdk/datasources.ts +++ b/packages/types/src/sdk/datasources.ts @@ -150,4 +150,5 @@ export interface DatasourcePlus extends IntegrationBase { getBindingIdentifier(): string getStringConcat(parts: string[]): string buildSchema(datasourceId: string, entities: Record): any + getTableNames(): Promise } From 2223027d285544f84638d72a48eec42121ad292e Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Thu, 18 May 2023 23:22:52 +0100 Subject: [PATCH 02/13] Adding API for retrieving table names. --- .../server/src/api/controllers/datasource.ts | 16 ++++++++++++++++ packages/server/src/api/routes/datasource.ts | 5 +++++ packages/types/src/api/web/app/datasource.ts | 4 ++++ 3 files changed, 25 insertions(+) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 8f13e0e618..5c2c4ef684 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -21,6 +21,7 @@ import { CreateDatasourceRequest, VerifyDatasourceRequest, VerifyDatasourceResponse, + FetchTablesDatasourceResponse, IntegrationBase, DatasourcePlus, } from "@budibase/types" @@ -153,6 +154,21 @@ export async function verify( } } +export async function tables( + ctx: UserCtx +) { + const datasourceId = ctx.params.datasourceId + const datasource = await sdk.datasources.get(datasourceId, { enriched: true }) + const connector = (await getConnector(datasource)) as DatasourcePlus + if (!connector.getTableNames) { + ctx.throw(400, "Table name fetching not supported by datasource") + } + const tables = await connector.getTableNames() + ctx.body = { + tables, + } +} + export async function buildSchemaFromDb(ctx: UserCtx) { const db = context.getAppDB() const datasource = await sdk.datasources.get(ctx.params.datasourceId) diff --git a/packages/server/src/api/routes/datasource.ts b/packages/server/src/api/routes/datasource.ts index 654fb794e3..89bc12c543 100644 --- a/packages/server/src/api/routes/datasource.ts +++ b/packages/server/src/api/routes/datasource.ts @@ -20,6 +20,11 @@ router authorized(permissions.BUILDER), datasourceController.verify ) + .get( + "/api/datasources/:datasourceId/tables", + authorized(permissions.BUILDER), + datasourceController.tables + ) .get( "/api/datasources/:datasourceId", authorized( diff --git a/packages/types/src/api/web/app/datasource.ts b/packages/types/src/api/web/app/datasource.ts index 983fd45b92..12c9753aa1 100644 --- a/packages/types/src/api/web/app/datasource.ts +++ b/packages/types/src/api/web/app/datasource.ts @@ -23,6 +23,10 @@ export interface VerifyDatasourceResponse { error?: string } +export interface FetchTablesDatasourceResponse { + tables: string[] +} + export interface UpdateDatasourceRequest extends Datasource { datasource: Datasource } From d85bcbc7e53674c29c4b5c6117c2415b5dbd05e1 Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Fri, 19 May 2023 12:19:55 +0100 Subject: [PATCH 03/13] Adding test for postgres verify. --- .../server/src/api/controllers/datasource.ts | 2 +- packages/server/src/api/routes/datasource.ts | 2 +- .../src/integration-test/postgres.spec.ts | 23 ++++++++++++++++--- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 5c2c4ef684..0650305ac1 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -154,7 +154,7 @@ export async function verify( } } -export async function tables( +export async function fetchTables( ctx: UserCtx ) { const datasourceId = ctx.params.datasourceId diff --git a/packages/server/src/api/routes/datasource.ts b/packages/server/src/api/routes/datasource.ts index 89bc12c543..e740510d7f 100644 --- a/packages/server/src/api/routes/datasource.ts +++ b/packages/server/src/api/routes/datasource.ts @@ -23,7 +23,7 @@ router .get( "/api/datasources/:datasourceId/tables", authorized(permissions.BUILDER), - datasourceController.tables + datasourceController.fetchTables ) .get( "/api/datasources/:datasourceId", diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index 79f6db5cd1..c34f2a9bff 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -52,8 +52,8 @@ describe("row api - postgres", () => { makeRequest = generateMakeRequest(apiKey, true) }) - beforeEach(async () => { - postgresDatasource = await config.createDatasource({ + function pgDatasourceConfig() { + return { datasource: { type: "datasource", source: SourceName.POSTGRES, @@ -70,7 +70,11 @@ describe("row api - postgres", () => { ca: false, }, }, - }) + } + } + + beforeEach(async () => { + postgresDatasource = await config.createDatasource(pgDatasourceConfig()) async function createAuxTable(prefix: string) { return await config.createTable({ @@ -439,6 +443,19 @@ describe("row api - postgres", () => { }) }) + describe("POST /api/datasources/verify", () => { + it("should be able to verify the connection", async () => { + const config = pgDatasourceConfig() + const response = await makeRequest( + "post", + "/api/datasources/verify", + config + ) + expect(response.status).toBe(200) + expect(response.body.connected).toBe(true) + }) + }) + describe("DELETE /api/:tableId/rows", () => { const deleteRow = ( tableId: string | undefined, From 38e39cf2f2b71c49fbe42a0ee272e4cb57ba2a5c Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Fri, 19 May 2023 13:36:05 +0100 Subject: [PATCH 04/13] Adding negative test case for connections and adding test of table name fetching for postgres. --- .../server/src/api/controllers/datasource.ts | 4 +- .../src/integration-test/postgres.spec.ts | 54 ++++++++++++++----- packages/types/src/api/web/app/datasource.ts | 2 +- 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 0650305ac1..9ae72ddf50 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -163,9 +163,9 @@ export async function fetchTables( if (!connector.getTableNames) { ctx.throw(400, "Table name fetching not supported by datasource") } - const tables = await connector.getTableNames() + const tableNames = await connector.getTableNames() ctx.body = { - tables, + tableNames, } } diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index c34f2a9bff..b1455f4419 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -26,7 +26,7 @@ jest.setTimeout(30000) jest.unmock("pg") -describe("row api - postgres", () => { +describe("postgres integrations", () => { let makeRequest: MakeRequestResponse, postgresDatasource: Datasource, primaryPostgresTable: Table, @@ -443,19 +443,6 @@ describe("row api - postgres", () => { }) }) - describe("POST /api/datasources/verify", () => { - it("should be able to verify the connection", async () => { - const config = pgDatasourceConfig() - const response = await makeRequest( - "post", - "/api/datasources/verify", - config - ) - expect(response.status).toBe(200) - expect(response.body.connected).toBe(true) - }) - }) - describe("DELETE /api/:tableId/rows", () => { const deleteRow = ( tableId: string | undefined, @@ -1041,4 +1028,43 @@ describe("row api - postgres", () => { }) }) }) + + describe("POST /api/datasources/verify", () => { + it("should be able to verify the connection", async () => { + const config = pgDatasourceConfig() + const response = await makeRequest( + "post", + "/api/datasources/verify", + config + ) + expect(response.status).toBe(200) + expect(response.body.connected).toBe(true) + }) + + it("should state an invalid datasource cannot connect", async () => { + const config = pgDatasourceConfig() + config.datasource.config.password = "wrongpassword" + const response = await makeRequest( + "post", + "/api/datasources/verify", + config + ) + expect(response.status).toBe(200) + expect(response.body.connected).toBe(false) + expect(response.body.error).toBeDefined() + }) + }) + + describe("GET /api/datasources/:datasourceId/tables", () => { + it("should fetch tables within postgres datasource", async () => { + const primaryName = primaryPostgresTable.name + const response = await makeRequest( + "get", + `/api/datasources/${postgresDatasource._id}/tables` + ) + expect(response.status).toBe(200) + expect(response.body.tableNames).toBeDefined() + expect(response.body.tableNames.indexOf(primaryName)).not.toBe(-10) + }) + }) }) diff --git a/packages/types/src/api/web/app/datasource.ts b/packages/types/src/api/web/app/datasource.ts index 12c9753aa1..0d0406fe55 100644 --- a/packages/types/src/api/web/app/datasource.ts +++ b/packages/types/src/api/web/app/datasource.ts @@ -24,7 +24,7 @@ export interface VerifyDatasourceResponse { } export interface FetchTablesDatasourceResponse { - tables: string[] + tableNames: string[] } export interface UpdateDatasourceRequest extends Datasource { From d6c07f47b649c7f14ae93a697fc7eccc98a63372 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 22 May 2023 13:57:56 +0100 Subject: [PATCH 05/13] Adding base implementation for googlesheets integration. --- packages/server/src/integrations/googlesheets.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index eea9cc4176..0a5d2acefc 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -240,6 +240,11 @@ class GoogleSheetsIntegration implements DatasourcePlus { } } + getTableNames(): Promise { + // TODO: implement + return Promise.resolve([]) + } + getTableSchema(title: string, headerValues: string[], id?: string) { // base table const table: Table = { From 83ddb9c8dde1f8b5f4ea13e3bde1dc637ddb6002 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 22 May 2023 14:28:18 +0100 Subject: [PATCH 06/13] Adding feature to denote fetch table names function. --- packages/server/src/integrations/googlesheets.ts | 7 +++++-- packages/server/src/integrations/microsoftSqlServer.ts | 5 ++++- packages/server/src/integrations/mysql.ts | 5 ++++- packages/server/src/integrations/oracle.ts | 5 ++++- packages/server/src/integrations/postgres.ts | 5 ++++- packages/types/src/sdk/datasources.ts | 1 + 6 files changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index 0a5d2acefc..e965ebc86c 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -63,10 +63,13 @@ const SCHEMA: Integration = { relationships: false, docs: "https://developers.google.com/sheets/api/quickstart/nodejs", description: - "Create and collaborate on online spreadsheets in real-time and from any device. ", + "Create and collaborate on online spreadsheets in real-time and from any device.", friendlyName: "Google Sheets", type: "Spreadsheet", - features: [DatasourceFeature.CONNECTION_CHECKING], + features: [ + DatasourceFeature.CONNECTION_CHECKING, + DatasourceFeature.FETCH_TABLE_NAMES, + ], datasource: { spreadsheetId: { display: "Google Sheet URL", diff --git a/packages/server/src/integrations/microsoftSqlServer.ts b/packages/server/src/integrations/microsoftSqlServer.ts index ab0a3e14e8..a83630afbb 100644 --- a/packages/server/src/integrations/microsoftSqlServer.ts +++ b/packages/server/src/integrations/microsoftSqlServer.ts @@ -40,7 +40,10 @@ const SCHEMA: Integration = { "Microsoft SQL Server is a relational database management system developed by Microsoft. ", friendlyName: "MS SQL Server", type: "Relational", - features: [DatasourceFeature.CONNECTION_CHECKING], + features: [ + DatasourceFeature.CONNECTION_CHECKING, + DatasourceFeature.FETCH_TABLE_NAMES, + ], datasource: { user: { type: DatasourceFieldType.STRING, diff --git a/packages/server/src/integrations/mysql.ts b/packages/server/src/integrations/mysql.ts index ca0c962d25..d83ce300d3 100644 --- a/packages/server/src/integrations/mysql.ts +++ b/packages/server/src/integrations/mysql.ts @@ -36,7 +36,10 @@ const SCHEMA: Integration = { type: "Relational", description: "MySQL Database Service is a fully managed database service to deploy cloud-native applications. ", - features: [DatasourceFeature.CONNECTION_CHECKING], + features: [ + DatasourceFeature.CONNECTION_CHECKING, + DatasourceFeature.FETCH_TABLE_NAMES, + ], datasource: { host: { type: DatasourceFieldType.STRING, diff --git a/packages/server/src/integrations/oracle.ts b/packages/server/src/integrations/oracle.ts index 53e88752cd..55b425f480 100644 --- a/packages/server/src/integrations/oracle.ts +++ b/packages/server/src/integrations/oracle.ts @@ -50,7 +50,10 @@ const SCHEMA: Integration = { type: "Relational", description: "Oracle Database is an object-relational database management system developed by Oracle Corporation", - features: [DatasourceFeature.CONNECTION_CHECKING], + features: [ + DatasourceFeature.CONNECTION_CHECKING, + DatasourceFeature.FETCH_TABLE_NAMES, + ], datasource: { host: { type: DatasourceFieldType.STRING, diff --git a/packages/server/src/integrations/postgres.ts b/packages/server/src/integrations/postgres.ts index 0954f94ecb..d8385705f1 100644 --- a/packages/server/src/integrations/postgres.ts +++ b/packages/server/src/integrations/postgres.ts @@ -52,7 +52,10 @@ const SCHEMA: Integration = { type: "Relational", description: "PostgreSQL, also known as Postgres, is a free and open-source relational database management system emphasizing extensibility and SQL compliance.", - features: [DatasourceFeature.CONNECTION_CHECKING], + features: [ + DatasourceFeature.CONNECTION_CHECKING, + DatasourceFeature.FETCH_TABLE_NAMES, + ], datasource: { host: { type: DatasourceFieldType.STRING, diff --git a/packages/types/src/sdk/datasources.ts b/packages/types/src/sdk/datasources.ts index 4631a091b4..24ccea7e83 100644 --- a/packages/types/src/sdk/datasources.ts +++ b/packages/types/src/sdk/datasources.ts @@ -75,6 +75,7 @@ export enum FilterType { export enum DatasourceFeature { CONNECTION_CHECKING = "connection", + FETCH_TABLE_NAMES = "fetch_table_names", } export interface StepDefinition { From 76eef8d3d4a54ff999c0e35f022612e0d8dcc70e Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 22 May 2023 16:25:50 +0100 Subject: [PATCH 07/13] Fixing some test issues. --- packages/server/src/api/routes/tests/datasource.spec.ts | 2 +- packages/server/src/integration-test/postgres.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index 53d04bdbff..5019073db4 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -87,7 +87,7 @@ describe("/datasources", () => { expect(contents.rows.length).toEqual(1) // update the datasource to remove the variables - datasource.config.dynamicVariables = [] + datasource.config!.dynamicVariables = [] const res = await request .put(`/api/datasources/${datasource._id}`) .send(datasource) diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index b1455f4419..0fd9b7c143 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -1064,7 +1064,7 @@ describe("postgres integrations", () => { ) expect(response.status).toBe(200) expect(response.body.tableNames).toBeDefined() - expect(response.body.tableNames.indexOf(primaryName)).not.toBe(-10) + expect(response.body.tableNames.indexOf(primaryName)).not.toBe(-1) }) }) }) From 74ea851fa32998ac755c628c11117da1e40b07aa Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 23 May 2023 09:55:46 +0200 Subject: [PATCH 08/13] Fetch google sheets --- packages/server/src/integrations/googlesheets.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index e965ebc86c..5cef17dde8 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -243,9 +243,10 @@ class GoogleSheetsIntegration implements DatasourcePlus { } } - getTableNames(): Promise { - // TODO: implement - return Promise.resolve([]) + async getTableNames(): Promise { + await this.connect() + const sheets = this.client.sheetsByIndex + return sheets.map(s => s.title) } getTableSchema(title: string, headerValues: string[], id?: string) { From 5ea19986b13c21db69457d714c1194fe8050a667 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 23 May 2023 10:14:06 +0200 Subject: [PATCH 09/13] Add basic test --- .../integrations/tests/googlesheets.spec.ts | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 0e7669a957..7f6c641aa9 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -17,12 +17,14 @@ jest.mock("google-spreadsheet") const { GoogleSpreadsheet } = require("google-spreadsheet") const sheetsByTitle: { [title: string]: GoogleSpreadsheetWorksheet } = {} +const sheetsByIndex: GoogleSpreadsheetWorksheet[] = [] GoogleSpreadsheet.mockImplementation(() => { return { useOAuth2Client: jest.fn(), loadInfo: jest.fn(), sheetsByTitle, + sheetsByIndex, } }) @@ -88,7 +90,7 @@ describe("Google Sheets Integration", () => { } describe("update table", () => { - test("adding a new field will be adding a new header row", async () => { + it("adding a new field will be adding a new header row", async () => { await config.doInContext(structures.uuid(), async () => { const tableColumns = ["name", "description", "new field"] const table = createBasicTable(structures.uuid(), tableColumns) @@ -103,7 +105,7 @@ describe("Google Sheets Integration", () => { }) }) - test("removing an existing field will remove the header from the google sheet", async () => { + it("removing an existing field will remove the header from the google sheet", async () => { const sheet = await config.doInContext(structures.uuid(), async () => { const tableColumns = ["name"] const table = createBasicTable(structures.uuid(), tableColumns) @@ -123,4 +125,20 @@ describe("Google Sheets Integration", () => { expect((sheet.setHeaderRow as any).mock.calls[0][0]).toHaveLength(1) }) }) + + describe("getTableNames", () => { + it("can fetch table names", async () => { + await config.doInContext(structures.uuid(), async () => { + const sheetNames: string[] = [] + for (let i = 0; i < 5; i++) { + const sheet = createSheet({ headerValues: [] }) + sheetsByIndex.push(sheet) + sheetNames.push(sheet.title) + } + + const res = await integration.getTableNames() + expect(res).toEqual(sheetNames) + }) + }) + }) }) From b7b604ca0005c8467aaa5f1e4713e6eb85ab536f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 23 May 2023 10:17:42 +0200 Subject: [PATCH 10/13] Improve test --- .../integrations/tests/googlesheets.spec.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 7f6c641aa9..5aabb13afc 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -18,15 +18,14 @@ const { GoogleSpreadsheet } = require("google-spreadsheet") const sheetsByTitle: { [title: string]: GoogleSpreadsheetWorksheet } = {} const sheetsByIndex: GoogleSpreadsheetWorksheet[] = [] +const mockGoogleIntegration = { + useOAuth2Client: jest.fn(), + loadInfo: jest.fn(), + sheetsByTitle, + sheetsByIndex, +} -GoogleSpreadsheet.mockImplementation(() => { - return { - useOAuth2Client: jest.fn(), - loadInfo: jest.fn(), - sheetsByTitle, - sheetsByIndex, - } -}) +GoogleSpreadsheet.mockImplementation(() => mockGoogleIntegration) import { structures } from "@budibase/backend-core/tests" import TestConfiguration from "../../tests/utilities/TestConfiguration" @@ -55,6 +54,8 @@ describe("Google Sheets Integration", () => { }, }) await config.init() + + jest.clearAllMocks() }) function createBasicTable(name: string, columns: string[]): Table { @@ -137,6 +138,8 @@ describe("Google Sheets Integration", () => { } const res = await integration.getTableNames() + + expect(mockGoogleIntegration.loadInfo).toBeCalledTimes(1) expect(res).toEqual(sheetNames) }) }) From 023373bb254326ef96e30c25a8c8d597baeab871 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 23 May 2023 10:18:37 +0200 Subject: [PATCH 11/13] Remove unnecessary load info --- packages/server/src/integrations/googlesheets.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index 5cef17dde8..d1f3f9e950 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -148,7 +148,6 @@ class GoogleSheetsIntegration implements DatasourcePlus { async testConnection(): Promise { try { await this.connect() - await this.client.loadInfo() return { connected: true } } catch (e: any) { return { From e838a90d3e22914be68be43ace9e28a75fe56905 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 23 May 2023 10:22:26 +0200 Subject: [PATCH 12/13] Add small unit test --- .../src/integrations/tests/googlesheets.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 5aabb13afc..fcb24c152a 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -144,4 +144,15 @@ describe("Google Sheets Integration", () => { }) }) }) + + describe("testConnection", () => { + it("can test successful connections", async () => { + await config.doInContext(structures.uuid(), async () => { + const res = await integration.testConnection() + + expect(mockGoogleIntegration.loadInfo).toBeCalledTimes(1) + expect(res).toEqual({ connected: true }) + }) + }) + }) }) From 590844c8a96f0bd3d68a88e432f87589213dadfc Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 23 May 2023 12:22:22 +0100 Subject: [PATCH 13/13] PR comments. --- packages/server/src/api/controllers/datasource.ts | 6 +++--- packages/server/src/api/routes/datasource.ts | 4 ++-- packages/server/src/integration-test/postgres.spec.ts | 6 +++--- packages/server/src/integrations/oracle.ts | 6 +----- packages/types/src/api/web/app/datasource.ts | 2 +- 5 files changed, 10 insertions(+), 14 deletions(-) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 9ae72ddf50..2d7ba8224f 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -21,7 +21,7 @@ import { CreateDatasourceRequest, VerifyDatasourceRequest, VerifyDatasourceResponse, - FetchTablesDatasourceResponse, + FetchDatasourceInfoResponse, IntegrationBase, DatasourcePlus, } from "@budibase/types" @@ -154,8 +154,8 @@ export async function verify( } } -export async function fetchTables( - ctx: UserCtx +export async function information( + ctx: UserCtx ) { const datasourceId = ctx.params.datasourceId const datasource = await sdk.datasources.get(datasourceId, { enriched: true }) diff --git a/packages/server/src/api/routes/datasource.ts b/packages/server/src/api/routes/datasource.ts index e740510d7f..ac8256539c 100644 --- a/packages/server/src/api/routes/datasource.ts +++ b/packages/server/src/api/routes/datasource.ts @@ -21,9 +21,9 @@ router datasourceController.verify ) .get( - "/api/datasources/:datasourceId/tables", + "/api/datasources/:datasourceId/info", authorized(permissions.BUILDER), - datasourceController.fetchTables + datasourceController.information ) .get( "/api/datasources/:datasourceId", diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index 0fd9b7c143..ce16ca4aba 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -1055,12 +1055,12 @@ describe("postgres integrations", () => { }) }) - describe("GET /api/datasources/:datasourceId/tables", () => { - it("should fetch tables within postgres datasource", async () => { + describe("GET /api/datasources/:datasourceId/info", () => { + it("should fetch information about postgres datasource", async () => { const primaryName = primaryPostgresTable.name const response = await makeRequest( "get", - `/api/datasources/${postgresDatasource._id}/tables` + `/api/datasources/${postgresDatasource._id}/info` ) expect(response.status).toBe(200) expect(response.body.tableNames).toBeDefined() diff --git a/packages/server/src/integrations/oracle.ts b/packages/server/src/integrations/oracle.ts index 55b425f480..afb7021a74 100644 --- a/packages/server/src/integrations/oracle.ts +++ b/packages/server/src/integrations/oracle.ts @@ -330,11 +330,7 @@ class OracleIntegration extends Sql implements DatasourcePlus { const columnsResponse = await this.internalQuery({ sql: this.COLUMNS_SQL, }) - if (!columnsResponse.rows) { - return [] - } else { - return columnsResponse.rows.map(row => row.TABLE_NAME) - } + return (columnsResponse.rows || []).map(row => row.TABLE_NAME) } async testConnection() { diff --git a/packages/types/src/api/web/app/datasource.ts b/packages/types/src/api/web/app/datasource.ts index 0d0406fe55..d15f65eb35 100644 --- a/packages/types/src/api/web/app/datasource.ts +++ b/packages/types/src/api/web/app/datasource.ts @@ -23,7 +23,7 @@ export interface VerifyDatasourceResponse { error?: string } -export interface FetchTablesDatasourceResponse { +export interface FetchDatasourceInfoResponse { tableNames: string[] }