From a6e6f252db36a0649ec3db8c97b533c1e0d71cb8 Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 24 Feb 2026 11:50:35 +0100 Subject: [PATCH] refactor: remove redundant Begin() calls after NewSession auto-begins Since NewSession() now auto-begins a transaction, explicit Begin() calls are redundant (xorm's Begin() is a no-op when already in a transaction). Removing them reduces confusion. Special case: user_delete.go's loop previously called Begin/Commit per user on a shared session. Restructured to create a new session per user deletion so each gets its own transaction. --- pkg/cmd/repair_file_mime_types.go | 8 -------- pkg/cmd/repair_task_positions.go | 9 --------- pkg/cmd/user.go | 4 ---- pkg/models/listeners.go | 17 +--------------- pkg/models/typesense.go | 1 - pkg/models/user_delete.go | 32 ++++++++++++++---------------- pkg/routes/api/v1/user_deletion.go | 15 -------------- pkg/routes/api/v1/user_export.go | 5 ----- 8 files changed, 16 insertions(+), 75 deletions(-) diff --git a/pkg/cmd/repair_file_mime_types.go b/pkg/cmd/repair_file_mime_types.go index 897a4974c..efc94b089 100644 --- a/pkg/cmd/repair_file_mime_types.go +++ b/pkg/cmd/repair_file_mime_types.go @@ -44,14 +44,6 @@ on file creation. Only files with an empty or NULL mime column are affected.`, s := db.NewSession() defer s.Close() - if err := s.Begin(); err != nil { - log.Errorf("Failed to start transaction: %s", err) - return - } - defer func() { - _ = s.Rollback() - }() - result, err := files.RepairFileMimeTypes(s) if err != nil { log.Errorf("Failed to repair file MIME types: %s", err) diff --git a/pkg/cmd/repair_task_positions.go b/pkg/cmd/repair_task_positions.go index 456f780e8..08a177638 100644 --- a/pkg/cmd/repair_task_positions.go +++ b/pkg/cmd/repair_task_positions.go @@ -55,15 +55,6 @@ Use --dry-run to preview what would be fixed without making changes.`, if dryRun { log.Infof("Running in dry-run mode - no changes will be made") - } else { - if err := s.Begin(); err != nil { - log.Errorf("Failed to start transaction: %s", err) - return - } - defer func() { - // Rollback is a no-op if commit already succeeded - _ = s.Rollback() - }() } result, err := models.RepairTaskPositions(s, dryRun) diff --git a/pkg/cmd/user.go b/pkg/cmd/user.go index e8ceb24ab..7a89466ca 100644 --- a/pkg/cmd/user.go +++ b/pkg/cmd/user.go @@ -369,10 +369,6 @@ var userDeleteCmd = &cobra.Command{ s := db.NewSession() defer s.Close() - if err := s.Begin(); err != nil { - log.Fatalf("Count not start transaction: %s", err) - } - u := getUserFromArg(s, args[0]) if userFlagDeleteNow { diff --git a/pkg/models/listeners.go b/pkg/models/listeners.go index b508d88c2..41e9ca8d2 100644 --- a/pkg/models/listeners.go +++ b/pkg/models/listeners.go @@ -1199,11 +1199,6 @@ func (l *CleanupTaskAssignmentsAfterTeamRemoval) Handle(msg *message.Message) (e return nil } - err = s.Begin() - if err != nil { - return err - } - err = cleanupTaskMembersAfterTeamRemoval(s, event.Team.ID, event.Member.ID) if err != nil { _ = s.Rollback() @@ -1263,10 +1258,6 @@ func (s *HandleUserDataExport) Handle(msg *message.Message) (err error) { sess := db.NewSession() defer sess.Close() - err = sess.Begin() - if err != nil { - return - } err = ExportUserData(sess, event.User) if err != nil { @@ -1276,8 +1267,7 @@ func (s *HandleUserDataExport) Handle(msg *message.Message) (err error) { log.Debugf("Done exporting user data for user %d...", event.User.ID) - err = sess.Commit() - return err + return sess.Commit() } type MarkTaskUnreadOnComment struct { @@ -1297,11 +1287,6 @@ func (s *MarkTaskUnreadOnComment) Handle(msg *message.Message) (err error) { sess := db.NewSession() defer sess.Close() - err = sess.Begin() - if err != nil { - return err - } - project, err := GetProjectSimpleByID(sess, event.Task.ProjectID) if err != nil { _ = sess.Rollback() diff --git a/pkg/models/typesense.go b/pkg/models/typesense.go index d7d964093..2938fdca7 100644 --- a/pkg/models/typesense.go +++ b/pkg/models/typesense.go @@ -536,7 +536,6 @@ func SyncUpdatedTasksIntoTypesense() (err error) { tasks := make(map[int64]*Task) s := db.NewSession() - _ = s.Begin() defer s.Close() lastSync := &TypesenseSync{} diff --git a/pkg/models/user_delete.go b/pkg/models/user_delete.go index 8a45f5a7c..5beffc813 100644 --- a/pkg/models/user_delete.go +++ b/pkg/models/user_delete.go @@ -65,26 +65,24 @@ func deleteUsers() { continue } - err = s.Begin() - if err != nil { - log.Errorf("Could not start transaction: %s", err) - return - } + func() { + us := db.NewSession() + defer us.Close() - err = DeleteUser(s, u) - if err != nil { - _ = s.Rollback() - log.Errorf("Could not delete u %d: %s", u.ID, err) - return - } + err = DeleteUser(us, u) + if err != nil { + _ = us.Rollback() + log.Errorf("Could not delete u %d: %s", u.ID, err) + return + } - log.Debugf("Deleted user %d", u.ID) + log.Debugf("Deleted user %d", u.ID) - err = s.Commit() - if err != nil { - log.Errorf("Could not commit transaction: %s", err) - return - } + err = us.Commit() + if err != nil { + log.Errorf("Could not commit transaction: %s", err) + } + }() } } diff --git a/pkg/routes/api/v1/user_deletion.go b/pkg/routes/api/v1/user_deletion.go index 8e3cf869b..9ff3210d9 100644 --- a/pkg/routes/api/v1/user_deletion.go +++ b/pkg/routes/api/v1/user_deletion.go @@ -50,11 +50,6 @@ func UserRequestDeletion(c *echo.Context) error { s := db.NewSession() defer s.Close() - err := s.Begin() - if err != nil { - return err - } - u, err := user.GetCurrentUserFromDB(s, c) if err != nil { _ = s.Rollback() @@ -119,11 +114,6 @@ func UserConfirmDeletion(c *echo.Context) error { s := db.NewSession() defer s.Close() - err = s.Begin() - if err != nil { - return err - } - u, err := user.GetCurrentUserFromDB(s, c) if err != nil { _ = s.Rollback() @@ -161,11 +151,6 @@ func UserCancelDeletion(c *echo.Context) error { s := db.NewSession() defer s.Close() - err := s.Begin() - if err != nil { - return err - } - u, err := user.GetCurrentUserFromDB(s, c) if err != nil { _ = s.Rollback() diff --git a/pkg/routes/api/v1/user_export.go b/pkg/routes/api/v1/user_export.go index 134230867..2def8d6b7 100644 --- a/pkg/routes/api/v1/user_export.go +++ b/pkg/routes/api/v1/user_export.go @@ -34,11 +34,6 @@ func checkExportRequest(c *echo.Context) (s *xorm.Session, u *user.User, err err s = db.NewSession() defer s.Close() - err = s.Begin() - if err != nil { - return nil, nil, err - } - u, err = user.GetCurrentUserFromDB(s, c) if err != nil { _ = s.Rollback()