[MM-15297] Migrate "Preference.Get" to Sync by default #10721 (#10811)

* modify Get method in preference_store

* modify interface to match changes on preference store Get method

* modify variable initialization

* fix behavior on GetPreferenceByCategoryAndNameForUser

app/email_batching.go

@@ -171,13 +171,11 @@ func (job *EmailBatchingJob) checkPendingNotifications(now time.Time, handler fu
 
 		// get how long we need to wait to send notifications to the user
 		var interval int64
-		pchan := job.server.Store.Preference().Get(userId, model.PREFERENCE_CATEGORY_NOTIFICATIONS, model.PREFERENCE_NAME_EMAIL_INTERVAL)
-		if result := <-pchan; result.Err != nil {
+		preference, err := job.server.Store.Preference().Get(userId, model.PREFERENCE_CATEGORY_NOTIFICATIONS, model.PREFERENCE_NAME_EMAIL_INTERVAL)
+		if err != nil {
 			// use the default batching interval if an error ocurrs while fetching user preferences
 			interval, _ = strconv.ParseInt(model.PREFERENCE_EMAIL_INTERVAL_BATCHING_SECONDS, 10, 64)
 		} else {
-			preference := result.Data.(model.Preference)
-
 			if value, err := strconv.ParseInt(preference.Value, 10, 64); err != nil {
 				// // use the default batching interval if an error ocurrs while deserializing user preferences
 				interval, _ = strconv.ParseInt(model.PREFERENCE_EMAIL_INTERVAL_BATCHING_SECONDS, 10, 64)

app/notification_email.go

@@ -48,12 +48,12 @@ func (a *App) sendNotificationEmail(notification *postNotification, user *model.
 
 	if *a.Config().EmailSettings.EnableEmailBatching {
 		var sendBatched bool
-		if result := <-a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_NOTIFICATIONS, model.PREFERENCE_NAME_EMAIL_INTERVAL); result.Err != nil {
+		if data, err := a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_NOTIFICATIONS, model.PREFERENCE_NAME_EMAIL_INTERVAL); err != nil {
 			// if the call fails, assume that the interval has not been explicitly set and batch the notifications
 			sendBatched = true
 		} else {
 			// if the user has chosen to receive notifications immediately, don't batch them
-			sendBatched = result.Data.(model.Preference).Value != model.PREFERENCE_EMAIL_INTERVAL_NO_BATCHING_SECONDS
+			sendBatched = data.Value != model.PREFERENCE_EMAIL_INTERVAL_NO_BATCHING_SECONDS
 		}
 
 		if sendBatched {
@@ -68,17 +68,17 @@ func (a *App) sendNotificationEmail(notification *postNotification, user *model.
 	translateFunc := utils.GetUserTranslations(user.Locale)
 
 	var useMilitaryTime bool
-	if result := <-a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_USE_MILITARY_TIME); result.Err != nil {
+	if data, err := a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_USE_MILITARY_TIME); err != nil {
 		useMilitaryTime = true
 	} else {
-		useMilitaryTime = result.Data.(model.Preference).Value == "true"
+		useMilitaryTime = data.Value == "true"
 	}
 
 	var nameFormat string
-	if result := <-a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_NAME_FORMAT); result.Err != nil {
+	if data, err := a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_NAME_FORMAT); err != nil {
 		nameFormat = *a.Config().TeamSettings.TeammateNameDisplay
 	} else {
-		nameFormat = result.Data.(model.Preference).Value
+		nameFormat = data.Value
 	}
 
 	channelName := notification.GetChannelName(nameFormat, "")

app/notification_push.go

@@ -5,11 +5,12 @@ package app
 
 import (
 	"fmt"
-	"github.com/pkg/errors"
 	"hash/fnv"
 	"net/http"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	"github.com/mattermost/go-i18n/i18n"
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
@@ -151,10 +152,10 @@ func (a *App) sendPushNotification(notification *postNotification, user *model.U
 	post := notification.post
 
 	var nameFormat string
-	if result := <-a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_NAME_FORMAT); result.Err != nil {
+	if data, err := a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_NAME_FORMAT); err != nil {
 		nameFormat = *a.Config().TeamSettings.TeammateNameDisplay
 	} else {
-		nameFormat = result.Data.(model.Preference).Value
+		nameFormat = data.Value
 	}
 
 	channelName := notification.GetChannelName(nameFormat, user.Id)

app/preference.go

@@ -32,13 +32,12 @@ func (a *App) GetPreferenceByCategoryForUser(userId string, category string) (mo
 }
 
 func (a *App) GetPreferenceByCategoryAndNameForUser(userId string, category string, preferenceName string) (*model.Preference, *model.AppError) {
-	result := <-a.Srv.Store.Preference().Get(userId, category, preferenceName)
-	if result.Err != nil {
-		result.Err.StatusCode = http.StatusBadRequest
-		return nil, result.Err
+	res, err := a.Srv.Store.Preference().Get(userId, category, preferenceName)
+	if err != nil {
+		err.StatusCode = http.StatusBadRequest
+		return nil, err
 	}
-	data := result.Data.(model.Preference)
-	return &data, nil
+	return res, nil
 }
 
 func (a *App) UpdatePreferences(userId string, preferences model.Preferences) *model.AppError {

store/sqlstore/preference_store.go

@@ -163,24 +163,21 @@ func (s SqlPreferenceStore) update(transaction *gorp.Transaction, preference *mo
 	return result
 }
 
-func (s SqlPreferenceStore) Get(userId string, category string, name string) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		var preference model.Preference
+func (s SqlPreferenceStore) Get(userId string, category string, name string) (*model.Preference, *model.AppError) {
+	var preference *model.Preference
 
-		if err := s.GetReplica().SelectOne(&preference,
-			`SELECT
-				*
-			FROM
-				Preferences
-			WHERE
-				UserId = :UserId
-				AND Category = :Category
-				AND Name = :Name`, map[string]interface{}{"UserId": userId, "Category": category, "Name": name}); err != nil {
-			result.Err = model.NewAppError("SqlPreferenceStore.Get", "store.sql_preference.get.app_error", nil, err.Error(), http.StatusInternalServerError)
-		} else {
-			result.Data = preference
-		}
-	})
+	if err := s.GetReplica().SelectOne(&preference,
+		`SELECT
+			*
+		FROM
+			Preferences
+		WHERE
+			UserId = :UserId
+			AND Category = :Category
+			AND Name = :Name`, map[string]interface{}{"UserId": userId, "Category": category, "Name": name}); err != nil {
+		return nil, model.NewAppError("SqlPreferenceStore.Get", "store.sql_preference.get.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+	return preference, nil
 }
 
 func (s SqlPreferenceStore) GetCategory(userId string, category string) store.StoreChannel {

store/store.go

@@ -429,7 +429,7 @@ type CommandWebhookStore interface {
 
 type PreferenceStore interface {
 	Save(preferences *model.Preferences) StoreChannel
-	Get(userId string, category string, name string) StoreChannel
+	Get(userId string, category string, name string) (*model.Preference, *model.AppError)
 	GetCategory(userId string, category string) StoreChannel
 	GetAll(userId string) StoreChannel
 	Delete(userId, category, name string) StoreChannel

store/storetest/mocks/PreferenceStore.go

@@ -78,19 +78,28 @@ func (_m *PreferenceStore) DeleteCategoryAndName(category string, name string) s
 }
 
 // Get provides a mock function with given fields: userId, category, name
-func (_m *PreferenceStore) Get(userId string, category string, name string) store.StoreChannel {
+func (_m *PreferenceStore) Get(userId string, category string, name string) (*model.Preference, *model.AppError) {
 	ret := _m.Called(userId, category, name)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string, string, string) store.StoreChannel); ok {
+	var r0 *model.Preference
+	if rf, ok := ret.Get(0).(func(string, string, string) *model.Preference); ok {
 		r0 = rf(userId, category, name)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.Preference)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(string, string, string) *model.AppError); ok {
+		r1 = rf(userId, category, name)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetAll provides a mock function with given fields: userId

store/storetest/preference_store.go

@@ -47,7 +47,7 @@ func testPreferenceSave(t *testing.T, ss store.Store) {
 	}
 
 	for _, preference := range preferences {
-		if data := store.Must(ss.Preference().Get(preference.UserId, preference.Category, preference.Name)).(model.Preference); preference != data {
+		if data, _ := ss.Preference().Get(preference.UserId, preference.Category, preference.Name); preference.ToJson() != data.ToJson() {
 			t.Fatal("got incorrect preference after first Save")
 		}
 	}
@@ -59,7 +59,7 @@ func testPreferenceSave(t *testing.T, ss store.Store) {
 	}
 
 	for _, preference := range preferences {
-		if data := store.Must(ss.Preference().Get(preference.UserId, preference.Category, preference.Name)).(model.Preference); preference != data {
+		if data, _ := ss.Preference().Get(preference.UserId, preference.Category, preference.Name); preference.ToJson() != data.ToJson() {
 			t.Fatal("got incorrect preference after second Save")
 		}
 	}
@@ -95,14 +95,14 @@ func testPreferenceGet(t *testing.T, ss store.Store) {
 
 	store.Must(ss.Preference().Save(&preferences))
 
-	if result := <-ss.Preference().Get(userId, category, name); result.Err != nil {
-		t.Fatal(result.Err)
-	} else if data := result.Data.(model.Preference); data != preferences[0] {
+	if data, err := ss.Preference().Get(userId, category, name); err != nil {
+		t.Fatal(err)
+	} else if data.ToJson() != preferences[0].ToJson() {
 		t.Fatal("got incorrect preference")
 	}
 
 	// make sure getting a missing preference fails
-	if result := <-ss.Preference().Get(model.NewId(), model.NewId(), model.NewId()); result.Err == nil {
+	if _, err := ss.Preference().Get(model.NewId(), model.NewId(), model.NewId()); err == nil {
 		t.Fatal("no error on getting a missing preference")
 	}
 }
@@ -439,9 +439,9 @@ func testPreferenceCleanupFlagsBatch(t *testing.T, ss store.Store) {
 	result := <-ss.Preference().CleanupFlagsBatch(10000)
 	assert.Nil(t, result.Err)
 
-	result = <-ss.Preference().Get(userId, category, preference1.Name)
-	assert.Nil(t, result.Err)
+	_, err := ss.Preference().Get(userId, category, preference1.Name)
+	assert.Nil(t, err)
 
-	result = <-ss.Preference().Get(userId, category, preference2.Name)
-	assert.NotNil(t, result.Err)
+	_, err = ss.Preference().Get(userId, category, preference2.Name)
+	assert.NotNil(t, err)
 }