Skip to content

Commit 7d80d21

Browse files
JessRynkarJessRynkar
authored
fix(plugin-cloud-storage): should persist external data returned by handleUpload (#15188)
### What Fixes issue where storage metadata (object keys, timestamps, custom IDs, etc.) from external providers (s3, azure, etc) is no longer persisted back to the main doc. This issue was introduced in PR #14988 when the external upload process was moved from the `beforeChange` to `afterChange` hook. ### Why Without automatically persisting metadata from external storage, users have to manually update the doc themselves. This restores backward compatibility for existing custom storage adapters. ### Where Added automatic data capture/persistence in `packages/plugin-cloud-storage/src/hooks/afterChange` #### Testing Added to test suite `test/plugin-cloud-storage`
1 parent a8bfade commit 7d80d21

7 files changed

Lines changed: 302 additions & 28 deletions

File tree

‎packages/plugin-cloud-storage/src/hooks/afterChange.ts‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,44 @@ export const getAfterChangeHook =
4242
await Promise.all(deletionPromises)
4343
}
4444

45-
const promises = files.map(async (file) => {
46-
await adapter.handleUpload({
47-
clientUploadContext: file.clientUploadContext,
48-
collection,
49-
data: doc,
50-
file,
51-
req,
52-
})
53-
})
45+
const uploadResults = await Promise.all(
46+
files.map((file) =>
47+
adapter.handleUpload({
48+
clientUploadContext: file.clientUploadContext,
49+
collection,
50+
data: doc,
51+
file,
52+
req,
53+
}),
54+
),
55+
)
56+
57+
const uploadMetadata = uploadResults
58+
.filter(
59+
(result): result is Partial<FileData & TypeWithID> =>
60+
result != null && typeof result === 'object',
61+
)
62+
.reduce(
63+
(acc, metadata) => ({ ...acc, ...metadata }),
64+
{} as Partial<FileData & TypeWithID>,
65+
)
5466

55-
await Promise.all(promises)
67+
if (Object.keys(uploadMetadata).length > 0) {
68+
try {
69+
await req.payload.update({
70+
id: doc.id,
71+
collection: collection.slug,
72+
data: uploadMetadata,
73+
depth: 0,
74+
req,
75+
})
76+
return { ...doc, ...uploadMetadata }
77+
} catch (updateError: unknown) {
78+
req.payload.logger.warn(
79+
`Failed to persist upload data for collection ${collection.slug} document ${doc.id}: ${String(updateError)}`,
80+
)
81+
}
82+
}
5683
}
5784
} catch (err: unknown) {
5885
req.payload.logger.error(

‎packages/plugin-cloud-storage/src/types.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ export type HandleUpload = (args: {
3434
data: any
3535
file: File
3636
req: PayloadRequest
37-
}) => Promise<void> | void
37+
}) =>
38+
| Partial<FileData & TypeWithID>
39+
| Promise<Partial<FileData & TypeWithID>>
40+
| Promise<void>
41+
| void
3842

3943
export interface TypeWithPrefix {
4044
prefix?: string

‎test/plugin-cloud-storage/collections/TestMetadata.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import type { CollectionConfig } from 'payload'
2+
3+
import { testMetadataSlug } from '../shared.js'
4+
5+
export const TestMetadata: CollectionConfig = {
6+
slug: testMetadataSlug,
7+
access: {
8+
create: () => true,
9+
read: () => true,
10+
update: () => true,
11+
delete: () => true,
12+
},
13+
fields: [
14+
{
15+
name: 'testNote',
16+
type: 'text',
17+
admin: {
18+
description: 'Test note to identify this upload',
19+
},
20+
},
21+
],
22+
upload: {
23+
adminThumbnail: 'thumbnail',
24+
imageSizes: [
25+
{
26+
name: 'thumbnail',
27+
width: 300,
28+
},
29+
],
30+
},
31+
}

‎test/plugin-cloud-storage/config.ts‎

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Plugin } from 'payload'
22

