diff --git a/package.json b/package.json index 52830995f7..152576f2c2 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-stuck-manifest-backlog": "bun scripts/cleanup_stuck_manifest_backlog.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_stuck_manifest_backlog.ts b/scripts/cleanup_stuck_manifest_backlog.ts new file mode 100644 index 0000000000..8c86ad34a1 --- /dev/null +++ b/scripts/cleanup_stuck_manifest_backlog.ts @@ -0,0 +1,248 @@ +/* + * Audit and clear old manifest rows stuck behind soft-deleted bundles. + * + * Dry run: + * bun run admin:cleanup-stuck-manifest-backlog + * + * Apply: + * bun run admin:cleanup-stuck-manifest-backlog --apply + * + * Optional: + * bun run admin:cleanup-stuck-manifest-backlog --apply --db-url="$DATABASE_URL" + * bun run admin:cleanup-stuck-manifest-backlog --apply --env-file=./internal/cloudflare/.env.prod + * bun run admin:cleanup-stuck-manifest-backlog --apply --max-batches=1000 --pause-ms=250 + * bun run admin:cleanup-stuck-manifest-backlog --apply --skip-vacuum + */ +import process from 'node:process' +import { setTimeout as sleep } from 'node:timers/promises' +import { Client } from 'pg' +import { DEFAULT_ENV_FILE, getArgValue, loadEnv, parsePositiveInteger } from './admin_stripe_backfill_utils.ts' + +interface TableSizeRow { + heap: string + indexes: string + total: string +} + +interface VacuumStatsRow { + last_autoanalyze: string | null + last_autovacuum: string | null + n_dead_tup: string + n_live_tup: string +} + +interface BucketRow { + bucket: string + manifest_rows: string + versions: string +} + +interface EligibleVersionRow { + eligible_versions: string +} + +const DEFAULT_MAX_BATCHES = 1000 +const DEFAULT_PAUSE_MS = 250 + +function printHelp() { + console.log(`Audit and clear old manifest rows stuck behind soft-deleted bundles. + +Usage: + bun run admin:cleanup-stuck-manifest-backlog [options] + +Options: + --apply Delete old soft-deleted versions by calling public.delete_old_deleted_versions(). + --db-url=URL Postgres connection string. Overrides env file values. + --env-file=PATH Env file to load. Default: ${DEFAULT_ENV_FILE}. + --max-batches=N Maximum cleanup batches to run. Default: ${DEFAULT_MAX_BATCHES}. + --pause-ms=N Delay between batches. Default: ${DEFAULT_PAUSE_MS}. + --skip-vacuum Do not run VACUUM (ANALYZE) public.manifest after apply. + --help Show this help. + +Required env: + DATABASE_URL, SUPABASE_DB_URL, POSTGRES_URL, or PGDATABASE_URL +`) +} + +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 +} + +async function getTableSize(client: Client) { + const result = await client.query(` + SELECT + pg_size_pretty(pg_relation_size('public.manifest')) AS heap, + pg_size_pretty(pg_indexes_size('public.manifest')) AS indexes, + pg_size_pretty(pg_total_relation_size('public.manifest')) AS total + `) + return result.rows[0] +} + +async function getVacuumStats(client: Client) { + const result = await client.query(` + SELECT n_live_tup, n_dead_tup, last_autovacuum, last_autoanalyze + FROM pg_stat_user_tables + WHERE schemaname = 'public' AND relname = 'manifest' + `) + return result.rows[0] +} + +async function getManifestBuckets(client: Client) { + const result = await client.query(` + SELECT + CASE + WHEN av.deleted = false THEN 'active' + WHEN av.deleted_at < now() - interval '3 months' THEN 'past_hard_delete' + ELSE 'soft_deleted_waiting' + END AS bucket, + count(*)::text AS manifest_rows, + count(DISTINCT av.id)::text AS versions + FROM public.manifest m + JOIN public.app_versions av ON av.id = m.app_version_id + GROUP BY 1 + ORDER BY count(*) DESC + `) + return result.rows +} + +async function getEligibleVersionCount(client: Client) { + const result = await client.query(` + SELECT count(*)::text AS eligible_versions + FROM public.app_versions av + WHERE av.deleted_at IS NOT NULL + AND av.deleted_at < now() - interval '3 months' + AND av.name NOT IN ('builtin', 'unknown') + AND NOT EXISTS ( + SELECT 1 + FROM public.channels + WHERE channels.version = av.id + ) + `) + return Number.parseInt(result.rows[0]?.eligible_versions ?? '0', 10) +} + +function printAudit(title: string, size: TableSizeRow, stats: VacuumStatsRow | undefined, buckets: BucketRow[]) { + console.log(`\n${title}`) + console.table([size]) + if (stats) + console.table([stats]) + console.table(buckets) +} + +async function runCleanupLoop(client: Client, maxBatches: number, pauseMs: number) { + let batches = 0 + let previousRemaining = await getEligibleVersionCount(client) + console.log(`Eligible old deleted versions before cleanup: ${previousRemaining}`) + + while (previousRemaining > 0 && batches < maxBatches) { + batches += 1 + await client.query('SELECT public.delete_old_deleted_versions()') + + const remaining = await getEligibleVersionCount(client) + const deleted = Math.max(previousRemaining - remaining, 0) + console.log(`Batch ${batches}: deleted about ${deleted} versions, ${remaining} eligible remain`) + + if (remaining >= previousRemaining) { + console.log('No progress detected; stopping to avoid looping on a blocked cleanup.') + break + } + + previousRemaining = remaining + if (pauseMs > 0) + await sleep(pauseMs) + } + + return { + batches, + remaining: previousRemaining, + } +} + +async function main() { + const args = Bun.argv.slice(2) + if (args.includes('--help')) { + printHelp() + return + } + + const apply = args.includes('--apply') + const skipVacuum = args.includes('--skip-vacuum') + 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 maxBatches = parsePositiveInteger(getArgValue(args, '--max-batches'), '--max-batches', DEFAULT_MAX_BATCHES) + const pauseMs = parseNonNegativeInteger(getArgValue(args, '--pause-ms'), '--pause-ms', DEFAULT_PAUSE_MS) + const client = new Client({ + application_name: 'capgo_cleanup_stuck_manifest_backlog', + 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']) + + printAudit( + 'Before cleanup', + await getTableSize(client), + await getVacuumStats(client), + await getManifestBuckets(client), + ) + + if (!apply) { + console.log('\nDry run only. Re-run with --apply to delete old soft-deleted versions and cascade stuck manifest rows.') + return + } + + const cleanup = await runCleanupLoop(client, maxBatches, pauseMs) + if (cleanup.remaining > 0) + console.log(`Stopped with ${cleanup.remaining} eligible versions still remaining. Increase --max-batches after checking database load.`) + + if (!skipVacuum) { + console.log('\nRunning VACUUM (ANALYZE) public.manifest...') + await client.query('VACUUM (ANALYZE) public.manifest') + } + + printAudit( + 'After cleanup', + await getTableSize(client), + await getVacuumStats(client), + await getManifestBuckets(client), + ) + } + finally { + await client.end() + } +} + +await main() diff --git a/supabase/functions/_backend/files/files.ts b/supabase/functions/_backend/files/files.ts index 73400849a2..86a6b58ebc 100644 --- a/supabase/functions/_backend/files/files.ts +++ b/supabase/functions/_backend/files/files.ts @@ -376,9 +376,9 @@ async function getSupabaseStorageResponse(c: Context, fileId: string): Promise { const fileId = c.get('fileId') - // It is imperative and inalthat we read file without any Database READ to avoid any potential bottlenecks and ensure high performance and availability of file downloads, especially under heavy load. - // This had beed designed that way and access to file going to be delete is non imporant compared to availability of file download, so we are not doing any check in DB or R2 before serving the file, if file is missing in R2 it will be 404 and that is expected and we want to avoid any potential bottlenecks. - // File access security is not a matter here and will NERVER BE. + // It is imperative that we read files without any database read to avoid bottlenecks and keep file downloads available under heavy load. + // This was designed that way: access to a file being deleted is less important than download availability, so we are not doing any check in DB or R2 before serving the file. If the file is missing in R2 it will be 404, and that is expected. + // File access security is not a matter here and will never be. cloudlog({ requestId: c.get('requestId'), message: 'getHandler files', fileId }) if (getRuntimeKey() !== 'workerd') { diff --git a/supabase/functions/_backend/triggers/on_version_update.ts b/supabase/functions/_backend/triggers/on_version_update.ts index 711130bda3..2c521139f5 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) @@ -317,19 +318,18 @@ export async function deleteIt(c: Context, record: Database['public']['Tables'][ .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) + 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 update version meta size during delete', id: record.id, error: errorUpdate }) } await deleteManifest(c, record) diff --git a/supabase/migrations/20260513003104_manifest_cleanup_health.sql b/supabase/migrations/20260513003104_manifest_cleanup_health.sql new file mode 100644 index 0000000000..faff2832f6 --- /dev/null +++ b/supabase/migrations/20260513003104_manifest_cleanup_health.sql @@ -0,0 +1,44 @@ +-- Keep hard-deleted bundle cleanup bounded so manifest cascades do not create +-- one very large delete transaction when retention has a backlog. +CREATE OR REPLACE FUNCTION "public"."delete_old_deleted_versions"() RETURNS "void" + LANGUAGE "plpgsql" + SECURITY DEFINER + SET search_path = '' +AS $$ +DECLARE + deleted_count bigint; +BEGIN + WITH deleted_versions AS ( + SELECT "app_versions"."id" + FROM "public"."app_versions" + WHERE "app_versions"."deleted_at" IS NOT NULL + AND "app_versions"."deleted_at" < now() - INTERVAL '3 months' + AND "app_versions"."name" NOT IN ('builtin', 'unknown') + AND NOT EXISTS ( + SELECT 1 + FROM "public"."channels" + WHERE "channels"."version" = "app_versions"."id" + ) + ORDER BY "app_versions"."deleted_at", "app_versions"."id" + LIMIT 500 + FOR UPDATE SKIP LOCKED + ) + DELETE FROM "public"."app_versions" + USING deleted_versions + WHERE "app_versions"."id" = deleted_versions."id"; + + GET DIAGNOSTICS deleted_count = ROW_COUNT; + + IF deleted_count > 0 THEN + RAISE NOTICE 'delete_old_deleted_versions: permanently deleted % app versions', deleted_count; + END IF; +END; +$$; + +ALTER FUNCTION "public"."delete_old_deleted_versions"() OWNER TO "postgres"; +COMMENT ON FUNCTION "public"."delete_old_deleted_versions"() IS 'Permanently deletes up to 500 soft-deleted app versions older than 3 months per run; related manifest rows cascade through foreign keys.'; + +REVOKE ALL ON FUNCTION "public"."delete_old_deleted_versions"() FROM PUBLIC; +REVOKE ALL ON FUNCTION "public"."delete_old_deleted_versions"() FROM "anon"; +REVOKE ALL ON FUNCTION "public"."delete_old_deleted_versions"() FROM "authenticated"; +GRANT EXECUTE ON FUNCTION "public"."delete_old_deleted_versions"() TO "service_role"; diff --git a/supabase/tests/16_test_retention.sql b/supabase/tests/16_test_retention.sql index e9b356c9e0..5d0814e5e6 100644 --- a/supabase/tests/16_test_retention.sql +++ b/supabase/tests/16_test_retention.sql @@ -1,7 +1,7 @@ BEGIN; -SELECT plan(15); +SELECT plan(18); CREATE OR REPLACE FUNCTION my_tests() RETURNS SETOF TEXT AS $$ DECLARE @@ -15,6 +15,8 @@ DECLARE version_id_recent BIGINT; version_id_2year BIGINT; version_id_zero BIGINT; + active_shared_version BIGINT; + remaining_batch_versions BIGINT; BEGIN -- Clean up any existing test data @@ -239,6 +241,83 @@ RETURN NEXT IS ( 'unknown version should never be hard-deleted' ); +-- Test 16-18: hard-delete should run in a bounded batch, cascade old manifest rows, +-- and keep manifest rows for active versions that reuse the same stored file. +CREATE TEMP TABLE retention_batch_targets(id BIGINT PRIMARY KEY) ON COMMIT DROP; + +WITH inserted_versions AS ( + INSERT INTO app_versions (app_id, name, deleted, deleted_at, storage_provider, owner_org) + SELECT + test_app_id, + 'batch-hard-delete-' || gs::text, + true, + '2019-01-01'::timestamp, + 'r2', + (SELECT owner_org FROM apps WHERE app_id = test_app_id) + FROM generate_series(1, 505) AS gs + RETURNING id +) +INSERT INTO retention_batch_targets(id) +SELECT id +FROM inserted_versions; + +INSERT INTO manifest (app_version_id, file_name, s3_path, file_hash, file_size) +SELECT + id, + 'file-' || id::text, + 'path-' || id::text, + 'hash-' || id::text, + 1 +FROM retention_batch_targets; + +INSERT INTO app_versions (app_id, name, deleted, storage_provider, owner_org) +VALUES ( + test_app_id, + 'active-reuses-hard-delete-file', + false, + 'r2', + (SELECT owner_org FROM apps WHERE app_id = test_app_id) +) +RETURNING id INTO active_shared_version; + +INSERT INTO manifest (app_version_id, file_name, s3_path, file_hash, file_size) +SELECT + active_shared_version, + 'file-' || id::text, + 'path-' || id::text, + 'hash-' || id::text, + 1 +FROM retention_batch_targets +ORDER BY id +LIMIT 1; + +PERFORM delete_old_deleted_versions(); + +SELECT COUNT(*) +INTO remaining_batch_versions +FROM app_versions +WHERE id IN (SELECT id FROM retention_batch_targets); + +-- delete_old_deleted_versions deletes up to 500 rows per call, so the +-- retention_batch_targets fixture starts with 505 rows and expects 5 remaining. +RETURN NEXT IS ( + remaining_batch_versions, + 5::BIGINT, + 'delete_old_deleted_versions should delete one bounded batch per run' +); + +RETURN NEXT IS ( + (SELECT COUNT(*) FROM manifest WHERE app_version_id IN (SELECT id FROM retention_batch_targets)), + remaining_batch_versions, + 'manifest rows should cascade for hard-deleted batch versions' +); + +RETURN NEXT IS ( + (SELECT COUNT(*) FROM manifest WHERE app_version_id = active_shared_version), + 1::BIGINT, + 'hard-delete should keep active-version manifest rows that reuse the same stored file' +); + END; $$ LANGUAGE plpgsql; 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..4e77a6b974 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,70 @@ 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('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') + }) + + it('resets manifest counters after deleting manifest entries', 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', + }) + + const response = await deleteIt(createContext(), createVersion({ r2_path: null })) + + expect(response.status).toBe(200) + expect(pgQuery).toHaveBeenCalledWith( + expect.stringContaining('UPDATE app_versions SET manifest_count = 0 WHERE id = $1'), + [123], + ) + expect(pgQuery).toHaveBeenCalledWith( + expect.stringContaining('manifest_bundle_count = GREATEST(manifest_bundle_count - 1, 0)'), + ['com.cleanup.test'], + ) + }) })