From f01ac91bf95918e8c719b150af04a8eff76e23fd Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Wed, 13 May 2026 16:56:46 +0200 Subject: [PATCH] fix(backend): keep manifest cleanup on soft delete --- package.json | 1 + scripts/cleanup_inactive_manifest.ts | 418 ++++++++++++++++++ .../_backend/triggers/on_version_update.ts | 55 ++- tests/files-app-read-guard.unit.test.ts | 16 +- tests/files-local-read-proxy.unit.test.ts | 3 +- tests/files-security.test.ts | 7 +- tests/on-version-update-cleanup.unit.test.ts | 100 ++++- 7 files changed, 564 insertions(+), 36 deletions(-) create mode 100644 scripts/cleanup_inactive_manifest.ts diff --git a/package.json b/package.json index 52830995f7..05379b1309 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "test:cloudflare:updates": "vitest run tests/updates* --config vitest.config.cloudflare.ts", "bench": "vitest bench --config vitest.config.bench.ts --run", "admin:backfill-paid-product-activity": "bun scripts/backfill_paid_product_activity.ts", + "admin:cleanup-inactive-manifest": "bun scripts/cleanup_inactive_manifest.ts", "admin:backfill-plugin-version-ladder": "bun scripts/backfill_plugin_version_ladder.ts", "admin:backfill-missing-app-icons": "bun scripts/backfill_missing_app_icons.ts", "admin:backfill-missing-store-urls": "bun scripts/backfill_missing_store_urls.ts", diff --git a/scripts/cleanup_inactive_manifest.ts b/scripts/cleanup_inactive_manifest.ts new file mode 100644 index 0000000000..2a45f883ca --- /dev/null +++ b/scripts/cleanup_inactive_manifest.ts @@ -0,0 +1,418 @@ +/* + * Audit and clean manifest rows whose stored object is not referenced by any active version. + * + * Dry run: + * bun run admin:cleanup-inactive-manifest + * + * Apply: + * bun run admin:cleanup-inactive-manifest --apply + * + * Optional: + * bun run admin:cleanup-inactive-manifest --apply --db-url="$DATABASE_URL" + * bun run admin:cleanup-inactive-manifest --apply --env-file=./internal/cloudflare/.env.prod + * bun run admin:cleanup-inactive-manifest --apply --batch-size=250 --max-batches=100 --pause-ms=250 + */ +import process from 'node:process' +import { setTimeout as sleep } from 'node:timers/promises' +import { DeleteObjectCommand, S3Client } from '@aws-sdk/client-s3' +import { Client } from 'pg' +import { asyncPool, DEFAULT_ENV_FILE, getArgValue, getRequiredEnv, loadEnv, parsePositiveInteger } from './admin_stripe_backfill_utils.ts' + +interface CandidateSummaryRow { + manifest_rows: string + s3_paths: string + versions: string +} + +interface CandidatePathRow { + first_manifest_id: string + manifest_rows: string + s3_path: string + versions: string +} + +interface DeletedManifestRow { + app_version_id: string + id: string + s3_path: string +} + +const DEFAULT_BATCH_SIZE = 250 +const DEFAULT_MAX_BATCHES = 100 +const DEFAULT_PAUSE_MS = 250 +const DEFAULT_R2_CONCURRENCY = 10 + +function printHelp() { + console.log(`Audit and clean inactive manifest rows. + +The script only targets manifest rows attached to deleted app_versions when +their s3_path is not referenced by any active app_versions row. + +Usage: + bun run admin:cleanup-inactive-manifest [options] + +Options: + --apply Delete matching manifest rows from Supabase and matching objects from R2. + --db-url=URL Postgres connection string. Overrides env file values. + --env-file=PATH Env file to load. Default: ${DEFAULT_ENV_FILE}. + --batch-size=N Number of distinct s3_path objects to process per batch. Default: ${DEFAULT_BATCH_SIZE}. + --max-batches=N Maximum cleanup batches to run. Default: ${DEFAULT_MAX_BATCHES}. + --pause-ms=N Delay between batches. Default: ${DEFAULT_PAUSE_MS}. + --r2-concurrency=N Parallel R2 deletes per batch. Default: ${DEFAULT_R2_CONCURRENCY}. + --help Show this help. + +Required env: + DATABASE_URL, SUPABASE_DB_URL, POSTGRES_URL, or PGDATABASE_URL + S3_ACCESS_KEY_ID, S3_SECRET_ACCESS_KEY, S3_ENDPOINT + +Optional env: + S3_BUCKET, S3_REGION, S3_SSL +`) +} + +function parseNonNegativeInteger(value: string | null, label: string, fallback: number) { + if (value === null) + return fallback + + const parsed = Number.parseInt(value, 10) + if (!Number.isInteger(parsed) || parsed < 0) + throw new Error(`${label} must be a non-negative integer`) + + return parsed +} + +function getDatabaseUrl(env: Record, args: string[]) { + return getArgValue(args, '--db-url') + ?? env.DATABASE_URL?.trim() + ?? env.SUPABASE_DB_URL?.trim() + ?? env.POSTGRES_URL?.trim() + ?? env.PGDATABASE_URL?.trim() + ?? null +} + +function shouldUseSsl(databaseUrl: string) { + const url = new URL(databaseUrl) + const sslMode = url.searchParams.get('sslmode') + if (sslMode === 'disable') + return false + if (url.hostname === 'localhost' || url.hostname === '127.0.0.1') + return false + return true +} + +function createR2Client(env: Record) { + const endpoint = getRequiredEnv(env, 'S3_ENDPOINT') + const protocol = env.S3_SSL === 'false' ? 'http' : 'https' + const normalizedEndpoint = endpoint.includes('://') ? endpoint : `${protocol}://${endpoint}` + + return { + bucket: env.S3_BUCKET?.trim() || 'capgo', + client: new S3Client({ + credentials: { + accessKeyId: getRequiredEnv(env, 'S3_ACCESS_KEY_ID'), + secretAccessKey: getRequiredEnv(env, 'S3_SECRET_ACCESS_KEY'), + }, + endpoint: normalizedEndpoint, + region: env.S3_REGION || 'auto', + }), + } +} + +function toInt(value: string | number | null | undefined) { + return Number.parseInt(String(value ?? '0'), 10) +} + +function uniq(values: string[]) { + return Array.from(new Set(values)) +} + +function describeError(error: unknown) { + if (error instanceof Error) + return error.message + return String(error) +} + +async function getCandidateSummary(client: Client) { + const result = await client.query(` + SELECT + count(*)::text AS manifest_rows, + count(DISTINCT m.app_version_id)::text AS versions, + count(DISTINCT m.s3_path)::text AS s3_paths + FROM public.manifest m + JOIN public.app_versions av ON av.id = m.app_version_id + WHERE av.deleted = true + AND m.s3_path <> '' + AND NOT EXISTS ( + SELECT 1 + FROM public.manifest active_m + JOIN public.app_versions active_av ON active_av.id = active_m.app_version_id + WHERE active_av.deleted = false + AND active_m.s3_path = m.s3_path + ) + `) + return result.rows[0] +} + +async function fetchCandidatePaths(client: Client, batchSize: number) { + const result = await client.query(` + SELECT + m.s3_path, + count(*)::text AS manifest_rows, + count(DISTINCT m.app_version_id)::text AS versions, + min(m.id)::text AS first_manifest_id + FROM public.manifest m + JOIN public.app_versions av ON av.id = m.app_version_id + WHERE av.deleted = true + AND m.s3_path <> '' + AND NOT EXISTS ( + SELECT 1 + FROM public.manifest active_m + JOIN public.app_versions active_av ON active_av.id = active_m.app_version_id + WHERE active_av.deleted = false + AND active_m.s3_path = m.s3_path + ) + GROUP BY m.s3_path + ORDER BY min(m.id) + LIMIT $1 + `, [batchSize]) + return result.rows +} + +async function getActiveReferencedPaths(client: Client, paths: string[]) { + if (paths.length === 0) + return new Set() + + const result = await client.query<{ s3_path: string }>(` + SELECT DISTINCT m.s3_path + FROM public.manifest m + JOIN public.app_versions av ON av.id = m.app_version_id + WHERE av.deleted = false + AND m.s3_path = ANY($1::text[]) + `, [paths]) + + return new Set(result.rows.map(row => row.s3_path)) +} + +async function refreshManifestCounters(client: Client, versionIds: string[]) { + const uniqueVersionIds = uniq(versionIds) + if (uniqueVersionIds.length === 0) + return [] + + await client.query(` + UPDATE public.app_versions av + SET manifest_count = manifest_counts.manifest_count + FROM ( + SELECT + av_inner.id, + count(m.id)::integer AS manifest_count + FROM public.app_versions av_inner + LEFT JOIN public.manifest m ON m.app_version_id = av_inner.id + WHERE av_inner.id = ANY($1::bigint[]) + GROUP BY av_inner.id + ) manifest_counts + WHERE av.id = manifest_counts.id + `, [uniqueVersionIds]) + + const appResult = await client.query<{ app_id: string }>(` + SELECT DISTINCT app_id + FROM public.app_versions + WHERE id = ANY($1::bigint[]) + `, [uniqueVersionIds]) + + const appIds = appResult.rows.map(row => row.app_id) + if (appIds.length === 0) + return [] + + await client.query(` + UPDATE public.apps app + SET + manifest_bundle_count = ( + SELECT count(DISTINCT av.id)::bigint + FROM public.app_versions av + WHERE av.app_id = app.app_id + AND EXISTS ( + SELECT 1 + FROM public.manifest m + WHERE m.app_version_id = av.id + ) + ), + updated_at = now() + WHERE app.app_id = ANY($1::varchar[]) + `, [appIds]) + + return appIds +} + +async function deleteManifestRows(client: Client, paths: string[]) { + await client.query('BEGIN') + try { + const deleteResult = await client.query(` + DELETE FROM public.manifest m + USING public.app_versions av + WHERE av.id = m.app_version_id + AND av.deleted = true + AND m.s3_path = ANY($1::text[]) + AND NOT EXISTS ( + SELECT 1 + FROM public.manifest active_m + JOIN public.app_versions active_av ON active_av.id = active_m.app_version_id + WHERE active_av.deleted = false + AND active_m.s3_path = m.s3_path + ) + RETURNING m.id::text, m.app_version_id::text, m.s3_path + `, [paths]) + + const appIds = await refreshManifestCounters( + client, + deleteResult.rows.map(row => row.app_version_id), + ) + + await client.query('COMMIT') + + return { + appIds, + deletedRows: deleteResult.rows, + paths: uniq(deleteResult.rows.map(row => row.s3_path)), + versionIds: uniq(deleteResult.rows.map(row => row.app_version_id)), + } + } + catch (error) { + await client.query('ROLLBACK') + throw error + } +} + +async function deleteR2Objects(s3: S3Client, bucket: string, paths: string[], concurrency: number) { + const failed: Array<{ error: string, path: string }> = [] + let deleted = 0 + + await asyncPool(concurrency, paths, async (path) => { + try { + await s3.send(new DeleteObjectCommand({ Bucket: bucket, Key: path })) + deleted += 1 + } + catch (error) { + failed.push({ error: describeError(error), path }) + } + }) + + return { deleted, failed } +} + +function printSummary(label: string, summary: CandidateSummaryRow | undefined) { + console.log(`\n${label}`) + console.table([{ + manifest_rows: toInt(summary?.manifest_rows), + s3_paths: toInt(summary?.s3_paths), + versions: toInt(summary?.versions), + }]) +} + +async function runApply( + client: Client, + s3: S3Client, + bucket: string, + batchSize: number, + maxBatches: number, + pauseMs: number, + r2Concurrency: number, +) { + let totalDbRows = 0 + let totalR2Deleted = 0 + let totalR2Failed = 0 + + for (let batch = 1; batch <= maxBatches; batch += 1) { + const candidatePaths = await fetchCandidatePaths(client, batchSize) + if (candidatePaths.length === 0) { + console.log(`No inactive manifest paths found after ${batch - 1} batch(es).`) + break + } + + const selectedPaths = candidatePaths.map(row => row.s3_path) + const deleted = await deleteManifestRows(client, selectedPaths) + if (deleted.deletedRows.length === 0) { + console.log(`Batch ${batch}: no rows deleted, stopping to avoid looping on changing data.`) + break + } + + const activePaths = await getActiveReferencedPaths(client, deleted.paths) + const r2Paths = deleted.paths.filter(path => !activePaths.has(path)) + const r2Result = await deleteR2Objects(s3, bucket, r2Paths, r2Concurrency) + + totalDbRows += deleted.deletedRows.length + totalR2Deleted += r2Result.deleted + totalR2Failed += r2Result.failed.length + + console.log(`Batch ${batch}: deleted ${deleted.deletedRows.length} Supabase manifest rows across ${deleted.versionIds.length} version(s), deleted ${r2Result.deleted}/${r2Paths.length} R2 object(s).`) + if (activePaths.size > 0) + console.log(`Batch ${batch}: skipped ${activePaths.size} R2 object(s) because active references appeared before R2 cleanup.`) + if (r2Result.failed.length > 0) + console.table(r2Result.failed.slice(0, 20)) + + if (pauseMs > 0) + await sleep(pauseMs) + } + + return { totalDbRows, totalR2Deleted, totalR2Failed } +} + +async function main() { + const args = Bun.argv.slice(2) + if (args.includes('--help')) { + printHelp() + return + } + + const apply = args.includes('--apply') + const envFile = getArgValue(args, '--env-file') ?? DEFAULT_ENV_FILE + const env = { ...process.env, ...await loadEnv(envFile) } + const databaseUrl = getDatabaseUrl(env, args) + if (!databaseUrl) + throw new Error('Missing database URL. Set DATABASE_URL, SUPABASE_DB_URL, POSTGRES_URL, PGDATABASE_URL, or pass --db-url.') + + const batchSize = parsePositiveInteger(getArgValue(args, '--batch-size'), '--batch-size', DEFAULT_BATCH_SIZE) + const maxBatches = parsePositiveInteger(getArgValue(args, '--max-batches'), '--max-batches', DEFAULT_MAX_BATCHES) + const pauseMs = parseNonNegativeInteger(getArgValue(args, '--pause-ms'), '--pause-ms', DEFAULT_PAUSE_MS) + const r2Concurrency = parsePositiveInteger(getArgValue(args, '--r2-concurrency'), '--r2-concurrency', DEFAULT_R2_CONCURRENCY) + + const client = new Client({ + application_name: 'capgo_cleanup_inactive_manifest', + connectionString: databaseUrl, + ssl: shouldUseSsl(databaseUrl) ? { rejectUnauthorized: true } : undefined, + }) + + await client.connect() + try { + await client.query('SELECT set_config($1, $2, false)', ['statement_timeout', '15min']) + await client.query('SELECT set_config($1, $2, false)', ['lock_timeout', '10s']) + + printSummary('Before cleanup', await getCandidateSummary(client)) + + const sample = await fetchCandidatePaths(client, Math.min(batchSize, 20)) + if (sample.length > 0) { + console.log('\nSample inactive manifest paths') + console.table(sample.map(row => ({ + first_manifest_id: row.first_manifest_id, + manifest_rows: toInt(row.manifest_rows), + s3_path: row.s3_path, + versions: toInt(row.versions), + }))) + } + + if (!apply) { + console.log('\nDry run only. Re-run with --apply to delete inactive manifest rows from Supabase and matching objects from R2.') + return + } + + const { bucket, client: s3 } = createR2Client(env) + const result = await runApply(client, s3, bucket, batchSize, maxBatches, pauseMs, r2Concurrency) + console.log(`\nCleanup result: deleted ${result.totalDbRows} Supabase manifest row(s), deleted ${result.totalR2Deleted} R2 object(s), ${result.totalR2Failed} R2 delete failure(s).`) + + printSummary('After cleanup', await getCandidateSummary(client)) + } + finally { + await client.end() + } +} + +await main() diff --git a/supabase/functions/_backend/triggers/on_version_update.ts b/supabase/functions/_backend/triggers/on_version_update.ts index 711130bda3..3341638467 100644 --- a/supabase/functions/_backend/triggers/on_version_update.ts +++ b/supabase/functions/_backend/triggers/on_version_update.ts @@ -232,6 +232,7 @@ async function deleteManifest(c: Context, record: Database['public']['Tables'][' .select('*', { count: 'exact', head: true }) .eq('file_name', entry.file_name) .eq('file_hash', entry.file_hash) + .eq('s3_path', entry.s3_path) }) .then((v) => { if (!v) @@ -291,6 +292,7 @@ async function deleteManifest(c: Context, record: Database['public']['Tables'][' export async function deleteIt(c: Context, record: Database['public']['Tables']['app_versions']['Row']) { cloudlog({ requestId: c.get('requestId'), message: 'Delete', r2_path: record.r2_path }) + let deferredError: (() => never) | null = null if (record.r2_path) { let deleted = false @@ -299,41 +301,46 @@ export async function deleteIt(c: Context, record: Database['public']['Tables'][ } catch (error) { cloudlog({ requestId: c.get('requestId'), message: 'Cannot delete s3 (v2)', error }) - throw simpleError('cannot_delete_s3', 'Cannot delete S3 object for deleted version', { id: record.id, r2_path: record.r2_path }, error) + deferredError = () => simpleError('cannot_delete_s3', 'Cannot delete S3 object for deleted version', { id: record.id, r2_path: record.r2_path }, error) } - if (!deleted) { - throw simpleError('cannot_delete_s3', 'Cannot delete S3 object for deleted version', { id: record.id, r2_path: record.r2_path }) - } + if (!deleted && !deferredError) + deferredError = () => simpleError('cannot_delete_s3', 'Cannot delete S3 object for deleted version', { id: record.id, r2_path: record.r2_path }) } else { cloudlog({ requestId: c.get('requestId'), message: 'No r2 path for deleted version', id: record.id }) } - const { data, error: dbError } = await supabaseAdmin(c) - .from('app_versions_meta') - .select() - .eq('id', record.id) - .single() - if (dbError || !data) { - cloudlog({ requestId: c.get('requestId'), message: 'Cannot find version meta', id: record.id }) - return c.json(BRES) - } - const { error: errorCreateStatsMeta } = await createStatsMeta(c, record.app_id, record.id, -data.size) - if (errorCreateStatsMeta) - cloudlog({ requestId: c.get('requestId'), message: 'error createStatsMeta', error: errorCreateStatsMeta }) - // set app_versions_meta versionSize = 0 - const { error: errorUpdate } = await supabaseAdmin(c) - .from('app_versions_meta') - .update({ size: 0 }) - .eq('id', record.id) - if (errorUpdate) { - cloudlog({ requestId: c.get('requestId'), message: 'error', error: errorUpdate }) - throw simpleError('cannot_update_version_meta', 'Cannot update version metadata for deleted version', { id: record.id }, errorUpdate) + if (!deferredError) { + const { data, error: dbError } = await supabaseAdmin(c) + .from('app_versions_meta') + .select() + .eq('id', record.id) + .single() + if (dbError || !data) { + cloudlog({ requestId: c.get('requestId'), message: 'Cannot find version meta', id: record.id }) + } + else { + const { error: errorCreateStatsMeta } = await createStatsMeta(c, record.app_id, record.id, -data.size) + if (errorCreateStatsMeta) + cloudlog({ requestId: c.get('requestId'), message: 'error createStatsMeta', error: errorCreateStatsMeta }) + // set app_versions_meta versionSize = 0 + const { error: errorUpdate } = await supabaseAdmin(c) + .from('app_versions_meta') + .update({ size: 0 }) + .eq('id', record.id) + if (errorUpdate) { + cloudlog({ requestId: c.get('requestId'), message: 'error', error: errorUpdate }) + deferredError ??= () => simpleError('cannot_update_version_meta', 'Cannot update version metadata for deleted version', { id: record.id }, errorUpdate) + } + } } await deleteManifest(c, record) + if (deferredError) + deferredError() + return c.json(BRES) } diff --git a/tests/files-app-read-guard.unit.test.ts b/tests/files-app-read-guard.unit.test.ts index 0b063dc938..3544276671 100644 --- a/tests/files-app-read-guard.unit.test.ts +++ b/tests/files-app-read-guard.unit.test.ts @@ -28,13 +28,13 @@ vi.mock('../supabase/functions/_backend/utils/pg_files.ts', () => ({ getUserIdFromApikey: vi.fn(), })) -describe('files app-scoped read guard', () => { +describe('files app-scoped read availability', () => { beforeEach(() => { vi.resetModules() vi.clearAllMocks() }) - it('returns 404 for deleted app-scoped files before serving cached content', async () => { + it('serves cached app-scoped files without checking app deletion state', async () => { getAppByAppIdPgMock.mockResolvedValue(null) const bucketPut = vi.fn() @@ -65,12 +65,14 @@ describe('files app-scoped read guard', () => { { waitUntil: () => { } } as any, ) - expect(response.status).toBe(404) + expect(response.status).toBe(200) + expect(await response.text()).toBe('cached orphan bytes') expect(bucketPut).not.toHaveBeenCalled() - expect(getPgClientMock).toHaveBeenCalledWith(expect.anything(), false) + expect(getPgClientMock).not.toHaveBeenCalled() + expect(getAppByAppIdPgMock).not.toHaveBeenCalled() }) - it('returns 404 for malformed app-scoped paths before serving cached content', async () => { + it('serves cached malformed app-scoped paths without database reads', async () => { const bucketPut = vi.fn() globalThis.caches = { default: { @@ -99,8 +101,10 @@ describe('files app-scoped read guard', () => { { waitUntil: () => { } } as any, ) - expect(response.status).toBe(404) + expect(response.status).toBe(200) + expect(await response.text()).toBe('cached malformed bytes') expect(bucketPut).not.toHaveBeenCalled() expect(getAppByAppIdPgMock).not.toHaveBeenCalled() + expect(getPgClientMock).not.toHaveBeenCalled() }) }) diff --git a/tests/files-local-read-proxy.unit.test.ts b/tests/files-local-read-proxy.unit.test.ts index dd3d7ce052..11da36d6b9 100644 --- a/tests/files-local-read-proxy.unit.test.ts +++ b/tests/files-local-read-proxy.unit.test.ts @@ -91,7 +91,8 @@ describe('files local read proxy', () => { expect(await response.text()).toBe('proxied local bytes') expect(response.headers.get('cache-control')).toBe('public, max-age=60, no-transform') expect(response.headers.get('content-disposition')).toBe('attachment; filename="orgs/test-org/apps/test-app/local.txt"') - expect(getPgClientMock).toHaveBeenCalledWith(expect.anything(), false) + expect(getPgClientMock).not.toHaveBeenCalled() + expect(getAppByAppIdPgMock).not.toHaveBeenCalled() expect(storageFromMock).toHaveBeenCalledWith('capgo') expect(createSignedUrlMock).toHaveBeenCalledWith('orgs/test-org/apps/test-app/local.txt', 60) }) diff --git a/tests/files-security.test.ts b/tests/files-security.test.ts index c75bf38a48..5e6c8f6a80 100644 --- a/tests/files-security.test.ts +++ b/tests/files-security.test.ts @@ -192,7 +192,7 @@ describe('ready bundle upload immutability regression', () => { }) }) -describe('attachment cleanup on app deletion regression', () => { +describe('attachment availability after app deletion', () => { const scopeId = randomUUID().replaceAll('-', '') const orgId = randomUUID() const stripeCustomerId = `cus_files_delete_${scopeId}` @@ -217,7 +217,7 @@ describe('attachment cleanup on app deletion regression', () => { await cleanupSeededOrg(appId, orgId, stripeCustomerId, [uploadKeyId, deleteKeyId]) }, 60_000) - it('returns not_found for uploaded attachments after the app is deleted', async () => { + it('keeps cached uploaded attachments readable after the app is deleted', async () => { await seedApp(appId, orgId, stripeCustomerId) const body = new TextEncoder().encode('delete-me-after-app-delete') @@ -263,6 +263,7 @@ describe('attachment cleanup on app deletion regression', () => { expect(deleteResponse.status).toBe(200) const readAfterDelete = await fetch(getEndpointUrl(`/files/read/attachments/${filePath}`)) - expect(readAfterDelete.status).toBe(404) + expect(readAfterDelete.status).toBe(200) + expect(await readAfterDelete.text()).toBe('delete-me-after-app-delete') }) }) diff --git a/tests/on-version-update-cleanup.unit.test.ts b/tests/on-version-update-cleanup.unit.test.ts index 339667782d..4f4b008014 100644 --- a/tests/on-version-update-cleanup.unit.test.ts +++ b/tests/on-version-update-cleanup.unit.test.ts @@ -9,12 +9,25 @@ const { deleteObject, getDrizzleClient, getPgClient, + manifestDeleteEq, + manifestEntries, + manifestSelectFileNameEq, + manifestSelectHashEq, + manifestSelectPathEq, + pgQuery, supabaseAdmin, } = vi.hoisted(() => { const appVersionsMetaSelectEq = vi.fn() const appVersionsMetaSelect = vi.fn(() => ({ eq: appVersionsMetaSelectEq })) const appVersionsMetaUpdateEq = vi.fn() const appVersionsMetaUpdate = vi.fn(() => ({ eq: appVersionsMetaUpdateEq })) + const manifestEntries: any[] = [] + const manifestDeleteEq = vi.fn() + const manifestDelete = vi.fn(() => ({ eq: manifestDeleteEq })) + const manifestSelectPathEq = vi.fn() + const manifestSelectHashEq = vi.fn(() => ({ eq: manifestSelectPathEq })) + const manifestSelectFileNameEq = vi.fn(() => ({ eq: manifestSelectHashEq })) + const manifestSelect = vi.fn(() => ({ eq: manifestSelectFileNameEq })) const supabaseFrom = vi.fn((table: string) => { if (table === 'app_versions_meta') { return { @@ -22,8 +35,15 @@ const { update: appVersionsMetaUpdate, } } + if (table === 'manifest') { + return { + delete: manifestDelete, + select: manifestSelect, + } + } return {} }) + const pgQuery = vi.fn() return { appVersionsMetaSelectEq, @@ -35,11 +55,17 @@ const { getDrizzleClient: vi.fn(() => ({ select: vi.fn(() => ({ from: vi.fn(() => ({ - where: vi.fn(async () => []), + where: vi.fn(async () => manifestEntries), })), })), })), - getPgClient: vi.fn(() => ({})), + getPgClient: vi.fn(() => ({ query: pgQuery })), + manifestDeleteEq, + manifestEntries, + manifestSelectFileNameEq, + manifestSelectHashEq, + manifestSelectPathEq, + pgQuery, supabaseAdmin: vi.fn(() => ({ from: supabaseFrom })), supabaseFrom, } @@ -71,6 +97,10 @@ vi.mock('../supabase/functions/_backend/utils/logging.ts', () => ({ cloudlogErr: vi.fn(), })) +vi.mock('../supabase/functions/_backend/utils/utils.ts', () => ({ + backgroundTask: vi.fn((_c: unknown, promise: Promise) => promise), +})) + const { deleteIt } = await import('../supabase/functions/_backend/triggers/on_version_update.ts') function createContext() { @@ -95,8 +125,12 @@ function createVersion(overrides: Record = {}) { describe('on_version_update deleted version cleanup', () => { beforeEach(() => { vi.clearAllMocks() + manifestEntries.length = 0 deleteObject.mockResolvedValue(true) createStatsMeta.mockResolvedValue({ error: null }) + manifestDeleteEq.mockResolvedValue({ error: null }) + manifestSelectPathEq.mockResolvedValue({ error: null, count: 0 }) + pgQuery.mockResolvedValue({ rows: [], rowCount: 1 }) appVersionsMetaSelectEq.mockReturnValue({ single: vi.fn(async () => ({ data: { size: 1234 }, error: null })), }) @@ -129,4 +163,66 @@ describe('on_version_update deleted version cleanup', () => { expect(appVersionsMetaUpdate).not.toHaveBeenCalled() expect(createStatsMeta).not.toHaveBeenCalled() }) + + it('still deletes manifest rows when version metadata is missing', async () => { + manifestEntries.push({ + id: 456, + app_version_id: 123, + file_name: 'www/app.js', + file_hash: 'hash-1', + s3_path: 'orgs/org-1/apps/com.cleanup.test/manifest/www/app.js', + }) + appVersionsMetaSelectEq.mockReturnValue({ + single: vi.fn(async () => ({ data: null, error: { message: 'not found' } })), + }) + + const response = await deleteIt(createContext(), createVersion({ r2_path: null })) + + expect(response.status).toBe(200) + expect(appVersionsMetaUpdate).not.toHaveBeenCalled() + expect(createStatsMeta).not.toHaveBeenCalled() + expect(manifestDeleteEq).toHaveBeenCalledWith('id', 456) + expect(manifestSelectFileNameEq).toHaveBeenCalledWith('file_name', 'www/app.js') + expect(manifestSelectHashEq).toHaveBeenCalledWith('file_hash', 'hash-1') + expect(manifestSelectPathEq).toHaveBeenCalledWith('s3_path', 'orgs/org-1/apps/com.cleanup.test/manifest/www/app.js') + expect(deleteObject).toHaveBeenCalledWith(expect.anything(), 'orgs/org-1/apps/com.cleanup.test/manifest/www/app.js') + }) + + it('still attempts manifest cleanup when bundle deletion fails', async () => { + manifestEntries.push({ + id: 789, + app_version_id: 123, + file_name: 'www/index.html', + file_hash: 'hash-2', + s3_path: 'orgs/org-1/apps/com.cleanup.test/manifest/www/index.html', + }) + deleteObject.mockResolvedValue(false) + + await expect(deleteIt(createContext(), createVersion())).rejects.toThrow('Cannot delete S3 object for deleted version') + + expect(manifestDeleteEq).toHaveBeenCalledWith('id', 789) + expect(manifestSelectPathEq).toHaveBeenCalledWith('s3_path', 'orgs/org-1/apps/com.cleanup.test/manifest/www/index.html') + expect(appVersionsMetaUpdate).not.toHaveBeenCalled() + expect(createStatsMeta).not.toHaveBeenCalled() + }) + + it('keeps shared manifest files in storage when another version still references them', async () => { + manifestEntries.push({ + id: 654, + app_version_id: 123, + file_name: 'www/shared.js', + file_hash: 'hash-shared', + s3_path: 'orgs/org-1/apps/com.cleanup.test/manifest/www/shared.js', + }) + manifestSelectPathEq.mockResolvedValue({ error: null, count: 1 }) + + const response = await deleteIt(createContext(), createVersion({ r2_path: null })) + + expect(response.status).toBe(200) + expect(manifestDeleteEq).toHaveBeenCalledWith('id', 654) + expect(manifestSelectFileNameEq).toHaveBeenCalledWith('file_name', 'www/shared.js') + expect(manifestSelectHashEq).toHaveBeenCalledWith('file_hash', 'hash-shared') + expect(manifestSelectPathEq).toHaveBeenCalledWith('s3_path', 'orgs/org-1/apps/com.cleanup.test/manifest/www/shared.js') + expect(deleteObject).not.toHaveBeenCalledWith(expect.anything(), 'orgs/org-1/apps/com.cleanup.test/manifest/www/shared.js') + }) })