Chore: Delete UpdateUser, ChangeUserPassword, UpdateLastSeenAt from sqlstore (#55928)

pkg/services/sqlstore/user.go

@@ -201,67 +201,6 @@ func (ss *SQLStore) GetUserById(ctx context.Context, query *models.GetUserByIdQu
 	})
 }
 
-func (ss *SQLStore) UpdateUser(ctx context.Context, cmd *models.UpdateUserCommand) error {
-	if ss.Cfg.CaseInsensitiveLogin {
-		cmd.Login = strings.ToLower(cmd.Login)
-		cmd.Email = strings.ToLower(cmd.Email)
-	}
-
-	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
-		user := user.User{
-			Name:    cmd.Name,
-			Email:   cmd.Email,
-			Login:   cmd.Login,
-			Theme:   cmd.Theme,
-			Updated: TimeNow(),
-		}
-
-		if _, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Update(&user); err != nil {
-			return err
-		}
-
-		if ss.Cfg.CaseInsensitiveLogin {
-			if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil {
-				return err
-			}
-		}
-
-		sess.publishAfterCommit(&events.UserUpdated{
-			Timestamp: user.Created,
-			Id:        user.ID,
-			Name:      user.Name,
-			Login:     user.Login,
-			Email:     user.Email,
-		})
-
-		return nil
-	})
-}
-
-func (ss *SQLStore) ChangeUserPassword(ctx context.Context, cmd *models.ChangeUserPasswordCommand) error {
-	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
-		user := user.User{
-			Password: cmd.NewPassword,
-			Updated:  TimeNow(),
-		}
-
-		_, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Update(&user)
-		return err
-	})
-}
-
-func (ss *SQLStore) UpdateUserLastSeenAt(ctx context.Context, cmd *models.UpdateUserLastSeenAtCommand) error {
-	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
-		user := user.User{
-			ID:         cmd.UserId,
-			LastSeenAt: TimeNow(),
-		}
-
-		_, err := sess.ID(cmd.UserId).Update(&user)
-		return err
-	})
-}
-
 func (ss *SQLStore) SetUsingOrg(ctx context.Context, cmd *models.SetUsingOrgCommand) error {
 	getOrgsForUserCmd := &models.GetUserOrgListQuery{UserId: cmd.UserId}
 	if err := ss.GetUserOrgList(ctx, getOrgsForUserCmd); err != nil {

pkg/services/sqlstore/user_test.go

@@ -12,73 +12,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestIntegrationUserUpdate(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skipping integration test")
-	}
-
-	ss := InitTestDB(t)
-
-	users := createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand {
-		return &user.CreateUserCommand{
-			Email:      fmt.Sprint("USER", i, "@test.com"),
-			Name:       fmt.Sprint("USER", i),
-			Login:      fmt.Sprint("loginUSER", i),
-			IsDisabled: false,
-		}
-	})
-
-	ss.Cfg.CaseInsensitiveLogin = true
-
-	t.Run("Testing DB - update generates duplicate user", func(t *testing.T) {
-		err := ss.UpdateUser(context.Background(), &models.UpdateUserCommand{
-			Login:  "loginuser2",
-			UserId: users[0].ID,
-		})
-
-		require.Error(t, err)
-	})
-
-	t.Run("Testing DB - update lowercases existing user", func(t *testing.T) {
-		err := ss.UpdateUser(context.Background(), &models.UpdateUserCommand{
-			Login:  "loginUSER0",
-			Email:  "USER0@test.com",
-			UserId: users[0].ID,
-		})
-		require.NoError(t, err)
-
-		query := models.GetUserByIdQuery{Id: users[0].ID}
-		err = ss.GetUserById(context.Background(), &query)
-		require.NoError(t, err)
-
-		require.Equal(t, "loginuser0", query.Result.Login)
-		require.Equal(t, "user0@test.com", query.Result.Email)
-	})
-
-	t.Run("Testing DB - no user info provided", func(t *testing.T) {
-		err := ss.UpdateUser(context.Background(), &models.UpdateUserCommand{
-			Login:  "",
-			Email:  "",
-			Name:   "Change Name",
-			UserId: users[3].ID,
-		})
-		require.NoError(t, err)
-
-		query := models.GetUserByIdQuery{Id: users[3].ID}
-		err = ss.GetUserById(context.Background(), &query)
-		require.NoError(t, err)
-
-		// Changed
-		require.Equal(t, "Change Name", query.Result.Name)
-
-		// Unchanged
-		require.Equal(t, "loginUSER3", query.Result.Login)
-		require.Equal(t, "USER3@test.com", query.Result.Email)
-	})
-
-	ss.Cfg.CaseInsensitiveLogin = false
-}
-
 func TestIntegrationUserDataAccess(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")

pkg/services/user/userimpl/store.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/grafana/grafana/pkg/events"
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -23,6 +24,9 @@ type store interface {
 	CaseInsensitiveLoginConflict(context.Context, string, string) error
 	GetByLogin(context.Context, *user.GetUserByLoginQuery) (*user.User, error)
 	GetByEmail(context.Context, *user.GetUserByEmailQuery) (*user.User, error)
+	Update(context.Context, *user.UpdateUserCommand) error
+	ChangePassword(context.Context, *user.ChangeUserPasswordCommand) error
+	UpdateLastSeenAt(context.Context, *user.UpdateUserLastSeenAtCommand) error
 }
 
 type sqlStore struct {
@@ -246,3 +250,64 @@ func (ss *sqlStore) userCaseInsensitiveLoginConflict(ctx context.Context, sess *
 
 	return nil
 }
+
+func (ss *sqlStore) Update(ctx context.Context, cmd *user.UpdateUserCommand) error {
+	if ss.cfg.CaseInsensitiveLogin {
+		cmd.Login = strings.ToLower(cmd.Login)
+		cmd.Email = strings.ToLower(cmd.Email)
+	}
+
+	return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		user := user.User{
+			Name:    cmd.Name,
+			Email:   cmd.Email,
+			Login:   cmd.Login,
+			Theme:   cmd.Theme,
+			Updated: time.Now(),
+		}
+
+		if _, err := sess.ID(cmd.UserID).Where(ss.notServiceAccountFilter()).Update(&user); err != nil {
+			return err
+		}
+
+		if ss.cfg.CaseInsensitiveLogin {
+			if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil {
+				return err
+			}
+		}
+
+		sess.PublishAfterCommit(&events.UserUpdated{
+			Timestamp: user.Created,
+			Id:        user.ID,
+			Name:      user.Name,
+			Login:     user.Login,
+			Email:     user.Email,
+		})
+
+		return nil
+	})
+}
+
+func (ss *sqlStore) ChangePassword(ctx context.Context, cmd *user.ChangeUserPasswordCommand) error {
+	return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		user := user.User{
+			Password: cmd.NewPassword,
+			Updated:  time.Now(),
+		}
+
+		_, err := sess.ID(cmd.UserID).Where(ss.notServiceAccountFilter()).Update(&user)
+		return err
+	})
+}
+
+func (ss *sqlStore) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLastSeenAtCommand) error {
+	return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		user := user.User{
+			ID:         cmd.UserID,
+			LastSeenAt: time.Now(),
+		}
+
+		_, err := sess.ID(cmd.UserID).Update(&user)
+		return err
+	})
+}

