pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


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

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

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

Commit e526c88

Browse files
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 <[email protected]>
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,
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)
pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.





Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy