Skip to content

Commit 6690769

Browse files
refactor: simplify storage adapter handler pattern (#16231)
## Summary - Replace factory function pattern (`getHandleUpload`, `getHandleDelete`, etc.) with simple helper functions - Create `adapter.ts` in each storage package that maps the plugin interface to the helpers - Fix R2 prefix bug: use OR logic (`filePrefix || prefix`) to match `handleUpload` behavior - Remove redundant `async/await` on pass-through handlers I think this approach makes it easier to trace backwards to the cloud-storage plugin the arguments that are actually passed into the adapter methods. ## Changes **Pattern change (all 6 storage adapters):** Before (factory functions): ```typescript export const getHandleDelete = ({ bucket }: Args): HandleDelete => { return async ({ doc: { prefix = '' }, filename }) => { await bucket.delete(path.posix.join(prefix, filename)) } } ``` After (simple functions + adapter mapping): ```typescript // handleDelete.ts export async function deleteFile({ bucket, prefix, filename }: DeleteArgs): Promise<void> { await bucket.delete(path.posix.join(prefix, filename)) } // adapter.ts export function createR2Adapter({ bucket, clientUploads }: CreateR2AdapterArgs): Adapter { return ({ collection, prefix = '' }): GeneratedAdapter => ({ name: 'r2', clientUploads, handleDelete: ({ doc: { prefix: docPrefix = '' }, filename }) => deleteFile({ bucket, filename, prefix: docPrefix }), handleUpload: ({ data, file }) => uploadFile({ bucket, buffer: file.buffer, filename: file.filename, mimeType: file.mimeType, prefix: data.prefix || prefix, }), staticHandler: ( req, { headers, params: { clientUploadContext, filename, prefix: prefixQueryParam } }, ) => getFile({ bucket, clientUploadContext, collection, filename, incomingHeaders: headers, prefix, prefixQueryParam, req, }), }) } ``` **Bug fix (storage-r2):** The prefix query param feature (#15844) introduced inconsistent logic in R2: - `handleUpload`: uses `data.prefix || prefix` (OR logic) - `staticHandler`: was using `prefix + filePrefix` (AND logic) Fixed to use consistent OR logic: `filePrefix || prefix` ## Packages affected - `@payloadcms/storage-r2` - `@payloadcms/storage-s3` - `@payloadcms/storage-gcs` - `@payloadcms/storage-azure` - `@payloadcms/storage-vercel-blob` - `@payloadcms/storage-uploadthing`
1 parent aa44649 commit 6690769

52 files changed

Lines changed: 1858 additions & 1579 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import type { ContainerClient } from '@azure/storage-blob'
2+
import type {
3+
Adapter,
4+
ClientUploadsConfig,
5+
GeneratedAdapter,
6+
} from '@payloadcms/plugin-cloud-storage/types'
7+
8+
import { deleteFile } from './deleteFile.js'
9+
import { generateURL } from './generateURL.js'
10+
import { getFile } from './getFile.js'
11+
import { uploadFile } from './uploadFile.js'
12+
13+
interface CreateAzureAdapterArgs {
14+
allowContainerCreate: boolean
15+
baseURL: string
16+
clientUploads?: ClientUploadsConfig
17+
containerName: string
18+
createContainerIfNotExists: () => void
19+
getStorageClient: () => ContainerClient
20+
}
21+
22+
export function createAzureAdapter({
23+
allowContainerCreate,
24+
baseURL,
25+
clientUploads,
26+
containerName,
27+
createContainerIfNotExists,
28+
getStorageClient,
29+
}: CreateAzureAdapterArgs): Adapter {
30+
return ({ collection, prefix = '' }): GeneratedAdapter => ({
31+
name: 'azure',
32+
clientUploads,
33+
34+
generateURL: ({ filename, prefix: urlPrefix = '' }) =>
35+
generateURL({ baseURL, containerName, filename, prefix: urlPrefix }),
36+
37+
handleDelete: ({ doc: { prefix: docPrefix = '' }, filename }) =>
38+
deleteFile({ client: getStorageClient(), filename, prefix: docPrefix }),
39+
40+
handleUpload: async ({ data, file }) => {
41+
await uploadFile({
42+
buffer: file.buffer,
43+
client: getStorageClient(),
44+
filename: file.filename,
45+
mimeType: file.mimeType,
46+
prefix: data.prefix || prefix,
47+
tempFilePath: file.tempFilePath,
48+
})
49+
50+
return data
51+
},
52+
53+
staticHandler: (
54+
req,
55+
{ headers, params: { clientUploadContext, filename, prefix: prefixQueryParam } },
56+
) =>
57+
getFile({
58+
client: getStorageClient(),
59+
clientUploadContext,
60+
collection,
61+
filename,
62+
incomingHeaders: headers,
63+
prefixQueryParam,
64+
req,
65+
}),
66+
67+
...(allowContainerCreate && { onInit: createContainerIfNotExists }),
68+
})
69+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import type { ContainerClient } from '@azure/storage-blob'
2+
3+
import path from 'path'
4+
5+
interface DeleteArgs {
6+
client: ContainerClient
7+
filename: string
8+
prefix: string
9+
}
10+
11+
export async function deleteFile({ client, filename, prefix }: DeleteArgs): Promise<void> {
12+
const blockBlobClient = client.getBlockBlobClient(path.posix.join(prefix, filename))
13+
await blockBlobClient.deleteIfExists()
14+
}
Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
1-
import type { GenerateURL } from '@payloadcms/plugin-cloud-storage/types'
2-
31
import path from 'path'
42

5-
interface Args {
3+
interface GenerateURLArgs {
64
baseURL: string
75
containerName: string
6+
filename: string
7+
prefix: string
88
}
99

10-
export const getGenerateURL =
11-
({ baseURL, containerName }: Args): GenerateURL =>
12-
({ filename, prefix = '' }) => {
13-
return `${baseURL}/${containerName}/${path.posix.join(prefix, filename)}`
14-
}
10+
export function generateURL({ baseURL, containerName, filename, prefix }: GenerateURLArgs): string {
11+
return `${baseURL}/${containerName}/${path.posix.join(prefix, filename)}`
12+
}
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
import type { BlobDownloadResponseParsed, ContainerClient } from '@azure/storage-blob'
2+
import type { CollectionConfig, PayloadRequest } from 'payload'
3+
import type { Readable } from 'stream'
4+
5+
import { RestError } from '@azure/storage-blob'
6+
import { getFilePrefix } from '@payloadcms/plugin-cloud-storage/utilities'
7+
import path from 'path'
8+
import { getRangeRequestInfo } from 'payload/internal'
9+
import { sanitizeFilename } from 'payload/shared'
10+
11+
interface GetFileArgs {
12+
client: ContainerClient
13+
clientUploadContext?: unknown
14+
collection: CollectionConfig
15+
filename: string
16+
incomingHeaders?: Headers
17+
prefixQueryParam?: string
18+
req: PayloadRequest
19+
}
20+
21+
const isNodeReadableStream = (
22+
body: BlobDownloadResponseParsed['readableStreamBody'],
23+
): body is Readable => {
24+
return (
25+
typeof body === 'object' &&
26+
body !== null &&
27+
'pipe' in body &&
28+
typeof body.pipe === 'function' &&
29+
'destroy' in body &&
30+
typeof body.destroy === 'function'
31+
)
32+
}
33+
34+
const abortRequestAndDestroyStream = ({
35+
abortController,
36+
blob,
37+
}: {
38+
abortController: AbortController
39+
blob?: BlobDownloadResponseParsed
40+
}) => {
41+
try {
42+
abortController.abort()
43+
} catch {
44+
/* noop */
45+
}
46+
if (blob?.readableStreamBody && isNodeReadableStream(blob.readableStreamBody)) {
47+
blob.readableStreamBody.destroy()
48+
}
49+
}
50+
51+
export async function getFile({
52+
client,
53+
clientUploadContext,
54+
collection,
55+
filename,
56+
incomingHeaders,
57+
prefixQueryParam,
58+
req,
59+
}: GetFileArgs): Promise<Response> {
60+
let blob: BlobDownloadResponseParsed | undefined = undefined
61+
let streamed = false
62+
63+
const abortController = new AbortController()
64+
if (req.signal) {
65+
req.signal.addEventListener('abort', () => {
66+
abortRequestAndDestroyStream({ abortController, blob })
67+
})
68+
}
69+
70+
try {
71+
const prefix = await getFilePrefix({
72+
clientUploadContext,
73+
collection,
74+
filename,
75+
prefixQueryParam,
76+
req,
77+
})
78+
const blockBlobClient = client.getBlockBlobClient(
79+
path.posix.join(prefix, sanitizeFilename(filename)),
80+
)
81+
82+
// Get file size for range validation
83+
const properties = await blockBlobClient.getProperties()
84+
const fileSize = properties.contentLength
85+
86+
if (!fileSize) {
87+
return new Response('Internal Server Error', { status: 500 })
88+
}
89+
90+
// Handle range request
91+
const rangeHeader = req.headers.get('range')
92+
const rangeResult = getRangeRequestInfo({ fileSize, rangeHeader })
93+
94+
if (rangeResult.type === 'invalid') {
95+
return new Response(null, {
96+
headers: new Headers(rangeResult.headers),
97+
status: rangeResult.status,
98+
})
99+
}
100+
101+
// Download with range if partial
102+
blob =
103+
rangeResult.type === 'partial'
104+
? await blockBlobClient.download(
105+
rangeResult.rangeStart,
106+
rangeResult.rangeEnd - rangeResult.rangeStart + 1,
107+
{ abortSignal: abortController.signal },
108+
)
109+
: await blockBlobClient.download(0, undefined, { abortSignal: abortController.signal })
110+
111+
let headers = new Headers(incomingHeaders)
112+
113+
// Add range-related headers from the result
114+
for (const [key, value] of Object.entries(rangeResult.headers)) {
115+
headers.append(key, value)
116+
}
117+
118+
// Add Azure-specific headers
119+
headers.append('Content-Type', String(properties.contentType))
120+
if (properties.etag) {
121+
headers.append('ETag', String(properties.etag))
122+
}
123+
124+
// Add Content-Security-Policy header for SVG files to prevent executable code
125+
if (properties.contentType === 'image/svg+xml') {
126+
headers.append('Content-Security-Policy', "script-src 'none'")
127+
}
128+
129+
const etagFromHeaders = req.headers.get('etag') || req.headers.get('if-none-match')
130+
131+
if (
132+
collection.upload &&
133+
typeof collection.upload === 'object' &&
134+
typeof collection.upload.modifyResponseHeaders === 'function'
135+
) {
136+
headers = collection.upload.modifyResponseHeaders({ headers }) || headers
137+
}
138+
139+
if (etagFromHeaders && etagFromHeaders === properties.etag) {
140+
return new Response(null, {
141+
headers,
142+
status: 304,
143+
})
144+
}
145+
146+
if (!blob.readableStreamBody || !isNodeReadableStream(blob.readableStreamBody)) {
147+
return new Response('Internal Server Error', { status: 500 })
148+
}
149+
150+
const stream = blob.readableStreamBody
151+
stream.on('error', (err: Error) => {
152+
req.payload.logger.error({
153+
err,
154+
msg: 'Error while streaming Azure blob (aborting)',
155+
})
156+
abortRequestAndDestroyStream({ abortController, blob })
157+
})
158+
159+
streamed = true
160+
return new Response(stream, { headers, status: rangeResult.status })
161+
} catch (err: unknown) {
162+
if (err instanceof RestError && err.statusCode === 404) {
163+
return new Response(null, { status: 404, statusText: 'Not Found' })
164+
}
165+
req.payload.logger.error(err)
166+
return new Response('Internal Server Error', { status: 500 })
167+
} finally {
168+
if (!streamed) {
169+
abortRequestAndDestroyStream({ abortController, blob })
170+
}
171+
}
172+
}

packages/storage-azure/src/handleDelete.ts

Lines changed: 0 additions & 17 deletions
This file was deleted.

packages/storage-azure/src/handleUpload.ts

Lines changed: 0 additions & 43 deletions
This file was deleted.

0 commit comments

Comments
 (0)