From 54d62dbadf13260d4a8a3d683255b5f3a83099c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Villablanca=20V=C3=A1squez?= Date: Tue, 25 Jun 2019 06:13:39 -0400 Subject: [PATCH] Migrates Channel.IncrementMentionCount to sync by default (#11237) * Migrates Channel.IncrementMentionCount to sync by default * Calls to Channel.IncrementMentionCount are concurrent now --- app/notification.go | 17 +++++++++----- store/sqlstore/channel_store.go | 32 +++++++++++++-------------- store/store.go | 2 +- store/storetest/channel_store.go | 8 +++---- store/storetest/mocks/ChannelStore.go | 8 +++---- store/storetest/user_store.go | 9 +++++--- 6 files changed, 42 insertions(+), 34 deletions(-) diff --git a/app/notification.go b/app/notification.go index 764f907032..3facbdf968 100644 --- a/app/notification.go +++ b/app/notification.go @@ -65,7 +65,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod hereNotification := false channelNotification := false allNotification := false - updateMentionChans := []store.StoreChannel{} + updateMentionChans := []chan *model.AppError{} if channel.Type == model.CHANNEL_DIRECT { var otherUserId string @@ -136,8 +136,8 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod } if len(m.OtherPotentialMentions) > 0 && !post.IsSystemMessage() { - if result := <-a.Srv.Store.User().GetProfilesByUsernames(m.OtherPotentialMentions, &model.ViewUsersRestrictions{Teams: []string{team.Id}}); result.Err == nil { - channelMentions := model.UserSlice(result.Data.([]*model.User)).FilterByActive(true) + if profilesResult := <-a.Srv.Store.User().GetProfilesByUsernames(m.OtherPotentialMentions, &model.ViewUsersRestrictions{Teams: []string{team.Id}}); profilesResult.Err == nil { + channelMentions := model.UserSlice(profilesResult.Data.([]*model.User)).FilterByActive(true) var outOfChannelMentions model.UserSlice var outOfGroupsMentions model.UserSlice @@ -176,7 +176,12 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod mentionedUsersList := make([]string, 0, len(mentionedUserIds)) for id := range mentionedUserIds { mentionedUsersList = append(mentionedUsersList, id) - updateMentionChans = append(updateMentionChans, a.Srv.Store.Channel().IncrementMentionCount(post.ChannelId, id)) + umc := make(chan *model.AppError, 1) + go func() { + umc <- a.Srv.Store.Channel().IncrementMentionCount(post.ChannelId, id) + close(umc) + }() + updateMentionChans = append(updateMentionChans, umc) } notification := &postNotification{ @@ -275,8 +280,8 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod // Make sure all mention updates are complete to prevent race // Probably better to batch these DB updates in the future // MUST be completed before push notifications send - for _, uchan := range updateMentionChans { - if result := <-uchan; result.Err != nil { + for _, umc := range updateMentionChans { + if err := <-umc; err != nil { mlog.Warn(fmt.Sprintf("Failed to update mention count, post_id=%v channel_id=%v err=%v", post.Id, post.ChannelId, result.Err), mlog.String("post_id", post.Id)) } } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 9d60c28636..12faa7229b 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1783,22 +1783,22 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelIds []string, userId string) }) } -func (s SqlChannelStore) IncrementMentionCount(channelId string, userId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - _, err := s.GetMaster().Exec( - `UPDATE - ChannelMembers - SET - MentionCount = MentionCount + 1, - LastUpdateAt = :LastUpdateAt - WHERE - UserId = :UserId - AND ChannelId = :ChannelId`, - map[string]interface{}{"ChannelId": channelId, "UserId": userId, "LastUpdateAt": model.GetMillis()}) - if err != nil { - result.Err = model.NewAppError("SqlChannelStore.IncrementMentionCount", "store.sql_channel.increment_mention_count.app_error", nil, "channel_id="+channelId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError) - } - }) +func (s SqlChannelStore) IncrementMentionCount(channelId string, userId string) *model.AppError { + _, err := s.GetMaster().Exec( + `UPDATE + ChannelMembers + SET + MentionCount = MentionCount + 1, + LastUpdateAt = :LastUpdateAt + WHERE + UserId = :UserId + AND ChannelId = :ChannelId`, + map[string]interface{}{"ChannelId": channelId, "UserId": userId, "LastUpdateAt": model.GetMillis()}) + if err != nil { + return model.NewAppError("SqlChannelStore.IncrementMentionCount", "store.sql_channel.increment_mention_count.app_error", nil, "channel_id="+channelId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError) + } + + return nil } func (s SqlChannelStore) GetAll(teamId string) ([]*model.Channel, *model.AppError) { diff --git a/store/store.go b/store/store.go index 90139937a3..b1e719d805 100644 --- a/store/store.go +++ b/store/store.go @@ -176,7 +176,7 @@ type ChannelStore interface { PermanentDeleteMembersByUser(userId string) StoreChannel PermanentDeleteMembersByChannel(channelId string) *model.AppError UpdateLastViewedAt(channelIds []string, userId string) StoreChannel - IncrementMentionCount(channelId string, userId string) StoreChannel + IncrementMentionCount(channelId string, userId string) *model.AppError AnalyticsTypeCount(teamId string, channelType string) (int64, *model.AppError) GetMembersForUser(teamId string, userId string) StoreChannel GetMembersForUserWithPagination(teamId, userId string, page, perPage int) StoreChannel diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index aca19d4eec..f43c2af956 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -1722,22 +1722,22 @@ func testChannelStoreIncrementMentionCount(t *testing.T, ss store.Store) { m1.NotifyProps = model.GetDefaultChannelNotifyProps() store.Must(ss.Channel().SaveMember(&m1)) - err = (<-ss.Channel().IncrementMentionCount(m1.ChannelId, m1.UserId)).Err + err = ss.Channel().IncrementMentionCount(m1.ChannelId, m1.UserId) if err != nil { t.Fatal("failed to update") } - err = (<-ss.Channel().IncrementMentionCount(m1.ChannelId, "missing id")).Err + err = ss.Channel().IncrementMentionCount(m1.ChannelId, "missing id") if err != nil { t.Fatal("failed to update") } - err = (<-ss.Channel().IncrementMentionCount("missing id", m1.UserId)).Err + err = ss.Channel().IncrementMentionCount("missing id", m1.UserId) if err != nil { t.Fatal("failed to update") } - err = (<-ss.Channel().IncrementMentionCount("missing id", "missing id")).Err + err = ss.Channel().IncrementMentionCount("missing id", "missing id") if err != nil { t.Fatal("failed to update") } diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index d0203d151c..8cf2a81b00 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -980,15 +980,15 @@ func (_m *ChannelStore) GetTeamChannels(teamId string) (*model.ChannelList, *mod } // IncrementMentionCount provides a mock function with given fields: channelId, userId -func (_m *ChannelStore) IncrementMentionCount(channelId string, userId string) store.StoreChannel { +func (_m *ChannelStore) IncrementMentionCount(channelId string, userId string) *model.AppError { ret := _m.Called(channelId, userId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok { r0 = rf(channelId, userId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index ce86117c27..42b99af72f 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -1851,7 +1851,8 @@ func testUserUnreadCount(t *testing.T, ss store.Store) { // Post one message with mention to open channel _, err := ss.Post().Save(&p1) require.Nil(t, err) - store.Must(ss.Channel().IncrementMentionCount(c1.Id, u2.Id)) + err = ss.Channel().IncrementMentionCount(c1.Id, u2.Id) + require.Nil(t, err) // Post 2 messages without mention to direct channel p2 := model.Post{} @@ -1861,7 +1862,8 @@ func testUserUnreadCount(t *testing.T, ss store.Store) { _, err = ss.Post().Save(&p2) require.Nil(t, err) - store.Must(ss.Channel().IncrementMentionCount(c2.Id, u2.Id)) + err = ss.Channel().IncrementMentionCount(c2.Id, u2.Id) + require.Nil(t, err) p3 := model.Post{} p3.ChannelId = c2.Id @@ -1870,7 +1872,8 @@ func testUserUnreadCount(t *testing.T, ss store.Store) { _, err = ss.Post().Save(&p3) require.Nil(t, err) - store.Must(ss.Channel().IncrementMentionCount(c2.Id, u2.Id)) + err = ss.Channel().IncrementMentionCount(c2.Id, u2.Id) + require.Nil(t, err) badge := (<-ss.User().GetUnreadCount(u2.Id)).Data.(int64) if badge != 3 {