From 9912904bd1faf766c4cb7c58877c95634090ad91 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 31 May 2024 16:07:46 +0100 Subject: [PATCH 01/24] Fixing an issue with error cases that have a content-disposition being downloaded as a file. --- packages/server/src/integrations/rest.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/server/src/integrations/rest.ts b/packages/server/src/integrations/rest.ts index 451c319aa9..86c059bc82 100644 --- a/packages/server/src/integrations/rest.ts +++ b/packages/server/src/integrations/rest.ts @@ -149,13 +149,12 @@ class RestIntegration implements IntegrationBase { { downloadImages: this.config.downloadImages } ) let contentLength = response.headers.get("content-length") - if (!contentLength && raw) { - contentLength = Buffer.byteLength(raw, "utf8").toString() - } + let isSuccess = response.status >= 200 && response.status < 300 if ( - contentDisposition.includes("filename") || - contentDisposition.includes("attachment") || - contentDisposition.includes("form-data") + (contentDisposition.includes("filename") || + contentDisposition.includes("attachment") || + contentDisposition.includes("form-data")) && + isSuccess ) { filename = path.basename(parse(contentDisposition).parameters?.filename) || "" @@ -168,6 +167,9 @@ class RestIntegration implements IntegrationBase { return handleFileResponse(response, filename, this.startTimeMs) } else { responseTxt = response.text ? await response.text() : "" + if (!contentLength && responseTxt) { + contentLength = Buffer.byteLength(responseTxt, "utf8").toString() + } const hasContent = (contentLength && parseInt(contentLength) > 0) || responseTxt.length > 0 From 4a1f24d0d8925979a4d6d6322ba34909f7431ffe Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 31 May 2024 16:08:10 +0100 Subject: [PATCH 02/24] Fixing an issue with default parameters not being passed into dynamic parameters. --- packages/server/src/threads/query.ts | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/server/src/threads/query.ts b/packages/server/src/threads/query.ts index 54322b1156..1210461bba 100644 --- a/packages/server/src/threads/query.ts +++ b/packages/server/src/threads/query.ts @@ -14,7 +14,13 @@ import { context, cache, auth } from "@budibase/backend-core" import { getGlobalIDFromUserMetadataID } from "../db/utils" import sdk from "../sdk" import { cloneDeep } from "lodash/fp" -import { Datasource, Query, SourceName, Row } from "@budibase/types" +import { + Datasource, + Query, + SourceName, + Row, + QueryParameter, +} from "@budibase/types" import { isSQL } from "../integrations/utils" import { interpolateSQL } from "../integrations/queries/sql" @@ -196,12 +202,22 @@ class QueryRunner { return { rows, keys, info, extra, pagination } } - async runAnotherQuery(queryId: string, parameters: any) { + async runAnotherQuery( + queryId: string, + currentParameters: Record + ) { const db = context.getAppDB() const query = await db.get(queryId) const datasource = await sdk.datasources.get(query.datasourceId, { enriched: true, }) + // enrich parameters with dynamic queries defaults + const defaultParams = query.parameters || [] + for (let param of defaultParams) { + if (!currentParameters[param.name]) { + currentParameters[param.name] = param.default + } + } return new QueryRunner( { schema: query.schema, @@ -210,7 +226,7 @@ class QueryRunner { transformer: query.transformer, nullDefaultSupport: query.nullDefaultSupport, ctx: this.ctx, - parameters, + parameters: currentParameters, datasource, queryId, }, From 6df5315b32b660414dbd0150fe728714a83d1342 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 31 May 2024 16:08:33 +0100 Subject: [PATCH 03/24] Update yarn lock --- yarn.lock | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/yarn.lock b/yarn.lock index 677b7cb441..d85a50e938 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11904,6 +11904,17 @@ glob@^10.0.0, glob@^10.2.2: minipass "^7.0.4" path-scurry "^1.10.2" +glob@^10.3.7: + version "10.4.1" + resolved "https://registry.yarnpkg.com/glob/-/glob-10.4.1.tgz#0cfb01ab6a6b438177bfe6a58e2576f6efe909c2" + integrity sha512-2jelhlq3E4ho74ZyVLN03oKdAZVUa6UDZzFLVH1H7dnoax+y9qyaq8zBkfDIggjniU19z0wU18y16jMB2eyVIw== + dependencies: + foreground-child "^3.1.0" + jackspeak "^3.1.2" + minimatch "^9.0.4" + minipass "^7.1.2" + path-scurry "^1.11.1" + glob@^5.0.15: version "5.0.15" resolved "https://registry.yarnpkg.com/glob/-/glob-5.0.15.tgz#1bc936b9e02f4a603fcc222ecf7633d30b8b93b1" @@ -13472,6 +13483,15 @@ jackspeak@^2.3.6: optionalDependencies: "@pkgjs/parseargs" "^0.11.0" +jackspeak@^3.1.2: + version "3.1.2" + resolved "https://registry.yarnpkg.com/jackspeak/-/jackspeak-3.1.2.tgz#eada67ea949c6b71de50f1b09c92a961897b90ab" + integrity sha512-kWmLKn2tRtfYMF/BakihVVRzBKOxz4gJMiL2Rj91WnAB5TPZumSH99R/Yf1qE1u4uRimvCSJfm6hnxohXeEXjQ== + dependencies: + "@isaacs/cliui" "^8.0.2" + optionalDependencies: + "@pkgjs/parseargs" "^0.11.0" + jake@^10.8.5: version "10.8.5" resolved "https://registry.yarnpkg.com/jake/-/jake-10.8.5.tgz#f2183d2c59382cb274226034543b9c03b8164c46" @@ -15751,6 +15771,13 @@ minimatch@^8.0.2: dependencies: brace-expansion "^2.0.1" +minimatch@^9.0.4: + version "9.0.4" + resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-9.0.4.tgz#8e49c731d1749cbec05050ee5145147b32496a51" + integrity sha512-KqWh+VchfxcMNRAJjj2tnsSJdNbHsVgnkBhTNrW7AjVo6OvLtxw8zfT9oLw1JSohlFzJ8jCoTgaoXvJ+kHt6fw== + dependencies: + brace-expansion "^2.0.1" + minimist-options@4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/minimist-options/-/minimist-options-4.1.0.tgz#c0655713c53a8a2ebd77ffa247d342c40f010619" @@ -15845,6 +15872,11 @@ minipass@^5.0.0: resolved "https://registry.yarnpkg.com/minipass/-/minipass-7.0.4.tgz#dbce03740f50a4786ba994c1fb908844d27b038c" integrity sha512-jYofLM5Dam9279rdkWzqHozUo4ybjdZmCsDHePy5V/PbBcVMiSZR97gmAy45aqi8CK1lG2ECd356FU86avfwUQ== +minipass@^7.1.2: + version "7.1.2" + resolved "https://registry.yarnpkg.com/minipass/-/minipass-7.1.2.tgz#93a9626ce5e5e66bd4db86849e7515e92340a707" + integrity sha512-qOOzS1cBTWYF4BH8fVePDBOO9iptMnGUEZwNc/cMWnTV2nVLZ7VoNWEPHkYczZA0pdoA7dl6e7FL659nX9S2aw== + minizlib@^2.1.1, minizlib@^2.1.2: version "2.1.2" resolved "https://registry.yarnpkg.com/minizlib/-/minizlib-2.1.2.tgz#e90d3466ba209b932451508a11ce3d3632145931" @@ -16033,10 +16065,10 @@ mute-stream@~1.0.0: resolved "https://registry.yarnpkg.com/mute-stream/-/mute-stream-1.0.0.tgz#e31bd9fe62f0aed23520aa4324ea6671531e013e" integrity sha512-avsJQhyd+680gKXyG/sQc0nXaC6rBkPOfyHYcFb9+hdkqQkR9bdnkJ0AMZhke0oesPqIO+mFFJ+IdBc7mst4IA== -mysql2@3.9.7: - version "3.9.7" - resolved "https://registry.yarnpkg.com/mysql2/-/mysql2-3.9.7.tgz#843755daf65b5ef08afe545fe14b8fb62824741a" - integrity sha512-KnJT8vYRcNAZv73uf9zpXqNbvBG7DJrs+1nACsjZP1HMJ1TgXEy8wnNilXAn/5i57JizXKtrUtwDB7HxT9DDpw== +mysql2@3.9.8: + version "3.9.8" + resolved "https://registry.yarnpkg.com/mysql2/-/mysql2-3.9.8.tgz#fe8a0f975f2c495ed76ca988ddc5505801dc49ce" + integrity sha512-+5JKNjPuks1FNMoy9TYpl77f+5frbTklz7eb3XDwbpsERRLEeXiW2PDEkakYF50UuKU2qwfGnyXpKYvukv8mGA== dependencies: denque "^2.1.0" generate-function "^2.3.1" @@ -17378,6 +17410,14 @@ path-scurry@^1.10.2, path-scurry@^1.6.1: lru-cache "^10.2.0" minipass "^5.0.0 || ^6.0.2 || ^7.0.0" +path-scurry@^1.11.1: + version "1.11.1" + resolved "https://registry.yarnpkg.com/path-scurry/-/path-scurry-1.11.1.tgz#7960a668888594a0720b12a911d1a742ab9f11d2" + integrity sha512-Xa4Nw17FS9ApQFJ9umLiJS4orGjm7ZzwUrwamcGQuHSzDyth9boKDaycYdDcZDuqYATXw4HFXgaqWTctW/v1HA== + dependencies: + lru-cache "^10.2.0" + minipass "^5.0.0 || ^6.0.2 || ^7.0.0" + path-to-regexp@1.x: version "1.8.0" resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-1.8.0.tgz#887b3ba9d84393e87a0a0b9f4cb756198b53548a" @@ -19318,6 +19358,13 @@ rimraf@^4.4.1: dependencies: glob "^9.2.0" +rimraf@^5.0.7: + version "5.0.7" + resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-5.0.7.tgz#27bddf202e7d89cb2e0381656380d1734a854a74" + integrity sha512-nV6YcJo5wbLW77m+8KjH8aB/7/rxQy9SZ0HY5shnwULfS+9nmTtVXAJET5NdZmCzA4fPI/Hm1wo/Po/4mopOdg== + dependencies: + glob "^10.3.7" + ripemd160@^2.0.0, ripemd160@^2.0.1: version "2.0.2" resolved "https://registry.yarnpkg.com/ripemd160/-/ripemd160-2.0.2.tgz#a1c1a6f624751577ba5d07914cbc92850585890c" From 75501c225198a43ffceae4f8eb8df44faf9c2532 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 31 May 2024 17:57:31 +0100 Subject: [PATCH 04/24] Updating object store stream upload to make sure the stream has finished being processed before trying to upload to AWS (and only uploading a partial stream). --- .../src/objectStore/objectStore.ts | 35 +++++++++++++++---- .../types/src/documents/app/automation.ts | 2 +- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/packages/backend-core/src/objectStore/objectStore.ts b/packages/backend-core/src/objectStore/objectStore.ts index 0ac2c35179..35748d8f76 100644 --- a/packages/backend-core/src/objectStore/objectStore.ts +++ b/packages/backend-core/src/objectStore/objectStore.ts @@ -41,10 +41,7 @@ type UploadParams = BaseUploadParams & { path?: string | PathLike } -export type StreamTypes = - | ReadStream - | NodeJS.ReadableStream - | ReadableStream +export type StreamTypes = ReadStream | NodeJS.ReadableStream export type StreamUploadParams = BaseUploadParams & { stream?: StreamTypes @@ -222,6 +219,9 @@ export async function streamUpload({ extra, ttl, }: StreamUploadParams) { + if (!stream) { + throw new Error("Stream to upload is invalid/undefined") + } const extension = filename.split(".").pop() const objectStore = ObjectStore(bucketName) const bucketCreated = await createBucketIfNotExists(objectStore, bucketName) @@ -251,14 +251,35 @@ export async function streamUpload({ : CONTENT_TYPE_MAP.txt } + const bucket = sanitizeBucket(bucketName), + objKey = sanitizeKey(filename) const params = { - Bucket: sanitizeBucket(bucketName), - Key: sanitizeKey(filename), + Bucket: bucket, + Key: objKey, Body: stream, ContentType: contentType, ...extra, } - return objectStore.upload(params).promise() + + // make sure we have the stream before we try to push it to object store + if (stream.on) { + await new Promise((resolve, reject) => { + stream.on("finish", resolve) + stream.on("error", reject) + }) + } + + const details = await objectStore.upload(params).promise() + const headDetails = await objectStore + .headObject({ + Bucket: bucket, + Key: objKey, + }) + .promise() + return { + ...details, + ContentLength: headDetails.ContentLength, + } } /** diff --git a/packages/types/src/documents/app/automation.ts b/packages/types/src/documents/app/automation.ts index 6d1753dc28..63291fa3bb 100644 --- a/packages/types/src/documents/app/automation.ts +++ b/packages/types/src/documents/app/automation.ts @@ -245,7 +245,7 @@ export type AutomationAttachment = { export type AutomationAttachmentContent = { filename: string - content: ReadStream | NodeJS.ReadableStream | ReadableStream + content: ReadStream | NodeJS.ReadableStream } export type BucketedContent = AutomationAttachmentContent & { From d90763dd3c88008454a0cbe0a51175cfa454af0d Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 31 May 2024 17:59:16 +0100 Subject: [PATCH 05/24] Getting size parameter right for streams. --- packages/server/src/integrations/utils/utils.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/integrations/utils/utils.ts b/packages/server/src/integrations/utils/utils.ts index 9f04457d7a..be689fbfd3 100644 --- a/packages/server/src/integrations/utils/utils.ts +++ b/packages/server/src/integrations/utils/utils.ts @@ -368,13 +368,16 @@ export async function handleFileResponse( size = parseInt(contentLength, 10) } - await objectStore.streamUpload({ + const details = await objectStore.streamUpload({ bucket, filename: key, stream, ttl: 1, type: response.headers["content-type"], }) + if (!size && details.ContentLength) { + size = details.ContentLength + } } presignedUrl = objectStore.getPresignedUrl(bucket, key) return { From 26a0801b755ff704c62d6a7ac1755e8bd8b8ad5a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 3 Jun 2024 10:15:16 +0100 Subject: [PATCH 06/24] Linting. --- packages/server/src/threads/query.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/threads/query.ts b/packages/server/src/threads/query.ts index 1210461bba..d76714661d 100644 --- a/packages/server/src/threads/query.ts +++ b/packages/server/src/threads/query.ts @@ -19,7 +19,6 @@ import { Query, SourceName, Row, - QueryParameter, } from "@budibase/types" import { isSQL } from "../integrations/utils" From 5912c2b129f9888268a95afa23514668dcb97d43 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 31 May 2024 13:30:05 +0200 Subject: [PATCH 07/24] Copy change --- packages/server/src/api/routes/tests/viewV2.spec.ts | 3 +-- packages/server/src/sdk/app/views/index.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 962d6e82a3..7e02966b44 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -349,8 +349,7 @@ describe.each([ await config.api.viewV2.create(newView, { status: 400, body: { - message: - 'Field "name" cannot be readonly as it is a required field', + message: 'You can\'t make the required field "name" read only', status: 400, }, }) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 18ab94be21..150fe2c678 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -60,7 +60,7 @@ async function guardViewSchema( if (isRequired(tableSchemaField.constraints)) { throw new HTTPError( - `Field "${field}" cannot be readonly as it is a required field`, + `You can't make the required field "${field}" read only`, 400 ) } From efc9d3399e11c20daf744e30194bcc7aac79e195 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 31 May 2024 17:08:50 +0200 Subject: [PATCH 08/24] Validate schema --- .../src/api/routes/tests/viewV2.spec.ts | 126 +++++++++++++++++- .../server/src/api/routes/utils/validators.ts | 36 ++++- 2 files changed, 153 insertions(+), 9 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 7e02966b44..af5cbfc063 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -23,9 +23,6 @@ import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" import merge from "lodash/merge" import { quotas } from "@budibase/pro" import { roles } from "@budibase/backend-core" -import * as schemaUtils from "../../../utilities/schema" - -jest.mock("../../../utilities/schema") describe.each([ ["internal", undefined], @@ -318,15 +315,13 @@ describe.each([ }) it("required fields cannot be marked as readonly", async () => { - const isRequiredSpy = jest.spyOn(schemaUtils, "isRequired") - isRequiredSpy.mockReturnValueOnce(true) - const table = await config.api.table.save( saveTableRequest({ schema: { name: { name: "name", type: FieldType.STRING, + constraints: { presence: true }, }, description: { name: "description", @@ -1347,4 +1342,123 @@ describe.each([ }) }) }) + + describe("updating table schema", () => { + describe("existing columns changed to required", () => { + beforeEach(async () => { + table = await config.api.table.save( + saveTableRequest({ + schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + }, + name: { + name: "name", + type: FieldType.STRING, + }, + }, + }) + ) + }) + + it("allows updating when no views constrains the field", async () => { + await config.api.viewV2.create({ + name: "view a", + tableId: table._id!, + schema: { + id: { visible: true }, + name: { visible: true }, + }, + }) + + table = await config.api.table.get(table._id!) + await config.api.table.save( + { + ...table, + schema: { + ...table.schema, + name: { + name: "name", + type: FieldType.STRING, + constraints: { presence: { allowEmpty: false } }, + }, + }, + }, + { status: 200 } + ) + }) + + it("rejects if field is readonly in any view", async () => { + mocks.licenses.useViewReadonlyColumns() + + await config.api.viewV2.create({ + name: "view a", + tableId: table._id!, + schema: { + id: { visible: true }, + name: { + visible: true, + readonly: true, + }, + }, + }) + + table = await config.api.table.get(table._id!) + await config.api.table.save( + { + ...table, + schema: { + ...table.schema, + name: { + name: "name", + type: FieldType.STRING, + constraints: { presence: true }, + }, + }, + }, + { + status: 400, + body: { + status: 400, + message: + 'Invalid body - Required field "name" is missing in view "view a"', + }, + } + ) + }) + + it("rejects if field is hidden in any view", async () => { + await config.api.viewV2.create({ + name: "view a", + tableId: table._id!, + schema: { id: { visible: true } }, + }) + + table = await config.api.table.get(table._id!) + await config.api.table.save( + { + ...table, + schema: { + ...table.schema, + name: { + name: "name", + type: FieldType.STRING, + constraints: { presence: true }, + }, + }, + }, + { + status: 400, + body: { + status: 400, + message: + 'Invalid body - Required field "name" is missing in view "view a"', + }, + } + ) + }) + }) + }) }) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 424d0d6c79..0a4eff4247 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -1,14 +1,44 @@ import { auth, permissions } from "@budibase/backend-core" import { DataSourceOperation } from "../../../constants" -import { WebhookActionType } from "@budibase/types" -import Joi from "joi" +import { Table, WebhookActionType } from "@budibase/types" +import Joi, { CustomValidator } from "joi" import { ValidSnippetNameRegex } from "@budibase/shared-core" +import { isRequired } from "../../../utilities/schema" +import sdk from "../../../sdk" const OPTIONAL_STRING = Joi.string().optional().allow(null).allow("") const OPTIONAL_NUMBER = Joi.number().optional().allow(null) const OPTIONAL_BOOLEAN = Joi.boolean().optional().allow(null) const APP_NAME_REGEX = /^[\w\s]+$/ +const validateViewSchemas: CustomValidator = (table, helpers) => { + if (table.views && Object.entries(table.views).length) { + const requiredFields = Object.entries(table.schema) + .filter(([_, v]) => isRequired(v.constraints)) + .map(([key]) => key) + if (requiredFields.length) { + for (const view of Object.values(table.views)) { + if (!sdk.views.isV2(view)) { + continue + } + + const editableViewFields = Object.entries(view.schema || {}) + .filter(([_, f]) => f.visible && !f.readonly) + .map(([key]) => key) + const missingField = requiredFields.find( + f => !editableViewFields.includes(f) + ) + if (missingField) { + return helpers.message({ + custom: `Required field "${missingField}" is missing in view "${view.name}"`, + }) + } + } + } + } + return table +} + export function tableValidator() { // prettier-ignore return auth.joiValidator.body(Joi.object({ @@ -20,7 +50,7 @@ export function tableValidator() { name: Joi.string().required(), views: Joi.object(), rows: Joi.array(), - }).unknown(true)) + }).custom(validateViewSchemas).unknown(true)) } export function nameValidator() { From 326a90a41e550b9859933fc7531b547efd06c622 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 May 2024 14:37:19 +0200 Subject: [PATCH 09/24] Allow modifying views with readonly configs (other fields) --- packages/server/src/sdk/app/views/index.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 150fe2c678..292d643648 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -54,7 +54,10 @@ async function guardViewSchema( } if (viewSchema[field].readonly) { - if (!(await features.isViewReadonlyColumnsEnabled())) { + if ( + !(await features.isViewReadonlyColumnsEnabled()) && + !(tableSchemaField as ViewUIFieldMetadata).readonly + ) { throw new HTTPError(`Readonly fields are not enabled`, 400) } From dad689c78700c1344abffbf01276889d6a6deb46 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 30 May 2024 14:37:35 +0200 Subject: [PATCH 10/24] Reset schema mutations on erroring --- .../src/components/grid/controls/ColumnsSettingButton.svelte | 2 ++ .../frontend-core/src/components/grid/stores/datasource.js | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte index 3f0e2341be..aa27871f92 100644 --- a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte +++ b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte @@ -24,6 +24,8 @@ await datasource.actions.saveSchemaMutations() } catch (e) { notifications.error(e.message) + } finally { + datasource.actions.resetSchemaMutations() } dispatch(visible ? "show-column" : "hide-column") } diff --git a/packages/frontend-core/src/components/grid/stores/datasource.js b/packages/frontend-core/src/components/grid/stores/datasource.js index 1fc973f171..09b8be4868 100644 --- a/packages/frontend-core/src/components/grid/stores/datasource.js +++ b/packages/frontend-core/src/components/grid/stores/datasource.js @@ -204,6 +204,10 @@ export const createActions = context => { ...$definition, schema: newSchema, }) + resetSchemaMutations() + } + + const resetSchemaMutations = () => { schemaMutations.set({}) } @@ -253,6 +257,7 @@ export const createActions = context => { addSchemaMutation, addSchemaMutations, saveSchemaMutations, + resetSchemaMutations, }, }, } From d73d7113aedd31a4674ca4560080f34b98d67f5f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 3 Jun 2024 10:07:42 +0200 Subject: [PATCH 11/24] Refresh on error --- .../components/grid/controls/ColumnsSettingButton.svelte | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte index aa27871f92..228cf69e34 100644 --- a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte +++ b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte @@ -19,13 +19,17 @@ const visible = permission !== PERMISSION_OPTIONS.HIDDEN const readonly = permission === PERMISSION_OPTIONS.READONLY - datasource.actions.addSchemaMutation(column.name, { visible, readonly }) + await datasource.actions.addSchemaMutation(column.name, { + visible, + readonly, + }) try { await datasource.actions.saveSchemaMutations() } catch (e) { notifications.error(e.message) } finally { - datasource.actions.resetSchemaMutations() + await datasource.actions.resetSchemaMutations() + await datasource.actions.refreshDefinition() } dispatch(visible ? "show-column" : "hide-column") } From 91c20213dc20341d1cee5dacc27b7a957ffadc37 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 3 Jun 2024 10:29:26 +0200 Subject: [PATCH 12/24] Validate readonly --- .../src/api/routes/tests/viewV2.spec.ts | 3 ++- packages/server/src/sdk/app/views/index.ts | 25 +++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index af5cbfc063..4e98ffe3cc 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -336,6 +336,7 @@ describe.each([ tableId: table._id!, schema: { name: { + visible: true, readonly: true, }, }, @@ -344,7 +345,7 @@ describe.each([ await config.api.viewV2.create(newView, { status: 400, body: { - message: 'You can\'t make the required field "name" read only', + message: 'You can\'t make read only the required field "name"', status: 400, }, }) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 292d643648..ff51a73e99 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -39,9 +39,7 @@ async function guardViewSchema( tableId: string, viewSchema?: Record ) { - if (!viewSchema || !Object.keys(viewSchema).length) { - return - } + viewSchema ??= {} const table = await sdk.tables.getTable(tableId) for (const field of Object.keys(viewSchema)) { @@ -61,13 +59,6 @@ async function guardViewSchema( throw new HTTPError(`Readonly fields are not enabled`, 400) } - if (isRequired(tableSchemaField.constraints)) { - throw new HTTPError( - `You can't make the required field "${field}" read only`, - 400 - ) - } - if (!viewSchema[field].visible) { throw new HTTPError( `Field "${field}" must be visible if you want to make it readonly`, @@ -76,6 +67,20 @@ async function guardViewSchema( } } } + + for (const field of Object.values(table.schema)) { + if (!isRequired(field.constraints)) { + continue + } + + const viewSchemaField = viewSchema[field.name] + if (viewSchemaField?.readonly) { + throw new HTTPError( + `You can't make read only the required field "${field.name}"`, + 400 + ) + } + } } export async function create( From c1b760ca9e1c977396fa0fd201c7b09ab0b0ba39 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 3 Jun 2024 12:43:51 +0200 Subject: [PATCH 13/24] Validate that required fields can't be hidden in views --- packages/server/src/sdk/app/views/index.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index ff51a73e99..98871539c2 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -74,7 +74,15 @@ async function guardViewSchema( } const viewSchemaField = viewSchema[field.name] - if (viewSchemaField?.readonly) { + + if (!viewSchemaField?.visible) { + throw new HTTPError( + `You can't hide the required field "${field.name}"`, + 400 + ) + } + + if (viewSchemaField.readonly) { throw new HTTPError( `You can't make read only the required field "${field.name}"`, 400 From 155de99b68c8fc481cc4d0ac37eba751595cb22f Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 3 Jun 2024 11:46:20 +0100 Subject: [PATCH 14/24] Streaming to disk before passing onto S3. --- .../src/objectStore/objectStore.ts | 9 +-- packages/server/package.json | 5 +- .../src/integrations/tests/rest.spec.ts | 2 + .../server/src/integrations/utils/utils.ts | 70 +++++++++++-------- yarn.lock | 12 +++- 5 files changed, 56 insertions(+), 42 deletions(-) diff --git a/packages/backend-core/src/objectStore/objectStore.ts b/packages/backend-core/src/objectStore/objectStore.ts index 35748d8f76..de94e3968b 100644 --- a/packages/backend-core/src/objectStore/objectStore.ts +++ b/packages/backend-core/src/objectStore/objectStore.ts @@ -14,6 +14,7 @@ import { v4 } from "uuid" import { APP_PREFIX, APP_DEV_PREFIX } from "../db" import fsp from "fs/promises" import { HeadObjectOutput } from "aws-sdk/clients/s3" +import { ReadableStream } from "stream/web" const streamPipeline = promisify(stream.pipeline) // use this as a temporary store of buckets that are being created @@ -261,14 +262,6 @@ export async function streamUpload({ ...extra, } - // make sure we have the stream before we try to push it to object store - if (stream.on) { - await new Promise((resolve, reject) => { - stream.on("finish", resolve) - stream.on("error", reject) - }) - } - const details = await objectStore.upload(params).promise() const headDetails = await objectStore .headObject({ diff --git a/packages/server/package.json b/packages/server/package.json index bd5a82cb29..b3beac7ffb 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -68,7 +68,6 @@ "aws-sdk": "2.1030.0", "bcrypt": "5.1.0", "bcryptjs": "2.4.3", - "bl": "^6.0.12", "bull": "4.10.1", "chokidar": "3.5.3", "content-disposition": "^0.5.4", @@ -116,7 +115,8 @@ "uuid": "^8.3.2", "validate.js": "0.13.1", "worker-farm": "1.7.0", - "xml2js": "0.5.0" + "xml2js": "0.5.0", + "tmp": "0.2.3" }, "devDependencies": { "@babel/preset-env": "7.16.11", @@ -137,6 +137,7 @@ "@types/supertest": "2.0.14", "@types/tar": "6.1.5", "@types/uuid": "8.3.4", + "@types/tmp": "0.2.6", "copyfiles": "2.4.1", "docker-compose": "0.23.17", "jest": "29.7.0", diff --git a/packages/server/src/integrations/tests/rest.spec.ts b/packages/server/src/integrations/tests/rest.spec.ts index f20f369c25..dee17a5497 100644 --- a/packages/server/src/integrations/tests/rest.spec.ts +++ b/packages/server/src/integrations/tests/rest.spec.ts @@ -657,6 +657,7 @@ describe("REST Integration", () => { mockReadable.push(null) ;(fetch as unknown as jest.Mock).mockImplementationOnce(() => Promise.resolve({ + status: 200, headers: { raw: () => ({ "content-type": [contentType], @@ -700,6 +701,7 @@ describe("REST Integration", () => { mockReadable.push(null) ;(fetch as unknown as jest.Mock).mockImplementationOnce(() => Promise.resolve({ + status: 200, headers: { raw: () => ({ "content-type": [contentType], diff --git a/packages/server/src/integrations/utils/utils.ts b/packages/server/src/integrations/utils/utils.ts index be689fbfd3..157bdba3bd 100644 --- a/packages/server/src/integrations/utils/utils.ts +++ b/packages/server/src/integrations/utils/utils.ts @@ -9,10 +9,12 @@ import { context, objectStore, sql } from "@budibase/backend-core" import { v4 } from "uuid" import { parseStringPromise as xmlParser } from "xml2js" import { formatBytes } from "../../utilities" -import bl from "bl" import env from "../../environment" import { InvalidColumns } from "../../constants" import { helpers, utils } from "@budibase/shared-core" +import { pipeline } from "stream/promises" +import tmp from "tmp" +import fs from "fs" type PrimitiveTypes = | FieldType.STRING @@ -360,38 +362,44 @@ export async function handleFileResponse( const key = `${context.getProdAppId()}/${processedFileName}` const bucket = objectStore.ObjectStoreBuckets.TEMP - const stream = response.body.pipe(bl((error, data) => data)) + // put the response stream to disk temporarily as a buffer + const tmpObj = tmp.fileSync() + try { + await pipeline(response.body, fs.createWriteStream(tmpObj.name)) + if (response.body) { + const contentLength = response.headers.get("content-length") + if (contentLength) { + size = parseInt(contentLength, 10) + } - if (response.body) { - const contentLength = response.headers.get("content-length") - if (contentLength) { - size = parseInt(contentLength, 10) + const details = await objectStore.streamUpload({ + bucket, + filename: key, + stream: fs.createReadStream(tmpObj.name), + ttl: 1, + type: response.headers["content-type"], + }) + if (!size && details.ContentLength) { + size = details.ContentLength + } } - - const details = await objectStore.streamUpload({ - bucket, - filename: key, - stream, - ttl: 1, - type: response.headers["content-type"], - }) - if (!size && details.ContentLength) { - size = details.ContentLength + presignedUrl = objectStore.getPresignedUrl(bucket, key) + return { + data: { + size, + name: processedFileName, + url: presignedUrl, + extension: fileExtension, + key: key, + }, + info: { + code: response.status, + size: formatBytes(size.toString()), + time: `${Math.round(performance.now() - startTime)}ms`, + }, } - } - presignedUrl = objectStore.getPresignedUrl(bucket, key) - return { - data: { - size, - name: processedFileName, - url: presignedUrl, - extension: fileExtension, - key: key, - }, - info: { - code: response.status, - size: formatBytes(size.toString()), - time: `${Math.round(performance.now() - startTime)}ms`, - }, + } finally { + // cleanup tmp + tmpObj.removeCallback() } } diff --git a/yarn.lock b/yarn.lock index d85a50e938..5297fe0cad 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6348,6 +6348,11 @@ dependencies: "@types/estree" "*" +"@types/tmp@0.2.6": + version "0.2.6" + resolved "https://registry.yarnpkg.com/@types/tmp/-/tmp-0.2.6.tgz#d785ee90c52d7cc020e249c948c36f7b32d1e217" + integrity sha512-chhaNf2oKHlRkDGt+tiKE2Z5aJ6qalm7Z9rlLdBwmOiAAf09YQvvoLXjWK4HWPF1xU/fqvMgfNfpVoBscA/tKA== + "@types/tough-cookie@*", "@types/tough-cookie@^4.0.2": version "4.0.2" resolved "https://registry.yarnpkg.com/@types/tough-cookie/-/tough-cookie-4.0.2.tgz#6286b4c7228d58ab7866d19716f3696e03a09397" @@ -7700,7 +7705,7 @@ bl@^4.0.3, bl@^4.1.0: inherits "^2.0.4" readable-stream "^3.4.0" -bl@^6.0.12, bl@^6.0.3: +bl@^6.0.3: version "6.0.12" resolved "https://registry.yarnpkg.com/bl/-/bl-6.0.12.tgz#77c35b96e13aeff028496c798b75389ddee9c7f8" integrity sha512-EnEYHilP93oaOa2MnmNEjAcovPS3JlQZOyzGXi3EyEpPhm9qWvdDp7BmAVEVusGzp8LlwQK56Av+OkDoRjzE0w== @@ -21283,6 +21288,11 @@ tlhunter-sorted-set@^0.1.0: resolved "https://registry.yarnpkg.com/tlhunter-sorted-set/-/tlhunter-sorted-set-0.1.0.tgz#1c3eae28c0fa4dff97e9501d2e3c204b86406f4b" integrity sha512-eGYW4bjf1DtrHzUYxYfAcSytpOkA44zsr7G2n3PV7yOUR23vmkGe3LL4R+1jL9OsXtbsFOwe8XtbCrabeaEFnw== +tmp@0.2.3: + version "0.2.3" + resolved "https://registry.yarnpkg.com/tmp/-/tmp-0.2.3.tgz#eb783cc22bc1e8bebd0671476d46ea4eb32a79ae" + integrity sha512-nZD7m9iCPC5g0pYmcaxogYKggSfLsdxl8of3Q/oIbqCqLLIO9IAF0GWjX1z9NZRHPiXv8Wex4yDCaZsgEw0Y8w== + tmp@^0.0.33: version "0.0.33" resolved "https://registry.yarnpkg.com/tmp/-/tmp-0.0.33.tgz#6d34335889768d21b2bcda0aa277ced3b1bfadf9" From 38ff7debb46dc32f8d7f1b8d3ce073d5ad893594 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 3 Jun 2024 12:08:54 +0100 Subject: [PATCH 15/24] Linting. --- packages/server/src/threads/query.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/server/src/threads/query.ts b/packages/server/src/threads/query.ts index d76714661d..ba451a3325 100644 --- a/packages/server/src/threads/query.ts +++ b/packages/server/src/threads/query.ts @@ -14,12 +14,7 @@ import { context, cache, auth } from "@budibase/backend-core" import { getGlobalIDFromUserMetadataID } from "../db/utils" import sdk from "../sdk" import { cloneDeep } from "lodash/fp" -import { - Datasource, - Query, - SourceName, - Row, -} from "@budibase/types" +import { Datasource, Query, SourceName, Row } from "@budibase/types" import { isSQL } from "../integrations/utils" import { interpolateSQL } from "../integrations/queries/sql" From cc3808997cb3938df99e793c13aad2935ef4e446 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 3 Jun 2024 13:26:49 +0200 Subject: [PATCH 16/24] Fix viewV2 tests --- .../src/api/routes/tests/viewV2.spec.ts | 59 ++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 4e98ffe3cc..57a26dd6c7 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -117,6 +117,9 @@ describe.each([ const newView: CreateViewRequest = { name: generator.name(), tableId: table._id!, + schema: { + id: { visible: true }, + }, } const res = await config.api.viewV2.create(newView) @@ -145,6 +148,7 @@ describe.each([ type: SortType.STRING, }, schema: { + id: { visible: true }, Price: { visible: true, }, @@ -155,6 +159,7 @@ describe.each([ expect(res).toEqual({ ...newView, schema: { + id: { visible: true }, Price: { visible: true, }, @@ -169,6 +174,11 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { + name: "id", + type: FieldType.NUMBER, + visible: true, + }, Price: { name: "Price", type: FieldType.NUMBER, @@ -190,6 +200,7 @@ describe.each([ expect(createdView).toEqual({ ...newView, schema: { + id: { visible: true }, Price: { visible: true, order: 1, @@ -206,6 +217,12 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + visible: true, + }, Price: { name: "Price", type: FieldType.NUMBER, @@ -229,6 +246,7 @@ describe.each([ tableId: table._id!, primaryDisplay: generator.word(), schema: { + id: { visible: true }, Price: { visible: true }, Category: { visible: false }, }, @@ -238,6 +256,7 @@ describe.each([ expect(res).toEqual({ ...newView, schema: { + id: { visible: true }, Price: { visible: true, }, @@ -252,6 +271,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, nonExisting: { visible: true, }, @@ -290,6 +310,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, name: { visible: true, readonly: true, @@ -303,6 +324,7 @@ describe.each([ const res = await config.api.viewV2.create(newView) expect(res.schema).toEqual({ + id: { visible: true }, name: { visible: true, readonly: true, @@ -335,6 +357,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, name: { visible: true, readonly: true, @@ -371,6 +394,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, name: { visible: false, readonly: true, @@ -409,6 +433,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, name: { visible: true, readonly: true, @@ -436,6 +461,9 @@ describe.each([ view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + schema: { + id: { visible: true }, + }, }) }) @@ -484,6 +512,7 @@ describe.each([ type: SortType.STRING, }, schema: { + id: { visible: true }, Category: { visible: false, }, @@ -501,7 +530,7 @@ describe.each([ schema: { ...table.schema, id: expect.objectContaining({ - visible: false, + visible: true, }), Category: expect.objectContaining({ visible: false, @@ -598,6 +627,9 @@ describe.each([ const anotherView = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + schema: { + id: { visible: true }, + }, }) const result = await config .request!.put(`/api/v2/views/${anotherView.id}`) @@ -616,6 +648,7 @@ describe.each([ const updatedView = await config.api.viewV2.update({ ...view, schema: { + ...view.schema, Price: { name: "Price", type: FieldType.NUMBER, @@ -635,6 +668,7 @@ describe.each([ expect(updatedView).toEqual({ ...view, schema: { + id: { visible: true }, Price: { visible: true, order: 1, @@ -651,6 +685,7 @@ describe.each([ { ...view, schema: { + ...view.schema, Price: { name: "Price", type: FieldType.NUMBER, @@ -674,6 +709,7 @@ describe.each([ view = await config.api.viewV2.update({ ...view, schema: { + id: { visible: true }, Price: { visible: true, readonly: true, @@ -696,6 +732,7 @@ describe.each([ view = await config.api.viewV2.update({ ...view, schema: { + id: { visible: true }, Price: { visible: true, readonly: true, @@ -710,6 +747,7 @@ describe.each([ const res = await config.api.viewV2.update({ ...view, schema: { + id: { visible: true }, Price: { visible: true, readonly: false, @@ -720,6 +758,7 @@ describe.each([ expect.objectContaining({ ...view, schema: { + id: { visible: true }, Price: { visible: true, readonly: false, @@ -737,6 +776,9 @@ describe.each([ view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + schema: { + id: { visible: true }, + }, }) }) @@ -759,6 +801,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, Price: { visible: false }, Category: { visible: true }, }, @@ -781,6 +824,7 @@ describe.each([ name: generator.name(), tableId: table._id!, schema: { + id: { visible: true }, Price: { visible: true, readonly: true }, }, }) @@ -816,6 +860,7 @@ describe.each([ tableId: table._id!, name: generator.guid(), schema: { + id: { visible: true }, Country: { visible: true, }, @@ -850,6 +895,7 @@ describe.each([ tableId: table._id!, name: generator.guid(), schema: { + id: { visible: true }, two: { visible: true }, }, }) @@ -875,6 +921,7 @@ describe.each([ tableId: table._id!, name: generator.guid(), schema: { + id: { visible: true }, one: { visible: true, readonly: true }, two: { visible: true }, }, @@ -916,6 +963,7 @@ describe.each([ tableId: table._id!, name: generator.guid(), schema: { + id: { visible: true }, one: { visible: true, readonly: true }, two: { visible: true }, }, @@ -983,6 +1031,7 @@ describe.each([ rows.map(r => ({ _viewId: view.id, tableId: table._id, + id: r.id, _id: r._id, _rev: r._rev, ...(isInternal @@ -1023,6 +1072,7 @@ describe.each([ }, ], schema: { + id: { visible: true }, two: { visible: true }, }, }) @@ -1034,6 +1084,7 @@ describe.each([ { _viewId: view.id, tableId: table._id, + id: two.id, two: two.two, _id: two._id, _rev: two._rev, @@ -1187,7 +1238,11 @@ describe.each([ describe("sorting", () => { let table: Table - const viewSchema = { age: { visible: true }, name: { visible: true } } + const viewSchema = { + id: { visible: true }, + age: { visible: true }, + name: { visible: true }, + } beforeAll(async () => { table = await config.api.table.save( From 64a5accffd13661464197e07bb234eb1631326f7 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Mon, 3 Jun 2024 13:02:24 +0000 Subject: [PATCH 17/24] Bump version to 2.27.6 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 335df975af..d90f7732a2 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.27.5", + "version": "2.27.6", "npmClient": "yarn", "packages": [ "packages/*", From 020477f1f69abf9e0c265e4f452cfbf382b35e5f Mon Sep 17 00:00:00 2001 From: Christos Alexiou Date: Mon, 3 Jun 2024 17:15:51 +0300 Subject: [PATCH 18/24] qa-arc-runner-set -> ubuntu-latest --- .github/workflows/force-release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/force-release.yml b/.github/workflows/force-release.yml index 8a9d444f51..3d96d51484 100644 --- a/.github/workflows/force-release.yml +++ b/.github/workflows/force-release.yml @@ -9,7 +9,7 @@ on: jobs: ensure-is-master-tag: name: Ensure is a master tag - runs-on: qa-arc-runner-set + runs-on: ubuntu-latest steps: - name: Checkout monorepo uses: actions/checkout@v4 From 20c18259a95cf9303ece127c76456aef7011c10f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 3 Jun 2024 17:38:22 +0100 Subject: [PATCH 19/24] Update CouchDB chart from 4.3.0 to 4.5.3. --- charts/budibase/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/budibase/Chart.yaml b/charts/budibase/Chart.yaml index e2c9378f2c..83a72d203f 100644 --- a/charts/budibase/Chart.yaml +++ b/charts/budibase/Chart.yaml @@ -17,6 +17,6 @@ version: 0.0.0 appVersion: 0.0.0 dependencies: - name: couchdb - version: 4.3.0 + version: 4.5.3 repository: https://apache.github.io/couchdb-helm condition: services.couchdb.enabled From 9b82116c61f7ea9854bea5cefe7dfd1be9c85c19 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 10:39:56 +0200 Subject: [PATCH 20/24] Copy changes --- packages/server/src/api/routes/tests/viewV2.spec.ts | 3 ++- packages/server/src/sdk/app/views/index.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 4e98ffe3cc..663cf5c864 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -345,7 +345,8 @@ describe.each([ await config.api.viewV2.create(newView, { status: 400, body: { - message: 'You can\'t make read only the required field "name"', + message: + 'You can\'t make field "name" readonly because it is a required field.', status: 400, }, }) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index ff51a73e99..6cdd38bf43 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -76,7 +76,7 @@ async function guardViewSchema( const viewSchemaField = viewSchema[field.name] if (viewSchemaField?.readonly) { throw new HTTPError( - `You can't make read only the required field "${field.name}"`, + `You can't make field "${field.name}" readonly because it is a required field.`, 400 ) } From 2d953f19cc37a47756a0655cacf7273b19bf0333 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 11:11:50 +0200 Subject: [PATCH 21/24] Clean validation message --- .../src/middleware/joi-validator.ts | 23 +- .../server/src/api/routes/utils/validators.ts | 276 ++++++++++-------- 2 files changed, 172 insertions(+), 127 deletions(-) diff --git a/packages/backend-core/src/middleware/joi-validator.ts b/packages/backend-core/src/middleware/joi-validator.ts index ac8064a512..5047cdbbc1 100644 --- a/packages/backend-core/src/middleware/joi-validator.ts +++ b/packages/backend-core/src/middleware/joi-validator.ts @@ -3,7 +3,8 @@ import { Ctx } from "@budibase/types" function validate( schema: Joi.ObjectSchema | Joi.ArraySchema, - property: string + property: string, + opts: { errorPrefix: string } = { errorPrefix: `Invalid ${property}` } ) { // Return a Koa middleware function return (ctx: Ctx, next: any) => { @@ -29,16 +30,26 @@ function validate( const { error } = schema.validate(params) if (error) { - ctx.throw(400, `Invalid ${property} - ${error.message}`) + let message = error.message + if (opts.errorPrefix) { + message = `Invalid ${property} - ${message}` + } + ctx.throw(400, message) } return next() } } -export function body(schema: Joi.ObjectSchema | Joi.ArraySchema) { - return validate(schema, "body") +export function body( + schema: Joi.ObjectSchema | Joi.ArraySchema, + opts?: { errorPrefix: string } +) { + return validate(schema, "body", opts) } -export function params(schema: Joi.ObjectSchema | Joi.ArraySchema) { - return validate(schema, "params") +export function params( + schema: Joi.ObjectSchema | Joi.ArraySchema, + opts?: { errorPrefix: string } +) { + return validate(schema, "params", opts) } diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 0a4eff4247..9a726dc757 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -40,42 +40,49 @@ const validateViewSchemas: CustomValidator
= (table, helpers) => { } export function tableValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - _id: OPTIONAL_STRING, - _rev: OPTIONAL_STRING, - type: OPTIONAL_STRING.valid("table", "internal", "external"), - primaryDisplay: OPTIONAL_STRING, - schema: Joi.object().required(), - name: Joi.string().required(), - views: Joi.object(), - rows: Joi.array(), - }).custom(validateViewSchemas).unknown(true)) + return auth.joiValidator.body( + Joi.object({ + _id: OPTIONAL_STRING, + _rev: OPTIONAL_STRING, + type: OPTIONAL_STRING.valid("table", "internal", "external"), + primaryDisplay: OPTIONAL_STRING, + schema: Joi.object().required(), + name: Joi.string().required(), + views: Joi.object(), + rows: Joi.array(), + }) + .custom(validateViewSchemas) + .unknown(true), + { errorPrefix: "" } + ) } export function nameValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - name: OPTIONAL_STRING, - })) + return auth.joiValidator.body( + Joi.object({ + name: OPTIONAL_STRING, + }) + ) } export function datasourceValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - _id: Joi.string(), - _rev: Joi.string(), - type: OPTIONAL_STRING.allow("datasource_plus"), - relationships: Joi.array().items(Joi.object({ - from: Joi.string().required(), - to: Joi.string().required(), - cardinality: Joi.valid("1:N", "1:1", "N:N").required() - })), - }).unknown(true)) + return auth.joiValidator.body( + Joi.object({ + _id: Joi.string(), + _rev: Joi.string(), + type: OPTIONAL_STRING.allow("datasource_plus"), + relationships: Joi.array().items( + Joi.object({ + from: Joi.string().required(), + to: Joi.string().required(), + cardinality: Joi.valid("1:N", "1:1", "N:N").required(), + }) + ), + }).unknown(true) + ) } function filterObject() { - // prettier-ignore return Joi.object({ string: Joi.object().optional(), fuzzy: Joi.object().optional(), @@ -92,17 +99,20 @@ function filterObject() { } export function internalSearchValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - tableId: OPTIONAL_STRING, - query: filterObject(), - limit: OPTIONAL_NUMBER, - sort: OPTIONAL_STRING, - sortOrder: OPTIONAL_STRING, - sortType: OPTIONAL_STRING, - paginate: Joi.boolean(), - bookmark: Joi.alternatives().try(OPTIONAL_STRING, OPTIONAL_NUMBER).optional(), - })) + return auth.joiValidator.body( + Joi.object({ + tableId: OPTIONAL_STRING, + query: filterObject(), + limit: OPTIONAL_NUMBER, + sort: OPTIONAL_STRING, + sortOrder: OPTIONAL_STRING, + sortType: OPTIONAL_STRING, + paginate: Joi.boolean(), + bookmark: Joi.alternatives() + .try(OPTIONAL_STRING, OPTIONAL_NUMBER) + .optional(), + }) + ) } export function externalSearchValidator() { @@ -124,92 +134,110 @@ export function externalSearchValidator() { } export function datasourceQueryValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - endpoint: Joi.object({ - datasourceId: Joi.string().required(), - operation: Joi.string().required().valid(...Object.values(DataSourceOperation)), - entityId: Joi.string().required(), - }).required(), - resource: Joi.object({ - fields: Joi.array().items(Joi.string()).optional(), - }).optional(), - body: Joi.object().optional(), - sort: Joi.object().optional(), - filters: filterObject().optional(), - paginate: Joi.object({ - page: Joi.string().alphanum().optional(), - limit: Joi.number().optional(), - }).optional(), - })) + return auth.joiValidator.body( + Joi.object({ + endpoint: Joi.object({ + datasourceId: Joi.string().required(), + operation: Joi.string() + .required() + .valid(...Object.values(DataSourceOperation)), + entityId: Joi.string().required(), + }).required(), + resource: Joi.object({ + fields: Joi.array().items(Joi.string()).optional(), + }).optional(), + body: Joi.object().optional(), + sort: Joi.object().optional(), + filters: filterObject().optional(), + paginate: Joi.object({ + page: Joi.string().alphanum().optional(), + limit: Joi.number().optional(), + }).optional(), + }) + ) } export function webhookValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - live: Joi.bool(), - _id: OPTIONAL_STRING, - _rev: OPTIONAL_STRING, - name: Joi.string().required(), - bodySchema: Joi.object().optional(), - action: Joi.object({ - type: Joi.string().required().valid(WebhookActionType.AUTOMATION), - target: Joi.string().required(), - }).required(), - }).unknown(true)) + return auth.joiValidator.body( + Joi.object({ + live: Joi.bool(), + _id: OPTIONAL_STRING, + _rev: OPTIONAL_STRING, + name: Joi.string().required(), + bodySchema: Joi.object().optional(), + action: Joi.object({ + type: Joi.string().required().valid(WebhookActionType.AUTOMATION), + target: Joi.string().required(), + }).required(), + }).unknown(true) + ) } export function roleValidator() { const permLevelArray = Object.values(permissions.PermissionLevel) - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - _id: OPTIONAL_STRING, - _rev: OPTIONAL_STRING, - name: Joi.string().regex(/^[a-zA-Z0-9_]*$/).required(), - // this is the base permission ID (for now a built in) - permissionId: Joi.string().valid(...Object.values(permissions.BuiltinPermissionID)).required(), - permissions: Joi.object() - .pattern(/.*/, [Joi.string().valid(...permLevelArray)]) - .optional(), - inherits: OPTIONAL_STRING, - }).unknown(true)) + + return auth.joiValidator.body( + Joi.object({ + _id: OPTIONAL_STRING, + _rev: OPTIONAL_STRING, + name: Joi.string() + .regex(/^[a-zA-Z0-9_]*$/) + .required(), + // this is the base permission ID (for now a built in) + permissionId: Joi.string() + .valid(...Object.values(permissions.BuiltinPermissionID)) + .required(), + permissions: Joi.object() + .pattern(/.*/, [Joi.string().valid(...permLevelArray)]) + .optional(), + inherits: OPTIONAL_STRING, + }).unknown(true) + ) } export function permissionValidator() { const permLevelArray = Object.values(permissions.PermissionLevel) - // prettier-ignore - return auth.joiValidator.params(Joi.object({ - level: Joi.string().valid(...permLevelArray).required(), - resourceId: Joi.string(), - roleId: Joi.string(), - }).unknown(true)) + + return auth.joiValidator.params( + Joi.object({ + level: Joi.string() + .valid(...permLevelArray) + .required(), + resourceId: Joi.string(), + roleId: Joi.string(), + }).unknown(true) + ) } export function screenValidator() { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - name: Joi.string().required(), - showNavigation: OPTIONAL_BOOLEAN, - width: OPTIONAL_STRING, - routing: Joi.object({ - route: Joi.string().required(), - roleId: Joi.string().required().allow(""), - homeScreen: OPTIONAL_BOOLEAN, - }).required().unknown(true), - props: Joi.object({ - _id: Joi.string().required(), - _component: Joi.string().required(), - _children: Joi.array().required(), - _styles: Joi.object().required(), - type: OPTIONAL_STRING, - table: OPTIONAL_STRING, - layoutId: OPTIONAL_STRING, - }).required().unknown(true), - }).unknown(true)) + return auth.joiValidator.body( + Joi.object({ + name: Joi.string().required(), + showNavigation: OPTIONAL_BOOLEAN, + width: OPTIONAL_STRING, + routing: Joi.object({ + route: Joi.string().required(), + roleId: Joi.string().required().allow(""), + homeScreen: OPTIONAL_BOOLEAN, + }) + .required() + .unknown(true), + props: Joi.object({ + _id: Joi.string().required(), + _component: Joi.string().required(), + _children: Joi.array().required(), + _styles: Joi.object().required(), + type: OPTIONAL_STRING, + table: OPTIONAL_STRING, + layoutId: OPTIONAL_STRING, + }) + .required() + .unknown(true), + }).unknown(true) + ) } function generateStepSchema(allowStepTypes: string[]) { - // prettier-ignore return Joi.object({ stepId: Joi.string().required(), id: Joi.string().required(), @@ -219,33 +247,39 @@ function generateStepSchema(allowStepTypes: string[]) { icon: Joi.string().required(), params: Joi.object(), args: Joi.object(), - type: Joi.string().required().valid(...allowStepTypes), + type: Joi.string() + .required() + .valid(...allowStepTypes), }).unknown(true) } export function automationValidator(existing = false) { - // prettier-ignore - return auth.joiValidator.body(Joi.object({ - _id: existing ? Joi.string().required() : OPTIONAL_STRING, - _rev: existing ? Joi.string().required() : OPTIONAL_STRING, - name: Joi.string().required(), - type: Joi.string().valid("automation").required(), - definition: Joi.object({ - steps: Joi.array().required().items(generateStepSchema(["ACTION", "LOGIC"])), - trigger: generateStepSchema(["TRIGGER"]).allow(null), - }).required().unknown(true), - }).unknown(true)) + return auth.joiValidator.body( + Joi.object({ + _id: existing ? Joi.string().required() : OPTIONAL_STRING, + _rev: existing ? Joi.string().required() : OPTIONAL_STRING, + name: Joi.string().required(), + type: Joi.string().valid("automation").required(), + definition: Joi.object({ + steps: Joi.array() + .required() + .items(generateStepSchema(["ACTION", "LOGIC"])), + trigger: generateStepSchema(["TRIGGER"]).allow(null), + }) + .required() + .unknown(true), + }).unknown(true) + ) } export function applicationValidator(opts = { isCreate: true }) { - // prettier-ignore const base: any = { _id: OPTIONAL_STRING, _rev: OPTIONAL_STRING, url: OPTIONAL_STRING, template: Joi.object({ templateString: OPTIONAL_STRING, - }) + }), } const appNameValidator = Joi.string() From 819cc6bebb8c2db492c2bbfe8641200bf8dc5b47 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 11:18:33 +0200 Subject: [PATCH 22/24] Fix tests --- packages/server/src/api/routes/tests/viewV2.spec.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 663cf5c864..3376cb86fa 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1424,8 +1424,7 @@ describe.each([ status: 400, body: { status: 400, - message: - 'Invalid body - Required field "name" is missing in view "view a"', + message: 'Required field "name" is missing in view "view a"', }, } ) @@ -1455,8 +1454,7 @@ describe.each([ status: 400, body: { status: 400, - message: - 'Invalid body - Required field "name" is missing in view "view a"', + message: 'Required field "name" is missing in view "view a"', }, } ) From aefedce568168d116e9046b12278cfca1a4f69fa Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 11:35:09 +0200 Subject: [PATCH 23/24] Renames --- packages/server/src/api/routes/tests/viewV2.spec.ts | 6 ++++-- packages/server/src/api/routes/utils/validators.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 3376cb86fa..cf9db761ce 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1424,7 +1424,8 @@ describe.each([ status: 400, body: { status: 400, - message: 'Required field "name" is missing in view "view a"', + message: + 'To make field "name" required, this field must be present and writable in views: view a.', }, } ) @@ -1454,7 +1455,8 @@ describe.each([ status: 400, body: { status: 400, - message: 'Required field "name" is missing in view "view a"', + message: + 'To make field "name" required, this field must be present and writable in views: view a.', }, } ) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 9a726dc757..e2cc463f38 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -30,7 +30,7 @@ const validateViewSchemas: CustomValidator
= (table, helpers) => { ) if (missingField) { return helpers.message({ - custom: `Required field "${missingField}" is missing in view "${view.name}"`, + custom: `To make field "${missingField}" required, this field must be present and writable in views: ${view.name}.`, }) } } From 1c8feaedb1da457c5593732025467604de810367 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 12:03:42 +0200 Subject: [PATCH 24/24] Copy change --- packages/server/src/api/routes/tests/viewV2.spec.ts | 2 +- packages/server/src/sdk/app/views/index.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index b0ffd1e85c..650c36794b 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -369,7 +369,7 @@ describe.each([ status: 400, body: { message: - 'You can\'t make field "name" readonly because it is a required field.', + 'You can\'t make "name" readonly because it is a required field.', status: 400, }, }) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 52ab323faa..f82ef133d7 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -77,14 +77,14 @@ async function guardViewSchema( if (!viewSchemaField?.visible) { throw new HTTPError( - `You can't hide the required field "${field.name}"`, + `You can't hide "${field.name} because it is a required field."`, 400 ) } if (viewSchemaField.readonly) { throw new HTTPError( - `You can't make field "${field.name}" readonly because it is a required field.`, + `You can't make "${field.name}" readonly because it is a required field.`, 400 ) }