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

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.populateUsers()
-			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"},
-			})
-			if err != nil {
-				b.Log.Errorf("Channel list failed: %#v", err)
-			}
 			b.si = ev.Info
-			b.users, err = b.sc.GetUsers()
+			b.populateChannels()
-			if err != nil {
+			b.populateUsers()
-				b.Log.Errorf("Could not reload users: %#v", err)
-			}
 		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 {
+		b.populateUsers()
-			return nil, err
-		}
 	}
 
 	// Edit message

bridge/slack/helpers.go

@@ -38,29 +38,74 @@ func (b *Bslack) getChannel(channel string) (*slack.Channel, error) {
 }
 
 func (b *Bslack) getChannelByName(name string) (*slack.Channel, error) {
-	if b.channels == nil {
+	b.channelsMutex.RLock()
-		return nil, fmt.Errorf("%s: channel %s not found (no channels found)", b.Account, name)
+	defer b.channelsMutex.RUnlock()
-	}
+
-	for _, channel := range b.channels {
+	if channel, ok := b.channelsByName[name]; ok {
-		if channel.Name == name {
+		return channel, nil
-			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 {
+	b.channelsMutex.RLock()
-		return nil, fmt.Errorf("%s: channel %s not found (no channels found)", b.Account, ID)
+	defer b.channelsMutex.RUnlock()
-	}
+
-	for _, channel := range b.channels {
+	if channel, ok := b.channelsByID[ID]; ok {
-		if channel.ID == ID {
+		return channel, nil
-			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
+	}
+
+	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
+	}
+
+	b.channelsMutex.Lock()
+	defer b.channelsMutex.Unlock()
+	b.channelsByID = newChannelsByID
+	b.channelsByName = newChannelsByName
+}
+
 var (
 	mentionRE  = regexp.MustCompile(`<@([a-zA-Z0-9]+)>`)
 	channelRE  = regexp.MustCompile(`<#[a-zA-Z0-9]+\|(.+?)>`)

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
-	cache        *lru.Cache
-	useChannelID bool
-	uuid         string
-	*bridge.Config
 	sync.RWMutex
+	*bridge.Config
+
+	mh  *matterhook.Client
+	sc  *slack.Client
+	rtm *slack.RTM
+	si  *slack.Info
+
+	cache        *lru.Cache
+	uuid         string
+	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,
+		Config:         cfg,
-		uuid:   xid.New().String(),
+		uuid:           xid.New().String(),
-		cache:  newCache,
+		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
+	b.populateChannels()
-	idcheck := strings.Split(channel.Name, "ID:")
+
-	if len(idcheck) > 1 {
+	channelInfo, err := b.getChannel(channel.Name)
-		b.useChannelID = true
+	if err != nil {
-		ch, err := b.sc.GetChannelInfo(idcheck[1])
+		return fmt.Errorf("could not join channel: %#v", err)
-		if err != nil {
-			return err
-		}
-		channel.Name = ch.Name
-		if err != nil {
-			return err
-		}
 	}
 
-	// we can only join channels using the API
+	if strings.HasPrefix(channel.Name, "ID:") {
-	if b.sc != nil {
+		b.useChannelID = true
-		if strings.HasPrefix(b.GetString(tokenConfig), "xoxb") {
+		channel.Name = channelInfo.Name
-			// TODO check if bot has already joined channel
+	}
-			return nil
+
-		}
+	if !channelInfo.IsMember {
-		_, err := b.sc.JoinChannel(channel.Name)
+		return fmt.Errorf("slack integration that matterbridge is using is not member of channel '%s', please add it manually", channelInfo.Name)
-		if err != nil {
-			switch err.Error() {
-			case "name_taken", "restricted_action":
-			case "default":
-				{
-					return err
-				}
-			}
-		}
 	}
 	return nil
 }