Merge branch 'master' into mark-as-unread

Makefile

@@ -87,7 +87,7 @@ PLUGIN_PACKAGES += mattermost-plugin-github-v0.10.2
 PLUGIN_PACKAGES += mattermost-plugin-welcomebot-v1.1.0
 PLUGIN_PACKAGES += mattermost-plugin-aws-SNS-v1.0.2
 PLUGIN_PACKAGES += mattermost-plugin-antivirus-v0.1.1
-PLUGIN_PACKAGES += mattermost-plugin-jira-v2.1.1
+PLUGIN_PACKAGES += mattermost-plugin-jira-v2.1.3
 PLUGIN_PACKAGES += mattermost-plugin-gitlab-v1.0.0
 PLUGIN_PACKAGES += mattermost-plugin-jenkins-v1.0.0
 
@@ -144,6 +144,7 @@ govet: ## Runs govet against all packages.
 	$(GO) get golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
 	$(GO) vet $(GOFLAGS) $(ALL_PACKAGES) || exit 1
 	$(GO) vet -vettool=$(GOPATH)/bin/shadow $(GOFLAGS) $(ALL_PACKAGES) || exit 1
+	$(GO) run plugin/checker/main.go
 
 gofmt: ## Runs gofmt against all packages.
 	@echo Running GOFMT

api4/apitestlib.go

@@ -9,7 +9,6 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"path/filepath"
 	"reflect"
 	"strconv"
 	"strings"
@@ -138,16 +137,6 @@ func setupTestHelper(enterprise bool, updateConfig func(*model.Config)) *TestHel
 		th.tempWorkspace = dir
 	}
 
-	pluginDir := filepath.Join(th.tempWorkspace, "plugins")
-	webappDir := filepath.Join(th.tempWorkspace, "webapp")
-
-	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.PluginSettings.Directory = pluginDir
-		*cfg.PluginSettings.ClientDirectory = webappDir
-	})
-
-	th.App.InitPlugins(pluginDir, webappDir)
-
 	return th
 }
 

api4/channel.go

@@ -35,6 +35,7 @@ func (api *API) InitChannel() {
 	api.BaseRoutes.Channel.Handle("", api.ApiSessionRequired(updateChannel)).Methods("PUT")
 	api.BaseRoutes.Channel.Handle("/patch", api.ApiSessionRequired(patchChannel)).Methods("PUT")
 	api.BaseRoutes.Channel.Handle("/convert", api.ApiSessionRequired(convertChannelToPrivate)).Methods("POST")
+	api.BaseRoutes.Channel.Handle("/privacy", api.ApiSessionRequired(updateChannelPrivacy)).Methods("PUT")
 	api.BaseRoutes.Channel.Handle("/restore", api.ApiSessionRequired(restoreChannel)).Methods("POST")
 	api.BaseRoutes.Channel.Handle("", api.ApiSessionRequired(deleteChannel)).Methods("DELETE")
 	api.BaseRoutes.Channel.Handle("/stats", api.ApiSessionRequired(getChannelStats)).Methods("GET")
@@ -229,6 +230,54 @@ func convertChannelToPrivate(c *Context, w http.ResponseWriter, r *http.Request)
 	w.Write([]byte(rchannel.ToJson()))
 }
 
+func updateChannelPrivacy(c *Context, w http.ResponseWriter, r *http.Request) {
+	c.RequireChannelId()
+	if c.Err != nil {
+		return
+	}
+
+	props := model.StringInterfaceFromJson(r.Body)
+	privacy, ok := props["privacy"].(string)
+	if !ok || (privacy != model.CHANNEL_OPEN && privacy != model.CHANNEL_PRIVATE) {
+		c.SetInvalidParam("privacy")
+		return
+	}
+
+	channel, err := c.App.GetChannel(c.Params.ChannelId)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	if !c.App.SessionHasPermissionToTeam(c.App.Session, channel.TeamId, model.PERMISSION_MANAGE_TEAM) {
+		c.SetPermissionError(model.PERMISSION_MANAGE_TEAM)
+		return
+	}
+
+	if channel.Name == model.DEFAULT_CHANNEL && privacy == model.CHANNEL_PRIVATE {
+		c.Err = model.NewAppError("updateChannelPrivacy", "api.channel.update_channel_privacy.default_channel_error", nil, "", http.StatusBadRequest)
+		return
+	}
+
+	user, err := c.App.GetUser(c.App.Session.UserId)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	channel.Type = privacy
+
+	updatedChannel, err := c.App.UpdateChannelPrivacy(channel, user)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	c.LogAudit("name=" + updatedChannel.Name)
+
+	w.Write([]byte(updatedChannel.ToJson()))
+}
+
 func patchChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 	c.RequireChannelId()
 	if c.Err != nil {

api4/channel_test.go

@@ -1387,6 +1387,68 @@ func TestConvertChannelToPrivate(t *testing.T) {
 	}
 }
 
+func TestUpdateChannelPrivacy(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+	Client := th.Client
+
+	type testTable []struct {
+		name            string
+		channel         *model.Channel
+		expectedPrivacy string
+	}
+
+	defaultChannel, _ := th.App.GetChannelByName(model.DEFAULT_CHANNEL, th.BasicTeam.Id, false)
+	privateChannel := th.CreatePrivateChannel()
+	publicChannel := th.CreatePublicChannel()
+
+	tt := testTable{
+		{"Updating default channel should fail with forbidden status if not logged in", defaultChannel, model.CHANNEL_OPEN},
+		{"Updating private channel should fail with forbidden status if not logged in", privateChannel, model.CHANNEL_PRIVATE},
+		{"Updating public channel should fail with forbidden status if not logged in", publicChannel, model.CHANNEL_OPEN},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			_, resp := Client.UpdateChannelPrivacy(tc.channel.Id, tc.expectedPrivacy)
+			CheckForbiddenStatus(t, resp)
+		})
+	}
+
+	th.LoginTeamAdmin()
+
+	tt = testTable{
+		{"Converting default channel to private should fail", defaultChannel, model.CHANNEL_PRIVATE},
+		{"Updating privacy to an invalid setting should fail", publicChannel, "invalid"},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			_, resp := Client.UpdateChannelPrivacy(tc.channel.Id, tc.expectedPrivacy)
+			CheckBadRequestStatus(t, resp)
+		})
+	}
+
+	tt = testTable{
+		{"Default channel should stay public", defaultChannel, model.CHANNEL_OPEN},
+		{"Public channel should stay public", publicChannel, model.CHANNEL_OPEN},
+		{"Private channel should stay private", privateChannel, model.CHANNEL_PRIVATE},
+		{"Public channel should convert to private", publicChannel, model.CHANNEL_PRIVATE},
+		{"Private channel should convert to public", privateChannel, model.CHANNEL_OPEN},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			updatedChannel, resp := Client.UpdateChannelPrivacy(tc.channel.Id, tc.expectedPrivacy)
+			CheckNoError(t, resp)
+			assert.Equal(t, updatedChannel.Type, tc.expectedPrivacy)
+			updatedChannel, err := th.App.GetChannel(tc.channel.Id)
+			require.Nil(t, err)
+			assert.Equal(t, updatedChannel.Type, tc.expectedPrivacy)
+		})
+	}
+}
+
 func TestRestoreChannel(t *testing.T) {
 	th := Setup().InitBasic()
 	defer th.TearDown()

api4/helpers.go

@@ -0,0 +1,25 @@
+// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package api4
+
+import (
+	"net/url"
+	"strconv"
+
+	"github.com/pkg/errors"
+)
+
+func parseInt(u *url.URL, name string, defaultValue int) (int, error) {
+	valueStr := u.Query().Get(name)
+	if valueStr == "" {
+		return defaultValue, nil
+	}
+
+	value, err := strconv.Atoi(valueStr)
+	if err != nil {
+		return 0, errors.Wrapf(err, "failed to parse %s as integer", name)
+	}
+
+	return value, nil
+}

api4/integration_action_test.go

@@ -142,8 +142,20 @@ func TestOpenDialog(t *testing.T) {
 	CheckBadRequestStatus(t, resp)
 	assert.False(t, pass)
 
-	// Should pass with no elements
+	// Should pass with markdown formatted introduction text
 	request.URL = "http://localhost:8065"
+	request.Dialog.IntroductionText = "**Some** _introduction text"
+	pass, resp = Client.OpenInteractiveDialog(request)
+	CheckNoError(t, resp)
+	assert.True(t, pass)
+
+	// Should pass with empty introduction text
+	request.Dialog.IntroductionText = ""
+	pass, resp = Client.OpenInteractiveDialog(request)
+	CheckNoError(t, resp)
+	assert.True(t, pass)
+
+	// Should pass with no elements
 	request.Dialog.Elements = nil
 	pass, resp = Client.OpenInteractiveDialog(request)
 	CheckNoError(t, resp)

api4/plugin.go

@@ -7,6 +7,7 @@ package api4
 
 import (
 	"bytes"
+	"encoding/json"
 	"io/ioutil"
 	"net/http"
 	"net/url"
@@ -36,6 +37,8 @@ func (api *API) InitPlugin() {
 	api.BaseRoutes.Plugin.Handle("/disable", api.ApiSessionRequired(disablePlugin)).Methods("POST")
 
 	api.BaseRoutes.Plugins.Handle("/webapp", api.ApiHandler(getWebappPlugins)).Methods("GET")
+
+	api.BaseRoutes.Plugins.Handle("/marketplace", api.ApiSessionRequired(getMarketplacePlugins)).Methods("GET")
 }
 
 func uploadPlugin(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -239,6 +242,43 @@ func getWebappPlugins(c *Context, w http.ResponseWriter, r *http.Request) {
 	w.Write([]byte(model.ManifestListToJson(clientManifests)))
 }
 
+func getMarketplacePlugins(c *Context, w http.ResponseWriter, r *http.Request) {
+	if !*c.App.Config().PluginSettings.Enable {
+		c.Err = model.NewAppError("getMarketplacePlugins", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented)
+		return
+	}
+
+	if !*c.App.Config().PluginSettings.EnableMarketplace {
+		c.Err = model.NewAppError("getMarketplacePlugins", "app.plugin.marketplace_disabled.app_error", nil, "", http.StatusNotImplemented)
+		return
+	}
+
+	if !c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_MANAGE_SYSTEM) {
+		c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM)
+		return
+	}
+
+	filter, err := parseMarketplacePluginFilter(r.URL)
+	if err != nil {
+		c.Err = model.NewAppError("getMarketplacePlugins", "app.plugin.marshal.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	plugins, appErr := c.App.GetMarketplacePlugins(filter)
+	if appErr != nil {
+		c.Err = appErr
+		return
+	}
+
+	json, err := json.Marshal(plugins)
+	if err != nil {
+		c.Err = model.NewAppError("getMarketplacePlugins", "app.plugin.marshal.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	w.Write(json)
+}
+
 func enablePlugin(c *Context, w http.ResponseWriter, r *http.Request) {
 	c.RequirePluginId()
 	if c.Err != nil {
@@ -286,3 +326,25 @@ func disablePlugin(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	ReturnStatusOK(w)
 }
+
+func parseMarketplacePluginFilter(u *url.URL) (*model.MarketplacePluginFilter, error) {
+	page, err := parseInt(u, "page", 0)
+	if err != nil {
+		return nil, err
+	}
+
+	perPage, err := parseInt(u, "per_page", 100)
+	if err != nil {
+		return nil, err
+	}
+
+	filter := u.Query().Get("filter")
+	serverVersion := u.Query().Get("server_version")
+
+	return &model.MarketplacePluginFilter{
+		Page:          page,
+		PerPage:       perPage,
+		Filter:        filter,
+		ServerVersion: serverVersion,
+	}, nil
+}

api4/plugin_test.go

@@ -11,6 +11,8 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"sort"
+	"strings"
 	"testing"
 	"time"
 
@@ -36,9 +38,7 @@ func TestPlugin(t *testing.T) {
 
 	path, _ := fileutils.FindDir("tests")
 	tarData, err := ioutil.ReadFile(filepath.Join(path, "testplugin.tar.gz"))
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatal(err)
-	}
 
 	// Install from URL
 	testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
@@ -270,9 +270,7 @@ func TestNotifyClusterPluginEvent(t *testing.T) {
 
 	path, _ := fileutils.FindDir("tests")
 	tarData, err := ioutil.ReadFile(filepath.Join(path, "testplugin.tar.gz"))
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatal(err)
-	}
 
 	testCluster.ClearMessages()
 
@@ -331,9 +329,7 @@ func TestNotifyClusterPluginEvent(t *testing.T) {
 func TestDisableOnRemove(t *testing.T) {
 	path, _ := fileutils.FindDir("tests")
 	tarData, err := ioutil.ReadFile(filepath.Join(path, "testplugin.tar.gz"))
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatal(err)
-	}
 
 	testCases := []struct {
 		Description string
@@ -432,6 +428,324 @@ func TestDisableOnRemove(t *testing.T) {
 	}
 }
 
+func TestGetMarketplacePlugins(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.PluginSettings.Enable = true
+		*cfg.PluginSettings.EnableUploads = true
+		*cfg.PluginSettings.EnableMarketplace = false
+	})
+
+	t.Run("marketplace disabled", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.PluginSettings.EnableMarketplace = false
+			*cfg.PluginSettings.MarketplaceUrl = "invalid.com"
+		})
+
+		plugins, resp := th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckNotImplementedStatus(t, resp)
+		require.Nil(t, plugins)
+	})
+
+	t.Run("no server", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.PluginSettings.EnableMarketplace = true
+			*cfg.PluginSettings.MarketplaceUrl = "invalid.com"
+		})
+
+		plugins, resp := th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckInternalErrorStatus(t, resp)
+		require.Nil(t, plugins)
+	})
+
+	t.Run("no permission", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.PluginSettings.EnableMarketplace = true
+			*cfg.PluginSettings.MarketplaceUrl = "invalid.com"
+		})
+
+		plugins, resp := th.Client.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckForbiddenStatus(t, resp)
+		require.Nil(t, plugins)
+	})
+
+	t.Run("empty response from server", func(t *testing.T) {
+		testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
+			res.WriteHeader(http.StatusOK)
+			json, err := json.Marshal([]*model.MarketplacePlugin{})
+			require.NoError(t, err)
+			res.Write(json)
+		}))
+		defer func() { testServer.Close() }()
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.PluginSettings.EnableMarketplace = true
+			*cfg.PluginSettings.MarketplaceUrl = testServer.URL
+		})
+
+		plugins, resp := th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckNoError(t, resp)
+		require.Len(t, plugins, 0)
+	})
+
+	t.Run("verify server version is passed through", func(t *testing.T) {
+		testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
+			serverVersion, ok := req.URL.Query()["server_version"]
+			require.True(t, ok)
+			require.Len(t, serverVersion, 1)
+			require.Equal(t, model.CurrentVersion, serverVersion[0])
+			require.NotEqual(t, 0, len(serverVersion[0]))
+
+			res.WriteHeader(http.StatusOK)
+			json, err := json.Marshal([]*model.MarketplacePlugin{})
+			require.NoError(t, err)
+			res.Write(json)
+		}))
+		defer func() { testServer.Close() }()
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.PluginSettings.EnableMarketplace = true
+			*cfg.PluginSettings.MarketplaceUrl = testServer.URL
+		})
+
+		plugins, resp := th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckNoError(t, resp)
+		require.Len(t, plugins, 0)
+	})
+}
+
+func TestGetInstalledMarketplacePlugins(t *testing.T) {
+	samplePlugins := []*model.MarketplacePlugin{
+		{
+			BaseMarketplacePlugin: &model.BaseMarketplacePlugin{
+				HomepageURL: "https://github.com/mattermost/mattermost-plugin-nps",
+				IconData:    "http://example.com/icon.svg",
+				DownloadURL: "https://github.com/mattermost/mattermost-plugin-nps/releases/download/v1.0.3/com.mattermost.nps-1.0.3.tar.gz",
+				Manifest: &model.Manifest{
+					Id:               "com.mattermost.nps",
+					Name:             "User Satisfaction Surveys",
+					Description:      "This plugin sends quarterly user satisfaction surveys to gather feedback and help improve Mattermost.",
+					Version:          "1.0.3",
+					MinServerVersion: "5.14.0",
+				},
+			},
+			InstalledVersion: "",
+		},
+	}
+
+	path, _ := fileutils.FindDir("tests")
+	tarData, err := ioutil.ReadFile(filepath.Join(path, "testplugin.tar.gz"))
+	require.NoError(t, err)
+
+	t.Run("marketplace client returns not-installed plugin", func(t *testing.T) {
+		th := Setup().InitBasic()
+		defer th.TearDown()
+
+		testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
+			res.WriteHeader(http.StatusOK)
+			json, err := json.Marshal(samplePlugins)
+			require.NoError(t, err)
+			res.Write(json)
+		}))
+		defer func() { testServer.Close() }()
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.PluginSettings.Enable = true
+			*cfg.PluginSettings.EnableUploads = true
+			*cfg.PluginSettings.EnableMarketplace = true
+			*cfg.PluginSettings.MarketplaceUrl = testServer.URL
+		})
+
+		plugins, resp := th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckNoError(t, resp)
+		require.Equal(t, samplePlugins, plugins)
+
+		manifest, resp := th.SystemAdminClient.UploadPlugin(bytes.NewReader(tarData))
+		CheckNoError(t, resp)
+
+		expectedPlugins := append(samplePlugins, &model.MarketplacePlugin{
+			BaseMarketplacePlugin: &model.BaseMarketplacePlugin{
+				HomepageURL: "",
+				IconData:    "",
+				DownloadURL: "",
+				Manifest:    manifest,
+			},
+			InstalledVersion: manifest.Version,
+		})
+		sort.SliceStable(expectedPlugins, func(i, j int) bool {
+			return strings.ToLower(expectedPlugins[i].Manifest.Name) < strings.ToLower(expectedPlugins[j].Manifest.Name)
+		})
+
+		plugins, resp = th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckNoError(t, resp)
+		require.Equal(t, expectedPlugins, plugins)
+
+		ok, resp := th.SystemAdminClient.RemovePlugin(manifest.Id)
+		CheckNoError(t, resp)
+		assert.True(t, ok)
+
+		plugins, resp = th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckNoError(t, resp)
+		require.Equal(t, samplePlugins, plugins)
+	})
+
+	t.Run("marketplace client returns installed plugin", func(t *testing.T) {
+		th := Setup().InitBasic()
+		defer th.TearDown()
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.PluginSettings.Enable = true
+			*cfg.PluginSettings.EnableUploads = true
+			*cfg.PluginSettings.EnableMarketplace = true
+		})
+
+		manifest, resp := th.SystemAdminClient.UploadPlugin(bytes.NewReader(tarData))
+		CheckNoError(t, resp)
+
+		newPlugin := &model.MarketplacePlugin{
+			BaseMarketplacePlugin: &model.BaseMarketplacePlugin{
+				HomepageURL: "HomepageURL",
+				IconData:    "IconData",
+				DownloadURL: "DownloadURL",
+				Manifest:    manifest,
+			},
+			InstalledVersion: manifest.Version,
+		}
+		expectedPlugins := append(samplePlugins, newPlugin)
+		sort.SliceStable(expectedPlugins, func(i, j int) bool {
+			return strings.ToLower(expectedPlugins[i].Manifest.Name) < strings.ToLower(expectedPlugins[j].Manifest.Name)
+		})
+
+		testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
+			res.WriteHeader(http.StatusOK)
+			json, err := json.Marshal([]*model.MarketplacePlugin{samplePlugins[0], newPlugin})
+			require.NoError(t, err)
+			res.Write(json)
+		}))
+		defer func() { testServer.Close() }()
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.PluginSettings.MarketplaceUrl = testServer.URL
+		})
+
+		plugins, resp := th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckNoError(t, resp)
+		require.Equal(t, expectedPlugins, plugins)
+
+		ok, resp := th.SystemAdminClient.RemovePlugin(manifest.Id)
+		CheckNoError(t, resp)
+		assert.True(t, ok)
+
+		plugins, resp = th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckNoError(t, resp)
+		newPlugin.InstalledVersion = manifest.Version
+		require.Equal(t, expectedPlugins, plugins)
+	})
+}
+
+func TestSearchGetMarketplacePlugins(t *testing.T) {
+	samplePlugins := []*model.MarketplacePlugin{
+		{
+			BaseMarketplacePlugin: &model.BaseMarketplacePlugin{
+				HomepageURL: "https://github.com/mattermost/mattermost-plugin-nps",
+				IconData:    "Cjxzdmcgdmlld0JveD0nMCAwIDEwNSA5MycgeG1sbnM9J2h0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnJz4KPHBhdGggZD0nTTY2LDBoMzl2OTN6TTM4LDBoLTM4djkzek01MiwzNWwyNSw1OGgtMTZsLTgtMThoLTE4eicgZmlsbD0nI0VEMUMyNCcvPgo8L3N2Zz4K",
+				DownloadURL: "https://github.com/mattermost/mattermost-plugin-nps/releases/download/v1.0.3/com.mattermost.nps-1.0.3.tar.gz",
+				Manifest: &model.Manifest{
+					Id:               "com.mattermost.nps",
+					Name:             "User Satisfaction Surveys",
+					Description:      "This plugin sends quarterly user satisfaction surveys to gather feedback and help improve Mattermost.",
+					Version:          "1.0.3",
+					MinServerVersion: "5.14.0",
+				},
+			},
+			InstalledVersion: "",
+		},
+	}
+
+	path, _ := fileutils.FindDir("tests")
+	tarData, err := ioutil.ReadFile(filepath.Join(path, "testplugin.tar.gz"))
+	require.NoError(t, err)
+
+	tarDataV2, err := ioutil.ReadFile(filepath.Join(path, "testpluginv2.tar.gz"))
+	require.NoError(t, err)
+
+	t.Run("search installed plugin", func(t *testing.T) {
+		th := Setup().InitBasic()
+		defer th.TearDown()
+
+		testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
+			res.WriteHeader(http.StatusOK)
+			json, err := json.Marshal(samplePlugins)
+			require.NoError(t, err)
+			res.Write(json)
+		}))
+		defer func() { testServer.Close() }()
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.PluginSettings.Enable = true
+			*cfg.PluginSettings.EnableUploads = true
+			*cfg.PluginSettings.EnableMarketplace = true
+			*cfg.PluginSettings.MarketplaceUrl = testServer.URL
+		})
+
+		plugins, resp := th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckNoError(t, resp)
+		require.Equal(t, samplePlugins, plugins)
+
+		manifest, resp := th.SystemAdminClient.UploadPlugin(bytes.NewReader(tarData))
+		CheckNoError(t, resp)
+
+		newPluginV1 := &model.MarketplacePlugin{
+			BaseMarketplacePlugin: &model.BaseMarketplacePlugin{
+				HomepageURL: "",
+				IconData:    "",
+				DownloadURL: "",
+				Manifest:    manifest,
+			},
+			InstalledVersion: manifest.Version,
+		}
+		expectedPlugins := append(samplePlugins, newPluginV1)
+
+		manifest, resp = th.SystemAdminClient.UploadPlugin(bytes.NewReader(tarDataV2))
+		CheckNoError(t, resp)
+		newPluginV2 := &model.MarketplacePlugin{
+			BaseMarketplacePlugin: &model.BaseMarketplacePlugin{
+				HomepageURL: "",
+				IconData:    "",
+				DownloadURL: "",
+				Manifest:    manifest,
+			},
+			InstalledVersion: manifest.Version,
+		}
+		expectedPlugins = append(expectedPlugins, newPluginV2)
+		sort.SliceStable(expectedPlugins, func(i, j int) bool {
+			return strings.ToLower(expectedPlugins[i].Manifest.Name) < strings.ToLower(expectedPlugins[j].Manifest.Name)
+		})
+
+		plugins, resp = th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{})
+		CheckNoError(t, resp)
+		require.Equal(t, expectedPlugins, plugins)
+
+		// Search for plugins from the server
+		plugins, resp = th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{Filter: "testplugin_v2"})
+		CheckNoError(t, resp)
+		require.Equal(t, []*model.MarketplacePlugin{newPluginV2}, plugins)
+
+		plugins, resp = th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{Filter: "dsgsdg_v2"})
+		CheckNoError(t, resp)
+		require.Equal(t, []*model.MarketplacePlugin{newPluginV2}, plugins)
+
+		plugins, resp = th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{Filter: "User Satisfaction Surveys"})
+		CheckNoError(t, resp)
+		require.Equal(t, samplePlugins, plugins)
+
+		plugins, resp = th.SystemAdminClient.GetMarketplacePlugins(&model.MarketplacePluginFilter{Filter: "NOFILTER"})
+		CheckNoError(t, resp)
+		require.Nil(t, plugins)
+	})
+}
+
 func findClusterMessages(event string, msgs []*model.ClusterMessage) []*model.ClusterMessage {
 	var result []*model.ClusterMessage
 	for _, msg := range msgs {

api4/post.go

@@ -134,6 +134,10 @@ func getPostsForChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 			return
 		}
 	}
+	skipFetchThreads := false
+	if r.URL.Query().Get("fetchThreads") == "false" {
+		skipFetchThreads = true
+	}
 
 	channelId := c.Params.ChannelId
 	page := c.Params.Page