pkg/services/user/userimpl/store_test.go

@@ -2,6 +2,7 @@ package userimpl
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -194,4 +195,97 @@ func TestIntegrationUserDataAccess(t *testing.T) {
 
 		ss.Cfg.CaseInsensitiveLogin = false
 	})
+
+	t.Run("Change user password", func(t *testing.T) {
+		err := userStore.ChangePassword(context.Background(), &user.ChangeUserPasswordCommand{})
+		require.NoError(t, err)
+	})
+
+	t.Run("update last seen at", func(t *testing.T) {
+		err := userStore.UpdateLastSeenAt(context.Background(), &user.UpdateUserLastSeenAtCommand{})
+		require.NoError(t, err)
+	})
+}
+
+func TestIntegrationUserUpdate(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
+
+	ss := sqlstore.InitTestDB(t)
+	userStore := ProvideStore(ss, setting.NewCfg())
+
+	users := createFiveTestUsers(t, ss, func(i int) *user.CreateUserCommand {
+		return &user.CreateUserCommand{
+			Email:      fmt.Sprint("USER", i, "@test.com"),
+			Name:       fmt.Sprint("USER", i),
+			Login:      fmt.Sprint("loginUSER", i),
+			IsDisabled: false,
+		}
+	})
+
+	userStore.cfg.CaseInsensitiveLogin = true
+
+	t.Run("Testing DB - update generates duplicate user", func(t *testing.T) {
+		err := userStore.Update(context.Background(), &user.UpdateUserCommand{
+			Login:  "loginuser2",
+			UserID: users[0].ID,
+		})
+
+		require.Error(t, err)
+	})
+
+	t.Run("Testing DB - update lowercases existing user", func(t *testing.T) {
+		err := userStore.Update(context.Background(), &user.UpdateUserCommand{
+			Login:  "loginUSER0",
+			Email:  "USER0@test.com",
+			UserID: users[0].ID,
+		})
+		require.NoError(t, err)
+
+		result, err := userStore.GetByID(context.Background(), users[0].ID)
+		require.NoError(t, err)
+
+		require.Equal(t, "loginuser0", result.Login)
+		require.Equal(t, "user0@test.com", result.Email)
+	})
+
+	t.Run("Testing DB - no user info provided", func(t *testing.T) {
+		err := userStore.Update(context.Background(), &user.UpdateUserCommand{
+			Login:  "",
+			Email:  "",
+			Name:   "Change Name",
+			UserID: users[3].ID,
+		})
+		require.NoError(t, err)
+
+		// query := user.GetUserByIDQuery{ID: users[3].ID}
+		result, err := userStore.GetByID(context.Background(), users[3].ID)
+		require.NoError(t, err)
+
+		// Changed
+		require.Equal(t, "Change Name", result.Name)
+
+		// Unchanged
+		require.Equal(t, "loginUSER3", result.Login)
+		require.Equal(t, "USER3@test.com", result.Email)
+	})
+
+	ss.Cfg.CaseInsensitiveLogin = false
+}
+
+func createFiveTestUsers(t *testing.T, sqlStore *sqlstore.SQLStore, fn func(i int) *user.CreateUserCommand) []user.User {
+	t.Helper()
+
+	users := []user.User{}
+	for i := 0; i < 5; i++ {
+		cmd := fn(i)
+
+		user, err := sqlStore.CreateUser(context.Background(), *cmd)
+		users = append(users, *user)
+
+		require.Nil(t, err)
+	}
+
+	return users
 }

