wip

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<string, Table> = {}
     let errors: Record<string, string> = {}
-    await utils.parallelForeach(
-      sheets,
-      async sheet => {
-        // must fetch rows to determine schema
+
+    let promises: Promise<void>[] = []
+    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 }

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: "",
+          },
+        }
+      )
+    })
+  })
 })

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<WorksheetProperties>
 }
 
 // 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<WorksheetProperties>): 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<WorksheetProperties, "dataSourceSheetProperties"> = {
+  private defaultWorksheetProperties(
+    opts: Partial<WorksheetProperties>
+  ): 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,

packages/shared-core/src/utils.ts

@@ -7,43 +7,38 @@ export function unreachable(
   throw new Error(message)
 }
 
+interface PromiseWithId<T> {
+  promise: Promise<T>
+  id: number
+}
+
 export async function parallelForeach<T>(
   items: T[],
   task: (item: T) => Promise<void>,
   maxConcurrency: number
 ): Promise<void> {
-  const promises: Promise<void>[] = []
-  let index = 0
+  try {
+    let next = 0
+    let inProgress: PromiseWithId<number>[] = []
+    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() {