3+
import { cloudStoragePlugin } from '@payloadcms/plugin-cloud-storage'
34
import { azureStorage } from '@payloadcms/storage-azure'
45
import { gcsStorage } from '@payloadcms/storage-gcs'
56
import { s3Storage } from '@payloadcms/storage-s3'
@@ -12,14 +13,21 @@ import { devUser } from '../credentials.js'
1213
import { Media } from './collections/Media.js'
1314
import { MediaWithPrefix } from './collections/MediaWithPrefix.js'
1415
import { RestrictedMedia } from './collections/RestrictedMedia.js'
16+
import { TestMetadata } from './collections/TestMetadata.js'
1517
import { Users } from './collections/Users.js'
16-
import { mediaSlug, mediaWithPrefixSlug, restrictedMediaSlug, prefix } from './shared.js'
18+
import {
19+
mediaSlug,
20+
mediaWithPrefixSlug,
21+
prefix,
22+
restrictedMediaSlug,
23+
testMetadataSlug,
24+
} from './shared.js'
1725
import { createTestBucket } from './utils.js'
1826

1927
const filename = fileURLToPath(import.meta.url)
2028
const dirname = path.dirname(filename)
2129

22-
let storagePlugin: Plugin
30+
let storagePlugin: Plugin = {} as Plugin
2331
let uploadOptions
2432

2533
// Load config to work with emulated services
@@ -37,9 +45,9 @@ if (process.env.PAYLOAD_PUBLIC_CLOUD_STORAGE_ADAPTER === 'azure') {
3745
[restrictedMediaSlug]: true,
3846
},
3947
allowContainerCreate: process.env.AZURE_STORAGE_ALLOW_CONTAINER_CREATE === 'true',
40-
baseURL: process.env.AZURE_STORAGE_ACCOUNT_BASEURL,
41-
connectionString: process.env.AZURE_STORAGE_CONNECTION_STRING,
42-
containerName: process.env.AZURE_STORAGE_CONTAINER_NAME,
48+
baseURL: process.env.AZURE_STORAGE_ACCOUNT_BASEURL!,
49+
connectionString: process.env.AZURE_STORAGE_CONNECTION_STRING!,
50+
containerName: process.env.AZURE_STORAGE_CONTAINER_NAME!,
4351
})
4452
}
4553

@@ -52,7 +60,7 @@ if (process.env.PAYLOAD_PUBLIC_CLOUD_STORAGE_ADAPTER === 'gcs') {
5260
},
5361
[restrictedMediaSlug]: true,
5462
},
55-
bucket: process.env.GCS_BUCKET,
63+
bucket: process.env.GCS_BUCKET!,
5664
options: {
5765
apiEndpoint: process.env.GCS_ENDPOINT,
5866
projectId: process.env.GCS_PROJECT_ID,
@@ -77,11 +85,11 @@ if (
7785
},
7886
[restrictedMediaSlug]: true,
7987
},
80-
bucket: process.env.S3_BUCKET,
88+
bucket: process.env.S3_BUCKET ?? '',
8189
config: {
8290
credentials: {
83-
accessKeyId: process.env.S3_ACCESS_KEY_ID,
84-
secretAccessKey: process.env.S3_SECRET_ACCESS_KEY,
91+
accessKeyId: process.env.S3_ACCESS_KEY_ID ?? '',
92+
secretAccessKey: process.env.S3_SECRET_ACCESS_KEY ?? '',
8593
},
8694
endpoint: process.env.S3_ENDPOINT,
8795
forcePathStyle: process.env.S3_FORCE_PATH_STYLE === 'true',
@@ -98,11 +106,11 @@ if (process.env.PAYLOAD_PUBLIC_CLOUD_STORAGE_ADAPTER === 'r2') {
98106
prefix,
99107
},
100108
},
101-
bucket: process.env.R2_BUCKET,
109+
bucket: process.env.R2_BUCKET ?? '',
102110
config: {
103111
credentials: {
104-
accessKeyId: process.env.S3_ACCESS_KEY_ID,
105-
secretAccessKey: process.env.S3_SECRET_ACCESS_KEY,
112+
accessKeyId: process.env.S3_ACCESS_KEY_ID ?? '',
113+
secretAccessKey: process.env.S3_SECRET_ACCESS_KEY ?? '',
106114
},
107115
endpoint: process.env.S3_ENDPOINT,
108116
forcePathStyle: process.env.S3_FORCE_PATH_STYLE === 'true',
@@ -111,13 +119,38 @@ if (process.env.PAYLOAD_PUBLIC_CLOUD_STORAGE_ADAPTER === 'r2') {
111119
})
112120
}
113121

