fix: address review comments on session lifecycle

- user_export.go: Remove defer s.Close() from checkExportRequest since
  it returns the session to callers. Callers now own the session
  lifecycle with their own defer s.Close(). Close session on all error
  paths within checkExportRequest.

- user_delete.go: Close the read session immediately after Find() before
  the per-user deletion loop, avoiding a long-lived transaction holding
  locks unnecessarily.

- user/delete.go: Remove double s.Close() in notifyUsersScheduledForDeletion
  by closing immediately after Find() instead of using both defer and
  explicit close.

- caldav_token.go: Return nil token on Commit() error to prevent callers
  from using an unpersisted token.

pkg/models/user_delete.go

@@ -42,10 +42,10 @@ func RegisterUserDeletionCron() {
 
 func deleteUsers() {
 	s := db.NewSession()
-	defer s.Close()
 	users := []*user.User{}
 	err := s.Where(builder.Lt{"deletion_scheduled_at": time.Now()}).
 		Find(&users)
+	s.Close()
 	if err != nil {
 		log.Errorf("Could not get users scheduled for deletion: %s", err)
 		return

pkg/routes/api/v1/user_export.go

@@ -32,11 +32,10 @@ import (
 
 func checkExportRequest(c *echo.Context) (s *xorm.Session, u *user.User, err error) {
 	s = db.NewSession()
-	defer s.Close()
 
 	u, err = user.GetCurrentUserFromDB(s, c)
 	if err != nil {
-		_ = s.Rollback()
+		s.Close()
 		return nil, nil, err
 	}
 
@@ -47,17 +46,19 @@ func checkExportRequest(c *echo.Context) (s *xorm.Session, u *user.User, err err
 
 	var pass UserPasswordConfirmation
 	if err := c.Bind(&pass); err != nil {
+		s.Close()
 		return nil, nil, echo.NewHTTPError(http.StatusBadRequest, "No password provided.").Wrap(err)
 	}
 
 	err = c.Validate(pass)
 	if err != nil {
+		s.Close()
 		return nil, nil, echo.NewHTTPError(http.StatusBadRequest, err.Error()).Wrap(err)
 	}
 
 	err = user.CheckUserPassword(u, pass.Password)
 	if err != nil {
-		_ = s.Rollback()
+		s.Close()
 		return nil, nil, err
 	}
 
@@ -80,6 +81,7 @@ func RequestUserDataExport(c *echo.Context) error {
 	if err != nil {
 		return err
 	}
+	defer s.Close()
 
 	err = events.Dispatch(&models.UserDataExportRequestedEvent{
 		User: u,
@@ -115,6 +117,7 @@ func DownloadUserDataExport(c *echo.Context) error {
 	if err != nil {
 		return err
 	}
+	defer s.Close()
 
 	err = s.Commit()
 	if err != nil {

pkg/user/caldav_token.go

@@ -25,10 +25,14 @@ func GenerateNewCaldavToken(u *User) (token *Token, err error) {
 	token, err = generateHashedToken(s, u, TokenCaldavAuth)
 	if err != nil {
 		_ = s.Rollback()
-		return
+		return nil, err
 	}
 
-	return token, s.Commit()
+	if err = s.Commit(); err != nil {
+		return nil, err
+	}
+
+	return token, nil
 }
 
 func GetCaldavTokens(u *User) (tokens []*Token, err error) {

pkg/user/delete.go

@@ -38,10 +38,10 @@ func RegisterDeletionNotificationCron() {
 
 func notifyUsersScheduledForDeletion() {
 	s := db.NewSession()
-	defer s.Close()
 	users := []*User{}
 	err := s.Where(builder.NotNull{"deletion_scheduled_at"}).
 		Find(&users)
+	s.Close()
 	if err != nil {
 		log.Errorf("Could not get users scheduled for deletion: %s", err)
 		return
@@ -51,9 +51,6 @@ func notifyUsersScheduledForDeletion() {
 		return
 	}
 
-	// Close the read-only session before processing each user in its own transaction.
-	s.Close()
-
 	log.Debugf("Found %d users scheduled for deletion to notify", len(users))
 
 	for _, user := range users {