fix(plugin-cloud-storage): fixes an issue with generateFileURL not being respected (#15565)

Fixes #15554

When `generateFileURL` is defined at the collection level, the
beforeChange hook was not using the collection level `generateFileURL`
but instead would fallback to `adapter.generateURL`. The user did not
have an endpoint set on their cloud storage plugin, so this resulted in
the url being generated and stored with a prefix of `undefined/`.

Author: JarrodMFlesch

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

@@ -75,7 +75,12 @@ export const getFields = ({
           ...(existingURLField?.hooks?.afterRead || []),
         ],
         beforeChange: [
-          getBeforeChangeHook({ adapter, collection, disablePayloadAccessControl }),
+          getBeforeChangeHook({
+            adapter,
+            collection,
+            disablePayloadAccessControl,
+            generateFileURL,
+          }),
           ...(existingURLField?.hooks?.beforeChange || []),
         ],
       },
@@ -143,6 +148,7 @@ export const getFields = ({
                     adapter,
                     collection,
                     disablePayloadAccessControl,
+                    generateFileURL,
                     size,
                   }),
                   ...((typeof existingSizeURLField === 'object' &&

packages/plugin-cloud-storage/src/hooks/afterRead.ts

@@ -18,21 +18,21 @@ export const getAfterReadHook =
     let url = value
 
     if (disablePayloadAccessControl && filename) {
-      url = await adapter.generateURL?.({
-        collection,
-        data,
-        filename,
-        prefix,
-      })
-    }
-
-    if (generateFileURL) {
-      url = await generateFileURL({
-        collection,
-        filename,
-        prefix,
-        size,
-      })
+      if (generateFileURL) {
+        url = await generateFileURL({
+          collection,
+          filename,
+          prefix,
+          size,
+        })
+      } else if (adapter.generateURL) {
+        url = await adapter.generateURL({
+          collection,
+          data,
+          filename,
+          prefix,
+        })
+      }
     }
 
     return url

packages/plugin-cloud-storage/src/hooks/beforeChange.ts

@@ -1,41 +1,47 @@
 import type { CollectionConfig, FieldHook, ImageSize } from 'payload'
 
-import type { GeneratedAdapter } from '../types.js'
+import type { GeneratedAdapter, GenerateFileURL } from '../types.js'
 
 interface Args {
   adapter: GeneratedAdapter
   collection: CollectionConfig
   disablePayloadAccessControl?: boolean
+  generateFileURL?: GenerateFileURL
   size?: ImageSize
 }
 
 export const getBeforeChangeHook =
-  ({ adapter, collection, disablePayloadAccessControl, size }: Args): FieldHook =>
+  ({ adapter, collection, disablePayloadAccessControl, generateFileURL, size }: Args): FieldHook =>
   async ({ data, originalDoc, value }) => {
-    // Only handle the disablePayloadAccessControl: true case here
-    // When false, let the core beforeChange hook from getBaseFields handle it
     if (!disablePayloadAccessControl) {
       return value
+    } else {
+      const newFilename = size ? data?.sizes?.[size.name]?.filename : data?.filename
+      const originalFilename = size
+        ? originalDoc?.sizes?.[size.name]?.filename
+        : originalDoc?.filename
+      const filename = newFilename || originalFilename
+      const prefix = data?.prefix
+      let url = value
+
+      // Store the full URL in the database so files can be accessed directly
+      // from the storage provider without going through Payload's API
+      if (generateFileURL) {
+        url = await generateFileURL({
+          collection,
+          filename,
+          prefix,
+          size,
+        })
+      } else if (adapter.generateURL) {
+        url = await adapter.generateURL({
+          collection,
+          data: data || originalDoc,
+          filename,
+          prefix,
+        })
+      }
+
+      return url
     }
-
-    const filename = size ? data?.sizes?.[size.name]?.filename : data?.filename
-
-    if (!filename) {
-      return value
-    }
-
-    const prefix = data?.prefix
-
-    // Store the full URL in the database so files can be accessed directly
-    // from the storage provider without going through Payload's API
-    if (adapter.generateURL) {
-      return await adapter.generateURL({
-        collection,
-        data: data || originalDoc,
-        filename,
-        prefix,
-      })
-    }
-
-    return value
   }

test/plugin-cloud-storage/collections/MediaWithCustomURL.ts

@@ -0,0 +1,9 @@
+import type { CollectionConfig } from 'payload'
+
+export const MediaWithCustomURL: CollectionConfig = {
+  slug: 'media-with-custom-url',
+  upload: {
+    disableLocalStorage: true,
+  },
+  fields: [],
+}

test/plugin-cloud-storage/config.ts

@@ -1,3 +1,4 @@
+import type { S3StorageOptions } from '@payloadcms/storage-s3'
 import type { Plugin } from 'payload'
 
 import { cloudStoragePlugin } from '@payloadcms/plugin-cloud-storage'
@@ -11,12 +12,14 @@ import path from 'path'
 import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
 import { devUser } from '../credentials.js'
 import { Media } from './collections/Media.js'
+import { MediaWithCustomURL } from './collections/MediaWithCustomURL.js'
 import { MediaWithPrefix } from './collections/MediaWithPrefix.js'
 import { RestrictedMedia } from './collections/RestrictedMedia.js'
 import { TestMetadata } from './collections/TestMetadata.js'
 import { Users } from './collections/Users.js'
 import {
   mediaSlug,
+  mediaWithCustomURLSlug,
   mediaWithPrefixSlug,
   prefix,
   restrictedMediaSlug,
@@ -83,6 +86,14 @@ if (
       [mediaWithPrefixSlug]: {
         prefix,
       },
+      [mediaWithCustomURLSlug]: {
+        prefix,
+        disablePayloadAccessControl: true,
+        generateFileURL: ({ filename, prefix }) =>
+          filename
+            ? `https://test-cdn.example.com/${prefix}/${encodeURIComponent(filename)}`
+            : null,
+      } as S3StorageOptions['collections'][keyof S3StorageOptions['collections']],
       [restrictedMediaSlug]: true,
     },
     bucket: process.env.S3_BUCKET ?? '',
@@ -150,7 +161,7 @@ export default buildConfigWithDefaults({
       baseDir: path.resolve(dirname),
     },
   },
-  collections: [Media, MediaWithPrefix, RestrictedMedia, TestMetadata, Users],
+  collections: [Media, MediaWithCustomURL, MediaWithPrefix, RestrictedMedia, TestMetadata, Users],
   onInit: async (payload) => {
     /*const client = new AWS.S3({
       endpoint: process.env.S3_ENDPOINT,

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

@@ -12,6 +12,7 @@ import type { Config } from './payload-types.js'
 import { initPayloadInt } from '../__helpers/shared/initPayloadInt.js'
 import {
   mediaSlug,
+  mediaWithCustomURLSlug,
   mediaWithPrefixSlug,
   prefix,
   restrictedMediaSlug,
@@ -226,6 +227,46 @@ describe('@payloadcms/plugin-cloud-storage', () => {
         expect(dbRecord.url).toEqual(`/api/${mediaWithPrefixSlug}/file/${upload.filename}`)
         expect((rawDbData as any)?.sizes).toBeFalsy()
       })
+
+      it('should use custom generateFileURL in beforeChange when disablePayloadAccessControl is true', async () => {
+        // This test verifies that custom generateFileURL is used in beforeChange hook
+        // when disablePayloadAccessControl: true, preventing "undefined" from appearing in URLs
+        const upload = await payload.create({
+          collection: mediaWithCustomURLSlug,
+          data: {},
+          filePath: path.resolve(dirname, '../uploads/image.png'),
+        })
+
+        expect(upload.id).toBeTruthy()
+        expect(upload.filename).toBeTruthy()
+
+        // Get the raw DB data
+        const rawDbData = await payload.db.findOne({
+          collection: mediaWithCustomURLSlug,
+          where: { id: { equals: upload.id } },
+        })
+
+        const dbRecord = rawDbData as unknown as {
+          filename: string
+          prefix: string
+          url: string
+        }
+
+        // The custom generateFileURL should be used, resulting in test-cdn.example.com URL
+        expect(dbRecord.url).toContain('test-cdn.example.com')
+        expect(dbRecord.url).toContain(prefix)
+        expect(dbRecord.url).toContain(encodeURIComponent(dbRecord.filename))
+        expect(dbRecord.url).toMatch(/^https:\/\//)
+
+        // Verify the API response also returns the custom URL
+        const apiResponse = await payload.findByID({
+          collection: mediaWithCustomURLSlug,
+          id: upload.id,
+        })
+
+        expect(apiResponse.url).toContain('test-cdn.example.com')
+        expect(apiResponse.url).toContain(prefix)
+      })
     })
   })
 

test/plugin-cloud-storage/payload-types.ts

@@ -68,6 +68,7 @@ export interface Config {
   blocks: {};
   collections: {
     media: Media;
+    'media-with-custom-url': MediaWithCustomUrl;
     'media-with-prefix': MediaWithPrefix;
     'restricted-media': RestrictedMedia;
     'test-metadata': TestMetadatum;
@@ -80,6 +81,7 @@ export interface Config {
   collectionsJoins: {};
   collectionsSelect: {
     media: MediaSelect<false> | MediaSelect<true>;
+    'media-with-custom-url': MediaWithCustomUrlSelect<false> | MediaWithCustomUrlSelect<true>;
     'media-with-prefix': MediaWithPrefixSelect<false> | MediaWithPrefixSelect<true>;
     'restricted-media': RestrictedMediaSelect<false> | RestrictedMediaSelect<true>;
     'test-metadata': TestMetadataSelect<false> | TestMetadataSelect<true>;
@@ -157,6 +159,25 @@ export interface Media {
     };
   };
 }
+/**
+ * This interface was referenced by `Config`'s JSON-Schema
+ * via the `definition` "media-with-custom-url".
+ */
+export interface MediaWithCustomUrl {
+  id: string;
+  prefix?: string | null;
+  updatedAt: string;
+  createdAt: string;
+  url?: string | null;
+  thumbnailURL?: string | null;
+  filename?: string | null;
+  mimeType?: string | null;
+  filesize?: number | null;
+  width?: number | null;
+  height?: number | null;
+  focalX?: number | null;
+  focalY?: number | null;
+}
 /**
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "media-with-prefix".
@@ -280,6 +301,10 @@ export interface PayloadLockedDocument {
         relationTo: 'media';
         value: string | Media;
       } | null)
+    | ({
+        relationTo: 'media-with-custom-url';
+        value: string | MediaWithCustomUrl;
+      } | null)
     | ({
         relationTo: 'media-with-prefix';
         value: string | MediaWithPrefix;
@@ -380,6 +405,24 @@ export interface MediaSelect<T extends boolean = true> {
             };
       };
 }
+/**
+ * This interface was referenced by `Config`'s JSON-Schema
+ * via the `definition` "media-with-custom-url_select".
+ */
+export interface MediaWithCustomUrlSelect<T extends boolean = true> {
+  prefix?: T;
+  updatedAt?: T;
+  createdAt?: T;
+  url?: T;
+  thumbnailURL?: T;
+  filename?: T;
+  mimeType?: T;
+  filesize?: T;
+  width?: T;
+  height?: T;
+  focalX?: T;
+  focalY?: T;
+}
 /**
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "media-with-prefix_select".

test/plugin-cloud-storage/shared.ts

@@ -1,5 +1,6 @@
 export const mediaSlug = 'media'
 export const mediaWithPrefixSlug = 'media-with-prefix'
+export const mediaWithCustomURLSlug = 'media-with-custom-url'
 export const restrictedMediaSlug = 'restricted-media'
 export const testMetadataSlug = 'test-metadata'
 export const prefix = 'test-prefix'