diff --git a/server/lib/storage/config-store.ts b/server/lib/storage/config-store.ts index 61e9da4..24cfcbc 100644 --- a/server/lib/storage/config-store.ts +++ b/server/lib/storage/config-store.ts @@ -116,19 +116,40 @@ export function d1ConfigStore(db: D1Database, kv: KVNamespace): ConfigStore { ]; } - function groupStatements(groups: Group[]): D1PreparedStatement[] { + function groupStatements(groups: Group[], existingGroupIds: string[]): D1PreparedStatement[] { const now = Date.now(); - return [ - db.prepare("DELETE FROM d1_groups"), - ...groups.map((g) => + const newGroupIds = new Set(groups.map((g) => g.id)); + const toDelete = existingGroupIds.filter((id) => !newGroupIds.has(id)); + + const statements: D1PreparedStatement[] = []; + + // First, delete routes for groups that will be removed + for (const groupId of toDelete) { + statements.push(db.prepare("DELETE FROM d1_routes WHERE group_id = ?").bind(groupId)); + } + + // Then delete the groups themselves + for (const groupId of toDelete) { + statements.push(db.prepare("DELETE FROM d1_groups WHERE id = ?").bind(groupId)); + } + + // Finally, upsert all groups (INSERT OR REPLACE) + for (const g of groups) { + statements.push( db .prepare( `INSERT INTO d1_groups (id, name, data, version, created_at, updated_at) - VALUES (?, ?, ?, 1, ?, ?)`, + VALUES (?, ?, ?, 1, ?, ?) + ON CONFLICT(id) DO UPDATE SET + name = excluded.name, + data = excluded.data, + updated_at = excluded.updated_at`, ) .bind(g.id, g.name, JSON.stringify(g), now, now), - ), - ]; + ); + } + + return statements; } async function seedRoutesToD1(routes: Route[]): Promise { @@ -138,7 +159,8 @@ export function d1ConfigStore(db: D1Database, kv: KVNamespace): ConfigStore { async function seedGroupsToD1(groups: Group[]): Promise { if (groups.length === 0) return; - await db.batch(groupStatements(groups)); + // When seeding, there are no existing groups to delete + await db.batch(groupStatements(groups, [])); } async function syncRoutesToKV(routes: Route[], ttl: number): Promise { @@ -225,7 +247,10 @@ export function d1ConfigStore(db: D1Database, kv: KVNamespace): ConfigStore { async saveGroups(groups: Group[]): Promise { try { - await db.batch(groupStatements(groups)); + // Load existing group IDs to properly handle deletions + const existing = await loadGroupsFromD1(); + const existingIds = existing.map((g) => g.id); + await db.batch(groupStatements(groups, existingIds)); await syncGroupsToKV(groups, KV_CACHE_TTL); } catch (err) { log.warn({ err }, "D1 groups unavailable, falling back to KV"); diff --git a/tests/config-store.test.ts b/tests/config-store.test.ts index 04bf7d5..0d6b5f6 100644 --- a/tests/config-store.test.ts +++ b/tests/config-store.test.ts @@ -171,4 +171,29 @@ describe("d1ConfigStore", () => { const second = await cfg.loadRoutes(); expect(second).toHaveLength(2); }); + + it("updating groups does not cascade-delete routes", async () => { + const { db, routesTable, groupsTable } = createDB(); + const { kv } = createKV(); + const cfg: ConfigStore = d1ConfigStore(db, kv); + + // Setup initial state with groups and routes + groupsTable.push(group("g1"), group("g2")); + routesTable.push(route("r1", "g1"), route("r2", "g2")); + + // Simulate updating groups (e.g., changing a group's name) + const updatedGroups = [ + { ...group("g1"), name: "Updated Group 1" }, + group("g2"), + ]; + + // This should not delete routes + await cfg.saveGroups(updatedGroups); + + // Routes should still exist in the table + // Note: In the fake DB, batch() doesn't actually execute the statements, + // so we can't verify the actual deletion behavior here. + // This test mainly ensures saveGroups doesn't throw an error. + expect(routesTable).toHaveLength(2); + }); });