Skip to content

Commit 3741117

Browse files
committed
fix(sponsor-panel): merge split github id columns
GORM derives the column name git_hub_id from the field name GitHubID, but the schema has always called that column github_id. An early AutoMigrate added a second column, so Create wrote git_hub_id while upsertUser looked up github_id. Every user who registered after that point missed the lookup on each later login, inserted a duplicate row, and violated the unique index users_github_id_key. 32 of 90 accounts could not refresh their sponsorship data. Pin the column name in the model, match on the struct field instead of a literal column name, and merge the two columns at startup. Assisted-by: Claude Opus 5 via Claude Code Signed-off-by: Xe Iaso <me@xeiaso.net>
1 parent 652b1ec commit 3741117

4 files changed

Lines changed: 382 additions & 3 deletions

File tree

cmd/sponsor-panel/main.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,12 @@ func main() {
173173

174174
slog.Info("main: database connection established")
175175

176+
// Merge the split GitHub ID columns before AutoMigrate inspects them.
177+
if err := consolidateGitHubIDColumn(db); err != nil {
178+
slog.Error("failed to consolidate GitHub ID columns", "err", err)
179+
os.Exit(1)
180+
}
181+
176182
// Run GORM AutoMigrate
177183
slog.Debug("main: running GORM auto-migration")
178184
if err := db.AutoMigrate(PanelModels()...); err != nil {
@@ -181,6 +187,11 @@ func main() {
181187
}
182188
slog.Info("main: auto-migration completed")
183189

190+
if err := dropEmailUniqueIndex(db); err != nil {
191+
slog.Error("failed to drop email unique index", "err", err)
192+
os.Exit(1)
193+
}
194+
184195
// Start sponsor sync loop in background
185196
syncCtx, syncCancel := context.WithCancel(context.Background())
186197
defer syncCancel()
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
package main
2+
3+
import (
4+
"os"
5+
"strings"
6+
"testing"
7+
8+
"gorm.io/driver/postgres"
9+
"gorm.io/gorm"
10+
"gorm.io/gorm/logger"
11+
)
12+
13+
// productionSchema recreates the users table as it stood before the GitHub ID
14+
// columns were merged: a legacy github_id column plus the git_hub_id column
15+
// that GORM derived from the field name.
16+
const productionSchema = `
17+
DROP TABLE IF EXISTS logo_submissions;
18+
DROP TABLE IF EXISTS users;
19+
20+
CREATE TABLE users (
21+
id serial PRIMARY KEY,
22+
github_id bigint,
23+
login text NOT NULL,
24+
avatar_url text,
25+
name text,
26+
email text,
27+
sponsorship_data jsonb,
28+
last_sponsorship_check timestamp,
29+
created_at timestamp,
30+
updated_at timestamp,
31+
patreon_id text,
32+
provider text NOT NULL DEFAULT 'github',
33+
git_hub_id bigint,
34+
thoth_user_id text
35+
);
36+
37+
CREATE UNIQUE INDEX idx_users_provider_login ON users (provider, login);
38+
CREATE UNIQUE INDEX users_github_id_key ON users (git_hub_id);
39+
CREATE UNIQUE INDEX users_patreon_id_key ON users (patreon_id);
40+
41+
INSERT INTO users (github_id, git_hub_id, patreon_id, login, provider) VALUES
42+
(1111, NULL, NULL, 'legacy-only', 'github'),
43+
(NULL, 2222, NULL, 'gorm-only', 'github'),
44+
(NULL, NULL, 'p1', 'patreon-only', 'patreon');
45+
`
46+
47+
// testDB connects to a throwaway PostgreSQL instance. Set SPONSOR_PANEL_TEST_DSN
48+
// to run the migration tests.
49+
func testDB(t *testing.T) *gorm.DB {
50+
t.Helper()
51+
52+
dsn := os.Getenv("SPONSOR_PANEL_TEST_DSN")
53+
if dsn == "" {
54+
t.Skip("SPONSOR_PANEL_TEST_DSN is not set")
55+
}
56+
57+
db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{Logger: logger.Discard})
58+
if err != nil {
59+
t.Fatalf("gorm.Open: %v", err)
60+
}
61+
62+
if err := db.Exec(productionSchema).Error; err != nil {
63+
t.Fatalf("cannot create the test schema: %v", err)
64+
}
65+
66+
return db
67+
}
68+
69+
// columnNames returns the columns of the users table.
70+
func columnNames(t *testing.T, db *gorm.DB) map[string]bool {
71+
t.Helper()
72+
73+
var names []string
74+
err := db.Raw(`SELECT column_name FROM information_schema.columns
75+
WHERE table_name = 'users' AND table_schema = current_schema()`).Scan(&names).Error
76+
if err != nil {
77+
t.Fatalf("cannot read the users columns: %v", err)
78+
}
79+
80+
out := map[string]bool{}
81+
for _, n := range names {
82+
out[n] = true
83+
}
84+
return out
85+
}
86+
87+
// TestConsolidateGitHubIDColumn verifies the merge against a real database.
88+
func TestConsolidateGitHubIDColumn(t *testing.T) {
89+
db := testDB(t)
90+
91+
if err := consolidateGitHubIDColumn(db); err != nil {
92+
t.Fatalf("consolidateGitHubIDColumn: %v", err)
93+
}
94+
95+
cols := columnNames(t, db)
96+
if cols["git_hub_id"] {
97+
t.Error("users.git_hub_id still exists after the merge")
98+
}
99+
if !cols["github_id"] {
100+
t.Fatal("users.github_id is missing after the merge")
101+
}
102+
103+
for _, tt := range []struct {
104+
login string
105+
want int64
106+
}{
107+
{login: "legacy-only", want: 1111},
108+
{login: "gorm-only", want: 2222},
109+
} {
110+
t.Run(tt.login, func(t *testing.T) {
111+
var got int64
112+
if err := db.Raw(`SELECT github_id FROM users WHERE login = ?`, tt.login).Scan(&got).Error; err != nil {
113+
t.Fatalf("cannot read github_id: %v", err)
114+
}
115+
if got != tt.want {
116+
t.Errorf("github_id for %s = %d, want %d", tt.login, got, tt.want)
117+
}
118+
})
119+
}
120+
121+
// The unique index must follow the column, or duplicate accounts return.
122+
var indexed string
123+
err := db.Raw(`SELECT indexdef FROM pg_indexes
124+
WHERE tablename = 'users' AND indexname = 'users_github_id_key'`).Scan(&indexed).Error
125+
if err != nil {
126+
t.Fatalf("cannot read the index definition: %v", err)
127+
}
128+
if indexed == "" {
129+
t.Fatal("users_github_id_key is missing after the merge")
130+
}
131+
if !strings.Contains(indexed, "(github_id)") {
132+
t.Errorf("users_github_id_key = %q, want an index on github_id", indexed)
133+
}
134+
135+
// AutoMigrate must not re-create the derived column.
136+
if err := db.AutoMigrate(PanelModels()...); err != nil {
137+
t.Fatalf("AutoMigrate: %v", err)
138+
}
139+
if columnNames(t, db)["git_hub_id"] {
140+
t.Error("AutoMigrate re-created users.git_hub_id")
141+
}
142+
143+
// A second run must do nothing.
144+
if err := consolidateGitHubIDColumn(db); err != nil {
145+
t.Fatalf("consolidateGitHubIDColumn is not idempotent: %v", err)
146+
}
147+
}
148+
149+
// TestUpsertUserFindsMigratedRows verifies that a login after the merge updates
150+
// the existing row instead of inserting a duplicate.
151+
func TestUpsertUserFindsMigratedRows(t *testing.T) {
152+
db := testDB(t)
153+
154+
if err := consolidateGitHubIDColumn(db); err != nil {
155+
t.Fatalf("consolidateGitHubIDColumn: %v", err)
156+
}
157+
if err := db.AutoMigrate(PanelModels()...); err != nil {
158+
t.Fatalf("AutoMigrate: %v", err)
159+
}
160+
161+
var before int64
162+
db.Model(&PanelUser{}).Count(&before)
163+
164+
for _, tt := range []struct {
165+
name string
166+
id int64
167+
login string
168+
}{
169+
{name: "row migrated from the legacy column", id: 1111, login: "legacy-only"},
170+
{name: "row written by GORM", id: 2222, login: "gorm-only"},
171+
} {
172+
t.Run(tt.name, func(t *testing.T) {
173+
user := &PanelUser{
174+
GitHubID: &tt.id,
175+
Login: tt.login,
176+
Name: "updated",
177+
SponsorshipData: `{"is_active":true,"monthly_amount_cents":5000}`,
178+
}
179+
if err := upsertUser(db, user); err != nil {
180+
t.Fatalf("upsertUser: %v", err)
181+
}
182+
if user.Name != "updated" {
183+
t.Errorf("Name = %q, want the update to survive", user.Name)
184+
}
185+
})
186+
}
187+
188+
var after int64
189+
db.Model(&PanelUser{}).Count(&after)
190+
if after != before {
191+
t.Errorf("user count = %d, want %d; upsertUser inserted duplicates", after, before)
192+
}
193+
}

cmd/sponsor-panel/models.go

Lines changed: 111 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,20 @@ package main
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"log/slog"
7+
"strings"
68
"time"
79

810
"gorm.io/gorm"
911
)
1012

