diff --git a/.circleci/config.yml b/.circleci/config.yml index d2c96cf9d8..fc344c5fce 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -30,7 +30,7 @@ jobs: if [ $CURL_FAILED -eq 1 ] then - npm ci && cd node_modules/mattermost-redux && npm install && npm run build && cd ../.. && make build + npm ci && make build fi - persist_to_workspace: root: /go/src/github.com/mattermost @@ -232,7 +232,7 @@ jobs: --env MM_ELASTICSEARCHSETTINGS_CONNECTIONURL=http://elasticsearch:9200 \ -v ~/go/src:/go/src \ -w /go/src/github.com/mattermost/mattermost-server \ - mattermost/mattermost-build-server:feb-28-2019 \ + mattermost/mattermost-build-server:oct-18-2019 \ bash -c 'ulimit -n 8096; make ARGS="version" run-cli && make MM_SQLSETTINGS_DATASOURCE="mmuser:mostest@tcp(mysql:3306)/latest?charset=utf8mb4,utf8&readTimeout=30s&writeTimeout=30s" ARGS="version" run-cli' echo "Ignoring known MySQL mismatch: ChannelMembers.SchemeGuest" diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..6788712517 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,32 @@ +run: + timeout: 5m + modules-download-mode: vendor + +linters-settings: + govet: + check-shadowing: true + gofmt: + simplify: true + +linters: + disable-all: true + enable: + - deadcode + - gofmt + - gosimple + - govet + - ineffassign + - structcheck + - unconvert + - unused + - varcheck + # TODO: enable this later + # - errcheck + +issues: + exclude-rules: + - linters: + # ignore unused warnings from enterprise code + # add more as required. + - unused + text: "RedisSupplier|LocalCacheSupplier|Enterprise" diff --git a/Makefile b/Makefile index f799b8e966..5b159471d0 100644 --- a/Makefile +++ b/Makefile @@ -173,6 +173,18 @@ gofmt: ## Runs gofmt against all packages. done @echo "gofmt success"; \ +golangci-lint: +# https://stackoverflow.com/a/677212/1027058 (check if a command exists or not) +# https://github.com/golangci/golangci-lint#binary-release +# It is recommended to NOT use go get, but instead use a binary release pinned to a version. + @if ! [ -x "$$(command -v golangci-lint)" ]; then \ + echo "golangci-lint is not installed. Please run: curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(GOPATH)/bin v1.21.0"; \ + exit 1; \ + fi; \ + + @echo Running golangci-lint + $(GOPATH)/bin/golangci-lint run + megacheck: ## Run megacheck on codebasis env GO111MODULE=off go get -u honnef.co/go/tools/cmd/megacheck $(GOPATH)/bin/megacheck $(TE_PACKAGES) @@ -219,6 +231,7 @@ check-licenses: ## Checks license status. check-prereqs: ## Checks prerequisite software status. ./scripts/prereq-check.sh +# TODO: remove govet and gofmt checks once golangci-lint is being enforced. check-style: govet gofmt check-licenses ## Runs govet and gofmt against all packages. test-te-race: ## Checks for race conditions in the team edition. diff --git a/api4/apitestlib.go b/api4/apitestlib.go index ba0a3fb24f..8be1af367f 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -9,8 +9,6 @@ import ( "net" "net/http" "os" - "reflect" - "strconv" "strings" "testing" "time" @@ -26,6 +24,7 @@ import ( s3 "github.com/minio/minio-go/v6" "github.com/minio/minio-go/v6/pkg/credentials" + "github.com/stretchr/testify/require" ) type TestHelper struct { @@ -560,57 +559,34 @@ func GenerateTestId() string { func CheckUserSanitization(t *testing.T, user *model.User) { t.Helper() - if user.Password != "" { - t.Fatal("password wasn't blank") - } - - if user.AuthData != nil && *user.AuthData != "" { - t.Fatal("auth data wasn't blank") - } - - if user.MfaSecret != "" { - t.Fatal("mfa secret wasn't blank") - } + require.Equal(t, "", user.Password, "password wasn't blank") + require.Empty(t, user.AuthData, "auth data wasn't blank") + require.Equal(t, "", user.MfaSecret, "mfa secret wasn't blank") } func CheckEtag(t *testing.T, data interface{}, resp *model.Response) { t.Helper() - if !reflect.ValueOf(data).IsNil() { - t.Fatal("etag data was not nil") - } - - if resp.StatusCode != http.StatusNotModified { - t.Log("actual: " + strconv.Itoa(resp.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusNotModified)) - t.Fatal("wrong status code for etag") - } + require.Empty(t, data) + require.Equal(t, resp.StatusCode, http.StatusNotModified, "wrong status code for etag") } func CheckNoError(t *testing.T, resp *model.Response) { t.Helper() - if resp.Error != nil { - t.Fatalf("Expected no error, got %q", resp.Error.Error()) - } + require.Nil(t, resp.Error) } func checkHTTPStatus(t *testing.T, resp *model.Response, expectedStatus int, expectError bool) { t.Helper() - switch { - case resp == nil: - t.Fatalf("Unexpected nil response, expected http:%v, expectError:%v)", expectedStatus, expectError) - - case expectError && resp.Error == nil: - t.Fatalf("Expected a non-nil error and http status:%v, got nil, %v", expectedStatus, resp.StatusCode) - - case !expectError && resp.Error != nil: - t.Fatalf("Expected no error and http status:%v, got %q, http:%v", expectedStatus, resp.Error, resp.StatusCode) - - case resp.StatusCode != expectedStatus: - t.Fatalf("Expected http status:%v, got %v (err: %q)", expectedStatus, resp.StatusCode, resp.Error) + require.NotNilf(t, resp, "Unexpected nil response, expected http:%v, expectError:%v", expectedStatus, expectError) + if expectError { + require.NotNil(t, resp.Error, "Expected a non-nil error and http status:%v, got nil, %v", expectedStatus, resp.StatusCode) + } else { + require.Nil(t, resp.Error, "Expected no error and http status:%v, got %q, http:%v", expectedStatus, resp.Error, resp.StatusCode) } + require.Equalf(t, expectedStatus, resp.StatusCode, "Expected http status:%v, got %v (err: %q)", expectedStatus, resp.StatusCode, resp.Error) } func CheckOKStatus(t *testing.T, resp *model.Response) { @@ -661,16 +637,12 @@ func CheckInternalErrorStatus(t *testing.T, resp *model.Response) { func CheckErrorMessage(t *testing.T, resp *model.Response, errorId string) { t.Helper() - if resp.Error == nil { - t.Fatal("should have errored with message:" + errorId) - return - } + require.NotNil(t, resp.Error) + require.Equal(t, resp.Error.Id, errorId, "incorrect error message") +} - if resp.Error.Id != errorId { - t.Log("actual: " + resp.Error.Id) - t.Log("expected: " + errorId) - t.Fatal("incorrect error message") - } +func CheckStartsWith(t *testing.T, value, prefix, message string) { + require.True(t, strings.HasPrefix(value, prefix), message, value) } // Similar to s3.New() but allows initialization of signature v2 or signature v4 client. diff --git a/api4/channel_test.go b/api4/channel_test.go index 893c69bfcc..3b634cd84a 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -10,6 +10,7 @@ import ( "sort" "strconv" "strings" + "sync" "testing" "time" @@ -2502,15 +2503,80 @@ func TestRemoveChannelMember(t *testing.T) { _, resp = Client.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser.Id) CheckForbiddenStatus(t, resp) - th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel) - _, resp = Client.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser2.Id) - CheckNoError(t, resp) + t.Run("success", func(t *testing.T) { + // Setup the system administrator to listen for websocket events from the channels. + th.LinkUserToTeam(th.SystemAdminUser, th.BasicTeam) + _, err := th.App.AddUserToChannel(th.SystemAdminUser, th.BasicChannel) + require.Nil(t, err) + _, err = th.App.AddUserToChannel(th.SystemAdminUser, th.BasicChannel2) + require.Nil(t, err) + props := map[string]string{} + props[model.DESKTOP_NOTIFY_PROP] = model.CHANNEL_NOTIFY_ALL + _, resp = th.SystemAdminClient.UpdateChannelNotifyProps(th.BasicChannel.Id, th.SystemAdminUser.Id, props) + _, resp = th.SystemAdminClient.UpdateChannelNotifyProps(th.BasicChannel2.Id, th.SystemAdminUser.Id, props) + CheckNoError(t, resp) - _, resp = Client.RemoveUserFromChannel(th.BasicChannel2.Id, th.BasicUser.Id) - CheckNoError(t, resp) + wsClient, err := th.CreateWebSocketSystemAdminClient() + require.Nil(t, err) + wsClient.Listen() + var closeWsClient sync.Once + defer closeWsClient.Do(func() { + wsClient.Close() + }) - _, resp = th.SystemAdminClient.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser.Id) - CheckNoError(t, resp) + wsr := <-wsClient.EventChannel + require.Equal(t, wsr.Event, model.WEBSOCKET_EVENT_HELLO) + + // requirePost listens for websocket events and tries to find the post matching + // the expected post's channel and message. + requirePost := func(expectedPost *model.Post) { + t.Helper() + for { + select { + case event := <-wsClient.EventChannel: + postData, ok := event.Data["post"] + if !ok { + continue + } + + post := model.PostFromJson(strings.NewReader(postData.(string))) + if post.ChannelId == expectedPost.ChannelId && post.Message == expectedPost.Message { + return + } + case <-time.After(5 * time.Second): + t.Fatal("failed to find expected post after 5 seconds") + return + } + } + } + + th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel) + _, resp = Client.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser2.Id) + CheckNoError(t, resp) + + requirePost(&model.Post{ + Message: fmt.Sprintf("@%s left the channel.", th.BasicUser2.Username), + ChannelId: th.BasicChannel.Id, + }) + + _, resp = Client.RemoveUserFromChannel(th.BasicChannel2.Id, th.BasicUser.Id) + CheckNoError(t, resp) + requirePost(&model.Post{ + Message: fmt.Sprintf("@%s removed from the channel.", th.BasicUser.Username), + ChannelId: th.BasicChannel2.Id, + }) + + _, resp = th.SystemAdminClient.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser.Id) + CheckNoError(t, resp) + requirePost(&model.Post{ + Message: fmt.Sprintf("@%s removed from the channel.", th.BasicUser.Username), + ChannelId: th.BasicChannel.Id, + }) + + closeWsClient.Do(func() { + wsClient.Close() + }) + }) // Leave deleted channel th.LoginBasic() diff --git a/api4/command_help_test.go b/api4/command_help_test.go index c5ff819b3f..87a223e8dd 100644 --- a/api4/command_help_test.go +++ b/api4/command_help_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/mattermost/mattermost-server/model" + "github.com/stretchr/testify/assert" ) func TestHelpCommand(t *testing.T) { @@ -23,15 +24,11 @@ func TestHelpCommand(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.SupportSettings.HelpLink = "" }) rs1, _ := Client.ExecuteCommand(channel.Id, "/help ") - if rs1.GotoLocation != model.SUPPORT_SETTINGS_DEFAULT_HELP_LINK { - t.Fatal("failed to default help link") - } + assert.Equal(t, rs1.GotoLocation, model.SUPPORT_SETTINGS_DEFAULT_HELP_LINK, "failed to default help link") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.SupportSettings.HelpLink = "https://docs.mattermost.com/guides/user.html" }) rs2, _ := Client.ExecuteCommand(channel.Id, "/help ") - if rs2.GotoLocation != "https://docs.mattermost.com/guides/user.html" { - t.Fatal("failed to help link") - } + assert.Equal(t, rs2.GotoLocation, "https://docs.mattermost.com/guides/user.html", "failed to help link") } diff --git a/api4/command_test.go b/api4/command_test.go index f091c0dbe1..7dddd313b1 100644 --- a/api4/command_test.go +++ b/api4/command_test.go @@ -39,12 +39,8 @@ func TestCreateCommand(t *testing.T) { createdCmd, resp := th.SystemAdminClient.CreateCommand(newCmd) CheckNoError(t, resp) CheckCreatedStatus(t, resp) - if createdCmd.CreatorId != th.SystemAdminUser.Id { - t.Fatal("user ids didn't match") - } - if createdCmd.TeamId != th.BasicTeam.Id { - t.Fatal("team ids didn't match") - } + require.Equal(t, th.SystemAdminUser.Id, createdCmd.CreatorId, "user ids didn't match") + require.Equal(t, th.BasicTeam.Id, createdCmd.TeamId, "team ids didn't match") _, resp = th.SystemAdminClient.CreateCommand(newCmd) CheckBadRequestStatus(t, resp) @@ -100,34 +96,22 @@ func TestUpdateCommand(t *testing.T) { rcmd, resp := Client.UpdateCommand(cmd2) CheckNoError(t, resp) - if rcmd.Trigger != cmd2.Trigger { - t.Fatal("Trigger should have updated") - } + require.Equal(t, cmd2.Trigger, rcmd.Trigger, "Trigger should have updated") - if rcmd.Method != cmd2.Method { - t.Fatal("Method should have updated") - } + require.Equal(t, cmd2.Method, rcmd.Method, "Method should have updated") - if rcmd.URL != cmd2.URL { - t.Fatal("URL should have updated") - } + require.Equal(t, cmd2.URL, rcmd.URL, "URL should have updated") - if rcmd.CreatorId != cmd1.CreatorId { - t.Fatal("CreatorId should have not updated") - } + require.Equal(t, cmd1.CreatorId, rcmd.CreatorId, "CreatorId should have not updated") - if rcmd.Token != cmd1.Token { - t.Fatal("Token should have not updated") - } + require.Equal(t, cmd1.Token, rcmd.Token, "Token should have not updated") cmd2.Id = GenerateTestId() rcmd, resp = Client.UpdateCommand(cmd2) CheckNotFoundStatus(t, resp) - if rcmd != nil { - t.Fatal("should be empty") - } + require.Nil(t, rcmd, "should be empty") cmd2.Id = "junk" @@ -176,21 +160,15 @@ func TestDeleteCommand(t *testing.T) { ok, resp := Client.DeleteCommand(rcmd1.Id) CheckNoError(t, resp) - if !ok { - t.Fatal("should have returned true") - } + require.True(t, ok) rcmd1, _ = th.App.GetCommand(rcmd1.Id) - if rcmd1 != nil { - t.Fatal("should be nil") - } + require.Nil(t, rcmd1) ok, resp = Client.DeleteCommand("junk") CheckBadRequestStatus(t, resp) - if ok { - t.Fatal("should have returned false") - } + require.False(t, ok) _, resp = Client.DeleteCommand(GenerateTestId()) CheckNotFoundStatus(t, resp) @@ -248,24 +226,16 @@ func TestListCommands(t *testing.T) { foundCustom = true } } - if !foundEcho { - t.Fatal("Couldn't find echo command") - } - if !foundCustom { - t.Fatal("Should list the custom command") - } + require.True(t, foundEcho, "Couldn't find echo command") + require.True(t, foundCustom, "Should list the custom command") }) t.Run("ListCustomOnlyCommands", func(t *testing.T) { listCommands, resp := th.SystemAdminClient.ListCommands(th.BasicTeam.Id, true) CheckNoError(t, resp) - if len(listCommands) > 1 { - t.Fatal("Should list just one custom command") - } - if listCommands[0].Trigger != "custom_command" { - t.Fatal("Wrong custom command trigger") - } + require.Len(t, listCommands, 1, "Should list just one custom command") + require.Equal(t, listCommands[0].Trigger, "custom_command", "Wrong custom command trigger") }) t.Run("UserWithNoPermissionForCustomCommands", func(t *testing.T) { @@ -287,12 +257,8 @@ func TestListCommands(t *testing.T) { foundCustom = true } } - if !foundEcho { - t.Fatal("Couldn't find echo command") - } - if foundCustom { - t.Fatal("Should not list the custom command") - } + require.True(t, foundEcho, "Couldn't find echo command") + require.False(t, foundCustom, "Should not list the custom command") }) t.Run("NoMember", func(t *testing.T) { @@ -344,12 +310,8 @@ func TestListAutocompleteCommands(t *testing.T) { foundCustom = true } } - if !foundEcho { - t.Fatal("Couldn't find echo command") - } - if foundCustom { - t.Fatal("Should not list the custom command") - } + require.True(t, foundEcho, "Couldn't find echo command") + require.False(t, foundCustom, "Should not list the custom command") }) t.Run("RegularUserCanListOnlySystemCommands", func(t *testing.T) { @@ -366,12 +328,8 @@ func TestListAutocompleteCommands(t *testing.T) { foundCustom = true } } - if !foundEcho { - t.Fatal("Couldn't find echo command") - } - if foundCustom { - t.Fatal("Should not list the custom command") - } + require.True(t, foundEcho, "Couldn't find echo command") + require.False(t, foundCustom, "Should not list the custom command") }) t.Run("NoMember", func(t *testing.T) { @@ -414,15 +372,11 @@ func TestRegenToken(t *testing.T) { token, resp := th.SystemAdminClient.RegenCommandToken(createdCmd.Id) CheckNoError(t, resp) - if token == createdCmd.Token { - t.Fatal("should update the token") - } + require.NotEqual(t, createdCmd.Token, token, "should update the token") token, resp = Client.RegenCommandToken(createdCmd.Id) CheckForbiddenStatus(t, resp) - if token != "" { - t.Fatal("should not return the token") - } + require.Empty(t, token, "should not return the token") } func TestExecuteInvalidCommand(t *testing.T) { @@ -457,9 +411,8 @@ func TestExecuteInvalidCommand(t *testing.T) { Trigger: "getcommand", } - if _, err := th.App.CreateCommand(getCmd); err != nil { - t.Fatal("failed to create get command") - } + _, err := th.App.CreateCommand(getCmd) + require.Nil(t, err, "failed to create get command") _, resp := Client.ExecuteCommand(channel.Id, "") CheckBadRequestStatus(t, resp) @@ -537,9 +490,8 @@ func TestExecuteGetCommand(t *testing.T) { Token: token, } - if _, err := th.App.CreateCommand(getCmd); err != nil { - t.Fatal("failed to create get command") - } + _, err := th.App.CreateCommand(getCmd) + require.Nil(t, err, "failed to create get command") commandResponse, resp := Client.ExecuteCommand(channel.Id, "/getcommand") CheckNoError(t, resp) @@ -597,9 +549,8 @@ func TestExecutePostCommand(t *testing.T) { Token: token, } - if _, err := th.App.CreateCommand(postCmd); err != nil { - t.Fatal("failed to create get command") - } + _, err := th.App.CreateCommand(postCmd) + require.Nil(t, err, "failed to create get command") commandResponse, resp := Client.ExecuteCommand(channel.Id, "/postcommand") CheckNoError(t, resp) @@ -652,9 +603,8 @@ func TestExecuteCommandAgainstChannelOnAnotherTeam(t *testing.T) { Method: model.COMMAND_METHOD_POST, Trigger: "postcommand", } - if _, err := th.App.CreateCommand(postCmd); err != nil { - t.Fatal("failed to create post command") - } + _, err := th.App.CreateCommand(postCmd) + require.Nil(t, err, "failed to create post command") // the execute command endpoint will always search for the command by trigger and team id, inferring team id from the // channel id, so there is no way to use that slash command on a channel that belongs to some other team @@ -702,15 +652,13 @@ func TestExecuteCommandAgainstChannelUserIsNotIn(t *testing.T) { Method: model.COMMAND_METHOD_POST, Trigger: "postcommand", } - if _, err := th.App.CreateCommand(postCmd); err != nil { - t.Fatal("failed to create post command") - } + _, err := th.App.CreateCommand(postCmd) + require.Nil(t, err, "failed to create post command") // make a channel on that team, ensuring that our test user isn't in it channel2 := th.CreateChannelWithClientAndTeam(client, model.CHANNEL_OPEN, team2.Id) - if success, _ := client.RemoveUserFromChannel(channel2.Id, th.BasicUser.Id); !success { - t.Fatal("Failed to remove user from channel") - } + success, _ := client.RemoveUserFromChannel(channel2.Id, th.BasicUser.Id) + require.True(t, success, "Failed to remove user from channel") // we should not be able to run the slash command in channel2, because we aren't in it _, resp := client.ExecuteCommandWithTeam(channel2.Id, team2.Id, "/postcommand") @@ -760,9 +708,8 @@ func TestExecuteCommandInDirectMessageChannel(t *testing.T) { Method: model.COMMAND_METHOD_POST, Trigger: "postcommand", } - if _, err := th.App.CreateCommand(postCmd); err != nil { - t.Fatal("failed to create post command") - } + _, err := th.App.CreateCommand(postCmd) + require.Nil(t, err, "failed to create post command") // make a direct message channel dmChannel, response := client.CreateDirectChannel(th.BasicUser.Id, th.BasicUser2.Id) @@ -823,9 +770,8 @@ func TestExecuteCommandInTeamUserIsNotOn(t *testing.T) { Method: model.COMMAND_METHOD_POST, Trigger: "postcommand", } - if _, err := th.App.CreateCommand(postCmd); err != nil { - t.Fatal("failed to create post command") - } + _, err := th.App.CreateCommand(postCmd) + require.Nil(t, err, "failed to create post command") // make a direct message channel dmChannel, response := client.CreateDirectChannel(th.BasicUser.Id, th.BasicUser2.Id) @@ -836,9 +782,9 @@ func TestExecuteCommandInTeamUserIsNotOn(t *testing.T) { CheckOKStatus(t, resp) // if the user is removed from the team, they should NOT be able to run the slash command in the DM channel - if success, _ := client.RemoveTeamMember(team2.Id, th.BasicUser.Id); !success { - t.Fatal("Failed to remove user from team") - } + success, _ := client.RemoveTeamMember(team2.Id, th.BasicUser.Id) + require.True(t, success, "Failed to remove user from team") + _, resp = client.ExecuteCommandWithTeam(dmChannel.Id, team2.Id, "/postcommand") CheckForbiddenStatus(t, resp) diff --git a/api4/cors_test.go b/api4/cors_test.go index 74702b284e..34dd0fd4b6 100644 --- a/api4/cors_test.go +++ b/api4/cors_test.go @@ -7,6 +7,7 @@ import ( "github.com/mattermost/mattermost-server/model" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -127,16 +128,12 @@ func TestCORSRequestHandling(t *testing.T) { url := fmt.Sprintf("%v/api/v4/system/ping", host) req, err := http.NewRequest("GET", url, nil) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) testcase.ModifyRequest(req) client := &http.Client{} resp, err := client.Do(req) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) assert.Equal(t, testcase.ExpectedAllowOrigin, resp.Header.Get(acAllowOrigin)) assert.Equal(t, testcase.ExpectedExposeHeaders, resp.Header.Get(acExposeHeaders)) @@ -146,5 +143,4 @@ func TestCORSRequestHandling(t *testing.T) { assert.Equal(t, "", resp.Header.Get(acAllowHeaders)) }) } - } diff --git a/api4/file.go b/api4/file.go index 3426a04844..ef3f410d4b 100644 --- a/api4/file.go +++ b/api4/file.go @@ -65,92 +65,6 @@ func (api *API) InitFile() { } -func uploadFile(c *Context, w http.ResponseWriter, r *http.Request) { - defer io.Copy(ioutil.Discard, r.Body) - - if !*c.App.Config().FileSettings.EnableFileAttachments { - c.Err = model.NewAppError("uploadFile", "api.file.attachments.disabled.app_error", nil, "", http.StatusNotImplemented) - return - } - - if r.ContentLength > *c.App.Config().FileSettings.MaxFileSize { - c.Err = model.NewAppError("uploadFile", "api.file.upload_file.too_large.app_error", nil, "", http.StatusRequestEntityTooLarge) - return - } - - now := time.Now() - var resStruct *model.FileUploadResponse - var appErr *model.AppError - - if err := r.ParseMultipartForm(*c.App.Config().FileSettings.MaxFileSize); err != nil && err != http.ErrNotMultipart { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } else if err == http.ErrNotMultipart { - defer r.Body.Close() - - c.RequireChannelId() - c.RequireFilename() - - if c.Err != nil { - return - } - - channelId := c.Params.ChannelId - filename := c.Params.Filename - - if !c.App.SessionHasPermissionToChannel(c.App.Session, channelId, model.PERMISSION_UPLOAD_FILE) { - c.SetPermissionError(model.PERMISSION_UPLOAD_FILE) - return - } - - resStruct, appErr = c.App.UploadFiles( - FILE_TEAM_ID, - channelId, - c.App.Session.UserId, - []io.ReadCloser{r.Body}, - []string{filename}, - []string{}, - now, - ) - } else { - m := r.MultipartForm - - props := m.Value - if len(props["channel_id"]) == 0 { - c.SetInvalidParam("channel_id") - return - } - channelId := props["channel_id"][0] - c.Params.ChannelId = channelId - c.RequireChannelId() - if c.Err != nil { - return - } - - if !c.App.SessionHasPermissionToChannel(c.App.Session, channelId, model.PERMISSION_UPLOAD_FILE) { - c.SetPermissionError(model.PERMISSION_UPLOAD_FILE) - return - } - - resStruct, appErr = c.App.UploadMultipartFiles( - FILE_TEAM_ID, - channelId, - c.App.Session.UserId, - m.File["files"], - m.Value["client_ids"], - now, - ) - } - - if appErr != nil { - c.Err = appErr - return - } - - w.WriteHeader(http.StatusCreated) - w.Write([]byte(resStruct.ToJson())) -} - func parseMultipartRequestHeader(req *http.Request) (boundary string, err error) { v := req.Header.Get("Content-Type") if v == "" { diff --git a/api4/file_test.go b/api4/file_test.go index a12015294a..b451155715 100644 --- a/api4/file_test.go +++ b/api4/file_test.go @@ -40,26 +40,20 @@ func escapeQuotes(s string) string { return quoteEscaper.Replace(s) } -func randomBytes(n int) []byte { +func randomBytes(t *testing.T, n int) []byte { bb := make([]byte, n) _, err := rand.Read(bb) - if err != nil { - panic(err.Error()) - } + require.NoError(t, err) return bb } -func fileBytes(path string) []byte { +func fileBytes(t *testing.T, path string) []byte { path = filepath.Join(testDir, path) f, err := os.Open(path) - if err != nil { - panic(err.Error()) - } + require.NoError(t, err) defer f.Close() bb, err := ioutil.ReadAll(f) - if err != nil { - panic(err.Error()) - } + require.NoError(t, err) return bb } @@ -183,7 +177,7 @@ func testUploadFilesMultipart( require.Nil(t, err) } - mw.Close() + require.NoError(t, mw.Close()) return testDoUploadFileRequest(t, c, "", mwBody.Bytes(), mw.FormDataContentType(), -1) } @@ -251,7 +245,7 @@ func TestUploadFiles(t *testing.T) { { title: "Happy invalid image", names: []string{"testgif.gif"}, - blobs: [][]byte{fileBytes("test-search.md")}, + blobs: [][]byte{fileBytes(t, "test-search.md")}, skipPayloadValidation: true, expectedCreatorId: th.BasicUser.Id, }, @@ -382,7 +376,7 @@ func TestUploadFiles(t *testing.T) { useChunkedInSimplePost: true, skipPayloadValidation: true, names: []string{"1Mb-stream"}, - blobs: [][]byte{randomBytes(1024 * 1024)}, + blobs: [][]byte{randomBytes(t, 1024*1024)}, setupConfig: func(a *app.App) func(a *app.App) { maxFileSize := *a.Config().FileSettings.MaxFileSize a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.MaxFileSize = 1024 * 1024 }) @@ -480,7 +474,7 @@ func TestUploadFiles(t *testing.T) { title: "Error stream too large", skipPayloadValidation: true, names: []string{"1Mb-stream"}, - blobs: [][]byte{randomBytes(1024 * 1024)}, + blobs: [][]byte{randomBytes(t, 1024*1024)}, skipSuccessValidation: true, checkResponse: CheckRequestEntityTooLargeStatus, setupConfig: func(a *app.App) func(a *app.App) { @@ -532,7 +526,7 @@ func TestUploadFiles(t *testing.T) { blobs := tc.blobs if len(blobs) == 0 { for _, name := range tc.names { - blobs = append(blobs, fileBytes(name)) + blobs = append(blobs, fileBytes(t, name)) } } @@ -555,9 +549,9 @@ func TestUploadFiles(t *testing.T) { return } - if fileResp == nil || len(fileResp.FileInfos) == 0 || len(fileResp.FileInfos) != len(tc.names) { - t.Fatal("Empty or mismatched actual or expected FileInfos") - } + require.NotNil(t, fileResp, "Nil fileResp") + require.NotEqual(t, 0, len(fileResp.FileInfos), "Empty FileInfos") + require.Equal(t, len(tc.names), len(fileResp.FileInfos), "Mismatched actual or expected FileInfos") for i, ri := range fileResp.FileInfos { // The returned file info from the upload call will be missing some fields that will be stored in the database @@ -613,8 +607,7 @@ func TestUploadFiles(t *testing.T) { expected, err := ioutil.ReadFile(filepath.Join(testDir, name)) require.Nil(t, err) - - if bytes.Compare(data, expected) != 0 { + if !bytes.Equal(data, expected) { tf, err := ioutil.TempFile("", fmt.Sprintf("test_%v_*_%s", i, name)) require.Nil(t, err) _, _ = io.Copy(tf, bytes.NewReader(data)) @@ -653,29 +646,20 @@ func TestGetFile(t *testing.T) { t.Skip("skipping because no file driver is enabled") } - fileId := "" - var sent []byte - var err error - if sent, err = testutils.ReadTestFile("test.png"); err != nil { - t.Fatal(err) - } else { - fileResp, resp := Client.UploadFile(sent, channel.Id, "test.png") - CheckNoError(t, resp) + sent, err := testutils.ReadTestFile("test.png") + require.NoError(t, err) - fileId = fileResp.FileInfos[0].Id - } + fileResp, resp := Client.UploadFile(sent, channel.Id, "test.png") + CheckNoError(t, resp) + + fileId := fileResp.FileInfos[0].Id data, resp := Client.GetFile(fileId) CheckNoError(t, resp) - - if len(data) == 0 { - t.Fatal("should not be empty") - } + require.NotEqual(t, 0, len(data), "should not be empty") for i := range data { - if data[i] != sent[i] { - t.Fatal("received file didn't match sent one") - } + require.Equal(t, sent[i], data[i], "received file didn't match sent one") } _, resp = Client.GetFile("junk") @@ -713,30 +697,19 @@ func TestGetFileHeaders(t *testing.T) { _, resp = Client.GetFile(fileId) CheckNoError(t, resp) - if contentType := resp.Header.Get("Content-Type"); !strings.HasPrefix(contentType, expectedContentType) { - t.Fatal("returned incorrect Content-Type", contentType) - } + CheckStartsWith(t, resp.Header.Get("Content-Type"), expectedContentType, "returned incorrect Content-Type") if getInline { - if contentDisposition := resp.Header.Get("Content-Disposition"); !strings.HasPrefix(contentDisposition, "inline") { - t.Fatal("returned incorrect Content-Disposition", contentDisposition) - } + CheckStartsWith(t, resp.Header.Get("Content-Disposition"), "inline", "returned incorrect Content-Disposition") } else { - if contentDisposition := resp.Header.Get("Content-Disposition"); !strings.HasPrefix(contentDisposition, "attachment") { - t.Fatal("returned incorrect Content-Disposition", contentDisposition) - } + CheckStartsWith(t, resp.Header.Get("Content-Disposition"), "attachment", "returned incorrect Content-Disposition") } _, resp = Client.DownloadFile(fileId, true) CheckNoError(t, resp) - if contentType := resp.Header.Get("Content-Type"); !strings.HasPrefix(contentType, expectedContentType) { - t.Fatal("returned incorrect Content-Type", contentType) - } - - if contentDisposition := resp.Header.Get("Content-Disposition"); !strings.HasPrefix(contentDisposition, "attachment") { - t.Fatal("returned incorrect Content-Disposition", contentDisposition) - } + CheckStartsWith(t, resp.Header.Get("Content-Type"), expectedContentType, "returned incorrect Content-Type") + CheckStartsWith(t, resp.Header.Get("Content-Disposition"), "attachment", "returned incorrect Content-Disposition") } } @@ -768,27 +741,20 @@ func TestGetFileThumbnail(t *testing.T) { t.Skip("skipping because no file driver is enabled") } - fileId := "" - var sent []byte - var err error - if sent, err = testutils.ReadTestFile("test.png"); err != nil { - t.Fatal(err) - } else { - fileResp, resp := Client.UploadFile(sent, channel.Id, "test.png") - CheckNoError(t, resp) + sent, err := testutils.ReadTestFile("test.png") + require.NoError(t, err) - fileId = fileResp.FileInfos[0].Id - } + fileResp, resp := Client.UploadFile(sent, channel.Id, "test.png") + CheckNoError(t, resp) + + fileId := fileResp.FileInfos[0].Id // Wait a bit for files to ready time.Sleep(2 * time.Second) data, resp := Client.GetFileThumbnail(fileId) CheckNoError(t, resp) - - if len(data) == 0 { - t.Fatal("should not be empty") - } + require.NotEqual(t, 0, len(data), "should not be empty") _, resp = Client.GetFileThumbnail("junk") CheckBadRequestStatus(t, resp) @@ -823,21 +789,19 @@ func TestGetFileLink(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = model.NewRandomString(32) }) - fileId := "" - if data, err := testutils.ReadTestFile("test.png"); err != nil { - t.Fatal(err) - } else { - fileResp, resp := Client.UploadFile(data, channel.Id, "test.png") - CheckNoError(t, resp) + data, err := testutils.ReadTestFile("test.png") + require.NoError(t, err) - fileId = fileResp.FileInfos[0].Id - } + fileResp, uploadResp := Client.UploadFile(data, channel.Id, "test.png") + CheckNoError(t, uploadResp) + + fileId := fileResp.FileInfos[0].Id _, resp := Client.GetFileLink(fileId) CheckBadRequestStatus(t, resp) // Hacky way to assign file to a post (usually would be done by CreatePost call) - err := th.App.Srv.Store.FileInfo().AttachToPost(fileId, th.BasicPost.Id, th.BasicUser.Id) + err = th.App.Srv.Store.FileInfo().AttachToPost(fileId, th.BasicPost.Id, th.BasicUser.Id) require.Nil(t, err) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = false }) @@ -850,10 +814,7 @@ func TestGetFileLink(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) link, resp := Client.GetFileLink(fileId) CheckNoError(t, resp) - - if link == "" { - t.Fatal("should've received public link") - } + require.NotEqual(t, "", link, "should've received public link") _, resp = Client.GetFileLink("junk") CheckBadRequestStatus(t, resp) @@ -889,27 +850,19 @@ func TestGetFilePreview(t *testing.T) { t.Skip("skipping because no file driver is enabled") } - fileId := "" - var sent []byte - var err error - if sent, err = testutils.ReadTestFile("test.png"); err != nil { - t.Fatal(err) - } else { - fileResp, resp := Client.UploadFile(sent, channel.Id, "test.png") - CheckNoError(t, resp) + sent, err := testutils.ReadTestFile("test.png") + require.NoError(t, err) - fileId = fileResp.FileInfos[0].Id - } + fileResp, resp := Client.UploadFile(sent, channel.Id, "test.png") + CheckNoError(t, resp) + fileId := fileResp.FileInfos[0].Id // Wait a bit for files to ready time.Sleep(2 * time.Second) data, resp := Client.GetFilePreview(fileId) CheckNoError(t, resp) - - if len(data) == 0 { - t.Fatal("should not be empty") - } + require.NotEqual(t, 0, len(data), "should not be empty") _, resp = Client.GetFilePreview("junk") CheckBadRequestStatus(t, resp) @@ -942,17 +895,12 @@ func TestGetFileInfo(t *testing.T) { t.Skip("skipping because no file driver is enabled") } - fileId := "" - var sent []byte - var err error - if sent, err = testutils.ReadTestFile("test.png"); err != nil { - t.Fatal(err) - } else { - fileResp, resp := Client.UploadFile(sent, channel.Id, "test.png") - CheckNoError(t, resp) + sent, err := testutils.ReadTestFile("test.png") + require.NoError(t, err) - fileId = fileResp.FileInfos[0].Id - } + fileResp, resp := Client.UploadFile(sent, channel.Id, "test.png") + CheckNoError(t, resp) + fileId := fileResp.FileInfos[0].Id // Wait a bit for files to ready time.Sleep(2 * time.Second) @@ -960,23 +908,14 @@ func TestGetFileInfo(t *testing.T) { info, resp := Client.GetFileInfo(fileId) CheckNoError(t, resp) - if err != nil { - t.Fatal(err) - } else if info.Id != fileId { - t.Fatal("got incorrect file") - } else if info.CreatorId != user.Id { - t.Fatal("file should be assigned to user") - } else if info.PostId != "" { - t.Fatal("file shouldn't have a post") - } else if info.Path != "" { - t.Fatal("file path shouldn't have been returned to client") - } else if info.ThumbnailPath != "" { - t.Fatal("file thumbnail path shouldn't have been returned to client") - } else if info.PreviewPath != "" { - t.Fatal("file preview path shouldn't have been returned to client") - } else if info.MimeType != "image/png" { - t.Fatal("mime type should've been image/png") - } + require.NoError(t, err) + require.Equal(t, fileId, info.Id, "got incorrect file") + require.Equal(t, user.Id, info.CreatorId, "file should be assigned to user") + require.Equal(t, "", info.PostId, "file shouldn't have a post") + require.Equal(t, "", info.Path, "file path shouldn't have been returned to client") + require.Equal(t, "", info.ThumbnailPath, "file thumbnail path shouldn't have been returned to client") + require.Equal(t, "", info.PreviewPath, "file preview path shouldn't have been returned to client") + require.Equal(t, "image/png", info.MimeType, "mime type should've been image/png") _, resp = Client.GetFileInfo("junk") CheckBadRequestStatus(t, resp) @@ -1007,18 +946,16 @@ func TestGetPublicFile(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = model.NewRandomString(32) }) - fileId := "" - if data, err := testutils.ReadTestFile("test.png"); err != nil { - t.Fatal(err) - } else { - fileResp, resp := Client.UploadFile(data, channel.Id, "test.png") - CheckNoError(t, resp) + data, err := testutils.ReadTestFile("test.png") + require.NoError(t, err) - fileId = fileResp.FileInfos[0].Id - } + fileResp, httpResp := Client.UploadFile(data, channel.Id, "test.png") + CheckNoError(t, httpResp) + + fileId := fileResp.FileInfos[0].Id // Hacky way to assign file to a post (usually would be done by CreatePost call) - err := th.App.Srv.Store.FileInfo().AttachToPost(fileId, th.BasicPost.Id, th.BasicUser.Id) + err = th.App.Srv.Store.FileInfo().AttachToPost(fileId, th.BasicPost.Id, th.BasicUser.Id) require.Nil(t, err) info, err := th.App.Srv.Store.FileInfo().Get(fileId) @@ -1028,31 +965,31 @@ func TestGetPublicFile(t *testing.T) { // Wait a bit for files to ready time.Sleep(2 * time.Second) - if resp, err := http.Get(link); err != nil || resp.StatusCode != http.StatusOK { - t.Log(link) - t.Fatal("failed to get image with public link", err) - } + resp, err := http.Get(link) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode, "failed to get image with public link") - if resp, err := http.Get(link[:strings.LastIndex(link, "?")]); err == nil && resp.StatusCode != http.StatusBadRequest { - t.Fatal("should've failed to get image with public link without hash", resp.Status) - } + resp, err = http.Get(link[:strings.LastIndex(link, "?")]) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, resp.StatusCode, "should've failed to get image with public link without hash", resp.Status) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = false }) - if resp, err := http.Get(link); err == nil && resp.StatusCode != http.StatusNotImplemented { - t.Fatal("should've failed to get image with disabled public link") - } + + resp, err = http.Get(link) + require.NoError(t, err) + require.Equal(t, http.StatusNotImplemented, resp.StatusCode, "should've failed to get image with disabled public link") // test after the salt has changed th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.PublicLinkSalt = model.NewRandomString(32) }) - if resp, err := http.Get(link); err == nil && resp.StatusCode != http.StatusBadRequest { - t.Fatal("should've failed to get image with public link after salt changed") - } + resp, err = http.Get(link) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, resp.StatusCode, "should've failed to get image with public link after salt changed") - if resp, err := http.Get(link); err == nil && resp.StatusCode != http.StatusBadRequest { - t.Fatal("should've failed to get image with public link after salt changed") - } + resp, err = http.Get(link) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, resp.StatusCode, "should've failed to get image with public link after salt changed") fileInfo, err := th.App.Srv.Store.FileInfo().Get(fileId) require.Nil(t, err) diff --git a/api4/group.go b/api4/group.go index 24c7869551..4f799c3b58 100644 --- a/api4/group.go +++ b/api4/group.go @@ -13,11 +13,6 @@ import ( "github.com/mattermost/mattermost-server/model" ) -const ( - groupMemberActionCreate = iota - groupMemberActionDelete -) - func (api *API) InitGroup() { // GET /api/v4/groups api.BaseRoutes.Groups.Handle("", api.ApiSessionRequired(getGroups)).Methods("GET") diff --git a/api4/integration_action_test.go b/api4/integration_action_test.go index 3838d321fb..7b629f4a04 100644 --- a/api4/integration_action_test.go +++ b/api4/integration_action_test.go @@ -112,7 +112,7 @@ func TestOpenDialog(t *testing.T) { CallbackId: "callbackid", Title: "Some Title", Elements: []model.DialogElement{ - model.DialogElement{ + { DisplayName: "Element Name", Name: "element_name", Type: "text", diff --git a/api4/job_test.go b/api4/job_test.go index 61172897de..e2426c5478 100644 --- a/api4/job_test.go +++ b/api4/job_test.go @@ -23,7 +23,7 @@ func TestCreateJob(t *testing.T) { } received, resp := th.SystemAdminClient.CreateJob(job) - CheckNoError(t, resp) + require.Nil(t, resp.Error) defer th.App.Srv.Store.Job().Delete(received.Id) @@ -52,11 +52,10 @@ func TestGetJob(t *testing.T) { defer th.App.Srv.Store.Job().Delete(job.Id) received, resp := th.SystemAdminClient.GetJob(job.Id) - CheckNoError(t, resp) + require.Nil(t, resp.Error) - if received.Id != job.Id || received.Status != job.Status { - t.Fatal("incorrect job received") - } + require.Equal(t, job.Id, received.Id, "incorrect job received") + require.Equal(t, job.Status, received.Status, "incorrect job received") _, resp = th.SystemAdminClient.GetJob("1234") CheckBadRequestStatus(t, resp) @@ -100,22 +99,16 @@ func TestGetJobs(t *testing.T) { } received, resp := th.SystemAdminClient.GetJobs(0, 2) - CheckNoError(t, resp) + require.Nil(t, resp.Error) - if len(received) != 2 { - t.Fatal("received wrong number of jobs") - } else if received[0].Id != jobs[2].Id { - t.Fatal("should've received newest job first") - } else if received[1].Id != jobs[0].Id { - t.Fatal("should've received second newest job second") - } + require.Len(t, received, 2, "received wrong number of jobs") + require.Equal(t, jobs[2].Id, received[0].Id, "should've received newest job first") + require.Equal(t, jobs[0].Id, received[1].Id, "should've received second newest job second") received, resp = th.SystemAdminClient.GetJobs(1, 2) - CheckNoError(t, resp) + require.Nil(t, resp.Error) - if received[0].Id != jobs[1].Id { - t.Fatal("should've received oldest job last") - } + require.Equal(t,jobs[1].Id, received[0].Id, "should've received oldest job last") _, resp = th.Client.GetJobs(0, 60) CheckForbiddenStatus(t, resp) @@ -157,24 +150,17 @@ func TestGetJobsByType(t *testing.T) { } received, resp := th.SystemAdminClient.GetJobsByType(jobType, 0, 2) - CheckNoError(t, resp) + require.Nil(t, resp.Error) - if len(received) != 2 { - t.Fatal("received wrong number of jobs") - } else if received[0].Id != jobs[2].Id { - t.Fatal("should've received newest job first") - } else if received[1].Id != jobs[0].Id { - t.Fatal("should've received second newest job second") - } + require.Len(t, received, 2, "received wrong number of jobs") + require.Equal(t, jobs[2].Id, received[0].Id, "should've received newest job first") + require.Equal(t, jobs[0].Id, received[1].Id, "should've received second newest job second") received, resp = th.SystemAdminClient.GetJobsByType(jobType, 1, 2) - CheckNoError(t, resp) + require.Nil(t, resp.Error) - if len(received) != 1 { - t.Fatal("received wrong number of jobs") - } else if received[0].Id != jobs[1].Id { - t.Fatal("should've received oldest job last") - } + require.Len(t, received, 1, "received wrong number of jobs") + require.Equal(t, jobs[1].Id, received[0].Id, "should've received oldest job last") _, resp = th.SystemAdminClient.GetJobsByType("", 0, 60) CheckNotFoundStatus(t, resp) @@ -218,10 +204,10 @@ func TestCancelJob(t *testing.T) { CheckForbiddenStatus(t, resp) _, resp = th.SystemAdminClient.CancelJob(jobs[0].Id) - CheckNoError(t, resp) + require.Nil(t, resp.Error) _, resp = th.SystemAdminClient.CancelJob(jobs[1].Id) - CheckNoError(t, resp) + require.Nil(t, resp.Error) _, resp = th.SystemAdminClient.CancelJob(jobs[2].Id) CheckInternalErrorStatus(t, resp) diff --git a/api4/plugin_test.go b/api4/plugin_test.go index 210ec1b722..a1732b4db1 100644 --- a/api4/plugin_test.go +++ b/api4/plugin_test.go @@ -387,7 +387,7 @@ func TestDisableOnRemove(t *testing.T) { pluginsResp, resp := th.SystemAdminClient.GetPlugins() CheckNoError(t, resp) require.Len(t, pluginsResp.Active, 0) - require.Equal(t, pluginsResp.Inactive, []*model.PluginInfo{&model.PluginInfo{ + require.Equal(t, pluginsResp.Inactive, []*model.PluginInfo{{ Manifest: *manifest, }}) @@ -400,7 +400,7 @@ func TestDisableOnRemove(t *testing.T) { pluginsResp, resp = th.SystemAdminClient.GetPlugins() CheckNoError(t, resp) require.Len(t, pluginsResp.Inactive, 0) - require.Equal(t, pluginsResp.Active, []*model.PluginInfo{&model.PluginInfo{ + require.Equal(t, pluginsResp.Active, []*model.PluginInfo{{ Manifest: *manifest, }}) @@ -414,7 +414,7 @@ func TestDisableOnRemove(t *testing.T) { pluginsResp, resp = th.SystemAdminClient.GetPlugins() CheckNoError(t, resp) require.Len(t, pluginsResp.Inactive, 0) - require.Equal(t, pluginsResp.Active, []*model.PluginInfo{&model.PluginInfo{ + require.Equal(t, pluginsResp.Active, []*model.PluginInfo{{ Manifest: *manifest, }}) } @@ -439,7 +439,7 @@ func TestDisableOnRemove(t *testing.T) { pluginsResp, resp = th.SystemAdminClient.GetPlugins() CheckNoError(t, resp) require.Len(t, pluginsResp.Active, 0) - require.Equal(t, pluginsResp.Inactive, []*model.PluginInfo{&model.PluginInfo{ + require.Equal(t, pluginsResp.Inactive, []*model.PluginInfo{{ Manifest: *manifest, }}) diff --git a/api4/post.go b/api4/post.go index 685ce9bf0e..4ec2411469 100644 --- a/api4/post.go +++ b/api4/post.go @@ -476,7 +476,7 @@ func searchPosts(c *Context, w http.ResponseWriter, r *http.Request) { startTime := time.Now() - results, err := c.App.SearchPostsInTeamForUser(terms, c.App.Session.UserId, c.Params.TeamId, isOrSearch, includeDeletedChannels, int(timeZoneOffset), page, perPage) + results, err := c.App.SearchPostsInTeamForUser(terms, c.App.Session.UserId, c.Params.TeamId, isOrSearch, includeDeletedChannels, timeZoneOffset, page, perPage) elapsedTime := float64(time.Since(startTime)) / float64(time.Second) metrics := c.App.Metrics diff --git a/api4/preference_test.go b/api4/preference_test.go index 41681ff69c..114c73ff4e 100644 --- a/api4/preference_test.go +++ b/api4/preference_test.go @@ -8,6 +8,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/model" ) @@ -42,14 +44,10 @@ func TestGetPreferences(t *testing.T) { prefs, resp := Client.GetPreferences(user1.Id) CheckNoError(t, resp) - if len(prefs) != 4 { - t.Fatal("received the wrong number of preferences") - } + require.Equal(t, len(prefs), 4, "received the wrong number of preferences") for _, preference := range prefs { - if preference.UserId != th.BasicUser.Id { - t.Fatal("user id does not match") - } + require.Equal(t, preference.UserId, th.BasicUser.Id, "user id does not match") } th.LoginBasic2() @@ -57,9 +55,7 @@ func TestGetPreferences(t *testing.T) { prefs, resp = Client.GetPreferences(th.BasicUser2.Id) CheckNoError(t, resp) - if len(prefs) == 0 { - t.Fatal("received the wrong number of preferences") - } + require.Greater(t, len(prefs), 0, "received the wrong number of preferences") _, resp = Client.GetPreferences(th.BasicUser.Id) CheckForbiddenStatus(t, resp) @@ -101,9 +97,7 @@ func TestGetPreferencesByCategory(t *testing.T) { prefs, resp := Client.GetPreferencesByCategory(user1.Id, category) CheckNoError(t, resp) - if len(prefs) != 2 { - t.Fatalf("received the wrong number of preferences %v:%v", len(prefs), 2) - } + require.Equal(t, len(prefs), 2, "received the wrong number of preferences") _, resp = Client.GetPreferencesByCategory(user1.Id, "junk") CheckNotFoundStatus(t, resp) @@ -119,9 +113,7 @@ func TestGetPreferencesByCategory(t *testing.T) { prefs, resp = Client.GetPreferencesByCategory(th.BasicUser2.Id, "junk") CheckNotFoundStatus(t, resp) - if len(prefs) != 0 { - t.Fatal("received the wrong number of preferences") - } + require.Equal(t, len(prefs), 0, "received the wrong number of preferences") Client.Logout() _, resp = Client.GetPreferencesByCategory(th.BasicUser2.Id, category) @@ -158,9 +150,9 @@ func TestGetPreferenceByCategoryAndName(t *testing.T) { pref, resp := Client.GetPreferenceByCategoryAndName(user.Id, model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW, name) CheckNoError(t, resp) - if (pref.UserId != preferences[0].UserId) && (pref.Category != preferences[0].Category) && (pref.Name != preferences[0].Name) { - t.Fatal("preference saved incorrectly") - } + require.Equal(t, preferences[0].UserId, pref.UserId, "UserId preference not saved") + require.Equal(t, preferences[0].Category, pref.Category, "Category preference not saved") + require.Equal(t, preferences[0].Name, pref.Name, "Name preference not saved") preferences[0].Value = model.NewId() Client.UpdatePreferences(user.Id, &preferences) @@ -247,15 +239,12 @@ func TestUpdatePreferencesWebsocket(t *testing.T) { defer th.TearDown() WebSocketClient, err := th.CreateWebSocketClient() - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) WebSocketClient.Listen() time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Status != model.STATUS_OK { - t.Fatal("should have responded OK to authentication challenge") - } + wsResp := <-WebSocketClient.ResponseChannel + require.Equal(t, wsResp.Status, model.STATUS_OK, "expected OK from auth challenge") userId := th.BasicUser.Id preferences := &model.Preferences{ @@ -285,19 +274,17 @@ func TestUpdatePreferencesWebsocket(t *testing.T) { } received, err := model.PreferencesFromJson(strings.NewReader(event.Data["preferences"].(string))) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - for i, preference := range *preferences { - if preference.UserId != received[i].UserId || preference.Category != received[i].Category || preference.Name != received[i].Name { - t.Fatal("received incorrect preference") - } + for i, p := range *preferences { + require.Equal(t, received[i].UserId, p.UserId, "received incorrect UserId") + require.Equal(t, received[i].Category, p.Category, "received incorrect Category") + require.Equal(t, received[i].Name, p.Name, "received incorrect Name") } waiting = false case <-timeout: - t.Fatal("timed out waiting for preference update event") + require.Fail(t, "timed timed out waiting for preference update event") } } } diff --git a/api4/status_test.go b/api4/status_test.go index ced87c0d7b..5d47a9ead3 100644 --- a/api4/status_test.go +++ b/api4/status_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/mattermost/mattermost-server/model" + "github.com/stretchr/testify/assert" ) func TestGetUserStatus(t *testing.T) { @@ -13,44 +14,32 @@ func TestGetUserStatus(t *testing.T) { userStatus, resp := Client.GetUserStatus(th.BasicUser.Id, "") CheckNoError(t, resp) - if userStatus.Status != "offline" { - t.Fatal("Should return offline status") - } + assert.Equal(t, "offline", userStatus.Status) th.App.SetStatusOnline(th.BasicUser.Id, true) userStatus, resp = Client.GetUserStatus(th.BasicUser.Id, "") CheckNoError(t, resp) - if userStatus.Status != "online" { - t.Fatal("Should return online status") - } + assert.Equal(t, "online", userStatus.Status) th.App.SetStatusAwayIfNeeded(th.BasicUser.Id, true) userStatus, resp = Client.GetUserStatus(th.BasicUser.Id, "") CheckNoError(t, resp) - if userStatus.Status != "away" { - t.Fatal("Should return away status") - } + assert.Equal(t, "away", userStatus.Status) th.App.SetStatusDoNotDisturb(th.BasicUser.Id) userStatus, resp = Client.GetUserStatus(th.BasicUser.Id, "") CheckNoError(t, resp) - if userStatus.Status != "dnd" { - t.Fatal("Should return dnd status") - } + assert.Equal(t, "dnd", userStatus.Status) th.App.SetStatusOffline(th.BasicUser.Id, true) userStatus, resp = Client.GetUserStatus(th.BasicUser.Id, "") CheckNoError(t, resp) - if userStatus.Status != "offline" { - t.Fatal("Should return offline status") - } + assert.Equal(t, "offline", userStatus.Status) //Get user2 status logged as user1 userStatus, resp = Client.GetUserStatus(th.BasicUser2.Id, "") CheckNoError(t, resp) - if userStatus.Status != "offline" { - t.Fatal("Should return offline status") - } + assert.Equal(t, "offline", userStatus.Status) Client.Logout() @@ -60,9 +49,7 @@ func TestGetUserStatus(t *testing.T) { th.LoginBasic2() userStatus, resp = Client.GetUserStatus(th.BasicUser2.Id, "") CheckNoError(t, resp) - if userStatus.Status != "offline" { - t.Fatal("Should return offline status") - } + assert.Equal(t, "offline", userStatus.Status) } func TestGetUsersStatusesByIds(t *testing.T) { @@ -75,9 +62,7 @@ func TestGetUsersStatusesByIds(t *testing.T) { usersStatuses, resp := Client.GetUsersStatusesByIds(usersIds) CheckNoError(t, resp) for _, userStatus := range usersStatuses { - if userStatus.Status != "offline" { - t.Fatal("Status should be offline") - } + assert.Equal(t, "offline", userStatus.Status) } th.App.SetStatusOnline(th.BasicUser.Id, true) @@ -85,9 +70,7 @@ func TestGetUsersStatusesByIds(t *testing.T) { usersStatuses, resp = Client.GetUsersStatusesByIds(usersIds) CheckNoError(t, resp) for _, userStatus := range usersStatuses { - if userStatus.Status != "online" { - t.Fatal("Status should be offline") - } + assert.Equal(t, "online", userStatus.Status) } th.App.SetStatusAwayIfNeeded(th.BasicUser.Id, true) @@ -95,9 +78,7 @@ func TestGetUsersStatusesByIds(t *testing.T) { usersStatuses, resp = Client.GetUsersStatusesByIds(usersIds) CheckNoError(t, resp) for _, userStatus := range usersStatuses { - if userStatus.Status != "away" { - t.Fatal("Status should be offline") - } + assert.Equal(t, "away", userStatus.Status) } th.App.SetStatusDoNotDisturb(th.BasicUser.Id) @@ -105,9 +86,7 @@ func TestGetUsersStatusesByIds(t *testing.T) { usersStatuses, resp = Client.GetUsersStatusesByIds(usersIds) CheckNoError(t, resp) for _, userStatus := range usersStatuses { - if userStatus.Status != "dnd" { - t.Fatal("Status should be offline") - } + assert.Equal(t, "dnd", userStatus.Status) } Client.Logout() @@ -124,30 +103,22 @@ func TestUpdateUserStatus(t *testing.T) { toUpdateUserStatus := &model.Status{Status: "online", UserId: th.BasicUser.Id} updateUserStatus, resp := Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus) CheckNoError(t, resp) - if updateUserStatus.Status != "online" { - t.Fatal("Should return online status") - } + assert.Equal(t, "online", updateUserStatus.Status) toUpdateUserStatus.Status = "away" updateUserStatus, resp = Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus) CheckNoError(t, resp) - if updateUserStatus.Status != "away" { - t.Fatal("Should return away status") - } + assert.Equal(t, "away", updateUserStatus.Status) toUpdateUserStatus.Status = "dnd" updateUserStatus, resp = Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus) CheckNoError(t, resp) - if updateUserStatus.Status != "dnd" { - t.Fatal("Should return dnd status") - } + assert.Equal(t, "dnd", updateUserStatus.Status) toUpdateUserStatus.Status = "offline" updateUserStatus, resp = Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus) CheckNoError(t, resp) - if updateUserStatus.Status != "offline" { - t.Fatal("Should return offline status") - } + assert.Equal(t, "offline", updateUserStatus.Status) toUpdateUserStatus.Status = "online" toUpdateUserStatus.UserId = th.BasicUser2.Id @@ -156,9 +127,7 @@ func TestUpdateUserStatus(t *testing.T) { toUpdateUserStatus.Status = "online" updateUserStatus, _ = th.SystemAdminClient.UpdateUserStatus(th.BasicUser2.Id, toUpdateUserStatus) - if updateUserStatus.Status != "online" { - t.Fatal("Should return online status") - } + assert.Equal(t, "online", updateUserStatus.Status) _, resp = Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus) CheckBadRequestStatus(t, resp) diff --git a/api4/system.go b/api4/system.go index 878d407d63..6e3a8866b5 100644 --- a/api4/system.go +++ b/api4/system.go @@ -397,7 +397,6 @@ func getRedirectLocation(c *Context, w http.ResponseWriter, r *http.Request) { m["location"] = location w.Write([]byte(model.MapToJson(m))) - return } func pushNotificationAck(c *Context, w http.ResponseWriter, r *http.Request) { @@ -415,5 +414,4 @@ func pushNotificationAck(c *Context, w http.ResponseWriter, r *http.Request) { } ReturnStatusOK(w) - return } diff --git a/api4/team.go b/api4/team.go index 7891f68074..391376e332 100644 --- a/api4/team.go +++ b/api4/team.go @@ -911,7 +911,7 @@ func importTeam(c *Context, w http.ResponseWriter, r *http.Request) { } data := map[string]string{} - data["results"] = base64.StdEncoding.EncodeToString([]byte(log.Bytes())) + data["results"] = base64.StdEncoding.EncodeToString(log.Bytes()) if c.Err != nil { w.WriteHeader(c.Err.StatusCode) } diff --git a/api4/user.go b/api4/user.go index 315c36e970..81fda670f5 100644 --- a/api4/user.go +++ b/api4/user.go @@ -1422,8 +1422,6 @@ func login(c *Context, w http.ResponseWriter, r *http.Request) { user.TermsOfServiceCreateAt = userTermsOfService.CreateAt } - c.App.Session = *session - user.Sanitize(map[string]bool{}) w.Write([]byte(user.ToJson())) diff --git a/api4/user_test.go b/api4/user_test.go index ea710dc8f3..452a045e21 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -1369,7 +1369,8 @@ func TestGetUsersByGroupChannelIds(t *testing.T) { usersByChannelId, resp := th.Client.GetUsersByGroupChannelIds([]string{gc1.Id}) CheckNoError(t, resp) - users, _ := usersByChannelId[gc1.Id] + users, ok := usersByChannelId[gc1.Id] + assert.True(t, ok) userIds := []string{} for _, user := range users { userIds = append(userIds, user.Id) @@ -1381,7 +1382,7 @@ func TestGetUsersByGroupChannelIds(t *testing.T) { usersByChannelId, resp = th.Client.GetUsersByGroupChannelIds([]string{gc1.Id}) CheckNoError(t, resp) - _, ok := usersByChannelId[gc1.Id] + _, ok = usersByChannelId[gc1.Id] require.False(t, ok) th.Client.Logout() diff --git a/api4/websocket.go b/api4/websocket.go index 164ae83b79..85eaf87d39 100644 --- a/api4/websocket.go +++ b/api4/websocket.go @@ -12,7 +12,8 @@ import ( ) func (api *API) InitWebSocket() { - api.BaseRoutes.ApiRoot.Handle("/websocket", api.ApiHandlerTrustRequester(connectWebSocket)).Methods("GET") + // Optionally supports a trailing slash + api.BaseRoutes.ApiRoot.Handle("/{websocket:websocket(?:\\/)?}", api.ApiHandlerTrustRequester(connectWebSocket)).Methods("GET") } func connectWebSocket(c *Context, w http.ResponseWriter, r *http.Request) { diff --git a/api4/websocket_test.go b/api4/websocket_test.go index f6f2ae8795..8450684f7a 100644 --- a/api4/websocket_test.go +++ b/api4/websocket_test.go @@ -20,62 +20,59 @@ func TestWebSocket(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() WebSocketClient, err := th.CreateWebSocketClient() - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) defer WebSocketClient.Close() time.Sleep(300 * time.Millisecond) // Test closing and reconnecting WebSocketClient.Close() - if err := WebSocketClient.Connect(); err != nil { - t.Fatal(err) - } + err = WebSocketClient.Connect() + require.Nil(t, err) WebSocketClient.Listen() time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Status != model.STATUS_OK { - t.Fatal("should have responded OK to authentication challenge") - } + resp := <-WebSocketClient.ResponseChannel + require.Equal(t, resp.Status, model.STATUS_OK, "should have responded OK to authentication challenge") WebSocketClient.SendMessage("ping", nil) time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Data["text"].(string) != "pong" { - t.Fatal("wrong response") - } + resp = <-WebSocketClient.ResponseChannel + require.Equal(t, resp.Data["text"].(string), "pong", "wrong response") WebSocketClient.SendMessage("", nil) time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Error.Id != "api.web_socket_router.no_action.app_error" { - t.Fatal("should have been no action response") - } + resp = <-WebSocketClient.ResponseChannel + require.Equal(t, resp.Error.Id, "api.web_socket_router.no_action.app_error", "should have been no action response") WebSocketClient.SendMessage("junk", nil) time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Error.Id != "api.web_socket_router.bad_action.app_error" { - t.Fatal("should have been bad action response") - } + resp = <-WebSocketClient.ResponseChannel + require.Equal(t, resp.Error.Id, "api.web_socket_router.bad_action.app_error", "should have been bad action response") req := &model.WebSocketRequest{} req.Seq = 0 req.Action = "ping" WebSocketClient.Conn.WriteJSON(req) time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Error.Id != "api.web_socket_router.bad_seq.app_error" { - t.Fatal("should have been bad action response") - } + resp = <-WebSocketClient.ResponseChannel + require.Equal(t, resp.Error.Id, "api.web_socket_router.bad_seq.app_error", "should have been bad action response") WebSocketClient.UserTyping("", "") time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Error.Id != "api.websocket_handler.invalid_param.app_error" { - t.Fatal("should have been invalid param response") - } else { - if resp.Error.DetailedError != "" { - t.Fatal("detailed error not cleared") - } - } + resp = <-WebSocketClient.ResponseChannel + require.Equal(t, resp.Error.Id, "api.websocket_handler.invalid_param.app_error", "should have been invalid param response") + require.Equal(t, resp.Error.DetailedError, "", "detailed error not cleared") +} + +func TestWebSocketTrailingSlash(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + url := fmt.Sprintf("ws://localhost:%v", th.App.Srv.ListenAddr.Port) + _, _, err := websocket.DefaultDialer.Dial(url+model.API_URL_SUFFIX+"/websocket/", nil) + require.NoError(t, err) } func TestWebSocketEvent(t *testing.T) { @@ -83,17 +80,14 @@ func TestWebSocketEvent(t *testing.T) { defer th.TearDown() WebSocketClient, err := th.CreateWebSocketClient() - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) defer WebSocketClient.Close() WebSocketClient.Listen() time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Status != model.STATUS_OK { - t.Fatal("should have responded OK to authentication challenge") - } + resp := <-WebSocketClient.ResponseChannel + require.Equal(t, resp.Status, model.STATUS_OK, "should have responded OK to authentication challenge") omitUser := make(map[string]bool, 1) omitUser["somerandomid"] = true @@ -123,9 +117,7 @@ func TestWebSocketEvent(t *testing.T) { stop <- true - if !eventHit { - t.Fatal("did not receive typing event") - } + require.True(t, eventHit, "did not receive typing event") evt2 := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_TYPING, "", "somerandomid", "", nil) th.App.Publish(evt2) @@ -150,9 +142,7 @@ func TestWebSocketEvent(t *testing.T) { stop <- true - if eventHit { - t.Fatal("got typing event for bad channel id") - } + require.False(t, eventHit, "got typing event for bad channel id") } func TestCreateDirectChannelWithSocket(t *testing.T) { @@ -170,21 +160,16 @@ func TestCreateDirectChannelWithSocket(t *testing.T) { } WebSocketClient, err := th.CreateWebSocketClient() - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) defer WebSocketClient.Close() WebSocketClient.Listen() time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Status != model.STATUS_OK { - t.Fatal("should have responded OK to authentication challenge") - } + resp := <-WebSocketClient.ResponseChannel + require.Equal(t, resp.Status, model.STATUS_OK, "should have responded OK to authentication challenge") wsr := <-WebSocketClient.EventChannel - if wsr.Event != model.WEBSOCKET_EVENT_HELLO { - t.Fatal("missing hello") - } + require.Equal(t, wsr.Event, model.WEBSOCKET_EVENT_HELLO, "missing hello") stop := make(chan bool) count := 0 @@ -205,19 +190,15 @@ func TestCreateDirectChannelWithSocket(t *testing.T) { for _, user := range users { time.Sleep(100 * time.Millisecond) - if _, resp := Client.CreateDirectChannel(th.BasicUser.Id, user.Id); resp.Error != nil { - t.Fatal("failed to create DM channel") - } + _, resp := Client.CreateDirectChannel(th.BasicUser.Id, user.Id) + require.Nil(t, resp.Error, "failed to create DM channel") } time.Sleep(5000 * time.Millisecond) stop <- true - if count != len(users) { - t.Fatal("We didn't get the proper amount of direct_added messages") - } - + require.Equal(t, count, len(users), "We didn't get the proper amount of direct_added messages") } func TestWebsocketOriginSecurity(t *testing.T) { @@ -230,53 +211,42 @@ func TestWebsocketOriginSecurity(t *testing.T) { _, _, err := websocket.DefaultDialer.Dial(url+model.API_URL_SUFFIX+"/websocket", http.Header{ "Origin": []string{"http://www.evil.com"}, }) - if err == nil { - t.Fatal("Should have errored because Origin does not match host! SECURITY ISSUE!") - } + + require.NotNil(t, err, "Should have errored because Origin does not match host! SECURITY ISSUE!") // We are not a browser so we can spoof this just fine _, _, err = websocket.DefaultDialer.Dial(url+model.API_URL_SUFFIX+"/websocket", http.Header{ "Origin": []string{fmt.Sprintf("http://localhost:%v", th.App.Srv.ListenAddr.Port)}, }) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err, err) // Should succeed now because open CORS th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowCorsFrom = "*" }) _, _, err = websocket.DefaultDialer.Dial(url+model.API_URL_SUFFIX+"/websocket", http.Header{ "Origin": []string{"http://www.evil.com"}, }) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err, err) // Should succeed now because matching CORS th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowCorsFrom = "http://www.evil.com" }) _, _, err = websocket.DefaultDialer.Dial(url+model.API_URL_SUFFIX+"/websocket", http.Header{ "Origin": []string{"http://www.evil.com"}, }) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err, err) // Should fail because non-matching CORS th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowCorsFrom = "http://www.good.com" }) _, _, err = websocket.DefaultDialer.Dial(url+model.API_URL_SUFFIX+"/websocket", http.Header{ "Origin": []string{"http://www.evil.com"}, }) - if err == nil { - t.Fatal("Should have errored because Origin contain AllowCorsFrom") - } + require.NotNil(t, err, "Should have errored because Origin contain AllowCorsFrom") // Should fail because non-matching CORS th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowCorsFrom = "http://www.good.com" }) _, _, err = websocket.DefaultDialer.Dial(url+model.API_URL_SUFFIX+"/websocket", http.Header{ "Origin": []string{"http://www.good.co"}, }) - if err == nil { - t.Fatal("Should have errored because Origin does not match host! SECURITY ISSUE!") - } + require.NotNil(t, err, "Should have errored because Origin does not match host! SECURITY ISSUE!") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowCorsFrom = "" }) } @@ -287,16 +257,13 @@ func TestWebSocketStatuses(t *testing.T) { Client := th.Client WebSocketClient, err := th.CreateWebSocketClient() - if err != nil { - t.Fatal(err) - } + require.Nil(t, err, err) defer WebSocketClient.Close() WebSocketClient.Listen() time.Sleep(300 * time.Millisecond) - if resp := <-WebSocketClient.ResponseChannel; resp.Status != model.STATUS_OK { - t.Fatal("should have responded OK to authentication challenge") - } + resp := <-WebSocketClient.ResponseChannel + require.Equal(t, resp.Status, model.STATUS_OK, "should have responded OK to authentication challenge") team := model.Team{DisplayName: "Name", Name: "z-z-" + model.NewId() + "a", Email: "test@nowhere.com", Type: model.TEAM_OPEN} rteam, _ := Client.CreateTeam(&team) @@ -318,79 +285,53 @@ func TestWebSocketStatuses(t *testing.T) { th.LoginBasic2() WebSocketClient2, err2 := th.CreateWebSocketClient() - if err2 != nil { - t.Fatal(err2) - } + require.Nil(t, err2, err2) time.Sleep(1000 * time.Millisecond) WebSocketClient.GetStatuses() - if resp := <-WebSocketClient.ResponseChannel; resp.Error != nil { - t.Fatal(resp.Error) - } else { - if resp.SeqReply != WebSocketClient.Sequence-1 { - t.Fatal("bad sequence number") - } + resp = <-WebSocketClient.ResponseChannel + require.Nil(t, resp.Error, resp.Error) - for _, status := range resp.Data { - if status != model.STATUS_OFFLINE && status != model.STATUS_AWAY && status != model.STATUS_ONLINE && status != model.STATUS_DND { - t.Fatalf("one of the statuses had an invalid value status=%v", status) - } - } + require.Equal(t, resp.SeqReply, WebSocketClient.Sequence-1, "bad sequence number") - if status, ok := resp.Data[th.BasicUser2.Id]; !ok { - t.Log(resp.Data) - t.Fatal("should have had user status") - } else if status != model.STATUS_ONLINE { - t.Log(status) - t.Fatal("status should have been online") - } + allowedValues := [4]string{model.STATUS_OFFLINE, model.STATUS_AWAY, model.STATUS_ONLINE, model.STATUS_DND} + for _, status := range resp.Data { + require.Containsf(t, allowedValues, status, "one of the statuses had an invalid value status=%v", status) } + status, ok := resp.Data[th.BasicUser2.Id] + require.True(t, ok, "should have had user status") + + require.Equal(t, status, model.STATUS_ONLINE, "status should have been online status=%v", status) + WebSocketClient.GetStatusesByIds([]string{th.BasicUser2.Id}) - if resp := <-WebSocketClient.ResponseChannel; resp.Error != nil { - t.Fatal(resp.Error) - } else { - if resp.SeqReply != WebSocketClient.Sequence-1 { - t.Fatal("bad sequence number") - } + resp = <-WebSocketClient.ResponseChannel + require.Nil(t, resp.Error, resp.Error) - for _, status := range resp.Data { - if status != model.STATUS_OFFLINE && status != model.STATUS_AWAY && status != model.STATUS_ONLINE { - t.Fatal("one of the statuses had an invalid value") - } - } + require.Equal(t, resp.SeqReply, WebSocketClient.Sequence-1, "bad sequence number") - if status, ok := resp.Data[th.BasicUser2.Id]; !ok { - t.Log(len(resp.Data)) - t.Fatal("should have had user status") - } else if status != model.STATUS_ONLINE { - t.Log(status) - t.Fatal("status should have been online") - } else if len(resp.Data) != 1 { - t.Fatal("only 1 status should be returned") - } + allowedValues = [4]string{model.STATUS_OFFLINE, model.STATUS_AWAY, model.STATUS_ONLINE} + for _, status := range resp.Data { + require.Containsf(t, allowedValues, status, "one of the statuses had an invalid value status") } + status, ok = resp.Data[th.BasicUser2.Id] + require.True(t, ok, "should have had user status") + + require.Equal(t, status, model.STATUS_ONLINE, "status should have been online status=%v", status) + require.Equal(t, len(resp.Data), 1, "only 1 status should be returned") + WebSocketClient.GetStatusesByIds([]string{ruser2.Id, "junk"}) - if resp := <-WebSocketClient.ResponseChannel; resp.Error != nil { - t.Fatal(resp.Error) - } else { - if resp.SeqReply != WebSocketClient.Sequence-1 { - t.Fatal("bad sequence number") - } - - if len(resp.Data) != 2 { - t.Fatal("2 statuses should be returned") - } - } + resp = <-WebSocketClient.ResponseChannel + require.Nil(t, resp.Error, resp.Error) + require.Equal(t, resp.SeqReply, WebSocketClient.Sequence-1, "bad sequence number") + require.Equal(t, len(resp.Data), 2, "2 statuses should be returned") WebSocketClient.GetStatusesByIds([]string{}) - if resp := <-WebSocketClient.ResponseChannel; resp.Error == nil { - if resp.SeqReply != WebSocketClient.Sequence-1 { - t.Fatal("bad sequence number") - } - t.Fatal("should have errored - empty user ids") + if resp2 := <-WebSocketClient.ResponseChannel; resp2.Error == nil { + require.Equal(t, resp2.SeqReply, WebSocketClient.Sequence-1, "bad sequence number") + require.NotNil(t, resp2.Error, "should have errored - empty user ids") } WebSocketClient2.Close() @@ -411,17 +352,12 @@ func TestWebSocketStatuses(t *testing.T) { time.Sleep(1500 * time.Millisecond) WebSocketClient.GetStatuses() - if resp := <-WebSocketClient.ResponseChannel; resp.Error != nil { - t.Fatal(resp.Error) - } else { - if resp.SeqReply != WebSocketClient.Sequence-1 { - t.Fatal("bad sequence number") - } + resp = <-WebSocketClient.ResponseChannel + require.Nil(t, resp.Error) - if _, ok := resp.Data[th.BasicUser2.Id]; ok { - t.Fatal("should not have had user status") - } - } + require.Equal(t, resp.SeqReply, WebSocketClient.Sequence-1, "bad sequence number") + _, ok = resp.Data[th.BasicUser2.Id] + require.False(t, ok, "should not have had user status") stop := make(chan bool) onlineHit := false @@ -449,12 +385,8 @@ func TestWebSocketStatuses(t *testing.T) { stop <- true - if !onlineHit { - t.Fatal("didn't get online event") - } - if !awayHit { - t.Fatal("didn't get away event") - } + require.True(t, onlineHit, "didn't get online event") + require.True(t, awayHit, "didn't get away event") time.Sleep(500 * time.Millisecond) diff --git a/app/app_test.go b/app/app_test.go index 647d2dc4b3..2701829a34 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -69,7 +69,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { assert.Equal(t, len(roles1), len(roleNames)) expected1 := map[string][]string{ - "channel_user": []string{ + "channel_user": { model.PERMISSION_READ_CHANNEL.Id, model.PERMISSION_ADD_REACTION.Id, model.PERMISSION_REMOVE_REACTION.Id, @@ -86,10 +86,10 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_DELETE_POST.Id, model.PERMISSION_EDIT_POST.Id, }, - "channel_admin": []string{ + "channel_admin": { model.PERMISSION_MANAGE_CHANNEL_ROLES.Id, }, - "team_user": []string{ + "team_user": { model.PERMISSION_LIST_TEAM_CHANNELS.Id, model.PERMISSION_JOIN_PUBLIC_CHANNELS.Id, model.PERMISSION_READ_PUBLIC_CHANNEL.Id, @@ -99,13 +99,13 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_INVITE_USER.Id, model.PERMISSION_ADD_USER_TO_TEAM.Id, }, - "team_post_all": []string{ + "team_post_all": { model.PERMISSION_CREATE_POST.Id, }, - "team_post_all_public": []string{ + "team_post_all_public": { model.PERMISSION_CREATE_POST_PUBLIC.Id, }, - "team_admin": []string{ + "team_admin": { model.PERMISSION_REMOVE_USER_FROM_TEAM.Id, model.PERMISSION_MANAGE_TEAM.Id, model.PERMISSION_IMPORT_TEAM.Id, @@ -120,7 +120,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_DELETE_POST.Id, model.PERMISSION_DELETE_OTHERS_POSTS.Id, }, - "system_user": []string{ + "system_user": { model.PERMISSION_LIST_PUBLIC_TEAMS.Id, model.PERMISSION_JOIN_PUBLIC_TEAMS.Id, model.PERMISSION_CREATE_DIRECT_CHANNEL.Id, @@ -128,18 +128,18 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_VIEW_MEMBERS.Id, model.PERMISSION_CREATE_TEAM.Id, }, - "system_post_all": []string{ + "system_post_all": { model.PERMISSION_CREATE_POST.Id, }, - "system_post_all_public": []string{ + "system_post_all_public": { model.PERMISSION_CREATE_POST_PUBLIC.Id, }, - "system_user_access_token": []string{ + "system_user_access_token": { model.PERMISSION_CREATE_USER_ACCESS_TOKEN.Id, model.PERMISSION_READ_USER_ACCESS_TOKEN.Id, model.PERMISSION_REVOKE_USER_ACCESS_TOKEN.Id, }, - "system_admin": []string{ + "system_admin": { model.PERMISSION_ASSIGN_SYSTEM_ADMIN_ROLE.Id, model.PERMISSION_MANAGE_SYSTEM.Id, model.PERMISSION_MANAGE_ROLES.Id, @@ -254,7 +254,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { // Check the role permissions. expected2 := map[string][]string{ - "channel_user": []string{ + "channel_user": { model.PERMISSION_READ_CHANNEL.Id, model.PERMISSION_ADD_REACTION.Id, model.PERMISSION_REMOVE_REACTION.Id, @@ -269,10 +269,10 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_DELETE_POST.Id, model.PERMISSION_EDIT_POST.Id, }, - "channel_admin": []string{ + "channel_admin": { model.PERMISSION_MANAGE_CHANNEL_ROLES.Id, }, - "team_user": []string{ + "team_user": { model.PERMISSION_LIST_TEAM_CHANNELS.Id, model.PERMISSION_JOIN_PUBLIC_CHANNELS.Id, model.PERMISSION_READ_PUBLIC_CHANNEL.Id, @@ -282,13 +282,13 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_INVITE_USER.Id, model.PERMISSION_ADD_USER_TO_TEAM.Id, }, - "team_post_all": []string{ + "team_post_all": { model.PERMISSION_CREATE_POST.Id, }, - "team_post_all_public": []string{ + "team_post_all_public": { model.PERMISSION_CREATE_POST_PUBLIC.Id, }, - "team_admin": []string{ + "team_admin": { model.PERMISSION_REMOVE_USER_FROM_TEAM.Id, model.PERMISSION_MANAGE_TEAM.Id, model.PERMISSION_IMPORT_TEAM.Id, @@ -305,7 +305,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_DELETE_POST.Id, model.PERMISSION_DELETE_OTHERS_POSTS.Id, }, - "system_user": []string{ + "system_user": { model.PERMISSION_LIST_PUBLIC_TEAMS.Id, model.PERMISSION_JOIN_PUBLIC_TEAMS.Id, model.PERMISSION_CREATE_DIRECT_CHANNEL.Id, @@ -313,18 +313,18 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_VIEW_MEMBERS.Id, model.PERMISSION_CREATE_TEAM.Id, }, - "system_post_all": []string{ + "system_post_all": { model.PERMISSION_CREATE_POST.Id, }, - "system_post_all_public": []string{ + "system_post_all_public": { model.PERMISSION_CREATE_POST_PUBLIC.Id, }, - "system_user_access_token": []string{ + "system_user_access_token": { model.PERMISSION_CREATE_USER_ACCESS_TOKEN.Id, model.PERMISSION_READ_USER_ACCESS_TOKEN.Id, model.PERMISSION_REVOKE_USER_ACCESS_TOKEN.Id, }, - "system_admin": []string{ + "system_admin": { model.PERMISSION_ASSIGN_SYSTEM_ADMIN_ROLE.Id, model.PERMISSION_MANAGE_SYSTEM.Id, model.PERMISSION_MANAGE_ROLES.Id, diff --git a/app/channel.go b/app/channel.go index 10c9c5226c..d5e10f8b1f 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1525,9 +1525,12 @@ func (a *App) LeaveChannel(channelId string, userId string) *model.AppError { func (a *App) postLeaveChannelMessage(user *model.User, channel *model.Channel) *model.AppError { post := &model.Post{ ChannelId: channel.Id, - Message: fmt.Sprintf(utils.T("api.channel.leave.left"), user.Username), - Type: model.POST_LEAVE_CHANNEL, - UserId: user.Id, + // Message here embeds `@username`, not just `username`, to ensure that mentions + // treat this as a username mention even though the user has now left the channel. + // The client renders its own system message, ignoring this value altogether. + Message: fmt.Sprintf(utils.T("api.channel.leave.left"), fmt.Sprintf("@%s", user.Username)), + Type: model.POST_LEAVE_CHANNEL, + UserId: user.Id, Props: model.StringInterface{ "username": user.Username, }, @@ -1595,9 +1598,12 @@ func (a *App) postAddToTeamMessage(user *model.User, addedUser *model.User, chan func (a *App) postRemoveFromChannelMessage(removerUserId string, removedUser *model.User, channel *model.Channel) *model.AppError { post := &model.Post{ ChannelId: channel.Id, - Message: fmt.Sprintf(utils.T("api.channel.remove_member.removed"), removedUser.Username), - Type: model.POST_REMOVE_FROM_CHANNEL, - UserId: removerUserId, + // Message here embeds `@username`, not just `username`, to ensure that mentions + // treat this as a username mention even though the user has now left the channel. + // The client renders its own system message, ignoring this value altogether. + Message: fmt.Sprintf(utils.T("api.channel.remove_member.removed"), fmt.Sprintf("@%s", removedUser.Username)), + Type: model.POST_REMOVE_FROM_CHANNEL, + UserId: removerUserId, Props: model.StringInterface{ "removedUserId": removedUser.Id, "removedUsername": removedUser.Username, diff --git a/app/command.go b/app/command.go index c7f83c5b1e..678031dbbb 100644 --- a/app/command.go +++ b/app/command.go @@ -39,8 +39,13 @@ func GetCommandProvider(name string) CommandProvider { return nil } -func (a *App) CreateCommandPost(post *model.Post, teamId string, response *model.CommandResponse) (*model.Post, *model.AppError) { - post.Message = model.ParseSlackLinksToMarkdown(response.Text) +func (a *App) CreateCommandPost(post *model.Post, teamId string, response *model.CommandResponse, skipSlackParsing bool) (*model.Post, *model.AppError) { + if skipSlackParsing { + post.Message = response.Text + } else { + post.Message = model.ParseSlackLinksToMarkdown(response.Text) + } + post.CreateAt = model.GetMillis() if strings.HasPrefix(post.Type, model.POST_SYSTEM_MESSAGE_PREFIX) { @@ -430,11 +435,16 @@ func (a *App) HandleCommandResponsePost(command *model.Command, args *model.Comm post.AddProp("from_webhook", "true") } - // Process Slack text replacements - response.Text = a.ProcessSlackText(response.Text) - response.Attachments = a.ProcessSlackAttachments(response.Attachments) + // Do not process text if this is a code block + skipSlackParsing := command.Trigger == "code" - if _, err := a.CreateCommandPost(post, args.TeamId, response); err != nil { + // Process Slack text replacements + if !skipSlackParsing { + response.Text = a.ProcessSlackText(response.Text) + response.Attachments = a.ProcessSlackAttachments(response.Attachments) + } + + if _, err := a.CreateCommandPost(post, args.TeamId, response, skipSlackParsing); err != nil { return post, err } diff --git a/app/command_test.go b/app/command_test.go index 85e614d4f6..03e0bda7f3 100644 --- a/app/command_test.go +++ b/app/command_test.go @@ -68,7 +68,8 @@ func TestCreateCommandPost(t *testing.T) { Text: "some message", } - _, err := th.App.CreateCommandPost(post, th.BasicTeam.Id, resp) + skipSlackParsing := false + _, err := th.App.CreateCommandPost(post, th.BasicTeam.Id, resp, skipSlackParsing) if err == nil || err.Id != "api.context.invalid_param.app_error" { t.Fatal("should have failed - bad post type") } @@ -188,7 +189,7 @@ func TestHandleCommandResponsePost(t *testing.T) { // Test Slack attachments text conversion. resp.Attachments = []*model.SlackAttachment{ - &model.SlackAttachment{ + { Text: "", }, } @@ -205,7 +206,23 @@ func TestHandleCommandResponsePost(t *testing.T) { if err == nil || err.Id != "api.command.command_post.forbidden.app_error" { t.Fatal("should have failed - forbidden channel post") } + + // Test that /code text is not converted with the Slack text conversion. + command.Trigger = "code" + resp.ChannelId = "" + resp.Text = "" + resp.Attachments = []*model.SlackAttachment{ + { + Text: "", + }, + } + post, err = th.App.HandleCommandResponsePost(command, args, resp, builtIn) + + assert.Nil(t, err) + assert.Equal(t, resp.Text, post.Message, "/code text should not be converted to Slack links") + assert.Equal(t, "", resp.Attachments[0].Text) } + func TestHandleCommandResponse(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -240,10 +257,10 @@ func TestHandleCommandResponse(t *testing.T) { resp = &model.CommandResponse{ Text: "message 1", ExtraResponses: []*model.CommandResponse{ - &model.CommandResponse{ + { Text: "message 2", }, - &model.CommandResponse{ + { Type: model.POST_SYSTEM_GENERIC, Text: "message 3", }, @@ -257,8 +274,8 @@ func TestHandleCommandResponse(t *testing.T) { resp = &model.CommandResponse{ ExtraResponses: []*model.CommandResponse{ - &model.CommandResponse{}, - &model.CommandResponse{}, + {}, + {}, }, } diff --git a/app/diagnostics.go b/app/diagnostics.go index ac7203156d..08d9c150e3 100644 --- a/app/diagnostics.go +++ b/app/diagnostics.go @@ -300,6 +300,7 @@ func (a *App) trackConfig() { "disable_bots_when_owner_is_deactivated": *cfg.ServiceSettings.DisableBotsWhenOwnerIsDeactivated, "enable_bot_account_creation": *cfg.ServiceSettings.EnableBotAccountCreation, "enable_svgs": *cfg.ServiceSettings.EnableSVGs, + "enable_latex": *cfg.ServiceSettings.EnableLatex, }) a.SendDiagnostic(TRACK_CONFIG_TEAM, map[string]interface{}{ diff --git a/app/diagnostics_test.go b/app/diagnostics_test.go index 8d9b51da1c..50d8b85a18 100644 --- a/app/diagnostics_test.go +++ b/app/diagnostics_test.go @@ -20,7 +20,7 @@ import ( func TestPluginSetting(t *testing.T) { settings := &model.PluginSettings{ Plugins: map[string]map[string]interface{}{ - "test": map[string]interface{}{ + "test": { "foo": "bar", }, }, @@ -31,10 +31,10 @@ func TestPluginSetting(t *testing.T) { func TestPluginActivated(t *testing.T) { states := map[string]*model.PluginState{ - "foo": &model.PluginState{ + "foo": { Enable: true, }, - "bar": &model.PluginState{ + "bar": { Enable: false, }, } diff --git a/app/email_batching_test.go b/app/email_batching_test.go index 09c2511ccf..aed286f0d9 100644 --- a/app/email_batching_test.go +++ b/app/email_batching_test.go @@ -64,11 +64,11 @@ func TestHandleNewNotifications(t *testing.T) { job.Add(&model.User{Id: id1}, &model.Post{UserId: id1, Message: "test4"}, &model.Team{Name: "team"}) job.Add(&model.User{Id: id2}, &model.Post{UserId: id1, Message: "test5"}, &model.Team{Name: "team"}) job.handleNewNotifications() - assert.Equal(t, job.pendingNotifications[id1][0].post.Message, "test1", "incorrect order of received posts for user1"); - assert.Equal(t, job.pendingNotifications[id1][1].post.Message, "test2", "incorrect order of received posts for user1"); - assert.Equal(t, job.pendingNotifications[id1][2].post.Message, "test4", "incorrect order of received posts for user1"); - assert.Equal(t, job.pendingNotifications[id2][0].post.Message, "test3", "incorrect order of received posts for user2"); - assert.Equal(t, job.pendingNotifications[id2][1].post.Message, "test5", "incorrect order of received posts for user2"); + assert.Equal(t, job.pendingNotifications[id1][0].post.Message, "test1", "incorrect order of received posts for user1") + assert.Equal(t, job.pendingNotifications[id1][1].post.Message, "test2", "incorrect order of received posts for user1") + assert.Equal(t, job.pendingNotifications[id1][2].post.Message, "test4", "incorrect order of received posts for user1") + assert.Equal(t, job.pendingNotifications[id2][0].post.Message, "test3", "incorrect order of received posts for user2") + assert.Equal(t, job.pendingNotifications[id2][1].post.Message, "test5", "incorrect order of received posts for user2") } func TestCheckPendingNotifications(t *testing.T) { diff --git a/app/group.go b/app/group.go index e84cd34d9e..690e5bf582 100644 --- a/app/group.go +++ b/app/group.go @@ -11,6 +11,10 @@ func (a *App) GetGroup(id string) (*model.Group, *model.AppError) { return a.Srv.Store.Group().Get(id) } +func (a *App) GetGroupByName(name string) (*model.Group, *model.AppError) { + return a.Srv.Store.Group().GetByName(name) +} + func (a *App) GetGroupByRemoteID(remoteID string, groupSource model.GroupSource) (*model.Group, *model.AppError) { return a.Srv.Store.Group().GetByRemoteID(remoteID, groupSource) } @@ -19,6 +23,10 @@ func (a *App) GetGroupsBySource(groupSource model.GroupSource) ([]*model.Group, return a.Srv.Store.Group().GetAllBySource(groupSource) } +func (a *App) GetGroupsByUserId(userId string) ([]*model.Group, *model.AppError) { + return a.Srv.Store.Group().GetByUser(userId) +} + func (a *App) CreateGroup(group *model.Group) (*model.Group, *model.AppError) { return a.Srv.Store.Group().Create(group) } diff --git a/app/import_validators_test.go b/app/import_validators_test.go index 48de801350..59d8188cc5 100644 --- a/app/import_validators_test.go +++ b/app/import_validators_test.go @@ -487,7 +487,7 @@ func TestImportValidateUserImportData(t *testing.T) { data.Username = ptrStr("i am a username with spaces and !!!") err = validateUserImportData(&data) - require.NotNil(t, err, "Validation should have failed due to invalid characters in Username."); + require.NotNil(t, err, "Validation should have failed due to invalid characters in Username.") data.Username = ptrStr("bob") @@ -759,7 +759,6 @@ func TestImportValidateReactionImportData(t *testing.T) { err = validateReactionImportData(&data, parentCreateAt) require.NotNil(t, err, "Should have failed due to missing required property.") - data = ReactionImportData{ User: ptrStr("username"), EmojiName: ptrStr("emoji"), @@ -938,13 +937,13 @@ func TestImportValidatePostImportData(t *testing.T) { require.NotNil(t, err, "Should have failed due to 0 create-at value.") // Test with valid all optional parameters. - reactions := []ReactionImportData{ReactionImportData{ + reactions := []ReactionImportData{{ User: ptrStr("username"), EmojiName: ptrStr("emoji"), CreateAt: ptrInt64(model.GetMillis()), }} - replies := []ReplyImportData{ReplyImportData{ + replies := []ReplyImportData{{ User: ptrStr("username"), Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), @@ -1240,13 +1239,13 @@ func TestImportValidateDirectPostImportData(t *testing.T) { require.Nil(t, err, "Validation should succeed with post flagged by members") // Test with valid all optional parameters. - reactions := []ReactionImportData{ReactionImportData{ + reactions := []ReactionImportData{{ User: ptrStr("username"), EmojiName: ptrStr("emoji"), CreateAt: ptrInt64(model.GetMillis()), }} - replies := []ReplyImportData{ReplyImportData{ + replies := []ReplyImportData{{ User: ptrStr("username"), Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), diff --git a/app/integration_action_test.go b/app/integration_action_test.go index d204efcd23..6474a16b29 100644 --- a/app/integration_action_test.go +++ b/app/integration_action_test.go @@ -430,7 +430,7 @@ func TestSubmitInteractiveDialog(t *testing.T) { assert.Equal(t, "value1", val) resp := model.SubmitDialogResponse{ - Error: "some generic error", + Error: "some generic error", Errors: map[string]string{"name1": "some error"}, } diff --git a/app/ldap.go b/app/ldap.go index 5219ef8249..22a8568a68 100644 --- a/app/ldap.go +++ b/app/ldap.go @@ -52,7 +52,7 @@ func (a *App) GetLdapGroup(ldapGroupID string) (*model.Group, *model.AppError) { } } else { ae := model.NewAppError("GetLdapGroup", "ent.ldap.app_error", nil, "", http.StatusNotImplemented) - mlog.Error(fmt.Sprintf("%v", ae.Error())) + mlog.Error("Unable to use ldap", mlog.String("ldap_group_id", ldapGroupID), mlog.Err(ae)) return nil, ae } @@ -73,7 +73,7 @@ func (a *App) GetAllLdapGroupsPage(page int, perPage int, opts model.LdapGroupSe } } else { ae := model.NewAppError("GetAllLdapGroupsPage", "ent.ldap.app_error", nil, "", http.StatusNotImplemented) - mlog.Error(fmt.Sprintf("%v", ae.Error())) + mlog.Error("Unable to use ldap", mlog.Err(ae)) return nil, 0, ae } diff --git a/app/login.go b/app/login.go index 8b4a7fcaf7..31f1126fdd 100644 --- a/app/login.go +++ b/app/login.go @@ -162,6 +162,7 @@ func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, } w.Header().Set(model.HEADER_TOKEN, session.Token) + a.Session = *session if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { a.Srv.Go(func() { diff --git a/app/migrations.go b/app/migrations.go index 1503e44604..8c4b5fe0c5 100644 --- a/app/migrations.go +++ b/app/migrations.go @@ -95,9 +95,9 @@ func (a *App) DoEmojisPermissionsMigration() { return } - var role *model.Role = nil - var systemAdminRole *model.Role = nil - var err *model.AppError = nil + var role *model.Role + var systemAdminRole *model.Role + var err *model.AppError mlog.Info("Migrating emojis config to database.") switch *a.Config().ServiceSettings.DEPRECATED_DO_NOT_USE_RestrictCustomEmojiCreation { diff --git a/app/notification_email.go b/app/notification_email.go index a2fbb6766d..cdd7ae7ef9 100644 --- a/app/notification_email.go +++ b/app/notification_email.go @@ -100,7 +100,7 @@ func (a *App) sendNotificationEmail(notification *postNotification, user *model. a.Srv.Go(func() { if err := a.SendNotificationMail(user.Email, html.UnescapeString(subjectText), bodyText); err != nil { - mlog.Error(fmt.Sprint("Error to send the email", user.Email, err)) + mlog.Error("Error while sending the email", mlog.String("email", user.Email), mlog.Err(err)) } }) @@ -320,7 +320,7 @@ func (a *App) GetMessageForNotification(post *model.Post, translateFunc i18n.Tra // extract the filenames from their paths and determine what type of files are attached infos, err := a.Srv.Store.FileInfo().GetForPost(post.Id, true, false, true) if err != nil { - mlog.Warn(fmt.Sprintf("Encountered error when getting files for notification message, post_id=%v, err=%v", post.Id, err), mlog.String("post_id", post.Id)) + mlog.Warn("Encountered error when getting files for notification message", mlog.String("post_id", post.Id), mlog.Err(err)) } filenames := make([]string, len(infos)) diff --git a/app/notification_push.go b/app/notification_push.go index 546ba2ff3a..f20f3cd669 100644 --- a/app/notification_push.go +++ b/app/notification_push.go @@ -30,7 +30,6 @@ type PushNotificationsHub struct { } type PushNotification struct { - id string notificationType NotificationType currentSessionId string userId string @@ -368,8 +367,8 @@ func ShouldSendPushNotification(user *model.User, channelNotifyProps model.Strin func DoesNotifyPropsAllowPushNotification(user *model.User, channelNotifyProps model.StringMap, post *model.Post, wasMentioned bool) bool { userNotifyProps := user.NotifyProps userNotify := userNotifyProps[model.PUSH_NOTIFY_PROP] - channelNotify, _ := channelNotifyProps[model.PUSH_NOTIFY_PROP] - if channelNotify == "" { + channelNotify, ok := channelNotifyProps[model.PUSH_NOTIFY_PROP] + if !ok || channelNotify == "" { channelNotify = model.CHANNEL_NOTIFY_DEFAULT } diff --git a/app/permissions_migrations.go b/app/permissions_migrations.go index 31ecc8318f..d4fe8019fe 100644 --- a/app/permissions_migrations.go +++ b/app/permissions_migrations.go @@ -282,11 +282,7 @@ func getAddManageGuestsPermissionsMigration() permissionsMap { return permissionsMap{ permissionTransformation{ On: isRole(model.SYSTEM_ADMIN_ROLE_ID), - Add: []string{PERMISSION_PROMOTE_GUEST, PERMISSION_DEMOTE_TO_GUEST}, - }, - permissionTransformation{ - On: permissionExists(PERMISSION_INVITE_USER), - Add: []string{PERMISSION_INVITE_GUEST}, + Add: []string{PERMISSION_PROMOTE_GUEST, PERMISSION_DEMOTE_TO_GUEST, PERMISSION_INVITE_GUEST}, }, } } diff --git a/app/plugin_api.go b/app/plugin_api.go index dda5e1ae10..d228a88d0d 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -445,6 +445,18 @@ func (api *PluginAPI) DeleteChannelMember(channelId, userId string) *model.AppEr return api.app.LeaveChannel(channelId, userId) } +func (api *PluginAPI) GetGroup(groupId string) (*model.Group, *model.AppError) { + return api.app.GetGroup(groupId) +} + +func (api *PluginAPI) GetGroupByName(name string) (*model.Group, *model.AppError) { + return api.app.GetGroupByName(name) +} + +func (api *PluginAPI) GetGroupsForUser(userId string) ([]*model.Group, *model.AppError) { + return api.app.GetGroupsByUserId(userId) +} + func (api *PluginAPI) CreatePost(post *model.Post) (*model.Post, *model.AppError) { return api.app.CreatePostMissingChannel(post, true) } diff --git a/app/plugin_hooks_test.go b/app/plugin_hooks_test.go index df88b99b9b..2feabc26de 100644 --- a/app/plugin_hooks_test.go +++ b/app/plugin_hooks_test.go @@ -571,7 +571,7 @@ func TestHookFileWillBeUploaded(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, response) assert.Equal(t, 1, len(response.FileInfos)) - + fileId := response.FileInfos[0].Id fileInfo, err := th.App.GetFileInfo(fileId) assert.Nil(t, err) diff --git a/app/post_metadata.go b/app/post_metadata.go index f5d751bbf5..b67f6f1ca4 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -73,8 +73,6 @@ func (a *App) OverrideIconURLIfEmoji(post *model.Post) { } else { mlog.Warn("Failed to retrieve URL for overriden profile icon (emoji)", mlog.String("emojiName", emojiName), mlog.Err(err)) } - - return } func (a *App) PreparePostForClient(originalPost *model.Post, isNewPost bool, isEditPost bool) *model.Post { @@ -87,6 +85,11 @@ func (a *App) PreparePostForClient(originalPost *model.Post, isNewPost bool, isE post.Metadata = &model.PostMetadata{} + if post.DeleteAt > 0 { + // Don't fill out metadata for deleted posts + return post + } + // Emojis and reaction counts if emojis, reactions, err := a.getEmojisAndReactionsForPost(post); err != nil { mlog.Warn("Failed to get emojis and reactions for a post", mlog.String("post_id", post.Id), mlog.Err(err)) diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index 240f18df1d..fa3840dd73 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -307,18 +307,22 @@ func TestPreparePostForClient(t *testing.T) { t.Run("does not override icon URL", func(t *testing.T) { clientPost := prepare(false, url, emoji) - s, _ := clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_URL] + s, ok := clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_URL] + assert.True(t, ok) assert.EqualValues(t, url, s) - s, _ = clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_EMOJI] + s, ok = clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_EMOJI] + assert.True(t, ok) assert.EqualValues(t, emoji, s) }) t.Run("overrides icon URL", func(t *testing.T) { clientPost := prepare(true, url, emoji) - s, _ := clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_URL] + s, ok := clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_URL] + assert.True(t, ok) assert.EqualValues(t, overridenUrl, s) - s, _ = clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_EMOJI] + s, ok = clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_EMOJI] + assert.True(t, ok) assert.EqualValues(t, emoji, s) }) @@ -477,6 +481,36 @@ func TestPreparePostForClient(t *testing.T) { }, imageDimensions[server.URL+"/test-image1.png"]) }) }) + + t.Run("no metadata for deleted posts", func(t *testing.T) { + th := setup() + defer th.TearDown() + + fileInfo, err := th.App.DoUploadFile(time.Now(), th.BasicTeam.Id, th.BasicChannel.Id, th.BasicUser.Id, "test.txt", []byte("test")) + require.Nil(t, err) + + post, err := th.App.CreatePost(&model.Post{ + Message: "test", + FileIds: []string{fileInfo.Id}, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + }, th.BasicChannel, false) + require.Nil(t, err) + + th.AddReactionToPost(post, th.BasicUser, "taco") + + post, err = th.App.DeletePost(post.Id, th.BasicUser.Id) + require.Nil(t, err) + + // DeleteAt isn't set on the post returned by App.DeletePost + post.DeleteAt = model.GetMillis() + + clientPost := th.App.PreparePostForClient(post, false, false) + + assert.NotEqual(t, nil, clientPost.Metadata, "should've populated Metadata“") + assert.Nil(t, clientPost.Metadata.Reactions, "should not have populated Reactions") + assert.Nil(t, clientPost.Metadata.Files, "should not have populated Files") + }) } func TestPreparePostForClientWithImageProxy(t *testing.T) { diff --git a/app/server.go b/app/server.go index 827367dc06..fb7ec94f25 100644 --- a/app/server.go +++ b/app/server.go @@ -633,7 +633,7 @@ func (a *App) OriginChecker() func(*http.Request) bool { func (s *Server) checkPushNotificationServerUrl() { notificationServer := *s.Config().EmailSettings.PushNotificationServer - if strings.HasPrefix(notificationServer, "http://") == true { + if strings.HasPrefix(notificationServer, "http://") { mlog.Warn("Your push notification server is configured with HTTP. For improved security, update to HTTPS in your configuration.") } } diff --git a/app/user.go b/app/user.go index da2e016c21..878ce66a31 100644 --- a/app/user.go +++ b/app/user.go @@ -1456,6 +1456,10 @@ func (a *App) PermanentDeleteUser(user *model.User) *model.AppError { return err } + if err := a.Srv.Store.Group().PermanentDeleteMembersByUser(user.Id); err != nil { + return err + } + if err := a.Srv.Store.Post().PermanentDeleteByUser(user.Id); err != nil { return err } diff --git a/app/web_conn.go b/app/web_conn.go index bc014371ca..bb12e7074b 100644 --- a/app/web_conn.go +++ b/app/web_conn.go @@ -331,10 +331,7 @@ func (webCon *WebConn) ShouldSendEvent(msg *model.WebSocketEvent) bool { // If the event is destined to a specific user if len(msg.Broadcast.UserId) > 0 { - if webCon.UserId == msg.Broadcast.UserId { - return true - } - return false + return webCon.UserId == msg.Broadcast.UserId } // if the user is omitted don't send the message @@ -397,10 +394,5 @@ func (webCon *WebConn) IsMemberOfTeam(teamId string) bool { currentSession = session } - member := currentSession.GetTeamByTeamId(teamId) - - if member != nil { - return true - } - return false + return currentSession.GetTeamByTeamId(teamId) != nil } diff --git a/app/web_hub_test.go b/app/web_hub_test.go index 27b922a3a3..780c10b6e7 100644 --- a/app/web_hub_test.go +++ b/app/web_hub_test.go @@ -50,7 +50,7 @@ func TestHubStopWithMultipleConnections(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - s := httptest.NewServer(http.HandlerFunc(dummyWebsocketHandler(t))) + s := httptest.NewServer(dummyWebsocketHandler(t)) defer s.Close() th.App.HubStart() @@ -68,7 +68,7 @@ func TestHubStopRaceCondition(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - s := httptest.NewServer(http.HandlerFunc(dummyWebsocketHandler(t))) + s := httptest.NewServer(dummyWebsocketHandler(t)) th.App.HubStart() wc1 := registerDummyWebConn(t, th.App, s.Listener.Addr(), th.BasicUser.Id) diff --git a/app/webhook_test.go b/app/webhook_test.go index d56f45f14a..3822d16c79 100644 --- a/app/webhook_test.go +++ b/app/webhook_test.go @@ -389,13 +389,13 @@ func TestSplitWebhookPost(t *testing.T) { Message: strings.Repeat("本", maxPostSize*3/2), Props: map[string]interface{}{ "attachments": []*model.SlackAttachment{ - &model.SlackAttachment{ + { Text: strings.Repeat("本", 1000), }, - &model.SlackAttachment{ + { Text: strings.Repeat("本", 2000), }, - &model.SlackAttachment{ + { Text: strings.Repeat("本", model.POST_PROPS_MAX_USER_RUNES-1000), }, }, @@ -409,10 +409,10 @@ func TestSplitWebhookPost(t *testing.T) { Message: strings.Repeat("本", maxPostSize/2), Props: map[string]interface{}{ "attachments": []*model.SlackAttachment{ - &model.SlackAttachment{ + { Text: strings.Repeat("本", 1000), }, - &model.SlackAttachment{ + { Text: strings.Repeat("本", 2000), }, }, @@ -421,7 +421,7 @@ func TestSplitWebhookPost(t *testing.T) { { Props: map[string]interface{}{ "attachments": []*model.SlackAttachment{ - &model.SlackAttachment{ + { Text: strings.Repeat("本", model.POST_PROPS_MAX_USER_RUNES-1000), }, }, diff --git a/build/Jenkinsfile.branch b/build/Jenkinsfile.branch index 2446270ae2..339c9e3b81 100644 --- a/build/Jenkinsfile.branch +++ b/build/Jenkinsfile.branch @@ -124,7 +124,7 @@ pipeline { withCredentials([usernamePassword(credentialsId: 'matterbuild-docker-hub', usernameVariable: 'DOCKER_USER', passwordVariable: 'DOCKER_PASS')]) { sh 'docker login --username ${DOCKER_USER} --password ${DOCKER_PASS}' sh """ - export TAG='${env.BRANCH_NAME}.${env.BUILD_NUMBER}.${SERVER_GIT_COMMIT}' + export TAG='${env.BRANCH_NAME}' docker build --no-cache --build-arg MM_PACKAGE=https://releases.mattermost.com/mattermost-platform/${env.BRANCH_NAME}/mattermost-enterprise-linux-amd64.tar.gz -t mattermost/mattermost-enterprise-edition:\${TAG} build docker push mattermost/mattermost-enterprise-edition:\${TAG} docker logout @@ -133,6 +133,25 @@ pipeline { } } } + + stage ('Build Docker mattermost-team-edition') { + agent { + label 'default-mm-builder' + } + steps { + dir('src/github.com/mattermost/mattermost-server') { + withCredentials([usernamePassword(credentialsId: 'matterbuild-docker-hub', usernameVariable: 'DOCKER_USER', passwordVariable: 'DOCKER_PASS')]) { + sh 'docker login --username ${DOCKER_USER} --password ${DOCKER_PASS}' + sh """ + export TAG='${env.BRANCH_NAME}' + docker build --no-cache --build-arg MM_PACKAGE=https://releases.mattermost.com/mattermost-platform/${env.BRANCH_NAME}/mattermost-team-linux-amd64.tar.gz -t mattermost/mattermost-team-edition:\${TAG} build + docker push mattermost/mattermost-team-edition:\${TAG} + docker logout + """ + } + } + } + } } post { diff --git a/cmd/mattermost/commands/channel_test.go b/cmd/mattermost/commands/channel_test.go index e6109a7d7a..d6003adc3b 100644 --- a/cmd/mattermost/commands/channel_test.go +++ b/cmd/mattermost/commands/channel_test.go @@ -86,17 +86,17 @@ func TestListChannels(t *testing.T) { output := th.CheckCommand(t, "channel", "list", th.BasicTeam.Name) - require.True(t, strings.Contains(string(output), "town-square"), "should have channels") + require.True(t, strings.Contains(output, "town-square"), "should have channels") - require.True(t, strings.Contains(string(output), channel.Name+" (archived)"), "should have archived channel") + require.True(t, strings.Contains(output, channel.Name+" (archived)"), "should have archived channel") - require.True(t, strings.Contains(string(output), privateChannel.Name+" (private)"), "should have private channel") + require.True(t, strings.Contains(output, privateChannel.Name+" (private)"), "should have private channel") th.Client.Must(th.Client.DeleteChannel(privateChannel.Id)) output = th.CheckCommand(t, "channel", "list", th.BasicTeam.Name) - require.True(t, strings.Contains(string(output), privateChannel.Name+" (archived) (private)"), "should have a channel both archived and private") + require.True(t, strings.Contains(output, privateChannel.Name+" (archived) (private)"), "should have a channel both archived and private") } func TestRestoreChannel(t *testing.T) { diff --git a/cmd/mattermost/commands/command.go b/cmd/mattermost/commands/command.go index c264a7e772..a7637f3ba1 100644 --- a/cmd/mattermost/commands/command.go +++ b/cmd/mattermost/commands/command.go @@ -153,7 +153,7 @@ func createCommandCmdF(command *cobra.Command, args []string) error { autocompleteHint, _ := command.Flags().GetString("autocompleteHint") post, errp := command.Flags().GetBool("post") method := "P" - if errp != nil || post == false { + if errp != nil || !post { method = "G" } @@ -371,7 +371,7 @@ func modifyCommandCmdF(command *cobra.Command, args []string) error { post, err := command.Flags().GetBool("post") method := "P" - if err != nil || post == false { + if err != nil || !post { method = "G" } modifiedCommand.Method = method diff --git a/cmd/mattermost/commands/command_test.go b/cmd/mattermost/commands/command_test.go index b3e304c4da..3d599e0136 100644 --- a/cmd/mattermost/commands/command_test.go +++ b/cmd/mattermost/commands/command_test.go @@ -119,14 +119,12 @@ func TestCreateCommand(t *testing.T) { require.Nil(t, response.Error, "Failed to list commands") if testCase.ExpectedErr == "" { - require.NotEmpty(t, cmds, "Failed to create command") - require.Equal(t, "testcmd", cmds[0].Trigger) - assert.Contains(t, string(actual), "PASS") + assert.NotZero(t, len(cmds), "Failed to create command") + assert.Equal(t, cmds[0].Trigger, "testcmd", "Failed to create command") + assert.Contains(t, actual, "PASS") } else { - if len(cmds) > 1 { - require.Fail(t, "Created command that shouldn't have been created") - } - assert.Contains(t, string(actual), testCase.ExpectedErr) + assert.LessOrEqual(t, len(cmds), 1, "Created command that shouldn't have been created") + assert.Contains(t, actual, testCase.ExpectedErr) } }) } @@ -161,11 +159,11 @@ func TestShowCommand(t *testing.T) { assert.Equal(t, len(commands), 1) output := th.CheckCommand(t, "command", "show", command.Id) - assert.Contains(t, string(output), command.Id) - assert.Contains(t, string(output), command.TeamId) - assert.Contains(t, string(output), trigger) - assert.Contains(t, string(output), displayName) - assert.Contains(t, string(output), user.Username) + assert.Contains(t, output, command.Id) + assert.Contains(t, output, command.TeamId) + assert.Contains(t, output, trigger) + assert.Contains(t, output, displayName) + assert.Contains(t, output, user.Username) }) t.Run("not existing command", func(t *testing.T) { @@ -255,14 +253,14 @@ func TestModifyCommand(t *testing.T) { t.Run("command not specified", func(t *testing.T) { args := []string{"command", "", command.Id, "--trigger-word", "sometrigger"} output, _ := th.RunCommandWithOutput(t, args...) - assert.Contains(t, string(output), "Error: unknown flag: --trigger-word") + assert.Contains(t, output, "Error: unknown flag: --trigger-word") }) t.Run("modify command unchanged", func(t *testing.T) { args := []string{"command", "modify", command.Id} output, _ := th.RunCommandWithOutput(t, args...) cmd, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd.DisplayName, command.DisplayName) assert.Equal(t, cmd.Method, command.Method) assert.Equal(t, cmd.TeamId, command.TeamId) @@ -279,7 +277,7 @@ func TestModifyCommand(t *testing.T) { t.Run("misspelled flag", func(t *testing.T) { args := []string{"command", "", command.Id, "--trigger-wor", "sometrigger"} output, _ := th.RunCommandWithOutput(t, args...) - assert.Contains(t, string(output), "Error: unknown flag:") + assert.Contains(t, output, "Error: unknown flag:") }) t.Run("multiple flags nil error", func(t *testing.T) { @@ -289,7 +287,7 @@ func TestModifyCommand(t *testing.T) { args := []string{"command", "modify", command.Id, "--trigger-word", testName, "--url", testURL, "--description", testDescription} output, _ := th.RunCommandWithOutput(t, args...) cmd, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd.Trigger, testName) assert.Equal(t, cmd.URL, testURL) assert.Equal(t, cmd.Description, testDescription) @@ -300,7 +298,7 @@ func TestModifyCommand(t *testing.T) { args := []string{"command", "modify", command.Id, "--title", testVal} output, _ := th.RunCommandWithOutput(t, args...) cmd, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd.DisplayName, testVal) }) @@ -309,7 +307,7 @@ func TestModifyCommand(t *testing.T) { args := []string{"command", "modify", command.Id, "--description", testVal} output, _ := th.RunCommandWithOutput(t, args...) cmd, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd.Description, testVal) }) @@ -318,7 +316,7 @@ func TestModifyCommand(t *testing.T) { args := []string{"command", "modify", command.Id, "--trigger-word", testVal} output, _ := th.RunCommandWithOutput(t, args...) cmd, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd.Trigger, testVal) }) @@ -326,14 +324,14 @@ func TestModifyCommand(t *testing.T) { testVal := "bad trigger" args := []string{"command", "modify", command.Id, "--trigger-word", testVal} output, _ := th.RunCommandWithOutput(t, args...) - assert.Contains(t, string(output), "Error: a trigger word must not contain spaces") + assert.Contains(t, output, "Error: a trigger word must not contain spaces") }) t.Run("trigger with leading /", func(t *testing.T) { testVal := "/bad-trigger" args := []string{"command", "modify", command.Id, "--trigger-word", testVal} output, _ := th.RunCommandWithOutput(t, args...) - assert.Contains(t, string(output), "Error: a trigger word cannot begin with a /") + assert.Contains(t, output, "Error: a trigger word cannot begin with a /") }) t.Run("blank trigger", func(t *testing.T) { @@ -343,7 +341,7 @@ func TestModifyCommand(t *testing.T) { cmd_modified, _ := th.App.GetCommand(command.Id) // assert trigger remains unchanged - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd_unmodified.Trigger, cmd_modified.Trigger) }) @@ -353,7 +351,7 @@ func TestModifyCommand(t *testing.T) { args := []string{"command", "modify", command.Id, "--url", testVal} output, _ := th.RunCommandWithOutput(t, args...) cmd, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd.URL, testVal) }) @@ -364,7 +362,7 @@ func TestModifyCommand(t *testing.T) { cmd_modified, _ := th.App.GetCommand(command.Id) //assert URL remains unchanged - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd_unmodified.URL, cmd_modified.URL) }) @@ -373,7 +371,7 @@ func TestModifyCommand(t *testing.T) { args := []string{"command", "modify", command.Id, "--icon", testVal} output, _ := th.RunCommandWithOutput(t, args...) cmd, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd.IconURL, testVal) }) @@ -382,7 +380,7 @@ func TestModifyCommand(t *testing.T) { args := []string{"command", "modify", command.Id, "--creator", testVal.Username} output, _ := th.RunCommandWithOutput(t, args...) cmd, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd.CreatorId, testVal.Id) }) @@ -390,14 +388,14 @@ func TestModifyCommand(t *testing.T) { testVal := "fakeuser" args := []string{"command", "modify", command.Id, "--creator", testVal} output, _ := th.RunCommandWithOutput(t, args...) - assert.Contains(t, string(output), "unable to find user") + assert.Contains(t, output, "unable to find user") }) t.Run("creator not admin user", func(t *testing.T) { testVal := user.Username args := []string{"command", "modify", command.Id, "--creator", testVal} output, _ := th.RunCommandWithOutput(t, args...) - assert.Contains(t, string(output), "the creator must be a user who has permissions to manage slash commands") + assert.Contains(t, output, "the creator must be a user who has permissions to manage slash commands") }) t.Run("response username nil error", func(t *testing.T) { @@ -405,7 +403,7 @@ func TestModifyCommand(t *testing.T) { args := []string{"command", "modify", command.Id, "--response-username", testVal} output, _ := th.RunCommandWithOutput(t, args...) cmd, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output), "PASS") + assert.Contains(t, output, "PASS") assert.Equal(t, cmd.Username, testVal) }) @@ -416,13 +414,13 @@ func TestModifyCommand(t *testing.T) { // set post and check output_set, _ := th.RunCommandWithOutput(t, args_set...) cmd_set, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output_set), "PASS") + assert.Contains(t, output_set, "PASS") assert.Equal(t, cmd_set.Method, "P") // unset post and check output_unset, _ := th.RunCommandWithOutput(t, args_unset...) cmd_unset, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output_unset), "PASS") + assert.Contains(t, output_unset, "PASS") assert.Equal(t, cmd_unset.Method, "G") }) @@ -433,13 +431,13 @@ func TestModifyCommand(t *testing.T) { // set autocomplete and check output_set, _ := th.RunCommandWithOutput(t, args_set...) cmd_set, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output_set), "PASS") + assert.Contains(t, output_set, "PASS") assert.Equal(t, cmd_set.AutoComplete, true) // unset autocomplete and check output_unset, _ := th.RunCommandWithOutput(t, args_unset...) cmd_unset, _ := th.App.GetCommand(command.Id) - assert.Contains(t, string(output_unset), "PASS") + assert.Contains(t, output_unset, "PASS") assert.Equal(t, cmd_unset.AutoComplete, false) }) } diff --git a/cmd/mattermost/commands/config_test.go b/cmd/mattermost/commands/config_test.go index 1bd6c63c0a..3268066084 100644 --- a/cmd/mattermost/commands/config_test.go +++ b/cmd/mattermost/commands/config_test.go @@ -111,10 +111,10 @@ func TestConfigGet(t *testing.T) { t.Run("check output", func(t *testing.T) { output := th.CheckCommand(t, "config", "get", "MessageExportSettings") - assert.Contains(t, string(output), "EnableExport") - assert.Contains(t, string(output), "ExportFormat") - assert.Contains(t, string(output), "DailyRunTime") - assert.Contains(t, string(output), "ExportFromTimestamp") + assert.Contains(t, output, "EnableExport") + assert.Contains(t, output, "ExportFormat") + assert.Contains(t, output, "DailyRunTime") + assert.Contains(t, output, "ExportFromTimestamp") }) } @@ -138,33 +138,33 @@ func TestConfigSet(t *testing.T) { t.Run("Error when the wrong value is set", func(t *testing.T) { assert.Error(t, th.RunCommand(t, "config", "set", "EmailSettings.ConnectionSecurity", "invalid-key")) output := th.CheckCommand(t, "config", "get", "EmailSettings.ConnectionSecurity") - assert.NotContains(t, string(output), "invalid-key") + assert.NotContains(t, output, "invalid-key") }) t.Run("Error when the parameter of an unknown plugin is set", func(t *testing.T) { output, err := th.RunCommandWithOutput(t, "config", "set", "PluginSettings.Plugins.someplugin", "true") assert.Error(t, err) - assert.NotContains(t, string(output), "panic") + assert.NotContains(t, output, "panic") }) t.Run("Error when the wrong locale is set", func(t *testing.T) { th.CheckCommand(t, "config", "set", "LocalizationSettings.DefaultServerLocale", "es") assert.Error(t, th.RunCommand(t, "config", "set", "LocalizationSettings.DefaultServerLocale", "invalid-key")) output := th.CheckCommand(t, "config", "get", "LocalizationSettings.DefaultServerLocale") - assert.NotContains(t, string(output), "invalid-key") - assert.NotContains(t, string(output), "\"en\"") + assert.NotContains(t, output, "invalid-key") + assert.NotContains(t, output, "\"en\"") }) t.Run("Success when a valid value is set", func(t *testing.T) { assert.NoError(t, th.RunCommand(t, "config", "set", "EmailSettings.ConnectionSecurity", "TLS")) output := th.CheckCommand(t, "config", "get", "EmailSettings.ConnectionSecurity") - assert.Contains(t, string(output), "TLS") + assert.Contains(t, output, "TLS") }) t.Run("Success when a valid locale is set", func(t *testing.T) { assert.NoError(t, th.RunCommand(t, "config", "set", "LocalizationSettings.DefaultServerLocale", "es")) output := th.CheckCommand(t, "config", "get", "LocalizationSettings.DefaultServerLocale") - assert.Contains(t, string(output), "\"es\"") + assert.Contains(t, output, "\"es\"") }) } @@ -339,9 +339,9 @@ func TestConfigShow(t *testing.T) { t.Run("successfully dumping config", func(t *testing.T) { output := th.CheckCommand(t, "config", "show") - assert.Contains(t, string(output), "SqlSettings") - assert.Contains(t, string(output), "MessageExportSettings") - assert.Contains(t, string(output), "AnnouncementSettings") + assert.Contains(t, output, "SqlSettings") + assert.Contains(t, output, "MessageExportSettings") + assert.Contains(t, output, "AnnouncementSettings") }) t.Run("successfully dumping config as json", func(t *testing.T) { diff --git a/cmd/mattermost/commands/group_test.go b/cmd/mattermost/commands/group_test.go index 7569de2a13..2f2bee18de 100644 --- a/cmd/mattermost/commands/group_test.go +++ b/cmd/mattermost/commands/group_test.go @@ -114,7 +114,7 @@ func TestChannelGroupStatus(t *testing.T) { // get status, should be Disabled output := th.CheckCommand(t, "group", "channel", "status", th.BasicTeam.Name+":"+channel.Name) - require.Contains(t, string(output), "Disabled") + require.Contains(t, output, "Disabled") // add group and enable id := model.NewId() @@ -143,7 +143,7 @@ func TestChannelGroupStatus(t *testing.T) { // get status, should be enabled output = th.CheckCommand(t, "group", "channel", "status", th.BasicTeam.Name+":"+channel.Name) - require.Contains(t, string(output), "Enabled") + require.Contains(t, output, "Enabled") // try to get status of nonexistent channel, should fail require.Error(t, th.RunCommand(t, "group", "channel", "status", th.BasicTeam.Name+":"+channel.Name+"asdf")) @@ -204,8 +204,8 @@ func TestChannelGroupList(t *testing.T) { // list groups output := th.CheckCommand(t, "group", "channel", "list", th.BasicTeam.Name+":"+channel.Name) - require.Contains(t, string(output), g1.DisplayName) - require.Contains(t, string(output), g2.DisplayName) + require.Contains(t, output, g1.DisplayName) + require.Contains(t, output, g2.DisplayName) // try to get list of nonexistent channel, should fail require.Error(t, th.RunCommand(t, "group", "channel", "list", th.BasicTeam.Name+":"+channel.Name+"asdf")) @@ -301,7 +301,7 @@ func TestTeamGroupStatus(t *testing.T) { // get status, should be Disabled output := th.CheckCommand(t, "group", "team", "status", th.BasicTeam.Name) - require.Contains(t, string(output), "Disabled") + require.Contains(t, output, "Disabled") // add group and enable id := model.NewId() @@ -330,7 +330,7 @@ func TestTeamGroupStatus(t *testing.T) { // get status, should be enabled output = th.CheckCommand(t, "group", "team", "status", th.BasicTeam.Name) - require.Contains(t, string(output), "Enabled") + require.Contains(t, output, "Enabled") // try to get status of nonexistent channel, should fail require.Error(t, th.RunCommand(t, "group", "team", "status", th.BasicTeam.Name+"asdf")) @@ -388,8 +388,8 @@ func TestTeamGroupList(t *testing.T) { // list groups output := th.CheckCommand(t, "group", "team", "list", th.BasicTeam.Name) - require.Contains(t, string(output), g1.DisplayName) - require.Contains(t, string(output), g2.DisplayName) + require.Contains(t, output, g1.DisplayName) + require.Contains(t, output, g2.DisplayName) // try to get list of nonexistent team, should fail require.Error(t, th.RunCommand(t, "group", "team", "list", th.BasicTeam.Name+"asdf")) diff --git a/cmd/mattermost/commands/server_test.go b/cmd/mattermost/commands/server_test.go index c34a3358b8..dfcf78caf1 100644 --- a/cmd/mattermost/commands/server_test.go +++ b/cmd/mattermost/commands/server_test.go @@ -16,7 +16,6 @@ import ( ) type ServerTestHelper struct { - configStore config.Store disableConfigWatch bool interruptChan chan os.Signal originalInterval int diff --git a/cmd/mattermost/commands/user_test.go b/cmd/mattermost/commands/user_test.go index 1c5e3d8acc..e7a5de50af 100644 --- a/cmd/mattermost/commands/user_test.go +++ b/cmd/mattermost/commands/user_test.go @@ -89,7 +89,6 @@ func TestChangeUserEmail(t *testing.T) { require.Equal(t, user.Email, newEmail, "should've updated to the new email") - // should fail because using an invalid email require.Error(t, th.RunCommand(t, "user", "email", th.BasicUser.Username, "wrong$email.com")) diff --git a/config/client.go b/config/client.go index 3c1eca98cb..a0f47f86a4 100644 --- a/config/client.go +++ b/config/client.go @@ -44,6 +44,7 @@ func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.L props["ExperimentalGroupUnreadChannels"] = *c.ServiceSettings.ExperimentalGroupUnreadChannels props["EnableSVGs"] = strconv.FormatBool(*c.ServiceSettings.EnableSVGs) props["EnableMarketplace"] = strconv.FormatBool(*c.PluginSettings.EnableMarketplace) + props["EnableLatex"] = strconv.FormatBool(*c.ServiceSettings.EnableLatex) // This setting is only temporary, so keep using the old setting name for the mobile and web apps props["ExperimentalEnablePostMetadata"] = "true" diff --git a/config/database_test.go b/config/database_test.go index ead6be3571..0ce33316aa 100644 --- a/config/database_test.go +++ b/config/database_test.go @@ -764,7 +764,7 @@ func TestDatabaseStoreLoad(t *testing.T) { func TestDatabaseGetFile(t *testing.T) { _, tearDown := setupConfigDatabase(t, minimalConfig, map[string][]byte{ - "empty-file": []byte{}, + "empty-file": {}, "test-file": []byte("test"), }) defer tearDown() diff --git a/config/memory_test.go b/config/memory_test.go index ede50fe549..bc1df36f97 100644 --- a/config/memory_test.go +++ b/config/memory_test.go @@ -282,7 +282,7 @@ func TestMemoryGetFile(t *testing.T) { ms, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{ InitialConfig: minimalConfig, InitialFiles: map[string][]byte{ - "empty-file": []byte{}, + "empty-file": {}, "test-file": []byte("test"), }, }) diff --git a/config/utils_test.go b/config/utils_test.go index 8bfc2eb33b..131c843e7c 100644 --- a/config/utils_test.go +++ b/config/utils_test.go @@ -204,7 +204,3 @@ func sToP(s string) *string { func bToP(b bool) *bool { return &b } - -func iToP(i int) *int { - return &i -} diff --git a/i18n/en.json b/i18n/en.json index 936b4cb2f3..eb29f5cc57 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1320,10 +1320,6 @@ "id": "api.file.upload_file.storage.app_error", "translation": "Unable to upload file. Image storage is not configured." }, - { - "id": "api.file.upload_file.too_large.app_error", - "translation": "Unable to upload file. File is too large." - }, { "id": "api.file.upload_file.too_large_detailed.app_error", "translation": "Unable to upload file {{.Filename}}. {{.Length}} bytes exceeds the maximum allowed {{.Limit}} bytes." @@ -6134,6 +6130,10 @@ "id": "store.sql_group.no_rows_changed", "translation": "no rows changed" }, + { + "id": "store.sql_group.permanent_delete_members_by_user.app_error", + "translation": "Unable to remove the group members with UserID \"{{.UserId}}\"" + }, { "id": "store.sql_group.unique_constraint", "translation": "a group with that name already exists" diff --git a/mlog/human/parser.go b/mlog/human/parser.go index f279ecf672..1aa738aeaa 100644 --- a/mlog/human/parser.go +++ b/mlog/human/parser.go @@ -41,7 +41,7 @@ func parseLogMessage(msg string) (result LogEntry, err error) { } else { d, ok := token.(json.Delim) if !ok || d != '{' { - return result, errors.New(fmt.Sprintf("input is not a JSON object, found: %v", token)) + return result, fmt.Errorf("input is not a JSON object, found: %v", token) } } @@ -110,7 +110,7 @@ func parseLogMessage(msg string) (result LogEntry, err error) { } else { d, ok := token.(json.Delim) if !ok || d != '}' { - return result, errors.New(fmt.Sprintf("failed to read '}', read: %v", token)) + return result, fmt.Errorf("failed to read '}', read: %v", token) } } diff --git a/mlog/stdlog.go b/mlog/stdlog.go index 7839ddfa05..1abf30b2ca 100644 --- a/mlog/stdlog.go +++ b/mlog/stdlog.go @@ -81,7 +81,7 @@ type loggerWriter struct { func (l *loggerWriter) Write(p []byte) (int, error) { trimmed := string(bytes.TrimSpace(p)) for _, line := range strings.Split(trimmed, "\n") { - l.logFunc(string(line)) + l.logFunc(line) } return len(p), nil } diff --git a/model/client4_test.go b/model/client4_test.go index b165e6b610..87f2c285da 100644 --- a/model/client4_test.go +++ b/model/client4_test.go @@ -17,9 +17,9 @@ func TestClient4CreatePost(t *testing.T) { post := &Post{ Props: map[string]interface{}{ "attachments": []*SlackAttachment{ - &SlackAttachment{ + { Actions: []*PostAction{ - &PostAction{ + { Integration: &PostActionIntegration{ Context: map[string]interface{}{ "foo": "bar", @@ -37,9 +37,9 @@ func TestClient4CreatePost(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { attachments := PostFromJson(r.Body).Attachments() assert.Equal(t, []*SlackAttachment{ - &SlackAttachment{ + { Actions: []*PostAction{ - &PostAction{ + { Integration: &PostActionIntegration{ Context: map[string]interface{}{ "foo": "bar", diff --git a/model/command_response_test.go b/model/command_response_test.go index 071cd4129a..76d46da0c3 100644 --- a/model/command_response_test.go +++ b/model/command_response_test.go @@ -146,7 +146,7 @@ func TestCommandResponseFromJson(t *testing.T) { &CommandResponse{ Text: "message 1", ExtraResponses: []*CommandResponse{ - &CommandResponse{ + { Text: "message 2", }, }, @@ -180,7 +180,7 @@ func TestCommandResponseFromJson(t *testing.T) { }, }, ExtraResponses: []*CommandResponse{ - &CommandResponse{ + { Text: "message 2", Attachments: []*SlackAttachment{ { diff --git a/model/config.go b/model/config.go index 9fd2d291e3..8d61e1ec8f 100644 --- a/model/config.go +++ b/model/config.go @@ -325,6 +325,7 @@ type ServiceSettings struct { DisableBotsWhenOwnerIsDeactivated *bool `restricted:"true"` EnableBotAccountCreation *bool EnableSVGs *bool + EnableLatex *bool } func (s *ServiceSettings) SetDefaults(isUpdate bool) { @@ -689,6 +690,14 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) { s.EnableSVGs = NewBool(false) } } + + if s.EnableLatex == nil { + if isUpdate { + s.EnableLatex = NewBool(true) + } else { + s.EnableLatex = NewBool(false) + } + } } type ClusterSettings struct { @@ -2871,7 +2880,7 @@ func (ss *ServiceSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.webserver_security.app_error", nil, "", http.StatusBadRequest) } - if *ss.ConnectionSecurity == CONN_SECURITY_TLS && *ss.UseLetsEncrypt == false { + if *ss.ConnectionSecurity == CONN_SECURITY_TLS && !*ss.UseLetsEncrypt { appErr := NewAppError("Config.IsValid", "model.config.is_valid.tls_cert_file.app_error", nil, "", http.StatusBadRequest) if *ss.TLSCertFile == "" { diff --git a/model/config_test.go b/model/config_test.go index dde02b517b..bc88b18937 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -186,7 +186,6 @@ func TestConfigIsValidFakeAlgorithm(t *testing.T) { require.Equal(t, "model.config.is_valid.saml_digest_algorithm.app_error", err.Message) *c1.SamlSettings.DigestAlgorithm = temp - temp = *c1.SamlSettings.SignatureAlgorithm *c1.SamlSettings.SignatureAlgorithm = "Fake Algorithm" err = c1.SamlSettings.isValid() if err == nil { diff --git a/model/file_info_test.go b/model/file_info_test.go index f87dd46404..bf2e449ac2 100644 --- a/model/file_info_test.go +++ b/model/file_info_test.go @@ -5,6 +5,8 @@ package model import ( "encoding/base64" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" _ "image/gif" _ "image/png" "io/ioutil" @@ -22,179 +24,109 @@ func TestFileInfoIsValid(t *testing.T) { Path: "fake/path.png", } - if err := info.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, info.IsValid()) info.Id = "" - if err := info.IsValid(); err == nil { - t.Fatal("empty Id isn't valid") - } + require.NotNil(t, info.IsValid(), "empty Id isn't valid") info.Id = NewId() info.CreateAt = 0 - if err := info.IsValid(); err == nil { - t.Fatal("empty CreateAt isn't valid") - } + require.NotNil(t, info.IsValid(), "empty CreateAt isn't valid") info.CreateAt = 1234 info.UpdateAt = 0 - if err := info.IsValid(); err == nil { - t.Fatal("empty UpdateAt isn't valid") - } + require.NotNil(t, info.IsValid(), "empty UpdateAt isn't valid") info.UpdateAt = 1234 info.PostId = NewId() - if err := info.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, info.IsValid()) info.Path = "" - if err := info.IsValid(); err == nil { - t.Fatal("empty Path isn't valid") - } + require.NotNil(t, info.IsValid(), "empty Path isn't valid") info.Path = "fake/path.png" - if err := info.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, info.IsValid()) } func TestFileInfoIsImage(t *testing.T) { - info := &FileInfo{ - MimeType: "image/png", - } - - if !info.IsImage() { - t.Fatal("file is an image") - } + info := &FileInfo{MimeType: "image/png"} + assert.True(t, info.IsImage(), "file is an image") info.MimeType = "text/plain" - if info.IsImage() { - t.Fatal("file is not an image") - } + assert.False(t, info.IsImage(), "file is not an image") } func TestGetInfoForFile(t *testing.T) { fakeFile := make([]byte, 1000) - if info, err := GetInfoForBytes("file.txt", fakeFile); err != nil { - t.Fatal(err) - } else if info.Name != "file.txt" { - t.Fatalf("Got incorrect filename: %v", info.Name) - } else if info.Extension != "txt" { - t.Fatalf("Got incorrect extension: %v", info.Extension) - } else if info.Size != 1000 { - t.Fatalf("Got incorrect size: %v", info.Size) - } else if !strings.HasPrefix(info.MimeType, "text/plain") { - t.Fatalf("Got incorrect mime type: %v", info.MimeType) - } else if info.Width != 0 { - t.Fatalf("Got incorrect width: %v", info.Width) - } else if info.Height != 0 { - t.Fatalf("Got incorrect height: %v", info.Height) - } else if info.HasPreviewImage { - t.Fatalf("Got incorrect has preview image: %v", info.HasPreviewImage) - } + info, errApp := GetInfoForBytes("file.txt", fakeFile) + require.Nil(t, errApp) + assert.Equalf(t, info.Name, "file.txt", "Got incorrect filename: %v", info.Name) + assert.Equalf(t, info.Extension, "txt", "Got incorrect extension: %v", info.Extension) + assert.EqualValuesf(t, info.Size, 1000, "Got incorrect size: %v", info.Size) + assert.Truef(t, strings.HasPrefix(info.MimeType, "text/plain"), "Got incorrect mime type: %v", info.MimeType) + assert.Equalf(t, info.Width, 0, "Got incorrect width: %v", info.Width) + assert.Equalf(t, info.Height, 0, "Got incorrect height: %v", info.Height) + assert.Falsef(t, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) pngFile, err := ioutil.ReadFile("../tests/test.png") - if err != nil { - t.Fatalf("Failed to load test.png: %v", err.Error()) - } - if info, err := GetInfoForBytes("test.png", pngFile); err != nil { - t.Fatal(err) - } else if info.Name != "test.png" { - t.Fatalf("Got incorrect filename: %v", info.Name) - } else if info.Extension != "png" { - t.Fatalf("Got incorrect extension: %v", info.Extension) - } else if info.Size != 279591 { - t.Fatalf("Got incorrect size: %v", info.Size) - } else if info.MimeType != "image/png" { - t.Fatalf("Got incorrect mime type: %v", info.MimeType) - } else if info.Width != 408 { - t.Fatalf("Got incorrect width: %v", info.Width) - } else if info.Height != 336 { - t.Fatalf("Got incorrect height: %v", info.Height) - } else if !info.HasPreviewImage { - t.Fatalf("Got incorrect has preview image: %v", info.HasPreviewImage) - } + require.Nilf(t, err, "Failed to load test.png") + + info, err = GetInfoForBytes("test.png", pngFile) + require.Nil(t, err) + assert.Equalf(t, info.Name, "test.png", "Got incorrect filename: %v", info.Name) + assert.Equalf(t, info.Extension, "png", "Got incorrect extension: %v", info.Extension) + assert.EqualValues(t, info.Size, 279591, "Got incorrect size: %v", info.Size) + assert.Equalf(t, info.MimeType, "image/png", "Got incorrect mime type: %v", info.MimeType) + assert.Equalf(t, info.Width, 408, "Got incorrect width: %v", info.Width) + assert.Equalf(t, info.Height, 336, "Got incorrect height: %v", info.Height) + assert.Truef(t, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) // base 64 encoded version of handtinywhite.gif from http://probablyprogramming.com/2009/03/15/the-tiniest-gif-ever gifFile, _ := base64.StdEncoding.DecodeString("R0lGODlhAQABAIABAP///wAAACwAAAAAAQABAAACAkQBADs=") - if info, err := GetInfoForBytes("handtinywhite.gif", gifFile); err != nil { - t.Fatal(err) - } else if info.Name != "handtinywhite.gif" { - t.Fatalf("Got incorrect filename: %v", info.Name) - } else if info.Extension != "gif" { - t.Fatalf("Got incorrect extension: %v", info.Extension) - } else if info.Size != 35 { - t.Fatalf("Got incorrect size: %v", info.Size) - } else if info.MimeType != "image/gif" { - t.Fatalf("Got incorrect mime type: %v", info.MimeType) - } else if info.Width != 1 { - t.Fatalf("Got incorrect width: %v", info.Width) - } else if info.Height != 1 { - t.Fatalf("Got incorrect height: %v", info.Height) - } else if !info.HasPreviewImage { - t.Fatalf("Got incorrect has preview image: %v", info.HasPreviewImage) - } + info, err = GetInfoForBytes("handtinywhite.gif", gifFile) + require.Nil(t, err) + assert.Equalf(t, info.Name, "handtinywhite.gif", "Got incorrect filename: %v", info.Name) + assert.Equalf(t, info.Extension, "gif", "Got incorrect extension: %v", info.Extension) + assert.EqualValuesf(t, info.Size, 35, "Got incorrect size: %v", info.Size) + assert.Equalf(t, info.MimeType, "image/gif", "Got incorrect mime type: %v", info.MimeType) + assert.Equalf(t, info.Width, 1, "Got incorrect width: %v", info.Width) + assert.Equalf(t, info.Height, 1, "Got incorrect height: %v", info.Height) + assert.Truef(t, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) animatedGifFile, err := ioutil.ReadFile("../tests/testgif.gif") - if err != nil { - t.Fatalf("Failed to load testgif.gif: %v", err.Error()) - } - if info, err := GetInfoForBytes("testgif.gif", animatedGifFile); err != nil { - t.Fatal(err) - } else if info.Name != "testgif.gif" { - t.Fatalf("Got incorrect filename: %v", info.Name) - } else if info.Extension != "gif" { - t.Fatalf("Got incorrect extension: %v", info.Extension) - } else if info.Size != 38689 { - t.Fatalf("Got incorrect size: %v", info.Size) - } else if info.MimeType != "image/gif" { - t.Fatalf("Got incorrect mime type: %v", info.MimeType) - } else if info.Width != 118 { - t.Fatalf("Got incorrect width: %v", info.Width) - } else if info.Height != 118 { - t.Fatalf("Got incorrect height: %v", info.Height) - } else if info.HasPreviewImage { - t.Fatalf("Got incorrect has preview image: %v", info.HasPreviewImage) - } + require.Nilf(t, err, "Failed to load testgif.gif") - if info, err := GetInfoForBytes("filewithoutextension", fakeFile); err != nil { - t.Fatal(err) - } else if info.Name != "filewithoutextension" { - t.Fatalf("Got incorrect filename: %v", info.Name) - } else if info.Extension != "" { - t.Fatalf("Got incorrect extension: %v", info.Extension) - } else if info.Size != 1000 { - t.Fatalf("Got incorrect size: %v", info.Size) - } else if info.MimeType != "" { - t.Fatalf("Got incorrect mime type: %v", info.MimeType) - } else if info.Width != 0 { - t.Fatalf("Got incorrect width: %v", info.Width) - } else if info.Height != 0 { - t.Fatalf("Got incorrect height: %v", info.Height) - } else if info.HasPreviewImage { - t.Fatalf("Got incorrect has preview image: %v", info.HasPreviewImage) - } + info, err = GetInfoForBytes("testgif.gif", animatedGifFile) + require.Nil(t, err) + assert.Equalf(t, info.Name, "testgif.gif", "Got incorrect filename: %v", info.Name) + assert.Equalf(t, info.Extension, "gif", "Got incorrect extension: %v", info.Extension) + assert.EqualValuesf(t, info.Size, 38689, "Got incorrect size: %v", info.Size) + assert.Equalf(t, info.MimeType, "image/gif", "Got incorrect mime type: %v", info.MimeType) + assert.Equalf(t, info.Width, 118, "Got incorrect width: %v", info.Width) + assert.Equalf(t, info.Height, 118, "Got incorrect height: %v", info.Height) + assert.Falsef(t, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) + + info, err = GetInfoForBytes("filewithoutextension", fakeFile) + require.Nil(t, err) + assert.Equalf(t, info.Name, "filewithoutextension", "Got incorrect filename: %v", info.Name) + assert.Equalf(t, info.Extension, "", "Got incorrect extension: %v", info.Extension) + assert.EqualValuesf(t, info.Size, 1000, "Got incorrect size: %v", info.Size) + assert.Equalf(t, info.MimeType, "", "Got incorrect mime type: %v", info.MimeType) + assert.Equalf(t, info.Width, 0, "Got incorrect width: %v", info.Width) + assert.Equalf(t, info.Height, 0, "Got incorrect height: %v", info.Height) + assert.Falsef(t, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) // Always make the extension lower case to make it easier to use in other places - if info, err := GetInfoForBytes("file.TXT", fakeFile); err != nil { - t.Fatal(err) - } else if info.Name != "file.TXT" { - t.Fatalf("Got incorrect filename: %v", info.Name) - } else if info.Extension != "txt" { - t.Fatalf("Got incorrect extension: %v", info.Extension) - } + info, err = GetInfoForBytes("file.TXT", fakeFile) + require.Nil(t, err) + assert.Equalf(t, info.Name, "file.TXT", "Got incorrect filename: %v", info.Name) + assert.Equalf(t, info.Extension, "txt", "Got incorrect extension: %v", info.Extension) // Don't error out for image formats we don't support - if info, err := GetInfoForBytes("file.tif", fakeFile); err != nil { - t.Fatal(err) - } else if info.Name != "file.tif" { - t.Fatalf("Got incorrect filename: %v", info.Name) - } else if info.Extension != "tif" { - t.Fatalf("Got incorrect extension: %v", info.Extension) - } else if info.MimeType != "image/tiff" && info.MimeType != "image/x-tiff" { - t.Fatalf("Got incorrect mime type: %v", info.MimeType) - } + info, err = GetInfoForBytes("file.tif", fakeFile) + require.Nil(t, err) + assert.Equalf(t, info.Name, "file.tif", "Got incorrect filename: %v", info.Name) + assert.Equalf(t, info.Extension, "tif", "Got incorrect extension: %v", info.Extension) + assert.True(t, info.MimeType == "image/x-tiff" || info.MimeType == "image/tiff", "Got incorrect mime type: %v", info.MimeType) } diff --git a/model/integration_action.go b/model/integration_action.go index f57611f6d6..cef7256abb 100644 --- a/model/integration_action.go +++ b/model/integration_action.go @@ -284,7 +284,7 @@ func DecodeAndVerifyTriggerId(triggerId string, s *ecdsa.PrivateKey) (string, st R, S *big.Int } - if _, err := asn1.Unmarshal([]byte(signature), &esig); err != nil { + if _, err := asn1.Unmarshal(signature, &esig); err != nil { return "", "", NewAppError("DecodeAndVerifyTriggerId", "interactive_message.decode_trigger_id.signature_decode_failed", nil, err.Error(), http.StatusBadRequest) } diff --git a/model/link_metadata_test.go b/model/link_metadata_test.go index a3f63c506e..61c8acb09a 100644 --- a/model/link_metadata_test.go +++ b/model/link_metadata_test.go @@ -305,8 +305,8 @@ func TestTruncateOpenGraph(t *testing.T) { sampleImage("metoo.gif"), sampleImage("fifth.ico"), sampleImage("notme.tiff")}, - Audios: []*opengraph.Audio{&opengraph.Audio{}}, - Videos: []*opengraph.Video{&opengraph.Video{}}, + Audios: []*opengraph.Audio{{}}, + Videos: []*opengraph.Video{{}}, Article: &opengraph.Article{}, Book: &opengraph.Book{}, Profile: &opengraph.Profile{}, diff --git a/model/manifest.go b/model/manifest.go index 92565e2d1f..c0eced6920 100644 --- a/model/manifest.go +++ b/model/manifest.go @@ -130,6 +130,12 @@ type Manifest struct { // A description of what your plugin is and does. Description string `json:"description,omitempty" yaml:"description,omitempty"` + // HomepageURL is an optional link to learn more about the plugin. + HomepageURL string `json:"homepage_url,omitempty" yaml:"homepage_url,omitempty"` + + // SupportURL is an optional URL where plugin issues can be reported. + SupportURL string `json:"support_url,omitempty" yaml:"support_url,omitempty"` + // A relative file path in the bundle that points to the plugins svg icon for use with the Plugin Marketplace. // This should be relative to the root of your bundle and the location of the manifest file. Bitmap image formats are not supported. IconPath string `json:"icon_path,omitempty" yaml:"icon_path,omitempty"` diff --git a/model/manifest_test.go b/model/manifest_test.go index 53e2370af0..f93b642acc 100644 --- a/model/manifest_test.go +++ b/model/manifest_test.go @@ -64,6 +64,8 @@ func TestFindManifest(t *testing.T) { func TestManifestUnmarshal(t *testing.T) { expected := Manifest{ Id: "theid", + HomepageURL: "https://example.com", + SupportURL: "https://example.com/support", IconPath: "assets/icon.svg", MinServerVersion: "5.6.0", Server: &ManifestServer{ @@ -81,7 +83,7 @@ func TestManifestUnmarshal(t *testing.T) { Header: "theheadertext", Footer: "thefootertext", Settings: []*PluginSetting{ - &PluginSetting{ + { Key: "thesetting", DisplayName: "thedisplayname", Type: "dropdown", @@ -89,7 +91,7 @@ func TestManifestUnmarshal(t *testing.T) { RegenerateHelpText: "theregeneratehelptext", Placeholder: "theplaceholder", Options: []*PluginOption{ - &PluginOption{ + { DisplayName: "theoptiondisplayname", Value: "thevalue", }, @@ -103,6 +105,8 @@ func TestManifestUnmarshal(t *testing.T) { var yamlResult Manifest require.NoError(t, yaml.Unmarshal([]byte(` id: theid +homepage_url: https://example.com +support_url: https://example.com/support icon_path: assets/icon.svg min_server_version: 5.6.0 server: @@ -133,6 +137,8 @@ settings_schema: var jsonResult Manifest require.NoError(t, json.Unmarshal([]byte(`{ "id": "theid", + "homepage_url": "https://example.com", + "support_url": "https://example.com/support", "icon_path": "assets/icon.svg", "min_server_version": "5.6.0", "server": { @@ -226,7 +232,7 @@ func TestManifestJson(t *testing.T) { Header: "theheadertext", Footer: "thefootertext", Settings: []*PluginSetting{ - &PluginSetting{ + { Key: "thesetting", DisplayName: "thedisplayname", Type: "dropdown", @@ -234,7 +240,7 @@ func TestManifestJson(t *testing.T) { RegenerateHelpText: "theregeneratehelptext", Placeholder: "theplaceholder", Options: []*PluginOption{ - &PluginOption{ + { DisplayName: "theoptiondisplayname", Value: "thevalue", }, @@ -293,7 +299,7 @@ func TestManifestClientManifest(t *testing.T) { Header: "theheadertext", Footer: "thefootertext", Settings: []*PluginSetting{ - &PluginSetting{ + { Key: "thesetting", DisplayName: "thedisplayname", Type: "dropdown", @@ -301,7 +307,7 @@ func TestManifestClientManifest(t *testing.T) { RegenerateHelpText: "theregeneratehelptext", Placeholder: "theplaceholder", Options: []*PluginOption{ - &PluginOption{ + { DisplayName: "theoptiondisplayname", Value: "thevalue", }, diff --git a/model/session_test.go b/model/session_test.go index fb89f89633..0171f9e0a9 100644 --- a/model/session_test.go +++ b/model/session_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSessionDeepCopy(t *testing.T) { @@ -17,7 +18,7 @@ func TestSessionDeepCopy(t *testing.T) { mapKey := "key" mapValue := "val" - session := &Session{Id: sessionId, Props: map[string]string{mapKey: mapValue}, TeamMembers: []*TeamMember{&TeamMember{UserId: userId, TeamId: "someteamId"}}} + session := &Session{Id: sessionId, Props: map[string]string{mapKey: mapValue}, TeamMembers: []*TeamMember{{UserId: userId, TeamId: "someteamId"}}} copySession := session.DeepCopy() copySession.Id = "changed" @@ -45,21 +46,16 @@ func TestSessionJson(t *testing.T) { json := session.ToJson() rsession := SessionFromJson(strings.NewReader(json)) - if rsession.Id != session.Id { - t.Fatal("Ids do not match") - } + require.Equal(t, rsession.Id, session.Id, "Ids do not match") session.Sanitize() - if session.IsExpired() { - t.Fatal("Shouldn't expire") - } + require.False(t, session.IsExpired(), "Shouldn't expire") session.ExpiresAt = GetMillis() time.Sleep(10 * time.Millisecond) - if !session.IsExpired() { - t.Fatal("Should expire") - } + + require.True(t, session.IsExpired(), "Should expire") session.SetExpireInDays(10) } diff --git a/model/slack_attachment.go b/model/slack_attachment.go index 0f1b96fb50..30757061e6 100644 --- a/model/slack_attachment.go +++ b/model/slack_attachment.go @@ -111,11 +111,7 @@ func (s *SlackAttachment) Equals(input *SlackAttachment) bool { } } - if s.Timestamp != input.Timestamp { - return false - } - - return true + return s.Timestamp == input.Timestamp } type SlackAttachmentField struct { diff --git a/model/user_test.go b/model/user_test.go index d3d64da78b..5fe39917c6 100644 --- a/model/user_test.go +++ b/model/user_test.go @@ -63,7 +63,7 @@ func TestUserPreSave(t *testing.T) { user.PreSave() user.Etag(true, true) assert.NotNil(t, user.Timezone, "Timezone is nil") - assert.Equal(t, user.Timezone["useAutomaticTimezone"], "true", "Timezone is not set to default"); + assert.Equal(t, user.Timezone["useAutomaticTimezone"], "true", "Timezone is not set to default") } func TestUserPreUpdate(t *testing.T) { diff --git a/plugin/api.go b/plugin/api.go index 1afadc2b89..49ac0ebf8f 100644 --- a/plugin/api.go +++ b/plugin/api.go @@ -378,6 +378,21 @@ type API interface { // Minimum server version: 5.2 UpdateChannelMemberNotifications(channelId, userId string, notifications map[string]string) (*model.ChannelMember, *model.AppError) + // GetGroup gets a group by ID. + // + // Minimum server version: 5.18 + GetGroup(groupId string) (*model.Group, *model.AppError) + + // GetGroupByName gets a group by name. + // + // Minimum server version: 5.18 + GetGroupByName(name string) (*model.Group, *model.AppError) + + // GetGroupsForUser gets the groups a user is in. + // + // Minimum server version: 5.18 + GetGroupsForUser(userId string) ([]*model.Group, *model.AppError) + // DeleteChannelMember deletes a channel membership for a user. // // Minimum server version: 5.2 diff --git a/plugin/client_rpc_generated.go b/plugin/client_rpc_generated.go index c50730fbef..d91ae36db6 100644 --- a/plugin/client_rpc_generated.go +++ b/plugin/client_rpc_generated.go @@ -2498,6 +2498,93 @@ func (s *apiRPCServer) UpdateChannelMemberNotifications(args *Z_UpdateChannelMem return nil } +type Z_GetGroupArgs struct { + A string +} + +type Z_GetGroupReturns struct { + A *model.Group + B *model.AppError +} + +func (g *apiRPCClient) GetGroup(groupId string) (*model.Group, *model.AppError) { + _args := &Z_GetGroupArgs{groupId} + _returns := &Z_GetGroupReturns{} + if err := g.client.Call("Plugin.GetGroup", _args, _returns); err != nil { + log.Printf("RPC call to GetGroup API failed: %s", err.Error()) + } + return _returns.A, _returns.B +} + +func (s *apiRPCServer) GetGroup(args *Z_GetGroupArgs, returns *Z_GetGroupReturns) error { + if hook, ok := s.impl.(interface { + GetGroup(groupId string) (*model.Group, *model.AppError) + }); ok { + returns.A, returns.B = hook.GetGroup(args.A) + } else { + return encodableError(fmt.Errorf("API GetGroup called but not implemented.")) + } + return nil +} + +type Z_GetGroupByNameArgs struct { + A string +} + +type Z_GetGroupByNameReturns struct { + A *model.Group + B *model.AppError +} + +func (g *apiRPCClient) GetGroupByName(name string) (*model.Group, *model.AppError) { + _args := &Z_GetGroupByNameArgs{name} + _returns := &Z_GetGroupByNameReturns{} + if err := g.client.Call("Plugin.GetGroupByName", _args, _returns); err != nil { + log.Printf("RPC call to GetGroupByName API failed: %s", err.Error()) + } + return _returns.A, _returns.B +} + +func (s *apiRPCServer) GetGroupByName(args *Z_GetGroupByNameArgs, returns *Z_GetGroupByNameReturns) error { + if hook, ok := s.impl.(interface { + GetGroupByName(name string) (*model.Group, *model.AppError) + }); ok { + returns.A, returns.B = hook.GetGroupByName(args.A) + } else { + return encodableError(fmt.Errorf("API GetGroupByName called but not implemented.")) + } + return nil +} + +type Z_GetGroupsForUserArgs struct { + A string +} + +type Z_GetGroupsForUserReturns struct { + A []*model.Group + B *model.AppError +} + +func (g *apiRPCClient) GetGroupsForUser(userId string) ([]*model.Group, *model.AppError) { + _args := &Z_GetGroupsForUserArgs{userId} + _returns := &Z_GetGroupsForUserReturns{} + if err := g.client.Call("Plugin.GetGroupsForUser", _args, _returns); err != nil { + log.Printf("RPC call to GetGroupsForUser API failed: %s", err.Error()) + } + return _returns.A, _returns.B +} + +func (s *apiRPCServer) GetGroupsForUser(args *Z_GetGroupsForUserArgs, returns *Z_GetGroupsForUserReturns) error { + if hook, ok := s.impl.(interface { + GetGroupsForUser(userId string) ([]*model.Group, *model.AppError) + }); ok { + returns.A, returns.B = hook.GetGroupsForUser(args.A) + } else { + return encodableError(fmt.Errorf("API GetGroupsForUser called but not implemented.")) + } + return nil +} + type Z_DeleteChannelMemberArgs struct { A string B string diff --git a/plugin/helpers_kv_test.go b/plugin/helpers_kv_test.go index f61033dacb..3c63137401 100644 --- a/plugin/helpers_kv_test.go +++ b/plugin/helpers_kv_test.go @@ -84,7 +84,7 @@ func TestKVSetJSON(t *testing.T) { p := &plugin.HelpersImpl{API: api} - err := p.KVSetJSON("test-key", func() { return }) + err := p.KVSetJSON("test-key", func() {}) api.AssertExpectations(t) assert.Error(t, err) }) @@ -124,7 +124,7 @@ func TestKVCompareAndSetJSON(t *testing.T) { api.AssertNotCalled(t, "KVCompareAndSet") p := &plugin.HelpersImpl{API: api} - ok, err := p.KVCompareAndSetJSON("test-key", func() { return }, map[string]interface{}{}) + ok, err := p.KVCompareAndSetJSON("test-key", func() {}, map[string]interface{}{}) api.AssertExpectations(t) assert.Equal(t, false, ok) @@ -137,7 +137,7 @@ func TestKVCompareAndSetJSON(t *testing.T) { p := &plugin.HelpersImpl{API: api} - ok, err := p.KVCompareAndSetJSON("test-key", map[string]interface{}{}, func() { return }) + ok, err := p.KVCompareAndSetJSON("test-key", map[string]interface{}{}, func() {}) api.AssertExpectations(t) assert.False(t, ok) @@ -211,7 +211,7 @@ func TestKVCompareAndDeleteJSON(t *testing.T) { api.AssertNotCalled(t, "KVCompareAndDelete") p := &plugin.HelpersImpl{API: api} - ok, err := p.KVCompareAndDeleteJSON("test-key", func() { return }) + ok, err := p.KVCompareAndDeleteJSON("test-key", func() {}) api.AssertExpectations(t) assert.Equal(t, false, ok) @@ -266,7 +266,7 @@ func TestKVSetWithExpiryJSON(t *testing.T) { p := &plugin.HelpersImpl{API: api} - err := p.KVSetWithExpiryJSON("test-key", func() { return }, 100) + err := p.KVSetWithExpiryJSON("test-key", func() {}, 100) api.AssertExpectations(t) assert.Error(t, err) diff --git a/plugin/plugintest/api.go b/plugin/plugintest/api.go index f308746374..4db7d5e5e2 100644 --- a/plugin/plugintest/api.go +++ b/plugin/plugintest/api.go @@ -996,6 +996,56 @@ func (_m *API) GetFileLink(fileId string) (string, *model.AppError) { return r0, r1 } +// GetGroup provides a mock function with given fields: groupId +func (_m *API) GetGroup(groupId string) (*model.Group, *model.AppError) { + ret := _m.Called(groupId) + + var r0 *model.Group + if rf, ok := ret.Get(0).(func(string) *model.Group); ok { + r0 = rf(groupId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Group) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(groupId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + +// GetGroupByName provides a mock function with given fields: name +func (_m *API) GetGroupByName(name string) (*model.Group, *model.AppError) { + ret := _m.Called(name) + + var r0 *model.Group + if rf, ok := ret.Get(0).(func(string) *model.Group); ok { + r0 = rf(name) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Group) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(name) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // GetGroupChannel provides a mock function with given fields: userIds func (_m *API) GetGroupChannel(userIds []string) (*model.Channel, *model.AppError) { ret := _m.Called(userIds) @@ -1021,6 +1071,31 @@ func (_m *API) GetGroupChannel(userIds []string) (*model.Channel, *model.AppErro return r0, r1 } +// GetGroupsForUser provides a mock function with given fields: userId +func (_m *API) GetGroupsForUser(userId string) ([]*model.Group, *model.AppError) { + ret := _m.Called(userId) + + var r0 []*model.Group + if rf, ok := ret.Get(0).(func(string) []*model.Group); ok { + r0 = rf(userId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*model.Group) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(userId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // GetLDAPUserAttributes provides a mock function with given fields: userId, attributes func (_m *API) GetLDAPUserAttributes(userId string, attributes []string) (map[string]string, *model.AppError) { ret := _m.Called(userId, attributes) diff --git a/services/httpservice/client_test.go b/services/httpservice/client_test.go index f24d5cce14..61c0132975 100644 --- a/services/httpservice/client_test.go +++ b/services/httpservice/client_test.go @@ -6,6 +6,7 @@ package httpservice import ( "context" "fmt" + "github.com/stretchr/testify/assert" "io/ioutil" "net" "net/http" @@ -35,9 +36,7 @@ func TestHTTPClient(t *testing.T) { } { _, err := c.Get(tc.URL) if !tc.IsInternal { - if err != nil { - t.Fatal("google is down?") - } + require.Nil(t, err, "google is down?") } else { allowed := !tc.IsInternal || allowInternal success := err == nil @@ -47,9 +46,7 @@ func TestHTTPClient(t *testing.T) { case *url.Error: success = e.Err != AddressForbidden } - if success != allowed { - t.Fatalf("failed for %v. allowed: %v, success %v", tc.URL, allowed, success) - } + require.Equalf(t, success, allowed, "failed for %v. allowed: %v, success %v", tc.URL, allowed, success) } } } @@ -64,18 +61,12 @@ func TestHTTPClientWithProxy(t *testing.T) { c.Transport.(*MattermostTransport).Transport.(*http.Transport).Proxy = http.ProxyURL(purl) resp, err := c.Get("http://acme.com") - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) defer resp.Body.Close() body, err := ioutil.ReadAll(resp.Body) - if err != nil { - t.Fatal(err) - } - if string(body) != "proxy" { - t.FailNow() - } + require.Nil(t, err) + require.Equal(t, "proxy", string(body)) } func createProxyServer() *httptest.Server { @@ -113,11 +104,14 @@ func TestDialContextFilter(t *testing.T) { return nil, nil }, func(host string) bool { return host == "10.0.0.1" }, func(ip net.IP) bool { return !IsReservedIP(ip) }) _, err := filter(context.Background(), "", tc.Addr) - switch { - case tc.IsValid == (err == AddressForbidden) || (err != nil && err != AddressForbidden): - t.Errorf("unexpected err for %v (%v)", tc.Addr, err) - case tc.IsValid != didDial: - t.Errorf("unexpected didDial for %v", tc.Addr) + + if tc.IsValid { + require.Nil(t, err) + require.True(t, didDial) + } else { + require.NotNil(t, err) + require.Equal(t, err, AddressForbidden) + require.False(t, didDial) } } } @@ -127,19 +121,15 @@ func TestUserAgentIsSet(t *testing.T) { defaultUserAgent = testUserAgent ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { ua := req.UserAgent() - if ua == "" { - t.Error("expected user-agent to be non-empty") - } - if ua != testUserAgent { - t.Errorf("expected user-agent to be %q but was %q", testUserAgent, ua) - } + assert.NotEqual(t, "", ua, "expected user-agent to be non-empty") + assert.Equalf(t, testUserAgent, ua, "expected user-agent to be %q but was %q", testUserAgent, ua) })) defer ts.Close() client := NewHTTPClient(NewTransport(true, nil, nil)) req, err := http.NewRequest("GET", ts.URL, nil) - if err != nil { - t.Fatal("NewRequest failed", err) - } + + require.Nil(t, err, "NewRequest failed", err) + client.Do(req) } @@ -164,9 +154,8 @@ func TestIsReservedIP(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := IsReservedIP(tt.ip); got != tt.want { - t.Errorf("IsReservedIP() = %v, want %v", got, tt.want) - } + got := IsReservedIP(tt.ip) + assert.Equalf(t, tt.want, got, "IsReservedIP() = %v, want %v", got, tt.want) }) } } @@ -182,10 +171,8 @@ func TestIsOwnIP(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got, _ := IsOwnIP(tt.ip); got != tt.want { - t.Errorf("IsOwnIP() = %v, want %v", got, tt.want) - t.Errorf(tt.ip.String()) - } + got, _ := IsOwnIP(tt.ip) + assert.Equalf(t, tt.want, got, "IsOwnIP() = %v, want %v for IP %s", got, tt.want, tt.ip.String()) }) } } diff --git a/services/imageproxy/local.go b/services/imageproxy/local.go index 0ffc4dc025..ad282502a9 100644 --- a/services/imageproxy/local.go +++ b/services/imageproxy/local.go @@ -10,7 +10,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "time" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/services/httpservice" @@ -58,7 +57,7 @@ func makeLocalBackend(proxy *ImageProxy) *LocalBackend { impl.DefaultBaseURL = baseURL } - impl.Timeout = time.Duration(httpservice.RequestTimeout) + impl.Timeout = httpservice.RequestTimeout impl.ContentTypes = imageContentTypes return &LocalBackend{ diff --git a/services/mailservice/mail_test.go b/services/mailservice/mail_test.go index 0b62fcab15..2e2b87302f 100644 --- a/services/mailservice/mail_test.go +++ b/services/mailservice/mail_test.go @@ -9,7 +9,6 @@ import ( "io" "io/ioutil" "os" - "strings" "testing" "time" @@ -29,24 +28,19 @@ func TestMailConnectionFromConfig(t *testing.T) { require.Nil(t, err) cfg := fs.Get() + conn, err := ConnectToSMTPServer(cfg) + require.Nil(t, err, "Should connect to the SMTP Server %v", err) - if conn, err := ConnectToSMTPServer(cfg); err != nil { - t.Log(err) - t.Fatal("Should connect to the STMP Server") - } else { - if _, err1 := NewSMTPClient(conn, cfg); err1 != nil { - t.Log(err) - t.Fatal("Should get new smtp client") - } - } + _, err = NewSMTPClient(conn, cfg) + + require.Nil(t, err, "Should get new SMTP client") *cfg.EmailSettings.SMTPServer = "wrongServer" *cfg.EmailSettings.SMTPPort = "553" - if _, err := ConnectToSMTPServer(cfg); err == nil { - t.Log(err) - t.Fatal("Should not to the STMP Server") - } + _, err = ConnectToSMTPServer(cfg) + + require.NotNil(t, err, "Should not connect to the SMTP Server") } func TestMailConnectionAdvanced(t *testing.T) { @@ -55,7 +49,7 @@ func TestMailConnectionAdvanced(t *testing.T) { cfg := fs.Get() - if conn, err := ConnectToSMTPServerAdvanced( + conn, err := ConnectToSMTPServerAdvanced( &SmtpConnectionInfo{ ConnectionSecurity: *cfg.EmailSettings.ConnectionSecurity, SkipCertVerification: *cfg.EmailSettings.SkipServerCertificateVerification, @@ -63,30 +57,28 @@ func TestMailConnectionAdvanced(t *testing.T) { SmtpServerHost: *cfg.EmailSettings.SMTPServer, SmtpPort: *cfg.EmailSettings.SMTPPort, }, - ); err != nil { - t.Log(err) - t.Fatal("Should connect to the STMP Server") - } else { - if _, err1 := NewSMTPClientAdvanced( - conn, - utils.GetHostnameFromSiteURL(*cfg.ServiceSettings.SiteURL), - &SmtpConnectionInfo{ - ConnectionSecurity: *cfg.EmailSettings.ConnectionSecurity, - SkipCertVerification: *cfg.EmailSettings.SkipServerCertificateVerification, - SmtpServerName: *cfg.EmailSettings.SMTPServer, - SmtpServerHost: *cfg.EmailSettings.SMTPServer, - SmtpPort: *cfg.EmailSettings.SMTPPort, - Auth: *cfg.EmailSettings.EnableSMTPAuth, - SmtpUsername: *cfg.EmailSettings.SMTPUsername, - SmtpPassword: *cfg.EmailSettings.SMTPPassword, - }, - ); err1 != nil { - t.Log(err) - t.Fatal("Should get new smtp client") - } - } + ) - if _, err := ConnectToSMTPServerAdvanced( + require.Nil(t, err, "Should connect to the SMTP Server") + + _, err2 := NewSMTPClientAdvanced( + conn, + utils.GetHostnameFromSiteURL(*cfg.ServiceSettings.SiteURL), + &SmtpConnectionInfo{ + ConnectionSecurity: *cfg.EmailSettings.ConnectionSecurity, + SkipCertVerification: *cfg.EmailSettings.SkipServerCertificateVerification, + SmtpServerName: *cfg.EmailSettings.SMTPServer, + SmtpServerHost: *cfg.EmailSettings.SMTPServer, + SmtpPort: *cfg.EmailSettings.SMTPPort, + Auth: *cfg.EmailSettings.EnableSMTPAuth, + SmtpUsername: *cfg.EmailSettings.SMTPUsername, + SmtpPassword: *cfg.EmailSettings.SMTPPassword, + }, + ) + + require.Nil(t, err2, "Should get new SMTP client") + + _, err3 := ConnectToSMTPServerAdvanced( &SmtpConnectionInfo{ ConnectionSecurity: *cfg.EmailSettings.ConnectionSecurity, SkipCertVerification: *cfg.EmailSettings.SkipServerCertificateVerification, @@ -94,11 +86,8 @@ func TestMailConnectionAdvanced(t *testing.T) { SmtpServerHost: "wrongServer", SmtpPort: "553", }, - ); err == nil { - t.Log(err) - t.Fatal("Should not to the STMP Server") - } - + ) + require.NotNil(t, err3, "Should not connect to the SMTP Server") } func TestSendMailUsingConfig(t *testing.T) { @@ -116,32 +105,25 @@ func TestSendMailUsingConfig(t *testing.T) { //Delete all the messages before check the sample email DeleteMailBox(emailTo) - if err := SendMailUsingConfig(emailTo, emailSubject, emailBody, cfg, true); err != nil { - t.Log(err) - t.Fatal("Should connect to the STMP Server") + err2 := SendMailUsingConfig(emailTo, emailSubject, emailBody, cfg, true) + require.Nil(t, err2, "Should connect to the SMTP Server") + + //Check if the email was send to the right email address + var resultsMailbox JSONMessageHeaderInbucket + err3 := RetryInbucket(5, func() error { + var err error + resultsMailbox, err = GetMailBox(emailTo) + return err + }) + if err3 != nil { + t.Log(err3) + t.Log("No email was received, maybe due load on the server. Skipping this verification") } else { - //Check if the email was send to the right email address - var resultsMailbox JSONMessageHeaderInbucket - err := RetryInbucket(5, func() error { - var err error - resultsMailbox, err = GetMailBox(emailTo) - return err - }) - if err != nil { - t.Log(err) - t.Log("No email was received, maybe due load on the server. Disabling this verification") - } - if err == nil && len(resultsMailbox) > 0 { - if !strings.ContainsAny(resultsMailbox[0].To[0], emailTo) { - t.Fatal("Wrong To recipient") - } else { - if resultsEmail, err := GetMessageFromMailbox(emailTo, resultsMailbox[0].ID); err == nil { - if !strings.Contains(resultsEmail.Body.Text, emailBody) { - t.Log(resultsEmail.Body.Text) - t.Fatal("Received message") - } - } - } + if len(resultsMailbox) > 0 { + require.Contains(t, resultsMailbox[0].To[0], emailTo, "Wrong To: recipient") + resultsEmail, err := GetMessageFromMailbox(emailTo, resultsMailbox[0].ID) + require.Nil(t, err, "Could not get message from mailbox") + require.Contains(t, emailBody, resultsEmail.Body.Text, "Wrong received message %s", resultsEmail.Body.Text) } } } @@ -299,9 +281,7 @@ func TestAuthMethods(t *testing.T) { if err != nil { got = err.Error() } - if got != test.err { - t.Errorf("%d. got error = %q; want %q", i, got, test.err) - } + assert.True(t, got == test.err, "%d. got error = %q; want %q", i, got, test.err) }) } } diff --git a/store/localcachelayer/role_layer.go b/store/localcachelayer/role_layer.go index 0cd03c1427..33aaa2a7c6 100644 --- a/store/localcachelayer/role_layer.go +++ b/store/localcachelayer/role_layer.go @@ -55,11 +55,10 @@ func (s LocalCacheRoleStore) GetByNames(names []string) ([]*model.Role, *model.A roles, _ := s.RoleStore.GetByNames(rolesToQuery) - if roles != nil { - for _, role := range roles { - s.rootStore.doStandardAddToCache(s.rootStore.roleCache, role.Name, role) - } + for _, role := range roles { + s.rootStore.doStandardAddToCache(s.rootStore.roleCache, role.Name, role) } + return append(foundRoles, roles...), nil } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 42e9647b9e..6de2649b29 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1790,7 +1790,7 @@ func (s SqlChannelStore) RemoveAllDeactivatedMembers(channelId string) *model.Ap func (s SqlChannelStore) PermanentDeleteMembersByUser(userId string) *model.AppError { if _, err := s.GetMaster().Exec("DELETE FROM ChannelMembers WHERE UserId = :UserId", map[string]interface{}{"UserId": userId}); err != nil { - return model.NewAppError("SqlChannelStore.RemoveMember", "store.sql_channel.permanent_delete_members_by_user.app_error", nil, "user_id="+userId+", "+err.Error(), http.StatusInternalServerError) + return model.NewAppError("SqlChannelStore.ChannelPermanentDeleteMembersByUser", "store.sql_channel.permanent_delete_members_by_user.app_error", nil, "user_id="+userId+", "+err.Error(), http.StatusInternalServerError) } return nil } diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index 0895ef0ae9..f0e4555ff3 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -123,6 +123,18 @@ func (s *SqlGroupStore) Get(groupId string) (*model.Group, *model.AppError) { return group, nil } +func (s *SqlGroupStore) GetByName(name string) (*model.Group, *model.AppError) { + var group *model.Group + if err := s.GetReplica().SelectOne(&group, "SELECT * from UserGroups WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil { + if err == sql.ErrNoRows { + return nil, model.NewAppError("SqlGroupStore.GroupGetByName", "store.sql_group.no_rows", nil, err.Error(), http.StatusNotFound) + } + return nil, model.NewAppError("SqlGroupStore.GroupGetByName", "store.select_error", nil, err.Error(), http.StatusInternalServerError) + } + + return group, nil +} + func (s *SqlGroupStore) GetByIDs(groupIDs []string) ([]*model.Group, *model.AppError) { var groups []*model.Group query := s.getQueryBuilder().Select("*").From("UserGroups").Where(sq.Eq{"Id": groupIDs}) @@ -158,6 +170,26 @@ func (s *SqlGroupStore) GetAllBySource(groupSource model.GroupSource) ([]*model. return groups, nil } +func (s *SqlGroupStore) GetByUser(userId string) ([]*model.Group, *model.AppError) { + var groups []*model.Group + + query := ` + SELECT + UserGroups.* + FROM + GroupMembers + JOIN UserGroups ON UserGroups.Id = GroupMembers.GroupId + WHERE + GroupMembers.DeleteAt = 0 + AND UserId = :UserId` + + if _, err := s.GetReplica().Select(&groups, query, map[string]interface{}{"UserId": userId}); err != nil { + return nil, model.NewAppError("SqlGroupStore.GetByUser", "store.select_error", nil, err.Error(), http.StatusInternalServerError) + } + + return groups, nil +} + func (s *SqlGroupStore) Update(group *model.Group) (*model.Group, *model.AppError) { var retrievedGroup *model.Group if err := s.GetMaster().SelectOne(&retrievedGroup, "SELECT * FROM UserGroups WHERE Id = :Id", map[string]interface{}{"Id": group.Id}); err != nil { @@ -341,6 +373,13 @@ func (s *SqlGroupStore) DeleteMember(groupID string, userID string) (*model.Grou return retrievedMember, nil } +func (s *SqlGroupStore) PermanentDeleteMembersByUser(userId string) *model.AppError { + if _, err := s.GetMaster().Exec("DELETE FROM GroupMembers WHERE UserId = :UserId", map[string]interface{}{"UserId": userId}); err != nil { + return model.NewAppError("SqlGroupStore.GroupPermanentDeleteMembersByUser", "store.sql_group.permanent_delete_members_by_user.app_error", map[string]interface{}{"UserId": userId}, "", http.StatusInternalServerError) + } + return nil +} + func (s *SqlGroupStore) CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) { if err := groupSyncable.IsValid(); err != nil { return nil, err diff --git a/store/sqlstore/integrity_test.go b/store/sqlstore/integrity_test.go index 02125a4cee..d511f32f83 100644 --- a/store/sqlstore/integrity_test.go +++ b/store/sqlstore/integrity_test.go @@ -14,10 +14,10 @@ import ( func createAudit(ss store.Store, userId, sessionId string) *model.Audit { audit := model.Audit{ - UserId: userId, + UserId: userId, SessionId: sessionId, IpAddress: "ipaddress", - Action: "Action", + Action: "Action", } ss.Audit().Save(&audit) return &audit @@ -46,7 +46,7 @@ func createChannelWithSchemeId(ss store.Store, schemeId *string) *model.Channel return c } -func createCommand(ss store.Store, userId, teamId string) * model.Command { +func createCommand(ss store.Store, userId, teamId string) *model.Command { m := model.Command{} m.CreatorId = userId m.Method = model.COMMAND_METHOD_POST @@ -75,19 +75,15 @@ func createChannelMemberHistory(ss store.Store, channelId, userId string) *model } func createChannelWithTeamId(ss store.Store, id string) *model.Channel { - return createChannel(ss, id, model.NewId()); + return createChannel(ss, id, model.NewId()) } func createChannelWithCreatorId(ss store.Store, id string) *model.Channel { - return createChannel(ss, model.NewId(), id); + return createChannel(ss, model.NewId(), id) } func createChannelMemberWithChannelId(ss store.Store, id string) *model.ChannelMember { - return createChannelMember(ss, id, model.NewId()); -} - -func createChannelMemberWithUserId(ss store.Store, id string) *model.ChannelMember { - return createChannelMember(ss, model.NewId(), id); + return createChannelMember(ss, id, model.NewId()) } func createCommandWebhook(ss store.Store, commandId, userId, channelId string) *model.CommandWebhook { @@ -192,20 +188,20 @@ func createPost(ss store.Store, channelId, userId, rootId, parentId string) *mod } func createPostWithChannelId(ss store.Store, id string) *model.Post { - return createPost(ss, id, model.NewId(), "", ""); + return createPost(ss, id, model.NewId(), "", "") } func createPostWithUserId(ss store.Store, id string) *model.Post { - return createPost(ss, model.NewId(), id, "", ""); + return createPost(ss, model.NewId(), id, "", "") } func createPreferences(ss store.Store, userId string) *model.Preferences { preferences := model.Preferences{ { - UserId: userId, - Name: model.NewId(), + UserId: userId, + Name: model.NewId(), Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW, - Value: "somevalue", + Value: "somevalue", }, } ss.Preference().Save(&preferences) @@ -369,10 +365,10 @@ func TestCheckParentChildIntegrity(t *testing.T) { supplier := ss.(*store.LayeredStore).DatabaseLayer.(*SqlSupplier) t.Run("should receive an error", func(t *testing.T) { config := relationalCheckConfig{ - parentName: "NotValid", + parentName: "NotValid", parentIdAttr: "NotValid", - childName: "NotValid", - childIdAttr: "NotValid", + childName: "NotValid", + childIdAttr: "NotValid", } result := checkParentChildIntegrity(supplier, config) require.NotNil(t, result.Err) @@ -402,7 +398,7 @@ func TestCheckChannelsCommandWebhooksIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: channelId, - ChildId: cwh.Id, + ChildId: cwh.Id, }, data.Records[0]) dbmap.Delete(cwh) }) @@ -488,7 +484,7 @@ func TestCheckChannelsIncomingWebhooksIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: channelId, - ChildId: wh.Id, + ChildId: wh.Id, }, data.Records[0]) dbmap.Delete(wh) }) @@ -518,7 +514,7 @@ func TestCheckChannelsOutgoingWebhooksIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: channelId, - ChildId: wh.Id, + ChildId: wh.Id, }, data.Records[0]) dbmap.Delete(wh) }) @@ -545,7 +541,7 @@ func TestCheckChannelsPostsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: post.ChannelId, - ChildId: post.Id, + ChildId: post.Id, }, data.Records[0]) dbmap.Delete(post) }) @@ -573,7 +569,7 @@ func TestCheckCommandsCommandWebhooksIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: commandId, - ChildId: cwh.Id, + ChildId: cwh.Id, }, data.Records[0]) dbmap.Delete(cwh) }) @@ -631,7 +627,7 @@ func TestCheckPostsPostsParentIdIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: parentId, - ChildId: post.Id, + ChildId: post.Id, }, data.Records[0]) dbmap.Delete(root) dbmap.Delete(post) @@ -662,7 +658,7 @@ func TestCheckPostsPostsRootIdIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: rootId, - ChildId: post.Id, + ChildId: post.Id, }, data.Records[0]) dbmap.Delete(post) }) @@ -720,7 +716,7 @@ func TestCheckSchemesChannelsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: schemeId, - ChildId: channel.Id, + ChildId: channel.Id, }, data.Records[0]) dbmap.Delete(channel) }) @@ -751,7 +747,7 @@ func TestCheckSchemesTeamsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: schemeId, - ChildId: team.Id, + ChildId: team.Id, }, data.Records[0]) dbmap.Delete(team) }) @@ -782,7 +778,7 @@ func TestCheckSessionsAuditsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: sessionId, - ChildId: audit.Id, + ChildId: audit.Id, }, data.Records[0]) ss.Audit().PermanentDeleteByUser(userId) }) @@ -809,7 +805,7 @@ func TestCheckTeamsChannelsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: channel.TeamId, - ChildId: channel.Id, + ChildId: channel.Id, }, data.Records[0]) dbmap.Delete(channel) }) @@ -837,7 +833,7 @@ func TestCheckTeamsCommandsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: teamId, - ChildId: cmd.Id, + ChildId: cmd.Id, }, data.Records[0]) dbmap.Delete(cmd) }) @@ -865,7 +861,7 @@ func TestCheckTeamsIncomingWebhooksIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: teamId, - ChildId: wh.Id, + ChildId: wh.Id, }, data.Records[0]) dbmap.Delete(wh) }) @@ -893,7 +889,7 @@ func TestCheckTeamsOutgoingWebhooksIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: teamId, - ChildId: wh.Id, + ChildId: wh.Id, }, data.Records[0]) dbmap.Delete(wh) }) @@ -951,7 +947,7 @@ func TestCheckUsersAuditsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: audit.Id, + ChildId: audit.Id, }, data.Records[0]) ss.Audit().PermanentDeleteByUser(userId) }) @@ -979,7 +975,7 @@ func TestCheckUsersCommandWebhooksIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: cwh.Id, + ChildId: cwh.Id, }, data.Records[0]) dbmap.Delete(cwh) }) @@ -1006,7 +1002,7 @@ func TestCheckUsersChannelsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: channel.CreatorId, - ChildId: channel.Id, + ChildId: channel.Id, }, data.Records[0]) dbmap.Delete(channel) }) @@ -1094,7 +1090,7 @@ func TestCheckUsersCommandsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: cmd.Id, + ChildId: cmd.Id, }, data.Records[0]) dbmap.Delete(cmd) }) @@ -1124,7 +1120,7 @@ func TestCheckUsersCompliancesIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: compliance.Id, + ChildId: compliance.Id, }, data.Records[0]) dbmap.Delete(compliance) }) @@ -1154,7 +1150,7 @@ func TestCheckUsersEmojiIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: emoji.Id, + ChildId: emoji.Id, }, data.Records[0]) dbmap.Delete(emoji) }) @@ -1211,7 +1207,7 @@ func TestCheckUsersIncomingWebhooksIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: wh.Id, + ChildId: wh.Id, }, data.Records[0]) dbmap.Delete(wh) }) @@ -1241,7 +1237,7 @@ func TestCheckUsersOAuthAccessDataIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: ad.Token, + ChildId: ad.Token, }, data.Records[0]) ss.OAuth().RemoveAccessData(ad.Token) }) @@ -1271,7 +1267,7 @@ func TestCheckUsersOAuthAppsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: app.Id, + ChildId: app.Id, }, data.Records[0]) ss.OAuth().DeleteApp(app.Id) }) @@ -1301,7 +1297,7 @@ func TestCheckUsersOAuthAuthDataIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: ad.Code, + ChildId: ad.Code, }, data.Records[0]) ss.OAuth().RemoveAuthData(ad.Code) }) @@ -1329,7 +1325,7 @@ func TestCheckUsersOutgoingWebhooksIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: wh.Id, + ChildId: wh.Id, }, data.Records[0]) dbmap.Delete(wh) }) @@ -1356,7 +1352,7 @@ func TestCheckUsersPostsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: post.UserId, - ChildId: post.Id, + ChildId: post.Id, }, data.Records[0]) dbmap.Delete(post) }) @@ -1442,7 +1438,7 @@ func TestCheckUsersSessionsIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: session.Id, + ChildId: session.Id, }, data.Records[0]) dbmap.Delete(session) }) @@ -1531,7 +1527,7 @@ func TestCheckUsersUserAccessTokensIntegrity(t *testing.T) { require.Len(t, data.Records, 1) require.Equal(t, store.OrphanedRecord{ ParentId: userId, - ChildId: uat.Id, + ChildId: uat.Id, }, data.Records[0]) ss.UserAccessToken().Delete(uat.Id) }) diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index a58d63bcc0..3560fd61c7 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -1346,7 +1346,6 @@ func (s *SqlPostStore) GetOldest() (*model.Post, *model.AppError) { } func (s *SqlPostStore) determineMaxPostSize() int { - var maxPostSize int = model.POST_MESSAGE_MAX_RUNES_V1 var maxPostSizeBytes int32 if s.DriverName() == model.DATABASE_DRIVER_POSTGRES { @@ -1384,7 +1383,7 @@ func (s *SqlPostStore) determineMaxPostSize() int { } // Assume a worst-case representation of four bytes per rune. - maxPostSize = int(maxPostSizeBytes) / 4 + maxPostSize := int(maxPostSizeBytes) / 4 // To maintain backwards compatibility, don't yield a maximum post // size smaller than the previous limit, even though it wasn't diff --git a/store/sqlstore/supplier.go b/store/sqlstore/supplier.go index 9d04cedf7b..3db37c90d3 100644 --- a/store/sqlstore/supplier.go +++ b/store/sqlstore/supplier.go @@ -699,7 +699,7 @@ func (ss *SqlSupplier) AlterColumnDefaultIfExists(tableName string, columnName s return false } - var defaultValue = "" + var defaultValue string if ss.DriverName() == model.DATABASE_DRIVER_MYSQL { // Some column types in MySQL cannot have defaults, so don't try to configure anything. if mySqlColDefault == nil { diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index 4b216a4a7a..fe29f711e7 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -4,7 +4,6 @@ package sqlstore import ( - "database/sql" "encoding/json" "os" "strings" @@ -602,33 +601,6 @@ func UpgradeDatabaseToVersion57(sqlStore SqlStore) { } } -func getRole(sqlStore SqlStore, name string) (*model.Role, error) { - var dbRole Role - - if err := sqlStore.GetReplica().SelectOne(&dbRole, "SELECT * from Roles WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil { - if err == sql.ErrNoRows { - return nil, errors.Wrapf(err, "failed to find role %s", name) - } else { - return nil, errors.Wrapf(err, "failed to query role %s", name) - } - } - - return dbRole.ToModel(), nil -} - -func saveRole(sqlStore SqlStore, role *model.Role) error { - dbRole := NewRoleFromModel(role) - - dbRole.UpdateAt = model.GetMillis() - if rowsChanged, err := sqlStore.GetMaster().Update(dbRole); err != nil { - return errors.Wrap(err, "failed to update role") - } else if rowsChanged != 1 { - return errors.New("found no role to update") - } - - return nil -} - func UpgradeDatabaseToVersion58(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_7_0, VERSION_5_8_0) { // idx_channels_txt was removed in `UpgradeDatabaseToVersion50`, but merged as part of diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 4c3acf993f..a30035b52f 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -1267,7 +1267,7 @@ func generateSearchQuery(query sq.SelectBuilder, terms []string, fields []string func (us SqlUserStore) performSearch(query sq.SelectBuilder, term string, options *model.UserSearchOptions) ([]*model.User, *model.AppError) { term = sanitizeSearchTerm(term, "*") - searchType := USER_SEARCH_TYPE_NAMES_NO_FULL_NAME + var searchType []string if options.AllowEmails { if options.AllowFullNames { searchType = USER_SEARCH_TYPE_ALL @@ -1359,8 +1359,7 @@ func (us SqlUserStore) GetProfilesNotInTeam(teamId string, groupConstrained bool } func (us SqlUserStore) GetEtagForProfilesNotInTeam(teamId string) string { - var querystr string - querystr = ` + querystr := ` SELECT CONCAT(MAX(UpdateAt), '.', COUNT(Id)) as etag FROM diff --git a/store/store.go b/store/store.go index a1385967b2..399ceb898b 100644 --- a/store/store.go +++ b/store/store.go @@ -574,9 +574,11 @@ type UserTermsOfServiceStore interface { type GroupStore interface { Create(group *model.Group) (*model.Group, *model.AppError) Get(groupID string) (*model.Group, *model.AppError) + GetByName(name string) (*model.Group, *model.AppError) GetByIDs(groupIDs []string) ([]*model.Group, *model.AppError) GetByRemoteID(remoteID string, groupSource model.GroupSource) (*model.Group, *model.AppError) GetAllBySource(groupSource model.GroupSource) ([]*model.Group, *model.AppError) + GetByUser(userId string) ([]*model.Group, *model.AppError) Update(group *model.Group) (*model.Group, *model.AppError) Delete(groupID string) (*model.Group, *model.AppError) @@ -585,6 +587,7 @@ type GroupStore interface { GetMemberCount(groupID string) (int64, *model.AppError) UpsertMember(groupID string, userID string) (*model.GroupMember, *model.AppError) DeleteMember(groupID string, userID string) (*model.GroupMember, *model.AppError) + PermanentDeleteMembersByUser(userId string) *model.AppError CreateGroupSyncable(groupSyncable *model.GroupSyncable) (*model.GroupSyncable, *model.AppError) GetGroupSyncable(groupID string, syncableID string, syncableType model.GroupSyncableType) (*model.GroupSyncable, *model.AppError) diff --git a/store/storetest/bot_store.go b/store/storetest/bot_store.go index 683eaf4e7b..37f07d45d2 100644 --- a/store/storetest/bot_store.go +++ b/store/storetest/bot_store.go @@ -175,13 +175,13 @@ func testBotStoreGetAll(t *testing.T, ss store.Store) { Email: MakeEmail(), Username: model.NewId(), } - if _, err := ss.User().Save(&deletedUser); err != nil { - t.Fatal("couldn't save user", err) - } + _, err1 := ss.User().Save(&deletedUser) + require.Nil(t, err1, "couldn't save user") + deletedUser.DeleteAt = model.GetMillis() - if _, err := ss.User().Update(&deletedUser, true); err != nil { - t.Fatal("couldn't delete user", err) - } + _, err2 := ss.User().Update(&deletedUser, true) + require.Nil(t, err2, "couldn't delete user") + defer func() { require.Nil(t, ss.User().PermanentDelete(deletedUser.Id)) }() ob5, _ := makeBotWithUser(t, ss, &model.Bot{ Username: "ob5", diff --git a/store/storetest/command_webhook_store.go b/store/storetest/command_webhook_store.go index 9fdcb319fe..77cef61999 100644 --- a/store/storetest/command_webhook_store.go +++ b/store/storetest/command_webhook_store.go @@ -4,14 +4,13 @@ package storetest import ( - "testing" - - "github.com/stretchr/testify/require" - "net/http" + "testing" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCommandWebhookStore(t *testing.T, ss store.Store) { @@ -29,17 +28,12 @@ func testCommandWebhookStore(t *testing.T, ss store.Store) { require.Nil(t, err) var r1 *model.CommandWebhook - if r1, err = cws.Get(h1.Id); err != nil { - t.Fatal(err) - } else { - if *r1 != *h1 { - t.Fatal("invalid returned webhook") - } - } + r1, err = cws.Get(h1.Id) + require.Nil(t, err) + assert.Equal(t, *r1, *h1, "invalid returned webhook") - if _, err = cws.Get("123"); err.StatusCode != http.StatusNotFound { - t.Fatal("Should have set the status as not found for missing id") - } + _, err = cws.Get("123") + assert.Equal(t, err.StatusCode, http.StatusNotFound, "Should have set the status as not found for missing id") h2 := &model.CommandWebhook{} h2.CreateAt = model.GetMillis() - 2*model.COMMAND_WEBHOOK_LIFETIME @@ -49,25 +43,22 @@ func testCommandWebhookStore(t *testing.T, ss store.Store) { h2, err = cws.Save(h2) require.Nil(t, err) - if _, err := cws.Get(h2.Id); err == nil || err.StatusCode != http.StatusNotFound { - t.Fatal("Should have set the status as not found for expired webhook") - } + _, err = cws.Get(h2.Id) + require.NotNil(t, err, "Should have set the status as not found for expired webhook") + assert.Equal(t, err.StatusCode, http.StatusNotFound, "Should have set the status as not found for expired webhook") cws.Cleanup() - if _, err := cws.Get(h1.Id); err != nil { - t.Fatal("Should have no error getting unexpired webhook") - } + _, err = cws.Get(h1.Id) + require.Nil(t, err, "Should have no error getting unexpired webhook") - if _, err := cws.Get(h2.Id); err.StatusCode != http.StatusNotFound { - t.Fatal("Should have set the status as not found for expired webhook") - } + _, err = cws.Get(h2.Id) + assert.Equal(t, err.StatusCode, http.StatusNotFound, "Should have set the status as not found for expired webhook") - if err := cws.TryUse(h1.Id, 1); err != nil { - t.Fatal("Should be able to use webhook once") - } + err = cws.TryUse(h1.Id, 1) + require.Nil(t, err, "Should be able to use webhook once") - if err := cws.TryUse(h1.Id, 1); err == nil || err.StatusCode != http.StatusBadRequest { - t.Fatal("Should be able to use webhook once") - } + err = cws.TryUse(h1.Id, 1) + require.NotNil(t, err, "Should be able to use webhook once") + assert.Equal(t, err.StatusCode, http.StatusBadRequest, "Should be able to use webhook once") } diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index 11afeb3fe8..d91103cde0 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" @@ -20,9 +21,11 @@ import ( func TestGroupStore(t *testing.T, ss store.Store) { t.Run("Create", func(t *testing.T) { testGroupStoreCreate(t, ss) }) t.Run("Get", func(t *testing.T) { testGroupStoreGet(t, ss) }) + t.Run("GetByName", func(t *testing.T) { testGroupStoreGetByName(t, ss) }) t.Run("GetByIDs", func(t *testing.T) { testGroupStoreGetByIDs(t, ss) }) t.Run("GetByRemoteID", func(t *testing.T) { testGroupStoreGetByRemoteID(t, ss) }) t.Run("GetAllBySource", func(t *testing.T) { testGroupStoreGetAllByType(t, ss) }) + t.Run("GetByUser", func(t *testing.T) { testGroupStoreGetByUser(t, ss) }) t.Run("Update", func(t *testing.T) { testGroupStoreUpdate(t, ss) }) t.Run("Delete", func(t *testing.T) { testGroupStoreDelete(t, ss) }) @@ -30,6 +33,7 @@ func TestGroupStore(t *testing.T, ss store.Store) { t.Run("GetMemberUsersPage", func(t *testing.T) { testGroupGetMemberUsersPage(t, ss) }) t.Run("UpsertMember", func(t *testing.T) { testUpsertMember(t, ss) }) t.Run("DeleteMember", func(t *testing.T) { testGroupDeleteMember(t, ss) }) + t.Run("PermanentDeleteMembersByUser", func(t *testing.T) { testGroupPermanentDeleteMembersByUser(t, ss) }) t.Run("CreateGroupSyncable", func(t *testing.T) { testCreateGroupSyncable(t, ss) }) t.Run("GetGroupSyncable", func(t *testing.T) { testGetGroupSyncable(t, ss) }) @@ -181,6 +185,37 @@ func testGroupStoreGet(t *testing.T, ss store.Store) { require.Equal(t, err.Id, "store.sql_group.no_rows") } +func testGroupStoreGetByName(t *testing.T, ss store.Store) { + // Create a group + g1 := &model.Group{ + Name: model.NewId(), + DisplayName: model.NewId(), + Description: model.NewId(), + Source: model.GroupSourceLdap, + RemoteId: model.NewId(), + } + d1, err := ss.Group().Create(g1) + require.Nil(t, err) + require.Len(t, d1.Id, 26) + + // Get the group + d2, err := ss.Group().GetByName(d1.Name) + require.Nil(t, err) + require.Equal(t, d1.Id, d2.Id) + require.Equal(t, d1.Name, d2.Name) + require.Equal(t, d1.DisplayName, d2.DisplayName) + require.Equal(t, d1.Description, d2.Description) + require.Equal(t, d1.RemoteId, d2.RemoteId) + require.Equal(t, d1.CreateAt, d2.CreateAt) + require.Equal(t, d1.UpdateAt, d2.UpdateAt) + require.Equal(t, d1.DeleteAt, d2.DeleteAt) + + // Get an invalid group + _, err = ss.Group().GetByName(model.NewId()) + require.NotNil(t, err) + require.Equal(t, err.Id, "store.sql_group.no_rows") +} + func testGroupStoreGetByIDs(t *testing.T, ss store.Store) { var group1 *model.Group var group2 *model.Group @@ -280,6 +315,76 @@ func testGroupStoreGetAllByType(t *testing.T, ss store.Store) { } } +func testGroupStoreGetByUser(t *testing.T, ss store.Store) { + // Save a group + g1 := &model.Group{ + Name: model.NewId(), + DisplayName: model.NewId(), + Description: model.NewId(), + Source: model.GroupSourceLdap, + RemoteId: model.NewId(), + } + g1, err := ss.Group().Create(g1) + require.Nil(t, err) + + g2 := &model.Group{ + Name: model.NewId(), + DisplayName: model.NewId(), + Description: model.NewId(), + Source: model.GroupSourceLdap, + RemoteId: model.NewId(), + } + g2, err = ss.Group().Create(g2) + require.Nil(t, err) + + u1 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + } + u1, err = ss.User().Save(u1) + require.Nil(t, err) + + _, err = ss.Group().UpsertMember(g1.Id, u1.Id) + require.Nil(t, err) + _, err = ss.Group().UpsertMember(g2.Id, u1.Id) + require.Nil(t, err) + + u2 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + } + u2, err = ss.User().Save(u2) + require.Nil(t, err) + + _, err = ss.Group().UpsertMember(g2.Id, u2.Id) + require.Nil(t, err) + + groups, err := ss.Group().GetByUser(u1.Id) + require.Nil(t, err) + assert.Equal(t, 2, len(groups)) + found1 := false + found2 := false + for _, g := range groups { + if g.Id == g1.Id { + found1 = true + } + if g.Id == g2.Id { + found2 = true + } + } + assert.True(t, found1) + assert.True(t, found2) + + groups, err = ss.Group().GetByUser(u2.Id) + require.Nil(t, err) + require.Equal(t, 1, len(groups)) + assert.Equal(t, g2.Id, groups[0].Id) + + groups, err = ss.Group().GetByUser(model.NewId()) + require.Nil(t, err) + assert.Equal(t, 0, len(groups)) +} + func testGroupStoreUpdate(t *testing.T, ss store.Store) { // Save a new group g1 := &model.Group{ @@ -658,6 +763,42 @@ func testGroupDeleteMember(t *testing.T, ss store.Store) { require.Equal(t, err.Id, "store.sql_group.no_rows") } +func testGroupPermanentDeleteMembersByUser(t *testing.T, ss store.Store) { + var g *model.Group + var groups []*model.Group + numberOfGroups := 5 + + for i := 0; i < numberOfGroups; i++ { + g = &model.Group{ + Name: model.NewId(), + DisplayName: model.NewId(), + Source: model.GroupSourceLdap, + RemoteId: model.NewId(), + } + group, err := ss.Group().Create(g) + groups = append(groups, group) + require.Nil(t, err) + } + + // Create user + u1 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + } + user, err := ss.User().Save(u1) + require.Nil(t, err) + + // Create members + for _, group := range groups { + _, err = ss.Group().UpsertMember(group.Id, user.Id) + require.Nil(t, err) + } + + // Happy path + err = ss.Group().PermanentDeleteMembersByUser(user.Id) + require.Nil(t, err) +} + func testCreateGroupSyncable(t *testing.T, ss store.Store) { // Invalid GroupID _, err := ss.Group().CreateGroupSyncable(model.NewGroupTeam("x", model.NewId(), false)) @@ -897,7 +1038,7 @@ func testDeleteGroupSyncable(t *testing.T, ss store.Store) { require.Equal(t, err.Id, "store.sql_group.no_rows") // Non-existent Team - _, err = ss.Group().DeleteGroupSyncable(groupTeam.GroupId, string(model.NewId()), model.GroupSyncableTypeTeam) + _, err = ss.Group().DeleteGroupSyncable(groupTeam.GroupId, model.NewId(), model.GroupSyncableTypeTeam) require.Equal(t, err.Id, "store.sql_group.no_rows") // Happy path... @@ -1654,10 +1795,10 @@ func testGetGroupsByChannel(t *testing.T, ss store.Store) { _, err = ss.User().Update(user2, true) require.Nil(t, err) - group1WithMemberCount := model.Group(*group1) + group1WithMemberCount := *group1 group1WithMemberCount.MemberCount = model.NewInt(1) - group2WithMemberCount := model.Group(*group2) + group2WithMemberCount := *group2 group2WithMemberCount.MemberCount = model.NewInt(0) testCases := []struct { @@ -1868,10 +2009,10 @@ func testGetGroupsByTeam(t *testing.T, ss store.Store) { _, err = ss.User().Update(user2, true) require.Nil(t, err) - group1WithMemberCount := model.Group(*group1) + group1WithMemberCount := *group1 group1WithMemberCount.MemberCount = model.NewInt(1) - group2WithMemberCount := model.Group(*group2) + group2WithMemberCount := *group2 group2WithMemberCount.MemberCount = model.NewInt(0) testCases := []struct { diff --git a/store/storetest/mocks/GroupStore.go b/store/storetest/mocks/GroupStore.go index f630d27581..2c1a7fbefc 100644 --- a/store/storetest/mocks/GroupStore.go +++ b/store/storetest/mocks/GroupStore.go @@ -406,6 +406,31 @@ func (_m *GroupStore) GetByIDs(groupIDs []string) ([]*model.Group, *model.AppErr return r0, r1 } +// GetByName provides a mock function with given fields: name +func (_m *GroupStore) GetByName(name string) (*model.Group, *model.AppError) { + ret := _m.Called(name) + + var r0 *model.Group + if rf, ok := ret.Get(0).(func(string) *model.Group); ok { + r0 = rf(name) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Group) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(name) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // GetByRemoteID provides a mock function with given fields: remoteID, groupSource func (_m *GroupStore) GetByRemoteID(remoteID string, groupSource model.GroupSource) (*model.Group, *model.AppError) { ret := _m.Called(remoteID, groupSource) @@ -431,6 +456,31 @@ func (_m *GroupStore) GetByRemoteID(remoteID string, groupSource model.GroupSour return r0, r1 } +// GetByUser provides a mock function with given fields: userId +func (_m *GroupStore) GetByUser(userId string) ([]*model.Group, *model.AppError) { + ret := _m.Called(userId) + + var r0 []*model.Group + if rf, ok := ret.Get(0).(func(string) []*model.Group); ok { + r0 = rf(userId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*model.Group) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(userId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // GetGroupSyncable provides a mock function with given fields: groupID, syncableID, syncableType func (_m *GroupStore) GetGroupSyncable(groupID string, syncableID string, syncableType model.GroupSyncableType) (*model.GroupSyncable, *model.AppError) { ret := _m.Called(groupID, syncableID, syncableType) @@ -604,6 +654,22 @@ func (_m *GroupStore) GetMemberUsersPage(groupID string, page int, perPage int) return r0, r1 } +// PermanentDeleteMembersByUser provides a mock function with given fields: userId +func (_m *GroupStore) PermanentDeleteMembersByUser(userId string) *model.AppError { + ret := _m.Called(userId) + + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string) *model.AppError); ok { + r0 = rf(userId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.AppError) + } + } + + return r0 +} + // TeamMembersMinusGroupMembers provides a mock function with given fields: teamID, groupIDs, page, perPage func (_m *GroupStore) TeamMembersMinusGroupMembers(teamID string, groupIDs []string, page int, perPage int) ([]*model.UserWithGroups, *model.AppError) { ret := _m.Called(teamID, groupIDs, page, perPage) diff --git a/store/storetest/oauth_store.go b/store/storetest/oauth_store.go index c754a2c277..6d3298c718 100644 --- a/store/storetest/oauth_store.go +++ b/store/storetest/oauth_store.go @@ -213,7 +213,7 @@ func testOAuthStoreRemoveAccessData(t *testing.T, ss store.Store) { require.Nil(t, result, "did not delete access token") } -func testOAuthStoreRemoveAllAccessData(t *testing.T, ss store.Store) { +func TestOAuthStoreRemoveAllAccessData(t *testing.T, ss store.Store) { a1 := model.AccessData{} a1.ClientId = model.NewId() a1.UserId = model.NewId() diff --git a/store/storetest/preference_store.go b/store/storetest/preference_store.go index 3456ce2f33..247f84cfb9 100644 --- a/store/storetest/preference_store.go +++ b/store/storetest/preference_store.go @@ -42,26 +42,22 @@ func testPreferenceSave(t *testing.T, ss store.Store) { Value: "value1b", }, } - if err := ss.Preference().Save(&preferences); err != nil { - t.Fatal("saving preference returned error") - } + err := ss.Preference().Save(&preferences) + require.Nil(t, err, "saving preference returned error") for _, preference := range preferences { - if data, _ := ss.Preference().Get(preference.UserId, preference.Category, preference.Name); preference.ToJson() != data.ToJson() { - t.Fatal("got incorrect preference after first Save") - } + data, _ := ss.Preference().Get(preference.UserId, preference.Category, preference.Name) + require.Equal(t, data.ToJson(), preference.ToJson(), "got incorrect preference after first Save") } preferences[0].Value = "value2a" preferences[1].Value = "value2b" - if err := ss.Preference().Save(&preferences); err != nil { - t.Fatal("saving preference returned error") - } + err = ss.Preference().Save(&preferences) + require.Nil(t, err, "saving preference returned error") for _, preference := range preferences { - if data, _ := ss.Preference().Get(preference.UserId, preference.Category, preference.Name); preference.ToJson() != data.ToJson() { - t.Fatal("got incorrect preference after second Save") - } + data, _ := ss.Preference().Get(preference.UserId, preference.Category, preference.Name) + require.Equal(t, data.ToJson(), preference.ToJson(), "got incorrect preference after second Save") } } @@ -96,16 +92,13 @@ func testPreferenceGet(t *testing.T, ss store.Store) { err := ss.Preference().Save(&preferences) require.Nil(t, err) - if data, err := ss.Preference().Get(userId, category, name); err != nil { - t.Fatal(err) - } else if data.ToJson() != preferences[0].ToJson() { - t.Fatal("got incorrect preference") - } + data, err := ss.Preference().Get(userId, category, name) + require.Nil(t, err) + require.Equal(t, preferences[0].ToJson(), data.ToJson(), "got incorrect preference") // make sure getting a missing preference fails - if _, err := ss.Preference().Get(model.NewId(), model.NewId(), model.NewId()); err == nil { - t.Fatal("no error on getting a missing preference") - } + _, err = ss.Preference().Get(model.NewId(), model.NewId(), model.NewId()) + require.NotNil(t, err, "no error on getting a missing preference") } func testPreferenceGetCategory(t *testing.T, ss store.Store) { @@ -142,20 +135,19 @@ func testPreferenceGetCategory(t *testing.T, ss store.Store) { err := ss.Preference().Save(&preferences) require.Nil(t, err) - if preferencesByCategory, err := ss.Preference().GetCategory(userId, category); err != nil { - t.Fatal(err) - } else if len(preferencesByCategory) != 2 { - t.Fatal("got the wrong number of preferences") - } else if !((preferencesByCategory[0] == preferences[0] && preferencesByCategory[1] == preferences[1]) || (preferencesByCategory[0] == preferences[1] && preferencesByCategory[1] == preferences[0])) { - t.Fatal("got incorrect preferences") - } + preferencesByCategory, err := ss.Preference().GetCategory(userId, category) + require.Nil(t, err) + require.Equal(t, 2, len(preferencesByCategory), "got the wrong number of preferences") + require.True( + t, + ((preferencesByCategory[0] == preferences[0] && preferencesByCategory[1] == preferences[1]) || (preferencesByCategory[0] == preferences[1] && preferencesByCategory[1] == preferences[0])), + "got incorrect preferences", + ) // make sure getting a missing preference category doesn't fail - if preferencesByCategory, err := ss.Preference().GetCategory(model.NewId(), model.NewId()); err != nil { - t.Fatal(err) - } else if len(preferencesByCategory) != 0 { - t.Fatal("shouldn't have got any preferences") - } + preferencesByCategory, err = ss.Preference().GetCategory(model.NewId(), model.NewId()) + require.Nil(t, err) + require.Equal(t, 0, len(preferencesByCategory), "shouldn't have got any preferences") } func testPreferenceGetAll(t *testing.T, ss store.Store) { @@ -192,17 +184,14 @@ func testPreferenceGetAll(t *testing.T, ss store.Store) { err := ss.Preference().Save(&preferences) require.Nil(t, err) - if result, err := ss.Preference().GetAll(userId); err != nil { - t.Fatal(err) - } else if len(result) != 3 { - t.Fatal("got the wrong number of preferences") - } else { - for i := 0; i < 3; i++ { - if result[0] != preferences[i] && result[1] != preferences[i] && result[2] != preferences[i] { - t.Fatal("got incorrect preferences") - } - } + result, err := ss.Preference().GetAll(userId) + require.Nil(t, err) + require.Equal(t, 3, len(result), "got the wrong number of preferences") + + for i := 0; i < 3; i++ { + assert.Falsef(t, result[0] != preferences[i] && result[1] != preferences[i] && result[2] != preferences[i], "got incorrect preferences") } + } func testPreferenceDeleteByUser(t *testing.T, ss store.Store) { @@ -239,9 +228,8 @@ func testPreferenceDeleteByUser(t *testing.T, ss store.Store) { err := ss.Preference().Save(&preferences) require.Nil(t, err) - if err := ss.Preference().PermanentDeleteByUser(userId); err != nil { - t.Fatal(err) - } + err = ss.Preference().PermanentDeleteByUser(userId) + require.Nil(t, err) } func testPreferenceDelete(t *testing.T, ss store.Store) { @@ -259,13 +247,11 @@ func testPreferenceDelete(t *testing.T, ss store.Store) { require.Nil(t, err) assert.Len(t, preferences, 1, "should've returned 1 preference") - if err = ss.Preference().Delete(preference.UserId, preference.Category, preference.Name); err != nil { - t.Fatal(err) - } + err = ss.Preference().Delete(preference.UserId, preference.Category, preference.Name) + require.Nil(t, err) preferences, err = ss.Preference().GetAll(preference.UserId) require.Nil(t, err) assert.Len(t, preferences, 0, "should've returned no preferences") - } func testPreferenceDeleteCategory(t *testing.T, ss store.Store) { @@ -293,9 +279,8 @@ func testPreferenceDeleteCategory(t *testing.T, ss store.Store) { require.Nil(t, err) assert.Len(t, preferences, 2, "should've returned 2 preferences") - if err = ss.Preference().DeleteCategory(userId, category); err != nil { - t.Fatal(err) - } + err = ss.Preference().DeleteCategory(userId, category) + require.Nil(t, err) preferences, err = ss.Preference().GetAll(userId) require.Nil(t, err) @@ -333,9 +318,8 @@ func testPreferenceDeleteCategoryAndName(t *testing.T, ss store.Store) { require.Nil(t, err) assert.Len(t, preferences, 1, "should've returned 1 preference") - if err = ss.Preference().DeleteCategoryAndName(category, name); err != nil { - t.Fatal(err) - } + err = ss.Preference().DeleteCategoryAndName(category, name) + require.Nil(t, err) preferences, err = ss.Preference().GetAll(userId) require.Nil(t, err) diff --git a/store/storetest/terms_of_service_store.go b/store/storetest/terms_of_service_store.go index 4e309bf213..af20c5327e 100644 --- a/store/storetest/terms_of_service_store.go +++ b/store/storetest/terms_of_service_store.go @@ -30,13 +30,9 @@ func testSaveTermsOfService(t *testing.T, ss store.Store) { savedTermsOfService, err := ss.TermsOfService().Save(termsOfService) require.Nil(t, err) - if len(savedTermsOfService.Id) != 26 { - t.Fatal("Id should have been populated") - } + require.Len(t, savedTermsOfService.Id, 26, "Id should have been populated") - if savedTermsOfService.CreateAt == 0 { - t.Fatal("Create at should have been populated") - } + require.NotEqual(t, savedTermsOfService.CreateAt, 0, "Create at should have been populated") } func testGetLatestTermsOfService(t *testing.T, ss store.Store) { diff --git a/store/storetest/user_access_token_store.go b/store/storetest/user_access_token_store.go index 0a3c82f47c..0e4e6ce390 100644 --- a/store/storetest/user_access_token_store.go +++ b/store/storetest/user_access_token_store.go @@ -31,49 +31,36 @@ func testUserAccessTokenSaveGetDelete(t *testing.T, ss store.Store) { s1, err := ss.Session().Save(s1) require.Nil(t, err) - if _, err = ss.UserAccessToken().Save(uat); err != nil { - t.Fatal(err) - } + _, err = ss.UserAccessToken().Save(uat) + require.Nil(t, err) - if result, terr := ss.UserAccessToken().Get(uat.Id); terr != nil { - t.Fatal(terr) - } else if result.Token != uat.Token { - t.Fatal("received incorrect token after save") - } + result, terr := ss.UserAccessToken().Get(uat.Id) + require.Nil(t, terr) + require.Equal(t, result.Token, uat.Token, "received incorrect token after save") - if received, err2 := ss.UserAccessToken().GetByToken(uat.Token); err2 != nil { - t.Fatal(err2) - } else if received.Token != uat.Token { - t.Fatal("received incorrect token after save") - } + received, err2 := ss.UserAccessToken().GetByToken(uat.Token) + require.Nil(t, err2) + require.Equal(t, received.Token, uat.Token, "received incorrect token after save") - if _, err = ss.UserAccessToken().GetByToken("notarealtoken"); err == nil { - t.Fatal("should have failed on bad token") - } + _, err = ss.UserAccessToken().GetByToken("notarealtoken") + require.NotNil(t, err, "should have failed on bad token") - if received, err2 := ss.UserAccessToken().GetByUser(uat.UserId, 0, 100); err2 != nil { - t.Fatal(err2) - } else if len(received) != 1 { - t.Fatal("received incorrect number of tokens after save") - } + received2, err2 := ss.UserAccessToken().GetByUser(uat.UserId, 0, 100) + require.Nil(t, err2) + require.Equal(t, 1, len(received2), "received incorrect number of tokens after save") - if result, appError := ss.UserAccessToken().GetAll(0, 100); appError != nil { - t.Fatal(appError) - } else if len(result) != 1 { - t.Fatal("received incorrect number of tokens after save") - } + result2, appError := ss.UserAccessToken().GetAll(0, 100) + require.Nil(t, appError) + require.Equal(t, 1, len(result2), "received incorrect number of tokens after save") - if err = ss.UserAccessToken().Delete(uat.Id); err != nil { - t.Fatal(err) - } + err = ss.UserAccessToken().Delete(uat.Id) + require.Nil(t, err) - if _, err = ss.Session().Get(s1.Token); err == nil { - t.Fatal("should error - session should be deleted") - } + _, err = ss.Session().Get(s1.Token) + require.NotNil(t, err, "should error - session should be deleted") - if _, err = ss.UserAccessToken().GetByToken(s1.Token); err == nil { - t.Fatal("should error - access token should be deleted") - } + _, err = ss.UserAccessToken().GetByToken(s1.Token) + require.NotNil(t, err, "should error - access token should be deleted") s2 := &model.Session{} s2.UserId = uat.UserId @@ -82,21 +69,17 @@ func testUserAccessTokenSaveGetDelete(t *testing.T, ss store.Store) { s2, err = ss.Session().Save(s2) require.Nil(t, err) - if _, err = ss.UserAccessToken().Save(uat); err != nil { - t.Fatal(err) - } + _, err = ss.UserAccessToken().Save(uat) + require.Nil(t, err) - if err := ss.UserAccessToken().DeleteAllForUser(uat.UserId); err != nil { - t.Fatal(err) - } + err = ss.UserAccessToken().DeleteAllForUser(uat.UserId) + require.Nil(t, err) - if _, err := ss.Session().Get(s2.Token); err == nil { - t.Fatal("should error - session should be deleted") - } + _, err = ss.Session().Get(s2.Token) + require.NotNil(t, err, "should error - session should be deleted") - if _, err := ss.UserAccessToken().GetByToken(s2.Token); err == nil { - t.Fatal("should error - access token should be deleted") - } + _, err = ss.UserAccessToken().GetByToken(s2.Token) + require.NotNil(t, err, "should error - access token should be deleted") } func testUserAccessTokenDisableEnable(t *testing.T, ss store.Store) { @@ -113,17 +96,14 @@ func testUserAccessTokenDisableEnable(t *testing.T, ss store.Store) { s1, err := ss.Session().Save(s1) require.Nil(t, err) - if _, err = ss.UserAccessToken().Save(uat); err != nil { - t.Fatal(err) - } + _, err = ss.UserAccessToken().Save(uat) + require.Nil(t, err) - if err = ss.UserAccessToken().UpdateTokenDisable(uat.Id); err != nil { - t.Fatal(err) - } + err = ss.UserAccessToken().UpdateTokenDisable(uat.Id) + require.Nil(t, err) - if _, err = ss.Session().Get(s1.Token); err == nil { - t.Fatal("should error - session should be deleted") - } + _, err = ss.Session().Get(s1.Token) + require.NotNil(t, err, "should error - session should be deleted") s2 := &model.Session{} s2.UserId = uat.UserId @@ -132,9 +112,8 @@ func testUserAccessTokenDisableEnable(t *testing.T, ss store.Store) { s2, err = ss.Session().Save(s2) require.Nil(t, err) - if err = ss.UserAccessToken().UpdateTokenEnable(uat.Id); err != nil { - t.Fatal(err) - } + err = ss.UserAccessToken().UpdateTokenEnable(uat.Id) + require.Nil(t, err) } func testUserAccessTokenSearch(t *testing.T, ss store.Store) { @@ -158,25 +137,19 @@ func testUserAccessTokenSearch(t *testing.T, ss store.Store) { s1, err = ss.Session().Save(s1) require.Nil(t, err) - if _, err = ss.UserAccessToken().Save(uat); err != nil { - t.Fatal(err) - } + _, err = ss.UserAccessToken().Save(uat) + require.Nil(t, err) - if received, err := ss.UserAccessToken().Search(uat.Id); err != nil { - t.Fatal(err) - } else if len(received) != 1 { - t.Fatal("received incorrect number of tokens after search") - } + received, err := ss.UserAccessToken().Search(uat.Id) + require.Nil(t, err) - if received, err := ss.UserAccessToken().Search(uat.UserId); err != nil { - t.Fatal(err) - } else if len(received) != 1 { - t.Fatal("received incorrect number of tokens after search") - } + require.Equal(t, 1, len(received), "received incorrect number of tokens after search") - if received, err := ss.UserAccessToken().Search(u1.Username); err != nil { - t.Fatal(err) - } else if len(received) != 1 { - t.Fatal("received incorrect number of tokens after search") - } + received, err = ss.UserAccessToken().Search(uat.UserId) + require.Nil(t, err) + require.Equal(t, 1, len(received), "received incorrect number of tokens after search") + + received, err = ss.UserAccessToken().Search(u1.Username) + require.Nil(t, err) + require.Equal(t, 1, len(received), "received incorrect number of tokens after search") } diff --git a/store/storetest/webhook_store.go b/store/storetest/webhook_store.go index d9aff7048b..810a37224b 100644 --- a/store/storetest/webhook_store.go +++ b/store/storetest/webhook_store.go @@ -228,7 +228,7 @@ func TestWebhookStoreGetIncomingByTeamByUser(t *testing.T, ss store.Store) { }) } -func testWebhookStoreGetIncomingByChannel(t *testing.T, ss store.Store) { +func TestWebhookStoreGetIncomingByChannel(t *testing.T, ss store.Store) { o1 := buildIncomingWebhook() o1, err := ss.Webhook().SaveIncoming(o1) diff --git a/store/timer_layer.go b/store/timer_layer.go index b4983eab0f..8259cc370e 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -2776,6 +2776,23 @@ func (s *TimerLayerGroupStore) GetByIDs(groupIDs []string) ([]*model.Group, *mod return resultVar0, resultVar1 } +func (s *TimerLayerGroupStore) GetByName(name string) (*model.Group, *model.AppError) { + start := timemodule.Now() + + resultVar0, resultVar1 := s.GroupStore.GetByName(name) + + t := timemodule.Now() + elapsed := t.Sub(start) + if s.Root.Metrics != nil { + success := "false" + if resultVar1 == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("GroupStore.GetByName", success, float64(elapsed)) + } + return resultVar0, resultVar1 +} + func (s *TimerLayerGroupStore) GetByRemoteID(remoteID string, groupSource model.GroupSource) (*model.Group, *model.AppError) { start := timemodule.Now() @@ -2792,6 +2809,23 @@ func (s *TimerLayerGroupStore) GetByRemoteID(remoteID string, groupSource model. return resultVar0, resultVar1 } +func (s *TimerLayerGroupStore) GetByUser(userId string) ([]*model.Group, *model.AppError) { + start := timemodule.Now() + + resultVar0, resultVar1 := s.GroupStore.GetByUser(userId) + + t := timemodule.Now() + elapsed := t.Sub(start) + if s.Root.Metrics != nil { + success := "false" + if resultVar1 == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("GroupStore.GetByUser", success, float64(elapsed)) + } + return resultVar0, resultVar1 +} + func (s *TimerLayerGroupStore) GetGroupSyncable(groupID string, syncableID string, syncableType model.GroupSyncableType) (*model.GroupSyncable, *model.AppError) { start := timemodule.Now() diff --git a/testlib/resources.go b/testlib/resources.go index 8f058df58d..698e20197f 100644 --- a/testlib/resources.go +++ b/testlib/resources.go @@ -84,7 +84,7 @@ func getTestResourcesToSetup() []testResourceDetails { testResourcesToSetup[i].src = srcPath } else if testResource.resType == resourceTypeFolder { srcPath, found = findDir(testResource.src) - if found == false { + if !found { panic(fmt.Sprintf("Failed to find folder %s", testResource.src)) } diff --git a/utils/emoji.go b/utils/emoji.go index d7e8a8b126..6797e5e0a2 100644 --- a/utils/emoji.go +++ b/utils/emoji.go @@ -11,14 +11,15 @@ import ( "image/jpeg" "image/png" "testing" + + "github.com/stretchr/testify/require" ) func CreateTestGif(t *testing.T, width int, height int) []byte { var buffer bytes.Buffer - if err := gif.Encode(&buffer, image.NewRGBA(image.Rect(0, 0, width, height)), nil); err != nil { - t.Fatalf("failed to create gif: %v", err.Error()) - } + err := gif.Encode(&buffer, image.NewRGBA(image.Rect(0, 0, width, height)), nil) + require.NoErrorf(t, err, "failed to create gif: %v", err) return buffer.Bytes() } @@ -34,9 +35,8 @@ func CreateTestAnimatedGif(t *testing.T, width int, height int, frames int) []by img.Image[i] = image.NewPaletted(image.Rect(0, 0, width, height), color.Palette{color.Black}) img.Delay[i] = 0 } - if err := gif.EncodeAll(&buffer, &img); err != nil { - t.Fatalf("failed to create animated gif: %v", err.Error()) - } + err := gif.EncodeAll(&buffer, &img) + require.NoErrorf(t, err, "failed to create animated gif: %v", err) return buffer.Bytes() } @@ -44,9 +44,8 @@ func CreateTestAnimatedGif(t *testing.T, width int, height int, frames int) []by func CreateTestJpeg(t *testing.T, width int, height int) []byte { var buffer bytes.Buffer - if err := jpeg.Encode(&buffer, image.NewRGBA(image.Rect(0, 0, width, height)), nil); err != nil { - t.Fatalf("failed to create jpeg: %v", err.Error()) - } + err := jpeg.Encode(&buffer, image.NewRGBA(image.Rect(0, 0, width, height)), nil) + require.NoErrorf(t, err, "failed to create jpeg: %v", err) return buffer.Bytes() } @@ -54,9 +53,8 @@ func CreateTestJpeg(t *testing.T, width int, height int) []byte { func CreateTestPng(t *testing.T, width int, height int) []byte { var buffer bytes.Buffer - if err := png.Encode(&buffer, image.NewRGBA(image.Rect(0, 0, width, height))); err != nil { - t.Fatalf("failed to create png: %v", err.Error()) - } + err := png.Encode(&buffer, image.NewRGBA(image.Rect(0, 0, width, height))) + require.NoErrorf(t, err, "failed to create png: %v", err) return buffer.Bytes() } diff --git a/utils/imgutils/gif.go b/utils/imgutils/gif.go index 0e48f25ece..48aa988b2f 100644 --- a/utils/imgutils/gif.go +++ b/utils/imgutils/gif.go @@ -32,7 +32,6 @@ type reader interface { const ( // Fields. fColorTable = 1 << 7 - fInterlace = 1 << 6 fColorTableBitsMask = 7 // Graphic control flags. diff --git a/utils/merge_test.go b/utils/merge_test.go index a002a33797..a2b5af7e57 100644 --- a/utils/merge_test.go +++ b/utils/merge_test.go @@ -513,7 +513,8 @@ func TestMergeWithSlices(t *testing.T) { assert.Equal(t, expected, merged) // of course this won't change merged, even if it did copy... but just in case. m2 = append(m2, "test") - assert.Equal(t, 0, len(merged)) + assert.Len(t, m2, 1) + assert.Len(t, merged, 0) }) t.Run("slice is not copied. change in patch will not affect merged", func(t *testing.T) { diff --git a/utils/password_test.go b/utils/password_test.go index e0d120eed4..f5ef44f81d 100644 --- a/utils/password_test.go +++ b/utils/password_test.go @@ -19,10 +19,10 @@ func TestIsPasswordValidWithSettings(t *testing.T) { Password: strings.Repeat("x", 3), Settings: &model.PasswordSettings{ MinimumLength: model.NewInt(3), - Lowercase: model.NewBool(false), - Uppercase: model.NewBool(false), - Number: model.NewBool(false), - Symbol: model.NewBool(false), + Lowercase: model.NewBool(false), + Uppercase: model.NewBool(false), + Number: model.NewBool(false), + Symbol: model.NewBool(false), }, }, "Long": { @@ -30,28 +30,28 @@ func TestIsPasswordValidWithSettings(t *testing.T) { Settings: &model.PasswordSettings{ Lowercase: model.NewBool(false), Uppercase: model.NewBool(false), - Number: model.NewBool(false), - Symbol: model.NewBool(false), + Number: model.NewBool(false), + Symbol: model.NewBool(false), }, }, "TooShort": { Password: strings.Repeat("x", 2), Settings: &model.PasswordSettings{ MinimumLength: model.NewInt(3), - Lowercase: model.NewBool(false), - Uppercase: model.NewBool(false), - Number: model.NewBool(false), - Symbol: model.NewBool(false), + Lowercase: model.NewBool(false), + Uppercase: model.NewBool(false), + Number: model.NewBool(false), + Symbol: model.NewBool(false), }, ExpectedError: "model.user.is_valid.pwd.app_error", }, "TooLong": { - Password: strings.Repeat("x", model.PASSWORD_MAXIMUM_LENGTH+1), - Settings: &model.PasswordSettings{ + Password: strings.Repeat("x", model.PASSWORD_MAXIMUM_LENGTH+1), + Settings: &model.PasswordSettings{ Lowercase: model.NewBool(false), Uppercase: model.NewBool(false), - Number: model.NewBool(false), - Symbol: model.NewBool(false), + Number: model.NewBool(false), + Symbol: model.NewBool(false), }, ExpectedError: "model.user.is_valid.pwd.app_error", }, @@ -60,8 +60,8 @@ func TestIsPasswordValidWithSettings(t *testing.T) { Settings: &model.PasswordSettings{ Lowercase: model.NewBool(true), Uppercase: model.NewBool(false), - Number: model.NewBool(false), - Symbol: model.NewBool(false), + Number: model.NewBool(false), + Symbol: model.NewBool(false), }, ExpectedError: "model.user.is_valid.pwd_lowercase.app_error", }, @@ -70,28 +70,28 @@ func TestIsPasswordValidWithSettings(t *testing.T) { Settings: &model.PasswordSettings{ Uppercase: model.NewBool(true), Lowercase: model.NewBool(false), - Number: model.NewBool(false), - Symbol: model.NewBool(false), + Number: model.NewBool(false), + Symbol: model.NewBool(false), }, ExpectedError: "model.user.is_valid.pwd_uppercase.app_error", }, "MissingNumber": { Password: "asasdasdsadASD!@#", Settings: &model.PasswordSettings{ - Number: model.NewBool(true), + Number: model.NewBool(true), Lowercase: model.NewBool(false), Uppercase: model.NewBool(false), - Symbol: model.NewBool(false), + Symbol: model.NewBool(false), }, ExpectedError: "model.user.is_valid.pwd_number.app_error", }, "MissingSymbol": { Password: "asdasdasdasdasdASD123", Settings: &model.PasswordSettings{ - Symbol: model.NewBool(true), + Symbol: model.NewBool(true), Lowercase: model.NewBool(false), Uppercase: model.NewBool(false), - Number: model.NewBool(false), + Number: model.NewBool(false), }, ExpectedError: "model.user.is_valid.pwd_symbol.app_error", }, diff --git a/utils/subpath_test.go b/utils/subpath_test.go index 0c76bd7368..ceb063fc40 100644 --- a/utils/subpath_test.go +++ b/utils/subpath_test.go @@ -281,8 +281,6 @@ const newSubpathRootHtml = ` Mattermost