@@ -149,7 +153,7 @@ func getPostsForChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 	etag := ""
 
 	if since > 0 {
-		list, err = c.App.GetPostsSince(channelId, since)
+		list, err = c.App.GetPostsSince(model.GetPostsSinceOptions{ChannelId: channelId, Time: since, SkipFetchThreads: skipFetchThreads})
 	} else if len(afterPost) > 0 {
 		etag = c.App.GetPostsEtag(channelId)
 
@@ -157,7 +161,7 @@ func getPostsForChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		list, err = c.App.GetPostsAfterPost(channelId, afterPost, page, perPage)
+		list, err = c.App.GetPostsAfterPost(model.GetPostsOptions{ChannelId: channelId, PostId: afterPost, Page: page, PerPage: perPage, SkipFetchThreads: skipFetchThreads})
 	} else if len(beforePost) > 0 {
 		etag = c.App.GetPostsEtag(channelId)
 
@@ -165,7 +169,7 @@ func getPostsForChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		list, err = c.App.GetPostsBeforePost(channelId, beforePost, page, perPage)
+		list, err = c.App.GetPostsBeforePost(model.GetPostsOptions{ChannelId: channelId, PostId: beforePost, Page: page, PerPage: perPage, SkipFetchThreads: skipFetchThreads})
 	} else {
 		etag = c.App.GetPostsEtag(channelId)
 
@@ -173,7 +177,7 @@ func getPostsForChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		list, err = c.App.GetPostsPage(channelId, page, perPage)
+		list, err = c.App.GetPostsPage(model.GetPostsOptions{ChannelId: channelId, Page: page, PerPage: perPage, SkipFetchThreads: skipFetchThreads})
 	}
 
 	if err != nil {
@@ -209,7 +213,11 @@ func getPostsForChannelAroundLastUnread(c *Context, w http.ResponseWriter, r *ht
 		return
 	}
 
-	postList, err := c.App.GetPostsForChannelAroundLastUnread(channelId, userId, c.Params.LimitBefore, c.Params.LimitAfter)
+	skipFetchThreads := false
+	if r.URL.Query().Get("fetchThreads") == "false" {
+		skipFetchThreads = true
+	}
+	postList, err := c.App.GetPostsForChannelAroundLastUnread(channelId, userId, c.Params.LimitBefore, c.Params.LimitAfter, skipFetchThreads)
 	if err != nil {
 		c.Err = err
 		return
@@ -223,7 +231,11 @@ func getPostsForChannelAroundLastUnread(c *Context, w http.ResponseWriter, r *ht
 			return
 		}
 
-		postList, err = c.App.GetPostsPage(channelId, app.PAGE_DEFAULT, c.Params.LimitBefore)
+		postList, err = c.App.GetPostsPage(model.GetPostsOptions{ChannelId: channelId, Page: app.PAGE_DEFAULT, PerPage: c.Params.LimitBefore, SkipFetchThreads: skipFetchThreads})
+		if err != nil {
+			c.Err = err
+			return
+		}
 	}
 
 	postList.NextPostId = c.App.GetNextPostIdFromPostList(postList)

api4/system.go

@@ -56,7 +56,7 @@ func getSystemPing(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	actualGoroutines := runtime.NumGoroutine()
 	if *c.App.Config().ServiceSettings.GoroutineHealthThreshold > 0 && actualGoroutines >= *c.App.Config().ServiceSettings.GoroutineHealthThreshold {
-		mlog.Warn(fmt.Sprintf("The number of running goroutines (%v) is over the health threshold (%v)", actualGoroutines, *c.App.Config().ServiceSettings.GoroutineHealthThreshold))
+		mlog.Warn("The number of running goroutines is over the health threshold", mlog.Int("goroutines", actualGoroutines), mlog.Int("health_threshold", *c.App.Config().ServiceSettings.GoroutineHealthThreshold))
 		s[model.STATUS] = model.STATUS_UNHEALTHY
 	}
 
@@ -76,17 +76,17 @@ func getSystemPing(c *Context, w http.ResponseWriter, r *http.Request) {
 			Value: currentTime,
 		})
 		if writeErr != nil {
-			mlog.Debug(fmt.Sprintf("Unable to write to database: %s", writeErr.Error()))
+			mlog.Debug("Unable to write to database.", mlog.Err(writeErr))
 			s[dbStatusKey] = model.STATUS_UNHEALTHY
 			s[model.STATUS] = model.STATUS_UNHEALTHY
 		} else {
 			healthCheck, readErr := c.App.Srv.Store.System().GetByName(healthCheckKey)
 			if readErr != nil {
-				mlog.Debug(fmt.Sprintf("Unable to read from database: %s", readErr.Error()))
+				mlog.Debug("Unable to read from database.", mlog.Err(readErr))
 				s[dbStatusKey] = model.STATUS_UNHEALTHY
 				s[model.STATUS] = model.STATUS_UNHEALTHY
 			} else if healthCheck.Value != currentTime {
-				mlog.Debug(fmt.Sprintf("Incorrect healthcheck value, expected %s, got %s", currentTime, healthCheck.Value))
+				mlog.Debug("Incorrect healthcheck value", mlog.String("expected", currentTime), mlog.String("got", healthCheck.Value))
 				s[dbStatusKey] = model.STATUS_UNHEALTHY
 				s[model.STATUS] = model.STATUS_UNHEALTHY
 			} else {
@@ -105,7 +105,7 @@ func getSystemPing(c *Context, w http.ResponseWriter, r *http.Request) {
 				s[model.STATUS] = model.STATUS_UNHEALTHY
 			}
 		} else {
-			mlog.Debug(fmt.Sprintf("Unable to get filestore for ping status: %s", appErr.Error()))
+			mlog.Debug("Unable to get filestore for ping status.", mlog.Err(appErr))
 			s[filestoreStatusKey] = model.STATUS_UNHEALTHY
 			s[model.STATUS] = model.STATUS_UNHEALTHY
 		}
@@ -269,7 +269,7 @@ func postLog(c *Context, w http.ResponseWriter, r *http.Request) {
 		err.Where = "client"
 		c.LogError(err)
 	} else {
-		mlog.Debug(fmt.Sprint(msg))
+		mlog.Debug("message", mlog.String("message", msg))
 	}
 
 	m["message"] = msg

api4/user.go

@@ -557,7 +557,7 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		profiles, err = c.App.GetUsersWithoutTeamPage(c.Params.Page, c.Params.PerPage, c.IsSystemAdmin(), restrictions)
+		profiles, err = c.App.GetUsersWithoutTeamPage(userGetOptions, c.IsSystemAdmin())
 	} else if len(notInChannelId) > 0 {
 		if !c.App.SessionHasPermissionToChannel(c.App.Session, notInChannelId, model.PERMISSION_READ_CHANNEL) {
 			c.SetPermissionError(model.PERMISSION_READ_CHANNEL)

api4/websocket.go

@@ -4,7 +4,6 @@
 package api4
 
 import (
-	"fmt"
 	"net/http"
 
 	"github.com/gorilla/websocket"
@@ -25,7 +24,7 @@ func connectWebSocket(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	ws, err := upgrader.Upgrade(w, r, nil)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("websocket connect err: %v", err))
+		mlog.Error("websocket connect err.", mlog.Err(err))
 		c.Err = model.NewAppError("connect", "api.web_socket.connect.upgrade.app_error", nil, "", http.StatusInternalServerError)
 		return
 	}

app/analytics.go

@@ -4,8 +4,6 @@
 package app
 
 import (
-	"fmt"
-
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/store"
@@ -25,7 +23,7 @@ func (a *App) GetAnalytics(name string, teamId string) (model.AnalyticsRows, *mo
 	}
 
 	if systemUserCount > int64(*a.Config().AnalyticsSettings.MaxUsersForStatistics) {
-		mlog.Debug(fmt.Sprintf("More than %v users on the system, intensive queries skipped", *a.Config().AnalyticsSettings.MaxUsersForStatistics))
+		mlog.Debug("More than limit users are on the system, intensive queries skipped", mlog.Int("limit", *a.Config().AnalyticsSettings.MaxUsersForStatistics))
 		skipIntensiveQueries = true
 	}
 

app/auto_posts.go

@@ -66,6 +66,10 @@ func (cfg *AutoPostCreator) UploadTestFile() ([]string, bool) {
 }
 
 func (cfg *AutoPostCreator) CreateRandomPost() (*model.Post, bool) {
+	return cfg.CreateRandomPostNested("", "")
+}
+
+func (cfg *AutoPostCreator) CreateRandomPostNested(parentId, rootId string) (*model.Post, bool) {
 	var fileIds []string
 	if cfg.HasImage {
 		var err1 bool
@@ -84,6 +88,8 @@ func (cfg *AutoPostCreator) CreateRandomPost() (*model.Post, bool) {
 
 	post := &model.Post{
 		ChannelId: cfg.channelid,
+		ParentId:  parentId,
+		RootId:    rootId,
 		Message:   postText,
 		FileIds:   fileIds}
 	rpost, err2 := cfg.client.CreatePost(post)

app/channel_test.go

@@ -526,7 +526,7 @@ func TestAddChannelMemberNoUserRequestor(t *testing.T) {
 	}
 	assert.Equal(t, groupUserIds, channelMemberHistoryUserIds)
 
-	postList, err := th.App.Srv.Store.Post().GetPosts(channel.Id, 0, 1, false)
+	postList, err := th.App.Srv.Store.Post().GetPosts(model.GetPostsOptions{ChannelId: channel.Id, Page: 0, PerPage: 1}, false)
 	require.Nil(t, err)
 
 	if assert.Len(t, postList.Order, 1) {

app/cluster_discovery.go

@@ -4,7 +4,6 @@
 package app
 
 import (
-	"fmt"
 	"time"
 
 	"github.com/mattermost/mattermost-server/mlog"
@@ -34,41 +33,41 @@ func (a *App) NewClusterDiscoveryService() *ClusterDiscoveryService {
 func (me *ClusterDiscoveryService) Start() {
 	err := me.app.Srv.Store.ClusterDiscovery().Cleanup()
 	if err != nil {
-		mlog.Error(fmt.Sprintf("ClusterDiscoveryService failed to cleanup the outdated cluster discovery information err=%v", err))
+		mlog.Error("ClusterDiscoveryService failed to cleanup the outdated cluster discovery information", mlog.Err(err))
 	}
 
 	exists, err := me.app.Srv.Store.ClusterDiscovery().Exists(&me.ClusterDiscovery)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("ClusterDiscoveryService failed to check if row exists for %v with err=%v", me.ClusterDiscovery.ToJson(), err))
+		mlog.Error("ClusterDiscoveryService failed to check if row exists", mlog.String("ClusterDiscovery", me.ClusterDiscovery.ToJson()), mlog.Err(err))
 	} else {
 		if exists {
 			if _, err := me.app.Srv.Store.ClusterDiscovery().Delete(&me.ClusterDiscovery); err != nil {
-				mlog.Error(fmt.Sprintf("ClusterDiscoveryService failed to start clean for %v with err=%v", me.ClusterDiscovery.ToJson(), err))
+				mlog.Error("ClusterDiscoveryService failed to start clean", mlog.String("ClusterDiscovery", me.ClusterDiscovery.ToJson()), mlog.Err(err))
 			}
 		}
 	}
 
 	if err := me.app.Srv.Store.ClusterDiscovery().Save(&me.ClusterDiscovery); err != nil {
-		mlog.Error(fmt.Sprintf("ClusterDiscoveryService failed to save for %v with err=%v", me.ClusterDiscovery.ToJson(), err))
+		mlog.Error("ClusterDiscoveryService failed to save", mlog.String("ClusterDiscovery", me.ClusterDiscovery.ToJson()), mlog.Err(err))
 		return
 	}
 
 	go func() {
-		mlog.Debug(fmt.Sprintf("ClusterDiscoveryService ping writer started for %v", me.ClusterDiscovery.ToJson()))
+		mlog.Debug("ClusterDiscoveryService ping writer started", mlog.String("ClusterDiscovery", me.ClusterDiscovery.ToJson()))
 		ticker := time.NewTicker(DISCOVERY_SERVICE_WRITE_PING)
 		defer func() {
 			ticker.Stop()
 			if _, err := me.app.Srv.Store.ClusterDiscovery().Delete(&me.ClusterDiscovery); err != nil {
-				mlog.Error(fmt.Sprintf("ClusterDiscoveryService failed to cleanup for %v with err=%v", me.ClusterDiscovery.ToJson(), err))
+				mlog.Error("ClusterDiscoveryService failed to cleanup", mlog.String("ClusterDiscovery", me.ClusterDiscovery.ToJson()), mlog.Err(err))
 			}
-			mlog.Debug(fmt.Sprintf("ClusterDiscoveryService ping writer stopped for %v", me.ClusterDiscovery.ToJson()))
+			mlog.Debug("ClusterDiscoveryService ping writer stopped", mlog.String("ClusterDiscovery", me.ClusterDiscovery.ToJson()))
 		}()
 
 		for {
 			select {
 			case <-ticker.C:
 				if err := me.app.Srv.Store.ClusterDiscovery().SetLastPingAt(&me.ClusterDiscovery); err != nil {
-					mlog.Error(fmt.Sprintf("ClusterDiscoveryService failed to write ping for %v with err=%v", me.ClusterDiscovery.ToJson(), err))
+					mlog.Error("ClusterDiscoveryService failed to write ping", mlog.String("ClusterDiscovery", me.ClusterDiscovery.ToJson()), mlog.Err(err))
 				}
 			case <-me.stop:
 				return

app/command_groupmsg.go

@@ -47,14 +47,26 @@ func (me *groupmsgProvider) DoCommand(a *App, args *model.CommandArgs, message s
 	for _, username := range users {
 		username = strings.TrimSpace(username)
 		username = strings.TrimPrefix(username, "@")
-		if targetUser, err := a.Srv.Store.User().GetByUsername(username); err != nil {
+		targetUser, err := a.Srv.Store.User().GetByUsername(username)
+		if err != nil {
 			invalidUsernames = append(invalidUsernames, username)
-		} else {
+			continue
-			_, exists := targetUsers[targetUser.Id]
+		}
-			if !exists && targetUser.Id != args.UserId {
+
-				targetUsers[targetUser.Id] = targetUser
+		canSee, err := a.UserCanSeeOtherUser(args.UserId, targetUser.Id)
-				targetUsersSlice = append(targetUsersSlice, targetUser.Id)
+		if err != nil {
-			}
+			return &model.CommandResponse{Text: args.T("api.command_groupmsg.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+		}
+
+		if !canSee {
+			invalidUsernames = append(invalidUsernames, username)
+			continue
+		}
+
+		_, exists := targetUsers[targetUser.Id]
+		if !exists && targetUser.Id != args.UserId {
+			targetUsers[targetUser.Id] = targetUser
+			targetUsersSlice = append(targetUsersSlice, targetUser.Id)
 		}
 	}
 

app/command_groupmsg_test.go

@@ -52,46 +52,67 @@ func TestGroupMsgProvider(t *testing.T) {
 	th.LinkUserToTeam(th.BasicUser, team)
 	cmd := &groupmsgProvider{}
 
-	// Check without permission to create a GM channel.
+	t.Run("Check without permission to create a GM channel.", func(t *testing.T) {
-	resp := cmd.DoCommand(th.App, &model.CommandArgs{
+		resp := cmd.DoCommand(th.App, &model.CommandArgs{
-		T:       i18n.IdentityTfunc(),
+			T:       i18n.IdentityTfunc(),
-		SiteURL: "http://test.url",
+			SiteURL: "http://test.url",
-		TeamId:  team.Id,
+			TeamId:  team.Id,
-		UserId:  th.BasicUser.Id,
+			UserId:  th.BasicUser.Id,
-		Session: model.Session{
+			Session: model.Session{
-			Roles: "",
+				Roles: "",
-		},
+			},
-	}, targetUsers+"hello")
+		}, targetUsers+"hello")
 
-	channelName := model.GetGroupNameFromUserIds([]string{th.BasicUser.Id, th.BasicUser2.Id, user3.Id})
+		assert.Equal(t, "api.command_groupmsg.permission.app_error", resp.Text)
-	assert.Equal(t, "api.command_groupmsg.permission.app_error", resp.Text)
+		assert.Equal(t, "", resp.GotoLocation)
-	assert.Equal(t, "", resp.GotoLocation)
+	})
 
-	// Check with permission to create a GM channel.
+	t.Run("Check without permissions to view a user in the list.", func(t *testing.T) {
-	resp = cmd.DoCommand(th.App, &model.CommandArgs{
+		th.RemovePermissionFromRole(model.PERMISSION_VIEW_MEMBERS.Id, model.SYSTEM_USER_ROLE_ID)
-		T:       i18n.IdentityTfunc(),
+		defer th.AddPermissionToRole(model.PERMISSION_VIEW_MEMBERS.Id, model.SYSTEM_USER_ROLE_ID)
-		SiteURL: "http://test.url",
+		resp := cmd.DoCommand(th.App, &model.CommandArgs{
-		TeamId:  team.Id,
+			T:       i18n.IdentityTfunc(),
-		UserId:  th.BasicUser.Id,
+			SiteURL: "http://test.url",
-		Session: model.Session{
+			TeamId:  team.Id,
-			Roles: model.SYSTEM_USER_ROLE_ID,
+			UserId:  th.BasicUser.Id,
-		},
+			Session: model.Session{
-	}, targetUsers+"hello")
+				Roles: model.SYSTEM_USER_ROLE_ID,
+			},
+		}, targetUsers+"hello")
 
-	assert.Equal(t, "", resp.Text)
+		assert.Equal(t, "api.command_groupmsg.invalid_user.app_error", resp.Text)
-	assert.Equal(t, "http://test.url/"+team.Name+"/channels/"+channelName, resp.GotoLocation)
+		assert.Equal(t, "", resp.GotoLocation)
+	})
 
-	// Check without permission to post to an existing GM channel.
+	t.Run("Check with permission to create a GM channel.", func(t *testing.T) {
-	resp = cmd.DoCommand(th.App, &model.CommandArgs{
+		resp := cmd.DoCommand(th.App, &model.CommandArgs{
-		T:       i18n.IdentityTfunc(),
+			T:       i18n.IdentityTfunc(),
-		SiteURL: "http://test.url",
+			SiteURL: "http://test.url",
-		TeamId:  team.Id,
+			TeamId:  team.Id,
-		UserId:  th.BasicUser.Id,
+			UserId:  th.BasicUser.Id,
-		Session: model.Session{
+			Session: model.Session{
-			Roles: "",
+				Roles: model.SYSTEM_USER_ROLE_ID,
-		},
+			},
-	}, targetUsers+"hello")
+		}, targetUsers+"hello")
 
-	assert.Equal(t, "", resp.Text)
+		channelName := model.GetGroupNameFromUserIds([]string{th.BasicUser.Id, th.BasicUser2.Id, user3.Id})
-	assert.Equal(t, "http://test.url/"+team.Name+"/channels/"+channelName, resp.GotoLocation)
+		assert.Equal(t, "", resp.Text)
+		assert.Equal(t, "http://test.url/"+team.Name+"/channels/"+channelName, resp.GotoLocation)
+	})
+
+	t.Run("Check without permission to post to an existing GM channel.", func(t *testing.T) {
+		resp := cmd.DoCommand(th.App, &model.CommandArgs{
+			T:       i18n.IdentityTfunc(),
+			SiteURL: "http://test.url",
+			TeamId:  team.Id,
+			UserId:  th.BasicUser.Id,
+			Session: model.Session{
+				Roles: "",
+			},
+		}, targetUsers+"hello")
+
+		channelName := model.GetGroupNameFromUserIds([]string{th.BasicUser.Id, th.BasicUser2.Id, user3.Id})
+		assert.Equal(t, "", resp.Text)
+		assert.Equal(t, "http://test.url/"+team.Name+"/channels/"+channelName, resp.GotoLocation)
+	})
 }

app/command_loadtest.go

@@ -39,6 +39,9 @@ var usage = `Mattermost testing commands to help configure the system
 		Example:
 			/test channels fuzz 5 10
 
+	ThreadedPost - create a large threaded post
+        /test threaded_post
+
 	Posts - Add some random posts with fuzz text to current channel.
 		/test posts [fuzz] <Min Posts> <Max Posts> <Max Images>
 
@@ -127,6 +130,10 @@ func (me *LoadTestProvider) DoCommand(a *App, args *model.CommandArgs, message s
 		return me.PostCommand(a, args, message)
 	}
 
+	if strings.HasPrefix(message, "threaded_post") {
+		return me.ThreadedPostCommand(a, args, message)
+	}
+
 	if strings.HasPrefix(message, "url") {
 		return me.UrlCommand(a, args, message)
 	}
@@ -277,6 +284,34 @@ func (me *LoadTestProvider) ChannelsCommand(a *App, args *model.CommandArgs, mes
 	return &model.CommandResponse{Text: "Added channels", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
 }
 
+func (me *LoadTestProvider) ThreadedPostCommand(a *App, args *model.CommandArgs, message string) *model.CommandResponse {
+	var usernames []string
+	options := &model.UserGetOptions{InTeamId: args.TeamId, Page: 0, PerPage: 1000}
+	if profileUsers, err := a.Srv.Store.User().GetProfiles(options); err == nil {
+		usernames = make([]string, len(profileUsers))
+		i := 0
+		for _, userprof := range profileUsers {
+			usernames[i] = userprof.Username
+			i++
+		}
+	}
+
+	client := model.NewAPIv4Client(args.SiteURL)
+	client.MockSession(args.Session.Token)
+	testPoster := NewAutoPostCreator(client, args.ChannelId)
+	testPoster.Fuzzy = true
+	testPoster.Users = usernames
+	rpost, ok := testPoster.CreateRandomPost()
+	if !ok {
+		return &model.CommandResponse{Text: "Cannot create a post", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+	}
+	for i := 0; i < 1000; i++ {
+		testPoster.CreateRandomPostNested(rpost.Id, rpost.Id)
+	}
+
+	return &model.CommandResponse{Text: "Added threaded post", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+}
+
 func (me *LoadTestProvider) PostsCommand(a *App, args *model.CommandArgs, message string) *model.CommandResponse {
 	cmd := strings.TrimSpace(strings.TrimPrefix(message, "posts"))
 

app/diagnostics.go

@@ -602,6 +602,8 @@ func (a *App) trackConfig() {
 		"enable_uploads":                *cfg.PluginSettings.EnableUploads,
 		"allow_insecure_download_url":   *cfg.PluginSettings.AllowInsecureDownloadUrl,
 		"enable_health_check":           *cfg.PluginSettings.EnableHealthCheck,
+		"enable_marketplace":            *cfg.PluginSettings.EnableMarketplace,
+		"is_default_marketplace_url":    isDefault(*cfg.PluginSettings.MarketplaceUrl, model.PLUGIN_SETTINGS_DEFAULT_MARKETPLACE_URL),
 	})
 
 	a.SendDiagnostic(TRACK_CONFIG_DATA_RETENTION, map[string]interface{}{

app/diagnostics_test.go

@@ -68,7 +68,7 @@ func TestDiagnostics(t *testing.T) {
 	case identifyMessage := <-data:
 		require.Contains(t, identifyMessage, diagnosticID)
 	case <-time.After(time.Second * 1):
-		t.Fatal("Did not receive ID message")
+		require.Fail(t,"Did not receive ID message")
 	}
 
 	t.Run("Send", func(t *testing.T) {
@@ -80,7 +80,7 @@ func TestDiagnostics(t *testing.T) {
 		case result := <-data:
 			require.Contains(t, result, testValue)
 		case <-time.After(time.Second * 1):
-			t.Fatal("Did not receive diagnostic")
+			require.Fail(t,"Did not receive diagnostic")
 		}
 	})
 
@@ -137,7 +137,7 @@ func TestDiagnostics(t *testing.T) {
 
 		select {
 		case <-data:
-			t.Fatal("Should not send diagnostics when the segment key is not set")
+			require.Fail(t,"Should not send diagnostics when the segment key is not set")
 		case <-time.After(time.Second * 1):
 			// Did not receive diagnostics
 		}
@@ -150,7 +150,7 @@ func TestDiagnostics(t *testing.T) {
 
 		select {
 		case <-data:
-			t.Fatal("Should not send diagnostics when they are disabled")
+			require.Fail(t,"Should not send diagnostics when they are disabled")
 		case <-time.After(time.Second * 1):
 			// Did not receive diagnostics
 		}

app/email_batching.go

@@ -142,7 +142,7 @@ func (job *EmailBatchingJob) checkPendingNotifications(now time.Time, handler fu
 
 			team, err := job.server.Store.Team().GetByName(notifications[0].teamName)
 			if err != nil {
-				mlog.Error(fmt.Sprint("Unable to find Team id for notification", err))
+				mlog.Error("Unable to find Team id for notification", mlog.Err(err))
 				continue
 			}
 
@@ -154,13 +154,13 @@ func (job *EmailBatchingJob) checkPendingNotifications(now time.Time, handler fu
 			// all queued notifications
 			channelMembers, err := job.server.Store.Channel().GetMembersForUser(inspectedTeamNames[notification.teamName], userId)
 			if err != nil {
-				mlog.Error(fmt.Sprint("Unable to find ChannelMembers for user", err))
+				mlog.Error("Unable to find ChannelMembers for user", mlog.Err(err))
 				continue
 			}
 
 			for _, channelMember := range *channelMembers {
 				if channelMember.LastViewedAt >= batchStartTime {
-					mlog.Debug(fmt.Sprintf("Deleted notifications for user %s", userId), mlog.String("user_id", userId))
+					mlog.Debug("Deleted notifications for user", mlog.String("user_id", userId))
 					delete(job.pendingNotifications, userId)
 					break
 				}
@@ -241,7 +241,7 @@ func (s *Server) sendBatchedEmailNotification(userId string, notifications []*ba
 	body.Props["BodyText"] = translateFunc("api.email_batching.send_batched_email_notification.body_text", len(notifications))
 
 	if err := s.FakeApp().SendNotificationMail(user.Email, subject, body.Render()); err != nil {
-		mlog.Warn(fmt.Sprintf("Unable to send batched email notification err=%v", err), mlog.String("email", user.Email))
+		mlog.Warn("Unable to send batched email notification", mlog.String("email", user.Email), mlog.Err(err))
 	}
 }
 

app/emoji.go

@@ -5,7 +5,6 @@ package app
 
 import (
 	"bytes"
-	"fmt"
 	"image"
 	"image/draw"
 	"image/gif"
@@ -285,13 +284,12 @@ func imageToPaletted(img image.Image) *image.Paletted {
 
 func (a *App) deleteEmojiImage(id string) {
 	if err := a.MoveFile(getEmojiImagePath(id), "emoji/"+id+"/image_deleted"); err != nil {
-		mlog.Error(fmt.Sprintf("Failed to rename image when deleting emoji %v", id))
+		mlog.Error("Failed to rename image when deleting emoji", mlog.String("emoji_id", id))
 	}
 }
 
 func (a *App) deleteReactionsForEmoji(emojiName string) {
 	if err := a.Srv.Store.Reaction().DeleteAllWithEmojiName(emojiName); err != nil {
-		mlog.Warn(fmt.Sprintf("Unable to delete reactions when deleting emoji with emoji name %v", emojiName))
+		mlog.Warn("Unable to delete reactions when deleting emoji", mlog.String("emoji_name", emojiName), mlog.Err(err))
-		mlog.Warn(fmt.Sprint(err))
 	}
 }

app/export_test.go

@@ -115,9 +115,8 @@ func TestDirCreationForEmoji(t *testing.T) {
 
 	pathToDir := th.App.createDirForEmoji("test.json", "exported_emoji_test")
 	defer os.Remove(pathToDir)
-	if _, err := os.Stat(pathToDir); os.IsNotExist(err) {
+	_, err := os.Stat(pathToDir)
-		t.Fatal("Directory exported_emoji_test should exist")
+	require.False(t, os.IsNotExist(err), "Directory exported_emoji_test should exist")
-	}
 }
 
 func TestCopyEmojiImages(t *testing.T) {
@@ -147,13 +146,10 @@ func TestCopyEmojiImages(t *testing.T) {
 	defer os.RemoveAll(filePath)
 
 	copyError := th.App.copyEmojiImages(emoji.Id, emojiImagePath, pathToDir)
-	if copyError != nil {
+	require.Nil(t, copyError)
-		t.Fatal(copyError)
-	}
 
-	if _, err := os.Stat(pathToDir + "/" + emoji.Id + "/image"); os.IsNotExist(err) {
+	_, err = os.Stat(pathToDir + "/" + emoji.Id + "/image")
-		t.Fatal("File should exist ", err)
+	require.False(t, os.IsNotExist(err), "File should exist ")
-	}
 }
 
 func TestExportCustomEmoji(t *testing.T) {
@@ -170,9 +166,8 @@ func TestExportCustomEmoji(t *testing.T) {
 	dirNameToExportEmoji := "exported_emoji_test"
 	defer os.RemoveAll("../" + dirNameToExportEmoji)
 
-	if err := th.App.ExportCustomEmoji(fileWriter, filePath, pathToEmojiDir, dirNameToExportEmoji); err != nil {
+	err = th.App.ExportCustomEmoji(fileWriter, filePath, pathToEmojiDir, dirNameToExportEmoji)
-		t.Fatal(err)
+	require.Nil(t, err, "should not have failed")
-	}
 }
 
 func TestExportAllUsers(t *testing.T) {

app/file.go

@@ -139,7 +139,11 @@ func (a *App) GetInfoForFilename(post *model.Post, teamId string, filename strin
 	// Find the path from the Filename of the form /{channelId}/{userId}/{uid}/{nameWithExtension}
 	split := strings.SplitN(filename, "/", 5)
 	if len(split) < 5 {
-		mlog.Error("Unable to decipher filename when migrating post to use FileInfos", mlog.String("post_id", post.Id), mlog.String("filename", filename))
+		mlog.Error(
+			"Unable to decipher filename when migrating post to use FileInfos",
+			mlog.String("post_id", post.Id),
+			mlog.String("filename", filename),
+		)
 		return nil
 	}
 
@@ -176,9 +180,10 @@ func (a *App) GetInfoForFilename(post *model.Post, teamId string, filename strin
 	info, err := model.GetInfoForBytes(name, data)
 	if err != nil {
 		mlog.Warn(
-			fmt.Sprintf("Unable to fully decode file info when migrating post to use FileInfos, err=%v", err),
+			"Unable to fully decode file info when migrating post to use FileInfos",
 			mlog.String("post_id", post.Id),
 			mlog.String("filename", filename),
+			mlog.Err(err),
 		)
 	}
 
@@ -207,7 +212,7 @@ func (a *App) FindTeamIdForFilename(post *model.Post, filename string) string {
 	// This post is in a direct channel so we need to figure out what team the files are stored under.
 	teams, err := a.Srv.Store.Team().GetTeamsByUserId(post.UserId)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Unable to get teams when migrating post to use FileInfo, err=%v", err), mlog.String("post_id", post.Id))
+		mlog.Error("Unable to get teams when migrating post to use FileInfo", mlog.Err(err), mlog.String("post_id", post.Id))
 		return ""
 	}
 
@@ -241,9 +246,10 @@ func (a *App) MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo {
 	filenames := utils.RemoveDuplicatesFromStringArray(post.Filenames)
 	if errCh != nil {
 		mlog.Error(
-			fmt.Sprintf("Unable to get channel when migrating post to use FileInfos, err=%v", errCh),
+			"Unable to get channel when migrating post to use FileInfos",
 			mlog.String("post_id", post.Id),
 			mlog.String("channel_id", post.ChannelId),
+			mlog.Err(errCh),
 		)
 		return []*model.FileInfo{}
 	}
@@ -261,7 +267,8 @@ func (a *App) MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo {
 	infos := make([]*model.FileInfo, 0, len(filenames))
 	if teamId == "" {
 		mlog.Error(
-			fmt.Sprintf("Unable to find team id for files when migrating post to use FileInfos, filenames=%v", filenames),
+			"Unable to find team id for files when migrating post to use FileInfos",
+			mlog.String("filenames", strings.Join(filenames, ",")),
 			mlog.String("post_id", post.Id),
 		)
 	} else {
@@ -279,9 +286,9 @@ func (a *App) MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo {
 	fileMigrationLock.Lock()
 	defer fileMigrationLock.Unlock()
 
-	result, err := a.Srv.Store.Post().Get(post.Id)
+	result, err := a.Srv.Store.Post().Get(post.Id, false)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Unable to get post when migrating post to use FileInfos, err=%v", err), mlog.String("post_id", post.Id))
+		mlog.Error("Unable to get post when migrating post to use FileInfos", mlog.Err(err), mlog.String("post_id", post.Id))
 		return []*model.FileInfo{}
 	}
 
@@ -290,7 +297,7 @@ func (a *App) MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo {
 		var fileInfos []*model.FileInfo
 		fileInfos, err = a.Srv.Store.FileInfo().GetForPost(post.Id, true, false, false)
 		if err != nil {
-			mlog.Error(fmt.Sprintf("Unable to get FileInfos for migrated post, err=%v", err), mlog.String("post_id", post.Id))
+			mlog.Error("Unable to get FileInfos for migrated post", mlog.Err(err), mlog.String("post_id", post.Id))
 			return []*model.FileInfo{}
 		}
 
@@ -305,10 +312,11 @@ func (a *App) MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo {
 	for _, info := range infos {
 		if _, err = a.Srv.Store.FileInfo().Save(info); err != nil {
 			mlog.Error(
-				fmt.Sprintf("Unable to save file info when migrating post to use FileInfos, err=%v", err),
+				"Unable to save file info when migrating post to use FileInfos",
 				mlog.String("post_id", post.Id),
 				mlog.String("file_info_id", info.Id),
 				mlog.String("file_info_path", info.Path),
+				mlog.Err(err),
 			)
 			continue
 		}
@@ -326,7 +334,13 @@ func (a *App) MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo {
 
 	// Update Posts to clear Filenames and set FileIds
 	if _, err = a.Srv.Store.Post().Update(newPost, post); err != nil {
-		mlog.Error(fmt.Sprintf("Unable to save migrated post when migrating to use FileInfos, new_file_ids=%v, old_filenames=%v, err=%v", newPost.FileIds, post.Filenames, err), mlog.String("post_id", post.Id))
+		mlog.Error(
+			"Unable to save migrated post when migrating to use FileInfos",
+			mlog.String("new_file_ids", strings.Join(newPost.FileIds, ",")),
+			mlog.String("old_filenames", strings.Join(post.Filenames, ",")),
+			mlog.String("post_id", post.Id),
+			mlog.Err(err),
+		)
 		return []*model.FileInfo{}
 	}
 	return savedInfos
@@ -755,7 +769,7 @@ func (t *uploadFileTask) postprocessImage() {
 		var err error
 		decoded, typ, err = image.Decode(t.newReader())
 		if err != nil {
-			mlog.Error(fmt.Sprintf("Unable to decode image err=%v", err))
+			mlog.Error("Unable to decode image", mlog.Err(err))
 			return
 		}
 	}
@@ -779,14 +793,14 @@ func (t *uploadFileTask) postprocessImage() {
 		go func() {
 			_, aerr := t.writeFile(r, path)
 			if aerr != nil {
-				mlog.Error(fmt.Sprintf("Unable to upload path=%v err=%v", path, aerr))
+				mlog.Error("Unable to upload", mlog.String("path", path), mlog.Err(aerr))
 				return
 			}
 		}()
 
 		err := jpeg.Encode(w, img, &jpeg.Options{Quality: 90})
 		if err != nil {
-			mlog.Error(fmt.Sprintf("Unable to encode image as jpeg path=%v err=%v", path, err))
+			mlog.Error("Unable to encode image as jpeg", mlog.String("path", path), mlog.Err(err))
 			w.CloseWithError(err)
 		} else {
 			w.Close()
@@ -959,7 +973,7 @@ func prepareImage(fileData []byte) (image.Image, int, int) {
 	// Decode image bytes into Image object
 	img, imgType, err := image.Decode(bytes.NewReader(fileData))
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Unable to decode image err=%v", err))
+		mlog.Error("Unable to decode image", mlog.Err(err))
 		return nil, 0, 0
 	}
 
@@ -1038,12 +1052,12 @@ func (a *App) generateThumbnailImage(img image.Image, thumbnailPath string, widt
 
 	buf := new(bytes.Buffer)
 	if err := jpeg.Encode(buf, thumbnail, &jpeg.Options{Quality: 90}); err != nil {
-		mlog.Error(fmt.Sprintf("Unable to encode image as jpeg path=%v err=%v", thumbnailPath, err))
+		mlog.Error("Unable to encode image as jpeg", mlog.String("path", thumbnailPath), mlog.Err(err))
 		return
 	}
 
 	if _, err := a.WriteFile(buf, thumbnailPath); err != nil {
-		mlog.Error(fmt.Sprintf("Unable to upload thumbnail path=%v err=%v", thumbnailPath, err))
+		mlog.Error("Unable to upload thumbnail", mlog.String("path", thumbnailPath), mlog.Err(err))
 		return
 	}
 }
@@ -1060,12 +1074,12 @@ func (a *App) generatePreviewImage(img image.Image, previewPath string, width in
 	buf := new(bytes.Buffer)
 
 	if err := jpeg.Encode(buf, preview, &jpeg.Options{Quality: 90}); err != nil {
-		mlog.Error(fmt.Sprintf("Unable to encode image as preview jpg err=%v", err), mlog.String("path", previewPath))
+		mlog.Error("Unable to encode image as preview jpg", mlog.Err(err), mlog.String("path", previewPath))
 		return
 	}
 
 	if _, err := a.WriteFile(buf, previewPath); err != nil {
-		mlog.Error(fmt.Sprintf("Unable to upload preview err=%v", err), mlog.String("path", previewPath))
+		mlog.Error("Unable to upload preview", mlog.Err(err), mlog.String("path", previewPath))
 		return
 	}
 }

app/file_test.go

@@ -27,17 +27,12 @@ func TestGeneratePublicLinkHash(t *testing.T) {
 	hash2 := GeneratePublicLinkHash(filename2, salt1)
 	hash3 := GeneratePublicLinkHash(filename1, salt2)
 
-	if hash1 != GeneratePublicLinkHash(filename1, salt1) {
+	hash := GeneratePublicLinkHash(filename1, salt1)
-		t.Fatal("hash should be equal for the same file name and salt")
+	assert.Equal(t, hash, hash1, "hash should be equal for the same file name and salt")
-	}
 
-	if hash1 == hash2 {
+	assert.NotEqual(t, hash1, hash2, "hashes for different files should not be equal")
-		t.Fatal("hashes for different files should not be equal")
-	}
 
-	if hash1 == hash3 {
+	assert.NotEqual(t, hash1, hash3, "hashes for the same file with different salts should not be equal")
-		t.Fatal("hashes for the same file with different salts should not be equal")
-	}
 }
 
 func TestDoUploadFile(t *testing.T) {
@@ -51,60 +46,44 @@ func TestDoUploadFile(t *testing.T) {
 	data := []byte("abcd")
 
 	info1, err := th.App.DoUploadFile(time.Date(2007, 2, 4, 1, 2, 3, 4, time.Local), teamId, channelId, userId, filename, data)
-	if err != nil {
+	require.Nil(t, err, "DoUploadFile should succeed with valid data")
-		t.Fatal(err)
+	defer func() {
-	} else {
+		th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id)
-		defer func() {
+		th.App.RemoveFile(info1.Path)
-			th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id)
+	}()
-			th.App.RemoveFile(info1.Path)
-		}()
-	}
 
-	if info1.Path != fmt.Sprintf("20070204/teams/%v/channels/%v/users/%v/%v/%v", teamId, channelId, userId, info1.Id, filename) {
+	value := fmt.Sprintf("20070204/teams/%v/channels/%v/users/%v/%v/%v", teamId, channelId, userId, info1.Id, filename)
-		t.Fatal("stored file at incorrect path", info1.Path)
+	assert.Equal(t, value, info1.Path, "stored file at incorrect path" )
-	}
 
 	info2, err := th.App.DoUploadFile(time.Date(2007, 2, 4, 1, 2, 3, 4, time.Local), teamId, channelId, userId, filename, data)
-	if err != nil {
+	require.Nil(t, err, "DoUploadFile should succeed with valid data")
-		t.Fatal(err)
+	defer func() {
-	} else {
+		th.App.Srv.Store.FileInfo().PermanentDelete(info2.Id)
-		defer func() {
+		th.App.RemoveFile(info2.Path)
-			th.App.Srv.Store.FileInfo().PermanentDelete(info2.Id)
+	}()
-			th.App.RemoveFile(info2.Path)
-		}()
-	}
 
-	if info2.Path != fmt.Sprintf("20070204/teams/%v/channels/%v/users/%v/%v/%v", teamId, channelId, userId, info2.Id, filename) {
+	value = fmt.Sprintf("20070204/teams/%v/channels/%v/users/%v/%v/%v", teamId, channelId, userId, info2.Id, filename)
-		t.Fatal("stored file at incorrect path", info2.Path)
+	assert.Equal(t, value, info2.Path, "stored file at incorrect path")
-	}
 
 	info3, err := th.App.DoUploadFile(time.Date(2008, 3, 5, 1, 2, 3, 4, time.Local), teamId, channelId, userId, filename, data)
-	if err != nil {
+	require.Nil(t, err, "DoUploadFile should succeed with valid data")
-		t.Fatal(err)
+	defer func() {
-	} else {
+		th.App.Srv.Store.FileInfo().PermanentDelete(info3.Id)
-		defer func() {
+		th.App.RemoveFile(info3.Path)
-			th.App.Srv.Store.FileInfo().PermanentDelete(info3.Id)
+	}()
-			th.App.RemoveFile(info3.Path)
-		}()
-	}
 
-	if info3.Path != fmt.Sprintf("20080305/teams/%v/channels/%v/users/%v/%v/%v", teamId, channelId, userId, info3.Id, filename) {
+	value = fmt.Sprintf("20080305/teams/%v/channels/%v/users/%v/%v/%v", teamId, channelId, userId, info3.Id, filename)
-		t.Fatal("stored file at incorrect path", info3.Path)
+	assert.Equal(t, value, info3.Path, "stored file at incorrect path")
-	}
 
 	info4, err := th.App.DoUploadFile(time.Date(2009, 3, 5, 1, 2, 3, 4, time.Local), "../../"+teamId, "../../"+channelId, "../../"+userId, "../../"+filename, data)
-	if err != nil {
+	require.Nil(t, err, "DoUploadFile should succeed with valid data")
-		t.Fatal(err)
+	defer func() {
-	} else {
+		th.App.Srv.Store.FileInfo().PermanentDelete(info4.Id)
-		defer func() {
+		th.App.RemoveFile(info4.Path)
-			th.App.Srv.Store.FileInfo().PermanentDelete(info4.Id)
+	}()
-			th.App.RemoveFile(info4.Path)
-		}()
-	}
 
-	if info4.Path != fmt.Sprintf("20090305/teams/%v/channels/%v/users/%v/%v/%v", teamId, channelId, userId, info4.Id, filename) {
+	value = fmt.Sprintf("20090305/teams/%v/channels/%v/users/%v/%v/%v", teamId, channelId, userId, info4.Id, filename)
-		t.Fatal("stored file at incorrect path", info4.Path)
+	assert.Equal(t, value, info4.Path, "stored file at incorrect path")
-	}
 }
 
 func TestUploadFile(t *testing.T) {
@@ -116,19 +95,15 @@ func TestUploadFile(t *testing.T) {
 	data := []byte("abcd")
 
 	info1, err := th.App.UploadFile(data, channelId, filename)
-	if err != nil {
+	require.Nil(t, err, "UploadFile should succeed with valid data")
-		t.Fatal(err)
+	defer func() {
-	} else {
+		th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id)
-		defer func() {
+		th.App.RemoveFile(info1.Path)
-			th.App.Srv.Store.FileInfo().PermanentDelete(info1.Id)
+	}()
-			th.App.RemoveFile(info1.Path)
-		}()
-	}
 
-	if info1.Path != fmt.Sprintf("%v/teams/noteam/channels/%v/users/nouser/%v/%v",
+	value := fmt.Sprintf("%v/teams/noteam/channels/%v/users/nouser/%v/%v",
-		time.Now().Format("20060102"), channelId, info1.Id, filename) {
+		time.Now().Format("20060102"), channelId, info1.Id, filename)
-		t.Fatal("stored file at incorrect path", info1.Path)
+	assert.Equal(t, value, info1.Path, "Stored file at incorrect path")
-	}
 }
 
 func TestGetInfoForFilename(t *testing.T) {

app/helper_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/utils"
+	"github.com/stretchr/testify/require"
 )
 
 type TestHelper struct {
@@ -499,22 +500,14 @@ func (me *TestHelper) ResetEmojisMigration() {
 
 func (me *TestHelper) CheckTeamCount(t *testing.T, expected int64) {
 	teamCount, err := me.App.Srv.Store.Team().AnalyticsTeamCount()
-	if err != nil {
+	require.Nil(t, err, "Failed to get team count.")
-		t.Fatalf("Failed to get team count.")
+	require.Equalf(t, teamCount, expected, "Unexpected number of teams. Expected: %v, found: %v", expected, teamCount)
-	}
-	if teamCount != expected {
-		t.Fatalf("Unexpected number of teams. Expected: %v, found: %v", expected, teamCount)
-	}
 }
 
 func (me *TestHelper) CheckChannelsCount(t *testing.T, expected int64) {
-	if count, err := me.App.Srv.Store.Channel().AnalyticsTypeCount("", model.CHANNEL_OPEN); err == nil {
+	count, err := me.App.Srv.Store.Channel().AnalyticsTypeCount("", model.CHANNEL_OPEN)
-		if count != expected {
+	require.Nilf(t, err, "Failed to get channel count.")
-			t.Fatalf("Unexpected number of channels. Expected: %v, found: %v", expected, count)
+	require.Equalf(t, count, expected, "Unexpected number of channels. Expected: %v, found: %v", expected, count)
-		}
-	} else {
-		t.Fatalf("Failed to get channel count.")
-	}
 }
 
 func (me *TestHelper) SetupTeamScheme() *model.Scheme {

app/integration_action_test.go

@@ -430,6 +430,7 @@ func TestSubmitInteractiveDialog(t *testing.T) {
 		assert.Equal(t, "value1", val)
 
 		resp := model.SubmitDialogResponse{
+			Error: "some generic error",
 			Errors: map[string]string{"name1": "some error"},
 		}
 
@@ -444,6 +445,7 @@ func TestSubmitInteractiveDialog(t *testing.T) {
 	resp, err := th.App.SubmitInteractiveDialog(submit)
 	assert.Nil(t, err)
 	require.NotNil(t, resp)
+	assert.Equal(t, "some generic error", resp.Error)
 	assert.Equal(t, "some error", resp.Errors["name1"])
 
 	submit.URL = ""

app/license.go

@@ -29,7 +29,7 @@ func (a *App) LoadLicense() {
 
 		if license != nil {
 			if _, err = a.SaveLicense(licenseBytes); err != nil {
-				mlog.Info(fmt.Sprintf("Failed to save license key loaded from disk err=%v", err.Error()))
+				mlog.Info("Failed to save license key loaded from disk.", mlog.Err(err))
 			} else {
 				licenseId = license.Id
 			}

app/license_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestLoadLicense(t *testing.T) {
@@ -15,9 +16,7 @@ func TestLoadLicense(t *testing.T) {
 	defer th.TearDown()
 
 	th.App.LoadLicense()
-	if th.App.License() != nil {
+	require.Nil(t, th.App.License(), "shouldn't have a valid license")
-		t.Fatal("shouldn't have a valid license")
-	}
 }
 
 func TestSaveLicense(t *testing.T) {
@@ -26,18 +25,16 @@ func TestSaveLicense(t *testing.T) {
 
 	b1 := []byte("junk")
 
-	if _, err := th.App.SaveLicense(b1); err == nil {
+	_, err := th.App.SaveLicense(b1)
-		t.Fatal("shouldn't have saved license")
+	require.NotNil(t, err, "shouldn't have saved license")
-	}
 }
 
 func TestRemoveLicense(t *testing.T) {
 	th := Setup(t)
 	defer th.TearDown()
 
-	if err := th.App.RemoveLicense(); err != nil {
+	err := th.App.RemoveLicense()
-		t.Fatal("should have removed license")
+	require.Nil(t, err, "should have removed license")
-	}
 }
 
 func TestSetLicense(t *testing.T) {
@@ -49,18 +46,16 @@ func TestSetLicense(t *testing.T) {
 	l1.Customer = &model.Customer{}
 	l1.StartsAt = model.GetMillis() - 1000
 	l1.ExpiresAt = model.GetMillis() + 100000
-	if ok := th.App.SetLicense(l1); !ok {
+	ok := th.App.SetLicense(l1)
-		t.Fatal("license should have worked")
+	require.True(t, ok, "license should have worked")
-	}
 
 	l3 := &model.License{}
 	l3.Features = &model.Features{}
 	l3.Customer = &model.Customer{}
 	l3.StartsAt = model.GetMillis() + 10000
 	l3.ExpiresAt = model.GetMillis() + 100000
-	if ok := th.App.SetLicense(l3); !ok {
+	ok = th.App.SetLicense(l3)
-		t.Fatal("license should have passed")
+	require.True(t, ok, "license should have passed")
-	}
 }
 
 func TestClientLicenseEtag(t *testing.T) {
@@ -72,16 +67,12 @@ func TestClientLicenseEtag(t *testing.T) {
 	th.App.SetClientLicense(map[string]string{"SomeFeature": "true", "IsLicensed": "true"})
 
 	etag2 := th.App.GetClientLicenseEtag(false)
-	if etag1 == etag2 {
+	require.NotEqual(t, etag1, etag2, "etags should not match")
-		t.Fatal("etags should not match")
-	}
 
 	th.App.SetClientLicense(map[string]string{"SomeFeature": "true", "IsLicensed": "false"})
 
 	etag3 := th.App.GetClientLicenseEtag(false)
-	if etag2 == etag3 {
+	require.NotEqual(t, etag2, etag3, "etags should not match")
-		t.Fatal("etags should not match")
-	}
 }
 
 func TestGetSanitizedClientLicense(t *testing.T) {

app/notification.go

@@ -4,7 +4,6 @@
 package app
 
 import (
-	"fmt"
 	"sort"
 	"strings"
 	"unicode"
@@ -175,14 +174,14 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 			// Remove the user as recipient when the user has muted the channel.
 			if channelMuted, ok := channelMemberNotifyPropsMap[id][model.MARK_UNREAD_NOTIFY_PROP]; ok {
 				if channelMuted == model.CHANNEL_MARK_UNREAD_MENTION {
-					mlog.Debug(fmt.Sprintf("Channel muted for user_id %v, channel_mute %v", id, channelMuted))
+					mlog.Debug("Channel muted for user", mlog.String("user_id", id), mlog.String("channel_mute", channelMuted))
 					userAllowsEmails = false
 				}
 			}
 
 			//If email verification is required and user email is not verified don't send email.
 			if *a.Config().EmailSettings.RequireEmailVerification && !profileMap[id].EmailVerified {
-				mlog.Error(fmt.Sprintf("Skipped sending notification email to %v, address not verified. [details: user_id=%v]", profileMap[id].Email, id))
+				mlog.Error("Skipped sending notification email, address not verified.", mlog.String("user_email", profileMap[id].Email), mlog.String("user_id", id))
 				continue
 			}
 
@@ -250,7 +249,12 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 	// MUST be completed before push notifications send
 	for _, umc := range updateMentionChans {
 		if err := <-umc; err != nil {
-			mlog.Warn(fmt.Sprintf("Failed to update mention count, post_id=%v channel_id=%v err=%v", post.Id, post.ChannelId, result.Err), mlog.String("post_id", post.Id))
+			mlog.Warn(
+				"Failed to update mention count",
+				mlog.String("post_id", post.Id),
+				mlog.String("channel_id", post.ChannelId),
+				mlog.Err(err),
+			)
 		}
 	}
 
@@ -352,7 +356,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 
 		var infos []*model.FileInfo
 		if result := <-fchan; result.Err != nil {
-			mlog.Warn(fmt.Sprint("Unable to get fileInfo for push notifications.", post.Id, result.Err), mlog.String("post_id", post.Id))
+			mlog.Warn("Unable to get fileInfo for push notifications.", mlog.String("post_id", post.Id), mlog.Err(result.Err))
 		} else {
 			infos = result.Data.([]*model.FileInfo)
 		}

app/notification_email_test.go

@@ -109,8 +109,8 @@ func TestGetNotificationEmailBodyFullNotificationPublicChannel(t *testing.T) {
 	if !strings.Contains(body, "Channel: "+channel.DisplayName) {
 		t.Fatal("Expected email text 'Channel: " + channel.DisplayName + "'. Got " + body)
 	}
-	if !strings.Contains(body, "@"+senderName+" - ") {
+	if !strings.Contains(body, senderName+" - ") {
-		t.Fatal("Expected email text '@" + senderName + " - '. Got " + body)
+		t.Fatal("Expected email text '" + senderName + " - '. Got " + body)
 	}
 	if !strings.Contains(body, post.Message) {
 		t.Fatal("Expected email text '" + post.Message + "'. Got " + body)
@@ -146,8 +146,8 @@ func TestGetNotificationEmailBodyFullNotificationGroupChannel(t *testing.T) {
 	if !strings.Contains(body, "Channel: ChannelName") {
 		t.Fatal("Expected email text 'Channel: ChannelName'. Got " + body)
 	}
-	if !strings.Contains(body, "@"+senderName+" - ") {
+	if !strings.Contains(body, senderName+" - ") {
-		t.Fatal("Expected email text '@" + senderName + " - '. Got " + body)
+		t.Fatal("Expected email text '" + senderName + " - '. Got " + body)
 	}
 	if !strings.Contains(body, post.Message) {
 		t.Fatal("Expected email text '" + post.Message + "'. Got " + body)
@@ -183,8 +183,8 @@ func TestGetNotificationEmailBodyFullNotificationPrivateChannel(t *testing.T) {
 	if !strings.Contains(body, "Channel: "+channel.DisplayName) {
 		t.Fatal("Expected email text 'Channel: " + channel.DisplayName + "'. Got " + body)
 	}
-	if !strings.Contains(body, "@"+senderName+" - ") {
+	if !strings.Contains(body, senderName+" - ") {
-		t.Fatal("Expected email text '@" + senderName + " - '. Got " + body)
+		t.Fatal("Expected email text '" + senderName + " - '. Got " + body)
 	}
 	if !strings.Contains(body, post.Message) {
 		t.Fatal("Expected email text '" + post.Message + "'. Got " + body)
@@ -217,8 +217,8 @@ func TestGetNotificationEmailBodyFullNotificationDirectChannel(t *testing.T) {
 	if !strings.Contains(body, "You have a new Direct Message.") {
 		t.Fatal("Expected email text 'You have a new Direct Message. Got " + body)
 	}
-	if !strings.Contains(body, "@"+senderName+" - ") {
+	if !strings.Contains(body, senderName+" - ") {
-		t.Fatal("Expected email text '@" + senderName + " - '. Got " + body)
+		t.Fatal("Expected email text '" + senderName + " - '. Got " + body)
 	}
 	if !strings.Contains(body, post.Message) {
 		t.Fatal("Expected email text '" + post.Message + "'. Got " + body)
@@ -386,8 +386,8 @@ func TestGetNotificationEmailBodyGenericNotificationPublicChannel(t *testing.T)
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new notification from @"+senderName) {
+	if !strings.Contains(body, "You have a new notification from "+senderName) {
-		t.Fatal("Expected email text 'You have a new notification from @" + senderName + "'. Got " + body)
+		t.Fatal("Expected email text 'You have a new notification from " + senderName + "'. Got " + body)
 	}
 	if strings.Contains(body, "Channel: "+channel.DisplayName) {
 		t.Fatal("Did not expect email text 'Channel: " + channel.DisplayName + "'. Got " + body)
@@ -420,8 +420,8 @@ func TestGetNotificationEmailBodyGenericNotificationGroupChannel(t *testing.T) {
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new Group Message from @"+senderName) {
+	if !strings.Contains(body, "You have a new Group Message from "+senderName) {
-		t.Fatal("Expected email text 'You have a new Group Message from @" + senderName + "'. Got " + body)
+		t.Fatal("Expected email text 'You have a new Group Message from " + senderName + "'. Got " + body)
 	}
 	if strings.Contains(body, "CHANNEL: "+channel.DisplayName) {
 		t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body)
@@ -454,8 +454,8 @@ func TestGetNotificationEmailBodyGenericNotificationPrivateChannel(t *testing.T)
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new notification from @"+senderName) {
+	if !strings.Contains(body, "You have a new notification from "+senderName) {
-		t.Fatal("Expected email text 'You have a new notification from @" + senderName + "'. Got " + body)
+		t.Fatal("Expected email text 'You have a new notification from " + senderName + "'. Got " + body)
 	}
 	if strings.Contains(body, "CHANNEL: "+channel.DisplayName) {
 		t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body)
@@ -488,8 +488,8 @@ func TestGetNotificationEmailBodyGenericNotificationDirectChannel(t *testing.T)
 	translateFunc := utils.GetUserTranslations("en")
 
 	body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, true, translateFunc)
-	if !strings.Contains(body, "You have a new Direct Message from @"+senderName) {
+	if !strings.Contains(body, "You have a new Direct Message from "+senderName) {
-		t.Fatal("Expected email text 'You have a new Direct Message from @" + senderName + "'. Got " + body)
+		t.Fatal("Expected email text 'You have a new Direct Message from " + senderName + "'. Got " + body)
 	}
 	if strings.Contains(body, "CHANNEL: "+channel.DisplayName) {
 		t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body)

app/plugin.go

@@ -7,12 +7,14 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/plugin"
 	"github.com/mattermost/mattermost-server/services/filesstore"
+	"github.com/mattermost/mattermost-server/services/marketplace"
 	"github.com/mattermost/mattermost-server/utils/fileutils"
 	"github.com/pkg/errors"
 )
@@ -400,6 +402,103 @@ func (a *App) GetPlugins() (*model.PluginsResponse, *model.AppError) {
 	return resp, nil
 }
 
+// GetMarketplacePlugins returns a list of plugins from the marketplace-server,
+// and plugins that are installed locally.
+func (a *App) GetMarketplacePlugins(filter *model.MarketplacePluginFilter) ([]*model.MarketplacePlugin, *model.AppError) {
+	var result []*model.MarketplacePlugin
+	pluginSet := map[string]bool{}
+	pluginsEnvironment := a.GetPluginsEnvironment()
+	if pluginsEnvironment == nil {
+		return nil, model.NewAppError("GetMarketplacePlugins", "app.plugin.config.app_error", nil, "", http.StatusInternalServerError)
+	}
+
+	marketplaceClient, err := marketplace.NewClient(
+		*a.Config().PluginSettings.MarketplaceUrl,
+		a.HTTPService,
+	)
+	if err != nil {
+		return nil, model.NewAppError("GetMarketplacePlugins", "app.plugin.marketplace_client.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	// Fetch all plugins from marketplace.
+	marketplacePlugins, err := marketplaceClient.GetPlugins(&model.MarketplacePluginFilter{
+		PerPage:       -1,
+		ServerVersion: model.CurrentVersion,
+	})
+	if err != nil {
+		return nil, model.NewAppError("GetMarketplacePlugins", "app.plugin.marketplace_plugins.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	for _, p := range marketplacePlugins {
+		if p.Manifest == nil || !pluginMatchesFilter(p.Manifest, filter.Filter) {
+			continue
+		}
+
+		marketplacePlugin := &model.MarketplacePlugin{
+			BaseMarketplacePlugin: p,
+		}
+
+		var manifest *model.Manifest
+		if manifest, err = pluginsEnvironment.GetManifest(p.Manifest.Id); err != nil && err != plugin.ErrNotFound {
+			return nil, model.NewAppError("GetMarketplacePlugins", "app.plugin.config.app_error", nil, err.Error(), http.StatusInternalServerError)
+		} else if err == nil {
+			// Plugin is installed.
+			marketplacePlugin.InstalledVersion = manifest.Version
+		}
+
+		pluginSet[p.Manifest.Id] = true
+		result = append(result, marketplacePlugin)
+	}
+
+	// Include all other installed plugins.
+	plugins, err := pluginsEnvironment.Available()
+	if err != nil {
+		return nil, model.NewAppError("GetMarketplacePlugins", "app.plugin.config.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	for _, plugin := range plugins {
+		if plugin.Manifest == nil || pluginSet[plugin.Manifest.Id] || !pluginMatchesFilter(plugin.Manifest, filter.Filter) {
+			continue
+		}
+
+		result = append(result, &model.MarketplacePlugin{
+			BaseMarketplacePlugin: &model.BaseMarketplacePlugin{
+				Manifest: plugin.Manifest,
+			},
+			InstalledVersion: plugin.Manifest.Version,
+		})
+	}
+
+	// Sort result alphabetically.
+	sort.SliceStable(result, func(i, j int) bool {
+		return strings.ToLower(result[i].Manifest.Name) < strings.ToLower(result[j].Manifest.Name)
+	})
+
+	return result, nil
+}
+
+func pluginMatchesFilter(manifest *model.Manifest, filter string) bool {
+	filter = strings.TrimSpace(strings.ToLower(filter))
+
+	if filter == "" {
+		return true
+	}
+
+	if strings.ToLower(manifest.Id) == filter {
+		return true
+	}
+
+	if strings.Contains(strings.ToLower(manifest.Name), filter) {
+		return true
+	}
+
+	if strings.Contains(strings.ToLower(manifest.Description), filter) {
+		return true
+	}
+
+	return false
+}
+
 // notifyPluginEnabled notifies connected websocket clients across all peers if the version of the given
 // plugin is same across them.
 //

app/plugin_api.go

@@ -474,19 +474,19 @@ func (api *PluginAPI) GetPost(postId string) (*model.Post, *model.AppError) {
 }
 
 func (api *PluginAPI) GetPostsSince(channelId string, time int64) (*model.PostList, *model.AppError) {
-	return api.app.GetPostsSince(channelId, time)
+	return api.app.GetPostsSince(model.GetPostsSinceOptions{ChannelId: channelId, Time: time})
 }
 
 func (api *PluginAPI) GetPostsAfter(channelId, postId string, page, perPage int) (*model.PostList, *model.AppError) {
-	return api.app.GetPostsAfterPost(channelId, postId, page, perPage)
+	return api.app.GetPostsAfterPost(model.GetPostsOptions{ChannelId: channelId, PostId: postId, Page: page, PerPage: perPage})
 }
 
 func (api *PluginAPI) GetPostsBefore(channelId, postId string, page, perPage int) (*model.PostList, *model.AppError) {
-	return api.app.GetPostsBeforePost(channelId, postId, page, perPage)
+	return api.app.GetPostsBeforePost(model.GetPostsOptions{ChannelId: channelId, PostId: postId, Page: page, PerPage: perPage})
 }
 
 func (api *PluginAPI) GetPostsForChannel(channelId string, page, perPage int) (*model.PostList, *model.AppError) {
-	return api.app.GetPostsPage(channelId, page, perPage)
+	return api.app.GetPostsPage(model.GetPostsOptions{ChannelId: channelId, Page: perPage, PerPage: page})
 }
 
 func (api *PluginAPI) UpdatePost(post *model.Post) (*model.Post, *model.AppError) {

app/plugin_deadlock_test.go

@@ -4,6 +4,7 @@
 package app
 
 import (
+	"github.com/stretchr/testify/require"
 	"os"
 	"strings"
 	"testing"
@@ -94,7 +95,7 @@ func TestPluginDeadlock(t *testing.T) {
 		select {
 		case <-done:
 		case <-time.After(30 * time.Second):
-			t.Fatal("plugin failed to activate: likely deadlocked")
+			require.Fail(t, "plugin failed to activate: likely deadlocked")
 			go func() {
 				time.Sleep(5 * time.Second)
 				os.Exit(1)
@@ -201,7 +202,7 @@ func TestPluginDeadlock(t *testing.T) {
 		select {
 		case <-done:
 		case <-time.After(30 * time.Second):
-			t.Fatal("plugin failed to activate: likely deadlocked")
+			require.Fail(t, "plugin failed to activate: likely deadlocked")
 			go func() {
 				time.Sleep(5 * time.Second)
 				os.Exit(1)

app/post.go

@@ -75,7 +75,12 @@ func (a *App) CreatePostAsUser(post *model.Post, currentSessionId string) (*mode
 	// Update the LastViewAt only if the post does not have from_webhook prop set (eg. Zapier app)
 	if _, ok := post.Props["from_webhook"]; !ok {
 		if _, err := a.MarkChannelsAsViewed([]string{post.ChannelId}, post.UserId, currentSessionId); err != nil {
-			mlog.Error(fmt.Sprintf("Encountered error updating last viewed, channel_id=%s, user_id=%s, err=%v", post.ChannelId, post.UserId, err))
+			mlog.Error(
+				"Encountered error updating last viewed",
+				mlog.String("channel_id", post.ChannelId),
+				mlog.String("user_id", post.UserId),
+				mlog.Err(err),
+			)
 		}
 	}
 
@@ -162,7 +167,7 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo
 	if len(post.RootId) > 0 {
 		pchan = make(chan store.StoreResult, 1)
 		go func() {
-			r, pErr := a.Srv.Store.Post().Get(post.RootId)
+			r, pErr := a.Srv.Store.Post().Get(post.RootId, true)
 			pchan <- store.StoreResult{Data: r, Err: pErr}
 			close(pchan)
 		}()
@@ -228,7 +233,7 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo
 			post.Props["attachments"] = attachmentsInterface
 		}
 		if err != nil {
-			mlog.Error("Could not convert post attachments to map interface, err=%s" + err.Error())
+			mlog.Error("Could not convert post attachments to map interface.", mlog.Err(err))
 		}
 	}
 
@@ -470,7 +475,7 @@ func (a *App) DeleteEphemeralPost(userId, postId string) {
 func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) {
 	post.SanitizeProps()
 
-	postLists, err := a.Srv.Store.Post().Get(post.Id)
+	postLists, err := a.Srv.Store.Post().Get(post.Id, true)
 	if err != nil {
 		return nil, err
 	}
@@ -563,7 +568,7 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model
 		a.Srv.Go(func() {
 			channel, chanErr := a.Srv.Store.Channel().GetForPost(rpost.Id)
 			if chanErr != nil {
-				mlog.Error(fmt.Sprintf("Couldn't get channel %v for post %v for Elasticsearch indexing.", rpost.ChannelId, rpost.Id))
+				mlog.Error("Couldn't get channel for post for Elasticsearch indexing.", mlog.String("channel_id", rpost.ChannelId), mlog.String("post_id", rpost.Id))
 				return
 			}
 			if err := a.Elasticsearch.IndexPost(rpost, channel.TeamId); err != nil {
@@ -609,20 +614,20 @@ func (a *App) PatchPost(postId string, patch *model.PostPatch) (*model.Post, *mo
 	return updatedPost, nil
 }
 
-func (a *App) GetPostsPage(channelId string, page int, perPage int) (*model.PostList, *model.AppError) {
+func (a *App) GetPostsPage(options model.GetPostsOptions) (*model.PostList, *model.AppError) {
-	return a.Srv.Store.Post().GetPosts(channelId, page*perPage, perPage, true)
+	return a.Srv.Store.Post().GetPosts(options, false)
 }
 
 func (a *App) GetPosts(channelId string, offset int, limit int) (*model.PostList, *model.AppError) {
-	return a.Srv.Store.Post().GetPosts(channelId, offset, limit, true)
+	return a.Srv.Store.Post().GetPosts(model.GetPostsOptions{ChannelId: channelId, Page: offset, PerPage: limit}, true)
 }
 
 func (a *App) GetPostsEtag(channelId string) string {
 	return a.Srv.Store.Post().GetEtag(channelId, true)
 }
 
-func (a *App) GetPostsSince(channelId string, time int64) (*model.PostList, *model.AppError) {
+func (a *App) GetPostsSince(options model.GetPostsSinceOptions) (*model.PostList, *model.AppError) {
-	return a.Srv.Store.Post().GetPostsSince(channelId, time, true)
+	return a.Srv.Store.Post().GetPostsSince(options, true)
 }
 
 func (a *App) GetSinglePost(postId string) (*model.Post, *model.AppError) {
@@ -630,7 +635,7 @@ func (a *App) GetSinglePost(postId string) (*model.Post, *model.AppError) {
 }
 
 func (a *App) GetPostThread(postId string) (*model.PostList, *model.AppError) {
-	return a.Srv.Store.Post().Get(postId)
+	return a.Srv.Store.Post().Get(postId, false)
 }
 
 func (a *App) GetFlaggedPosts(userId string, offset int, limit int) (*model.PostList, *model.AppError) {
@@ -646,7 +651,7 @@ func (a *App) GetFlaggedPostsForChannel(userId, channelId string, offset int, li
 }
 
 func (a *App) GetPermalinkPost(postId string, userId string) (*model.PostList, *model.AppError) {
-	list, err := a.Srv.Store.Post().Get(postId)
+	list, err := a.Srv.Store.Post().Get(postId, false)
 	if err != nil {
 		return nil, err
 	}
@@ -668,19 +673,19 @@ func (a *App) GetPermalinkPost(postId string, userId string) (*model.PostList, *
 	return list, nil
 }
 
-func (a *App) GetPostsBeforePost(channelId, postId string, page, perPage int) (*model.PostList, *model.AppError) {
+func (a *App) GetPostsBeforePost(options model.GetPostsOptions) (*model.PostList, *model.AppError) {
-	return a.Srv.Store.Post().GetPostsBefore(channelId, postId, perPage, page*perPage)
+	return a.Srv.Store.Post().GetPostsBefore(options)
 }
 
-func (a *App) GetPostsAfterPost(channelId, postId string, page, perPage int) (*model.PostList, *model.AppError) {
+func (a *App) GetPostsAfterPost(options model.GetPostsOptions) (*model.PostList, *model.AppError) {
-	return a.Srv.Store.Post().GetPostsAfter(channelId, postId, perPage, page*perPage)
+	return a.Srv.Store.Post().GetPostsAfter(options)
 }
 
-func (a *App) GetPostsAroundPost(postId, channelId string, offset, limit int, before bool) (*model.PostList, *model.AppError) {
+func (a *App) GetPostsAroundPost(before bool, options model.GetPostsOptions) (*model.PostList, *model.AppError) {
 	if before {
-		return a.Srv.Store.Post().GetPostsBefore(channelId, postId, limit, offset)
+		return a.Srv.Store.Post().GetPostsBefore(options)
 	}
-	return a.Srv.Store.Post().GetPostsAfter(channelId, postId, limit, offset)
+	return a.Srv.Store.Post().GetPostsAfter(options)
 }
 
 func (a *App) GetPostAfterTime(channelId string, time int64) (*model.Post, *model.AppError) {
@@ -768,8 +773,7 @@ func (a *App) AddCursorIdsForPostList(originalList *model.PostList, afterPost, b
 	originalList.NextPostId = nextPostId
 	originalList.PrevPostId = prevPostId
 }
-
+func (a *App) GetPostsForChannelAroundLastUnread(channelId, userId string, limitBefore, limitAfter int, skipFetchThreads bool) (*model.PostList, *model.AppError) {
-func (a *App) GetPostsForChannelAroundLastUnread(channelId, userId string, limitBefore, limitAfter int) (*model.PostList, *model.AppError) {
 	var member *model.ChannelMember
 	var err *model.AppError
 	if member, err = a.GetChannelMember(channelId, userId); err != nil {
@@ -793,13 +797,13 @@ func (a *App) GetPostsForChannelAroundLastUnread(channelId, userId string, limit
 	// channel organically, those replies will be added below.
 	postList.Order = []string{lastUnreadPostId}
 
-	if postListBefore, err := a.GetPostsBeforePost(channelId, lastUnreadPostId, PAGE_DEFAULT, limitBefore); err != nil {
+	if postListBefore, err := a.GetPostsBeforePost(model.GetPostsOptions{ChannelId: channelId, PostId: lastUnreadPostId, Page: PAGE_DEFAULT, PerPage: limitBefore, SkipFetchThreads: skipFetchThreads}); err != nil {
 		return nil, err
 	} else if postListBefore != nil {
 		postList.Extend(postListBefore)
 	}
 
-	if postListAfter, err := a.GetPostsAfterPost(channelId, lastUnreadPostId, PAGE_DEFAULT, limitAfter-1); err != nil {
+	if postListAfter, err := a.GetPostsAfterPost(model.GetPostsOptions{ChannelId: channelId, PostId: lastUnreadPostId, Page: PAGE_DEFAULT, PerPage: limitAfter - 1, SkipFetchThreads: skipFetchThreads}); err != nil {
 		return nil, err
 	} else if postListAfter != nil {
 		postList.Extend(postListAfter)
@@ -856,7 +860,7 @@ func (a *App) DeletePost(postId, deleteByID string) (*model.Post, *model.AppErro
 
 func (a *App) DeleteFlaggedPosts(postId string) {
 	if err := a.Srv.Store.Preference().DeleteCategoryAndName(model.PREFERENCE_CATEGORY_FLAGGED_POST, postId); err != nil {
-		mlog.Warn(fmt.Sprintf("Unable to delete flagged post preference when deleting post, err=%v", err))
+		mlog.Warn("Unable to delete flagged post preference when deleting post.", mlog.Err(err))
 		return
 	}
 }
@@ -867,7 +871,7 @@ func (a *App) DeletePostFiles(post *model.Post) {
 	}
 
 	if _, err := a.Srv.Store.FileInfo().DeleteForPost(post.Id); err != nil {
-		mlog.Warn(fmt.Sprintf("Encountered error when deleting files for post, post_id=%v, err=%v", post.Id, err), mlog.String("post_id", post.Id))
+		mlog.Warn("Encountered error when deleting files for post", mlog.String("post_id", post.Id), mlog.Err(err))
 	}
 }
 
@@ -949,7 +953,7 @@ func (a *App) convertChannelNamesToChannelIds(channels []string, userId string,
 	for idx, channelName := range channels {
 		channel, err := a.parseAndFetchChannelIdByNameFromInFilter(channelName, userId, teamId, includeDeletedChannels)
 		if err != nil {
-			mlog.Error(fmt.Sprint(err))
+			mlog.Error("error getting channel id by name from in filter", mlog.Err(err))
 			continue
 		}
 		channels[idx] = channel.Id
@@ -960,7 +964,7 @@ func (a *App) convertChannelNamesToChannelIds(channels []string, userId string,
 func (a *App) convertUserNameToUserIds(usernames []string) []string {
 	for idx, username := range usernames {
 		if user, err := a.GetUserByUsername(username); err != nil {
-			mlog.Error(fmt.Sprint(err))
+			mlog.Error("error getting user by username", mlog.String("user_name", username), mlog.Err(err))
 		} else {
 			usernames[idx] = user.Id
 		}
@@ -1005,7 +1009,7 @@ func (a *App) esSearchPostsInTeamForUser(paramsList []*model.SearchParams, userI
 	// We only allow the user to search in channels they are a member of.
 	userChannels, err := a.GetChannelsForUser(teamId, userId, includeDeleted)
 	if err != nil {
-		mlog.Error(fmt.Sprint(err))
+		mlog.Error("error getting channel for user", mlog.Err(err))
 		return nil, err
 	}
 
@@ -1062,7 +1066,7 @@ func (a *App) SearchPostsInTeamForUser(terms string, userId string, teamId strin
 				if strings.HasPrefix(channelName, "@") {
 					channel, err := a.parseAndFetchChannelIdByNameFromInFilter(channelName, userId, teamId, includeDeletedChannels)
 					if err != nil {
-						mlog.Error(fmt.Sprint(err))
+						mlog.Error("error getting channel_id by name from in filter", mlog.Err(err))
 						continue
 					}
 					params.InChannels[idx] = channel.Name
@@ -1072,7 +1076,7 @@ func (a *App) SearchPostsInTeamForUser(terms string, userId string, teamId strin
 				if strings.HasPrefix(channelName, "@") {
 					channel, err := a.parseAndFetchChannelIdByNameFromInFilter(channelName, userId, teamId, includeDeletedChannels)
 					if err != nil {
-						mlog.Error(fmt.Sprint(err))
+						mlog.Error("error getting channel_id by name from in filter", mlog.Err(err))
 						continue
 					}
 					params.ExcludedChannels[idx] = channel.Name

app/post_metadata_test.go

@@ -544,7 +544,7 @@ func TestGetEmbedForPost(t *testing.T) {
 			</head>
 			</html>`))
 		} else {
-			t.Fatal("Invalid path", r.URL.Path)
+			require.Fail(t, "Invalid path", r.URL.Path)
 		}
 	}))
 	defer server.Close()

app/ratelimit.go

@@ -4,7 +4,6 @@
 package app
 
 import (
-	"fmt"
 	"math"
 	"net/http"
 	"strconv"
@@ -77,14 +76,14 @@ func (rl *RateLimiter) GenerateKey(r *http.Request) string {
 func (rl *RateLimiter) RateLimitWriter(key string, w http.ResponseWriter) bool {
 	limited, context, err := rl.throttledRateLimiter.RateLimit(key, 1)
 	if err != nil {
-		mlog.Critical("Internal server error when rate limiting. Rate Limiting broken. Error:" + err.Error())
+		mlog.Critical("Internal server error when rate limiting. Rate Limiting broken.", mlog.Err(err))
 		return false
 	}
 
 	setRateLimitHeaders(w, context)
 
 	if limited {
-		mlog.Error(fmt.Sprintf("Denied due to throttling settings code=429 key=%v", key))
+		mlog.Error("Denied due to throttling settings code=429", mlog.String("key", key))
 		http.Error(w, "limit exceeded", 429)
 	}
 

app/security_update_check.go

@@ -4,7 +4,6 @@
 package app
 
 import (
-	"fmt"
 	"io/ioutil"
 	"net/http"
 	"net/url"
@@ -114,7 +113,7 @@ func (s *Server) DoSecurityUpdateCheck() {
 					}
 
 					for _, user := range users {
-						mlog.Info(fmt.Sprintf("Sending security bulletin for %v to %v", bulletin.Id, user.Email))
+						mlog.Info("Sending security bulletin", mlog.String("bulletin_id", bulletin.Id), mlog.String("user_email", user.Email))
 						license := s.License()
 						mailservice.SendMailUsingConfig(user.Email, utils.T("mattermost.bulletin.subject"), string(body), s.Config(), license != nil && *license.Features.Compliance)
 					}

app/slackimport.go

@@ -7,7 +7,6 @@ import (
 	"archive/zip"
 	"bytes"
 	"encoding/json"
-	"fmt"
 	"io"
 	"mime/multipart"
 	"net/http"
@@ -168,7 +167,7 @@ func (a *App) SlackAddUsers(teamId string, slackusers []SlackUser, importerLog *
 		if email == "" {
 			email = sUser.Username + "@example.com"
 			importerLog.WriteString(utils.T("api.slackimport.slack_add_users.missing_email_address", map[string]interface{}{"Email": email, "Username": sUser.Username}))
-			mlog.Warn(fmt.Sprintf("Slack Import: User %v does not have an email address in the Slack export. Used %v as a placeholder. The user should update their email address once logged in to the system.", email, sUser.Username))
+			mlog.Warn("Slack Import: User does not have an email address in the Slack export. Used username as a placeholder. The user should update their email address once logged in to the system.", mlog.String("user_email", email), mlog.String("user_name", sUser.Username))
 		}
 
 		password := model.NewId()
@@ -246,7 +245,7 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack
 				continue
 			}
 			if users[sPost.User] == nil {
-				mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User))
+				mlog.Debug("Slack Import: Unable to add the message as the Slack user does not exist in Mattermost.", mlog.String("user", sPost.User))
 				continue
 			}
 			newPost := model.Post{
@@ -288,7 +287,7 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack
 				continue
 			}
 			if users[sPost.Comment.User] == nil {
-				mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User))
+				mlog.Debug("Slack Import: Unable to add the message as the Slack user does not exist in Mattermost.", mlog.String("user", sPost.User))
 				continue
 			}
 			newPost := model.Post{
@@ -333,7 +332,7 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack
 				continue
 			}
 			if users[sPost.User] == nil {
-				mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User))
+				mlog.Debug("Slack Import: Unable to add the message as the Slack user does not exist in Mattermost.", mlog.String("user", sPost.User))
 				continue
 			}
 
@@ -361,7 +360,7 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack
 				continue
 			}
 			if users[sPost.User] == nil {
-				mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User))
+				mlog.Debug("Slack Import: Unable to add the message as the Slack user does not exist in Mattermost.", mlog.String("user", sPost.User))
 				continue
 			}
 			newPost := model.Post{
@@ -381,7 +380,7 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack
 				continue
 			}
 			if users[sPost.User] == nil {
-				mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User))
+				mlog.Debug("Slack Import: Unable to add the message as the Slack user does not exist in Mattermost.", mlog.String("user", sPost.User))
 				continue
 			}
 			newPost := model.Post{
@@ -398,7 +397,7 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack
 				continue
 			}
 			if users[sPost.User] == nil {
-				mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User))
+				mlog.Debug("Slack Import: Unable to add the message as the Slack user does not exist in Mattermost.", mlog.String("user", sPost.User))
 				continue
 			}
 			newPost := model.Post{
@@ -415,7 +414,7 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack
 				continue
 			}
 			if users[sPost.User] == nil {
-				mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User))
+				mlog.Debug("Slack Import: Unable to add the message as the Slack user does not exist in Mattermost.", mlog.String("user", sPost.User))
 				continue
 			}
 			newPost := model.Post{
@@ -427,7 +426,11 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack
 			}
 			a.OldImportPost(&newPost)
 		default:
-			mlog.Warn(fmt.Sprintf("Slack Import: Unable to import the message as its type is not supported: post_type=%v, post_subtype=%v.", sPost.Type, sPost.SubType))
+			mlog.Warn(
+				"Slack Import: Unable to import the message as its type is not supported",
+				mlog.String("post_type", sPost.Type),
+				mlog.String("post_subtype", sPost.SubType),
+			)
 		}
 	}
 }
@@ -439,12 +442,12 @@ func (a *App) SlackUploadFile(slackPostFile *SlackFile, uploads map[string]*zip.
 	}
 	file, ok := uploads[slackPostFile.Id]
 	if !ok {
-		mlog.Warn(fmt.Sprintf("Slack Import: Unable to import file %v as the file is missing from the Slack export zip file.", slackPostFile.Id))
+		mlog.Warn("Slack Import: Unable to import file as the file is missing from the Slack export zip file.", mlog.String("file_id", slackPostFile.Id))
 		return nil, false
 	}
 	openFile, err := file.Open()
 	if err != nil {
-		mlog.Warn(fmt.Sprintf("Slack Import: Unable to open the file %v from the Slack export: %v.", slackPostFile.Id, err.Error()))
+		mlog.Warn("Slack Import: Unable to open the file from the Slack export.", mlog.String("file_id", slackPostFile.Id), mlog.Err(err))
 		return nil, false
 	}
 	defer openFile.Close()
@@ -452,7 +455,7 @@ func (a *App) SlackUploadFile(slackPostFile *SlackFile, uploads map[string]*zip.
 	timestamp := utils.TimeFromMillis(SlackConvertTimeStamp(slackTimestamp))
 	uploadedFile, err := a.OldImportFile(timestamp, openFile, teamId, channelId, userId, filepath.Base(file.Name))
 	if err != nil {
-		mlog.Warn(fmt.Sprintf("Slack Import: An error occurred when uploading file %v: %v.", slackPostFile.Id, err.Error()))
+		mlog.Warn("Slack Import: An error occurred when uploading file.", mlog.String("file_id", slackPostFile.Id), mlog.Err(err))
 		return nil, false
 	}
 
@@ -480,22 +483,22 @@ func (a *App) addSlackUsersToChannel(members []string, users map[string]*model.U
 
 func SlackSanitiseChannelProperties(channel model.Channel) model.Channel {
 	if utf8.RuneCountInString(channel.DisplayName) > model.CHANNEL_DISPLAY_NAME_MAX_RUNES {
-		mlog.Warn(fmt.Sprintf("Slack Import: Channel %v display name exceeds the maximum length. It will be truncated when imported.", channel.DisplayName))
+		mlog.Warn("Slack Import: Channel display name exceeds the maximum length. It will be truncated when imported.", mlog.String("channel_display_name", channel.DisplayName))
 		channel.DisplayName = truncateRunes(channel.DisplayName, model.CHANNEL_DISPLAY_NAME_MAX_RUNES)
 	}
 
 	if len(channel.Name) > model.CHANNEL_NAME_MAX_LENGTH {
-		mlog.Warn(fmt.Sprintf("Slack Import: Channel %v handle exceeds the maximum length. It will be truncated when imported.", channel.DisplayName))
+		mlog.Warn("Slack Import: Channel handle exceeds the maximum length. It will be truncated when imported.", mlog.String("channel_display_name", channel.DisplayName))
 		channel.Name = channel.Name[0:model.CHANNEL_NAME_MAX_LENGTH]
 	}
 
 	if utf8.RuneCountInString(channel.Purpose) > model.CHANNEL_PURPOSE_MAX_RUNES {
-		mlog.Warn(fmt.Sprintf("Slack Import: Channel %v purpose exceeds the maximum length. It will be truncated when imported.", channel.DisplayName))
+		mlog.Warn("Slack Import: Channel purpose exceeds the maximum length. It will be truncated when imported.", mlog.String("channel_display_name", channel.DisplayName))
 		channel.Purpose = truncateRunes(channel.Purpose, model.CHANNEL_PURPOSE_MAX_RUNES)
 	}
 
 	if utf8.RuneCountInString(channel.Header) > model.CHANNEL_HEADER_MAX_RUNES {
-		mlog.Warn(fmt.Sprintf("Slack Import: Channel %v header exceeds the maximum length. It will be truncated when imported.", channel.DisplayName))
+		mlog.Warn("Slack Import: Channel header exceeds the maximum length. It will be truncated when imported.", mlog.String("channel_display_name", channel.DisplayName))
 		channel.Header = truncateRunes(channel.Header, model.CHANNEL_HEADER_MAX_RUNES)
 	}
 
@@ -540,7 +543,7 @@ func (a *App) SlackAddChannels(teamId string, slackchannels []SlackChannel, post
 			// Haven't found an existing channel to merge with. Try importing it as a new one.
 			mChannel = a.OldImportChannel(&newChannel, sChannel, users)
 			if mChannel == nil {
-				mlog.Warn(fmt.Sprintf("Slack Import: Unable to import Slack channel: %s.", newChannel.DisplayName))
+				mlog.Warn("Slack Import: Unable to import Slack channel.", mlog.String("channel_display_name", newChannel.DisplayName))
 				importerLog.WriteString(utils.T("api.slackimport.slack_add_channels.import_failed", map[string]interface{}{"DisplayName": newChannel.DisplayName}))
 				continue
 			}
@@ -563,7 +566,7 @@ func SlackConvertUserMentions(users []SlackUser, posts map[string][]SlackPost) m
 	for _, user := range users {
 		r, err := regexp.Compile("<@" + user.Id + `(\|` + user.Username + ")?>")
 		if err != nil {
-			mlog.Warn(fmt.Sprintf("Slack Import: Unable to compile the @mention, matching regular expression for the Slack user %v (id=%v).", user.Id, user.Username), mlog.String("user_id", user.Id))
+			mlog.Warn("Slack Import: Unable to compile the @mention, matching regular expression for the Slack user.", mlog.String("user_name", user.Username), mlog.String("user_id", user.Id))
 			continue
 		}
 		regexes["@"+user.Username] = r
@@ -591,7 +594,7 @@ func SlackConvertChannelMentions(channels []SlackChannel, posts map[string][]Sla
 	for _, channel := range channels {
 		r, err := regexp.Compile("<#" + channel.Id + `(\|` + channel.Name + ")?>")
 		if err != nil {
-			mlog.Warn(fmt.Sprintf("Slack Import: Unable to compile the !channel, matching regular expression for the Slack channel %v (id=%v).", channel.Id, channel.Name))
+			mlog.Warn("Slack Import: Unable to compile the !channel, matching regular expression for the Slack channel.", mlog.String("channel_id", channel.Id), mlog.String("channel_name", channel.Name))
 			continue
 		}
 		regexes["~"+channel.Name] = r
@@ -783,7 +786,7 @@ func (a *App) OldImportPost(post *model.Post) string {
 
 		_, err := a.Srv.Store.Post().Save(post)
 		if err != nil {
-			mlog.Debug(fmt.Sprintf("Error saving post. user=%v, message=%v", post.UserId, post.Message))
+			mlog.Debug("Error saving post.", mlog.String("user_id", post.UserId), mlog.String("message", post.Message))
 		}
 
 		if firstIteration {
@@ -792,7 +795,13 @@ func (a *App) OldImportPost(post *model.Post) string {
 			}
 			for _, fileId := range post.FileIds {
 				if err := a.Srv.Store.FileInfo().AttachToPost(fileId, post.Id, post.UserId); err != nil {
-					mlog.Error(fmt.Sprintf("Error attaching files to post. postId=%v, fileIds=%v, message=%v", post.Id, post.FileIds, err), mlog.String("post_id", post.Id))
+					mlog.Error(
+						"Error attaching files to post.",
+						mlog.String("post_id", post.Id),
+						mlog.String("file_ids", strings.Join(post.FileIds, ",")),
+						mlog.String("user_id", post.UserId),
+						mlog.Err(err),
+					)
 				}
 			}
 			post.FileIds = nil
@@ -813,16 +822,16 @@ func (a *App) OldImportUser(team *model.Team, user *model.User) *model.User {
 
 	ruser, err := a.Srv.Store.User().Save(user)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Error saving user. err=%v", err))
+		mlog.Error("Error saving user.", mlog.Err(err))
 		return nil
 	}
 
 	if _, err = a.Srv.Store.User().VerifyEmail(ruser.Id, ruser.Email); err != nil {
-		mlog.Error(fmt.Sprintf("Failed to set email verified err=%v", err))
+		mlog.Error("Failed to set email verified.", mlog.Err(err))
 	}
 
 	if err = a.JoinUserToTeam(team, user, ""); err != nil {
-		mlog.Error(fmt.Sprintf("Failed to join team when importing err=%v", err))
+		mlog.Error("Failed to join team when importing.", mlog.Err(err))
 	}
 
 	return ruser

app/team_test.go

@@ -26,14 +26,11 @@ func TestCreateTeam(t *testing.T) {
 		Type:        model.TEAM_OPEN,
 	}
 
-	if _, err := th.App.CreateTeam(team); err != nil {
+	_, err := th.App.CreateTeam(team)
-		t.Log(err)
+	require.Nil(t, err, "Should create a new team")
-		t.Fatal("Should create a new team")
-	}
 
-	if _, err := th.App.CreateTeam(th.BasicTeam); err == nil {
+	_, err = th.App.CreateTeam(th.BasicTeam)
-		t.Fatal("Should not create a new team - team already exist")
+	require.NotNil(t, err, "Should not create a new team - team already exist")
-	}
 }
 
 func TestCreateTeamWithUser(t *testing.T) {
@@ -48,13 +45,11 @@ func TestCreateTeamWithUser(t *testing.T) {
 		Type:        model.TEAM_OPEN,
 	}
 
-	if _, err := th.App.CreateTeamWithUser(team, th.BasicUser.Id); err != nil {
+	_, err := th.App.CreateTeamWithUser(team, th.BasicUser.Id)
-		t.Fatal("Should create a new team with existing user", err)
+	require.Nil(t, err, "Should create a new team with existing user")
-	}
 
-	if _, err := th.App.CreateTeamWithUser(team, model.NewId()); err == nil {
+	_, err = th.App.CreateTeamWithUser(team, model.NewId())
-		t.Fatal("Should not create a new team - user does not exist")
+	require.NotNil(t, err, "Should not create a new team - user does not exist")
-	}
 }
 
 func TestUpdateTeam(t *testing.T) {
@@ -63,14 +58,9 @@ func TestUpdateTeam(t *testing.T) {
 
 	th.BasicTeam.DisplayName = "Testing 123"
 
-	if updatedTeam, err := th.App.UpdateTeam(th.BasicTeam); err != nil {
+	updatedTeam, err := th.App.UpdateTeam(th.BasicTeam)
-		t.Log(err)
+	require.Nil(t, err, "Should update the team")
-		t.Fatal("Should update the team")
+	require.Equal(t, "Testing 123", updatedTeam.DisplayName, "Wrong Team DisplayName")
-	} else {
-		if updatedTeam.DisplayName != "Testing 123" {
-			t.Fatal("Wrong Team DisplayName")
-		}
-	}
 }
 
 func TestAddUserToTeam(t *testing.T) {
@@ -82,59 +72,45 @@ func TestAddUserToTeam(t *testing.T) {
 		ruser, _ := th.App.CreateUser(&user)
 		defer th.App.PermanentDeleteUser(&user)
 
-		if _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, ""); err != nil {
+		_, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "")
-			t.Log(err)
+		require.Nil(t, err, "Should add user to the team")
-			t.Fatal("Should add user to the team")
-		}
 	})
 
 	t.Run("allow user by domain", func(t *testing.T) {
 		th.BasicTeam.AllowedDomains = "example.com"
-		if _, err := th.App.UpdateTeam(th.BasicTeam); err != nil {
+		_, err := th.App.UpdateTeam(th.BasicTeam)
-			t.Log(err)
+		require.Nil(t, err, "Should update the team")
-			t.Fatal("Should update the team")
-		}
 
 		user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser, _ := th.App.CreateUser(&user)
 		defer th.App.PermanentDeleteUser(&user)
 
-		if _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, ""); err != nil {
+		_, err = th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "")
-			t.Log(err)
+		require.Nil(t, err, "Should have allowed whitelisted user")
-			t.Fatal("Should have allowed whitelisted user")
-		}
 	})
 
 	t.Run("block user by domain but allow bot", func(t *testing.T) {
 		th.BasicTeam.AllowedDomains = "example.com"
-		if _, err := th.App.UpdateTeam(th.BasicTeam); err != nil {
+		_, err := th.App.UpdateTeam(th.BasicTeam)
-			t.Log(err)
+		require.Nil(t, err, "Should update the team")
-			t.Fatal("Should update the team")
-		}
 
 		user := model.User{Email: strings.ToLower(model.NewId()) + "test@invalid.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser, err := th.App.CreateUser(&user)
-		if err != nil {
+		require.Nil(t, err, "Error creating user: %s", err)
-			t.Fatalf("Error creating user: %s", err)
-		}
 		defer th.App.PermanentDeleteUser(&user)
 
-		if _, err = th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, ""); err == nil || err.Where != "JoinUserToTeam" {
+		_, err = th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "")
-			t.Log(err)
+		require.NotNil(t, err, "Should not add restricted user")
-			t.Fatal("Should not add restricted user")
+		require.Equal(t, "JoinUserToTeam", err.Where, "Error should be JoinUserToTeam")
-		}
 
 		user = model.User{Email: strings.ToLower(model.NewId()) + "test@invalid.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), AuthService: "notnil", AuthData: model.NewString("notnil")}
 		ruser, err = th.App.CreateUser(&user)
-		if err != nil {
+		require.Nil(t, err, "Error creating authservice user: %s", err)
-			t.Fatalf("Error creating authservice user: %s", err)
-		}
 		defer th.App.PermanentDeleteUser(&user)
 
-		if _, err = th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, ""); err == nil || err.Where != "JoinUserToTeam" {
+		_, err = th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "")
-			t.Log(err)
+		require.NotNil(t, err, "Should not add authservice user")
-			t.Fatal("Should not add authservice user")
+		require.Equal(t, "JoinUserToTeam", err.Where, "Error should be JoinUserToTeam")
-		}
 
 		bot, err := th.App.CreateBot(&model.Bot{
 			Username:    "somebot",
@@ -149,50 +125,45 @@ func TestAddUserToTeam(t *testing.T) {
 
 	t.Run("block user with subdomain", func(t *testing.T) {
 		th.BasicTeam.AllowedDomains = "example.com"
-		if _, err := th.App.UpdateTeam(th.BasicTeam); err != nil {
+		_, err := th.App.UpdateTeam(th.BasicTeam)
-			t.Log(err)
+		require.Nil(t, err, "Should update the team")
-			t.Fatal("Should update the team")
-		}
 
 		user := model.User{Email: strings.ToLower(model.NewId()) + "test@invalid.example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser, _ := th.App.CreateUser(&user)
 		defer th.App.PermanentDeleteUser(&user)
 
-		if _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, ""); err == nil || err.Where != "JoinUserToTeam" {
+		_, err = th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "")
-			t.Log(err)
+		require.NotNil(t, err, "Should not add restricted user")
-			t.Fatal("Should not add restricted user")
+		require.Equal(t, "JoinUserToTeam", err.Where, "Error should be JoinUserToTeam")
-		}
 	})
 
 	t.Run("allow users by multiple domains", func(t *testing.T) {
 		th.BasicTeam.AllowedDomains = "foo.com, bar.com"
-		if _, err := th.App.UpdateTeam(th.BasicTeam); err != nil {
+		_, err := th.App.UpdateTeam(th.BasicTeam)
-			t.Log(err)
+		require.Nil(t, err, "Should update the team")
-			t.Fatal("Should update the team")
-		}
 
 		user1 := model.User{Email: strings.ToLower(model.NewId()) + "success+test@foo.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser1, _ := th.App.CreateUser(&user1)
+
 		user2 := model.User{Email: strings.ToLower(model.NewId()) + "success+test@bar.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser2, _ := th.App.CreateUser(&user2)
+
 		user3 := model.User{Email: strings.ToLower(model.NewId()) + "success+test@invalid.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser3, _ := th.App.CreateUser(&user3)
+
 		defer th.App.PermanentDeleteUser(&user1)
 		defer th.App.PermanentDeleteUser(&user2)
 		defer th.App.PermanentDeleteUser(&user3)
 
-		if _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser1.Id, ""); err != nil {
+		_, err = th.App.AddUserToTeam(th.BasicTeam.Id, ruser1.Id, "")
-			t.Log(err)
+		require.Nil(t, err, "Should have allowed whitelisted user1")
-			t.Fatal("Should have allowed whitelisted user1")
+
-		}
+		_, err = th.App.AddUserToTeam(th.BasicTeam.Id, ruser2.Id, "")
-		if _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser2.Id, ""); err != nil {
+		require.Nil(t, err, "Should have allowed whitelisted user2")
-			t.Log(err)
+
-			t.Fatal("Should have allowed whitelisted user2")
+		_, err = th.App.AddUserToTeam(th.BasicTeam.Id, ruser3.Id, "")
-		}
+		require.NotNil(t, err, "Should not have allowed restricted user3")
-		if _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser3.Id, ""); err == nil || err.Where != "JoinUserToTeam" {
+		require.Equal(t, "JoinUserToTeam", err.Where, "Error should be JoinUserToTeam")
-			t.Log(err)
-			t.Fatal("Should not have allowed restricted user3")
-		}
 
 	})
 }
@@ -206,9 +177,8 @@ func TestAddUserToTeamByToken(t *testing.T) {
 	rguest := th.CreateGuest()
 
 	t.Run("invalid token", func(t *testing.T) {
-		if _, err := th.App.AddUserToTeamByToken(ruser.Id, "123"); err == nil {
+		_, err := th.App.AddUserToTeamByToken(ruser.Id, "123")
-			t.Fatal("Should fail on unexisting token")
+		require.NotNil(t, err, "Should fail on unexisting token")
-		}
 	})
 
 	t.Run("invalid token type", func(t *testing.T) {
@@ -216,11 +186,12 @@ func TestAddUserToTeamByToken(t *testing.T) {
 			TOKEN_TYPE_VERIFY_EMAIL,
 			model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id}),
 		)
+
 		require.Nil(t, th.App.Srv.Store.Token().Save(token))
 		defer th.App.DeleteToken(token)
-		if _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token); err == nil {
+
-			t.Fatal("Should fail on bad token type")
+		_, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token)
-		}
+		require.NotNil(t, err, "Should fail on bad token type")
 	})
 
 	t.Run("expired token", func(t *testing.T) {
@@ -228,12 +199,13 @@ func TestAddUserToTeamByToken(t *testing.T) {
 			TOKEN_TYPE_TEAM_INVITATION,
 			model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id}),
 		)
+
 		token.CreateAt = model.GetMillis() - INVITATION_EXPIRY_TIME - 1
 		require.Nil(t, th.App.Srv.Store.Token().Save(token))
 		defer th.App.DeleteToken(token)
-		if _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token); err == nil {
+
-			t.Fatal("Should fail on expired token")
+		_, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token)
-		}
+		require.NotNil(t, err, "Should fail on expired token")
 	})
 
 	t.Run("invalid team id", func(t *testing.T) {
@@ -243,9 +215,9 @@ func TestAddUserToTeamByToken(t *testing.T) {
 		)
 		require.Nil(t, th.App.Srv.Store.Token().Save(token))
 		defer th.App.DeleteToken(token)
-		if _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token); err == nil {
+
-			t.Fatal("Should fail on bad team id")
+		_, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token)
-		}
+		require.NotNil(t, err, "Should fail on bad team id")
 	})
 
 	t.Run("invalid user id", func(t *testing.T) {
@@ -255,9 +227,9 @@ func TestAddUserToTeamByToken(t *testing.T) {
 		)
 		require.Nil(t, th.App.Srv.Store.Token().Save(token))
 		defer th.App.DeleteToken(token)
-		if _, err := th.App.AddUserToTeamByToken(model.NewId(), token.Token); err == nil {
+
-			t.Fatal("Should fail on bad user id")
+		_, err := th.App.AddUserToTeamByToken(model.NewId(), token.Token)
-		}
+		require.NotNil(t, err, "Should fail on bad user id")
 	})
 
 	t.Run("valid request", func(t *testing.T) {
@@ -266,11 +238,10 @@ func TestAddUserToTeamByToken(t *testing.T) {
 			model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id}),
 		)
 		require.Nil(t, th.App.Srv.Store.Token().Save(token))
-		if _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token); err != nil {
+		_, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token)
-			t.Log(err)
+		require.Nil(t, err, "Should add user to the team")
-			t.Fatal("Should add user to the team")
+
-		}
+		_, err = th.App.Srv.Store.Token().GetByToken(token.Token)
-		_, err := th.App.Srv.Store.Token().GetByToken(token.Token)
 		require.NotNil(t, err, "The token must be deleted after be used")
 
 		members, err := th.App.GetChannelMembersForUser(th.BasicTeam.Id, ruser.Id)
@@ -304,11 +275,11 @@ func TestAddUserToTeamByToken(t *testing.T) {
 			model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "channels": th.BasicChannel.Id}),
 		)
 		require.Nil(t, th.App.Srv.Store.Token().Save(token))
-		if _, err := th.App.AddUserToTeamByToken(rguest.Id, token.Token); err != nil {
+
-			t.Log(err)
+		_, err := th.App.AddUserToTeamByToken(rguest.Id, token.Token)
-			t.Fatal("Should add user to the team")
+		require.Nil(t, err, "Should add user to the team")
-		}
+
-		_, err := th.App.Srv.Store.Token().GetByToken(token.Token)
+		_, err = th.App.Srv.Store.Token().GetByToken(token.Token)
 		require.NotNil(t, err, "The token must be deleted after be used")
 
 		members, err := th.App.GetChannelMembersForUser(th.BasicTeam.Id, rguest.Id)
