From 7cc1f19453e9aeec6d6534b3e4dfd64c478b2574 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Collin?= Date: Wed, 30 Oct 2019 04:17:04 +0100 Subject: [PATCH] [MM-18898] Stringify plugin.Log* parameters (#12700) - Add stringutils with method that "stringify" object slices - "stringify" means convert each object to its string representation - Move plugin.Log* implementations to client_rpc, use stringify method before calling server method - Exclude Log* methods from generated RPC methods - No signature change for plugin.Log* API - Add test in plugin_api_test to use plugin.Log* methods with RPC --- app/plugin_api_test.go | 11 ++ app/plugin_api_test/plugin_using_log_api.go | 29 +++++ plugin/api.go | 4 - plugin/client_rpc.go | 117 +++++++++++++++++++- plugin/client_rpc_generated.go | 112 ------------------- plugin/interface_generator/main.go | 4 + plugin/stringifier.go | 31 ++++++ plugin/stringifier_test.go | 93 ++++++++++++++++ 8 files changed, 284 insertions(+), 117 deletions(-) create mode 100644 app/plugin_api_test/plugin_using_log_api.go create mode 100644 plugin/stringifier.go create mode 100644 plugin/stringifier_test.go diff --git a/app/plugin_api_test.go b/app/plugin_api_test.go index dc5fab7d3e..a353e5ce74 100644 --- a/app/plugin_api_test.go +++ b/app/plugin_api_test.go @@ -1440,6 +1440,17 @@ func TestPluginAPIGetUnsanitizedConfig(t *testing.T) { } } +func TestPluginCallLogAPI(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + pluginID := "com.mattermost.sample" + path, _ := fileutils.FindDir("mattermost-server/app/plugin_api_test") + pluginCode, err := ioutil.ReadFile(filepath.Join(path, "plugin_using_log_api.go")) + assert.NoError(t, err) + setupPluginApiTest(t, string(pluginCode), + `{"id": "com.mattermost.sample", "server": {"executable": "backend.exe"}, "settings_schema": {"settings": []}}`, pluginID, th.App) +} + func TestPluginAddUserToChannel(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/app/plugin_api_test/plugin_using_log_api.go b/app/plugin_api_test/plugin_using_log_api.go new file mode 100644 index 0000000000..159ec7f3b3 --- /dev/null +++ b/app/plugin_api_test/plugin_using_log_api.go @@ -0,0 +1,29 @@ +// Copyright (c) 2019-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package main + +import ( + "github.com/mattermost/mattermost-server/plugin" + "github.com/pkg/errors" +) + +type PluginUsingLogAPI struct { + plugin.MattermostPlugin +} + +type Foo struct { + bar float64 +} + +func main() { + plugin.ClientMain(&PluginUsingLogAPI{}) +} + +func (p *PluginUsingLogAPI) OnActivate() error { + p.API.LogDebug("LogDebug", "one", 1, "two", "two", "foo", Foo{bar: 3.1416}) + p.API.LogInfo("LogInfo", "one", 1, "two", "two", "foo", Foo{bar: 3.1416}) + p.API.LogWarn("LogWarn", "one", 1, "two", "two", "foo", Foo{bar: 3.1416}) + p.API.LogError("LogError", "error", errors.WithStack(errors.New("boom!"))) + return nil +} diff --git a/plugin/api.go b/plugin/api.go index 49ac0ebf8f..6a2ce229c3 100644 --- a/plugin/api.go +++ b/plugin/api.go @@ -663,7 +663,6 @@ type API interface { // LogDebug 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 LogDebug(msg string, keyValuePairs ...interface{}) @@ -671,7 +670,6 @@ type API 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{}) @@ -679,7 +677,6 @@ type API 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{}) @@ -687,7 +684,6 @@ type API 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{}) diff --git a/plugin/client_rpc.go b/plugin/client_rpc.go index 1bf1f4e435..0ef0709358 100644 --- a/plugin/client_rpc.go +++ b/plugin/client_rpc.go @@ -19,7 +19,7 @@ import ( "reflect" "github.com/dyatlov/go-opengraph/opengraph" - plugin "github.com/hashicorp/go-plugin" + "github.com/hashicorp/go-plugin" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" ) @@ -531,3 +531,118 @@ func (s *hooksRPCServer) MessageWillBeUpdated(args *Z_MessageWillBeUpdatedArgs, } return nil } + +type Z_LogDebugArgs struct { + A string + B []interface{} +} + +type Z_LogDebugReturns struct { +} + +func (g *apiRPCClient) LogDebug(msg string, keyValuePairs ...interface{}) { + stringifiedPairs := stringifyToObjects(keyValuePairs) + _args := &Z_LogDebugArgs{msg, stringifiedPairs} + _returns := &Z_LogDebugReturns{} + if err := g.client.Call("Plugin.LogDebug", _args, _returns); err != nil { + log.Printf("RPC call to LogDebug API failed: %s", err.Error()) + } + +} + +func (s *apiRPCServer) LogDebug(args *Z_LogDebugArgs, returns *Z_LogDebugReturns) error { + if hook, ok := s.impl.(interface { + LogDebug(msg string, keyValuePairs ...interface{}) + }); ok { + hook.LogDebug(args.A, args.B...) + } else { + return encodableError(fmt.Errorf("API LogDebug called but not implemented.")) + } + return nil +} + +type Z_LogInfoArgs struct { + A string + B []interface{} +} + +type Z_LogInfoReturns struct { +} + +func (g *apiRPCClient) LogInfo(msg string, keyValuePairs ...interface{}) { + stringifiedPairs := stringifyToObjects(keyValuePairs) + _args := &Z_LogInfoArgs{msg, stringifiedPairs} + _returns := &Z_LogInfoReturns{} + if err := g.client.Call("Plugin.LogInfo", _args, _returns); err != nil { + log.Printf("RPC call to LogInfo API failed: %s", err.Error()) + } + +} + +func (s *apiRPCServer) LogInfo(args *Z_LogInfoArgs, returns *Z_LogInfoReturns) error { + if hook, ok := s.impl.(interface { + LogInfo(msg string, keyValuePairs ...interface{}) + }); ok { + hook.LogInfo(args.A, args.B...) + } else { + return encodableError(fmt.Errorf("API LogInfo called but not implemented.")) + } + return nil +} + +type Z_LogWarnArgs struct { + A string + B []interface{} +} + +type Z_LogWarnReturns struct { +} + +func (g *apiRPCClient) LogWarn(msg string, keyValuePairs ...interface{}) { + stringifiedPairs := stringifyToObjects(keyValuePairs) + _args := &Z_LogWarnArgs{msg, stringifiedPairs} + _returns := &Z_LogWarnReturns{} + if err := g.client.Call("Plugin.LogWarn", _args, _returns); err != nil { + log.Printf("RPC call to LogWarn API failed: %s", err.Error()) + } + +} + +func (s *apiRPCServer) LogWarn(args *Z_LogWarnArgs, returns *Z_LogWarnReturns) error { + if hook, ok := s.impl.(interface { + LogWarn(msg string, keyValuePairs ...interface{}) + }); ok { + hook.LogWarn(args.A, args.B...) + } else { + return encodableError(fmt.Errorf("API LogWarn called but not implemented.")) + } + return nil +} + +type Z_LogErrorArgs struct { + A string + B []interface{} +} + +type Z_LogErrorReturns struct { +} + +func (g *apiRPCClient) LogError(msg string, keyValuePairs ...interface{}) { + stringifiedPairs := stringifyToObjects(keyValuePairs) + _args := &Z_LogErrorArgs{msg, stringifiedPairs} + _returns := &Z_LogErrorReturns{} + if err := g.client.Call("Plugin.LogError", _args, _returns); err != nil { + log.Printf("RPC call to LogError API failed: %s", err.Error()) + } +} + +func (s *apiRPCServer) LogError(args *Z_LogErrorArgs, returns *Z_LogErrorReturns) error { + if hook, ok := s.impl.(interface { + LogError(msg string, keyValuePairs ...interface{}) + }); ok { + hook.LogError(args.A, args.B...) + } else { + return encodableError(fmt.Errorf("API LogError called but not implemented.")) + } + return nil +} diff --git a/plugin/client_rpc_generated.go b/plugin/client_rpc_generated.go index d91ae36db6..37e6587321 100644 --- a/plugin/client_rpc_generated.go +++ b/plugin/client_rpc_generated.go @@ -3989,118 +3989,6 @@ func (s *apiRPCServer) HasPermissionToChannel(args *Z_HasPermissionToChannelArgs return nil } -type Z_LogDebugArgs struct { - A string - B []interface{} -} - -type Z_LogDebugReturns struct { -} - -func (g *apiRPCClient) LogDebug(msg string, keyValuePairs ...interface{}) { - _args := &Z_LogDebugArgs{msg, keyValuePairs} - _returns := &Z_LogDebugReturns{} - if err := g.client.Call("Plugin.LogDebug", _args, _returns); err != nil { - log.Printf("RPC call to LogDebug API failed: %s", err.Error()) - } - -} - -func (s *apiRPCServer) LogDebug(args *Z_LogDebugArgs, returns *Z_LogDebugReturns) error { - if hook, ok := s.impl.(interface { - LogDebug(msg string, keyValuePairs ...interface{}) - }); ok { - hook.LogDebug(args.A, args.B...) - } else { - return encodableError(fmt.Errorf("API LogDebug called but not implemented.")) - } - return nil -} - -type Z_LogInfoArgs struct { - A string - B []interface{} -} - -type Z_LogInfoReturns struct { -} - -func (g *apiRPCClient) LogInfo(msg string, keyValuePairs ...interface{}) { - _args := &Z_LogInfoArgs{msg, keyValuePairs} - _returns := &Z_LogInfoReturns{} - if err := g.client.Call("Plugin.LogInfo", _args, _returns); err != nil { - log.Printf("RPC call to LogInfo API failed: %s", err.Error()) - } - -} - -func (s *apiRPCServer) LogInfo(args *Z_LogInfoArgs, returns *Z_LogInfoReturns) error { - if hook, ok := s.impl.(interface { - LogInfo(msg string, keyValuePairs ...interface{}) - }); ok { - hook.LogInfo(args.A, args.B...) - } else { - return encodableError(fmt.Errorf("API LogInfo called but not implemented.")) - } - return nil -} - -type Z_LogErrorArgs struct { - A string - B []interface{} -} - -type Z_LogErrorReturns struct { -} - -func (g *apiRPCClient) LogError(msg string, keyValuePairs ...interface{}) { - _args := &Z_LogErrorArgs{msg, keyValuePairs} - _returns := &Z_LogErrorReturns{} - if err := g.client.Call("Plugin.LogError", _args, _returns); err != nil { - log.Printf("RPC call to LogError API failed: %s", err.Error()) - } - -} - -func (s *apiRPCServer) LogError(args *Z_LogErrorArgs, returns *Z_LogErrorReturns) error { - if hook, ok := s.impl.(interface { - LogError(msg string, keyValuePairs ...interface{}) - }); ok { - hook.LogError(args.A, args.B...) - } else { - return encodableError(fmt.Errorf("API LogError called but not implemented.")) - } - return nil -} - -type Z_LogWarnArgs struct { - A string - B []interface{} -} - -type Z_LogWarnReturns struct { -} - -func (g *apiRPCClient) LogWarn(msg string, keyValuePairs ...interface{}) { - _args := &Z_LogWarnArgs{msg, keyValuePairs} - _returns := &Z_LogWarnReturns{} - if err := g.client.Call("Plugin.LogWarn", _args, _returns); err != nil { - log.Printf("RPC call to LogWarn API failed: %s", err.Error()) - } - -} - -func (s *apiRPCServer) LogWarn(args *Z_LogWarnArgs, returns *Z_LogWarnReturns) error { - if hook, ok := s.impl.(interface { - LogWarn(msg string, keyValuePairs ...interface{}) - }); ok { - hook.LogWarn(args.A, args.B...) - } else { - return encodableError(fmt.Errorf("API LogWarn called but not implemented.")) - } - return nil -} - type Z_SendMailArgs struct { A string B string diff --git a/plugin/interface_generator/main.go b/plugin/interface_generator/main.go index 4afb254436..e3a70fbd08 100644 --- a/plugin/interface_generator/main.go +++ b/plugin/interface_generator/main.go @@ -399,6 +399,10 @@ func removeExcluded(info *PluginInterfaceInfo) *PluginInterfaceInfo { "FileWillBeUploaded", "MessageWillBePosted", "MessageWillBeUpdated", + "LogDebug", + "LogInfo", + "LogWarn", + "LogError", } for _, exclusion := range excluded { if exclusion == item { diff --git a/plugin/stringifier.go b/plugin/stringifier.go new file mode 100644 index 0000000000..1455fb4e63 --- /dev/null +++ b/plugin/stringifier.go @@ -0,0 +1,31 @@ +// Copyright (c) 2019-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package plugin + +import ( + "fmt" +) + +func stringify(objects []interface{}) []string { + stringified := make([]string, len(objects), len(objects)) + for i, object := range objects { + stringified[i] = fmt.Sprintf("%+v", object) + } + return stringified +} + +func toObjects(strings []string) []interface{} { + if strings == nil { + return nil + } + objects := make([]interface{}, len(strings)) + for i, string := range strings { + objects[i] = string + } + return objects +} + +func stringifyToObjects(objects []interface{}) []interface{} { + return toObjects(stringify(objects)) +} diff --git a/plugin/stringifier_test.go b/plugin/stringifier_test.go new file mode 100644 index 0000000000..58ae7e44d1 --- /dev/null +++ b/plugin/stringifier_test.go @@ -0,0 +1,93 @@ +// Copyright (c) 2019-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package plugin + +import ( + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestStringify(t *testing.T) { + t.Run("NilShouldReturnEmpty", func(t *testing.T) { + strings := stringify(nil) + assert.Empty(t, strings) + }) + t.Run("EmptyShouldReturnEmpty", func(t *testing.T) { + strings := stringify(make([]interface{}, 0, 0)) + assert.Empty(t, strings) + }) + t.Run("PrimitivesAndCompositesShouldReturnCorrectValues", func(t *testing.T) { + strings := stringify([]interface{}{ + 1234, + 3.14159265358979323846264338327950288419716939937510, + true, + "foo", + nil, + []string{"foo", "bar"}, + map[string]int{"one": 1, "two": 2}, + &WithString{}, + &WithoutString{}, + &WithStringAndError{}, + }) + assert.Equal(t, []string{ + "1234", + "3.141592653589793", + "true", + "foo", + "", + "[foo bar]", + "map[one:1 two:2]", + "string", + "&{}", + "error", + }, strings) + }) + t.Run("ErrorShouldReturnFormattedStack", func(t *testing.T) { + strings := stringify([]interface{}{ + errors.New("error"), + errors.WithStack(errors.New("error")), + }) + stackRegexp := "error\n.*plugin.TestStringify.func\\d+\n\t.*plugin/stringifier_test.go:\\d+\ntesting.tRunner\n\t.*testing.go:\\d+.*" + assert.Len(t, strings, 2) + assert.Regexp(t, stackRegexp, strings[0]) + assert.Regexp(t, stackRegexp, strings[1]) + }) +} + +type WithString struct { +} + +func (*WithString) String() string { + return "string" +} + +type WithoutString struct { +} + +type WithStringAndError struct { +} + +func (*WithStringAndError) String() string { + return "string" +} + +func (*WithStringAndError) Error() string { + return "error" +} + +func TestToObjects(t *testing.T) { + t.Run("NilShouldReturnNil", func(t *testing.T) { + objects := toObjects(nil) + assert.Nil(t, objects) + }) + t.Run("EmptyShouldReturnEmpty", func(t *testing.T) { + objects := toObjects(make([]string, 0, 0)) + assert.Empty(t, objects) + }) + t.Run("ShouldReturnSliceOfObjects", func(t *testing.T) { + objects := toObjects([]string{"foo", "bar"}) + assert.Equal(t, []interface{}{"foo", "bar"}, objects) + }) +}