Skip to content

Commit

Permalink
[MI-3519] Fix the issue of receiving multiple update activities from …
Browse files Browse the repository at this point in the history 
…MS Teams (#315)

* [MI-3519] Fix issue of receiving multiple update activities from MS Teams

* [MI-3519] Update if conditions

* [MI-3519] Move store mutex to plugin and update mutex usage other functions

* [MI-3519] Fix unit test cases

* [MI-3519] Update mutex locks and update test cases mocking

* [MI-3519] Revert store mutex changes

* [MI-3519] Remove mocking of pluginiface interface
  • Loading branch information
@ayusht2810
ayusht2810 authored Sep 22, 2023
1 parent d5f686c commit 4b3808a
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 64 deletions.
13 changes: 10 additions & 3 deletions server/handlers/handlers.go
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package handlers

import (
"fmt"
"regexp"
"strconv"
"strings"
"sync"
"time"

"github.com/enescakir/emoji"
Expand Down Expand Up @@ -43,9 +45,10 @@ type PluginIface interface {
}

type ActivityHandler struct {
plugin PluginIface
queue chan msteams.Activity
quit chan bool
plugin PluginIface
queue chan msteams.Activity
quit chan bool
IgnorePluginHooksMap sync.Map
}

func New(plugin PluginIface) *ActivityHandler {
Expand Down Expand Up @@ -346,7 +349,9 @@ func (ah *ActivityHandler) handleUpdatedActivity(activityIds msteams.ActivityIds
post, _ := ah.msgToPost(channelID, senderID, msg, chat, true)
post.Id = postInfo.MattermostID

ah.IgnorePluginHooksMap.Store(fmt.Sprintf("post_%s", post.Id), true)
if _, appErr := ah.plugin.GetAPI().UpdatePost(post); appErr != nil {
ah.IgnorePluginHooksMap.Delete(fmt.Sprintf("post_%s", post.Id))
if strings.EqualFold(appErr.Id, "app.post.get.app_error") {
if err = ah.plugin.GetStore().RecoverPost(post.Id); err != nil {
ah.plugin.GetAPI().LogError("Unable to recover the post", "PostID", post.Id, "error", err)
Expand Down Expand Up @@ -416,13 +421,15 @@ func (ah *ActivityHandler) handleReactions(postID, channelID string, reactions [
continue
}
if !postReactionsByUserAndEmoji[reactionUserID+emojiName] {
ah.IgnorePluginHooksMap.Store(fmt.Sprintf("%s_%s_%s", postID, reactionUserID, emojiName), true)
r, appErr := ah.plugin.GetAPI().AddReaction(&model.Reaction{
UserId: reactionUserID,
PostId: postID,
ChannelId: channelID,
EmojiName: emojiName,
})
if appErr != nil {
ah.IgnorePluginHooksMap.Delete(fmt.Sprintf("reactions_%s_%s", reactionUserID, emojiName))
ah.plugin.GetAPI().LogError("failed to create the reaction", "err", appErr)
continue
}
Expand Down
27 changes: 14 additions & 13 deletions server/handlers/handlers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func TestHandleCreatedActivity(t *testing.T) {
},
setupAPI: func(mockAPI *plugintest.API) {
mockAPI.On("LogDebug", "duplicate post").Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Once()
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Once()
},
setupStore: func(store *mocksStore.Store) {
store.On("GetPostInfoByMSTeamsID", testutils.GetChatID(), testutils.GetMessageID()).Return(&storemodels.PostInfo{}, nil).Times(1)
Expand Down Expand Up @@ -353,7 +353,7 @@ func TestHandleCreatedActivity(t *testing.T) {
mockAPI.On("LogDebug", "Post generated").Times(1)
mockAPI.On("CreatePost", testutils.GetPostFromTeamsMessage()).Return(testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID()), nil).Times(1)
mockAPI.On("LogDebug", "Post created").Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Times(1)
mockAPI.On("LogWarn", "Error updating the MSTeams/Mattermost post link metadata", "error", mock.Anything).Times(1)
},
setupStore: func(store *mocksStore.Store) {
Expand Down Expand Up @@ -409,7 +409,7 @@ func TestHandleCreatedActivity(t *testing.T) {
mockAPI.On("LogDebug", "Post generated").Times(1)
mockAPI.On("CreatePost", testutils.GetPostFromTeamsMessage()).Return(testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID()), nil).Times(1)
mockAPI.On("LogDebug", "Post created").Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Times(1)
},
setupStore: func(store *mocksStore.Store) {
store.On("MattermostToTeamsUserID", "mock-BotUserID").Return(testutils.GetTeamsUserID(), nil).Times(1)
Expand Down Expand Up @@ -453,7 +453,7 @@ func TestHandleCreatedActivity(t *testing.T) {
mockAPI.On("LogDebug", "Post generated").Times(1)
mockAPI.On("CreatePost", testutils.GetPostFromTeamsMessage()).Return(testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID()), nil).Times(1)
mockAPI.On("LogDebug", "Post created").Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Times(1)
},
setupStore: func(store *mocksStore.Store) {
store.On("MattermostToTeamsUserID", "mock-BotUserID").Return(testutils.GetTeamsUserID(), nil).Times(1)
Expand Down Expand Up @@ -583,7 +583,7 @@ func TestHandleUpdatedActivity(t *testing.T) {
p.On("GetAPI").Return(mockAPI).Times(2)
p.On("GetStore").Return(store).Times(1)
p.On("GetBotUserID").Return("mock-BotUserID").Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Times(1)
},
setupClient: func(client *mocksClient.Client) {
client.On("GetChat", testutils.GetChatID()).Return(&msteams.Chat{
Expand Down Expand Up @@ -620,7 +620,6 @@ func TestHandleUpdatedActivity(t *testing.T) {
p.On("GetAPI").Return(mockAPI).Times(1)
p.On("GetStore").Return(store).Times(2)
p.On("GetBotUserID").Return("mock-BotUserID").Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Times(1)
},
setupClient: func(client *mocksClient.Client) {
client.On("GetChat", testutils.GetChatID()).Return(&msteams.Chat{
Expand All @@ -639,7 +638,9 @@ func TestHandleUpdatedActivity(t *testing.T) {
Text: "mockText",
}, nil).Times(1)
},
setupAPI: func(mockAPI *plugintest.API) {},
setupAPI: func(mockAPI *plugintest.API) {
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Times(1)
},
setupStore: func(store *mocksStore.Store) {
store.On("MattermostToTeamsUserID", "mock-BotUserID").Return(testutils.GetTeamsUserID(), nil).Times(1)
store.On("GetPostInfoByMSTeamsID", testutils.GetChatID(), testutils.GetMessageID()).Return(nil, nil).Times(1)
Expand All @@ -658,7 +659,6 @@ func TestHandleUpdatedActivity(t *testing.T) {
p.On("GetStore").Return(store).Times(2)
p.On("GetBotUserID").Return("mock-BotUserID").Times(1)
p.On("GetSyncDirectMessages").Return(true).Once()
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Times(1)
},
setupClient: func(client *mocksClient.Client) {
client.On("GetChat", testutils.GetChatID()).Return(&msteams.Chat{
Expand All @@ -680,6 +680,7 @@ func TestHandleUpdatedActivity(t *testing.T) {
setupAPI: func(mockAPI *plugintest.API) {
mockAPI.On("GetPost", "mockMattermostID").Return(nil, testutils.GetInternalServerAppError("unable to get the post")).Times(1)
mockAPI.On("LogError", "Unable to find the original post", "error", mock.Anything).Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Times(1)
},
setupStore: func(store *mocksStore.Store) {
store.On("MattermostToTeamsUserID", "mock-BotUserID").Return(testutils.GetTeamsUserID(), nil).Times(1)
Expand All @@ -704,7 +705,6 @@ func TestHandleUpdatedActivity(t *testing.T) {
p.On("GetStore").Return(store).Times(3)
p.On("GetBotUserID").Return("mock-BotUserID").Times(1)
p.On("GetSyncDirectMessages").Return(true).Once()
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Times(1)
},
setupClient: func(client *mocksClient.Client) {
client.On("GetChat", testutils.GetChatID()).Return(&msteams.Chat{
Expand All @@ -728,6 +728,7 @@ func TestHandleUpdatedActivity(t *testing.T) {
getPostError.Id = "app.post.get.app_error"
mockAPI.On("GetPost", "mockMattermostID").Return(nil, getPostError).Times(1)
mockAPI.On("LogError", "Unable to recover the post", "postID", "mockMattermostID", "error", errors.New("unable to recover")).Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Times(1)
},
setupStore: func(store *mocksStore.Store) {
store.On("MattermostToTeamsUserID", "mock-BotUserID").Return(testutils.GetTeamsUserID(), nil).Times(1)
Expand All @@ -752,7 +753,6 @@ func TestHandleUpdatedActivity(t *testing.T) {
p.On("GetAPI").Return(mockAPI).Times(6)
p.On("GetStore").Return(store).Times(3)
p.On("GetBotUserID").Return("mock-BotUserID").Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Times(1)
p.On("GetBotUserID").Return(testutils.GetSenderID()).Times(2)
p.On("GetSyncDirectMessages").Return(true).Once()
},
Expand All @@ -779,6 +779,7 @@ func TestHandleUpdatedActivity(t *testing.T) {
mockAPI.On("UpdatePost", mock.Anything).Return(nil, nil).Times(1)
mockAPI.On("GetReactions", "mockMattermostID").Return([]*model.Reaction{}, nil).Times(1)
mockAPI.On("GetUser", testutils.GetTeamsUserID()).Return(testutils.GetUser(model.ChannelAdminRoleId, "test@test.com"), nil).Once()
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Times(1)
},
setupStore: func(store *mocksStore.Store) {
store.On("MattermostToTeamsUserID", "mock-BotUserID").Return(testutils.GetTeamsUserID(), nil).Times(1)
Expand All @@ -803,7 +804,6 @@ func TestHandleUpdatedActivity(t *testing.T) {
p.On("GetAPI").Return(mockAPI).Times(5)
p.On("GetStore").Return(store).Times(4)
p.On("GetBotUserID").Return("mock-BotUserID").Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Times(1)
p.On("GetBotUserID").Return(testutils.GetSenderID()).Times(2)
},
setupClient: func(client *mocksClient.Client) {
Expand All @@ -823,6 +823,7 @@ func TestHandleUpdatedActivity(t *testing.T) {
mockAPI.On("UpdatePost", mock.Anything).Return(nil, nil).Times(1)
mockAPI.On("GetReactions", "mockMattermostID").Return([]*model.Reaction{}, nil).Times(1)
mockAPI.On("GetUser", testutils.GetUserID()).Return(testutils.GetUser(model.ChannelAdminRoleId, "test@test.com"), nil).Once()
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Times(1)
},
setupStore: func(store *mocksStore.Store) {
store.On("MattermostToTeamsUserID", "mock-BotUserID").Return(testutils.GetTeamsUserID(), nil).Times(1)
Expand Down Expand Up @@ -1075,7 +1076,7 @@ func TestUpdateLastReceivedChangeDate(t *testing.T) {
p.On("GetAPI").Return(mockAPI).Times(2)
},
setupAPI: func(mockAPI *plugintest.API) {
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(testutils.GetInternalServerAppError("unable to set the value in kv store")).Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(testutils.GetInternalServerAppError("unable to set the value in kv store")).Times(1)
mockAPI.On("LogError", "Unable to store properly the last received change").Times(1)
},
},
Expand All @@ -1085,7 +1086,7 @@ func TestUpdateLastReceivedChangeDate(t *testing.T) {
p.On("GetAPI").Return(mockAPI).Times(1)
},
setupAPI: func(mockAPI *plugintest.API) {
mockAPI.On("KVSet", lastReceivedChangeKey, mock.Anything).Return(nil).Times(1)
mockAPI.On("KVSet", lastReceivedChangeKey, mock.AnythingOfType("[]uint8")).Return(nil).Times(1)
},
},
} {
Expand Down
Loading

0 comments on commit 4b3808a

Please sign in to comment.