@@ -319,35 +290,28 @@ func TestAddUserToTeamByToken(t *testing.T) {
 
 	t.Run("group-constrained team", func(t *testing.T) {
 		th.BasicTeam.GroupConstrained = model.NewBool(true)
-		if _, err := th.App.UpdateTeam(th.BasicTeam); err != nil {
+		_, err := th.App.UpdateTeam(th.BasicTeam)
-			t.Log(err)
+		require.Nil(t, err, "Should update the team")
-			t.Fatal("Should update the team")
-		}
 
 		token := model.NewToken(
 			TOKEN_TYPE_TEAM_INVITATION,
 			model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id}),
 		)
 		require.Nil(t, th.App.Srv.Store.Token().Save(token))
-		if _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token); err == nil {
+
-			t.Fatal("Should return an error when trying to join a group-constrained team.")
+		_, err = th.App.AddUserToTeamByToken(ruser.Id, token.Token)
-		} else {
+		require.NotNil(t, err, "Should return an error when trying to join a group-constrained team.")
-			require.Equal(t, "app.team.invite_token.group_constrained.error", err.Id)
+		require.Equal(t, "app.team.invite_token.group_constrained.error", err.Id)
-		}
 
 		th.BasicTeam.GroupConstrained = model.NewBool(false)
-		if _, err := th.App.UpdateTeam(th.BasicTeam); err != nil {
+		_, err = th.App.UpdateTeam(th.BasicTeam)
-			t.Log(err)
+		require.Nil(t, err, "Should update the team")
-			t.Fatal("Should update the team")
-		}
 	})
 
 	t.Run("block user", func(t *testing.T) {
 		th.BasicTeam.AllowedDomains = "example.com"
-		if _, err := th.App.UpdateTeam(th.BasicTeam); err != nil {
+		_, err := th.App.UpdateTeam(th.BasicTeam)
-			t.Log(err)
+		require.Nil(t, err, "Should update the team")
-			t.Fatal("Should update the team")
-		}
 
 		user := model.User{Email: strings.ToLower(model.NewId()) + "test@invalid.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser, _ := th.App.CreateUser(&user)
@@ -359,10 +323,9 @@ func TestAddUserToTeamByToken(t *testing.T) {
 		)
 		require.Nil(t, th.App.Srv.Store.Token().Save(token))
 
-		if _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token); err == nil || err.Where != "JoinUserToTeam" {
+		_, err = th.App.AddUserToTeamByToken(ruser.Id, token.Token)
-			t.Log(err)
+		require.NotNil(t, err, "Should not add restricted user")
-			t.Fatal("Should not add restricted user")
+		require.Equal(t, "JoinUserToTeam", err.Where, "Error should be JoinUserToTeam")
-		}
 	})
 }
 
@@ -374,27 +337,22 @@ func TestAddUserToTeamByTeamId(t *testing.T) {
 		user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser, _ := th.App.CreateUser(&user)
 
-		if err := th.App.AddUserToTeamByTeamId(th.BasicTeam.Id, ruser); err != nil {
+		err := th.App.AddUserToTeamByTeamId(th.BasicTeam.Id, ruser)
-			t.Log(err)
+		require.Nil(t, err, "Should add user to the team")
-			t.Fatal("Should add user to the team")
-		}
 	})
 
 	t.Run("block user", func(t *testing.T) {
 		th.BasicTeam.AllowedDomains = "example.com"
-		if _, err := th.App.UpdateTeam(th.BasicTeam); err != nil {
+		_, err := th.App.UpdateTeam(th.BasicTeam)
-			t.Log(err)
+		require.Nil(t, err, "Should update the team")
-			t.Fatal("Should update the team")
-		}
 
 		user := model.User{Email: strings.ToLower(model.NewId()) + "test@invalid.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser, _ := th.App.CreateUser(&user)
 		defer th.App.PermanentDeleteUser(&user)
 
-		if err := th.App.AddUserToTeamByTeamId(th.BasicTeam.Id, ruser); err == nil || err.Where != "JoinUserToTeam" {
+		err = th.App.AddUserToTeamByTeamId(th.BasicTeam.Id, ruser)
-			t.Log(err)
+		require.NotNil(t, err, "Should not add restricted user")
-			t.Fatal("Should not add restricted user")
+		require.Equal(t, "JoinUserToTeam", err.Where, "Error should be JoinUserToTeam")
-		}
 	})
 
 }
@@ -409,9 +367,8 @@ func TestPermanentDeleteTeam(t *testing.T) {
 		Email:       "foo@foo.com",
 		Type:        model.TEAM_OPEN,
 	})
-	if err != nil {
+	require.Nil(t, err, "Should create a team")
-		t.Fatal(err.Error())
+
-	}
 	defer func() {
 		th.App.PermanentDeleteTeam(team)
 	}()
@@ -423,21 +380,19 @@ func TestPermanentDeleteTeam(t *testing.T) {
 		URL:       "http://foo",
 		Method:    model.COMMAND_METHOD_POST,
 	})
-	if err != nil {
+	require.Nil(t, err, "Should create a command")
-		t.Fatal(err.Error())
-	}
 	defer th.App.DeleteCommand(command.Id)
 
-	if command, err = th.App.GetCommand(command.Id); command == nil || err != nil {
+	command, err = th.App.GetCommand(command.Id)
-		t.Fatal("unable to get new command")
+	require.NotNil(t, command, "command should not be nil")
-	}
+	require.Nil(t, err, "unable to get new command")
 
 	err = th.App.PermanentDeleteTeam(team)
 	require.Nil(t, err)
 
-	if command, err = th.App.GetCommand(command.Id); command != nil || err == nil {
+	command, err = th.App.GetCommand(command.Id)
-		t.Fatal("command wasn't deleted")
+	require.Nil(t, command, "command wasn't deleted")
-	}
+	require.NotNil(t, err, "should not return an error")
 
 	// Test deleting a team with no channels.
 	team = th.CreateTeam()
@@ -445,19 +400,16 @@ func TestPermanentDeleteTeam(t *testing.T) {
 		th.App.PermanentDeleteTeam(team)
 	}()
 
-	if channels, err := th.App.GetPublicChannelsForTeam(team.Id, 0, 1000); err != nil {
+	channels, err := th.App.GetPublicChannelsForTeam(team.Id, 0, 1000)
-		t.Fatal(err)
+	require.Nil(t, err)
-	} else {
+
-		for _, channel := range *channels {
+	for _, channel := range *channels {
-			if err2 := th.App.PermanentDeleteChannel(channel); err2 != nil {
+		err2 := th.App.PermanentDeleteChannel(channel)
-				t.Fatal(err)
+		require.Nil(t, err2)
-			}
-		}
 	}
 
-	if err := th.App.PermanentDeleteTeam(team); err != nil {
+	err = th.App.PermanentDeleteTeam(team)
-		t.Fatal(err)
+	require.Nil(t, err)
-	}
 }
 
 func TestSanitizeTeam(t *testing.T) {
@@ -469,6 +421,7 @@ func TestSanitizeTeam(t *testing.T) {
 		Email:          th.MakeEmail(),
 		AllowedDomains: "example.com",
 	}
+
 	copyTeam := func() *model.Team {
 		copy := &model.Team{}
 		*copy = *team
@@ -489,9 +442,7 @@ func TestSanitizeTeam(t *testing.T) {
 		}
 
 		sanitized := th.App.SanitizeTeam(session, copyTeam())
-		if sanitized.Email != "" {
+		require.Empty(t, sanitized.Email, "should've sanitized team")
-			t.Fatal("should've sanitized team")
-		}
 	})
 
 	t.Run("user of the team", func(t *testing.T) {
@@ -508,9 +459,7 @@ func TestSanitizeTeam(t *testing.T) {
 		}
 
 		sanitized := th.App.SanitizeTeam(session, copyTeam())
-		if sanitized.Email != "" {
+		require.Empty(t, sanitized.Email, "should've sanitized team")
-			t.Fatal("should've sanitized team")
-		}
 	})
 
 	t.Run("team admin", func(t *testing.T) {
@@ -527,9 +476,7 @@ func TestSanitizeTeam(t *testing.T) {
 		}
 
 		sanitized := th.App.SanitizeTeam(session, copyTeam())
-		if sanitized.Email == "" {
+		require.NotEmpty(t, sanitized.Email, "shouldn't have sanitized team")
-			t.Fatal("shouldn't have sanitized team")
-		}
 	})
 
 	t.Run("team admin of another team", func(t *testing.T) {
@@ -546,9 +493,7 @@ func TestSanitizeTeam(t *testing.T) {
 		}
 
 		sanitized := th.App.SanitizeTeam(session, copyTeam())
-		if sanitized.Email != "" {
+		require.Empty(t, sanitized.Email, "should've sanitized team")
-			t.Fatal("should've sanitized team")
-		}
 	})
 
 	t.Run("system admin, not a user of team", func(t *testing.T) {
@@ -565,9 +510,7 @@ func TestSanitizeTeam(t *testing.T) {
 		}
 
 		sanitized := th.App.SanitizeTeam(session, copyTeam())
-		if sanitized.Email == "" {
+		require.NotEmpty(t, sanitized.Email, "shouldn't have sanitized team")
-			t.Fatal("shouldn't have sanitized team")
-		}
 	})
 
 	t.Run("system admin, user of team", func(t *testing.T) {
@@ -584,9 +527,7 @@ func TestSanitizeTeam(t *testing.T) {
 		}
 
 		sanitized := th.App.SanitizeTeam(session, copyTeam())
-		if sanitized.Email == "" {
+		require.NotEmpty(t, sanitized.Email, "shouldn't have sanitized team")
-			t.Fatal("shouldn't have sanitized team")
-		}
 	})
 }
 
@@ -627,13 +568,8 @@ func TestSanitizeTeams(t *testing.T) {
 
 		sanitized := th.App.SanitizeTeams(session, teams)
 
-		if sanitized[0].Email != "" {
+		require.Empty(t, sanitized[0].Email, "should've sanitized first team")
-			t.Fatal("should've sanitized first team")
+		require.NotEmpty(t, sanitized[1].Email, "shouldn't have sanitized second team")
-		}
-
-		if sanitized[1].Email == "" {
-			t.Fatal("shouldn't have sanitized second team")
-		}
 	})
 
 	t.Run("system admin", func(t *testing.T) {
@@ -663,14 +599,8 @@ func TestSanitizeTeams(t *testing.T) {
 		}
 
 		sanitized := th.App.SanitizeTeams(session, teams)
-
+		assert.NotEmpty(t, sanitized[0].Email, "shouldn't have sanitized first team")
-		if sanitized[0].Email == "" {
+		assert.NotEmpty(t, sanitized[1].Email, "shouldn't have sanitized second team")
-			t.Fatal("shouldn't have sanitized first team")
-		}
-
-		if sanitized[1].Email == "" {
-			t.Fatal("shouldn't have sanitized second team")
-		}
 	})
 }
 
@@ -686,10 +616,8 @@ func TestJoinUserToTeam(t *testing.T) {
 		Type:        model.TEAM_OPEN,
 	}
 
-	if _, err := th.App.CreateTeam(team); err != nil {
+	_, err := th.App.CreateTeam(team)
-		t.Log(err)
+	require.Nil(t, err, "Should create a new team")
-		t.Fatal("Should create a new team")
-	}
 
 	maxUsersPerTeam := th.App.Config().TeamSettings.MaxUsersPerTeam
 	defer func() {
@@ -704,9 +632,9 @@ func TestJoinUserToTeam(t *testing.T) {
 		ruser, _ := th.App.CreateUser(&user)
 		defer th.App.PermanentDeleteUser(&user)
 
-		if _, alreadyAdded, err := th.App.joinUserToTeam(team, ruser); alreadyAdded || err != nil {
+		_, alreadyAdded, err := th.App.joinUserToTeam(team, ruser)
-			t.Fatal("Should return already added equal to false and no error")
+		require.False(t, alreadyAdded, "Should return already added equal to false")
-		}
+		require.Nil(t, err, "Should return no error")
 	})
 
 	t.Run("join when you are a member", func(t *testing.T) {
@@ -715,9 +643,10 @@ func TestJoinUserToTeam(t *testing.T) {
 		defer th.App.PermanentDeleteUser(&user)
 
 		th.App.joinUserToTeam(team, ruser)
-		if _, alreadyAdded, err := th.App.joinUserToTeam(team, ruser); !alreadyAdded || err != nil {
+
-			t.Fatal("Should return already added and no error")
+		_, alreadyAdded, err := th.App.joinUserToTeam(team, ruser)
-		}
+		require.True(t, alreadyAdded, "Should return already added")
+		require.Nil(t, err, "Should return no error")
 	})
 
 	t.Run("re-join after leaving", func(t *testing.T) {
@@ -727,9 +656,10 @@ func TestJoinUserToTeam(t *testing.T) {
 
 		th.App.joinUserToTeam(team, ruser)
 		th.App.LeaveTeam(team, ruser, ruser.Id)
-		if _, alreadyAdded, err := th.App.joinUserToTeam(team, ruser); alreadyAdded || err != nil {
+
-			t.Fatal("Should return already added equal to false and no error")
+		_, alreadyAdded, err := th.App.joinUserToTeam(team, ruser)
-		}
+		require.False(t, alreadyAdded, "Should return already added equal to false")
+		require.Nil(t, err, "Should return no error")
 	})
 
 	t.Run("new join with limit problem", func(t *testing.T) {
@@ -737,28 +667,31 @@ func TestJoinUserToTeam(t *testing.T) {
 		ruser1, _ := th.App.CreateUser(&user1)
 		user2 := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser2, _ := th.App.CreateUser(&user2)
+
 		defer th.App.PermanentDeleteUser(&user1)
 		defer th.App.PermanentDeleteUser(&user2)
 		th.App.joinUserToTeam(team, ruser1)
-		if _, _, err := th.App.joinUserToTeam(team, ruser2); err == nil {
+
-			t.Fatal("Should fail")
+		_, _, err := th.App.joinUserToTeam(team, ruser2)
-		}
+		require.NotNil(t, err, "Should fail")
 	})
 
 	t.Run("re-join alfter leaving with limit problem", func(t *testing.T) {
 		user1 := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser1, _ := th.App.CreateUser(&user1)
+
 		user2 := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""}
 		ruser2, _ := th.App.CreateUser(&user2)
+
 		defer th.App.PermanentDeleteUser(&user1)
 		defer th.App.PermanentDeleteUser(&user2)
 
 		th.App.joinUserToTeam(team, ruser1)
 		th.App.LeaveTeam(team, ruser1, ruser1.Id)
 		th.App.joinUserToTeam(team, ruser2)
-		if _, _, err := th.App.joinUserToTeam(team, ruser1); err == nil {
+
-			t.Fatal("Should fail")
+		_, _, err := th.App.joinUserToTeam(team, ruser1)
-		}
+		require.NotNil(t, err, "Should fail")
 	})
 }
 
@@ -771,13 +704,8 @@ func TestAppUpdateTeamScheme(t *testing.T) {
 	team.SchemeId = mockID
 
 	updatedTeam, err := th.App.UpdateTeamScheme(th.BasicTeam)
-	if err != nil {
+	require.Nil(t, err)
-		t.Fatal(err)
+	require.Equal(t, mockID, updatedTeam.SchemeId, "Wrong Team SchemeId")
-	}
-
-	if updatedTeam.SchemeId != mockID {
-		t.Fatal("Wrong Team SchemeId")
-	}
 }
 
 func TestGetTeamMembers(t *testing.T) {
@@ -811,6 +739,7 @@ func TestGetTeamMembers(t *testing.T) {
 	// Fetch team members multipile times
 	members, err := th.App.GetTeamMembers(th.BasicTeam.Id, 0, 5, nil)
 	require.Nil(t, err)
+
 	// This should return 5 members
 	members2, err := th.App.GetTeamMembers(th.BasicTeam.Id, 5, 6, nil)
 	require.Nil(t, err)
@@ -897,9 +826,8 @@ func TestUpdateTeamMemberRolesChangingGuest(t *testing.T) {
 		_, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "")
 		require.Nil(t, err)
 
-		if _, err := th.App.UpdateTeamMemberRoles(th.BasicTeam.Id, ruser.Id, "team_user"); err == nil {
+		_, err = th.App.UpdateTeamMemberRoles(th.BasicTeam.Id, ruser.Id, "team_user")
-			t.Fatal("Should fail when try to modify the guest role")
+		require.NotNil(t, err, "Should fail when try to modify the guest role")
-		}
 	})
 
 	t.Run("from user to guest", func(t *testing.T) {
@@ -909,9 +837,8 @@ func TestUpdateTeamMemberRolesChangingGuest(t *testing.T) {
 		_, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "")
 		require.Nil(t, err)
 
-		if _, err := th.App.UpdateTeamMemberRoles(th.BasicTeam.Id, ruser.Id, "team_guest"); err == nil {
+		_, err = th.App.UpdateTeamMemberRoles(th.BasicTeam.Id, ruser.Id, "team_guest")
-			t.Fatal("Should fail when try to modify the guest role")
+		require.NotNil(t, err, "Should fail when try to modify the guest role")
-		}
 	})
 
 	t.Run("from user to admin", func(t *testing.T) {
@@ -921,9 +848,8 @@ func TestUpdateTeamMemberRolesChangingGuest(t *testing.T) {
 		_, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "")
 		require.Nil(t, err)
 
-		if _, err := th.App.UpdateTeamMemberRoles(th.BasicTeam.Id, ruser.Id, "team_user team_admin"); err != nil {
+		_, err = th.App.UpdateTeamMemberRoles(th.BasicTeam.Id, ruser.Id, "team_user team_admin")
-			t.Fatal("Should work when you not modify guest role")
+		require.Nil(t, err, "Should work when you not modify guest role")
-		}
 	})
 
 	t.Run("from guest to guest plus custom", func(t *testing.T) {
@@ -936,9 +862,8 @@ func TestUpdateTeamMemberRolesChangingGuest(t *testing.T) {
 		_, err = th.App.CreateRole(&model.Role{Name: "custom", DisplayName: "custom", Description: "custom"})
 		require.Nil(t, err)
 
-		if _, err := th.App.UpdateTeamMemberRoles(th.BasicTeam.Id, ruser.Id, "team_guest custom"); err != nil {
+		_, err = th.App.UpdateTeamMemberRoles(th.BasicTeam.Id, ruser.Id, "team_guest custom")
-			t.Fatal("Should work when you not modify guest role")
+		require.Nil(t, err, "Should work when you not modify guest role")
-		}
 	})
 
 	t.Run("a guest cant have user role", func(t *testing.T) {
@@ -948,9 +873,8 @@ func TestUpdateTeamMemberRolesChangingGuest(t *testing.T) {
 		_, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "")
 		require.Nil(t, err)
 
-		if _, err := th.App.UpdateTeamMemberRoles(th.BasicTeam.Id, ruser.Id, "team_guest team_user"); err == nil {
+		_, err = th.App.UpdateTeamMemberRoles(th.BasicTeam.Id, ruser.Id, "team_guest team_user")
-			t.Fatal("Should work when you not modify guest role")
+		require.NotNil(t, err, "Should work when you not modify guest role")
-		}
 	})
 }
 

app/user.go

@@ -95,7 +95,7 @@ func (a *App) CreateUserWithToken(user *model.User, token *model.Token) (*model.
 		for _, channel := range channels {
 			_, err := a.AddChannelMember(ruser.Id, channel, "", "")
 			if err != nil {
-				mlog.Error(err.Error())
+				mlog.Error("Failed to add channel member", mlog.Err(err))
 			}
 		}
 	}
@@ -135,7 +135,7 @@ func (a *App) CreateUserWithInviteId(user *model.User, inviteId string) (*model.
 	a.AddDirectChannels(team.Id, ruser)
 
 	if err := a.SendWelcomeEmail(ruser.Id, ruser.Email, ruser.EmailVerified, ruser.Locale, a.GetSiteURL()); err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Failed to send welcome email on create user with inviteId", mlog.Err(err))
 	}
 
 	return ruser, nil
@@ -148,7 +148,7 @@ func (a *App) CreateUserAsAdmin(user *model.User) (*model.User, *model.AppError)
 	}
 
 	if err := a.SendWelcomeEmail(ruser.Id, ruser.Email, ruser.EmailVerified, ruser.Locale, a.GetSiteURL()); err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Failed to send welcome email on create admin user", mlog.Err(err))
 	}
 
 	return ruser, nil
@@ -172,7 +172,7 @@ func (a *App) CreateUserFromSignup(user *model.User) (*model.User, *model.AppErr
 	}
 
 	if err := a.SendWelcomeEmail(ruser.Id, ruser.Email, ruser.EmailVerified, ruser.Locale, a.GetSiteURL()); err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Failed to send welcome email on create user from signup", mlog.Err(err))
 	}
 
 	return ruser, nil
@@ -190,7 +190,7 @@ func (a *App) IsFirstUserAccount() bool {
 	if a.SessionCacheLength() == 0 {
 		count, err := a.Srv.Store.User().Count(model.UserCountOptions{IncludeDeleted: true})
 		if err != nil {
-			mlog.Error(fmt.Sprint(err))
+			mlog.Error("There was a error fetching if first user account", mlog.Err(err))
 			return false
 		}
 		if count <= 0 {
@@ -314,19 +314,19 @@ func (a *App) createUser(user *model.User) (*model.User, *model.AppError) {
 
 	ruser, err := a.Srv.Store.User().Save(user)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Couldn't save the user err=%v", err))
+		mlog.Error("Couldn't save the user", mlog.Err(err))
 		return nil, err
 	}
 
 	if user.EmailVerified {
 		if err := a.VerifyUserEmail(ruser.Id, user.Email); err != nil {
-			mlog.Error(fmt.Sprintf("Failed to set email verified err=%v", err))
+			mlog.Error("Failed to set email verified", mlog.Err(err))
 		}
 	}
 
 	pref := model.Preference{UserId: ruser.Id, Category: model.PREFERENCE_CATEGORY_TUTORIAL_STEPS, Name: ruser.Id, Value: "0"}
 	if err := a.Srv.Store.Preference().Save(&model.Preferences{pref}); err != nil {
-		mlog.Error(fmt.Sprintf("Encountered error saving tutorial preference, err=%v", err.Message))
+		mlog.Error("Encountered error saving tutorial preference", mlog.Err(err))
 	}
 
 	ruser.Sanitize(map[string]bool{})
@@ -397,7 +397,7 @@ func (a *App) CreateOAuthUser(service string, userData io.Reader, teamId string)
 
 		err = a.AddDirectChannels(teamId, user)
 		if err != nil {
-			mlog.Error(err.Error())
+			mlog.Error("Failed to add direct channels", mlog.Err(err))
 		}
 	}
 
@@ -589,8 +589,8 @@ func (a *App) GetUsersNotInChannelPage(teamId string, channelId string, groupCon
 	return a.sanitizeProfiles(users, asAdmin), nil
 }
 
-func (a *App) GetUsersWithoutTeamPage(page int, perPage int, asAdmin bool, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) {
+func (a *App) GetUsersWithoutTeamPage(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) {
-	users, err := a.GetUsersWithoutTeam(page*perPage, perPage, viewRestrictions)
+	users, err := a.GetUsersWithoutTeam(options)
 	if err != nil {
 		return nil, err
 	}
@@ -598,8 +598,8 @@ func (a *App) GetUsersWithoutTeamPage(page int, perPage int, asAdmin bool, viewR
 	return a.sanitizeProfiles(users, asAdmin), nil
 }
 
-func (a *App) GetUsersWithoutTeam(offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) {
+func (a *App) GetUsersWithoutTeam(options *model.UserGetOptions) ([]*model.User, *model.AppError) {
-	return a.Srv.Store.User().GetProfilesWithoutTeam(offset, limit, viewRestrictions)
+	return a.Srv.Store.User().GetProfilesWithoutTeam(options)
 }
 
 // GetTeamGroupUsers returns the users who are associated to the team via GroupTeams and GroupMembers.
@@ -835,14 +835,14 @@ func (a *App) SetDefaultProfileImage(user *model.User) *model.AppError {
 	}
 
 	if err := a.Srv.Store.User().ResetLastPictureUpdate(user.Id); err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Failed to reset last picture update", mlog.Err(err))
 	}
 
 	a.InvalidateCacheForUser(user.Id)
 
 	updatedUser, appErr := a.GetUser(user.Id)
 	if appErr != nil {
-		mlog.Error(fmt.Sprintf("Error in getting users profile for id=%v forcing logout", user.Id), mlog.String("user_id", user.Id))
+		mlog.Error("Error in getting users profile forcing logout", mlog.String("user_id", user.Id), mlog.Err(appErr))
 		return nil
 	}
 
@@ -908,7 +908,7 @@ func (a *App) SetProfileImageFromFile(userId string, file io.Reader) *model.AppE
 	}
 
 	if err := a.Srv.Store.User().UpdateLastPictureUpdate(userId); err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Error with updating last picture update", mlog.Err(err))
 	}
 	a.invalidateUserCacheAndPublish(userId)
 
@@ -978,10 +978,11 @@ func (a *App) invalidateUserChannelMembersCaches(user *model.User) *model.AppErr
 }
 
 func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.AppError) {
+	user.UpdateAt = model.GetMillis()
 	if active {
 		user.DeleteAt = 0
 	} else {
-		user.DeleteAt = model.GetMillis()
+		user.DeleteAt = user.UpdateAt
 	}
 
 	userUpdate, err := a.Srv.Store.User().Update(user, true)
@@ -997,6 +998,7 @@ func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.A
 	}
 
 	a.invalidateUserChannelMembersCaches(user)
+	a.InvalidateCacheForUser(user.Id)
 
 	a.sendUpdatedUserEvent(*ruser)
 