122+
const testMetadataPlugin = cloudStoragePlugin({
123+
collections: {
124+
[testMetadataSlug]: {
125+
adapter: () => ({
126+
name: 'test-metadata-adapter',
127+
handleUpload: ({ file, data }) => {
128+
const metadata = {
129+
...data,
130+
customStorageId: `storage-${Date.now()}`,
131+
uploadTimestamp: new Date().toISOString(),
132+
storageProvider: 'test-adapter',
133+
bucketName: 'test-bucket',
134+
objectKey: data.filename || file.filename, // Use the processed filename from data
135+
processingStatus: 'completed',
136+
uploadVersion: '1.0.0',
137+
}
138+
return metadata
139+
},
140+
handleDelete: () => Promise.resolve(),
141+
staticHandler: () => new Response('Not found', { status: 404 }),
142+
}),
143+
},
144+
},
145+
})
146+
114147
export default buildConfigWithDefaults({
115148
admin: {
116149
importMap: {
117150
baseDir: path.resolve(dirname),
118151
},
119152
},
120-
collections: [Media, MediaWithPrefix, RestrictedMedia, Users],
153+
collections: [Media, MediaWithPrefix, RestrictedMedia, TestMetadata, Users],
121154
onInit: async (payload) => {
122155
/*const client = new AWS.S3({
123156
endpoint: process.env.S3_ENDPOINT,
@@ -151,7 +184,7 @@ export default buildConfigWithDefaults({
151184
`Using plugin-cloud-storage adapter: ${process.env.PAYLOAD_PUBLIC_CLOUD_STORAGE_ADAPTER}`,
152185
)
153186
},
154-
plugins: [storagePlugin],
187+
plugins: [storagePlugin, testMetadataPlugin],
155188
upload: uploadOptions,
156189
typescript: {
157190
outputFile: path.resolve(dirname, 'payload-types.ts'),

‎test/plugin-cloud-storage/int.spec.ts‎

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,13 @@ import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest'
1010
import type { Config } from './payload-types.js'
1111

1212
import { initPayloadInt } from '../helpers/initPayloadInt.js'
13-
import { mediaSlug, mediaWithPrefixSlug, prefix, restrictedMediaSlug } from './shared.js'
13+
import {
14+
mediaSlug,
15+
mediaWithPrefixSlug,
16+
prefix,
17+
restrictedMediaSlug,
18+
testMetadataSlug,
19+
} from './shared.js'
1420
import { clearTestBucket, createTestBucket } from './utils.js'
1521

1622
const filename = fileURLToPath(import.meta.url)
@@ -149,6 +155,107 @@ describe('@payloadcms/plugin-cloud-storage', () => {
149155
})
150156
})
151157

158+
describe('External data persistence', () => {
159+
const createdIDs: (number | string)[] = []
160+
161+
afterEach(async () => {
162+
for (const id of createdIDs) {
163+
try {
164+
await payload.delete({ collection: testMetadataSlug, id })
165+
} catch (e) {
166+
// Ignore
167+
}
168+
}
169+
createdIDs.length = 0
170+
})
171+
172+
it('should automatically persist metadata returned by custom adapters', async () => {
173+
const upload = await payload.create({
174+
collection: testMetadataSlug,
175+
data: {
176+
testNote: 'Testing automatic metadata persistence',
177+
},
178+
filePath: path.resolve(dirname, '../uploads/image.png'),
179+
})
180+
181+
createdIDs.push(upload.id)
182+
183+
expect(upload.id).toBeTruthy()
184+
expect(upload.filename).toBeTruthy()
185+
expect(upload.testNote).toBe('Testing automatic metadata persistence')
186+
187+
// Our afterChange hook should automatically persist whatever metadata the adapter returns
188+
expect(upload.customStorageId).toBeTruthy()
189+
expect(upload.customStorageId).toContain('storage-')
190+
expect(upload.uploadTimestamp).toBeTruthy()
191+
expect(upload.storageProvider).toBe('test-adapter')
192+
expect(upload.bucketName).toBe('test-bucket')
193+
expect(upload.objectKey).toBe(upload.filename)
194+
expect(upload.processingStatus).toBe('completed')
195+
expect(upload.uploadVersion).toBe('1.0.0')
196+
197+
console.log('Test adapter metadata automatically persisted:', {
198+
customStorageId: upload.customStorageId,
199+
uploadTimestamp: upload.uploadTimestamp,
200+
storageProvider: upload.storageProvider,
201+
bucketName: upload.bucketName,
202+
objectKey: upload.objectKey,
203+
processingStatus: upload.processingStatus,
204+
uploadVersion: upload.uploadVersion,
205+
})
206+
})
207+
208+
it('should persist metadata on update operations', async () => {
209+
const upload = await payload.create({
210+
collection: testMetadataSlug,
211+
data: {
212+
testNote: 'Testing update metadata persistence',
213+
},
214+
filePath: path.resolve(dirname, '../uploads/image.png'),
215+
})
216+
217+
createdIDs.push(upload.id)
218+
219+
const initialStorageId = upload.customStorageId
220+
const initialTimestamp = upload.uploadTimestamp
221+
222+
const updatedUpload = await payload.update({
223+
collection: testMetadataSlug,
224+
id: upload.id,
225+
data: {
226+
testNote: 'Updated test note',
227+
},
228+
filePath: path.resolve(dirname, './image.png'),
229+
})
230+
231+
expect(updatedUpload.testNote).toBe('Updated test note')
232+
233+
// Test that metadata persistence works on updates too
234+
expect(updatedUpload.customStorageId).toBeTruthy()
235+
expect(updatedUpload.uploadTimestamp).toBeTruthy()
236+
expect(updatedUpload.storageProvider).toBe('test-adapter')
237+
expect(updatedUpload.bucketName).toBe('test-bucket')
238+
expect(updatedUpload.objectKey).toBe(updatedUpload.filename)
239+
expect(updatedUpload.processingStatus).toBe('completed')
240+
expect(updatedUpload.uploadVersion).toBe('1.0.0')
241+
242+
const filenamesAreDifferent = upload.filename !== updatedUpload.filename
243+
const storageIdsAreDifferent = updatedUpload.customStorageId !== initialStorageId
244+
const timestampsAreDifferent = updatedUpload.uploadTimestamp !== initialTimestamp
245+
246+
// If filename changed, storage ID and timestamp should also change (new upload)
247+
expect(filenamesAreDifferent).toBe(storageIdsAreDifferent)
248+
expect(filenamesAreDifferent).toBe(timestampsAreDifferent)
249+
250+
console.log('Update test adapter metadata persistence:', {
251+
filenameChanged: filenamesAreDifferent,
252+
storageIdChanged: storageIdsAreDifferent,
253+
timestampChanged: timestampsAreDifferent,
254+
newStorageId: updatedUpload.customStorageId,
255+
})
256+
})
257+
})
258+
152259
describe('Azure', () => {
153260
it.todo('can upload')
154261
})

0 commit comments

Comments
 (0)