Avoid calling the user count query in future if we get a count > 0 (#23545)

* Avoid calling the user count query in future if we get a count > 0

* re-adding mock session to avoid adding the old mitigation in future

* adjustments based on feedback
Этот коммит содержится в:
Ben Cooke
2023-05-30 21:34:01 -04:00
коммит произвёл GitHub
родитель 14fcc8a22e
Коммит 427635a96f
3 изменённых файлов: 36 добавлений и 28 удалений

Просмотреть файл

@@ -326,12 +326,20 @@ func (ps *PlatformService) LimitedClientConfig() map[string]string {
} }
func (ps *PlatformService) IsFirstUserAccount() bool { func (ps *PlatformService) IsFirstUserAccount() bool {
count, err := ps.Store.User().Count(model.UserCountOptions{IncludeDeleted: true}) if ps.fetchUserCountForFirstUserAccountCheck.Load() {
if err != nil { count, err := ps.Store.User().Count(model.UserCountOptions{IncludeDeleted: true})
return false if err != nil {
return false
}
// Avoid calling the user count query in future if we get a count > 0
if count > 0 {
ps.fetchUserCountForFirstUserAccountCheck.Store(false)
return false
}
return true
} }
return count <= 0 return false
} }
func (ps *PlatformService) MaxPostSize() int { func (ps *PlatformService) MaxPostSize() int {

Просмотреть файл

@@ -109,37 +109,33 @@ func TestIsFirstUserAccount(t *testing.T) {
storeMock.On("User").Return(userStoreMock) storeMock.On("User").Return(userStoreMock)
type test struct { type test struct {
name string name string
count int64 count int64
err error err error
result bool result bool
shouldCallStore bool
} }
tests := []test{ tests := []test{
{"success no users", 0, nil, true}, {"failed request", 0, errors.New("error"), false, true},
{"success one user", 1, nil, false}, {"success negative users", -100, nil, true, true},
{"success multiple users", 42, nil, false}, {"success no users", 0, nil, true, true},
{"success negative users", -100, nil, true}, {"success one user", 1, nil, false, true},
{"failed request", 0, errors.New("error"), false}, {"success multiple users - no store call", 42, nil, false, false},
}
for _, te := range tests {
t.Run(te.name, func(t *testing.T) {
*userStoreMock = smocks.UserStore{}
userStoreMock.On("Count", model.UserCountOptions{IncludeDeleted: true}).Return(te.count, te.err)
require.Equal(t, te.result, th.Service.IsFirstUserAccount())
})
} }
// create a session, this should not affect IsFirstUserAccount // create a session, this should not affect IsFirstUserAccount
th.Service.sessionCache.Set("mock_session", 1) th.Service.sessionCache.Set("mock_session", 1)
for _, te := range tests { for _, te := range tests {
t.Run(te.name+" with session", func(t *testing.T) { t.Run(te.name, func(t *testing.T) {
*userStoreMock = smocks.UserStore{} *userStoreMock = smocks.UserStore{}
userStoreMock.On("Count", model.UserCountOptions{IncludeDeleted: true}).Return(te.count, te.err) userStoreMock.On("Count", model.UserCountOptions{IncludeDeleted: true}).Return(te.count, te.err).RunFn = func(args mock.Arguments) {
if !te.shouldCallStore {
assert.Fail(t, "should not have called the store")
}
}
require.Equal(t, te.result, th.Service.IsFirstUserAccount()) require.Equal(t, te.result, th.Service.IsFirstUserAccount())
}) })
} }

Просмотреть файл

@@ -49,10 +49,11 @@ type PlatformService struct {
sessionCache cache.Cache sessionCache cache.Cache
sessionPool sync.Pool sessionPool sync.Pool
asymmetricSigningKey atomic.Value asymmetricSigningKey atomic.Value
clientConfig atomic.Value clientConfig atomic.Value
clientConfigHash atomic.Value clientConfigHash atomic.Value
limitedClientConfig atomic.Value limitedClientConfig atomic.Value
fetchUserCountForFirstUserAccountCheck atomic.Bool
logger *mlog.Logger logger *mlog.Logger
notificationsLogger *mlog.Logger notificationsLogger *mlog.Logger
@@ -126,6 +127,9 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) {
additionalClusterHandlers: map[model.ClusterEvent]einterfaces.ClusterMessageHandler{}, additionalClusterHandlers: map[model.ClusterEvent]einterfaces.ClusterMessageHandler{},
} }
// Assume the first user account has not been created yet. A call to the DB will later check if this is really the case.
ps.fetchUserCountForFirstUserAccountCheck.Store(true)
// Step 1: Cache provider. // Step 1: Cache provider.
// At the moment we only have this implementation // At the moment we only have this implementation
// in the future the cache provider will be built based on the loaded config // in the future the cache provider will be built based on the loaded config