[MM-64633] Rewrite Go client using Generics (#31805)

Co-authored-by: Alejandro García Montoro <alejandro.garciamontoro@gmail.com>
This commit is contained in:
Ben Schumacher 2025-10-07 12:19:21 +02:00 committed by GitHub
parent df1c2920de
commit 71579a85a6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 1454 additions and 3657 deletions

View file

@ -98,9 +98,9 @@ func TestCreateCategoryForTeamForUser(t *testing.T) {
t.Run("should not crash with null input", func(t *testing.T) {
require.NotPanics(t, func() {
user, client := setupUserForSubtest(t, th)
payload := []byte(`null`)
payload := `null`
route := fmt.Sprintf("/users/%s/teams/%s/channels/categories", user.Id, th.BasicTeam.Id)
r, err := client.DoAPIPostBytes(context.Background(), route, payload)
r, err := client.DoAPIPost(context.Background(), route, payload)
require.Error(t, err)
closeBody(r)
})
@ -486,9 +486,9 @@ func TestUpdateCategoryForTeamForUser(t *testing.T) {
dmsCategory := categories.Categories[2]
payload := []byte(`null`)
payload := `null`
route := fmt.Sprintf("/users/%s/teams/%s/channels/categories/%s", user.Id, th.BasicTeam.Id, dmsCategory.Id)
r, err := client.DoAPIPutBytes(context.Background(), route, payload)
r, err := client.DoAPIPut(context.Background(), route, payload)
require.Error(t, err)
closeBody(r)
})
@ -988,9 +988,9 @@ func TestUpdateCategoryOrderForTeamForUser(t *testing.T) {
t.Run("should not crash with null input", func(t *testing.T) {
require.NotPanics(t, func() {
user, client := setupUserForSubtest(t, th)
payload := []byte(`null`)
payload := `null`
route := fmt.Sprintf("/users/%s/teams/%s/channels/categories/order", user.Id, th.BasicTeam.Id)
r, err := client.DoAPIPutBytes(context.Background(), route, payload)
r, err := client.DoAPIPut(context.Background(), route, payload)
require.Error(t, err)
closeBody(r)
})

View file

@ -2026,8 +2026,7 @@ func TestGetChannelsForTeamForUser(t *testing.T) {
}
channels, resp, err = client.GetChannelsForTeamForUser(context.Background(), th.BasicTeam.Id, th.BasicUser.Id, false, resp.Etag)
// an error is expected as the server didn't return any data and the client still tries to unmarshal the response
require.Error(t, err)
require.NoError(t, err)
CheckEtag(t, channels, resp)
_, resp, err = client.GetChannelsForTeamForUser(context.Background(), th.BasicTeam.Id, "junk", false, "")

View file

@ -173,7 +173,7 @@ func TestValidateBusinessEmail(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicense("cloud"))
r, err := th.SystemAdminClient.DoAPIPostBytes(context.Background(), "/cloud/validate-business-email", nil)
r, err := th.SystemAdminClient.DoAPIPost(context.Background(), "/cloud/validate-business-email", "")
require.Error(t, err)
closeBody(r)
require.Equal(t, http.StatusBadRequest, r.StatusCode, "Status Bad Request")
@ -282,7 +282,7 @@ func TestValidateWorkspaceBusinessEmail(t *testing.T) {
}()
th.App.Srv().Cloud = &cloud
r, err := th.SystemAdminClient.DoAPIPostBytes(context.Background(), "/cloud/validate-workspace-business-email", nil)
r, err := th.SystemAdminClient.DoAPIPost(context.Background(), "/cloud/validate-workspace-business-email", "")
require.Error(t, err)
closeBody(r)
require.Equal(t, http.StatusBadRequest, r.StatusCode, "Status Bad Request")

View file

@ -145,7 +145,7 @@ func TestGetPostPropertyValues(t *testing.T) {
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
require.NotNil(t, propertyValues)
require.Len(t, *propertyValues, 5)
require.Len(t, propertyValues, 5)
})
}

View file

@ -199,10 +199,10 @@ func getPostsForChannel(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
}
skipFetchThreads := r.URL.Query().Get("skipFetchThreads") == "true"
collapsedThreads := r.URL.Query().Get("collapsedThreads") == "true"
collapsedThreadsExtended := r.URL.Query().Get("collapsedThreadsExtended") == "true"
includeDeleted := r.URL.Query().Get("include_deleted") == "true"
skipFetchThreads, _ := strconv.ParseBool(r.URL.Query().Get("skipFetchThreads"))
collapsedThreads, _ := strconv.ParseBool(r.URL.Query().Get("collapsedThreads"))
collapsedThreadsExtended, _ := strconv.ParseBool(r.URL.Query().Get("collapsedThreadsExtended"))
includeDeleted, _ := strconv.ParseBool(r.URL.Query().Get("include_deleted"))
channelId := c.Params.ChannelId
page := c.Params.Page
perPage := c.Params.PerPage

View file

@ -552,7 +552,7 @@ func TestCreateUserWithToken(t *testing.T) {
_, _, err := th.Client.CreateUserWithToken(context.Background(), &user, "")
require.Error(t, err)
CheckErrorID(t, err, "api.user.create_user.missing_token.app_error")
assert.ErrorContains(t, err, "token ID is required")
})
t.Run("TokenExpired", func(t *testing.T) {
@ -902,7 +902,7 @@ func TestCreateUserWithInviteId(t *testing.T) {
_, _, err := th.Client.CreateUserWithInviteId(context.Background(), &user, "")
require.Error(t, err)
CheckErrorID(t, err, "api.user.create_user.missing_invite_id.app_error")
assert.ErrorContains(t, err, "invite ID is required")
})
t.Run("ExpiredInviteId", func(t *testing.T) {

View file

@ -123,8 +123,8 @@ type Client interface {
MigrateAuthToLdap(ctx context.Context, fromAuthService string, matchField string, force bool) (*model.Response, error)
MigrateAuthToSaml(ctx context.Context, fromAuthService string, usersMap map[string]string, auto bool) (*model.Response, error)
GetPing(ctx context.Context) (string, *model.Response, error)
GetPingWithFullServerStatus(ctx context.Context) (map[string]string, *model.Response, error)
GetPingWithOptions(ctx context.Context, options model.SystemPingOptions) (map[string]string, *model.Response, error)
GetPingWithFullServerStatus(ctx context.Context) (map[string]any, *model.Response, error)
GetPingWithOptions(ctx context.Context, options model.SystemPingOptions) (map[string]any, *model.Response, error)
CreateUpload(ctx context.Context, us *model.UploadSession) (*model.UploadSession, *model.Response, error)
GetUpload(ctx context.Context, uploadID string) (*model.UploadSession, *model.Response, error)
GetUploadsForUser(ctx context.Context, userID string) ([]*model.UploadSession, *model.Response, error)

View file

@ -173,10 +173,10 @@ Filestore Status: {{.filestore_status}}`, status)
if status["status"] != model.StatusOk {
return fmt.Errorf("server status is unhealthy: %s", status["status"])
}
if dbStatus := status["database_status"]; dbStatus != "" && dbStatus != model.StatusOk {
if dbStatus, ok := status["database_status"]; ok && dbStatus != model.StatusOk {
return fmt.Errorf("database status is unhealthy: %s", dbStatus)
}
if filestoreStatus := status["filestore_status"]; filestoreStatus != "" && filestoreStatus != model.StatusOk {
if filestoreStatus, ok := status["filestore_status"]; ok && filestoreStatus != model.StatusOk {
return fmt.Errorf("filestore status is unhealthy: %s", filestoreStatus)
}

View file

@ -160,7 +160,7 @@ func (s *MmctlUnitTestSuite) TestServerVersionCmd() {
s.Require().Nil(err)
s.Require().Len(printer.GetErrorLines(), 0)
s.Require().Len(printer.GetLines(), 1)
s.Require().Equal(printer.GetLines()[0], map[string]string{"version": expectedVersion})
s.Require().Equal(map[string]string{"version": expectedVersion}, printer.GetLines()[0])
})
s.Run("Request to the server fails", func() {
@ -183,7 +183,7 @@ func (s *MmctlUnitTestSuite) TestServerStatusCmd() {
s.Run("Print server status - all healthy", func() {
printer.Clean()
expectedStatus := map[string]string{
expectedStatus := map[string]any{
"status": model.StatusOk,
"database_status": model.StatusOk,
"filestore_status": model.StatusOk,
@ -207,7 +207,7 @@ func (s *MmctlUnitTestSuite) TestServerStatusCmd() {
s.Run("Status fields missing - should succeed", func() {
printer.Clean()
expectedStatus := map[string]string{"status": "OK"}
expectedStatus := map[string]any{"status": "OK"}
s.client.
EXPECT().
GetPingWithOptions(context.TODO(), model.SystemPingOptions{
@ -242,12 +242,11 @@ func (s *MmctlUnitTestSuite) TestServerStatusCmd() {
s.Require().Len(printer.GetLines(), 0)
})
s.Run("Empty string database status is ignored", func() {
s.Run("Missing database status is ignored", func() {
printer.Clean()
emptyDbStatus := map[string]string{
emptyDbStatus := map[string]any{
"status": model.StatusOk,
"database_status": "",
"filestore_status": model.StatusOk,
}
s.client.
@ -265,10 +264,32 @@ func (s *MmctlUnitTestSuite) TestServerStatusCmd() {
s.Require().Len(printer.GetLines(), 1)
})
s.Run("filestore database status is ignored", func() {
printer.Clean()
emptyDbStatus := map[string]any{
"status": model.StatusOk,
"database_status": model.StatusOk,
}
s.client.
EXPECT().
GetPingWithOptions(context.TODO(), model.SystemPingOptions{
FullStatus: true,
RESTSemantics: true,
}).
Return(emptyDbStatus, &model.Response{}, nil).
Times(1)
err := systemStatusCmdF(s.client, &cobra.Command{}, []string{})
s.Require().Nil(err)
s.Require().Len(printer.GetErrorLines(), 0)
s.Require().Len(printer.GetLines(), 1)
})
s.Run("Unhealthy server status should return true", func() {
printer.Clean()
unhealthyStatus := map[string]string{
unhealthyStatus := map[string]any{
"status": model.StatusUnhealthy,
"database_status": model.StatusOk,
"filestore_status": model.StatusOk,
@ -293,7 +314,7 @@ func (s *MmctlUnitTestSuite) TestServerStatusCmd() {
s.Run("Unhealthy database status should return true", func() {
printer.Clean()
unhealthyStatus := map[string]string{
unhealthyStatus := map[string]any{
"status": model.StatusOk,
"database_status": model.StatusUnhealthy,
"filestore_status": model.StatusOk,
@ -318,7 +339,7 @@ func (s *MmctlUnitTestSuite) TestServerStatusCmd() {
s.Run("Unhealthy filestore status should return true", func() {
printer.Clean()
unhealthyStatus := map[string]string{
unhealthyStatus := map[string]any{
"status": model.StatusOk,
"database_status": model.StatusOk,
"filestore_status": model.StatusUnhealthy,

View file

@ -1149,10 +1149,10 @@ func (mr *MockClientMockRecorder) GetPing(arg0 interface{}) *gomock.Call {
}
// GetPingWithFullServerStatus mocks base method.
func (m *MockClient) GetPingWithFullServerStatus(arg0 context.Context) (map[string]string, *model.Response, error) {
func (m *MockClient) GetPingWithFullServerStatus(arg0 context.Context) (map[string]interface{}, *model.Response, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetPingWithFullServerStatus", arg0)
ret0, _ := ret[0].(map[string]string)
ret0, _ := ret[0].(map[string]interface{})
ret1, _ := ret[1].(*model.Response)
ret2, _ := ret[2].(error)
return ret0, ret1, ret2
@ -1165,10 +1165,10 @@ func (mr *MockClientMockRecorder) GetPingWithFullServerStatus(arg0 interface{})
}
// GetPingWithOptions mocks base method.
func (m *MockClient) GetPingWithOptions(arg0 context.Context, arg1 model.SystemPingOptions) (map[string]string, *model.Response, error) {
func (m *MockClient) GetPingWithOptions(arg0 context.Context, arg1 model.SystemPingOptions) (map[string]interface{}, *model.Response, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetPingWithOptions", arg0, arg1)
ret0, _ := ret[0].(map[string]string)
ret0, _ := ret[0].(map[string]interface{})
ret1, _ := ret[1].(*model.Response)
ret2, _ := ret[2].(error)
return ret0, ret1, ret2

File diff suppressed because it is too large Load diff

View file

@ -17,6 +17,7 @@ import (
"github.com/mattermost/mattermost/server/public/model"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// https://github.com/mattermost/mattermost-plugin-starter-template/issues/115
@ -380,3 +381,43 @@ func ExampleClient4_GetUsers() {
page++
}
}
func TestBuildResponse(t *testing.T) {
t.Run("handles nil http.Response", func(t *testing.T) {
response := model.BuildResponse(nil)
assert.Nil(t, response)
})
t.Run("builds response from http.Response", func(t *testing.T) {
httpResp := &http.Response{
StatusCode: http.StatusOK,
Header: make(http.Header),
}
httpResp.Header.Set(model.HeaderRequestId, "test-request-id")
httpResp.Header.Set(model.HeaderEtagServer, "test-etag")
httpResp.Header.Set(model.HeaderVersionId, "test-version")
response := model.BuildResponse(httpResp)
require.NotNil(t, response)
assert.Equal(t, http.StatusOK, response.StatusCode)
assert.Equal(t, "test-request-id", response.RequestId)
assert.Equal(t, "test-etag", response.Etag)
assert.Equal(t, "test-version", response.ServerVersion)
assert.Equal(t, httpResp.Header, response.Header)
})
t.Run("handles response with empty headers", func(t *testing.T) {
httpResp := &http.Response{
StatusCode: http.StatusNoContent,
Header: http.Header{},
}
response := model.BuildResponse(httpResp)
require.NotNil(t, response)
assert.Equal(t, http.StatusNoContent, response.StatusCode)
assert.Empty(t, response.RequestId)
assert.Empty(t, response.Etag)
assert.Empty(t, response.ServerVersion)
assert.Equal(t, httpResp.Header, response.Header)
})
}