pkg/services/user/userimpl/user.go

@@ -160,34 +160,16 @@ func (s *Service) GetByEmail(ctx context.Context, query *user.GetUserByEmailQuer
 	return s.store.GetByEmail(ctx, query)
 }
 
-// TODO: remove wrapper around sqlstore
 func (s *Service) Update(ctx context.Context, cmd *user.UpdateUserCommand) error {
-	q := &models.UpdateUserCommand{
+	return s.store.Update(ctx, cmd)
-		Name:   cmd.Name,
-		Email:  cmd.Email,
-		Login:  cmd.Login,
-		Theme:  cmd.Theme,
-		UserId: cmd.UserID,
-	}
-	return s.sqlStore.UpdateUser(ctx, q)
 }
 
-// TODO: remove wrapper around sqlstore
 func (s *Service) ChangePassword(ctx context.Context, cmd *user.ChangeUserPasswordCommand) error {
-	q := &models.ChangeUserPasswordCommand{
+	return s.store.ChangePassword(ctx, cmd)
-		UserId:      cmd.UserID,
-		NewPassword: cmd.NewPassword,
-		OldPassword: cmd.OldPassword,
-	}
-	return s.sqlStore.ChangeUserPassword(ctx, q)
 }
 
-// TODO: remove wrapper around sqlstore
 func (s *Service) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLastSeenAtCommand) error {
-	q := &models.UpdateUserLastSeenAtCommand{
+	return s.store.UpdateLastSeenAt(ctx, cmd)
-		UserId: cmd.UserID,
-	}
-	return s.sqlStore.UpdateUserLastSeenAt(ctx, q)
 }
 
 // TODO: remove wrapper around sqlstore

pkg/services/user/userimpl/user_test.go

@@ -129,3 +129,15 @@ func (f *FakeUserStore) GetByLogin(ctx context.Context, query *user.GetUserByLog
 func (f *FakeUserStore) GetByEmail(ctx context.Context, query *user.GetUserByEmailQuery) (*user.User, error) {
 	return f.ExpectedUser, f.ExpectedError
 }
+
+func (f *FakeUserStore) Update(ctx context.Context, cmd *user.UpdateUserCommand) error {
+	return f.ExpectedError
+}
+
+func (f *FakeUserStore) ChangePassword(ctx context.Context, cmd *user.ChangeUserPasswordCommand) error {
+	return f.ExpectedError
+}
+
+func (f *FakeUserStore) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLastSeenAtCommand) error {
+	return f.ExpectedError
+}