[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

@@ -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()

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
+}

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{})

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
+}

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

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 {

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))
+}

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",
+			"<nil>",
+			"[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)
+	})
+}