From 382797febceec0eeb2d2f3c7a7d34a75601b87c1 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 1 Nov 2026 14:44:53 +0100 Subject: [PATCH] fix(storage): let the app boot with the reclaim routes and keep the ratchets where they were The two admin routes go on the managed allow list, without which buildApp refused to start. The reclaim calls move to their own api module so client.ts stays at its size, the phone card sizes its text through the type tiers, and the tests drop a hand-built service, a non-null assertion and a promise read as a boolean. --- client/src/api/client.ts | 8 ----- client/src/api/storageReclaim.ts | 12 ++++++++ .../Admin/storage/useStorageAdmin.ts | 5 ++-- .../screens/admin/MStorageReclaimCard.tsx | 29 +++++++++++++------ .../nest/common/validate-managed-routes.ts | 2 ++ .../tests/unit/nest/backup.controller.test.ts | 18 ++++++++---- .../nest/storage/storage-reclaim.job.test.ts | 5 ++-- .../storage/storage-reclaim.ledger.test.ts | 2 +- .../storage/storage-reclaim.service.test.ts | 6 ++-- 9 files changed, 58 insertions(+), 29 deletions(-) create mode 100644 client/src/api/storageReclaim.ts diff --git a/client/src/api/client.ts b/client/src/api/client.ts index ebf8dbfe6..fa7dc7b24 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -56,8 +56,6 @@ import { type BookingImportMode, type ReceiptScanResult, type StorageAdminState, - type StorageReclaimReport, - type StorageReclaimState, type StorageBackend, type StorageConfigPut, type StorageTestResponse, @@ -736,12 +734,6 @@ export const adminApi = { // A full scan of a large install can exceed the 8s instance timeout. refreshStorageStats: (): Promise => apiClient.post('/admin/storage/stats/refresh', undefined, { timeout: 120_000 }).then(r => r.data), - setStorageReclaimDelete: (deleteEnabled: boolean): Promise => - apiClient.put('/admin/storage/reclaim', { deleteEnabled }).then(r => r.data), - // A check lists every reclaimable category and asks each domain about it; a - // large bucket outlasts the 8s instance timeout. It only reports, whatever the switch says. - runStorageReclaimCheck: (): Promise => - apiClient.post('/admin/storage/reclaim/run', { reportOnly: true }, { timeout: 300_000 }).then(r => r.data), } export const addonsApi = { diff --git a/client/src/api/storageReclaim.ts b/client/src/api/storageReclaim.ts new file mode 100644 index 000000000..42786cd5d --- /dev/null +++ b/client/src/api/storageReclaim.ts @@ -0,0 +1,12 @@ +import type { StorageReclaimReport, StorageReclaimState } from '@trek/shared'; +import { apiClient } from './client'; + +/** Admin > Storage, unreferenced files: the nightly deletion switch and a check on demand. */ +export const storageReclaimApi = { + setDelete: (deleteEnabled: boolean): Promise => + apiClient.put('/admin/storage/reclaim', { deleteEnabled }).then((r) => r.data), + // A check lists every reclaimable category and asks each domain about it; a + // large bucket outlasts the 8s instance timeout. It only reports, whatever the switch says. + check: (): Promise => + apiClient.post('/admin/storage/reclaim/run', { reportOnly: true }, { timeout: 300_000 }).then((r) => r.data), +}; diff --git a/client/src/components/Admin/storage/useStorageAdmin.ts b/client/src/components/Admin/storage/useStorageAdmin.ts index 258f0d5d0..3f8999b2c 100644 --- a/client/src/components/Admin/storage/useStorageAdmin.ts +++ b/client/src/components/Admin/storage/useStorageAdmin.ts @@ -1,6 +1,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import type { StorageAdminState, StorageBackend, StorageConfig, StorageConfigPut, StorageTestResponse } from '@trek/shared' import { adminApi } from '../../../api/client' +import { storageReclaimApi } from '../../../api/storageReclaim' import { getApiErrorMessage, type ApiError } from '../../../types' import { settingsDocumentOf } from './storageModel' @@ -371,7 +372,7 @@ export function useStorageAdmin(genericError: string, conflictError: string): St const setReclaimDelete = useCallback(async (enabled: boolean): Promise => { try { - setReclaim(await adminApi.setStorageReclaimDelete(enabled)) + setReclaim(await storageReclaimApi.setDelete(enabled)) return null } catch (err: unknown) { return getApiErrorMessage(err, genericError) @@ -380,7 +381,7 @@ export function useStorageAdmin(genericError: string, conflictError: string): St const checkReclaim = useCallback(async (): Promise => { try { - const lastRun = await adminApi.runStorageReclaimCheck() + const lastRun = await storageReclaimApi.check() setReclaim({ deleteEnabled: stateRef.current?.reclaim?.deleteEnabled ?? false, lastRun }) return null } catch (err: unknown) { diff --git a/client/src/mobile/screens/admin/MStorageReclaimCard.tsx b/client/src/mobile/screens/admin/MStorageReclaimCard.tsx index f08c51aef..a6a596439 100644 --- a/client/src/mobile/screens/admin/MStorageReclaimCard.tsx +++ b/client/src/mobile/screens/admin/MStorageReclaimCard.tsx @@ -1,5 +1,6 @@ import React from 'react'; import type { ReclaimCount, ReclaimView } from '../../../components/Admin/storage/reclaimModel'; +import { fs } from '../../../components/shared/DialogShell'; import { useTranslation } from '../../../i18n'; import { formatBytes } from '../../../utils/formatBytes'; import { relativeTime } from '../../../utils/relativeTime'; @@ -39,13 +40,13 @@ export default function MStorageReclaimCard({ reclaim, checking, onCheck, onDele } />
-

+

{reclaim.checkedAt !== null ? t('storage.reclaim.checked', { age: relativeTime(reclaim.checkedAt, locale) }) : t('storage.reclaim.never')}

{reclaim.stopped && ( -

+

{t('storage.reclaim.stopped')}

)} @@ -57,24 +58,30 @@ export default function MStorageReclaimCard({ reclaim, checking, onCheck, onDele
)} {checked && reclaim.lines.length === 0 && ( -

{t('storage.reclaim.nothing')}

+

+ {t('storage.reclaim.nothing')} +

)} {reclaim.lines.length > 0 && (
{reclaim.lines.map((line) => (
-

{t(`storage.category.${line.category}`)}

+

+ {t(`storage.category.${line.category}`)} +

{line.skipped ? ( -

{t(`storage.reclaim.skip.${line.skipped}`)}

+

+ {t(`storage.reclaim.skip.${line.skipped}`)} +

) : ( <> {line.freeable.objects > 0 && ( -

+

{t('storage.reclaim.freeable')}: {count(line.freeable)}

)} {line.waiting.objects > 0 && ( -

+

{t('storage.reclaim.waiting')}: {count(line.waiting)}

)} @@ -92,8 +99,12 @@ export default function MStorageReclaimCard({ reclaim, checking, onCheck, onDele function Figure({ label, value }: { label: string; value: string }): React.ReactElement { return (
- {label} - {value} + + {label} + + + {value} +
); } diff --git a/server/src/nest/common/validate-managed-routes.ts b/server/src/nest/common/validate-managed-routes.ts index 2ce7ad056..1a6232f27 100644 --- a/server/src/nest/common/validate-managed-routes.ts +++ b/server/src/nest/common/validate-managed-routes.ts @@ -134,6 +134,8 @@ export const MANAGED_ROUTE_ALLOW_LIST: string[] = [ 'StorageAdminController.statsRefresh', 'StorageAdminController.test', 'StorageAdminController.update', + 'StorageReclaimController.run', + 'StorageReclaimController.update', ]; /** diff --git a/server/tests/unit/nest/backup.controller.test.ts b/server/tests/unit/nest/backup.controller.test.ts index 98fd919d2..82fd7ca06 100644 --- a/server/tests/unit/nest/backup.controller.test.ts +++ b/server/tests/unit/nest/backup.controller.test.ts @@ -7,13 +7,14 @@ import * as backupSvc from '../../../src/nest/backup/backup.impl'; import { BackupService as RealBackupService } from '../../../src/nest/backup/backup.service'; import type { BackupService } from '../../../src/nest/backup/backup.service'; import { StorageJobsService } from '../../../src/nest/storage/storage-jobs.service'; -import type { StorageRegistryService } from '../../../src/nest/storage/storage-registry.service'; +import { StorageRegistryService } from '../../../src/nest/storage/storage-registry.service'; import { StorageNotFoundError } from '../../../src/nest/storage/storage.types'; import type { User } from '../../../src/types'; import { HttpException } from '@nestjs/common'; +import { Test } from '@nestjs/testing'; import type { Request, Response } from 'express'; -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from 'vitest'; vi.mock('../../../src/nest/audit/client-ip', () => ({ getClientIp: vi.fn(() => '1.2.3.4') })); vi.mock('../../../src/nest/audit/audit-log.logger', () => ({ @@ -376,9 +377,16 @@ describe('BackupService (wrapper)', () => { const database = { __sentinel: 'database', } as unknown as import('../../../src/nest/database/database-backup.interface').DatabaseBackupStrategy; - // The real one-job slot: the restore marks itself running there. - const jobs = new StorageJobsService({} as StorageRegistryService); - const wrapper = new RealBackupService(storage, database, jobs); + // The real one-job slot, taken from a testing module: the restore marks itself running there. + let jobs: StorageJobsService; + let wrapper: RealBackupService; + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + providers: [StorageJobsService, { provide: StorageRegistryService, useValue: {} }], + }).compile(); + jobs = moduleRef.get(StorageJobsService); + wrapper = new RealBackupService(storage, database, jobs); + }); it('forwards every call straight to the legacy backup service', async () => { expect(await wrapper.listBackups()).toEqual([{ filename: 'svc.zip' }]); diff --git a/server/tests/unit/nest/storage/storage-reclaim.job.test.ts b/server/tests/unit/nest/storage/storage-reclaim.job.test.ts index 5cddc3157..f696ee34b 100644 --- a/server/tests/unit/nest/storage/storage-reclaim.job.test.ts +++ b/server/tests/unit/nest/storage/storage-reclaim.job.test.ts @@ -65,8 +65,9 @@ describe('StorageReclaimJob', () => { const { job, registrar, reclaim } = makeJob(() => new Promise((resolve) => (finish = resolve))); job.onApplicationBootstrap(); expect(registrar.register).toHaveBeenCalledWith('storage-reclaim', '5 4 * * *', expect.any(Function)); - const onTick = registrar.register.mock.calls[0]![2]; - const ticking = onTick(); + const [call] = registrar.register.mock.calls; + if (!call) throw new Error('the job registered nothing'); + const ticking = call[2](); expect(ticking).toBeInstanceOf(Promise); let settled = false; void (ticking as Promise).then(() => (settled = true)); diff --git a/server/tests/unit/nest/storage/storage-reclaim.ledger.test.ts b/server/tests/unit/nest/storage/storage-reclaim.ledger.test.ts index 455ee22a0..8eb766a8e 100644 --- a/server/tests/unit/nest/storage/storage-reclaim.ledger.test.ts +++ b/server/tests/unit/nest/storage/storage-reclaim.ledger.test.ts @@ -58,7 +58,7 @@ describe('RECLAIM_LEDGER', () => { for (const category of ['photos', 'photos-google', 'photos-trek', 'backups'] as const) { const rule: ReclaimRule = RECLAIM_LEDGER[category]; expect(rule.reclaim).toBe(false); - if (!rule.reclaim) expect(rule.reason.length).toBeGreaterThan(10); + if (rule.reclaim === false) expect(rule.reason.length).toBeGreaterThan(10); } }); diff --git a/server/tests/unit/nest/storage/storage-reclaim.service.test.ts b/server/tests/unit/nest/storage/storage-reclaim.service.test.ts index 09c3a72aa..ddef374d4 100644 --- a/server/tests/unit/nest/storage/storage-reclaim.service.test.ts +++ b/server/tests/unit/nest/storage/storage-reclaim.service.test.ts @@ -497,9 +497,11 @@ describe('StorageReclaimService: the switch and the stored report', () => { await reclaim.run({ now: T0 }); const jobs = t.get(StorageJobsService); let restoreStarted: Promise | null = null; + let restoreAsked = false; let restoreWent = false; onAsk = (_category, keys) => { - if (keys.length !== 1 || restoreStarted) return; + if (keys.length !== 1 || restoreAsked) return; + restoreAsked = true; restoreStarted = jobs.beginRestore().then(() => { restoreWent = true; }); @@ -508,7 +510,7 @@ describe('StorageReclaimService: the switch and the stored report', () => { const report = await reclaim.run({ now: WEEK_LATER }); expect(report.stopped).toBe('database_reopened'); expect(fs.existsSync(objectPath('covers', 'orphan.jpg'))).toBe(true); - expect(restoreStarted).not.toBeNull(); + expect(restoreAsked).toBe(true); await restoreStarted; expect(restoreWent).toBe(true); } finally {