From 687c280a767d189770b0d9a98ea75d666ce1d981 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Wed, 6 May 2026 19:56:25 +0000 Subject: [PATCH 1/2] feat: add deleteFile mutation with sync S3 delete + async job fallback - Add deleteObject to s3-utils (DeleteObjectCommand) - Add deleteS3Object to s3-signer (used by presigned-url plugin) - Add deleteFile GraphQL mutation to presigned-url plugin: - Resolves file across all storage modules (app + entity-scoped) - DELETE file row (RLS enforced) - Check refcount for content-hash dedup safety - Try sync S3 DeleteObject, fall back to async delete_s3_object job - Returns { success, deletedFromS3, key } --- .../src/plugin.ts | 171 +++++++++++++++++- .../src/s3-signer.ts | 25 +++ uploads/s3-utils/src/utils.ts | 11 ++ 3 files changed, 205 insertions(+), 2 deletions(-) diff --git a/graphile/graphile-presigned-url-plugin/src/plugin.ts b/graphile/graphile-presigned-url-plugin/src/plugin.ts index 030095498..5a49986a1 100644 --- a/graphile/graphile-presigned-url-plugin/src/plugin.ts +++ b/graphile/graphile-presigned-url-plugin/src/plugin.ts @@ -19,8 +19,8 @@ import { extendSchema, gql } from 'graphile-utils'; import { Logger } from '@pgpmjs/logger'; import type { PresignedUrlPluginOptions, S3Config, StorageModuleConfig, BucketConfig } from './types'; -import { getStorageModuleConfig, getStorageModuleConfigForOwner, getBucketConfig, isS3BucketProvisioned, markS3BucketProvisioned } from './storage-module-cache'; -import { generatePresignedPutUrl } from './s3-signer'; +import { getStorageModuleConfig, getStorageModuleConfigForOwner, getBucketConfig, resolveStorageModuleByFileId, isS3BucketProvisioned, markS3BucketProvisioned } from './storage-module-cache'; +import { generatePresignedPutUrl, deleteS3Object } from './s3-signer'; const log = new Logger('graphile-presigned-url:plugin'); @@ -264,6 +264,20 @@ export function createPresignedUrlPlugin( files: [BulkUploadFilePayload!]! } + input DeleteFileInput { + """File ID to delete""" + fileId: UUID! + } + + type DeleteFilePayload { + """Whether the file record was deleted from the database""" + success: Boolean! + """Whether the S3 object was deleted (false if other files reference the same key)""" + deletedFromS3: Boolean! + """The S3 key that was (or would have been) deleted""" + key: String + } + extend type Mutation { """ Request a presigned URL for uploading a file directly to S3. @@ -283,6 +297,17 @@ export function createPresignedUrlPlugin( requestBulkUploadUrls( input: RequestBulkUploadUrlsInput! ): RequestBulkUploadUrlsPayload + + """ + Delete a file record and its S3 object. + The DB record is always deleted (subject to RLS). The S3 object is + deleted only if no other file records reference the same key in the + same bucket (content-addressed dedup safety). If the inline S3 + delete fails, cleanup falls back to the async delete_s3_object job. + """ + deleteFile( + input: DeleteFileInput! + ): DeleteFilePayload } `, plans: { @@ -302,6 +327,21 @@ export function createPresignedUrlPlugin( return result; }); }, + deleteFile(_$mutation: any, fieldArgs: any) { + const $input = fieldArgs.getRaw('input'); + const $withPgClient = (grafastContext() as any).get('withPgClient'); + const $pgSettings = (grafastContext() as any).get('pgSettings'); + const $combined = object({ + input: $input, + withPgClient: $withPgClient, + pgSettings: $pgSettings, + }); + + return lambda($combined, async ({ input, withPgClient, pgSettings }: any) => { + const result = await processDelete(options, input, withPgClient, pgSettings); + return result; + }); + }, requestBulkUploadUrls(_$mutation: any, fieldArgs: any) { const $input = fieldArgs.getRaw('input'); const $withPgClient = (grafastContext() as any).get('withPgClient'); @@ -635,5 +675,132 @@ async function processSingleFile( }; } +// --- Delete logic --- + +/** + * Process a file deletion: remove the DB record, then attempt S3 cleanup. + * + * 1. Resolve the file row (key, bucket_id) and storage config + * 2. DELETE the file row (RLS enforced — only owner/admin can delete) + * 3. Check refcount: any other file with same key in the same bucket? + * 4. If orphaned: try S3 DeleteObject inline (sync) + * - On S3 failure: enqueue delete_s3_object job (async fallback) + * 5. Return result + */ +async function processDelete( + options: PresignedUrlPluginOptions, + input: any, + withPgClient: any, + pgSettings: any, +) { + const { fileId } = input; + + if (!fileId || typeof fileId !== 'string') { + throw new Error('INVALID_FILE_ID'); + } + + return withPgClient(pgSettings, async (pgClient: any) => { + return pgClient.withTransaction(async (txClient: any) => { + const databaseId = await resolveDatabaseId(txClient); + if (!databaseId) { + throw new Error('DATABASE_NOT_FOUND'); + } + + // 1. Resolve storage config + file across all storage modules (app-level + entity-scoped) + const resolved = await resolveStorageModuleByFileId(txClient, databaseId, fileId); + if (!resolved) { + throw new Error('FILE_NOT_FOUND: file does not exist or access denied'); + } + + const { storageConfig, file } = resolved; + const { key, bucket_id } = file; + + // 2. DELETE the file row (RLS enforced — will fail if user lacks permission) + const deleteResult = await txClient.query({ + text: `DELETE FROM ${storageConfig.filesQualifiedName} + WHERE id = $1 + RETURNING id`, + values: [fileId], + }); + + if (deleteResult.rows.length === 0) { + throw new Error('DELETE_DENIED: insufficient permissions to delete this file'); + } + + // 3. Check refcount: any other file with same key in this bucket? + const refcountResult = await txClient.query({ + text: `SELECT COUNT(*)::int AS ref_count + FROM ${storageConfig.filesQualifiedName} + WHERE key = $1 + AND bucket_id = $2`, + values: [key, bucket_id], + }); + + const refCount = refcountResult.rows[0]?.ref_count ?? 0; + + if (refCount > 0) { + // Other files reference this S3 key — do not delete from S3 + log.info(`File ${fileId} deleted from DB; S3 key ${key} still referenced by ${refCount} file(s)`); + return { + success: true, + deletedFromS3: false, + key, + }; + } + + // 4. Attempt sync S3 delete + try { + const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId); + await deleteS3Object(s3ForDb, key); + log.info(`File ${fileId} deleted from DB and S3 (key=${key})`); + return { + success: true, + deletedFromS3: true, + key, + }; + } catch (s3Error: any) { + // S3 delete failed — enqueue async job as fallback + log.warn(`S3 delete failed for key=${key}, falling back to async job: ${s3Error.message}`); + + try { + await txClient.query({ + text: `SELECT app_jobs.add_job( + $1::text, + $2::json, + job_key := $3::text, + queue_name := $4::text, + priority := $5::int, + run_at := NOW() + interval '5 seconds' + )`, + values: [ + 'delete_s3_object', + JSON.stringify({ + key, + bucket_id, + database_id: databaseId, + schema_name: storageConfig.schemaName, + table_name: storageConfig.filesTableName, + }), + `gc:${bucket_id}:${key}`, + 'storage_gc', + 100, + ], + }); + log.info(`Enqueued delete_s3_object job for key=${key}`); + } catch (jobError: any) { + // app_jobs might not be installed — log but don't fail the delete + log.warn(`Failed to enqueue delete_s3_object job: ${jobError.message}`); + } + + return { + success: true, + deletedFromS3: false, + key, + }; + } + }); + }); +} + export const PresignedUrlPlugin = createPresignedUrlPlugin; export default PresignedUrlPlugin; diff --git a/graphile/graphile-presigned-url-plugin/src/s3-signer.ts b/graphile/graphile-presigned-url-plugin/src/s3-signer.ts index ded687974..1a3828a22 100644 --- a/graphile/graphile-presigned-url-plugin/src/s3-signer.ts +++ b/graphile/graphile-presigned-url-plugin/src/s3-signer.ts @@ -1,5 +1,6 @@ import { S3Client, + DeleteObjectCommand, PutObjectCommand, GetObjectCommand, HeadObjectCommand, @@ -117,3 +118,27 @@ export async function headObject( throw e; } } + +/** + * Delete an object from S3. + * + * Returns true if the object was deleted (or didn't exist — S3 DeleteObject + * is idempotent). Throws on unexpected errors (permissions, network). + * + * @param s3Config - S3 client and bucket configuration + * @param key - S3 object key to delete + * @returns true if deletion succeeded + */ +export async function deleteS3Object( + s3Config: S3Config, + key: string, +): Promise { + await s3Config.client.send( + new DeleteObjectCommand({ + Bucket: s3Config.bucket, + Key: key, + }), + ); + log.debug(`Deleted S3 object: key=${key}, bucket=${s3Config.bucket}`); + return true; +} diff --git a/uploads/s3-utils/src/utils.ts b/uploads/s3-utils/src/utils.ts index 434a9ac1b..40384e590 100644 --- a/uploads/s3-utils/src/utils.ts +++ b/uploads/s3-utils/src/utils.ts @@ -1,4 +1,5 @@ import { + DeleteObjectCommand, GetObjectCommand, HeadObjectCommand, S3Client} from '@aws-sdk/client-s3'; @@ -30,6 +31,16 @@ export const fileExists = async ({ client, bucket, key }: FileOperationArgs): Pr } }; +export const deleteObject = async ({ client, bucket, key }: FileOperationArgs): Promise => { + try { + await client.send(new DeleteObjectCommand({ Bucket: bucket, Key: key })); + return true; + } catch (e: any) { + if (e.name === 'NoSuchKey' || e.$metadata?.httpStatusCode === 404) return false; + throw e; + } +}; + export const download = async ({ client, writeStream, From e790fed9d2289b49210d58845b75d1dfe6905a42 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Wed, 6 May 2026 21:49:14 +0000 Subject: [PATCH 2/2] refactor: remove redundant job enqueue from processDelete The AFTER DELETE trigger on the files table (constructive-db PR #1033) already enqueues the async delete_s3_object job via SECURITY DEFINER. The manual job enqueue from the Graphile plugin would fail anyway since the authenticated role cannot access app_jobs. The sync S3 delete in the plugin is best-effort; the DB trigger is the reliable fallback. --- .../src/plugin.ts | 51 ++++--------------- 1 file changed, 11 insertions(+), 40 deletions(-) diff --git a/graphile/graphile-presigned-url-plugin/src/plugin.ts b/graphile/graphile-presigned-url-plugin/src/plugin.ts index 5a49986a1..1f0c7579d 100644 --- a/graphile/graphile-presigned-url-plugin/src/plugin.ts +++ b/graphile/graphile-presigned-url-plugin/src/plugin.ts @@ -678,13 +678,17 @@ async function processSingleFile( // --- Delete logic --- /** - * Process a file deletion: remove the DB record, then attempt S3 cleanup. + * Process a file deletion: remove the DB record, then attempt sync S3 cleanup. + * + * The AFTER DELETE trigger on the files table always enqueues an async + * delete_s3_object job as a safety net. This function attempts the S3 delete + * inline for immediate cleanup — if it fails, the async job handles it. * * 1. Resolve the file row (key, bucket_id) and storage config * 2. DELETE the file row (RLS enforced — only owner/admin can delete) + * → AFTER DELETE trigger enqueues async GC job (SECURITY DEFINER) * 3. Check refcount: any other file with same key in the same bucket? - * 4. If orphaned: try S3 DeleteObject inline (sync) - * - On S3 failure: enqueue delete_s3_object job (async fallback) + * 4. If orphaned: try S3 DeleteObject inline (sync, best-effort) * 5. Return result */ async function processDelete( @@ -706,7 +710,7 @@ async function processDelete( throw new Error('DATABASE_NOT_FOUND'); } - // 1. Resolve storage config + file across all storage modules (app-level + entity-scoped) + // 1. Resolve storage config + file across all storage modules const resolved = await resolveStorageModuleByFileId(txClient, databaseId, fileId); if (!resolved) { throw new Error('FILE_NOT_FOUND: file does not exist or access denied'); @@ -715,7 +719,7 @@ async function processDelete( const { storageConfig, file } = resolved; const { key, bucket_id } = file; - // 2. DELETE the file row (RLS enforced — will fail if user lacks permission) + // 2. DELETE the file row (RLS enforced) const deleteResult = await txClient.query({ text: `DELETE FROM ${storageConfig.filesQualifiedName} WHERE id = $1 @@ -739,7 +743,6 @@ async function processDelete( const refCount = refcountResult.rows[0]?.ref_count ?? 0; if (refCount > 0) { - // Other files reference this S3 key — do not delete from S3 log.info(`File ${fileId} deleted from DB; S3 key ${key} still referenced by ${refCount} file(s)`); return { success: true, @@ -748,7 +751,7 @@ async function processDelete( }; } - // 4. Attempt sync S3 delete + // 4. Attempt sync S3 delete (best-effort; async GC job is the fallback) try { const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId); await deleteS3Object(s3ForDb, key); @@ -759,39 +762,7 @@ async function processDelete( key, }; } catch (s3Error: any) { - // S3 delete failed — enqueue async job as fallback - log.warn(`S3 delete failed for key=${key}, falling back to async job: ${s3Error.message}`); - - try { - await txClient.query({ - text: `SELECT app_jobs.add_job( - $1::text, - $2::json, - job_key := $3::text, - queue_name := $4::text, - priority := $5::int, - run_at := NOW() + interval '5 seconds' - )`, - values: [ - 'delete_s3_object', - JSON.stringify({ - key, - bucket_id, - database_id: databaseId, - schema_name: storageConfig.schemaName, - table_name: storageConfig.filesTableName, - }), - `gc:${bucket_id}:${key}`, - 'storage_gc', - 100, - ], - }); - log.info(`Enqueued delete_s3_object job for key=${key}`); - } catch (jobError: any) { - // app_jobs might not be installed — log but don't fail the delete - log.warn(`Failed to enqueue delete_s3_object job: ${jobError.message}`); - } - + log.warn(`Sync S3 delete failed for key=${key}; async GC job will retry: ${s3Error.message}`); return { success: true, deletedFromS3: false,