From 427effcd5c8283959c42ac74177e53e886151276 Mon Sep 17 00:00:00 2001
From: Evan do Carmo <34245571+carmo-evan@users.noreply.github.com>
Date: Thu, 30 May 2019 08:43:45 -0400
Subject: [PATCH] MM-15843 migrating post.GetSingle() to sync by default
 (#10992)

---
 app/integration_action.go          | 12 +++++++++---
 app/integration_action_test.go     |  6 ++----
 app/plugin_hooks_test.go           | 14 ++++++--------
 app/post.go                        | 23 ++++++++++++-----------
 store/sqlstore/post_store.go       | 17 +++++++----------
 store/store.go                     |  2 +-
 store/storetest/mocks/PostStore.go | 19 ++++++++++++++-----
 store/storetest/post_store.go      |  8 ++++----
 8 files changed, 55 insertions(+), 46 deletions(-)

diff --git a/app/integration_action.go b/app/integration_action.go
index 7091c53224..0706b789c5 100644
--- a/app/integration_action.go
+++ b/app/integration_action.go
@@ -27,6 +27,7 @@ import (
 	"strings"
 
 	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/store"
 	"github.com/mattermost/mattermost-server/utils"
 )
 
@@ -61,8 +62,15 @@ func (a *App) DoPostActionWithCookie(postId, actionId, userId, selectedOption st
 
 	// See if the post exists in the DB, if so ignore the cookie.
 	// Start all queries here for parallel execution
-	pchan := a.Srv.Store.Post().GetSingle(postId)
+	pchan := make(chan store.StoreResult, 1)
+	go func() {
+		post, err := a.Srv.Store.Post().GetSingle(postId)
+		pchan <- store.StoreResult{Data: post, Err: err}
+		close(pchan)
+	}()
+
 	cchan := a.Srv.Store.Channel().GetForPost(postId)
+
 	result := <-pchan
 	if result.Err != nil {
 		if cookie == nil {
@@ -86,9 +94,7 @@ func (a *App) DoPostActionWithCookie(postId, actionId, userId, selectedOption st
 		rootPostId = cookie.RootPostId
 		upstreamURL = cookie.Integration.URL
 	} else {
-		// Get action metadata from the database
 		post := result.Data.(*model.Post)
-
 		result = <-cchan
 		if result.Err != nil {
 			return "", result.Err
diff --git a/app/integration_action_test.go b/app/integration_action_test.go
index 6ace637204..06c248ed56 100644
--- a/app/integration_action_test.go
+++ b/app/integration_action_test.go
@@ -381,10 +381,8 @@ func TestPostActionProps(t *testing.T) {
 	require.Nil(t, err)
 	assert.True(t, len(clientTriggerId) == 26)
 
-	pchan := th.App.Srv.Store.Post().GetSingle(post.Id)
-	result := <-pchan
-	require.Nil(t, result.Err)
-	newPost := result.Data.(*model.Post)
+	newPost, err := th.App.Srv.Store.Post().GetSingle(post.Id)
+	require.Nil(t, err)
 
 	assert.True(t, newPost.IsPinned)
 	assert.False(t, newPost.HasReactions)
diff --git a/app/plugin_hooks_test.go b/app/plugin_hooks_test.go
index a6f8da8bd9..b85b23790d 100644
--- a/app/plugin_hooks_test.go
+++ b/app/plugin_hooks_test.go
@@ -176,11 +176,9 @@ func TestHookMessageWillBePosted(t *testing.T) {
 			t.Fatal(err)
 		}
 		assert.Equal(t, "message", post.Message)
-		if result := <-th.App.Srv.Store.Post().GetSingle(post.Id); result.Err != nil {
-			t.Fatal(err)
-		} else {
-			assert.Equal(t, "message", result.Data.(*model.Post).Message)
-		}
+		retrievedPost, errSingle := th.App.Srv.Store.Post().GetSingle(post.Id)
+		require.Nil(t, errSingle)
+		assert.Equal(t, "message", retrievedPost.Message)
 	})
 
 	t.Run("updated", func(t *testing.T) {
@@ -223,10 +221,10 @@ func TestHookMessageWillBePosted(t *testing.T) {
 			t.Fatal(err)
 		}
 		assert.Equal(t, "message_fromplugin", post.Message)
-		if result := <-th.App.Srv.Store.Post().GetSingle(post.Id); result.Err != nil {
-			t.Fatal(err)
+		if retrievedPost, errSingle := th.App.Srv.Store.Post().GetSingle(post.Id); err != nil {
+			t.Fatal(errSingle)
 		} else {
-			assert.Equal(t, "message_fromplugin", result.Data.(*model.Post).Message)
+			assert.Equal(t, "message_fromplugin", retrievedPost.Message)
 		}
 	})
 
diff --git a/app/post.go b/app/post.go
index 199ce06522..2f795167de 100644
--- a/app/post.go
+++ b/app/post.go
@@ -627,11 +627,7 @@ func (a *App) GetPostsSince(channelId string, time int64) (*model.PostList, *mod
 }
 
 func (a *App) GetSinglePost(postId string) (*model.Post, *model.AppError) {
-	result := <-a.Srv.Store.Post().GetSingle(postId)
-	if result.Err != nil {
-		return nil, result.Err
-	}
-	return result.Data.(*model.Post), nil
+	return a.Srv.Store.Post().GetSingle(postId)
 }
 
 func (a *App) GetPostThread(postId string) (*model.PostList, *model.AppError) {
@@ -709,12 +705,11 @@ func (a *App) GetPostsAroundPost(postId, channelId string, offset, limit int, be
 }
 
 func (a *App) DeletePost(postId, deleteByID string) (*model.Post, *model.AppError) {
-	result := <-a.Srv.Store.Post().GetSingle(postId)
-	if result.Err != nil {
-		result.Err.StatusCode = http.StatusBadRequest
-		return nil, result.Err
+	post, err := a.Srv.Store.Post().GetSingle(postId)
+	if err != nil {
+		err.StatusCode = http.StatusBadRequest
+		return nil, err
 	}
-	post := result.Data.(*model.Post)
 
 	channel, err := a.GetChannel(post.ChannelId)
 	if err != nil {
@@ -945,7 +940,13 @@ func (a *App) SearchPostsInTeamForUser(terms string, userId string, teamId strin
 }
 
 func (a *App) GetFileInfosForPostWithMigration(postId string) ([]*model.FileInfo, *model.AppError) {
-	pchan := a.Srv.Store.Post().GetSingle(postId)
+
+	pchan := make(chan store.StoreResult, 1)
+	go func() {
+		post, err := a.Srv.Store.Post().GetSingle(postId)
+		pchan <- store.StoreResult{Data: post, Err: err}
+		close(pchan)
+	}()
 
 	infos, err := a.GetFileInfosForPost(postId, false)
 	if err != nil {
diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go
index d7d213dceb..4b62c20b8b 100644
--- a/store/sqlstore/post_store.go
+++ b/store/sqlstore/post_store.go
@@ -304,16 +304,13 @@ func (s *SqlPostStore) Get(id string) (*model.PostList, *model.AppError) {
 	return pl, nil
 }
 
-func (s *SqlPostStore) GetSingle(id string) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		var post model.Post
-		err := s.GetReplica().SelectOne(&post, "SELECT * FROM Posts WHERE Id = :Id AND DeleteAt = 0", map[string]interface{}{"Id": id})
-		if err != nil {
-			result.Err = model.NewAppError("SqlPostStore.GetSingle", "store.sql_post.get.app_error", nil, "id="+id+err.Error(), http.StatusNotFound)
-		}
-
-		result.Data = &post
-	})
+func (s *SqlPostStore) GetSingle(id string) (*model.Post, *model.AppError) {
+	var post model.Post
+	err := s.GetReplica().SelectOne(&post, "SELECT * FROM Posts WHERE Id = :Id AND DeleteAt = 0", map[string]interface{}{"Id": id})
+	if err != nil {
+		return nil, model.NewAppError("SqlPostStore.GetSingle", "store.sql_post.get.app_error", nil, "id="+id+err.Error(), http.StatusNotFound)
+	}
+	return &post, nil
 }
 
 type etagPosts struct {
diff --git a/store/store.go b/store/store.go
index 97917fe87c..d1a6873eb5 100644
--- a/store/store.go
+++ b/store/store.go
@@ -213,7 +213,7 @@ type PostStore interface {
 	Save(post *model.Post) StoreChannel
 	Update(newPost *model.Post, oldPost *model.Post) (*model.Post, *model.AppError)
 	Get(id string) (*model.PostList, *model.AppError)
-	GetSingle(id string) StoreChannel
+	GetSingle(id string) (*model.Post, *model.AppError)
 	Delete(postId string, time int64, deleteByID string) *model.AppError
 	PermanentDeleteByUser(userId string) StoreChannel
 	PermanentDeleteByChannel(channelId string) *model.AppError
diff --git a/store/storetest/mocks/PostStore.go b/store/storetest/mocks/PostStore.go
index d464188dd0..6c76081090 100644
--- a/store/storetest/mocks/PostStore.go
+++ b/store/storetest/mocks/PostStore.go
@@ -380,19 +380,28 @@ func (_m *PostStore) GetRepliesForExport(parentId string) store.StoreChannel {
 }
 
 // GetSingle provides a mock function with given fields: id
-func (_m *PostStore) GetSingle(id string) store.StoreChannel {
+func (_m *PostStore) GetSingle(id string) (*model.Post, *model.AppError) {
 	ret := _m.Called(id)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+	var r0 *model.Post
+	if rf, ok := ret.Get(0).(func(string) *model.Post); ok {
 		r0 = rf(id)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.Post)
 		}
 	}
 
-	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
 }
 
 // InvalidateLastPostTimeCache provides a mock function with given fields: channelId
diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go
index b25822629b..d11a411d1e 100644
--- a/store/storetest/post_store.go
+++ b/store/storetest/post_store.go
@@ -153,15 +153,15 @@ func testPostStoreGetSingle(t *testing.T, ss store.Store) {
 
 	o1 = (<-ss.Post().Save(o1)).Data.(*model.Post)
 
-	if r1 := <-ss.Post().GetSingle(o1.Id); r1.Err != nil {
-		t.Fatal(r1.Err)
+	if post, err := ss.Post().GetSingle(o1.Id); err != nil {
+		t.Fatal(err)
 	} else {
-		if r1.Data.(*model.Post).CreateAt != o1.CreateAt {
+		if post.CreateAt != o1.CreateAt {
 			t.Fatal("invalid returned post")
 		}
 	}
 
-	if err := (<-ss.Post().GetSingle("123")).Err; err == nil {
+	if _, err := ss.Post().GetSingle("123"); err == nil {
 		t.Fatal("Missing id should have failed")
 	}
 }