diff --git a/app/email_batching.go b/app/email_batching.go index 91a99edc78..b5ac5a7228 100644 --- a/app/email_batching.go +++ b/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) diff --git a/app/notification_email.go b/app/notification_email.go index 593401f9f7..c86a279f02 100644 --- a/app/notification_email.go +++ b/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, "") diff --git a/app/notification_push.go b/app/notification_push.go index 97060d317b..ceeac8d71c 100644 --- a/app/notification_push.go +++ b/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) diff --git a/app/preference.go b/app/preference.go index 3e4acd1159..7b5e4ca0cc 100644 --- a/app/preference.go +++ b/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 { diff --git a/store/sqlstore/preference_store.go b/store/sqlstore/preference_store.go index 60fadfa281..6becb03367 100644 --- a/store/sqlstore/preference_store.go +++ b/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 { diff --git a/store/store.go b/store/store.go index de7e9416dd..84fe8c3030 100644 --- a/store/store.go +++ b/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 diff --git a/store/storetest/mocks/PreferenceStore.go b/store/storetest/mocks/PreferenceStore.go index f53ae06d53..f3061e09c7 100644 --- a/store/storetest/mocks/PreferenceStore.go +++ b/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 diff --git a/store/storetest/preference_store.go b/store/storetest/preference_store.go index cf5dafb5e5..1b4a314156 100644 --- a/store/storetest/preference_store.go +++ b/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) }