-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[management] add domain and service cleanup migration #5850
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -441,3 +441,197 @@ func TestRemoveDuplicatePeerKeys_NoTable(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := migration.RemoveDuplicatePeerKeys(context.Background(), db) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err, "Should not fail when table does not exist") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type testParent struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ID string `gorm:"primaryKey"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (testParent) TableName() string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "test_parents" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type testChild struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ID string `gorm:"primaryKey"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ParentID string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (testChild) TableName() string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "test_children" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type testChildWithFK struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ID string `gorm:"primaryKey"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ParentID string `gorm:"index"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Parent *testParent `gorm:"foreignKey:ParentID"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (testChildWithFK) TableName() string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "test_children" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func setupOrphanTestDB(t *testing.T, models ...any) *gorm.DB { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.Helper() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db := setupDatabase(t) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, m := range models { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ = db.Migrator().DropTable(m) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := db.AutoMigrate(models...) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err, "Failed to auto-migrate tables") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return db | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestCleanupOrphanedResources_NoChildTable(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db := setupDatabase(t) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ = db.Migrator().DropTable(&testChild{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ = db.Migrator().DropTable(&testParent{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := migration.CleanupOrphanedResources[testChild, testParent](context.Background(), db, "parent_id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err, "Should not fail when child table does not exist") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestCleanupOrphanedResources_NoParentTable(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db := setupDatabase(t) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ = db.Migrator().DropTable(&testParent{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ = db.Migrator().DropTable(&testChild{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := db.AutoMigrate(&testChild{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err = migration.CleanupOrphanedResources[testChild, testParent](context.Background(), db, "parent_id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err, "Should not fail when parent table does not exist") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestCleanupOrphanedResources_EmptyTables(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db := setupOrphanTestDB(t, &testParent{}, &testChild{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := migration.CleanupOrphanedResources[testChild, testParent](context.Background(), db, "parent_id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err, "Should not fail on empty tables") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var count int64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.Model(&testChild{}).Count(&count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, int64(0), count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestCleanupOrphanedResources_NoOrphans(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db := setupOrphanTestDB(t, &testParent{}, &testChild{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testParent{ID: "p1"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testParent{ID: "p2"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testChild{ID: "c1", ParentID: "p1"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testChild{ID: "c2", ParentID: "p2"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := migration.CleanupOrphanedResources[testChild, testParent](context.Background(), db, "parent_id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var count int64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.Model(&testChild{}).Count(&count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, int64(2), count, "All children should remain when no orphans") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestCleanupOrphanedResources_AllOrphans(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db := setupOrphanTestDB(t, &testParent{}, &testChild{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("INSERT INTO test_children (id, parent_id) VALUES (?, ?)", "c1", "gone1").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("INSERT INTO test_children (id, parent_id) VALUES (?, ?)", "c2", "gone2").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("INSERT INTO test_children (id, parent_id) VALUES (?, ?)", "c3", "gone3").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := migration.CleanupOrphanedResources[testChild, testParent](context.Background(), db, "parent_id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var count int64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.Model(&testChild{}).Count(&count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, int64(0), count, "All orphaned children should be deleted") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestCleanupOrphanedResources_MixedValidAndOrphaned(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db := setupOrphanTestDB(t, &testParent{}, &testChild{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testParent{ID: "p1"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testParent{ID: "p2"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testChild{ID: "c1", ParentID: "p1"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testChild{ID: "c2", ParentID: "p2"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testChild{ID: "c3", ParentID: "p1"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("INSERT INTO test_children (id, parent_id) VALUES (?, ?)", "c4", "gone1").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("INSERT INTO test_children (id, parent_id) VALUES (?, ?)", "c5", "gone2").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := migration.CleanupOrphanedResources[testChild, testParent](context.Background(), db, "parent_id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var remaining []testChild | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Order("id").Find(&remaining).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Len(t, remaining, 3, "Only valid children should remain") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, "c1", remaining[0].ID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, "c2", remaining[1].ID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, "c3", remaining[2].ID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestCleanupOrphanedResources_Idempotent(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db := setupOrphanTestDB(t, &testParent{}, &testChild{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testParent{ID: "p1"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testChild{ID: "c1", ParentID: "p1"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("INSERT INTO test_children (id, parent_id) VALUES (?, ?)", "c2", "gone").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx := context.Background() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := migration.CleanupOrphanedResources[testChild, testParent](ctx, db, "parent_id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var count int64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.Model(&testChild{}).Count(&count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, int64(1), count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err = migration.CleanupOrphanedResources[testChild, testParent](ctx, db, "parent_id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.Model(&testChild{}).Count(&count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, int64(1), count, "Count should remain the same after second run") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestCleanupOrphanedResources_SkipsWhenForeignKeyExists(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| engine := os.Getenv("NETBIRD_STORE_ENGINE") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if engine != "postgres" && engine != "mysql" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.Skip("FK constraint early-exit test requires postgres or mysql") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db := setupDatabase(t) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ = db.Migrator().DropTable(&testChildWithFK{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ = db.Migrator().DropTable(&testParent{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := db.AutoMigrate(&testParent{}, &testChildWithFK{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testParent{ID: "p1"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testParent{ID: "p2"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testChildWithFK{ID: "c1", ParentID: "p1"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Create(&testChildWithFK{ID: "c2", ParentID: "p2"}).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| switch engine { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case "postgres": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("ALTER TABLE test_children DROP CONSTRAINT fk_test_children_parent").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("DELETE FROM test_parents WHERE id = ?", "p2").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "ALTER TABLE test_children ADD CONSTRAINT fk_test_children_parent "+ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "FOREIGN KEY (parent_id) REFERENCES test_parents(id) NOT VALID", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case "mysql": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("SET FOREIGN_KEY_CHECKS = 0").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("ALTER TABLE test_children DROP FOREIGN KEY fk_test_children_parent").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("DELETE FROM test_parents WHERE id = ?", "p2").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "ALTER TABLE test_children ADD CONSTRAINT fk_test_children_parent "+ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "FOREIGN KEY (parent_id) REFERENCES test_parents(id)", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ).Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, db.Exec("SET FOREIGN_KEY_CHECKS = 1").Error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+621
to
+628
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify current MySQL FK test uses unpinned sequential Exec calls.
rg -n -C3 'SET FOREIGN_KEY_CHECKS|DROP FOREIGN KEY|ADD CONSTRAINT fk_test_children_parent' management/server/migration/migration_test.go
rg -n -C2 'Transaction\\(|\\.DB\\(\\)|Conn\\(' management/server/migration/migration_test.goRepository: netbirdio/netbird Length of output: 1260 🌐 Web query:
💡 Result: Yes, FOREIGN_KEY_CHECKS is session-scoped in MySQL. It can be set per session with SET foreign_key_checks=0 (or 1), affecting only that connection/session. There is also a global scope (SET GLOBAL), but session settings override for the current connection, and new sessions inherit from global. Consecutive Exec calls on *sql.DB can run on different pooled connections, as *sql.DB manages a connection pool and each call acquires a connection from the pool (possibly different ones sequentially). To pin to the same connection, use sql.Conn (via db.Conn(ctx)) or a transaction (tx := db.BeginTx(ctx, nil)), where operations are guaranteed on the same connection. Citations:
Pin MySQL FK-check statements to one connection to avoid flaky behavior.
Suggested fix (pin to a dedicated SQL connection for the mysql branch) case "mysql":
- require.NoError(t, db.Exec("SET FOREIGN_KEY_CHECKS = 0").Error)
- require.NoError(t, db.Exec("ALTER TABLE test_children DROP FOREIGN KEY fk_test_children_parent").Error)
- require.NoError(t, db.Exec("DELETE FROM test_parents WHERE id = ?", "p2").Error)
- require.NoError(t, db.Exec(
- "ALTER TABLE test_children ADD CONSTRAINT fk_test_children_parent "+
- "FOREIGN KEY (parent_id) REFERENCES test_parents(id)",
- ).Error)
- require.NoError(t, db.Exec("SET FOREIGN_KEY_CHECKS = 1").Error)
+ sqlDB, err := db.DB()
+ require.NoError(t, err)
+ conn, err := sqlDB.Conn(context.Background())
+ require.NoError(t, err)
+ t.Cleanup(func() {
+ _, _ = conn.ExecContext(context.Background(), "SET FOREIGN_KEY_CHECKS = 1")
+ _ = conn.Close()
+ })
+
+ _, err = conn.ExecContext(context.Background(), "SET FOREIGN_KEY_CHECKS = 0")
+ require.NoError(t, err)
+ _, err = conn.ExecContext(context.Background(), "ALTER TABLE test_children DROP FOREIGN KEY fk_test_children_parent")
+ require.NoError(t, err)
+ _, err = conn.ExecContext(context.Background(), "DELETE FROM test_parents WHERE id = ?", "p2")
+ require.NoError(t, err)
+ _, err = conn.ExecContext(
+ context.Background(),
+ "ALTER TABLE test_children ADD CONSTRAINT fk_test_children_parent "+
+ "FOREIGN KEY (parent_id) REFERENCES test_parents(id)",
+ )
+ require.NoError(t, err)
+ _, err = conn.ExecContext(context.Background(), "SET FOREIGN_KEY_CHECKS = 1")
+ require.NoError(t, err)
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err = migration.CleanupOrphanedResources[testChildWithFK, testParent](context.Background(), db, "parent_id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var count int64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.Model(&testChildWithFK{}).Count(&count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, int64(2), count, "Both rows should survive — migration must skip when FK constraint exists") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: netbirdio/netbird
Length of output: 3828
🏁 Script executed:
Repository: netbirdio/netbird
Length of output: 15120
🏁 Script executed:
Repository: netbirdio/netbird
Length of output: 397
🏁 Script executed:
Repository: netbirdio/netbird
Length of output: 816
🏁 Script executed:
Repository: netbirdio/netbird
Length of output: 990
🏁 Script executed:
Repository: netbirdio/netbird
Length of output: 43
Change
hasForeignKeyto return(bool, error)so metadata lookup failures abort instead of silently allowing data loss.hasForeignKey()ignores.Scan().Errorfor every dialect. If metadata introspection fails (connection issue, permission denial, SQL syntax error), the function returnsfalse, andCleanupOrphanedResources()proceeds with the destructive DELETE as if no FK existed. Return(bool, error)and have the caller check and propagate errors.Proposed fix
🤖 Prompt for AI Agents