From 9ca36893ade270d38efde3dda8ba9a7a0ac1de97 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 29 Jun 2021 18:38:27 +0100 Subject: [PATCH] Managing the scenario where columns can overlap in SQL relationships which most JSON based libraries cannot manage, instead of trying to manage this just don't return the overlapping columns which are not of interest. --- .../src/api/controllers/row/external.js | 32 ++++++------ .../src/api/controllers/row/externalUtils.js | 50 +++++++++++++++++-- packages/server/src/integrations/base/sql.ts | 10 ++-- 3 files changed, 68 insertions(+), 24 deletions(-) diff --git a/packages/server/src/api/controllers/row/external.js b/packages/server/src/api/controllers/row/external.js index 2da4ecfa0a..581f620634 100644 --- a/packages/server/src/api/controllers/row/external.js +++ b/packages/server/src/api/controllers/row/external.js @@ -1,8 +1,5 @@ const { makeExternalQuery } = require("./utils") -const { - DataSourceOperation, - SortDirection, -} = require("../../../constants") +const { DataSourceOperation, SortDirection } = require("../../../constants") const { getAllExternalTables } = require("../table/utils") const { breakExternalTableId, @@ -14,6 +11,7 @@ const { inputProcessing, outputProcessing, generateIdForRow, + buildFields, } = require("./externalUtils") const { processObjectSync } = require("@budibase/string-templates") @@ -23,7 +21,7 @@ async function handleRequest( tableId, { id, row, filters, sort, paginate } = {} ) { - let {datasourceId, tableName} = breakExternalTableId(tableId) + let { datasourceId, tableName } = breakExternalTableId(tableId) const tables = await getAllExternalTables(appId, datasourceId) const table = tables[tableName] if (!table) { @@ -32,7 +30,7 @@ async function handleRequest( // clean up row on ingress using schema filters = buildFilters(id, filters, table) const relationships = buildRelationships(table, tables) - const processed = inputProcessing(row, table) + const processed = inputProcessing(row, table, tables) row = processed.row if ( operation === DataSourceOperation.DELETE && @@ -47,8 +45,8 @@ async function handleRequest( operation, }, resource: { - // not specifying any fields means "*" - fields: [], + // have to specify the fields to avoid column overlap + fields: buildFields(table, tables), }, filters, sort, @@ -66,15 +64,17 @@ async function handleRequest( if (processed.manyRelationships) { const promises = [] for (let toInsert of processed.manyRelationships) { - const {tableName} = breakExternalTableId(toInsert.tableId) + const { tableName } = breakExternalTableId(toInsert.tableId) delete toInsert.tableId - promises.push(makeExternalQuery(appId, { - endpoint: { - ...json.endpoint, - entityId: tableName, - }, - body: toInsert, - })) + promises.push( + makeExternalQuery(appId, { + endpoint: { + ...json.endpoint, + entityId: processObjectSync(tableName, row), + }, + body: toInsert, + }) + ) } await Promise.all(promises) } diff --git a/packages/server/src/api/controllers/row/externalUtils.js b/packages/server/src/api/controllers/row/externalUtils.js index 46284ca99f..b73d4ecdeb 100644 --- a/packages/server/src/api/controllers/row/externalUtils.js +++ b/packages/server/src/api/controllers/row/externalUtils.js @@ -10,7 +10,8 @@ exports.inputProcessing = (row, table, allTables) => { if (!row) { return { row, manyRelationships: [] } } - let newRow = {}, manyRelationships = [] + let newRow = {}, + manyRelationships = [] for (let [key, field] of Object.entries(table.schema)) { // currently excludes empty strings if (!row[key]) { @@ -21,18 +22,19 @@ exports.inputProcessing = (row, table, allTables) => { // we don't really support composite keys for relationships, this is why [0] is used newRow[key] = breakRowIdField(row[key][0])[0] } else if (isLink && field.through) { - const linkTable = allTables.find(table => table._id === field.tableId) + const { tableName: linkTableName } = breakExternalTableId(field.tableId) // table has to exist for many to many - if (!linkTable) { + if (!allTables[linkTableName]) { continue } + const linkTable = allTables[linkTableName] row[key].map(relationship => { // we don't really support composite keys for relationships, this is why [0] is used manyRelationships.push({ tableId: field.through, [linkTable.primary]: breakRowIdField(relationship)[0], // leave the ID for enrichment later - [table.primary]: `{{ id }}`, + [table.primary]: `{{ _id }}`, }) }) } else { @@ -120,6 +122,42 @@ exports.outputProcessing = (rows, table, relationships, allTables) => { return Object.values(finalRows) } +/** + * This function is a bit crazy, but the exact purpose of it is to protect against the scenario in which + * you have column overlap in relationships, e.g. we join a few different tables and they all have the + * concept of an ID, but for some of them it will be null (if they say don't have a relationship). + * Creating the specific list of fields that we desire, and excluding the ones that are no use to us + * is more performant and has the added benefit of protecting against this scenario. + * @param {Object} table The table we are retrieving fields for. + * @param {Object[]} allTables All of the tables that exist in the external data source, this is + * needed to work out what is needed from other tables based on relationships. + * @return {string[]} A list of fields like ["products.productid"] which can be used for an SQL select. + */ +exports.buildFields = (table, allTables) => { + function extractNonLinkFieldNames(table, existing = []) { + return Object.entries(table.schema) + .filter( + column => + column[1].type !== FieldTypes.LINK && + !existing.find(field => field.includes(column[0])) + ) + .map(column => `${table.name}.${column[0]}`) + } + let fields = extractNonLinkFieldNames(table) + for (let field of Object.values(table.schema)) { + if (field.type !== FieldTypes.LINK) { + continue + } + const { tableName: linkTableName } = breakExternalTableId(field.tableId) + const linkTable = allTables[linkTableName] + if (linkTable) { + const linkedFields = extractNonLinkFieldNames(linkTable, fields) + fields = fields.concat(linkedFields) + } + } + return fields +} + exports.buildFilters = (id, filters, table) => { const primary = table.primary // if passed in array need to copy for shifting etc @@ -177,7 +215,9 @@ exports.buildRelationships = (table, allTables) => { column: fieldName, } if (field.through) { - const { tableName: throughTableName } = breakExternalTableId(field.through) + const { tableName: throughTableName } = breakExternalTableId( + field.through + ) definition.through = throughTableName // don't support composite keys for relationships definition.from = table.primary[0] diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index 743d5e159a..ea887c5ba0 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -84,7 +84,7 @@ function addRelationships( toTable = relationship.tableName if (!relationship.through) { // @ts-ignore - query = query.innerJoin( + query = query.leftJoin( toTable, `${fromTable}.${from}`, `${relationship.tableName}.${to}` @@ -93,8 +93,12 @@ function addRelationships( const throughTable = relationship.through query = query // @ts-ignore - .innerJoin(throughTable, `${fromTable}.${from}`, `${throughTable}.${from}`) - .innerJoin(toTable, `${toTable}.${to}`, `${throughTable}.${to}`) + .leftJoin( + throughTable, + `${fromTable}.${from}`, + `${throughTable}.${from}` + ) + .leftJoin(toTable, `${toTable}.${to}`, `${throughTable}.${to}`) } } return query