From 635049ab0f641913022fa39d61bef35cdd203f59 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 12 Dec 2024 14:02:52 +0000 Subject: [PATCH 1/3] Making sure table IDs input are correctly formatted for comparison. --- globalSetup.ts | 8 ++++---- packages/backend-core/src/sql/utils.ts | 8 ++++++++ packages/server/src/api/controllers/row/index.ts | 12 +++++++----- .../server/src/api/controllers/row/utils/utils.ts | 12 +++++++----- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/globalSetup.ts b/globalSetup.ts index ec4f38b388..0b0e276b49 100644 --- a/globalSetup.ts +++ b/globalSetup.ts @@ -4,8 +4,8 @@ import { getContainerRuntimeClient, } from "testcontainers" import { ContainerInfo } from "dockerode" -import path from "path" -import lockfile from "proper-lockfile" +import * as path from "path" +import * as lockfile from "proper-lockfile" import { execSync } from "child_process" interface DockerContext { @@ -29,8 +29,8 @@ function getCurrentDockerContext(): DockerContext { async function getBudibaseContainers() { const client = await getContainerRuntimeClient() - const conatiners = await client.container.list() - return conatiners.filter( + const containers = await client.container.list() + return containers.filter( container => container.Labels["com.budibase"] === "true" && container.Labels["org.testcontainers"] === "true" diff --git a/packages/backend-core/src/sql/utils.ts b/packages/backend-core/src/sql/utils.ts index 1b80ff337d..f1e0c4c5ce 100644 --- a/packages/backend-core/src/sql/utils.ts +++ b/packages/backend-core/src/sql/utils.ts @@ -66,6 +66,14 @@ export function buildExternalTableId(datasourceId: string, tableName: string) { return `${datasourceId}${DOUBLE_SEPARATOR}${tableName}` } +export function checkTableId(tableId: string) { + if (isExternalTableID(tableId) && tableId.includes(" ")) { + return encodeURIComponent(tableId) + } else { + return tableId + } +} + export function breakExternalTableId(tableId: string) { const parts = tableId.split(DOUBLE_SEPARATOR) let datasourceId = parts.shift() diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 0463c0a565..77c05abb95 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -288,19 +288,21 @@ function replaceTableNamesInFilters( for (const key of Object.keys(filter)) { const matches = key.match(`^(?.+)\\.(?.+)`) - const relation = matches?.groups?.["relation"] + // this is the possible table name which we need to check if it needs to be converted + const relatedTableName = matches?.groups?.["relation"] const field = matches?.groups?.["field"] - if (!relation || !field) { + if (!relatedTableName || !field) { continue } - const table = allTables.find(r => r._id === tableId)! - if (Object.values(table.schema).some(f => f.name === relation)) { + const table = allTables.find(r => r._id === tableId) + const isColumnName = !!table?.schema[relatedTableName] + if (!table || isColumnName) { continue } - const matchedTable = allTables.find(t => t.name === relation) + const matchedTable = allTables.find(t => t.name === relatedTableName) const relationship = Object.values(table.schema).find( f => isRelationshipField(f) && f.tableId === matchedTable?._id ) diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index 5b60143792..b729d7470d 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -1,6 +1,6 @@ import * as utils from "../../../../db/utils" -import { docIds } from "@budibase/backend-core" +import { docIds, sql } from "@budibase/backend-core" import { Ctx, DatasourcePlusQueryResponse, @@ -65,19 +65,21 @@ export function getSourceId(ctx: Ctx): { tableId: string; viewId?: string } { const { sourceId } = ctx.params if (docIds.isViewId(sourceId)) { return { - tableId: utils.extractViewInfoFromID(sourceId).tableId, + tableId: sql.utils.checkTableId( + utils.extractViewInfoFromID(sourceId).tableId + ), viewId: sourceId, } } - return { tableId: ctx.params.sourceId } + return { tableId: sql.utils.checkTableId(ctx.params.sourceId) } } // now check for old way of specifying table ID if (ctx.params?.tableId) { - return { tableId: ctx.params.tableId } + return { tableId: sql.utils.checkTableId(ctx.params.tableId) } } // check body for a table ID if (ctx.request.body?.tableId) { - return { tableId: ctx.request.body.tableId } + return { tableId: sql.utils.checkTableId(ctx.request.body.tableId) } } throw new Error("Unable to find table ID in request") } From 7f62f32adc59e911c9d3e5f52492f472c77b36b9 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 12 Dec 2024 15:55:39 +0000 Subject: [PATCH 2/3] Adding test case for tables with spaces. --- .../src/api/routes/tests/search.spec.ts | 59 ++++++++++++++++--- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 67f303aac3..e97f48afbe 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -71,18 +71,27 @@ if (descriptions.length) { let tableOrViewId: string let rows: Row[] - async function basicRelationshipTables(type: RelationshipType) { + async function basicRelationshipTables( + type: RelationshipType, + opts?: { + tableName?: string + primaryColumn?: string + otherColumn?: string + } + ) { const relatedTable = await createTable({ - name: { name: "name", type: FieldType.STRING }, + name: { name: opts?.tableName || "name", type: FieldType.STRING }, }) + + const columnName = opts?.primaryColumn || "productCat" + //@ts-ignore - API accepts this structure, will build out rest of definition const tableId = await createTable({ - name: { name: "name", type: FieldType.STRING }, - //@ts-ignore - API accepts this structure, will build out rest of definition - productCat: { + name: { name: opts?.tableName || "name", type: FieldType.STRING }, + [columnName]: { type: FieldType.LINK, relationshipType: type, - name: "productCat", - fieldName: "product", + name: columnName, + fieldName: opts?.otherColumn || "product", tableId: relatedTable, constraints: { type: "array", @@ -2776,6 +2785,42 @@ if (descriptions.length) { }) }) + isSql && + describe("relationship - table with spaces", () => { + let primaryTable: Table, row: Row + + beforeAll(async () => { + const { relatedTable, tableId } = + await basicRelationshipTables( + RelationshipType.ONE_TO_MANY, + { + tableName: "table with spaces", + primaryColumn: "related", + otherColumn: "related", + } + ) + tableOrViewId = tableId + primaryTable = relatedTable + + row = await config.api.row.save(primaryTable._id!, { + name: "foo", + }) + + await config.api.row.save(tableOrViewId, { + name: "foo", + related: [row._id], + }) + }) + + it("should be able to search by table name with spaces", async () => { + await expectQuery({ + equal: { + ["table with spaces.name"]: "foo", + }, + }).toContain([{ name: "foo" }]) + }) + }) + isSql && describe.each([ RelationshipType.MANY_TO_ONE, From f857c36e09813c3224be4afaa5d6809f0612ccbf Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 12 Dec 2024 16:50:21 +0000 Subject: [PATCH 3/3] PR comments. --- packages/backend-core/src/sql/utils.ts | 10 +++------- packages/server/src/api/controllers/row/utils/utils.ts | 10 ++++------ packages/server/src/db/utils.ts | 10 ++++++++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/backend-core/src/sql/utils.ts b/packages/backend-core/src/sql/utils.ts index f1e0c4c5ce..14127a189f 100644 --- a/packages/backend-core/src/sql/utils.ts +++ b/packages/backend-core/src/sql/utils.ts @@ -59,15 +59,11 @@ export function isExternalTable(table: Table) { } export function buildExternalTableId(datasourceId: string, tableName: string) { - // encode spaces - if (tableName.includes(" ")) { - tableName = encodeURIComponent(tableName) - } - return `${datasourceId}${DOUBLE_SEPARATOR}${tableName}` + return `${datasourceId}${DOUBLE_SEPARATOR}${encodeURIComponent(tableName)}` } -export function checkTableId(tableId: string) { - if (isExternalTableID(tableId) && tableId.includes(" ")) { +export function encodeTableId(tableId: string) { + if (isExternalTableID(tableId)) { return encodeURIComponent(tableId) } else { return tableId diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index b729d7470d..baa811fe90 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -65,21 +65,19 @@ export function getSourceId(ctx: Ctx): { tableId: string; viewId?: string } { const { sourceId } = ctx.params if (docIds.isViewId(sourceId)) { return { - tableId: sql.utils.checkTableId( - utils.extractViewInfoFromID(sourceId).tableId - ), + tableId: utils.extractViewInfoFromID(sourceId).tableId, viewId: sourceId, } } - return { tableId: sql.utils.checkTableId(ctx.params.sourceId) } + return { tableId: sql.utils.encodeTableId(ctx.params.sourceId) } } // now check for old way of specifying table ID if (ctx.params?.tableId) { - return { tableId: sql.utils.checkTableId(ctx.params.tableId) } + return { tableId: sql.utils.encodeTableId(ctx.params.tableId) } } // check body for a table ID if (ctx.request.body?.tableId) { - return { tableId: sql.utils.checkTableId(ctx.request.body.tableId) } + return { tableId: sql.utils.encodeTableId(ctx.request.body.tableId) } } throw new Error("Unable to find table ID in request") } diff --git a/packages/server/src/db/utils.ts b/packages/server/src/db/utils.ts index 6c1065e847..70c69b3c60 100644 --- a/packages/server/src/db/utils.ts +++ b/packages/server/src/db/utils.ts @@ -1,4 +1,10 @@ -import { context, db as dbCore, docIds, utils } from "@budibase/backend-core" +import { + context, + db as dbCore, + docIds, + utils, + sql, +} from "@budibase/backend-core" import { DatabaseQueryOpts, Datasource, @@ -328,7 +334,7 @@ export function extractViewInfoFromID(viewId: string) { const regex = new RegExp(`^(?.+)${SEPARATOR}([^${SEPARATOR}]+)$`) const res = regex.exec(viewId) return { - tableId: res!.groups!["tableId"], + tableId: sql.utils.encodeTableId(res!.groups!["tableId"]), } }