From c9c250fb1c6ed878a1bfbad6947ab66cac9d14d5 Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 24 Feb 2026 11:47:35 +0100 Subject: [PATCH] fix: add missing Commit() to write callers After NewSession() auto-begins a transaction, callers that perform writes must explicitly call Commit() for changes to persist. Without this, writes are silently rolled back when Close() is called. Affected callers: - user deletion notification cron - caldav token generation/deletion - token cleanup cron - mark-all-notifications-read endpoint - saved filter view cron - project background delete - typesense reindex - export cleanup cron - task last-updated listener - saved filter view listener - SSO team cleanup cron - migration status start/finish - background set/remove handlers - orphaned task position cleanup - file creation --- pkg/cmd/maintenance.go | 5 +++++ pkg/files/files.go | 3 ++- pkg/models/export.go | 3 +++ pkg/models/listeners.go | 14 +++++++++++--- pkg/models/project.go | 6 +++++- pkg/models/saved_filters.go | 4 ++++ pkg/models/typesense.go | 2 +- pkg/modules/auth/openid/cron.go | 4 ++++ pkg/modules/background/handler/background.go | 11 +++++++++++ pkg/modules/migration/migration_status.go | 14 ++++++++++++-- pkg/routes/api/v1/notifications.go | 5 +++++ pkg/user/caldav_token.go | 16 ++++++++++++++-- pkg/user/delete.go | 5 +++++ pkg/user/token.go | 4 ++++ 14 files changed, 86 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/maintenance.go b/pkg/cmd/maintenance.go index d206205f4..2da1cb5a5 100644 --- a/pkg/cmd/maintenance.go +++ b/pkg/cmd/maintenance.go @@ -46,6 +46,11 @@ var deleteOrphanTaskPositions = &cobra.Command{ return } + if err := s.Commit(); err != nil { + log.Errorf("Could not commit orphaned task position deletion: %s", err) + return + } + if count == 0 { log.Infof("No orphaned task positions found.") return diff --git a/pkg/files/files.go b/pkg/files/files.go index 4c8460c8b..5426c9ebf 100644 --- a/pkg/files/files.go +++ b/pkg/files/files.go @@ -107,7 +107,8 @@ func CreateWithMime(f io.ReadSeeker, realname string, realsize uint64, a web.Aut _ = s.Rollback() return } - return + + return file, s.Commit() } func CreateWithMimeAndSession(s *xorm.Session, f io.ReadSeeker, realname string, realsize uint64, a web.Auth, mime string, checkFileSizeLimit bool) (file *File, err error) { diff --git a/pkg/models/export.go b/pkg/models/export.go index 27230f805..2189057be 100644 --- a/pkg/models/export.go +++ b/pkg/models/export.go @@ -452,6 +452,9 @@ func RegisterOldExportCleanupCron() { log.Debugf(logPrefix+"Removed %d old user data exports...", len(fs)) + if err := s.Commit(); err != nil { + log.Errorf(logPrefix+"Error committing export cleanup: %s", err) + } }) if err != nil { log.Fatalf("Could not register old export cleanup cron: %s", err) diff --git a/pkg/models/listeners.go b/pkg/models/listeners.go index 58578091f..b508d88c2 100644 --- a/pkg/models/listeners.go +++ b/pkg/models/listeners.go @@ -486,7 +486,12 @@ func (s *HandleTaskUpdateLastUpdated) Handle(msg *message.Message) (err error) { sess := db.NewSession() defer sess.Close() - return updateTaskLastUpdated(sess, &Task{ID: taskIDInt}) + err = updateTaskLastUpdated(sess, &Task{ID: taskIDInt}) + if err != nil { + return err + } + + return sess.Commit() } // RemoveTaskFromTypesense represents a listener @@ -751,10 +756,13 @@ func (l *UpdateTaskInSavedFilterViews) Handle(msg *message.Message) (err error) task := make(map[int64]*Task, 1) task[event.Task.ID] = event.Task // Will be filled with all data by the Typesense connector - return reindexTasksInTypesense(s, task) + err = reindexTasksInTypesense(s, task) + if err != nil { + return err + } } - return nil + return s.Commit() } /////// diff --git a/pkg/models/project.go b/pkg/models/project.go index adf77c740..f96f39b54 100644 --- a/pkg/models/project.go +++ b/pkg/models/project.go @@ -1291,8 +1291,12 @@ func (p *Project) DeleteBackgroundFileIfExists() (err error) { if err != nil && files.IsErrFileDoesNotExist(err) { return nil } + if err != nil { + _ = s.Rollback() + return err + } - return err + return s.Commit() } // SetProjectBackground sets a background file as project background in the db diff --git a/pkg/models/saved_filters.go b/pkg/models/saved_filters.go index 7f886f723..87c401c40 100644 --- a/pkg/models/saved_filters.go +++ b/pkg/models/saved_filters.go @@ -534,6 +534,10 @@ func RegisterAddTaskToFilterViewCron() { log.Errorf("%sError recalculating task positions for view %d: %s", logPrefix, data.view.ID, err) } } + + if err := s.Commit(); err != nil { + log.Errorf("%sError committing: %s", logPrefix, err) + } }) if err != nil { log.Fatalf("Could register add task to filter view cron: %s", err) diff --git a/pkg/models/typesense.go b/pkg/models/typesense.go index a1c311b58..d7d964093 100644 --- a/pkg/models/typesense.go +++ b/pkg/models/typesense.go @@ -244,7 +244,7 @@ func ReindexAllTasks() (err error) { return fmt.Errorf("could update last sync state: %s", err.Error()) } - return + return s.Commit() } func reindexTasksInTypesense(s *xorm.Session, tasks map[int64]*Task) (err error) { diff --git a/pkg/modules/auth/openid/cron.go b/pkg/modules/auth/openid/cron.go index 3fcca6204..7ff47fb3d 100644 --- a/pkg/modules/auth/openid/cron.go +++ b/pkg/modules/auth/openid/cron.go @@ -65,6 +65,10 @@ func RegisterEmptyOpenIDTeamCleanupCron() { log.Errorf(logPrefix+"Error removing empty openid team: %s", err) return } + + if err := s.Commit(); err != nil { + log.Errorf(logPrefix+"Error committing: %s", err) + } }) if err != nil { log.Fatalf("Could not register empty openid teams cleanup cron: %s", err) diff --git a/pkg/modules/background/handler/background.go b/pkg/modules/background/handler/background.go index 6564ccdde..9ecd39c93 100644 --- a/pkg/modules/background/handler/background.go +++ b/pkg/modules/background/handler/background.go @@ -161,6 +161,10 @@ func (bp *BackgroundProvider) SetBackground(c *echo.Context) error { return err } + if err := s.Commit(); err != nil { + return err + } + return c.JSON(http.StatusOK, project) } @@ -403,11 +407,13 @@ func RemoveProjectBackground(c *echo.Context) error { project, auth, err := checkProjectBackgroundRights(s, c) if err != nil { + _ = s.Rollback() return err } err = project.DeleteBackgroundFileIfExists() if err != nil { + _ = s.Rollback() return err } @@ -416,6 +422,11 @@ func RemoveProjectBackground(c *echo.Context) error { project.BackgroundBlurHash = "" err = models.UpdateProject(s, project, auth, true) if err != nil { + _ = s.Rollback() + return err + } + + if err := s.Commit(); err != nil { return err } diff --git a/pkg/modules/migration/migration_status.go b/pkg/modules/migration/migration_status.go index 71053e6ec..a967c950c 100644 --- a/pkg/modules/migration/migration_status.go +++ b/pkg/modules/migration/migration_status.go @@ -48,7 +48,12 @@ func StartMigration(m MigratorName, u *user.User) (status *Status, err error) { StartedAt: time.Now(), } _, err = s.Insert(status) - return + if err != nil { + _ = s.Rollback() + return + } + + return status, s.Commit() } // FinishMigration sets the finished at time and calls it a day @@ -59,7 +64,12 @@ func FinishMigration(status *Status) (err error) { status.FinishedAt = time.Now() _, err = s.Where("id = ?", status.ID).Update(status) - return + if err != nil { + _ = s.Rollback() + return + } + + return s.Commit() } // GetMigrationStatus returns the migration status for a migration and a user diff --git a/pkg/routes/api/v1/notifications.go b/pkg/routes/api/v1/notifications.go index a44f81ea4..4c0cf24e4 100644 --- a/pkg/routes/api/v1/notifications.go +++ b/pkg/routes/api/v1/notifications.go @@ -49,6 +49,11 @@ func MarkAllNotificationsAsRead(c *echo.Context) error { err = notifications.MarkAllNotificationsAsRead(s, a.GetID()) if err != nil { + _ = s.Rollback() + return err + } + + if err := s.Commit(); err != nil { return err } diff --git a/pkg/user/caldav_token.go b/pkg/user/caldav_token.go index 8209eaaa6..096fe3add 100644 --- a/pkg/user/caldav_token.go +++ b/pkg/user/caldav_token.go @@ -22,7 +22,13 @@ func GenerateNewCaldavToken(u *User) (token *Token, err error) { s := db.NewSession() defer s.Close() - return generateHashedToken(s, u, TokenCaldavAuth) + token, err = generateHashedToken(s, u, TokenCaldavAuth) + if err != nil { + _ = s.Rollback() + return + } + + return token, s.Commit() } func GetCaldavTokens(u *User) (tokens []*Token, err error) { @@ -36,5 +42,11 @@ func DeleteCaldavTokenByID(u *User, id int64) error { s := db.NewSession() defer s.Close() - return removeTokenByID(s, u, TokenCaldavAuth, id) + err := removeTokenByID(s, u, TokenCaldavAuth, id) + if err != nil { + _ = s.Rollback() + return err + } + + return s.Commit() } diff --git a/pkg/user/delete.go b/pkg/user/delete.go index 75f300593..c08e094a7 100644 --- a/pkg/user/delete.go +++ b/pkg/user/delete.go @@ -37,6 +37,7 @@ func RegisterDeletionNotificationCron() { func notifyUsersScheduledForDeletion() { s := db.NewSession() + defer s.Close() users := []*User{} err := s.Where(builder.NotNull{"deletion_scheduled_at"}). Find(&users) @@ -83,6 +84,10 @@ func notifyUsersScheduledForDeletion() { log.Errorf("Could update user %d last deletion reminder sent date: %s", user.ID, err) } } + + if err := s.Commit(); err != nil { + log.Errorf("Could not commit user deletion notifications: %s", err) + } } // RequestDeletion creates a user deletion confirm token and sends a notification to the user diff --git a/pkg/user/token.go b/pkg/user/token.go index 1db3d328f..a2e3dd60f 100644 --- a/pkg/user/token.go +++ b/pkg/user/token.go @@ -140,6 +140,10 @@ func RegisterTokenCleanupCron() { if deleted > 0 { log.Debugf(logPrefix+"Deleted %d old password reset tokens", deleted) } + + if err := s.Commit(); err != nil { + log.Errorf(logPrefix+"Error committing token cleanup: %s", err) + } }) if err != nil { log.Fatalf("Could not register token cleanup cron: %s", err)