diff --git a/.tool-versions b/.tool-versions index 8a1af3c071..6ee8cc60be 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,2 +1,2 @@ nodejs 14.19.3 -python 3.11.1 \ No newline at end of file +python 3.10.0 \ No newline at end of file diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 2faff95595..c3b7636636 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -142,7 +142,11 @@ function cleanupConfig(config: RunConfig, table: Table): RunConfig { return config } -function generateIdForRow(row: Row | undefined, table: Table): string { +function generateIdForRow( + row: Row | undefined, + table: Table, + isLinked: boolean = false +): string { const primary = table.primary if (!row || !primary) { return "" @@ -150,8 +154,12 @@ function generateIdForRow(row: Row | undefined, table: Table): string { // build id array let idParts = [] for (let field of primary) { - // need to handle table name + field or just field, depending on if relationships used - const fieldValue = row[`${table.name}.${field}`] || row[field] + let fieldValue = extractFieldValue({ + row, + tableName: table.name, + fieldName: field, + isLinked, + }) if (fieldValue) { idParts.push(fieldValue) } @@ -174,18 +182,52 @@ function getEndpoint(tableId: string | undefined, operation: string) { } } -function basicProcessing(row: Row, table: Table): Row { +// need to handle table name + field or just field, depending on if relationships used +function extractFieldValue({ + row, + tableName, + fieldName, + isLinked, +}: { + row: Row + tableName: string + fieldName: string + isLinked: boolean +}) { + let value = row[`${tableName}.${fieldName}`] + if (value == null && !isLinked) { + value = row[fieldName] + } + return value +} + +function basicProcessing({ + row, + table, + isLinked, +}: { + row: Row + table: Table + isLinked: boolean +}): Row { const thisRow: Row = {} // filter the row down to what is actually the row (not joined) - for (let fieldName of Object.keys(table.schema)) { - const pathValue = row[`${table.name}.${fieldName}`] - const value = pathValue != null ? pathValue : row[fieldName] + for (let field of Object.values(table.schema)) { + const fieldName = field.name + + const value = extractFieldValue({ + row, + tableName: table.name, + fieldName, + isLinked, + }) + // all responses include "select col as table.col" so that overlaps are handled if (value != null) { thisRow[fieldName] = value } } - thisRow._id = generateIdForRow(row, table) + thisRow._id = generateIdForRow(row, table, isLinked) thisRow.tableId = table._id thisRow._rev = "rev" return processFormulas(table, thisRow) @@ -293,7 +335,7 @@ export class ExternalRequest { // we're not inserting a doc, will be a bunch of update calls const otherKey: string = field.throughFrom || linkTablePrimary const thisKey: string = field.throughTo || tablePrimary - row[key].map((relationship: any) => { + row[key].forEach((relationship: any) => { manyRelationships.push({ tableId: field.through || field.tableId, isUpdate: false, @@ -309,7 +351,7 @@ export class ExternalRequest { const thisKey: string = "id" // @ts-ignore const otherKey: string = field.fieldName - row[key].map((relationship: any) => { + row[key].forEach((relationship: any) => { manyRelationships.push({ tableId: field.tableId, isUpdate: true, @@ -379,7 +421,8 @@ export class ExternalRequest { ) { continue } - let linked = basicProcessing(row, linkedTable) + + let linked = basicProcessing({ row, table: linkedTable, isLinked: true }) if (!linked._id) { continue } @@ -427,7 +470,10 @@ export class ExternalRequest { ) continue } - const thisRow = fixArrayTypes(basicProcessing(row, table), table) + const thisRow = fixArrayTypes( + basicProcessing({ row, table, isLinked: false }), + table + ) if (thisRow._id == null) { throw "Unable to generate row ID for SQL rows" } @@ -567,19 +613,41 @@ export class ExternalRequest { const { key, tableId, isUpdate, id, ...rest } = relationship const body: { [key: string]: any } = processObjectSync(rest, row, {}) const linkTable = this.getTable(tableId) - // @ts-ignore - const linkPrimary = linkTable?.primary[0] + const relationshipPrimary = linkTable?.primary || [] + const linkPrimary = relationshipPrimary[0] if (!linkTable || !linkPrimary) { return } + + const linkSecondary = relationshipPrimary[1] + const rows = related[key]?.rows || [] - const found = rows.find( - (row: { [key: string]: any }) => + + function relationshipMatchPredicate({ + row, + linkPrimary, + linkSecondary, + }: { + row: { [key: string]: any } + linkPrimary: string + linkSecondary?: string + }) { + const matchesPrimaryLink = row[linkPrimary] === relationship.id || row[linkPrimary] === body?.[linkPrimary] + if (!matchesPrimaryLink || !linkSecondary) { + return matchesPrimaryLink + } + + const matchesSecondayLink = row[linkSecondary] === body?.[linkSecondary] + return matchesPrimaryLink && matchesSecondayLink + } + + const existingRelationship = rows.find((row: { [key: string]: any }) => + relationshipMatchPredicate({ row, linkPrimary, linkSecondary }) ) const operation = isUpdate ? Operation.UPDATE : Operation.CREATE - if (!found) { + if (!existingRelationship) { promises.push( getDatasourceAndQuery({ endpoint: getEndpoint(tableId, operation), @@ -590,7 +658,7 @@ export class ExternalRequest { ) } else { // remove the relationship from cache so it isn't adjusted again - rows.splice(rows.indexOf(found), 1) + rows.splice(rows.indexOf(existingRelationship), 1) } } // finally cleanup anything that needs to be removed @@ -629,10 +697,7 @@ export class ExternalRequest { * 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. */ - buildFields( - table: Table, - includeRelations: IncludeRelationship = IncludeRelationship.INCLUDE - ) { + buildFields(table: Table, includeRelations: boolean) { function extractRealFields(table: Table, existing: string[] = []) { return Object.entries(table.schema) .filter( @@ -691,6 +756,10 @@ export class ExternalRequest { } filters = buildFilters(id, filters || {}, table) const relationships = this.buildRelationships(table) + + const includeSqlRelationships = + config.includeSqlRelationships === IncludeRelationship.INCLUDE + // clean up row on ingress using schema const processed = this.inputProcessing(row, table) row = processed.row @@ -708,9 +777,7 @@ export class ExternalRequest { }, resource: { // have to specify the fields to avoid column overlap (for SQL) - fields: isSql - ? this.buildFields(table, config.includeSqlRelationships) - : [], + fields: isSql ? this.buildFields(table, includeSqlRelationships) : [], }, filters, sort, @@ -725,6 +792,7 @@ export class ExternalRequest { table, }, } + // can't really use response right now const response = await getDatasourceAndQuery(json) // handle many to many relationships now if we know the ID (could be auto increment) diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index 6120c3cdcb..1b2301f139 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -58,7 +58,7 @@ export async function patch(ctx: BBContext) { return handleRequest(Operation.UPDATE, tableId, { id: breakRowIdField(id), row: inputs, - includeSqlRelationships: IncludeRelationship.EXCLUDE, + includeSqlRelationships: IncludeRelationship.INCLUDE, }) } diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index c688600e8d..2afff8b786 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -27,7 +27,9 @@ describe("row api - postgres", () => { let makeRequest: MakeRequestResponse, postgresDatasource: Datasource, primaryPostgresTable: Table, - auxPostgresTable: Table + oneToManyRelationshipInfo: ForeignTableInfo, + manyToOneRelationshipInfo: ForeignTableInfo, + manyToManyRelationshipInfo: ForeignTableInfo let host: string let port: number @@ -67,37 +69,58 @@ describe("row api - postgres", () => { }, }) - auxPostgresTable = await config.createTable({ - name: generator.word({ length: 10 }), - type: "external", - primary: ["id"], - schema: { - id: { - name: "id", - type: FieldType.AUTO, - constraints: { - presence: true, + async function createAuxTable(prefix: string) { + return await config.createTable({ + name: `${prefix}_${generator.word({ length: 6 })}`, + type: "external", + primary: ["id"], + primaryDisplay: "title", + schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + constraints: { + presence: true, + }, + }, + title: { + name: "title", + type: FieldType.STRING, + constraints: { + presence: true, + }, }, }, - title: { - name: "title", - type: FieldType.STRING, - constraints: { - presence: true, - }, - }, - }, - sourceId: postgresDatasource._id, - }) + sourceId: postgresDatasource._id, + }) + } + + oneToManyRelationshipInfo = { + table: await createAuxTable("o2m"), + fieldName: "oneToManyRelation", + relationshipType: RelationshipTypes.ONE_TO_MANY, + } + manyToOneRelationshipInfo = { + table: await createAuxTable("m2o"), + fieldName: "manyToOneRelation", + relationshipType: RelationshipTypes.MANY_TO_ONE, + } + manyToManyRelationshipInfo = { + table: await createAuxTable("m2m"), + fieldName: "manyToManyRelation", + relationshipType: RelationshipTypes.MANY_TO_MANY, + } primaryPostgresTable = await config.createTable({ - name: generator.word({ length: 10 }), + name: `p_${generator.word({ length: 6 })}`, type: "external", primary: ["id"], schema: { id: { name: "id", type: FieldType.AUTO, + autocolumn: true, constraints: { presence: true, }, @@ -117,25 +140,48 @@ describe("row api - postgres", () => { name: "value", type: FieldType.NUMBER, }, - linkedField: { + oneToManyRelation: { type: FieldType.LINK, constraints: { type: "array", presence: false, }, - fieldName: "foreignField", - name: "linkedField", + fieldName: oneToManyRelationshipInfo.fieldName, + name: "oneToManyRelation", relationshipType: RelationshipTypes.ONE_TO_MANY, - tableId: auxPostgresTable._id, + tableId: oneToManyRelationshipInfo.table._id, + main: true, + }, + manyToOneRelation: { + type: FieldType.LINK, + constraints: { + type: "array", + presence: false, + }, + fieldName: manyToOneRelationshipInfo.fieldName, + name: "manyToOneRelation", + relationshipType: RelationshipTypes.MANY_TO_ONE, + tableId: manyToOneRelationshipInfo.table._id, + main: true, + }, + manyToManyRelation: { + type: FieldType.LINK, + constraints: { + type: "array", + presence: false, + }, + fieldName: manyToManyRelationshipInfo.fieldName, + name: "manyToManyRelation", + relationshipType: RelationshipTypes.MANY_TO_MANY, + tableId: manyToManyRelationshipInfo.table._id, + main: true, }, }, sourceId: postgresDatasource._id, }) }) - afterAll(async () => { - await config.end() - }) + afterAll(config.end) function generateRandomPrimaryRowData() { return { @@ -151,22 +197,99 @@ describe("row api - postgres", () => { value: number } + type ForeignTableInfo = { + table: Table + fieldName: string + relationshipType: RelationshipTypes + } + + type ForeignRowsInfo = { + row: Row + relationshipType: RelationshipTypes + } + async function createPrimaryRow(opts: { rowData: PrimaryRowData - createForeignRow?: boolean + createForeignRows?: { + createOneToMany?: boolean + createManyToOne?: number + createManyToMany?: number + } }) { - let { rowData } = opts - let foreignRow: Row | undefined - if (opts?.createForeignRow) { - foreignRow = await config.createRow({ - tableId: auxPostgresTable._id, + let { rowData } = opts as any + let foreignRows: ForeignRowsInfo[] = [] + + async function createForeignRow(tableInfo: ForeignTableInfo) { + const foreignKey = `fk_${tableInfo.table.name}_${tableInfo.fieldName}` + + const foreignRow = await config.createRow({ + tableId: tableInfo.table._id, title: generator.name(), }) rowData = { ...rowData, - [`fk_${auxPostgresTable.name}_foreignField`]: foreignRow.id, + [foreignKey]: foreignRow.id, } + foreignRows.push({ + row: foreignRow, + + relationshipType: tableInfo.relationshipType, + }) + } + + if (opts?.createForeignRows?.createOneToMany) { + const foreignKey = `fk_${oneToManyRelationshipInfo.table.name}_${oneToManyRelationshipInfo.fieldName}` + + const foreignRow = await config.createRow({ + tableId: oneToManyRelationshipInfo.table._id, + title: generator.name(), + }) + + rowData = { + ...rowData, + [foreignKey]: foreignRow.id, + } + foreignRows.push({ + row: foreignRow, + relationshipType: oneToManyRelationshipInfo.relationshipType, + }) + } + + for (let i = 0; i < (opts?.createForeignRows?.createManyToOne || 0); i++) { + const foreignRow = await config.createRow({ + tableId: manyToOneRelationshipInfo.table._id, + title: generator.name(), + }) + + rowData = { + ...rowData, + [manyToOneRelationshipInfo.fieldName]: + rowData[manyToOneRelationshipInfo.fieldName] || [], + } + rowData[manyToOneRelationshipInfo.fieldName].push(foreignRow._id) + foreignRows.push({ + row: foreignRow, + relationshipType: RelationshipTypes.MANY_TO_ONE, + }) + } + + for (let i = 0; i < (opts?.createForeignRows?.createManyToMany || 0); i++) { + const foreignRow = await config.createRow({ + tableId: manyToManyRelationshipInfo.table._id, + title: generator.name(), + }) + + rowData = { + ...rowData, + [manyToManyRelationshipInfo.fieldName]: + rowData[manyToManyRelationshipInfo.fieldName] || [], + } + rowData[manyToManyRelationshipInfo.fieldName].push(foreignRow._id) + foreignRows.push({ + row: foreignRow, + relationshipType: RelationshipTypes.MANY_TO_MANY, + }) } const row = await config.createRow({ @@ -174,7 +297,7 @@ describe("row api - postgres", () => { ...rowData, }) - return { row, foreignRow } + return { row, foreignRows } } async function createDefaultPgTable() { @@ -198,7 +321,9 @@ describe("row api - postgres", () => { async function populatePrimaryRows( count: number, opts?: { - createForeignRow?: boolean + createOneToMany?: boolean + createManyToOne?: number + createManyToMany?: number } ) { return await Promise.all( @@ -210,7 +335,7 @@ describe("row api - postgres", () => { rowData, ...(await createPrimaryRow({ rowData, - createForeignRow: opts?.createForeignRow, + createForeignRows: opts, })), } }) @@ -295,7 +420,7 @@ describe("row api - postgres", () => { describe("given than a row exists", () => { let row: Row beforeEach(async () => { - let rowResponse = _.sample(await populatePrimaryRows(10))! + let rowResponse = _.sample(await populatePrimaryRows(1))! row = rowResponse.row }) @@ -403,7 +528,7 @@ describe("row api - postgres", () => { let rows: { row: Row; rowData: PrimaryRowData }[] beforeEach(async () => { - rows = await populatePrimaryRows(10) + rows = await populatePrimaryRows(5) }) it("a single row can be retrieved successfully", async () => { @@ -419,34 +544,136 @@ describe("row api - postgres", () => { describe("given a row with relation data", () => { let row: Row - let foreignRow: Row - beforeEach(async () => { - let [createdRow] = await populatePrimaryRows(1, { - createForeignRow: true, + let rowData: { + name: string + description: string + value: number + } + let foreignRows: ForeignRowsInfo[] + + describe("with all relationship types", () => { + beforeEach(async () => { + let [createdRow] = await populatePrimaryRows(1, { + createOneToMany: true, + createManyToOne: 3, + createManyToMany: 2, + }) + row = createdRow.row + rowData = createdRow.rowData + foreignRows = createdRow.foreignRows + }) + + it("only one to many foreign keys are retrieved", async () => { + const res = await getRow(primaryPostgresTable._id, row.id) + + expect(res.status).toBe(200) + + const one2ManyForeignRows = foreignRows.filter( + x => x.relationshipType === RelationshipTypes.ONE_TO_MANY + ) + expect(one2ManyForeignRows).toHaveLength(1) + + expect(res.body).toEqual({ + ...rowData, + id: row.id, + tableId: row.tableId, + _id: expect.any(String), + _rev: expect.any(String), + [`fk_${oneToManyRelationshipInfo.table.name}_${oneToManyRelationshipInfo.fieldName}`]: + one2ManyForeignRows[0].row.id, + }) + + expect(res.body[oneToManyRelationshipInfo.fieldName]).toBeUndefined() }) - row = createdRow.row - foreignRow = createdRow.foreignRow! }) - it("only foreign keys are retrieved", async () => { - const res = await getRow(primaryPostgresTable._id, row.id) - - expect(res.status).toBe(200) - - expect(res.body).toEqual({ - ...row, - _id: expect.any(String), - _rev: expect.any(String), + describe("with only one to many", () => { + beforeEach(async () => { + let [createdRow] = await populatePrimaryRows(1, { + createOneToMany: true, + }) + row = createdRow.row + rowData = createdRow.rowData + foreignRows = createdRow.foreignRows }) - expect(res.body.foreignField).toBeUndefined() + it("only one to many foreign keys are retrieved", async () => { + const res = await getRow(primaryPostgresTable._id, row.id) - expect( - res.body[`fk_${auxPostgresTable.name}_foreignField`] - ).toBeDefined() - expect(res.body[`fk_${auxPostgresTable.name}_foreignField`]).toBe( - foreignRow.id - ) + expect(res.status).toBe(200) + + expect(foreignRows).toHaveLength(1) + + expect(res.body).toEqual({ + ...rowData, + id: row.id, + tableId: row.tableId, + _id: expect.any(String), + _rev: expect.any(String), + [`fk_${oneToManyRelationshipInfo.table.name}_${oneToManyRelationshipInfo.fieldName}`]: + foreignRows[0].row.id, + }) + + expect(res.body[oneToManyRelationshipInfo.fieldName]).toBeUndefined() + }) + }) + + describe("with only many to one", () => { + beforeEach(async () => { + let [createdRow] = await populatePrimaryRows(1, { + createManyToOne: 3, + }) + row = createdRow.row + rowData = createdRow.rowData + foreignRows = createdRow.foreignRows + }) + + it("only one to many foreign keys are retrieved", async () => { + const res = await getRow(primaryPostgresTable._id, row.id) + + expect(res.status).toBe(200) + + expect(foreignRows).toHaveLength(3) + + expect(res.body).toEqual({ + ...rowData, + id: row.id, + tableId: row.tableId, + _id: expect.any(String), + _rev: expect.any(String), + }) + + expect(res.body[oneToManyRelationshipInfo.fieldName]).toBeUndefined() + }) + }) + + describe("with only many to many", () => { + beforeEach(async () => { + let [createdRow] = await populatePrimaryRows(1, { + createManyToMany: 2, + }) + row = createdRow.row + rowData = createdRow.rowData + foreignRows = createdRow.foreignRows + }) + + it("only one to many foreign keys are retrieved", async () => { + const res = await getRow(primaryPostgresTable._id, row.id) + + expect(res.status).toBe(200) + + expect(foreignRows).toHaveLength(2) + + expect(res.body).toEqual({ + ...rowData, + id: row.id, + tableId: row.tableId, + _id: expect.any(String), + _rev: expect.any(String), + }) + + expect(res.body[oneToManyRelationshipInfo.fieldName]).toBeUndefined() + }) }) }) }) @@ -667,31 +894,74 @@ describe("row api - postgres", () => { const getAll = (tableId: string | undefined, rowId: string | undefined) => makeRequest("get", `/api/${tableId}/${rowId}/enrich`) describe("given a row with relation data", () => { - let row: Row, foreignRow: Row | undefined + let row: Row, rowData: PrimaryRowData, foreignRows: ForeignRowsInfo[] - beforeEach(async () => { - const rowsInfo = await createPrimaryRow({ - rowData: generateRandomPrimaryRowData(), - createForeignRow: true, + describe("with all relationship types", () => { + beforeEach(async () => { + rowData = generateRandomPrimaryRowData() + const rowsInfo = await createPrimaryRow({ + rowData, + createForeignRows: { + createOneToMany: true, + createManyToOne: 3, + createManyToMany: 2, + }, + }) + + row = rowsInfo.row + foreignRows = rowsInfo.foreignRows }) - row = rowsInfo.row - foreignRow = rowsInfo.foreignRow - }) + it("enrich populates the foreign fields", async () => { + const res = await getAll(primaryPostgresTable._id, row.id) - it("enrich populates the foreign field", async () => { - const res = await getAll(primaryPostgresTable._id, row.id) + expect(res.status).toBe(200) - expect(res.status).toBe(200) - - expect(foreignRow).toBeDefined() - expect(res.body).toEqual({ - ...row, - linkedField: [ - { - ...foreignRow, - }, - ], + const foreignRowsByType = _.groupBy( + foreignRows, + x => x.relationshipType + ) + expect(res.body).toEqual({ + ...rowData, + [`fk_${oneToManyRelationshipInfo.table.name}_${oneToManyRelationshipInfo.fieldName}`]: + foreignRowsByType[RelationshipTypes.ONE_TO_MANY][0].row.id, + [oneToManyRelationshipInfo.fieldName]: [ + { + ...foreignRowsByType[RelationshipTypes.ONE_TO_MANY][0].row, + _id: expect.any(String), + _rev: expect.any(String), + }, + ], + [manyToOneRelationshipInfo.fieldName]: [ + { + ...foreignRowsByType[RelationshipTypes.MANY_TO_ONE][0].row, + [`fk_${manyToOneRelationshipInfo.table.name}_${manyToOneRelationshipInfo.fieldName}`]: + row.id, + }, + { + ...foreignRowsByType[RelationshipTypes.MANY_TO_ONE][1].row, + [`fk_${manyToOneRelationshipInfo.table.name}_${manyToOneRelationshipInfo.fieldName}`]: + row.id, + }, + { + ...foreignRowsByType[RelationshipTypes.MANY_TO_ONE][2].row, + [`fk_${manyToOneRelationshipInfo.table.name}_${manyToOneRelationshipInfo.fieldName}`]: + row.id, + }, + ], + [manyToManyRelationshipInfo.fieldName]: [ + { + ...foreignRowsByType[RelationshipTypes.MANY_TO_MANY][0].row, + }, + { + ...foreignRowsByType[RelationshipTypes.MANY_TO_MANY][1].row, + }, + ], + id: row.id, + tableId: row.tableId, + _id: expect.any(String), + _rev: expect.any(String), + }) }) }) }) @@ -714,7 +984,7 @@ describe("row api - postgres", () => { const rowsCount = 6 let rows: { row: Row - foreignRow: Row | undefined + foreignRows: ForeignRowsInfo[] rowData: PrimaryRowData }[] beforeEach(async () => { diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index e66795a6db..6871d7b0ba 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -415,9 +415,7 @@ class InternalBuilder { if (opts.disableReturning) { return query.insert(parsedBody) } else { - return query - .insert(parsedBody) - .returning(generateSelectStatement(json, knex)) + return query.insert(parsedBody).returning("*") } } @@ -502,9 +500,7 @@ class InternalBuilder { if (opts.disableReturning) { return query.update(parsedBody) } else { - return query - .update(parsedBody) - .returning(generateSelectStatement(json, knex)) + return query.update(parsedBody).returning("*") } }