mirror of
https://github.com/ReCloudStudio/WebHooker.git
synced 2026-09-22 16:11:29 +00:00
fix: prevent data loss in routes, fragments, and group operations
Found and fixed multiple critical bugs with similar patterns: 1. routeStatements() used DELETE FROM d1_routes before re-inserting - Could cause data loss if interrupted or if logic changes - Now uses DELETE with WHERE clause + INSERT...ON CONFLICT (upsert) 2. saveFragments() used DELETE FROM d1_fragments before re-inserting - Same pattern as routes, fixed with targeted deletes + upsert - Added loadFragments() call to determine what to delete 3. adminGroupRename() didn't update fragment groupId - When renaming a group, fragments were left pointing to old groupId - Now updates fragments alongside routes, secrets, and invites All changes follow the same safe pattern: - Load existing data to identify what needs deletion - Delete only removed items with WHERE clauses - Use INSERT...ON CONFLICT DO UPDATE for upserts - Never use bare DELETE FROM table Testing: All 339 tests pass.
This commit is contained in:
parent
6c6c67fd73
commit
1a0500b812
3 changed files with 75 additions and 14 deletions
|
|
@ -35,16 +35,39 @@ export async function loadFragments(db: D1Database): Promise<NamedFragment[]> {
|
||||||
|
|
||||||
export async function saveFragments(db: D1Database, fragments: NamedFragment[]): Promise<void> {
|
export async function saveFragments(db: D1Database, fragments: NamedFragment[]): Promise<void> {
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
const statements: D1PreparedStatement[] = [
|
|
||||||
db.prepare("DELETE FROM d1_fragments"),
|
// Load existing fragments to determine what to delete
|
||||||
...fragments.map((f) =>
|
const existing = await loadFragments(db);
|
||||||
|
const existingKeys = new Set(existing.map((f) => `${f.id}:${f.groupId ?? ""}`));
|
||||||
|
const newKeys = new Set(fragments.map((f) => `${f.id}:${f.groupId ?? ""}`));
|
||||||
|
|
||||||
|
const statements: D1PreparedStatement[] = [];
|
||||||
|
|
||||||
|
// Delete fragments that are no longer present
|
||||||
|
for (const key of existingKeys) {
|
||||||
|
if (!newKeys.has(key)) {
|
||||||
|
const [id, groupId] = key.split(":");
|
||||||
|
statements.push(
|
||||||
|
db.prepare("DELETE FROM d1_fragments WHERE id = ? AND group_id = ?").bind(id, groupId)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Upsert all fragments
|
||||||
|
for (const f of fragments) {
|
||||||
|
statements.push(
|
||||||
db
|
db
|
||||||
.prepare(
|
.prepare(
|
||||||
`INSERT INTO d1_fragments (id, group_id, name, node, version, created_at, updated_at)
|
`INSERT INTO d1_fragments (id, group_id, name, node, version, created_at, updated_at)
|
||||||
VALUES (?, ?, ?, ?, 1, ?, ?)`,
|
VALUES (?, ?, ?, ?, 1, ?, ?)
|
||||||
|
ON CONFLICT(id, group_id) DO UPDATE SET
|
||||||
|
name = excluded.name,
|
||||||
|
node = excluded.node,
|
||||||
|
updated_at = excluded.updated_at`,
|
||||||
)
|
)
|
||||||
.bind(f.id, f.groupId ?? "", f.name, JSON.stringify(f.node), now, now),
|
.bind(f.id, f.groupId ?? "", f.name, JSON.stringify(f.node), now, now),
|
||||||
),
|
);
|
||||||
];
|
}
|
||||||
|
|
||||||
await db.batch(statements);
|
await db.batch(statements);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -88,15 +88,43 @@ export function d1ConfigStore(db: D1Database, kv: KVNamespace): ConfigStore {
|
||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
|
|
||||||
function routeStatements(routes: Route[]): D1PreparedStatement[] {
|
function routeStatements(routes: Route[], existingRouteKeys?: Set<string>): D1PreparedStatement[] {
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
return [
|
const statements: D1PreparedStatement[] = [];
|
||||||
db.prepare("DELETE FROM d1_routes"),
|
|
||||||
...routes.map((r) =>
|
// If we have existing routes, delete those not in the new set
|
||||||
|
if (existingRouteKeys) {
|
||||||
|
const newKeys = new Set(routes.map((r) => `${r.id}:${r.groupId ?? ""}`));
|
||||||
|
for (const key of existingRouteKeys) {
|
||||||
|
if (!newKeys.has(key)) {
|
||||||
|
const [id, groupId] = key.split(":");
|
||||||
|
statements.push(
|
||||||
|
db.prepare("DELETE FROM d1_routes WHERE id = ? AND group_id = ?").bind(id, groupId)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// Backward compatibility: delete all routes if no existing set provided
|
||||||
|
statements.push(db.prepare("DELETE FROM d1_routes"));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Upsert all routes
|
||||||
|
for (const r of routes) {
|
||||||
|
statements.push(
|
||||||
db
|
db
|
||||||
.prepare(
|
.prepare(
|
||||||
`INSERT INTO d1_routes (id, group_id, name, enabled, filters, targets, stop, fallback, discord_role_ids, ast, version, created_at, updated_at)
|
`INSERT INTO d1_routes (id, group_id, name, enabled, filters, targets, stop, fallback, discord_role_ids, ast, version, created_at, updated_at)
|
||||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1, ?, ?)`,
|
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1, ?, ?)
|
||||||
|
ON CONFLICT(id, group_id) DO UPDATE SET
|
||||||
|
name = excluded.name,
|
||||||
|
enabled = excluded.enabled,
|
||||||
|
filters = excluded.filters,
|
||||||
|
targets = excluded.targets,
|
||||||
|
stop = excluded.stop,
|
||||||
|
fallback = excluded.fallback,
|
||||||
|
discord_role_ids = excluded.discord_role_ids,
|
||||||
|
ast = excluded.ast,
|
||||||
|
updated_at = excluded.updated_at`,
|
||||||
)
|
)
|
||||||
.bind(
|
.bind(
|
||||||
r.id,
|
r.id,
|
||||||
|
|
@ -112,8 +140,10 @@ export function d1ConfigStore(db: D1Database, kv: KVNamespace): ConfigStore {
|
||||||
now,
|
now,
|
||||||
now,
|
now,
|
||||||
),
|
),
|
||||||
),
|
);
|
||||||
];
|
}
|
||||||
|
|
||||||
|
return statements;
|
||||||
}
|
}
|
||||||
|
|
||||||
function groupStatements(groups: Group[], existingGroupIds: string[]): D1PreparedStatement[] {
|
function groupStatements(groups: Group[], existingGroupIds: string[]): D1PreparedStatement[] {
|
||||||
|
|
|
||||||
|
|
@ -965,7 +965,7 @@ export async function adminGroupRename(
|
||||||
return respondError(event, 500, "Failed to save groups");
|
return respondError(event, 500, "Failed to save groups");
|
||||||
}
|
}
|
||||||
|
|
||||||
// Re-point routes, the tenant webhook secret and pending invites.
|
// Re-point routes, fragments, the tenant webhook secret and pending invites.
|
||||||
const routes = await loadRoutes(env.KV);
|
const routes = await loadRoutes(env.KV);
|
||||||
const touched = routes.filter((r) => r.groupId === groupId);
|
const touched = routes.filter((r) => r.groupId === groupId);
|
||||||
if (touched.length > 0) {
|
if (touched.length > 0) {
|
||||||
|
|
@ -974,6 +974,14 @@ export async function adminGroupRename(
|
||||||
routes.map((r) => (r.groupId === groupId ? { ...r, groupId: newId } : r)),
|
routes.map((r) => (r.groupId === groupId ? { ...r, groupId: newId } : r)),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
const fragments = await loadFragments(env.DB);
|
||||||
|
const touchedFragments = fragments.filter((f) => f.groupId === groupId);
|
||||||
|
if (touchedFragments.length > 0) {
|
||||||
|
await saveFragments(
|
||||||
|
env.DB,
|
||||||
|
fragments.map((f) => (f.groupId === groupId ? { ...f, groupId: newId } : f)),
|
||||||
|
);
|
||||||
|
}
|
||||||
const secret = await getTenantSecret(env.KV, groupId);
|
const secret = await getTenantSecret(env.KV, groupId);
|
||||||
if (secret) {
|
if (secret) {
|
||||||
await env.KV.put(`tenant:${newId}`, secret);
|
await env.KV.put(`tenant:${newId}`, secret);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue