Skip to content

Commit

Permalink
MM-58756: paginate webhooks list (mattermost#27368)
Browse files Browse the repository at this point in the history 
* MM-58756: paginate webhooks list

* show error if webhook list fails
  • Loading branch information
@lieut-data
lieut-data authored Jul 17, 2024
1 parent b25820b commit 9304c40
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 39 deletions.
13 changes: 8 additions & 5 deletions server/cmd/mmctl/commands/webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,19 @@ func listWebhookCmdF(c client.Client, command *cobra.Command, args []string) err
continue
}

// Fetch all hooks with a very large limit so we get them all.
incomingResult := make(chan StoreResult, 1)
go func() {
incomingHooks, _, err := c.GetIncomingWebhooksForTeam(context.TODO(), team.Id, 0, 100000000, "")
incomingHooks, err := getPages(func(page, numPerPage int, etag string) ([]*model.IncomingWebhook, *model.Response, error) {
return c.GetIncomingWebhooksForTeam(context.TODO(), team.Id, page, numPerPage, etag)
}, DefaultPageSize)
incomingResult <- StoreResult{Data: incomingHooks, Err: err}
close(incomingResult)
}()
outgoingResult := make(chan StoreResult, 1)
go func() {
outgoingHooks, _, err := c.GetOutgoingWebhooksForTeam(context.TODO(), team.Id, 0, 100000000, "")
outgoingHooks, err := getPages(func(page, numPerPage int, etag string) ([]*model.OutgoingWebhook, *model.Response, error) {
return c.GetOutgoingWebhooksForTeam(context.TODO(), team.Id, page, numPerPage, etag)
}, DefaultPageSize)
outgoingResult <- StoreResult{Data: outgoingHooks, Err: err}
close(outgoingResult)
}()
Expand All @@ -128,7 +131,7 @@ func listWebhookCmdF(c client.Client, command *cobra.Command, args []string) err
printer.PrintT("Incoming:\t{{.DisplayName}} ({{.Id}}", hook)
}
} else {
printer.PrintError("Unable to list incoming webhooks for '" + team.Id + "'")
printer.PrintError("Unable to list incoming webhooks for '" + team.Id + "': " + result.Err.Error())
}

if result := <-outgoingResult; result.Err == nil {
Expand All @@ -137,7 +140,7 @@ func listWebhookCmdF(c client.Client, command *cobra.Command, args []string) err
printer.PrintT("Outgoing:\t {{.DisplayName}} ({{.Id}})", hook)
}
} else {
printer.PrintError("Unable to list outgoing webhooks for '" + team.Id + "'")
printer.PrintError("Unable to list outgoing webhooks for '" + team.Id + "': " + result.Err.Error())
}
}

Expand Down
147 changes: 113 additions & 34 deletions server/cmd/mmctl/commands/webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (
"net/http"
"strconv"

gomock "github.com/golang/mock/gomock"
"github.com/mattermost/mattermost/server/public/model"
"github.com/pkg/errors"

"github.com/mattermost/mattermost/server/v8/cmd/mmctl/mocks"
"github.com/mattermost/mattermost/server/v8/cmd/mmctl/printer"

"github.com/spf13/cobra"
Expand All @@ -23,20 +25,41 @@ func (s *MmctlUnitTestSuite) TestListWebhookCmd() {
outgoingWebhookID := "outgoingWebhookID"
outgoingWebhookDisplayName := "outgoingWebhookDisplayName"

mockIncomingWebhook := model.IncomingWebhook{
Id: incomingWebhookID,
DisplayName: incomingWebhookDisplayName,
}
mockOutgoingWebhook := model.OutgoingWebhook{
Id: outgoingWebhookID,
DisplayName: outgoingWebhookDisplayName,
}

var mockIncomingWebhooksPage1 []*model.IncomingWebhook
for i := 0; i < 200; i++ {
mockIncomingWebhooksPage1 = append(mockIncomingWebhooksPage1, &mockIncomingWebhook)
}

var mockIncomingWebhooksPage2 []*model.IncomingWebhook
for i := 0; i < 50; i++ {
mockIncomingWebhooksPage2 = append(mockIncomingWebhooksPage2, &mockIncomingWebhook)
}

var mockOutgoingWebhooksPage1 []*model.OutgoingWebhook
for i := 0; i < 200; i++ {
mockOutgoingWebhooksPage1 = append(mockOutgoingWebhooksPage1, &mockOutgoingWebhook)
}

var mockOutgoingWebhooksPage2 []*model.OutgoingWebhook
for i := 0; i < 50; i++ {
mockOutgoingWebhooksPage2 = append(mockOutgoingWebhooksPage2, &mockOutgoingWebhook)
}

s.Run("Listing all webhooks", func() {
printer.Clean()

mockTeam := model.Team{
Id: teamID,
}
mockIncomingWebhook := model.IncomingWebhook{
Id: incomingWebhookID,
DisplayName: incomingWebhookDisplayName,
}
mockOutgoingWebhook := model.OutgoingWebhook{
Id: outgoingWebhookID,
DisplayName: outgoingWebhookDisplayName,
}

s.client.
EXPECT().
Expand All @@ -52,38 +75,63 @@ func (s *MmctlUnitTestSuite) TestListWebhookCmd() {

s.client.
EXPECT().
GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
Return([]*model.IncomingWebhook{&mockIncomingWebhook}, &model.Response{}, nil).
GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
Return(mockIncomingWebhooksPage1, &model.Response{}, nil).
Times(1)

s.client.
EXPECT().
GetIncomingWebhooksForTeam(context.TODO(), teamID, 1, 200, "").
Return(mockIncomingWebhooksPage2, &model.Response{}, nil).
Times(1)

s.client.
EXPECT().
GetIncomingWebhooksForTeam(context.TODO(), teamID, 2, 200, "").
Return(nil, &model.Response{}, nil).
Times(1)

s.client.
EXPECT().
GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
Return(mockOutgoingWebhooksPage1, &model.Response{}, nil).
Times(1)

s.client.
EXPECT().
GetOutgoingWebhooksForTeam(context.TODO(), teamID, 1, 200, "").
Return(mockOutgoingWebhooksPage2, &model.Response{}, nil).
Times(1)

s.client.
EXPECT().
GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
Return([]*model.OutgoingWebhook{&mockOutgoingWebhook}, &model.Response{}, nil).
GetOutgoingWebhooksForTeam(context.TODO(), teamID, 2, 200, "").
Return(nil, &model.Response{}, nil).
Times(1)

err := listWebhookCmdF(s.client, &cobra.Command{}, []string{})
s.Require().Nil(err)
s.Len(printer.GetLines(), 2)
if s.Len(printer.GetLines(), 500) {
for i := 0; i < 250; i++ {
s.Require().Equal(&mockIncomingWebhook, printer.GetLines()[i])
}
for i := 250; i < 500; i++ {
s.Require().Equal(&mockOutgoingWebhook, printer.GetLines()[i])
}
}

s.Len(printer.GetErrorLines(), 0)
s.Require().Equal(&mockIncomingWebhook, printer.GetLines()[0])
s.Require().Equal(&mockOutgoingWebhook, printer.GetLines()[1])
})

s.Run("List webhooks by team", func() {
printer.Clean()
s.mockCtrl = gomock.NewController(s.T())
s.client = mocks.NewMockClient(s.mockCtrl)

mockTeam := model.Team{
Id: teamID,
}
mockIncomingWebhook := model.IncomingWebhook{
Id: incomingWebhookID,
DisplayName: incomingWebhookDisplayName,
}
mockOutgoingWebhook := model.OutgoingWebhook{
Id: outgoingWebhookID,
DisplayName: outgoingWebhookDisplayName,
}

s.client.
EXPECT().
GetTeam(context.TODO(), teamID, "").
Expand All @@ -92,26 +140,57 @@ func (s *MmctlUnitTestSuite) TestListWebhookCmd() {

s.client.
EXPECT().
GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
Return([]*model.IncomingWebhook{&mockIncomingWebhook}, &model.Response{}, nil).
GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
Return(mockIncomingWebhooksPage1, &model.Response{}, nil).
Times(1)

s.client.
EXPECT().
GetIncomingWebhooksForTeam(context.TODO(), teamID, 1, 200, "").
Return(mockIncomingWebhooksPage2, &model.Response{}, nil).
Times(1)

s.client.
EXPECT().
GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
Return([]*model.OutgoingWebhook{&mockOutgoingWebhook}, &model.Response{}, nil).
GetIncomingWebhooksForTeam(context.TODO(), teamID, 2, 200, "").
Return(nil, &model.Response{}, nil).
Times(1)

s.client.
EXPECT().
GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
Return(mockOutgoingWebhooksPage1, &model.Response{}, nil).
Times(1)

s.client.
EXPECT().
GetOutgoingWebhooksForTeam(context.TODO(), teamID, 1, 200, "").
Return(mockOutgoingWebhooksPage2, &model.Response{}, nil).
Times(1)

s.client.
EXPECT().
GetOutgoingWebhooksForTeam(context.TODO(), teamID, 2, 200, "").
Return(nil, &model.Response{}, nil).
Times(1)

err := listWebhookCmdF(s.client, &cobra.Command{}, []string{teamID})
s.Require().Nil(err)
s.Len(printer.GetLines(), 2)
s.Len(printer.GetErrorLines(), 0)
s.Require().Equal(&mockIncomingWebhook, printer.GetLines()[0])
s.Require().Equal(&mockOutgoingWebhook, printer.GetLines()[1])
if s.Len(printer.GetLines(), 500) {
for i := 0; i < 250; i++ {
s.Require().Equal(&mockIncomingWebhook, printer.GetLines()[i])
}
for i := 250; i < 500; i++ {
s.Require().Equal(&mockOutgoingWebhook, printer.GetLines()[i])
}
}
})

s.Run("Unable to list webhooks", func() {
printer.Clean()
s.mockCtrl = gomock.NewController(s.T())
s.client = mocks.NewMockClient(s.mockCtrl)

mockTeam := model.Team{
Id: teamID,
Expand All @@ -132,22 +211,22 @@ func (s *MmctlUnitTestSuite) TestListWebhookCmd() {

s.client.
EXPECT().
GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
Return(nil, &model.Response{}, mockError).
Times(1)

s.client.
EXPECT().
GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
Return(nil, &model.Response{}, mockError).
Times(1)

err := listWebhookCmdF(s.client, &cobra.Command{}, []string{})
s.Require().Nil(err)
s.Len(printer.GetLines(), 0)
s.Len(printer.GetErrorLines(), 2)
s.Require().Equal("Unable to list incoming webhooks for '"+teamID+"'", printer.GetErrorLines()[0])
s.Require().Equal("Unable to list outgoing webhooks for '"+teamID+"'", printer.GetErrorLines()[1])
s.Require().Equal("Unable to list incoming webhooks for '"+teamID+"': mock error", printer.GetErrorLines()[0])
s.Require().Equal("Unable to list outgoing webhooks for '"+teamID+"': mock error", printer.GetErrorLines()[1])
})
}

Expand Down

0 comments on commit 9304c40

Please sign in to comment.