From 6238effdc27e4f439cb49bfa5fb5cd09fd88375e Mon Sep 17 00:00:00 2001 From: Duco van Amstel Date: Tue, 16 Oct 2018 19:34:09 +0100 Subject: Clean up user and channel information management (slack) (#521) --- bridge/slack/handlers.go | 24 +++------------ bridge/slack/helpers.go | 71 +++++++++++++++++++++++++++++++++++--------- bridge/slack/slack.go | 76 +++++++++++++++++++++++------------------------- 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 } -- cgit v1.2.3