Merge pull request #11013 from Budibase/fix/sql-relationship-fixes

Further SQL relationship fixes

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 = {}
-    }
     for (let key in tables) {
       if (
         tablesFilter.some(
@@ -200,7 +198,9 @@ export async function buildSchemaFromDb(ctx: UserCtx) {
   }
 
   setDefaultDisplayColumns(datasource)
-  const dbResp = await db.put(datasource)
+  const dbResp = await db.put(
+    sdk.tables.populateExternalTableSchemas(datasource)
+  )
   datasource._rev = dbResp.rev
   const cleanedDatasource = await sdk.datasources.removeSecretSingle(datasource)
 
@@ -286,7 +286,9 @@ export async function update(ctx: UserCtx<any, UpdateDatasourceResponse>) {
     datasource.config!.auth = auth
   }
 
-  const response = await db.put(datasource)
+  const response = await db.put(
+    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(datasource)
+  const dbResp = await db.put(
+    sdk.tables.populateExternalTableSchemas(datasource)
+  )
   await events.datasource.created(datasource)
   datasource._rev = dbResp.rev
 

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
       }

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.populateExternalTableSchemas(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.populateExternalTableSchemas(datasource))
 
   // Since tables are stored inside datasources, we need to notify clients
   // that the datasource definition changed

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 { populateExternalTableSchemas, isEditableColumn } from "./validation"
 
 async function getAllInternalTables(db?: Database): Promise<Table[]> {
   if (!db) {
@@ -60,4 +61,6 @@ export default {
   getAllExternalTables,
   getExternalTable,
   getTable,
+  populateExternalTableSchemas,
+  isEditableColumn,
 }

packages/server/src/sdk/app/tables/tests/validation.spec.ts

@@ -0,0 +1,129 @@
+import { populateExternalTableSchemas } from "../validation"
+import { cloneDeep } from "lodash/fp"
+import { Datasource, Table } from "@budibase/types"
+import { isEqual } from "lodash"
+
+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",
+    },
+  },
+}
+
+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 = 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 = populateExternalTableSchemas(cloneDeep(SCHEMA) as any)
+    const beforeFk = getForeignKeyColumn(setResponse)
+    delete setResponse.entities!.client.schema.project
+    delete setResponse.entities!.project.schema.client
+    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)
+  })
+})

packages/server/src/sdk/app/tables/validation.ts

@@ -0,0 +1,67 @@
+import {
+  AutoReason,
+  Datasource,
+  FieldSchema,
+  FieldType,
+  RelationshipTypes,
+} from "@budibase/types"
+import { FieldTypes } from "../../../constants"
+
+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
+      )
+      // 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) {
+        delete column.autocolumn
+        delete column.autoReason
+      }
+    }
+  }
+
+  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 populateExternalTableSchemas(datasource: Datasource) {
+  return checkForeignKeysAreAutoColumns(datasource)
+}

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