MM-44088: Add teamID filter to channelMembers (#20176)

We add 2 new params to channel members query.
1. Filter by teamId.
2. Negate that filter.

We include some more optimizations like:
- Moved the team role checks inside the dataloader.
- Moved the channel pretty name computation inside the loader.

Now that we load less data on initial load, we can reduce
the concurrency requirement to be a bit on the safer side.

```release-note
NONE
```
Этот коммит содержится в:
Agniva De Sarker
2022-05-11 13:54:12 +05:30
коммит произвёл GitHub
родитель 5ac3dbf058
Коммит a6d8e45297
14 изменённых файлов: 225 добавлений и 75 удалений

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

@@ -34,7 +34,7 @@ const (
usersLoaderCtx ctxKey = 4
)
const loaderBatchCapacity = web.PerPageMaximum + 100
const loaderBatchCapacity = web.PerPageMaximum
//go:embed schema.graphqls
var schemaRaw string

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

@@ -11,6 +11,7 @@ import (
"github.com/graph-gophers/dataloader/v6"
"github.com/mattermost/mattermost-server/v6/app"
"github.com/mattermost/mattermost-server/v6/model"
"github.com/mattermost/mattermost-server/v6/store"
"github.com/mattermost/mattermost-server/v6/web"
)
@@ -211,7 +212,9 @@ func (*resolver) ChannelsLeft(ctx context.Context, args struct {
// match with api4.getChannelMember
func (*resolver) ChannelMembers(ctx context.Context, args struct {
UserID string
TeamID string
ChannelID string
ExcludeTeam bool
First int32
After string
LastUpdateAt float64
@@ -263,7 +266,29 @@ func (*resolver) ChannelMembers(ctx context.Context, args struct {
}
}
members, err := c.App.Srv().Store.Channel().GetMembersForUserWithCursor(args.UserID, afterChannel, afterUser, limit, int(args.LastUpdateAt))
if args.TeamID != "" {
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), args.TeamID, model.PermissionViewTeam) {
primaryTeam := *c.App.Config().TeamSettings.ExperimentalPrimaryTeam
if primaryTeam != "" {
team, appErr := c.App.GetTeamByName(primaryTeam)
if appErr != nil {
return []*channelMember{}, appErr
}
args.TeamID = team.Id
} else {
return []*channelMember{}, nil
}
}
}
opts := &store.ChannelMemberGraphQLSearchOpts{
AfterChannel: afterChannel,
AfterUser: afterUser,
Limit: limit,
LastUpdateAt: int(args.LastUpdateAt),
ExcludeTeam: args.ExcludeTeam,
}
members, err := c.App.Srv().Store.Channel().GetMembersForUserWithCursor(args.UserID, args.TeamID, opts)
if err != nil {
return nil, err
}

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

@@ -41,7 +41,7 @@ func (cm *channelMember) Channel(ctx context.Context) (*channel, error) {
if err != nil {
return nil, err
}
channel := result.(*model.Channel)
channel := result.(*channel)
if channel.Type == model.ChannelTypeOpen {
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionReadPublicChannel) &&
@@ -56,20 +56,7 @@ func (cm *channelMember) Channel(ctx context.Context) (*channel, error) {
}
}
appErr := c.App.FillInChannelProps(channel)
if appErr != nil {
return nil, appErr
}
res, err := postProcessChannels(c, []*model.Channel{channel})
if err != nil {
return nil, err
}
// A bit of defence-in-depth; can probably be removed after a deeper look.
if len(res) != 1 {
return nil, fmt.Errorf("postProcessChannels: incorrect number of channels returned %d", len(res))
}
return res[0], nil
return channel, nil
}
func graphQLChannelsLoader(ctx context.Context, keys dataloader.Keys) []*dataloader.Result {
@@ -98,7 +85,7 @@ func graphQLChannelsLoader(ctx context.Context, keys dataloader.Keys) []*dataloa
return result
}
func getGraphQLChannels(c *web.Context, channelIDs []string) ([]*model.Channel, error) {
func getGraphQLChannels(c *web.Context, channelIDs []string) ([]*channel, error) {
channels, appErr := c.App.GetChannels(channelIDs)
if appErr != nil {
return nil, appErr
@@ -108,18 +95,28 @@ func getGraphQLChannels(c *web.Context, channelIDs []string) ([]*model.Channel,
return nil, fmt.Errorf("all channels were not found. Requested %d; Found %d", len(channelIDs), len(channels))
}
appErr = c.App.FillInChannelsProps(model.ChannelList(channels))
if appErr != nil {
return nil, appErr
}
res, err := postProcessChannels(c, channels)
if err != nil {
return nil, err
}
// The channels need to be in the exact same order as the input slice.
tmp := make(map[string]*model.Channel)
for _, ch := range channels {
tmp := make(map[string]*channel)
for _, ch := range res {
tmp[ch.Id] = ch
}
// We reuse the same slice and just rewrite the channels.
for i, id := range channelIDs {
channels[i] = tmp[id]
res[i] = tmp[id]
}
return channels, nil
return res, nil
}
func (cm *channelMember) Roles_(ctx context.Context) ([]*model.Role, error) {

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

@@ -292,6 +292,45 @@ func TestGraphQLChannelMembers(t *testing.T) {
require.Len(t, resp.Errors, 1)
})
t.Run("team_filter", func(t *testing.T) {
query := `query channelMembers($teamId: String, $excludeTeam: Boolean = false) {
channelMembers(userId: "me", teamId: $teamId, excludeTeam: $excludeTeam) {
channel {
id
}
}
}
`
input := graphQLInput{
OperationName: "channelMembers",
Query: query,
Variables: map[string]interface{}{
"teamId": th.BasicTeam.Id,
},
}
resp, err := th.MakeGraphQLRequest(&input)
require.NoError(t, err)
require.Len(t, resp.Errors, 0)
require.NoError(t, json.Unmarshal(resp.Data, &q))
assert.Len(t, q.ChannelMembers, 5)
input = graphQLInput{
OperationName: "channelMembers",
Query: query,
Variables: map[string]interface{}{
"teamId": th.BasicTeam.Id,
"excludeTeam": true,
},
}
resp, err = th.MakeGraphQLRequest(&input)
require.NoError(t, err)
require.Len(t, resp.Errors, 0)
require.NoError(t, json.Unmarshal(resp.Data, &q))
assert.Len(t, q.ChannelMembers, 4)
})
t.Run("UpdateAt", func(t *testing.T) {
query := `query channelMembers($first: Int, $after: String = "", $lastUpdateAt: Float) {
channelMembers(userId: "me", first: $first, after: $after, lastUpdateAt: $lastUpdateAt) {

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

@@ -14,11 +14,6 @@ import (
)
func getGraphQLTeam(ctx context.Context, id string) (*model.Team, error) {
c, err := getCtx(ctx)
if err != nil {
return nil, err
}
loader, err := getTeamsLoader(ctx)
if err != nil {
return nil, err
@@ -30,15 +25,6 @@ func getGraphQLTeam(ctx context.Context, id string) (*model.Team, error) {
return nil, err
}
team := result.(*model.Team)
team = team.ShallowCopy()
if (!team.AllowOpenInvite || team.Type != model.TeamOpen) &&
!c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), team.Id, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return nil, c.Err
}
team = c.App.SanitizeTeam(*c.AppContext.Session(), team)
return team, nil
}
@@ -78,6 +64,18 @@ func getGraphQLTeams(c *web.Context, teamIDs []string) ([]*model.Team, error) {
return nil, fmt.Errorf("All teams were not found. Requested %d; Found %d", len(teamIDs), len(teams))
}
// We pre-calculate this so that it's not computed in separate goroutines outside
// the dataloader.
for i := range teams {
if (!teams[i].AllowOpenInvite || teams[i].Type != model.TeamOpen) &&
!c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), teams[i].Id, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return nil, c.Err
}
teams[i] = c.App.SanitizeTeam(*c.AppContext.Session(), teams[i])
}
// The teams need to be in the exact same order as the input slice.
tmp := make(map[string]*model.Team)
for _, ch := range teams {

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

@@ -19,6 +19,8 @@ type Query {
since: Float!): [String!]!
channelMembers(userId: String!,
channelId: String = "",
teamId: String = "",
excludeTeam: Boolean = false,
first: Int = 60,
after: String = "",
lastUpdateAt: Float = 0): [ChannelMember]!