diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index eff218d5001..d1202dd620b 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -510,7 +510,7 @@ func SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *models } func SetAlertNotificationStateToPendingCommand(ctx context.Context, cmd *models.SetAlertNotificationStateToPendingCommand) error { - return withDbSession(ctx, func(sess *DBSession) error { + return withDbSession(ctx, x, func(sess *DBSession) error { newVersion := cmd.Version + 1 sql := `UPDATE alert_notification_state SET state = ?, diff --git a/pkg/services/sqlstore/annotation_cleanup.go b/pkg/services/sqlstore/annotation_cleanup.go index a878b0879d2..78227366530 100644 --- a/pkg/services/sqlstore/annotation_cleanup.go +++ b/pkg/services/sqlstore/annotation_cleanup.go @@ -91,7 +91,7 @@ func (acs *AnnotationCleanupService) executeUntilDoneOrCancelled(ctx context.Con return totalAffected, ctx.Err() default: var affected int64 - err := withDbSession(ctx, func(session *DBSession) error { + err := withDbSession(ctx, x, func(session *DBSession) error { res, err := session.Exec(sql) if err != nil { return err diff --git a/pkg/services/sqlstore/apikey.go b/pkg/services/sqlstore/apikey.go index 468f14ce427..3298f170555 100644 --- a/pkg/services/sqlstore/apikey.go +++ b/pkg/services/sqlstore/apikey.go @@ -28,13 +28,17 @@ func GetApiKeys(query *models.GetApiKeysQuery) error { } func DeleteApiKeyCtx(ctx context.Context, cmd *models.DeleteApiKeyCommand) error { - return withDbSession(ctx, func(sess *DBSession) error { - var rawSQL = "DELETE FROM api_key WHERE id=? and org_id=?" - _, err := sess.Exec(rawSQL, cmd.Id, cmd.OrgId) - return err + return withDbSession(ctx, x, func(sess *DBSession) error { + return deleteAPIKey(sess, cmd.Id, cmd.OrgId) }) } +func deleteAPIKey(sess *DBSession, id, orgID int64) error { + rawSQL := "DELETE FROM api_key WHERE id=? and org_id=?" + _, err := sess.Exec(rawSQL, id, orgID) + return err +} + func AddApiKey(cmd *models.AddApiKeyCommand) error { return inTransaction(func(sess *DBSession) error { key := models.ApiKey{OrgId: cmd.OrgId, Name: cmd.Name} diff --git a/pkg/services/sqlstore/session.go b/pkg/services/sqlstore/session.go index c6c0176d0ef..84f51af354a 100644 --- a/pkg/services/sqlstore/session.go +++ b/pkg/services/sqlstore/session.go @@ -46,22 +46,14 @@ func startSession(ctx context.Context, engine *xorm.Engine, beginTran bool) (*DB return newSess, nil } -// WithDbSession calls the callback with an session attached to the context. +// WithDbSession calls the callback with a session. func (ss *SQLStore) WithDbSession(ctx context.Context, callback dbTransactionFunc) error { - sess, err := startSession(ctx, ss.engine, false) - if err != nil { - return err - } - defer sess.Close() - - return callback(sess) + return withDbSession(ctx, ss.engine, callback) } -func withDbSession(ctx context.Context, callback dbTransactionFunc) error { - sess, err := startSession(ctx, x, false) - if err != nil { - return err - } +func withDbSession(ctx context.Context, engine *xorm.Engine, callback dbTransactionFunc) error { + sess := &DBSession{Session: engine.NewSession()} + defer sess.Close() return callback(sess) } diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index ea8148c95f8..f1a8f8a7456 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -141,21 +141,15 @@ func (ss *SQLStore) Reset() error { } func (ss *SQLStore) ensureMainOrgAndAdminUser() error { - err := ss.InTransaction(context.Background(), func(ctx context.Context) error { + ctx := context.Background() + err := ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { ss.log.Debug("Ensuring main org and admin user exist") var stats models.SystemUserCountStats - err := ss.WithDbSession(ctx, func(sess *DBSession) error { - // TODO: Should be able to rename "Count" to "count", for more standard SQL style - // Just have to make sure it gets deserialized properly into models.SystemUserCountStats - rawSQL := `SELECT COUNT(id) AS Count FROM ` + dialect.Quote("user") - if _, err := sess.SQL(rawSQL).Get(&stats); err != nil { - return fmt.Errorf("could not determine if admin user exists: %w", err) - } - - return nil - }) - if err != nil { - return err + // TODO: Should be able to rename "Count" to "count", for more standard SQL style + // Just have to make sure it gets deserialized properly into models.SystemUserCountStats + rawSQL := `SELECT COUNT(id) AS Count FROM ` + dialect.Quote("user") + if _, err := sess.SQL(rawSQL).Get(&stats); err != nil { + return fmt.Errorf("could not determine if admin user exists: %w", err) } if stats.Count > 0 { @@ -166,7 +160,7 @@ func (ss *SQLStore) ensureMainOrgAndAdminUser() error { if !ss.Cfg.DisableInitAdminCreation { ss.log.Debug("Creating default admin user") ss.log.Debug("Creating default admin user") - if _, err := ss.createUser(ctx, userCreationArgs{ + if _, err := ss.createUser(ctx, sess, userCreationArgs{ Login: ss.Cfg.AdminUser, Email: ss.Cfg.AdminUser + "@localhost", Password: ss.Cfg.AdminPassword, @@ -181,11 +175,8 @@ func (ss *SQLStore) ensureMainOrgAndAdminUser() error { // return nil } - if err := inTransactionWithRetryCtx(ctx, ss.engine, func(sess *DBSession) error { - ss.log.Debug("Creating default org", "name", MainOrgName) - _, err := ss.getOrCreateOrg(sess, MainOrgName) - return err - }, 0); err != nil { + ss.log.Debug("Creating default org", "name", MainOrgName) + if _, err := ss.getOrCreateOrg(sess, MainOrgName); err != nil { return fmt.Errorf("failed to create default organization: %w", err) } diff --git a/pkg/services/sqlstore/stats.go b/pkg/services/sqlstore/stats.go index 8a19a54311f..47ccb010086 100644 --- a/pkg/services/sqlstore/stats.go +++ b/pkg/services/sqlstore/stats.go @@ -170,7 +170,7 @@ func GetAdminStats(query *models.GetAdminStatsQuery) error { } func GetSystemUserCountStats(ctx context.Context, query *models.GetSystemUserCountStatsQuery) error { - return withDbSession(ctx, func(sess *DBSession) error { + return withDbSession(ctx, x, func(sess *DBSession) error { var rawSQL = `SELECT COUNT(id) AS Count FROM ` + dialect.Quote("user") var stats models.SystemUserCountStats _, err := sess.SQL(rawSQL).Get(&stats) diff --git a/pkg/services/sqlstore/transactions_test.go b/pkg/services/sqlstore/transactions_test.go index 8c25924b983..4b4d280d7a4 100644 --- a/pkg/services/sqlstore/transactions_test.go +++ b/pkg/services/sqlstore/transactions_test.go @@ -23,11 +23,9 @@ func TestTransaction(t *testing.T) { err := AddApiKey(cmd) So(err, ShouldBeNil) - deleteApiKeyCmd := &models.DeleteApiKeyCommand{Id: cmd.Result.Id, OrgId: 1} - Convey("can update key", func() { - err := ss.InTransaction(context.Background(), func(ctx context.Context) error { - return DeleteApiKeyCtx(ctx, deleteApiKeyCmd) + err := ss.WithTransactionalDbSession(context.Background(), func(sess *DBSession) error { + return deleteAPIKey(sess, cmd.Result.Id, 1) }) So(err, ShouldBeNil) @@ -38,8 +36,8 @@ func TestTransaction(t *testing.T) { }) Convey("won't update if one handler fails", func() { - err := ss.InTransaction(context.Background(), func(ctx context.Context) error { - err := DeleteApiKeyCtx(ctx, deleteApiKeyCmd) + err := ss.WithTransactionalDbSession(context.Background(), func(sess *DBSession) error { + err := deleteAPIKey(sess, cmd.Result.Id, 1) if err != nil { return err } diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index a4c936dab5f..0ebf2e09d38 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -86,105 +86,99 @@ func (ss *SQLStore) getOrgIDForNewUser(sess *DBSession, args userCreationArgs) ( } // createUser creates a user in the database. -func (ss *SQLStore) createUser(ctx context.Context, args userCreationArgs, skipOrgSetup bool) (models.User, error) { +func (ss *SQLStore) createUser(ctx context.Context, sess *DBSession, args userCreationArgs, skipOrgSetup bool) (models.User, error) { var user models.User - if err := inTransactionWithRetryCtx(ctx, ss.engine, func(sess *DBSession) error { - var orgID int64 = -1 - if !skipOrgSetup { - var err error - orgID, err = ss.getOrgIDForNewUser(sess, args) - if err != nil { - return err - } - } - - if args.Email == "" { - args.Email = args.Login - } - - exists, err := sess.Where("email=? OR login=?", args.Email, args.Login).Get(&models.User{}) + var orgID int64 = -1 + if !skipOrgSetup { + var err error + orgID, err = ss.getOrgIDForNewUser(sess, args) if err != nil { - return err - } - if exists { - return models.ErrUserAlreadyExists + return user, err } + } - // create user - user = models.User{ - Email: args.Email, - Name: args.Name, - Login: args.Login, - Company: args.Company, - IsAdmin: args.IsAdmin, - IsDisabled: args.IsDisabled, - OrgId: orgID, - EmailVerified: args.EmailVerified, - Created: time.Now(), - Updated: time.Now(), - LastSeenAt: time.Now().AddDate(-10, 0, 0), - } + if args.Email == "" { + args.Email = args.Login + } - salt, err := util.GetRandomString(10) - if err != nil { - return err - } - user.Salt = salt - rands, err := util.GetRandomString(10) - if err != nil { - return err - } - user.Rands = rands - - if len(args.Password) > 0 { - encodedPassword, err := util.EncodePassword(args.Password, user.Salt) - if err != nil { - return err - } - user.Password = encodedPassword - } - - sess.UseBool("is_admin") - - if _, err := sess.Insert(&user); err != nil { - return err - } - - sess.publishAfterCommit(&events.UserCreated{ - Timestamp: user.Created, - Id: user.Id, - Name: user.Name, - Login: user.Login, - Email: user.Email, - }) - - // create org user link - if !skipOrgSetup { - orgUser := models.OrgUser{ - OrgId: orgID, - UserId: user.Id, - Role: models.ROLE_ADMIN, - Created: time.Now(), - Updated: time.Now(), - } - - if ss.Cfg.AutoAssignOrg && !user.IsAdmin { - if len(args.DefaultOrgRole) > 0 { - orgUser.Role = models.RoleType(args.DefaultOrgRole) - } else { - orgUser.Role = models.RoleType(ss.Cfg.AutoAssignOrgRole) - } - } - - if _, err = sess.Insert(&orgUser); err != nil { - return err - } - } - - return nil - }, 0); err != nil { + exists, err := sess.Where("email=? OR login=?", args.Email, args.Login).Get(&models.User{}) + if err != nil { return user, err } + if exists { + return user, models.ErrUserAlreadyExists + } + + // create user + user = models.User{ + Email: args.Email, + Name: args.Name, + Login: args.Login, + Company: args.Company, + IsAdmin: args.IsAdmin, + IsDisabled: args.IsDisabled, + OrgId: orgID, + EmailVerified: args.EmailVerified, + Created: time.Now(), + Updated: time.Now(), + LastSeenAt: time.Now().AddDate(-10, 0, 0), + } + + salt, err := util.GetRandomString(10) + if err != nil { + return user, err + } + user.Salt = salt + rands, err := util.GetRandomString(10) + if err != nil { + return user, err + } + user.Rands = rands + + if len(args.Password) > 0 { + encodedPassword, err := util.EncodePassword(args.Password, user.Salt) + if err != nil { + return user, err + } + user.Password = encodedPassword + } + + sess.UseBool("is_admin") + + if _, err := sess.Insert(&user); err != nil { + return user, err + } + + sess.publishAfterCommit(&events.UserCreated{ + Timestamp: user.Created, + Id: user.Id, + Name: user.Name, + Login: user.Login, + Email: user.Email, + }) + + // create org user link + if !skipOrgSetup { + orgUser := models.OrgUser{ + OrgId: orgID, + UserId: user.Id, + Role: models.ROLE_ADMIN, + Created: time.Now(), + Updated: time.Now(), + } + + if ss.Cfg.AutoAssignOrg && !user.IsAdmin { + if len(args.DefaultOrgRole) > 0 { + orgUser.Role = models.RoleType(args.DefaultOrgRole) + } else { + orgUser.Role = models.RoleType(ss.Cfg.AutoAssignOrgRole) + } + } + + if _, err = sess.Insert(&orgUser); err != nil { + return user, err + } + } return user, nil }