From 498377a230b8712792a47a61ad63ab746f7379d1 Mon Sep 17 00:00:00 2001 From: Duco van Amstel Date: Sat, 13 Oct 2018 00:02:14 +0100 Subject: Clean up code and strengthening (slack) (#519) Changes include: - Refactor of strings into package-wide constants. - Predeclaration of regexps to be instantiated at package load time. - Checking of unchecked errors. - Structural changes: - Adding verifications to type-casting code. - Remove unnecessary 'len(X) > 0' checks before iterating over X. - Remove unnecessary 'else' clause after 'if' with 'return'. - Unexporting of public fields of Bridge struct. - Formatting: - One-field-per-line struct definitions. --- bridge/slack/handlers.go | 128 +++++++++++++++------------- bridge/slack/helpers.go | 73 ++++++++-------- bridge/slack/slack.go | 214 ++++++++++++++++++++++++++++++----------------- 3 files changed, 243 insertions(+), 172 deletions(-) diff --git a/bridge/slack/handlers.go b/bridge/slack/handlers.go index 1ef2d614..56fd7026 100644 --- a/bridge/slack/handlers.go +++ b/bridge/slack/handlers.go @@ -14,7 +14,7 @@ import ( func (b *Bslack) handleSlack() { messages := make(chan *config.Message) - if b.GetString("WebhookBindAddress") != "" { + if b.GetString(incomingWebhookConfig) != "" { b.Log.Debugf("Choosing webhooks based receiving") go b.handleMatterHook(messages) } else { @@ -43,7 +43,7 @@ func (b *Bslack) handleSlack() { func (b *Bslack) handleSlackClient(messages chan *config.Message) { for msg := range b.rtm.IncomingEvents { - if msg.Type != "user_typing" && msg.Type != "latency_report" { + if msg.Type != sUserTyping && msg.Type != sLatencyReport { b.Log.Debugf("== Receiving event %#v", msg.Data) } switch ev := msg.Data.(type) { @@ -61,17 +61,25 @@ func (b *Bslack) handleSlackClient(messages chan *config.Message) { case *slack.OutgoingErrorEvent: b.Log.Debugf("%#v", ev.Error()) case *slack.ChannelJoinedEvent: - b.Users, _ = b.sc.GetUsers() - b.Usergroups, _ = b.sc.GetUserGroups() + var err error + b.users, err = b.sc.GetUsers() + if err != nil { + b.Log.Errorf("Could not reload users: %#v", err) + } case *slack.ConnectedEvent: var err error - b.channels, _, err = b.sc.GetConversations(&slack.GetConversationsParameters{Limit: 1000, Types: []string{"public_channel,private_channel,mpim,im"}}) + b.channels, _, err = b.sc.GetConversations(&slack.GetConversationsParameters{ + Limit: 1000, + Types: []string{"public_channel,private_channel,mpim,im"}, + }) if err != nil { b.Log.Errorf("Channel list failed: %#v", err) } b.si = ev.Info - b.Users, _ = b.sc.GetUsers() - b.Usergroups, _ = b.sc.GetUserGroups() + b.users, err = b.sc.GetUsers() + if err != nil { + b.Log.Errorf("Could not reload users: %#v", err) + } case *slack.InvalidAuthEvent: b.Log.Fatalf("Invalid Token %#v", ev) case *slack.ConnectionErrorEvent: @@ -88,16 +96,22 @@ func (b *Bslack) handleMatterHook(messages chan *config.Message) { if message.UserName == "slackbot" { continue } - messages <- &config.Message{Username: message.UserName, Text: message.Text, Channel: message.ChannelName} + messages <- &config.Message{ + Username: message.UserName, + Text: message.Text, + Channel: message.ChannelName, + } } } +var commentRE = regexp.MustCompile(`.*?commented: (.*)`) + // handleDownloadFile handles file download func (b *Bslack) handleDownloadFile(rmsg *config.Message, file *slack.File) error { // if we have a file attached, download it (in memory) and put a pointer to it in msg.Extra // limit to 1MB for now comment := "" - results := regexp.MustCompile(`.*?commented: (.*)`).FindAllStringSubmatch(rmsg.Text, -1) + results := commentRE.FindAllStringSubmatch(rmsg.Text, -1) if len(results) > 0 { comment = results[0][1] } @@ -106,7 +120,7 @@ func (b *Bslack) handleDownloadFile(rmsg *config.Message, file *slack.File) erro return err } // actually download the file - data, err := helper.DownloadFileAuth(file.URLPrivateDownload, "Bearer "+b.GetString("Token")) + data, err := helper.DownloadFileAuth(file.URLPrivateDownload, "Bearer "+b.GetString(tokenConfig)) if err != nil { return fmt.Errorf("download %s failed %#v", file.URLPrivateDownload, err) } @@ -116,7 +130,7 @@ func (b *Bslack) handleDownloadFile(rmsg *config.Message, file *slack.File) erro } // handleUploadFile handles native upload of files -func (b *Bslack) handleUploadFile(msg *config.Message, channelID string) (string, error) { +func (b *Bslack) handleUploadFile(msg *config.Message, channelID string) { for _, f := range msg.Extra["file"] { fi := f.(config.FileInfo) if msg.Text == fi.Comment { @@ -141,37 +155,46 @@ func (b *Bslack) handleUploadFile(msg *config.Message, channelID string) (string b.Log.Errorf("uploadfile %#v", err) } } - return "", nil } // handleMessageEvent handles the message events func (b *Bslack) handleMessageEvent(ev *slack.MessageEvent) (*config.Message, error) { + var err error + // update the userlist on a channel_join - if ev.SubType == "channel_join" { - b.Users, _ = b.sc.GetUsers() + if ev.SubType == sChannelJoin { + if b.users, err = b.sc.GetUsers(); err != nil { + return nil, err + } } // Edit message - if !b.GetBool("EditDisable") && ev.SubMessage != nil && ev.SubMessage.ThreadTimestamp != ev.SubMessage.Timestamp { + if !b.GetBool(editDisableConfig) && ev.SubMessage != nil && ev.SubMessage.ThreadTimestamp != ev.SubMessage.Timestamp { b.Log.Debugf("SubMessage %#v", ev.SubMessage) ev.User = ev.SubMessage.User - ev.Text = ev.SubMessage.Text + b.GetString("EditSuffix") + ev.Text = ev.SubMessage.Text + b.GetString(editSuffixConfig) } // use our own func because rtm.GetChannelInfo doesn't work for private channels - channel, err := b.getChannelByID(ev.Channel) + channelInfo, err := b.getChannelByID(ev.Channel) if err != nil { return nil, err } - rmsg := config.Message{Text: ev.Text, Channel: channel.Name, Account: b.Account, ID: "slack " + ev.Timestamp, Extra: make(map[string][]interface{})} + rmsg := config.Message{ + Text: ev.Text, + Channel: channelInfo.Name, + Account: b.Account, + ID: "slack " + ev.Timestamp, + Extra: map[string][]interface{}{}, + } - if b.UseChannelID { - rmsg.Channel = "ID:" + channel.ID + if b.useChannelID { + rmsg.Channel = "ID:" + channelInfo.ID } // find the user id and name - if ev.User != "" && ev.SubType != messageDeleted && ev.SubType != "file_comment" { + if ev.User != "" && ev.SubType != sMessageDeleted && ev.SubType != sFileComment { user, err := b.rtm.GetUserInfo(ev.User) if err != nil { return nil, err @@ -198,7 +221,7 @@ func (b *Bslack) handleMessageEvent(ev *slack.MessageEvent) (*config.Message, er } // when using webhookURL we can't check if it's our webhook or not for now - if rmsg.Username == "" && ev.BotID != "" && b.GetString("WebhookURL") == "" { + if rmsg.Username == "" && ev.BotID != "" && b.GetString(outgoingWebhookConfig) == "" { bot, err := b.rtm.GetBotInfo(ev.BotID) if err != nil { return nil, err @@ -226,18 +249,18 @@ func (b *Bslack) handleMessageEvent(ev *slack.MessageEvent) (*config.Message, er } // file comments are set by the system (because there is no username given) - if ev.SubType == "file_comment" { - rmsg.Username = "system" + if ev.SubType == sFileComment { + rmsg.Username = sSystemUser } // do we have a /me action - if ev.SubType == "me_message" { + if ev.SubType == sMeMessage { rmsg.Event = config.EVENT_USER_ACTION } // Handle join/leave - if ev.SubType == "channel_leave" || ev.SubType == "channel_join" { - rmsg.Username = "system" + if ev.SubType == sChannelLeave || ev.SubType == sChannelJoin { + rmsg.Username = sSystemUser rmsg.Event = config.EVENT_JOIN_LEAVE } @@ -247,19 +270,19 @@ func (b *Bslack) handleMessageEvent(ev *slack.MessageEvent) (*config.Message, er } // deleted message event - if ev.SubType == messageDeleted { + if ev.SubType == sMessageDeleted { rmsg.Text = config.EVENT_MSG_DELETE rmsg.Event = config.EVENT_MSG_DELETE rmsg.ID = "slack " + ev.DeletedTimestamp } // topic change event - if ev.SubType == "channel_topic" || ev.SubType == "channel_purpose" { + if ev.SubType == sChannelTopic || ev.SubType == sChannelPurpose { rmsg.Event = config.EVENT_TOPIC_CHANGE } // Only deleted messages can have a empty username and text - if (rmsg.Text == "" || rmsg.Username == "") && ev.SubType != messageDeleted && len(ev.Files) == 0 { + if (rmsg.Text == "" || rmsg.Username == "") && ev.SubType != sMessageDeleted && len(ev.Files) == 0 { // this is probably a webhook we couldn't resolve if ev.BotID != "" { return nil, fmt.Errorf("probably an incoming webhook we couldn't resolve (maybe ourselves)") @@ -269,16 +292,14 @@ func (b *Bslack) handleMessageEvent(ev *slack.MessageEvent) (*config.Message, er // save the attachments, so that we can send them to other slack (compatible) bridges if len(ev.Attachments) > 0 { - rmsg.Extra["slack_attachment"] = append(rmsg.Extra["slack_attachment"], ev.Attachments) + rmsg.Extra[sSlackAttachment] = append(rmsg.Extra[sSlackAttachment], ev.Attachments) } // if we have a file attached, download it (in memory) and put a pointer to it in msg.Extra - if len(ev.Files) > 0 { - for _, f := range ev.Files { - err := b.handleDownloadFile(&rmsg, &f) - if err != nil { - b.Log.Errorf("download failed: %s", err) - } + for _, f := range ev.Files { + err := b.handleDownloadFile(&rmsg, &f) + if err != nil { + b.Log.Errorf("download failed: %s", err) } } @@ -287,17 +308,17 @@ func (b *Bslack) handleMessageEvent(ev *slack.MessageEvent) (*config.Message, er // skipMessageEvent skips event that need to be skipped :-) func (b *Bslack) skipMessageEvent(ev *slack.MessageEvent) bool { - if ev.SubType == "channel_leave" || ev.SubType == "channel_join" { - return b.GetBool("nosendjoinpart") + if ev.SubType == sChannelLeave || ev.SubType == sChannelJoin { + return b.GetBool(noSendJoinConfig) } // ignore pinned items - if ev.SubType == "pinned_item" || ev.SubType == "unpinned_item" { + if ev.SubType == sPinnedItem || ev.SubType == sUnpinnedItem { return true } // do not send messages from ourself - if b.GetString("WebhookURL") == "" && b.GetString("WebhookBindAddress") == "" && ev.Username == b.si.User.Name { + if b.GetString(outgoingWebhookConfig) == "" && b.GetString(incomingWebhookConfig) == "" && ev.Username == b.si.User.Name { return true } @@ -308,7 +329,7 @@ func (b *Bslack) skipMessageEvent(ev *slack.MessageEvent) bool { } } - if !b.GetBool("EditDisable") && ev.SubMessage != nil && ev.SubMessage.ThreadTimestamp != ev.SubMessage.Timestamp { + if !b.GetBool(editDisableConfig) && ev.SubMessage != nil && ev.SubMessage.ThreadTimestamp != ev.SubMessage.Timestamp { // it seems ev.SubMessage.Edited == nil when slack unfurls // do not forward these messages #266 if ev.SubMessage.Edited == nil { @@ -316,21 +337,16 @@ func (b *Bslack) skipMessageEvent(ev *slack.MessageEvent) bool { } } - if len(ev.Files) > 0 { - for _, f := range ev.Files { - // if the file is in the cache and isn't older then a minute, skip it - if ts, ok := b.cache.Get("file" + f.ID); ok && time.Since(ts.(time.Time)) < time.Minute { - b.Log.Debugf("Not downloading file id %s which we uploaded", f.ID) - return true - } else { - if ts, ok := b.cache.Get("filename" + f.Name); ok && time.Since(ts.(time.Time)) < time.Second*10 { - b.Log.Debugf("Not downloading file name %s which we uploaded", f.Name) - return true - } else { - b.Log.Debugf("Not skipping %s %s", f.Name, time.Now().String()) - } - } + for _, f := range ev.Files { + // if the file is in the cache and isn't older then a minute, skip it + if ts, ok := b.cache.Get("file" + f.ID); ok && time.Since(ts.(time.Time)) < time.Minute { + b.Log.Debugf("Not downloading file id %s which we uploaded", f.ID) + return true + } else if ts, ok := b.cache.Get("filename" + f.Name); ok && time.Since(ts.(time.Time)) < 10*time.Second { + b.Log.Debugf("Not downloading file name %s which we uploaded", f.Name) + return true } + b.Log.Debugf("Not skipping %s %s", f.Name, time.Now().String()) } return false diff --git a/bridge/slack/helpers.go b/bridge/slack/helpers.go index 273cf7c6..9af5dfac 100644 --- a/bridge/slack/helpers.go +++ b/bridge/slack/helpers.go @@ -8,8 +8,8 @@ import ( "github.com/nlopes/slack" ) -func (b *Bslack) userName(id string) string { - for _, u := range b.Users { +func (b *Bslack) getUsername(id string) string { + for _, u := range b.users { if u.ID == id { if u.Profile.DisplayName != "" { return u.Profile.DisplayName @@ -17,22 +17,26 @@ func (b *Bslack) userName(id string) string { return u.Name } } + b.Log.Warnf("Could not find user with ID '%s'", id) return "" } func (b *Bslack) getAvatar(userid string) string { - var avatar string - if b.Users != nil { - for _, u := range b.Users { - if userid == u.ID { - return u.Profile.Image48 - } + for _, u := range b.users { + if userid == u.ID { + return u.Profile.Image48 } } - return avatar + return "" +} + +func (b *Bslack) getChannel(channel string) (*slack.Channel, error) { + if strings.HasPrefix(channel, "ID:") { + return b.getChannelByID(strings.TrimPrefix(channel, "ID:")) + } + return b.getChannelByName(channel) } -/* func (b *Bslack) getChannelByName(name string) (*slack.Channel, error) { if b.channels == nil { return nil, fmt.Errorf("%s: channel %s not found (no channels found)", b.Account, name) @@ -44,7 +48,6 @@ func (b *Bslack) getChannelByName(name string) (*slack.Channel, error) { } return nil, fmt.Errorf("%s: channel %s not found", b.Account, name) } -*/ func (b *Bslack) getChannelByID(ID string) (*slack.Channel, error) { if b.channels == nil { @@ -58,45 +61,40 @@ func (b *Bslack) getChannelByID(ID string) (*slack.Channel, error) { return nil, fmt.Errorf("%s: channel %s not found", b.Account, ID) } -func (b *Bslack) getChannelID(name string) string { - idcheck := strings.Split(name, "ID:") - if len(idcheck) > 1 { - return idcheck[1] - } - for _, channel := range b.channels { - if channel.Name == name { - return channel.ID - } - } - return "" -} +var ( + mentionRE = regexp.MustCompile(`<@([a-zA-Z0-9]+)>`) + channelRE = regexp.MustCompile(`<#[a-zA-Z0-9]+\|(.+?)>`) + variableRE = regexp.MustCompile(``) + urlRE = regexp.MustCompile(`<(.*?)(\|.*?)?>`) +) // @see https://api.slack.com/docs/message-formatting#linking_to_channels_and_users func (b *Bslack) replaceMention(text string) string { - results := regexp.MustCompile(`<@([a-zA-Z0-9]+)>`).FindAllStringSubmatch(text, -1) - for _, r := range results { - text = strings.Replace(text, "<@"+r[1]+">", "@"+b.userName(r[1]), -1) + replaceFunc := func(match string) string { + userID := strings.Trim(match, "@<>") + if username := b.getUsername(userID); userID != "" { + return "@" + username + } + return match } - return text + return mentionRE.ReplaceAllStringFunc(text, replaceFunc) } // @see https://api.slack.com/docs/message-formatting#linking_to_channels_and_users func (b *Bslack) replaceChannel(text string) string { - results := regexp.MustCompile(`<#[a-zA-Z0-9]+\|(.+?)>`).FindAllStringSubmatch(text, -1) - for _, r := range results { - text = strings.Replace(text, r[0], "#"+r[1], -1) + for _, r := range channelRE.FindAllStringSubmatch(text, -1) { + text = strings.Replace(text, r[0], "#"+r[1], 1) } return text } // @see https://api.slack.com/docs/message-formatting#variables func (b *Bslack) replaceVariable(text string) string { - results := regexp.MustCompile(``).FindAllStringSubmatch(text, -1) - for _, r := range results { + for _, r := range variableRE.FindAllStringSubmatch(text, -1) { if r[2] != "" { - text = strings.Replace(text, r[0], "@"+r[2], -1) + text = strings.Replace(text, r[0], "@"+r[2], 1) } else { - text = strings.Replace(text, r[0], "@"+r[1], -1) + text = strings.Replace(text, r[0], "@"+r[1], 1) } } return text @@ -104,12 +102,11 @@ func (b *Bslack) replaceVariable(text string) string { // @see https://api.slack.com/docs/message-formatting#linking_to_urls func (b *Bslack) replaceURL(text string) string { - results := regexp.MustCompile(`<(.*?)(\|.*?)?>`).FindAllStringSubmatch(text, -1) - for _, r := range results { + for _, r := range urlRE.FindAllStringSubmatch(text, -1) { if len(strings.TrimSpace(r[2])) == 1 { // A display text separator was found, but the text was blank - text = strings.Replace(text, r[0], "", -1) + text = strings.Replace(text, r[0], "", 1) } else { - text = strings.Replace(text, r[0], r[1], -1) + text = strings.Replace(text, r[0], r[1], 1) } } return text diff --git a/bridge/slack/slack.go b/bridge/slack/slack.go index 6aa89375..6524f974 100644 --- a/bridge/slack/slack.go +++ b/bridge/slack/slack.go @@ -2,6 +2,7 @@ package bslack import ( "errors" + "fmt" "strings" "sync" @@ -18,22 +19,52 @@ type Bslack struct { mh *matterhook.Client sc *slack.Client rtm *slack.RTM - Users []slack.User - Usergroups []slack.UserGroup + users []slack.User si *slack.Info channels []slack.Channel cache *lru.Cache - UseChannelID bool + useChannelID bool uuid string *bridge.Config sync.RWMutex } -const messageDeleted = "message_deleted" +const ( + sChannelJoin = "channel_join" + sChannelLeave = "channel_leave" + sMessageDeleted = "message_deleted" + sSlackAttachment = "slack_attachment" + sPinnedItem = "pinned_item" + sUnpinnedItem = "unpinned_item" + sChannelTopic = "channel_topic" + sChannelPurpose = "channel_purpose" + sFileComment = "file_comment" + sMeMessage = "me_message" + sUserTyping = "user_typing" + sLatencyReport = "latency_report" + sSystemUser = "system" + + tokenConfig = "Token" + incomingWebhookConfig = "WebhookBindAddress" + outgoingWebhookConfig = "WebhookURL" + skipTLSConfig = "SkipTLSVerify" + useNickPrefixConfig = "PrefixMessagesWithNick" + editDisableConfig = "EditDisable" + editSuffixConfig = "EditSuffix" + iconURLConfig = "iconurl" + noSendJoinConfig = "nosendjoinpart" +) func New(cfg *bridge.Config) bridge.Bridger { - b := &Bslack{Config: cfg, uuid: xid.New().String()} - b.cache, _ = lru.New(5000) + newCache, err := lru.New(5000) + if err != nil { + cfg.Log.Fatalf("Could not create LRU cache for Slack bridge: %v", err) + } + b := &Bslack{ + Config: cfg, + uuid: xid.New().String(), + cache: newCache, + } return b } @@ -44,50 +75,54 @@ func (b *Bslack) Command(cmd string) string { func (b *Bslack) Connect() error { b.RLock() defer b.RUnlock() - if b.GetString("WebhookBindAddress") != "" { - if b.GetString("WebhookURL") != "" { + if b.GetString(incomingWebhookConfig) != "" { + if b.GetString(outgoingWebhookConfig) != "" { b.Log.Info("Connecting using webhookurl (sending) and webhookbindaddress (receiving)") - b.mh = matterhook.New(b.GetString("WebhookURL"), - matterhook.Config{InsecureSkipVerify: b.GetBool("SkipTLSVerify"), - BindAddress: b.GetString("WebhookBindAddress")}) - } else if b.GetString("Token") != "" { + b.mh = matterhook.New(b.GetString(outgoingWebhookConfig), matterhook.Config{ + InsecureSkipVerify: b.GetBool(skipTLSConfig), + BindAddress: b.GetString(incomingWebhookConfig), + }) + } else if b.GetString(tokenConfig) != "" { b.Log.Info("Connecting using token (sending)") - b.sc = slack.New(b.GetString("Token")) + b.sc = slack.New(b.GetString(tokenConfig)) b.rtm = b.sc.NewRTM() go b.rtm.ManageConnection() b.Log.Info("Connecting using webhookbindaddress (receiving)") - b.mh = matterhook.New(b.GetString("WebhookURL"), - matterhook.Config{InsecureSkipVerify: b.GetBool("SkipTLSVerify"), - BindAddress: b.GetString("WebhookBindAddress")}) + b.mh = matterhook.New(b.GetString(outgoingWebhookConfig), matterhook.Config{ + InsecureSkipVerify: b.GetBool(skipTLSConfig), + BindAddress: b.GetString(incomingWebhookConfig), + }) } else { b.Log.Info("Connecting using webhookbindaddress (receiving)") - b.mh = matterhook.New(b.GetString("WebhookURL"), - matterhook.Config{InsecureSkipVerify: b.GetBool("SkipTLSVerify"), - BindAddress: b.GetString("WebhookBindAddress")}) + b.mh = matterhook.New(b.GetString(outgoingWebhookConfig), matterhook.Config{ + InsecureSkipVerify: b.GetBool(skipTLSConfig), + BindAddress: b.GetString(incomingWebhookConfig), + }) } go b.handleSlack() return nil } - if b.GetString("WebhookURL") != "" { + if b.GetString(outgoingWebhookConfig) != "" { b.Log.Info("Connecting using webhookurl (sending)") - b.mh = matterhook.New(b.GetString("WebhookURL"), - matterhook.Config{InsecureSkipVerify: b.GetBool("SkipTLSVerify"), - DisableServer: true}) - if b.GetString("Token") != "" { + b.mh = matterhook.New(b.GetString(outgoingWebhookConfig), matterhook.Config{ + InsecureSkipVerify: b.GetBool(skipTLSConfig), + DisableServer: true, + }) + if b.GetString(tokenConfig) != "" { b.Log.Info("Connecting using token (receiving)") - b.sc = slack.New(b.GetString("Token")) + b.sc = slack.New(b.GetString(tokenConfig)) b.rtm = b.sc.NewRTM() go b.rtm.ManageConnection() go b.handleSlack() } - } else if b.GetString("Token") != "" { + } else if b.GetString(tokenConfig) != "" { b.Log.Info("Connecting using token (sending and receiving)") - b.sc = slack.New(b.GetString("Token")) + b.sc = slack.New(b.GetString(tokenConfig)) b.rtm = b.sc.NewRTM() go b.rtm.ManageConnection() go b.handleSlack() } - if b.GetString("WebhookBindAddress") == "" && b.GetString("WebhookURL") == "" && b.GetString("Token") == "" { + if b.GetString(incomingWebhookConfig) == "" && b.GetString(outgoingWebhookConfig) == "" && b.GetString(tokenConfig) == "" { return errors.New("no connection method found. See that you have WebhookBindAddress, WebhookURL or Token configured") } return nil @@ -101,7 +136,7 @@ func (b *Bslack) JoinChannel(channel config.ChannelInfo) error { // use ID:channelid and resolve it to the actual name idcheck := strings.Split(channel.Name, "ID:") if len(idcheck) > 1 { - b.UseChannelID = true + b.useChannelID = true ch, err := b.sc.GetChannelInfo(idcheck[1]) if err != nil { return err @@ -114,7 +149,7 @@ func (b *Bslack) JoinChannel(channel config.ChannelInfo) error { // we can only join channels using the API if b.sc != nil { - if strings.HasPrefix(b.GetString("Token"), "xoxb") { + if strings.HasPrefix(b.GetString(tokenConfig), "xoxb") { // TODO check if bot has already joined channel return nil } @@ -141,11 +176,14 @@ func (b *Bslack) Send(msg config.Message) (string, error) { } // Use webhook to send the message - if b.GetString("WebhookURL") != "" { + if b.GetString(outgoingWebhookConfig) != "" { return b.sendWebhook(msg) } - channelID := b.getChannelID(msg.Channel) + channelInfo, err := b.getChannel(msg.Channel) + if err != nil { + return "", fmt.Errorf("could not send message: %v", err) + } // Delete message if msg.Event == config.EVENT_MSG_DELETE { @@ -155,7 +193,7 @@ func (b *Bslack) Send(msg config.Message) (string, error) { } // we get a "slack ", split it ts := strings.Fields(msg.ID) - _, _, err := b.sc.DeleteMessage(channelID, ts[1]) + _, _, err = b.sc.DeleteMessage(channelInfo.ID, ts[1]) if err != nil { return msg.ID, err } @@ -163,14 +201,14 @@ func (b *Bslack) Send(msg config.Message) (string, error) { } // Prepend nick if configured - if b.GetBool("PrefixMessagesWithNick") { + if b.GetBool(useNickPrefixConfig) { msg.Text = msg.Username + msg.Text } // Edit message if we have an ID if msg.ID != "" { ts := strings.Fields(msg.ID) - _, _, _, err := b.sc.UpdateMessage(channelID, ts[1], msg.Text) + _, _, _, err = b.sc.UpdateMessage(channelInfo.ID, ts[1], msg.Text) if err != nil { return msg.ID, err } @@ -179,12 +217,12 @@ func (b *Bslack) Send(msg config.Message) (string, error) { // create slack new post parameters np := slack.NewPostMessageParameters() - if b.GetBool("PrefixMessagesWithNick") { + if b.GetBool(useNickPrefixConfig) { np.AsUser = true } np.Username = msg.Username np.LinkNames = 1 // replace mentions - np.IconURL = config.GetIconURL(&msg, b.GetString("iconurl")) + np.IconURL = config.GetIconURL(&msg, b.GetString(iconURLConfig)) if msg.Avatar != "" { np.IconURL = msg.Avatar } @@ -193,8 +231,8 @@ func (b *Bslack) Send(msg config.Message) (string, error) { // add file attachments np.Attachments = append(np.Attachments, b.createAttach(msg.Extra)...) // add slack attachments (from another slack bridge) - if len(msg.Extra["slack_attachment"]) > 0 { - for _, attach := range msg.Extra["slack_attachment"] { + if msg.Extra != nil { + for _, attach := range msg.Extra[sSlackAttachment] { np.Attachments = append(np.Attachments, attach.([]slack.Attachment)...) } } @@ -202,16 +240,17 @@ func (b *Bslack) Send(msg config.Message) (string, error) { // Upload a file if it exists if msg.Extra != nil { for _, rmsg := range helper.HandleExtra(&msg, b.General) { - b.sc.PostMessage(channelID, rmsg.Username+rmsg.Text, np) - } - // check if we have files to upload (from slack, telegram or mattermost) - if len(msg.Extra["file"]) > 0 { - b.handleUploadFile(&msg, channelID) + _, _, err = b.sc.PostMessage(channelInfo.ID, rmsg.Username+rmsg.Text, np) + if err != nil { + b.Log.Error(err) + } } + // Upload files if necessary (from Slack, Telegram or Mattermost). + b.handleUploadFile(&msg, channelInfo.ID) } // Post normal message - _, id, err := b.sc.PostMessage(channelID, msg.Text, np) + _, id, err := b.sc.PostMessage(channelInfo.ID, msg.Text, np) if err != nil { return "", err } @@ -223,26 +262,36 @@ func (b *Bslack) Reload(cfg *bridge.Config) (string, error) { } func (b *Bslack) createAttach(extra map[string][]interface{}) []slack.Attachment { - var attachs []slack.Attachment + var attachements []slack.Attachment for _, v := range extra["attachments"] { entry := v.(map[string]interface{}) - s := slack.Attachment{} - s.Fallback = entry["fallback"].(string) - s.Color = entry["color"].(string) - s.Pretext = entry["pretext"].(string) - s.AuthorName = entry["author_name"].(string) - s.AuthorLink = entry["author_link"].(string) - s.AuthorIcon = entry["author_icon"].(string) - s.Title = entry["title"].(string) - s.TitleLink = entry["title_link"].(string) - s.Text = entry["text"].(string) - s.ImageURL = entry["image_url"].(string) - s.ThumbURL = entry["thumb_url"].(string) - s.Footer = entry["footer"].(string) - s.FooterIcon = entry["footer_icon"].(string) - attachs = append(attachs, s) + s := slack.Attachment{ + Fallback: extractStringField(entry, "fallback"), + Color: extractStringField(entry, "color"), + Pretext: extractStringField(entry, "pretext"), + AuthorName: extractStringField(entry, "author_name"), + AuthorLink: extractStringField(entry, "author_link"), + AuthorIcon: extractStringField(entry, "author_icon"), + Title: extractStringField(entry, "title"), + TitleLink: extractStringField(entry, "title_link"), + Text: extractStringField(entry, "text"), + ImageURL: extractStringField(entry, "image_url"), + ThumbURL: extractStringField(entry, "thumb_url"), + Footer: extractStringField(entry, "footer"), + FooterIcon: extractStringField(entry, "footer_icon"), + } + attachements = append(attachements, s) + } + return attachements +} + +func extractStringField(data map[string]interface{}, field string) string { + if rawValue, found := data[field]; found { + if value, ok := rawValue.(string); ok { + return value + } } - return attachs + return "" } // sendWebhook uses the configured WebhookURL to send the message @@ -252,39 +301,48 @@ func (b *Bslack) sendWebhook(msg config.Message) (string, error) { return "", nil } - if b.GetBool("PrefixMessagesWithNick") { + if b.GetBool(useNickPrefixConfig) { msg.Text = msg.Username + msg.Text } if msg.Extra != nil { // this sends a message only if we received a config.EVENT_FILE_FAILURE_SIZE for _, rmsg := range helper.HandleExtra(&msg, b.General) { - iconURL := config.GetIconURL(&rmsg, b.GetString("iconurl")) - matterMessage := matterhook.OMessage{IconURL: iconURL, Channel: msg.Channel, UserName: rmsg.Username, Text: rmsg.Text} - b.mh.Send(matterMessage) + iconURL := config.GetIconURL(&rmsg, b.GetString(iconURLConfig)) + matterMessage := matterhook.OMessage{ + IconURL: iconURL, + Channel: msg.Channel, + UserName: rmsg.Username, + Text: rmsg.Text, + } + if err := b.mh.Send(matterMessage); err != nil { + b.Log.Errorf("Failed to send message: %v", err) + } } // webhook doesn't support file uploads, so we add the url manually - if len(msg.Extra["file"]) > 0 { - for _, f := range msg.Extra["file"] { - fi := f.(config.FileInfo) - if fi.URL != "" { - msg.Text += " " + fi.URL - } + for _, f := range msg.Extra["file"] { + fi := f.(config.FileInfo) + if fi.URL != "" { + msg.Text += " " + fi.URL } } } // if we have native slack_attachments add them var attachs []slack.Attachment - if len(msg.Extra["slack_attachment"]) > 0 { - for _, attach := range msg.Extra["slack_attachment"] { - attachs = append(attachs, attach.([]slack.Attachment)...) - } + for _, attach := range msg.Extra[sSlackAttachment] { + attachs = append(attachs, attach.([]slack.Attachment)...) } - iconURL := config.GetIconURL(&msg, b.GetString("iconurl")) - matterMessage := matterhook.OMessage{IconURL: iconURL, Attachments: attachs, Channel: msg.Channel, UserName: msg.Username, Text: msg.Text} + iconURL := config.GetIconURL(&msg, b.GetString(iconURLConfig)) + matterMessage := matterhook.OMessage{ + IconURL: iconURL, + Attachments: attachs, + Channel: msg.Channel, + UserName: msg.Username, + Text: msg.Text, + } if msg.Avatar != "" { matterMessage.IconURL = msg.Avatar } -- cgit v1.2.3