From 217d10f5fb81f7e247ed13ba381bef559eebcfd6 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 14 Sep 2023 14:00:49 +0100 Subject: [PATCH 1/4] Fix for more than/less than ranges, zeros were ignored when building up ranges, so that it simply acted like an upper limit, rather than a range. --- packages/server/src/api/controllers/row/utils.ts | 4 ++++ packages/server/src/integrations/base/sql.ts | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils.ts b/packages/server/src/api/controllers/row/utils.ts index cc27f4c2a3..92db29d303 100644 --- a/packages/server/src/api/controllers/row/utils.ts +++ b/packages/server/src/api/controllers/row/utils.ts @@ -147,6 +147,10 @@ export async function validate({ return { valid: Object.keys(errors).length === 0, errors } } +export function isValidFilter(value: any) { + return value != null && value !== "" +} + // don't do a pure falsy check, as 0 is included // https://github.com/Budibase/budibase/issues/10118 export function removeEmptyFilters(filters: SearchFilters) { diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index bf19ec9afe..3cdded69b4 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -11,6 +11,7 @@ import { QueryOptions } from "../../definitions/datasource" import { isIsoDateString, SqlClient } from "../utils" import SqlTableQueryBuilder from "./sqlTable" import environment from "../../environment" +import { isValidFilter } from "../../api/controllers/row/utils" const envLimit = environment.SQL_MAX_ROWS ? parseInt(environment.SQL_MAX_ROWS) @@ -261,15 +262,15 @@ class InternalBuilder { if (isEmptyObject(value.high)) { value.high = "" } - if (value.low && value.high) { + if (isValidFilter(value.low) && isValidFilter(value.high)) { // Use a between operator if we have 2 valid range values const fnc = allOr ? "orWhereBetween" : "whereBetween" query = query[fnc](key, [value.low, value.high]) - } else if (value.low) { + } else if (isValidFilter(value.low)) { // Use just a single greater than operator if we only have a low const fnc = allOr ? "orWhere" : "where" query = query[fnc](key, ">", value.low) - } else if (value.high) { + } else if (isValidFilter(value.high)) { // Use just a single less than operator if we only have a high const fnc = allOr ? "orWhere" : "where" query = query[fnc](key, "<", value.high) From c466f35a98a3e92ca35afd2d9037147d63dac58c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 13 Sep 2023 15:23:30 +0100 Subject: [PATCH 2/4] Found some discussion of testcontainers being problematic when nearly out of disk space, we have seen issues with the default Github runners as they have extremely limited disk space, this should help a bit removing android and dotnet, two pieces of functionality we will never need. --- .github/workflows/budibase_ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index d670e222d3..fc35575ec6 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -25,6 +25,13 @@ jobs: lint: runs-on: ubuntu-latest steps: + - name: Maximize build space + uses: easimon/maximize-build-space@master + with: + root-reserve-mb: 35000 + swap-size-mb: 1024 + remove-android: 'true' + remove-dotnet: 'true' - name: Checkout repo and submodules uses: actions/checkout@v3 if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'Budibase/budibase' From 8ca3f13a1cea7171d4e6a51b547bebdf28b65d19 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 14 Sep 2023 16:53:36 +0100 Subject: [PATCH 3/4] Quick re-jig based on test failure, seems the base sql.ts is depended on fairly heavily, importing the SDK can create a lot of cycles. --- .../src/api/controllers/row/external.ts | 3 +- .../server/src/api/controllers/row/utils.ts | 33 --------------- packages/server/src/integrations/base/sql.ts | 18 ++++---- packages/server/src/integrations/utils.ts | 42 ++++++++++++++++++- packages/server/src/sdk/app/rows/search.ts | 1 + 5 files changed, 52 insertions(+), 45 deletions(-) diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index a04584e6bd..6cc6337e0d 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -18,7 +18,6 @@ import { import sdk from "../../../sdk" import * as utils from "./utils" import { dataFilters } from "@budibase/shared-core" -import { removeEmptyFilters } from "./utils" export async function handleRequest( operation: Operation, @@ -27,7 +26,7 @@ export async function handleRequest( ) { // make sure the filters are cleaned up, no empty strings for equals, fuzzy or string if (opts && opts.filters) { - opts.filters = utils.removeEmptyFilters(opts.filters) + opts.filters = sdk.rows.removeEmptyFilters(opts.filters) } if ( !dataFilters.hasFilters(opts?.filters) && diff --git a/packages/server/src/api/controllers/row/utils.ts b/packages/server/src/api/controllers/row/utils.ts index 92db29d303..82e7c7b0d8 100644 --- a/packages/server/src/api/controllers/row/utils.ts +++ b/packages/server/src/api/controllers/row/utils.ts @@ -146,36 +146,3 @@ export async function validate({ } return { valid: Object.keys(errors).length === 0, errors } } - -export function isValidFilter(value: any) { - return value != null && value !== "" -} - -// don't do a pure falsy check, as 0 is included -// https://github.com/Budibase/budibase/issues/10118 -export function removeEmptyFilters(filters: SearchFilters) { - for (let filterField of NoEmptyFilterStrings) { - if (!filters[filterField]) { - continue - } - - for (let filterType of Object.keys(filters)) { - if (filterType !== filterField) { - continue - } - // don't know which one we're checking, type could be anything - const value = filters[filterType] as unknown - if (typeof value === "object") { - for (let [key, value] of Object.entries( - filters[filterType] as object - )) { - if (value == null || value === "") { - // @ts-ignore - delete filters[filterField][key] - } - } - } - } - } - return filters -} diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index 3cdded69b4..add7596165 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -1,4 +1,8 @@ import { Knex, knex } from "knex" +import { db as dbCore } from "@budibase/backend-core" +import { QueryOptions } from "../../definitions/datasource" +import { isIsoDateString, SqlClient } from "../utils" +import SqlTableQueryBuilder from "./sqlTable" import { Operation, QueryJson, @@ -6,12 +10,8 @@ import { SearchFilters, SortDirection, } from "@budibase/types" -import { db as dbCore } from "@budibase/backend-core" -import { QueryOptions } from "../../definitions/datasource" -import { isIsoDateString, SqlClient } from "../utils" -import SqlTableQueryBuilder from "./sqlTable" import environment from "../../environment" -import { isValidFilter } from "../../api/controllers/row/utils" +import { isValidFilter } from "../utils" const envLimit = environment.SQL_MAX_ROWS ? parseInt(environment.SQL_MAX_ROWS) @@ -262,15 +262,17 @@ class InternalBuilder { if (isEmptyObject(value.high)) { value.high = "" } - if (isValidFilter(value.low) && isValidFilter(value.high)) { + const lowValid = isValidFilter(value.low), + highValid = isValidFilter(value.high) + if (lowValid && highValid) { // Use a between operator if we have 2 valid range values const fnc = allOr ? "orWhereBetween" : "whereBetween" query = query[fnc](key, [value.low, value.high]) - } else if (isValidFilter(value.low)) { + } else if (lowValid) { // Use just a single greater than operator if we only have a low const fnc = allOr ? "orWhere" : "where" query = query[fnc](key, ">", value.low) - } else if (isValidFilter(value.high)) { + } else if (highValid) { // Use just a single less than operator if we only have a high const fnc = allOr ? "orWhere" : "where" query = query[fnc](key, "<", value.high) diff --git a/packages/server/src/integrations/utils.ts b/packages/server/src/integrations/utils.ts index 2883e4471c..75f4bcbfa1 100644 --- a/packages/server/src/integrations/utils.ts +++ b/packages/server/src/integrations/utils.ts @@ -1,6 +1,11 @@ -import { SourceName, SqlQuery, Datasource, Table } from "@budibase/types" +import { SqlQuery, Table, SearchFilters } from "@budibase/types" import { DocumentType, SEPARATOR } from "../db/utils" -import { FieldTypes, BuildSchemaErrors, InvalidColumns } from "../constants" +import { + FieldTypes, + BuildSchemaErrors, + InvalidColumns, + NoEmptyFilterStrings, +} from "../constants" import { helpers } from "@budibase/shared-core" const DOUBLE_SEPARATOR = `${SEPARATOR}${SEPARATOR}` @@ -343,3 +348,36 @@ export function getPrimaryDisplay(testValue: unknown): string | undefined { } return testValue as string } + +export function isValidFilter(value: any) { + return value != null && value !== "" +} + +// don't do a pure falsy check, as 0 is included +// https://github.com/Budibase/budibase/issues/10118 +export function removeEmptyFilters(filters: SearchFilters) { + for (let filterField of NoEmptyFilterStrings) { + if (!filters[filterField]) { + continue + } + + for (let filterType of Object.keys(filters)) { + if (filterType !== filterField) { + continue + } + // don't know which one we're checking, type could be anything + const value = filters[filterType] as unknown + if (typeof value === "object") { + for (let [key, value] of Object.entries( + filters[filterType] as object + )) { + if (value == null || value === "") { + // @ts-ignore + delete filters[filterField][key] + } + } + } + } + } + return filters +} diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 4861f473ea..1d06108b67 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -3,6 +3,7 @@ import { isExternalTable } from "../../../integrations/utils" import * as internal from "./search/internal" import * as external from "./search/external" import { Format } from "../../../api/controllers/view/exporters" +export { isValidFilter, removeEmptyFilters } from "../../../integrations/utils" export interface ViewParams { calculation: string From 68f31975223a4c1a187f0281161baf4b49491980 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 14 Sep 2023 17:12:09 +0100 Subject: [PATCH 4/4] Moving test to where the functions are now. --- .../tests/utils.spec.ts => sdk/tests/rows/search.spec.ts} | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename packages/server/src/{api/controllers/row/tests/utils.spec.ts => sdk/tests/rows/search.spec.ts} (71%) diff --git a/packages/server/src/api/controllers/row/tests/utils.spec.ts b/packages/server/src/sdk/tests/rows/search.spec.ts similarity index 71% rename from packages/server/src/api/controllers/row/tests/utils.spec.ts rename to packages/server/src/sdk/tests/rows/search.spec.ts index e0ad637e9d..feae5e7ee8 100644 --- a/packages/server/src/api/controllers/row/tests/utils.spec.ts +++ b/packages/server/src/sdk/tests/rows/search.spec.ts @@ -1,8 +1,8 @@ -import * as utils from "../utils" +import * as search from "../../app/rows/search" describe("removeEmptyFilters", () => { it("0 should not be removed", () => { - const filters = utils.removeEmptyFilters({ + const filters = search.removeEmptyFilters({ equal: { column: 0, }, @@ -11,7 +11,7 @@ describe("removeEmptyFilters", () => { }) it("empty string should be removed", () => { - const filters = utils.removeEmptyFilters({ + const filters = search.removeEmptyFilters({ equal: { column: "", },