From a1b91da40f6bea16b81000ba9e7d0711c4b99f12 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 10:40:16 +0200 Subject: [PATCH 01/13] Add basic fetch schema test --- .../src/api/routes/tests/datasource.spec.ts | 21 +++++++++++++++++++ .../server/src/api/routes/tests/table.spec.ts | 4 +--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index 0066be2a64..f5e31b6a1a 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -7,6 +7,7 @@ import sdk from "../../../sdk" import tk from "timekeeper" import { mocks } from "@budibase/backend-core/tests" import { QueryPreview, SourceName } from "@budibase/types" +import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" tk.freeze(mocks.date.MOCK_DATE) @@ -223,4 +224,24 @@ describe("/datasources", () => { }) }) }) + + describe.only.each([ + [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], + [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], + [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], + [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], + ])("fetch schema (%s)", (_, dsProvider) => { + beforeAll(async () => { + datasource = await config.api.datasource.create(await dsProvider) + }) + + it("aa", async () => { + const datasourceId = datasource!._id! + const persisted = await config.api.datasource.get(datasourceId) + await config.api.datasource.fetchSchema(datasourceId) + + const updated = await config.api.datasource.get(datasourceId) + expect(updated).toEqual(persisted) + }) + }) }) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 77e05b8e07..019b392408 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -34,7 +34,7 @@ describe.each([ [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], ])("/tables (%s)", (_, dsProvider) => { - let isInternal: boolean + const isInternal: boolean = !dsProvider let datasource: Datasource | undefined let config = setup.getConfig() @@ -44,9 +44,7 @@ describe.each([ await config.init() if (dsProvider) { datasource = await config.api.datasource.create(await dsProvider) - isInternal = false } else { - isInternal = true } }) From c34cd470ee34ac9537bc8f093de24e7beb9c45b9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 11:05:36 +0200 Subject: [PATCH 02/13] Fix dropping columns existing only internally --- packages/server/src/integrations/utils/utils.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/server/src/integrations/utils/utils.ts b/packages/server/src/integrations/utils/utils.ts index cc75f0444d..ba8d288a55 100644 --- a/packages/server/src/integrations/utils/utils.ts +++ b/packages/server/src/integrations/utils/utils.ts @@ -353,6 +353,7 @@ function copyExistingPropsOver( // If the db column type changed to a non-compatible one, we want to re-fetch it if ( + updatedColumnType && updatedColumnType !== existingColumnType && !SWITCHABLE_TYPES[updatedColumnType]?.includes(existingColumnType) ) { @@ -384,7 +385,7 @@ function copyExistingPropsOver( ...existingTableSchema[key], externalType: existingTableSchema[key].externalType || - table.schema[key].externalType, + table.schema[key]?.externalType, } } } From e03975462e56a15dc280e90d9fc3a6e2a4609e6d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 11:09:20 +0200 Subject: [PATCH 03/13] Improve tests --- .../src/api/routes/tests/datasource.spec.ts | 147 +++++++++++++++++- .../src/tests/utilities/api/datasource.ts | 10 ++ .../types/src/documents/app/datasource.ts | 4 +- 3 files changed, 153 insertions(+), 8 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index f5e31b6a1a..1e463115c2 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -5,9 +5,19 @@ import { context, events } from "@budibase/backend-core" import sdk from "../../../sdk" import tk from "timekeeper" -import { mocks } from "@budibase/backend-core/tests" -import { QueryPreview, SourceName } from "@budibase/types" +import { generator, mocks } from "@budibase/backend-core/tests" +import { + Datasource, + FieldSchema, + FieldType, + QueryPreview, + RelationshipType, + SourceName, + Table, + TableSchema, +} from "@budibase/types" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" +import { tableForDatasource } from "../../../tests/utilities/structures" tk.freeze(mocks.date.MOCK_DATE) @@ -225,7 +235,7 @@ describe("/datasources", () => { }) }) - describe.only.each([ + describe.each([ [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], @@ -235,13 +245,140 @@ describe("/datasources", () => { datasource = await config.api.datasource.create(await dsProvider) }) - it("aa", async () => { + it("fetching schema will not drop tables or columns", async () => { const datasourceId = datasource!._id! + + const simpleTable = await config.api.table.save( + tableForDatasource(datasource, { + name: generator.guid(), + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + }, + }) + ) + const fullSchema: { [type in FieldType]: FieldSchema & { type: type } } = + { + [FieldType.STRING]: { + name: "string", + type: FieldType.STRING, + }, + // [FieldType.LONGFORM]: { + // name: "longform", + // type: FieldType.LONGFORM, + // }, + // [FieldType.OPTIONS]: { + // name: "options", + // type: FieldType.OPTIONS, + // }, + [FieldType.NUMBER]: { + name: "number", + type: FieldType.NUMBER, + }, + // [FieldType.BOOLEAN]: { + // name: "boolean", + // type: FieldType.BOOLEAN, + // }, + // [FieldType.ARRAY]: { + // name: "array", + // type: FieldType.ARRAY, + // }, + // [FieldType.DATETIME]: { + // name: "datetime", + // type: FieldType.DATETIME, + // }, + // [FieldType.ATTACHMENTS]: { + // name: "attachments", + // type: FieldType.ATTACHMENTS, + // }, + // [FieldType.ATTACHMENT_SINGLE]: { + // name: "attachment_single", + // type: FieldType.ATTACHMENT_SINGLE, + // }, + // [FieldType.LINK]: { + // name: "link", + // type: FieldType.LINK, + // tableId: simpleTable._id!, + // relationshipType: RelationshipType.ONE_TO_MANY, + // fieldName: "link", + // foreignKey: "fk", + // }, + // [FieldType.FORMULA]: { + // name: "formula", + // type: FieldType.FORMULA, + // formula: "any formula", + // }, + // [FieldType.AUTO]: { + // name: "auto", + // type: FieldType.AUTO, + // }, + // [FieldType.JSON]: { + // name: "json", + // type: FieldType.JSON, + // }, + // [FieldType.INTERNAL]: { + // name: "internal", + // type: FieldType.INTERNAL, + // }, + // [FieldType.BARCODEQR]: { + // name: "barcodeqr", + // type: FieldType.BARCODEQR, + // }, + // [FieldType.BIGINT]: { + // name: "bigint", + // type: FieldType.BIGINT, + // }, + // [FieldType.BB_REFERENCE]: { + // name: "bb_reference", + // type: FieldType.BB_REFERENCE, + // }, + } + + const fullTable = await config.api.table.save( + tableForDatasource(datasource, { + name: generator.guid(), + schema: fullSchema, + }) + ) + const persisted = await config.api.datasource.get(datasourceId) await config.api.datasource.fetchSchema(datasourceId) const updated = await config.api.datasource.get(datasourceId) - expect(updated).toEqual(persisted) + const expected: Datasource = { + ...persisted, + entities: + persisted?.entities && + Object.entries(persisted.entities).reduce>( + (acc, [tableName, table]) => { + acc[tableName] = { + ...table, + primaryDisplay: expect.not.stringMatching( + new RegExp(`^${table.primaryDisplay || ""}$`) + ), + schema: Object.entries(table.schema).reduce( + (acc, [fieldName, field]) => { + acc[fieldName] = expect.objectContaining({ + ...field, + externalType: expect.not.stringMatching( + new RegExp(`^${field.externalType || ""}$`) + ), + }) + return acc + }, + {} + ), + } + return acc + }, + {} + ), + + _rev: expect.any(String), + } + expect(updated).toEqual(expected) }) }) }) diff --git a/packages/server/src/tests/utilities/api/datasource.ts b/packages/server/src/tests/utilities/api/datasource.ts index 6ac624f0db..bb4c74093c 100644 --- a/packages/server/src/tests/utilities/api/datasource.ts +++ b/packages/server/src/tests/utilities/api/datasource.ts @@ -5,6 +5,7 @@ import { UpdateDatasourceResponse, UpdateDatasourceRequest, QueryJson, + BuildSchemaFromSourceResponse, } from "@budibase/types" import { Expectations, TestAPI } from "./base" @@ -69,4 +70,13 @@ export class DatasourceAPI extends TestAPI { expectations, }) } + + fetchSchema = async (id: string, expectations?: Expectations) => { + return await this._post( + `/api/datasources/${id}/schema`, + { + expectations, + } + ) + } } diff --git a/packages/types/src/documents/app/datasource.ts b/packages/types/src/documents/app/datasource.ts index 8976e1cae3..32f5bbb132 100644 --- a/packages/types/src/documents/app/datasource.ts +++ b/packages/types/src/documents/app/datasource.ts @@ -13,9 +13,7 @@ export interface Datasource extends Document { config?: Record plus?: boolean isSQL?: boolean - entities?: { - [key: string]: Table - } + entities?: Record } export enum RestAuthType { From 1732e143539c6dfc191d8402256b5cf1fe6beccb Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 11:12:52 +0200 Subject: [PATCH 04/13] Add extra fields --- .../src/api/routes/tests/datasource.spec.ts | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index 1e463115c2..0e1b20d379 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -277,18 +277,18 @@ describe("/datasources", () => { name: "number", type: FieldType.NUMBER, }, - // [FieldType.BOOLEAN]: { - // name: "boolean", - // type: FieldType.BOOLEAN, - // }, + [FieldType.BOOLEAN]: { + name: "boolean", + type: FieldType.BOOLEAN, + }, // [FieldType.ARRAY]: { // name: "array", // type: FieldType.ARRAY, // }, - // [FieldType.DATETIME]: { - // name: "datetime", - // type: FieldType.DATETIME, - // }, + [FieldType.DATETIME]: { + name: "datetime", + type: FieldType.DATETIME, + }, // [FieldType.ATTACHMENTS]: { // name: "attachments", // type: FieldType.ATTACHMENTS, @@ -322,10 +322,10 @@ describe("/datasources", () => { // name: "internal", // type: FieldType.INTERNAL, // }, - // [FieldType.BARCODEQR]: { - // name: "barcodeqr", - // type: FieldType.BARCODEQR, - // }, + [FieldType.BARCODEQR]: { + name: "barcodeqr", + type: FieldType.BARCODEQR, + }, // [FieldType.BIGINT]: { // name: "bigint", // type: FieldType.BIGINT, From 168556808944f39585b2fdf27daaf742026db8be Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 12:38:57 +0200 Subject: [PATCH 05/13] Improve and fix test --- .../src/api/routes/tests/datasource.spec.ts | 160 +++++++++--------- .../server/src/integrations/utils/utils.ts | 160 +++++++++++++----- 2 files changed, 197 insertions(+), 123 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index 0e1b20d379..0bdbbee27a 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -5,10 +5,11 @@ import { context, events } from "@budibase/backend-core" import sdk from "../../../sdk" import tk from "timekeeper" -import { generator, mocks } from "@budibase/backend-core/tests" +import { mocks } from "@budibase/backend-core/tests" import { Datasource, FieldSchema, + FieldSubtype, FieldType, QueryPreview, RelationshipType, @@ -250,7 +251,7 @@ describe("/datasources", () => { const simpleTable = await config.api.table.save( tableForDatasource(datasource, { - name: generator.guid(), + name: "simple", schema: { name: { name: "name", @@ -259,86 +260,82 @@ describe("/datasources", () => { }, }) ) - const fullSchema: { [type in FieldType]: FieldSchema & { type: type } } = - { - [FieldType.STRING]: { - name: "string", - type: FieldType.STRING, - }, - // [FieldType.LONGFORM]: { - // name: "longform", - // type: FieldType.LONGFORM, - // }, - // [FieldType.OPTIONS]: { - // name: "options", - // type: FieldType.OPTIONS, - // }, - [FieldType.NUMBER]: { - name: "number", - type: FieldType.NUMBER, - }, - [FieldType.BOOLEAN]: { - name: "boolean", - type: FieldType.BOOLEAN, - }, - // [FieldType.ARRAY]: { - // name: "array", - // type: FieldType.ARRAY, - // }, - [FieldType.DATETIME]: { - name: "datetime", - type: FieldType.DATETIME, - }, - // [FieldType.ATTACHMENTS]: { - // name: "attachments", - // type: FieldType.ATTACHMENTS, - // }, - // [FieldType.ATTACHMENT_SINGLE]: { - // name: "attachment_single", - // type: FieldType.ATTACHMENT_SINGLE, - // }, - // [FieldType.LINK]: { - // name: "link", - // type: FieldType.LINK, - // tableId: simpleTable._id!, - // relationshipType: RelationshipType.ONE_TO_MANY, - // fieldName: "link", - // foreignKey: "fk", - // }, - // [FieldType.FORMULA]: { - // name: "formula", - // type: FieldType.FORMULA, - // formula: "any formula", - // }, - // [FieldType.AUTO]: { - // name: "auto", - // type: FieldType.AUTO, - // }, - // [FieldType.JSON]: { - // name: "json", - // type: FieldType.JSON, - // }, - // [FieldType.INTERNAL]: { - // name: "internal", - // type: FieldType.INTERNAL, - // }, - [FieldType.BARCODEQR]: { - name: "barcodeqr", - type: FieldType.BARCODEQR, - }, - // [FieldType.BIGINT]: { - // name: "bigint", - // type: FieldType.BIGINT, - // }, - // [FieldType.BB_REFERENCE]: { - // name: "bb_reference", - // type: FieldType.BB_REFERENCE, - // }, - } - const fullTable = await config.api.table.save( + type SupportedTypes = + | FieldType.STRING + | FieldType.BARCODEQR + | FieldType.LONGFORM + | FieldType.OPTIONS + | FieldType.DATETIME + | FieldType.NUMBER + | FieldType.BOOLEAN + | FieldType.FORMULA + | FieldType.BIGINT + | FieldType.BB_REFERENCE + | FieldType.LINK + | FieldType.ARRAY + + const fullSchema: { + [type in SupportedTypes]: FieldSchema & { type: type } + } = { + [FieldType.STRING]: { + name: "string", + type: FieldType.STRING, + }, + [FieldType.LONGFORM]: { + name: "longform", + type: FieldType.LONGFORM, + }, + [FieldType.OPTIONS]: { + name: "options", + type: FieldType.OPTIONS, + }, + [FieldType.NUMBER]: { + name: "number", + type: FieldType.NUMBER, + }, + [FieldType.BOOLEAN]: { + name: "boolean", + type: FieldType.BOOLEAN, + }, + [FieldType.ARRAY]: { + name: "array", + type: FieldType.ARRAY, + }, + [FieldType.DATETIME]: { + name: "datetime", + type: FieldType.DATETIME, + }, + [FieldType.LINK]: { + name: "link", + type: FieldType.LINK, + tableId: simpleTable._id!, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + }, + [FieldType.FORMULA]: { + name: "formula", + type: FieldType.FORMULA, + formula: "any formula", + }, + [FieldType.BARCODEQR]: { + name: "barcodeqr", + type: FieldType.BARCODEQR, + }, + [FieldType.BIGINT]: { + name: "bigint", + type: FieldType.BIGINT, + }, + [FieldType.BB_REFERENCE]: { + name: "bb_reference", + type: FieldType.BB_REFERENCE, + subtype: FieldSubtype.USERS, + }, + } + + await config.api.table.save( tableForDatasource(datasource, { - name: generator.guid(), + name: "full", schema: fullSchema, }) ) @@ -362,9 +359,6 @@ describe("/datasources", () => { (acc, [fieldName, field]) => { acc[fieldName] = expect.objectContaining({ ...field, - externalType: expect.not.stringMatching( - new RegExp(`^${field.externalType || ""}$`) - ), }) return acc }, diff --git a/packages/server/src/integrations/utils/utils.ts b/packages/server/src/integrations/utils/utils.ts index ba8d288a55..8d919159d8 100644 --- a/packages/server/src/integrations/utils/utils.ts +++ b/packages/server/src/integrations/utils/utils.ts @@ -15,7 +15,28 @@ const DOUBLE_SEPARATOR = `${SEPARATOR}${SEPARATOR}` const ROW_ID_REGEX = /^\[.*]$/g const ENCODED_SPACE = encodeURIComponent(" ") -const SQL_NUMBER_TYPE_MAP = { +type PrimitiveTypes = + | FieldType.STRING + | FieldType.NUMBER + | FieldType.BOOLEAN + | FieldType.DATETIME + | FieldType.JSON + | FieldType.BIGINT + | FieldType.OPTIONS + +function isPrimitiveType(type: FieldType): type is PrimitiveTypes { + return [ + FieldType.STRING, + FieldType.NUMBER, + FieldType.BOOLEAN, + FieldType.DATETIME, + FieldType.JSON, + FieldType.BIGINT, + FieldType.OPTIONS, + ].includes(type) +} + +const SQL_NUMBER_TYPE_MAP: Record = { integer: FieldType.NUMBER, int: FieldType.NUMBER, decimal: FieldType.NUMBER, @@ -35,7 +56,7 @@ const SQL_NUMBER_TYPE_MAP = { smallmoney: FieldType.NUMBER, } -const SQL_DATE_TYPE_MAP = { +const SQL_DATE_TYPE_MAP: Record = { timestamp: FieldType.DATETIME, time: FieldType.DATETIME, datetime: FieldType.DATETIME, @@ -46,7 +67,7 @@ const SQL_DATE_TYPE_MAP = { const SQL_DATE_ONLY_TYPES = ["date"] const SQL_TIME_ONLY_TYPES = ["time"] -const SQL_STRING_TYPE_MAP = { +const SQL_STRING_TYPE_MAP: Record = { varchar: FieldType.STRING, char: FieldType.STRING, nchar: FieldType.STRING, @@ -58,22 +79,22 @@ const SQL_STRING_TYPE_MAP = { text: FieldType.STRING, } -const SQL_BOOLEAN_TYPE_MAP = { +const SQL_BOOLEAN_TYPE_MAP: Record = { boolean: FieldType.BOOLEAN, bit: FieldType.BOOLEAN, tinyint: FieldType.BOOLEAN, } -const SQL_OPTIONS_TYPE_MAP = { +const SQL_OPTIONS_TYPE_MAP: Record = { "user-defined": FieldType.OPTIONS, } -const SQL_MISC_TYPE_MAP = { +const SQL_MISC_TYPE_MAP: Record = { json: FieldType.JSON, bigint: FieldType.BIGINT, } -const SQL_TYPE_MAP = { +const SQL_TYPE_MAP: Record = { ...SQL_NUMBER_TYPE_MAP, ...SQL_DATE_TYPE_MAP, ...SQL_STRING_TYPE_MAP, @@ -317,6 +338,80 @@ function shouldCopySpecialColumn( return fetchedIsNumber && column.type === FieldType.BOOLEAN } +enum CopyAction { + ALWAYS_KEEP = "alwaysKeep", + COPY_IF_TYPE = "copyIfType", +} + +SQL_TYPE_MAP + +const SqlCopyTypeByFieldMapping: Record< + FieldType, + | { action: CopyAction.ALWAYS_KEEP } + | { action: CopyAction.COPY_IF_TYPE; types: PrimitiveTypes[] } +> = { + [FieldType.LINK]: { action: CopyAction.ALWAYS_KEEP }, + [FieldType.FORMULA]: { action: CopyAction.ALWAYS_KEEP }, + [FieldType.STRING]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.STRING], + }, + [FieldType.OPTIONS]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.STRING], + }, + [FieldType.LONGFORM]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.STRING], + }, + [FieldType.NUMBER]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.BOOLEAN, FieldType.NUMBER], + }, + [FieldType.BOOLEAN]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.BOOLEAN, FieldType.NUMBER], + }, + [FieldType.ARRAY]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.JSON, FieldType.STRING], + }, + [FieldType.DATETIME]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.DATETIME, FieldType.STRING], + }, + [FieldType.ATTACHMENTS]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.JSON, FieldType.STRING], + }, + [FieldType.ATTACHMENT_SINGLE]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.JSON, FieldType.STRING], + }, + [FieldType.AUTO]: { + action: CopyAction.ALWAYS_KEEP, + }, + [FieldType.JSON]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.JSON, FieldType.STRING], + }, + [FieldType.INTERNAL]: { + action: CopyAction.ALWAYS_KEEP, + }, + [FieldType.BARCODEQR]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.STRING], + }, + [FieldType.BIGINT]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.BIGINT, FieldType.NUMBER], + }, + [FieldType.BB_REFERENCE]: { + action: CopyAction.COPY_IF_TYPE, + types: [FieldType.JSON, FieldType.STRING], + }, +} + /** * Looks for columns which need to be copied over into the new table definitions, like relationships, * options types and views. @@ -338,6 +433,9 @@ function copyExistingPropsOver( if (entities[tableName]?.created) { table.created = entities[tableName]?.created } + if (entities[tableName]?.constrained) { + table.constrained = entities[tableName]?.constrained + } table.views = entities[tableName].views @@ -351,41 +449,23 @@ function copyExistingPropsOver( const existingColumnType = column?.type const updatedColumnType = table.schema[key]?.type - // If the db column type changed to a non-compatible one, we want to re-fetch it - if ( - updatedColumnType && - updatedColumnType !== existingColumnType && - !SWITCHABLE_TYPES[updatedColumnType]?.includes(existingColumnType) - ) { - continue + const map = SqlCopyTypeByFieldMapping[existingColumnType] + + let keepExistingSchema = map.action === CopyAction.ALWAYS_KEEP + if (map.action === CopyAction.COPY_IF_TYPE) { + keepExistingSchema = + isPrimitiveType(updatedColumnType) && + table.schema[key] && + map.types?.includes(updatedColumnType) } - if ( - column.type === FieldType.LINK && - !shouldCopyRelationship(column, tableIds) - ) { - continue - } - - const specialTypes = [ - FieldType.OPTIONS, - FieldType.LONGFORM, - FieldType.ARRAY, - FieldType.FORMULA, - FieldType.BB_REFERENCE, - ] - if ( - specialTypes.includes(column.type) && - !shouldCopySpecialColumn(column, table.schema[key]) - ) { - continue - } - - table.schema[key] = { - ...existingTableSchema[key], - externalType: - existingTableSchema[key].externalType || - table.schema[key]?.externalType, + if (keepExistingSchema) { + table.schema[key] = { + ...existingTableSchema[key], + externalType: + existingTableSchema[key].externalType || + table.schema[key]?.externalType, + } } } } From 72c65cd7fd6e217268713471a48ca96000fbcd91 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 12:39:04 +0200 Subject: [PATCH 06/13] Fix test --- packages/server/src/sdk/app/tables/validation.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/tables/validation.ts b/packages/server/src/sdk/app/tables/validation.ts index 1609bdfcda..26e1cc8ef2 100644 --- a/packages/server/src/sdk/app/tables/validation.ts +++ b/packages/server/src/sdk/app/tables/validation.ts @@ -40,8 +40,11 @@ function checkForeignKeysAreAutoColumns(datasource: Datasource) { const shouldBeForeign = foreignKeys.find( options => options.tableId === table._id && options.key === column.name ) + if (column.autocolumn) { + continue + } // don't change already auto-columns to it, e.g. primary keys that are foreign - if (shouldBeForeign && !column.autocolumn) { + if (shouldBeForeign) { column.autocolumn = true column.autoReason = AutoReason.FOREIGN_KEY } else if (column.autoReason === AutoReason.FOREIGN_KEY) { From 16d2c06b8a33144a3364a581b86133a2a9feecc6 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 12:40:39 +0200 Subject: [PATCH 07/13] Add constraints and extra fields --- packages/server/src/api/routes/tests/datasource.spec.ts | 8 ++++++++ packages/server/src/integrations/utils/utils.ts | 5 +++-- packages/types/src/documents/app/table/schema.ts | 1 + 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index 0bdbbee27a..ec438611d4 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -281,6 +281,9 @@ describe("/datasources", () => { [FieldType.STRING]: { name: "string", type: FieldType.STRING, + constraints: { + presence: true, + }, }, [FieldType.LONGFORM]: { name: "longform", @@ -289,6 +292,9 @@ describe("/datasources", () => { [FieldType.OPTIONS]: { name: "options", type: FieldType.OPTIONS, + constraints: { + presence: { allowEmpty: false }, + }, }, [FieldType.NUMBER]: { name: "number", @@ -305,6 +311,8 @@ describe("/datasources", () => { [FieldType.DATETIME]: { name: "datetime", type: FieldType.DATETIME, + dateOnly: true, + timeOnly: false, }, [FieldType.LINK]: { name: "link", diff --git a/packages/server/src/integrations/utils/utils.ts b/packages/server/src/integrations/utils/utils.ts index 8d919159d8..e6d5bb872f 100644 --- a/packages/server/src/integrations/utils/utils.ts +++ b/packages/server/src/integrations/utils/utils.ts @@ -4,6 +4,7 @@ import { Datasource, FieldType, TableSourceType, + FieldSchema, } from "@budibase/types" import { DocumentType, SEPARATOR } from "../../db/utils" import { InvalidColumns, DEFAULT_BB_DATASOURCE_ID } from "../../constants" @@ -260,14 +261,14 @@ export function generateColumnDefinition(config: { constraints.inclusion = options } - const schema: any = { + const schema: FieldSchema = { type: foundType, externalType, autocolumn, name, constraints, } - if (foundType === FieldType.DATETIME) { + if (schema.type === FieldType.DATETIME) { schema.dateOnly = SQL_DATE_ONLY_TYPES.includes(lowerCaseType) schema.timeOnly = SQL_TIME_ONLY_TYPES.includes(lowerCaseType) } diff --git a/packages/types/src/documents/app/table/schema.ts b/packages/types/src/documents/app/table/schema.ts index 86c34b6a5c..63a5876bc0 100644 --- a/packages/types/src/documents/app/table/schema.ts +++ b/packages/types/src/documents/app/table/schema.ts @@ -91,6 +91,7 @@ export interface DateFieldMetadata extends Omit { type: FieldType.DATETIME ignoreTimezones?: boolean timeOnly?: boolean + dateOnly?: boolean subtype?: AutoFieldSubType.CREATED_AT | AutoFieldSubType.UPDATED_AT } From 874c698776c38783798df7e5686523a608e6a648 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 12:52:25 +0200 Subject: [PATCH 08/13] lint --- packages/server/src/api/routes/tests/table.spec.ts | 1 - packages/server/src/integrations/utils/utils.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 019b392408..ede1e7af94 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -44,7 +44,6 @@ describe.each([ await config.init() if (dsProvider) { datasource = await config.api.datasource.create(await dsProvider) - } else { } }) diff --git a/packages/server/src/integrations/utils/utils.ts b/packages/server/src/integrations/utils/utils.ts index e6d5bb872f..ddea9f3573 100644 --- a/packages/server/src/integrations/utils/utils.ts +++ b/packages/server/src/integrations/utils/utils.ts @@ -8,7 +8,7 @@ import { } from "@budibase/types" import { DocumentType, SEPARATOR } from "../../db/utils" import { InvalidColumns, DEFAULT_BB_DATASOURCE_ID } from "../../constants" -import { SWITCHABLE_TYPES, helpers } from "@budibase/shared-core" +import { helpers } from "@budibase/shared-core" import env from "../../environment" import { Knex } from "knex" From 11ef351400889583d6e88884233daffdf2ecb678 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 12:56:48 +0200 Subject: [PATCH 09/13] Fix dropping links --- .../server/src/integrations/utils/utils.ts | 50 +++---------------- 1 file changed, 6 insertions(+), 44 deletions(-) diff --git a/packages/server/src/integrations/utils/utils.ts b/packages/server/src/integrations/utils/utils.ts index ddea9f3573..c00cca5f23 100644 --- a/packages/server/src/integrations/utils/utils.ts +++ b/packages/server/src/integrations/utils/utils.ts @@ -296,49 +296,6 @@ export function isIsoDateString(str: string) { return d.toISOString() === trimmedValue } -/** - * This function will determine whether a column is a relationship and whether it - * is currently valid. The reason for the validity check is that tables can be deleted - * outside of Budibase control and if this is the case it will break Budibase relationships. - * The tableIds is a list passed down from the main finalise tables function, which is - * based on the tables that have just been fetched. This will only really be used on subsequent - * fetches to the first one - if the user is periodically refreshing Budibase knowledge of tables. - * @param column The column to check, to see if it is a valid relationship. - * @param tableIds The IDs of the tables which currently exist. - */ -function shouldCopyRelationship( - column: { type: FieldType.LINK; tableId?: string }, - tableIds: string[] -) { - return ( - column.type === FieldType.LINK && - column.tableId && - tableIds.includes(column.tableId) - ) -} - -/** - * Similar function to the shouldCopyRelationship function, but instead this looks for options and boolean - * types. It is possible to switch a string -> options and a number -> boolean (and vice versus) need to make - * sure that these get copied over when tables are fetched. Also checks whether they are still valid, if a - * column has changed type in the external database then copying it over may not be possible. - * @param column The column to check for options or boolean type. - * @param fetchedColumn The fetched column to check for the type in the external database. - */ -function shouldCopySpecialColumn( - column: { type: FieldType }, - fetchedColumn: { type: FieldType } | undefined -) { - const isFormula = column.type === FieldType.FORMULA - // column has been deleted, remove - formulas will never exist, always copy - if (!isFormula && column && !fetchedColumn) { - return false - } - const fetchedIsNumber = - !fetchedColumn || fetchedColumn.type === FieldType.NUMBER - return fetchedIsNumber && column.type === FieldType.BOOLEAN -} - enum CopyAction { ALWAYS_KEEP = "alwaysKeep", COPY_IF_TYPE = "copyIfType", @@ -454,10 +411,15 @@ function copyExistingPropsOver( let keepExistingSchema = map.action === CopyAction.ALWAYS_KEEP if (map.action === CopyAction.COPY_IF_TYPE) { + const shouldDropLink = + existingColumnType === FieldType.LINK && + !tableIds.includes(column.tableId) + keepExistingSchema = isPrimitiveType(updatedColumnType) && table.schema[key] && - map.types?.includes(updatedColumnType) + map.types?.includes(updatedColumnType) && + !shouldDropLink } if (keepExistingSchema) { From b05f56222e87adf4d8687466dc64ac71903fddce Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 13:13:12 +0200 Subject: [PATCH 10/13] Fix tests --- .../server/src/sdk/app/tables/tests/validation.spec.ts | 4 ++-- packages/server/src/sdk/app/tables/validation.ts | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) 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 66b4222005..6f8efbaed1 100644 --- a/packages/server/src/sdk/app/tables/tests/validation.spec.ts +++ b/packages/server/src/sdk/app/tables/tests/validation.spec.ts @@ -125,7 +125,7 @@ describe("validation and update of external table schemas", () => { } it("should correctly set utilised foreign keys to autocolumns", () => { - const response = populateExternalTableSchemas(cloneDeep(SCHEMA) as any) + const response = populateExternalTableSchemas(cloneDeep(SCHEMA)) const foreignKey = getForeignKeyColumn(response) expect(foreignKey.autocolumn).toBe(true) expect(foreignKey.autoReason).toBe(AutoReason.FOREIGN_KEY) @@ -133,7 +133,7 @@ describe("validation and update of external table schemas", () => { }) it("should correctly unset foreign keys when no longer used", () => { - const setResponse = populateExternalTableSchemas(cloneDeep(SCHEMA) as any) + const setResponse = populateExternalTableSchemas(cloneDeep(SCHEMA)) const beforeFk = getForeignKeyColumn(setResponse) delete setResponse.entities!.client.schema.project delete setResponse.entities!.project.schema.client diff --git a/packages/server/src/sdk/app/tables/validation.ts b/packages/server/src/sdk/app/tables/validation.ts index 26e1cc8ef2..d71a156fdb 100644 --- a/packages/server/src/sdk/app/tables/validation.ts +++ b/packages/server/src/sdk/app/tables/validation.ts @@ -40,14 +40,14 @@ function checkForeignKeysAreAutoColumns(datasource: Datasource) { const shouldBeForeign = foreignKeys.find( options => options.tableId === table._id && options.key === column.name ) - if (column.autocolumn) { - continue - } // don't change already auto-columns to it, e.g. primary keys that are foreign - if (shouldBeForeign) { + if (shouldBeForeign && !column.autocolumn) { column.autocolumn = true column.autoReason = AutoReason.FOREIGN_KEY - } else if (column.autoReason === AutoReason.FOREIGN_KEY) { + } else if ( + !shouldBeForeign && + column.autoReason === AutoReason.FOREIGN_KEY + ) { delete column.autocolumn delete column.autoReason } From 1af0eaae9385f0a02fca1328b3e07cdeb5324604 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 19 Apr 2024 12:36:05 +0100 Subject: [PATCH 11/13] PR comment. --- packages/server/src/api/routes/tests/datasource.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index ec438611d4..d5c0a256a1 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -261,7 +261,7 @@ describe("/datasources", () => { }) ) - type SupportedTypes = + type SupportedSqlTypes = | FieldType.STRING | FieldType.BARCODEQR | FieldType.LONGFORM @@ -276,7 +276,7 @@ describe("/datasources", () => { | FieldType.ARRAY const fullSchema: { - [type in SupportedTypes]: FieldSchema & { type: type } + [type in SupportedSqlTypes]: FieldSchema & { type: type } } = { [FieldType.STRING]: { name: "string", From 2bce7424f1b876895963cfa596477ff0f830d883 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 13:42:59 +0200 Subject: [PATCH 12/13] Refactor logic --- .../server/src/integrations/utils/utils.ts | 129 ++++++------------ 1 file changed, 43 insertions(+), 86 deletions(-) diff --git a/packages/server/src/integrations/utils/utils.ts b/packages/server/src/integrations/utils/utils.ts index c00cca5f23..0a71c85905 100644 --- a/packages/server/src/integrations/utils/utils.ts +++ b/packages/server/src/integrations/utils/utils.ts @@ -296,80 +296,6 @@ export function isIsoDateString(str: string) { return d.toISOString() === trimmedValue } -enum CopyAction { - ALWAYS_KEEP = "alwaysKeep", - COPY_IF_TYPE = "copyIfType", -} - -SQL_TYPE_MAP - -const SqlCopyTypeByFieldMapping: Record< - FieldType, - | { action: CopyAction.ALWAYS_KEEP } - | { action: CopyAction.COPY_IF_TYPE; types: PrimitiveTypes[] } -> = { - [FieldType.LINK]: { action: CopyAction.ALWAYS_KEEP }, - [FieldType.FORMULA]: { action: CopyAction.ALWAYS_KEEP }, - [FieldType.STRING]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.STRING], - }, - [FieldType.OPTIONS]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.STRING], - }, - [FieldType.LONGFORM]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.STRING], - }, - [FieldType.NUMBER]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.BOOLEAN, FieldType.NUMBER], - }, - [FieldType.BOOLEAN]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.BOOLEAN, FieldType.NUMBER], - }, - [FieldType.ARRAY]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.JSON, FieldType.STRING], - }, - [FieldType.DATETIME]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.DATETIME, FieldType.STRING], - }, - [FieldType.ATTACHMENTS]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.JSON, FieldType.STRING], - }, - [FieldType.ATTACHMENT_SINGLE]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.JSON, FieldType.STRING], - }, - [FieldType.AUTO]: { - action: CopyAction.ALWAYS_KEEP, - }, - [FieldType.JSON]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.JSON, FieldType.STRING], - }, - [FieldType.INTERNAL]: { - action: CopyAction.ALWAYS_KEEP, - }, - [FieldType.BARCODEQR]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.STRING], - }, - [FieldType.BIGINT]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.BIGINT, FieldType.NUMBER], - }, - [FieldType.BB_REFERENCE]: { - action: CopyAction.COPY_IF_TYPE, - types: [FieldType.JSON, FieldType.STRING], - }, -} - /** * Looks for columns which need to be copied over into the new table definitions, like relationships, * options types and views. @@ -402,27 +328,58 @@ function copyExistingPropsOver( if (!Object.prototype.hasOwnProperty.call(existingTableSchema, key)) { continue } + const column = existingTableSchema[key] const existingColumnType = column?.type const updatedColumnType = table.schema[key]?.type - const map = SqlCopyTypeByFieldMapping[existingColumnType] - - let keepExistingSchema = map.action === CopyAction.ALWAYS_KEEP - if (map.action === CopyAction.COPY_IF_TYPE) { - const shouldDropLink = - existingColumnType === FieldType.LINK && - !tableIds.includes(column.tableId) - - keepExistingSchema = + const keepIfType = (...validTypes: PrimitiveTypes[]) => { + return ( isPrimitiveType(updatedColumnType) && table.schema[key] && - map.types?.includes(updatedColumnType) && - !shouldDropLink + validTypes.includes(updatedColumnType) + ) } - if (keepExistingSchema) { + const SqlCopyTypeByFieldMapping: Record boolean> = { + [FieldType.LINK]: () => { + const shouldKeepLink = + existingColumnType === FieldType.LINK && + tableIds.includes(column.tableId) + return shouldKeepLink + }, + [FieldType.FORMULA]: () => true, + [FieldType.AUTO]: () => true, + [FieldType.INTERNAL]: () => true, + [FieldType.STRING]: () => keepIfType(FieldType.STRING), + [FieldType.OPTIONS]: () => keepIfType(FieldType.STRING), + [FieldType.LONGFORM]: () => keepIfType(FieldType.STRING), + [FieldType.NUMBER]: () => + keepIfType(FieldType.BOOLEAN, FieldType.NUMBER), + + [FieldType.BOOLEAN]: () => + keepIfType(FieldType.BOOLEAN, FieldType.NUMBER), + [FieldType.ARRAY]: () => keepIfType(FieldType.JSON, FieldType.STRING), + [FieldType.DATETIME]: () => + keepIfType(FieldType.DATETIME, FieldType.STRING), + + [FieldType.ATTACHMENTS]: () => + keepIfType(FieldType.JSON, FieldType.STRING), + [FieldType.ATTACHMENT_SINGLE]: () => + keepIfType(FieldType.JSON, FieldType.STRING), + + [FieldType.JSON]: () => keepIfType(FieldType.JSON, FieldType.STRING), + [FieldType.BARCODEQR]: () => keepIfType(FieldType.STRING), + + [FieldType.BIGINT]: () => + keepIfType(FieldType.BIGINT, FieldType.NUMBER), + [FieldType.BB_REFERENCE]: () => + keepIfType(FieldType.JSON, FieldType.STRING), + } + + const shouldCopyDelegate = SqlCopyTypeByFieldMapping[existingColumnType] + if (shouldCopyDelegate()) { table.schema[key] = { ...existingTableSchema[key], externalType: From 706d0cb89ccc13e79fec3f8e878b9c4442b5ede8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 19 Apr 2024 14:06:47 +0200 Subject: [PATCH 13/13] Refactor --- .../server/src/integrations/utils/utils.ts | 71 +++++++++++-------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/packages/server/src/integrations/utils/utils.ts b/packages/server/src/integrations/utils/utils.ts index 0a71c85905..5a6073ccab 100644 --- a/packages/server/src/integrations/utils/utils.ts +++ b/packages/server/src/integrations/utils/utils.ts @@ -8,7 +8,7 @@ import { } from "@budibase/types" import { DocumentType, SEPARATOR } from "../../db/utils" import { InvalidColumns, DEFAULT_BB_DATASOURCE_ID } from "../../constants" -import { helpers } from "@budibase/shared-core" +import { helpers, utils } from "@budibase/shared-core" import env from "../../environment" import { Knex } from "knex" @@ -342,44 +342,53 @@ function copyExistingPropsOver( ) } - const SqlCopyTypeByFieldMapping: Record boolean> = { - [FieldType.LINK]: () => { - const shouldKeepLink = + let shouldKeepSchema = false + switch (existingColumnType) { + case FieldType.FORMULA: + case FieldType.AUTO: + case FieldType.INTERNAL: + shouldKeepSchema = true + break + + case FieldType.LINK: + shouldKeepSchema = existingColumnType === FieldType.LINK && tableIds.includes(column.tableId) - return shouldKeepLink - }, - [FieldType.FORMULA]: () => true, - [FieldType.AUTO]: () => true, - [FieldType.INTERNAL]: () => true, - [FieldType.STRING]: () => keepIfType(FieldType.STRING), - [FieldType.OPTIONS]: () => keepIfType(FieldType.STRING), - [FieldType.LONGFORM]: () => keepIfType(FieldType.STRING), - [FieldType.NUMBER]: () => - keepIfType(FieldType.BOOLEAN, FieldType.NUMBER), + break - [FieldType.BOOLEAN]: () => - keepIfType(FieldType.BOOLEAN, FieldType.NUMBER), - [FieldType.ARRAY]: () => keepIfType(FieldType.JSON, FieldType.STRING), - [FieldType.DATETIME]: () => - keepIfType(FieldType.DATETIME, FieldType.STRING), + case FieldType.STRING: + case FieldType.OPTIONS: + case FieldType.LONGFORM: + case FieldType.BARCODEQR: + shouldKeepSchema = keepIfType(FieldType.STRING) + break - [FieldType.ATTACHMENTS]: () => - keepIfType(FieldType.JSON, FieldType.STRING), - [FieldType.ATTACHMENT_SINGLE]: () => - keepIfType(FieldType.JSON, FieldType.STRING), + case FieldType.NUMBER: + case FieldType.BOOLEAN: + shouldKeepSchema = keepIfType(FieldType.BOOLEAN, FieldType.NUMBER) + break - [FieldType.JSON]: () => keepIfType(FieldType.JSON, FieldType.STRING), - [FieldType.BARCODEQR]: () => keepIfType(FieldType.STRING), + case FieldType.ARRAY: + case FieldType.ATTACHMENTS: + case FieldType.ATTACHMENT_SINGLE: + case FieldType.JSON: + case FieldType.BB_REFERENCE: + shouldKeepSchema = keepIfType(FieldType.JSON, FieldType.STRING) + break - [FieldType.BIGINT]: () => - keepIfType(FieldType.BIGINT, FieldType.NUMBER), - [FieldType.BB_REFERENCE]: () => - keepIfType(FieldType.JSON, FieldType.STRING), + case FieldType.DATETIME: + shouldKeepSchema = keepIfType(FieldType.DATETIME, FieldType.STRING) + break + + case FieldType.BIGINT: + shouldKeepSchema = keepIfType(FieldType.BIGINT, FieldType.NUMBER) + break + + default: + utils.unreachable(existingColumnType) } - const shouldCopyDelegate = SqlCopyTypeByFieldMapping[existingColumnType] - if (shouldCopyDelegate()) { + if (shouldKeepSchema) { table.schema[key] = { ...existingTableSchema[key], externalType: