mirror of
https://github.com/ReCloudStudio/WebHooker.git
synced 2026-09-22 16:11:29 +00:00
fix: prevent cascade deletion of routes when updating groups
Previously, saveGroups() used DELETE FROM d1_groups followed by re-insertion, which triggered ON DELETE CASCADE and wiped all routes whenever any group was updated. Changes: - Use INSERT...ON CONFLICT DO UPDATE (upsert) instead of delete-then-insert - Only delete groups that are actually being removed - Delete routes first, then delete groups (proper cascade order) - Load existing group IDs before saving to detect deletions This fixes the critical bug where updating one group's metadata would delete all routes across all groups. Data recovery: Used D1 Time Travel to restore from bookmark 000018f9-00000002-000050de-f23b7bfe5ccdc00e9c9dec3a4de8bc81 (before the problematic group update), recovering 18 routes.
This commit is contained in:
parent
fd6ccda411
commit
b313cdc2fe
2 changed files with 59 additions and 9 deletions
|
|
@ -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();
|
const now = Date.now();
|
||||||
return [
|
const newGroupIds = new Set(groups.map((g) => g.id));
|
||||||
db.prepare("DELETE FROM d1_groups"),
|
const toDelete = existingGroupIds.filter((id) => !newGroupIds.has(id));
|
||||||
...groups.map((g) =>
|
|
||||||
|
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
|
db
|
||||||
.prepare(
|
.prepare(
|
||||||
`INSERT INTO d1_groups (id, name, data, version, created_at, updated_at)
|
`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),
|
.bind(g.id, g.name, JSON.stringify(g), now, now),
|
||||||
),
|
);
|
||||||
];
|
}
|
||||||
|
|
||||||
|
return statements;
|
||||||
}
|
}
|
||||||
|
|
||||||
async function seedRoutesToD1(routes: Route[]): Promise<void> {
|
async function seedRoutesToD1(routes: Route[]): Promise<void> {
|
||||||
|
|
@ -138,7 +159,8 @@ export function d1ConfigStore(db: D1Database, kv: KVNamespace): ConfigStore {
|
||||||
|
|
||||||
async function seedGroupsToD1(groups: Group[]): Promise<void> {
|
async function seedGroupsToD1(groups: Group[]): Promise<void> {
|
||||||
if (groups.length === 0) return;
|
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<void> {
|
async function syncRoutesToKV(routes: Route[], ttl: number): Promise<void> {
|
||||||
|
|
@ -225,7 +247,10 @@ export function d1ConfigStore(db: D1Database, kv: KVNamespace): ConfigStore {
|
||||||
|
|
||||||
async saveGroups(groups: Group[]): Promise<void> {
|
async saveGroups(groups: Group[]): Promise<void> {
|
||||||
try {
|
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);
|
await syncGroupsToKV(groups, KV_CACHE_TTL);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
log.warn({ err }, "D1 groups unavailable, falling back to KV");
|
log.warn({ err }, "D1 groups unavailable, falling back to KV");
|
||||||
|
|
|
||||||
|
|
@ -171,4 +171,29 @@ describe("d1ConfigStore", () => {
|
||||||
const second = await cfg.loadRoutes();
|
const second = await cfg.loadRoutes();
|
||||||
expect(second).toHaveLength(2);
|
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);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue