From c18a3d4abb92c0b33221c0dc52969ee642fccfd1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 9 Feb 2024 15:27:31 +0100 Subject: [PATCH 01/11] Add creation tests --- .../server/src/api/controllers/table/index.ts | 2 - .../server/src/api/routes/tests/table.spec.ts | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index db2bd672d0..40fc2aedb4 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -14,7 +14,6 @@ import { events } from "@budibase/backend-core" import { BulkImportRequest, BulkImportResponse, - DocumentType, FetchTablesResponse, MigrateRequest, MigrateResponse, @@ -25,7 +24,6 @@ import { TableResponse, TableSourceType, UserCtx, - SEPARATOR, } from "@budibase/types" import sdk from "../../../sdk" import { jsonFromCsvString } from "../../../utilities/csv" diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 62efdda448..4da4e25ace 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -18,6 +18,12 @@ import * as setup from "./utilities" import sdk from "../../../sdk" import * as uuid from "uuid" +import tk from "timekeeper" +import { mocks } from "@budibase/backend-core/tests" +import { TableToBuild } from "src/tests/utilities/TestConfiguration" + +tk.freeze(mocks.date.MOCK_DATE) + const { basicTable } = setup.structures describe("/tables", () => { @@ -60,6 +66,47 @@ describe("/tables", () => { expect(events.table.created).toBeCalledWith(res.body) }) + it("creates all the passed fields", async () => { + const tableData: TableToBuild = { + name: "TestTable", + type: "table", + schema: { + autoId: { + name: "id", + type: FieldType.NUMBER, + subtype: AutoFieldSubType.AUTO_ID, + autocolumn: true, + constraints: { + type: "number", + presence: false, + }, + }, + }, + views: { + view1: { + id: "viewId", + version: 2, + name: "table view", + tableId: "tableId", + }, + }, + } + const testTable = await config.createTable(tableData) + + const expected: Table = { + ...tableData, + type: "table", + sourceType: TableSourceType.INTERNAL, + sourceId: expect.any(String), + _rev: expect.stringMatching(/^1-.+/), + updatedAt: mocks.date.MOCK_DATE.toISOString(), + } + expect(testTable).toEqual(expected) + + const persistedTable = await config.api.table.get(testTable._id!) + expect(persistedTable).toEqual(expected) + }) + it("creates a table via data import", async () => { const table: SaveTableRequest = basicTable() table.rows = [{ name: "test-name", description: "test-desc" }] From f1b31b4119486dfe42e038850f548e6f1fd9a553 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 9 Feb 2024 15:27:48 +0100 Subject: [PATCH 02/11] Export type --- packages/server/src/tests/utilities/TestConfiguration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index d96655af43..53a4e3432c 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -84,7 +84,7 @@ type DefaultUserValues = { csrfToken: string } -interface TableToBuild extends Omit { +export interface TableToBuild extends Omit { sourceId?: string sourceType?: TableSourceType } From f1a75b84b4c268906ce32d03c2ebbfa5fe5112b1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 9 Feb 2024 16:20:56 +0100 Subject: [PATCH 03/11] Add test --- .../server/src/api/routes/tests/table.spec.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 4da4e25ace..7fb4c0673a 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -199,6 +199,56 @@ describe("/tables", () => { expect(res.body.name).toBeUndefined() }) + it("updates only the passed fields", async () => { + const testTable = await config.createTable({ + name: "TestTable", + type: "table", + schema: { + autoId: { + name: "id", + type: FieldType.NUMBER, + subtype: AutoFieldSubType.AUTO_ID, + autocolumn: true, + constraints: { + type: "number", + presence: false, + }, + }, + }, + views: { + view1: { + id: "viewId", + version: 2, + name: "table view", + tableId: "tableId", + }, + }, + }) + + const response = await request + .post(`/api/tables`) + .send({ + ...testTable, + name: "UpdatedName", + }) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + + expect(response.body).toEqual({ + ...testTable, + name: "UpdatedName", + _rev: expect.stringMatching(/^2-.+/), + }) + + const persistedTable = await config.api.table.get(testTable._id!) + expect(persistedTable).toEqual({ + ...testTable, + name: "UpdatedName", + _rev: expect.stringMatching(/^2-.+/), + }) + }) + describe("user table", () => { it("should add roleId and email field when adjusting user table schema", async () => { const res = await request From 2c26b55a7c3fc87a0ea39c7d614043a34d4ee20c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 11:59:05 +0100 Subject: [PATCH 04/11] Handle view creation on new table requests --- .../server/src/api/controllers/table/index.ts | 3 ++- .../src/api/controllers/table/internal.ts | 7 +++--- .../server/src/api/routes/tests/table.spec.ts | 23 ++++++++++++++++++- .../src/sdk/app/tables/internal/index.ts | 12 ++++++---- 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 40fc2aedb4..55a896373f 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -75,9 +75,10 @@ export async function save(ctx: UserCtx) { const table = ctx.request.body const isImport = table.rows - const savedTable = await pickApi({ table }).save(ctx) + let savedTable = await pickApi({ table }).save(ctx) if (!table._id) { await events.table.created(savedTable) + savedTable = sdk.tables.enrichViewSchemas(savedTable) } else { await events.table.updated(savedTable) } diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index bb94f2bc01..34bc78b243 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -18,10 +18,11 @@ export async function save(ctx: UserCtx) { _rename?: RenameColumn } = { _id: generateTableID(), - ...rest, - type: "table", - sourceType: TableSourceType.INTERNAL, views: {}, + ...rest, + // Ensure these fields are populated, even if not sent in the request + type: rest.type || "table", + sourceType: rest.sourceType || TableSourceType.INTERNAL, } const renaming = tableToSave._rename delete tableToSave._rename diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 7fb4c0673a..b5d0107981 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -5,6 +5,7 @@ import { FieldType, INTERNAL_TABLE_SOURCE_ID, InternalTable, + NumberFieldMetadata, RelationshipType, Row, SaveTableRequest, @@ -83,7 +84,7 @@ describe("/tables", () => { }, }, views: { - view1: { + "table view": { id: "viewId", version: 2, name: "table view", @@ -96,9 +97,29 @@ describe("/tables", () => { const expected: Table = { ...tableData, type: "table", + views: { + "table view": { + ...tableData.views!["table view"], + schema: { + autoId: { + autocolumn: true, + constraints: { + presence: false, + type: "number", + }, + name: "id", + type: FieldType.NUMBER, + subtype: AutoFieldSubType.AUTO_ID, + visible: false, + } as NumberFieldMetadata, + }, + }, + }, sourceType: TableSourceType.INTERNAL, sourceId: expect.any(String), _rev: expect.stringMatching(/^1-.+/), + _id: expect.any(String), + createdAt: mocks.date.MOCK_DATE.toISOString(), updatedAt: mocks.date.MOCK_DATE.toISOString(), } expect(testTable).toEqual(expected) diff --git a/packages/server/src/sdk/app/tables/internal/index.ts b/packages/server/src/sdk/app/tables/internal/index.ts index 25fe145484..5d9feb5fe8 100644 --- a/packages/server/src/sdk/app/tables/internal/index.ts +++ b/packages/server/src/sdk/app/tables/internal/index.ts @@ -75,11 +75,13 @@ export async function save( if (!tableView) continue if (viewsSdk.isV2(tableView)) { - table.views[view] = viewsSdk.syncSchema( - oldTable!.views![view] as ViewV2, - table.schema, - renaming - ) + if (oldTable?.views && oldTable.views[view]) { + table.views[view] = viewsSdk.syncSchema( + oldTable.views[view] as ViewV2, + table.schema, + renaming + ) + } continue } From ffdfb731fb89fbad8f1b9e1925ece06fbbf4309f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 12:25:56 +0100 Subject: [PATCH 05/11] Fix tests --- .../server/src/api/routes/tests/table.spec.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index b5d0107981..efa9845fcb 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -348,6 +348,7 @@ describe("/tables", () => { describe("fetch", () => { let testTable: Table + const enrichViewSchemasMock = jest.spyOn(sdk.tables, "enrichViewSchemas") beforeEach(async () => { testTable = await config.createTable(testTable) @@ -357,6 +358,10 @@ describe("/tables", () => { delete testTable._rev }) + afterAll(() => { + enrichViewSchemasMock.mockRestore() + }) + it("returns all the tables for that instance in the response body", async () => { const res = await request .get(`/api/tables`) @@ -405,7 +410,7 @@ describe("/tables", () => { it("should enrich the view schemas for viewsV2", async () => { const tableId = config.table!._id! - jest.spyOn(sdk.tables, "enrichViewSchemas").mockImplementation(t => ({ + enrichViewSchemasMock.mockImplementation(t => ({ ...t, views: { view1: { @@ -413,7 +418,7 @@ describe("/tables", () => { name: "view1", schema: {}, id: "new_view_id", - tableId, + tableId: t._id!, }, }, })) @@ -480,11 +485,7 @@ describe("/tables", () => { let testTable: Table beforeEach(async () => { - testTable = await config.createTable(testTable) - }) - - afterEach(() => { - delete testTable._rev + testTable = await config.createTable() }) it("returns a success response when a table is deleted.", async () => { From 231c8ccaabb5d4f33fa095f72abfa1d432591beb Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 12:33:16 +0100 Subject: [PATCH 06/11] Make code more readable --- packages/server/src/api/controllers/table/internal.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index 34bc78b243..8e90007d88 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -18,12 +18,16 @@ export async function save(ctx: UserCtx) { _rename?: RenameColumn } = { _id: generateTableID(), - views: {}, ...rest, // Ensure these fields are populated, even if not sent in the request type: rest.type || "table", sourceType: rest.sourceType || TableSourceType.INTERNAL, } + + if (!tableToSave.views) { + tableToSave.views = {} + } + const renaming = tableToSave._rename delete tableToSave._rename From 8651a836a5f0094c90f4aafa4aa2edad65651608 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 12:34:39 +0100 Subject: [PATCH 07/11] Fix exports --- packages/server/src/tests/utilities/TestConfiguration.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 53a4e3432c..90d50e2816 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -89,7 +89,7 @@ export interface TableToBuild extends Omit { sourceType?: TableSourceType } -class TestConfiguration { +export default class TestConfiguration { server: any request: supertest.SuperTest | undefined started: boolean @@ -911,5 +911,3 @@ class TestConfiguration { return await this._req(config, null, layoutController.save) } } - -export = TestConfiguration From 6cdfd4b621a23e433dec500d4fc599a87da57ac7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 12:36:29 +0100 Subject: [PATCH 08/11] Lint --- packages/server/src/api/routes/tests/table.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index efa9845fcb..c8cb3ef21b 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -21,7 +21,7 @@ import * as uuid from "uuid" import tk from "timekeeper" import { mocks } from "@budibase/backend-core/tests" -import { TableToBuild } from "src/tests/utilities/TestConfiguration" +import { TableToBuild } from "../../../tests/utilities/TestConfiguration" tk.freeze(mocks.date.MOCK_DATE) From 3ee555e72af8986caf683e68ed1221469c1c3dd5 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 12:50:23 +0100 Subject: [PATCH 09/11] Fix js tests --- packages/server/src/tests/utilities/TestConfiguration.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 90d50e2816..ea3204536a 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -911,3 +911,5 @@ export default class TestConfiguration { return await this._req(config, null, layoutController.save) } } + +module.exports = TestConfiguration From b27ca57e1aabfac5c9bd4d585ea2775a7b89f179 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 14:00:32 +0100 Subject: [PATCH 10/11] Allow loging js execution errors --- packages/string-templates/src/helpers/javascript.js | 5 +++++ packages/string-templates/src/index.js | 1 + packages/string-templates/src/index.mjs | 1 + 3 files changed, 7 insertions(+) diff --git a/packages/string-templates/src/helpers/javascript.js b/packages/string-templates/src/helpers/javascript.js index 99d7df10f7..4a7f602690 100644 --- a/packages/string-templates/src/helpers/javascript.js +++ b/packages/string-templates/src/helpers/javascript.js @@ -8,6 +8,9 @@ const { getJsHelperList } = require("./list") let runJS module.exports.setJSRunner = runner => (runJS = runner) +let onErrorLog +module.exports.setOnErrorLog = delegate => (onErrorLog = delegate) + // Helper utility to strip square brackets from a value const removeSquareBrackets = value => { if (!value || typeof value !== "string") { @@ -56,6 +59,8 @@ module.exports.processJS = (handlebars, context) => { const res = { data: runJS(js, sandboxContext) } return `{{${LITERAL_MARKER} js_result-${JSON.stringify(res)}}}` } catch (error) { + onErrorLog && onErrorLog(error) + if (error.code === "ERR_SCRIPT_EXECUTION_TIMEOUT") { return "Timed out while executing JS" } diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js index 3636c0a456..f370b67272 100644 --- a/packages/string-templates/src/index.js +++ b/packages/string-templates/src/index.js @@ -365,6 +365,7 @@ module.exports.doesContainString = (template, string) => { } module.exports.setJSRunner = javascript.setJSRunner +module.exports.setOnErrorLog = javascript.setOnErrorLog module.exports.convertToJS = hbs => { const blocks = exports.findHBSBlocks(hbs) diff --git a/packages/string-templates/src/index.mjs b/packages/string-templates/src/index.mjs index bdded04b02..5ac7981fee 100644 --- a/packages/string-templates/src/index.mjs +++ b/packages/string-templates/src/index.mjs @@ -20,6 +20,7 @@ export const disableEscaping = templates.disableEscaping export const findHBSBlocks = templates.findHBSBlocks export const convertToJS = templates.convertToJS export const setJSRunner = templates.setJSRunner +export const setOnErrorLog = templates.setOnErrorLog export const FIND_ANY_HBS_REGEX = templates.FIND_ANY_HBS_REGEX export const helpersToRemoveForJs = templates.helpersToRemoveForJs From 93eb9fc9c8e475ab028db20caf54875578d41811 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 12 Feb 2024 14:01:00 +0100 Subject: [PATCH 11/11] Setup error logging --- packages/server/src/environment.ts | 1 + packages/server/src/jsRunner/index.ts | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/server/src/environment.ts b/packages/server/src/environment.ts index ba3aa280e2..fd4c3d205b 100644 --- a/packages/server/src/environment.ts +++ b/packages/server/src/environment.ts @@ -97,6 +97,7 @@ const environment = { APP_MIGRATION_TIMEOUT: parseIntSafe(process.env.APP_MIGRATION_TIMEOUT), JS_RUNNER_MEMORY_LIMIT: parseIntSafe(process.env.JS_RUNNER_MEMORY_LIMIT) || 64, + LOG_JS_ERRORS: process.env.LOG_JS_ERRORS, } // threading can cause memory issues with node-ts in development diff --git a/packages/server/src/jsRunner/index.ts b/packages/server/src/jsRunner/index.ts index 1936b0ef45..38cd0dd0d6 100644 --- a/packages/server/src/jsRunner/index.ts +++ b/packages/server/src/jsRunner/index.ts @@ -1,7 +1,7 @@ import vm from "vm" import env from "../environment" -import { setJSRunner } from "@budibase/string-templates" -import { context, timers } from "@budibase/backend-core" +import { setJSRunner, setOnErrorLog } from "@budibase/string-templates" +import { context, logging, timers } from "@budibase/backend-core" import tracer from "dd-trace" type TrackerFn = (f: () => T) => T @@ -58,4 +58,10 @@ export function init() { ) }) }) + + if (env.LOG_JS_ERRORS) { + setOnErrorLog((error: Error) => { + logging.logWarn(JSON.stringify(error)) + }) + } }