commit 3a9d99f462c7f21eaffa09cd9be689040e1392b2
Author: Duco van Amstel <duco.vanamstel@gmail.com>
Date: Wed Nov 07 20:35:59 2018 +0000
Refactor and clean-up handlers. (slack) (#533)
diff --git a/bridge/helper/helper.go b/bridge/helper/helper.go
index a13e02a..bfa7d82 100644
--- a/bridge/helper/helper.go
+++ b/bridge/helper/helper.go
@@ -5613 +5610 @@ func SplitStringLength(input string, length int) string {
func HandleExtra(msg *config.Message, general *config.Protocol) []config.Message {
extra := msg.Extra
rmsg := []config.Message{}
- if len(extra[config.EVENT_FILE_FAILURE_SIZE]) > 0 {
- for _, f := range extra[config.EVENT_FILE_FAILURE_SIZE] {
- 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})
- }
- return rmsg
+ for _, f := range extra[config.EVENT_FILE_FAILURE_SIZE] {
+ 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})
}
return rmsg
}
diff --git a/bridge/slack/handlers.go b/bridge/slack/handlers.go
index d6894bf..9bcafde 100644
--- a/bridge/slack/handlers.go
+++ b/bridge/slack/handlers.go
@@ -17 +16 @@
package bslack
import (
- "bytes"
"fmt"
"html"
"regexp"
@@ -617 +6015 @@ func (b *Bslack) handleSlackClient(messages chan *config.Message) {
case *slack.OutgoingErrorEvent:
b.Log.Debugf("%#v", ev.Error())
case *slack.ChannelJoinedEvent:
+ // When we join a channel we update the full list of users as
+ // well as the information for the channel that we joined as this
+ // should now tell that we are a member of it.
b.populateUsers()
+
+ b.channelsMutex.Lock()
+ b.channelsByID[ev.Channel.ID] = &ev.Channel
+ b.channelsByName[ev.Channel.Name] = &ev.Channel
+ b.channelsMutex.Unlock()
case *slack.ConnectedEvent:
b.si = ev.Info
b.populateChannels()
@@ -90249 +97177 @@ func (b *Bslack) handleMatterHook(messages chan *config.Message) {
}
}
-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 := commentRE.FindAllStringSubmatch(rmsg.Text, -1)
- if len(results) > 0 {
- comment = results[0][1]
- }
- err := helper.HandleDownloadSize(b.Log, rmsg, file.Name, int64(file.Size), b.General)
- if err != nil {
- return err
+// skipMessageEvent skips event that need to be skipped :-)
+func (b *Bslack) skipMessageEvent(ev *slack.MessageEvent) bool {
+ switch ev.SubType {
+ case sChannelLeave, sChannelJoin:
+ return b.GetBool(noSendJoinConfig)
+ case sPinnedItem, sUnpinnedItem:
+ return true
}
- // actually download the file
- data, err := helper.DownloadFileAuth(file.URLPrivateDownload, "Bearer "+b.GetString(tokenConfig))
- if err != nil {
- return fmt.Errorf("download %s failed %#v", file.URLPrivateDownload, err)
+
+ // Skip any messages that we made ourselves or from 'slackbot' (see #527).
+ if ev.Username == sSlackBotUser ||
+ (b.rtm != nil && ev.Username == b.si.User.Name) ||
+ (len(ev.Attachments) > 0 && ev.Attachments[0].CallbackID == "matterbridge_"+b.uuid) {
+ return true
}
- // add the downloaded data to the message
- helper.HandleDownloadData(b.Log, rmsg, file.Name, comment, file.URLPrivateDownload, data, b.General)
- return nil
-}
-// handleUploadFile handles native upload of files
-func (b *Bslack) handleUploadFile(msg *config.Message, channelID string) {
- for _, f := range msg.Extra["file"] {
- fi := f.(config.FileInfo)
- if msg.Text == fi.Comment {
- msg.Text = ""
- }
- /* because the result of the UploadFile is slower than the MessageEvent from slack
- we can't match on the file ID yet, so we have to match on the filename too
- */
- b.Log.Debugf("Adding file %s to cache %s", fi.Name, time.Now().String())
- b.cache.Add("filename"+fi.Name, time.Now())
- res, err := b.sc.UploadFile(slack.FileUploadParameters{
- Reader: bytes.NewReader(*fi.Data),
- Filename: fi.Name,
- Channels: []string{channelID},
- InitialComment: fi.Comment,
- })
- if res.ID != "" {
- b.Log.Debugf("Adding fileid %s to cache %s", res.ID, time.Now().String())
- b.cache.Add("file"+res.ID, time.Now())
- }
- if err != nil {
- b.Log.Errorf("uploadfile %#v", err)
- }
+ // It seems ev.SubMessage.Edited == nil when slack unfurls.
+ // Do not forward these messages. See Github issue #266.
+ if ev.SubMessage != nil &&
+ ev.SubMessage.ThreadTimestamp != ev.SubMessage.Timestamp &&
+ ev.SubMessage.Edited == nil {
+ return true
}
+ return false
}
-// handleMessageEvent handles the message events
+// handleMessageEvent handles the message events. Together with any called sub-methods,
+// this method implements the following event processing pipeline:
+//
+// 1. Check if the message should be ignored.
+// NOTE: This is not actually part of the method below but is done just before it
+// is called via the 'skipMessageEvent()' method.
+// 2. Populate the Matterbridge message that will be sent to the router based on the
+// received event and logic that is common to all events that are not skipped.
+// 3. Detect and handle any message that is "status" related (think join channel, etc.).
+// This might result in an early exit from the pipeline and passing of the
+// pre-populated message to the Matterbridge router.
+// 4. Handle the specific case of messages that edit existing messages depending on
+// configuration.
+// 5. Handle any attachments of the received event.
+// 6. Check that the Matterbridge message that we end up with after at the end of the
+// pipeline is valid before sending it to the Matterbridge router.
func (b *Bslack) handleMessageEvent(ev *slack.MessageEvent) (*config.Message, error) {
- var err error
-
- // update the userlist on a channel_join
- if ev.SubType == sChannelJoin {
- b.populateUsers()
- }
-
- // Edit message
- 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(editSuffixConfig)
- }
-
- // use our own func because rtm.GetChannelInfo doesn't work for private channels
- channelInfo, err := b.getChannelByID(ev.Channel)
+ rmsg, err := b.populateReceivedMessage(ev)
if err != nil {
return nil, err
}
- rmsg := config.Message{
- Text: ev.Text,
- Channel: channelInfo.Name,
- Account: b.Account,
- ID: "slack " + ev.Timestamp,
- Extra: map[string][]interface{}{},
- ParentID: ev.ThreadTimestamp,
- }
-
- if b.useChannelID {
- rmsg.Channel = "ID:" + channelInfo.ID
+ // Handle some message types early.
+ if b.handleStatusEvent(ev, rmsg) {
+ return rmsg, nil
}
- // find the user id and name
- if ev.User != "" && ev.SubType != sMessageDeleted && ev.SubType != sFileComment {
- user, err := b.rtm.GetUserInfo(ev.User)
- if err != nil {
- return nil, err
- }
- rmsg.UserID = user.ID
- rmsg.Username = user.Name
- if user.Profile.DisplayName != "" {
- rmsg.Username = user.Profile.DisplayName
- }
- }
-
- // See if we have some text in the attachments
- if rmsg.Text == "" {
- for _, attach := range ev.Attachments {
- if attach.Text != "" {
- if attach.Title != "" {
- rmsg.Text = attach.Title + "\n"
- }
- rmsg.Text += attach.Text
- } else {
- rmsg.Text = attach.Fallback
- }
+ // Handle 'edit' messages.
+ if ev.SubMessage != nil && !b.GetBool(editDisableConfig) {
+ rmsg.ID = "slack " + ev.SubMessage.Timestamp
+ if ev.SubMessage.ThreadTimestamp != ev.SubMessage.Timestamp {
+ b.Log.Debugf("SubMessage %#v", ev.SubMessage)
+ rmsg.Username = ev.SubMessage.User
+ rmsg.Text = ev.SubMessage.Text + b.GetString(editSuffixConfig)
}
}
- // when using webhookURL we can't check if it's our webhook or not for now
- if rmsg.Username == "" && ev.BotID != "" && b.GetString(outgoingWebhookConfig) == "" {
- bot, err := b.rtm.GetBotInfo(ev.BotID)
- if err != nil {
- return nil, err
- }
- if bot.Name != "" {
- rmsg.Username = bot.Name
- if ev.Username != "" {
- rmsg.Username = ev.Username
- }
- rmsg.UserID = bot.ID
- }
+ b.handleAttachments(ev, rmsg)
- // fixes issues with matterircd users
- if bot.Name == "Slack API Tester" {
- user, err := b.rtm.GetUserInfo(ev.User)
- if err != nil {
- return nil, err
- }
- rmsg.UserID = user.ID
- rmsg.Username = user.Name
- if user.Profile.DisplayName != "" {
- rmsg.Username = user.Profile.DisplayName
- }
+ // Verify that we have the right information and the message
+ // is well-formed before sending it out to the router.
+ if len(ev.Files) == 0 && (rmsg.Text == "" || rmsg.Username == "") {
+ if ev.BotID != "" {
+ // This is probably a webhook we couldn't resolve.
+ return nil, fmt.Errorf("message handling resulted in an empty bot message (probably an incoming webhook we couldn't resolve): %#v", ev)
}
+ return nil, fmt.Errorf("message handling resulted in an empty message: %#v", ev)
}
+ return rmsg, nil
+}
- // file comments are set by the system (because there is no username given)
- if ev.SubType == sFileComment {
- rmsg.Username = sSystemUser
- }
-
- // do we have a /me action
- if ev.SubType == sMeMessage {
- rmsg.Event = config.EVENT_USER_ACTION
- }
-
- // Handle join/leave
- if ev.SubType == sChannelLeave || ev.SubType == sChannelJoin {
+func (b *Bslack) handleStatusEvent(ev *slack.MessageEvent, rmsg *config.Message) bool {
+ switch ev.SubType {
+ case sChannelJoined, sMemberJoined:
+ b.populateUsers()
+ // There's no further processing needed on channel events
+ // so we return 'true'.
+ return true
+ case sChannelJoin, sChannelLeave:
rmsg.Username = sSystemUser
rmsg.Event = config.EVENT_JOIN_LEAVE
- }
-
- // edited messages have a submessage, use this timestamp
- if ev.SubMessage != nil {
- rmsg.ID = "slack " + ev.SubMessage.Timestamp
- }
-
- // deleted message event
- if ev.SubType == sMessageDeleted {
+ case sChannelTopic, sChannelPurpose:
+ rmsg.Event = config.EVENT_TOPIC_CHANGE
+ case sMessageDeleted:
rmsg.Text = config.EVENT_MSG_DELETE
rmsg.Event = config.EVENT_MSG_DELETE
rmsg.ID = "slack " + ev.DeletedTimestamp
+ // If a message is being deleted we do not need to process
+ // the event any further so we return 'true'.
+ return true
+ case sMeMessage:
+ rmsg.Event = config.EVENT_USER_ACTION
}
+ return false
+}
- // topic change event
- if ev.SubType == sChannelTopic || ev.SubType == sChannelPurpose {
- rmsg.Event = config.EVENT_TOPIC_CHANGE
+func (b *Bslack) handleAttachments(ev *slack.MessageEvent, rmsg *config.Message) {
+ // File comments are set by the system (because there is no username given).
+ if ev.SubType == sFileComment {
+ rmsg.Username = sSystemUser
}
- // Only deleted messages can have a empty username and text
- 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)")
+ // See if we have some text in the attachments.
+ if rmsg.Text == "" {
+ for _, attach := range ev.Attachments {
+ if attach.Text != "" {
+ if attach.Title != "" {
+ rmsg.Text = attach.Title + "\n"
+ }
+ rmsg.Text += attach.Text
+ } else {
+ rmsg.Text = attach.Fallback
+ }
}
- return nil, fmt.Errorf("empty message and not a deleted message")
}
- // save the attachments, so that we can send them to other slack (compatible) bridges
+ // Save the attachments, so that we can send them to other slack (compatible) bridges.
if len(ev.Attachments) > 0 {
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 we have files attached, download them (in memory) and put a pointer to it in msg.Extra.
for _, f := range ev.Files {
- err := b.handleDownloadFile(&rmsg, &f)
+ err := b.handleDownloadFile(rmsg, &f)
if err != nil {
- b.Log.Errorf("download failed: %s", err)
+ b.Log.Errorf("Could not download incoming file: %#v", err)
}
}
-
- return &rmsg, nil
}
-// skipMessageEvent skips event that need to be skipped :-)
-func (b *Bslack) skipMessageEvent(ev *slack.MessageEvent) bool {
- if ev.SubType == sChannelLeave || ev.SubType == sChannelJoin {
- return b.GetBool(noSendJoinConfig)
- }
+var commentRE = regexp.MustCompile(`.*?commented: (.*)`)
- // ignore pinned items
- if ev.SubType == sPinnedItem || ev.SubType == sUnpinnedItem {
- return true
+// handleDownloadFile handles file download
+func (b *Bslack) handleDownloadFile(rmsg *config.Message, file *slack.File) error {
+ if b.fileIsAvailable(file) {
+ return nil
}
- // do not send messages from ourself
- if b.GetString(outgoingWebhookConfig) == "" && b.GetString(incomingWebhookConfig) == "" && ev.Username == b.si.User.Name {
- return true
+ // Check that the file is neither too large nor blacklisted.
+ if err := helper.HandleDownloadSize(b.Log, rmsg, file.Name, int64(file.Size), b.General); err != nil {
+ b.Log.WithError(err).Infof("Skipping download of incoming file.")
+ return nil
}
- // skip messages we made ourselves
- if len(ev.Attachments) > 0 {
- if ev.Attachments[0].CallbackID == "matterbridge_"+b.uuid {
- return true
- }
+ // Actually download the file.
+ data, err := helper.DownloadFileAuth(file.URLPrivateDownload, "Bearer "+b.GetString(tokenConfig))
+ if err != nil {
+ return fmt.Errorf("download %s failed %#v", file.URLPrivateDownload, err)
}
- 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 {
- return true
- }
+ // Add the downloaded data to the message.
+ var comment string
+ if results := commentRE.FindAllStringSubmatch(rmsg.Text, -1); len(results) > 0 {
+ comment = results[0][1]
}
+ helper.HandleDownloadData(b.Log, rmsg, file.Name, comment, file.URLPrivateDownload, data, b.General)
+ return nil
+}
- 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())
+func (b *Bslack) fileIsAvailable(file *slack.File) bool {
+ // Only download a file if it is not in the cache or if it has been entered more than a minute ago.
+ if ts, ok := b.cache.Get("file" + file.ID); ok && time.Since(ts.(time.Time)) > time.Minute {
+ return true
+ } else if ts, ok = b.cache.Get("filename" + file.Name); ok && time.Since(ts.(time.Time)) > 10*time.Second {
+ return true
}
-
return false
}
diff --git a/bridge/slack/helpers.go b/bridge/slack/helpers.go
index 23d5816..e46a71a 100644
--- a/bridge/slack/helpers.go
+++ b/bridge/slack/helpers.go
@@ -627 +631 @@ import (
"strings"
"time"
+ "github.com/42wim/matterbridge/bridge/config"
"github.com/nlopes/slack"
)
+func (b *Bslack) getUser(id string) *slack.User {
+ b.usersMutex.RLock()
+ defer b.usersMutex.RUnlock()
+
+ return b.users[id]
+}
+
func (b *Bslack) getUsername(id string) string {
- for _, u := range b.users {
- if u.ID == id {
- if u.Profile.DisplayName != "" {
- return u.Profile.DisplayName
- }
- return u.Name
+ if user := b.getUser(id); user != nil {
+ if user.Profile.DisplayName != "" {
+ return user.Profile.DisplayName
}
+ return user.Name
}
b.Log.Warnf("Could not find user with ID '%s'", id)
return ""
}
-func (b *Bslack) getAvatar(userid string) string {
- for _, u := range b.users {
- if userid == u.ID {
- return u.Profile.Image48
- }
+func (b *Bslack) getAvatar(id string) string {
+ if user := b.getUser(id); user != nil {
+ return user.Profile.Image48
}
return ""
}
@@ -1426 +14666 @@ func (b *Bslack) populateChannels() {
b.refreshInProgress = false
}
+// populateReceivedMessage shapes the initial Matterbridge message that we will forward to the
+// router before we apply message-dependent modifications.
+func (b *Bslack) populateReceivedMessage(ev *slack.MessageEvent) (*config.Message, error) {
+ // Use our own func because rtm.GetChannelInfo doesn't work for private channels.
+ channel, 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{}),
+ ParentID: ev.ThreadTimestamp,
+ }
+ if b.useChannelID {
+ rmsg.Channel = "ID:" + channel.ID
+ }
+
+ if err = b.populateMessageWithUserInfo(ev, rmsg); err != nil {
+ return nil, err
+ }
+ return rmsg, err
+}
+
+func (b *Bslack) populateMessageWithUserInfo(ev *slack.MessageEvent, rmsg *config.Message) error {
+ if ev.SubType == sMessageDeleted || ev.SubType == sFileComment {
+ return nil
+ }
+
+ if ev.BotID != "" && b.GetString(outgoingWebhookConfig) == "" {
+ bot, err := b.rtm.GetBotInfo(ev.BotID)
+ if err != nil {
+ return err
+ }
+ if bot.Name != "" && bot.Name != "Slack API Tester" {
+ rmsg.Username = bot.Name
+ if ev.Username != "" {
+ rmsg.Username = ev.Username
+ }
+ rmsg.UserID = bot.ID
+ }
+ }
+
+ if ev.User != "" {
+ user := b.getUser(ev.User)
+ if user == nil {
+ return fmt.Errorf("could not find information for user with id %s", ev.User)
+ }
+ rmsg.UserID = user.ID
+ rmsg.Username = user.Name
+ if user.Profile.DisplayName != "" {
+ rmsg.Username = user.Profile.DisplayName
+ }
+ }
+ return nil
+}
+
var (
mentionRE = regexp.MustCompile(`<@([a-zA-Z0-9]+)>`)
channelRE = regexp.MustCompile(`<#[a-zA-Z0-9]+\|(.+?)>`)
diff --git a/bridge/slack/slack.go b/bridge/slack/slack.go
index c1386a4..8c96837 100644
--- a/bridge/slack/slack.go
+++ b/bridge/slack/slack.go
@@ -16 +17 @@
package bslack
import (
+ "bytes"
"errors"
"fmt"
"strings"
@@ -456 +468 @@ type Bslack struct {
const (
sChannelJoin = "channel_join"
sChannelLeave = "channel_leave"
+ sChannelJoined = "channel_joined"
+ sMemberJoined = "member_joined_channel"
sMessageDeleted = "message_deleted"
sSlackAttachment = "slack_attachment"
sPinnedItem = "pinned_item"
@@ -566 +597 @@ const (
sUserTyping = "user_typing"
sLatencyReport = "latency_report"
sSystemUser = "system"
+ sSlackBotUser = "slackbot"
tokenConfig = "Token"
incomingWebhookConfig = "WebhookBindAddress"
@@ -2957 +2997 @@ func (b *Bslack) sendRTM(msg config.Message) (string, error) {
messageParameters := b.prepareMessageParameters(&msg)
- // Upload a file if it exists
+ // Upload a file if it exists.
if msg.Extra != nil {
for _, rmsg := range helper.HandleExtra(&msg, b.General) {
_, _, err = b.rtm.PostMessage(channelInfo.ID, rmsg.Username+rmsg.Text, *messageParameters)
@@ -3156 +31939 @@ func (b *Bslack) sendRTM(msg config.Message) (string, error) {
return "slack " + id, nil
}
+// handleUploadFile handles native upload of files
+func (b *Bslack) handleUploadFile(msg *config.Message, channelID string) {
+ for _, f := range msg.Extra["file"] {
+ fi := f.(config.FileInfo)
+ if msg.Text == fi.Comment {
+ msg.Text = ""
+ }
+ // Because the result of the UploadFile is slower than the MessageEvent from slack
+ // we can't match on the file ID yet, so we have to match on the filename too.
+ ts := time.Now()
+ b.Log.Debugf("Adding file %s to cache at %s with timestamp", fi.Name, ts.String())
+ if !b.cache.Add("filename"+fi.Name, ts) {
+ b.Log.Warnf("Failed to add file %s to cache at %s with timestamp", fi.Name, ts.String())
+ }
+ res, err := b.sc.UploadFile(slack.FileUploadParameters{
+ Reader: bytes.NewReader(*fi.Data),
+ Filename: fi.Name,
+ Channels: []string{channelID},
+ InitialComment: fi.Comment,
+ })
+ if err != nil {
+ b.Log.Errorf("uploadfile %#v", err)
+ return
+ }
+ if res.ID != "" {
+ b.Log.Debugf("Adding file ID %s to cache with timestamp %s", res.ID, ts.String())
+ if !b.cache.Add("file"+res.ID, ts) {
+ b.Log.Warnf("Failed to add file ID %s to cache with timestamp %s", res.ID, ts.String())
+ }
+ }
+ }
+}
+
func (b *Bslack) prepareMessageParameters(msg *config.Message) *slack.PostMessageParameters {
params := slack.NewPostMessageParameters()
if b.GetBool(useNickPrefixConfig) {
