[MM-28210] sqlstore/user_store: filter deleted users for GetProfilesInChannel (#15390)

* sqlstore/user_store: filter deleted users for GetProfilesInChannel

* allow GetProfilesInChannel use userGetOptions

* sqlstore/user_store: add more test cases

* store/user_store: refine filter
Этот коммит содержится в:
Ibrahim Serdar Acikgoz
2020-09-16 11:04:17 +03:00
коммит произвёл GitHub
родитель 9c272f0b20
Коммит e69a2a41ca
14 изменённых файлов: 229 добавлений и 77 удалений

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

@@ -8215,7 +8215,7 @@ func (s *OpenTracingLayerUserStore) GetProfilesByUsernames(usernames []string, v
return result, err
}
func (s *OpenTracingLayerUserStore) GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) {
func (s *OpenTracingLayerUserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetProfilesInChannel")
s.Root.Store.SetContext(newCtx)
@@ -8224,7 +8224,7 @@ func (s *OpenTracingLayerUserStore) GetProfilesInChannel(channelId string, offse
}()
defer span.Finish()
result, err := s.UserStore.GetProfilesInChannel(channelId, offset, limit)
result, err := s.UserStore.GetProfilesInChannel(options)
if err != nil {
span.LogFields(spanlog.Error(err))
ext.Error.Set(span, true)
@@ -8233,7 +8233,7 @@ func (s *OpenTracingLayerUserStore) GetProfilesInChannel(channelId string, offse
return result, err
}
func (s *OpenTracingLayerUserStore) GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) {
func (s *OpenTracingLayerUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetProfilesInChannelByStatus")
s.Root.Store.SetContext(newCtx)
@@ -8242,7 +8242,7 @@ func (s *OpenTracingLayerUserStore) GetProfilesInChannelByStatus(channelId strin
}()
defer span.Finish()
result, err := s.UserStore.GetProfilesInChannelByStatus(channelId, offset, limit)
result, err := s.UserStore.GetProfilesInChannelByStatus(options)
if err != nil {
span.LogFields(spanlog.Error(err))
ext.Error.Set(span, true)

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

@@ -6844,15 +6844,15 @@ func (s *RetryLayerUserStore) GetProfilesByUsernames(usernames []string, viewRes
}
func (s *RetryLayerUserStore) GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) {
func (s *RetryLayerUserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
return s.UserStore.GetProfilesInChannel(channelId, offset, limit)
return s.UserStore.GetProfilesInChannel(options)
}
func (s *RetryLayerUserStore) GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) {
func (s *RetryLayerUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
return s.UserStore.GetProfilesInChannelByStatus(channelId, offset, limit)
return s.UserStore.GetProfilesInChannelByStatus(options)
}

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

@@ -667,12 +667,18 @@ func (us SqlUserStore) InvalidateProfilesInChannelCacheByUser(userId string) {}
func (us SqlUserStore) InvalidateProfilesInChannelCache(channelId string) {}
func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) {
func (us SqlUserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
query := us.usersQuery.
Join("ChannelMembers cm ON ( cm.UserId = u.Id )").
Where("cm.ChannelId = ?", channelId).
Where("cm.ChannelId = ?", options.InChannelId).
OrderBy("u.Username ASC").
Offset(uint64(offset)).Limit(uint64(limit))
Offset(uint64(options.Page * options.PerPage)).Limit(uint64(options.PerPage))
if options.Inactive {
query = query.Where("u.DeleteAt != 0")
} else if options.Active {
query = query.Where("u.DeleteAt = 0")
}
queryString, args, err := query.ToSql()
if err != nil {
@@ -691,11 +697,11 @@ func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit
return users, nil
}
func (us SqlUserStore) GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) {
func (us SqlUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
query := us.usersQuery.
Join("ChannelMembers cm ON ( cm.UserId = u.Id )").
LeftJoin("Status s ON ( s.UserId = u.Id )").
Where("cm.ChannelId = ?", channelId).
Where("cm.ChannelId = ?", options.InChannelId).
OrderBy(`
CASE s.Status
WHEN 'online' THEN 1
@@ -705,7 +711,13 @@ func (us SqlUserStore) GetProfilesInChannelByStatus(channelId string, offset int
END
`).
OrderBy("u.Username ASC").
Offset(uint64(offset)).Limit(uint64(limit))
Offset(uint64(options.Page * options.PerPage)).Limit(uint64(options.PerPage))
if options.Inactive && !options.Active {
query = query.Where("u.DeleteAt != 0")
} else if options.Active && !options.Inactive {
query = query.Where("u.DeleteAt = 0")
}
queryString, args, err := query.ToSql()
if err != nil {

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

@@ -302,8 +302,8 @@ type UserStore interface {
ClearCaches()
InvalidateProfilesInChannelCacheByUser(userId string)
InvalidateProfilesInChannelCache(channelId string)
GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError)
GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError)
GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError)
GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError)
GetAllProfilesInChannel(channelId string, allowFromCache bool) (map[string]*model.User, *model.AppError)
GetProfilesNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError)
GetProfilesWithoutTeam(options *model.UserGetOptions) ([]*model.User, *model.AppError)

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

@@ -732,13 +732,13 @@ func (_m *UserStore) GetProfilesByUsernames(usernames []string, viewRestrictions
return r0, r1
}
// GetProfilesInChannel provides a mock function with given fields: channelId, offset, limit
func (_m *UserStore) GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) {
ret := _m.Called(channelId, offset, limit)
// GetProfilesInChannel provides a mock function with given fields: options
func (_m *UserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
ret := _m.Called(options)
var r0 []*model.User
if rf, ok := ret.Get(0).(func(string, int, int) []*model.User); ok {
r0 = rf(channelId, offset, limit)
if rf, ok := ret.Get(0).(func(*model.UserGetOptions) []*model.User); ok {
r0 = rf(options)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]*model.User)
@@ -746,8 +746,8 @@ func (_m *UserStore) GetProfilesInChannel(channelId string, offset int, limit in
}
var r1 *model.AppError
if rf, ok := ret.Get(1).(func(string, int, int) *model.AppError); ok {
r1 = rf(channelId, offset, limit)
if rf, ok := ret.Get(1).(func(*model.UserGetOptions) *model.AppError); ok {
r1 = rf(options)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)
@@ -757,13 +757,13 @@ func (_m *UserStore) GetProfilesInChannel(channelId string, offset int, limit in
return r0, r1
}
// GetProfilesInChannelByStatus provides a mock function with given fields: channelId, offset, limit
func (_m *UserStore) GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) {
ret := _m.Called(channelId, offset, limit)
// GetProfilesInChannelByStatus provides a mock function with given fields: options
func (_m *UserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
ret := _m.Called(options)
var r0 []*model.User
if rf, ok := ret.Get(0).(func(string, int, int) []*model.User); ok {
r0 = rf(channelId, offset, limit)
if rf, ok := ret.Get(0).(func(*model.UserGetOptions) []*model.User); ok {
r0 = rf(options)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]*model.User)
@@ -771,8 +771,8 @@ func (_m *UserStore) GetProfilesInChannelByStatus(channelId string, offset int,
}
var r1 *model.AppError
if rf, ok := ret.Get(1).(func(string, int, int) *model.AppError); ok {
r1 = rf(channelId, offset, limit)
if rf, ok := ret.Get(1).(func(*model.UserGetOptions) *model.AppError); ok {
r1 = rf(options)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)

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

@@ -756,6 +756,15 @@ func testUserStoreGetProfilesInChannel(t *testing.T, ss store.Store) {
u3.IsBot = true
defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }()
u4, err := ss.User().Save(&model.User{
Email: MakeEmail(),
Username: "u4" + model.NewId(),
})
require.Nil(t, err)
defer func() { require.Nil(t, ss.User().PermanentDelete(u4.Id)) }()
_, nErr = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u4.Id}, -1)
require.Nil(t, nErr)
ch1 := &model.Channel{
TeamId: teamId,
DisplayName: "Profiles in channel",
@@ -795,26 +804,79 @@ func testUserStoreGetProfilesInChannel(t *testing.T, ss store.Store) {
})
require.Nil(t, nErr)
_, nErr = ss.Channel().SaveMember(&model.ChannelMember{
ChannelId: c1.Id,
UserId: u4.Id,
NotifyProps: model.GetDefaultChannelNotifyProps(),
})
require.Nil(t, nErr)
u4.DeleteAt = 1
_, err = ss.User().Update(u4, true)
require.Nil(t, err)
_, nErr = ss.Channel().SaveMember(&model.ChannelMember{
ChannelId: c2.Id,
UserId: u1.Id,
NotifyProps: model.GetDefaultChannelNotifyProps(),
})
require.Nil(t, nErr)
t.Run("get in channel 1, offset 0, limit 100", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannel(c1.Id, 0, 100)
t.Run("get all users in channel 1, offset 0, limit 100", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{
InChannelId: c1.Id,
Page: 0,
PerPage: 100,
})
require.Nil(t, err)
assert.Equal(t, []*model.User{sanitized(u1), sanitized(u2), sanitized(u3), sanitized(u4)}, users)
})
t.Run("get only active users in channel 1, offset 0, limit 100", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{
InChannelId: c1.Id,
Page: 0,
PerPage: 100,
Active: true,
})
require.Nil(t, err)
assert.Equal(t, []*model.User{sanitized(u1), sanitized(u2), sanitized(u3)}, users)
})
t.Run("get in channel 1, offset 1, limit 2", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannel(c1.Id, 1, 2)
t.Run("get inactive users in channel 1, offset 0, limit 100", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{
InChannelId: c1.Id,
Page: 0,
PerPage: 100,
Inactive: true,
})
require.Nil(t, err)
assert.Equal(t, []*model.User{sanitized(u4)}, users)
})
t.Run("get in channel 1, offset 1, limit 2", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{
InChannelId: c1.Id,
Page: 1,
PerPage: 1,
})
require.Nil(t, err)
users_p2, err2 := ss.User().GetProfilesInChannel(&model.UserGetOptions{
InChannelId: c1.Id,
Page: 2,
PerPage: 1,
})
require.Nil(t, err2)
users = append(users, users_p2...)
assert.Equal(t, []*model.User{sanitized(u2), sanitized(u3)}, users)
})
t.Run("get in channel 2, offset 0, limit 1", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannel(c2.Id, 0, 1)
users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{
InChannelId: c2.Id,
Page: 0,
PerPage: 1,
})
require.Nil(t, err)
assert.Equal(t, []*model.User{sanitized(u1)}, users)
})
@@ -861,6 +923,15 @@ func testUserStoreGetProfilesInChannelByStatus(t *testing.T, ss store.Store, s S
u3.IsBot = true
defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }()
u4, err := ss.User().Save(&model.User{
Email: MakeEmail(),
Username: "u4" + model.NewId(),
})
require.Nil(t, err)
defer func() { require.Nil(t, ss.User().PermanentDelete(u4.Id)) }()
_, nErr = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u4.Id}, -1)
require.Nil(t, nErr)
ch1 := &model.Channel{
TeamId: teamId,
DisplayName: "Profiles in channel",
@@ -900,6 +971,17 @@ func testUserStoreGetProfilesInChannelByStatus(t *testing.T, ss store.Store, s S
})
require.Nil(t, nErr)
_, nErr = ss.Channel().SaveMember(&model.ChannelMember{
ChannelId: c1.Id,
UserId: u4.Id,
NotifyProps: model.GetDefaultChannelNotifyProps(),
})
require.Nil(t, nErr)
u4.DeleteAt = 1
_, err = ss.User().Update(u4, true)
require.Nil(t, err)
_, nErr = ss.Channel().SaveMember(&model.ChannelMember{
ChannelId: c2.Id,
UserId: u1.Id,
@@ -919,14 +1001,44 @@ func testUserStoreGetProfilesInChannelByStatus(t *testing.T, ss store.Store, s S
Status: model.STATUS_ONLINE,
}))
t.Run("get in channel 1 by status, offset 0, limit 100", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannelByStatus(c1.Id, 0, 100)
t.Run("get all users in channel 1, offset 0, limit 100", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{
InChannelId: c1.Id,
Page: 0,
PerPage: 100,
})
require.Nil(t, err)
assert.Equal(t, []*model.User{sanitized(u1), sanitized(u2), sanitized(u3), sanitized(u4)}, users)
})
t.Run("get active in channel 1 by status, offset 0, limit 100", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannelByStatus(&model.UserGetOptions{
InChannelId: c1.Id,
Page: 0,
PerPage: 100,
Active: true,
})
require.Nil(t, err)
assert.Equal(t, []*model.User{sanitized(u3), sanitized(u2), sanitized(u1)}, users)
})
t.Run("get inactive users in channel 1, offset 0, limit 100", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{
InChannelId: c1.Id,
Page: 0,
PerPage: 100,
Inactive: true,
})
require.Nil(t, err)
assert.Equal(t, []*model.User{sanitized(u4)}, users)
})
t.Run("get in channel 2 by status, offset 0, limit 1", func(t *testing.T) {
users, err := ss.User().GetProfilesInChannelByStatus(c2.Id, 0, 1)
users, err := ss.User().GetProfilesInChannelByStatus(&model.UserGetOptions{
InChannelId: c2.Id,
Page: 0,
PerPage: 1,
})
require.Nil(t, err)
assert.Equal(t, []*model.User{sanitized(u1)}, users)
})

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

@@ -7426,10 +7426,10 @@ func (s *TimerLayerUserStore) GetProfilesByUsernames(usernames []string, viewRes
return result, err
}
func (s *TimerLayerUserStore) GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) {
func (s *TimerLayerUserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
start := timemodule.Now()
result, err := s.UserStore.GetProfilesInChannel(channelId, offset, limit)
result, err := s.UserStore.GetProfilesInChannel(options)
elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
if s.Root.Metrics != nil {
@@ -7442,10 +7442,10 @@ func (s *TimerLayerUserStore) GetProfilesInChannel(channelId string, offset int,
return result, err
}
func (s *TimerLayerUserStore) GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) {
func (s *TimerLayerUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
start := timemodule.Now()
result, err := s.UserStore.GetProfilesInChannelByStatus(channelId, offset, limit)
result, err := s.UserStore.GetProfilesInChannelByStatus(options)
elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
if s.Root.Metrics != nil {