From 99e8ef58dd9c9fa32b6f31228f0ddb513777577c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 1 Aug 2024 13:03:58 +0100 Subject: [PATCH] Adding test case - had to rejig how internal limit is retrieved but works without requiring thousands of rows. --- packages/backend-core/src/sql/sql.ts | 14 +-- .../scripts/integrations/postgres/init.sql | 2 +- .../src/api/routes/tests/search.spec.ts | 90 ++++++++++++++++++- 3 files changed, 96 insertions(+), 10 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index a67da7bc10..76c86b2b62 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -37,10 +37,12 @@ import { helpers } from "@budibase/shared-core" type QueryFunction = (query: SqlQuery | SqlQuery[], operation: Operation) => any -const envLimit = environment.SQL_MAX_ROWS - ? parseInt(environment.SQL_MAX_ROWS) - : null -const BASE_LIMIT = envLimit || 5000 +function getBaseLimit() { + const envLimit = environment.SQL_MAX_ROWS + ? parseInt(environment.SQL_MAX_ROWS) + : null + return envLimit || 5000 +} // Takes a string like foo and returns a quoted string like [foo] for SQL Server // and "foo" for Postgres. @@ -838,7 +840,7 @@ class SqlQueryBuilder extends SqlTableQueryBuilder { private readonly limit: number // pass through client to get flavour of SQL - constructor(client: SqlClient, limit: number = BASE_LIMIT) { + constructor(client: SqlClient, limit: number = getBaseLimit()) { super(client) this.limit = limit } @@ -882,7 +884,7 @@ class SqlQueryBuilder extends SqlTableQueryBuilder { query = builder.read(client, json, { limits: { query: this.limit, - base: BASE_LIMIT, + base: getBaseLimit(), }, }) break diff --git a/packages/server/scripts/integrations/postgres/init.sql b/packages/server/scripts/integrations/postgres/init.sql index f5b8086e32..9624208deb 100644 --- a/packages/server/scripts/integrations/postgres/init.sql +++ b/packages/server/scripts/integrations/postgres/init.sql @@ -89,4 +89,4 @@ INSERT INTO Products_Tasks (ProductID, TaskID) VALUES (1, 2); INSERT INTO "test-1".table1 (Name) VALUES ('Test'); INSERT INTO CompositeTable (KeyPartOne, KeyPartTwo, Name) VALUES ('aaa', 'bbb', 'Michael'); INSERT INTO CompositeTable (KeyPartOne, KeyPartTwo, Name) VALUES ('bbb', 'ccc', 'Andrew'); -INSERT INTO CompositeTable (KeyPartOne, KeyPartTwo, Name) VALUES ('ddd', '', 'OneKey'); \ No newline at end of file +INSERT INTO CompositeTable (KeyPartOne, KeyPartTwo, Name) VALUES ('ddd', '', 'OneKey'); diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 2c5756efe4..bc3cdccf18 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -53,6 +53,7 @@ describe.each([ const isLucene = name === "lucene" const isInMemory = name === "in-memory" const isInternal = isSqs || isLucene || isInMemory + const isSql = !isInMemory && !isLucene const config = setup.getConfig() let envCleanup: (() => void) | undefined @@ -192,7 +193,8 @@ describe.each([ // different to the one passed in will cause the assertion to fail. Extra // rows returned by the query will also cause the assertion to fail. async toMatchExactly(expectedRows: any[]) { - const { rows: foundRows } = await this.performSearch() + const response = await this.performSearch() + const foundRows = response.rows // eslint-disable-next-line jest/no-standalone-expect expect(foundRows).toHaveLength(expectedRows.length) @@ -202,13 +204,15 @@ describe.each([ expect.objectContaining(this.popRow(expectedRow, foundRows)) ) ) + return response } // Asserts that the query returns rows matching exactly the set of rows // passed in. The order of the rows is not important, but extra rows will // cause the assertion to fail. async toContainExactly(expectedRows: any[]) { - const { rows: foundRows } = await this.performSearch() + const response = await this.performSearch() + const foundRows = response.rows // eslint-disable-next-line jest/no-standalone-expect expect(foundRows).toHaveLength(expectedRows.length) @@ -220,6 +224,7 @@ describe.each([ ) ) ) + return response } // Asserts that the query returns some property values - this cannot be used @@ -236,6 +241,7 @@ describe.each([ expect(response[key]).toEqual(properties[key]) } } + return response } // Asserts that the query doesn't return a property, e.g. pagination parameters. @@ -245,13 +251,15 @@ describe.each([ // eslint-disable-next-line jest/no-standalone-expect expect(response[property]).toBeUndefined() } + return response } // Asserts that the query returns rows matching the set of rows passed in. // The order of the rows is not important. Extra rows will not cause the // assertion to fail. async toContain(expectedRows: any[]) { - const { rows: foundRows } = await this.performSearch() + const response = await this.performSearch() + const foundRows = response.rows // eslint-disable-next-line jest/no-standalone-expect expect([...foundRows]).toEqual( @@ -261,6 +269,7 @@ describe.each([ ) ) ) + return response } async toFindNothing() { @@ -2608,4 +2617,79 @@ describe.each([ }).toContainExactly([row]) }) }) + + isSql && + describe("pagination edge case with relationships", () => { + let mainRows: Row[] = [] + + beforeAll(async () => { + const toRelateTable = await createTable({ + name: { + name: "name", + type: FieldType.STRING, + }, + }) + table = await createTable({ + name: { + name: "name", + type: FieldType.STRING, + }, + rel: { + name: "rel", + type: FieldType.LINK, + relationshipType: RelationshipType.MANY_TO_ONE, + tableId: toRelateTable._id!, + fieldName: "rel", + }, + }) + const relatedRows = await Promise.all([ + config.api.row.save(toRelateTable._id!, { name: "tag 1" }), + config.api.row.save(toRelateTable._id!, { name: "tag 2" }), + config.api.row.save(toRelateTable._id!, { name: "tag 3" }), + config.api.row.save(toRelateTable._id!, { name: "tag 4" }), + config.api.row.save(toRelateTable._id!, { name: "tag 5" }), + config.api.row.save(toRelateTable._id!, { name: "tag 6" }), + ]) + mainRows = await Promise.all([ + config.api.row.save(table._id!, { + name: "product 1", + rel: relatedRows.map(row => row._id), + }), + config.api.row.save(table._id!, { + name: "product 2", + rel: [], + }), + config.api.row.save(table._id!, { + name: "product 3", + rel: [], + }), + ]) + }) + + it("can still page when the hard limit is hit", async () => { + await config.withCoreEnv( + { + SQL_MAX_ROWS: "6", + }, + async () => { + const params: Omit = { + query: {}, + paginate: true, + limit: 3, + sort: "name", + sortType: SortType.STRING, + sortOrder: SortOrder.ASCENDING, + } + const page1 = await expectSearch(params).toContain([mainRows[0]]) + expect(page1.hasNextPage).toBe(true) + expect(page1.bookmark).toBeDefined() + const page2 = await expectSearch({ + ...params, + bookmark: page1.bookmark, + }).toContain([mainRows[1], mainRows[2]]) + expect(page2.hasNextPage).toBe(false) + } + ) + }) + }) })