Make all loggers derive from non-default instance (#728)

5 files changed, 96 insertions, 86 deletions

diff --git a/bridge/bridge.go b/bridge/bridge.go
index 6b955a9e..fdc1ec8b 100644
--- a/bridge/bridge.go
+++ b/bridge/bridge.go
@@ -2,10 +2,10 @@ package bridge
 
 import (
 	"strings"
+	"sync"
 
 	"github.com/42wim/matterbridge/bridge/config"
 	"github.com/sirupsen/logrus"
-	"sync"
 )
 
 type Bridger interface {
@@ -17,6 +17,8 @@ type Bridger interface {
 
 type Bridge struct {
 	Bridger
+	*sync.RWMutex
+
 	Name           string
 	Account        string
 	Protocol       string
@@ -26,37 +28,34 @@ type Bridge struct {
 	Log            *logrus.Entry
 	Config         config.Config
 	General        *config.Protocol
-	*sync.RWMutex
 }
 
 type Config struct {
-	//	General *config.Protocol
-	Remote chan config.Message
-	Log    *logrus.Entry
 	*Bridge
+
+	Remote chan config.Message
 }
 
 // Factory is the factory function to create a bridge
 type Factory func(*Config) Bridger
 
 func New(bridge *config.Bridge) *Bridge {
-	b := &Bridge{
-		Channels: make(map[string]config.ChannelInfo),
-		RWMutex:  new(sync.RWMutex),
-		Joined:   make(map[string]bool),
-	}
 	accInfo := strings.Split(bridge.Account, ".")
 	protocol := accInfo[0]
 	name := accInfo[1]
-	b.Name = name
-	b.Protocol = protocol
-	b.Account = bridge.Account
-	return b
+
+	return &Bridge{
+		RWMutex:  new(sync.RWMutex),
+		Channels: make(map[string]config.ChannelInfo),
+		Name:     name,
+		Protocol: protocol,
+		Account:  bridge.Account,
+		Joined:   make(map[string]bool),
+	}
 }
 
 func (b *Bridge) JoinChannels() error {
-	err := b.joinChannels(b.Channels, b.Joined)
-	return err
+	return b.joinChannels(b.Channels, b.Joined)
 }
 
 // SetChannelMembers sets the newMembers to the bridge ChannelMembers
diff --git a/bridge/config/config.go b/bridge/config/config.go
index 47914951..61ffe913 100644
--- a/bridge/config/config.go
+++ b/bridge/config/config.go
@@ -8,7 +8,6 @@ import (
 	"time"
 
 	"github.com/fsnotify/fsnotify"
-	prefixed "github.com/matterbridge/logrus-prefixed-formatter"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/viper"
 )
@@ -204,63 +203,58 @@ type Config interface {
 }
 
 type config struct {
-	v *viper.Viper
 	sync.RWMutex
 
-	cv *BridgeValues
+	logger *logrus.Entry
+	v      *viper.Viper
+	cv     *BridgeValues
 }
 
-func NewConfig(cfgfile string) Config {
-	logrus.SetFormatter(&prefixed.TextFormatter{PrefixPadding: 13, DisableColors: true, FullTimestamp: false})
-	flog := logrus.WithFields(logrus.Fields{"prefix": "config"})
+// NewConfig instantiates a new configuration based on the specified configuration file path.
+func NewConfig(rootLogger *logrus.Logger, cfgfile string) Config {
+	logger := rootLogger.WithFields(logrus.Fields{"prefix": "config"})
+
 	viper.SetConfigFile(cfgfile)
-	input, err := getFileContents(cfgfile)
+	input, err := ioutil.ReadFile(cfgfile)
 	if err != nil {
-		logrus.Fatal(err)
+		logger.Fatalf("Failed to read configuration file: %#v", err)
 	}
-	mycfg := newConfigFromString(input)
+
+	mycfg := newConfigFromString(logger, input)
 	if mycfg.cv.General.MediaDownloadSize == 0 {
 		mycfg.cv.General.MediaDownloadSize = 1000000
 	}
 	viper.WatchConfig()
 	viper.OnConfigChange(func(e fsnotify.Event) {
-		flog.Println("Config file changed:", e.Name)
+		logger.Println("Config file changed:", e.Name)
 	})
 	return mycfg
 }
 
-func getFileContents(filename string) ([]byte, error) {
-	input, err := ioutil.ReadFile(filename)
-	if err != nil {
-		logrus.Fatal(err)
-		return []byte(nil), err
-	}
-	return input, nil
-}
-
-func NewConfigFromString(input []byte) Config {
-	return newConfigFromString(input)
+// NewConfigFromString instantiates a new configuration based on the specified string.
+func NewConfigFromString(rootLogger *logrus.Logger, input []byte) Config {
+	logger := rootLogger.WithFields(logrus.Fields{"prefix": "config"})
+	return newConfigFromString(logger, input)
 }
 
-func newConfigFromString(input []byte) *config {
+func newConfigFromString(logger *logrus.Entry, input []byte) *config {
 	viper.SetConfigType("toml")
 	viper.SetEnvPrefix("matterbridge")
-	viper.AddConfigPath(".")
 	viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_", "-", "_"))
 	viper.AutomaticEnv()
-	err := viper.ReadConfig(bytes.NewBuffer(input))
-	if err != nil {
-		logrus.Fatal(err)
+
+	if err := viper.ReadConfig(bytes.NewBuffer(input)); err != nil {
+		logger.Fatalf("Failed to parse the configuration: %#v", err)
 	}
 
 	cfg := &BridgeValues{}
-	err = viper.Unmarshal(cfg)
-	if err != nil {
-		logrus.Fatal(err)
+	if err := viper.Unmarshal(cfg); err != nil {
+		logger.Fatalf("Failed to load the configuration: %#v", err)
 	}
 	return &config{
-		v:  viper.GetViper(),
-		cv: cfg,
+		logger: logger,
+		v:      viper.GetViper(),
+		cv:     cfg,
 	}
 }
 
@@ -271,46 +265,44 @@ func (c *config) BridgeValues() *BridgeValues {
 func (c *config) GetBool(key string) (bool, bool) {
 	c.RLock()
 	defer c.RUnlock()
-	//	log.Debugf("getting bool %s = %#v", key, c.v.GetBool(key))
 	return c.v.GetBool(key), c.v.IsSet(key)
 }
 
 func (c *config) GetInt(key string) (int, bool) {
 	c.RLock()
 	defer c.RUnlock()
-	//	log.Debugf("getting int %s = %d", key, c.v.GetInt(key))
 	return c.v.GetInt(key), c.v.IsSet(key)
 }
 
 func (c *config) GetString(key string) (string, bool) {
 	c.RLock()
 	defer c.RUnlock()
-	//	log.Debugf("getting String %s = %s", key, c.v.GetString(key))
 	return c.v.GetString(key), c.v.IsSet(key)
 }
 
 func (c *config) GetStringSlice(key string) ([]string, bool) {
 	c.RLock()
 	defer c.RUnlock()
-	// log.Debugf("getting StringSlice %s = %#v", key, c.v.GetStringSlice(key))
 	return c.v.GetStringSlice(key), c.v.IsSet(key)
 }
 
 func (c *config) GetStringSlice2D(key string) ([][]string, bool) {
 	c.RLock()
 	defer c.RUnlock()
-	result := [][]string{}
-	if res, ok := c.v.Get(key).([]interface{}); ok {
-		for _, entry := range res {
-			result2 := []string{}
-			for _, entry2 := range entry.([]interface{}) {
-				result2 = append(result2, entry2.(string))
-			}
-			result = append(result, result2)
+
+	res, ok := c.v.Get(key).([]interface{})
+	if !ok {
+		return nil, false
+	}
+	var result [][]string
+	for _, entry := range res {
+		result2 := []string{}
+		for _, entry2 := range entry.([]interface{}) {
+			result2 = append(result2, entry2.(string))
 		}
-		return result, true
+		result = append(result, result2)
 	}
-	return result, false
+	return result, true
 }
 
 func GetIconURL(msg *Message, iconURL string) string {
diff --git a/bridge/helper/helper.go b/bridge/helper/helper.go
index 12a587f0..3836556f 100644
--- a/bridge/helper/helper.go
+++ b/bridge/helper/helper.go
@@ -15,10 +15,12 @@ import (
 	"gitlab.com/golang-commonmark/markdown"
 )
 
+// DownloadFile downloads the given non-authenticated URL.
 func DownloadFile(url string) (*[]byte, error) {
 	return DownloadFileAuth(url, "")
 }
 
+// DownloadFileAuth downloads the given URL using the specified authentication token.
 func DownloadFileAuth(url string, auth string) (*[]byte, error) {
 	var buf bytes.Buffer
 	client := &http.Client{
@@ -42,8 +44,8 @@ func DownloadFileAuth(url string, auth string) (*[]byte, error) {
 }
 
 // GetSubLines splits messages in newline-delimited lines. If maxLineLength is
-// specified as non-zero GetSubLines will and also clip long lines to the
-// maximum length and insert a warning marker that the line was clipped.
+// specified as non-zero GetSubLines will also clip long lines to the maximum
+// length and insert a warning marker that the line was clipped.
 //
 // TODO: The current implementation has the inconvenient that it disregards
 // word boundaries when splitting but this is hard to solve without potentially
@@ -79,18 +81,24 @@ func GetSubLines(message string, maxLineLength int) []string {
 	return lines
 }
 
-// handle all the stuff we put into extra
+// HandleExtra manages the supplementary details stored inside a message's 'Extra' field map.
 func HandleExtra(msg *config.Message, general *config.Protocol) []config.Message {
 	extra := msg.Extra
 	rmsg := []config.Message{}
 	for _, f := range extra[config.EventFileFailureSize] {
 		fi := f.(config.FileInfo)
 		text := fmt.Sprintf("file %s too big to download (%#v > allowed size: %#v)", fi.Name, fi.Size, general.MediaDownloadSize)
-		rmsg = append(rmsg, config.Message{Text: text, Username: "<system> ", Channel: msg.Channel, Account: msg.Account})
+		rmsg = append(rmsg, config.Message{
+			Text:     text,
+			Username: "<system> ",
+			Channel:  msg.Channel,
+			Account:  msg.Account,
+		})
 	}
 	return rmsg
 }
 
+// GetAvatar constructs a URL for a given user-avatar if it is available in the cache.
 func GetAvatar(av map[string]string, userid string, general *config.Protocol) string {
 	if sha, ok := av[userid]; ok {
 		return general.MediaServerDownload + "/" + sha + "/" + userid + ".png"
@@ -98,13 +106,15 @@ func GetAvatar(av map[string]string, userid string, general *config.Protocol) st
 	return ""
 }
 
-func HandleDownloadSize(flog *logrus.Entry, msg *config.Message, name string, size int64, general *config.Protocol) error {
+// HandleDownloadSize checks a specified filename against the configured download blacklist
+// and checks a specified file-size against the configure limit.
+func HandleDownloadSize(logger *logrus.Entry, msg *config.Message, name string, size int64, general *config.Protocol) error {
 	// check blacklist here
 	for _, entry := range general.MediaDownloadBlackList {
 		if entry != "" {
 			re, err := regexp.Compile(entry)
 			if err != nil {
-				flog.Errorf("incorrect regexp %s for %s", entry, msg.Account)
+				logger.Errorf("incorrect regexp %s for %s", entry, msg.Account)
 				continue
 			}
 			if re.MatchString(name) {
@@ -112,43 +122,53 @@ func HandleDownloadSize(flog *logrus.Entry, msg *config.Message, name string, si
 			}
 		}
 	}
-	flog.Debugf("Trying to download %#v with size %#v", name, size)
+	logger.Debugf("Trying to download %#v with size %#v", name, size)
 	if int(size) > general.MediaDownloadSize {
 		msg.Event = config.EventFileFailureSize
-		msg.Extra[msg.Event] = append(msg.Extra[msg.Event], config.FileInfo{Name: name, Comment: msg.Text, Size: size})
+		msg.Extra[msg.Event] = append(msg.Extra[msg.Event], config.FileInfo{
+			Name:    name,
+			Comment: msg.Text,
+			Size:    size,
+		})
 		return fmt.Errorf("File %#v to large to download (%#v). MediaDownloadSize is %#v", name, size, general.MediaDownloadSize)
 	}
 	return nil
 }
 
-func HandleDownloadData(flog *logrus.Entry, msg *config.Message, name, comment, url string, data *[]byte, general *config.Protocol) {
+// HandleDownloadData adds the data for a remote file into a Matterbridge gateway message.
+func HandleDownloadData(logger *logrus.Entry, msg *config.Message, name, comment, url string, data *[]byte, general *config.Protocol) {
 	var avatar bool
-	flog.Debugf("Download OK %#v %#v", name, len(*data))
+	logger.Debugf("Download OK %#v %#v", name, len(*data))
 	if msg.Event == config.EventAvatarDownload {
 		avatar = true
 	}
-	msg.Extra["file"] = append(msg.Extra["file"], config.FileInfo{Name: name, Data: data, URL: url, Comment: comment, Avatar: avatar})
+	msg.Extra["file"] = append(msg.Extra["file"], config.FileInfo{
+		Name:    name,
+		Data:    data,
+		URL:     url,
+		Comment: comment,
+		Avatar:  avatar,
+	})
 }
 
+var emptyLineMatcher = regexp.MustCompile("\n+")
+
+// RemoveEmptyNewLines collapses consecutive newline characters into a single one and
+// trims any preceding or trailing newline characters as well.
 func RemoveEmptyNewLines(msg string) string {
-	lines := ""
-	for _, line := range strings.Split(msg, "\n") {
-		if line != "" {
-			lines += line + "\n"
-		}
-	}
-	lines = strings.TrimRight(lines, "\n")
-	return lines
+	return emptyLineMatcher.ReplaceAllString(strings.Trim(msg, "\n"), "\n")
 }
 
+// ClipMessage trims a message to the specified length if it exceeds it and adds a warning
+// to the message in case it does so.
 func ClipMessage(text string, length int) string {
-	// clip too long messages
+	const clippingMessage = " <clipped message>"
 	if len(text) > length {
-		text = text[:length-len(" *message clipped*")]
+		text = text[:length-len(clippingMessage)]
 		if r, size := utf8.DecodeLastRuneInString(text); r == utf8.RuneError {
 			text = text[:len(text)-size]
 		}
-		text += " *message clipped*"
+		text += clippingMessage
 	}
 	return text
 }
diff --git a/bridge/slack/helpers_test.go b/bridge/slack/helpers_test.go
index c9ff647d..fe3ba416 100644
--- a/bridge/slack/helpers_test.go
+++ b/bridge/slack/helpers_test.go
@@ -25,7 +25,7 @@ func TestExtractTopicOrPurpose(t *testing.T) {
 
 	logger := logrus.New()
 	logger.SetOutput(ioutil.Discard)
-	cfg := &bridge.Config{Log: logger.WithFields(nil)}
+	cfg := &bridge.Config{Bridge: &bridge.Bridge{Log: logrus.NewEntry(logger)}}
 	b := newBridge(cfg)
 	for name, tc := range testcases {
 		gotChangeType, gotOutput := b.extractTopicOrPurpose(tc.input)
diff --git a/bridge/sshchat/sshchat.go b/bridge/sshchat/sshchat.go
index 5a8029cf..3a4512cb 100644
--- a/bridge/sshchat/sshchat.go
+++ b/bridge/sshchat/sshchat.go
@@ -9,7 +9,6 @@ import (
 	"github.com/42wim/matterbridge/bridge/config"
 	"github.com/42wim/matterbridge/bridge/helper"
 	"github.com/shazow/ssh-chat/sshd"
-	"github.com/sirupsen/logrus"
 )
 
 type Bsshchat struct {
@@ -134,7 +133,7 @@ func (b *Bsshchat) handleSSHChat() error {
 			res := strings.Split(stripPrompt(b.r.Text()), ":")
 			if res[0] == "-> Set theme" {
 				wait = false
-				logrus.Debugf("mono found, allowing")
+				b.Log.Debugf("mono found, allowing")
 				continue
 			}
 			if !wait {