Replace db count query in user signup process (#18072)

This commit is contained in:
Claudio Costa
2021-08-12 11:23:21 +02:00
committed by GitHub
parent d3973557fc
commit bd65e8daf9
12 changed files with 126 additions and 9 deletions

View File

@@ -493,8 +493,8 @@ func (a *App) importUser(data *UserImportData, dryRun bool) *model.AppError {
return appErr
case errors.Is(err, users.AcceptedDomainError):
return model.NewAppError("importUser", "api.user.create_user.accepted_domain.app_error", nil, "", http.StatusBadRequest)
case errors.Is(err, users.UserCountError):
return model.NewAppError("importUser", "app.user.get_total_users_count.app_error", nil, nErr.Error(), http.StatusInternalServerError)
case errors.Is(err, users.UserStoreIsEmptyError):
return model.NewAppError("importUser", "app.user.store_is_empty.app_error", nil, nErr.Error(), http.StatusInternalServerError)
case errors.As(err, &invErr):
switch invErr.Field {
case "email":

View File

@@ -226,8 +226,8 @@ func (a *App) createUserOrGuest(c *request.Context, user *model.User, guest bool
return nil, model.NewAppError("createUserOrGuest", "api.user.create_user.accepted_domain.app_error", nil, "", http.StatusBadRequest)
case errors.As(nErr, &nfErr):
return nil, model.NewAppError("createUserOrGuest", "api.user.check_user_password.invalid.app_error", nil, "", http.StatusBadRequest)
case errors.Is(nErr, users.UserCountError):
return nil, model.NewAppError("createUserOrGuest", "app.user.get_total_users_count.app_error", nil, nErr.Error(), http.StatusInternalServerError)
case errors.Is(nErr, users.UserStoreIsEmptyError):
return nil, model.NewAppError("createUserOrGuest", "app.user.store_is_empty.app_error", nil, nErr.Error(), http.StatusInternalServerError)
case errors.As(nErr, &invErr):
switch invErr.Field {
case "email":

View File

@@ -6410,6 +6410,10 @@
"id": "app.user.send_emails.app_error",
"translation": "No emails were successfully sent"
},
{
"id": "app.user.store_is_empty.app_error",
"translation": "Failed to check if user store is empty."
},
{
"id": "app.user.update.find.app_error",
"translation": "Unable to find the existing account to update."

View File

@@ -10,6 +10,7 @@ var (
VerifyUserError = errors.New("could not update verify email field")
UserCountError = errors.New("could not get the total number of the users.")
UserCreationDisabledError = errors.New("user creation is not allowed")
UserStoreIsEmptyError = errors.New("could not check if the user store is empty")
GetTokenError = errors.New("could not get token")
GetSessionError = errors.New("could not get session")

View File

@@ -13,6 +13,8 @@ import (
"github.com/mattermost/mattermost-server/v6/shared/mfa"
"github.com/mattermost/mattermost-server/v6/shared/mlog"
"github.com/mattermost/mattermost-server/v6/store"
"github.com/pkg/errors"
)
type UserCreateOptions struct {
@@ -41,11 +43,9 @@ func (us *UserService) CreateUser(user *model.User, opts UserCreateOptions) (*mo
// Below is a special case where the first user in the entire
// system is granted the system_admin role
count, err := us.store.Count(model.UserCountOptions{IncludeDeleted: true})
if err != nil {
return nil, UserCountError
}
if count <= 0 {
if ok, err := us.store.IsEmpty(); err != nil {
return nil, errors.Wrap(UserStoreIsEmptyError, err.Error())
} else if ok {
user.Roles = model.SystemAdminRoleId + " " + model.SystemUserRoleId
}

View File

@@ -10356,6 +10356,24 @@ func (s *OpenTracingLayerUserStore) InvalidateProfilesInChannelCacheByUser(userI
}
func (s *OpenTracingLayerUserStore) IsEmpty() (bool, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.IsEmpty")
s.Root.Store.SetContext(newCtx)
defer func() {
s.Root.Store.SetContext(origCtx)
}()
defer span.Finish()
result, err := s.UserStore.IsEmpty()
if err != nil {
span.LogFields(spanlog.Error(err))
ext.Error.Set(span, true)
}
return result, err
}
func (s *OpenTracingLayerUserStore) PermanentDelete(userID string) error {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.PermanentDelete")

View File

@@ -11236,6 +11236,26 @@ func (s *RetryLayerUserStore) InvalidateProfilesInChannelCacheByUser(userID stri
}
func (s *RetryLayerUserStore) IsEmpty() (bool, error) {
tries := 0
for {
result, err := s.UserStore.IsEmpty()
if err == nil {
return result, nil
}
if !isRepeatableError(err) {
return result, err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return result, err
}
}
}
func (s *RetryLayerUserStore) PermanentDelete(userID string) error {
tries := 0

View File

@@ -2002,3 +2002,13 @@ func (us SqlUserStore) GetKnownUsers(userId string) ([]string, error) {
return userIds, nil
}
// IsEmpty returns whether or not the Users table is empty.
func (us SqlUserStore) IsEmpty() (bool, error) {
var hasRows bool
err := us.GetReplica().SelectOne(&hasRows, `SELECT EXISTS (SELECT 1 FROM Users)`)
if err != nil {
return false, errors.Wrap(err, "failed to check if table is empty")
}
return !hasRows, nil
}

View File

@@ -424,6 +424,7 @@ type UserStore interface {
DeactivateGuests() ([]string, error)
AutocompleteUsersInChannel(teamID, channelID, term string, options *model.UserSearchOptions) (*model.UserAutocompleteInChannel, error)
GetKnownUsers(userID string) ([]string, error)
IsEmpty() (bool, error)
}
type BotStore interface {

View File

@@ -1038,6 +1038,27 @@ func (_m *UserStore) InvalidateProfilesInChannelCacheByUser(userID string) {
_m.Called(userID)
}
// IsEmpty provides a mock function with given fields:
func (_m *UserStore) IsEmpty() (bool, error) {
ret := _m.Called()
var r0 bool
if rf, ok := ret.Get(0).(func() bool); ok {
r0 = rf()
} else {
r0 = ret.Get(0).(bool)
}
var r1 error
if rf, ok := ret.Get(1).(func() error); ok {
r1 = rf()
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// PermanentDelete provides a mock function with given fields: userID
func (_m *UserStore) PermanentDelete(userID string) error {
ret := _m.Called(userID)

View File

@@ -36,6 +36,7 @@ func TestUserStore(t *testing.T, ss store.Store, s SqlStore) {
require.NoError(t, err, "failed cleaning up test user %s", u.Username)
}
t.Run("IsEmpty", func(t *testing.T) { testIsEmpty(t, ss) })
t.Run("Count", func(t *testing.T) { testCount(t, ss) })
t.Run("AnalyticsActiveCount", func(t *testing.T) { testUserStoreAnalyticsActiveCount(t, ss, s) })
t.Run("AnalyticsActiveCountForPeriod", func(t *testing.T) { testUserStoreAnalyticsActiveCountForPeriod(t, ss, s) })
@@ -5711,3 +5712,28 @@ func testGetKnownUsers(t *testing.T, ss store.Store) {
assert.ElementsMatch(t, userIds, []string{u2.Id, u3.Id})
})
}
func testIsEmpty(t *testing.T, ss store.Store) {
ok, err := ss.User().IsEmpty()
require.NoError(t, err)
require.True(t, ok)
u := model.User{
Email: MakeEmail(),
Username: model.NewId(),
}
_, err = ss.User().Save(&u)
require.NoError(t, err)
ok, err = ss.User().IsEmpty()
require.NoError(t, err)
require.False(t, ok)
err = ss.User().PermanentDelete(u.Id)
require.NoError(t, err)
ok, err = ss.User().IsEmpty()
require.NoError(t, err)
require.True(t, ok)
}

View File

@@ -9345,6 +9345,22 @@ func (s *TimerLayerUserStore) InvalidateProfilesInChannelCacheByUser(userID stri
}
}
func (s *TimerLayerUserStore) IsEmpty() (bool, error) {
start := timemodule.Now()
result, err := s.UserStore.IsEmpty()
elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
if s.Root.Metrics != nil {
success := "false"
if err == nil {
success = "true"
}
s.Root.Metrics.ObserveStoreMethodDuration("UserStore.IsEmpty", success, elapsed)
}
return result, err
}
func (s *TimerLayerUserStore) PermanentDelete(userID string) error {
start := timemodule.Now()