From c2b4dec88f6fd90994321b64f95f8f90434ed5a1 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 23 Jun 2023 18:09:52 +0100 Subject: [PATCH 1/6] Fix to make foreign keys auto-columns when used in relationships as well as making sure that when fetching tables that they can be removed, using the old fetch mechanism (to be replaced, but needs to work for now). --- .../server/src/api/controllers/datasource.ts | 12 ++-- .../src/api/controllers/table/external.ts | 26 +++++---- packages/server/src/sdk/app/tables/index.ts | 2 + .../server/src/sdk/app/tables/validation.ts | 57 +++++++++++++++++++ packages/types/src/documents/app/table.ts | 5 ++ 5 files changed, 84 insertions(+), 18 deletions(-) create mode 100644 packages/server/src/sdk/app/tables/validation.ts diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 8fe0ab70da..062a185a23 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -183,9 +183,7 @@ export async function buildSchemaFromDb(ctx: UserCtx) { let { tables, error } = await buildSchemaHelper(datasource) if (tablesFilter) { - if (!datasource.entities) { - datasource.entities = {} - } + datasource.entities = {} for (let key in tables) { if ( tablesFilter.some( @@ -200,7 +198,7 @@ export async function buildSchemaFromDb(ctx: UserCtx) { } setDefaultDisplayColumns(datasource) - const dbResp = await db.put(datasource) + const dbResp = await db.put(sdk.tables.checkExternalTableSchemas(datasource)) datasource._rev = dbResp.rev const cleanedDatasource = await sdk.datasources.removeSecretSingle(datasource) @@ -286,7 +284,9 @@ export async function update(ctx: UserCtx) { datasource.config!.auth = auth } - const response = await db.put(datasource) + const response = await db.put( + sdk.tables.checkExternalTableSchemas(datasource) + ) await events.datasource.updated(datasource) datasource._rev = response.rev @@ -327,7 +327,7 @@ export async function save( setDefaultDisplayColumns(datasource) } - const dbResp = await db.put(datasource) + const dbResp = await db.put(sdk.tables.checkExternalTableSchemas(datasource)) await events.datasource.created(datasource) datasource._rev = dbResp.rev diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index 24d4242478..8877f0f8d0 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -1,32 +1,34 @@ import { - buildExternalTableId, breakExternalTableId, + buildExternalTableId, } from "../../../integrations/utils" import { + foreignKeyStructure, generateForeignKey, generateJunctionTableName, - foreignKeyStructure, hasTypeChanged, setStaticSchemas, } from "./utils" import { FieldTypes } from "../../../constants" import { makeExternalQuery } from "../../../integrations/base/query" import { handleRequest } from "../row/external" -import { events, context } from "@budibase/backend-core" -import { parse, isRows, isSchema } from "../../../utilities/schema" +import { context, events } from "@budibase/backend-core" +import { isRows, isSchema, parse } from "../../../utilities/schema" import { + AutoReason, Datasource, - Table, - QueryJson, - Operation, - RenameColumn, FieldSchema, - UserCtx, - TableRequest, + Operation, + QueryJson, RelationshipTypes, + RenameColumn, + Table, + TableRequest, + UserCtx, } from "@budibase/types" import sdk from "../../../sdk" import { builderSocket } from "../../../websockets" + const { cloneDeep } = require("lodash/fp") async function makeTableRequest( @@ -317,7 +319,7 @@ export async function save(ctx: UserCtx) { delete tableToSave._rename // store it into couch now for budibase reference datasource.entities[tableToSave.name] = tableToSave - await db.put(datasource) + await db.put(sdk.tables.checkExternalTableSchemas(datasource)) // Since tables are stored inside datasources, we need to notify clients // that the datasource definition changed @@ -348,7 +350,7 @@ export async function destroy(ctx: UserCtx) { datasource.entities = tables } - await db.put(datasource) + await db.put(sdk.tables.checkExternalTableSchemas(datasource)) // Since tables are stored inside datasources, we need to notify clients // that the datasource definition changed diff --git a/packages/server/src/sdk/app/tables/index.ts b/packages/server/src/sdk/app/tables/index.ts index 6bb09ae845..92ef3f3291 100644 --- a/packages/server/src/sdk/app/tables/index.ts +++ b/packages/server/src/sdk/app/tables/index.ts @@ -7,6 +7,7 @@ import { } from "../../../integrations/utils" import { Table, Database } from "@budibase/types" import datasources from "../datasources" +import { checkExternalTableSchemas } from "./validation" async function getAllInternalTables(db?: Database): Promise { if (!db) { @@ -60,4 +61,5 @@ export default { getAllExternalTables, getExternalTable, getTable, + checkExternalTableSchemas, } diff --git a/packages/server/src/sdk/app/tables/validation.ts b/packages/server/src/sdk/app/tables/validation.ts new file mode 100644 index 0000000000..98ad72ea3d --- /dev/null +++ b/packages/server/src/sdk/app/tables/validation.ts @@ -0,0 +1,57 @@ +import { + Datasource, + FieldType, + RelationshipTypes, + AutoReason, +} from "@budibase/types" + +function checkForeignKeysAreAutoColumns(datasource: Datasource) { + if (!datasource.entities) { + return datasource + } + const tables = Object.values(datasource.entities) + // make sure all foreign key columns are marked as auto columns + const foreignKeys: { tableId: string; key: string }[] = [] + for (let table of tables) { + const relationships = Object.values(table.schema).filter( + column => column.type === FieldType.LINK + ) + relationships.forEach(relationship => { + if (relationship.relationshipType === RelationshipTypes.MANY_TO_MANY) { + const tableId = relationship.through! + foreignKeys.push({ key: relationship.throughTo!, tableId }) + foreignKeys.push({ key: relationship.throughFrom!, tableId }) + } else { + const fk = relationship.foreignKey! + const oneSide = + relationship.relationshipType === RelationshipTypes.ONE_TO_MANY + foreignKeys.push({ + tableId: oneSide ? table._id! : relationship.tableId!, + key: fk, + }) + } + }) + } + + // now make sure schemas are all accurate + for (let table of tables) { + for (let column of Object.values(table.schema)) { + const shouldBeForeign = foreignKeys.find( + options => options.tableId === table._id && options.key === column.name + ) + if (shouldBeForeign) { + column.autocolumn = true + column.autoReason = AutoReason.FOREIGN_KEY + } else if (column.autoReason === AutoReason.FOREIGN_KEY) { + delete column.autocolumn + delete column.autoReason + } + } + } + + return datasource +} + +export function checkExternalTableSchemas(datasource: Datasource) { + return checkForeignKeysAreAutoColumns(datasource) +} diff --git a/packages/types/src/documents/app/table.ts b/packages/types/src/documents/app/table.ts index 7089709808..18b415da5f 100644 --- a/packages/types/src/documents/app/table.ts +++ b/packages/types/src/documents/app/table.ts @@ -9,6 +9,10 @@ export enum RelationshipTypes { MANY_TO_MANY = "many-to-many", } +export enum AutoReason { + FOREIGN_KEY = "foreign_key", +} + export interface FieldSchema { type: FieldType externalType?: string @@ -21,6 +25,7 @@ export interface FieldSchema { foreignKey?: string icon?: string autocolumn?: boolean + autoReason?: AutoReason subtype?: string throughFrom?: string throughTo?: string From e022da5bc8578cde7ce20ac04dcb257432f2cf61 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 23 Jun 2023 18:31:02 +0100 Subject: [PATCH 2/6] Adding test case for foreign key autocolumn enrichment. --- .../sdk/app/tables/tests/validation.spec.ts | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 packages/server/src/sdk/app/tables/tests/validation.spec.ts diff --git a/packages/server/src/sdk/app/tables/tests/validation.spec.ts b/packages/server/src/sdk/app/tables/tests/validation.spec.ts new file mode 100644 index 0000000000..8acc315125 --- /dev/null +++ b/packages/server/src/sdk/app/tables/tests/validation.spec.ts @@ -0,0 +1,98 @@ +import { checkExternalTableSchemas } from "../validation" +import { cloneDeep } from "lodash/fp" +import { Datasource } from "@budibase/types" + +const SCHEMA = { + entities: { + client: { + _id: "tableA", + name: "client", + primary: ["idC"], + primaryDisplay: "Name", + schema: { + idC: { + autocolumn: true, + externalType: "int unsigned", + name: "idC", + type: "number", + }, + Name: { + autocolumn: false, + externalType: "varchar(255)", + name: "Name", + type: "string", + }, + project: { + fieldName: "idC", + foreignKey: "idC", + main: true, + name: "project", + relationshipType: "many-to-one", + tableId: "tableB", + type: "link", + }, + }, + }, + project: { + _id: "tableB", + name: "project", + primary: ["idP"], + primaryDisplay: "Name", + schema: { + idC: { + externalType: "int unsigned", + name: "idC", + type: "number", + }, + idP: { + autocolumn: true, + externalType: "int unsigned", + name: "idProject", + type: "number", + }, + Name: { + autocolumn: false, + externalType: "varchar(255)", + name: "Name", + type: "string", + }, + client: { + fieldName: "idC", + foreignKey: "idC", + name: "client", + relationshipType: "one-to-many", + tableId: "tableA", + type: "link", + }, + }, + sql: true, + type: "table", + }, + }, +} + +describe("validation and update of external table schemas", () => { + function getForeignKeyColumn(datasource: Datasource) { + return datasource.entities!["project"].schema.idC + } + + it("should correctly set utilised foreign keys to autocolumns", () => { + const response = checkExternalTableSchemas(cloneDeep(SCHEMA) as any) + const foreignKey = getForeignKeyColumn(response) + expect(foreignKey.autocolumn).toBe(true) + expect(foreignKey.autoReason).toBe("foreign_key") + }) + + it("should correctly unset foreign keys when no longer used", () => { + const setResponse = checkExternalTableSchemas(cloneDeep(SCHEMA) as any) + const beforeFk = getForeignKeyColumn(setResponse) + delete setResponse.entities!.client.schema.project + delete setResponse.entities!.project.schema.client + const response = checkExternalTableSchemas(cloneDeep(setResponse)) + const afterFk = getForeignKeyColumn(response) + expect(beforeFk.autocolumn).toBe(true) + expect(beforeFk.autoReason).toBe("foreign_key") + expect(afterFk.autocolumn).toBeUndefined() + expect(afterFk.autoReason).toBeUndefined() + }) +}) From 1c09913d33fb855d5f521fb2d02e32b569f477c9 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 26 Jun 2023 15:05:10 +0100 Subject: [PATCH 3/6] Fix for foreign keys being unsettlable after update, breaking Postgres test. --- .../src/api/controllers/row/ExternalRequest.ts | 4 ++-- packages/server/src/sdk/app/tables/index.ts | 3 ++- packages/server/src/sdk/app/tables/validation.ts | 14 ++++++++++++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index f6b75aca05..0139147e35 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -30,6 +30,7 @@ import { cloneDeep } from "lodash/fp" import { processDates, processFormulas } from "../../../utilities/rowProcessor" import { db as dbCore } from "@budibase/backend-core" import sdk from "../../../sdk" +import { isEditableColumn } from "../../../sdk/app/tables/validation" export interface ManyRelationship { tableId?: string @@ -298,8 +299,7 @@ export class ExternalRequest { if ( row[key] == null || newRow[key] || - field.autocolumn || - field.type === FieldTypes.FORMULA + !sdk.tables.isEditableColumn(field) ) { continue } diff --git a/packages/server/src/sdk/app/tables/index.ts b/packages/server/src/sdk/app/tables/index.ts index 92ef3f3291..fcb32f387b 100644 --- a/packages/server/src/sdk/app/tables/index.ts +++ b/packages/server/src/sdk/app/tables/index.ts @@ -7,7 +7,7 @@ import { } from "../../../integrations/utils" import { Table, Database } from "@budibase/types" import datasources from "../datasources" -import { checkExternalTableSchemas } from "./validation" +import { checkExternalTableSchemas, isEditableColumn } from "./validation" async function getAllInternalTables(db?: Database): Promise { if (!db) { @@ -62,4 +62,5 @@ export default { getExternalTable, getTable, checkExternalTableSchemas, + isEditableColumn, } diff --git a/packages/server/src/sdk/app/tables/validation.ts b/packages/server/src/sdk/app/tables/validation.ts index 98ad72ea3d..52385afb25 100644 --- a/packages/server/src/sdk/app/tables/validation.ts +++ b/packages/server/src/sdk/app/tables/validation.ts @@ -1,9 +1,11 @@ import { + AutoReason, Datasource, + FieldSchema, FieldType, RelationshipTypes, - AutoReason, } from "@budibase/types" +import { FieldTypes } from "../../../constants" function checkForeignKeysAreAutoColumns(datasource: Datasource) { if (!datasource.entities) { @@ -39,7 +41,8 @@ function checkForeignKeysAreAutoColumns(datasource: Datasource) { const shouldBeForeign = foreignKeys.find( options => options.tableId === table._id && options.key === column.name ) - if (shouldBeForeign) { + // don't change already auto-columns to it, e.g. primary keys that are foreign + if (shouldBeForeign && !column.autocolumn) { column.autocolumn = true column.autoReason = AutoReason.FOREIGN_KEY } else if (column.autoReason === AutoReason.FOREIGN_KEY) { @@ -52,6 +55,13 @@ function checkForeignKeysAreAutoColumns(datasource: Datasource) { return datasource } +export function isEditableColumn(column: FieldSchema) { + const isAutoColumn = + column.autocolumn && column.autoReason !== AutoReason.FOREIGN_KEY + const isFormula = column.type === FieldTypes.FORMULA + return !(isAutoColumn || isFormula) +} + export function checkExternalTableSchemas(datasource: Datasource) { return checkForeignKeysAreAutoColumns(datasource) } From 1d5bc3928291f7f322ce44a0ef4dc16dec118e3d Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 27 Jun 2023 12:24:50 +0100 Subject: [PATCH 4/6] PR comments. --- .../server/src/api/controllers/datasource.ts | 10 +++-- .../src/api/controllers/table/external.ts | 4 +- packages/server/src/sdk/app/tables/index.ts | 4 +- .../sdk/app/tables/tests/validation.spec.ts | 41 ++++++++++++++++--- .../server/src/sdk/app/tables/validation.ts | 2 +- 5 files changed, 48 insertions(+), 13 deletions(-) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 062a185a23..f8836d1958 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -198,7 +198,9 @@ export async function buildSchemaFromDb(ctx: UserCtx) { } setDefaultDisplayColumns(datasource) - const dbResp = await db.put(sdk.tables.checkExternalTableSchemas(datasource)) + const dbResp = await db.put( + sdk.tables.populateExternalTableSchemas(datasource) + ) datasource._rev = dbResp.rev const cleanedDatasource = await sdk.datasources.removeSecretSingle(datasource) @@ -285,7 +287,7 @@ export async function update(ctx: UserCtx) { } const response = await db.put( - sdk.tables.checkExternalTableSchemas(datasource) + sdk.tables.populateExternalTableSchemas(datasource) ) await events.datasource.updated(datasource) datasource._rev = response.rev @@ -327,7 +329,9 @@ export async function save( setDefaultDisplayColumns(datasource) } - const dbResp = await db.put(sdk.tables.checkExternalTableSchemas(datasource)) + const dbResp = await db.put( + sdk.tables.populateExternalTableSchemas(datasource) + ) await events.datasource.created(datasource) datasource._rev = dbResp.rev diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index 8877f0f8d0..9029d0468a 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -319,7 +319,7 @@ export async function save(ctx: UserCtx) { delete tableToSave._rename // store it into couch now for budibase reference datasource.entities[tableToSave.name] = tableToSave - await db.put(sdk.tables.checkExternalTableSchemas(datasource)) + await db.put(sdk.tables.populateExternalTableSchemas(datasource)) // Since tables are stored inside datasources, we need to notify clients // that the datasource definition changed @@ -350,7 +350,7 @@ export async function destroy(ctx: UserCtx) { datasource.entities = tables } - await db.put(sdk.tables.checkExternalTableSchemas(datasource)) + await db.put(sdk.tables.populateExternalTableSchemas(datasource)) // Since tables are stored inside datasources, we need to notify clients // that the datasource definition changed diff --git a/packages/server/src/sdk/app/tables/index.ts b/packages/server/src/sdk/app/tables/index.ts index fcb32f387b..c7de1b327c 100644 --- a/packages/server/src/sdk/app/tables/index.ts +++ b/packages/server/src/sdk/app/tables/index.ts @@ -7,7 +7,7 @@ import { } from "../../../integrations/utils" import { Table, Database } from "@budibase/types" import datasources from "../datasources" -import { checkExternalTableSchemas, isEditableColumn } from "./validation" +import { populateExternalTableSchemas, isEditableColumn } from "./validation" async function getAllInternalTables(db?: Database): Promise { if (!db) { @@ -61,6 +61,6 @@ export default { getAllExternalTables, getExternalTable, getTable, - checkExternalTableSchemas, + populateExternalTableSchemas, isEditableColumn, } diff --git a/packages/server/src/sdk/app/tables/tests/validation.spec.ts b/packages/server/src/sdk/app/tables/tests/validation.spec.ts index 8acc315125..ffc34d0afd 100644 --- a/packages/server/src/sdk/app/tables/tests/validation.spec.ts +++ b/packages/server/src/sdk/app/tables/tests/validation.spec.ts @@ -1,6 +1,7 @@ -import { checkExternalTableSchemas } from "../validation" +import { populateExternalTableSchemas } from "../validation" import { cloneDeep } from "lodash/fp" -import { Datasource } from "@budibase/types" +import { Datasource, Table } from "@budibase/types" +import { isEqual } from "lodash" const SCHEMA = { entities: { @@ -71,28 +72,58 @@ const SCHEMA = { }, } +const OTHER_CLIENT_COLS = ["idC", "Name", "project"] +const OTHER_PROJECT_COLS = ["idP", "Name", "client"] + describe("validation and update of external table schemas", () => { function getForeignKeyColumn(datasource: Datasource) { return datasource.entities!["project"].schema.idC } + function checkOtherColumns( + table: Table, + compareTable: Table, + columnsToCheck: string[] + ) { + for (let columnName of columnsToCheck) { + const columnA = table.schema[columnName] + const columnB = table.schema[columnName] + expect(isEqual(columnA, columnB)).toBe(true) + } + } + + function noOtherTableChanges(response: any) { + checkOtherColumns( + response.entities!.client!, + SCHEMA.entities.client as Table, + OTHER_CLIENT_COLS + ) + checkOtherColumns( + response.entities!.project!, + SCHEMA.entities.project as Table, + OTHER_PROJECT_COLS + ) + } + it("should correctly set utilised foreign keys to autocolumns", () => { - const response = checkExternalTableSchemas(cloneDeep(SCHEMA) as any) + const response = populateExternalTableSchemas(cloneDeep(SCHEMA) as any) const foreignKey = getForeignKeyColumn(response) expect(foreignKey.autocolumn).toBe(true) expect(foreignKey.autoReason).toBe("foreign_key") + noOtherTableChanges(response) }) it("should correctly unset foreign keys when no longer used", () => { - const setResponse = checkExternalTableSchemas(cloneDeep(SCHEMA) as any) + const setResponse = populateExternalTableSchemas(cloneDeep(SCHEMA) as any) const beforeFk = getForeignKeyColumn(setResponse) delete setResponse.entities!.client.schema.project delete setResponse.entities!.project.schema.client - const response = checkExternalTableSchemas(cloneDeep(setResponse)) + const response = populateExternalTableSchemas(cloneDeep(setResponse)) const afterFk = getForeignKeyColumn(response) expect(beforeFk.autocolumn).toBe(true) expect(beforeFk.autoReason).toBe("foreign_key") expect(afterFk.autocolumn).toBeUndefined() expect(afterFk.autoReason).toBeUndefined() + noOtherTableChanges(response) }) }) diff --git a/packages/server/src/sdk/app/tables/validation.ts b/packages/server/src/sdk/app/tables/validation.ts index 52385afb25..e09380c309 100644 --- a/packages/server/src/sdk/app/tables/validation.ts +++ b/packages/server/src/sdk/app/tables/validation.ts @@ -62,6 +62,6 @@ export function isEditableColumn(column: FieldSchema) { return !(isAutoColumn || isFormula) } -export function checkExternalTableSchemas(datasource: Datasource) { +export function populateExternalTableSchemas(datasource: Datasource) { return checkForeignKeysAreAutoColumns(datasource) } From ba55de39a25b64ab42b14a331a059f473b252e30 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 27 Jun 2023 15:20:21 +0000 Subject: [PATCH 5/6] Bump version to 2.7.36 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 65876a2c4d..a59a94209d 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.7.35", + "version": "2.7.36", "npmClient": "yarn", "packages": [ "packages/backend-core", From 0d1941f26413634ccaa4cad4c6d500750c55523c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 30 Jun 2023 11:35:59 +0100 Subject: [PATCH 6/6] Fix merge conflict. --- packages/server/src/api/controllers/datasource.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index c0aff2b4a9..069842a020 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -344,17 +344,13 @@ export async function save( setDefaultDisplayColumns(datasource) } -<<<<<<< HEAD if (preSaveAction[datasource.source]) { await preSaveAction[datasource.source](datasource) } - const dbResp = await db.put(datasource) -======= const dbResp = await db.put( sdk.tables.populateExternalTableSchemas(datasource) ) ->>>>>>> d4af3bf8029c38e19b95a4051c08fa72741ea142 await events.datasource.created(datasource) datasource._rev = dbResp.rev