Skip to content

Commit 7e0773a

Browse files
authored
PLU-818: [TILES-ATOMIC-INCREMENT-2] use .data() to automically update nested objects (#818)
### TL;DR Introduced a new `patchTableRow` function to atomically update specific columns in a table row while preserving other columns ### What changed? - Currently, the update row action fetches the old data, merges the new changes and writes back. - This new implementation uses [.data()](https://electrodb.dev/en/mutations/update/#data) to patch nested keys. ### How to test? 1. Create a table row with multiple columns 2. Update only specific columns using the update-row action 3. Verify that: - Only specified columns are updated - Non-existent columns are ignored - Other column values remain unchanged - Updates to non-existent rows fail gracefully - Number strings are automatically marshalled correctly ### Why make this change? The previous implementation required fetching the entire row before updating it, which could lead to race conditions and unnecessary data reads. The new atomic patch operation is more efficient and safer, as it updates only the specified columns in a single operation while preserving existing data.
1 parent fd57521 commit 7e0773a

File tree

4 files changed

+159
-40
lines changed

4 files changed

+159
-40
lines changed

packages/backend/src/apps/tiles/__tests__/actions/update-row.itest.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import Context from '@/types/express/context'
1818
import tiles from '../..'
1919
import updateRowAction from '../../actions/update-row'
2020

21-
describe('tiles create row action', () => {
21+
describe('tiles update row action', () => {
2222
let context: Context
2323
let dummyTable: TableMetadata
2424
let dummyColumnIds: string[]
@@ -113,4 +113,32 @@ describe('tiles create row action', () => {
113113
.where({ table_id: $.step.parameters.tableId })
114114
await expect(updateRowAction.run($)).rejects.toThrow(StepError)
115115
})
116+
117+
it('should not fail if row does not exist', async () => {
118+
$.step.parameters.rowId = '123'
119+
await expect(updateRowAction.run($)).resolves.not.toThrow(StepError)
120+
})
121+
122+
it('should not update columns that are not in the table', async () => {
123+
const data = generateMockTableRowData({
124+
columnIds: dummyColumnIds,
125+
})
126+
const row = await createTableRow({ tableId: dummyTable.id, data })
127+
$.step.parameters.rowId = row.rowId
128+
$.step.parameters.rowData = [
129+
{ columnId: 'invalid_column', cellValue: '123' },
130+
{ columnId: dummyColumnIds[0], cellValue: '123' },
131+
]
132+
await updateRowAction.run($)
133+
expect($.setActionItem).toBeCalledWith({
134+
raw: {
135+
rowId: row.rowId,
136+
updated: true,
137+
row: {
138+
...row.data,
139+
[dummyColumnIds[0]]: 123,
140+
},
141+
},
142+
})
143+
})
116144
})

packages/backend/src/apps/tiles/actions/update-row/index.ts

Lines changed: 38 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { IRawAction } from '@plumber/types'
22

33
import StepError from '@/errors/step'
44
import { stripInvalidKeys } from '@/models/dynamodb/helpers'
5-
import { getRawRowById, updateTableRow } from '@/models/dynamodb/table-row'
5+
import { patchTableRow } from '@/models/dynamodb/table-row'
66
import TableCollaborator from '@/models/table-collaborators'
77
import TableColumnMetadata from '@/models/table-column-metadata'
88

@@ -129,50 +129,49 @@ const action: IRawAction = {
129129
return
130130
}
131131

132-
const row = await getRawRowById({
133-
tableId,
134-
rowId,
135-
columnIds,
136-
})
137-
138-
/**
139-
* Row ID does not correspond to a row, we short circuit the action but do not fail it.
140-
*/
141-
if (!row) {
142-
$.setActionItem({
143-
raw: {
144-
updated: false,
145-
} satisfies UpdateRowOutput,
146-
})
147-
return
148-
}
149-
150-
const updatedData = {
151-
...(row.data ?? {}),
132+
const patchData = {
152133
...rowData.reduce((acc, { columnId, cellValue }) => {
153-
acc[columnId] = cellValue
134+
// Check that the column still exists
135+
if (columnIds.includes(columnId)) {
136+
acc[columnId] = cellValue
137+
}
154138
return acc
155139
}, {} as Record<string, string>),
156140
}
141+
try {
142+
const updatedRow = await patchTableRow({
143+
tableId,
144+
rowId,
145+
data: patchData,
146+
})
157147

158-
const strippedUpdatedData = stripInvalidKeys({
159-
columnIds,
160-
data: updatedData,
161-
})
162-
163-
await updateTableRow({
164-
tableId,
165-
rowId,
166-
data: strippedUpdatedData,
167-
})
148+
const updatedRowData = stripInvalidKeys({
149+
columnIds,
150+
data: updatedRow.data,
151+
})
168152

169-
$.setActionItem({
170-
raw: {
171-
row: strippedUpdatedData,
172-
rowId,
173-
updated: true,
174-
} satisfies UpdateRowOutput,
175-
})
153+
$.setActionItem({
154+
raw: {
155+
row: updatedRowData,
156+
rowId,
157+
updated: true,
158+
} satisfies UpdateRowOutput,
159+
})
160+
} catch (e) {
161+
if (
162+
e instanceof Error &&
163+
e.message.includes('The conditional request failed')
164+
) {
165+
// This means the corresponding row does not exist
166+
$.setActionItem({
167+
raw: {
168+
updated: false,
169+
} satisfies UpdateRowOutput,
170+
})
171+
return
172+
}
173+
throw e
174+
}
176175
},
177176
}
178177

packages/backend/src/models/dynamodb/__tests__/table-row/functions.itest.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
getRawRowById,
1717
getTableRowCount,
1818
getTableRows,
19+
patchTableRow,
1920
updateTableRow,
2021
} from '../../table-row/functions'
2122

@@ -333,4 +334,63 @@ describe('dynamodb table row functions', () => {
333334
expect(updatedRow.data).toEqual(newData)
334335
})
335336
})
337+
338+
describe('patchTableRow', () => {
339+
it('should patch the data map for the row, leaving other columns unchanged', async () => {
340+
const data = generateMockTableRowData({
341+
columnIds: dummyColumnIds,
342+
})
343+
const row = await createTableRow({ tableId: dummyTable.id, data })
344+
const newData = generateMockTableRowData({
345+
columnIds: [dummyColumnIds[0]],
346+
})
347+
const updatedRow = await patchTableRow({
348+
tableId: dummyTable.id,
349+
rowId: row.rowId,
350+
data: newData,
351+
})
352+
353+
expect(updatedRow.data).toEqual({ ...data, ...newData })
354+
})
355+
356+
it('should not change if no columns are provided', async () => {
357+
const data = generateMockTableRowData({
358+
columnIds: dummyColumnIds,
359+
})
360+
const row = await createTableRow({ tableId: dummyTable.id, data })
361+
const updatedRow = await patchTableRow({
362+
tableId: dummyTable.id,
363+
rowId: row.rowId,
364+
data: {},
365+
})
366+
367+
expect(updatedRow.data).toEqual(data)
368+
})
369+
370+
it('should auto-marshall the data', async () => {
371+
const data = generateMockTableRowData({
372+
columnIds: dummyColumnIds,
373+
})
374+
const row = await createTableRow({ tableId: dummyTable.id, data })
375+
const updatedRow = await patchTableRow({
376+
tableId: dummyTable.id,
377+
rowId: row.rowId,
378+
data: { [dummyColumnIds[0]]: '123' },
379+
})
380+
expect(updatedRow.data).toEqual({ ...data, [dummyColumnIds[0]]: 123 })
381+
})
382+
383+
it('should update the updatedAt value', async () => {
384+
const data = generateMockTableRowData({
385+
columnIds: dummyColumnIds,
386+
})
387+
const row = await createTableRow({ tableId: dummyTable.id, data })
388+
const updatedRow = await patchTableRow({
389+
tableId: dummyTable.id,
390+
rowId: row.rowId,
391+
data: { [dummyColumnIds[0]]: '123' },
392+
})
393+
expect(updatedRow.updatedAt).toBeGreaterThan(row.updatedAt)
394+
})
395+
})
336396
})

packages/backend/src/models/dynamodb/table-row/functions.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,38 @@ export const updateTableRow = async ({
221221
}
222222
}
223223

224+
/**
225+
* This atomically updates the data object for keys that are changed
226+
*/
227+
export const patchTableRow = async ({
228+
rowId,
229+
tableId,
230+
data: patchData,
231+
}: UpdateRowInput): Promise<TableRowItem> => {
232+
try {
233+
const res = await TableRow.patch({
234+
tableId,
235+
rowId,
236+
})
237+
.data(({ data }, { set }) => {
238+
for (const key in patchData) {
239+
set(
240+
data[key],
241+
patchData[key] ? autoMarshallNumberStrings(patchData[key]) : '',
242+
)
243+
}
244+
})
245+
.go({
246+
ignoreOwnership: true,
247+
// Return the new row data
248+
response: 'all_new',
249+
})
250+
return res.data
251+
} catch (e: unknown) {
252+
handleDynamoDBError(e)
253+
}
254+
}
255+
224256
export const deleteTableRows = async ({
225257
rowIds,
226258
tableId,

0 commit comments

Comments
 (0)