channel loading has been moved into utility object
Loading of channels is now unit tested to a certain degree. While the general
loading, appending of message and such isn't tested, the cache access is
tested. This is very important, since it'll avoid unnecessarily spamming
the discord API.
MODIFY
discordutil/message.go
discordutil/message.go
@@ -13,3 +13,84 @@ func MentionsCurrentUserExplicitly(state *discordgo.State, message *discordgo.Me
return false
}
+
+// MessageDataSupplier defines the method that is necessary for requesting
+// channels. This is satisfied by the discordgo.Session struct and can be
+// used in order to make testing easier.
+type MessageDataSupplier interface {
+ ChannelMessages(string, int, string, string, string) ([]*discordgo.Message, error)
+}
+
+// MessageLoader represents a util object that remember which channels have
+// already been cached and which not.
+type MessageLoader struct {
+ messageDateSupplier MessageDataSupplier
+ requestedChannels map[string]bool
+}
+
+// IsCached checks whether the channel has already been requested from the
+// backend once.
+func (l *MessageLoader) IsCached(channelID string) bool {
+ value, cached := l.requestedChannels[channelID]
+ return cached && value
+}
+
+func CreateMessageLoader(messageDataSupplier MessageDataSupplier) *MessageLoader {
+ loader := &MessageLoader{
+ requestedChannels: make(map[string]bool),
+ messageDateSupplier: messageDataSupplier,
+ }
+
+ return loader
+}
+
+// DeleteFromCache deletes the entry that indicates the channel has been
+// cached. The next call to LoadMessages with the same ID will ask for data
+// from the MessageDataSupplier.
+func (l *MessageLoader) DeleteFromCache(channelID string) {
+ delete(l.requestedChannels, channelID)
+}
+
+// LoadMessages returns the last 100 messages for a channel. If less messages
+// were sent, less will be returned. As soon as a channel has been loaded once
+// it won't ever be loaded again, instead a global cache will be accessed.
+func (l *MessageLoader) LoadMessages(channel *discordgo.Channel) ([]*discordgo.Message, error) {
+ var messages []*discordgo.Message
+
+ if channel.LastMessageID != "" {
+ if !l.IsCached(channel.ID) {
+ l.requestedChannels[channel.ID] = true
+
+ var beforeID string
+ localMessageCount := len(channel.Messages)
+ if localMessageCount > 0 {
+ beforeID = channel.Messages[0].ID
+ }
+
+ messagesToGet := 100 - localMessageCount
+ if messagesToGet > 0 {
+ var discordError error
+ messages, discordError = l.messageDateSupplier.ChannelMessages(channel.ID, messagesToGet, beforeID, "", "")
+ if discordError != nil {
+ return nil, discordError
+ }
+
+ if channel.GuildID != "" {
+ for _, message := range messages {
+ message.GuildID = channel.GuildID
+ }
+ }
+ if localMessageCount == 0 {
+ channel.Messages = messages
+ } else {
+ //There are already messages in cache; However, those came from updates events.
+ //Therefore those have to be newer than the newly retrieved ones.
+ channel.Messages = append(messages, channel.Messages...)
+ }
+ }
+ }
+ messages = channel.Messages
+ }
+
+ return messages, nil
+}

