fix(libsql): cancel scheduled backup jobs on database delete

This commit is contained in:
detail-app[bot] 2026-09-04 02:47:33 +00:00 committed by GitHub
parent 1572008cdf
commit d2f3bc66db
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 319 additions and 0 deletions

View File

@ -0,0 +1,102 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
// Unit coverage for cancelJobs (apps/dokploy/server/utils/backup.ts), the
// helper invoked by every database-service `remove` mutation to tear down the
// orphaned scheduled backup jobs. It had no tests; the libsql `remove`
// mutation now relies on it, so its cloud/non-cloud behavior is locked down
// here.
const state = vi.hoisted(() => ({ cloudFlag: false }));
const mockRemoveScheduleBackup = vi.hoisted(() => vi.fn());
const mockFetch = vi.hoisted(() => vi.fn());
vi.mock("@dokploy/server/index", () => ({
get IS_CLOUD() {
return state.cloudFlag;
},
removeScheduleBackup: mockRemoveScheduleBackup,
}));
import { cancelJobs } from "@/server/utils/backup";
const backup = (overrides: Record<string, unknown> = {}) => ({
backupId: "b-1",
enabled: true,
schedule: "*/5 * * * *",
...overrides,
});
beforeEach(() => {
mockRemoveScheduleBackup.mockReset();
mockFetch.mockReset();
mockFetch.mockResolvedValue({ json: async () => ({ ok: true }) } as never);
vi.stubGlobal("fetch", mockFetch);
state.cloudFlag = false;
});
describe("cancelJobs", () => {
it("non-cloud: cancels each enabled backup's in-process schedule", async () => {
state.cloudFlag = false;
await cancelJobs([
backup({ backupId: "b-1" }),
backup({ backupId: "b-2", enabled: false }),
backup({ backupId: "b-3" }),
] as never);
expect(mockRemoveScheduleBackup).toHaveBeenCalledWith("b-1");
expect(mockRemoveScheduleBackup).toHaveBeenCalledWith("b-3");
expect(mockRemoveScheduleBackup).not.toHaveBeenCalledWith("b-2");
expect(mockFetch).not.toHaveBeenCalled();
});
it("cloud: removes each enabled backup's repeatable job via the schedules API", async () => {
state.cloudFlag = true;
await cancelJobs([
backup({ backupId: "b-1" }),
backup({ backupId: "b-2", enabled: false }),
] as never);
expect(mockFetch).toHaveBeenCalledTimes(1);
const [url, init] = mockFetch.mock.calls[0] ?? [];
expect(url).toContain("/remove-job");
expect(init).toBeDefined();
const body = JSON.parse((init as RequestInit).body as string);
expect(body).toEqual({
type: "backup",
cronSchedule: "*/5 * * * *",
backupId: "b-1",
});
expect(mockRemoveScheduleBackup).not.toHaveBeenCalled();
});
it("skips disabled backups entirely", async () => {
state.cloudFlag = true;
await cancelJobs([backup({ backupId: "b-1", enabled: false })] as never);
expect(mockFetch).not.toHaveBeenCalled();
expect(mockRemoveScheduleBackup).not.toHaveBeenCalled();
});
it("is a no-op for an empty backup list", async () => {
state.cloudFlag = true;
await cancelJobs([] as never);
expect(mockFetch).not.toHaveBeenCalled();
expect(mockRemoveScheduleBackup).not.toHaveBeenCalled();
});
it("cancels every enabled backup in the list, not just the first", async () => {
state.cloudFlag = false;
await cancelJobs([
backup({ backupId: "b-1" }),
backup({ backupId: "b-2", enabled: false }),
backup({ backupId: "b-3" }),
backup({ backupId: "b-4" }),
] as never);
expect(mockRemoveScheduleBackup).toHaveBeenCalledTimes(3);
expect(mockRemoveScheduleBackup).toHaveBeenCalledWith("b-1");
expect(mockRemoveScheduleBackup).toHaveBeenCalledWith("b-3");
expect(mockRemoveScheduleBackup).toHaveBeenCalledWith("b-4");
});
});

View File

