From 3744c9093bc184d87862a64e023dcecbb207173f Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 5 Aug 2021 19:24:29 +0100 Subject: [PATCH 1/4] Fixing a variety of issues with internal relationships and external SQL relationships. --- .../CreateEditRelationship.svelte | 3 +++ .../src/api/controllers/row/ExternalRequest.ts | 17 +++++++++++------ .../server/src/db/linkedRows/LinkController.js | 2 +- packages/server/src/integrations/base/sql.ts | 2 +- packages/server/src/integrations/mysql.ts | 2 +- packages/server/src/integrations/utils.ts | 9 +++++++-- 6 files changed, 24 insertions(+), 11 deletions(-) diff --git a/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/CreateEditRelationship/CreateEditRelationship.svelte b/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/CreateEditRelationship/CreateEditRelationship.svelte index 33ca4608ff..fbc2b401ef 100644 --- a/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/CreateEditRelationship/CreateEditRelationship.svelte +++ b/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/CreateEditRelationship/CreateEditRelationship.svelte @@ -199,6 +199,9 @@ delete datasource.entities[toTable.name].schema[originalToName] } + // store the original names so it won't cause an error + originalToName = toRelationship.name + originalFromName = fromRelationship.name await save() await tables.fetch() } diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 92a3838f51..f729ee7ff3 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -25,10 +25,10 @@ interface ManyRelationship { interface RunConfig { id: string - row: Row filters: SearchFilters sort: SortJson paginate: PaginationJson + row: Row } module External { @@ -89,8 +89,9 @@ module External { // build id array let idParts = [] for (let field of primary) { - if (row[field]) { - idParts.push(row[field]) + const fieldValue = row[`${table.name}.${field}`] + if (fieldValue) { + idParts.push(fieldValue) } } if (idParts.length === 0) { @@ -115,7 +116,11 @@ module External { const thisRow: { [key: string]: any } = {} // filter the row down to what is actually the row (not joined) for (let fieldName of Object.keys(table.schema)) { - thisRow[fieldName] = row[fieldName] + const value = row[`${table.name}.${fieldName}`] + // all responses include "select col as table.col" so that overlaps are handled + if (value) { + thisRow[fieldName] = value + } } thisRow._id = generateIdForRow(row, table) thisRow.tableId = table._id @@ -191,7 +196,7 @@ module External { const isUpdate = !field.through const thisKey: string = isUpdate ? "id" : linkTablePrimary // @ts-ignore - const otherKey: string = isUpdate ? field.foreignKey : tablePrimary + const otherKey: string = isUpdate ? field.fieldName : tablePrimary row[key].map((relationship: any) => { // we don't really support composite keys for relationships, this is why [0] is used manyRelationships.push({ @@ -442,7 +447,7 @@ module External { .filter( column => column[1].type !== FieldTypes.LINK && - !existing.find((field: string) => field.includes(column[0])) + !existing.find((field: string) => field === column[0]) ) .map(column => `${table.name}.${column[0]}`) } diff --git a/packages/server/src/db/linkedRows/LinkController.js b/packages/server/src/db/linkedRows/LinkController.js index c8331dec6b..d526bf159b 100644 --- a/packages/server/src/db/linkedRows/LinkController.js +++ b/packages/server/src/db/linkedRows/LinkController.js @@ -322,7 +322,7 @@ class LinkController { // remove schema from other table let linkedTable = await this._db.get(field.tableId) delete linkedTable.schema[field.fieldName] - this._db.put(linkedTable) + await this._db.put(linkedTable) } /** diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index ea48eab793..0587e3df1d 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -151,7 +151,7 @@ function buildRead(knex: Knex, json: QueryJson, limit: number): KnexQuery { } // handle select if (resource.fields && resource.fields.length > 0) { - query = query.select(resource.fields) + query = query.select(resource.fields.map(field => `${field} as ${field}`)) } else { query = query.select("*") } diff --git a/packages/server/src/integrations/mysql.ts b/packages/server/src/integrations/mysql.ts index fab151fc0d..d71e85b0d2 100644 --- a/packages/server/src/integrations/mysql.ts +++ b/packages/server/src/integrations/mysql.ts @@ -242,7 +242,7 @@ module MySQLModule { const input = this._query(json, { disableReturning: true }) let row // need to manage returning, a feature mySQL can't do - if (operation === "awdawd") { + if (operation === operation.DELETE) { row = this.getReturningRow(json) } const results = await internalQuery(this.client, input, false) diff --git a/packages/server/src/integrations/utils.ts b/packages/server/src/integrations/utils.ts index 03751bb467..e49c1d6ff3 100644 --- a/packages/server/src/integrations/utils.ts +++ b/packages/server/src/integrations/utils.ts @@ -40,8 +40,13 @@ export function breakRowIdField(_id: string): any[] { // have to replace on the way back as we swapped out the double quotes // when encoding, but JSON can't handle the single quotes const decoded: string = decodeURIComponent(_id).replace(/'/g, '"') - const parsed = JSON.parse(decoded) - return Array.isArray(parsed) ? parsed : [parsed] + try { + const parsed = JSON.parse(decoded) + return Array.isArray(parsed) ? parsed : [parsed] + } catch (err) { + // wasn't json - likely was handlebars for a many to many + return [_id] + } } export function convertType(type: string, map: { [key: string]: any }) { From 8981db7f4c588e08d854df06306c4eac08fb2247 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 5 Aug 2021 19:26:00 +0100 Subject: [PATCH 2/4] Linting. --- packages/server/src/utilities/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/utilities/index.js b/packages/server/src/utilities/index.js index eab4265c19..9594092aa1 100644 --- a/packages/server/src/utilities/index.js +++ b/packages/server/src/utilities/index.js @@ -1,5 +1,5 @@ const env = require("../environment") -const { OBJ_STORE_DIRECTORY, ObjectStoreBuckets } = require("../constants") +const { OBJ_STORE_DIRECTORY } = require("../constants") const { getAllApps } = require("@budibase/auth/db") const { sanitizeKey } = require("@budibase/auth/src/objectStore") From a64ce3f55a770385d8892d2a656ac50ef5f92e38 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 5 Aug 2021 19:49:30 +0100 Subject: [PATCH 3/4] Fixing issues with many to many relationships in SQL, sometimes not creating right relationships. --- .../api/controllers/row/ExternalRequest.ts | 24 ++++++++++++------- packages/server/src/integrations/base/sql.ts | 2 ++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index f729ee7ff3..4107937cbd 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -364,7 +364,7 @@ module External { } } if (cache[fullKey] == null) { - cache[fullKey] = await makeExternalQuery(this.appId, { + const response = await makeExternalQuery(this.appId, { endpoint: getEndpoint(tableId, DataSourceOperation.READ), filters: { equal: { @@ -372,8 +372,12 @@ module External { }, }, }) + // this is the response from knex if no rows found + if (!response[0].read) { + cache[fullKey] = response + } } - return { rows: cache[fullKey], table } + return { rows: cache[fullKey] || [], table } } /** @@ -423,12 +427,16 @@ module External { const { tableName } = breakExternalTableId(tableId) const table = this.tables[tableName] for (let row of rows) { - promises.push( - makeExternalQuery(this.appId, { - endpoint: getEndpoint(tableId, DataSourceOperation.DELETE), - filters: buildFilters(generateIdForRow(row, table), {}, table), - }) - ) + const filters = buildFilters(generateIdForRow(row, table), {}, table) + // safety check, if there are no filters on deletion bad things happen + if (Object.keys(filters).length !== 0) { + promises.push( + makeExternalQuery(this.appId, { + endpoint: getEndpoint(tableId, DataSourceOperation.DELETE), + filters, + }) + ) + } } } await Promise.all(promises) diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index 0587e3df1d..ca6dcb15fd 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -151,6 +151,8 @@ function buildRead(knex: Knex, json: QueryJson, limit: number): KnexQuery { } // handle select if (resource.fields && resource.fields.length > 0) { + // select the resources as the format "table.columnName" - this is what is provided + // by the resource builder further up query = query.select(resource.fields.map(field => `${field} as ${field}`)) } else { query = query.select("*") From 7c7e3da2ab43205f3bfaac2f13aeafa53972ec92 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 6 Aug 2021 12:33:04 +0100 Subject: [PATCH 4/4] Fixing test cases. --- packages/server/src/api/routes/tests/datasource.spec.js | 4 ++-- packages/server/src/integrations/tests/sql.spec.js | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.js b/packages/server/src/api/routes/tests/datasource.spec.js index a041de4310..7387dd3c46 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.js +++ b/packages/server/src/api/routes/tests/datasource.spec.js @@ -82,7 +82,7 @@ describe("/datasources", () => { entityId: "users", }, resource: { - fields: ["name", "age"], + fields: ["users.name", "users.age"], }, filters: { string: { @@ -94,7 +94,7 @@ describe("/datasources", () => { .expect(200) // this is mock data, can't test it expect(res.body).toBeDefined() - expect(pg.queryMock).toHaveBeenCalledWith(`select "name", "age" from "users" where "users"."name" like $1 limit $2`, ["John%", 5000]) + expect(pg.queryMock).toHaveBeenCalledWith(`select "users"."name" as "users.name", "users"."age" as "users.age" from "users" where "users"."name" like $1 limit $2`, ["John%", 5000]) }) }) diff --git a/packages/server/src/integrations/tests/sql.spec.js b/packages/server/src/integrations/tests/sql.spec.js index fb57fe79e7..a02a7e8198 100644 --- a/packages/server/src/integrations/tests/sql.spec.js +++ b/packages/server/src/integrations/tests/sql.spec.js @@ -62,12 +62,13 @@ describe("SQL query builder", () => { }) it("should test a read with specific columns", () => { + const nameProp = `${TABLE_NAME}.name`, ageProp = `${TABLE_NAME}.age` const query = sql._query(generateReadJson({ - fields: ["name", "age"] + fields: [nameProp, ageProp] })) expect(query).toEqual({ bindings: [limit], - sql: `select "name", "age" from "${TABLE_NAME}" limit $1` + sql: `select "${TABLE_NAME}"."name" as "${nameProp}", "${TABLE_NAME}"."age" as "${ageProp}" from "${TABLE_NAME}" limit $1` }) })