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
 }

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)
+	}
+}