Cannot connect to Mattermost

We're having trouble connecting to Mattermost. If refreshing this page (Ctrl+R or Command+R) does not work, please verify that your computer is connected to the internet.


` - const baseManifestJson = `{ "icons": [ { diff --git a/utils/test_files_compiler.go b/utils/test_files_compiler.go index b58ad98bd2..6bf6ad05f0 100644 --- a/utils/test_files_compiler.go +++ b/utils/test_files_compiler.go @@ -10,21 +10,11 @@ import ( "os/exec" "path/filepath" "runtime" - "strings" "testing" "github.com/stretchr/testify/require" ) -func goMod(t *testing.T, dir string, args ...string) { - cmd := exec.Command("go", append([]string{"mod"}, args...)...) - cmd.Dir = dir - output, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("Failed to %s: %s", strings.Join(args, " "), string(output)) - } -} - func CompileGo(t *testing.T, sourceCode, outputPath string) { dir, err := ioutil.TempDir(".", "") require.NoError(t, err) diff --git a/utils/time_test.go b/utils/time_test.go index 451ceb5aa3..8f151ccc2e 100644 --- a/utils/time_test.go +++ b/utils/time_test.go @@ -6,6 +6,8 @@ package utils import ( "testing" "time" + + "github.com/stretchr/testify/assert" ) var format = "2006-01-02 15:04:05.000000000" @@ -15,18 +17,16 @@ func TestMillisFromTime(t *testing.T) { actual := MillisFromTime(input) expected := int64(1420115640000) - if actual != expected { - t.Fatalf("TestMillisFromTime failed, %v=%v", expected, actual) - } + assert.Equal(t, expected, actual) } func TestYesterday(t *testing.T) { actual := Yesterday() expected := time.Now().AddDate(0, 0, -1) - if actual.Year() != expected.Year() || actual.Day() != expected.Day() || actual.Month() != expected.Month() { - t.Fatalf("TestYesterday failed, %v=%v", expected, actual) - } + assert.Equal(t, expected.Year(), actual.Year()) + assert.Equal(t, expected.Day(), actual.Day()) + assert.Equal(t, expected.Month(), actual.Month()) } func TestStartOfDay(t *testing.T) { @@ -34,9 +34,7 @@ func TestStartOfDay(t *testing.T) { actual := StartOfDay(input) expected, _ := time.Parse(format, "2015-01-01 00:00:00.000000000") - if actual != expected { - t.Fatalf("TestStartOfDay failed, %v=%v", expected, actual) - } + assert.Equal(t, expected, actual) } func TestEndOfDay(t *testing.T) { @@ -44,7 +42,5 @@ func TestEndOfDay(t *testing.T) { actual := EndOfDay(input) expected, _ := time.Parse(format, "2015-01-01 23:59:59.999999999") - if actual != expected { - t.Fatalf("TestEndOfDay failed, %v=%v", expected, actual) - } + assert.Equal(t, expected, actual) } diff --git a/utils/utils_test.go b/utils/utils_test.go index 32c7b46acd..b33868b345 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -23,13 +23,8 @@ func TestStringArrayIntersection(t *testing.T) { "def", } - if len(StringArrayIntersection(a, b)) != 0 { - t.Fatal("should be 0") - } - - if len(StringArrayIntersection(a, c)) != 1 { - t.Fatal("should be 1") - } + assert.Len(t, StringArrayIntersection(a, b), 0) + assert.Len(t, StringArrayIntersection(a, c), 1) } func TestRemoveDuplicatesFromStringArray(t *testing.T) { @@ -43,9 +38,7 @@ func TestRemoveDuplicatesFromStringArray(t *testing.T) { "a", } - if len(RemoveDuplicatesFromStringArray(a)) != 3 { - t.Fatal("should be 3") - } + assert.Len(t, RemoveDuplicatesFromStringArray(a), 3) } func TestStringSliceDiff(t *testing.T) { @@ -53,7 +46,7 @@ func TestStringSliceDiff(t *testing.T) { b := []string{"two", "seven", "four", "six"} expected := []string{"one", "three", "five"} - assert.Equal(t, StringSliceDiff(a, b), expected) + assert.Equal(t, expected, StringSliceDiff(a, b)) } func TestGetIpAddress(t *testing.T) { diff --git a/web/handlers.go b/web/handlers.go index a666501f6d..ea2885ba13 100644 --- a/web/handlers.go +++ b/web/handlers.go @@ -72,14 +72,16 @@ type Handler struct { func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { now := time.Now() - mlog.Debug("request:", mlog.String("method", r.Method), mlog.String("url", r.URL.Path)) + + requestID := model.NewId() + mlog.Debug("Received HTTP request", mlog.String("method", r.Method), mlog.String("url", r.URL.Path), mlog.String("request_id", requestID)) c := &Context{} c.App = app.New( h.GetGlobalAppOptions()..., ) c.App.T, _ = utils.GetTranslationsAndLocale(w, r) - c.App.RequestId = model.NewId() + c.App.RequestId = requestID c.App.IpAddress = utils.GetIpAddress(r, c.App.Config().ServiceSettings.TrustedProxyIPHeader) c.App.UserAgent = r.UserAgent() c.App.AcceptLanguage = r.Header.Get("Accept-Language") diff --git a/web/unsupported_browser.go b/web/unsupported_browser.go index 8e5f09d64e..db02ab8a39 100644 --- a/web/unsupported_browser.go +++ b/web/unsupported_browser.go @@ -127,7 +127,7 @@ func renderBrowserFirefox(app *app.App) Browser { "/static/images/browser-icons/firefox.svg", app.T("web.error.unsupported_browser.browser_title.firefox"), app.T("web.error.unsupported_browser.min_browser_version.firefox"), - "https://www.mozilla.org/en-CA/firefox/new/", + "https://www.mozilla.org/firefox/new/", app.T("web.error.unsupported_browser.browser_get_latest.firefox"), } }