Migrate 'Channel.GetAllChannelMembersNotifyPropsForChannel' to Sync by default (#11248)

app/notification.go

@@ -29,7 +29,14 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 	}
 
 	pchan := a.Srv.Store.User().GetAllProfilesInChannel(channel.Id, true)
-	cmnchan := a.Srv.Store.Channel().GetAllChannelMembersNotifyPropsForChannel(channel.Id, true)
+
+	cmnchan := make(chan store.StoreResult, 1)
+	go func() {
+		props, err := a.Srv.Store.Channel().GetAllChannelMembersNotifyPropsForChannel(channel.Id, true)
+		cmnchan <- store.StoreResult{Data: props, Err: err}
+		close(cmnchan)
+	}()
+
 	var fchan chan store.StoreResult
 	if len(post.FileIds) != 0 {
 		fchan = make(chan store.StoreResult, 1)

store/sqlstore/channel_store.go

@@ -1586,42 +1586,38 @@ type allChannelMemberNotifyProps struct {
 	NotifyProps model.StringMap
 }
 
-func (s SqlChannelStore) GetAllChannelMembersNotifyPropsForChannel(channelId string, allowFromCache bool) store.StoreChannel {
+func (s SqlChannelStore) GetAllChannelMembersNotifyPropsForChannel(channelId string, allowFromCache bool) (map[string]model.StringMap, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	if allowFromCache {
-		if allowFromCache {
+		if cacheItem, ok := allChannelMembersNotifyPropsForChannelCache.Get(channelId); ok {
-			if cacheItem, ok := allChannelMembersNotifyPropsForChannelCache.Get(channelId); ok {
+			if s.metrics != nil {
-				if s.metrics != nil {
+				s.metrics.IncrementMemCacheHitCounter("All Channel Members Notify Props for Channel")
-					s.metrics.IncrementMemCacheHitCounter("All Channel Members Notify Props for Channel")
-				}
-				result.Data = cacheItem.(map[string]model.StringMap)
-				return
 			}
+			return cacheItem.(map[string]model.StringMap), nil
 		}
+	}
 
-		if s.metrics != nil {
+	if s.metrics != nil {
-			s.metrics.IncrementMemCacheMissCounter("All Channel Members Notify Props for Channel")
+		s.metrics.IncrementMemCacheMissCounter("All Channel Members Notify Props for Channel")
-		}
+	}
 
-		var data []allChannelMemberNotifyProps
+	var data []allChannelMemberNotifyProps
-		_, err := s.GetReplica().Select(&data, `
+	_, err := s.GetReplica().Select(&data, `
-			SELECT UserId, NotifyProps
+		SELECT UserId, NotifyProps
-			FROM ChannelMembers
+		FROM ChannelMembers
-			WHERE ChannelId = :ChannelId`, map[string]interface{}{"ChannelId": channelId})
+		WHERE ChannelId = :ChannelId`, map[string]interface{}{"ChannelId": channelId})
 
-		if err != nil {
+	if err != nil {
-			result.Err = model.NewAppError("SqlChannelStore.GetAllChannelMembersPropsForChannel", "store.sql_channel.get_members.app_error", nil, "channelId="+channelId+", err="+err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlChannelStore.GetAllChannelMembersPropsForChannel", "store.sql_channel.get_members.app_error", nil, "channelId="+channelId+", err="+err.Error(), http.StatusInternalServerError)
-			return
+	}
-		}
 
-		props := make(map[string]model.StringMap)
+	props := make(map[string]model.StringMap)
-		for i := range data {
+	for i := range data {
-			props[data[i].UserId] = data[i].NotifyProps
+		props[data[i].UserId] = data[i].NotifyProps
-		}
+	}
 
-		result.Data = props
+	allChannelMembersNotifyPropsForChannelCache.AddWithExpiresInSecs(channelId, props, ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SEC)
 
-		allChannelMembersNotifyPropsForChannelCache.AddWithExpiresInSecs(channelId, props, ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SEC)
+	return props, nil
-	})
 }
 
 func (s SqlChannelStore) InvalidateMemberCount(channelId string) {

store/store.go

@@ -165,7 +165,7 @@ type ChannelStore interface {
 	GetAllChannelMembersForUser(userId string, allowFromCache bool, includeDeleted bool) StoreChannel
 	InvalidateAllChannelMembersForUser(userId string)
 	IsUserInChannelUseCache(userId string, channelId string) bool
-	GetAllChannelMembersNotifyPropsForChannel(channelId string, allowFromCache bool) StoreChannel
+	GetAllChannelMembersNotifyPropsForChannel(channelId string, allowFromCache bool) (map[string]model.StringMap, *model.AppError)
 	InvalidateCacheForChannelMembersNotifyProps(channelId string)
 	GetMemberForPost(postId string, userId string) StoreChannel
 	InvalidateMemberCount(channelId string)

store/storetest/channel_store.go

@@ -1823,22 +1823,17 @@ func testGetMember(t *testing.T, ss store.Store) {
 		t.Fatal("should've gotten member for user")
 	}
 
-	if result := <-ss.Channel().GetAllChannelMembersNotifyPropsForChannel(c2.Id, false); result.Err != nil {
+	if props, err := ss.Channel().GetAllChannelMembersNotifyPropsForChannel(c2.Id, false); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
-	} else {
+	} else if len(props) == 0 {
-		props := result.Data.(map[string]model.StringMap)
+		t.Fatal("should not be empty")
-		if len(props) == 0 {
-			t.Fatal("should not be empty")
-		}
 	}
 
-	if result := <-ss.Channel().GetAllChannelMembersNotifyPropsForChannel(c2.Id, true); result.Err != nil {
+	if props, err := ss.Channel().GetAllChannelMembersNotifyPropsForChannel(c2.Id, true); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
-	} else {
+	} else if len(props) == 0 {
-		props := result.Data.(map[string]model.StringMap)
+		t.Fatal("should not be empty")
-		if len(props) == 0 {
+
-			t.Fatal("should not be empty")
-		}
 	}
 
 	ss.Channel().InvalidateCacheForChannelMembersNotifyProps(c2.Id)

store/storetest/mocks/ChannelStore.go

@@ -213,19 +213,28 @@ func (_m *ChannelStore) GetAllChannelMembersForUser(userId string, allowFromCach
 }
 
 // GetAllChannelMembersNotifyPropsForChannel provides a mock function with given fields: channelId, allowFromCache
-func (_m *ChannelStore) GetAllChannelMembersNotifyPropsForChannel(channelId string, allowFromCache bool) store.StoreChannel {
+func (_m *ChannelStore) GetAllChannelMembersNotifyPropsForChannel(channelId string, allowFromCache bool) (map[string]model.StringMap, *model.AppError) {
 	ret := _m.Called(channelId, allowFromCache)
 
-	var r0 store.StoreChannel
+	var r0 map[string]model.StringMap
-	if rf, ok := ret.Get(0).(func(string, bool) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string, bool) map[string]model.StringMap); ok {
 		r0 = rf(channelId, allowFromCache)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(map[string]model.StringMap)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, bool) *model.AppError); ok {
+		r1 = rf(channelId, allowFromCache)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetAllChannels provides a mock function with given fields: page, perPage, opts