diff --git a/.circleci/config.yml b/.circleci/config.yml index 9ce3c3b4ee..57a72507b2 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -205,7 +205,7 @@ jobs: docker-compose --no-ansi exec -T postgres sh -c 'exec echo "DROP DATABASE migrated; DROP DATABASE latest;" | exec psql -U mmuser mattermost_test' echo "Generating diff" - diff migrated.sql latest.sql > diff.txt && echo "Both schemas are same" || (cat diff.txt && exit 1) + diff migrated.sql latest.sql > diff.txt && echo "Both schemas are same" || (echo "Schema mismatch" && cat diff.txt && exit 1) no_output_timeout: 1h - run: name: MySQL schema migration validation @@ -234,6 +234,11 @@ jobs: -w /go/src/github.com/mattermost/mattermost-server \ mattermost/mattermost-build-server:feb-28-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" + docker-compose --no-ansi exec -T mysql mysql -D migrated -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;" + docker-compose --no-ansi exec -T mysql mysql -D latest -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;" + echo "Generating dump" docker-compose --no-ansi exec -T mysql mysqldump --skip-opt --no-data --compact -u root -pmostest migrated > migrated.sql docker-compose --no-ansi exec -T mysql mysqldump --skip-opt --no-data --compact -u root -pmostest latest > latest.sql @@ -242,7 +247,7 @@ jobs: docker-compose --no-ansi exec -T mysql mysql -uroot -pmostest -e 'DROP DATABASE migrated; DROP DATABASE latest' echo "Generating diff" - diff migrated.sql latest.sql > diff.txt && echo "Both schemas are same" || (cat diff.txt && exit 1) + diff migrated.sql latest.sql > diff.txt && echo "Both schemas are same" || (echo "Schema mismatch" && cat diff.txt && exit 1) no_output_timeout: 1h upload-s3-sha: docker: diff --git a/api4/ldap.go b/api4/ldap.go index 11f7fefe72..573e2b93af 100644 --- a/api4/ldap.go +++ b/api4/ldap.go @@ -155,13 +155,21 @@ func linkLdapGroup(c *Context, w http.ResponseWriter, r *http.Request) { var status int var newOrUpdatedGroup *model.Group + // Truncate display name if necessary + var displayName string + if len(ldapGroup.DisplayName) > model.GroupDisplayNameMaxLength { + displayName = ldapGroup.DisplayName[:model.GroupDisplayNameMaxLength] + } else { + displayName = ldapGroup.DisplayName + } + // Group has been previously linked if group != nil { if group.DeleteAt == 0 { newOrUpdatedGroup = group } else { group.DeleteAt = 0 - group.DisplayName = ldapGroup.DisplayName + group.DisplayName = displayName group.RemoteId = ldapGroup.RemoteId newOrUpdatedGroup, err = c.App.UpdateGroup(group) if err != nil { @@ -178,7 +186,7 @@ func linkLdapGroup(c *Context, w http.ResponseWriter, r *http.Request) { // the LDAP group name with an appended duplicate-breaker. newGroup := &model.Group{ Name: model.NewId(), - DisplayName: ldapGroup.DisplayName, + DisplayName: displayName, RemoteId: ldapGroup.RemoteId, Source: model.GroupSourceLdap, } diff --git a/app/export.go b/app/export.go index 8ce7d50cdd..4836547002 100644 --- a/app/export.go +++ b/app/export.go @@ -5,7 +5,6 @@ package app import ( "encoding/json" - "fmt" "io" "net/http" "os" @@ -414,7 +413,7 @@ func (a *App) BuildPostReactions(postId string) (*[]ReactionImportData, *model.A user, err = a.Srv.Store.User().Get(reaction.UserId) if err != nil { if err.Id == store.MISSING_ACCOUNT_ERROR { // this is a valid case, the user that reacted might've been deleted by now - mlog.Info(fmt.Sprintf("Skipping reactions by user %v, since the entity doesn't exist anymore", reaction.UserId)) + mlog.Info("Skipping reactions by user since the entity doesn't exist anymore", mlog.String("user_id", reaction.UserId)) continue } return nil, err diff --git a/app/import.go b/app/import.go index a4707543fe..f82c6da2b9 100644 --- a/app/import.go +++ b/app/import.go @@ -6,7 +6,6 @@ package app import ( "bufio" "encoding/json" - "fmt" "io" "net/http" "strings" @@ -19,7 +18,7 @@ import ( func stopOnError(err LineImportWorkerError) bool { if err.Error.Id == "api.file.upload_file.large_image.app_error" { - mlog.Warn(fmt.Sprintf("Large image import error: %s", err.Error.Error())) + mlog.Warn("Large image import error", mlog.Err(err.Error)) return false } return true diff --git a/app/post.go b/app/post.go index ca59f188c2..19a06fe417 100644 --- a/app/post.go +++ b/app/post.go @@ -789,7 +789,7 @@ func (a *App) GetPostsForChannelAroundLastUnread(channelId, userId string, limit return model.NewPostList(), nil } - postList, err := a.GetPostThread(lastUnreadPostId, false) + postList, err := a.GetPostThread(lastUnreadPostId, skipFetchThreads) if err != nil { return nil, err } diff --git a/build/Jenkinsfile.pr b/build/Jenkinsfile.pr index bf8ad7e5d2..21af2e7d34 100644 --- a/build/Jenkinsfile.pr +++ b/build/Jenkinsfile.pr @@ -353,6 +353,10 @@ pipeline { dir('src/github.com/mattermost/mattermost-server') { ansiColor('xterm') { sh """ + echo "Ignoring known MySQL mismatch: ChannelMembers.SchemeGuest" + /usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml exec -T mysql mysql -D migrated -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;" + /usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml exec -T mysql mysql -D latest -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;" + echo "Generating dump" /usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml exec -T mysql mysqldump --skip-opt --no-data --compact -u root -pmostest migrated > migrated.sql /usr/local/bin/docker-compose --no-ansi -f build/docker-compose.yml exec -T mysql mysqldump --skip-opt --no-data --compact -u root -pmostest latest > latest.sql diff --git a/model/emoji_test.go b/model/emoji_test.go index 4539db8731..c33a164d23 100644 --- a/model/emoji_test.go +++ b/model/emoji_test.go @@ -20,9 +20,7 @@ func TestEmojiIsValid(t *testing.T) { Name: "name", } - if err := emoji.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, emoji.IsValid()) emoji.Id = "1234" require.NotNil(t, emoji.IsValid()) diff --git a/model/manifest.go b/model/manifest.go index 0eb64534a0..92565e2d1f 100644 --- a/model/manifest.go +++ b/model/manifest.go @@ -130,6 +130,10 @@ type Manifest struct { // A description of what your plugin is and does. Description string `json:"description,omitempty" yaml:"description,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"` + // A version number for your plugin. Semantic versioning is recommended: http://semver.org Version string `json:"version" yaml:"version"` diff --git a/model/manifest_test.go b/model/manifest_test.go index 6cec274ec2..53e2370af0 100644 --- a/model/manifest_test.go +++ b/model/manifest_test.go @@ -64,6 +64,7 @@ func TestFindManifest(t *testing.T) { func TestManifestUnmarshal(t *testing.T) { expected := Manifest{ Id: "theid", + IconPath: "assets/icon.svg", MinServerVersion: "5.6.0", Server: &ManifestServer{ Executable: "theexecutable", @@ -102,6 +103,7 @@ func TestManifestUnmarshal(t *testing.T) { var yamlResult Manifest require.NoError(t, yaml.Unmarshal([]byte(` id: theid +icon_path: assets/icon.svg min_server_version: 5.6.0 server: executable: theexecutable @@ -131,7 +133,8 @@ settings_schema: var jsonResult Manifest require.NoError(t, json.Unmarshal([]byte(`{ "id": "theid", - "min_server_version": "5.6.0", + "icon_path": "assets/icon.svg", + "min_server_version": "5.6.0", "server": { "executable": "theexecutable", "executables": { diff --git a/model/post_list_test.go b/model/post_list_test.go index a24c710320..8ee618f2da 100644 --- a/model/post_list_test.go +++ b/model/post_list_test.go @@ -24,17 +24,9 @@ func TestPostListJson(t *testing.T) { json := pl.ToJson() rpl := PostListFromJson(strings.NewReader(json)) - if rpl.Posts[p1.Id].Message != p1.Message { - t.Fatal("failed to serialize") - } - - if rpl.Posts[p2.Id].Message != p2.Message { - t.Fatal("failed to serialize") - } - - if rpl.Order[1] != p2.Id { - t.Fatal("failed to serialize") - } + assert.Equal(t, p1.Message, rpl.Posts[p1.Id].Message, "failed to serialize p1 message") + assert.Equal(t, p2.Message, rpl.Posts[p2.Id].Message, "failed to serialize p2 message") + assert.Equal(t, p2.Id, rpl.Order[1], "failed to serialize p2 Id") } func TestPostListExtend(t *testing.T) { diff --git a/model/team_member_test.go b/model/team_member_test.go index 3661fff4b7..9adee8e9a7 100644 --- a/model/team_member_test.go +++ b/model/team_member_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestTeamMemberJson(t *testing.T) { @@ -13,22 +15,17 @@ func TestTeamMemberJson(t *testing.T) { json := o.ToJson() ro := TeamMemberFromJson(strings.NewReader(json)) - if o.TeamId != ro.TeamId { - t.Fatal("Ids do not match") - } + require.Equal(t, o.TeamId, ro.TeamId, "Ids do not match") } func TestTeamMemberIsValid(t *testing.T) { o := TeamMember{} - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.Error(t, o.IsValid(), "should be invalid") o.TeamId = NewId() - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + + require.Error(t, o.IsValid(), "should be invalid") /*o.UserId = NewId() o.Roles = "blahblah" @@ -47,11 +44,8 @@ func TestUnreadMemberJson(t *testing.T) { json := o.ToJson() r := TeamUnreadFromJson(strings.NewReader(json)) - if o.TeamId != r.TeamId { - t.Fatal("Ids do not match") - } - if o.MsgCount != r.MsgCount { - t.Fatal("MsgCount do not match") - } + require.Equal(t, o.TeamId, r.TeamId, "Ids do not match") + + require.Equal(t, o.MsgCount, r.MsgCount, "MsgCount do not match") } diff --git a/scripts/mysql-migration-test.sh b/scripts/mysql-migration-test.sh index 5da4cdde5b..1c8f9d3508 100755 --- a/scripts/mysql-migration-test.sh +++ b/scripts/mysql-migration-test.sh @@ -22,6 +22,10 @@ make ARGS="config set SqlSettings.DataSource 'mmuser:mostest@tcp(localhost:3306) echo "Setting up fresh db" make ARGS="version --config $TMPDIR/config.json" run-cli +echo "Ignoring known MySQL mismatch: ChannelMembers.SchemeGuest" +docker exec mattermost-mysql mysql -D migrated -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;" +docker exec mattermost-mysql mysql -D latest -uroot -pmostest -e "ALTER TABLE ChannelMembers DROP COLUMN SchemeGuest;" + echo "Generating dump" docker exec mattermost-mysql mysqldump --skip-opt --no-data --compact -u root -pmostest migrated > $DUMPDIR/migrated.sql docker exec mattermost-mysql mysqldump --skip-opt --no-data --compact -u root -pmostest latest > $DUMPDIR/latest.sql @@ -33,7 +37,12 @@ echo "Generating diff" diff $DUMPDIR/migrated.sql $DUMPDIR/latest.sql > $DUMPDIR/diff.txt diffErrorCode=$? -if [ $diffErrorCode -eq 0 ]; then echo "Both schemas are same";else cat $DUMPDIR/diff.txt; fi +if [ $diffErrorCode -eq 0 ]; then + echo "Both schemas are same" +else + echo "Schema mismatch" + cat $DUMPDIR/diff.txt +fi rm -rf $TMPDIR $DUMPDIR exit $diffErrorCode diff --git a/scripts/psql-migration-test.sh b/scripts/psql-migration-test.sh index 57997bcae1..a3861f4b7d 100755 --- a/scripts/psql-migration-test.sh +++ b/scripts/psql-migration-test.sh @@ -33,7 +33,12 @@ echo "Generating diff" diff $DUMPDIR/migrated.sql $DUMPDIR/latest.sql > $DUMPDIR/diff.txt diffErrorCode=$? -if [ $diffErrorCode -eq 0 ]; then echo "Both schemas are same";else cat $DUMPDIR/diff.txt; fi +if [ $diffErrorCode -eq 0 ]; then + echo "Both schemas are same" +else + echo "Schema mismatch" + cat $DUMPDIR/diff.txt +fi rm -rf $TMPDIR $DUMPDIR exit $diffErrorCode diff --git a/services/mfa/mfa_test.go b/services/mfa/mfa_test.go index b89199883d..6f826cdbda 100644 --- a/services/mfa/mfa_test.go +++ b/services/mfa/mfa_test.go @@ -41,9 +41,7 @@ func TestGenerateSecret(t *testing.T) { assert.Len(t, secret, 32) - if len(img) == 0 { - t.Fatal("no image set") - } + require.NotEmpty(t, img, "no image set") config.ServiceSettings.EnableMultifactorAuthentication = model.NewBool(false) diff --git a/services/timezones/default.go b/services/timezones/default.go index 065d0eef72..db84a2510c 100644 --- a/services/timezones/default.go +++ b/services/timezones/default.go @@ -378,7 +378,6 @@ var DefaultSupportedTimezones = []string{ "CST6CDT", "Canada/Atlantic", "Canada/Central", - "Canada/East-Saskatchewan", "Canada/Eastern", "Canada/Mountain", "Canada/Newfoundland", @@ -491,7 +490,6 @@ var DefaultSupportedTimezones = []string{ "Europe/Zagreb", "Europe/Zaporozhye", "Europe/Zurich", - "Factory", "GB", "GB-Eire", "GMT", diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index 1f77730da4..a58d63bcc0 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -191,7 +191,7 @@ func (s *SqlPostStore) GetFlaggedPosts(userId string, offset int, limit int) (*m pl := model.NewPostList() var posts []*model.Post - if _, err := s.GetReplica().Select(&posts, "SELECT * FROM Posts WHERE Id IN (SELECT Name FROM Preferences WHERE UserId = :UserId AND Category = :Category) AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset", map[string]interface{}{"UserId": userId, "Category": model.PREFERENCE_CATEGORY_FLAGGED_POST, "Offset": offset, "Limit": limit}); err != nil { + if _, err := s.GetReplica().Select(&posts, "SELECT *, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE Id IN (SELECT Name FROM Preferences WHERE UserId = :UserId AND Category = :Category) AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset", map[string]interface{}{"UserId": userId, "Category": model.PREFERENCE_CATEGORY_FLAGGED_POST, "Offset": offset, "Limit": limit}); err != nil { return nil, model.NewAppError("SqlPostStore.GetFlaggedPosts", "store.sql_post.get_flagged_posts.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -210,7 +210,7 @@ func (s *SqlPostStore) GetFlaggedPostsForTeam(userId, teamId string, offset int, query := ` SELECT - A.* + A.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = A.Id AND Posts.DeleteAt = 0) as ReplyCount FROM (SELECT * @@ -252,8 +252,8 @@ func (s *SqlPostStore) GetFlaggedPostsForChannel(userId, channelId string, offse var posts []*model.Post query := ` SELECT - * - FROM Posts + *, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount + FROM Posts p WHERE Id IN (SELECT Name FROM Preferences WHERE UserId = :UserId AND Category = :Category) AND ChannelId = :ChannelId @@ -280,12 +280,7 @@ func (s *SqlPostStore) Get(id string, skipFetchThreads bool) (*model.PostList, * } var post model.Post - var postFetchQuery string - if skipFetchThreads { - postFetchQuery = "SELECT p.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id) as ReplyCount FROM Posts p WHERE p.Id = :Id AND p.DeleteAt = 0" - } else { - postFetchQuery = "SELECT * FROM Posts WHERE Id = :Id AND DeleteAt = 0" - } + postFetchQuery := "SELECT p.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE p.Id = :Id AND p.DeleteAt = 0" err := s.GetReplica().SelectOne(&post, postFetchQuery, map[string]interface{}{"Id": id}) if err != nil { return nil, model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "id="+id+err.Error(), http.StatusNotFound) @@ -304,7 +299,7 @@ func (s *SqlPostStore) Get(id string, skipFetchThreads bool) (*model.PostList, * } var posts []*model.Post - _, err = s.GetReplica().Select(&posts, "SELECT * FROM Posts WHERE (Id = :Id OR RootId = :RootId) AND DeleteAt = 0", map[string]interface{}{"Id": rootId, "RootId": rootId}) + _, err = s.GetReplica().Select(&posts, "SELECT *, (SELECT count(Id) FROM Posts WHERE RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE (Id = :Id OR RootId = :RootId) AND DeleteAt = 0", map[string]interface{}{"Id": rootId, "RootId": rootId}) if err != nil { return nil, model.NewAppError("SqlPostStore.GetPost", "store.sql_post.get.app_error", nil, "root_id="+rootId+err.Error(), http.StatusInternalServerError) } @@ -545,8 +540,8 @@ func (s *SqlPostStore) GetPostsSince(options model.GetPostsSinceOptions, allowFr replyCountQuery1 := "" replyCountQuery2 := "" if options.SkipFetchThreads { - replyCountQuery1 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p1.RootId = '' AND Posts.RootId = p1.Id) as ReplyCount` - replyCountQuery2 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p2.RootId = '' AND Posts.RootId = p2.Id) as ReplyCount` + replyCountQuery1 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p1.RootId = '' AND Posts.RootId = p1.Id AND Posts.DeleteAt = 0) as ReplyCount` + replyCountQuery2 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p2.RootId = '' AND Posts.RootId = p2.Id AND Posts.DeleteAt = 0) as ReplyCount` } _, err := s.GetReplica().Select(&posts, @@ -621,7 +616,7 @@ func (s *SqlPostStore) getPostsAround(before bool, options model.GetPostsOptions direction = ">" sort = "ASC" } - replyCountSubQuery := s.getQueryBuilder().Select("COUNT(Posts.Id)").From("Posts").Where(sq.Expr("p.RootId = '' AND RootId = p.Id")) + replyCountSubQuery := s.getQueryBuilder().Select("COUNT(Posts.Id)").From("Posts").Where(sq.Expr("p.RootId = '' AND RootId = p.Id AND DeleteAt = 0")) query := s.getQueryBuilder().Select("p.*") if options.SkipFetchThreads { query = query.Column(sq.Alias(replyCountSubQuery, "ReplyCount")) @@ -782,7 +777,7 @@ func (s *SqlPostStore) getRootPosts(channelId string, offset int, limit int, ski var posts []*model.Post var fetchQuery string if skipFetchThreads { - fetchQuery = "SELECT p.*, (SELECT COUNT(Posts.Id) FROM Posts WHERE p.RootId = '' AND Posts.RootId = p.Id) as ReplyCount FROM Posts p WHERE ChannelId = :ChannelId AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset" + fetchQuery = "SELECT p.*, (SELECT COUNT(Posts.Id) FROM Posts WHERE p.RootId = '' AND Posts.RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE ChannelId = :ChannelId AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset" } else { fetchQuery = "SELECT * FROM Posts WHERE ChannelId = :ChannelId AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset" } @@ -798,7 +793,7 @@ func (s *SqlPostStore) getParentsPosts(channelId string, offset int, limit int, replyCountQuery := "" onStatement := "q1.RootId = q2.Id" if skipFetchThreads { - replyCountQuery = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE q2.RootId = '' AND Posts.RootId = q2.Id) as ReplyCount` + replyCountQuery = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE q2.RootId = '' AND Posts.RootId = q2.Id AND Posts.DeleteAt = 0) as ReplyCount` } else { onStatement += " OR q1.RootId = q2.RootId" } @@ -988,9 +983,9 @@ func (s *SqlPostStore) Search(teamId string, userId string, params *model.Search searchQuery := ` SELECT - * + * ,(SELECT COUNT(Posts.Id) FROM Posts WHERE q2.RootId = '' AND Posts.RootId = q2.Id AND Posts.DeleteAt = 0) as ReplyCount FROM - Posts + Posts q2 WHERE DeleteAt = 0 AND Type NOT LIKE '` + model.POST_SYSTEM_MESSAGE_PREFIX + `%' diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index d645282b95..4b216a4a7a 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -713,17 +713,6 @@ func UpgradeDatabaseToVersion513(sqlStore SqlStore) { func UpgradeDatabaseToVersion514(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_13_0, VERSION_5_14_0) { - sqlStore.AlterColumnTypeIfExists("TeamMembers", "SchemeGuest", "tinyint(4)", "boolean") - sqlStore.AlterColumnTypeIfExists("ChannelMembers", "SchemeGuest", "tinyint(4)", "boolean") - sqlStore.AlterColumnTypeIfExists("Schemes", "DefaultTeamGuestRole", "varchar(64)", "VARCHAR(64)") - sqlStore.AlterColumnTypeIfExists("Schemes", "DefaultChannelGuestRole", "varchar(64)", "VARCHAR(64)") - sqlStore.AlterColumnTypeIfExists("Teams", "AllowedDomains", "text", "VARCHAR(1000)") - sqlStore.AlterColumnTypeIfExists("Channels", "GroupConstrained", "tinyint(1)", "boolean") - sqlStore.AlterColumnTypeIfExists("Teams", "GroupConstrained", "tinyint(1)", "boolean") - - sqlStore.CreateIndexIfNotExists("idx_groupteams_teamid", "GroupTeams", "TeamId") - sqlStore.CreateIndexIfNotExists("idx_groupchannels_channelid", "GroupChannels", "ChannelId") - saveSchemaVersion(sqlStore, VERSION_5_14_0) } } @@ -742,5 +731,21 @@ func UpgradeDatabaseToVersion516(sqlStore SqlStore) { sqlStore.GetMaster().Exec("ALTER TABLE Tokens MODIFY Extra text") } saveSchemaVersion(sqlStore, VERSION_5_16_0) + + // Fix mismatches between the canonical and migrated schemas. + sqlStore.AlterColumnTypeIfExists("TeamMembers", "SchemeGuest", "tinyint(4)", "boolean") + sqlStore.AlterColumnTypeIfExists("Schemes", "DefaultTeamGuestRole", "varchar(64)", "VARCHAR(64)") + sqlStore.AlterColumnTypeIfExists("Schemes", "DefaultChannelGuestRole", "varchar(64)", "VARCHAR(64)") + sqlStore.AlterColumnTypeIfExists("Teams", "AllowedDomains", "text", "VARCHAR(1000)") + sqlStore.AlterColumnTypeIfExists("Channels", "GroupConstrained", "tinyint(1)", "boolean") + sqlStore.AlterColumnTypeIfExists("Teams", "GroupConstrained", "tinyint(1)", "boolean") + + // One known mismatch remains: ChannelMembers.SchemeGuest. The requisite migration + // is left here for posterity, but we're avoiding fix this given the corresponding + // table rewrite in most MySQL and Postgres instances. + // sqlStore.AlterColumnTypeIfExists("ChannelMembers", "SchemeGuest", "tinyint(4)", "boolean") + + sqlStore.CreateIndexIfNotExists("idx_groupteams_teamid", "GroupTeams", "TeamId") + sqlStore.CreateIndexIfNotExists("idx_groupchannels_channelid", "GroupChannels", "ChannelId") } } diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index 89ad1c7e7b..11afeb3fe8 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -2018,7 +2018,7 @@ func testGetGroups(t *testing.T, ss store.Store) { require.Nil(t, err) group2, err := ss.Group().Create(&model.Group{ - Name: model.NewId(), + Name: model.NewId() + "-group-2", DisplayName: "group-2", RemoteId: model.NewId(), Source: model.GroupSourceLdap, @@ -2062,7 +2062,7 @@ func testGetGroups(t *testing.T, ss store.Store) { // Create Group3 group3, err := ss.Group().Create(&model.Group{ - Name: model.NewId(), + Name: model.NewId() + "-group-3", DisplayName: "group-3", RemoteId: model.NewId(), Source: model.GroupSourceLdap, @@ -2122,7 +2122,7 @@ func testGetGroups(t *testing.T, ss store.Store) { user2.DeleteAt = 1 ss.User().Update(user2, true) - group2NameSubstring := string([]rune(group2.Name)[2:5]) + group2NameSubstring := "group-2" testCases := []struct { Name string diff --git a/store/storetest/session_store.go b/store/storetest/session_store.go index 4653ecea1e..c9e376f7fd 100644 --- a/store/storetest/session_store.go +++ b/store/storetest/session_store.go @@ -34,9 +34,8 @@ func testSessionStoreSave(t *testing.T, ss store.Store) { s1 := &model.Session{} s1.UserId = model.NewId() - if _, err := ss.Session().Save(s1); err != nil { - t.Fatal(err) - } + _, err := ss.Session().Save(s1) + require.Nil(t, err) } func testSessionGet(t *testing.T, ss store.Store) { @@ -59,21 +58,13 @@ func testSessionGet(t *testing.T, ss store.Store) { s3, err = ss.Session().Save(s3) require.Nil(t, err) - if session, err := ss.Session().Get(s1.Id); err != nil { - t.Fatal(err) - } else { - if session.Id != s1.Id { - t.Fatal("should match") - } - } + session, err := ss.Session().Get(s1.Id) + require.Nil(t, err) + require.Equal(t, session.Id, s1.Id, "should match") - if session, err := ss.Session().GetSessions(s1.UserId); err != nil { - t.Fatal(err) - } else { - if len(session) != 3 { - t.Fatal("should match len") - } - } + data, err := ss.Session().GetSessions(s1.UserId) + require.Nil(t, err) + require.Len(t, data, 3, "should match len") } func testSessionGetWithDeviceId(t *testing.T, ss store.Store) { @@ -100,13 +91,9 @@ func testSessionGetWithDeviceId(t *testing.T, ss store.Store) { s3, err = ss.Session().Save(s3) require.Nil(t, err) - if data, err := ss.Session().GetSessionsWithActiveDeviceIds(s1.UserId); err != nil { - t.Fatal(err) - } else { - if len(data) != 1 { - t.Fatal("should match len") - } - } + data, err := ss.Session().GetSessionsWithActiveDeviceIds(s1.UserId) + require.Nil(t, err) + require.Len(t, data, 1, "should match len") } func testSessionRemove(t *testing.T, ss store.Store) { @@ -116,19 +103,15 @@ func testSessionRemove(t *testing.T, ss store.Store) { s1, err := ss.Session().Save(s1) require.Nil(t, err) - if session, err := ss.Session().Get(s1.Id); err != nil { - t.Fatal(err) - } else { - if session.Id != s1.Id { - t.Fatal("should match") - } - } + session, err := ss.Session().Get(s1.Id) + require.Nil(t, err) + require.Equal(t, session.Id, s1.Id, "should match") removeErr := ss.Session().Remove(s1.Id) require.Nil(t, removeErr) - if _, err := ss.Session().Get(s1.Id); err == nil { - t.Fatal("should have been removed") - } + + _, err = ss.Session().Get(s1.Id) + require.NotNil(t, err, "should have been removed") } func testSessionRemoveAll(t *testing.T, ss store.Store) { @@ -138,20 +121,15 @@ func testSessionRemoveAll(t *testing.T, ss store.Store) { s1, err := ss.Session().Save(s1) require.Nil(t, err) - if session, err := ss.Session().Get(s1.Id); err != nil { - t.Fatal(err) - } else { - if session.Id != s1.Id { - t.Fatal("should match") - } - } + session, err := ss.Session().Get(s1.Id) + require.Nil(t, err) + require.Equal(t, session.Id, s1.Id, "should match") removeErr := ss.Session().RemoveAllSessions() require.Nil(t, removeErr) - if _, err := ss.Session().Get(s1.Id); err == nil { - t.Fatal("should have been removed") - } + _, err = ss.Session().Get(s1.Id) + require.NotNil(t, err, "should have been removed") } func testSessionRemoveByUser(t *testing.T, ss store.Store) { @@ -161,20 +139,15 @@ func testSessionRemoveByUser(t *testing.T, ss store.Store) { s1, err := ss.Session().Save(s1) require.Nil(t, err) - if session, err := ss.Session().Get(s1.Id); err != nil { - t.Fatal(err) - } else { - if session.Id != s1.Id { - t.Fatal("should match") - } - } + session, err := ss.Session().Get(s1.Id) + require.Nil(t, err) + require.Equal(t, session.Id, s1.Id, "should match") deleteErr := ss.Session().PermanentDeleteSessionsByUser(s1.UserId) require.Nil(t, deleteErr) - if _, err := ss.Session().Get(s1.Id); err == nil { - t.Fatal("should have been removed") - } + _, err = ss.Session().Get(s1.Id) + require.NotNil(t, err, "should have been removed") } func testSessionRemoveToken(t *testing.T, ss store.Store) { @@ -184,28 +157,19 @@ func testSessionRemoveToken(t *testing.T, ss store.Store) { s1, err := ss.Session().Save(s1) require.Nil(t, err) - if session, err := ss.Session().Get(s1.Id); err != nil { - t.Fatal(err) - } else { - if session.Id != s1.Id { - t.Fatal("should match") - } - } + session, err := ss.Session().Get(s1.Id) + require.Nil(t, err) + require.Equal(t, session.Id, s1.Id, "should match") removeErr := ss.Session().Remove(s1.Token) require.Nil(t, removeErr) - if _, err := ss.Session().Get(s1.Id); err == nil { - t.Fatal("should have been removed") - } + _, err = ss.Session().Get(s1.Id) + require.NotNil(t, err, "should have been removed") - if session, err := ss.Session().GetSessions(s1.UserId); err != nil { - t.Fatal(err) - } else { - if len(session) != 0 { - t.Fatal("should match len") - } - } + data, err := ss.Session().GetSessions(s1.UserId) + require.Nil(t, err) + require.Len(t, data, 0, "should match len") } func testSessionUpdateDeviceId(t *testing.T, ss store.Store) { @@ -215,9 +179,8 @@ func testSessionUpdateDeviceId(t *testing.T, ss store.Store) { s1, err := ss.Session().Save(s1) require.Nil(t, err) - if _, err = ss.Session().UpdateDeviceId(s1.Id, model.PUSH_NOTIFY_APPLE+":1234567890", s1.ExpiresAt); err != nil { - t.Fatal(err) - } + _, err = ss.Session().UpdateDeviceId(s1.Id, model.PUSH_NOTIFY_APPLE+":1234567890", s1.ExpiresAt) + require.Nil(t, err) s2 := &model.Session{} s2.UserId = model.NewId() @@ -225,9 +188,8 @@ func testSessionUpdateDeviceId(t *testing.T, ss store.Store) { s2, err = ss.Session().Save(s2) require.Nil(t, err) - if _, err := ss.Session().UpdateDeviceId(s2.Id, model.PUSH_NOTIFY_APPLE+":1234567890", s1.ExpiresAt); err != nil { - t.Fatal(err) - } + _, err = ss.Session().UpdateDeviceId(s2.Id, model.PUSH_NOTIFY_APPLE+":1234567890", s1.ExpiresAt) + require.Nil(t, err) } func testSessionUpdateDeviceId2(t *testing.T, ss store.Store) { @@ -237,9 +199,8 @@ func testSessionUpdateDeviceId2(t *testing.T, ss store.Store) { s1, err := ss.Session().Save(s1) require.Nil(t, err) - if _, err = ss.Session().UpdateDeviceId(s1.Id, model.PUSH_NOTIFY_APPLE_REACT_NATIVE+":1234567890", s1.ExpiresAt); err != nil { - t.Fatal(err) - } + _, err = ss.Session().UpdateDeviceId(s1.Id, model.PUSH_NOTIFY_APPLE_REACT_NATIVE+":1234567890", s1.ExpiresAt) + require.Nil(t, err) s2 := &model.Session{} s2.UserId = model.NewId() @@ -247,9 +208,8 @@ func testSessionUpdateDeviceId2(t *testing.T, ss store.Store) { s2, err = ss.Session().Save(s2) require.Nil(t, err) - if _, err := ss.Session().UpdateDeviceId(s2.Id, model.PUSH_NOTIFY_APPLE_REACT_NATIVE+":1234567890", s1.ExpiresAt); err != nil { - t.Fatal(err) - } + _, err = ss.Session().UpdateDeviceId(s2.Id, model.PUSH_NOTIFY_APPLE_REACT_NATIVE+":1234567890", s1.ExpiresAt) + require.Nil(t, err) } func testSessionStoreUpdateLastActivityAt(t *testing.T, ss store.Store) { @@ -262,14 +222,9 @@ func testSessionStoreUpdateLastActivityAt(t *testing.T, ss store.Store) { err = ss.Session().UpdateLastActivityAt(s1.Id, 1234567890) require.Nil(t, err) - if session, err := ss.Session().Get(s1.Id); err != nil { - t.Fatal(err) - } else { - if session.LastActivityAt != 1234567890 { - t.Fatal("LastActivityAt not updated correctly") - } - } - + session, err := ss.Session().Get(s1.Id) + require.Nil(t, err) + require.EqualValues(t, session.LastActivityAt, 1234567890, "LastActivityAt not updated correctly") } func testSessionCount(t *testing.T, ss store.Store) { @@ -280,13 +235,9 @@ func testSessionCount(t *testing.T, ss store.Store) { s1, err := ss.Session().Save(s1) require.Nil(t, err) - if count, err := ss.Session().AnalyticsSessionCount(); err != nil { - t.Fatal(err) - } else { - if count == 0 { - t.Fatal("should have at least 1 session") - } - } + count, err := ss.Session().AnalyticsSessionCount() + require.Nil(t, err) + require.NotZero(t, count, "should have at least 1 session") } func testSessionCleanup(t *testing.T, ss store.Store) { diff --git a/utils/subpath.go b/utils/subpath.go index 9bb75db3de..14c1ea6f69 100644 --- a/utils/subpath.go +++ b/utils/subpath.go @@ -148,6 +148,12 @@ func UpdateAssetsSubpathFromConfig(config *model.Config) error { return nil } + // Similarly, don't rewrite during a CI build, when the assets may not even be present. + if os.Getenv("IS_CI") == "true" { + mlog.Debug("Skipping update to assets subpath since CI build") + return nil + } + subpath, err := GetSubpathFromConfig(config) if err != nil { return err diff --git a/utils/subpath_test.go b/utils/subpath_test.go index aa6fbe7b98..0c76bd7368 100644 --- a/utils/subpath_test.go +++ b/utils/subpath_test.go @@ -14,6 +14,40 @@ import ( "github.com/mattermost/mattermost-server/utils" ) +func TestUpdateAssetsSubpathFromConfig(t *testing.T) { + t.Run("dev build", func(t *testing.T) { + var oldBuildNumber = model.BuildNumber + model.BuildNumber = "dev" + defer func() { + model.BuildNumber = oldBuildNumber + }() + + err := utils.UpdateAssetsSubpathFromConfig(nil) + require.NoError(t, err) + }) + + t.Run("IS_CI=true", func(t *testing.T) { + err := os.Setenv("IS_CI", "true") + require.NoError(t, err) + defer func() { + os.Unsetenv("IS_CI") + }() + + err = utils.UpdateAssetsSubpathFromConfig(nil) + require.NoError(t, err) + }) + + t.Run("no config", func(t *testing.T) { + tempDir, err := ioutil.TempDir("", "test_update_assets_subpath") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + os.Chdir(tempDir) + + err = utils.UpdateAssetsSubpathFromConfig(nil) + require.Error(t, err) + }) +} + func TestUpdateAssetsSubpath(t *testing.T) { t.Run("no client dir", func(t *testing.T) { tempDir, err := ioutil.TempDir("", "test_update_assets_subpath") @@ -157,10 +191,6 @@ func TestUpdateAssetsSubpath(t *testing.T) { } func TestGetSubpathFromConfig(t *testing.T) { - sToP := func(s string) *string { - return &s - } - testCases := []struct { Description string SiteURL *string @@ -231,6 +261,10 @@ func TestGetSubpathFromConfig(t *testing.T) { } } +func sToP(s string) *string { + return &s +} + const contentSecurityPolicyNotFoundHtml = ` 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 contentSecurityPolicyNotFound2Html = ` 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.


`