Clean up user and channel information management (slack) (#521)

3 files changed, 99 insertions, 72 deletions

diff --git a/bridge/slack/handlers.go b/bridge/slack/handlers.go
index 56fd7026..b6400a80 100644
--- a/bridge/slack/handlers.go
+++ b/bridge/slack/handlers.go
@@ -61,25 +61,11 @@ func (b *Bslack) handleSlackClient(messages chan *config.Message) {
 		case *slack.OutgoingErrorEvent:
 			b.Log.Debugf("%#v", ev.Error())
 		case *slack.ChannelJoinedEvent:
-			var err error
-			b.users, err = b.sc.GetUsers()
-			if err != nil {
-				b.Log.Errorf("Could not reload users: %#v", err)
-			}
+			b.populateUsers()
 		case *slack.ConnectedEvent:
-			var err error
-			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, err = b.sc.GetUsers()
-			if err != nil {
-				b.Log.Errorf("Could not reload users: %#v", err)
-			}
+			b.populateChannels()
+			b.populateUsers()
 		case *slack.InvalidAuthEvent:
 			b.Log.Fatalf("Invalid Token %#v", ev)
 		case *slack.ConnectionErrorEvent:
@@ -163,9 +149,7 @@ func (b *Bslack) handleMessageEvent(ev *slack.MessageEvent) (*config.Message, er
 
 	// update the userlist on a channel_join
 	if ev.SubType == sChannelJoin {
-		if b.users, err = b.sc.GetUsers(); err != nil {
-			return nil, err
-		}
+		b.populateUsers()
 	}
 
 	// Edit message
diff --git a/bridge/slack/helpers.go b/bridge/slack/helpers.go
index 9af5dfac..18dded3b 100644
--- a/bridge/slack/helpers.go
+++ b/bridge/slack/helpers.go
@@ -38,27 +38,72 @@ func (b *Bslack) getChannel(channel string) (*slack.Channel, error) {
 }
 
 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)
-	}
-	for _, channel := range b.channels {
-		if channel.Name == name {
-			return &channel, nil
-		}
+	b.channelsMutex.RLock()
+	defer b.channelsMutex.RUnlock()
+
+	if channel, ok := b.channelsByName[name]; ok {
+		return channel, nil
 	}
 	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 {
-		return nil, fmt.Errorf("%s: channel %s not found (no channels found)", b.Account, ID)
+	b.channelsMutex.RLock()
+	defer b.channelsMutex.RUnlock()
+
+	if channel, ok := b.channelsByID[ID]; ok {
+		return channel, nil
+	}
+	return nil, fmt.Errorf("%s: channel %s not found", b.Account, ID)
+}
+
+func (b *Bslack) populateUsers() {
+	users, err := b.sc.GetUsers()
+	if err != nil {
+		b.Log.Errorf("Could not reload users: %#v", err)
+		return
+	}
+
+	newUsers := map[string]*slack.User{}
+	for _, user := range users {
+		newUsers[user.ID] = &user
 	}
-	for _, channel := range b.channels {
-		if channel.ID == ID {
-			return &channel, nil
+
+	b.usersMutex.Lock()
+	defer b.usersMutex.Unlock()
+	b.users = newUsers
+}
+
+func (b *Bslack) populateChannels() {
+	newChannelsByID := map[string]*slack.Channel{}
+	newChannelsByName := map[string]*slack.Channel{}
+
+	// We only retrieve public and private channels, not IMs
+	// and MPIMs as those do not have a channel name.
+	queryParams := &slack.GetConversationsParameters{
+		ExcludeArchived: "true",
+		Types:           []string{"public_channel,private_channel"},
+	}
+	for {
+		channels, nextCursor, err := b.sc.GetConversations(queryParams)
+		if err != nil {
+			b.Log.Errorf("Could not reload channels: %#v", err)
+			return
 		}
+		for i := 0; i < len(channels); i++ {
+			newChannelsByID[channels[i].ID] = &channels[i]
+			newChannelsByName[channels[i].Name] = &channels[i]
+		}
+		if nextCursor == "" {
+			break
+		}
+		queryParams.Cursor = nextCursor
 	}
-	return nil, fmt.Errorf("%s: channel %s not found", b.Account, ID)
+
+	b.channelsMutex.Lock()
+	defer b.channelsMutex.Unlock()
+	b.channelsByID = newChannelsByID
+	b.channelsByName = newChannelsByName
 }
 
 var (
diff --git a/bridge/slack/slack.go b/bridge/slack/slack.go
index 6524f974..6585c9fc 100644
--- a/bridge/slack/slack.go
+++ b/bridge/slack/slack.go
@@ -16,17 +16,24 @@ import (
 )
 
 type Bslack struct {
-	mh           *matterhook.Client
-	sc           *slack.Client
-	rtm          *slack.RTM
-	users        []slack.User
-	si           *slack.Info
-	channels     []slack.Channel
+	sync.RWMutex
+	*bridge.Config
+
+	mh  *matterhook.Client
+	sc  *slack.Client
+	rtm *slack.RTM
+	si  *slack.Info
+
 	cache        *lru.Cache
-	useChannelID bool
 	uuid         string
-	*bridge.Config
-	sync.RWMutex
+	useChannelID bool
+
+	users      map[string]*slack.User
+	usersMutex sync.RWMutex
+
+	channelsByID   map[string]*slack.Channel
+	channelsByName map[string]*slack.Channel
+	channelsMutex  sync.RWMutex
 }
 
 const (
@@ -61,9 +68,12 @@ func New(cfg *bridge.Config) bridge.Bridger {
 		cfg.Log.Fatalf("Could not create LRU cache for Slack bridge: %v", err)
 	}
 	b := &Bslack{
-		Config: cfg,
-		uuid:   xid.New().String(),
-		cache:  newCache,
+		Config:         cfg,
+		uuid:           xid.New().String(),
+		cache:          newCache,
+		users:          map[string]*slack.User{},
+		channelsByID:   map[string]*slack.Channel{},
+		channelsByName: map[string]*slack.Channel{},
 	}
 	return b
 }
@@ -132,37 +142,25 @@ func (b *Bslack) Disconnect() error {
 	return b.rtm.Disconnect()
 }
 
+// JoinChannel only acts as a verification method that checks whether Matterbridge's
+// Slack integration is already member of the channel. This is because Slack does not
+// allow apps or bots to join channels themselves and they need to be invited
+// manually by a user.
 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.populateChannels()
+
+	channelInfo, err := b.getChannel(channel.Name)
+	if err != nil {
+		return fmt.Errorf("could not join channel: %#v", err)
+	}
+
+	if strings.HasPrefix(channel.Name, "ID:") {
 		b.useChannelID = true
-		ch, err := b.sc.GetChannelInfo(idcheck[1])
-		if err != nil {
-			return err
-		}
-		channel.Name = ch.Name
-		if err != nil {
-			return err
-		}
+		channel.Name = channelInfo.Name
 	}
 
-	// we can only join channels using the API
-	if b.sc != nil {
-		if strings.HasPrefix(b.GetString(tokenConfig), "xoxb") {
-			// TODO check if bot has already joined channel
-			return nil
-		}
-		_, err := b.sc.JoinChannel(channel.Name)
-		if err != nil {
-			switch err.Error() {
-			case "name_taken", "restricted_action":
-			case "default":
-				{
-					return err
-				}
-			}
-		}
+	if !channelInfo.IsMember {
+		return fmt.Errorf("slack integration that matterbridge is using is not member of channel '%s', please add it manually", channelInfo.Name)
 	}
 	return nil
 }