MM-15198 Migrate Channel.Get/GetFromMaster to Sync by default (#10667)

* MM-15198 Migrate Channel.Get/GetFromMaster to Sync by default * MM-15198 - Update store/storetest/post_store.go fix error handling in post_store.go test case Co-Authored-By: andresoro <ao15@my.fsu.edu>
2019-04-24 15:28:06 -04:00
--- a/store/sqlstore/channel_store.go
+++ b/store/sqlstore/channel_store.go
@@ -690,7 +690,7 @@ func (s SqlChannelStore) InvalidateChannelByName(teamId, name string) {
 	}
 }

-func (s SqlChannelStore) Get(id string, allowFromCache bool) store.StoreChannel {
+func (s SqlChannelStore) Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) {
 	return s.get(id, false, allowFromCache)
 }

@@ -712,47 +712,45 @@ func (s SqlChannelStore) GetPinnedPosts(channelId string) store.StoreChannel {
 	})
 }

-func (s SqlChannelStore) GetFromMaster(id string) store.StoreChannel {
+func (s SqlChannelStore) GetFromMaster(id string) (*model.Channel, *model.AppError) {
 	return s.get(id, true, false)
 }

-func (s SqlChannelStore) get(id string, master bool, allowFromCache bool) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		var db *gorp.DbMap
-		if master {
-			db = s.GetMaster()
-		} else {
-			db = s.GetReplica()
-		}
+func (s SqlChannelStore) get(id string, master bool, allowFromCache bool) (*model.Channel, *model.AppError) {
+	var db *gorp.DbMap

-		if allowFromCache {
-			if cacheItem, ok := channelCache.Get(id); ok {
-				if s.metrics != nil {
-					s.metrics.IncrementMemCacheHitCounter("Channel")
-				}
-				result.Data = (cacheItem.(*model.Channel)).DeepCopy()
-				return
+	if master {
+		db = s.GetMaster()
+	} else {
+		db = s.GetReplica()
+	}
+
+	if allowFromCache {
+		if cacheItem, ok := channelCache.Get(id); ok {
+			if s.metrics != nil {
+				s.metrics.IncrementMemCacheHitCounter("Channel")
 			}
+			ch := cacheItem.(*model.Channel).DeepCopy()
+			return ch, nil
 		}
+	}

-		if s.metrics != nil {
-			s.metrics.IncrementMemCacheMissCounter("Channel")
-		}
+	if s.metrics != nil {
+		s.metrics.IncrementMemCacheMissCounter("Channel")
+	}

-		obj, err := db.Get(model.Channel{}, id)
-		if err != nil {
-			result.Err = model.NewAppError("SqlChannelStore.Get", "store.sql_channel.get.find.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
-			return
-		}
+	obj, err := db.Get(model.Channel{}, id)
+	if err != nil {
+		return nil, model.NewAppError("SqlChannelStore.Get", "store.sql_channel.get.find.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError)
+	}

-		if obj == nil {
-			result.Err = model.NewAppError("SqlChannelStore.Get", "store.sql_channel.get.existing.app_error", nil, "id="+id, http.StatusNotFound)
-			return
-		}
+	if obj == nil {
+		return nil, model.NewAppError("SqlChannelStore.Get", "store.sql_channel.get.existing.app_error", nil, "id="+id, http.StatusNotFound)
+	}

-		result.Data = obj.(*model.Channel)
-		channelCache.AddWithExpiresInSecs(id, obj.(*model.Channel), CHANNEL_CACHE_SEC)
-	})
+	ch := obj.(*model.Channel)
+	channelCache.AddWithExpiresInSecs(id, ch, CHANNEL_CACHE_SEC)
+	return ch, nil
 }

 // Delete records the given deleted timestamp to the channel in question.
@@ -1289,14 +1287,12 @@ func (s SqlChannelStore) SaveMember(member *model.ChannelMember) store.StoreChan
 		defer s.InvalidateAllChannelMembersForUser(member.UserId)

 		// Grab the channel we are saving this member to
-		cr := <-s.GetFromMaster(member.ChannelId)
-		if cr.Err != nil {
-			result.Err = cr.Err
+		channel, errCh := s.GetFromMaster(member.ChannelId)
+		if errCh != nil {
+			result.Err = errCh
 			return
 		}

-		channel := cr.Data.(*model.Channel)
-
 		transaction, err := s.GetMaster().Begin()
 		if err != nil {
 			result.Err = model.NewAppError("SqlChannelStore.SaveMember", "store.sql_channel.save_member.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
--- a/store/sqlstore/group_supplier.go
+++ b/store/sqlstore/group_supplier.go
@@ -406,9 +406,9 @@ func (s *SqlSupplier) GroupCreateGroupSyncable(ctx context.Context, groupSyncabl

 		err = s.GetMaster().Insert(groupSyncableToGroupTeam(groupSyncable))
 	case model.GroupSyncableTypeChannel:
-		channelResult := <-s.Channel().Get(groupSyncable.SyncableId, false)
-		if channelResult.Err != nil {
-			result.Err = channelResult.Err
+		_, errCh := s.Channel().Get(groupSyncable.SyncableId, false)
+		if errCh != nil {
+			result.Err = errCh
 			return result
 		}

--- a/store/store.go
+++ b/store/store.go
@@ -129,10 +129,10 @@ type ChannelStore interface {
 	CreateDirectChannel(userId string, otherUserId string) StoreChannel
 	SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) StoreChannel
 	Update(channel *model.Channel) StoreChannel
-	Get(id string, allowFromCache bool) StoreChannel
+	Get(id string, allowFromCache bool) (*model.Channel, *model.AppError)
 	InvalidateChannel(id string)
 	InvalidateChannelByName(teamId, name string)
-	GetFromMaster(id string) StoreChannel
+	GetFromMaster(id string) (*model.Channel, *model.AppError)
 	Delete(channelId string, time int64) StoreChannel
 	Restore(channelId string, time int64) StoreChannel
 	SetDeleteAt(channelId string, deleteAt int64, updateAt int64) StoreChannel
--- a/store/storetest/channel_store.go
+++ b/store/storetest/channel_store.go
@@ -356,15 +356,15 @@ func testChannelStoreGet(t *testing.T, ss store.Store, s SqlSupplier) {
 	o1.Type = model.CHANNEL_OPEN
 	store.Must(ss.Channel().Save(&o1, -1))

-	if r1 := <-ss.Channel().Get(o1.Id, false); r1.Err != nil {
-		t.Fatal(r1.Err)
+	if c1, err := ss.Channel().Get(o1.Id, false); err != nil {
+		t.Fatal(err)
 	} else {
-		if r1.Data.(*model.Channel).ToJson() != o1.ToJson() {
+		if c1.ToJson() != o1.ToJson() {
 			t.Fatal("invalid returned channel")
 		}
 	}

-	if err := (<-ss.Channel().Get("", false)).Err; err == nil {
+	if _, err := ss.Channel().Get("", false); err == nil {
 		t.Fatal("Missing id should have failed")
 	}

@@ -398,18 +398,18 @@ func testChannelStoreGet(t *testing.T, ss store.Store, s SqlSupplier) {

 	store.Must(ss.Channel().SaveDirectChannel(&o2, &m1, &m2))

-	if r2 := <-ss.Channel().Get(o2.Id, false); r2.Err != nil {
-		t.Fatal(r2.Err)
+	if c2, err := ss.Channel().Get(o2.Id, false); err != nil {
+		t.Fatal(err)
 	} else {
-		if r2.Data.(*model.Channel).ToJson() != o2.ToJson() {
+		if c2.ToJson() != o2.ToJson() {
 			t.Fatal("invalid returned channel")
 		}
 	}

-	if r4 := <-ss.Channel().Get(o2.Id, true); r4.Err != nil {
-		t.Fatal(r4.Err)
+	if c4, err := ss.Channel().Get(o2.Id, true); err != nil {
+		t.Fatal(err)
 	} else {
-		if r4.Data.(*model.Channel).ToJson() != o2.ToJson() {
+		if c4.ToJson() != o2.ToJson() {
 			t.Fatal("invalid returned channel")
 		}
 	}
@@ -535,7 +535,7 @@ func testChannelStoreRestore(t *testing.T, ss store.Store) {
 		t.Fatal(r.Err)
 	}

-	if r := <-ss.Channel().Get(o1.Id, false); r.Data.(*model.Channel).DeleteAt == 0 {
+	if c, _ := ss.Channel().Get(o1.Id, false); c.DeleteAt == 0 {
 		t.Fatal("should have been deleted")
 	}

@@ -543,7 +543,7 @@ func testChannelStoreRestore(t *testing.T, ss store.Store) {
 		t.Fatal(r.Err)
 	}

-	if r := <-ss.Channel().Get(o1.Id, false); r.Data.(*model.Channel).DeleteAt != 0 {
+	if c, _ := ss.Channel().Get(o1.Id, false); c.DeleteAt != 0 {
 		t.Fatal("should have been restored")
 	}

@@ -594,7 +594,7 @@ func testChannelStoreDelete(t *testing.T, ss store.Store) {
 		t.Fatal(r.Err)
 	}

-	if r := <-ss.Channel().Get(o1.Id, false); r.Data.(*model.Channel).DeleteAt == 0 {
+	if c, _ := ss.Channel().Get(o1.Id, false); c.DeleteAt == 0 {
 		t.Fatal("should have been deleted")
 	}

@@ -826,7 +826,7 @@ func testChannelMemberStore(t *testing.T, ss store.Store) {
 	c1.Type = model.CHANNEL_OPEN
 	c1 = *store.Must(ss.Channel().Save(&c1, -1)).(*model.Channel)

-	c1t1 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel)
+	c1t1, _ := ss.Channel().Get(c1.Id, false)
 	assert.EqualValues(t, 0, c1t1.ExtraUpdateAt, "ExtraUpdateAt should be 0")

 	u1 := model.User{}
@@ -853,7 +853,7 @@ func testChannelMemberStore(t *testing.T, ss store.Store) {
 	o2.NotifyProps = model.GetDefaultChannelNotifyProps()
 	store.Must(ss.Channel().SaveMember(&o2))

-	c1t2 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel)
+	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)
@@ -886,7 +886,7 @@ func testChannelMemberStore(t *testing.T, ss store.Store) {
 		t.Fatal("should have removed 1 member")
 	}

-	c1t3 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel)
+	c1t3, _ := ss.Channel().Get(c1.Id, false)
 	assert.EqualValues(t, 0, c1t3.ExtraUpdateAt, "ExtraUpdateAt should be 0")

 	member, _ := ss.Channel().GetMember(o1.ChannelId, o1.UserId)
@@ -898,7 +898,7 @@ func testChannelMemberStore(t *testing.T, ss store.Store) {
 		t.Fatal("Should have been a duplicate")
 	}

-	c1t4 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel)
+	c1t4, _ := ss.Channel().Get(c1.Id, false)
 	assert.EqualValues(t, 0, c1t4.ExtraUpdateAt, "ExtraUpdateAt should be 0")
 }

@@ -910,7 +910,7 @@ func testChannelDeleteMemberStore(t *testing.T, ss store.Store) {
 	c1.Type = model.CHANNEL_OPEN
 	c1 = *store.Must(ss.Channel().Save(&c1, -1)).(*model.Channel)

-	c1t1 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel)
+	c1t1, _ := ss.Channel().Get(c1.Id, false)
 	assert.EqualValues(t, 0, c1t1.ExtraUpdateAt, "ExtraUpdateAt should be 0")

 	u1 := model.User{}
@@ -937,7 +937,7 @@ func testChannelDeleteMemberStore(t *testing.T, ss store.Store) {
 	o2.NotifyProps = model.GetDefaultChannelNotifyProps()
 	store.Must(ss.Channel().SaveMember(&o2))

-	c1t2 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel)
+	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)
@@ -2870,8 +2870,8 @@ func testResetAllChannelSchemes(t *testing.T, ss store.Store) {
 	res := <-ss.Channel().ResetAllChannelSchemes()
 	assert.Nil(t, res.Err)

-	c1 = (<-ss.Channel().Get(c1.Id, true)).Data.(*model.Channel)
-	c2 = (<-ss.Channel().Get(c2.Id, true)).Data.(*model.Channel)
+	c1, _ = ss.Channel().Get(c1.Id, true)
+	c2, _ = ss.Channel().Get(c2.Id, true)

 	assert.Equal(t, "", *c1.SchemeId)
 	assert.Equal(t, "", *c2.SchemeId)
--- a/store/storetest/mocks/ChannelStore.go
+++ b/store/storetest/mocks/ChannelStore.go
@@ -131,19 +131,28 @@ func (_m *ChannelStore) Delete(channelId string, time int64) store.StoreChannel
 }

 // Get provides a mock function with given fields: id, allowFromCache
-func (_m *ChannelStore) Get(id string, allowFromCache bool) store.StoreChannel {
+func (_m *ChannelStore) Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) {
 	ret := _m.Called(id, allowFromCache)

-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string, bool) store.StoreChannel); ok {
+	var r0 *model.Channel
+	if rf, ok := ret.Get(0).(func(string, bool) *model.Channel); ok {
 		r0 = rf(id, allowFromCache)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.Channel)
 		}
 	}

-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, bool) *model.AppError); ok {
+		r1 = rf(id, allowFromCache)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }

 // GetAll provides a mock function with given fields: teamId
@@ -467,19 +476,28 @@ func (_m *ChannelStore) GetForPost(postId string) store.StoreChannel {
 }

 // GetFromMaster provides a mock function with given fields: id
-func (_m *ChannelStore) GetFromMaster(id string) store.StoreChannel {
+func (_m *ChannelStore) GetFromMaster(id string) (*model.Channel, *model.AppError) {
 	ret := _m.Called(id)

-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	var r0 *model.Channel
+	if rf, ok := ret.Get(0).(func(string) *model.Channel); ok {
 		r0 = rf(id)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.Channel)
 		}
 	}

-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+		r1 = rf(id)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }

 // GetMember provides a mock function with given fields: channelId, userId
--- a/store/storetest/post_store.go
+++ b/store/storetest/post_store.go
@@ -80,9 +80,8 @@ func testPostStoreSaveChannelMsgCounts(t *testing.T, ss store.Store) {

 	require.Nil(t, (<-ss.Post().Save(&o1)).Err)

-	res = <-ss.Channel().Get(c1.Id, false)
-	require.Nil(t, res.Err)
-	c1 = res.Data.(*model.Channel)
+	c1, err := ss.Channel().Get(c1.Id, false)
+	require.Nil(t, err)
 	assert.Equal(t, int64(1), c1.TotalMsgCount, "Message count should update by 1")

 	o1.Id = ""
@@ -93,9 +92,8 @@ func testPostStoreSaveChannelMsgCounts(t *testing.T, ss store.Store) {
 	o1.Type = model.POST_REMOVE_FROM_TEAM
 	require.Nil(t, (<-ss.Post().Save(&o1)).Err)

-	res = <-ss.Channel().Get(c1.Id, false)
-	require.Nil(t, res.Err)
-	c1 = res.Data.(*model.Channel)
+	c1, err = ss.Channel().Get(c1.Id, false)
+	require.Nil(t, err)
 	assert.Equal(t, int64(1), c1.TotalMsgCount, "Message count should not update for team add/removed message")

 	oldLastPostAt := c1.LastPostAt
@@ -107,9 +105,8 @@ func testPostStoreSaveChannelMsgCounts(t *testing.T, ss store.Store) {
 	o2.CreateAt = int64(7)
 	require.Nil(t, (<-ss.Post().Save(&o2)).Err)

-	res = <-ss.Channel().Get(c1.Id, false)
-	require.Nil(t, res.Err)
-	c1 = res.Data.(*model.Channel)
+	c1, err = ss.Channel().Get(c1.Id, false)
+	require.Nil(t, err)
 	assert.Equal(t, oldLastPostAt, c1.LastPostAt, "LastPostAt should not update for old message save")
 }

--- a/store/storetest/scheme_store.go
+++ b/store/storetest/scheme_store.go
@@ -422,9 +422,8 @@ func testSchemeStoreDelete(t *testing.T, ss store.Store) {
 	sres5 := <-ss.Scheme().Delete(d5.Id)
 	assert.Nil(t, sres5.Err)

-	cres6 := <-ss.Channel().Get(c5.Id, true)
-	assert.Nil(t, cres6.Err)
-	c6 := cres6.Data.(*model.Channel)
+	c6, err := ss.Channel().Get(c5.Id, true)
+	assert.Nil(t, err)
 	assert.Equal(t, "", *c6.SchemeId)
 }