From 2acbc77d78456d7ba76ceb687b18985d7d92f814 Mon Sep 17 00:00:00 2001 From: Corey Hulen Date: Fri, 27 Apr 2018 10:38:40 -0700 Subject: MM-10375 Fixing connected socket count (#8682) * Fixing connected socket count * Adding unit test --- api4/system_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'api4') diff --git a/api4/system_test.go b/api4/system_test.go index f74d91563..b12421e62 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -534,6 +534,28 @@ func TestGetAnalyticsOld(t *testing.T) { _, resp = th.SystemAdminClient.GetAnalyticsOld("", th.BasicTeam.Id) CheckNoError(t, resp) + rows2, resp2 := th.SystemAdminClient.GetAnalyticsOld("standard", "") + CheckNoError(t, resp2) + assert.Equal(t, "total_websocket_connections", rows2[5].Name) + assert.Equal(t, float64(0), rows2[5].Value) + + WebSocketClient, err := th.CreateWebSocketClient() + if err != nil { + t.Fatal(err) + } + + rows2, resp2 = th.SystemAdminClient.GetAnalyticsOld("standard", "") + CheckNoError(t, resp2) + assert.Equal(t, "total_websocket_connections", rows2[5].Name) + assert.Equal(t, float64(1), rows2[5].Value) + + WebSocketClient.Close() + + rows2, resp2 = th.SystemAdminClient.GetAnalyticsOld("standard", "") + CheckNoError(t, resp2) + assert.Equal(t, "total_websocket_connections", rows2[5].Name) + assert.Equal(t, float64(0), rows2[5].Value) + Client.Logout() _, resp = Client.GetAnalyticsOld("", th.BasicTeam.Id) CheckUnauthorizedStatus(t, resp) -- cgit v1.2.3-1-g7c22 From 686c2fbab7607d42183ae685a27ea3d7dce8c3f6 Mon Sep 17 00:00:00 2001 From: Christopher Speller Date: Fri, 27 Apr 2018 12:49:45 -0700 Subject: Structured logging (#8673) * Implementing structured logging * Changes to en.json to allow refactor to run. * Fixing global logger * Structured logger initalization. * Add caller. * Do some log redirection. * Auto refactor * Cleaning up l4g reference and removing dependancy. * Removing junk. * Copyright headers. * Fixing tests * Revert "Changes to en.json to allow refactor to run." This reverts commit fd8249e99bcad0231e6ea65cd77c32aae9a54026. * Fixing some auto refactor strangeness and typo. * Making keys more human readable. --- api4/api.go | 5 +++-- api4/api_test.go | 15 ++++++++++++--- api4/apitestlib.go | 32 ++++++++++++++++---------------- api4/channel.go | 5 ++--- api4/context.go | 18 +++++++++--------- api4/oauth.go | 6 +++--- api4/plugin.go | 4 ++-- api4/system.go | 7 ++++--- api4/system_test.go | 4 ++-- api4/user.go | 4 ++-- api4/webhook.go | 7 +++---- api4/websocket.go | 6 +++--- 12 files changed, 61 insertions(+), 52 deletions(-) (limited to 'api4') diff --git a/api4/api.go b/api4/api.go index 88526e4d3..d36c3e3ee 100644 --- a/api4/api.go +++ b/api4/api.go @@ -4,11 +4,12 @@ package api4 import ( + "fmt" "net/http" - l4g "github.com/alecthomas/log4go" "github.com/gorilla/mux" "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" @@ -243,7 +244,7 @@ func Init(a *app.App, root *mux.Router, full bool) *API { func Handle404(w http.ResponseWriter, r *http.Request) { err := model.NewAppError("Handle404", "api.context.404.app_error", nil, "", http.StatusNotFound) - l4g.Debug("%v: code=404 ip=%v", r.URL.Path, utils.GetIpAddress(r)) + mlog.Debug(fmt.Sprintf("%v: code=404 ip=%v", r.URL.Path, utils.GetIpAddress(r))) w.WriteHeader(err.StatusCode) err.DetailedError = "There doesn't appear to be an api call for the url='" + r.URL.Path + "'." diff --git a/api4/api_test.go b/api4/api_test.go index fd804b70d..2efd21f22 100644 --- a/api4/api_test.go +++ b/api4/api_test.go @@ -8,20 +8,29 @@ import ( "os" "testing" - l4g "github.com/alecthomas/log4go" - + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/store/storetest" "github.com/mattermost/mattermost-server/utils" ) func TestMain(m *testing.M) { flag.Parse() + + // Setup a global logger to catch tests logging outside of app context + // The global logger will be stomped by apps initalizing but that's fine for testing. Ideally this won't happen. + mlog.InitGlobalLogger(mlog.NewLogger(&mlog.LoggerConfiguration{ + EnableConsole: true, + ConsoleJson: true, + ConsoleLevel: "error", + EnableFile: false, + })) + utils.TranslationsPreInit() // In the case where a dev just wants to run a single test, it's faster to just use the default // store. if filter := flag.Lookup("test.run").Value.String(); filter != "" && filter != "." { - l4g.Info("-test.run used, not creating temporary containers") + mlog.Info("-test.run used, not creating temporary containers") os.Exit(m.Run()) } diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 4620c5f4e..48765687a 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -19,8 +19,8 @@ import ( "testing" "time" - l4g "github.com/alecthomas/log4go" "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" "github.com/mattermost/mattermost-server/store/sqlstore" @@ -156,13 +156,13 @@ func (me *TestHelper) TearDown() { options := map[string]bool{} options[store.USER_SEARCH_OPTION_NAMES_ONLY_NO_FULL_NAME] = true if result := <-me.App.Srv.Store.User().Search("", "fakeuser", options); result.Err != nil { - l4g.Error("Error tearing down test users") + mlog.Error("Error tearing down test users") } else { users := result.Data.([]*model.User) for _, u := range users { if err := me.App.PermanentDeleteUser(u); err != nil { - l4g.Error(err.Error()) + mlog.Error(err.Error()) } } } @@ -171,13 +171,13 @@ func (me *TestHelper) TearDown() { go func() { defer wg.Done() if result := <-me.App.Srv.Store.Team().SearchByName("faketeam"); result.Err != nil { - l4g.Error("Error tearing down test teams") + mlog.Error("Error tearing down test teams") } else { teams := result.Data.([]*model.Team) for _, t := range teams { if err := me.App.PermanentDeleteTeam(t); err != nil { - l4g.Error(err.Error()) + mlog.Error(err.Error()) } } } @@ -186,7 +186,7 @@ func (me *TestHelper) TearDown() { go func() { defer wg.Done() if result := <-me.App.Srv.Store.OAuth().GetApps(0, 1000); result.Err != nil { - l4g.Error("Error tearing down test oauth apps") + mlog.Error("Error tearing down test oauth apps") } else { apps := result.Data.([]*model.OAuthApp) @@ -450,8 +450,8 @@ func (me *TestHelper) UpdateActiveUser(user *model.User, active bool) { _, err := me.App.UpdateActive(user, active) if err != nil { - l4g.Error(err.Error()) - l4g.Close() + mlog.Error(err.Error()) + time.Sleep(time.Second) panic(err) } @@ -464,8 +464,8 @@ func (me *TestHelper) LinkUserToTeam(user *model.User, team *model.Team) { err := me.App.JoinUserToTeam(team, user, "") if err != nil { - l4g.Error(err.Error()) - l4g.Close() + mlog.Error(err.Error()) + time.Sleep(time.Second) panic(err) } @@ -478,8 +478,8 @@ func (me *TestHelper) AddUserToChannel(user *model.User, channel *model.Channel) member, err := me.App.AddUserToChannel(user, channel) if err != nil { - l4g.Error(err.Error()) - l4g.Close() + mlog.Error(err.Error()) + time.Sleep(time.Second) panic(err) } @@ -784,8 +784,8 @@ func (me *TestHelper) UpdateUserToTeamAdmin(user *model.User, team *model.Team) tm := &model.TeamMember{TeamId: team.Id, UserId: user.Id, Roles: model.TEAM_USER_ROLE_ID + " " + model.TEAM_ADMIN_ROLE_ID} if tmr := <-me.App.Srv.Store.Team().UpdateMember(tm); tmr.Err != nil { utils.EnableDebugLogForTest() - l4g.Error(tmr.Err.Error()) - l4g.Close() + mlog.Error(tmr.Err.Error()) + time.Sleep(time.Second) panic(tmr.Err) } @@ -798,8 +798,8 @@ func (me *TestHelper) UpdateUserToNonTeamAdmin(user *model.User, team *model.Tea tm := &model.TeamMember{TeamId: team.Id, UserId: user.Id, Roles: model.TEAM_USER_ROLE_ID} if tmr := <-me.App.Srv.Store.Team().UpdateMember(tm); tmr.Err != nil { utils.EnableDebugLogForTest() - l4g.Error(tmr.Err.Error()) - l4g.Close() + mlog.Error(tmr.Err.Error()) + time.Sleep(time.Second) panic(tmr.Err) } diff --git a/api4/channel.go b/api4/channel.go index 685c188bc..83fa8eb18 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -6,8 +6,7 @@ package api4 import ( "net/http" - l4g "github.com/alecthomas/log4go" - + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" ) @@ -139,7 +138,7 @@ func updateChannel(c *Context, w http.ResponseWriter, r *http.Request) { } else { if oldChannelDisplayName != channel.DisplayName { if err := c.App.PostUpdateChannelDisplayNameMessage(c.Session.UserId, channel, oldChannelDisplayName, channel.DisplayName); err != nil { - l4g.Error(err.Error()) + mlog.Error(err.Error()) } } diff --git a/api4/context.go b/api4/context.go index 9f3822633..c965e1d80 100644 --- a/api4/context.go +++ b/api4/context.go @@ -10,10 +10,10 @@ import ( "strings" "time" - l4g "github.com/alecthomas/log4go" goi18n "github.com/nicksnyder/go-i18n/i18n" "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" ) @@ -90,7 +90,7 @@ type handler struct { func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { now := time.Now() - l4g.Debug("%v - %v", r.Method, r.URL.Path) + mlog.Debug(fmt.Sprintf("%v - %v", r.Method, r.URL.Path)) c := &Context{} c.App = h.app @@ -124,7 +124,7 @@ func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { session, err := c.App.GetSession(token) if err != nil { - l4g.Info(utils.T("api.context.invalid_session.error"), err.Error()) + mlog.Info(fmt.Sprintf("Invalid session err=%v", err.Error())) if err.StatusCode == http.StatusInternalServerError { c.Err = err } else if h.requireSession { @@ -220,19 +220,19 @@ func (c *Context) LogError(err *model.AppError) { err.Id == "web.check_browser_compatibility.app_error" { c.LogDebug(err) } else { - l4g.Error(utils.TDefault("api.context.log.error"), c.Path, err.Where, err.StatusCode, - c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError) + mlog.Error(fmt.Sprintf("%v:%v code=%v rid=%v uid=%v ip=%v %v [details: %v]", c.Path, err.Where, err.StatusCode, + c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError), mlog.String("user_id", c.Session.UserId)) } } func (c *Context) LogInfo(err *model.AppError) { - l4g.Info(utils.TDefault("api.context.log.error"), c.Path, err.Where, err.StatusCode, - c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError) + mlog.Info(fmt.Sprintf("%v:%v code=%v rid=%v uid=%v ip=%v %v [details: %v]", c.Path, err.Where, err.StatusCode, + c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError), mlog.String("user_id", c.Session.UserId)) } func (c *Context) LogDebug(err *model.AppError) { - l4g.Debug(utils.TDefault("api.context.log.error"), c.Path, err.Where, err.StatusCode, - c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError) + mlog.Debug(fmt.Sprintf("%v:%v code=%v rid=%v uid=%v ip=%v %v [details: %v]", c.Path, err.Where, err.StatusCode, + c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError), mlog.String("user_id", c.Session.UserId)) } func (c *Context) IsSystemAdmin() bool { diff --git a/api4/oauth.go b/api4/oauth.go index a173159b6..fa120ebbf 100644 --- a/api4/oauth.go +++ b/api4/oauth.go @@ -9,8 +9,8 @@ import ( "path/filepath" "strings" - l4g "github.com/alecthomas/log4go" "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" ) @@ -463,7 +463,7 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) { if err != nil { err.Translate(c.T) - l4g.Error(err.Error()) + mlog.Error(err.Error()) if action == model.OAUTH_ACTION_MOBILE { w.Write([]byte(err.ToJson())) } else { @@ -475,7 +475,7 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) { user, err := c.App.CompleteOAuth(service, body, teamId, props) if err != nil { err.Translate(c.T) - l4g.Error(err.Error()) + mlog.Error(err.Error()) if action == model.OAUTH_ACTION_MOBILE { w.Write([]byte(err.ToJson())) } else { diff --git a/api4/plugin.go b/api4/plugin.go index ee7121ffb..37fbf12cd 100644 --- a/api4/plugin.go +++ b/api4/plugin.go @@ -8,7 +8,7 @@ package api4 import ( "net/http" - l4g "github.com/alecthomas/log4go" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" ) @@ -17,7 +17,7 @@ const ( ) func (api *API) InitPlugin() { - l4g.Debug("EXPERIMENTAL: Initializing plugin api") + mlog.Debug("EXPERIMENTAL: Initializing plugin api") api.BaseRoutes.Plugins.Handle("", api.ApiSessionRequired(uploadPlugin)).Methods("POST") api.BaseRoutes.Plugins.Handle("", api.ApiSessionRequired(getPlugins)).Methods("GET") diff --git a/api4/system.go b/api4/system.go index c307a39b7..acb02bc3e 100644 --- a/api4/system.go +++ b/api4/system.go @@ -5,11 +5,12 @@ package api4 import ( "bytes" + "fmt" "io" "net/http" "runtime" - l4g "github.com/alecthomas/log4go" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" ) @@ -61,7 +62,7 @@ func getSystemPing(c *Context, w http.ResponseWriter, r *http.Request) { rdata := map[string]string{} rdata["status"] = "unhealthy" - l4g.Warn(utils.T("api.system.go_routines"), actualGoroutines, *c.App.Config().ServiceSettings.GoroutineHealthThreshold) + mlog.Warn(fmt.Sprintf("The number of running goroutines is over the health threshold %v of %v", actualGoroutines, *c.App.Config().ServiceSettings.GoroutineHealthThreshold)) w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(model.MapToJson(rdata))) @@ -229,7 +230,7 @@ func postLog(c *Context, w http.ResponseWriter, r *http.Request) { err.Where = "client" c.LogError(err) } else { - l4g.Debug(msg) + mlog.Debug(fmt.Sprint(msg)) } m["message"] = msg diff --git a/api4/system_test.go b/api4/system_test.go index b12421e62..c0fde6c39 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -7,7 +7,7 @@ import ( "strings" "testing" - l4g "github.com/alecthomas/log4go" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/stretchr/testify/assert" ) @@ -392,7 +392,7 @@ func TestGetLogs(t *testing.T) { Client := th.Client for i := 0; i < 20; i++ { - l4g.Info(i) + mlog.Info(fmt.Sprint(i)) } logs, resp := th.SystemAdminClient.GetLogs(0, 10) diff --git a/api4/user.go b/api4/user.go index e13bf9448..897c49ad1 100644 --- a/api4/user.go +++ b/api4/user.go @@ -9,8 +9,8 @@ import ( "strconv" "time" - l4g "github.com/alecthomas/log4go" "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" ) @@ -1177,7 +1177,7 @@ func sendVerificationEmail(c *Context, w http.ResponseWriter, r *http.Request) { err = c.App.SendEmailVerification(user) if err != nil { // Don't want to leak whether the email is valid or not - l4g.Error(err.Error()) + mlog.Error(err.Error()) ReturnStatusOK(w) return } diff --git a/api4/webhook.go b/api4/webhook.go index a0e7b5785..fadc3fbf3 100644 --- a/api4/webhook.go +++ b/api4/webhook.go @@ -4,16 +4,15 @@ package api4 import ( + "fmt" "io" "net/http" "strings" - l4g "github.com/alecthomas/log4go" - "github.com/gorilla/mux" "github.com/gorilla/schema" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils" ) func (api *API) InitWebhook() { @@ -492,7 +491,7 @@ func incomingWebhook(c *Context, w http.ResponseWriter, r *http.Request) { } if c.App.Config().LogSettings.EnableWebhookDebugging { - l4g.Debug(utils.T("api.webhook.incoming.debug"), incomingWebhookPayload.ToJson()) + mlog.Debug(fmt.Sprint("Incoming webhook received. Content=", incomingWebhookPayload.ToJson())) } err = c.App.HandleIncomingWebhook(id, incomingWebhookPayload) diff --git a/api4/websocket.go b/api4/websocket.go index 7ea19224b..68125621a 100644 --- a/api4/websocket.go +++ b/api4/websocket.go @@ -4,12 +4,12 @@ package api4 import ( + "fmt" "net/http" - l4g "github.com/alecthomas/log4go" "github.com/gorilla/websocket" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils" ) func (api *API) InitWebSocket() { @@ -25,7 +25,7 @@ func connectWebSocket(c *Context, w http.ResponseWriter, r *http.Request) { ws, err := upgrader.Upgrade(w, r, nil) if err != nil { - l4g.Error(utils.T("api.web_socket.connect.error"), err) + mlog.Error(fmt.Sprintf("websocket connect err: %v", err)) c.Err = model.NewAppError("connect", "api.web_socket.connect.upgrade.app_error", nil, "", http.StatusInternalServerError) return } -- cgit v1.2.3-1-g7c22 From 30011f67e88935f750bced6530e8ee92b352b7a3 Mon Sep 17 00:00:00 2001 From: Saturnino Abril Date: Mon, 30 Apr 2018 17:57:57 +0800 Subject: [MM-10354] Add feature to remove team icon (#8684) * set team.LastTeamIconUpdate to 0 when removing team icon * add APIv4 for removing team icon * removed comment and updated typo on AppError --- api4/team.go | 21 +++++++++++++++++++++ api4/team_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) (limited to 'api4') diff --git a/api4/team.go b/api4/team.go index 94035a770..023289579 100644 --- a/api4/team.go +++ b/api4/team.go @@ -32,6 +32,7 @@ func (api *API) InitTeam() { api.BaseRoutes.Team.Handle("/image", api.ApiSessionRequiredTrustRequester(getTeamIcon)).Methods("GET") api.BaseRoutes.Team.Handle("/image", api.ApiSessionRequired(setTeamIcon)).Methods("POST") + api.BaseRoutes.Team.Handle("/image", api.ApiSessionRequired(removeTeamIcon)).Methods("DELETE") api.BaseRoutes.TeamMembers.Handle("", api.ApiSessionRequired(getTeamMembers)).Methods("GET") api.BaseRoutes.TeamMembers.Handle("/ids", api.ApiSessionRequired(getTeamMembersByIds)).Methods("POST") @@ -812,3 +813,23 @@ func setTeamIcon(c *Context, w http.ResponseWriter, r *http.Request) { c.LogAudit("") ReturnStatusOK(w) } + +func removeTeamIcon(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireTeamId() + if c.Err != nil { + return + } + + if !c.App.SessionHasPermissionToTeam(c.Session, c.Params.TeamId, model.PERMISSION_MANAGE_TEAM) { + c.SetPermissionError(model.PERMISSION_MANAGE_TEAM) + return + } + + if err := c.App.RemoveTeamIcon(c.Params.TeamId); err != nil { + c.Err = err + return + } + + c.LogAudit("") + ReturnStatusOK(w) +} diff --git a/api4/team_test.go b/api4/team_test.go index cdf201771..705ff603b 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -2015,3 +2015,40 @@ func TestGetTeamIcon(t *testing.T) { _, resp = Client.GetTeamIcon(team.Id, "") CheckUnauthorizedStatus(t, resp) } + +func TestRemoveTeamIcon(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer th.TearDown() + Client := th.Client + team := th.BasicTeam + + th.LoginTeamAdmin() + data, _ := readTestFile("test.png") + Client.SetTeamIcon(team.Id, data) + + _, resp := Client.RemoveTeamIcon(team.Id) + CheckNoError(t, resp) + teamAfter, _ := th.App.GetTeam(team.Id) + if teamAfter.LastTeamIconUpdate != 0 { + t.Fatal("should update LastTeamIconUpdate to 0") + } + + Client.SetTeamIcon(team.Id, data) + + _, resp = th.SystemAdminClient.RemoveTeamIcon(team.Id) + CheckNoError(t, resp) + teamAfter, _ = th.App.GetTeam(team.Id) + if teamAfter.LastTeamIconUpdate != 0 { + t.Fatal("should update LastTeamIconUpdate to 0") + } + + Client.SetTeamIcon(team.Id, data) + Client.Logout() + + _, resp = Client.RemoveTeamIcon(team.Id) + CheckUnauthorizedStatus(t, resp) + + th.LoginBasic() + _, resp = Client.RemoveTeamIcon(team.Id) + CheckForbiddenStatus(t, resp) +} -- cgit v1.2.3-1-g7c22