From 712cd6bece9acdb7f98496e459bc99a02544cfce Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Tue, 28 Jul 2026 06:48:49 +0700 Subject: [PATCH] fix(backend): delete migration losers before winner rewrite --- backend/store.go | 17 ++++++++------- backend/store_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/backend/store.go b/backend/store.go index 443d4e7..1684446 100644 --- a/backend/store.go +++ b/backend/store.go @@ -192,22 +192,23 @@ func migrateAsuraKeys(db *sql.DB) error { winner = i } } + // Losers go first: rewriting the winner to the stripped key while a + // pre-existing hashless row still holds it is a primary-key collision. for i, r := range g { if i == winner { - if r.id == stripped { - continue // already stable - } - if _, err := db.Exec( - `UPDATE bookmarks SET key = ?, series_id = ? WHERE key = ?`, - "asura:"+stripped, stripped, r.key); err != nil { - return fmt.Errorf("rewrite key %q: %w", r.key, err) - } continue } if _, err := db.Exec(`DELETE FROM bookmarks WHERE key = ?`, r.key); err != nil { return fmt.Errorf("drop duplicate %q: %w", r.key, err) } } + if r := g[winner]; r.id != stripped { + if _, err := db.Exec( + `UPDATE bookmarks SET key = ?, series_id = ? WHERE key = ?`, + "asura:"+stripped, stripped, r.key); err != nil { + return fmt.Errorf("rewrite key %q: %w", r.key, err) + } + } } return nil } diff --git a/backend/store_test.go b/backend/store_test.go index e842461..3c1e231 100644 --- a/backend/store_test.go +++ b/backend/store_test.go @@ -983,3 +983,54 @@ func TestOpenStoreMigratesAsuraBuildHashKeys(t *testing.T) { } third.Close() } + +// A hashed row and a pre-existing hashless row of the same series collide on +// the stripped key. The winner rewrite must happen only after the loser is +// gone, or the UPDATE hits a primary-key collision and OpenStore fails. +func TestOpenStoreMigratesAsuraHashlessCollision(t *testing.T) { + dbPath := filepath.Join(t.TempDir(), "collision.db") + + store, err := OpenStore(dbPath) + if err != nil { + t.Fatalf("open: %v", err) + } + seed := []Bookmark{ + {Key: "asura:overgeared", Site: "asura", SeriesID: "overgeared", + Title: "Hashless", LastChapterNum: 10, UpdatedAt: 100}, + {Key: "asura:overgeared-059befe1", Site: "asura", + SeriesID: "overgeared-059befe1", Title: "Hashed newer", + LastChapterNum: 20, UpdatedAt: 200}, + } + for _, b := range seed { + if _, err := store.Upsert(b); err != nil { + t.Fatalf("seed %s: %v", b.Key, err) + } + } + if err := store.Close(); err != nil { + t.Fatalf("close: %v", err) + } + + reopened, err := OpenStore(dbPath) + if err != nil { + t.Fatalf("reopen: %v", err) + } + t.Cleanup(func() { reopened.Close() }) + + list, err := reopened.List() + if err != nil { + t.Fatalf("List: %v", err) + } + if len(list) != 1 { + t.Fatalf("want 1 row after merge, got %d: %+v", len(list), list) + } + merged := list[0] + if merged.Key != "asura:overgeared" { + t.Fatalf("merged key = %q, want asura:overgeared", merged.Key) + } + if merged.SeriesID != "overgeared" { + t.Fatalf("series_id = %q, want overgeared", merged.SeriesID) + } + if merged.Title != "Hashed newer" || merged.LastChapterNum != 20 || merged.UpdatedAt != 200 { + t.Fatalf("merge kept wrong row: %+v", merged) + } +}