Migrate Overwrite and GetMaxPostSize() in PostStore to sync by default (#10829)

* Migrate Overwrite and GetMaxPostSize() in PostStore to sync by default

* GH-10762: fix return type for mock func

* GH-10762: fix tests for MaxPostSize() mocks

* fix imports

app/import_functions.go

@@ -898,8 +898,8 @@ func (a *App) ImportReply(data *ReplyImportData, post *model.Post, teamId string
 			return result.Err
 		}
 	} else {
-		if result := <-a.Srv.Store.Post().Overwrite(reply); result.Err != nil {
+		if _, err := a.Srv.Store.Post().Overwrite(reply); err != nil {
-			return result.Err
+			return err
 		}
 	}
 
@@ -1000,8 +1000,8 @@ func (a *App) ImportPost(data *PostImportData, dryRun bool) *model.AppError {
 			return result.Err
 		}
 	} else {
-		if result := <-a.Srv.Store.Post().Overwrite(post); result.Err != nil {
+		if _, err := a.Srv.Store.Post().Overwrite(post); err != nil {
-			return result.Err
+			return err
 		}
 	}
 
@@ -1224,8 +1224,8 @@ func (a *App) ImportDirectPost(data *DirectPostImportData, dryRun bool) *model.A
 			return result.Err
 		}
 	} else {
-		if result := <-a.Srv.Store.Post().Overwrite(post); result.Err != nil {
+		if _, err := a.Srv.Store.Post().Overwrite(post); err != nil {
-			return result.Err
+			return err
 		}
 	}
 

app/post.go

@@ -315,9 +315,8 @@ func (a *App) attachFilesToPost(post *model.Post) *model.AppError {
 		// We couldn't attach all files to the post, so ensure that post.FileIds reflects what was actually attached
 		post.FileIds = attachedIds
 
-		result := <-a.Srv.Store.Post().Overwrite(post)
+		if _, err := a.Srv.Store.Post().Overwrite(post); err != nil {
-		if result.Err != nil {
+			return err
-			return result.Err
 		}
 	}
 
@@ -1027,10 +1026,10 @@ func (a *App) ImageProxyRemover() (f func(string) string) {
 }
 
 func (a *App) MaxPostSize() int {
-	result := <-a.Srv.Store.Post().GetMaxPostSize()
+	maxPostSize := a.Srv.Store.Post().GetMaxPostSize()
-	if result.Err != nil {
+	if maxPostSize == 0 {
-		mlog.Error(fmt.Sprint(result.Err))
 		return model.POST_MESSAGE_MAX_RUNES_V1
 	}
-	return result.Data.(int)
+
+	return maxPostSize
 }

app/post_test.go

@@ -14,7 +14,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/mattermost/mattermost-server/model"
-	"github.com/mattermost/mattermost-server/store"
 	"github.com/mattermost/mattermost-server/store/storetest"
 )
 
@@ -565,25 +564,21 @@ func TestMaxPostSize(t *testing.T) {
 		Description         string
 		StoreMaxPostSize    int
 		ExpectedMaxPostSize int
-		ExpectedError       *model.AppError
 	}{
 		{
-			"error fetching max post size",
+			"Max post size less than model.model.POST_MESSAGE_MAX_RUNES_V1 ",
 			0,
 			model.POST_MESSAGE_MAX_RUNES_V1,
-			model.NewAppError("TestMaxPostSize", "this is an error", nil, "", http.StatusBadRequest),
 		},
 		{
 			"4000 rune limit",
 			4000,
 			4000,
-			nil,
 		},
 		{
 			"16383 rune limit",
 			16383,
 			16383,
-			nil,
 		},
 	}
 
@@ -595,12 +590,7 @@ func TestMaxPostSize(t *testing.T) {
 			mockStore := &storetest.Store{}
 			defer mockStore.AssertExpectations(t)
 
-			mockStore.PostStore.On("GetMaxPostSize").Return(
+			mockStore.PostStore.On("GetMaxPostSize").Return(testCase.StoreMaxPostSize)
-				storetest.NewStoreChannel(store.StoreResult{
-					Data: testCase.StoreMaxPostSize,
-					Err:  testCase.ExpectedError,
-				}),
-			)
 
 			app := App{
 				Srv: &Server{

store/sqlstore/post_store.go

@@ -97,13 +97,7 @@ func (s *SqlPostStore) Save(post *model.Post) store.StoreChannel {
 			return
 		}
 
-		var maxPostSize int
+		maxPostSize := s.GetMaxPostSize()
-		if result := <-s.GetMaxPostSize(); result.Err != nil {
-			result.Err = model.NewAppError("SqlPostStore.Save", "store.sql_post.save.app_error", nil, "id="+post.Id+", "+result.Err.Error(), http.StatusInternalServerError)
-			return
-		} else {
-			maxPostSize = result.Data.(int)
-		}
 
 		post.PreSave()
 		if result.Err = post.IsValid(maxPostSize); result.Err != nil {
@@ -146,13 +140,7 @@ func (s *SqlPostStore) Update(newPost *model.Post, oldPost *model.Post) store.St
 		oldPost.Id = model.NewId()
 		oldPost.PreCommit()
 
-		var maxPostSize int
+		maxPostSize := s.GetMaxPostSize()
-		if result := <-s.GetMaxPostSize(); result.Err != nil {
-			result.Err = model.NewAppError("SqlPostStore.Save", "store.sql_post.update.app_error", nil, "id="+newPost.Id+", "+result.Err.Error(), http.StatusInternalServerError)
-			return
-		} else {
-			maxPostSize = result.Data.(int)
-		}
 
 		if result.Err = newPost.IsValid(maxPostSize); result.Err != nil {
 			return
@@ -176,28 +164,19 @@ func (s *SqlPostStore) Update(newPost *model.Post, oldPost *model.Post) store.St
 	})
 }
 
-func (s *SqlPostStore) Overwrite(post *model.Post) store.StoreChannel {
+func (s *SqlPostStore) Overwrite(post *model.Post) (*model.Post, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	post.UpdateAt = model.GetMillis()
-		post.UpdateAt = model.GetMillis()
 
-		var maxPostSize int
+	maxPostSize := s.GetMaxPostSize()
-		if result := <-s.GetMaxPostSize(); result.Err != nil {
+	if appErr := post.IsValid(maxPostSize); appErr != nil {
-			result.Err = model.NewAppError("SqlPostStore.Save", "store.sql_post.overwrite.app_error", nil, "id="+post.Id+", "+result.Err.Error(), http.StatusInternalServerError)
+		return nil, appErr
-			return
+	}
-		} else {
-			maxPostSize = result.Data.(int)
-		}
 
-		if result.Err = post.IsValid(maxPostSize); result.Err != nil {
+	if _, err := s.GetMaster().Update(post); err != nil {
-			return
+		return nil, model.NewAppError("SqlPostStore.Overwrite", "store.sql_post.overwrite.app_error", nil, "id="+post.Id+", "+err.Error(), http.StatusInternalServerError)
-		}
+	}
 
-		if _, err := s.GetMaster().Update(post); err != nil {
+	return post, nil
-			result.Err = model.NewAppError("SqlPostStore.Overwrite", "store.sql_post.overwrite.app_error", nil, "id="+post.Id+", "+err.Error(), http.StatusInternalServerError)
-		} else {
-			result.Data = post
-		}
-	})
 }
 
 func (s *SqlPostStore) GetFlaggedPosts(userId string, offset int, limit int) store.StoreChannel {
@@ -1296,13 +1275,11 @@ func (s *SqlPostStore) determineMaxPostSize() int {
 }
 
 // GetMaxPostSize returns the maximum number of runes that may be stored in a post.
-func (s *SqlPostStore) GetMaxPostSize() store.StoreChannel {
+func (s *SqlPostStore) GetMaxPostSize() int {
-	return store.Do(func(result *store.StoreResult) {
+	s.maxPostSizeOnce.Do(func() {
-		s.maxPostSizeOnce.Do(func() {
+		s.maxPostSizeCached = s.determineMaxPostSize()
-			s.maxPostSizeCached = s.determineMaxPostSize()
-		})
-		result.Data = s.maxPostSizeCached
 	})
+	return s.maxPostSizeCached
 }
 
 func (s *SqlPostStore) GetParentsForExportAfter(limit int, afterId string) store.StoreChannel {

store/store.go

@@ -232,12 +232,12 @@ type PostStore interface {
 	ClearCaches()
 	InvalidateLastPostTimeCache(channelId string)
 	GetPostsCreatedAt(channelId string, time int64) StoreChannel
-	Overwrite(post *model.Post) StoreChannel
+	Overwrite(post *model.Post) (*model.Post, *model.AppError)
 	GetPostsByIds(postIds []string) StoreChannel
 	GetPostsBatchForIndexing(startTime int64, endTime int64, limit int) StoreChannel
 	PermanentDeleteBatch(endTime int64, limit int64) StoreChannel
 	GetOldest() StoreChannel
-	GetMaxPostSize() StoreChannel
+	GetMaxPostSize() int
 	GetParentsForExportAfter(limit int, afterId string) StoreChannel
 	GetRepliesForExport(parentId string) StoreChannel
 	GetDirectPostParentsForExportAfter(limit int, afterId string) StoreChannel

store/storetest/mocks/PostStore.go

@@ -179,16 +179,14 @@ func (_m *PostStore) GetFlaggedPostsForTeam(userId string, teamId string, offset
 }
 
 // GetMaxPostSize provides a mock function with given fields:
-func (_m *PostStore) GetMaxPostSize() store.StoreChannel {
+func (_m *PostStore) GetMaxPostSize() int {
 	ret := _m.Called()
 
-	var r0 store.StoreChannel
+	var r0 int
-	if rf, ok := ret.Get(0).(func() store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func() int); ok {
 		r0 = rf()
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Get(0).(int)
-			r0 = ret.Get(0).(store.StoreChannel)
-		}
 	}
 
 	return r0
@@ -376,19 +374,28 @@ func (_m *PostStore) InvalidateLastPostTimeCache(channelId string) {
 }
 
 // Overwrite provides a mock function with given fields: post
-func (_m *PostStore) Overwrite(post *model.Post) store.StoreChannel {
+func (_m *PostStore) Overwrite(post *model.Post) (*model.Post, *model.AppError) {
 	ret := _m.Called(post)
 
-	var r0 store.StoreChannel
+	var r0 *model.Post
-	if rf, ok := ret.Get(0).(func(*model.Post) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(*model.Post) *model.Post); ok {
 		r0 = rf(post)
 	} 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(*model.Post) *model.AppError); ok {
+		r1 = rf(post)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // PermanentDeleteBatch provides a mock function with given fields: endTime, limit

store/storetest/post_store.go

@@ -1638,8 +1638,8 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) {
 	o1a := &model.Post{}
 	*o1a = *ro1
 	o1a.Message = ro1.Message + "BBBBBBBBBB"
-	if result := <-ss.Post().Overwrite(o1a); result.Err != nil {
+	if _, err := ss.Post().Overwrite(o1a); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
 	}
 
 	ro1a := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o1.Id]
@@ -1651,8 +1651,8 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) {
 	o2a := &model.Post{}
 	*o2a = *ro2
 	o2a.Message = ro2.Message + "DDDDDDD"
-	if result := <-ss.Post().Overwrite(o2a); result.Err != nil {
+	if _, err := ss.Post().Overwrite(o2a); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
 	}
 
 	ro2a := (<-ss.Post().Get(o1.Id)).Data.(*model.PostList).Posts[o2.Id]
@@ -1664,8 +1664,8 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) {
 	o3a := &model.Post{}
 	*o3a = *ro3
 	o3a.Message = ro3.Message + "WWWWWWW"
-	if result := <-ss.Post().Overwrite(o3a); result.Err != nil {
+	if _, err := ss.Post().Overwrite(o3a); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
 	}
 
 	ro3a := (<-ss.Post().Get(o3.Id)).Data.(*model.PostList).Posts[o3.Id]
@@ -1687,8 +1687,8 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) {
 	*o4a = *ro4
 	o4a.Filenames = []string{}
 	o4a.FileIds = []string{model.NewId()}
-	if result := <-ss.Post().Overwrite(o4a); result.Err != nil {
+	if _, err := ss.Post().Overwrite(o4a); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
 	}
 
 	if ro4a := store.Must(ss.Post().Get(o4.Id)).(*model.PostList).Posts[o4.Id]; len(ro4a.Filenames) != 0 {
@@ -1872,8 +1872,8 @@ func testPostStoreGetOldest(t *testing.T, ss store.Store) {
 }
 
 func testGetMaxPostSize(t *testing.T, ss store.Store) {
-	assert.Equal(t, model.POST_MESSAGE_MAX_RUNES_V2, (<-ss.Post().GetMaxPostSize()).Data.(int))
+	assert.Equal(t, model.POST_MESSAGE_MAX_RUNES_V2, ss.Post().GetMaxPostSize())
-	assert.Equal(t, model.POST_MESSAGE_MAX_RUNES_V2, (<-ss.Post().GetMaxPostSize()).Data.(int))
+	assert.Equal(t, model.POST_MESSAGE_MAX_RUNES_V2, ss.Post().GetMaxPostSize())
 }
 
 func testPostStoreGetParentsForExportAfter(t *testing.T, ss store.Store) {