From a26f2e83e49a1acc24f3b262666ec273e7c4d470 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 9 Nov 2023 14:45:23 +0000 Subject: [PATCH 1/4] Some of the new table getters did not account for table IDs not being found, adding a new function which properly accounts for IDs being missing and manages it correctly, rather than leaving tables in an undefined state. --- packages/backend-core/src/db/couch/DatabaseImpl.ts | 13 ++++++++++++- .../server/src/api/controllers/row/internal.ts | 11 ++++------- packages/server/src/api/controllers/row/utils.ts | 14 +------------- packages/server/src/db/linkedRows/index.ts | 6 ++---- packages/server/src/db/utils.ts | 10 ---------- packages/server/src/sdk/app/tables/getters.ts | 14 +++++++++----- packages/server/src/utilities/global.ts | 6 ++---- packages/types/src/sdk/db.ts | 3 ++- 8 files changed, 32 insertions(+), 45 deletions(-) diff --git a/packages/backend-core/src/db/couch/DatabaseImpl.ts b/packages/backend-core/src/db/couch/DatabaseImpl.ts index 1ace60ed5b..96b946fe16 100644 --- a/packages/backend-core/src/db/couch/DatabaseImpl.ts +++ b/packages/backend-core/src/db/couch/DatabaseImpl.ts @@ -109,7 +109,7 @@ export class DatabaseImpl implements Database { } } - async get(id?: string): Promise { + async get(id?: string): Promise { const db = await this.checkSetup() if (!id) { throw new Error("Unable to get doc without a valid _id.") @@ -117,6 +117,17 @@ export class DatabaseImpl implements Database { return this.updateOutput(() => db.get(id)) } + async getMultiple(ids: string[]): Promise { + // get unique + ids = [...new Set(ids)] + const response = await this.allDocs({ + keys: ids, + include_docs: true, + }) + const rows = response.rows.filter(row => row.error !== "not_found") + return rows.map(row => row.doc!) + } + async remove(idOrDoc: string | Document, rev?: string) { const db = await this.checkSetup() let _id: string diff --git a/packages/server/src/api/controllers/row/internal.ts b/packages/server/src/api/controllers/row/internal.ts index fe7d94547a..0edbe4ea86 100644 --- a/packages/server/src/api/controllers/row/internal.ts +++ b/packages/server/src/api/controllers/row/internal.ts @@ -1,9 +1,5 @@ import * as linkRows from "../../../db/linkedRows" -import { - generateRowID, - getMultiIDParams, - InternalTables, -} from "../../../db/utils" +import { generateRowID, InternalTables } from "../../../db/utils" import * as userController from "../user" import { cleanupAttachments, @@ -240,8 +236,9 @@ export async function fetchEnrichedRow(ctx: UserCtx) { const linkVals = links as LinkDocumentValue[] // look up the actual rows based on the ids - const params = getMultiIDParams(linkVals.map(linkVal => linkVal.id)) - let linkedRows = (await db.allDocs(params)).rows.map(row => row.doc!) + let linkedRows = await db.getMultiple( + linkVals.map(linkVal => linkVal.id) + ) // get the linked tables const linkTableIds = getLinkedTableIDs(table as Table) diff --git a/packages/server/src/api/controllers/row/utils.ts b/packages/server/src/api/controllers/row/utils.ts index cd311fdf0f..ed6ccd4c53 100644 --- a/packages/server/src/api/controllers/row/utils.ts +++ b/packages/server/src/api/controllers/row/utils.ts @@ -1,21 +1,9 @@ import { InternalTables } from "../../../db/utils" import * as userController from "../user" import { context } from "@budibase/backend-core" -import { - Ctx, - FieldType, - ManyToOneRelationshipFieldMetadata, - OneToManyRelationshipFieldMetadata, - Row, - SearchFilters, - Table, - UserCtx, -} from "@budibase/types" -import { FieldTypes, NoEmptyFilterStrings } from "../../../constants" -import sdk from "../../../sdk" +import { Ctx, Row, UserCtx } from "@budibase/types" import validateJs from "validate.js" -import { cloneDeep } from "lodash/fp" validateJs.extend(validateJs.validators.datetime, { parse: function (value: string) { diff --git a/packages/server/src/db/linkedRows/index.ts b/packages/server/src/db/linkedRows/index.ts index 7324fa1d94..24ff47ead3 100644 --- a/packages/server/src/db/linkedRows/index.ts +++ b/packages/server/src/db/linkedRows/index.ts @@ -8,7 +8,7 @@ import { getLinkedTable, } from "./linkUtils" import flatten from "lodash/flatten" -import { getMultiIDParams, USER_METDATA_PREFIX } from "../utils" +import { USER_METDATA_PREFIX } from "../utils" import partition from "lodash/partition" import { getGlobalUsersFromMetadata } from "../../utilities/global" import { processFormulas } from "../../utilities/rowProcessor" @@ -79,9 +79,7 @@ async function getFullLinkedDocs(links: LinkDocumentValue[]) { const db = context.getAppDB() const linkedRowIds = links.map(link => link.id) const uniqueRowIds = [...new Set(linkedRowIds)] - let dbRows = (await db.allDocs(getMultiIDParams(uniqueRowIds))).rows.map( - row => row.doc! - ) + let dbRows = await db.getMultiple(uniqueRowIds) // convert the unique db rows back to a full list of linked rows const linked = linkedRowIds .map(id => dbRows.find(row => row && row._id === id)) diff --git a/packages/server/src/db/utils.ts b/packages/server/src/db/utils.ts index 715db552c9..a5569f8166 100644 --- a/packages/server/src/db/utils.ts +++ b/packages/server/src/db/utils.ts @@ -283,16 +283,6 @@ export function generatePluginID(name: string) { return `${DocumentType.PLUGIN}${SEPARATOR}${name}` } -/** - * This can be used with the db.allDocs to get a list of IDs - */ -export function getMultiIDParams(ids: string[]) { - return { - keys: ids, - include_docs: true, - } -} - /** * Generates a new view ID. * @returns The new view ID which the view doc can be stored under. diff --git a/packages/server/src/sdk/app/tables/getters.ts b/packages/server/src/sdk/app/tables/getters.ts index a7074f95b2..a09d686b6d 100644 --- a/packages/server/src/sdk/app/tables/getters.ts +++ b/packages/server/src/sdk/app/tables/getters.ts @@ -1,5 +1,5 @@ import { context } from "@budibase/backend-core" -import { getMultiIDParams, getTableParams } from "../../../db/utils" +import { getTableParams } from "../../../db/utils" import { breakExternalTableId, isExternalTableID, @@ -17,6 +17,9 @@ import datasources from "../datasources" import sdk from "../../../sdk" export function processTable(table: Table): Table { + if (!table) { + return table + } if (table._id && isExternalTableID(table._id)) { return { ...table, @@ -73,6 +76,9 @@ export async function getExternalTable( tableName: string ): Promise { const entities = await getExternalTablesInDatasource(datasourceId) + if (!entities[tableName]) { + throw new Error(`Unable to find table named "${tableName}"`) + } return processTable(entities[tableName]) } @@ -124,10 +130,8 @@ export async function getTables(tableIds: string[]): Promise { } if (internalTableIds.length) { const db = context.getAppDB() - const internalTableDocs = await db.allDocs
( - getMultiIDParams(internalTableIds) - ) - tables = tables.concat(internalTableDocs.rows.map(row => row.doc!)) + const internalTables = await db.getMultiple
(internalTableIds) + tables = tables.concat(internalTables) } return processTables(tables) } diff --git a/packages/server/src/utilities/global.ts b/packages/server/src/utilities/global.ts index cdc2d84513..762d1fb7a0 100644 --- a/packages/server/src/utilities/global.ts +++ b/packages/server/src/utilities/global.ts @@ -1,4 +1,4 @@ -import { getMultiIDParams, getGlobalIDFromUserMetadataID } from "../db/utils" +import { getGlobalIDFromUserMetadataID } from "../db/utils" import { roles, db as dbCore, @@ -96,9 +96,7 @@ export async function getRawGlobalUsers(userIds?: string[]): Promise { const db = tenancy.getGlobalDB() let globalUsers: User[] if (userIds) { - globalUsers = (await db.allDocs(getMultiIDParams(userIds))).rows.map( - row => row.doc! - ) + globalUsers = await db.getMultiple(userIds) } else { globalUsers = ( await db.allDocs( diff --git a/packages/types/src/sdk/db.ts b/packages/types/src/sdk/db.ts index 26807d99ce..9ea854b3aa 100644 --- a/packages/types/src/sdk/db.ts +++ b/packages/types/src/sdk/db.ts @@ -122,7 +122,8 @@ export interface Database { exists(): Promise checkSetup(): Promise> - get(id?: string): Promise + get(id?: string): Promise + getMultiple(ids: string[]): Promise remove( id: string | Document, rev?: string From 37e34c8ed2e79c1c5981a95e6402736643d94d1e Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 9 Nov 2023 14:53:14 +0000 Subject: [PATCH 2/4] Adding the ability to fail on getMultiple if needed. --- packages/backend-core/src/db/couch/DatabaseImpl.ts | 14 ++++++++++++-- packages/types/src/sdk/db.ts | 5 ++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/db/couch/DatabaseImpl.ts b/packages/backend-core/src/db/couch/DatabaseImpl.ts index 96b946fe16..fa505847af 100644 --- a/packages/backend-core/src/db/couch/DatabaseImpl.ts +++ b/packages/backend-core/src/db/couch/DatabaseImpl.ts @@ -117,14 +117,24 @@ export class DatabaseImpl implements Database { return this.updateOutput(() => db.get(id)) } - async getMultiple(ids: string[]): Promise { + async getMultiple( + ids: string[], + opts?: { failIfMissing?: boolean } + ): Promise { // get unique ids = [...new Set(ids)] const response = await this.allDocs({ keys: ids, include_docs: true, }) - const rows = response.rows.filter(row => row.error !== "not_found") + const NOT_FOUND = "not_found" + const rows = response.rows.filter(row => row.error !== NOT_FOUND) + // some were filtered out - means some missing + if (opts?.failIfMissing && rows.length !== response.rows.length) { + const missing = response.rows.filter(row => row.error === NOT_FOUND) + const missingIds = missing.map(row => row.key).join(", ") + throw new Error(`Unable to get documents: ${missingIds}`) + } return rows.map(row => row.doc!) } diff --git a/packages/types/src/sdk/db.ts b/packages/types/src/sdk/db.ts index 9ea854b3aa..ff5742bcfc 100644 --- a/packages/types/src/sdk/db.ts +++ b/packages/types/src/sdk/db.ts @@ -123,7 +123,10 @@ export interface Database { exists(): Promise checkSetup(): Promise> get(id?: string): Promise - getMultiple(ids: string[]): Promise + getMultiple( + ids: string[], + opts?: { failIfMissing?: boolean } + ): Promise remove( id: string | Document, rev?: string From dde446286de6922d79953881b11abb1033d33e0e Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 9 Nov 2023 17:08:14 +0000 Subject: [PATCH 3/4] Switching getMultiple to default to failure if not all entries found, then updating usages. --- packages/backend-core/src/db/couch/DatabaseImpl.ts | 5 +++-- packages/server/src/api/controllers/row/internal.ts | 3 ++- packages/server/src/db/linkedRows/index.ts | 2 +- packages/server/src/sdk/app/tables/getters.ts | 4 +++- packages/server/src/utilities/global.ts | 2 +- packages/types/src/sdk/db.ts | 2 +- 6 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/backend-core/src/db/couch/DatabaseImpl.ts b/packages/backend-core/src/db/couch/DatabaseImpl.ts index fa505847af..6a1e575ac9 100644 --- a/packages/backend-core/src/db/couch/DatabaseImpl.ts +++ b/packages/backend-core/src/db/couch/DatabaseImpl.ts @@ -119,7 +119,7 @@ export class DatabaseImpl implements Database { async getMultiple( ids: string[], - opts?: { failIfMissing?: boolean } + opts?: { allowMissing?: boolean } ): Promise { // get unique ids = [...new Set(ids)] @@ -129,8 +129,9 @@ export class DatabaseImpl implements Database { }) const NOT_FOUND = "not_found" const rows = response.rows.filter(row => row.error !== NOT_FOUND) + const someMissing = rows.length !== response.rows.length // some were filtered out - means some missing - if (opts?.failIfMissing && rows.length !== response.rows.length) { + if (!opts?.allowMissing && someMissing) { const missing = response.rows.filter(row => row.error === NOT_FOUND) const missingIds = missing.map(row => row.key).join(", ") throw new Error(`Unable to get documents: ${missingIds}`) diff --git a/packages/server/src/api/controllers/row/internal.ts b/packages/server/src/api/controllers/row/internal.ts index 0edbe4ea86..0907c22f0e 100644 --- a/packages/server/src/api/controllers/row/internal.ts +++ b/packages/server/src/api/controllers/row/internal.ts @@ -237,7 +237,8 @@ export async function fetchEnrichedRow(ctx: UserCtx) { // look up the actual rows based on the ids let linkedRows = await db.getMultiple( - linkVals.map(linkVal => linkVal.id) + linkVals.map(linkVal => linkVal.id), + { allowMissing: true } ) // get the linked tables diff --git a/packages/server/src/db/linkedRows/index.ts b/packages/server/src/db/linkedRows/index.ts index 24ff47ead3..7af3f9392f 100644 --- a/packages/server/src/db/linkedRows/index.ts +++ b/packages/server/src/db/linkedRows/index.ts @@ -79,7 +79,7 @@ async function getFullLinkedDocs(links: LinkDocumentValue[]) { const db = context.getAppDB() const linkedRowIds = links.map(link => link.id) const uniqueRowIds = [...new Set(linkedRowIds)] - let dbRows = await db.getMultiple(uniqueRowIds) + let dbRows = await db.getMultiple(uniqueRowIds, { allowMissing: true }) // convert the unique db rows back to a full list of linked rows const linked = linkedRowIds .map(id => dbRows.find(row => row && row._id === id)) diff --git a/packages/server/src/sdk/app/tables/getters.ts b/packages/server/src/sdk/app/tables/getters.ts index a09d686b6d..72a6ab61f1 100644 --- a/packages/server/src/sdk/app/tables/getters.ts +++ b/packages/server/src/sdk/app/tables/getters.ts @@ -130,7 +130,9 @@ export async function getTables(tableIds: string[]): Promise { } if (internalTableIds.length) { const db = context.getAppDB() - const internalTables = await db.getMultiple
(internalTableIds) + const internalTables = await db.getMultiple
(internalTableIds, { + allowMissing: true, + }) tables = tables.concat(internalTables) } return processTables(tables) diff --git a/packages/server/src/utilities/global.ts b/packages/server/src/utilities/global.ts index 762d1fb7a0..bbb84c1882 100644 --- a/packages/server/src/utilities/global.ts +++ b/packages/server/src/utilities/global.ts @@ -96,7 +96,7 @@ export async function getRawGlobalUsers(userIds?: string[]): Promise { const db = tenancy.getGlobalDB() let globalUsers: User[] if (userIds) { - globalUsers = await db.getMultiple(userIds) + globalUsers = await db.getMultiple(userIds, { allowMissing: true }) } else { globalUsers = ( await db.allDocs( diff --git a/packages/types/src/sdk/db.ts b/packages/types/src/sdk/db.ts index ff5742bcfc..7613ac6aeb 100644 --- a/packages/types/src/sdk/db.ts +++ b/packages/types/src/sdk/db.ts @@ -125,7 +125,7 @@ export interface Database { get(id?: string): Promise getMultiple( ids: string[], - opts?: { failIfMissing?: boolean } + opts?: { allowMissing?: boolean } ): Promise remove( id: string | Document, From 20895cf4266f3accc30aba8bfd5678793195de04 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 9 Nov 2023 18:22:06 +0000 Subject: [PATCH 4/4] Adding test case. --- packages/server/src/sdk/tests/tables.spec.ts | 39 ++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 packages/server/src/sdk/tests/tables.spec.ts diff --git a/packages/server/src/sdk/tests/tables.spec.ts b/packages/server/src/sdk/tests/tables.spec.ts new file mode 100644 index 0000000000..0e3cd73cfd --- /dev/null +++ b/packages/server/src/sdk/tests/tables.spec.ts @@ -0,0 +1,39 @@ +import TestConfig from "../../tests/utilities/TestConfiguration" +import { basicTable } from "../../tests/utilities/structures" +import { Table } from "@budibase/types" +import sdk from "../" + +describe("tables", () => { + const config = new TestConfig() + let table: Table + + beforeAll(async () => { + await config.init() + table = await config.api.table.create(basicTable()) + }) + + describe("getTables", () => { + it("should be able to retrieve tables", async () => { + await config.doInContext(config.appId, async () => { + const tables = await sdk.tables.getTables([table._id!]) + expect(tables.length).toBe(1) + expect(tables[0]._id).toBe(table._id) + expect(tables[0].name).toBe(table.name) + }) + }) + + it("shouldn't fail when retrieving tables that don't exist", async () => { + await config.doInContext(config.appId, async () => { + const tables = await sdk.tables.getTables(["unknown"]) + expect(tables.length).toBe(0) + }) + }) + + it("should de-duplicate the IDs", async () => { + await config.doInContext(config.appId, async () => { + const tables = await sdk.tables.getTables([table._id!, table._id!]) + expect(tables.length).toBe(1) + }) + }) + }) +})