fix(plugin-cloud-storage): preserve user-defined prefix.defaultValue (#16529)

Backports #16523

Author: AlessioGr

.github/workflows/e2e.config.ts

@@ -86,6 +86,7 @@ export default createE2EConfig([
   { file: 'locked-documents', shards: 1 },
   { file: 'i18n', shards: 1 },
   { file: 'plugin-cloud-storage', shards: 1 },
+  { file: 'storage-azure__client-uploads#client-uploads/config.ts', shards: 1 },
   { file: 'storage-s3__client-uploads#client-uploads/config.ts', shards: 1 },
   { file: 'storage-vercel-blob__client-uploads#client-uploads/config.ts', shards: 1 },
   { file: 'plugin-form-builder', shards: 1 },

packages/plugin-cloud-storage/src/admin/fields/getFields.ts

@@ -132,7 +132,9 @@ export const getFields = ({
     fields.push({
       ...basePrefixField,
       ...(existingPrefixField || {}),
-      defaultValue: useCompositePrefixes ? '' : prefix ? path.posix.join(prefix) : '',
+      defaultValue:
+        existingPrefixField?.defaultValue ??
+        (useCompositePrefixes ? '' : prefix ? path.posix.join(prefix) : ''),
     } as TextField)
   }
 

packages/plugin-cloud-storage/src/fields/getFields.ts

@@ -200,7 +200,9 @@ export const getFields = ({
     fields.push({
       ...basePrefixField,
       ...(existingPrefixField || {}),
-      defaultValue: useCompositePrefixes ? '' : prefix ? path.posix.join(prefix) : '',
+      defaultValue:
+        existingPrefixField?.defaultValue ??
+        (useCompositePrefixes ? '' : prefix ? path.posix.join(prefix) : ''),
     } as TextField)
   }
 

test/playwright.config.ts

@@ -14,7 +14,7 @@ const CI = process.env.CI === 'true'
 let multiplier = CI ? 4 : 1
 let smallMultiplier = CI ? 3 : 1
 
-export const TEST_TIMEOUT_LONG = 320000 * multiplier // 4*8 minutes - used as timeOut for the beforeAll
+export const TEST_TIMEOUT_LONG = 60000 * multiplier // used as timeOut for the beforeAll
 export const TEST_TIMEOUT = 20000 * smallMultiplier
 export const EXPECT_TIMEOUT = 6000 * smallMultiplier
 export const POLL_TOPASS_TIMEOUT = EXPECT_TIMEOUT * 4 // That way expect.poll() or expect().toPass can retry 4 times. 4x higher than default expect timeout => can retry 4 times if retryable expects are used inside

test/storage-azure/client-uploads/collections/MediaWithDocPrefix.ts

@@ -0,0 +1,17 @@
+import type { CollectionConfig } from 'payload'
+
+export const mediaWithDocPrefixSlug = 'media-with-doc-prefix'
+
+export const MediaWithDocPrefix: CollectionConfig = {
+  slug: mediaWithDocPrefixSlug,
+  upload: {
+    filenameCompoundIndex: ['prefix', 'filename'],
+  },
+  fields: [
+    {
+      name: 'prefix',
+      type: 'text',
+      defaultValue: () => `doc-${Math.random().toString(36).slice(2, 10)}`,
+    },
+  ],
+}

test/storage-azure/client-uploads/config.ts

@@ -0,0 +1,60 @@
+import { azureStorage } from '@payloadcms/storage-azure'
+import dotenv from 'dotenv'
+import { fileURLToPath } from 'node:url'
+import path from 'path'
+
+import { buildConfigWithDefaults } from '../../buildConfigWithDefaults.js'
+import { devUser } from '../../credentials.js'
+import { Media } from '../collections/Media.js'
+import { MediaWithPrefix } from '../collections/MediaWithPrefix.js'
+import { Users } from '../collections/Users.js'
+import { mediaSlug, mediaWithPrefixSlug, prefix } from '../shared.js'
+import { MediaWithDocPrefix, mediaWithDocPrefixSlug } from './collections/MediaWithDocPrefix.js'
+
+const filename = fileURLToPath(import.meta.url)
+const dirname = path.dirname(filename)
+
+dotenv.config({
+  path: path.resolve(dirname, '../../plugin-cloud-storage/.env.emulated'),
+})
+
+export default buildConfigWithDefaults({
+  admin: {
+    importMap: {
+      baseDir: path.resolve(dirname, '..'),
+    },
+  },
+  collections: [Media, MediaWithPrefix, MediaWithDocPrefix, Users],
+  onInit: async (payload) => {
+    await payload.create({
+      collection: 'users',
+      data: {
+        email: devUser.email,
+        password: devUser.password,
+      },
+    })
+  },
+  plugins: [
+    azureStorage({
+      collections: {
+        [mediaSlug]: true,
+        [mediaWithPrefixSlug]: {
+          prefix,
+        },
+        // Configure a collection-level prefix on this slug to test that
+        // a custom `prefix.defaultValue` does override the static prefix
+        [mediaWithDocPrefixSlug]: {
+          prefix: 'docprefix-collection',
+        },
+      },
+      allowContainerCreate: process.env.AZURE_STORAGE_ALLOW_CONTAINER_CREATE === 'true',
+      baseURL: process.env.AZURE_STORAGE_ACCOUNT_BASEURL!,
+      clientUploads: true,
+      connectionString: process.env.AZURE_STORAGE_CONNECTION_STRING!,
+      containerName: process.env.AZURE_STORAGE_CONTAINER_NAME!,
+    }),
+  ],
+  typescript: {
+    outputFile: path.resolve(dirname, 'payload-types.ts'),
+  },
+})

test/storage-azure/client-uploads/e2e.spec.ts

@@ -0,0 +1,96 @@
+import type { ContainerClient } from '@azure/storage-blob'
+import type { Page } from '@playwright/test'
+
+import { BlobServiceClient } from '@azure/storage-blob'
+import { expect, test } from '@playwright/test'
+import dotenv from 'dotenv'
+import * as path from 'path'
+import { fileURLToPath } from 'url'
+
+import type { PayloadTestSDK } from '../../__helpers/shared/sdk/index.js'
+
+import { ensureCompilationIsDone, saveDocAndAssert } from '../../__helpers/e2e/helpers.js'
+import { AdminUrlUtil } from '../../__helpers/shared/adminUrlUtil.js'
+import { initPayloadE2ENoConfig } from '../../__helpers/shared/initPayloadE2ENoConfig.js'
+import { TEST_TIMEOUT_LONG } from '../../playwright.config.js'
+import { mediaWithDocPrefixSlug } from './collections/MediaWithDocPrefix.js'
+
+const filename = fileURLToPath(import.meta.url)
+const dirname = path.dirname(filename)
+
+dotenv.config({ path: path.resolve(dirname, '../../plugin-cloud-storage/.env.emulated') })
+
+test.describe('storage-azure client uploads E2E', () => {
+  let page: Page
+  let mediaWithDocPrefixURL: AdminUrlUtil
+  let payloadSDK: PayloadTestSDK<{ collections: Record<string, unknown> }>
+  let containerClient: ContainerClient
+
+  test.beforeAll(async ({ browser }, testInfo) => {
+    testInfo.setTimeout(TEST_TIMEOUT_LONG)
+    const { payload, serverURL } = await initPayloadE2ENoConfig({ dirname })
+
+    payloadSDK = payload as unknown as typeof payloadSDK
+    mediaWithDocPrefixURL = new AdminUrlUtil(serverURL, mediaWithDocPrefixSlug)
+
+    const blobService = BlobServiceClient.fromConnectionString(
+      process.env.AZURE_STORAGE_CONNECTION_STRING!,
+    )
+
+    await blobService.setProperties({
+      cors: [
+        {
+          allowedHeaders: '*',
+          allowedMethods: 'GET,PUT,POST,DELETE,OPTIONS,HEAD',
+          allowedOrigins: '*',
+          exposedHeaders: '*',
+          maxAgeInSeconds: 3600,
+        },
+      ],
+    })
+
+    containerClient = blobService.getContainerClient(process.env.AZURE_STORAGE_CONTAINER_NAME!)
+    await containerClient.createIfNotExists()
+
+    // Clear leftover blobs from previous runs so the listBlobsFlat assertion
+    // below only sees uploads from this test.
+    for await (const blob of containerClient.listBlobsFlat()) {
+      await containerClient.deleteBlob(blob.name)
+    }
+
+    const context = await browser.newContext()
+    page = await context.newPage()
+    await ensureCompilationIsDone({ page, serverURL })
+  })
+
+  /**
+   * Drives the actual admin form through a clientUploads submit and asserts
+   * that the user-defined `prefix.defaultValue` ends up on the saved doc — i.e.
+   * the plugin no longer clobbers the user's callback (see getFields.ts).
+   */
+  test('respects user-defined prefix.defaultValue when creating a doc via clientUploads', async () => {
+    await page.goto(mediaWithDocPrefixURL.create)
+    await page.setInputFiles('input[type="file"]', path.resolve(dirname, '../../uploads/image.png'))
+    await saveDocAndAssert(page)
+
+    const docId = page.url().split('/').pop()!
+
+    const result = await payloadSDK.find({
+      collection: mediaWithDocPrefixSlug,
+      where: { id: { equals: docId } },
+    })
+
+    const doc = result.docs[0]
+
+    const blobNames: string[] = []
+    for await (const blob of containerClient.listBlobsFlat()) {
+      blobNames.push(blob.name)
+    }
+
+    expect(blobNames).toEqual(
+      expect.arrayContaining([expect.stringMatching(/^doc-[a-z0-9]{1,8}\/image\.png$/)]),
+    )
+
+    expect(doc?.prefix).toMatch(/^doc-[a-z0-9]{1,8}$/)
+  })
+})

test/storage-azure/client-uploads/int.spec.ts

@@ -0,0 +1,103 @@
+import type { ContainerClient } from '@azure/storage-blob'
+import type { Payload } from 'payload'
+
+import { BlobServiceClient } from '@azure/storage-blob'
+import { readFile } from 'node:fs/promises'
+import path from 'path'
+import { fileURLToPath } from 'url'
+import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest'
+
+import type { NextRESTClient } from '../../__helpers/shared/NextRESTClient.js'
+
+import { initPayloadInt } from '../../__helpers/shared/initPayloadInt.js'
+import { mediaSlug } from '../shared.js'
+import { mediaWithDocPrefixSlug } from './collections/MediaWithDocPrefix.js'
+
+const filename = fileURLToPath(import.meta.url)
+const dirname = path.dirname(filename)
+
+let payload: Payload
+let restClient: NextRESTClient
+let containerClient: ContainerClient
+let TEST_CONTAINER: string
+
+describe('@payloadcms/storage-azure clientUploads', () => {
+  const clearContainer = async () => {
+    for await (const blob of containerClient.listBlobsFlat()) {
+      await containerClient.deleteBlob(blob.name)
+    }
+  }
+
+  beforeAll(async () => {
+    ;({ payload, restClient } = await initPayloadInt(dirname))
+
+    TEST_CONTAINER = process.env.AZURE_STORAGE_CONTAINER_NAME!
+    containerClient = BlobServiceClient.fromConnectionString(
+      process.env.AZURE_STORAGE_CONNECTION_STRING!,
+    ).getContainerClient(TEST_CONTAINER)
+    await containerClient.createIfNotExists()
+    await clearContainer()
+  }, 90000)
+
+  afterAll(async () => {
+    await payload.destroy()
+  })
+
+  afterEach(async () => {
+    await clearContainer()
+  })
+
+  /**
+   * When a doc with the same filename already exists, the signed-URL endpoint
+   * should sanitize the filename (e.g. `duplicate-target-1.png`) so the
+   * browser PUT lands on a fresh blob instead of overwriting the existing one.
+   */
+  it('sanitizes the filename when a duplicate already exists', async () => {
+    const dupFilename = 'duplicate-target.png'
+    const fileBuffer = await readFile(`${dirname}/../../uploads/image.png`)
+
+    const seedForm = new FormData()
+    seedForm.append('file', new Blob([fileBuffer], { type: 'image/png' }), dupFilename)
+    const seedRes = await restClient.POST(`/${mediaSlug}`, { body: seedForm })
+
+    expect(seedRes.status).toBe(201)
+    const { doc: seedDoc }: { doc: { filename: string; id: number | string } } =
+      await seedRes.json()
+
+    expect(seedDoc.filename).toBe(dupFilename)
+
+    const signedURLRes = await restClient.POST('/storage-azure-generate-signed-url', {
+      body: JSON.stringify({
+        collectionSlug: mediaSlug,
+        filename: dupFilename,
+        mimeType: 'image/png',
+      }),
+    })
+
+    expect(signedURLRes.status).toBe(200)
+    const { url: signedURL }: { url: string } = await signedURLRes.json()
+
+    const blobKey = decodeURIComponent(
+      new URL(signedURL).pathname.replace(`/devstoreaccount1/${TEST_CONTAINER}/`, ''),
+    )
+
+    expect(blobKey).toBe('duplicate-target-1.png')
+
+    await payload.delete({ collection: mediaSlug, id: seedDoc.id })
+  })
+
+  it('preserves a user-defined prefix.defaultValue across the plugin', async () => {
+    const upload = await payload.create({
+      collection: mediaWithDocPrefixSlug,
+      data: {},
+      filePath: path.resolve(dirname, '../../uploads/image.png'),
+    })
+
+    expect(upload.prefix).toMatch(/^doc-[a-z0-9]{1,8}$/)
+
+    const props = await containerClient
+      .getBlobClient(`${upload.prefix}/${upload.filename}`)
+      .getProperties()
+    expect(props.contentLength).toBeGreaterThan(0)
+  })
+})