From eae1acef5f5886566455282cff85119c8451d911 Mon Sep 17 00:00:00 2001 From: Eli Yukelzon Date: Tue, 15 Oct 2019 19:43:20 +0300 Subject: [PATCH 01/14] removed unsupported timezones (#12707) --- services/timezones/default.go | 2 -- 1 file changed, 2 deletions(-) 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", From 5d45aa81e0dc64684cb6799e6bda057a7829bef4 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Tue, 15 Oct 2019 15:47:09 -0300 Subject: [PATCH 02/14] MM-16888: fix missing indexes (#12746) * MM-16888: fix missing indexes As part of https://mattermost.atlassian.net/browse/MM-16888, we discovered and fixed a number of column and index mismatches between the canonical (i.e. created from scratch) and migrated schemas (i.e migrated from 5.0 through 5.16). Unfortunately, the migration to fix same was added to `UpgradeDatabaseToVersion514` but never cherry picked to the pending v5.14 release at the time. Customers who upgraded to v5.14 or v5.15 and then get this code as part of v5.16 will never run that migration. Copy it to the UpgradeDatabaseToVersion516 accordingly. * avoid fixing ChannelMembers.SchemeGuest on MySQL * synchronize .circleci/config.yml with scripts/mysql-migration-test.sh * fix circleci invocation * additional logging on diff * update build/Jenkinsfile.pr too! --- .circleci/config.yml | 9 +++++++-- build/Jenkinsfile.pr | 4 ++++ scripts/mysql-migration-test.sh | 11 ++++++++++- scripts/psql-migration-test.sh | 7 ++++++- store/sqlstore/upgrade.go | 27 ++++++++++++++++----------- 5 files changed, 43 insertions(+), 15 deletions(-) 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/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/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/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") } } From bc1ae7e93e69a52db488611a81d5369717a6285a Mon Sep 17 00:00:00 2001 From: Jesper Hansen Date: Tue, 15 Oct 2019 23:04:38 +0200 Subject: [PATCH 03/14] MM-19237: convert t.Fatal to use require calls (#12660) --- model/emoji_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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()) From b43b84807ee7529cac389b871c340b31d596d9fa Mon Sep 17 00:00:00 2001 From: Shodiq Muhammad <47969743+iDevoid@users.noreply.github.com> Date: Wed, 16 Oct 2019 04:05:58 +0700 Subject: [PATCH 04/14] change t.Fatal to assert.Equal in post_list_test.go (#12678) --- model/post_list_test.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) 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) { From ad474cb0d9305ce9791465169521badff246c3aa Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Tue, 15 Oct 2019 23:25:50 +0200 Subject: [PATCH 05/14] [MM-18592] Add IconPath field to plugin manifest (#12189) --- model/manifest.go | 4 ++++ model/manifest_test.go | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) 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": { From 1a2197ec8e384a1fdccf675210e84282acd56b62 Mon Sep 17 00:00:00 2001 From: Arjit Chaudhary Date: Wed, 16 Oct 2019 03:01:46 +0530 Subject: [PATCH 06/14] Refactor "app/export.go" to use structured logging #12727 (#12729) * Refactor "app/export.go" to use structured logging #12727 * use mlog.String * use mlog.Bool, mlog.string needs 2 values? * Update export.go * s/reaction_UserId/user_id --- app/export.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 From 42b011b81d912d0d94d566f016473fa656b9ca5d Mon Sep 17 00:00:00 2001 From: Phillip Ahereza Date: Wed, 16 Oct 2019 06:09:12 +0300 Subject: [PATCH 07/14] Migrate tests from mfa_test.go to use testify (#12720) --- services/mfa/mfa_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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) From 4225977966cf0855c8a5e55f8a0fef702b19dc18 Mon Sep 17 00:00:00 2001 From: Eli Yukelzon Date: Wed, 16 Oct 2019 12:55:36 +0300 Subject: [PATCH 08/14] =?UTF-8?q?MM-19154=20-=20Fix=20flaky=20test=20TestG?= =?UTF-8?q?roupStore/MySQL/GetGroups/Get=5Fg=E2=80=A6=20(#12705)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- store/storetest/group_store.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 From c742d1882835a54695dc3fa7f73d3d109aca9dc6 Mon Sep 17 00:00:00 2001 From: Martin Kraft Date: Wed, 16 Oct 2019 11:50:24 -0400 Subject: [PATCH 09/14] MM-17477: Truncates group display name upon linking. (#12751) --- api4/ldap.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 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, } From 66c66eef0dba4e1261a33891357038cf00685516 Mon Sep 17 00:00:00 2001 From: Eli Yukelzon Date: Thu, 17 Oct 2019 11:10:49 +0300 Subject: [PATCH 10/14] =?UTF-8?q?MM-19371=20-=20Reply=20count=20disappears?= =?UTF-8?q?=20from=20pinned=20and=20flagged=20conv=E2=80=A6=20(#12753)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/post.go | 2 +- store/sqlstore/post_store.go | 31 +++++++++++++------------------ 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/app/post.go b/app/post.go index f05ffce078..dd43b219b5 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/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 + `%' From c7e2689926867f424c8274b7b4851ea6a63b5384 Mon Sep 17 00:00:00 2001 From: Ben Sooraj Date: Thu, 17 Oct 2019 15:48:08 +0530 Subject: [PATCH 11/14] mlog standardisation for app/import.go (#12741) * mlog standardisation for app/import.go * using the corrent mlog.Field type for the error --- app/import.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 From 1ad0cb646f1ebb7c0f2b3e62eacd935419c531de Mon Sep 17 00:00:00 2001 From: pqzx Date: Thu, 17 Oct 2019 23:36:17 +1100 Subject: [PATCH 12/14] =?UTF-8?q?Migrate=20tests=20from=20"store/storetest?= =?UTF-8?q?/session=5Fstore.go"=20to=20use=20t=E2=80=A6=20(#12805)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- store/storetest/session_store.go | 147 +++++++++++-------------------- 1 file changed, 49 insertions(+), 98 deletions(-) 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) { From ff160a4d3291589c6a51842ecd634270c1bcbb36 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Thu, 17 Oct 2019 09:55:02 -0300 Subject: [PATCH 13/14] MM-19048: avoid error spam during CI builds (#12626) Detect the IS_CI flag and skip trying to rewrite subpaths that won't necessarily exist. --- utils/subpath.go | 6 ++++++ utils/subpath_test.go | 42 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 4 deletions(-) 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.


` From fffcef0b00898a24b8dd3f8da6580d75f37eaf7b Mon Sep 17 00:00:00 2001 From: Peeyush Gupta Date: Thu, 17 Oct 2019 18:52:55 +0530 Subject: [PATCH 14/14] MM-19234 Migrate tests from model/team_member_test.go to use testify (#12642) --- model/team_member_test.go | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) 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") }