From ba002f96492fd311cb0396262b4f77c2edb6f8f5 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 2 Feb 2024 09:30:33 +0000 Subject: [PATCH 01/35] Clean up isolates when a request is finished. --- packages/backend-core/src/context/types.ts | 1 + packages/server/src/jsRunner/index.ts | 8 ++++++++ packages/server/src/middleware/cleanup.ts | 16 ++++++++++++++++ 3 files changed, 25 insertions(+) create mode 100644 packages/server/src/middleware/cleanup.ts diff --git a/packages/backend-core/src/context/types.ts b/packages/backend-core/src/context/types.ts index cc052ca505..dad1af3bf8 100644 --- a/packages/backend-core/src/context/types.ts +++ b/packages/backend-core/src/context/types.ts @@ -15,4 +15,5 @@ export type ContextMap = { jsContext: Context helpersModule: Module } + cleanup?: (() => void | Promise)[] } diff --git a/packages/server/src/jsRunner/index.ts b/packages/server/src/jsRunner/index.ts index 8e529d533d..9441a74d07 100644 --- a/packages/server/src/jsRunner/index.ts +++ b/packages/server/src/jsRunner/index.ts @@ -115,6 +115,14 @@ export function init() { } bbCtx.isolateRefs = { jsContext, jsIsolate, helpersModule } + if (!bbCtx.cleanup) { + bbCtx.cleanup = [] + } + bbCtx.cleanup.push(() => { + helpersModule.release() + jsContext.release() + jsIsolate.dispose() + }) } let { jsIsolate, jsContext, helpersModule } = bbCtx.isolateRefs! diff --git a/packages/server/src/middleware/cleanup.ts b/packages/server/src/middleware/cleanup.ts new file mode 100644 index 0000000000..5204d5cfb1 --- /dev/null +++ b/packages/server/src/middleware/cleanup.ts @@ -0,0 +1,16 @@ +import { Ctx } from "@budibase/types" +import { context } from "@budibase/backend-core" + +export default async (ctx: Ctx, next: any) => { + const resp = next() + + const current = context.getCurrentContext() + if (current?.cleanup) { + for (let fn of current.cleanup || []) { + await fn() + } + delete current.cleanup + } + + return resp +} From 21dfbe75ffa19c4be85d4a8fb785f028a47a8aa3 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 2 Feb 2024 09:32:07 +0000 Subject: [PATCH 02/35] Use new cleanup middleware. --- packages/server/src/api/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/server/src/api/index.ts b/packages/server/src/api/index.ts index ad3d8307da..92cee95ea6 100644 --- a/packages/server/src/api/index.ts +++ b/packages/server/src/api/index.ts @@ -1,6 +1,7 @@ import Router from "@koa/router" import { auth, middleware, env as envCore } from "@budibase/backend-core" import currentApp from "../middleware/currentapp" +import cleanup from "../middleware/cleanup" import zlib from "zlib" import { mainRoutes, staticRoutes, publicRoutes } from "./routes" import { middleware as pro } from "@budibase/pro" @@ -62,6 +63,8 @@ if (apiEnabled()) { .use(auth.auditLog) // @ts-ignore .use(migrations) + // @ts-ignore + .use(cleanup) // authenticated routes for (let route of mainRoutes) { From a3efab01bf6ffb2b5af71486732f39dcc8f0ac79 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 2 Feb 2024 14:57:05 +0000 Subject: [PATCH 03/35] Update packages/server/src/middleware/cleanup.ts Co-authored-by: Adria Navarro --- packages/server/src/middleware/cleanup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/middleware/cleanup.ts b/packages/server/src/middleware/cleanup.ts index 5204d5cfb1..d59317feed 100644 --- a/packages/server/src/middleware/cleanup.ts +++ b/packages/server/src/middleware/cleanup.ts @@ -2,7 +2,7 @@ import { Ctx } from "@budibase/types" import { context } from "@budibase/backend-core" export default async (ctx: Ctx, next: any) => { - const resp = next() + const resp = await next() const current = context.getCurrentContext() if (current?.cleanup) { From b8b12ff9393a88d4f012e990d6093c7141c3d9b4 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 21 Feb 2024 15:26:26 +0000 Subject: [PATCH 04/35] Respond to PR feedback. --- packages/account-portal | 2 +- packages/pro | 2 +- packages/server/src/jsRunner/index.ts | 29 +++++++++++++---------- packages/server/src/middleware/cleanup.ts | 22 +++++++++++++---- 4 files changed, 37 insertions(+), 18 deletions(-) diff --git a/packages/account-portal b/packages/account-portal index 52f51dcfb9..4de0d98e2f 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 52f51dcfb96d3fe58c8cc7a905e7d733f7cd84c2 +Subproject commit 4de0d98e2f8d80ee7631dffe076063273812a441 diff --git a/packages/pro b/packages/pro index 4f9616f163..60e47a8249 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 4f9616f163039a0eea81319d8e2288340a2ebc79 +Subproject commit 60e47a8249fd6291a6bc20fe3fe6776b11938fa1 diff --git a/packages/server/src/jsRunner/index.ts b/packages/server/src/jsRunner/index.ts index f6a7f07cd1..3a2c0ac1a2 100644 --- a/packages/server/src/jsRunner/index.ts +++ b/packages/server/src/jsRunner/index.ts @@ -9,6 +9,7 @@ import { context, logging } from "@budibase/backend-core" import tracer from "dd-trace" import { IsolatedVM } from "./vm" +import type { VM } from "@budibase/types" export function init() { setJSRunner((js: string, ctx: Record) => { @@ -16,22 +17,26 @@ export function init() { try { const bbCtx = context.getCurrentContext() - const vm = bbCtx?.vm - ? bbCtx.vm - : new IsolatedVM({ - memoryLimit: env.JS_RUNNER_MEMORY_LIMIT, - invocationTimeout: env.JS_PER_INVOCATION_TIMEOUT_MS, - isolateAccumulatedTimeout: env.JS_PER_REQUEST_TIMEOUT_MS, - }).withHelpers() + let vm: VM + if (bbCtx && bbCtx.vm) { + vm = bbCtx.vm + } else { + vm = new IsolatedVM({ + memoryLimit: env.JS_RUNNER_MEMORY_LIMIT, + invocationTimeout: env.JS_PER_INVOCATION_TIMEOUT_MS, + isolateAccumulatedTimeout: env.JS_PER_REQUEST_TIMEOUT_MS, + }).withHelpers() + } - if (bbCtx) { - // If we have a context, we want to persist it to reuse the isolate + if (bbCtx && !bbCtx.vm) { bbCtx.vm = vm bbCtx.cleanup = bbCtx.cleanup || [] - bbCtx.cleanup.push(() => { - vm.close() - }) + bbCtx.cleanup.push(() => vm.close()) } + + // Because we can't pass functions into an Isolate, we remove them from + // the passed context and rely on the withHelpers() method to add them + // back in. const { helpers, ...rest } = ctx return vm.withContext(rest, () => vm.execute(js)) } catch (error: any) { diff --git a/packages/server/src/middleware/cleanup.ts b/packages/server/src/middleware/cleanup.ts index 5204d5cfb1..a810879a65 100644 --- a/packages/server/src/middleware/cleanup.ts +++ b/packages/server/src/middleware/cleanup.ts @@ -2,14 +2,28 @@ import { Ctx } from "@budibase/types" import { context } from "@budibase/backend-core" export default async (ctx: Ctx, next: any) => { - const resp = next() + const resp = await next() const current = context.getCurrentContext() - if (current?.cleanup) { - for (let fn of current.cleanup || []) { + if (!current || !current.cleanup) { + return resp + } + + let errors = [] + for (let fn of current.cleanup) { + try { await fn() + } catch (e) { + // We catch errors here to ensure we at least attempt to run all cleanup + // functions. We'll throw the first error we encounter after all cleanup + // functions have been run. + errors.push(e) } - delete current.cleanup + } + delete current.cleanup + + if (errors.length > 0) { + throw errors[0] } return resp From 6157e1becf9d89a6f5c525466d861b709eaf4c88 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 6 Mar 2024 14:55:59 +0000 Subject: [PATCH 05/35] Update pro submodule. --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 60e47a8249..22a278da72 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 60e47a8249fd6291a6bc20fe3fe6776b11938fa1 +Subproject commit 22a278da720d92991dabdcd4cb6c96e7abe29781 From ce599e775f001342526ead7c0df0c00d2b93abfd Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 7 Mar 2024 14:56:30 +0000 Subject: [PATCH 06/35] Add APM spans for request cleanup functions. --- packages/server/src/middleware/cleanup.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/middleware/cleanup.ts b/packages/server/src/middleware/cleanup.ts index a810879a65..43f945ab6b 100644 --- a/packages/server/src/middleware/cleanup.ts +++ b/packages/server/src/middleware/cleanup.ts @@ -1,5 +1,6 @@ import { Ctx } from "@budibase/types" import { context } from "@budibase/backend-core" +import { tracer } from "dd-trace" export default async (ctx: Ctx, next: any) => { const resp = await next() @@ -12,7 +13,9 @@ export default async (ctx: Ctx, next: any) => { let errors = [] for (let fn of current.cleanup) { try { - await fn() + await tracer.trace("cleanup", async span => { + await fn() + }) } catch (e) { // We catch errors here to ensure we at least attempt to run all cleanup // functions. We'll throw the first error we encounter after all cleanup From 9b91e47220304fc7e17b53ac842b8c2e3e710bd2 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 7 Mar 2024 15:01:38 +0000 Subject: [PATCH 07/35] Respond to Adri's feedback. --- packages/server/src/jsRunner/index.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/server/src/jsRunner/index.ts b/packages/server/src/jsRunner/index.ts index 61821c0d0e..19bf0fa6b5 100644 --- a/packages/server/src/jsRunner/index.ts +++ b/packages/server/src/jsRunner/index.ts @@ -16,16 +16,13 @@ export function init() { try { const bbCtx = context.getCurrentContext() - let vm: VM - if (bbCtx && bbCtx.vm) { - vm = bbCtx.vm - } else { - vm = new IsolatedVM({ + const vm = + bbCtx?.vm || + new IsolatedVM({ memoryLimit: env.JS_RUNNER_MEMORY_LIMIT, invocationTimeout: env.JS_PER_INVOCATION_TIMEOUT_MS, isolateAccumulatedTimeout: env.JS_PER_REQUEST_TIMEOUT_MS, }).withHelpers() - } if (bbCtx && !bbCtx.vm) { bbCtx.vm = vm From 4b38b5263b5028f728d953b0dfa200d96ee2126e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 11 Mar 2024 13:14:02 +0100 Subject: [PATCH 08/35] Allow group members edits to admins --- .../portal/users/users/[userId].svelte | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/builder/src/pages/builder/portal/users/users/[userId].svelte b/packages/builder/src/pages/builder/portal/users/users/[userId].svelte index 1d15b13107..eb0305e959 100644 --- a/packages/builder/src/pages/builder/portal/users/users/[userId].svelte +++ b/packages/builder/src/pages/builder/portal/users/users/[userId].svelte @@ -39,9 +39,10 @@ name: { width: "1fr", }, - ...(readonly + ...(!isAdmin ? {} - : { + : // Add + { _id: { displayName: "", width: "auto", @@ -90,7 +91,9 @@ $: internalGroups = $groups?.filter(g => !g?.scimInfo?.isSync) $: isSSO = !!user?.provider - $: readonly = !sdk.users.isAdmin($auth.user) || user?.scimInfo?.isSync + $: isAdmin = sdk.users.isAdmin($auth.user) + $: isScim = user?.scimInfo?.isSync + $: readonly = !isAdmin || isScim $: privileged = sdk.users.isAdminOrGlobalBuilder(user) $: nameLabel = getNameLabel(user) $: filteredGroups = getFilteredGroups(internalGroups, searchTerm) @@ -322,23 +325,23 @@
Groups - {#if internalGroups?.length} + {#if internalGroups?.length && isAdmin}
+ + addGroup(e.detail)} + on:deselect={e => removeGroup(e.detail)} + iconComponent={GroupIcon} + extractIconProps={item => ({ group: item, size: "S" })} + /> + {/if} - - addGroup(e.detail)} - on:deselect={e => removeGroup(e.detail)} - iconComponent={GroupIcon} - extractIconProps={item => ({ group: item, size: "S" })} - /> -
Date: Mon, 11 Mar 2024 15:16:07 +0100 Subject: [PATCH 09/35] Update pro ref --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 4e66a0f704..71b8e60f7c 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 4e66a0f7042652763c238b10367310b168905f87 +Subproject commit 71b8e60f7c4c80e9711569416450ab8f4a7fa7d1 From 4f5eb6110ac8dedd94efad5516d51750adb0c134 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 11 Mar 2024 16:46:33 +0100 Subject: [PATCH 10/35] Update pro ref --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 71b8e60f7c..e565db07f6 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 71b8e60f7c4c80e9711569416450ab8f4a7fa7d1 +Subproject commit e565db07f6c51868087e88dfebde0328493443e6 From c34fa61479f281e4d21d7438d482135bf23caf4a Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Mon, 11 Mar 2024 16:16:26 +0000 Subject: [PATCH 11/35] Bump version to 2.21.7 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 57e3a7b34e..f881568106 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.21.6", + "version": "2.21.7", "npmClient": "yarn", "packages": [ "packages/*", From 6f2f5fd5ce8eb0233dbc24ed71f4cc5ca7495427 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 08:38:47 +0100 Subject: [PATCH 12/35] Display AD message correctly for builders --- .../src/pages/builder/portal/users/groups/[groupId].svelte | 2 +- .../portal/users/groups/_components/GroupUsers.svelte | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte b/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte index d0eb50c8dd..632419e979 100644 --- a/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte +++ b/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte @@ -139,7 +139,7 @@ - + diff --git a/packages/builder/src/pages/builder/portal/users/groups/_components/GroupUsers.svelte b/packages/builder/src/pages/builder/portal/users/groups/_components/GroupUsers.svelte index 438feecc07..9f6703964a 100644 --- a/packages/builder/src/pages/builder/portal/users/groups/_components/GroupUsers.svelte +++ b/packages/builder/src/pages/builder/portal/users/groups/_components/GroupUsers.svelte @@ -13,6 +13,7 @@ export let groupId export let readonly + export let isScimGroup let emailSearch let fetchGroupUsers @@ -61,10 +62,10 @@
- {#if !readonly} - - {:else} + {#if isScimGroup} + {:else if !readonly} + {/if}
From 75df04fbda4c6f38269fc202cb5fcd89d331196c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 08:40:16 +0100 Subject: [PATCH 13/35] Fix group edition display for builders --- .../builder/portal/users/groups/[groupId].svelte | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte b/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte index 632419e979..5cb88149d5 100644 --- a/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte +++ b/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte @@ -49,7 +49,8 @@ $: group = $groups.find(x => x._id === groupId) $: isScimGroup = group?.scimInfo?.isSync - $: readonly = !sdk.users.isAdmin($auth.user) || isScimGroup + $: isAdmin = sdk.users.isAdmin($auth.user) + $: readonly = !isAdmin || isScimGroup $: groupApps = $apps .filter(app => groups.actions @@ -123,14 +124,18 @@ - editModal.show()}> + editModal.show()} + disabled={!isAdmin} + > Edit
deleteModal.show()} - disabled={isScimGroup} + disabled={readonly} > Delete From 4b2c16998ca12004731a1e00493dde669f54ea89 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 08:51:01 +0100 Subject: [PATCH 14/35] Fix SCIM groups edition --- packages/builder/src/stores/portal/groups.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/builder/src/stores/portal/groups.js b/packages/builder/src/stores/portal/groups.js index 34fe0f9231..2c2a4e14d0 100644 --- a/packages/builder/src/stores/portal/groups.js +++ b/packages/builder/src/stores/portal/groups.js @@ -35,7 +35,7 @@ export function createGroupsStore() { get: getGroup, save: async group => { - const { _scimInfo, ...dataToSave } = group + const { scimInfo: _, ...dataToSave } = group const response = await API.saveGroup(dataToSave) group._id = response._id group._rev = response._rev From e754c660ee39e220bf45bc3855fd1e74fa21d5c8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 08:56:36 +0100 Subject: [PATCH 15/35] Fix edition of groups with groups --- packages/builder/src/stores/portal/groups.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/builder/src/stores/portal/groups.js b/packages/builder/src/stores/portal/groups.js index 2c2a4e14d0..1edc8a461c 100644 --- a/packages/builder/src/stores/portal/groups.js +++ b/packages/builder/src/stores/portal/groups.js @@ -35,7 +35,9 @@ export function createGroupsStore() { get: getGroup, save: async group => { - const { scimInfo: _, ...dataToSave } = group + const { ...dataToSave } = group + delete dataToSave.scimInfo + delete dataToSave.userGroups const response = await API.saveGroup(dataToSave) group._id = response._id group._rev = response._rev From b2000c0805aea14ea6def1b2f5eeac1018a2da98 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 08:56:56 +0100 Subject: [PATCH 16/35] Lint test --- .../src/api/routes/global/tests/groups.spec.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/groups.spec.ts b/packages/worker/src/api/routes/global/tests/groups.spec.ts index b69c4781c4..dd39185181 100644 --- a/packages/worker/src/api/routes/global/tests/groups.spec.ts +++ b/packages/worker/src/api/routes/global/tests/groups.spec.ts @@ -103,18 +103,15 @@ describe("/api/global/groups", () => { expect(events.group.updated).toBeCalledTimes(1) expect(events.group.permissionsEdited).not.toBeCalled() }) + }) - describe("destroy", () => { - it("should be able to delete a basic group", async () => { - const group = structures.groups.UserGroup() - let oldGroup = await config.api.groups.saveGroup(group) - await config.api.groups.deleteGroup( - oldGroup.body._id, - oldGroup.body._rev - ) + describe("destroy", () => { + it("should be able to delete a basic group", async () => { + const group = structures.groups.UserGroup() + let oldGroup = await config.api.groups.saveGroup(group) + await config.api.groups.deleteGroup(oldGroup.body._id, oldGroup.body._rev) - expect(events.group.deleted).toBeCalledTimes(1) - }) + expect(events.group.deleted).toBeCalledTimes(1) }) }) From cd0004ec3dc31b5ce97cbad134c94aeff8cb205e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 09:46:10 +0100 Subject: [PATCH 17/35] Add scim tests --- packages/pro | 2 +- .../api/routes/global/tests/groups.spec.ts | 65 +++++++++++++++++++ packages/worker/src/tests/api/groups.ts | 13 +++- 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/packages/pro b/packages/pro index e565db07f6..7accd0cb0b 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit e565db07f6c51868087e88dfebde0328493443e6 +Subproject commit 7accd0cb0b21258e9085568143dbf885f9f87afe diff --git a/packages/worker/src/api/routes/global/tests/groups.spec.ts b/packages/worker/src/api/routes/global/tests/groups.spec.ts index dd39185181..52df4fa9fd 100644 --- a/packages/worker/src/api/routes/global/tests/groups.spec.ts +++ b/packages/worker/src/api/routes/global/tests/groups.spec.ts @@ -103,6 +103,71 @@ describe("/api/global/groups", () => { expect(events.group.updated).toBeCalledTimes(1) expect(events.group.permissionsEdited).not.toBeCalled() }) + + describe("scim", () => { + async function createScimGroup() { + mocks.licenses.useScimIntegration() + await config.setSCIMConfig(true) + + const scimGroup = await config.api.scimGroupsAPI.post({ + body: structures.scim.createGroupRequest({ + displayName: generator.word(), + }), + }) + + const { body: group } = await config.api.groups.find(scimGroup.id) + + expect(group).toBeDefined() + return group + } + + it("update will not allow sending SCIM fields", async () => { + const group = await createScimGroup() + + const updatedGroup: UserGroup = { + ...group, + name: generator.word(), + } + await config.api.groups.saveGroup(updatedGroup, { + expect: { + message: 'Invalid body - "scimInfo" is not allowed', + status: 400, + }, + }) + + expect(events.group.updated).not.toBeCalled() + }) + + it("update will not amend the SCIM fields", async () => { + const group: UserGroup = await createScimGroup() + + const updatedGroup: UserGroup = { + ...group, + name: generator.word(), + scimInfo: undefined, + } + + await config.api.groups.saveGroup(updatedGroup, { + expect: 200, + }) + + expect(events.group.updated).toBeCalledTimes(1) + expect( + ( + await config.api.groups.find(group._id!, { + expect: 200, + }) + ).body + ).toEqual( + expect.objectContaining({ + ...group, + name: updatedGroup.name, + scimInfo: group.scimInfo, + _rev: expect.any(String), + }) + ) + }) + }) }) describe("destroy", () => { diff --git a/packages/worker/src/tests/api/groups.ts b/packages/worker/src/tests/api/groups.ts index 0b9081cc92..cd95773e87 100644 --- a/packages/worker/src/tests/api/groups.ts +++ b/packages/worker/src/tests/api/groups.ts @@ -7,7 +7,10 @@ export class GroupsAPI extends TestAPI { super(config) } - saveGroup = (group: UserGroup, { expect } = { expect: 200 }) => { + saveGroup = ( + group: UserGroup, + { expect }: { expect: number | object } = { expect: 200 } + ) => { return this.request .post(`/api/global/groups`) .send(group) @@ -61,4 +64,12 @@ export class GroupsAPI extends TestAPI { .expect("Content-Type", /json/) .expect(expect) } + + find = (id: string, { expect } = { expect: 200 }) => { + return this.request + .get(`/api/global/groups/${id}`) + .set(this.config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(expect) + } } From 26c98ea084f61d886694dedafd8e84650ad82231 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 09:57:25 +0100 Subject: [PATCH 18/35] Fix tests --- .../src/api/routes/global/tests/groups.spec.ts | 14 +++++++++----- packages/worker/src/tests/api/groups.ts | 5 +++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/groups.spec.ts b/packages/worker/src/api/routes/global/tests/groups.spec.ts index 52df4fa9fd..0715c3a7bb 100644 --- a/packages/worker/src/api/routes/global/tests/groups.spec.ts +++ b/packages/worker/src/api/routes/global/tests/groups.spec.ts @@ -319,12 +319,16 @@ describe("/api/global/groups", () => { }) }) - it("update should return 200", async () => { + it("update should return forbidden", async () => { await config.withUser(builder, async () => { - await config.api.groups.updateGroupUsers(group._id!, { - add: [builder._id!], - remove: [], - }) + await config.api.groups.updateGroupUsers( + group._id!, + { + add: [builder._id!], + remove: [], + }, + { expect: 403 } + ) }) }) }) diff --git a/packages/worker/src/tests/api/groups.ts b/packages/worker/src/tests/api/groups.ts index cd95773e87..5153c19db0 100644 --- a/packages/worker/src/tests/api/groups.ts +++ b/packages/worker/src/tests/api/groups.ts @@ -47,14 +47,15 @@ export class GroupsAPI extends TestAPI { updateGroupUsers = ( id: string, - body: { add: string[]; remove: string[] } + body: { add: string[]; remove: string[] }, + { expect } = { expect: 200 } ) => { return this.request .post(`/api/global/groups/${id}/users`) .send(body) .set(this.config.defaultHeaders()) .expect("Content-Type", /json/) - .expect(200) + .expect(expect) } fetch = ({ expect } = { expect: 200 }) => { From 3efaf01684d520d2f38c9c82cd73cfb41cd583f8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 10:02:39 +0100 Subject: [PATCH 19/35] Fix multiple runs --- packages/worker/src/api/routes/global/tests/groups.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/worker/src/api/routes/global/tests/groups.spec.ts b/packages/worker/src/api/routes/global/tests/groups.spec.ts index 0715c3a7bb..414518d763 100644 --- a/packages/worker/src/api/routes/global/tests/groups.spec.ts +++ b/packages/worker/src/api/routes/global/tests/groups.spec.ts @@ -209,7 +209,7 @@ describe("/api/global/groups", () => { await Promise.all( Array.from({ length: 30 }).map(async (_, i) => { - const email = `user${i}@example.com` + const email = `user${i}+${generator.guid()}@example.com` const user = await config.api.users.saveUser({ ...structures.users.user(), email, From 106a71b6476e5f3cb601f2f88e07fd076464ca7c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 10:15:58 +0100 Subject: [PATCH 20/35] Update pro ref --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 7accd0cb0b..c4c98ae70f 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 7accd0cb0b21258e9085568143dbf885f9f87afe +Subproject commit c4c98ae70f2e936009250893898ecf11f4ddf2c3 From ae83f637b36acaac4a8b0c036cb2a20fd7e71a42 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 12 Mar 2024 09:35:05 +0000 Subject: [PATCH 21/35] Bump version to 2.21.8 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index f881568106..b845465de5 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.21.7", + "version": "2.21.8", "npmClient": "yarn", "packages": [ "packages/*", From 186f916b4084a0d99811d141b9533bf203421c72 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 12 Mar 2024 09:57:59 +0000 Subject: [PATCH 22/35] Get tests passing against a real MySQL. --- packages/server/src/api/routes/tests/row.spec.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index a032f4324c..5051fb1a5b 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -38,11 +38,16 @@ import * as uuid from "uuid" const timestamp = new Date("2023-01-26T11:48:57.597Z").toISOString() tk.freeze(timestamp) +jest.setTimeout(99999999) +jest.unmock("mysql2") +jest.unmock("mysql2/promise") + const { basicRow } = setup.structures describe.each([ - ["internal", undefined], - ["postgres", databaseTestProviders.postgres], + // ["internal", undefined], + // ["postgres", databaseTestProviders.postgres], + ["mysql", databaseTestProviders.mysql], ])("/rows (%s)", (__, dsProvider) => { const isInternal = !dsProvider @@ -70,7 +75,7 @@ describe.each([ const generateTableConfig: () => SaveTableRequest = () => { return { - name: uuid.v4(), + name: uuid.v4().substring(0, 16), type: "table", primary: ["id"], primaryDisplay: "name", @@ -641,7 +646,7 @@ describe.each([ const createdRow = await config.createRow() const res = await config.api.row.bulkDelete(table._id!, { - rows: [createdRow, { _id: "2" }], + rows: [createdRow, { _id: "9999999" }], }) expect(res[0]._id).toEqual(createdRow._id) From 3f302d300ec19759513fe1ecbcdb82ac6d4be5f1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 11:09:16 +0100 Subject: [PATCH 23/35] Add test, account holder cannot be removed --- .../src/api/routes/global/tests/scim.spec.ts | 20 +++++++++++++++++++ packages/worker/src/tests/api/scim/shared.ts | 8 ++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/scim.spec.ts b/packages/worker/src/api/routes/global/tests/scim.spec.ts index 3d5a884579..ad43f089db 100644 --- a/packages/worker/src/api/routes/global/tests/scim.spec.ts +++ b/packages/worker/src/api/routes/global/tests/scim.spec.ts @@ -2,6 +2,7 @@ import tk from "timekeeper" import _ from "lodash" import { generator, mocks, structures } from "@budibase/backend-core/tests" import { + CloudAccount, ScimCreateUserRequest, ScimGroupResponse, ScimUpdateRequest, @@ -604,6 +605,25 @@ describe("scim", () => { expect(events.user.deleted).toBeCalledTimes(1) }) + + it("an account holder cannot be removed even when synched", async () => { + const account: CloudAccount = { + ...structures.accounts.account(), + budibaseUserId: user.id, + email: user.emails![0].value, + } + mocks.accounts.getAccount.mockResolvedValue(account) + + await deleteScimUser(user.id, { + expect: { + message: "Account holder cannot be deleted", + status: 400, + error: { code: "http" }, + }, + }) + + await config.api.scimUsersAPI.find(user.id, { expect: 200 }) + }) }) }) diff --git a/packages/worker/src/tests/api/scim/shared.ts b/packages/worker/src/tests/api/scim/shared.ts index 1b064b8f41..546a940093 100644 --- a/packages/worker/src/tests/api/scim/shared.ts +++ b/packages/worker/src/tests/api/scim/shared.ts @@ -1,13 +1,17 @@ import TestConfiguration from "../../TestConfiguration" import { TestAPI } from "../base" -const defaultConfig = { +const defaultConfig: RequestSettings = { expect: 200, setHeaders: true, skipContentTypeCheck: false, } -export type RequestSettings = typeof defaultConfig +export type RequestSettings = { + expect: number | object + setHeaders: boolean + skipContentTypeCheck: boolean +} export abstract class ScimTestAPI extends TestAPI { constructor(config: TestConfiguration) { From ce209a16b3f85fbd6151b2a38cfa9e7976378be0 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 12 Mar 2024 10:42:42 +0000 Subject: [PATCH 24/35] Get tests running with SQL Server. Need to make them pass next. --- .../server/src/api/routes/tests/row.spec.ts | 4 +- .../src/integrations/tests/utils/index.ts | 3 +- .../src/integrations/tests/utils/mssql.ts | 53 +++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 packages/server/src/integrations/tests/utils/mssql.ts diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 5051fb1a5b..bf96dd62b5 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -41,13 +41,15 @@ tk.freeze(timestamp) jest.setTimeout(99999999) jest.unmock("mysql2") jest.unmock("mysql2/promise") +jest.unmock("mssql") const { basicRow } = setup.structures describe.each([ // ["internal", undefined], // ["postgres", databaseTestProviders.postgres], - ["mysql", databaseTestProviders.mysql], + // ["mysql", databaseTestProviders.mysql], + ["mssql", databaseTestProviders.mssql], ])("/rows (%s)", (__, dsProvider) => { const isInternal = !dsProvider diff --git a/packages/server/src/integrations/tests/utils/index.ts b/packages/server/src/integrations/tests/utils/index.ts index b6e4e43e7a..a5282bff6f 100644 --- a/packages/server/src/integrations/tests/utils/index.ts +++ b/packages/server/src/integrations/tests/utils/index.ts @@ -4,6 +4,7 @@ import { Datasource } from "@budibase/types" import * as postgres from "./postgres" import * as mongodb from "./mongodb" import * as mysql from "./mysql" +import * as mssql from "./mssql" import { StartedTestContainer } from "testcontainers" jest.setTimeout(30000) @@ -14,4 +15,4 @@ export interface DatabaseProvider { datasource(): Promise } -export const databaseTestProviders = { postgres, mongodb, mysql } +export const databaseTestProviders = { postgres, mongodb, mysql, mssql } diff --git a/packages/server/src/integrations/tests/utils/mssql.ts b/packages/server/src/integrations/tests/utils/mssql.ts new file mode 100644 index 0000000000..f548f0c42c --- /dev/null +++ b/packages/server/src/integrations/tests/utils/mssql.ts @@ -0,0 +1,53 @@ +import { Datasource, SourceName } from "@budibase/types" +import { GenericContainer, Wait, StartedTestContainer } from "testcontainers" + +let container: StartedTestContainer | undefined + +export async function start(): Promise { + return await new GenericContainer( + "mcr.microsoft.com/mssql/server:2022-latest" + ) + .withExposedPorts(1433) + .withEnvironment({ + ACCEPT_EULA: "Y", + MSSQL_SA_PASSWORD: "Password_123", + // This is important, as Microsoft allow us to use the "Developer" edition + // of SQL Server for development and testing purposes. We can't use other + // versions without a valid license, and we cannot use the Developer + // version in production. + MSSQL_PID: "Developer", + }) + .withWaitStrategy( + Wait.forSuccessfulCommand( + "/opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P Password_123 -q 'SELECT 1'" + ) + ) + .start() +} + +export async function datasource(): Promise { + if (!container) { + container = await start() + } + const host = container.getHost() + const port = container.getMappedPort(1433) + + return { + type: "datasource_plus", + source: SourceName.SQL_SERVER, + plus: true, + config: { + server: host, + port, + user: "sa", + password: "Password_123", + }, + } +} + +export async function stop() { + if (container) { + await container.stop() + container = undefined + } +} From d1f876d67f35541bc88d4fd034cdaf89b94893d9 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 12 Mar 2024 10:55:58 +0000 Subject: [PATCH 25/35] Fix test that was failing because SQL Server doesn't allow you to insert values into primary key columns unless you set a setting. --- packages/server/src/api/routes/tests/row.spec.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index bf96dd62b5..cd8580c64b 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -438,7 +438,7 @@ describe.each([ }) describe("view save", () => { - it("views have extra data trimmed", async () => { + it.only("views have extra data trimmed", async () => { const table = await createTable({ type: "table", name: "orders", @@ -474,7 +474,6 @@ describe.each([ const createRowResponse = await config.api.row.save( createViewResponse.id, { - OrderID: "1111", Country: "Aussy", Story: "aaaaa", } @@ -484,7 +483,7 @@ describe.each([ expect(row.Story).toBeUndefined() expect(row).toEqual({ ...defaultRowFields, - OrderID: 1111, + OrderID: createRowResponse.OrderID, Country: "Aussy", _id: createRowResponse._id, _rev: createRowResponse._rev, From 477d17b53ecd2f904b470fb93b09631734a71610 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 12 Mar 2024 12:25:30 +0000 Subject: [PATCH 26/35] Making progress on getting SQL Server working. --- .../server/src/api/routes/tests/row.spec.ts | 64 +++++++++---------- .../src/integrations/microsoftSqlServer.ts | 30 ++++++++- packages/server/src/sdk/app/rows/search.ts | 8 +-- 3 files changed, 61 insertions(+), 41 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index cd8580c64b..e95d441981 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -438,7 +438,7 @@ describe.each([ }) describe("view save", () => { - it.only("views have extra data trimmed", async () => { + it("views have extra data trimmed", async () => { const table = await createTable({ type: "table", name: "orders", @@ -1604,35 +1604,35 @@ describe.each([ }) describe.each([ - [ - "relationship fields", - (): Record => ({ - user: { - name: "user", - relationshipType: RelationshipType.ONE_TO_MANY, - type: FieldType.LINK, - tableId: o2mTable._id!, - fieldName: "fk_o2m", - }, - users: { - name: "users", - relationshipType: RelationshipType.MANY_TO_MANY, - type: FieldType.LINK, - tableId: m2mTable._id!, - fieldName: "fk_m2m", - }, - }), - (tableId: string) => - config.api.row.save(tableId, { - name: uuid.v4(), - description: generator.paragraph(), - tableId, - }), - (row: Row) => ({ - _id: row._id, - primaryDisplay: row.name, - }), - ], + // [ + // "relationship fields", + // (): Record => ({ + // user: { + // name: "user", + // relationshipType: RelationshipType.ONE_TO_MANY, + // type: FieldType.LINK, + // tableId: o2mTable._id!, + // fieldName: "fk_o2m", + // }, + // users: { + // name: "users", + // relationshipType: RelationshipType.MANY_TO_MANY, + // type: FieldType.LINK, + // tableId: m2mTable._id!, + // fieldName: "fk_m2m", + // }, + // }), + // (tableId: string) => + // config.api.row.save(tableId, { + // name: uuid.v4(), + // description: generator.paragraph(), + // tableId, + // }), + // (row: Row) => ({ + // _id: row._id, + // primaryDisplay: row.name, + // }), + // ], [ "bb reference fields", (): Record => ({ @@ -1960,8 +1960,8 @@ describe.each([ ] await config.api.row.save(tableId, rows[0]) - await config.api.row.save(tableId, rows[1]) - await config.api.row.save(tableId, rows[2]) + // await config.api.row.save(tableId, rows[1]) + // await config.api.row.save(tableId, rows[2]) const res = await config.api.row.search(tableId) diff --git a/packages/server/src/integrations/microsoftSqlServer.ts b/packages/server/src/integrations/microsoftSqlServer.ts index f87e248ac0..e53d2ddc44 100644 --- a/packages/server/src/integrations/microsoftSqlServer.ts +++ b/packages/server/src/integrations/microsoftSqlServer.ts @@ -14,6 +14,8 @@ import { Schema, TableSourceType, DatasourcePlusQueryResponse, + FieldType, + FieldSubtype, } from "@budibase/types" import { getSqlQuery, @@ -26,7 +28,7 @@ import { import Sql from "./base/sql" import { MSSQLTablesResponse, MSSQLColumn } from "./base/types" import { getReadableErrorMessage } from "./base/errorMapping" -import sqlServer from "mssql" +import sqlServer, { IRecordSet, IResult } from "mssql" const DEFAULT_SCHEMA = "dbo" @@ -503,10 +505,34 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { const operation = this._operation(json) const queryFn = (query: any, op: string) => this.internalQuery(query, op) const processFn = (result: any) => - result.recordset ? result.recordset : [{ [operation]: true }] + result.recordset + ? this._postProcessJson(json, result.recordset) + : [{ [operation]: true }] return this.queryWithReturning(json, queryFn, processFn) } + _postProcessJson(json: QueryJson, results: IRecordSet) { + const table = json.meta?.table + if (!table) { + return results + } + for (const [name, field] of Object.entries(table.schema)) { + if ( + field.type === FieldType.JSON || + (field.type === FieldType.BB_REFERENCE && + field.subtype === FieldSubtype.USERS) + ) { + const fullName = `${table.name}.${name}` + for (let row of results) { + if (typeof row[fullName] === "string") { + row[fullName] = JSON.parse(row[fullName]) + } + } + } + } + return results + } + async getExternalSchema() { // Query to retrieve table schema const query = ` diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 8b24f9bc5f..63bbd699fa 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -1,10 +1,4 @@ -import { - Row, - SearchFilters, - SearchParams, - SortOrder, - SortType, -} from "@budibase/types" +import { Row, SearchFilters, SearchParams, SortOrder } from "@budibase/types" import { isExternalTableID } from "../../../integrations/utils" import * as internal from "./search/internal" import * as external from "./search/external" From 1334f5dcc58066f3714cc675f11f69e1fd4650b9 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 12 Mar 2024 14:46:52 +0000 Subject: [PATCH 27/35] SQL Server fully passing. --- .../server/src/api/routes/tests/row.spec.ts | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index e95d441981..f902084a48 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -46,9 +46,9 @@ jest.unmock("mssql") const { basicRow } = setup.structures describe.each([ - // ["internal", undefined], - // ["postgres", databaseTestProviders.postgres], - // ["mysql", databaseTestProviders.mysql], + ["internal", undefined], + ["postgres", databaseTestProviders.postgres], + ["mysql", databaseTestProviders.mysql], ["mssql", databaseTestProviders.mssql], ])("/rows (%s)", (__, dsProvider) => { const isInternal = !dsProvider @@ -1604,35 +1604,35 @@ describe.each([ }) describe.each([ - // [ - // "relationship fields", - // (): Record => ({ - // user: { - // name: "user", - // relationshipType: RelationshipType.ONE_TO_MANY, - // type: FieldType.LINK, - // tableId: o2mTable._id!, - // fieldName: "fk_o2m", - // }, - // users: { - // name: "users", - // relationshipType: RelationshipType.MANY_TO_MANY, - // type: FieldType.LINK, - // tableId: m2mTable._id!, - // fieldName: "fk_m2m", - // }, - // }), - // (tableId: string) => - // config.api.row.save(tableId, { - // name: uuid.v4(), - // description: generator.paragraph(), - // tableId, - // }), - // (row: Row) => ({ - // _id: row._id, - // primaryDisplay: row.name, - // }), - // ], + [ + "relationship fields", + (): Record => ({ + user: { + name: "user", + relationshipType: RelationshipType.ONE_TO_MANY, + type: FieldType.LINK, + tableId: o2mTable._id!, + fieldName: "fk_o2m", + }, + users: { + name: "users", + relationshipType: RelationshipType.MANY_TO_MANY, + type: FieldType.LINK, + tableId: m2mTable._id!, + fieldName: "fk_m2m", + }, + }), + (tableId: string) => + config.api.row.save(tableId, { + name: uuid.v4(), + description: generator.paragraph(), + tableId, + }), + (row: Row) => ({ + _id: row._id, + primaryDisplay: row.name, + }), + ], [ "bb reference fields", (): Record => ({ @@ -1960,8 +1960,8 @@ describe.each([ ] await config.api.row.save(tableId, rows[0]) - // await config.api.row.save(tableId, rows[1]) - // await config.api.row.save(tableId, rows[2]) + await config.api.row.save(tableId, rows[1]) + await config.api.row.save(tableId, rows[2]) const res = await config.api.row.search(tableId) From aff0209176e00fb9557a699687a8aa07636beec7 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 12 Mar 2024 15:27:34 +0000 Subject: [PATCH 28/35] MariaDB tests passing. --- .../server/src/api/routes/tests/row.spec.ts | 1 + packages/server/src/integrations/base/sql.ts | 33 +++++++++++ .../src/integrations/microsoftSqlServer.ts | 32 +++------- packages/server/src/integrations/mysql.ts | 32 +++++++++- .../src/integrations/tests/utils/index.ts | 9 ++- .../src/integrations/tests/utils/mariadb.ts | 58 +++++++++++++++++++ 6 files changed, 138 insertions(+), 27 deletions(-) create mode 100644 packages/server/src/integrations/tests/utils/mariadb.ts diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index f902084a48..89da4175d9 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -50,6 +50,7 @@ describe.each([ ["postgres", databaseTestProviders.postgres], ["mysql", databaseTestProviders.mysql], ["mssql", databaseTestProviders.mssql], + ["mariadb", databaseTestProviders.mariadb], ])("/rows (%s)", (__, dsProvider) => { const isInternal = !dsProvider diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index be1883c8ec..e6b9f047b9 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -4,11 +4,15 @@ import { QueryOptions } from "../../definitions/datasource" import { isIsoDateString, SqlClient, isValidFilter } from "../utils" import SqlTableQueryBuilder from "./sqlTable" import { + FieldSchema, + FieldSubtype, + FieldType, Operation, QueryJson, RelationshipsJson, SearchFilters, SortDirection, + Table, } from "@budibase/types" import environment from "../../environment" @@ -691,6 +695,35 @@ class SqlQueryBuilder extends SqlTableQueryBuilder { return results.length ? results : [{ [operation.toLowerCase()]: true }] } + convertJsonStringColumns( + table: Table, + results: Record[] + ): Record[] { + for (const [name, field] of Object.entries(table.schema)) { + if (!this._isJsonColumn(field)) { + continue + } + const fullName = `${table.name}.${name}` + for (let row of results) { + if (typeof row[fullName] === "string") { + row[fullName] = JSON.parse(row[fullName]) + } + if (typeof row[name] === "string") { + row[name] = JSON.parse(row[name]) + } + } + } + return results + } + + _isJsonColumn(field: FieldSchema) { + return ( + field.type === FieldType.JSON || + (field.type === FieldType.BB_REFERENCE && + field.subtype === FieldSubtype.USERS) + ) + } + log(query: string, values?: any[]) { if (!environment.SQL_LOGGING_ENABLE) { return diff --git a/packages/server/src/integrations/microsoftSqlServer.ts b/packages/server/src/integrations/microsoftSqlServer.ts index e53d2ddc44..0c9b8f4547 100644 --- a/packages/server/src/integrations/microsoftSqlServer.ts +++ b/packages/server/src/integrations/microsoftSqlServer.ts @@ -504,33 +504,15 @@ class SqlServerIntegration extends Sql implements DatasourcePlus { } const operation = this._operation(json) const queryFn = (query: any, op: string) => this.internalQuery(query, op) - const processFn = (result: any) => - result.recordset - ? this._postProcessJson(json, result.recordset) - : [{ [operation]: true }] - return this.queryWithReturning(json, queryFn, processFn) - } - - _postProcessJson(json: QueryJson, results: IRecordSet) { - const table = json.meta?.table - if (!table) { - return results - } - for (const [name, field] of Object.entries(table.schema)) { - if ( - field.type === FieldType.JSON || - (field.type === FieldType.BB_REFERENCE && - field.subtype === FieldSubtype.USERS) - ) { - const fullName = `${table.name}.${name}` - for (let row of results) { - if (typeof row[fullName] === "string") { - row[fullName] = JSON.parse(row[fullName]) - } - } + const processFn = (result: any) => { + if (json?.meta?.table && result.recordset) { + return this.convertJsonStringColumns(json.meta.table, result.recordset) + } else if (result.recordset) { + return result.recordset } + return [{ [operation]: true }] } - return results + return this.queryWithReturning(json, queryFn, processFn) } async getExternalSchema() { diff --git a/packages/server/src/integrations/mysql.ts b/packages/server/src/integrations/mysql.ts index f629381807..1f50525675 100644 --- a/packages/server/src/integrations/mysql.ts +++ b/packages/server/src/integrations/mysql.ts @@ -13,6 +13,8 @@ import { Schema, TableSourceType, DatasourcePlusQueryResponse, + FieldType, + FieldSubtype, } from "@budibase/types" import { getSqlQuery, @@ -386,12 +388,40 @@ class MySQLIntegration extends Sql implements DatasourcePlus { try { const queryFn = (query: any) => this.internalQuery(query, { connect: false, disableCoercion: true }) - return await this.queryWithReturning(json, queryFn) + const processFn = (result: any) => { + if (json?.meta?.table && Array.isArray(result)) { + return this.convertJsonStringColumns(json.meta.table, result) + } + return result + } + return await this.queryWithReturning(json, queryFn, processFn) } finally { await this.disconnect() } } + _postProcessJson(json: QueryJson, results: any) { + const table = json.meta?.table + if (!table) { + return results + } + for (const [name, field] of Object.entries(table.schema)) { + if ( + field.type === FieldType.JSON || + (field.type === FieldType.BB_REFERENCE && + field.subtype === FieldSubtype.USERS) + ) { + const fullName = `${table.name}.${name}` + for (let row of results) { + if (typeof row[fullName] === "string") { + row[fullName] = JSON.parse(row[fullName]) + } + } + } + } + return results + } + async getExternalSchema() { try { const [databaseResult] = await this.internalQuery({ diff --git a/packages/server/src/integrations/tests/utils/index.ts b/packages/server/src/integrations/tests/utils/index.ts index a5282bff6f..b2be3df4e0 100644 --- a/packages/server/src/integrations/tests/utils/index.ts +++ b/packages/server/src/integrations/tests/utils/index.ts @@ -5,6 +5,7 @@ import * as postgres from "./postgres" import * as mongodb from "./mongodb" import * as mysql from "./mysql" import * as mssql from "./mssql" +import * as mariadb from "./mariadb" import { StartedTestContainer } from "testcontainers" jest.setTimeout(30000) @@ -15,4 +16,10 @@ export interface DatabaseProvider { datasource(): Promise } -export const databaseTestProviders = { postgres, mongodb, mysql, mssql } +export const databaseTestProviders = { + postgres, + mongodb, + mysql, + mssql, + mariadb, +} diff --git a/packages/server/src/integrations/tests/utils/mariadb.ts b/packages/server/src/integrations/tests/utils/mariadb.ts new file mode 100644 index 0000000000..a097e0aaa1 --- /dev/null +++ b/packages/server/src/integrations/tests/utils/mariadb.ts @@ -0,0 +1,58 @@ +import { Datasource, SourceName } from "@budibase/types" +import { GenericContainer, Wait, StartedTestContainer } from "testcontainers" +import { AbstractWaitStrategy } from "testcontainers/build/wait-strategies/wait-strategy" + +let container: StartedTestContainer | undefined + +class MariaDBWaitStrategy extends AbstractWaitStrategy { + async waitUntilReady(container: any, boundPorts: any, startTime?: Date) { + // Because MariaDB first starts itself up, runs an init script, then restarts, + // it's possible for the mysqladmin ping to succeed early and then tests to + // run against a MariaDB that's mid-restart and fail. To get around this, we + // wait for logs and then do a ping check. + + const logs = Wait.forLogMessage("mariadbd: ready for connections", 2) + await logs.waitUntilReady(container, boundPorts, startTime) + + const command = Wait.forSuccessfulCommand( + `mysqladmin ping -h localhost -P 3306 -u root -ppassword` + ) + await command.waitUntilReady(container) + } +} + +export async function start(): Promise { + return await new GenericContainer("mariadb:lts") + .withExposedPorts(3306) + .withEnvironment({ MARIADB_ROOT_PASSWORD: "password" }) + .withWaitStrategy(new MariaDBWaitStrategy()) + .start() +} + +export async function datasource(): Promise { + if (!container) { + container = await start() + } + const host = container.getHost() + const port = container.getMappedPort(3306) + + return { + type: "datasource_plus", + source: SourceName.MYSQL, + plus: true, + config: { + host, + port, + user: "root", + password: "password", + database: "mysql", + }, + } +} + +export async function stop() { + if (container) { + await container.stop() + container = undefined + } +} From a70cb903f6aa0cb61656e7345b08ee5e4b811fea Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 12 Mar 2024 17:17:01 +0000 Subject: [PATCH 29/35] Remove jest timeout. --- packages/server/src/api/routes/tests/row.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 89da4175d9..d50dd8a3d9 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -38,7 +38,6 @@ import * as uuid from "uuid" const timestamp = new Date("2023-01-26T11:48:57.597Z").toISOString() tk.freeze(timestamp) -jest.setTimeout(99999999) jest.unmock("mysql2") jest.unmock("mysql2/promise") jest.unmock("mssql") From d061c19c80233f10f18514ac41f4e3fb8f4bfd09 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 12 Mar 2024 17:21:33 +0000 Subject: [PATCH 30/35] Remove some extraneous, unused code. --- .../src/integrations/microsoftSqlServer.ts | 2 +- packages/server/src/integrations/mysql.ts | 22 ------------------- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/packages/server/src/integrations/microsoftSqlServer.ts b/packages/server/src/integrations/microsoftSqlServer.ts index 0c9b8f4547..c79eb136ed 100644 --- a/packages/server/src/integrations/microsoftSqlServer.ts +++ b/packages/server/src/integrations/microsoftSqlServer.ts @@ -28,7 +28,7 @@ import { import Sql from "./base/sql" import { MSSQLTablesResponse, MSSQLColumn } from "./base/types" import { getReadableErrorMessage } from "./base/errorMapping" -import sqlServer, { IRecordSet, IResult } from "mssql" +import sqlServer from "mssql" const DEFAULT_SCHEMA = "dbo" diff --git a/packages/server/src/integrations/mysql.ts b/packages/server/src/integrations/mysql.ts index 1f50525675..9638afa8ea 100644 --- a/packages/server/src/integrations/mysql.ts +++ b/packages/server/src/integrations/mysql.ts @@ -400,28 +400,6 @@ class MySQLIntegration extends Sql implements DatasourcePlus { } } - _postProcessJson(json: QueryJson, results: any) { - const table = json.meta?.table - if (!table) { - return results - } - for (const [name, field] of Object.entries(table.schema)) { - if ( - field.type === FieldType.JSON || - (field.type === FieldType.BB_REFERENCE && - field.subtype === FieldSubtype.USERS) - ) { - const fullName = `${table.name}.${name}` - for (let row of results) { - if (typeof row[fullName] === "string") { - row[fullName] = JSON.parse(row[fullName]) - } - } - } - } - return results - } - async getExternalSchema() { try { const [databaseResult] = await this.internalQuery({ From aa978b4caee7f6a73bc7c486977f0d518467fcfd Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Wed, 13 Mar 2024 09:51:59 +0000 Subject: [PATCH 31/35] Bump version to 2.21.9 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index b845465de5..0f6121bb18 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.21.8", + "version": "2.21.9", "npmClient": "yarn", "packages": [ "packages/*", From a5c8e8845f92a02e24c77f2793c2fde31e56b010 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 13 Mar 2024 13:38:08 +0000 Subject: [PATCH 32/35] Implement Adri's type guard suggestion. --- packages/server/src/integrations/base/sql.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index e6b9f047b9..8342c45fd7 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -4,9 +4,11 @@ import { QueryOptions } from "../../definitions/datasource" import { isIsoDateString, SqlClient, isValidFilter } from "../utils" import SqlTableQueryBuilder from "./sqlTable" import { + BBReferenceFieldMetadata, FieldSchema, FieldSubtype, FieldType, + JsonFieldMetadata, Operation, QueryJson, RelationshipsJson, @@ -716,7 +718,9 @@ class SqlQueryBuilder extends SqlTableQueryBuilder { return results } - _isJsonColumn(field: FieldSchema) { + _isJsonColumn( + field: FieldSchema + ): field is JsonFieldMetadata | BBReferenceFieldMetadata { return ( field.type === FieldType.JSON || (field.type === FieldType.BB_REFERENCE && From 0420734d9793403a6a119e9d48bdd32b0e9270ed Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 13 Mar 2024 16:48:32 +0100 Subject: [PATCH 33/35] Add failing test --- .../rowProcessor/tests/attachments.spec.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts b/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts index 43af79d82c..beea04829b 100644 --- a/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts +++ b/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts @@ -115,4 +115,17 @@ describe("attachment cleanup", () => { await AttachmentCleanup.rowUpdate(table(), { row: row(), oldRow: row() }) expect(mockedDeleteFiles).not.toBeCalled() }) + + it("should be able to cleanup a column and not throw when attachments are undefined", async () => { + const originalTable = table() + delete originalTable.schema["attach"] + await AttachmentCleanup.tableUpdate( + originalTable, + [{ attach: undefined }], + { + oldTable: table(), + } + ) + expect(mockedDeleteFiles).not.toBeCalled() + }) }) From 940ff5acd24b00ff489cc7a3e41c334cde13becb Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 13 Mar 2024 16:48:58 +0100 Subject: [PATCH 34/35] Fix table changes with empty attachments --- packages/server/src/utilities/rowProcessor/attachments.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/utilities/rowProcessor/attachments.ts b/packages/server/src/utilities/rowProcessor/attachments.ts index c289680eb3..e1c83352d4 100644 --- a/packages/server/src/utilities/rowProcessor/attachments.ts +++ b/packages/server/src/utilities/rowProcessor/attachments.ts @@ -43,7 +43,7 @@ export class AttachmentCleanup { if ((columnRemoved && !renaming) || opts.deleting) { rows.forEach(row => { files = files.concat( - row[key].map((attachment: any) => attachment.key) + (row[key] || []).map((attachment: any) => attachment.key) ) }) } From 625c1dda9fd49444e20bdc044c7da865495e8b05 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 13 Mar 2024 16:54:39 +0100 Subject: [PATCH 35/35] Improve tests --- .../rowProcessor/tests/attachments.spec.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts b/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts index beea04829b..3c58d2c056 100644 --- a/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts +++ b/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts @@ -121,7 +121,21 @@ describe("attachment cleanup", () => { delete originalTable.schema["attach"] await AttachmentCleanup.tableUpdate( originalTable, - [{ attach: undefined }], + [row("file 1"), { attach: undefined }, row("file 2")], + { + oldTable: table(), + } + ) + expect(mockedDeleteFiles).toBeCalledTimes(1) + expect(mockedDeleteFiles).toBeCalledWith(BUCKET, ["file 1", "file 2"]) + }) + + it("should be able to cleanup a column and not throw when ALL attachments are undefined", async () => { + const originalTable = table() + delete originalTable.schema["attach"] + await AttachmentCleanup.tableUpdate( + originalTable, + [{}, { attach: undefined }], { oldTable: table(), }