From 9fb1c6b98812319f6e84305510323312452d0834 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 26 Jul 2024 16:23:46 +0100 Subject: [PATCH 1/4] When searching by row ID with external DBs/SQS we can get into a situation where the limit of 1 which is applied by the frontend can cause problems, with many to many relationships we need to retrieve multiple rows (all of the joined related rows). This was raised by poirazis, it exhibits itself in one part of the platform, when attempting to a row by ID in a form block that has multiple many to many relationships. The frontend needs to be able to send a limit of 1 incase it is using a form block but hasn't gotten a row ID (this can happen in preview/the builder) and it just wants to populate with a row for display. --- .../src/api/routes/tests/search.spec.ts | 48 +++++++++++++++++- .../src/sdk/app/rows/search/external.ts | 50 ++++++++++++++----- .../server/src/sdk/app/rows/search/sqs.ts | 13 +++-- .../server/src/sdk/app/rows/search/utils.ts | 15 ++++++ 4 files changed, 106 insertions(+), 20 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index e774158c23..82763c78aa 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -5,12 +5,12 @@ import { knexClient, } from "../../../integrations/tests/utils" import { - db as dbCore, context, + db as dbCore, MAX_VALID_DATE, MIN_VALID_DATE, - utils, SQLITE_DESIGN_DOC_ID, + utils, } from "@budibase/backend-core" import * as setup from "./utilities" @@ -2560,4 +2560,48 @@ describe.each([ }).toContainExactly([{ name: "foo" }]) }) }) + + !isInMemory && + describe("search by _id", () => { + let row: Row + + beforeAll(async () => { + const toRelateTable = await createTable({ + name: { + name: "name", + type: FieldType.STRING, + }, + }) + table = await createTable({ + name: { + name: "name", + type: FieldType.STRING, + }, + relationship: { + name: "relationship", + type: FieldType.LINK, + relationshipType: RelationshipType.MANY_TO_MANY, + tableId: toRelateTable._id!, + fieldName: "relationship", + }, + }) + const [row1, row2] = await Promise.all([ + config.api.row.save(toRelateTable._id!, { name: "tag 1" }), + config.api.row.save(toRelateTable._id!, { name: "tag 2" }), + ]) + row = await config.api.row.save(table._id!, { + name: "product 1", + relationship: [row1._id, row2._id], + }) + }) + + it("can filter by the row ID with limit 1", async () => { + await expectSearch({ + query: { + equal: { _id: row._id }, + }, + limit: 1, + }).toContainExactly([row]) + }) + }) }) diff --git a/packages/server/src/sdk/app/rows/search/external.ts b/packages/server/src/sdk/app/rows/search/external.ts index c7a89bc0dd..25793a4554 100644 --- a/packages/server/src/sdk/app/rows/search/external.ts +++ b/packages/server/src/sdk/app/rows/search/external.ts @@ -22,21 +22,21 @@ import { HTTPError } from "@budibase/backend-core" import pick from "lodash/pick" import { outputProcessing } from "../../../../utilities/rowProcessor" import sdk from "../../../" +import { isSearchingByRowID } from "./utils" -export async function search( - options: RowSearchParams, - table: Table -): Promise> { - const { tableId } = options - const { countRows, paginate, query, ...params } = options - const { limit } = params - let bookmark = - (params.bookmark && parseInt(params.bookmark as string)) || undefined - if (paginate && !bookmark) { - bookmark = 0 - } +function getPaginationAndLimitParameters( + filters: SearchFilters, + paginate: boolean | undefined, + bookmark: number | undefined, + limit: number | undefined +): PaginationJson | undefined { let paginateObj: PaginationJson | undefined + // only try set limits/pagination if we aren't doing a row ID search + if (!isSearchingByRowID(filters)) { + return + } + if (paginate && !limit) { throw new Error("Cannot paginate query without a limit") } @@ -49,11 +49,35 @@ export async function search( if (bookmark) { paginateObj.offset = limit * bookmark } - } else if (params && limit) { + } else if (limit) { paginateObj = { limit: limit, } } + + return paginateObj +} + +export async function search( + options: RowSearchParams, + table: Table +): Promise> { + const { tableId } = options + const { countRows, paginate, query, ...params } = options + const { limit } = params + let bookmark = + (params.bookmark && parseInt(params.bookmark as string)) || undefined + if (paginate && !bookmark) { + bookmark = 0 + } + + let paginateObj = getPaginationAndLimitParameters( + query, + paginate, + bookmark, + limit + ) + let sort: SortJson | undefined if (params.sort) { const direction = diff --git a/packages/server/src/sdk/app/rows/search/sqs.ts b/packages/server/src/sdk/app/rows/search/sqs.ts index c3da565c87..018b2ae4a3 100644 --- a/packages/server/src/sdk/app/rows/search/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/sqs.ts @@ -42,6 +42,7 @@ import { getTableIDList, } from "./filters" import { dataFilters, PROTECTED_INTERNAL_COLUMNS } from "@budibase/shared-core" +import { isSearchingByRowID } from "./utils" const builder = new sql.Sql(SqlClient.SQL_LITE) const MISSING_COLUMN_REGEX = new RegExp(`no such column: .+`) @@ -264,6 +265,10 @@ export async function search( const relationships = buildInternalRelationships(table) + const searchFilters: SearchFilters = { + ...cleanupFilters(query, table, allTables), + documentType: DocumentType.ROW, + } const request: QueryJson = { endpoint: { // not important, we query ourselves @@ -271,10 +276,7 @@ export async function search( entityId: table._id!, operation: Operation.READ, }, - filters: { - ...cleanupFilters(query, table, allTables), - documentType: DocumentType.ROW, - }, + filters: searchFilters, table, meta: { table, @@ -304,7 +306,8 @@ export async function search( } const bookmark: number = (params.bookmark as number) || 0 - if (params.limit) { + // limits don't apply if we doing a row ID search + if (!isSearchingByRowID(searchFilters) && params.limit) { paginate = true request.paginate = { limit: params.limit + 1, diff --git a/packages/server/src/sdk/app/rows/search/utils.ts b/packages/server/src/sdk/app/rows/search/utils.ts index 797383eff0..70f4362238 100644 --- a/packages/server/src/sdk/app/rows/search/utils.ts +++ b/packages/server/src/sdk/app/rows/search/utils.ts @@ -108,3 +108,18 @@ export function searchInputMapping(table: Table, options: RowSearchParams) { } return options } + +export function isSearchingByRowID(query: SearchFilters): boolean { + for (let searchField of Object.values(query)) { + if (typeof searchField !== "object") { + continue + } + const hasId = Object.keys(searchField).find( + key => key.endsWith("_id") && searchField[key] + ) + if (hasId) { + return true + } + } + return false +} From 1beae2c0408c8475cf81a832a0cd63adfacffa97 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 26 Jul 2024 16:57:07 +0100 Subject: [PATCH 2/4] Fix bug (thanks tests) --- packages/server/src/sdk/app/rows/search/external.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search/external.ts b/packages/server/src/sdk/app/rows/search/external.ts index 25793a4554..e7fd2888de 100644 --- a/packages/server/src/sdk/app/rows/search/external.ts +++ b/packages/server/src/sdk/app/rows/search/external.ts @@ -33,7 +33,7 @@ function getPaginationAndLimitParameters( let paginateObj: PaginationJson | undefined // only try set limits/pagination if we aren't doing a row ID search - if (!isSearchingByRowID(filters)) { + if (isSearchingByRowID(filters)) { return } From 27d4226c6eb4d8027a20d73bf152dd1b69477ea1 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 26 Jul 2024 17:06:04 +0100 Subject: [PATCH 3/4] Shorten column name. --- packages/server/src/api/routes/tests/search.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 82763c78aa..bad512e5fd 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2577,12 +2577,12 @@ describe.each([ name: "name", type: FieldType.STRING, }, - relationship: { - name: "relationship", + rel: { + name: "rel", type: FieldType.LINK, relationshipType: RelationshipType.MANY_TO_MANY, tableId: toRelateTable._id!, - fieldName: "relationship", + fieldName: "rel", }, }) const [row1, row2] = await Promise.all([ @@ -2591,7 +2591,7 @@ describe.each([ ]) row = await config.api.row.save(table._id!, { name: "product 1", - relationship: [row1._id, row2._id], + rel: [row1._id, row2._id], }) }) From 03f9219d1da6ccd4a6db80c7257dde4b5a59b809 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 30 Jul 2024 11:57:06 +0100 Subject: [PATCH 4/4] PR comment. --- packages/server/src/sdk/app/rows/search/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search/utils.ts b/packages/server/src/sdk/app/rows/search/utils.ts index 70f4362238..5ffc065353 100644 --- a/packages/server/src/sdk/app/rows/search/utils.ts +++ b/packages/server/src/sdk/app/rows/search/utils.ts @@ -115,7 +115,7 @@ export function isSearchingByRowID(query: SearchFilters): boolean { continue } const hasId = Object.keys(searchField).find( - key => key.endsWith("_id") && searchField[key] + key => dbCore.removeKeyNumbering(key) === "_id" && searchField[key] ) if (hasId) { return true