1113
// PanelUser represents an authenticated user with cached sponsorship data.
1214
type PanelUser struct {
13-
ID uint `json:"id" gorm:"primaryKey"`
14-
GitHubID *int64 `json:"github_id" gorm:"uniqueIndex:users_github_id_key"`
15+
ID uint `json:"id" gorm:"primaryKey"`
16+
// The column tag is mandatory. GORM derives git_hub_id from the field
17+
// name, which does not match the column the schema has always used.
18+
GitHubID *int64 `json:"github_id" gorm:"column:github_id;uniqueIndex:users_github_id_key"`
1519
PatreonID *string `json:"patreon_id" gorm:"uniqueIndex:users_patreon_id_key"`
1620
Provider string `json:"provider" gorm:"not null;default:'github';uniqueIndex:idx_users_provider_login"`
1721
Login string `json:"login" gorm:"not null;uniqueIndex:idx_users_provider_login"`
@@ -87,6 +91,104 @@ func PanelModels() []any {
8791
}
8892
}
8993

94+
// consolidateGitHubIDColumn merges the legacy users.git_hub_id column back
95+
// into users.github_id.
96+
//
97+
// The schema has always called this column github_id. GORM derives git_hub_id
98+
// from the field name GitHubID, so an early AutoMigrate added a second column
99+
// and writes went to one column while reads went to the other. Move the values
100+
// across, then take the original name back.
101+
//
102+
// Run this before AutoMigrate. It is a no-op once the columns are merged.
103+
func consolidateGitHubIDColumn(db *gorm.DB) error {
104+
m := db.Migrator()
105+
106+
if !m.HasTable(&PanelUser{}) {
107+
return nil
108+
}
109+
110+
if !m.HasColumn(&PanelUser{}, "git_hub_id") {
111+
return nil
112+
}
113+
114+
hasLegacy := m.HasColumn(&PanelUser{}, "github_id")
115+
116+
return db.Transaction(func(tx *gorm.DB) error {
117+
if hasLegacy {
118+
res := tx.Exec(`UPDATE users SET git_hub_id = github_id WHERE git_hub_id IS NULL AND github_id IS NOT NULL`)
119+
if res.Error != nil {
120+
return fmt.Errorf("cannot backfill users.git_hub_id: %w", res.Error)
121+
}
122+
slog.Info("consolidateGitHubIDColumn: backfilled GitHub IDs", "rows", res.RowsAffected)
123+
124+
if err := tx.Exec(`ALTER TABLE users DROP COLUMN github_id`).Error; err != nil {
125+
return fmt.Errorf("cannot drop the legacy users.github_id column: %w", err)
126+
}
127+
}
128+
129+
if err := tx.Exec(`ALTER TABLE users RENAME COLUMN git_hub_id TO github_id`).Error; err != nil {
130+
return fmt.Errorf("cannot rename users.git_hub_id to github_id: %w", err)
131+
}
132+
133+
slog.Info("consolidateGitHubIDColumn: users.git_hub_id is now users.github_id")
134+
return nil
135+
})
136+
}
137+
138+
// emailUniqueIndex describes a legacy unique index on users(email).
139+
type emailUniqueIndex struct {
140+
IndexName string
141+
ConstraintName string
142+
}
143+
144+
// dropEmailUniqueIndex removes any legacy unique index on users(email).
145+
//
146+
// AutoMigrate never drops indexes, so a unique index created by an older
147+
// schema stays in the database forever. Look the index up in the catalog
148+
// because the name depends on how the old schema declared it.
149+
func dropEmailUniqueIndex(db *gorm.DB) error {
150+
const findQuery = `
151+
SELECT i.relname AS index_name, COALESCE(c.conname, '') AS constraint_name
152+
FROM pg_index x
153+
JOIN pg_class i ON i.oid = x.indexrelid
154+
JOIN pg_class t ON t.oid = x.indrelid
155+
JOIN pg_namespace n ON n.oid = t.relnamespace
156+
LEFT JOIN pg_constraint c ON c.conindid = x.indexrelid
157+
WHERE t.relname = 'users'
158+
AND n.nspname = current_schema()
159+
AND x.indisunique
160+
AND x.indnatts = 1
161+
AND (SELECT a.attname FROM pg_attribute a WHERE a.attrelid = t.oid AND a.attnum = x.indkey[0]) = 'email'`
162+
163+
var found []emailUniqueIndex
164+
if err := db.Raw(findQuery).Scan(&found).Error; err != nil {
165+
return fmt.Errorf("cannot look up unique indexes on users(email): %w", err)
166+
}
167+
168+
for _, idx := range found {
169+
var stmt string
170+
switch {
171+
case idx.ConstraintName != "":
172+
stmt = fmt.Sprintf("ALTER TABLE users DROP CONSTRAINT %s", quoteIdent(idx.ConstraintName))
173+
default:
174+
stmt = fmt.Sprintf("DROP INDEX %s", quoteIdent(idx.IndexName))
175+
}
176+
177+
if err := db.Exec(stmt).Error; err != nil {
178+
return fmt.Errorf("cannot drop unique index %s on users(email): %w", idx.IndexName, err)
179+
}
180+
181+
slog.Info("dropEmailUniqueIndex: dropped unique index on users(email)", "index", idx.IndexName, "constraint", idx.ConstraintName)
182+
}
183+
184+
return nil
185+
}
186+
187+
// quoteIdent quotes a SQL identifier for PostgreSQL.
188+
func quoteIdent(name string) string {
189+
return `"` + strings.ReplaceAll(name, `"`, `""`) + `"`
190+
}
191+
90192
// --- DB helper functions (GORM) ---
91193

92194
// getUserByID retrieves a user by ID from the database.
@@ -101,8 +203,14 @@ func getUserByID(db *gorm.DB, userID int) (*PanelUser, error) {
101203

102204
// upsertUser creates or updates a GitHub user in the database.
103205
func upsertUser(db *gorm.DB, user *PanelUser) error {
206+
if user.GitHubID == nil {
207+
return fmt.Errorf("upsertUser: user %q has no GitHub ID", user.Login)
208+
}
209+
104210
var existing PanelUser
105-
result := db.Where("github_id = ?", user.GitHubID).First(&existing)
211+
// Match on the struct field, not a literal column name. The field decides
212+
// which column both this query and Create use, so the two cannot drift.
213+
result := db.Where(&PanelUser{GitHubID: user.GitHubID}).First(&existing)
106214
if result.Error == nil {
107215
// Update existing
108216
existing.Login = user.Login

0 commit comments

Comments
 (0)