MODIFY
discordutil/message_test.go
discordutil/message_test.go
@@ -96,3 +96,100 @@ func Test_MentionsCurrentUserExplicitly(t *testing.T) {
})
}
}
+
+type messageSupplier struct {
+ requestAmount int
+}
+
+func (l *messageSupplier) ChannelMessages(channelID string, limit int, beforeID, afterID, aroundID string) ([]*discordgo.Message, error) {
+ l.requestAmount++
+ return nil, nil
+}
+
+func Test_LoadMessages_CacheAccess(t *testing.T) {
+ t.Run("Test emtpy channel", func(t *testing.T) {
+ channelEmpty := &discordgo.Channel{
+ ID: "1",
+ LastMessageID: "",
+ }
+ supplier := &messageSupplier{}
+ loader := CreateMessageLoader(supplier)
+
+ _, loadError := loader.LoadMessages(channelEmpty)
+ if loadError != nil {
+ t.Errorf("Error loading channels: %s", loadError)
+ }
+
+ if supplier.requestAmount != 0 {
+ t.Errorf("Cache was accessed %d times instead of 1 time.", supplier.requestAmount)
+ }
+
+ _, loadError = loader.LoadMessages(channelEmpty)
+ if loadError != nil {
+ t.Errorf("Error loading channels: %s", loadError)
+ }
+
+ if supplier.requestAmount != 0 {
+ t.Errorf("Cache was accessed %d times instead of 1 time.", supplier.requestAmount)
+ }
+ })
+
+ t.Run("Test cache access", func(t *testing.T) {
+ channelNotEmpty := &discordgo.Channel{
+ ID: "1",
+ LastMessageID: "123",
+ }
+ supplier := &messageSupplier{}
+ loader := CreateMessageLoader(supplier)
+
+ _, loadError := loader.LoadMessages(channelNotEmpty)
+ if loadError != nil {
+ t.Errorf("Error loading channels: %s", loadError)
+ }
+ _, loadError = loader.LoadMessages(channelNotEmpty)
+ if loadError != nil {
+ t.Errorf("Error loading channels: %s", loadError)
+ }
+
+ if supplier.requestAmount != 1 {
+ t.Errorf("Cache was accessed %d times instead of 1 time.", supplier.requestAmount)
+ }
+ })
+
+ t.Run("Test cache access with cacheclear", func(t *testing.T) {
+ channelNotEmpty := &discordgo.Channel{
+ ID: "1",
+ LastMessageID: "123",
+ }
+ supplier := &messageSupplier{}
+ loader := CreateMessageLoader(supplier)
+
+ if loader.IsCached(channelNotEmpty.ID) {
+ t.Error("Channel cache should still be clear.")
+ }
+
+ _, loadError := loader.LoadMessages(channelNotEmpty)
+ if loadError != nil {
+ t.Errorf("Error loading channels: %s", loadError)
+ }
+ _, loadError = loader.LoadMessages(channelNotEmpty)
+ if loadError != nil {
+ t.Errorf("Error loading channels: %s", loadError)
+ }
+
+ if supplier.requestAmount != 1 {
+ t.Errorf("Cache was accessed %d times instead of 1 time.", supplier.requestAmount)
+ }
+
+ if !loader.IsCached(channelNotEmpty.ID) {
+ t.Error("Channel should've been cached but wasn't.")
+ }
+
+ loader.DeleteFromCache(channelNotEmpty.ID)
+
+ if loader.IsCached(channelNotEmpty.ID) {
+ t.Error("Cache should've been cleared, but wasn't.")
+ }
+
+ })
+}

MODIFY
ui/window.go
ui/window.go
@@ -66,6 +66,7 @@ type Window struct {
messageInput *Editor
editingMessageID *string
+ messageLoader *discordutil.MessageLoader
userList *UserTree
@@ -102,6 +103,7 @@ func NewWindow(doRestart chan bool, app *tview.Application, session *discordgo.S
app: app,
jsEngine: js.New(),
userActiveTimer: time.NewTimer(userInactiveTime),
+ messageLoader: discordutil.CreateMessageLoader(session),
}
go func() {
@@ -1933,7 +1935,7 @@ func (window *Window) registerGuildChannelHandler() {
})
}
- delete(requestedChannels, event.Channel.ID)
+ window.messageLoader.DeleteFromCache(event.Channel.ID)
//On purpose, since we don't care much about removing the channel timely.
window.app.QueueUpdateDraw(func() {
window.channelTree.Lock()
@@ -2302,44 +2304,11 @@ func (window *Window) RefreshLayout() {
window.app.ForceDraw()
}
-var requestedChannels = make(map[string]bool)
-
//LoadChannel eagerly loads the channels messages.
func (window *Window) LoadChannel(channel *discordgo.Channel) error {
- var messages []*discordgo.Message
-
- if channel.LastMessageID != "" {
- localMessageCount := len(channel.Messages)
- hasBeenRequested := requestedChannels[channel.ID]
- if !hasBeenRequested {
- requestedChannels[channel.ID] = true
-
- var beforeID string
- if localMessageCount > 0 {
- beforeID = channel.Messages[0].ID
- }
-
- var discordError error
- messagesToGet := 100 - localMessageCount
- if messagesToGet > 0 {
- messages, discordError = window.session.ChannelMessages(channel.ID, messagesToGet, beforeID, "", "")
- if discordError == nil {
- if channel.GuildID != "" {
- for _, message := range messages {
- message.GuildID = channel.GuildID
- }
- }
- if localMessageCount == 0 {
- channel.Messages = messages
- } else {
- //There are already messages in cache; However, those came from updates events.
- //Therefore those have to be newer than the newly retrieved ones.
- channel.Messages = append(messages, channel.Messages...)
- }
- }
- }
- }
- messages = channel.Messages
+ messages, loadError := window.messageLoader.LoadMessages(channel)
+ if loadError != nil {
+ return loadError
}
discordutil.SortMessagesByTimestamp(messages)

channel loading has been moved into utility object

Loading of channels is now unit tested to a certain degree. While the general
loading, appending of message and such isn't tested, the cache access is
tested. This is very important, since it'll avoid unnecessarily spamming
the discord API.