Skip to content

Commit ccfaee0

Browse files
authored
fix(storage-*): upload filename respects prefix (#14566)
## Overview Fixes filename collision detection to respect storage adapter prefix/suffix patterns. When using cloud storage with prefixes (e.g., multi-tenant scenarios), files with identical names under different prefixes were incorrectly triggering deduplication. Resolves #14561 ## Changes ### Core Upload Logic - **`docWithFilenameExists`**: Now accepts and checks `prefix` parameter in WHERE clause, scoping collision detection to prefix namespace - **`getSafeFileName`**: Accepts `prefix` parameter and passes to collision check - **`generateFileData`**: Extracts `prefix` from data (set by storage plugins) and threads through to collision detection ### Test Coverage - **New collection `MediaWithDynamicPrefix`**: Tests multi-tenant scenario with hook-based prefix assignment - **Updated `MediaWithPrefix`**: Added `prefix` field and `filenameCompoundIndex: true` - **Comprehensive test suite**: Validates collision within prefix, no collision across prefixes, backward compatibility for collections without prefix, and dynamic prefix from hooks
1 parent d86c174 commit ccfaee0

File tree

9 files changed

+289
-28
lines changed

9 files changed

+289
-28
lines changed
Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,34 @@
1-
import type { PayloadRequest } from '../types/index.js'
1+
import type { PayloadRequest, Where } from '../types/index.js'
22

33
type Args = {
44
collectionSlug: string
55
filename: string
66
path: string
7+
prefix?: string
78
req: PayloadRequest
89
}
910

1011
export const docWithFilenameExists = async ({
1112
collectionSlug,
1213
filename,
14+
prefix,
1315
req,
1416
}: Args): Promise<boolean> => {
17+
const where: Where = {
18+
filename: {
19+
equals: filename,
20+
},
21+
}
22+
23+
if (prefix) {
24+
where.prefix = { equals: prefix }
25+
}
26+
1527
const doc = await req.payload.db.findOne({
1628
collection: collectionSlug,
1729
req,
18-
where: {
19-
filename: {
20-
equals: filename,
21-
},
22-
},
30+
where,
2331
})
24-
if (doc) {
25-
return true
26-
}
2732

28-
return false
33+
return !!doc
2934
}

packages/payload/src/uploads/generateFileData.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,12 @@ export const generateFileData = async <T>({
255255
fsSafeName = `${baseFilename}${ext ? `.${ext}` : ''}`
256256

257257
if (!overwriteExistingFiles) {
258+
// Extract prefix if present (added by plugin-cloud-storage)
259+
const prefix = (data as Record<string, unknown>)?.prefix as string | undefined
258260
fsSafeName = await getSafeFileName({
259261
collectionSlug: collectionConfig.slug,
260262
desiredFilename: fsSafeName,
263+
prefix,
261264
req,
262265
staticPath: staticPath!,
263266
})

packages/payload/src/uploads/getSafeFilename.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@ const incrementName = (name: string) => {
2525
type Args = {
2626
collectionSlug: string
2727
desiredFilename: string
28+
prefix?: string
2829
req: PayloadRequest
2930
staticPath: string
3031
}
3132

3233
export async function getSafeFileName({
3334
collectionSlug,
3435
desiredFilename,
36+
prefix,
3537
req,
3638
staticPath,
3739
}: Args): Promise<string> {
@@ -42,6 +44,7 @@ export async function getSafeFileName({
4244
collectionSlug,
4345
filename: modifiedFilename,
4446
path: staticPath,
47+
prefix,
4548
req,
4649
})) ||
4750
(await fileExists(`${staticPath}/${modifiedFilename}`))
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import type { CollectionConfig } from 'payload'
2+
3+
export const MediaWithDynamicPrefix: CollectionConfig<'media-with-dynamic-prefix'> = {
4+
slug: 'media-with-dynamic-prefix',
5+
fields: [
6+
{
7+
name: 'tenant',
8+
type: 'text',
9+
required: true,
10+
},
11+
{
12+
name: 'prefix',
13+
type: 'text',
14+
},
15+
],
16+
hooks: {
17+
beforeOperation: [
18+
({ args, operation }) => {
19+
if (operation === 'create') {
20+
return {
21+
...args,
22+
data: {
23+
...args.data,
24+
prefix: `tenant-${args.data.tenant}`,
25+
},
26+
}
27+
}
28+
return args
29+
},
30+
],
31+
},
32+
upload: {
33+
disableLocalStorage: false,
34+
filenameCompoundIndex: ['filename', 'prefix'],
35+
},
36+
}

test/storage-s3/collections/MediaWithPrefix.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@ export const MediaWithPrefix: CollectionConfig = {
44
slug: 'media-with-prefix',
55
upload: {
66
disableLocalStorage: false,
7+
filenameCompoundIndex: ['filename', 'prefix'],
78
},
8-
fields: [],
9+
fields: [
10+
{
11+
name: 'prefix',
12+
type: 'text',
13+
},
14+
],
915
}

test/storage-s3/config.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,17 @@ import path from 'path'
66
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
77
import { devUser } from '../credentials.js'
88
import { Media } from './collections/Media.js'
9+
import { MediaWithDynamicPrefix } from './collections/MediaWithDynamicPrefix.js'
910
import { MediaWithPrefix } from './collections/MediaWithPrefix.js'
1011
import { MediaWithSignedDownloads } from './collections/MediaWithSignedDownloads.js'
1112
import { Users } from './collections/Users.js'
12-
import { mediaSlug, mediaWithPrefixSlug, mediaWithSignedDownloadsSlug, prefix } from './shared.js'
13+
import {
14+
mediaSlug,
15+
mediaWithDynamicPrefixSlug,
16+
mediaWithPrefixSlug,
17+
mediaWithSignedDownloadsSlug,
18+
prefix,
19+
} from './shared.js'
1320
const filename = fileURLToPath(import.meta.url)
1421
const dirname = path.dirname(filename)
1522

@@ -26,7 +33,7 @@ export default buildConfigWithDefaults({
2633
baseDir: path.resolve(dirname),
2734
},
2835
},
29-
collections: [Media, MediaWithPrefix, MediaWithSignedDownloads, Users],
36+
collections: [Media, MediaWithDynamicPrefix, MediaWithPrefix, MediaWithSignedDownloads, Users],
3037
onInit: async (payload) => {
3138
await payload.create({
3239
collection: 'users',
@@ -40,6 +47,7 @@ export default buildConfigWithDefaults({
4047
s3Storage({
4148
collections: {
4249
[mediaSlug]: true,
50+
[mediaWithDynamicPrefixSlug]: true,
4351
[mediaWithPrefixSlug]: {
4452
prefix,
4553
},

test/storage-s3/int.spec.ts

Lines changed: 128 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Payload } from 'payload'
1+
import type { CollectionSlug, Payload } from 'payload'
22

33
import * as AWS from '@aws-sdk/client-s3'
44
import path from 'path'
@@ -7,7 +7,13 @@ import { fileURLToPath } from 'url'
77
import type { NextRESTClient } from '../helpers/NextRESTClient.js'
88

99
import { initPayloadInt } from '../helpers/initPayloadInt.js'
10-
import { mediaSlug, mediaWithPrefixSlug, mediaWithSignedDownloadsSlug, prefix } from './shared.js'
10+
import {
11+
mediaSlug,
12+
mediaWithDynamicPrefixSlug,
13+
mediaWithPrefixSlug,
14+
mediaWithSignedDownloadsSlug,
15+
prefix,
16+
} from './shared.js'
1117

1218
const filename = fileURLToPath(import.meta.url)
1319
const dirname = path.dirname(filename)
@@ -22,15 +28,15 @@ describe('@payloadcms/storage-s3', () => {
2228

2329
beforeAll(async () => {
2430
;({ payload, restClient } = await initPayloadInt(dirname))
25-
TEST_BUCKET = process.env.S3_BUCKET
31+
TEST_BUCKET = process.env.S3_BUCKET!
2632

2733
client = new AWS.S3({
28-
endpoint: process.env.S3_ENDPOINT,
34+
endpoint: process.env.S3_ENDPOINT!,
2935
forcePathStyle: process.env.S3_FORCE_PATH_STYLE === 'true',
30-
region: process.env.S3_REGION,
36+
region: process.env.S3_REGION!,
3137
credentials: {
32-
accessKeyId: process.env.S3_ACCESS_KEY_ID,
33-
secretAccessKey: process.env.S3_SECRET_ACCESS_KEY,
38+
accessKeyId: process.env.S3_ACCESS_KEY_ID!,
39+
secretAccessKey: process.env.S3_SECRET_ACCESS_KEY!,
3440
},
3541
})
3642

@@ -119,6 +125,117 @@ describe('@payloadcms/storage-s3', () => {
119125
it.todo('can upload')
120126
})
121127

128+
describe('prefix collision detection', () => {
129+
beforeEach(async () => {
130+
// Clear S3 bucket before each test
131+
await clearTestBucket()
132+
// Clear database records before each test
133+
await payload.delete({
134+
collection: mediaWithPrefixSlug,
135+
where: {},
136+
})
137+
await payload.delete({
138+
collection: mediaSlug,
139+
where: {},
140+
})
141+
})
142+
143+
it('detects collision within same prefix', async () => {
144+
const imageFile = path.resolve(dirname, '../uploads/image.png')
145+
146+
// Upload twice with same prefix
147+
const upload1 = await payload.create({
148+
collection: mediaWithPrefixSlug,
149+
data: {},
150+
filePath: imageFile,
151+
})
152+
153+
const upload2 = await payload.create({
154+
collection: mediaWithPrefixSlug,
155+
data: {},
156+
filePath: imageFile,
157+
})
158+
159+
expect(upload1.filename).toBe('image.png')
160+
expect(upload2.filename).toBe('image-1.png')
161+
expect(upload1.prefix).toBe(prefix)
162+
expect(upload2.prefix).toBe(prefix)
163+
})
164+
165+
it('works normally for collections without prefix', async () => {
166+
const imageFile = path.resolve(dirname, '../uploads/image.png')
167+
168+
// Upload twice to collection without prefix
169+
const upload1 = await payload.create({
170+
collection: mediaSlug,
171+
data: {},
172+
filePath: imageFile,
173+
})
174+
175+
const upload2 = await payload.create({
176+
collection: mediaSlug,
177+
data: {},
178+
filePath: imageFile,
179+
})
180+
181+
expect(upload1.filename).toBe('image.png')
182+
expect(upload2.filename).toBe('image-1.png')
183+
// @ts-expect-error prefix should never be set
184+
expect(upload1.prefix).toBeUndefined()
185+
// @ts-expect-error prefix should never be set
186+
expect(upload2.prefix).toBeUndefined()
187+
})
188+
189+
it('allows same filename under different prefixes', async () => {
190+
const imageFile = path.resolve(dirname, '../uploads/image.png')
191+
192+
// Upload with default prefix from config ('test-prefix')
193+
const upload1 = await payload.create({
194+
collection: mediaWithPrefixSlug,
195+
data: {},
196+
filePath: imageFile,
197+
})
198+
199+
// Upload with different prefix
200+
const upload2 = await payload.create({
201+
collection: mediaWithPrefixSlug,
202+
data: {
203+
prefix: 'different-prefix',
204+
},
205+
filePath: imageFile,
206+
})
207+
208+
expect(upload1.filename).toBe('image.png')
209+
expect(upload2.filename).toBe('image.png') // Should NOT increment
210+
expect(upload1.prefix).toBe(prefix) // 'test-prefix'
211+
expect(upload2.prefix).toBe('different-prefix')
212+
})
213+
214+
it('supports multi-tenant scenario with dynamic prefix from hook', async () => {
215+
const imageFile = path.resolve(dirname, '../uploads/image.png')
216+
217+
// Tenant A uploads logo.png
218+
const tenantAUpload = await payload.create({
219+
collection: mediaWithDynamicPrefixSlug,
220+
data: { tenant: 'a' },
221+
filePath: imageFile,
222+
})
223+
224+
// Tenant B uploads logo.png
225+
const tenantBUpload = await payload.create({
226+
collection: mediaWithDynamicPrefixSlug,
227+
data: { tenant: 'b' },
228+
filePath: imageFile,
229+
})
230+
231+
// Both should keep original filename
232+
expect(tenantAUpload.filename).toBe('image.png')
233+
expect(tenantBUpload.filename).toBe('image.png')
234+
expect(tenantAUpload.prefix).toBe('tenant-a')
235+
expect(tenantBUpload.prefix).toBe('tenant-b')
236+
})
237+
})
238+
122239
async function createTestBucket() {
123240
try {
124241
const makeBucketRes = await client.send(new AWS.CreateBucketCommand({ Bucket: TEST_BUCKET }))
@@ -144,15 +261,11 @@ describe('@payloadcms/storage-s3', () => {
144261
return
145262
}
146263

147-
const deleteParams = {
264+
const deleteParams: AWS.DeleteObjectsCommandInput = {
148265
Bucket: TEST_BUCKET,
149-
Delete: { Objects: [] },
266+
Delete: { Objects: listedObjects.Contents.map(({ Key }) => ({ Key })) },
150267
}
151268

152-
listedObjects.Contents.forEach(({ Key }) => {
153-
deleteParams.Delete.Objects.push({ Key })
154-
})
155-
156269
const deleteResult = await client.send(new AWS.DeleteObjectsCommand(deleteParams))
157270
if (deleteResult.Errors?.length) {
158271
throw new Error(JSON.stringify(deleteResult.Errors))
@@ -169,12 +282,12 @@ describe('@payloadcms/storage-s3', () => {
169282
uploadId: number | string
170283
}) {
171284
const uploadData = (await payload.findByID({
172-
collection: collectionSlug,
285+
collection: collectionSlug as CollectionSlug,
173286
id: uploadId,
174287
})) as unknown as { filename: string; sizes: Record<string, { filename: string }> }
175288

176289
const fileKeys = Object.keys(uploadData.sizes || {}).map((key) => {
177-
const rawFilename = uploadData.sizes[key].filename
290+
const rawFilename = uploadData?.sizes?.[key]?.filename
178291
return prefix ? `${prefix}/${rawFilename}` : rawFilename
179292
})
180293

0 commit comments

Comments
 (0)