Migrate logging to use RPC Logging methods (#102)

server/autolinkplugin/config.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	"github.com/mattermost/mattermost-plugin-autolink/server/autolink"
-	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
 )
 
@@ -33,7 +32,7 @@ func (p *Plugin) OnConfigurationChange() error {
 
 	for i := range c.Links {
 		if err := c.Links[i].Compile(); err != nil {
-			mlog.Error(fmt.Sprintf("Error creating autolinker: %+v: %v", c.Links[i], err))
+			p.API.LogError("Error creating autolinker", "link", c.Links[i], "error", err.Error())
 		}
 	}
 
@@ -138,8 +137,7 @@ func (conf *Config) parsePluginAdminList(p *Plugin) {
 		// Let's verify that the given user really exists
 		_, appErr := p.API.GetUser(userId)
 		if appErr != nil {
-			mlog.Error(fmt.Sprintf(
-				"error occured while verifying userId %s: %v", v, appErr))
+			p.API.LogError(fmt.Sprintf("error occured while verifying userId %s: %v", v, appErr))
 		} else {
 			conf.AdminUserIds[userId] = struct{}{}
 		}

server/autolinkplugin/config_test.go

@@ -0,0 +1,63 @@
+package autolinkplugin
+
+import (
+	"errors"
+	"github.com/mattermost/mattermost-plugin-autolink/server/autolink"
+	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/plugin/plugintest"
+	"github.com/mattermost/mattermost-server/v5/plugin/plugintest/mock"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestOnConfigurationChange(t *testing.T) {
+
+	t.Run("Invalid Configuration", func(t *testing.T) {
+		api := &plugintest.API{}
+		api.On("LoadPluginConfiguration",
+			mock.AnythingOfType("*autolinkplugin.Config")).Return(func(dest interface{}) error {
+			return errors.New("LoadPluginConfiguration Error")
+		})
+
+		p := Plugin{}
+		p.SetAPI(api)
+
+		err := p.OnConfigurationChange()
+		assert.Error(t, err)
+	})
+
+	t.Run("Invalid Autolink", func(t *testing.T) {
+		conf := Config{
+			Links: []autolink.Autolink{
+				autolink.Autolink{
+					Name:     "existing",
+					Pattern:  ")",
+					Template: "otherthing",
+				},
+			},
+		}
+
+		api := &plugintest.API{}
+		api.On("LoadPluginConfiguration",
+			mock.AnythingOfType("*autolinkplugin.Config")).Return(func(dest interface{}) error {
+			*dest.(*Config) = conf
+			return nil
+		})
+
+		api.On("LogError",
+			mock.AnythingOfType("string"),
+			mock.AnythingOfType("string"),
+			mock.AnythingOfType("autolink.Autolink"),
+			mock.AnythingOfType("string"),
+			mock.AnythingOfType("string")).Return(nil)
+
+		api.On("UnregisterCommand", mock.AnythingOfType("string"),
+			mock.AnythingOfType("string")).Return((*model.AppError)(nil))
+
+		p := New()
+		p.SetAPI(api)
+		p.OnConfigurationChange()
+
+		api.AssertNumberOfCalls(t, "LogError", 1)
+	})
+}

server/autolinkplugin/plugin.go

@@ -44,14 +44,14 @@ func (p *Plugin) IsAuthorizedAdmin(userId string) (bool, error) {
 			"failed to obtain information about user `%s`: %w", userId, err)
 	}
 	if strings.Contains(user.Roles, "system_admin") {
-		mlog.Info(
+		p.API.LogInfo(
 			fmt.Sprintf("UserId `%s` is authorized basing on the sysadmin role membership", userId))
 		return true, nil
 	}
 
 	conf := p.getConfig()
 	if _, ok := conf.AdminUserIds[userId]; ok {
-		mlog.Info(
+		p.API.LogInfo(
 			fmt.Sprintf("UserId `%s` is authorized basing on the list of plugin admins list", userId))
 		return true, nil
 	}
@@ -103,14 +103,16 @@ func (p *Plugin) ProcessPost(c *plugin.Context, post *model.Post) (*model.Post,
 			startPos, endPos = autolinkNode.RawDestination.Position+offset, autolinkNode.RawDestination.End+offset
 			origText = postText[startPos:endPos]
 			if autolinkNode.Destination() != origText {
-				mlog.Error(fmt.Sprintf("Markdown autolink did not match range text, '%s' != '%s'", autolinkNode.Destination(), origText))
+				p.API.LogError(fmt.Sprintf("Markdown autolink did not match range text, '%s' != '%s'",
+					autolinkNode.Destination(), origText))
 				return true
 			}
 		} else if textNode, ok := node.(*markdown.Text); ok {
 			startPos, endPos = textNode.Range.Position+offset, textNode.Range.End+offset
 			origText = postText[startPos:endPos]
 			if textNode.Text != origText {
-				mlog.Error(fmt.Sprintf("Markdown text did not match range text, '%s' != '%s'", textNode.Text, origText))
+				p.API.LogError(fmt.Sprintf("Markdown text did not match range text, '%s' != '%s'", textNode.Text,
+					origText))
 				return true
 			}
 		}
@@ -120,14 +122,14 @@ func (p *Plugin) ProcessPost(c *plugin.Context, post *model.Post) (*model.Post,
 
 			channel, cErr := p.API.GetChannel(post.ChannelId)
 			if cErr != nil {
-				mlog.Error(cErr.Error())
+				p.API.LogError("Failed to get Channel", "error", cErr.Error())
 				return false
 			}
 			teamName := ""
 			if channel.TeamId != "" {
 				team, tErr := p.API.GetTeam(channel.TeamId)
 				if tErr != nil {
-					mlog.Error(tErr.Error())
+					p.API.LogError("Failed to get Team", "error", tErr.Error())
 					return false
 				}
 				teamName = team.Name

server/autolinkplugin/plugin_test.go

@@ -115,6 +115,8 @@ func (suite *SuiteAuthorization) TestSysadminIsAuthorized() {
 		Roles:    "smurf,system_admin,reaper",
 	}
 
+	suite.api.On("LogInfo", mock.AnythingOfType("string")).Return(nil)
+
 	p := New()
 	p.SetAPI(suite.api)
 
@@ -152,6 +154,8 @@ func (suite *SuiteAuthorization) TestAdminUserIsAuthorized() {
 	}
 	suite.adminUsernames = "marynaId"
 
+	suite.api.On("LogInfo", mock.AnythingOfType("string")).Return(nil)
+
 	p := New()
 	p.SetAPI(suite.api)
 
@@ -181,6 +185,8 @@ func (suite *SuiteAuthorization) TestMultipleUsersAreAuthorized() {
 	}
 	suite.adminUsernames = "marynaId,karynaId"
 
+	suite.api.On("LogInfo", mock.AnythingOfType("string")).Return(nil)
+
 	p := New()
 	p.SetAPI(suite.api)
 
@@ -218,6 +224,8 @@ func (suite *SuiteAuthorization) TestWhitespaceIsIgnored() {
 	}
 	suite.adminUsernames = "marynaId , karynaId, borynaId "
 
+	suite.api.On("LogInfo", mock.AnythingOfType("string")).Return(nil)
+
 	p := New()
 	p.SetAPI(suite.api)
 
@@ -245,6 +253,9 @@ func (suite *SuiteAuthorization) TestNonExistantUsersAreIgnored() {
 	}
 	suite.adminUsernames = "marynaId,karynaId"
 
+	suite.api.On("LogError", mock.AnythingOfType("string")).Return(nil)
+	suite.api.On("LogInfo", mock.AnythingOfType("string")).Return(nil)
+
 	p := New()
 	p.SetAPI(suite.api)