@@ -1135,13 +1137,13 @@ func (a *App) UpdateUser(user *model.User, sendNotifications bool) (*model.User,
 			if *a.Config().EmailSettings.RequireEmailVerification {
 				a.Srv.Go(func() {
 					if err := a.SendEmailVerification(userUpdate.New, newEmail); err != nil {
-						mlog.Error(err.Error())
+						mlog.Error("Failed to send email verification", mlog.Err(err))
 					}
 				})
 			} else {
 				a.Srv.Go(func() {
 					if err := a.SendEmailChangeEmail(userUpdate.Old.Email, userUpdate.New.Email, userUpdate.New.Locale, a.GetSiteURL()); err != nil {
-						mlog.Error(err.Error())
+						mlog.Error("Failed to send email change email", mlog.Err(err))
 					}
 				})
 			}
@@ -1150,7 +1152,7 @@ func (a *App) UpdateUser(user *model.User, sendNotifications bool) (*model.User,
 		if userUpdate.New.Username != userUpdate.Old.Username {
 			a.Srv.Go(func() {
 				if err := a.SendChangeUsernameEmail(userUpdate.Old.Username, userUpdate.New.Username, userUpdate.New.Email, userUpdate.New.Locale, a.GetSiteURL()); err != nil {
-					mlog.Error(err.Error())
+					mlog.Error("Failed to send change username email", mlog.Err(err))
 				}
 			})
 		}
@@ -1212,12 +1214,12 @@ func (a *App) UpdateMfa(activate bool, userId, token string) *model.AppError {
 	a.Srv.Go(func() {
 		user, err := a.GetUser(userId)
 		if err != nil {
-			mlog.Error(err.Error())
+			mlog.Error("Failed to get user", mlog.Err(err))
 			return
 		}
 
 		if err := a.SendMfaChangeEmail(user.Email, activate, user.Locale, a.GetSiteURL()); err != nil {
-			mlog.Error(err.Error())
+			mlog.Error("Failed to send mfa change email", mlog.Err(err))
 		}
 	})
 
@@ -1254,7 +1256,7 @@ func (a *App) UpdatePasswordSendEmail(user *model.User, newPassword, method stri
 
 	a.Srv.Go(func() {
 		if err := a.SendPasswordChangeEmail(user.Email, method, user.Locale, a.GetSiteURL()); err != nil {
-			mlog.Error(err.Error())
+			mlog.Error("Failed to send password change email", mlog.Err(err))
 		}
 	})
 
@@ -1300,7 +1302,7 @@ func (a *App) ResetPasswordFromToken(userSuppliedTokenString, newPassword string
 	}
 
 	if err := a.DeleteToken(token); err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Failed to delete token", mlog.Err(err))
 	}
 
 	return nil
@@ -1397,7 +1399,7 @@ func (a *App) UpdateUserRoles(userId string, newRoles string, sendWebSocketEvent
 
 	if result := <-schan; result.Err != nil {
 		// soft error since the user roles were still updated
-		mlog.Error(fmt.Sprint(result.Err))
+		mlog.Error("Failed during updating user roles", mlog.Err(result.Err))
 	}
 
 	a.ClearSessionCacheForUser(user.Id)
@@ -1413,9 +1415,9 @@ func (a *App) UpdateUserRoles(userId string, newRoles string, sendWebSocketEvent
 }
 
 func (a *App) PermanentDeleteUser(user *model.User) *model.AppError {
-	mlog.Warn(fmt.Sprintf("Attempting to permanently delete account %v id=%v", user.Email, user.Id), mlog.String("user_id", user.Id))
+	mlog.Warn("Attempting to permanently delete account", mlog.String("user_id", user.Id), mlog.String("user_email", user.Email))
 	if user.IsInRole(model.SYSTEM_ADMIN_ROLE_ID) {
-		mlog.Warn(fmt.Sprintf("You are deleting %v that is a system administrator.  You may need to set another account as the system administrator using the command line tools.", user.Email))
+		mlog.Warn("You are deleting a user that is a system administrator.  You may need to set another account as the system administrator using the command line tools.", mlog.String("user_email", user.Email))
 	}
 
 	if _, err := a.UpdateActive(user, false); err != nil {
@@ -1464,7 +1466,7 @@ func (a *App) PermanentDeleteUser(user *model.User) *model.AppError {
 
 	infos, err := a.Srv.Store.FileInfo().GetForUser(user.Id)
 	if err != nil {
-		mlog.Warn("Error getting file list for user from FileInfoStore")
+		mlog.Warn("Error getting file list for user from FileInfoStore", mlog.Err(err))
 	}
 
 	for _, info := range infos {
@@ -1510,7 +1512,7 @@ func (a *App) PermanentDeleteUser(user *model.User) *model.AppError {
 		return err
 	}
 
-	mlog.Warn(fmt.Sprintf("Permanently deleted account %v id=%v", user.Email, user.Id), mlog.String("user_id", user.Id))
+	mlog.Warn("Permanently deleted account", mlog.String("user_email", user.Email), mlog.String("user_id", user.Id))
 
 	if a.IsESIndexingEnabled() {
 		a.Srv.Go(func() {
@@ -1578,13 +1580,13 @@ func (a *App) VerifyEmailFromToken(userSuppliedTokenString string) *model.AppErr
 	if user.Email != tokenData.Email {
 		a.Srv.Go(func() {
 			if err := a.SendEmailChangeEmail(user.Email, tokenData.Email, user.Locale, a.GetSiteURL()); err != nil {
-				mlog.Error(err.Error())
+				mlog.Error("Failed to send email change email", mlog.Err(err))
 			}
 		})
 	}
 
 	if err := a.DeleteToken(token); err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Failed to delete token", mlog.Err(err))
 	}
 
 	return nil
@@ -2240,13 +2242,13 @@ func (a *App) PromoteGuestToUser(user *model.User, requestorId string) *model.Ap
 	for _, team := range userTeams {
 		// Soft error if there is an issue joining the default channels
 		if err = a.JoinDefaultChannels(team.Id, user, false, requestorId); err != nil {
-			mlog.Error(fmt.Sprintf("Encountered an issue joining default channels err=%v", err), mlog.String("user_id", user.Id), mlog.String("team_id", team.Id), mlog.String("requestor_id", requestorId))
+			mlog.Error("Failed to join default channels", mlog.String("user_id", user.Id), mlog.String("team_id", team.Id), mlog.String("requestor_id", requestorId), mlog.Err(err))
 		}
 	}
 
 	promotedUser, err := a.GetUser(user.Id)
 	if err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Failed to get user on promote guest to user", mlog.Err(err))
 	} else {
 		a.sendUpdatedUserEvent(*promotedUser)
 		a.UpdateSessionsIsGuest(promotedUser.Id, promotedUser.IsGuest())
@@ -2254,7 +2256,7 @@ func (a *App) PromoteGuestToUser(user *model.User, requestorId string) *model.Ap
 
 	teamMembers, err := a.GetTeamMembersForUser(user.Id)
 	if err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Failed to get team members for user on promote guest to user", mlog.Err(err))
 	}
 
 	for _, member := range teamMembers {
@@ -2262,7 +2264,7 @@ func (a *App) PromoteGuestToUser(user *model.User, requestorId string) *model.Ap
 
 		channelMembers, err := a.GetChannelMembersForUser(member.TeamId, user.Id)
 		if err != nil {
-			mlog.Error(err.Error())
+			mlog.Error("Failed to get channel members for user on promote guest to user", mlog.Err(err))
 		}
 
 		for _, member := range *channelMembers {
@@ -2285,7 +2287,7 @@ func (a *App) DemoteUserToGuest(user *model.User) *model.AppError {
 
 	demotedUser, err := a.GetUser(user.Id)
 	if err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Failed to get user on demote user to guest", mlog.Err(err))
 	} else {
 		a.sendUpdatedUserEvent(*demotedUser)
 		a.UpdateSessionsIsGuest(demotedUser.Id, demotedUser.IsGuest())
@@ -2293,7 +2295,7 @@ func (a *App) DemoteUserToGuest(user *model.User) *model.AppError {
 
 	teamMembers, err := a.GetTeamMembersForUser(user.Id)
 	if err != nil {
-		mlog.Error(err.Error())
+		mlog.Error("Failed to get team members for users on demote user to guest", mlog.Err(err))
 	}
 
 	for _, member := range teamMembers {
@@ -2301,7 +2303,7 @@ func (a *App) DemoteUserToGuest(user *model.User) *model.AppError {
 
 		channelMembers, err := a.GetChannelMembersForUser(member.TeamId, user.Id)
 		if err != nil {
-			mlog.Error(err.Error())
+			mlog.Error("Failed to get channel members for users on demote user to guest", mlog.Err(err))
 		}
 
 		for _, member := range *channelMembers {
@@ -2320,7 +2322,7 @@ func (a *App) invalidateUserCacheAndPublish(userId string) {
 
 	user, userErr := a.GetUser(userId)
 	if userErr != nil {
-		mlog.Error(fmt.Sprintf("Error in getting users profile for id=%v, err=%v", userId, userErr.Error()), mlog.String("user_id", userId))
+		mlog.Error("Error in getting users profile", mlog.String("user_id", userId), mlog.Err(userErr))
 		return
 	}
 

app/user_viewmembers_test.go

@@ -618,7 +618,7 @@ func TestResctrictedViewMembers(t *testing.T) {
 
 		for _, tc := range testCases {
 			t.Run(tc.Name, func(t *testing.T) {
-				results, err := th.App.GetUsersWithoutTeam(0, 100, tc.Restrictions)
+				results, err := th.App.GetUsersWithoutTeam(&model.UserGetOptions{Page: 0, PerPage: 100, ViewRestrictions: tc.Restrictions})
 				require.Nil(t, err)
 				ids := []string{}
 				for _, result := range results {

app/web_conn.go

@@ -144,9 +144,9 @@ func (c *WebConn) readPump() {
 		if err := c.WebSocket.ReadJSON(&req); err != nil {
 			// browsers will appear as CloseNoStatusReceived
 			if websocket.IsCloseError(err, websocket.CloseNormalClosure, websocket.CloseNoStatusReceived) {
-				mlog.Debug(fmt.Sprintf("websocket.read: client side closed socket userId=%v", c.UserId))
+				mlog.Debug("websocket.read: client side closed socket", mlog.String("user_id", c.UserId))
 			} else {
-				mlog.Debug(fmt.Sprintf("websocket.read: closing websocket for userId=%v error=%v", c.UserId, err.Error()))
+				mlog.Debug("websocket.read: closing websocket", mlog.String("user_id", c.UserId), mlog.Err(err))
 			}
 			return
 		}
@@ -181,7 +181,12 @@ func (c *WebConn) writePump() {
 				if msg.EventType() == model.WEBSOCKET_EVENT_TYPING ||
 					msg.EventType() == model.WEBSOCKET_EVENT_STATUS_CHANGE ||
 					msg.EventType() == model.WEBSOCKET_EVENT_CHANNEL_VIEWED {
-					mlog.Info(fmt.Sprintf("websocket.slow: dropping message userId=%v type=%v channelId=%v", c.UserId, msg.EventType(), evt.Broadcast.ChannelId))
+					mlog.Info(
+						"websocket.slow: dropping message",
+						mlog.String("user_id", c.UserId),
+						mlog.String("type", msg.EventType()),
+						mlog.String("channel_id", evt.Broadcast.ChannelId),
+					)
 					skipSend = true
 				}
 			}
@@ -200,9 +205,20 @@ func (c *WebConn) writePump() {
 
 				if len(c.Send) >= SEND_DEADLOCK_WARN {
 					if evtOk {
-						mlog.Warn(fmt.Sprintf("websocket.full: message userId=%v type=%v channelId=%v size=%v", c.UserId, msg.EventType(), evt.Broadcast.ChannelId, len(msg.ToJson())))
+						mlog.Warn(
+							"websocket.full",
+							mlog.String("user_id", c.UserId),
+							mlog.String("type", msg.EventType()),
+							mlog.String("channel_id", evt.Broadcast.ChannelId),
+							mlog.Int("size", len(msg.ToJson())),
+						)
 					} else {
-						mlog.Warn(fmt.Sprintf("websocket.full: message userId=%v type=%v size=%v", c.UserId, msg.EventType(), len(msg.ToJson())))
+						mlog.Warn(
+							"websocket.full",
+							mlog.String("user_id", c.UserId),
+							mlog.String("type", msg.EventType()),
+							mlog.Int("size", len(msg.ToJson())),
+						)
 					}
 				}
 
@@ -210,9 +226,9 @@ func (c *WebConn) writePump() {
 				if err := c.WebSocket.WriteMessage(websocket.TextMessage, msgBytes); err != nil {
 					// browsers will appear as CloseNoStatusReceived
 					if websocket.IsCloseError(err, websocket.CloseNormalClosure, websocket.CloseNoStatusReceived) {
-						mlog.Debug(fmt.Sprintf("websocket.send: client side closed socket userId=%v", c.UserId))
+						mlog.Debug("websocket.send: client side closed socket", mlog.String("user_id", c.UserId))
 					} else {
-						mlog.Debug(fmt.Sprintf("websocket.send: closing websocket for userId=%v, error=%v", c.UserId, err.Error()))
+						mlog.Debug("websocket.send: closing websocket", mlog.String("user_id", c.UserId), mlog.Err(err))
 					}
 					return
 				}
@@ -229,9 +245,9 @@ func (c *WebConn) writePump() {
 			if err := c.WebSocket.WriteMessage(websocket.PingMessage, []byte{}); err != nil {
 				// browsers will appear as CloseNoStatusReceived
 				if websocket.IsCloseError(err, websocket.CloseNormalClosure, websocket.CloseNoStatusReceived) {
-					mlog.Debug(fmt.Sprintf("websocket.ticker: client side closed socket userId=%v", c.UserId))
+					mlog.Debug("websocket.ticker: client side closed socket", mlog.String("user_id", c.UserId))
 				} else {
-					mlog.Debug(fmt.Sprintf("websocket.ticker: closing websocket for userId=%v error=%v", c.UserId, err.Error()))
+					mlog.Debug("websocket.ticker: closing websocket", mlog.String("user_id", c.UserId), mlog.Err(err))
 				}
 				return
 			}
@@ -241,7 +257,7 @@ func (c *WebConn) writePump() {
 
 		case <-authTicker.C:
 			if c.GetSessionToken() == "" {
-				mlog.Debug(fmt.Sprintf("websocket.authTicker: did not authenticate ip=%v", c.WebSocket.RemoteAddr()))
+				mlog.Debug("websocket.authTicker: did not authenticate", mlog.Any("ip_address", c.WebSocket.RemoteAddr()))
 				return
 			}
 			authTicker.Stop()
@@ -265,7 +281,7 @@ func (webCon *WebConn) IsAuthenticated() bool {
 
 		session, err := webCon.App.GetSession(webCon.GetSessionToken())
 		if err != nil {
-			mlog.Error(fmt.Sprintf("Invalid session err=%v", err.Error()))
+			mlog.Error("Invalid session.", mlog.Err(err))
 			webCon.SetSessionToken("")
 			webCon.SetSession(nil)
 			webCon.SetSessionExpiresAt(0)
@@ -338,7 +354,7 @@ func (webCon *WebConn) ShouldSendEvent(msg *model.WebSocketEvent) bool {
 		if webCon.AllChannelMembers == nil {
 			result, err := webCon.App.Srv.Store.Channel().GetAllChannelMembersForUser(webCon.UserId, true, false)
 			if err != nil {
-				mlog.Error("webhub.shouldSendEvent: " + err.Error())
+				mlog.Error("webhub.shouldSendEvent.", mlog.Err(err))
 				return false
 			}
 			webCon.AllChannelMembers = result
@@ -359,7 +375,7 @@ func (webCon *WebConn) ShouldSendEvent(msg *model.WebSocketEvent) bool {
 	if msg.Event == model.WEBSOCKET_EVENT_USER_UPDATED && webCon.GetSession().Props[model.SESSION_PROP_IS_GUEST] == "true" {
 		canSee, err := webCon.App.UserCanSeeOtherUser(webCon.UserId, msg.Data["user"].(*model.User).Id)
 		if err != nil {
-			mlog.Error("webhub.shouldSendEvent: " + err.Error())
+			mlog.Error("webhub.shouldSendEvent.", mlog.Err(err))
 			return false
 		}
 		return canSee
@@ -374,7 +390,7 @@ func (webCon *WebConn) IsMemberOfTeam(teamId string) bool {
 	if currentSession == nil || len(currentSession.Token) == 0 {
 		session, err := webCon.App.GetSession(webCon.GetSessionToken())
 		if err != nil {
-			mlog.Error(fmt.Sprintf("Invalid session err=%v", err.Error()))
+			mlog.Error("Invalid session.", mlog.Err(err))
 			return false
 		}
 		webCon.SetSession(session)

app/webhook.go

@@ -4,7 +4,6 @@
 package app
 
 import (
-	"fmt"
 	"io"
 	"net/http"
 	"regexp"
@@ -108,7 +107,7 @@ func (a *App) TriggerWebhook(payload *model.OutgoingWebhookPayload, hook *model.
 		a.Srv.Go(func() {
 			webhookResp, err := a.doOutgoingWebhookRequest(url, body, contentType)
 			if err != nil {
-				mlog.Error(fmt.Sprintf("Event POST failed, err=%s", err.Error()))
+				mlog.Error("Event POST failed.", mlog.Err(err))
 				return
 			}
 
@@ -139,7 +138,7 @@ func (a *App) TriggerWebhook(payload *model.OutgoingWebhookPayload, hook *model.
 					webhookResp.IconURL = hook.IconURL
 				}
 				if _, err := a.CreateWebhookPost(hook.CreatorId, channel, text, webhookResp.Username, webhookResp.IconURL, "", webhookResp.Props, webhookResp.Type, postRootId); err != nil {
-					mlog.Error(fmt.Sprintf("Failed to create response post, err=%v", err))
+					mlog.Error("Failed to create response post.", mlog.Err(err))
 				}
 			}
 		})

app/websocket_router.go

@@ -4,7 +4,6 @@
 package app
 
 import (
-	"fmt"
 	"net/http"
 
 	"github.com/mattermost/mattermost-server/mlog"
@@ -89,7 +88,13 @@ func (wr *WebSocketRouter) ServeWebSocket(conn *WebConn, r *model.WebSocketReque
 }
 
 func ReturnWebSocketError(conn *WebConn, r *model.WebSocketRequest, err *model.AppError) {
-	mlog.Error(fmt.Sprintf("websocket routing error: seq=%v uid=%v %v [details: %v]", r.Seq, conn.UserId, err.SystemMessage(utils.T), err.DetailedError))
+	mlog.Error(
+		"websocket routing error.",
+		mlog.Int64("seq", r.Seq),
+		mlog.String("user_id", conn.UserId),
+		mlog.String("system_message", err.SystemMessage(utils.T)),
+		mlog.Err(err),
+	)
 
 	err.DetailedError = ""
 	errorResp := model.NewWebSocketError(r.Seq, err)

cmd/mattermost/commands/config_test.go

@@ -136,16 +136,16 @@ func TestConfigSet(t *testing.T) {
 	})
 
 	t.Run("Error when the wrong value is set", func(t *testing.T) {
-		assert.Error(t, th.RunCommand(t, "config", "set", "EmailSettings.ConnectionSecurity", "invalid"))
+		assert.Error(t, th.RunCommand(t, "config", "set", "EmailSettings.ConnectionSecurity", "invalid-key"))
 		output := th.CheckCommand(t, "config", "get", "EmailSettings.ConnectionSecurity")
-		assert.NotContains(t, string(output), "invalid")
+		assert.NotContains(t, string(output), "invalid-key")
 	})
 
 	t.Run("Error when the wrong locale is set", func(t *testing.T) {
 		th.CheckCommand(t, "config", "set", "LocalizationSettings.DefaultServerLocale", "es")
-		assert.Error(t, th.RunCommand(t, "config", "set", "LocalizationSettings.DefaultServerLocale", "invalid"))
+		assert.Error(t, th.RunCommand(t, "config", "set", "LocalizationSettings.DefaultServerLocale", "invalid-key"))
 		output := th.CheckCommand(t, "config", "get", "LocalizationSettings.DefaultServerLocale")
-		assert.NotContains(t, string(output), "invalid")
+		assert.NotContains(t, string(output), "invalid-key")
 		assert.NotContains(t, string(output), "\"en\"")
 	})
 

cmd/mattermost/commands/utils.go

@@ -22,7 +22,7 @@ func prettyPrintStruct(t interface{}) string {
 func structToMap(t interface{}) map[string]interface{} {
 	defer func() {
 		if r := recover(); r != nil {
-			mlog.Error(fmt.Sprintf("Panicked in structToMap. This should never happen. %v", r))
+			mlog.Error("Panicked in structToMap. This should never happen.", mlog.Any("recover", r))
 		}
 	}()
 

cmd/mattermost/main.go

@@ -16,9 +16,9 @@ import (
 
 	// Enterprise Deps
 	_ "github.com/dgryski/dgoogauth"
-	_ "github.com/go-ldap/ldap"
 	_ "github.com/hako/durafmt"
 	_ "github.com/hashicorp/memberlist"
+	_ "github.com/mattermost/ldap"
 	_ "github.com/mattermost/rsc/qr"
 	_ "github.com/prometheus/client_golang/prometheus"
 	_ "github.com/prometheus/client_golang/prometheus/promhttp"

config/client.go

@@ -43,6 +43,7 @@ func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.L
 	props["ExperimentalEnableDefaultChannelLeaveJoinMessages"] = strconv.FormatBool(*c.ServiceSettings.ExperimentalEnableDefaultChannelLeaveJoinMessages)
 	props["ExperimentalGroupUnreadChannels"] = *c.ServiceSettings.ExperimentalGroupUnreadChannels
 	props["EnableSVGs"] = strconv.FormatBool(*c.ServiceSettings.EnableSVGs)
+	props["EnableMarketplace"] = strconv.FormatBool(*c.PluginSettings.EnableMarketplace)
 
 	// This setting is only temporary, so keep using the old setting name for the mobile and web apps
 	props["ExperimentalEnablePostMetadata"] = "true"

go.mod

@@ -16,7 +16,7 @@ require (
 	github.com/fortytw2/leaktest v1.3.0 // indirect
 	github.com/fsnotify/fsnotify v1.4.7
 	github.com/go-gorp/gorp v2.0.0+incompatible // indirect
-	github.com/go-ldap/ldap v3.0.3+incompatible
+	github.com/go-ldap/ldap v3.0.3+incompatible // indirect
 	github.com/go-redis/redis v6.15.2+incompatible
 	github.com/go-sql-driver/mysql v1.4.1
 	github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0
@@ -45,6 +45,7 @@ require (
 	github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e // indirect
 	github.com/mattermost/go-i18n v1.11.0
 	github.com/mattermost/gorp v2.0.1-0.20190301154413-3b31e9a39d05+incompatible
+	github.com/mattermost/ldap v3.0.4+incompatible
 	github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0
 	github.com/mattermost/viper v1.0.4
 	github.com/mattn/go-runewidth v0.0.4 // indirect
@@ -87,6 +88,7 @@ require (
 	golang.org/x/net v0.0.0-20190628185345-da137c7871d7
 	golang.org/x/sys v0.0.0-20190712062909-fae7ac547cb7 // indirect
 	golang.org/x/text v0.3.2
+	golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b
 	google.golang.org/appengine v1.6.1 // indirect
 	google.golang.org/genproto v0.0.0-20190716160619-c506a9f90610 // indirect
 	google.golang.org/grpc v1.22.0 // indirect

go.sum

@@ -24,6 +24,7 @@ github.com/a8m/mark v0.1.1-0.20170507133748-44f2db618845/go.mod h1:c8Mh99Cw82nrs
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c=
+github.com/apache/thrift v0.0.0-20180902110319-2566ecd5d999/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
 github.com/apache/thrift v0.12.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
 github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
 github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da h1:8GUt8eRujhVEGZFFEjBj46YV4rDjvGrNxb0KMWYkL2I=
@@ -239,6 +240,8 @@ github.com/mattermost/go-i18n v1.11.0 h1:1hLKqn/ZvhZ80OekjVPGYcCrBfMz+YxNNgqS+be
 github.com/mattermost/go-i18n v1.11.0/go.mod h1:RyS7FDNQlzF1PsjbJWHRI35exqaKGSO9qD4iv8QjE34=
 github.com/mattermost/gorp v2.0.1-0.20190301154413-3b31e9a39d05+incompatible h1:FN4zK2wNig7MVVsOsGEZ+LeIq0gUcudn3LEGgbodMq8=
 github.com/mattermost/gorp v2.0.1-0.20190301154413-3b31e9a39d05+incompatible/go.mod h1:0kX1qa3DOpaPJyOdMLeo7TcBN0QmUszj9a/VygOhDe0=
+github.com/mattermost/ldap v3.0.4+incompatible h1:SOeNnz+JNR+foQ3yHkYqijb9MLPhXN2BZP/PdX23VDU=
+github.com/mattermost/ldap v3.0.4+incompatible/go.mod h1:b4reDCcGpBxJ4WX0f224KFY+OR0npin7or7EFpeIko4=
 github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0 h1:G9tL6JXRBMzjuD1kkBtcnd42kUiT6QDwxfFYu7adM6o=
 github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0/go.mod h1:nV5bfVpT//+B1RPD2JvRnxbkLmJEYXmRaaVl15fsXjs=
 github.com/mattermost/viper v1.0.4 h1:cMYOz4PhguscGSPxrSokUtib5HrG4gCpiUh27wyA3d0=
@@ -506,6 +509,7 @@ golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3
 golang.org/x/tools v0.0.0-20190312170243-e65039ee4138/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 golang.org/x/tools v0.0.0-20190328211700-ab21143f2384/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
+golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b h1:mSUCVIwDx4hfXJfWsOPfdzEHxzb2Xjl6BQ8YgPnazQA=
 golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
 google.golang.org/api v0.0.0-20180910000450-7ca32eb868bf/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0=
 google.golang.org/api v0.0.0-20181030000543-1d582fd0359e/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0=

i18n/de.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "Konnte die zu löschenden Nachrichten für den Benutzer nicht auswählen (zu viele), bitte wiederholen."
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Konnte die maximal unterstützte Nachrichtengröße nicht bestimmen."
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "Konnte die Nachricht nicht speichern."

i18n/en.json

@@ -387,6 +387,10 @@
     "id": "api.channel.update_channel_member_roles.scheme_role.app_error",
     "translation": "The provided role is managed by a Scheme and therefore cannot be applied directly to a Channel Member"
   },
+  {
+    "id": "api.channel.update_channel_privacy.default_channel_error",
+    "translation": "The default channel cannot be made private."
+  },
   {
     "id": "api.channel.update_channel_scheme.license.error",
     "translation": "Your license does not support updating a channel's scheme"
@@ -3376,7 +3380,7 @@
   },
   {
     "id": "app.notification.body.intro.direct.generic",
-    "translation": "You have a new Direct Message from @{{.SenderName}}"
+    "translation": "You have a new Direct Message from {{.SenderName}}"
   },
   {
     "id": "app.notification.body.intro.group_message.full",
@@ -3384,7 +3388,7 @@
   },
   {
     "id": "app.notification.body.intro.group_message.generic",
-    "translation": "You have a new Group Message from @{{.SenderName}}"
+    "translation": "You have a new Group Message from {{.SenderName}}"
   },
   {
     "id": "app.notification.body.intro.notification.full",
@@ -3392,11 +3396,11 @@
   },
   {
     "id": "app.notification.body.intro.notification.generic",
-    "translation": "You have a new notification from @{{.SenderName}}"
+    "translation": "You have a new notification from {{.SenderName}}"
   },
   {
     "id": "app.notification.body.text.direct.full",
-    "translation": "@{{.SenderName}} - {{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}"
+    "translation": "{{.SenderName}} - {{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}"
   },
   {
     "id": "app.notification.body.text.direct.generic",
@@ -3408,7 +3412,7 @@
   },
   {
     "id": "app.notification.body.text.group_message.full2",
-    "translation": "@{{.SenderName}} - {{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}"
+    "translation": "{{.SenderName}} - {{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}"
   },
   {
     "id": "app.notification.body.text.group_message.generic",
@@ -3420,7 +3424,7 @@
   },
   {
     "id": "app.notification.body.text.notification.full2",
-    "translation": "@{{.SenderName}} - {{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}"
+    "translation": "{{.SenderName}} - {{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}"
   },
   {
     "id": "app.notification.body.text.notification.generic",
@@ -3502,6 +3506,22 @@
     "id": "app.plugin.manifest.app_error",
     "translation": "Unable to find manifest for extracted plugin"
   },
+  {
+    "id": "app.plugin.marketplace_client.app_error",
+    "translation": "Failed to create marketplace client."
+  },
+  {
+    "id": "app.plugin.marketplace_disabled.app_error",
+    "translation": "Marketplace has been disabled. Please check your logs for details."
+  },
+  {
+    "id": "app.plugin.marketplace_plugins.app_error",
+    "translation": "Failed to get plugins from the marketplace server."
+  },
+  {
+    "id": "app.plugin.marshal.app_error",
+    "translation": "Failed to marshal marketplace plugins."
+  },
   {
     "id": "app.plugin.mvdir.app_error",
     "translation": "Unable to move plugin from temporary directory to final destination. Another plugin may be using the same directory name."
@@ -6362,10 +6382,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "Unable to select the posts to delete for the user (too many), please re-run"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Unable to determine the maximum supported post size"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "Unable to save the Post"

i18n/es.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "No se puede seleccionar los mensajes a eliminar del usuario (son demasiados), por favor ejecuta de nuevo"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "No se puede determinar el tamaño máximo soportado de los mensajes"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "No se puede guardar el mensaje"

i18n/fr.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "Impossible de sélectionner les messages à supprimer pour l'utilisateur (ils sont trop nombreux), veuillez relancer l'opération"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Impossible de déterminer la taille maximale supportée pour les messages"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "Impossible de sauvegarder le message"

i18n/it.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "Non è stato possibile selezionare le pubblicazioni da eliminare per l'utente (troppe pubblicazioni), per favore rilancia"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Impossibile determinare la dimensione massima supportata per le pubblicazioni"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "Impossibile salvare la pubblicazione"

i18n/ja.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "ユーザーの削除すべき投稿を選択できませんでした(数が多過ぎます)。再度実行してください"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "投稿サイズの最大値を定義できませんでした"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "投稿を保存できませんでした"

i18n/ko.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "We couldn't select the posts to delete for the user (too many), please re-run"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Unable to determine the maximum supported post size"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "내용을 가져올수 없습니다."

i18n/nl.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "We kunnen de geselecteerde berichten voor de gebruiker niet verwijderen (het zijn er te veel), probeer opnieuw"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Unable to determine the maximum supported post size"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "Bericht kan niet opgehaald worden"

i18n/pl.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "Nie można wybrać postów do usunięcia dla użytkownika (zbyt wielu), uruchom ponownie"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Nie można określić maksymalnego obsługiwanego rozmiaru postu"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "Nie można zapisać wpisu"

i18n/pt-BR.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "Não foi possível selecionar as publicações para excluir do usuário (muitos), por favor, re-executar"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Não foi possível determinar o tamanho máximo de publicação suportado"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "Não foi possível salvar a Publicação"

i18n/ro.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "Am putut selectaţi mesaje pentru a şterge pentru utilizator (prea multe), vă rugăm să re-rula"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Noi nu a putut determina dimensiunea maximă suportate de post"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "Imposibil de salvat postarea"

i18n/ru.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "Не удалось выбрать для удаления посты пользователя (слишком много), пожалуйста, запустите повторно"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Unable to determine the maximum supported post size"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "Не удалось удалить плагин"

i18n/tr.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "Kullanıcının silinecek iletileri seçilemedi (çok fazla), lütfen yeniden çalıştırın"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Desteklenen en büyük ileti boyutu belirlenemedi"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "İleti kaydedilemedi"

i18n/uk.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "Не вдається вибрати повідомлення, які потрібно видалити для користувача (забагато), будь ласка, повторно запустіть"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "Ми не змогли визначити максимально підтримуваний розмір повідомлення"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "Не вдається зберегти публікацію"

i18n/zh-CN.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "无法删除该用户被选择的信息(数量太多)，请重新运行"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "无法判断最大支持的消息大小"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "无法保存消息"

i18n/zh-TW.json

@@ -6022,10 +6022,6 @@
     "id": "store.sql_post.permanent_delete_by_user.too_many.app_error",
     "translation": "無法選擇該使用者的訊息以刪除 (數量太多)，請重新執行"
   },
-  {
-    "id": "store.sql_post.query_max_post_size.error",
-    "translation": "無法判定最大支援的訊息大小"
-  },
   {
     "id": "store.sql_post.save.app_error",
     "translation": "無法儲存訊息"

jobs/jobs.go

@@ -5,7 +5,6 @@ package jobs
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	"net/http"
@@ -135,10 +134,10 @@ func (srv *JobServer) CancellationWatcher(ctx context.Context, jobId string, can
 	for {
 		select {
 		case <-ctx.Done():
-			mlog.Debug(fmt.Sprintf("CancellationWatcher for Job: %v Aborting as job has finished.", jobId))
+			mlog.Debug("CancellationWatcher for Job Aborting as job has finished.", mlog.String("job_id", jobId))
 			return
 		case <-time.After(CANCEL_WATCHER_POLLING_INTERVAL * time.Millisecond):
-			mlog.Debug(fmt.Sprintf("CancellationWatcher for Job: %v polling.", jobId))
+			mlog.Debug("CancellationWatcher for Job started polling.", mlog.String("job_id", jobId))
 			if jobStatus, err := srv.Store.Job().Get(jobId); err == nil {
 				if jobStatus.Status == model.JOB_STATUS_CANCEL_REQUESTED {
 					close(cancelChan)

jobs/jobs_watcher.go

@@ -4,7 +4,6 @@
 package jobs
 
 import (
-	"fmt"
 	"math/rand"
 	"time"
 
@@ -68,7 +67,7 @@ func (watcher *Watcher) Stop() {
 func (watcher *Watcher) PollAndNotify() {
 	jobs, err := watcher.srv.Store.Job().GetAllByStatus(model.JOB_STATUS_PENDING)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Error occurred getting all pending statuses: %v", err.Error()))
+		mlog.Error("Error occurred getting all pending statuses.", mlog.Err(err))
 		return
 	}
 

mlog/log.go

@@ -28,6 +28,7 @@ const (
 type Field = zapcore.Field
 
 var Int64 = zap.Int64
+var Int32 = zap.Int32
 var Int = zap.Int
 var Uint32 = zap.Uint32
 var String = zap.String

model/client4.go

@@ -2100,6 +2100,17 @@ func (c *Client4) ConvertChannelToPrivate(channelId string) (*Channel, *Response
 	return ChannelFromJson(r.Body), BuildResponse(r)
 }
 
+// UpdateChannelPrivacy updates channel privacy
+func (c *Client4) UpdateChannelPrivacy(channelId string, privacy string) (*Channel, *Response) {
+	requestBody := map[string]string{"privacy": privacy}
+	r, err := c.DoApiPut(c.GetChannelRoute(channelId)+"/privacy", MapToJson(requestBody))
+	if err != nil {
+		return nil, BuildErrorResponse(r, err)
+	}
+	defer closeBody(r)
+	return ChannelFromJson(r.Body), BuildResponse(r)
+}
+
 // RestoreChannel restores a previously deleted channel. Any missing fields are not updated.
 func (c *Client4) RestoreChannel(channelId string) (*Channel, *Response) {
 	r, err := c.DoApiPost(c.GetChannelRoute(channelId)+"/restore", "")
@@ -4518,6 +4529,31 @@ func (c *Client4) DisablePlugin(id string) (bool, *Response) {
 	return CheckStatusOK(r), BuildResponse(r)
 }
 
+// GetMarketplacePlugins will return a list of plugins that an admin can install.
+// WARNING: PLUGINS ARE STILL EXPERIMENTAL. THIS FUNCTION IS SUBJECT TO CHANGE.
+func (c *Client4) GetMarketplacePlugins(filter *MarketplacePluginFilter) ([]*MarketplacePlugin, *Response) {
+	route := c.GetPluginsRoute() + "/marketplace"
+	u, parseErr := url.Parse(route)
+	if parseErr != nil {
+		return nil, &Response{Error: NewAppError("GetMarketplacePlugins", "model.client.parse_plugins.app_error", nil, parseErr.Error(), http.StatusBadRequest)}
+	}
+
+	filter.ApplyToURL(u)
+
+	r, err := c.DoApiGet(u.String(), "")
+	if err != nil {
+		return nil, BuildErrorResponse(r, err)
+	}
+	defer closeBody(r)
+
+	plugins, readerErr := MarketplacePluginsFromReader(r.Body)
+	if readerErr != nil {
+		return nil, BuildErrorResponse(r, NewAppError(route, "model.client.parse_plugins.app_error", nil, err.Error(), http.StatusBadRequest))
+	}
+
+	return plugins, BuildResponse(r)
+}
+
 // UpdateChannelScheme will update a channel's scheme.
 func (c *Client4) UpdateChannelScheme(channelId, schemeId string) (bool, *Response) {
 	sip := &SchemeIDPatch{SchemeID: &schemeId}

model/config.go

@@ -17,7 +17,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/go-ldap/ldap"
+	"github.com/mattermost/ldap"
 )
 
 const (
@@ -171,8 +171,10 @@ const (
 	DATA_RETENTION_SETTINGS_DEFAULT_FILE_RETENTION_DAYS     = 365
 	DATA_RETENTION_SETTINGS_DEFAULT_DELETION_JOB_START_TIME = "02:00"
 
-	PLUGIN_SETTINGS_DEFAULT_DIRECTORY        = "./plugins"
+	PLUGIN_SETTINGS_DEFAULT_DIRECTORY          = "./plugins"
-	PLUGIN_SETTINGS_DEFAULT_CLIENT_DIRECTORY = "./client/plugins"
+	PLUGIN_SETTINGS_DEFAULT_CLIENT_DIRECTORY   = "./client/plugins"
+	PLUGIN_SETTINGS_DEFAULT_ENABLE_MARKETPLACE = true
+	PLUGIN_SETTINGS_DEFAULT_MARKETPLACE_URL    = "https://marketplace.integrations.mattermost.com"
 
 	COMPLIANCE_EXPORT_TYPE_CSV             = "csv"
 	COMPLIANCE_EXPORT_TYPE_ACTIANCE        = "actiance"
@@ -2201,6 +2203,8 @@ type PluginSettings struct {
 	ClientDirectory          *string `restricted:"true"`
 	Plugins                  map[string]map[string]interface{}
 	PluginStates             map[string]*PluginState
+	EnableMarketplace        *bool
+	MarketplaceUrl           *string
 }
 
 func (s *PluginSettings) SetDefaults(ls LogSettings) {
@@ -2220,22 +2224,14 @@ func (s *PluginSettings) SetDefaults(ls LogSettings) {
 		s.EnableHealthCheck = NewBool(true)
 	}
 
-	if s.Directory == nil {
+	if s.Directory == nil || *s.Directory == "" {
 		s.Directory = NewString(PLUGIN_SETTINGS_DEFAULT_DIRECTORY)
 	}
 
-	if *s.Directory == "" {
+	if s.ClientDirectory == nil || *s.ClientDirectory == "" {
-		*s.Directory = PLUGIN_SETTINGS_DEFAULT_DIRECTORY
-	}
-
-	if s.ClientDirectory == nil {
 		s.ClientDirectory = NewString(PLUGIN_SETTINGS_DEFAULT_CLIENT_DIRECTORY)
 	}
 
-	if *s.ClientDirectory == "" {
-		*s.ClientDirectory = PLUGIN_SETTINGS_DEFAULT_CLIENT_DIRECTORY
-	}
-
 	if s.Plugins == nil {
 		s.Plugins = make(map[string]map[string]interface{})
 	}
@@ -2248,6 +2244,14 @@ func (s *PluginSettings) SetDefaults(ls LogSettings) {
 		// Enable the NPS plugin by default if diagnostics are enabled
 		s.PluginStates["com.mattermost.nps"] = &PluginState{Enable: ls.EnableDiagnostics == nil || *ls.EnableDiagnostics}
 	}
+
+	if s.EnableMarketplace == nil {
+		s.EnableMarketplace = NewBool(PLUGIN_SETTINGS_DEFAULT_ENABLE_MARKETPLACE)
+	}
+
+	if s.MarketplaceUrl == nil || *s.MarketplaceUrl == "" {
+		s.MarketplaceUrl = NewString(PLUGIN_SETTINGS_DEFAULT_MARKETPLACE_URL)
+	}
 }
 
 type GlobalRelayMessageExportSettings struct {

model/integration_action.go

@@ -178,13 +178,14 @@ type PostActionAPIResponse struct {
 }
 
 type Dialog struct {
-	CallbackId     string          `json:"callback_id"`
+	CallbackId       string          `json:"callback_id"`
-	Title          string          `json:"title"`
+	Title            string          `json:"title"`
-	IconURL        string          `json:"icon_url"`
+	IntroductionText string          `json:"introduction_text"`
-	Elements       []DialogElement `json:"elements"`
+	IconURL          string          `json:"icon_url"`
-	SubmitLabel    string          `json:"submit_label"`
+	Elements         []DialogElement `json:"elements"`
-	NotifyOnCancel bool            `json:"notify_on_cancel"`
+	SubmitLabel      string          `json:"submit_label"`
-	State          string          `json:"state"`
+	NotifyOnCancel   bool            `json:"notify_on_cancel"`
+	State            string          `json:"state"`
 }
 
 type DialogElement struct {
@@ -221,6 +222,7 @@ type SubmitDialogRequest struct {
 }
 
 type SubmitDialogResponse struct {
+	Error  string            `json:"error,omitempty"`
 	Errors map[string]string `json:"errors,omitempty"`
 }
 

model/integration_action_test.go

@@ -141,6 +141,7 @@ func TestSubmitDialogRequestToJson(t *testing.T) {
 func TestSubmitDialogResponseToJson(t *testing.T) {
 	t.Run("all fine", func(t *testing.T) {
 		request := SubmitDialogResponse{
+			Error: "some generic error",
 			Errors: map[string]string{
 				"text":  "some text",
 				"float": "1.2",

model/marketplace_plugin.go

@@ -0,0 +1,69 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package model
+
+import (
+	"encoding/json"
+	"io"
+	"net/url"
+	"strconv"
+)
+
+// BaseMarketplacePlugin is a Mattermost plugin received from the marketplace server.
+type BaseMarketplacePlugin struct {
+	HomepageURL string    `json:"homepage_url"`
+	DownloadURL string    `json:"download_url"`
+	IconData    string    `json:"icon_data"`
+	Manifest    *Manifest `json:"manifest"`
+}
+
+// MarketplacePlugin is a state aware marketplace plugin.
+type MarketplacePlugin struct {
+	*BaseMarketplacePlugin
+	InstalledVersion string `json:"installed_version"`
+}
+
+// BaseMarketplacePluginsFromReader decodes a json-encoded list of plugins from the given io.Reader.
+func BaseMarketplacePluginsFromReader(reader io.Reader) ([]*BaseMarketplacePlugin, error) {
+	plugins := []*BaseMarketplacePlugin{}
+	decoder := json.NewDecoder(reader)
+
+	if err := decoder.Decode(&plugins); err != nil && err != io.EOF {
+		return nil, err
+	}
+
+	return plugins, nil
+}
+
+// MarketplacePluginsFromReader decodes a json-encoded list of plugins from the given io.Reader.
+func MarketplacePluginsFromReader(reader io.Reader) ([]*MarketplacePlugin, error) {
+	plugins := []*MarketplacePlugin{}
+	decoder := json.NewDecoder(reader)
+
+	if err := decoder.Decode(&plugins); err != nil && err != io.EOF {
+		return nil, err
+	}
+
+	return plugins, nil
+}
+
+// MarketplacePluginFilter describes the parameters to request a list of plugins.
+type MarketplacePluginFilter struct {
+	Page          int
+	PerPage       int
+	Filter        string
+	ServerVersion string
+}
+
+// ApplyToURL modifies the given url to include query string parameters for the request.
+func (filter *MarketplacePluginFilter) ApplyToURL(u *url.URL) {
+	q := u.Query()
+	q.Add("page", strconv.Itoa(filter.Page))
+	if filter.PerPage > 0 {
+		q.Add("per_page", strconv.Itoa(filter.PerPage))
+	}
+	q.Add("filter", filter.Filter)
+	q.Add("server_version", filter.ServerVersion)
+	u.RawQuery = q.Encode()
+}

model/message_export.go

@@ -20,6 +20,7 @@ type MessageExport struct {
 
 	PostId         *string
 	PostCreateAt   *int64
+	PostUpdateAt   *int64
 	PostMessage    *string
 	PostType       *string
 	PostRootId     *string

model/post.go

@@ -73,7 +73,6 @@ type Post struct {
 	OriginalId string `json:"original_id"`
 
 	Message string `json:"message"`
-
 	// MessageSource will contain the message as submitted by the user if Message has been modified
 	// by Mattermost for presentation (e.g if an image proxy is being used). It should be used to
 	// populate edit boxes if present.
@@ -88,7 +87,8 @@ type Post struct {
 	HasReactions  bool            `json:"has_reactions,omitempty"`
 
 	// Transient data populated before sending a post to the client
-	Metadata *PostMetadata `json:"metadata,omitempty" db:"-"`
+	ReplyCount int64         `json:"reply_count" db:"-"`
+	Metadata   *PostMetadata `json:"metadata,omitempty" db:"-"`
 }
 
 type PostEphemeral struct {
@@ -170,6 +170,20 @@ func (o *Post) ToUnsanitizedJson() string {
 	return string(b)
 }
 
+type GetPostsSinceOptions struct {
+	ChannelId        string
+	Time             int64
+	SkipFetchThreads bool
+}
+
+type GetPostsOptions struct {
+	ChannelId        string
+	PostId           string
+	Page             int
+	PerPage          int
+	SkipFetchThreads bool
+}
+
 func PostFromJson(data io.Reader) *Post {
 	var o *Post
 	json.NewDecoder(data).Decode(&o)

model/user_search.go

@@ -33,14 +33,14 @@ func (u *UserSearch) ToJson() []byte {
 
 // UserSearchFromJson will decode the input and return a User
 func UserSearchFromJson(data io.Reader) *UserSearch {
-	var us *UserSearch
+	us := UserSearch{}
 	json.NewDecoder(data).Decode(&us)
 
 	if us.Limit == 0 {
 		us.Limit = USER_SEARCH_DEFAULT_LIMIT
 	}
 
-	return us
+	return &us
 }
 
 // UserSearchOptions captures internal parameters derived from the user's permissions and a

model/version.go

@@ -13,6 +13,7 @@ import (
 // It should be maintained in chronological order with most current
 // release at the front of the list.
 var versions = []string{
+	"5.16.0",
 	"5.15.0",
 	"5.14.0",
 	"5.13.0",

plugin/api.go

@@ -16,19 +16,29 @@ import (
 type API interface {
 	// LoadPluginConfiguration loads the plugin's configuration. dest should be a pointer to a
 	// struct that the configuration JSON can be unmarshalled to.
+	//
+	// Minimum server version: 5.2
 	LoadPluginConfiguration(dest interface{}) error
 
 	// RegisterCommand registers a custom slash command. When the command is triggered, your plugin
 	// can fulfill it via the ExecuteCommand hook.
+	//
+	// Minimum server version: 5.2
 	RegisterCommand(command *model.Command) error
 
 	// UnregisterCommand unregisters a command previously registered via RegisterCommand.
+	//
+	// Minimum server version: 5.2
 	UnregisterCommand(teamId, trigger string) error
 
 	// GetSession returns the session object for the Session ID
+	//
+	// Minimum server version: 5.2
 	GetSession(sessionId string) (*model.Session, *model.AppError)
 
 	// GetConfig fetches the currently persisted config
+	//
+	// Minimum server version: 5.2
 	GetConfig() *model.Config
 
 	// GetUnsanitizedConfig fetches the currently persisted config without removing secrets.
@@ -37,6 +47,8 @@ type API interface {
 	GetUnsanitizedConfig() *model.Config
 
 	// SaveConfig sets the given config and persists the changes
+	//
+	// Minimum server version: 5.2
 	SaveConfig(config *model.Config) *model.AppError
 
 	// GetPluginConfig fetches the currently persisted config of plugin
@@ -76,9 +88,13 @@ type API interface {
 	GetDiagnosticId() string
 
 	// CreateUser creates a user.
+	//
+	// Minimum server version: 5.2
 	CreateUser(user *model.User) (*model.User, *model.AppError)
 
 	// DeleteUser deletes a user.
+	//
+	// Minimum server version: 5.2
 	DeleteUser(userId string) *model.AppError
 
 	// GetUsers a list of users based on search options.
@@ -87,12 +103,18 @@ type API interface {
 	GetUsers(options *model.UserGetOptions) ([]*model.User, *model.AppError)
 
 	// GetUser gets a user.
+	//
+	// Minimum server version: 5.2
 	GetUser(userId string) (*model.User, *model.AppError)
 
 	// GetUserByEmail gets a user by their email address.
+	//
+	// Minimum server version: 5.2
 	GetUserByEmail(email string) (*model.User, *model.AppError)
 
 	// GetUserByUsername gets a user by their username.
+	//
+	// Minimum server version: 5.2
 	GetUserByUsername(name string) (*model.User, *model.AppError)
 
 	// GetUsersByUsernames gets users by their usernames.
@@ -121,16 +143,24 @@ type API interface {
 	RemoveTeamIcon(teamId string) *model.AppError
 
 	// UpdateUser updates a user.
+	//
+	// Minimum server version: 5.2
 	UpdateUser(user *model.User) (*model.User, *model.AppError)
 
 	// GetUserStatus will get a user's status.
+	//
+	// Minimum server version: 5.2
 	GetUserStatus(userId string) (*model.Status, *model.AppError)
 
 	// GetUserStatusesByIds will return a list of user statuses based on the provided slice of user IDs.
+	//
+	// Minimum server version: 5.2
 	GetUserStatusesByIds(userIds []string) ([]*model.Status, *model.AppError)
 
 	// UpdateUserStatus will set a user's status until the user, or another integration/plugin, sets it back to online.
 	// The status parameter can be: "online", "away", "dnd", or "offline".
+	//
+	// Minimum server version: 5.2
 	UpdateUserStatus(userId, status string) (*model.Status, *model.AppError)
 
 	// UpdateUserActive deactivates or reactivates an user.
@@ -153,18 +183,28 @@ type API interface {
 	GetLDAPUserAttributes(userId string, attributes []string) (map[string]string, *model.AppError)
 
 	// CreateTeam creates a team.
+	//
+	// Minimum server version: 5.2
 	CreateTeam(team *model.Team) (*model.Team, *model.AppError)
 
 	// DeleteTeam deletes a team.
+	//
+	// Minimum server version: 5.2
 	DeleteTeam(teamId string) *model.AppError
 
 	// GetTeam gets all teams.
+	//
+	// Minimum server version: 5.2
 	GetTeams() ([]*model.Team, *model.AppError)
 
 	// GetTeam gets a team.
+	//
+	// Minimum server version: 5.2
 	GetTeam(teamId string) (*model.Team, *model.AppError)
 
 	// GetTeamByName gets a team by its name.
+	//
+	// Minimum server version: 5.2
 	GetTeamByName(name string) (*model.Team, *model.AppError)
 
 	// GetTeamsUnreadForUser gets the unread message and mention counts for each team to which the given user belongs.
@@ -173,6 +213,8 @@ type API interface {
 	GetTeamsUnreadForUser(userId string) ([]*model.TeamUnread, *model.AppError)
 
 	// UpdateTeam updates a team.
+	//
+	// Minimum server version: 5.2
 	UpdateTeam(team *model.Team) (*model.Team, *model.AppError)
 
 	// SearchTeams search a team.
@@ -186,18 +228,28 @@ type API interface {
 	GetTeamsForUser(userId string) ([]*model.Team, *model.AppError)
 
 	// CreateTeamMember creates a team membership.
+	//
+	// Minimum server version: 5.2
 	CreateTeamMember(teamId, userId string) (*model.TeamMember, *model.AppError)
 
 	// CreateTeamMember creates a team membership for all provided user ids.
+	//
+	// Minimum server version: 5.2
 	CreateTeamMembers(teamId string, userIds []string, requestorId string) ([]*model.TeamMember, *model.AppError)
 
 	// DeleteTeamMember deletes a team membership.
+	//
+	// Minimum server version: 5.2
 	DeleteTeamMember(teamId, userId, requestorId string) *model.AppError
 
 	// GetTeamMembers returns the memberships of a specific team.
+	//
+	// Minimum server version: 5.2
 	GetTeamMembers(teamId string, page, perPage int) ([]*model.TeamMember, *model.AppError)
 
 	// GetTeamMember returns a specific membership.
+	//
+	// Minimum server version: 5.2
 	GetTeamMember(teamId, userId string) (*model.TeamMember, *model.AppError)
 
 	// GetTeamMembersForUser returns all team memberships for a user.
@@ -206,24 +258,38 @@ type API interface {
 	GetTeamMembersForUser(userId string, page int, perPage int) ([]*model.TeamMember, *model.AppError)
 
 	// UpdateTeamMemberRoles updates the role for a team membership.
+	//
+	// Minimum server version: 5.2
 	UpdateTeamMemberRoles(teamId, userId, newRoles string) (*model.TeamMember, *model.AppError)
 
 	// CreateChannel creates a channel.
+	//
+	// Minimum server version: 5.2
 	CreateChannel(channel *model.Channel) (*model.Channel, *model.AppError)
 
 	// DeleteChannel deletes a channel.
+	//
+	// Minimum server version: 5.2
 	DeleteChannel(channelId string) *model.AppError
 
 	// GetPublicChannelsForTeam gets a list of all channels.
+	//
+	// Minimum server version: 5.2
 	GetPublicChannelsForTeam(teamId string, page, perPage int) ([]*model.Channel, *model.AppError)
 
 	// GetChannel gets a channel.
+	//
+	// Minimum server version: 5.2
 	GetChannel(channelId string) (*model.Channel, *model.AppError)
 
 	// GetChannelByName gets a channel by its name, given a team id.
+	//
+	// Minimum server version: 5.2
 	GetChannelByName(teamId, name string, includeDeleted bool) (*model.Channel, *model.AppError)
 
 	// GetChannelByNameForTeamName gets a channel by its name, given a team name.
+	//
+	// Minimum server version: 5.2
 	GetChannelByNameForTeamName(teamName, channelName string, includeDeleted bool) (*model.Channel, *model.AppError)
 
 	// GetChannelsForTeamForUser gets a list of channels for given user ID in given team ID.
@@ -238,13 +304,19 @@ type API interface {
 
 	// GetDirectChannel gets a direct message channel.
 	// If the channel does not exist it will create it.
+	//
+	// Minimum server version: 5.2
 	GetDirectChannel(userId1, userId2 string) (*model.Channel, *model.AppError)
 
 	// GetGroupChannel gets a group message channel.
 	// If the channel does not exist it will create it.
+	//
+	// Minimum server version: 5.2
 	GetGroupChannel(userIds []string) (*model.Channel, *model.AppError)
 
 	// UpdateChannel updates a channel.
+	//
+	// Minimum server version: 5.2
 	UpdateChannel(channel *model.Channel) (*model.Channel, *model.AppError)
 
 	// SearchChannels returns the channels on a team matching the provided search term.
@@ -263,9 +335,13 @@ type API interface {
 	SearchPostsInTeam(teamId string, paramsList []*model.SearchParams) ([]*model.Post, *model.AppError)
 
 	// AddChannelMember creates a channel membership for a user.
+	//
+	// Minimum server version: 5.2
 	AddChannelMember(channelId, userId string) (*model.ChannelMember, *model.AppError)
 
 	// GetChannelMember gets a channel membership for a user.
+	//
+	// Minimum server version: 5.2
 	GetChannelMember(channelId, userId string) (*model.ChannelMember, *model.AppError)
 
 	// GetChannelMembers gets a channel membership for all users.
@@ -284,15 +360,23 @@ type API interface {
 	GetChannelMembersForUser(teamId, userId string, page, perPage int) ([]*model.ChannelMember, *model.AppError)
 
 	// UpdateChannelMemberRoles updates a user's roles for a channel.
+	//
+	// Minimum server version: 5.2
 	UpdateChannelMemberRoles(channelId, userId, newRoles string) (*model.ChannelMember, *model.AppError)
 
 	// UpdateChannelMemberNotifications updates a user's notification properties for a channel.
+	//
+	// Minimum server version: 5.2
 	UpdateChannelMemberNotifications(channelId, userId string, notifications map[string]string) (*model.ChannelMember, *model.AppError)
 
 	// DeleteChannelMember deletes a channel membership for a user.
+	//
+	// Minimum server version: 5.2
 	DeleteChannelMember(channelId, userId string) *model.AppError
 
 	// CreatePost creates a post.
+	//
+	// Minimum server version: 5.2
 	CreatePost(post *model.Post) (*model.Post, *model.AppError)
 
 	// AddReaction add a reaction to a post.
@@ -311,17 +395,25 @@ type API interface {
 	GetReactions(postId string) ([]*model.Reaction, *model.AppError)
 
 	// SendEphemeralPost creates an ephemeral post.
+	//
+	// Minimum server version: 5.2
 	SendEphemeralPost(userId string, post *model.Post) *model.Post
 
 	// UpdateEphemeralPost updates an ephemeral message previously sent to the user.
 	// EXPERIMENTAL: This API is experimental and can be changed without advance notice.
+	//
+	// Minimum server version: 5.2
 	UpdateEphemeralPost(userId string, post *model.Post) *model.Post
 
 	// DeleteEphemeralPost deletes an ephemeral message previously sent to the user.
 	// EXPERIMENTAL: This API is experimental and can be changed without advance notice.
+	//
+	// Minimum server version: 5.2
 	DeleteEphemeralPost(userId, postId string)
 
 	// DeletePost deletes a post.
+	//
+	// Minimum server version: 5.2
 	DeletePost(postId string) *model.AppError
 
 	// GetPostThread gets a post with all the other posts in the same thread.
@@ -330,6 +422,8 @@ type API interface {
 	GetPostThread(postId string) (*model.PostList, *model.AppError)
 
 	// GetPost gets a post.
+	//
+	// Minimum server version: 5.2
 	GetPost(postId string) (*model.Post, *model.AppError)
 
 	// GetPostsSince gets posts created after a specified time as Unix time in milliseconds.
@@ -358,6 +452,8 @@ type API interface {
 	GetTeamStats(teamId string) (*model.TeamStats, *model.AppError)
 
 	// UpdatePost updates a post.
+	//
+	// Minimum server version: 5.2
 	UpdatePost(post *model.Post) (*model.Post, *model.AppError)
 
 	// GetProfileImage gets user's profile image.
@@ -393,6 +489,8 @@ type API interface {
 	// The duplicate FileInfo objects are not initially linked to a post, but may now be passed
 	// to CreatePost. Use this API to duplicate a post and its file attachments without
 	// actually duplicating the uploaded files.
+	//
+	// Minimum server version: 5.2
 	CopyFileInfos(userId string, fileIds []string) ([]string, *model.AppError)
 
 	// GetFileInfo gets a File Info for a specific fileId
@@ -402,7 +500,7 @@ type API interface {
 
 	// GetFile gets content of a file by it's ID
 	//
-	// Minimum Server version: 5.8
+	// Minimum server version: 5.8
 	GetFile(fileId string) ([]byte, *model.AppError)
 
 	// GetFileLink gets the public link to a file by fileId.
@@ -463,6 +561,8 @@ type API interface {
 
 	// KVSet stores a key-value pair, unique per plugin.
 	// Provided helper functions and internal plugin code will use the prefix `mmi_` before keys. Do not use this prefix.
+	//
+	// Minimum server version: 5.2
 	KVSet(key string, value []byte) *model.AppError
 
 	// KVCompareAndSet updates a key-value pair, unique per plugin, but only if the current value matches the given oldValue.
@@ -488,9 +588,13 @@ type API interface {
 	KVSetWithExpiry(key string, value []byte, expireInSeconds int64) *model.AppError
 
 	// KVGet retrieves a value based on the key, unique per plugin. Returns nil for non-existent keys.
+	//
+	// Minimum server version: 5.2
 	KVGet(key string) ([]byte, *model.AppError)
 
 	// KVDelete removes a key-value pair, unique per plugin. Returns nil for non-existent keys.
+	//
+	// Minimum server version: 5.2
 	KVDelete(key string) *model.AppError
 
 	// KVDeleteAll removes all key-value pairs for a plugin.
@@ -507,6 +611,8 @@ type API interface {
 	// event is the type and will be prepended with "custom_<pluginid>_".
 	// payload is the data sent with the event. Interface values must be primitive Go types or mattermost-server/model types.
 	// broadcast determines to which users to send the event.
+	//
+	// Minimum server version: 5.2
 	PublishWebSocketEvent(event string, payload map[string]interface{}, broadcast *model.WebsocketBroadcast)
 
 	// HasPermissionTo check if the user has the permission at system scope.
@@ -528,24 +634,32 @@ type API interface {
 	// Appropriate context such as the plugin name will already be added as fields so plugins
 	// do not need to add that info.
 	// keyValuePairs should be primitive go types or other values that can be encoded by encoding/gob
+	//
+	// Minimum server version: 5.2
 	LogDebug(msg string, keyValuePairs ...interface{})
 
 	// LogInfo writes a log message to the Mattermost server log file.
 	// Appropriate context such as the plugin name will already be added as fields so plugins
 	// do not need to add that info.
 	// keyValuePairs should be primitive go types or other values that can be encoded by encoding/gob
+	//
+	// Minimum server version: 5.2
 	LogInfo(msg string, keyValuePairs ...interface{})
 
 	// LogError writes a log message to the Mattermost server log file.
 	// Appropriate context such as the plugin name will already be added as fields so plugins
 	// do not need to add that info.
 	// keyValuePairs should be primitive go types or other values that can be encoded by encoding/gob
+	//
+	// Minimum server version: 5.2
 	LogError(msg string, keyValuePairs ...interface{})
 
 	// LogWarn writes a log message to the Mattermost server log file.
 	// Appropriate context such as the plugin name will already be added as fields so plugins
 	// do not need to add that info.
 	// keyValuePairs should be primitive go types or other values that can be encoded by encoding/gob
+	//
+	// Minimum server version: 5.2
 	LogWarn(msg string, keyValuePairs ...interface{})
 
 	// SendMail sends an email to a specific address

plugin/checker/main.go

@@ -0,0 +1,126 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package main
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+
+	"go/ast"
+
+	"golang.org/x/tools/go/packages"
+
+	"github.com/pkg/errors"
+)
+
+const pluginPackagePath = "github.com/mattermost/mattermost-server/plugin"
+
+func main() {
+	if err := runCheck(pluginPackagePath); err != nil {
+		fmt.Fprintln(os.Stderr, "#", pluginPackagePath)
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+}
+
+func runCheck(pkgPath string) error {
+	pkg, err := getPackage(pkgPath)
+	if err != nil {
+		return err
+	}
+
+	apiInterface := findAPIInterface(pkg.Syntax)
+	if apiInterface == nil {
+		return errors.Errorf("could not find API interface in package %s", pkgPath)
+	}
+
+	invalidMethods := findInvalidMethods(apiInterface.Methods.List)
+	if len(invalidMethods) > 0 {
+		return errors.New(renderErrorMessage(pkg, invalidMethods))
+	}
+	return nil
+}
+
+func getPackage(pkgPath string) (*packages.Package, error) {
+	cfg := &packages.Config{
+		Mode: packages.NeedName | packages.NeedTypes | packages.NeedSyntax,
+	}
+	pkgs, err := packages.Load(cfg, pkgPath)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(pkgs) == 0 {
+		return nil, errors.Errorf("could not find package %s", pkgPath)
+	}
+	return pkgs[0], nil
+}
+
+func findAPIInterface(files []*ast.File) *ast.InterfaceType {
+	for _, f := range files {
+		var iface *ast.InterfaceType
+
+		ast.Inspect(f, func(n ast.Node) bool {
+			if t, ok := n.(*ast.TypeSpec); ok {
+				if i, ok := t.Type.(*ast.InterfaceType); ok && t.Name.Name == "API" {
+					iface = i
+					return false
+				}
+			}
+			return true
+		})
+
+		if iface != nil {
+			return iface
+		}
+	}
+	return nil
+}
+
+func findInvalidMethods(methods []*ast.Field) []*ast.Field {
+	var invalid []*ast.Field
+	for _, m := range methods {
+		if !hasValidMinimumVersionComment(m.Doc.Text()) {
+			invalid = append(invalid, m)
+		}
+	}
+	return invalid
+}
+
+var versionRequirementRE = regexp.MustCompile(`^Minimum server version: \d+\.\d+(\.\d+)?$`)
+
+func hasValidMinimumVersionComment(s string) bool {
+	lines := strings.Split(strings.TrimSpace(s), "\n")
+	if len(lines) > 0 {
+		lastLine := lines[len(lines)-1]
+		return versionRequirementRE.MatchString(lastLine)
+	}
+	return false
+}
+
+func renderErrorMessage(pkg *packages.Package, methods []*ast.Field) string {
+	cwd, _ := os.Getwd()
+	out := &bytes.Buffer{}
+
+	for _, m := range methods {
+		pos := pkg.Fset.Position(m.Pos())
+		filename, err := filepath.Rel(cwd, pos.Filename)
+		if err != nil {
+			// If deriving a relative path fails for some reason,
+			// we prefer to still print the absolute path to the file.
+			filename = pos.Filename
+		}
+		fmt.Fprintf(out,
+			"%s:%d:%d: missing a minimum server version comment\n",
+			filename,
+			pos.Line,
+			pos.Column,
+		)
+	}
+	return out.String()
+}

plugin/checker/main_test.go

@@ -0,0 +1,49 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRunCheck(t *testing.T) {
+	testCases := []struct {
+		name, pkgPath, err string
+	}{
+		{
+			name:    "valid comments",
+			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/test/valid",
+			err:     "",
+		},
+		{
+			name:    "invalid comments",
+			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/test/invalid",
+			err:     "test/invalid/invalid.go:15:2: missing a minimum server version comment\n",
+		},
+		{
+			name:    "missing API interface",
+			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/test/missing",
+			err:     "could not find API interface in package github.com/mattermost/mattermost-server/plugin/checker/test/missing",
+		},
+		{
+			name:    "non-existent package path",
+			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/test/does_not_exist",
+			err:     "could not find API interface in package github.com/mattermost/mattermost-server/plugin/checker/test/does_not_exist",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := runCheck(tc.pkgPath)
+
+			if tc.err != "" {
+				assert.EqualError(t, err, tc.err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}

plugin/checker/test/invalid/invalid.go

@@ -0,0 +1,16 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package invalid
+
+type API interface {
+	// ValidMethod is a fake method for testing the
+	// plugin comment checker with a valid comment.
+	//
+	// Minimum server version: 1.2.3
+	ValidMethod()
+
+	// InvalidMethod is a fake method for testing the
+	// plugin comment checker with an invalid comment.
+	InvalidMethod()
+}

plugin/checker/test/missing/missing.go

@@ -0,0 +1,8 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package missing
+
+// SomeType is a fake interface for testing the plugin comment checker.
+type SomeType interface {
+}

plugin/checker/test/valid/valid.go

@@ -0,0 +1,12 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package valid
+
+type API interface {
+	// ValidMethod is a fake method for testing the
+	// plugin comment checker with a valid comment.
+	//
+	// Minimum server version: 1.2.3
+	ValidMethod()
+}

plugin/environment.go

@@ -18,6 +18,8 @@ import (
 	"github.com/pkg/errors"
 )
 
+var ErrNotFound = errors.New("Item not found")
+
 type apiImplCreatorFunc func(*model.Manifest) API
 
 // registeredPlugin stores the state for a given plugin that has been activated
@@ -162,6 +164,23 @@ func (env *Environment) Statuses() (model.PluginStatuses, error) {
 	return pluginStatuses, nil
 }
 
+// GetManifest returns a manifest for a given pluginId.
+// Returns ErrNotFound if plugin is not found.
+func (env *Environment) GetManifest(pluginId string) (*model.Manifest, error) {
+	plugins, err := env.Available()
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to get plugin statuses")
+	}
+
+	for _, plugin := range plugins {
+		if plugin.Manifest != nil && plugin.Manifest.Id == pluginId {
+			return plugin.Manifest, nil
+		}
+	}
+
+	return nil, ErrNotFound
+}
+
 func (env *Environment) Activate(id string) (manifest *model.Manifest, activated bool, reterr error) {
 	// Check if we are already active
 	if env.IsActive(id) {

plugin/supervisor.go

@@ -118,7 +118,7 @@ func (sup *supervisor) PerformHealthCheck() error {
 			}
 		}
 		if pingErr != nil {
-			mlog.Debug(fmt.Sprintf("Error pinging plugin, error: %s", pingErr.Error()))
+			mlog.Debug("Error pinging plugin", mlog.Err(pingErr))
 			return fmt.Errorf("Plugin RPC connection is not responding")
 		}
 	}

services/marketplace/client.go

@@ -0,0 +1,79 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package marketplace
+
+import (
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/url"
+
+	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/services/httpservice"
+	"github.com/pkg/errors"
+)
+
+// Client is the programmatic interface to the marketplace server API.
+type Client struct {
+	address    string
+	httpClient *http.Client
+}
+
+// NewClient creates a client to the marketplace server at the given address.
+func NewClient(address string, httpService httpservice.HTTPService) (*Client, error) {
+	var httpClient *http.Client
+	addressUrl, err := url.Parse(address)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to parse marketplace address")
+	}
+	if addressUrl.Hostname() == "localhost" || addressUrl.Hostname() == "127.0.0.1" {
+		httpClient = httpService.MakeClient(true)
+	} else {
+		httpClient = httpService.MakeClient(false)
+	}
+
+	return &Client{
+		address:    address,
+		httpClient: httpClient,
+	}, nil
+}
+
+// GetPlugins fetches the list of plugins from the configured server.
+func (c *Client) GetPlugins(request *model.MarketplacePluginFilter) ([]*model.BaseMarketplacePlugin, error) {
+	u, err := url.Parse(c.buildURL("/api/v1/plugins"))
+	if err != nil {
+		return nil, err
+	}
+
+	request.ApplyToURL(u)
+
+	resp, err := c.doGet(u.String())
+	if err != nil {
+		return nil, err
+	}
+	defer closeBody(resp)
+
+	switch resp.StatusCode {
+	case http.StatusOK:
+		return model.BaseMarketplacePluginsFromReader(resp.Body)
+	default:
+		return nil, errors.Errorf("failed with status code %d", resp.StatusCode)
+	}
+}
+
+// closeBody ensures the Body of an http.Response is properly closed.
+func closeBody(r *http.Response) {
+	if r.Body != nil {
+		_, _ = ioutil.ReadAll(r.Body)
+		_ = r.Body.Close()
+	}
+}
+
+func (c *Client) buildURL(urlPath string, args ...interface{}) string {
+	return fmt.Sprintf("%s%s", c.address, fmt.Sprintf(urlPath, args...))
+}
+
+func (c *Client) doGet(u string) (*http.Response, error) {
+	return c.httpClient.Get(u)
+}

store/layered_store.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/mattermost/mattermost-server/einterfaces"
 	"github.com/mattermost/mattermost-server/mlog"
-	"github.com/mattermost/mattermost-server/model"
 )
 
 const (
@@ -22,8 +21,6 @@ type LayeredStoreDatabaseLayer interface {
 
 type LayeredStore struct {
 	TmpContext      context.Context
-	RoleStore       RoleStore
-	SchemeStore     SchemeStore
 	DatabaseLayer   LayeredStoreDatabaseLayer
 	LocalCacheLayer *LocalCacheSupplier
 	RedisLayer      *RedisSupplier
@@ -37,9 +34,6 @@ func NewLayeredStore(db LayeredStoreDatabaseLayer, metrics einterfaces.MetricsIn
 		LocalCacheLayer: NewLocalCacheSupplier(metrics, cluster),
 	}
 
-	store.RoleStore = &LayeredRoleStore{store}
-	store.SchemeStore = &LayeredSchemeStore{store}
-
 	// Setup the chain
 	if ENABLE_EXPERIMENTAL_REDIS {
 		mlog.Debug("Experimental redis enabled.")
@@ -161,7 +155,7 @@ func (s *LayeredStore) Plugin() PluginStore {
 }
 
 func (s *LayeredStore) Role() RoleStore {
-	return s.RoleStore
+	return s.DatabaseLayer.Role()
 }
 
 func (s *LayeredStore) TermsOfService() TermsOfServiceStore {
@@ -173,7 +167,7 @@ func (s *LayeredStore) UserTermsOfService() UserTermsOfServiceStore {
 }
 
 func (s *LayeredStore) Scheme() SchemeStore {
-	return s.SchemeStore
+	return s.DatabaseLayer.Scheme()
 }
 
 func (s *LayeredStore) Group() GroupStore {
@@ -220,63 +214,3 @@ func (s *LayeredStore) TotalSearchDbConnections() int {
 func (s *LayeredStore) CheckIntegrity() <-chan IntegrityCheckResult {
 	return s.DatabaseLayer.CheckIntegrity()
 }
-
-type LayeredRoleStore struct {
-	*LayeredStore
-}
-
-func (s *LayeredRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) {
-	return s.LayerChainHead.RoleSave(s.TmpContext, role)
-}
-
-func (s *LayeredRoleStore) Get(roleId string) (*model.Role, *model.AppError) {
-	return s.LayerChainHead.RoleGet(s.TmpContext, roleId)
-}
-
-func (s *LayeredRoleStore) GetAll() ([]*model.Role, *model.AppError) {
-	return s.LayerChainHead.RoleGetAll(s.TmpContext)
-}
-
-func (s *LayeredRoleStore) GetByName(name string) (*model.Role, *model.AppError) {
-	return s.LayerChainHead.RoleGetByName(s.TmpContext, name)
-}
-
-func (s *LayeredRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) {
-	return s.LayerChainHead.RoleGetByNames(s.TmpContext, names)
-}
-
-func (s *LayeredRoleStore) Delete(roldId string) (*model.Role, *model.AppError) {
-	return s.LayerChainHead.RoleDelete(s.TmpContext, roldId)
-}
-
-func (s *LayeredRoleStore) PermanentDeleteAll() *model.AppError {
-	return s.LayerChainHead.RolePermanentDeleteAll(s.TmpContext)
-}
-
-type LayeredSchemeStore struct {
-	*LayeredStore
-}
-
-func (s *LayeredSchemeStore) Save(scheme *model.Scheme) (*model.Scheme, *model.AppError) {
-	return s.LayerChainHead.SchemeSave(s.TmpContext, scheme)
-}
-
-func (s *LayeredSchemeStore) Get(schemeId string) (*model.Scheme, *model.AppError) {
-	return s.LayerChainHead.SchemeGet(s.TmpContext, schemeId)
-}
-
-func (s *LayeredSchemeStore) GetByName(schemeName string) (*model.Scheme, *model.AppError) {
-	return s.LayerChainHead.SchemeGetByName(s.TmpContext, schemeName)
-}
-
-func (s *LayeredSchemeStore) Delete(schemeId string) (*model.Scheme, *model.AppError) {
-	return s.LayerChainHead.SchemeDelete(s.TmpContext, schemeId)
-}
-
-func (s *LayeredSchemeStore) GetAllPage(scope string, offset int, limit int) ([]*model.Scheme, *model.AppError) {
-	return s.LayerChainHead.SchemeGetAllPage(s.TmpContext, scope, offset, limit)
-}
-
-func (s *LayeredSchemeStore) PermanentDeleteAll() *model.AppError {
-	return s.LayerChainHead.SchemePermanentDeleteAll(s.TmpContext)
-}

store/layered_store_supplier.go

@@ -3,9 +3,6 @@
 
 package store
 
-import "github.com/mattermost/mattermost-server/model"
-import "context"
-
 type LayeredStoreSupplierResult struct {
 	StoreResult
 }
@@ -20,21 +17,4 @@ type LayeredStoreSupplier interface {
 	//
 	SetChainNext(LayeredStoreSupplier)
 	Next() LayeredStoreSupplier
-
-	// Roles
-	RoleSave(ctx context.Context, role *model.Role, hints ...LayeredStoreHint) (*model.Role, *model.AppError)
-	RoleGet(ctx context.Context, roleId string, hints ...LayeredStoreHint) (*model.Role, *model.AppError)
-	RoleGetAll(ctx context.Context, hints ...LayeredStoreHint) ([]*model.Role, *model.AppError)
-	RoleGetByName(ctx context.Context, name string, hints ...LayeredStoreHint) (*model.Role, *model.AppError)
-	RoleGetByNames(ctx context.Context, names []string, hints ...LayeredStoreHint) ([]*model.Role, *model.AppError)
-	RoleDelete(ctx context.Context, roldId string, hints ...LayeredStoreHint) (*model.Role, *model.AppError)
-	RolePermanentDeleteAll(ctx context.Context, hints ...LayeredStoreHint) *model.AppError
-
-	// Schemes
-	SchemeSave(ctx context.Context, scheme *model.Scheme, hints ...LayeredStoreHint) (*model.Scheme, *model.AppError)
-	SchemeGet(ctx context.Context, schemeId string, hints ...LayeredStoreHint) (*model.Scheme, *model.AppError)
-	SchemeGetByName(ctx context.Context, schemeName string, hints ...LayeredStoreHint) (*model.Scheme, *model.AppError)
-	SchemeDelete(ctx context.Context, schemeId string, hints ...LayeredStoreHint) (*model.Scheme, *model.AppError)
-	SchemeGetAllPage(ctx context.Context, scope string, offset int, limit int, hints ...LayeredStoreHint) ([]*model.Scheme, *model.AppError)
-	SchemePermanentDeleteAll(ctx context.Context, hints ...LayeredStoreHint) *model.AppError
 }

store/local_cache_supplier.go

@@ -8,28 +8,16 @@ import (
 
 	"github.com/mattermost/mattermost-server/einterfaces"
 	"github.com/mattermost/mattermost-server/model"
-	"github.com/mattermost/mattermost-server/utils"
 )
 
 const (
-	ROLE_CACHE_SIZE = 20000
-	ROLE_CACHE_SEC  = 30 * 60
-
-	SCHEME_CACHE_SIZE = 20000
-	SCHEME_CACHE_SEC  = 30 * 60
-
-	GROUP_CACHE_SIZE = 20000
-	GROUP_CACHE_SEC  = 30 * 60
-
 	CLEAR_CACHE_MESSAGE_DATA = ""
 )
 
 type LocalCacheSupplier struct {
-	next        LayeredStoreSupplier
+	next    LayeredStoreSupplier
-	roleCache   *utils.Cache
+	metrics einterfaces.MetricsInterface
-	schemeCache *utils.Cache
+	cluster einterfaces.ClusterInterface
-	metrics     einterfaces.MetricsInterface
-	cluster     einterfaces.ClusterInterface
 }
 
 // Caching Interface
@@ -46,14 +34,8 @@ type ObjectCache interface {
 
 func NewLocalCacheSupplier(metrics einterfaces.MetricsInterface, cluster einterfaces.ClusterInterface) *LocalCacheSupplier {
 	supplier := &LocalCacheSupplier{
-		roleCache:   utils.NewLruWithParams(ROLE_CACHE_SIZE, "Role", ROLE_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES),
+		metrics: metrics,
-		schemeCache: utils.NewLruWithParams(SCHEME_CACHE_SIZE, "Scheme", SCHEME_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES),
+		cluster: cluster,
-		metrics:     metrics,
-		cluster:     cluster,
-	}
-
-	if cluster != nil {
-		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES, supplier.handleClusterInvalidateRole)
 	}
 
 	return supplier
@@ -122,6 +104,4 @@ func (s *LocalCacheSupplier) doClearCacheCluster(cache ObjectCache) {
 }
 
 func (s *LocalCacheSupplier) Invalidate() {
-	s.doClearCacheCluster(s.roleCache)
-	s.doClearCacheCluster(s.schemeCache)
 }

store/local_cache_supplier_roles.go

@@ -1,96 +0,0 @@
-// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
-// See License.txt for license information.
-
-package store
-
-import (
-	"context"
-
-	"github.com/mattermost/mattermost-server/model"
-)
-
-func (s *LocalCacheSupplier) handleClusterInvalidateRole(msg *model.ClusterMessage) {
-	if msg.Data == CLEAR_CACHE_MESSAGE_DATA {
-		s.roleCache.Purge()
-	} else {
-		s.roleCache.Remove(msg.Data)
-	}
-}
-
-func (s *LocalCacheSupplier) RoleSave(ctx context.Context, role *model.Role, hints ...LayeredStoreHint) (*model.Role, *model.AppError) {
-	if len(role.Name) != 0 {
-		defer s.doInvalidateCacheCluster(s.roleCache, role.Name)
-	}
-	return s.Next().RoleSave(ctx, role, hints...)
-}
-
-func (s *LocalCacheSupplier) RoleGet(ctx context.Context, roleId string, hints ...LayeredStoreHint) (*model.Role, *model.AppError) {
-	// Roles are cached by name, as that is most commonly how they are looked up.
-	// This means that no caching is supported on roles being looked up by ID.
-	return s.Next().RoleGet(ctx, roleId, hints...)
-}
-
-func (s *LocalCacheSupplier) RoleGetAll(ctx context.Context, hints ...LayeredStoreHint) ([]*model.Role, *model.AppError) {
-	// Roles are cached by name, as that is most commonly how they are looked up.
-	// This means that no caching is supported on roles being listed.
-	return s.Next().RoleGetAll(ctx, hints...)
-}
-
-func (s *LocalCacheSupplier) RoleGetByName(ctx context.Context, name string, hints ...LayeredStoreHint) (*model.Role, *model.AppError) {
-	if result := s.doStandardReadCache(ctx, s.roleCache, name, hints...); result != nil {
-		return result.Data.(*model.Role), nil
-	}
-
-	role, err := s.Next().RoleGetByName(ctx, name, hints...)
-	if err != nil {
-		return nil, err
-	}
-
-	result := NewSupplierResult()
-	result.Data = role
-	s.doStandardAddToCache(ctx, s.roleCache, name, result, hints...)
-
-	return role, nil
-}
-
-func (s *LocalCacheSupplier) RoleGetByNames(ctx context.Context, roleNames []string, hints ...LayeredStoreHint) ([]*model.Role, *model.AppError) {
-	var foundRoles []*model.Role
-	var rolesToQuery []string
-
-	for _, roleName := range roleNames {
-		if result := s.doStandardReadCache(ctx, s.roleCache, roleName, hints...); result != nil {
-			foundRoles = append(foundRoles, result.Data.(*model.Role))
-		} else {
-			rolesToQuery = append(rolesToQuery, roleName)
-		}
-	}
-
-	rolesFound, err := s.Next().RoleGetByNames(ctx, rolesToQuery, hints...)
-
-	for _, role := range rolesFound {
-		res := NewSupplierResult()
-		res.Data = role
-		s.doStandardAddToCache(ctx, s.roleCache, role.Name, res, hints...)
-	}
-	foundRoles = append(foundRoles, rolesFound...)
-
-	return foundRoles, err
-}
-
-func (s *LocalCacheSupplier) RoleDelete(ctx context.Context, roleId string, hints ...LayeredStoreHint) (*model.Role, *model.AppError) {
-	role, err := s.Next().RoleDelete(ctx, roleId, hints...)
-	if err != nil {
-		return nil, err
-	}
-
-	s.doInvalidateCacheCluster(s.roleCache, role.Name)
-
-	return role, nil
-}
-
-func (s *LocalCacheSupplier) RolePermanentDeleteAll(ctx context.Context, hints ...LayeredStoreHint) *model.AppError {
-	defer s.roleCache.Purge()
-	defer s.doClearCacheCluster(s.roleCache)
-
-	return s.Next().RolePermanentDeleteAll(ctx, hints...)
-}

store/local_cache_supplier_schemes.go

@@ -1,64 +0,0 @@
-// Copyright (c) 2018-present Mattermost, Inc. All Rights Reserved.
-// See License.txt for license information.
-
-package store
-
-import (
-	"context"
-
-	"github.com/mattermost/mattermost-server/model"
-)
-
-func (s *LocalCacheSupplier) handleClusterInvalidateScheme(msg *model.ClusterMessage) {
-	if msg.Data == CLEAR_CACHE_MESSAGE_DATA {
-		s.schemeCache.Purge()
-	} else {
-		s.schemeCache.Remove(msg.Data)
-	}
-}
-
-func (s *LocalCacheSupplier) SchemeSave(ctx context.Context, scheme *model.Scheme, hints ...LayeredStoreHint) (*model.Scheme, *model.AppError) {
-	if len(scheme.Id) != 0 {
-		defer s.doInvalidateCacheCluster(s.schemeCache, scheme.Id)
-	}
-	return s.Next().SchemeSave(ctx, scheme, hints...)
-}
-
-func (s *LocalCacheSupplier) SchemeGet(ctx context.Context, schemeId string, hints ...LayeredStoreHint) (*model.Scheme, *model.AppError) {
-	if result := s.doStandardReadCache(ctx, s.schemeCache, schemeId, hints...); result != nil {
-		return result.Data.(*model.Scheme), nil
-	}
-
-	scheme, err := s.Next().SchemeGet(ctx, schemeId, hints...)
-	if err != nil {
-		return nil, err
-	}
-
-	result := NewSupplierResult()
-	result.Data = scheme
-	s.doStandardAddToCache(ctx, s.schemeCache, schemeId, result, hints...)
-
-	return scheme, nil
-}
-
-func (s *LocalCacheSupplier) SchemeGetByName(ctx context.Context, schemeName string, hints ...LayeredStoreHint) (*model.Scheme, *model.AppError) {
-	return s.Next().SchemeGetByName(ctx, schemeName, hints...)
-}
-
-func (s *LocalCacheSupplier) SchemeDelete(ctx context.Context, schemeId string, hints ...LayeredStoreHint) (*model.Scheme, *model.AppError) {
-	defer s.doInvalidateCacheCluster(s.schemeCache, schemeId)
-	defer s.doClearCacheCluster(s.roleCache)
-
-	return s.Next().SchemeDelete(ctx, schemeId, hints...)
-}
-
-func (s *LocalCacheSupplier) SchemeGetAllPage(ctx context.Context, scope string, offset int, limit int, hints ...LayeredStoreHint) ([]*model.Scheme, *model.AppError) {
-	return s.Next().SchemeGetAllPage(ctx, scope, offset, limit, hints...)
-}
-
-func (s *LocalCacheSupplier) SchemePermanentDeleteAll(ctx context.Context, hints ...LayeredStoreHint) *model.AppError {
-	defer s.doClearCacheCluster(s.schemeCache)
-	defer s.doClearCacheCluster(s.roleCache)
-
-	return s.Next().SchemePermanentDeleteAll(ctx, hints...)
-}

store/localcachelayer/layer.go

@@ -14,6 +14,12 @@ const (
 	REACTION_CACHE_SIZE = 20000
 	REACTION_CACHE_SEC  = 30 * 60
 
+	ROLE_CACHE_SIZE = 20000
+	ROLE_CACHE_SEC  = 30 * 60
+
+	SCHEME_CACHE_SIZE = 20000
+	SCHEME_CACHE_SEC  = 30 * 60
+
 	CLEAR_CACHE_MESSAGE_DATA = ""
 )
 
@@ -23,6 +29,10 @@ type LocalCacheStore struct {
 	cluster       einterfaces.ClusterInterface
 	reaction      LocalCacheReactionStore
 	reactionCache *utils.Cache
+	role          LocalCacheRoleStore
+	roleCache     *utils.Cache
+	scheme        LocalCacheSchemeStore
+	schemeCache   *utils.Cache
 }
 
 func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterface, cluster einterfaces.ClusterInterface) LocalCacheStore {
@@ -33,9 +43,15 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf
 	}
 	localCacheStore.reactionCache = utils.NewLruWithParams(REACTION_CACHE_SIZE, "Reaction", REACTION_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_REACTIONS)
 	localCacheStore.reaction = LocalCacheReactionStore{ReactionStore: baseStore.Reaction(), rootStore: &localCacheStore}
+	localCacheStore.roleCache = utils.NewLruWithParams(ROLE_CACHE_SIZE, "Role", ROLE_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES)
+	localCacheStore.role = LocalCacheRoleStore{RoleStore: baseStore.Role(), rootStore: &localCacheStore}
+	localCacheStore.schemeCache = utils.NewLruWithParams(SCHEME_CACHE_SIZE, "Scheme", SCHEME_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES)
+	localCacheStore.scheme = LocalCacheSchemeStore{SchemeStore: baseStore.Scheme(), rootStore: &localCacheStore}
 
 	if cluster != nil {
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_REACTIONS, localCacheStore.reaction.handleClusterInvalidateReaction)
+		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES, localCacheStore.role.handleClusterInvalidateRole)
+		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES, localCacheStore.scheme.handleClusterInvalidateScheme)
 	}
 	return localCacheStore
 }
@@ -44,6 +60,14 @@ func (s LocalCacheStore) Reaction() store.ReactionStore {
 	return s.reaction
 }
 
+func (s LocalCacheStore) Role() store.RoleStore {
+	return s.role
+}
+
+func (s LocalCacheStore) Scheme() store.SchemeStore {
+	return s.scheme
+}
+
 func (s LocalCacheStore) DropAllTables() {
 	s.Invalidate()
 	s.Store.DropAllTables()

store/localcachelayer/main_test.go

@@ -6,11 +6,44 @@ package localcachelayer
 import (
 	"testing"
 
+	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/store/storetest/mocks"
 	"github.com/mattermost/mattermost-server/testlib"
 )
 
 var mainHelper *testlib.MainHelper
 
+func getMockStore() *mocks.Store {
+	mockStore := mocks.Store{}
+
+	fakeReaction := model.Reaction{PostId: "123"}
+	mockReactionsStore := mocks.ReactionStore{}
+	mockReactionsStore.On("Save", &fakeReaction).Return(&model.Reaction{}, nil)
+	mockReactionsStore.On("Delete", &fakeReaction).Return(&model.Reaction{}, nil)
+	mockReactionsStore.On("GetForPost", "123", false).Return([]*model.Reaction{&fakeReaction}, nil)
+	mockReactionsStore.On("GetForPost", "123", true).Return([]*model.Reaction{&fakeReaction}, nil)
+	mockStore.On("Reaction").Return(&mockReactionsStore)
+
+	fakeRole := model.Role{Id: "123", Name: "role-name"}
+	mockRolesStore := mocks.RoleStore{}
+	mockRolesStore.On("Save", &fakeRole).Return(&model.Role{}, nil)
+	mockRolesStore.On("Delete", "123").Return(&fakeRole, nil)
+	mockRolesStore.On("GetByName", "role-name").Return(&fakeRole, nil)
+	mockRolesStore.On("GetByNames", []string{"role-name"}).Return([]*model.Role{&fakeRole}, nil)
+	mockRolesStore.On("PermanentDeleteAll").Return(nil)
+	mockStore.On("Role").Return(&mockRolesStore)
+
+	fakeScheme := model.Scheme{Id: "123", Name: "scheme-name"}
+	mockSchemesStore := mocks.SchemeStore{}
+	mockSchemesStore.On("Save", &fakeScheme).Return(&model.Scheme{}, nil)
+	mockSchemesStore.On("Delete", "123").Return(&model.Scheme{}, nil)
+	mockSchemesStore.On("Get", "123").Return(&fakeScheme, nil)
+	mockSchemesStore.On("PermanentDeleteAll").Return(nil)
+	mockStore.On("Scheme").Return(&mockSchemesStore)
+
+	return &mockStore
+}
+
 func TestMain(m *testing.M) {
 	mainHelper = testlib.NewMainHelperWithOptions(nil)
 	defer mainHelper.Close()

store/localcachelayer/reaction_layer_test.go

@@ -21,72 +21,48 @@ func TestReactionStoreCache(t *testing.T) {
 	fakeReaction := model.Reaction{PostId: "123"}
 
 	t.Run("first call not cached, second cached and returning same data", func(t *testing.T) {
-		mockStore := mocks.Store{}
+		mockStore := getMockStore()
-		mockReactionsStore := mocks.ReactionStore{}
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
-		mockReactionsStore.On("Save", &fakeReaction).Return(&model.Reaction{}, nil)
-		mockReactionsStore.On("Delete", &fakeReaction).Return(&model.Reaction{}, nil)
-		mockReactionsStore.On("GetForPost", "123", false).Return([]*model.Reaction{&fakeReaction}, nil)
-		mockReactionsStore.On("GetForPost", "123", true).Return([]*model.Reaction{&fakeReaction}, nil)
-		mockStore.On("Reaction").Return(&mockReactionsStore)
-		cachedStore := NewLocalCacheLayer(&mockStore, nil, nil)
 
 		reaction, err := cachedStore.Reaction().GetForPost("123", true)
 		require.Nil(t, err)
 		assert.Equal(t, reaction, []*model.Reaction{&fakeReaction})
-		mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 1)
+		mockStore.Reaction().(*mocks.ReactionStore).AssertNumberOfCalls(t, "GetForPost", 1)
 		require.Nil(t, err)
 		assert.Equal(t, reaction, []*model.Reaction{&fakeReaction})
 		cachedStore.Reaction().GetForPost("123", true)
-		mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 1)
+		mockStore.Reaction().(*mocks.ReactionStore).AssertNumberOfCalls(t, "GetForPost", 1)
 	})
 
 	t.Run("first call not cached, second force no cached", func(t *testing.T) {
-		mockStore := mocks.Store{}
+		mockStore := getMockStore()
-		mockReactionsStore := mocks.ReactionStore{}
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
-		mockReactionsStore.On("Save", &fakeReaction).Return(&model.Reaction{}, nil)
-		mockReactionsStore.On("Delete", &fakeReaction).Return(&model.Reaction{}, nil)
-		mockReactionsStore.On("GetForPost", "123", false).Return([]*model.Reaction{&fakeReaction}, nil)
-		mockReactionsStore.On("GetForPost", "123", true).Return([]*model.Reaction{&fakeReaction}, nil)
-		mockStore.On("Reaction").Return(&mockReactionsStore)
-		cachedStore := NewLocalCacheLayer(&mockStore, nil, nil)
 
 		cachedStore.Reaction().GetForPost("123", true)
-		mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 1)
+		mockStore.Reaction().(*mocks.ReactionStore).AssertNumberOfCalls(t, "GetForPost", 1)
 		cachedStore.Reaction().GetForPost("123", false)
-		mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 2)
+		mockStore.Reaction().(*mocks.ReactionStore).AssertNumberOfCalls(t, "GetForPost", 2)
 	})
 
 	t.Run("first call not cached, save, and then not cached again", func(t *testing.T) {
-		mockStore := mocks.Store{}
+		mockStore := getMockStore()
-		mockReactionsStore := mocks.ReactionStore{}
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
-		mockReactionsStore.On("Save", &fakeReaction).Return(&model.Reaction{}, nil)
-		mockReactionsStore.On("Delete", &fakeReaction).Return(&model.Reaction{}, nil)
-		mockReactionsStore.On("GetForPost", "123", false).Return([]*model.Reaction{&fakeReaction}, nil)
-		mockReactionsStore.On("GetForPost", "123", true).Return([]*model.Reaction{&fakeReaction}, nil)
-		mockStore.On("Reaction").Return(&mockReactionsStore)
-		cachedStore := NewLocalCacheLayer(&mockStore, nil, nil)
 
 		cachedStore.Reaction().GetForPost("123", true)
-		mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 1)
+		mockStore.Reaction().(*mocks.ReactionStore).AssertNumberOfCalls(t, "GetForPost", 1)
 		cachedStore.Reaction().Save(&fakeReaction)
 		cachedStore.Reaction().GetForPost("123", true)
-		mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 2)
+		mockStore.Reaction().(*mocks.ReactionStore).AssertNumberOfCalls(t, "GetForPost", 2)
 	})
 
 	t.Run("first call not cached, delete, and then not cached again", func(t *testing.T) {
-		mockStore := mocks.Store{}
+		mockStore := getMockStore()
-		mockReactionsStore := mocks.ReactionStore{}
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
-		mockReactionsStore.On("Save", &fakeReaction).Return(&model.Reaction{}, nil)
-		mockReactionsStore.On("Delete", &fakeReaction).Return(&model.Reaction{}, nil)
-		mockReactionsStore.On("GetForPost", "123", false).Return([]*model.Reaction{&fakeReaction}, nil)
-		mockReactionsStore.On("GetForPost", "123", true).Return([]*model.Reaction{&fakeReaction}, nil)
-		mockStore.On("Reaction").Return(&mockReactionsStore)
-		cachedStore := NewLocalCacheLayer(&mockStore, nil, nil)
 
 		cachedStore.Reaction().GetForPost("123", true)
-		mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 1)
+		mockStore.Reaction().(*mocks.ReactionStore).AssertNumberOfCalls(t, "GetForPost", 1)
 		cachedStore.Reaction().Delete(&fakeReaction)
 		cachedStore.Reaction().GetForPost("123", true)
-		mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 2)
+		mockStore.Reaction().(*mocks.ReactionStore).AssertNumberOfCalls(t, "GetForPost", 2)
 	})
 }

store/localcachelayer/role_layer.go

@@ -0,0 +1,80 @@
+// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package localcachelayer
+
+import (
+	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/store"
+)
+
+type LocalCacheRoleStore struct {
+	store.RoleStore
+	rootStore *LocalCacheStore
+}
+
+func (s *LocalCacheRoleStore) handleClusterInvalidateRole(msg *model.ClusterMessage) {
+	if msg.Data == CLEAR_CACHE_MESSAGE_DATA {
+		s.rootStore.roleCache.Purge()
+	} else {
+		s.rootStore.roleCache.Remove(msg.Data)
+	}
+}
+
+func (s LocalCacheRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) {
+	if len(role.Name) != 0 {
+		defer s.rootStore.doInvalidateCacheCluster(s.rootStore.roleCache, role.Name)
+	}
+	return s.RoleStore.Save(role)
+}
+
+func (s LocalCacheRoleStore) GetByName(name string) (*model.Role, *model.AppError) {
+	if role := s.rootStore.doStandardReadCache(s.rootStore.roleCache, name); role != nil {
+		return role.(*model.Role), nil
+	}
+
+	role, err := s.RoleStore.GetByName(name)
+	if err != nil {
+		return nil, err
+	}
+	s.rootStore.doStandardAddToCache(s.rootStore.roleCache, name, role)
+	return role, nil
+}
+
+func (s LocalCacheRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) {
+	var foundRoles []*model.Role
+	var rolesToQuery []string
+
+	for _, roleName := range names {
+		if role := s.rootStore.doStandardReadCache(s.rootStore.roleCache, roleName); role != nil {
+			foundRoles = append(foundRoles, role.(*model.Role))
+		} else {
+			rolesToQuery = append(rolesToQuery, roleName)
+		}
+	}
+
+	roles, _ := s.RoleStore.GetByNames(rolesToQuery)
+
+	if roles != nil {
+		for _, role := range roles {
+			s.rootStore.doStandardAddToCache(s.rootStore.roleCache, role.Name, role)
+		}
+	}
+	return append(foundRoles, roles...), nil
+}
+
+func (s LocalCacheRoleStore) Delete(roleId string) (*model.Role, *model.AppError) {
+	role, err := s.RoleStore.Delete(roleId)
+
+	if err == nil {
+		s.rootStore.doInvalidateCacheCluster(s.rootStore.roleCache, role.Name)
+	}
+	return role, err
+}
+
+func (s LocalCacheRoleStore) PermanentDeleteAll() *model.AppError {
+	defer s.rootStore.roleCache.Purge()
+	defer s.rootStore.doClearCacheCluster(s.rootStore.roleCache)
+
+	return s.RoleStore.PermanentDeleteAll()
+}

store/localcachelayer/role_layer_test.go

@@ -0,0 +1,69 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package localcachelayer
+
+import (
+	"testing"
+
+	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/store/storetest"
+	"github.com/mattermost/mattermost-server/store/storetest/mocks"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRoleStore(t *testing.T) {
+	StoreTest(t, storetest.TestRoleStore)
+}
+
+func TestRoleStoreCache(t *testing.T) {
+	fakeRole := model.Role{Id: "123", Name: "role-name"}
+
+	t.Run("first call not cached, second cached and returning same data", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		role, err := cachedStore.Role().GetByName("role-name")
+		require.Nil(t, err)
+		assert.Equal(t, role, &fakeRole)
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 1)
+		require.Nil(t, err)
+		assert.Equal(t, role, &fakeRole)
+		cachedStore.Role().GetByName("role-name")
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 1)
+	})
+
+	t.Run("first call not cached, save, and then not cached again", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		cachedStore.Role().GetByName("role-name")
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 1)
+		cachedStore.Role().Save(&fakeRole)
+		cachedStore.Role().GetByName("role-name")
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 2)
+	})
+
+	t.Run("first call not cached, delete, and then not cached again", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		cachedStore.Role().GetByName("role-name")
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 1)
+		cachedStore.Role().Delete("123")
+		cachedStore.Role().GetByName("role-name")
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 2)
+	})
+
+	t.Run("first call not cached, permanent delete all, and then not cached again", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		cachedStore.Role().GetByName("role-name")
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 1)
+		cachedStore.Role().PermanentDeleteAll()
+		cachedStore.Role().GetByName("role-name")
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 2)
+	})
+}

store/localcachelayer/scheme_layer.go

@@ -0,0 +1,58 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package localcachelayer
+
+import (
+	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/store"
+)
+
+type LocalCacheSchemeStore struct {
+	store.SchemeStore
+	rootStore *LocalCacheStore
+}
+
+func (s *LocalCacheSchemeStore) handleClusterInvalidateScheme(msg *model.ClusterMessage) {
+	if msg.Data == CLEAR_CACHE_MESSAGE_DATA {
+		s.rootStore.schemeCache.Purge()
+	} else {
+		s.rootStore.schemeCache.Remove(msg.Data)
+	}
+}
+
+func (s LocalCacheSchemeStore) Save(scheme *model.Scheme) (*model.Scheme, *model.AppError) {
+	if len(scheme.Id) != 0 {
+		defer s.rootStore.doInvalidateCacheCluster(s.rootStore.schemeCache, scheme.Id)
+	}
+	return s.SchemeStore.Save(scheme)
+}
+
+func (s LocalCacheSchemeStore) Get(schemeId string) (*model.Scheme, *model.AppError) {
+	if scheme := s.rootStore.doStandardReadCache(s.rootStore.schemeCache, schemeId); scheme != nil {
+		return scheme.(*model.Scheme), nil
+	}
+
+	scheme, err := s.SchemeStore.Get(schemeId)
+	if err != nil {
+		return nil, err
+	}
+
+	s.rootStore.doStandardAddToCache(s.rootStore.schemeCache, schemeId, scheme)
+
+	return scheme, nil
+}
+
+func (s LocalCacheSchemeStore) Delete(schemeId string) (*model.Scheme, *model.AppError) {
+	defer s.rootStore.doInvalidateCacheCluster(s.rootStore.schemeCache, schemeId)
+	defer s.rootStore.doClearCacheCluster(s.rootStore.roleCache)
+
+	return s.SchemeStore.Delete(schemeId)
+}
+
+func (s LocalCacheSchemeStore) PermanentDeleteAll() *model.AppError {
+	defer s.rootStore.doClearCacheCluster(s.rootStore.schemeCache)
+	defer s.rootStore.doClearCacheCluster(s.rootStore.roleCache)
+
+	return s.SchemeStore.PermanentDeleteAll()
+}