mirror of
https://github.com/payloadcms/payload.git
synced 2026-09-14 20:07:19 +08:00
fix(plugin-cloud-storage): don't skip upload when select projects out filename/mimeType (#16916)
## What Fixes #16670. `getIncomingFiles` (in `@payloadcms/plugin-cloud-storage`, used by `storage-s3` and the other storage adapters) guards the upload on the doc's projected fields: ```ts if (file && data.filename && data.mimeType) { ... } ``` When a create request uses `?select[...]` to project `filename`/`mimeType` out of the returned document, `data.filename`/`data.mimeType` are `undefined`. The guard then fails, `getIncomingFiles` returns `[]`, the `afterChange` hook never calls `adapter.handleUpload`, and the binary is **silently never uploaded** to the bucket — the request still returns `201` and writes the DB row, so it fails silently. The same request without `?select` uploads normally. `req.file` carries the real `name`/`mimetype` the whole time, so the information is available — it's just being read from the wrong place. ## Fix Fall back to the uploaded file's own `name`/`mimetype` when the projected doc lacks them: ```ts const filename = data.filename ?? file?.name const mimeType = data.mimeType ?? file?.mimetype if (file && filename && mimeType) { const mainFile: File = { buffer: file.data, clientUploadContext: file.clientUploadContext, filename, filesize: file.size, mimeType, tempFilePath: file.tempFilePath, } // ... } ``` This is type-safe: `req.file` is payload's `File` type, which already exposes `name`, `mimetype`, `data` and `size` (the latter two are already used here). ## Tests Added `getIncomingFiles.spec.ts` (unit, no infra) covering: - `select` projecting `filename`/`mimeType` out of `data` → the upload still resolves from `req.file` (this is the bug; it returns `[]` before the fix) - `data` values still take precedence when present - no uploaded file → no files returned Verified the new test fails on the previous code and passes with the fix. `pnpm exec vitest run --project unit .../getIncomingFiles.spec.ts` is green and ESLint is clean. --------- Co-authored-by: Alessio Gravili <github@gravili.net>
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
import type { File } from '@payloadcms/plugin-cloud-storage/types'
|
||||
import type { S3StorageOptions } from '@payloadcms/storage-s3'
|
||||
import type { StorageAdapter } from 'payload'
|
||||
|
||||
@@ -38,6 +39,8 @@ import {
|
||||
const filename = fileURLToPath(import.meta.url)
|
||||
const dirname = path.dirname(filename)
|
||||
|
||||
export const uploadedTestFiles = new Map<string, { prefix?: string } & File>()
|
||||
|
||||
export type BuildPluginCloudStorageIntConfigArgs = {
|
||||
/** When false, S3 uses non-composite prefix resolution (single stored prefix segment; pre-composite behavior). */
|
||||
useCompositePrefixes: boolean
|
||||
@@ -162,8 +165,12 @@ export function buildPluginCloudStorageIntConfig({
|
||||
[testMetadataSlug]: {
|
||||
adapter: () => ({
|
||||
name: 'test-metadata-adapter',
|
||||
handleDelete: () => Promise.resolve(),
|
||||
handleDelete: ({ filename }) => {
|
||||
uploadedTestFiles.delete(filename)
|
||||
},
|
||||
handleUpload: ({ data, file }) => {
|
||||
uploadedTestFiles.set(file.filename, { ...file, prefix: data.prefix })
|
||||
|
||||
const metadata = {
|
||||
...data,
|
||||
bucketName: 'test-bucket',
|
||||
@@ -178,6 +185,7 @@ export function buildPluginCloudStorageIntConfig({
|
||||
},
|
||||
staticHandler: () => new Response('Not found', { status: 404 }),
|
||||
}),
|
||||
prefix: 'test-prefix',
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
@@ -21,6 +21,7 @@ export const TestMetadata: CollectionConfig = {
|
||||
],
|
||||
upload: {
|
||||
adminThumbnail: 'thumbnail',
|
||||
formatOptions: { format: 'webp' },
|
||||
imageSizes: [
|
||||
{
|
||||
name: 'thumbnail',
|
||||
|
||||
@@ -14,6 +14,7 @@ import { expect } from 'vitest'
|
||||
import type { Config } from './payload-types.js'
|
||||
|
||||
import { test } from '../__helpers/int/vitest.js'
|
||||
import { uploadedTestFiles } from './buildPluginCloudStorageIntConfig.js'
|
||||
import {
|
||||
mediaSlug,
|
||||
mediaWithCustomURLSlug,
|
||||
@@ -554,6 +555,90 @@ test.suite({ config: './config.ts' })('@payloadcms/plugin-cloud-storage', () =>
|
||||
}
|
||||
}
|
||||
createdIDs.length = 0
|
||||
uploadedTestFiles.clear()
|
||||
})
|
||||
|
||||
test('should upload the original and image sizes when create only selects id', async ({
|
||||
payload,
|
||||
}) => {
|
||||
expect(uploadedTestFiles.size).toBe(0)
|
||||
|
||||
const original = await payload.create({
|
||||
collection: testMetadataSlug,
|
||||
data: {},
|
||||
filePath: path.resolve(dirname, '../uploads/image.png'),
|
||||
})
|
||||
|
||||
createdIDs.push(original.id)
|
||||
|
||||
const upload = await payload.create({
|
||||
collection: testMetadataSlug,
|
||||
data: {},
|
||||
filePath: path.resolve(dirname, '../uploads/image.png'),
|
||||
select: {},
|
||||
})
|
||||
|
||||
createdIDs.push(upload.id)
|
||||
|
||||
expect(upload).toEqual({ id: expect.anything() })
|
||||
|
||||
const saved = await payload.findByID({
|
||||
id: upload.id,
|
||||
collection: testMetadataSlug,
|
||||
})
|
||||
|
||||
expect(saved.filename).toBeTruthy()
|
||||
expect(saved.filename).not.toBe(original.filename)
|
||||
expect(saved.mimeType).toBe('image/webp')
|
||||
expect(saved.sizes?.thumbnail?.filename).toBeTruthy()
|
||||
expect([...uploadedTestFiles.values()]).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
filename: saved.filename,
|
||||
mimeType: saved.mimeType,
|
||||
prefix: 'test-prefix',
|
||||
}),
|
||||
expect.objectContaining({
|
||||
filename: saved.sizes?.thumbnail?.filename,
|
||||
mimeType: saved.sizes?.thumbnail?.mimeType,
|
||||
prefix: 'test-prefix',
|
||||
}),
|
||||
]),
|
||||
)
|
||||
})
|
||||
|
||||
test('should keep overwritten files when update only selects id', async ({ payload }) => {
|
||||
const upload = await payload.create({
|
||||
collection: testMetadataSlug,
|
||||
data: {},
|
||||
filePath: path.resolve(dirname, '../uploads/image.png'),
|
||||
})
|
||||
|
||||
createdIDs.push(upload.id)
|
||||
|
||||
const originalFiles = [...uploadedTestFiles.values()]
|
||||
const updated = await payload.update({
|
||||
id: upload.id,
|
||||
collection: testMetadataSlug,
|
||||
data: {},
|
||||
filePath: path.resolve(dirname, '../uploads/image.png'),
|
||||
overwriteExistingFiles: true,
|
||||
select: {},
|
||||
})
|
||||
|
||||
expect(updated).toEqual({ id: upload.id })
|
||||
expect(uploadedTestFiles.size).toBe(originalFiles.length)
|
||||
|
||||
for (const original of originalFiles) {
|
||||
const replaced = uploadedTestFiles.get(original.filename)
|
||||
|
||||
expect(replaced).not.toBe(original)
|
||||
expect(replaced).toMatchObject({
|
||||
filename: original.filename,
|
||||
mimeType: original.mimeType,
|
||||
prefix: original.prefix,
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
test('should automatically persist metadata returned by custom adapters', async ({
|
||||
|
||||
@@ -335,6 +335,7 @@ export interface TestMetadatum {
|
||||
* Test note to identify this upload
|
||||
*/
|
||||
testNote?: string | null;
|
||||
prefix?: string | null;
|
||||
updatedAt: string;
|
||||
createdAt: string;
|
||||
url?: string | null;
|
||||
@@ -687,6 +688,7 @@ export interface RestrictedMediaSelect<T extends boolean = true> {
|
||||
*/
|
||||
export interface TestMetadataSelect<T extends boolean = true> {
|
||||
testNote?: T;
|
||||
prefix?: T;
|
||||
updatedAt?: T;
|
||||
createdAt?: T;
|
||||
url?: T;
|
||||
@@ -797,4 +799,4 @@ export interface Auth {
|
||||
declare module 'payload' {
|
||||
// @ts-ignore
|
||||
export interface GeneratedTypes extends Config {}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user