@ -0,0 +1,212 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
// Regression coverage for the libsql `remove` mutation. The mutation must,
// like the postgres/mysql/mariadb/mongo peers, fetch the libsql's scheduled
// backups and cancel them before deleting the database row — otherwise the
// backup scheduler is left with orphan jobs (cloud: repeatable bullmq jobs in
// Redis; non-cloud: in-process node-schedule jobs that crash the server on the
// next tick).
//
// The router is exercised directly via a lightweight tRPC stub so the
// mutation logic runs verbatim without loading the real tRPC server (which
// pulls the auth/db graph). Dependencies the mutation touches are mocked.
const stub = vi.hoisted(() => ({
checkServiceAccess: vi.fn(async () => {}),
audit: vi.fn(async () => {}),
findLibsqlById: vi.fn(),
findBackupsByDbId: vi.fn(async (): Promise<any[]> => []),
removeService: vi.fn(async () => {}),
removeLibsqlById: vi.fn(async () => ({})),
cancelJobs: vi.fn(async () => {}),
}));
// The vi.mock factory below is hoisted above imports, so the procedure stub
// it references must also be hoisted. The inner `p` is closed over so the
// builder can chain (input -> mutation/query/subscription).
const tRpcProcedure: any = vi.hoisted(() => {
const p: any = {
input: () => p,
meta: () => p,
query: (h: unknown) => ({ __handler: h }),
mutation: (h: unknown) => ({ __handler: h }),
subscription: (h: unknown) => ({ __handler: h }),
};
return p;
});
vi.mock("@/server/api/trpc", () => ({
createTRPCRouter: (routes: unknown) => routes,
protectedProcedure: tRpcProcedure,
publicProcedure: tRpcProcedure,
}));
vi.mock("@/server/api/utils/audit", () => ({ audit: stub.audit }));
vi.mock("@/server/utils/backup", () => ({ cancelJobs: stub.cancelJobs }));
vi.mock("@/server/db", () => ({ db: {} }));
vi.mock("@dokploy/server", () => ({
checkPortInUse: vi.fn(),
createLibsql: vi.fn(),
createMount: vi.fn(),
deployLibsql: vi.fn(),
findBackupsByDbId: stub.findBackupsByDbId,
findEnvironmentById: vi.fn(),
findLibsqlById: stub.findLibsqlById,
findProjectById: vi.fn(),
getAccessibleServerIds: vi.fn(),
getContainerLogs: vi.fn(),
getWebServerSettings: vi.fn(),
IS_CLOUD: false,
rebuildDatabase: vi.fn(),
removeLibsqlById: stub.removeLibsqlById,
removeService: stub.removeService,
startService: vi.fn(),
startServiceRemote: vi.fn(),
stopService: vi.fn(),
stopServiceRemote: vi.fn(),
updateLibsqlById: vi.fn(),
}));
vi.mock("@dokploy/server/services/permission", () => ({
addNewService: vi.fn(),
checkServiceAccess: stub.checkServiceAccess,
checkServicePermissionAndAccess: vi.fn(),
}));
import { libsqlRouter } from "@/server/api/routers/libsql";
type RemoveHandler = (opts: {
input: { libsqlId: string };
ctx: unknown;
}) => Promise<unknown>;
const removeHandler = (
libsqlRouter as unknown as { remove: { __handler: RemoveHandler } }
).remove.__handler;
const ORG = "org-1";
const ctx = { session: { activeOrganizationId: ORG }, user: { id: "u-1" } };
const libsqlDb = {
libsqlId: "lib-1",
appName: "libsql-app-1",
serverId: null as string | null,
environment: { project: { organizationId: ORG } },
};
beforeEach(() => {
vi.clearAllMocks();
stub.checkServiceAccess.mockResolvedValue(undefined);
stub.audit.mockResolvedValue(undefined);
stub.removeService.mockResolvedValue(undefined);
stub.removeLibsqlById.mockResolvedValue({});
stub.cancelJobs.mockResolvedValue(undefined);
stub.findLibsqlById.mockResolvedValue(libsqlDb);
});
describe("libsql remove mutation — scheduled backup cleanup", () => {
it("fetches the libsql's backups with type 'libsql' and cancels them", async () => {
const backups = [
{
backupId: "b-1",
enabled: true,
schedule: "* * * * *",
libsql: libsqlDb,
},
{
backupId: "b-2",
enabled: false,
schedule: "0 * * * *",
libsql: libsqlDb,
},
];
stub.findBackupsByDbId.mockResolvedValue(backups);
await removeHandler({ input: { libsqlId: "lib-1" }, ctx });
expect(stub.findBackupsByDbId).toHaveBeenCalledWith("lib-1", "libsql");
expect(stub.cancelJobs).toHaveBeenCalledTimes(1);
// Must be the exact backups list returned by findBackupsByDbId
expect(stub.cancelJobs).toHaveBeenCalledWith(backups);
expect(stub.removeService).toHaveBeenCalledWith("libsql-app-1", null);
expect(stub.removeLibsqlById).toHaveBeenCalledWith("lib-1");
});
it("fetches backups BEFORE deleting the libsql row (cascade-safe ordering)", async () => {
// backups.libsqlId is ON DELETE CASCADE, so findBackupsByDbId MUST run
// before removeLibsqlById — otherwise the cascade wipes the rows and
// cancelJobs would receive an empty list (the original bug).
stub.findBackupsByDbId.mockResolvedValue([
{ backupId: "b-1", enabled: true, schedule: "* * * * *" },
]);
await removeHandler({ input: { libsqlId: "lib-1" }, ctx });
expect(stub.findBackupsByDbId).toHaveBeenCalledTimes(1);
expect(stub.removeLibsqlById).toHaveBeenCalledTimes(1);
const findOrder = stub.findBackupsByDbId.mock.invocationCallOrder[0];
const deleteOrder = stub.removeLibsqlById.mock.invocationCallOrder[0];
expect(findOrder).toBeDefined();
expect(deleteOrder).toBeDefined();
expect(findOrder!).toBeLessThan(deleteOrder!);
});
it("mirrors the peer routers: cancelJobs runs between removeService and removeLibsqlById", async () => {
stub.findBackupsByDbId.mockResolvedValue([
{ backupId: "b-1", enabled: true, schedule: "* * * * *" },
]);
await removeHandler({ input: { libsqlId: "lib-1" }, ctx });
const serviceOrder = stub.removeService.mock.invocationCallOrder[0];
const cancelOrder = stub.cancelJobs.mock.invocationCallOrder[0];
const deleteOrder = stub.removeLibsqlById.mock.invocationCallOrder[0];
expect(serviceOrder).toBeDefined();
expect(cancelOrder).toBeDefined();
expect(deleteOrder).toBeDefined();
expect(serviceOrder!).toBeLessThan(cancelOrder!);
expect(cancelOrder!).toBeLessThan(deleteOrder!);
});
it("returns the libsql record and still deletes the row when there are no backups", async () => {
stub.findBackupsByDbId.mockResolvedValue([]);
const result = await removeHandler({ input: { libsqlId: "lib-1" }, ctx });
expect(stub.findBackupsByDbId).toHaveBeenCalledWith("lib-1", "libsql");
// cancelJobs is still invoked (with an empty list) so the cleanup step
// is always present — the divergence that caused this bug would be
// detected as a missing call.
expect(stub.cancelJobs).toHaveBeenCalledWith([]);
expect(stub.removeLibsqlById).toHaveBeenCalledWith("lib-1");
expect(result).toBe(libsqlDb);
});
it("still deletes the libsql row when cancelling jobs throws (cleanup is best-effort)", async () => {
stub.findBackupsByDbId.mockResolvedValue([
{ backupId: "b-1", enabled: true, schedule: "* * * * *" },
]);
stub.cancelJobs.mockRejectedValue(new Error("schedules API down"));
await removeHandler({ input: { libsqlId: "lib-1" }, ctx });
expect(stub.cancelJobs).toHaveBeenCalled();
// The per-operation try/catch swallows the cancelJobs failure so the
// database row is still removed.
expect(stub.removeLibsqlById).toHaveBeenCalledWith("lib-1");
});
it("does not cancel jobs for a libsql owned by another organization", async () => {
stub.findLibsqlById.mockResolvedValue({
...libsqlDb,
environment: { project: { organizationId: "other-org" } },
});
await expect(
removeHandler({ input: { libsqlId: "lib-1" }, ctx }),
).rejects.toThrow(/authorized to delete this Libsql/);
expect(stub.findBackupsByDbId).not.toHaveBeenCalled();
expect(stub.cancelJobs).not.toHaveBeenCalled();
expect(stub.removeLibsqlById).not.toHaveBeenCalled();
});
});

View File

@ -3,6 +3,7 @@ import {
createLibsql,
createMount,
deployLibsql,
findBackupsByDbId,
findEnvironmentById,
findLibsqlById,
findProjectById,
@ -42,6 +43,7 @@ import {
apiUpdateLibsql,
libsql as libsqlTable,
} from "@/server/db/schema";
import { cancelJobs } from "@/server/utils/backup";
export const libsqlRouter = createTRPCRouter({
create: protectedProcedure
.input(apiCreateLibsql)
@ -326,8 +328,11 @@ export const libsqlRouter = createTRPCRouter({
resourceId: libsql.libsqlId,
resourceName: libsql.appName,
});
const backups = await findBackupsByDbId(input.libsqlId, "libsql");
const cleanupOperations = [
async () => await removeService(libsql?.appName, libsql.serverId),
async () => await cancelJobs(backups),
async () => await removeLibsqlById(input.libsqlId),
];