From 99bb296866e4bc7e7960a2a62f089f1f5b22eb37 Mon Sep 17 00:00:00 2001 From: Nico <47644445+nicotsx@users.noreply.github.com> Date: Mon, 16 Feb 2026 21:36:07 +0100 Subject: [PATCH] fix: cascade delete not correctly applied (#531) --- .../modules/volumes/routes/volume-details.tsx | 11 +- app/server.ts | 9 +- app/server/db/db.ts | 13 ++- .../backups/__tests__/backups.service.test.ts | 100 ++++++++++++++++++ app/server/modules/backups/backups.service.ts | 48 ++++++++- app/server/modules/lifecycle/bootstrap.ts | 2 +- app/server/modules/lifecycle/startup.ts | 10 ++ 7 files changed, 175 insertions(+), 18 deletions(-) diff --git a/app/client/modules/volumes/routes/volume-details.tsx b/app/client/modules/volumes/routes/volume-details.tsx index 50f7465a..b4a29126 100644 --- a/app/client/modules/volumes/routes/volume-details.tsx +++ b/app/client/modules/volumes/routes/volume-details.tsx @@ -94,14 +94,6 @@ export function VolumeDetails({ volumeId }: { volumeId: string }) { deleteVol.mutate({ path: { id: volumeId } }); }; - if (!volumeId) { - return
Volume not found
; - } - - if (!data) { - return
Loading...
; - } - const { volume, statfs } = data; return ( @@ -164,6 +156,9 @@ export function VolumeDetails({ volumeId }: { volumeId: string }) { Delete volume? Are you sure you want to delete the volume {volume.name}? This action cannot be undone. +
+
+ All backup schedules associated with this volume will also be removed.
diff --git a/app/server.ts b/app/server.ts index 0ccf59f8..4f8d2559 100644 --- a/app/server.ts +++ b/app/server.ts @@ -1,12 +1,11 @@ import { logger } from "./server/utils/logger"; import { shutdown } from "./server/modules/lifecycle/shutdown"; import { runCLI } from "./server/cli"; -import { - createStartHandler, - defaultStreamHandler, - defineHandlerCallback, -} from "@tanstack/react-start/server"; +import { createStartHandler, defaultStreamHandler, defineHandlerCallback } from "@tanstack/react-start/server"; import { createServerEntry } from "@tanstack/react-start/server-entry"; +import { runDbMigrations } from "./server/db/db"; + +await runDbMigrations(); const cliRun = await runCLI(Bun.argv); if (cliRun) { diff --git a/app/server/db/db.ts b/app/server/db/db.ts index 54b4fd21..53cdbafa 100644 --- a/app/server/db/db.ts +++ b/app/server/db/db.ts @@ -17,7 +17,9 @@ if (fs.existsSync(path.join(path.dirname(DATABASE_URL), "ironmount.db")) && !fs. const sqlite = new Database(DATABASE_URL); export const db = drizzle({ client: sqlite, relations, schema }); -export const runDbMigrations = () => { +let migrationsPromise: Promise | undefined; + +const runMigrations = async () => { let migrationsFolder: string; if (config.migrationsPath) { @@ -32,3 +34,12 @@ export const runDbMigrations = () => { sqlite.run("PRAGMA foreign_keys = ON;"); }; + +export const runDbMigrations = () => { + if (migrationsPromise) { + return migrationsPromise; + } + + migrationsPromise = runMigrations(); + return migrationsPromise; +}; diff --git a/app/server/modules/backups/__tests__/backups.service.test.ts b/app/server/modules/backups/__tests__/backups.service.test.ts index 5570a6ef..2dbf05cf 100644 --- a/app/server/modules/backups/__tests__/backups.service.test.ts +++ b/app/server/modules/backups/__tests__/backups.service.test.ts @@ -1,5 +1,6 @@ import waitForExpect from "wait-for-expect"; import { test, describe, mock, expect, beforeEach, afterEach, spyOn } from "bun:test"; +import { eq } from "drizzle-orm"; import { backupsService } from "../backups.service"; import { createTestVolume } from "~/test/helpers/volume"; import { createTestBackupSchedule } from "~/test/helpers/backup"; @@ -7,6 +8,8 @@ import { createTestRepository } from "~/test/helpers/repository"; import { generateBackupOutput } from "~/test/helpers/restic"; import { faker } from "@faker-js/faker"; import * as spawnModule from "~/server/utils/spawn"; +import { db } from "~/server/db/db"; +import { backupScheduleMirrorsTable, repositoriesTable, volumesTable } from "~/server/db/schema"; import { TEST_ORG_ID } from "~/test/helpers/organization"; import * as context from "~/server/core/request-context"; import { backupsExecutionService } from "../backups.execution"; @@ -183,3 +186,100 @@ describe("getSchedulesToExecute", () => { expect(schedulesToExecute).toContain(schedule.id); }); }); + +describe("listSchedules", () => { + test("should ignore schedules with missing relations", async () => { + const healthyVolume = await createTestVolume(); + const healthyRepository = await createTestRepository(); + const healthySchedule = await createTestBackupSchedule({ + volumeId: healthyVolume.id, + repositoryId: healthyRepository.id, + }); + + const orphanVolume = await createTestVolume(); + const orphanRepository = await createTestRepository(); + const orphanSchedule = await createTestBackupSchedule({ + volumeId: orphanVolume.id, + repositoryId: orphanRepository.id, + }); + + await db.delete(volumesTable).where(eq(volumesTable.id, orphanVolume.id)); + + const schedules = await backupsService.listSchedules(); + + expect(schedules.map((schedule) => schedule.id)).toContain(healthySchedule.id); + expect(schedules.map((schedule) => schedule.id)).not.toContain(orphanSchedule.id); + }); + + test("should ignore schedules with missing repository relation", async () => { + const healthyVolume = await createTestVolume(); + const healthyRepository = await createTestRepository(); + const healthySchedule = await createTestBackupSchedule({ + volumeId: healthyVolume.id, + repositoryId: healthyRepository.id, + }); + + const orphanVolume = await createTestVolume(); + const orphanRepository = await createTestRepository(); + const orphanSchedule = await createTestBackupSchedule({ + volumeId: orphanVolume.id, + repositoryId: orphanRepository.id, + }); + + await db.delete(repositoriesTable).where(eq(repositoriesTable.id, orphanRepository.id)); + + const schedules = await backupsService.listSchedules(); + + expect(schedules.map((schedule) => schedule.id)).toContain(healthySchedule.id); + expect(schedules.map((schedule) => schedule.id)).not.toContain(orphanSchedule.id); + }); +}); + +describe("cleanupOrphanedSchedules", () => { + test("should delete orphaned schedules and their mirror assignments", async () => { + const healthyVolume = await createTestVolume(); + const healthyRepository = await createTestRepository(); + const healthySchedule = await createTestBackupSchedule({ + volumeId: healthyVolume.id, + repositoryId: healthyRepository.id, + }); + + const orphanVolume = await createTestVolume(); + const orphanRepository = await createTestRepository(); + const orphanSchedule = await createTestBackupSchedule({ + volumeId: orphanVolume.id, + repositoryId: orphanRepository.id, + }); + + const mirrorRepository = await createTestRepository(); + await db.insert(backupScheduleMirrorsTable).values({ + scheduleId: orphanSchedule.id, + repositoryId: mirrorRepository.id, + enabled: true, + }); + + await db.delete(volumesTable).where(eq(volumesTable.id, orphanVolume.id)); + + const cleanupResult = await backupsService.cleanupOrphanedSchedules(); + + expect(cleanupResult.deletedSchedules).toBeGreaterThanOrEqual(1); + + const deletedSchedule = await db.query.backupSchedulesTable.findFirst({ + where: { id: orphanSchedule.id }, + columns: { id: true }, + }); + expect(deletedSchedule).toBeUndefined(); + + const remainingHealthySchedule = await db.query.backupSchedulesTable.findFirst({ + where: { id: healthySchedule.id }, + columns: { id: true }, + }); + expect(remainingHealthySchedule?.id).toBe(healthySchedule.id); + + const orphanMirrors = await db.query.backupScheduleMirrorsTable.findMany({ + where: { scheduleId: orphanSchedule.id }, + columns: { id: true }, + }); + expect(orphanMirrors).toHaveLength(0); + }); +}); diff --git a/app/server/modules/backups/backups.service.ts b/app/server/modules/backups/backups.service.ts index 99cf88a7..d6c7f272 100644 --- a/app/server/modules/backups/backups.service.ts +++ b/app/server/modules/backups/backups.service.ts @@ -1,8 +1,8 @@ -import { and, eq } from "drizzle-orm"; +import { and, eq, inArray } from "drizzle-orm"; import cron from "node-cron"; import { NotFoundError, BadRequestError, ConflictError } from "http-errors-enhanced"; import { db } from "../../db/db"; -import { backupSchedulesTable, backupScheduleMirrorsTable } from "../../db/schema"; +import { backupScheduleMirrorsTable, backupScheduleNotificationsTable, backupSchedulesTable } from "../../db/schema"; import type { CreateBackupScheduleBody, UpdateBackupScheduleBody, UpdateScheduleMirrorsBody } from "./backups.dto"; import { checkMirrorCompatibility, getIncompatibleMirrorError } from "~/server/utils/backend-compatibility"; @@ -17,7 +17,7 @@ const listSchedules = async () => { with: { volume: true, repository: true }, orderBy: { sortOrder: "asc", id: "asc" }, }); - return schedules; + return schedules.filter((schedule) => schedule.volume && schedule.repository); }; const getScheduleById = async (scheduleId: number) => { @@ -31,6 +31,10 @@ const getScheduleById = async (scheduleId: number) => { throw new NotFoundError("Backup schedule not found"); } + if (!schedule.volume || !schedule.repository) { + throw new NotFoundError("Backup schedule not found"); + } + return schedule; }; @@ -45,6 +49,10 @@ const getScheduleByShortId = async (shortId: string) => { throw new NotFoundError("Backup schedule not found"); } + if (!schedule.volume || !schedule.repository) { + throw new NotFoundError("Backup schedule not found"); + } + return schedule; }; @@ -190,6 +198,10 @@ const getScheduleForVolume = async (volumeId: number) => { with: { volume: true, repository: true }, }); + if (schedule && (!schedule.volume || !schedule.repository)) { + return null; + } + return schedule ?? null; }; @@ -331,6 +343,35 @@ const reorderSchedules = async (scheduleIds: number[]) => { }); }; +const cleanupOrphanedSchedules = async () => { + const schedules = await db.query.backupSchedulesTable.findMany({ + with: { volume: true, repository: true }, + columns: { id: true }, + }); + + const orphanScheduleIds = schedules + .filter((schedule) => schedule.volume === null || schedule.repository === null) + .map((schedule) => schedule.id); + + if (orphanScheduleIds.length === 0) { + return { deletedSchedules: 0 }; + } + + db.transaction((tx) => { + tx.delete(backupScheduleNotificationsTable) + .where(inArray(backupScheduleNotificationsTable.scheduleId, orphanScheduleIds)) + .run(); + + tx.delete(backupScheduleMirrorsTable) + .where(inArray(backupScheduleMirrorsTable.scheduleId, orphanScheduleIds)) + .run(); + + tx.delete(backupSchedulesTable).where(inArray(backupSchedulesTable.id, orphanScheduleIds)).run(); + }); + + return { deletedSchedules: orphanScheduleIds.length }; +}; + export const backupsService = { listSchedules, getScheduleById, @@ -343,4 +384,5 @@ export const backupsService = { getMirrorCompatibility, reorderSchedules, getScheduleByShortId, + cleanupOrphanedSchedules, }; diff --git a/app/server/modules/lifecycle/bootstrap.ts b/app/server/modules/lifecycle/bootstrap.ts index 92bb6428..bb8f44ea 100644 --- a/app/server/modules/lifecycle/bootstrap.ts +++ b/app/server/modules/lifecycle/bootstrap.ts @@ -5,7 +5,7 @@ import { startup } from "./startup"; let bootstrapPromise: Promise | undefined; const runBootstrap = async () => { - runDbMigrations(); + await runDbMigrations(); await runMigrations(); await startup(); }; diff --git a/app/server/modules/lifecycle/startup.ts b/app/server/modules/lifecycle/startup.ts index dbe98f49..eb788943 100644 --- a/app/server/modules/lifecycle/startup.ts +++ b/app/server/modules/lifecycle/startup.ts @@ -13,6 +13,7 @@ import { notificationsService } from "../notifications/notifications.service"; import { VolumeAutoRemountJob } from "~/server/jobs/auto-remount"; import { cache } from "~/server/utils/cache"; import { withContext } from "~/server/core/request-context"; +import { backupsService } from "../backups/backups.service"; const ensureLatestConfigurationSchema = async () => { const volumes = await db.query.volumesTable.findMany({}); @@ -54,6 +55,15 @@ export const startup = async () => { await ensureLatestConfigurationSchema(); + const { deletedSchedules } = await backupsService.cleanupOrphanedSchedules().catch((err) => { + logger.error(`Failed to cleanup orphaned backup schedules on startup: ${err.message}`); + return { deletedSchedules: 0 }; + }); + + if (deletedSchedules > 0) { + logger.warn(`Removed ${deletedSchedules} orphaned backup schedule(s) during startup`); + } + const volumes = await db.query.volumesTable.findMany({ where: { OR: [