Content-Length: 1078004 | pFad | https://github.com/payloadcms/payload/commit/e526c88

A4 fix: regenerate reused array and block row IDs during bulk update (3.… · payloadcms/payload@e526c88 · GitHub
Skip to content

Commit e526c88

Browse files
GermanJabloGermanJablo
authored
fix: regenerate reused array and block row IDs during bulk update (3.x) (#17018)
3.x backport of #17017. ## What Fixes #13783 on the `3.x` branch. On relational databases (Postgres/SQLite) every array and block row lives in its own sub-table, and the row `id` is the **primary key, unique across the whole table** (not just within one document). A bulk edit applies the **same submitted data**, including the row ids the admin UI generated, to **every** matched document. The first document inserts fine; the next one tries to insert a row with an id that already exists, causing a duplicate-key `ValidationError` (Postgres `23505`). ## How In the bulk update operation, before applying the shared data to each matched document, we make a structural copy of the incoming data guided by the collection schema. During that single pass, for every array/block row we check the row `id` against the set of ids already stored in **that** document: - if the id already belongs to the document, we keep it and the row is updated in place; - if the id is not in the document, the row is new to that document, so we drop the client-supplied id and let Payload generate a fresh unique one (the same path as a row created without an id). Why this is safe on both fronts: - We never change an id that should stay: only ids absent from the document's stored rows are dropped, so an existing row keeps its id (and its saved values, including translations). - We never miss an id that should change: any row not already in the document gets a fresh id. This is scoped to the **bulk** update operation only. Single-document updates go through `updateByID` and are untouched, so there is no per-save cost on normal edits. The copy walks the schema recursively, so nested arrays/blocks, groups, and localized arrays are handled; the document's existing row ids are gathered with `traverseFields`. ## Tests Added integration tests in `test/array-update/int.spec.ts`: - A reused row id across two docs no longer collides; each doc receives its row with fresh, distinct ids (top-level and nested arrays). - A bulk update that reuses an existing row's id keeps that row's id and updates it in place, while other docs get fresh ids. - Localized arrays, a group with a nested array, and blocks with a nested array all get fresh ids per document on bulk update. Verified the new tests fail without the fix (duplicate-key errors on Postgres) and pass with it on both Postgres and MongoDB. --------- Co-authored-by: German Jablonski <GermanJablo@users.noreply.github.com>
1 parent a18ae22 commit e526c88

5 files changed

Lines changed: 494 additions & 5 deletions

File tree

‎packages/payload/src/collections/operations/update.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { combineQueries } from '../../database/combineQueries.js'
1717
import { validateQueryPaths } from '../../database/queryValidation/validateQueryPaths.js'
1818
import { sanitizeWhereQuery } from '../../database/sanitizeWhereQuery.js'
1919
import { APIError } from '../../errors/index.js'
20-
import { type CollectionSlug, deepCopyObjectSimple, type FindOptions } from '../../index.js'
20+
import { type CollectionSlug, type FindOptions } from '../../index.js'
2121
import { generateFileData } from '../../uploads/generateFileData.js'
2222
import { unlinkTempFiles } from '../../uploads/unlinkTempFiles.js'
2323
import { appendNonTrashedFilter } from '../../utilities/appendNonTrashedFilter.js'
@@ -32,6 +32,7 @@ import { appendVersionToQueryKey } from '../../versions/drafts/appendVersionToQu
3232
import { getQueryDraftsSort } from '../../versions/drafts/getQueryDraftsSort.js'
3333
import { buildAfterOperation } from './utilities/buildAfterOperation.js'
3434
import { buildBeforeOperation } from './utilities/buildBeforeOperation.js'
35+
import { copyDataWithFreshRowIDs } from './utilities/copyDataWithFreshRowIDs.js'
3536
import { sanitizeSortQuery } from './utilities/sanitizeSortQuery.js'
3637
import { updateDocument } from './utilities/update.js'
3738

@@ -255,7 +256,12 @@ export const updateOperation = async <
255256
autosave,
256257
collectionConfig,
257258
config,
258-
data: deepCopyObjectSimple(data),
259+
data: copyDataWithFreshRowIDs({
260+
config,
261+
data,
262+
existingDoc: docWithLocales,
263+
fields: collectionConfig.fields,
264+
}),
259265
depth: depth!,
260266
docWithLocales,
261267
draftArg,

‎packages/payload/src/collections/operations/utilities/copyDataWithFreshRowIDs.ts‎

Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,270 @@
1+
import type { SanitizedConfig } from '../../../config/types.js'
2+
import type { ArrayField, Block, BlocksField, Field } from '../../../fields/config/types.js'
3+
4+
import { tabHasName } from '../../../fields/config/types.js'
5+
import { deepCopyObjectSimple } from '../../../utilities/deepCopyObject.js'
6+
import { traverseFields } from '../../../utilities/traverseFields.js'
7+
8+
type LevelEntry =
9+
| { field: ArrayField | BlocksField; localized: boolean; type: 'rows' }
10+
| { fields: Field[]; localized: boolean; type: 'subfields' }
11+
12+
const buildLevelMap = (fields: Field[], map: Map<string, LevelEntry> = new Map()) => {
13+
for (const field of fields) {
14+
if (field.type === 'row' || field.type === 'collapsible') {
15+
buildLevelMap(field.fields, map)
16+
} else if (field.type === 'tabs') {
17+
for (const tab of field.tabs) {
18+
if (tabHasName(tab)) {
19+
map.set(tab.name, {
20+
type: 'subfields',
21+
fields: tab.fields,
22+
localized: Boolean(tab.localized),
23+
})
24+
} else {
25+
buildLevelMap(tab.fields, map)
26+
}
27+
}
28+
} else if (field.type === 'group') {
29+
if ('name' in field && field.name) {
30+
map.set(field.name, {
31+
type: 'subfields',
32+
fields: field.fields,
33+
localized: Boolean(field.localized),
34+
})
35+
} else {
36+
buildLevelMap(field.fields, map)
37+
}
38+
} else if ((field.type === 'array' || field.type === 'blocks') && field.name) {
39+
map.set(field.name, { type: 'rows', field, localized: Boolean(field.localized) })
40+
}
41+
}
42+
return map
43+
}
44+
45+
const isLocaleKeyed = (value: object, config: SanitizedConfig): boolean => {
46+
if (Array.isArray(value)) {
47+
return false
48+
}
49+
const localeCodes = config.localization ? config.localization.localeCodes : []
50+
const keys = Object.keys(value)
51+
return keys.length > 0 && keys.every((key) => localeCodes.includes(key))
52+
}
53+
54+
const resolveBlockFields = (
55+
field: BlocksField,
56+
blockType: unknown,
57+
config: SanitizedConfig,
58+
): Field[] => {
59+
if (typeof blockType !== 'string') {
60+
return []
61+
}
62+
const block =
63+
config.blocks?.find((b) => b.slug === blockType) ??
64+
field.blocks.find((b): b is Block => typeof b !== 'string' && b.slug === blockType)
65+
return block?.fields ?? []
66+
}
67+
68+
const copyRow = ({
69+
config,
70+
existingRowIDs,
71+
field,
72+
parentIsLocalized,
73+
row,
74+
}: {
75+
config: SanitizedConfig
76+
existingRowIDs: Set<string>
77+
field: ArrayField | BlocksField
78+
parentIsLocalized: boolean
79+
row: unknown
80+
}): unknown => {
81+
if (!row || typeof row !== 'object') {
82+
return deepCopyObjectSimple(row as never)
83+
}
84+
85+
const subfields =
86+
field.type === 'array'
87+
? field.fields
88+
: resolveBlockFields(field, (row as Record<string, unknown>).blockType, config)
89+
90+
const newRow = copyObject({
91+
config,
92+
data: row as Record<string, unknown>,
93+
existingRowIDs,
94+
fields: subfields,
95+
parentIsLocalized,
96+
})
97+
98+
if (typeof newRow.id === 'string' && !existingRowIDs.has(newRow.id)) {
99+
delete newRow.id
100+
}
101+
102+
return newRow
103+
}
104+
105+
const copyObject = ({
106+
config,
107+
data,
108+
existingRowIDs,
109+
fields,
110+
parentIsLocalized,
111+
}: {
112+
config: SanitizedConfig
113+
data: Record<string, unknown>
114+
existingRowIDs: Set<string>
115+
fields: Field[]
116+
parentIsLocalized: boolean
117+
}): Record<string, unknown> => {
118+
const levelMap = buildLevelMap(fields)
119+
const result: Record<string, unknown> = {}
120+
121+
for (const key of Object.keys(data)) {
122+
const value = data[key]
123+
const entry = levelMap.get(key)
124+
125+
if (!entry || value === null || typeof value !== 'object') {
126+
result[key] = deepCopyObjectSimple(value as never)
127+
continue
128+
}
129+
130+
const childParentIsLocalized = parentIsLocalized || entry.localized
131+
132+
if (entry.type === 'subfields') {
133+
const valueIsLocaleKeyed =
134+
entry.localized && !parentIsLocalized && isLocaleKeyed(value, config)
135+
136+
result[key] = valueIsLocaleKeyed
137+
? mapLocales(value, (localeValue) =>
138+
copyObject({
139+
config,
140+
data: localeValue,
141+
existingRowIDs,
142+
fields: entry.fields,
143+
parentIsLocalized: true,
144+
}),
145+
)
146+
: copyObject({
147+
config,
148+
data: value as Record<string, unknown>,
149+
existingRowIDs,
150+
fields: entry.fields,
151+
parentIsLocalized: childParentIsLocalized,
152+
})
153+
continue
154+
}
155+
156+
const copyRows = (rows: unknown) =>
157+
Array.isArray(rows)
158+
? rows.map((row) =>
159+
copyRow({
160+
config,
161+
existingRowIDs,
162+
field: entry.field,
163+
parentIsLocalized: childParentIsLocalized,
164+
row,
165+
}),
166+
)
167+
: deepCopyObjectSimple(rows as never)
168+
169+
result[key] = Array.isArray(value) ? copyRows(value) : mapLocales(value, copyRows)
170+
}
171+
172+
return result
173+
}
174+
175+
const mapLocales = (
176+
value: object,
177+
copyLocaleValue: (localeValue: Record<string, unknown>) => unknown,
178+
): unknown => {
179+
if (Array.isArray(value)) {
180+
return deepCopyObjectSimple(value as never)
181+
}
182+
const out: Record<string, unknown> = {}
183+
for (const locale of Object.keys(value)) {
184+
const localeValue = (value as Record<string, unknown>)[locale]
185+
out[locale] =
186+
localeValue && typeof localeValue === 'object'
187+
? copyLocaleValue(localeValue as Record<string, unknown>)
188+
: deepCopyObjectSimple(localeValue as never)
189+
}
190+
return out
191+
}
192+
193+
const collectExistingRowIDs = ({
194+
config,
195+
existingDoc,
196+
fields,
197+
}: {
198+
config: SanitizedConfig
199+
existingDoc: Record<string, unknown>
200+
fields: Field[]
201+
}): Set<string> => {
202+
const existingRowIDs = new Set<string>()
203+
204+
traverseFields({
205+
callback: ({ field, ref }) => {
206+
if (
207+
(field.type !== 'array' && field.type !== 'blocks') ||
208+
!('name' in field) ||
209+
!field.name ||
210+
!ref ||
211+
typeof ref !== 'object'
212+
) {
213+
return
214+
}
215+
216+
const value = (ref as Record<string, unknown>)[field.name]
217+
218+
const visitRows = (rows: unknown) => {
219+
if (!Array.isArray(rows)) {
220+
return
221+
}
222+
for (const row of rows) {
223+
if (
224+
row &&
225+
typeof row === 'object' &&
226+
typeof (row as Record<string, unknown>).id === 'string'
227+
) {
228+
existingRowIDs.add((row as Record<string, unknown>).id as string)
229+
}
230+
}
231+
}
232+
233+
if (Array.isArray(value)) {
234+
visitRows(value)
235+
} else if (value && typeof value === 'object') {
236+
for (const localeValue of Object.values(value as Record<string, unknown>)) {
237+
visitRows(localeValue)
238+
}
239+
}
240+
},
241+
config,
242+
fields,
243+
fillEmpty: false,
244+
ref: existingDoc,
245+
})
246+
247+
return existingRowIDs
248+
}
249+
250+
export const copyDataWithFreshRowIDs = ({
251+
config,
252+
data,
253+
existingDoc,
254+
fields,
255+
}: {
256+
config: SanitizedConfig
257+
data: Record<string, unknown>
258+
existingDoc: Record<string, unknown>
259+
fields: Field[]
260+
}): Record<string, unknown> => {
261+
const existingRowIDs = collectExistingRowIDs({ config, existingDoc, fields })
262+
263+
return copyObject({
264+
config,
265+
data,
266+
existingRowIDs,
267+
fields,
268+
parentIsLocalized: false,
269+
})
270+
}

‎test/array-update/config.ts‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const filename = fileURLToPath(import.meta.url)
44
const dirname = path.dirname(filename)
55
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
66
import { devUser } from '../credentials.js'
7-
import { arraySlug } from './shared.js'
7+
import { arraySlug, complexSlug } from './shared.js'
88

99
export default buildConfigWithDefaults({
1010
admin: {
@@ -51,7 +51,85 @@ export default buildConfigWithDefaults({
5151
},
5252
],
5353
},
54+
{
55+
slug: complexSlug,
56+
fields: [
57+
{
58+
name: 'localizedArray',
59+
type: 'array',
60+
localized: true,
61+
fields: [
62+
{
63+
name: 'text',
64+
type: 'text',
65+
},
66+
],
67+
},
68+
{
69+
name: 'group',
70+
type: 'group',
71+
fields: [
72+
{
73+
name: 'groupArray',
74+
type: 'array',
75+
fields: [
76+
{
77+
name: 'text',
78+
type: 'text',
79+
},
80+
],
81+
},
82+
],
83+
},
84+
{
85+
name: 'localizedGroup',
86+
type: 'group',
87+
localized: true,
88+
fields: [
89+
{
90+
name: 'localizedGroupArray',
91+
type: 'array',
92+
fields: [
93+
{
94+
name: 'text',
95+
type: 'text',
96+
},
97+
],
98+
},
99+
],
100+
},
101+
{
102+
name: 'blocks',
103+
type: 'blocks',
104+
blocks: [
105+
{
106+
slug: 'content',
107+
fields: [
108+
{
109+
name: 'text',
110+
type: 'text',
111+
},
112+
{
113+
name: 'innerArray',
114+
type: 'array',
115+
fields: [
116+
{
117+
name: 'text',
118+
type: 'text',
119+
},
120+
],
121+
},
122+
],
123+
},
124+
],
125+
},
126+
],
127+
},
54128
],
129+
localization: {
130+
defaultLocale: 'en',
131+
locales: ['en', 'es'],
132+
},
55133
onInit: async (payload) => {
56134
await payload.create({
57135
collection: 'users',

0 commit comments

Comments
 (0)








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/payloadcms/payload/commit/e526c88

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy