From fd353b2ecf6c292a4ba6bcd5e18ba5c40ddfc194 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 19 Sep 2024 10:09:21 +0100 Subject: [PATCH] wip --- .../server/src/integrations/googlesheets.ts | 63 ++++++++------ .../integrations/tests/googlesheets.spec.ts | 19 ++++- .../integrations/tests/utils/googlesheets.ts | 82 ++++++++++++------- packages/shared-core/src/utils.ts | 53 ++++++------ 4 files changed, 133 insertions(+), 84 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index dd9bef84ab..65606ad75c 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -25,7 +25,11 @@ import { checkExternalTables, finaliseExternalTables, } from "./utils" -import { GoogleSpreadsheet, GoogleSpreadsheetRow } from "google-spreadsheet" +import { + GoogleSpreadsheet, + GoogleSpreadsheetRow, + GoogleSpreadsheetWorksheet, +} from "google-spreadsheet" import fetch from "node-fetch" import { cache, configs, context, HTTPError } from "@budibase/backend-core" import { dataFilters, utils } from "@budibase/shared-core" @@ -330,15 +334,16 @@ export class GoogleSheetsIntegration implements DatasourcePlus { return { tables: {}, errors: {} } } await this.connect() + const sheets = this.client.sheetsByIndex const tables: Record = {} let errors: Record = {} - await utils.parallelForeach( - sheets, - async sheet => { - // must fetch rows to determine schema + + let promises: Promise[] = [] + for (const sheet of sheets) { + const f = async (sheet: GoogleSpreadsheetWorksheet) => { try { - await sheet.getRows() + await sheet.getRows({ limit: 1 }) } 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. @@ -346,26 +351,36 @@ export class GoogleSheetsIntegration implements DatasourcePlus { throw err } - if (err.message.startsWith("No values in the header row")) { - errors[sheet.title] = err.message - } else { - // If we get an error we don't expect, rethrow to avoid failing - // quietly. - throw err + if ( + err.message.startsWith("No values in the header row") || + err.message.startsWith("Header values are not yet loaded") + ) { + errors[ + sheet.title + ] = `Failed to find a header row in sheet "${sheet.title}", is the first row blank?` + return } - return - } - const id = buildExternalTableId(datasourceId, sheet.title) - tables[sheet.title] = this.getTableSchema( - sheet.title, - sheet.headerValues, - datasourceId, - id - ) - }, - 10 - ) + // If we get an error we don't expect, rethrow to avoid failing + // quietly. + throw err + } + } + promises.push(f(sheet)) + } + + await Promise.all(promises) + + for (const sheet of sheets) { + const id = buildExternalTableId(datasourceId, sheet.title) + tables[sheet.title] = this.getTableSchema( + sheet.title, + sheet.headerValues, + datasourceId, + id + ) + } + let externalTables = finaliseExternalTables(tables, entities) errors = { ...errors, ...checkExternalTables(externalTables) } return { tables: externalTables, errors } diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index cef4decfa7..fe2da479c3 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -10,7 +10,6 @@ import { TableSourceType, } from "@budibase/types" import { GoogleSheetsMock } from "./utils/googlesheets" -import rows from "src/sdk/app/rows" describe("Google Sheets Integration", () => { const config = new TestConfiguration() @@ -506,4 +505,22 @@ describe("Google Sheets Integration", () => { expect(emptyRows.length).toEqual(0) }) }) + + describe("import spreadsheet", () => { + it.only("should fail to import a completely blank sheet", async () => { + mock.createSheet({ title: "Sheet1" }) + await config.api.datasource.fetchSchema( + { + datasourceId: datasource._id!, + tablesFilter: ["Sheet1"], + }, + { + status: 400, + body: { + message: "", + }, + } + ) + }) + }) }) diff --git a/packages/server/src/integrations/tests/utils/googlesheets.ts b/packages/server/src/integrations/tests/utils/googlesheets.ts index 4b17c25b01..4747f5f9bf 100644 --- a/packages/server/src/integrations/tests/utils/googlesheets.ts +++ b/packages/server/src/integrations/tests/utils/googlesheets.ts @@ -22,6 +22,7 @@ import type { CellPadding, Color, GridRange, + DataSourceSheetProperties, } from "google-spreadsheet/src/lib/types/sheets-types" const BLACK: Color = { red: 0, green: 0, blue: 0 } @@ -91,7 +92,7 @@ interface UpdateValuesResponse { // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/request#AddSheetRequest interface AddSheetRequest { - properties: WorksheetProperties + properties: Partial } // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/response#AddSheetResponse @@ -236,6 +237,38 @@ export class GoogleSheetsMock { this.mockAPI() } + public cell(cell: string): Value | undefined { + const cellData = this.cellData(cell) + if (!cellData) { + return undefined + } + return this.cellValue(cellData) + } + + public set(cell: string, value: Value): void { + const cellData = this.cellData(cell) + if (!cellData) { + throw new Error(`Cell ${cell} not found`) + } + cellData.userEnteredValue = this.createValue(value) + } + + public sheet(name: string | number): Sheet | undefined { + if (typeof name === "number") { + return this.getSheetById(name) + } + return this.getSheetByName(name) + } + + public createSheet(opts: Partial): Sheet { + const properties = this.defaultWorksheetProperties(opts) + if (this.getSheetByName(properties.title)) { + throw new Error(`Sheet ${properties.title} already exists`) + } + const resp = this.handleAddSheet({ properties }) + return this.getSheetById(resp.properties.sheetId)! + } + private route( method: "get" | "put" | "post", path: string | RegExp, @@ -462,35 +495,39 @@ export class GoogleSheetsMock { return response } - private handleAddSheet(request: AddSheetRequest): AddSheetResponse { - const properties: Omit = { + private defaultWorksheetProperties( + opts: Partial + ): WorksheetProperties { + return { index: this.spreadsheet.sheets.length, hidden: false, rightToLeft: false, tabColor: BLACK, tabColorStyle: { rgbColor: BLACK }, sheetType: "GRID", - title: request.properties.title, + title: "Sheet", sheetId: this.spreadsheet.sheets.length, gridProperties: { rowCount: 100, columnCount: 26, - frozenRowCount: 0, - frozenColumnCount: 0, - hideGridlines: false, - rowGroupControlAfter: false, - columnGroupControlAfter: false, }, + dataSourceSheetProperties: {} as DataSourceSheetProperties, + ...opts, } + } + private handleAddSheet(request: AddSheetRequest): AddSheetResponse { + const properties = this.defaultWorksheetProperties(request.properties) this.spreadsheet.sheets.push({ - properties: properties as WorksheetProperties, - data: [this.createEmptyGrid(100, 26)], + properties, + data: [ + this.createEmptyGrid( + properties.gridProperties.rowCount, + properties.gridProperties.columnCount + ), + ], }) - - // dataSourceSheetProperties is only returned by the API if the sheet type is - // DATA_SOURCE, which we aren't using, so sadly we need to cast here. - return { properties: properties as WorksheetProperties } + return { properties } } private handleDeleteRange(request: DeleteRangeRequest) { @@ -767,21 +804,6 @@ export class GoogleSheetsMock { return this.getCellNumericIndexes(sheetId, startRowIndex, startColumnIndex) } - public cell(cell: string): Value | undefined { - const cellData = this.cellData(cell) - if (!cellData) { - return undefined - } - return this.cellValue(cellData) - } - - public sheet(name: string | number): Sheet | undefined { - if (typeof name === "number") { - return this.getSheetById(name) - } - return this.getSheetByName(name) - } - private getCellNumericIndexes( sheet: Sheet | number, row: number, diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index b69a059745..31ce8778cd 100644 --- a/packages/shared-core/src/utils.ts +++ b/packages/shared-core/src/utils.ts @@ -7,43 +7,38 @@ export function unreachable( throw new Error(message) } +interface PromiseWithId { + promise: Promise + id: number +} + export async function parallelForeach( items: T[], task: (item: T) => Promise, maxConcurrency: number ): Promise { - const promises: Promise[] = [] - let index = 0 + try { + let next = 0 + let inProgress: PromiseWithId[] = [] + while (next < items.length) { + if (inProgress.length === maxConcurrency) { + const finished = await Promise.race(inProgress.map(t => t.promise)) + inProgress = inProgress.filter(task => task.id !== finished) + } - const processItem = async (item: T) => { - try { - await task(item) - } finally { - processNext() + const promise = async (next: number) => { + await task(items[next]) + return next + } + + inProgress.push({ promise: promise(next), id: next }) + next++ } + await Promise.all(inProgress.map(t => t.promise)) + } catch (e) { + console.error(e) + throw e } - - const processNext = () => { - if (index >= items.length) { - // No more items to process - return - } - - const item = items[index] - index++ - - const promise = processItem(item) - promises.push(promise) - - if (promises.length >= maxConcurrency) { - Promise.race(promises).then(processNext) - } else { - processNext() - } - } - processNext() - - await Promise.all(promises) } export function filterValueToLabel() {