[MM-37898] Exclude bots from initial user store emptiness check (#18139)

* Exclude bots from initial user store emptiness check

* Add test

services/users/users.go

@@ -43,7 +43,7 @@ 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
-	if ok, err := us.store.IsEmpty(); err != nil {
+	if ok, err := us.store.IsEmpty(true); err != nil {
 		return nil, errors.Wrap(UserStoreIsEmptyError, err.Error())
 	} else if ok {
 		user.Roles = model.SystemAdminRoleId + " " + model.SystemUserRoleId

services/users/users_test.go

@@ -5,6 +5,10 @@ package users
 
 import (
 	"testing"
+
+	"github.com/mattermost/mattermost-server/v6/model"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestIsUsernameTaken(t *testing.T) {
@@ -27,3 +31,59 @@ func TestIsUsernameTaken(t *testing.T) {
 		t.FailNow()
 	}
 }
+
+func TestFirstUserPromoted(t *testing.T) {
+	th := Setup(t)
+	defer th.TearDown()
+
+	user, err := th.service.CreateUser(&model.User{
+		Username: model.NewId(),
+		Password: model.NewId(),
+		Email:    "user@example.com",
+	}, UserCreateOptions{})
+	require.NoError(t, err)
+	require.NotNil(t, user)
+
+	require.Equal(t, model.SystemAdminRoleId+" "+model.SystemUserRoleId, user.Roles)
+
+	user2, err := th.service.CreateUser(&model.User{
+		Username: model.NewId(),
+		Password: model.NewId(),
+		Email:    "user2@example.com",
+	}, UserCreateOptions{})
+	require.NoError(t, err)
+	require.NotNil(t, user2)
+
+	require.Equal(t, model.SystemUserRoleId, user2.Roles)
+
+	th.dbStore.User().PermanentDelete(user.Id)
+
+	b := &model.Bot{
+		UserId:   user2.Id,
+		OwnerId:  model.NewId(),
+		Username: model.NewId(),
+	}
+
+	_, err = th.dbStore.Bot().Save(b)
+	require.NoError(t, err)
+
+	user3, err := th.service.CreateUser(&model.User{
+		Username: model.NewId(),
+		Password: model.NewId(),
+		Email:    "user3@example.com",
+	}, UserCreateOptions{})
+	require.NoError(t, err)
+	require.NotNil(t, user3)
+
+	require.Equal(t, model.SystemAdminRoleId+" "+model.SystemUserRoleId, user3.Roles)
+
+	user4, err := th.service.CreateUser(&model.User{
+		Username: model.NewId(),
+		Password: model.NewId(),
+		Email:    "user4@example.com",
+	}, UserCreateOptions{})
+	require.NoError(t, err)
+	require.NotNil(t, user4)
+
+	require.Equal(t, model.SystemUserRoleId, user4.Roles)
+}

store/opentracinglayer/opentracinglayer.go

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

store/retrylayer/retrylayer.go

@@ -11250,11 +11250,11 @@ func (s *RetryLayerUserStore) InvalidateProfilesInChannelCacheByUser(userID stri
 
 }
 
-func (s *RetryLayerUserStore) IsEmpty() (bool, error) {
+func (s *RetryLayerUserStore) IsEmpty(excludeBots bool) (bool, error) {
 
 	tries := 0
 	for {
-		result, err := s.UserStore.IsEmpty()
+		result, err := s.UserStore.IsEmpty(excludeBots)
 		if err == nil {
 			return result, nil
 		}

store/sqlstore/user_store.go

@@ -2004,10 +2004,26 @@ func (us SqlUserStore) GetKnownUsers(userId string) ([]string, error) {
 }
 
 // IsEmpty returns whether or not the Users table is empty.
-func (us SqlUserStore) IsEmpty() (bool, error) {
+func (us SqlUserStore) IsEmpty(excludeBots bool) (bool, error) {
 	var hasRows bool
-	err := us.GetReplica().SelectOne(&hasRows, `SELECT EXISTS (SELECT 1 FROM Users)`)
+
+	builder := us.getQueryBuilder().
+		Select("1").
+		Prefix("SELECT EXISTS (").
+		From("Users")
+
+	if excludeBots {
+		builder = builder.LeftJoin("Bots ON Users.Id = Bots.UserId").Where("Bots.UserId IS NULL")
+	}
+
+	builder = builder.Suffix(")")
+
+	query, args, err := builder.ToSql()
 	if err != nil {
+		return false, errors.Wrapf(err, "users_is_empty_to_sql")
+	}
+
+	if err = us.GetReplica().SelectOne(&hasRows, query, args...); err != nil {
 		return false, errors.Wrap(err, "failed to check if table is empty")
 	}
 	return !hasRows, nil

store/store.go

@@ -424,7 +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)
+	IsEmpty(excludeBots bool) (bool, error)
 }
 
 type BotStore interface {

store/storetest/mocks/UserStore.go

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

store/storetest/user_store.go

@@ -5714,26 +5714,51 @@ func testGetKnownUsers(t *testing.T, ss store.Store) {
 }
 
 func testIsEmpty(t *testing.T, ss store.Store) {
-	ok, err := ss.User().IsEmpty()
+	ok, err := ss.User().IsEmpty(false)
 	require.NoError(t, err)
 	require.True(t, ok)
 
-	u := model.User{
+	ok, err = ss.User().IsEmpty(true)
+	require.NoError(t, err)
+	require.True(t, ok)
+
+	u := &model.User{
 		Email:    MakeEmail(),
 		Username: model.NewId(),
 	}
 
-	_, err = ss.User().Save(&u)
+	u, err = ss.User().Save(u)
 	require.NoError(t, err)
 
-	ok, err = ss.User().IsEmpty()
+	ok, err = ss.User().IsEmpty(false)
 	require.NoError(t, err)
 	require.False(t, ok)
 
+	ok, err = ss.User().IsEmpty(true)
+	require.NoError(t, err)
+	require.False(t, ok)
+
+	b := &model.Bot{
+		UserId:   u.Id,
+		OwnerId:  model.NewId(),
+		Username: model.NewId(),
+	}
+
+	_, err = ss.Bot().Save(b)
+	require.NoError(t, err)
+
+	ok, err = ss.User().IsEmpty(false)
+	require.NoError(t, err)
+	require.False(t, ok)
+
+	ok, err = ss.User().IsEmpty(true)
+	require.NoError(t, err)
+	require.True(t, ok)
+
 	err = ss.User().PermanentDelete(u.Id)
 	require.NoError(t, err)
 
-	ok, err = ss.User().IsEmpty()
+	ok, err = ss.User().IsEmpty(false)
 	require.NoError(t, err)
 	require.True(t, ok)
 }

store/timerlayer/timerlayer.go

@@ -9346,10 +9346,10 @@ func (s *TimerLayerUserStore) InvalidateProfilesInChannelCacheByUser(userID stri
 	}
 }
 
-func (s *TimerLayerUserStore) IsEmpty() (bool, error) {
+func (s *TimerLayerUserStore) IsEmpty(excludeBots bool) (bool, error) {
 	start := timemodule.Now()
 
-	result, err := s.UserStore.IsEmpty()
+	result, err := s.UserStore.IsEmpty(excludeBots)
 
 	elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
 	if s.Root.Metrics != nil {