diff options
28 files changed, 342 insertions, 445 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index af8fcfc8f..fb640da3d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,6 +1,6 @@ # Code Contribution Guidelines -Thank you for your interest in contributing! Please see the [Mattermost Contribution Guide](http://docs.mattermost.com/developer/contribution-guide.html) which describes the process for making code contributions across Mattermost projects and [join our "Contributors" community channel](https://pre-release.mattermost.com/core/channels/tickets) to ask questions from community members and the Mattermost core team. +Thank you for your interest in contributing! Please see the [Mattermost Contribution Guide](https://developers.mattermost.com/contribute/getting-started/) which describes the process for making code contributions across Mattermost projects and [join our "Contributors" community channel](https://pre-release.mattermost.com/core/channels/tickets) to ask questions from community members and the Mattermost core team. ### Review Process for this Repo @@ -275,15 +275,15 @@ gofmt: ## Runs gofmt against all packages. @echo "gofmt success"; \ store-mocks: ## Creates mock files. - go get github.com/vektra/mockery/... + go get -u github.com/vektra/mockery/... $(GOPATH)/bin/mockery -dir store -all -output store/storetest/mocks -note 'Regenerate this file using `make store-mocks`.' ldap-mocks: ## Creates mock files for ldap. - go get github.com/vektra/mockery/... + go get -u github.com/vektra/mockery/... $(GOPATH)/bin/mockery -dir enterprise/ldap -all -output enterprise/ldap/mocks -note 'Regenerate this file using `make ldap-mocks`.' plugin-mocks: ## Creates mock files for plugins. - go get github.com/vektra/mockery/... + go get -u github.com/vektra/mockery/... $(GOPATH)/bin/mockery -dir plugin -name API -output plugin/plugintest -outpkg plugintest -case underscore -note 'Regenerate this file using `make plugin-mocks`.' $(GOPATH)/bin/mockery -dir plugin -name Hooks -output plugin/plugintest -outpkg plugintest -case underscore -note 'Regenerate this file using `make plugin-mocks`.' @@ -5,6 +5,7 @@ Mattermost is an open source, private cloud, Slack-alternative from [https://mat It's written in Golang and React and runs as a single Linux binary with MySQL or PostgreSQL. Every month on the 16th [a new compiled version is released under an MIT license](https://www.mattermost.org/download/). - [Review product documentation](http://docs.mattermost.com/). +- [Review developer documentation](http://developers.mattermost.com/). - [Download compiled version](https://mattermost.org/download). <img width="1006" alt="screenshot at nov 29 14-11-32" src="https://user-images.githubusercontent.com/29708087/33394101-404e23e4-d50f-11e7-8fe5-99d4802a9768.png"> diff --git a/api4/system_test.go b/api4/system_test.go index 099c193d0..205572cf6 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -10,6 +10,7 @@ import ( "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetPing(t *testing.T) { @@ -47,9 +48,7 @@ func TestGetConfig(t *testing.T) { cfg, resp := th.SystemAdminClient.GetConfig() CheckNoError(t, resp) - if len(cfg.TeamSettings.SiteName) == 0 { - t.Fatal() - } + require.NotEqual(t, "", cfg.TeamSettings.SiteName) if *cfg.LdapSettings.BindPassword != model.FAKE_SETTING && len(*cfg.LdapSettings.BindPassword) != 0 { t.Fatal("did not sanitize properly") @@ -121,28 +120,14 @@ func TestUpdateConfig(t *testing.T) { cfg, resp = th.SystemAdminClient.UpdateConfig(cfg) CheckNoError(t, resp) - if len(cfg.TeamSettings.SiteName) == 0 { - t.Fatal() - } else { - if cfg.TeamSettings.SiteName != "MyFancyName" { - t.Log("It should update the SiteName") - t.Fatal() - } - } + require.Equal(t, "MyFancyName", cfg.TeamSettings.SiteName, "It should update the SiteName") //Revert the change cfg.TeamSettings.SiteName = SiteName cfg, resp = th.SystemAdminClient.UpdateConfig(cfg) CheckNoError(t, resp) - if len(cfg.TeamSettings.SiteName) == 0 { - t.Fatal() - } else { - if cfg.TeamSettings.SiteName != SiteName { - t.Log("It should update the SiteName") - t.Fatal() - } - } + require.Equal(t, SiteName, cfg.TeamSettings.SiteName, "It should update the SiteName") t.Run("Should not be able to modify PluginSettings.EnableUploads", func(t *testing.T) { oldEnableUploads := *th.App.GetConfig().PluginSettings.EnableUploads diff --git a/cmd/mattermost/commands/roles_test.go b/cmd/mattermost/commands/roles_test.go index 4f11ce7ed..da33a73cc 100644 --- a/cmd/mattermost/commands/roles_test.go +++ b/cmd/mattermost/commands/roles_test.go @@ -17,7 +17,7 @@ func TestAssignRole(t *testing.T) { CheckCommand(t, "roles", "system_admin", th.BasicUser.Email) if result := <-th.App.Srv.Store.User().GetByEmail(th.BasicUser.Email); result.Err != nil { - t.Fatal() + t.Fatal(result.Err) } else { user := result.Data.(*model.User) if user.Roles != "system_user system_admin" { @@ -28,7 +28,7 @@ func TestAssignRole(t *testing.T) { CheckCommand(t, "roles", "member", th.BasicUser.Email) if result := <-th.App.Srv.Store.User().GetByEmail(th.BasicUser.Email); result.Err != nil { - t.Fatal() + t.Fatal(result.Err) } else { user := result.Data.(*model.User) if user.Roles != "system_user" { diff --git a/cmd/mattermost/commands/user_test.go b/cmd/mattermost/commands/user_test.go index 69ca9ecb8..088893602 100644 --- a/cmd/mattermost/commands/user_test.go +++ b/cmd/mattermost/commands/user_test.go @@ -50,12 +50,10 @@ func TestCreateUserWithoutTeam(t *testing.T) { CheckCommand(t, "user", "create", "--email", email, "--password", "mypassword1", "--username", username) if result := <-th.App.Srv.Store.User().GetByEmail(email); result.Err != nil { - t.Fatal() + t.Fatal(result.Err) } else { user := result.Data.(*model.User) - if user.Email != email { - t.Fatal() - } + require.Equal(t, email, user.Email) } } @@ -92,7 +90,7 @@ func TestChangeUserEmail(t *testing.T) { t.Fatal("should've updated to the new email") } if result := <-th.App.Srv.Store.User().GetByEmail(newEmail); result.Err != nil { - t.Fatal() + t.Fatal(result.Err) } else { user := result.Data.(*model.User) if user.Email != newEmail { diff --git a/model/access_test.go b/model/access_test.go index f0ed2da77..0f124a107 100644 --- a/model/access_test.go +++ b/model/access_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestAccessJson(t *testing.T) { @@ -26,9 +28,7 @@ func TestAccessJson(t *testing.T) { func TestAccessIsValid(t *testing.T) { ad := AccessData{} - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.ClientId = NewRandomString(28) if err := ad.IsValid(); err == nil { @@ -41,9 +41,7 @@ func TestAccessIsValid(t *testing.T) { } ad.ClientId = NewId() - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.UserId = NewRandomString(28) if err := ad.IsValid(); err == nil { @@ -66,9 +64,7 @@ func TestAccessIsValid(t *testing.T) { } ad.Token = NewId() - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.RefreshToken = NewRandomString(28) if err := ad.IsValid(); err == nil { @@ -76,9 +72,7 @@ func TestAccessIsValid(t *testing.T) { } ad.RefreshToken = NewId() - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.RedirectUri = "" if err := ad.IsValid(); err == nil { diff --git a/model/authorize_test.go b/model/authorize_test.go index 81e059305..0775b06c1 100644 --- a/model/authorize_test.go +++ b/model/authorize_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestAuthJson(t *testing.T) { @@ -46,9 +48,7 @@ func TestAuthIsValid(t *testing.T) { ad := AuthData{} - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.ClientId = NewRandomString(28) if err := ad.IsValid(); err == nil { @@ -56,9 +56,7 @@ func TestAuthIsValid(t *testing.T) { } ad.ClientId = NewId() - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.UserId = NewRandomString(28) if err := ad.IsValid(); err == nil { @@ -66,9 +64,7 @@ func TestAuthIsValid(t *testing.T) { } ad.UserId = NewId() - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.Code = NewRandomString(129) if err := ad.IsValid(); err == nil { @@ -81,9 +77,7 @@ func TestAuthIsValid(t *testing.T) { } ad.Code = NewId() - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.ExpiresIn = 0 if err := ad.IsValid(); err == nil { @@ -91,9 +85,7 @@ func TestAuthIsValid(t *testing.T) { } ad.ExpiresIn = 1 - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.CreateAt = 0 if err := ad.IsValid(); err == nil { @@ -101,9 +93,7 @@ func TestAuthIsValid(t *testing.T) { } ad.CreateAt = 1 - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.State = NewRandomString(129) if err := ad.IsValid(); err == nil { @@ -121,9 +111,7 @@ func TestAuthIsValid(t *testing.T) { } ad.Scope = NewRandomString(128) - if err := ad.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, ad.IsValid()) ad.RedirectUri = "" if err := ad.IsValid(); err == nil { diff --git a/model/cluster_message_test.go b/model/cluster_message_test.go index 38603e577..e9225a5c5 100644 --- a/model/cluster_message_test.go +++ b/model/cluster_message_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestClusterMessage(t *testing.T) { @@ -17,9 +19,7 @@ func TestClusterMessage(t *testing.T) { json := m.ToJson() result := ClusterMessageFromJson(strings.NewReader(json)) - if result.Data != "hello" { - t.Fatal() - } + require.Equal(t, "hello", result.Data) badresult := ClusterMessageFromJson(strings.NewReader("junk")) if badresult != nil { diff --git a/model/compliance_post_test.go b/model/compliance_post_test.go index ff159ef1b..21044c128 100644 --- a/model/compliance_post_test.go +++ b/model/compliance_post_test.go @@ -5,25 +5,20 @@ package model import ( "testing" + + "github.com/stretchr/testify/require" ) func TestCompliancePostHeader(t *testing.T) { - if CompliancePostHeader()[0] != "TeamName" { - t.Fatal() - } + require.Equal(t, "TeamName", CompliancePostHeader()[0]) } func TestCompliancePost(t *testing.T) { o := CompliancePost{TeamName: "test", PostFileIds: "files", PostCreateAt: GetMillis()} r := o.Row() - if r[0] != "test" { - t.Fatal() - } - - if r[len(r)-1] != "files" { - t.Fatal() - } + require.Equal(t, "test", r[0]) + require.Equal(t, "files", r[len(r)-1]) } var cleanTests = []struct { diff --git a/model/emoji_test.go b/model/emoji_test.go index 50d741214..4539db873 100644 --- a/model/emoji_test.go +++ b/model/emoji_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestEmojiIsValid(t *testing.T) { @@ -23,61 +25,39 @@ func TestEmojiIsValid(t *testing.T) { } emoji.Id = "1234" - if err := emoji.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, emoji.IsValid()) emoji.Id = NewId() emoji.CreateAt = 0 - if err := emoji.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, emoji.IsValid()) emoji.CreateAt = 1234 emoji.UpdateAt = 0 - if err := emoji.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, emoji.IsValid()) emoji.UpdateAt = 1234 emoji.CreatorId = strings.Repeat("1", 27) - if err := emoji.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, emoji.IsValid()) emoji.CreatorId = NewId() emoji.Name = strings.Repeat("1", 65) - if err := emoji.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, emoji.IsValid()) emoji.Name = "" - if err := emoji.IsValid(); err == nil { - t.Fatal(err) - } + require.NotNil(t, emoji.IsValid()) emoji.Name = strings.Repeat("1", 64) - if err := emoji.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, emoji.IsValid()) emoji.Name = "name-" - if err := emoji.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, emoji.IsValid()) emoji.Name = "name_" - if err := emoji.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, emoji.IsValid()) emoji.Name = "name:" - if err := emoji.IsValid(); err == nil { - t.Fatal(err) - } + require.NotNil(t, emoji.IsValid()) emoji.Name = "croissant" - if err := emoji.IsValid(); err == nil { - t.Fatal(err) - } + require.NotNil(t, emoji.IsValid()) } diff --git a/model/oauth_test.go b/model/oauth_test.go index 5c0547717..cbed8a633 100644 --- a/model/oauth_test.go +++ b/model/oauth_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestOAuthAppJson(t *testing.T) { @@ -52,52 +54,32 @@ func TestOAuthAppPreUpdate(t *testing.T) { func TestOAuthAppIsValid(t *testing.T) { app := OAuthApp{} - if err := app.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, app.IsValid()) app.Id = NewId() - if err := app.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, app.IsValid()) app.CreateAt = 1 - if err := app.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, app.IsValid()) app.UpdateAt = 1 - if err := app.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, app.IsValid()) app.CreatorId = NewId() - if err := app.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, app.IsValid()) app.ClientSecret = NewId() - if err := app.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, app.IsValid()) app.Name = "TestOAuthApp" - if err := app.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, app.IsValid()) app.CallbackUrls = []string{"https://nowhere.com"} - if err := app.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, app.IsValid()) app.Homepage = "https://nowhere.com" - if err := app.IsValid(); err != nil { - t.Fatal() - } + require.Nil(t, app.IsValid()) app.IconURL = "https://nowhere.com/icon_image.png" - if err := app.IsValid(); err != nil { - t.Fatal() - } + require.Nil(t, app.IsValid()) } diff --git a/model/preference_test.go b/model/preference_test.go index c56d46e2c..d4035125a 100644 --- a/model/preference_test.go +++ b/model/preference_test.go @@ -7,6 +7,8 @@ import ( "encoding/json" "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestPreferenceIsValid(t *testing.T) { @@ -16,54 +18,34 @@ func TestPreferenceIsValid(t *testing.T) { Name: NewId(), } - if err := preference.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, preference.IsValid()) preference.UserId = NewId() - if err := preference.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, preference.IsValid()) preference.Category = strings.Repeat("01234567890", 20) - if err := preference.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, preference.IsValid()) preference.Category = PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW - if err := preference.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, preference.IsValid()) preference.Name = strings.Repeat("01234567890", 20) - if err := preference.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, preference.IsValid()) preference.Name = NewId() - if err := preference.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, preference.IsValid()) preference.Value = strings.Repeat("01234567890", 201) - if err := preference.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, preference.IsValid()) preference.Value = "1234garbage" - if err := preference.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, preference.IsValid()) preference.Category = PREFERENCE_CATEGORY_THEME - if err := preference.IsValid(); err == nil { - t.Fatal() - } + require.NotNil(t, preference.IsValid()) preference.Value = `{"color": "#ff0000", "color2": "#faf"}` - if err := preference.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, preference.IsValid()) } func TestPreferencePreUpdate(t *testing.T) { @@ -79,11 +61,9 @@ func TestPreferencePreUpdate(t *testing.T) { t.Fatal(err) } - if props["color"] != "#ff0000" || props["color2"] != "#faf" || props["codeTheme"] != "github" { - t.Fatal("shouldn't have changed valid props") - } + require.Equal(t, "#ff0000", props["color"], "shouldn't have changed valid props") + require.Equal(t, "#faf", props["color2"], "shouldn't have changed valid props") + require.Equal(t, "github", props["codeTheme"], "shouldn't have changed valid props") - if props["invalid"] == "invalid" { - t.Fatal("should have changed invalid prop") - } + require.NotEqual(t, "invalid", props["invalid"], "should have changed invalid prop") } diff --git a/model/user_test.go b/model/user_test.go index b3aaad522..f86b52919 100644 --- a/model/user_test.go +++ b/model/user_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestPasswordHash(t *testing.T) { @@ -117,41 +118,34 @@ func TestUserIsValid(t *testing.T) { } user.Id = NewId() - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "create_at", user.Id) { - t.Fatal(err) - } + err := user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "create_at", user.Id), "expected user is valid error: %s", err.Error()) user.CreateAt = GetMillis() - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "update_at", user.Id) { - t.Fatal(err) - } + err = user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "update_at", user.Id), "expected user is valid error: %s", err.Error()) user.UpdateAt = GetMillis() - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "username", user.Id) { - t.Fatal(err) - } + err = user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "username", user.Id), "expected user is valid error: %s", err.Error()) user.Username = NewId() + "^hello#" - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "username", user.Id) { - t.Fatal(err) - } + err = user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "username", user.Id), "expected user is valid error: %s", err.Error()) user.Username = NewId() - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "email", user.Id) { - t.Fatal(err) - } + err = user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "email", user.Id), "expected user is valid error: %s", err.Error()) user.Email = strings.Repeat("01234567890", 20) - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "email", user.Id) { - t.Fatal(err) - } + err = user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "email", user.Id), "expected user is valid error: %s", err.Error()) user.Email = "user@example.com" user.Nickname = strings.Repeat("a", 65) - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "nickname", user.Id) { - t.Fatal(err) - } + err = user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "nickname", user.Id), "expected user is valid error: %s", err.Error()) user.Nickname = strings.Repeat("a", 64) if err := user.IsValid(); err != nil { @@ -331,28 +325,10 @@ func TestCleanUsername(t *testing.T) { } func TestRoles(t *testing.T) { - - if !IsValidUserRoles("team_user") { - t.Fatal() - } - - if IsValidUserRoles("system_admin") { - t.Fatal() - } - - if !IsValidUserRoles("system_user system_admin") { - t.Fatal() - } - - if IsInRole("system_admin junk", "admin") { - t.Fatal() - } - - if !IsInRole("system_admin junk", "system_admin") { - t.Fatal() - } - - if IsInRole("admin", "system_admin") { - t.Fatal() - } + require.True(t, IsValidUserRoles("team_user")) + require.False(t, IsValidUserRoles("system_admin")) + require.True(t, IsValidUserRoles("system_user system_admin")) + require.False(t, IsInRole("system_admin junk", "admin")) + require.True(t, IsInRole("system_admin junk", "system_admin")) + require.False(t, IsInRole("admin", "system_admin")) } diff --git a/model/utils.go b/model/utils.go index 0d8d359a6..574f43e06 100644 --- a/model/utils.go +++ b/model/utils.go @@ -263,7 +263,7 @@ func GetServerIpAddress() string { } else { for _, addr := range addrs { - if ip, ok := addr.(*net.IPNet); ok && !ip.IP.IsLoopback() { + if ip, ok := addr.(*net.IPNet); ok && !ip.IP.IsLoopback() && !ip.IP.IsLinkLocalUnicast() && !ip.IP.IsLinkLocalMulticast() { if ip.IP.To4() != nil { return ip.IP.String() } diff --git a/model/utils_test.go b/model/utils_test.go index 9797c7090..d35146b30 100644 --- a/model/utils_test.go +++ b/model/utils_test.go @@ -34,18 +34,14 @@ func TestAppError(t *testing.T) { err := NewAppError("TestAppError", "message", nil, "", http.StatusInternalServerError) json := err.ToJson() rerr := AppErrorFromJson(strings.NewReader(json)) - if err.Message != rerr.Message { - t.Fatal() - } + require.Equal(t, err.Message, rerr.Message) t.Log(err.Error()) } func TestAppErrorJunk(t *testing.T) { rerr := AppErrorFromJson(strings.NewReader("<html><body>This is a broken test</body></html>")) - if "body: <html><body>This is a broken test</body></html>" != rerr.DetailedError { - t.Fatal() - } + require.Equal(t, "body: <html><body>This is a broken test</body></html>", rerr.DetailedError) } func TestCopyStringMap(t *testing.T) { @@ -173,9 +169,7 @@ func TestValidLower(t *testing.T) { func TestEtag(t *testing.T) { etag := Etag("hello", 24) - if len(etag) <= 0 { - t.Fatal() - } + require.NotEqual(t, "", etag) } var hashtags = map[string]string{ diff --git a/model/version_test.go b/model/version_test.go index 869ed8ad0..a06db326c 100644 --- a/model/version_test.go +++ b/model/version_test.go @@ -6,100 +6,59 @@ package model import ( "fmt" "testing" + + "github.com/stretchr/testify/require" ) func TestSplitVersion(t *testing.T) { major1, minor1, patch1 := SplitVersion("junk") - if major1 != 0 || minor1 != 0 || patch1 != 0 { - t.Fatal() - } + require.EqualValues(t, 0, major1) + require.EqualValues(t, 0, minor1) + require.EqualValues(t, 0, patch1) major2, minor2, patch2 := SplitVersion("1.2.3") - if major2 != 1 || minor2 != 2 || patch2 != 3 { - t.Fatal() - } + require.EqualValues(t, 1, major2) + require.EqualValues(t, 2, minor2) + require.EqualValues(t, 3, patch2) major3, minor3, patch3 := SplitVersion("1.2") - if major3 != 1 || minor3 != 2 || patch3 != 0 { - t.Fatal() - } + require.EqualValues(t, 1, major3) + require.EqualValues(t, 2, minor3) + require.EqualValues(t, 0, patch3) major4, minor4, patch4 := SplitVersion("1") - if major4 != 1 || minor4 != 0 || patch4 != 0 { - t.Fatal() - } + require.EqualValues(t, 1, major4) + require.EqualValues(t, 0, minor4) + require.EqualValues(t, 0, patch4) major5, minor5, patch5 := SplitVersion("1.2.3.junkgoeswhere") - if major5 != 1 || minor5 != 2 || patch5 != 3 { - t.Fatal() - } + require.EqualValues(t, 1, major5) + require.EqualValues(t, 2, minor5) + require.EqualValues(t, 3, patch5) } func TestGetPreviousVersion(t *testing.T) { - if GetPreviousVersion("1.3.0") != "1.2.0" { - t.Fatal() - } - - if GetPreviousVersion("1.2.1") != "1.1.0" { - t.Fatal() - } - - if GetPreviousVersion("1.1.0") != "1.0.0" { - t.Fatal() - } - - if GetPreviousVersion("1.0.0") != "0.7.0" { - t.Fatal() - } - - if GetPreviousVersion("0.7.1") != "0.6.0" { - t.Fatal() - } - - if GetPreviousVersion("0.5.0") != "" { - t.Fatal() - } + require.Equal(t, "1.2.0", GetPreviousVersion("1.3.0")) + require.Equal(t, "1.1.0", GetPreviousVersion("1.2.1")) + require.Equal(t, "1.0.0", GetPreviousVersion("1.1.0")) + require.Equal(t, "0.7.0", GetPreviousVersion("1.0.0")) + require.Equal(t, "0.6.0", GetPreviousVersion("0.7.1")) + require.Equal(t, "", GetPreviousVersion("0.5.0")) } func TestIsCurrentVersion(t *testing.T) { major, minor, patch := SplitVersion(CurrentVersion) - if !IsCurrentVersion(CurrentVersion) { - t.Fatal() - } - - if !IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major, minor, patch+100)) { - t.Fatal() - } - - if IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major, minor+1, patch)) { - t.Fatal() - } - - if IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major+1, minor, patch)) { - t.Fatal() - } + require.True(t, IsCurrentVersion(CurrentVersion)) + require.True(t, IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major, minor, patch+100))) + require.False(t, IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major, minor+1, patch))) + require.False(t, IsCurrentVersion(fmt.Sprintf("%v.%v.%v", major+1, minor, patch))) } func TestIsPreviousVersionsSupported(t *testing.T) { - - if !IsPreviousVersionsSupported(versionsWithoutHotFixes[0]) { - t.Fatal() - } - - if !IsPreviousVersionsSupported(versionsWithoutHotFixes[1]) { - t.Fatal() - } - - if !IsPreviousVersionsSupported(versionsWithoutHotFixes[2]) { - t.Fatal() - } - - if IsPreviousVersionsSupported(versionsWithoutHotFixes[4]) { - t.Fatal() - } - - if IsPreviousVersionsSupported(versionsWithoutHotFixes[5]) { - t.Fatal() - } + require.True(t, IsPreviousVersionsSupported(versionsWithoutHotFixes[0])) + require.True(t, IsPreviousVersionsSupported(versionsWithoutHotFixes[1])) + require.True(t, IsPreviousVersionsSupported(versionsWithoutHotFixes[2])) + require.False(t, IsPreviousVersionsSupported(versionsWithoutHotFixes[4])) + require.False(t, IsPreviousVersionsSupported(versionsWithoutHotFixes[5])) } diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index cc4e5a0cc..ab3bd202b 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -15,6 +15,7 @@ import ( ) const ( + VERSION_5_3_0 = "5.3.0" VERSION_5_2_0 = "5.2.0" VERSION_5_1_0 = "5.1.0" VERSION_5_0_0 = "5.0.0" @@ -82,6 +83,7 @@ func UpgradeDatabase(sqlStore SqlStore) { UpgradeDatabaseToVersion50(sqlStore) UpgradeDatabaseToVersion51(sqlStore) UpgradeDatabaseToVersion52(sqlStore) + UpgradeDatabaseToVersion53(sqlStore) // If the SchemaVersion is empty this this is the first time it has ran // so lets set it to the current version. @@ -480,3 +482,11 @@ func UpgradeDatabaseToVersion52(sqlStore SqlStore) { saveSchemaVersion(sqlStore, VERSION_5_2_0) } } + +func UpgradeDatabaseToVersion53(sqlStore SqlStore) { + // TODO: Uncomment following condition when version 5.3.0 is released + // if shouldPerformUpgrade(sqlStore, VERSION_5_2_0, VERSION_5_3_0) { + + // saveSchemaVersion(sqlStore, VERSION_5_3_0) + // } +} diff --git a/store/storetest/compliance_store.go b/store/storetest/compliance_store.go index a772f6e44..f7f095a00 100644 --- a/store/storetest/compliance_store.go +++ b/store/storetest/compliance_store.go @@ -10,6 +10,7 @@ import ( "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestComplianceStore(t *testing.T, ss store.Store) { @@ -35,9 +36,8 @@ func testComplianceStore(t *testing.T, ss store.Store) { result := <-c compliances := result.Data.(model.Compliances) - if compliances[0].Status != model.COMPLIANCE_STATUS_RUNNING && compliance2.Id != compliances[0].Id { - t.Fatal() - } + require.Equal(t, model.COMPLIANCE_STATUS_RUNNING, compliances[0].Status) + require.Equal(t, compliance2.Id, compliances[0].Id) compliance2.Status = model.COMPLIANCE_STATUS_FAILED store.Must(ss.Compliance().Update(compliance2)) @@ -46,17 +46,14 @@ func testComplianceStore(t *testing.T, ss store.Store) { result = <-c compliances = result.Data.(model.Compliances) - if compliances[0].Status != model.COMPLIANCE_STATUS_FAILED && compliance2.Id != compliances[0].Id { - t.Fatal() - } + require.Equal(t, model.COMPLIANCE_STATUS_FAILED, compliances[0].Status) + require.Equal(t, compliance2.Id, compliances[0].Id) c = ss.Compliance().GetAll(0, 1) result = <-c compliances = result.Data.(model.Compliances) - if len(compliances) != 1 { - t.Fatal("should only have returned 1") - } + require.Len(t, compliances, 1) c = ss.Compliance().GetAll(1, 1) result = <-c @@ -67,9 +64,7 @@ func testComplianceStore(t *testing.T, ss store.Store) { } rc2 := (<-ss.Compliance().Get(compliance2.Id)).Data.(*model.Compliance) - if rc2.Status != compliance2.Status { - t.Fatal() - } + require.Equal(t, compliance2.Status, rc2.Status) } func testComplianceExport(t *testing.T, ss store.Store) { diff --git a/store/storetest/status_store.go b/store/storetest/status_store.go index b26be4c19..5231bc29a 100644 --- a/store/storetest/status_store.go +++ b/store/storetest/status_store.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" @@ -103,9 +104,7 @@ func testActiveUserCount(t *testing.T, ss store.Store) { t.Fatal(result.Err) } else { count := result.Data.(int64) - if count <= 0 { - t.Fatal() - } + require.True(t, count > 0, "expected count > 0, got %d", count) } } diff --git a/store/storetest/system_store.go b/store/storetest/system_store.go index a06b72a83..6dc1efe41 100644 --- a/store/storetest/system_store.go +++ b/store/storetest/system_store.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" @@ -25,9 +26,7 @@ func testSystemStore(t *testing.T, ss store.Store) { result := <-ss.System().Get() systems := result.Data.(model.StringMap) - if systems[system.Name] != system.Value { - t.Fatal() - } + require.Equal(t, system.Value, systems[system.Name]) system.Value = "value2" store.Must(ss.System().Update(system)) @@ -35,15 +34,11 @@ func testSystemStore(t *testing.T, ss store.Store) { result2 := <-ss.System().Get() systems2 := result2.Data.(model.StringMap) - if systems2[system.Name] != system.Value { - t.Fatal() - } + require.Equal(t, system.Value, systems2[system.Name]) result3 := <-ss.System().GetByName(system.Name) rsystem := result3.Data.(*model.System) - if rsystem.Value != system.Value { - t.Fatal() - } + require.Equal(t, system.Value, rsystem.Value) } func testSystemStoreSaveOrUpdate(t *testing.T, ss store.Store) { diff --git a/store/storetest/team_store.go b/store/storetest/team_store.go index 40d69a2f2..2840f2965 100644 --- a/store/storetest/team_store.go +++ b/store/storetest/team_store.go @@ -590,10 +590,7 @@ func testTeamMembers(t *testing.T, ss store.Store) { t.Fatal(r1.Err) } else { ms := r1.Data.([]*model.TeamMember) - - if len(ms) != 2 { - t.Fatal() - } + require.Len(t, ms, 2) } if r1 := <-ss.Team().GetMembers(teamId2, 0, 100); r1.Err != nil { @@ -601,14 +598,8 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 1 { - t.Fatal() - } - - if ms[0].UserId != m3.UserId { - t.Fatal() - - } + require.Len(t, ms, 1) + require.Equal(t, m3.UserId, ms[0].UserId) } if r1 := <-ss.Team().GetTeamsForUser(m1.UserId); r1.Err != nil { @@ -616,14 +607,8 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 1 { - t.Fatal() - } - - if ms[0].TeamId != m1.TeamId { - t.Fatal() - - } + require.Len(t, ms, 1) + require.Equal(t, m1.TeamId, ms[0].TeamId) } if r1 := <-ss.Team().RemoveMember(teamId1, m1.UserId); r1.Err != nil { @@ -635,14 +620,8 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 1 { - t.Fatal() - } - - if ms[0].UserId != m2.UserId { - t.Fatal() - - } + require.Len(t, ms, 1) + require.Equal(t, m2.UserId, ms[0].UserId) } store.Must(ss.Team().SaveMember(m1, -1)) @@ -656,9 +635,7 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 0 { - t.Fatal() - } + require.Len(t, ms, 0) } uid := model.NewId() @@ -672,9 +649,7 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 2 { - t.Fatal() - } + require.Len(t, ms, 2) } if r1 := <-ss.Team().RemoveAllMembersByUser(uid); r1.Err != nil { @@ -686,9 +661,7 @@ func testTeamMembers(t *testing.T, ss store.Store) { } else { ms := r1.Data.([]*model.TeamMember) - if len(ms) != 0 { - t.Fatal() - } + require.Len(t, ms, 0) } } diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 10fb6a4d9..d1a373f9b 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -246,9 +246,7 @@ func testUserCount(t *testing.T, ss store.Store) { t.Fatal(result.Err) } else { count := result.Data.(int64) - if count <= 0 { - t.Fatal() - } + require.False(t, count <= 0, "expected count > 0, got %d", count) } } diff --git a/utils/markdown/autolink.go b/utils/markdown/autolink.go index 16c40e609..7f7d1117f 100644 --- a/utils/markdown/autolink.go +++ b/utils/markdown/autolink.go @@ -16,27 +16,27 @@ var ( DefaultUrlSchemes = []string{"http", "https", "ftp", "mailto", "tel"} ) -// Given a string with a w at the given position, tries to parse and return a link starting with "www." +// Given a string with a w at the given position, tries to parse and return a range containing a www link. // if one exists. If the text at the given position isn't a link, returns an empty string. Equivalent to // www_match from the reference code. -func parseWWWAutolink(data string, position int) string { +func parseWWWAutolink(data string, position int) (Range, bool) { // Check that this isn't part of another word if position > 1 { prevChar := data[position-1] if !isWhitespaceByte(prevChar) && !isAllowedBeforeWWWLink(prevChar) { - return "" + return Range{}, false } } // Check that this starts with www if len(data)-position < 4 || !regexp.MustCompile(`^www\d{0,3}\.`).MatchString(data[position:]) { - return "" + return Range{}, false } end := checkDomain(data[position:], false) if end == 0 { - return "" + return Range{}, false } end += position @@ -47,12 +47,12 @@ func parseWWWAutolink(data string, position int) string { } // Trim trailing punctuation - link := trimTrailingCharactersFromLink(data[position:end]) - if link == "" { - return "" + end = trimTrailingCharactersFromLink(data, position, end) + if position == end { + return Range{}, false } - return link + return Range{position, end}, true } func isAllowedBeforeWWWLink(c byte) bool { @@ -64,13 +64,13 @@ func isAllowedBeforeWWWLink(c byte) bool { } } -// Given a string with a : at the given position, tried to parse and return a link starting with a URL scheme +// Given a string with a : at the given position, tried to parse and return a range containing a URL scheme // if one exists. If the text around the given position isn't a link, returns an empty string. Equivalent to // url_match from the reference code. -func parseURLAutolink(data string, position int) string { +func parseURLAutolink(data string, position int) (Range, bool) { // Check that a :// exists. This doesn't match the clients that treat the slashes as optional. if len(data)-position < 4 || data[position+1] != '/' || data[position+2] != '/' { - return "" + return Range{}, false } start := position - 1 @@ -81,12 +81,12 @@ func parseURLAutolink(data string, position int) string { // Ensure that the URL scheme is allowed and that at least one character after the scheme is valid. scheme := data[start:position] if !isSchemeAllowed(scheme) || !isValidHostCharacter(data[position+3:]) { - return "" + return Range{}, false } end := checkDomain(data[position+3:], true) if end == 0 { - return "" + return Range{}, false } end += position @@ -97,12 +97,12 @@ func parseURLAutolink(data string, position int) string { } // Trim trailing punctuation - link := trimTrailingCharactersFromLink(data[start:end]) - if link == "" { - return "" + end = trimTrailingCharactersFromLink(data, start, end) + if start == end { + return Range{}, false } - return link + return Range{start, end}, true } func isSchemeAllowed(scheme string) bool { @@ -166,9 +166,9 @@ func isValidHostCharacter(link string) bool { } // Removes any trailing characters such as punctuation or stray brackets that shouldn't be part of the link. -// Equivalent to autolink_delim from the reference code. -func trimTrailingCharactersFromLink(link string) string { - runes := []rune(link) +// Returns a new end position for the link. Equivalent to autolink_delim from the reference code. +func trimTrailingCharactersFromLink(markdown string, start int, end int) int { + runes := []rune(markdown[start:end]) linkEnd := len(runes) // Cut off the link before an open angle bracket if it contains one @@ -240,7 +240,7 @@ func trimTrailingCharactersFromLink(link string) string { } } - return string(runes[:linkEnd]) + return start + len(string(runes[:linkEnd])) } func canEndAutolink(c rune) bool { diff --git a/utils/markdown/autolink_test.go b/utils/markdown/autolink_test.go index d0ea53fa4..997124338 100644 --- a/utils/markdown/autolink_test.go +++ b/utils/markdown/autolink_test.go @@ -134,7 +134,15 @@ func TestParseURLAutolink(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Description, func(t *testing.T) { - assert.Equal(t, testCase.Expected, parseURLAutolink(testCase.Input, testCase.Position)) + rawRange, ok := parseURLAutolink(testCase.Input, testCase.Position) + + if testCase.Expected == "" { + assert.False(t, ok) + assert.Equal(t, Range{0, 0}, rawRange) + } else { + assert.True(t, ok) + assert.Equal(t, testCase.Expected, testCase.Input[rawRange.Position:rawRange.End]) + } }) } } @@ -264,89 +272,153 @@ func TestParseWWWAutolink(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Description, func(t *testing.T) { - assert.Equal(t, testCase.Expected, parseWWWAutolink(testCase.Input, testCase.Position)) + rawRange, ok := parseWWWAutolink(testCase.Input, testCase.Position) + + if testCase.Expected == "" { + assert.False(t, ok) + assert.Equal(t, Range{0, 0}, rawRange) + } else { + assert.True(t, ok) + assert.Equal(t, testCase.Expected, testCase.Input[rawRange.Position:rawRange.End]) + } }) } } func TestTrimTrailingCharactersFromLink(t *testing.T) { testCases := []struct { - Input string - Expected string + Input string + Start int + End int + ExpectedEnd int }{ { - Input: "http://www.example.com", - Expected: "http://www.example.com", + Input: "http://www.example.com", + ExpectedEnd: 22, + }, + { + Input: "http://www.example.com/abcd", + ExpectedEnd: 27, + }, + { + Input: "http://www.example.com/abcd/", + ExpectedEnd: 28, + }, + { + Input: "http://www.example.com/1234", + ExpectedEnd: 27, + }, + { + Input: "http://www.example.com/abcd?foo=bar", + ExpectedEnd: 35, }, { - Input: "http://www.example.com/abcd", - Expected: "http://www.example.com/abcd", + Input: "http://www.example.com/abcd#heading", + ExpectedEnd: 35, }, { - Input: "http://www.example.com/abcd/", - Expected: "http://www.example.com/abcd/", + Input: "http://www.example.com.", + ExpectedEnd: 22, }, { - Input: "http://www.example.com/1234", - Expected: "http://www.example.com/1234", + Input: "http://www.example.com,", + ExpectedEnd: 22, }, { - Input: "http://www.example.com/abcd?foo=bar", - Expected: "http://www.example.com/abcd?foo=bar", + Input: "http://www.example.com?", + ExpectedEnd: 22, }, { - Input: "http://www.example.com/abcd#heading", - Expected: "http://www.example.com/abcd#heading", + Input: "http://www.example.com)", + ExpectedEnd: 22, }, { - Input: "http://www.example.com.", - Expected: "http://www.example.com", + Input: "http://www.example.com", + ExpectedEnd: 22, }, { - Input: "http://www.example.com,", - Expected: "http://www.example.com", + Input: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation)", + ExpectedEnd: 54, }, { - Input: "http://www.example.com?", - Expected: "http://www.example.com", + Input: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation", + ExpectedEnd: 53, }, { - Input: "http://www.example.com)", - Expected: "http://www.example.com", + Input: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation))", + ExpectedEnd: 54, }, { - Input: "http://www.example.com", - Expected: "http://www.example.com", + Input: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation)_(disambiguation)", + ExpectedEnd: 71, }, { - Input: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation)", - Expected: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation)", + Input: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation_(disambiguation))", + ExpectedEnd: 71, }, { - Input: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation", - Expected: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation", + Input: "http://www.example.com"", + ExpectedEnd: 22, }, { - Input: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation))", - Expected: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation)", + Input: "this is a sentence containing http://www.example.com in it", + Start: 30, + End: 52, + ExpectedEnd: 52, }, { - Input: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation)_(disambiguation)", - Expected: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation)_(disambiguation)", + Input: "this is a sentence containing http://www.example.com???", + Start: 30, + End: 55, + ExpectedEnd: 52, }, { - Input: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation_(disambiguation))", - Expected: "https://en.wikipedia.org/wiki/Dolphin_(disambiguation_(disambiguation))", + Input: "http://google.com/å", + ExpectedEnd: len("http://google.com/å"), }, { - Input: "http://www.example.com"", - Expected: "http://www.example.com", + Input: "http://google.com/å...", + ExpectedEnd: len("http://google.com/å"), + }, + { + Input: "This is http://google.com/å, a link, and http://google.com/å", + Start: 8, + End: len("This is http://google.com/å,"), + ExpectedEnd: len("This is http://google.com/å"), + }, + { + Input: "This is http://google.com/å, a link, and http://google.com/å", + Start: 41, + End: len("This is http://google.com/å, a link, and http://google.com/å"), + ExpectedEnd: len("This is http://google.com/å, a link, and http://google.com/å"), + }, + { + Input: "This is http://google.com/å, a link, and http://google.com/å.", + Start: 41, + End: len("This is http://google.com/å, a link, and http://google.com/å."), + ExpectedEnd: len("This is http://google.com/å, a link, and http://google.com/å"), + }, + { + Input: "http://🍄.ga/ http://x🍄.ga/", + Start: 0, + End: len("http://🍄.ga/"), + ExpectedEnd: len("http://🍄.ga/"), + }, + { + Input: "http://🍄.ga/ http://x🍄.ga/", + Start: len("http://🍄.ga/ "), + End: len("http://🍄.ga/ http://x🍄.ga/"), + ExpectedEnd: len("http://🍄.ga/ http://x🍄.ga/"), }, } for _, testCase := range testCases { t.Run(testCase.Input, func(t *testing.T) { - assert.Equal(t, testCase.Expected, trimTrailingCharactersFromLink(testCase.Input)) + if testCase.End == 0 { + testCase.End = len(testCase.Input) - testCase.Start + } + + assert.Equal(t, testCase.ExpectedEnd, trimTrailingCharactersFromLink(testCase.Input, testCase.Start, testCase.End)) }) } } diff --git a/utils/markdown/commonmark_test.go b/utils/markdown/commonmark_test.go index 13e61f52d..d1381cee5 100644 --- a/utils/markdown/commonmark_test.go +++ b/utils/markdown/commonmark_test.go @@ -1000,7 +1000,7 @@ func TestCommonMarkReferenceStrings(t *testing.T) { } } -func TestCommonMarkRefernceAutolinks(t *testing.T) { +func TestCommonMarkReferenceAutolinks(t *testing.T) { // These tests are adapted from the GitHub-flavoured CommonMark extension tests located at // https://github.com/github/cmark/blob/master/test/extensions.txt for name, tc := range map[string]struct { diff --git a/utils/markdown/html.go b/utils/markdown/html.go index 1a857afed..afb72bff3 100644 --- a/utils/markdown/html.go +++ b/utils/markdown/html.go @@ -157,7 +157,7 @@ func RenderInlineHTML(inline Inline) (result string) { } result += "</a>" case *Autolink: - result += `<a href="` + htmlEscaper.Replace(escapeURL(v.Link)) + `">` + result += `<a href="` + htmlEscaper.Replace(escapeURL(v.Destination())) + `">` for _, inline := range v.Children { result += RenderInlineHTML(inline) } diff --git a/utils/markdown/inlines.go b/utils/markdown/inlines.go index 453f4bbe5..a3abccef3 100644 --- a/utils/markdown/inlines.go +++ b/utils/markdown/inlines.go @@ -86,7 +86,19 @@ type Autolink struct { Children []Inline - Link string + RawDestination Range + + markdown string +} + +func (i *Autolink) Destination() string { + destination := Unescape(i.markdown[i.RawDestination.Position:i.RawDestination.End]) + + if strings.HasPrefix(destination, "www") { + destination = "http://" + destination + } + + return destination } type delimiterType int @@ -486,15 +498,18 @@ func (p *inlineParser) parseAutolink(c rune) bool { } } - link := "" - text := "" + var link Range if c == ':' { - text = parseURLAutolink(p.raw, p.position) - link = text + var ok bool + link, ok = parseURLAutolink(p.raw, p.position) + + if !ok { + return false + } // Since the current position is at the colon, we have to rewind the parsing slightly so that // we don't duplicate the URL scheme - rewind := strings.Index(text, ":") + rewind := strings.Index(p.raw[link.Position:link.End], ":") if rewind != -1 { lastInline := p.inlines[len(p.inlines)-1] lastText, ok := lastInline.(*Text) @@ -512,22 +527,30 @@ func (p *inlineParser) parseAutolink(c rune) bool { Range: Range{lastText.Range.Position, lastText.Range.End - rewind}, }) p.position -= rewind + } + } else if c == 'w' || c == 'W' { + var ok bool + link, ok = parseWWWAutolink(p.raw, p.position) + if !ok { + return false } - } else if c == 'w' { - text = parseWWWAutolink(p.raw, p.position) - link = "http://" + text } - if text == "" { - return false - } + linkMarkdownPosition := relativeToAbsolutePosition(p.ranges, link.Position) + linkRange := Range{linkMarkdownPosition, linkMarkdownPosition + link.End - link.Position} p.inlines = append(p.inlines, &Autolink{ - Link: link, - Children: []Inline{&Text{Text: text}}, + Children: []Inline{ + &Text{ + Text: p.raw[link.Position:link.End], + Range: linkRange, + }, + }, + RawDestination: linkRange, + markdown: p.markdown, }) - p.position += len(text) + p.position += (link.End - link.Position) return true } |