MirrorSave
/
budibase
mirror of https://github.com/Budibase/budibase.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #9777 from Budibase/bug/budi-6619/cannot_update_relationships_in_postgresql_many_to_one 

Bug - budi-6619 - Cannot update relationships in postgresql many to one
This commit is contained in:
Adria Navarro 2023-02-24 10:39:45 +00:00 committed by GitHub
parent 3f50c04335 d5fd635d6c
commit ec1ed02c0d
5 changed files with 451 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.tool-versions
View File

@ -1,2 +1,2 @@
nodejs 14.19.3
python 3.11.1
python 3.10.0

118
packages/server/src/api/controllers/row/ExternalRequest.ts
View File

@ -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)

2
packages/server/src/api/controllers/row/external.ts
View File

@ -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,
})
}

360
packages/server/src/integration-test/postgres.spec.ts
View File

@ -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,14 +69,17 @@ describe("row api - postgres", () => {
},
})
auxPostgresTable = await config.createTable({
name: generator.word({ length: 10 }),
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,
},
@ -89,15 +94,33 @@ describe("row api - postgres", () => {
},
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
let rowData: {
name: string
description: string
value: number
}
let foreignRows: ForeignRowsInfo[]
describe("with all relationship types", () => {
beforeEach(async () => {
let [createdRow] = await populatePrimaryRows(1, {
createForeignRow: true,
createOneToMany: true,
createManyToOne: 3,
createManyToMany: 2,
})
row = createdRow.row
foreignRow = createdRow.foreignRow!
rowData = createdRow.rowData
foreignRows = createdRow.foreignRows
})
it("only foreign keys are retrieved", async () => {
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({
...row,
...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()
})
})
describe("with only one to many", () => {
beforeEach(async () => {
let [createdRow] = await populatePrimaryRows(1, {
createOneToMany: true,
})
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(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.foreignField).toBeUndefined()
expect(res.body[oneToManyRelationshipInfo.fieldName]).toBeUndefined()
})
})
expect(
res.body[`fk_${auxPostgresTable.name}_foreignField`]
).toBeDefined()
expect(res.body[`fk_${auxPostgresTable.name}_foreignField`]).toBe(
foreignRow.id
)
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[]
describe("with all relationship types", () => {
beforeEach(async () => {
rowData = generateRandomPrimaryRowData()
const rowsInfo = await createPrimaryRow({
rowData: generateRandomPrimaryRowData(),
createForeignRow: true,
rowData,
createForeignRows: {
createOneToMany: true,
createManyToOne: 3,
createManyToMany: 2,
},
})
row = rowsInfo.row
foreignRow = rowsInfo.foreignRow
foreignRows = rowsInfo.foreignRows
})
it("enrich populates the foreign field", async () => {
it("enrich populates the foreign fields", async () => {
const res = await getAll(primaryPostgresTable._id, row.id)
expect(res.status).toBe(200)
expect(foreignRow).toBeDefined()
const foreignRowsByType = _.groupBy(
foreignRows,
x => x.relationshipType
)
expect(res.body).toEqual({
...row,
linkedField: [
...rowData,
[`fk_${oneToManyRelationshipInfo.table.name}_${oneToManyRelationshipInfo.fieldName}`]:
foreignRowsByType[RelationshipTypes.ONE_TO_MANY][0].row.id,
[oneToManyRelationshipInfo.fieldName]: [
{
...foreignRow,
...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 () => {

8
packages/server/src/integrations/base/sql.ts
View File

@ -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("*")
}
}