[MM-11222] Migrate 'Channel.GetMemberCount' to Sync by default (#11246)

* Migrate 'Channel.GetMemberCount' to Sync by default

* Rename member count chan

* Fix variable shadowing

app/channel.go

@@ -1317,11 +1317,7 @@ func (a *App) GetChannelMembersForUserWithPagination(teamId, userId string, page
 }
 
 func (a *App) GetChannelMemberCount(channelId string) (int64, *model.AppError) {
-	result := <-a.Srv.Store.Channel().GetMemberCount(channelId, true)
+	return a.Srv.Store.Channel().GetMemberCount(channelId, true)
-	if result.Err != nil {
-		return 0, result.Err
-	}
-	return result.Data.(int64), nil
 }
 
 func (a *App) GetChannelCounts(teamId string, userId string) (*model.ChannelCounts, *model.AppError) {
@@ -1452,7 +1448,13 @@ func (a *App) LeaveChannel(channelId string, userId string) *model.AppError {
 		uc <- store.StoreResult{Data: user, Err: err}
 		close(uc)
 	}()
-	ccm := a.Srv.Store.Channel().GetMemberCount(channelId, false)
+
+	mcc := make(chan store.StoreResult, 1)
+	go func() {
+		count, err := a.Srv.Store.Channel().GetMemberCount(channelId, false)
+		mcc <- store.StoreResult{Data: count, Err: err}
+		close(mcc)
+	}()
 
 	cresult := <-sc
 	if cresult.Err != nil {
@@ -1462,14 +1464,14 @@ func (a *App) LeaveChannel(channelId string, userId string) *model.AppError {
 	if uresult.Err != nil {
 		return cresult.Err
 	}
-	ccmresult := <-ccm
+	ccresult := <-mcc
-	if ccmresult.Err != nil {
+	if ccresult.Err != nil {
-		return ccmresult.Err
+		return ccresult.Err
 	}
 
 	channel := cresult.Data.(*model.Channel)
 	user := uresult.Data.(*model.User)
-	membersCount := ccmresult.Data.(int64)
+	membersCount := ccresult.Data.(int64)
 
 	if channel.IsGroupOrDirect() {
 		err := model.NewAppError("LeaveChannel", "api.channel.leave.direct.app_error", nil, "", http.StatusBadRequest)

store/sqlstore/channel_store.go

@@ -1618,50 +1618,47 @@ func (s SqlChannelStore) GetMemberCountFromCache(channelId string) int64 {
 		s.metrics.IncrementMemCacheMissCounter("Channel Member Counts")
 	}
 
-	result := <-s.GetMemberCount(channelId, true)
+	count, err := s.GetMemberCount(channelId, true)
-	if result.Err != nil {
+	if err != nil {
 		return 0
 	}
 
-	return result.Data.(int64)
+	return count
 }
 
-func (s SqlChannelStore) GetMemberCount(channelId string, allowFromCache bool) store.StoreChannel {
+func (s SqlChannelStore) GetMemberCount(channelId string, allowFromCache bool) (int64, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	if allowFromCache {
-		if allowFromCache {
+		if cacheItem, ok := channelMemberCountsCache.Get(channelId); ok {
-			if cacheItem, ok := channelMemberCountsCache.Get(channelId); ok {
+			if s.metrics != nil {
-				if s.metrics != nil {
+				s.metrics.IncrementMemCacheHitCounter("Channel Member Counts")
-					s.metrics.IncrementMemCacheHitCounter("Channel Member Counts")
-				}
-				result.Data = cacheItem.(int64)
-				return
 			}
+			return cacheItem.(int64), nil
 		}
+	}
 
-		if s.metrics != nil {
+	if s.metrics != nil {
-			s.metrics.IncrementMemCacheMissCounter("Channel Member Counts")
+		s.metrics.IncrementMemCacheMissCounter("Channel Member Counts")
-		}
+	}
 
-		count, err := s.GetReplica().SelectInt(`
+	count, err := s.GetReplica().SelectInt(`
-			SELECT
+		SELECT
-				count(*)
+			count(*)
-			FROM
+		FROM
-				ChannelMembers,
+			ChannelMembers,
-				Users
+			Users
-			WHERE
+		WHERE
-				ChannelMembers.UserId = Users.Id
+			ChannelMembers.UserId = Users.Id
-				AND ChannelMembers.ChannelId = :ChannelId
+			AND ChannelMembers.ChannelId = :ChannelId
-				AND Users.DeleteAt = 0`, map[string]interface{}{"ChannelId": channelId})
+			AND Users.DeleteAt = 0`, map[string]interface{}{"ChannelId": channelId})
-		if err != nil {
+	if err != nil {
-			result.Err = model.NewAppError("SqlChannelStore.GetMemberCount", "store.sql_channel.get_member_count.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError)
+		return 0, model.NewAppError("SqlChannelStore.GetMemberCount", "store.sql_channel.get_member_count.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError)
-			return
+	}
-		}
-		result.Data = count
 
-		if allowFromCache {
+	if allowFromCache {
-			channelMemberCountsCache.AddWithExpiresInSecs(channelId, count, CHANNEL_MEMBERS_COUNTS_CACHE_SEC)
+		channelMemberCountsCache.AddWithExpiresInSecs(channelId, count, CHANNEL_MEMBERS_COUNTS_CACHE_SEC)
-		}
+	}
-	})
+
+	return count, nil
 }
 
 func (s SqlChannelStore) RemoveMember(channelId string, userId string) *model.AppError {

store/store.go

@@ -170,7 +170,7 @@ type ChannelStore interface {
 	GetMemberForPost(postId string, userId string) (*model.ChannelMember, *model.AppError)
 	InvalidateMemberCount(channelId string)
 	GetMemberCountFromCache(channelId string) int64
-	GetMemberCount(channelId string, allowFromCache bool) StoreChannel
+	GetMemberCount(channelId string, allowFromCache bool) (int64, *model.AppError)
 	GetPinnedPosts(channelId string) StoreChannel
 	RemoveMember(channelId string, userId string) *model.AppError
 	PermanentDeleteMembersByUser(userId string) StoreChannel

store/storetest/channel_store.go

@@ -888,12 +888,14 @@ func testChannelMemberStore(t *testing.T, ss store.Store) {
 	c1t2, _ := ss.Channel().Get(c1.Id, false)
 	assert.EqualValues(t, 0, c1t2.ExtraUpdateAt, "ExtraUpdateAt should be 0")
 
-	count := (<-ss.Channel().GetMemberCount(o1.ChannelId, true)).Data.(int64)
+	count, err := ss.Channel().GetMemberCount(o1.ChannelId, true)
+	require.Nil(t, err)
 	if count != 2 {
 		t.Fatal("should have saved 2 members")
 	}
 
-	count = (<-ss.Channel().GetMemberCount(o1.ChannelId, true)).Data.(int64)
+	count, err = ss.Channel().GetMemberCount(o1.ChannelId, true)
+	require.Nil(t, err)
 	if count != 2 {
 		t.Fatal("should have saved 2 members")
 	}
@@ -906,7 +908,8 @@ func testChannelMemberStore(t *testing.T, ss store.Store) {
 		t.Fatal("should have saved 0 members")
 	}
 
-	count = (<-ss.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64)
+	count, err = ss.Channel().GetMemberCount(o1.ChannelId, false)
+	require.Nil(t, err)
 	if count != 2 {
 		t.Fatal("should have saved 2 members")
 	}
@@ -914,7 +917,8 @@ func testChannelMemberStore(t *testing.T, ss store.Store) {
 	err = ss.Channel().RemoveMember(o2.ChannelId, o2.UserId)
 	require.Nil(t, err)
 
-	count = (<-ss.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64)
+	count, err = ss.Channel().GetMemberCount(o1.ChannelId, false)
+	require.Nil(t, err)
 	if count != 1 {
 		t.Fatal("should have removed 1 member")
 	}
@@ -974,23 +978,26 @@ func testChannelDeleteMemberStore(t *testing.T, ss store.Store) {
 	c1t2, _ := ss.Channel().Get(c1.Id, false)
 	assert.EqualValues(t, 0, c1t2.ExtraUpdateAt, "ExtraUpdateAt should be 0")
 
-	count := (<-ss.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64)
+	count, err := ss.Channel().GetMemberCount(o1.ChannelId, false)
+	require.Nil(t, err)
 	if count != 2 {
 		t.Fatal("should have saved 2 members")
 	}
 
 	store.Must(ss.Channel().PermanentDeleteMembersByUser(o2.UserId))
 
-	count = (<-ss.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64)
+	count, err = ss.Channel().GetMemberCount(o1.ChannelId, false)
+	require.Nil(t, err)
 	if count != 1 {
 		t.Fatal("should have removed 1 member")
 	}
 
-	if err := ss.Channel().PermanentDeleteMembersByChannel(o1.ChannelId); err != nil {
+	if err = ss.Channel().PermanentDeleteMembersByChannel(o1.ChannelId); err != nil {
 		t.Fatal(err)
 	}
 
-	count = (<-ss.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64)
+	count, err = ss.Channel().GetMemberCount(o1.ChannelId, false)
+	require.Nil(t, err)
 	if count != 0 {
 		t.Fatal("should have removed all members")
 	}
@@ -1911,10 +1918,10 @@ func testGetMemberCount(t *testing.T, ss store.Store) {
 	}
 	store.Must(ss.Channel().SaveMember(&m1))
 
-	if result := <-ss.Channel().GetMemberCount(c1.Id, false); result.Err != nil {
+	if count, err := ss.Channel().GetMemberCount(c1.Id, false); err != nil {
-		t.Fatalf("failed to get member count: %v", result.Err)
+		t.Fatalf("failed to get member count: %v", err)
-	} else if result.Data.(int64) != 1 {
+	} else if count != 1 {
-		t.Fatalf("got incorrect member count %v", result.Data)
+		t.Fatalf("got incorrect member count %v", count)
 	}
 
 	u2 := model.User{
@@ -1931,10 +1938,10 @@ func testGetMemberCount(t *testing.T, ss store.Store) {
 	}
 	store.Must(ss.Channel().SaveMember(&m2))
 
-	if result := <-ss.Channel().GetMemberCount(c1.Id, false); result.Err != nil {
+	if count, err := ss.Channel().GetMemberCount(c1.Id, false); err != nil {
-		t.Fatalf("failed to get member count: %v", result.Err)
+		t.Fatalf("failed to get member count: %v", err)
-	} else if result.Data.(int64) != 2 {
+	} else if count != 2 {
-		t.Fatalf("got incorrect member count %v", result.Data)
+		t.Fatalf("got incorrect member count %v", count)
 	}
 
 	// make sure members of other channels aren't counted
@@ -1952,10 +1959,10 @@ func testGetMemberCount(t *testing.T, ss store.Store) {
 	}
 	store.Must(ss.Channel().SaveMember(&m3))
 
-	if result := <-ss.Channel().GetMemberCount(c1.Id, false); result.Err != nil {
+	if count, err := ss.Channel().GetMemberCount(c1.Id, false); err != nil {
-		t.Fatalf("failed to get member count: %v", result.Err)
+		t.Fatalf("failed to get member count: %v", err)
-	} else if result.Data.(int64) != 2 {
+	} else if count != 2 {
-		t.Fatalf("got incorrect member count %v", result.Data)
+		t.Fatalf("got incorrect member count %v", count)
 	}
 
 	// make sure inactive users aren't counted
@@ -1973,10 +1980,10 @@ func testGetMemberCount(t *testing.T, ss store.Store) {
 	}
 	store.Must(ss.Channel().SaveMember(&m4))
 
-	if result := <-ss.Channel().GetMemberCount(c1.Id, false); result.Err != nil {
+	if count, err := ss.Channel().GetMemberCount(c1.Id, false); err != nil {
-		t.Fatalf("failed to get member count: %v", result.Err)
+		t.Fatalf("failed to get member count: %v", err)
-	} else if result.Data.(int64) != 2 {
+	} else if count != 2 {
-		t.Fatalf("got incorrect member count %v", result.Data)
+		t.Fatalf("got incorrect member count %v", count)
 	}
 }
 

store/storetest/mocks/ChannelStore.go

@@ -693,19 +693,26 @@ func (_m *ChannelStore) GetMember(channelId string, userId string) (*model.Chann
 }
 
 // GetMemberCount provides a mock function with given fields: channelId, allowFromCache
-func (_m *ChannelStore) GetMemberCount(channelId string, allowFromCache bool) store.StoreChannel {
+func (_m *ChannelStore) GetMemberCount(channelId string, allowFromCache bool) (int64, *model.AppError) {
 	ret := _m.Called(channelId, allowFromCache)
 
-	var r0 store.StoreChannel
+	var r0 int64
-	if rf, ok := ret.Get(0).(func(string, bool) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string, bool) int64); ok {
 		r0 = rf(channelId, allowFromCache)
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Get(0).(int64)
-			r0 = ret.Get(0).(store.StoreChannel)
+	}
+
+	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
+	return r0, r1
 }
 
 // GetMemberCountFromCache provides a mock function with given fields: channelId