From c32163a9be9dae2b2f050dae88cccf8a3e479446 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 21 Mar 2024 18:26:35 +0000 Subject: [PATCH 01/15] Initial fix for defaulting parameters, switch to null rather than strings, this is important for prepared statements/SQL queries. --- .../server/src/api/controllers/query/index.ts | 55 +++++++++++-------- packages/server/src/threads/definitions.ts | 13 ++--- packages/types/src/documents/app/query.ts | 2 + 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index 0dba20dacd..b05b4b1222 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -6,7 +6,7 @@ import { invalidateDynamicVariables } from "../../../threads/utils" import env from "../../../environment" import { events, context, utils, constants } from "@budibase/backend-core" import sdk from "../../../sdk" -import { QueryEvent } from "../../../threads/definitions" +import { QueryEvent, QueryEventParameters } from "../../../threads/definitions" import { ConfigType, Query, @@ -18,7 +18,6 @@ import { FieldType, ExecuteQueryRequest, ExecuteQueryResponse, - QueryParameter, PreviewQueryRequest, PreviewQueryResponse, } from "@budibase/types" @@ -29,7 +28,7 @@ const Runner = new Thread(ThreadType.QUERY, { timeoutMs: env.QUERY_THREAD_TIMEOUT, }) -function validateQueryInputs(parameters: Record) { +function validateQueryInputs(parameters: QueryEventParameters) { for (let entry of Object.entries(parameters)) { const [key, value] = entry if (typeof value !== "string") { @@ -100,8 +99,10 @@ export async function save(ctx: UserCtx) { const datasource = await sdk.datasources.get(query.datasourceId) let eventFn - if (!query._id) { + if (!query._id && !query._rev) { query._id = generateQueryID(query.datasourceId) + // flag to state whether the default bindings are empty strings (old behaviour) or null + query.nullDefaultSupport = true eventFn = () => events.query.created(datasource, query) } else { eventFn = () => events.query.updated(datasource, query) @@ -135,16 +136,22 @@ function getAuthConfig(ctx: UserCtx) { } function enrichParameters( - queryParameters: QueryParameter[], - requestParameters: Record = {} -): Record { + query: Query, + requestParameters: QueryEventParameters = {} +): QueryEventParameters { + const paramNotSet = (val: unknown) => val === "" || val == undefined // first check parameters are all valid validateQueryInputs(requestParameters) // make sure parameters are fully enriched with defaults - for (let parameter of queryParameters) { - if (!requestParameters[parameter.name]) { - requestParameters[parameter.name] = parameter.default + for (let parameter of query.parameters) { + let value: string | null = requestParameters[parameter.name] + if (!value) { + value = parameter.default } + if (query.nullDefaultSupport && paramNotSet(value)) { + value = null + } + requestParameters[parameter.name] = value } return requestParameters } @@ -157,10 +164,15 @@ export async function preview( ) // preview may not have a queryId as it hasn't been saved, but if it does // this stops dynamic variables from calling the same query - const { fields, parameters, queryVerb, transformer, queryId, schema } = - ctx.request.body + const queryId = ctx.request.body.queryId + // the body contains the makings of a query, which has not been saved yet + const query: Query = ctx.request.body + // hasn't been saved, new query + if (!queryId && !query._id) { + query.nullDefaultSupport = true + } - let existingSchema = schema + let existingSchema = query.schema if (queryId && !existingSchema) { try { const db = context.getAppDB() @@ -268,13 +280,13 @@ export async function preview( try { const inputs: QueryEvent = { appId: ctx.appId, - datasource, - queryVerb, - fields, - parameters: enrichParameters(parameters), - transformer, + queryVerb: query.queryVerb, + fields: query.fields, + parameters: enrichParameters(query), + transformer: query.transformer, + schema: query.schema, queryId, - schema, + datasource, // have to pass down to the thread runner - can't put into context now environmentVariables: envVars, ctx: { @@ -336,10 +348,7 @@ async function execute( queryVerb: query.queryVerb, fields: query.fields, pagination: ctx.request.body.pagination, - parameters: enrichParameters( - query.parameters, - ctx.request.body.parameters - ), + parameters: enrichParameters(query, ctx.request.body.parameters), transformer: query.transformer, queryId: ctx.params.queryId, // have to pass down to the thread runner - can't put into context now diff --git a/packages/server/src/threads/definitions.ts b/packages/server/src/threads/definitions.ts index 14b97c57b1..85e546280d 100644 --- a/packages/server/src/threads/definitions.ts +++ b/packages/server/src/threads/definitions.ts @@ -1,21 +1,20 @@ -import { Datasource, QuerySchema, Row } from "@budibase/types" +import { Datasource, Row, Query } from "@budibase/types" export type WorkerCallback = (error: any, response?: any) => void -export interface QueryEvent { +export interface QueryEvent + extends Omit { appId?: string datasource: Datasource - queryVerb: string - fields: { [key: string]: any } - parameters: { [key: string]: unknown } pagination?: any - transformer: any queryId?: string environmentVariables?: Record + parameters: QueryEventParameters ctx?: any - schema?: Record } +export type QueryEventParameters = Record + export interface QueryResponse { rows: Row[] keys: string[] diff --git a/packages/types/src/documents/app/query.ts b/packages/types/src/documents/app/query.ts index 535c5dab3b..baba4def95 100644 --- a/packages/types/src/documents/app/query.ts +++ b/packages/types/src/documents/app/query.ts @@ -15,6 +15,8 @@ export interface Query extends Document { schema: Record readable: boolean queryVerb: string + // flag to state whether the default bindings are empty strings (old behaviour) or null + nullDefaultSupport?: boolean } export interface QueryPreview extends Omit { From f4f7ac42ec164af87044e745229c79134a5c4f6a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 22 Mar 2024 11:40:44 +0000 Subject: [PATCH 02/15] Adding test cases for interpolation of SQL, confirming that the context correctly gets cleaned up before passing into bindings. --- .../server/src/api/controllers/query/index.ts | 8 ++ .../routes/tests/queries/generic-sql.spec.ts | 85 +++++++++++++++++-- .../api/routes/tests/queries/mongodb.spec.ts | 2 +- .../server/src/integrations/queries/sql.ts | 15 +++- .../server/src/sdk/app/queries/queries.ts | 18 +++- .../server/src/tests/utilities/api/query.ts | 2 +- packages/server/src/threads/query.ts | 29 +++++-- 7 files changed, 137 insertions(+), 22 deletions(-) diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index b05b4b1222..03fcdb714a 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -105,6 +105,12 @@ export async function save(ctx: UserCtx) { query.nullDefaultSupport = true eventFn = () => events.query.created(datasource, query) } else { + // check if flag has previously been set, don't let it change + // allow it to be explicitly set to false via API incase this is ever needed + const existingQuery = await db.get(query._id) + if (existingQuery.nullDefaultSupport && query.nullDefaultSupport == null) { + query.nullDefaultSupport = true + } eventFn = () => events.query.updated(datasource, query) } const response = await db.put(query) @@ -285,6 +291,7 @@ export async function preview( parameters: enrichParameters(query), transformer: query.transformer, schema: query.schema, + nullDefaultSupport: query.nullDefaultSupport, queryId, datasource, // have to pass down to the thread runner - can't put into context now @@ -353,6 +360,7 @@ async function execute( queryId: ctx.params.queryId, // have to pass down to the thread runner - can't put into context now environmentVariables: envVars, + nullDefaultSupport: query.nullDefaultSupport, ctx: { user: ctx.user, auth: { ...authConfigCtx }, diff --git a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts index 1fc0ecb382..ac90efa71c 100644 --- a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts +++ b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts @@ -12,31 +12,51 @@ const createTableSQL: Record = { CREATE TABLE test_table ( id serial PRIMARY KEY, name VARCHAR ( 50 ) NOT NULL, - birthday TIMESTAMP + birthday TIMESTAMP, + number INT );`, [SourceName.MYSQL]: ` CREATE TABLE test_table ( id INT AUTO_INCREMENT PRIMARY KEY, name VARCHAR(50) NOT NULL, - birthday TIMESTAMP + birthday TIMESTAMP, + number INT );`, [SourceName.SQL_SERVER]: ` CREATE TABLE test_table ( id INT IDENTITY(1,1) PRIMARY KEY, name NVARCHAR(50) NOT NULL, - birthday DATETIME + birthday DATETIME, + number INT );`, } const insertSQL = `INSERT INTO test_table (name) VALUES ('one'), ('two'), ('three'), ('four'), ('five')` const dropTableSQL = `DROP TABLE test_table;` +const POSTGRES_SPECIFICS = { + nullError: 'invalid input syntax for type integer: ""', +} + +const MYSQL_SPECIFICS = { + nullError: "Incorrect integer value: '' for column 'number' at row 1", +} + +const MSSQL_SPECIFICS = { + nullError: "Cannot convert undefined or null to object", +} + +const MARIADB_SPECIFICS = { + nullError: + "Incorrect integer value: '' for column `mysql`.`test_table`.`number` at row 1", +} + describe.each([ - ["postgres", databaseTestProviders.postgres], - ["mysql", databaseTestProviders.mysql], - ["mssql", databaseTestProviders.mssql], - ["mariadb", databaseTestProviders.mariadb], -])("queries (%s)", (__, dsProvider) => { + ["postgres", databaseTestProviders.postgres, POSTGRES_SPECIFICS], + ["mysql", databaseTestProviders.mysql, MYSQL_SPECIFICS], + ["mssql", databaseTestProviders.mssql, MSSQL_SPECIFICS], + ["mariadb", databaseTestProviders.mariadb, MARIADB_SPECIFICS], +])("queries (%s)", (__, dsProvider, testSpecificInformation) => { const config = setup.getConfig() let datasource: Datasource @@ -51,7 +71,7 @@ describe.each([ transformer: "return data", readable: true, } - return await config.api.query.create({ ...defaultQuery, ...query }) + return await config.api.query.save({ ...defaultQuery, ...query }) } async function rawQuery(sql: string): Promise { @@ -398,4 +418,51 @@ describe.each([ expect(rows).toHaveLength(0) }) }) + + // this parameter really only impacts SQL queries + describe("confirm nullDefaultSupport", () => { + const queryParams = { + fields: { + sql: "INSERT INTO test_table (name, number) VALUES ({{ bindingName }}, {{ bindingNumber }})", + }, + parameters: [ + { + name: "bindingName", + default: "", + }, + { + name: "bindingNumber", + default: "", + }, + ], + queryVerb: "create", + } + + it("should error for old queries", async () => { + const query = await createQuery(queryParams) + await config.api.query.save({ ...query, nullDefaultSupport: false }) + let error: string | undefined + try { + await config.api.query.execute(query._id!, { + parameters: { + bindingName: "testing", + }, + }) + } catch (err: any) { + error = err.message + } + expect(error).toBeDefined() + expect(error).toContain(testSpecificInformation.nullError) + }) + + it("should not error for new queries", async () => { + const query = await createQuery(queryParams) + const results = await config.api.query.execute(query._id!, { + parameters: { + bindingName: "testing", + }, + }) + expect(results).toEqual({ data: [{ created: true }] }) + }) + }) }) diff --git a/packages/server/src/api/routes/tests/queries/mongodb.spec.ts b/packages/server/src/api/routes/tests/queries/mongodb.spec.ts index b61f905bef..3280c37e78 100644 --- a/packages/server/src/api/routes/tests/queries/mongodb.spec.ts +++ b/packages/server/src/api/routes/tests/queries/mongodb.spec.ts @@ -33,7 +33,7 @@ describe("/queries", () => { ) { combinedQuery.fields.extra.collection = collection } - return await config.api.query.create(combinedQuery) + return await config.api.query.save(combinedQuery) } async function withClient( diff --git a/packages/server/src/integrations/queries/sql.ts b/packages/server/src/integrations/queries/sql.ts index 6d42117d7d..fd1363c39a 100644 --- a/packages/server/src/integrations/queries/sql.ts +++ b/packages/server/src/integrations/queries/sql.ts @@ -1,13 +1,15 @@ import { findHBSBlocks } from "@budibase/string-templates" import { DatasourcePlus } from "@budibase/types" import sdk from "../../sdk" +import { enrichArrayContext } from "../../sdk/app/queries/queries" const CONST_CHAR_REGEX = new RegExp("'[^']*'", "g") export async function interpolateSQL( - fields: { [key: string]: any }, + fields: { sql: string; bindings: any[] }, parameters: { [key: string]: any }, - integration: DatasourcePlus + integration: DatasourcePlus, + opts: { nullDefaultSupport: boolean } ) { let sql = fields.sql if (!sql || typeof sql !== "string") { @@ -64,7 +66,14 @@ export async function interpolateSQL( } // replicate the knex structure fields.sql = sql - fields.bindings = await sdk.queries.enrichContext(variables, parameters) + fields.bindings = await sdk.queries.enrichArrayContext(variables, parameters) + if (opts.nullDefaultSupport) { + for (let index in fields.bindings) { + if (fields.bindings[index] === "") { + fields.bindings[index] = null + } + } + } // check for arrays in the data let updated: string[] = [] for (let i = 0; i < variables.length; i++) { diff --git a/packages/server/src/sdk/app/queries/queries.ts b/packages/server/src/sdk/app/queries/queries.ts index 3f967b7198..cf0ed4ec95 100644 --- a/packages/server/src/sdk/app/queries/queries.ts +++ b/packages/server/src/sdk/app/queries/queries.ts @@ -65,11 +65,27 @@ export async function fetch(opts: { enrich: boolean } = { enrich: true }) { return updateSchemas(queries) } +export async function enrichArrayContext( + fields: any[], + inputs = {} +): Promise { + const map: Record = {} + for (let [key, value] of Object.entries(fields)) { + map[key] = value + } + const output = await enrichContext(map, inputs) + const outputArray: any[] = [] + for (let [key, value] of Object.entries(output)) { + outputArray[parseInt(key)] = value + } + return outputArray +} + export async function enrichContext( fields: Record, inputs = {} ): Promise> { - const enrichedQuery: Record = Array.isArray(fields) ? [] : {} + const enrichedQuery: Record = {} if (!fields || !inputs) { return enrichedQuery } diff --git a/packages/server/src/tests/utilities/api/query.ts b/packages/server/src/tests/utilities/api/query.ts index 32866314ff..0dad6d2ad9 100644 --- a/packages/server/src/tests/utilities/api/query.ts +++ b/packages/server/src/tests/utilities/api/query.ts @@ -8,7 +8,7 @@ import { import { TestAPI } from "./base" export class QueryAPI extends TestAPI { - create = async (body: Query): Promise => { + save = async (body: Query): Promise => { return await this._post(`/api/queries`, { body }) } diff --git a/packages/server/src/threads/query.ts b/packages/server/src/threads/query.ts index 9f5e02bf69..dbac76c09a 100644 --- a/packages/server/src/threads/query.ts +++ b/packages/server/src/threads/query.ts @@ -26,10 +26,11 @@ class QueryRunner { fields: any parameters: any pagination: any - transformer: string + transformer: string | null cachedVariables: any[] ctx: any queryResponse: any + nullDefaultSupport: boolean noRecursiveQuery: boolean hasRerun: boolean hasRefreshedOAuth: boolean @@ -45,6 +46,7 @@ class QueryRunner { this.transformer = input.transformer this.queryId = input.queryId! this.schema = input.schema + this.nullDefaultSupport = !!input.nullDefaultSupport this.noRecursiveQuery = flags.noRecursiveQuery this.cachedVariables = [] // Additional context items for enrichment @@ -59,7 +61,14 @@ class QueryRunner { } async execute(): Promise { - let { datasource, fields, queryVerb, transformer, schema } = this + let { + datasource, + fields, + queryVerb, + transformer, + schema, + nullDefaultSupport, + } = this let datasourceClone = cloneDeep(datasource) let fieldsClone = cloneDeep(fields) @@ -103,7 +112,9 @@ class QueryRunner { let query // handle SQL injections by interpolating the variables if (isSQL(datasourceClone)) { - query = await interpolateSQL(fieldsClone, enrichedContext, integration) + query = await interpolateSQL(fieldsClone, enrichedContext, integration, { + nullDefaultSupport, + }) } else { query = await sdk.queries.enrichContext(fieldsClone, enrichedContext) } @@ -137,7 +148,9 @@ class QueryRunner { data: rows, params: enrichedParameters, } - rows = vm.withContext(ctx, () => vm.execute(transformer)) + if (transformer != null) { + rows = vm.withContext(ctx, () => vm.execute(transformer!)) + } } // if the request fails we retry once, invalidating the cached value @@ -191,13 +204,15 @@ class QueryRunner { }) return new QueryRunner( { - datasource, + schema: query.schema, queryVerb: query.queryVerb, fields: query.fields, - parameters, transformer: query.transformer, - queryId, + nullDefaultSupport: query.nullDefaultSupport, ctx: this.ctx, + parameters, + datasource, + queryId, }, { noRecursiveQuery: true } ).execute() From 3f225c94e745d60669bd22a5c31209d23a3af2fc Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 22 Mar 2024 11:50:07 +0000 Subject: [PATCH 03/15] Linting. --- packages/server/src/integrations/queries/sql.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/integrations/queries/sql.ts b/packages/server/src/integrations/queries/sql.ts index fd1363c39a..f6b0d68e7f 100644 --- a/packages/server/src/integrations/queries/sql.ts +++ b/packages/server/src/integrations/queries/sql.ts @@ -1,7 +1,6 @@ import { findHBSBlocks } from "@budibase/string-templates" import { DatasourcePlus } from "@budibase/types" import sdk from "../../sdk" -import { enrichArrayContext } from "../../sdk/app/queries/queries" const CONST_CHAR_REGEX = new RegExp("'[^']*'", "g") From 5b1db961291efe91eb233fae2fbe2a434ca8ee86 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 22 Mar 2024 12:05:55 +0000 Subject: [PATCH 04/15] Fixing typing issue. --- packages/server/src/threads/query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/threads/query.ts b/packages/server/src/threads/query.ts index dbac76c09a..97e7a05cf7 100644 --- a/packages/server/src/threads/query.ts +++ b/packages/server/src/threads/query.ts @@ -109,7 +109,7 @@ class QueryRunner { ) } - let query + let query: Record // handle SQL injections by interpolating the variables if (isSQL(datasourceClone)) { query = await interpolateSQL(fieldsClone, enrichedContext, integration, { From 2464020a225e70d8d64e4bd99f613f33162422f2 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 22 Mar 2024 12:36:00 +0000 Subject: [PATCH 05/15] Test fixes after running in CI. --- .../routes/tests/queries/generic-sql.spec.ts | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts index ac90efa71c..97e513af01 100644 --- a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts +++ b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts @@ -43,7 +43,8 @@ const MYSQL_SPECIFICS = { } const MSSQL_SPECIFICS = { - nullError: "Cannot convert undefined or null to object", + // MS-SQL doesn't throw an error here, it casts empty string to nothing + nullError: undefined, } const MARIADB_SPECIFICS = { @@ -241,26 +242,31 @@ describe.each([ id: 1, name: "one", birthday: null, + number: null, }, { id: 2, name: "two", birthday: null, + number: null, }, { id: 3, name: "three", birthday: null, + number: null, }, { id: 4, name: "four", birthday: null, + number: null, }, { id: 5, name: "five", birthday: null, + number: null, }, ]) }) @@ -283,6 +289,7 @@ describe.each([ id: 2, name: "one", birthday: null, + number: null, }, ]) }) @@ -311,6 +318,7 @@ describe.each([ id: 1, name: "one", birthday: null, + number: null, }, ]) }) @@ -349,7 +357,9 @@ describe.each([ ]) const rows = await rawQuery("SELECT * FROM test_table WHERE id = 1") - expect(rows).toEqual([{ id: 1, name: "foo", birthday: null }]) + expect(rows).toEqual([ + { id: 1, name: "foo", birthday: null, number: null }, + ]) }) it("should be able to execute an update that updates no rows", async () => { @@ -451,8 +461,13 @@ describe.each([ } catch (err: any) { error = err.message } - expect(error).toBeDefined() - expect(error).toContain(testSpecificInformation.nullError) + const testExpectation = testSpecificInformation.nullError + if (testExpectation) { + expect(error).toBeDefined() + expect(error).toContain(testExpectation) + } else { + expect(error).toBeUndefined() + } }) it("should not error for new queries", async () => { From 185e9c3425fe758a0d815a87b58adc2e1e22fd71 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 22 Mar 2024 13:01:51 +0000 Subject: [PATCH 06/15] Fixing test case. --- .../server/src/api/routes/tests/environmentVariables.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/environmentVariables.spec.ts b/packages/server/src/api/routes/tests/environmentVariables.spec.ts index 9104dedf4f..beb6012c9c 100644 --- a/packages/server/src/api/routes/tests/environmentVariables.spec.ts +++ b/packages/server/src/api/routes/tests/environmentVariables.spec.ts @@ -143,7 +143,10 @@ describe("/api/env/variables", () => { delete response.body.datasource.config expect(events.query.previewed).toHaveBeenCalledWith( response.body.datasource, - queryPreview + { + ...queryPreview, + nullDefaultSupport: true, + } ) expect(pg.Client).toHaveBeenCalledWith({ password: "test", ssl: undefined }) }) From defb925d16d1e182fba48e618ce170d7230ae8e0 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 22 Mar 2024 13:38:52 +0000 Subject: [PATCH 07/15] Query seq fix. --- .../src/api/routes/tests/queries/query.seq.spec.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/queries/query.seq.spec.ts b/packages/server/src/api/routes/tests/queries/query.seq.spec.ts index 10b90eafb1..4c25a762b8 100644 --- a/packages/server/src/api/routes/tests/queries/query.seq.spec.ts +++ b/packages/server/src/api/routes/tests/queries/query.seq.spec.ts @@ -78,6 +78,7 @@ describe("/queries", () => { _rev: res.body._rev, _id: res.body._id, ...query, + nullDefaultSupport: true, createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), }) @@ -103,6 +104,7 @@ describe("/queries", () => { _rev: res.body._rev, _id: res.body._id, ...query, + nullDefaultSupport: true, createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), }) @@ -130,6 +132,7 @@ describe("/queries", () => { _id: query._id, createdAt: new Date().toISOString(), ...basicQuery(datasource._id), + nullDefaultSupport: true, updatedAt: new Date().toISOString(), readable: true, }, @@ -245,10 +248,10 @@ describe("/queries", () => { expect(responseBody.rows.length).toEqual(1) expect(events.query.previewed).toHaveBeenCalledTimes(1) delete datasource.config - expect(events.query.previewed).toHaveBeenCalledWith( - datasource, - queryPreview - ) + expect(events.query.previewed).toHaveBeenCalledWith(datasource, { + ...queryPreview, + nullDefaultSupport: true, + }) }) it("should apply authorization to endpoint", async () => { From cc8a0274a4e1801f008e740f694cf3c96e1e8888 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 22 Mar 2024 15:26:02 +0000 Subject: [PATCH 08/15] Updating based on PR comments. --- .../server/src/api/controllers/query/index.ts | 8 ++-- .../routes/tests/queries/generic-sql.spec.ts | 37 +++++-------------- .../server/src/sdk/app/queries/queries.ts | 4 +- 3 files changed, 14 insertions(+), 35 deletions(-) diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index 03fcdb714a..055f3bd888 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -149,11 +149,9 @@ function enrichParameters( // first check parameters are all valid validateQueryInputs(requestParameters) // make sure parameters are fully enriched with defaults - for (let parameter of query.parameters) { - let value: string | null = requestParameters[parameter.name] - if (!value) { - value = parameter.default - } + for (const parameter of query.parameters) { + let value: string | null = + requestParameters[parameter.name] || parameter.default if (query.nullDefaultSupport && paramNotSet(value)) { value = null } diff --git a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts index 97e513af01..f9a3ac6e03 100644 --- a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts +++ b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts @@ -34,30 +34,12 @@ const createTableSQL: Record = { const insertSQL = `INSERT INTO test_table (name) VALUES ('one'), ('two'), ('three'), ('four'), ('five')` const dropTableSQL = `DROP TABLE test_table;` -const POSTGRES_SPECIFICS = { - nullError: 'invalid input syntax for type integer: ""', -} - -const MYSQL_SPECIFICS = { - nullError: "Incorrect integer value: '' for column 'number' at row 1", -} - -const MSSQL_SPECIFICS = { - // MS-SQL doesn't throw an error here, it casts empty string to nothing - nullError: undefined, -} - -const MARIADB_SPECIFICS = { - nullError: - "Incorrect integer value: '' for column `mysql`.`test_table`.`number` at row 1", -} - describe.each([ - ["postgres", databaseTestProviders.postgres, POSTGRES_SPECIFICS], - ["mysql", databaseTestProviders.mysql, MYSQL_SPECIFICS], - ["mssql", databaseTestProviders.mssql, MSSQL_SPECIFICS], - ["mariadb", databaseTestProviders.mariadb, MARIADB_SPECIFICS], -])("queries (%s)", (__, dsProvider, testSpecificInformation) => { + ["postgres", databaseTestProviders.postgres], + ["mysql", databaseTestProviders.mysql], + ["mssql", databaseTestProviders.mssql], + ["mariadb", databaseTestProviders.mariadb], +])("queries (%s)", (dbName, dsProvider) => { const config = setup.getConfig() let datasource: Datasource @@ -461,12 +443,11 @@ describe.each([ } catch (err: any) { error = err.message } - const testExpectation = testSpecificInformation.nullError - if (testExpectation) { - expect(error).toBeDefined() - expect(error).toContain(testExpectation) - } else { + if (dbName === "mssql") { expect(error).toBeUndefined() + } else { + expect(error).toBeDefined() + expect(error).toContain("integer") } }) diff --git a/packages/server/src/sdk/app/queries/queries.ts b/packages/server/src/sdk/app/queries/queries.ts index cf0ed4ec95..511d2233c2 100644 --- a/packages/server/src/sdk/app/queries/queries.ts +++ b/packages/server/src/sdk/app/queries/queries.ts @@ -70,8 +70,8 @@ export async function enrichArrayContext( inputs = {} ): Promise { const map: Record = {} - for (let [key, value] of Object.entries(fields)) { - map[key] = value + for (let index in fields) { + map[index] = fields[index] } const output = await enrichContext(map, inputs) const outputArray: any[] = [] From 79d1b22caeb6bf47974fb588f1bdf2d5de9d101a Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 25 Mar 2024 11:00:13 +0000 Subject: [PATCH 09/15] Pull images separately from tests, enable testcontainers debug logs. --- .github/workflows/budibase_ci.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index 5c474aa826..c2a4fcb176 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -138,6 +138,8 @@ jobs: test-server: runs-on: ubuntu-latest + env: + DEBUG: testcontainers,testcontainers:exec,testcontainers:build,testcontainers:pull steps: - name: Checkout repo uses: actions/checkout@v4 @@ -151,7 +153,17 @@ jobs: with: node-version: 20.x cache: yarn + + - name: Pull testcontainers images + run: | + docker pull mcr.microsoft.com/mssql/server:2022-latest + docker pull mysql:8.3 + docker pull postgres:16.1-bullseye + docker pull mongo:7.0-jammy + docker pull mariadb:lts + - run: yarn --frozen-lockfile + - name: Test server run: | if ${{ env.USE_NX_AFFECTED }}; then From 077ca2d12089a56efe5ff6963705c23de3a1b7cc Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 25 Mar 2024 11:16:37 +0000 Subject: [PATCH 10/15] Make a change in packages/server to trigger the tests. --- packages/server/src/integrations/tests/utils/mssql.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/src/integrations/tests/utils/mssql.ts b/packages/server/src/integrations/tests/utils/mssql.ts index 6bd4290a90..cff6809c0a 100644 --- a/packages/server/src/integrations/tests/utils/mssql.ts +++ b/packages/server/src/integrations/tests/utils/mssql.ts @@ -1,6 +1,8 @@ import { Datasource, SourceName } from "@budibase/types" import { GenericContainer, Wait, StartedTestContainer } from "testcontainers" +// TODO: remove this comment + let container: StartedTestContainer | undefined export async function start(): Promise { From 6824d8626af2619af6737a83cc2df2046238056b Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Mon, 25 Mar 2024 11:19:31 +0000 Subject: [PATCH 11/15] Fixing failing test. --- packages/server/src/api/routes/tests/queries/mongodb.spec.ts | 2 +- packages/server/src/sdk/app/queries/queries.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/queries/mongodb.spec.ts b/packages/server/src/api/routes/tests/queries/mongodb.spec.ts index bbbb9362fd..492f24abf9 100644 --- a/packages/server/src/api/routes/tests/queries/mongodb.spec.ts +++ b/packages/server/src/api/routes/tests/queries/mongodb.spec.ts @@ -464,7 +464,7 @@ describe("/queries", () => { }) }) - it("should ignore be able to save deeply nested data", async () => { + it("should be able to save deeply nested data", async () => { const data = { foo: "bar", data: [ diff --git a/packages/server/src/sdk/app/queries/queries.ts b/packages/server/src/sdk/app/queries/queries.ts index 511d2233c2..d37e53bec1 100644 --- a/packages/server/src/sdk/app/queries/queries.ts +++ b/packages/server/src/sdk/app/queries/queries.ts @@ -89,6 +89,9 @@ export async function enrichContext( if (!fields || !inputs) { return enrichedQuery } + if (Array.isArray(fields)) { + return enrichArrayContext(fields, inputs) + } const env = await getEnvironmentVariables() const parameters = { ...inputs, env } // enrich the fields with dynamic parameters From 44b448f3e13412953c11ddd8edc65948dcb47fa8 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 25 Mar 2024 11:52:01 +0000 Subject: [PATCH 12/15] Also pull couchdb --- .github/workflows/budibase_ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index c2a4fcb176..94a001ba96 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -161,6 +161,8 @@ jobs: docker pull postgres:16.1-bullseye docker pull mongo:7.0-jammy docker pull mariadb:lts + docker pull testcontainers/ryuk:0.3.0 + docker pull budibase/couchdb - run: yarn --frozen-lockfile From fc41d16d1d67a2e25c1dfbeb3246d559155c5808 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 25 Mar 2024 11:52:18 +0000 Subject: [PATCH 13/15] Remove comment added to trigger tests. --- packages/server/src/integrations/tests/utils/mssql.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/server/src/integrations/tests/utils/mssql.ts b/packages/server/src/integrations/tests/utils/mssql.ts index cff6809c0a..6bd4290a90 100644 --- a/packages/server/src/integrations/tests/utils/mssql.ts +++ b/packages/server/src/integrations/tests/utils/mssql.ts @@ -1,8 +1,6 @@ import { Datasource, SourceName } from "@budibase/types" import { GenericContainer, Wait, StartedTestContainer } from "testcontainers" -// TODO: remove this comment - let container: StartedTestContainer | undefined export async function start(): Promise { From 538f3b9dbe595bb246b51aca80bd83850e972e57 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 25 Mar 2024 13:51:27 +0000 Subject: [PATCH 14/15] Align our use of images across the codebase. --- packages/server/src/sdk/app/rows/search/tests/external.spec.ts | 2 +- .../src/integrations/external-schema/mysql.integration.spec.ts | 2 +- .../integrations/external-schema/postgres.integration.spec.ts | 2 +- qa-core/src/integrations/validators/mongo.integration.spec.ts | 2 +- qa-core/src/integrations/validators/mssql.integration.spec.ts | 2 +- qa-core/src/integrations/validators/mysql.integration.spec.ts | 2 +- .../src/integrations/validators/postgres.integration.spec.ts | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/tests/external.spec.ts b/packages/server/src/sdk/app/rows/search/tests/external.spec.ts index 8ecec784dd..bae58d6a2c 100644 --- a/packages/server/src/sdk/app/rows/search/tests/external.spec.ts +++ b/packages/server/src/sdk/app/rows/search/tests/external.spec.ts @@ -26,7 +26,7 @@ describe("external search", () => { const rows: Row[] = [] beforeAll(async () => { - const container = await new GenericContainer("mysql") + const container = await new GenericContainer("mysql:8.3") .withExposedPorts(3306) .withEnvironment({ MYSQL_ROOT_PASSWORD: "admin", diff --git a/qa-core/src/integrations/external-schema/mysql.integration.spec.ts b/qa-core/src/integrations/external-schema/mysql.integration.spec.ts index 5a7e1989d2..c8d285a021 100644 --- a/qa-core/src/integrations/external-schema/mysql.integration.spec.ts +++ b/qa-core/src/integrations/external-schema/mysql.integration.spec.ts @@ -6,7 +6,7 @@ describe("datasource validators", () => { let config: any beforeAll(async () => { - const container = await new GenericContainer("mysql") + const container = await new GenericContainer("mysql:8.3") .withExposedPorts(3306) .withEnv("MYSQL_ROOT_PASSWORD", "admin") .withEnv("MYSQL_DATABASE", "db") diff --git a/qa-core/src/integrations/external-schema/postgres.integration.spec.ts b/qa-core/src/integrations/external-schema/postgres.integration.spec.ts index a0812c9677..7581d7f88a 100644 --- a/qa-core/src/integrations/external-schema/postgres.integration.spec.ts +++ b/qa-core/src/integrations/external-schema/postgres.integration.spec.ts @@ -17,7 +17,7 @@ describe("getExternalSchema", () => { } beforeAll(async () => { - const container = await new GenericContainer("postgres:13.12") + const container = await new GenericContainer("postgres:16.1-bullseye") .withExposedPorts(5432) .withEnv("POSTGRES_PASSWORD", "password") .start() diff --git a/qa-core/src/integrations/validators/mongo.integration.spec.ts b/qa-core/src/integrations/validators/mongo.integration.spec.ts index 9132d79f74..b1bab3bd1f 100644 --- a/qa-core/src/integrations/validators/mongo.integration.spec.ts +++ b/qa-core/src/integrations/validators/mongo.integration.spec.ts @@ -26,7 +26,7 @@ describe("datasource validators", () => { beforeAll(async () => { const user = generator.name() const password = generator.hash() - const container = await new GenericContainer("mongo") + const container = await new GenericContainer("mongo:7.0-jammy") .withExposedPorts(27017) .withEnv("MONGO_INITDB_ROOT_USERNAME", user) .withEnv("MONGO_INITDB_ROOT_PASSWORD", password) diff --git a/qa-core/src/integrations/validators/mssql.integration.spec.ts b/qa-core/src/integrations/validators/mssql.integration.spec.ts index d8f36e8bd0..c07f1d1129 100644 --- a/qa-core/src/integrations/validators/mssql.integration.spec.ts +++ b/qa-core/src/integrations/validators/mssql.integration.spec.ts @@ -13,7 +13,7 @@ describe("datasource validators", () => { beforeAll(async () => { const container = await new GenericContainer( - "mcr.microsoft.com/mssql/server" + "mcr.microsoft.com/mssql/server:2022-latest" ) .withExposedPorts(1433) .withEnv("ACCEPT_EULA", "Y") diff --git a/qa-core/src/integrations/validators/mysql.integration.spec.ts b/qa-core/src/integrations/validators/mysql.integration.spec.ts index e828d192af..95f7d4abbd 100644 --- a/qa-core/src/integrations/validators/mysql.integration.spec.ts +++ b/qa-core/src/integrations/validators/mysql.integration.spec.ts @@ -7,7 +7,7 @@ describe("datasource validators", () => { let port: number beforeAll(async () => { - const container = await new GenericContainer("mysql") + const container = await new GenericContainer("mysql:8.3") .withExposedPorts(3306) .withEnv("MYSQL_ROOT_PASSWORD", "admin") .withEnv("MYSQL_DATABASE", "db") diff --git a/qa-core/src/integrations/validators/postgres.integration.spec.ts b/qa-core/src/integrations/validators/postgres.integration.spec.ts index 5101cf1d2d..9e3e1ab30f 100644 --- a/qa-core/src/integrations/validators/postgres.integration.spec.ts +++ b/qa-core/src/integrations/validators/postgres.integration.spec.ts @@ -9,7 +9,7 @@ describe("datasource validators", () => { let port: number beforeAll(async () => { - const container = await new GenericContainer("postgres") + const container = await new GenericContainer("postgres:16.1-bullseye") .withExposedPorts(5432) .withEnv("POSTGRES_PASSWORD", "password") .start() From d9033b2636476e943a19f786532583f004586671 Mon Sep 17 00:00:00 2001 From: Gerard Burns Date: Mon, 25 Mar 2024 16:39:42 +0000 Subject: [PATCH 15/15] Un-revert Skeleton Loader PR (#13180) * wip * wip * wip * client versions init * wip * wip * wip * wip * wip * linting * remove log * comment client version script * lint * skeleton loader type fix * fix types * lint * fix types again * fix manifest not being served locally * remove preinstalled old client version * add constant for dev client version * linting * Dean PR Feedback * linting * pr feedback * wip * wip * clientVersions empty array * delete from git * empty array again * fix tests * pr feedback --------- Co-authored-by: Andrew Kingston --- .eslintignore | 3 +- .gitignore | 3 + package.json | 1 + .../deploy/RevertModalVersionSelect.svelte | 33 +++ .../src/components/deploy/VersionModal.svelte | 18 +- .../src/components/deploy/clientVersions.json | 1 + .../[screenId]/_components/AppPreview.svelte | 43 ++- .../builder/portal/apps/[appId]/index.svelte | 63 +++- packages/client/manifest.json | 3 +- .../client/src/components/ClientApp.svelte | 276 ++++++++++-------- .../client/src/components/FreeFooter.svelte | 1 + packages/client/src/licensing/features.js | 5 - packages/client/src/licensing/index.js | 7 - packages/client/src/licensing/utils.js | 32 -- packages/client/src/stores/features.js | 42 +++ packages/client/src/stores/index.js | 1 + packages/frontend-core/src/api/app.js | 9 + .../src/components/ClientAppSkeleton.svelte | 244 ++++++++++++++++ .../frontend-core/src/components/index.js | 1 + .../frontend-core/src/themes/midnight.css | 1 - packages/server/package.json | 1 + .../server/src/api/controllers/application.ts | 30 +- .../src/api/controllers/static/index.ts | 34 ++- .../static/templates/BudibaseApp.svelte | 10 + .../api/controllers/static/templates/app.hbs | 8 +- packages/server/src/api/routes/application.ts | 5 + packages/server/src/api/routes/static.ts | 2 +- packages/server/src/constants/index.ts | 2 + packages/server/src/constants/themes.ts | 54 ++++ .../server/src/utilities/fileSystem/app.ts | 33 ++- .../src/utilities/fileSystem/clientLibrary.ts | 56 +++- packages/types/src/documents/app/app.ts | 1 + scripts/build.js | 28 +- scripts/getPastClientVersion.js | 45 +++ 34 files changed, 850 insertions(+), 246 deletions(-) create mode 100644 packages/builder/src/components/deploy/RevertModalVersionSelect.svelte create mode 100644 packages/builder/src/components/deploy/clientVersions.json delete mode 100644 packages/client/src/licensing/features.js delete mode 100644 packages/client/src/licensing/index.js delete mode 100644 packages/client/src/licensing/utils.js create mode 100644 packages/client/src/stores/features.js create mode 100644 packages/frontend-core/src/components/ClientAppSkeleton.svelte create mode 100644 packages/server/src/constants/themes.ts create mode 100644 scripts/getPastClientVersion.js diff --git a/.eslintignore b/.eslintignore index f2c53c2fdc..94984a446f 100644 --- a/.eslintignore +++ b/.eslintignore @@ -12,4 +12,5 @@ packages/sdk/sdk packages/account-portal/packages/server/build packages/account-portal/packages/ui/.routify packages/account-portal/packages/ui/build -**/*.ivm.bundle.js \ No newline at end of file +**/*.ivm.bundle.js +packages/server/build/oldClientVersions/**/** diff --git a/.gitignore b/.gitignore index 8861a14d20..661c60e95e 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,9 @@ packages/server/runtime_apps/ bb-airgapped.tar.gz *.iml +packages/server/build/oldClientVersions/**/* +packages/builder/src/components/deploy/clientVersions.json + # Logs logs *.log diff --git a/package.json b/package.json index 7de22ab456..79a7b06eff 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ }, "scripts": { "preinstall": "node scripts/syncProPackage.js", + "get-past-client-version": "node scripts/getPastClientVersion.js", "setup": "git config submodule.recurse true && git submodule update && node ./hosting/scripts/setup.js && yarn && yarn build && yarn dev", "build": "NODE_OPTIONS=--max-old-space-size=1500 lerna run build --stream", "build:dev": "lerna run --stream prebuild && yarn nx run-many --target=build --output-style=dynamic --watch --preserveWatchOutput", diff --git a/packages/builder/src/components/deploy/RevertModalVersionSelect.svelte b/packages/builder/src/components/deploy/RevertModalVersionSelect.svelte new file mode 100644 index 0000000000..ed40a101d0 --- /dev/null +++ b/packages/builder/src/components/deploy/RevertModalVersionSelect.svelte @@ -0,0 +1,33 @@ + + +
+