From 24a02b4168bc5cd8101aa2479102428ab3e8a391 Mon Sep 17 00:00:00 2001 From: Pradeep Murugesan Date: Wed, 15 May 2019 15:56:42 +0100 Subject: [PATCH] made the preference store GetCategory method sync (#10847) * made the preference store GetCategory method sync * fixed the review comments --- app/import_test.go | 3 +-- app/preference.go | 12 ++++++------ store/sqlstore/preference_store.go | 19 +++++++++---------- store/store.go | 2 +- store/storetest/mocks/PreferenceStore.go | 19 ++++++++++++++----- store/storetest/preference_store.go | 14 +++++++------- 6 files changed, 38 insertions(+), 31 deletions(-) diff --git a/app/import_test.go b/app/import_test.go index 6b1ece8468..1afa13f705 100644 --- a/app/import_test.go +++ b/app/import_test.go @@ -33,11 +33,10 @@ func ptrBool(b bool) *bool { } func checkPreference(t *testing.T, a *App, userId string, category string, name string, value string) { - if res := <-a.Srv.Store.Preference().GetCategory(userId, category); res.Err != nil { + if preferences, err := a.Srv.Store.Preference().GetCategory(userId, category); err != nil { debug.PrintStack() t.Fatalf("Failed to get preferences for user %v with category %v", userId, category) } else { - preferences := res.Data.(model.Preferences) found := false for _, preference := range preferences { if preference.Name == name { diff --git a/app/preference.go b/app/preference.go index 7b5e4ca0cc..2bf96aab35 100644 --- a/app/preference.go +++ b/app/preference.go @@ -19,16 +19,16 @@ func (a *App) GetPreferencesForUser(userId string) (model.Preferences, *model.Ap } func (a *App) GetPreferenceByCategoryForUser(userId string, category string) (model.Preferences, *model.AppError) { - result := <-a.Srv.Store.Preference().GetCategory(userId, category) - if result.Err != nil { - result.Err.StatusCode = http.StatusBadRequest - return nil, result.Err + preferences, err := a.Srv.Store.Preference().GetCategory(userId, category) + if err != nil { + err.StatusCode = http.StatusBadRequest + return nil, err } - if len(result.Data.(model.Preferences)) == 0 { + if len(preferences) == 0 { err := model.NewAppError("getPreferenceCategory", "api.preference.preferences_category.get.app_error", nil, "", http.StatusNotFound) return nil, err } - return result.Data.(model.Preferences), nil + return preferences, nil } func (a *App) GetPreferenceByCategoryAndNameForUser(userId string, category string, preferenceName string) (*model.Preference, *model.AppError) { diff --git a/store/sqlstore/preference_store.go b/store/sqlstore/preference_store.go index 6becb03367..d07837b8d0 100644 --- a/store/sqlstore/preference_store.go +++ b/store/sqlstore/preference_store.go @@ -180,23 +180,22 @@ func (s SqlPreferenceStore) Get(userId string, category string, name string) (*m return preference, nil } -func (s SqlPreferenceStore) GetCategory(userId string, category string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var preferences model.Preferences +func (s SqlPreferenceStore) GetCategory(userId string, category string) (model.Preferences, *model.AppError) { + var preferences model.Preferences - if _, err := s.GetReplica().Select(&preferences, - `SELECT + if _, err := s.GetReplica().Select(&preferences, + `SELECT * FROM Preferences WHERE UserId = :UserId AND Category = :Category`, map[string]interface{}{"UserId": userId, "Category": category}); err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.GetCategory", "store.sql_preference.get_category.app_error", nil, err.Error(), http.StatusInternalServerError) - } else { - result.Data = preferences - } - }) + return nil, model.NewAppError("SqlPreferenceStore.GetCategory", "store.sql_preference.get_category.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return preferences, nil + } func (s SqlPreferenceStore) GetAll(userId string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 84fe8c3030..bc18f9d1f7 100644 --- a/store/store.go +++ b/store/store.go @@ -429,8 +429,8 @@ type CommandWebhookStore interface { type PreferenceStore interface { Save(preferences *model.Preferences) StoreChannel + GetCategory(userId string, category string) (model.Preferences, *model.AppError) 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 DeleteCategory(userId string, category string) StoreChannel diff --git a/store/storetest/mocks/PreferenceStore.go b/store/storetest/mocks/PreferenceStore.go index f3061e09c7..86106cba72 100644 --- a/store/storetest/mocks/PreferenceStore.go +++ b/store/storetest/mocks/PreferenceStore.go @@ -119,19 +119,28 @@ func (_m *PreferenceStore) GetAll(userId string) store.StoreChannel { } // GetCategory provides a mock function with given fields: userId, category -func (_m *PreferenceStore) GetCategory(userId string, category string) store.StoreChannel { +func (_m *PreferenceStore) GetCategory(userId string, category string) (model.Preferences, *model.AppError) { ret := _m.Called(userId, category) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok { + var r0 model.Preferences + if rf, ok := ret.Get(0).(func(string, string) model.Preferences); ok { r0 = rf(userId, category) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(model.Preferences) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok { + r1 = rf(userId, category) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // IsFeatureEnabled provides a mock function with given fields: feature, userId diff --git a/store/storetest/preference_store.go b/store/storetest/preference_store.go index 1b4a314156..e713d54b6b 100644 --- a/store/storetest/preference_store.go +++ b/store/storetest/preference_store.go @@ -140,18 +140,18 @@ func testPreferenceGetCategory(t *testing.T, ss store.Store) { store.Must(ss.Preference().Save(&preferences)) - if result := <-ss.Preference().GetCategory(userId, category); result.Err != nil { - t.Fatal(result.Err) - } else if data := result.Data.(model.Preferences); len(data) != 2 { + if preferencesByCategory, err := ss.Preference().GetCategory(userId, category); err != nil { + t.Fatal(err) + } else if len(preferencesByCategory) != 2 { t.Fatal("got the wrong number of preferences") - } else if !((data[0] == preferences[0] && data[1] == preferences[1]) || (data[0] == preferences[1] && data[1] == preferences[0])) { + } else if !((preferencesByCategory[0] == preferences[0] && preferencesByCategory[1] == preferences[1]) || (preferencesByCategory[0] == preferences[1] && preferencesByCategory[1] == preferences[0])) { t.Fatal("got incorrect preferences") } // make sure getting a missing preference category doesn't fail - if result := <-ss.Preference().GetCategory(model.NewId(), model.NewId()); result.Err != nil { - t.Fatal(result.Err) - } else if data := result.Data.(model.Preferences); len(data) != 0 { + if preferencesByCategory, err := ss.Preference().GetCategory(model.NewId(), model.NewId()); err != nil { + t.Fatal(err) + } else if len(preferencesByCategory) != 0 { t.Fatal("shouldn't have got any preferences") } }