diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 1a6f19d3b2..cd629956cc 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -8802,11 +8802,11 @@ func (s *RetryLayerPreferenceStore) GetCategory(userID string, category string) } -func (s *RetryLayerPreferenceStore) GetCategoryAndName(category string, nane string) (model.Preferences, error) { +func (s *RetryLayerPreferenceStore) GetCategoryAndName(category string, name string) (model.Preferences, error) { tries := 0 for { - result, err := s.PreferenceStore.GetCategoryAndName(category, nane) + result, err := s.PreferenceStore.GetCategoryAndName(category, name) if err == nil { return result, nil } diff --git a/server/channels/store/sqlstore/preference_store.go b/server/channels/store/sqlstore/preference_store.go index d1dd0e41f2..49d524014d 100644 --- a/server/channels/store/sqlstore/preference_store.go +++ b/server/channels/store/sqlstore/preference_store.go @@ -14,10 +14,16 @@ import ( type SqlPreferenceStore struct { *SqlStore + preferenceSelectQuery sq.SelectBuilder } func newSqlPreferenceStore(sqlStore *SqlStore) store.PreferenceStore { - s := &SqlPreferenceStore{sqlStore} + s := &SqlPreferenceStore{SqlStore: sqlStore} + + s.preferenceSelectQuery = s.getQueryBuilder(). + Select("UserId", "Category", "Name", "Value"). + From("Preferences") + return s } @@ -121,18 +127,12 @@ func (s SqlPreferenceStore) saveTx(transaction *sqlxTxWrapper, preference *model func (s SqlPreferenceStore) Get(userId string, category string, name string) (*model.Preference, error) { var preference model.Preference - query, args, err := s.getQueryBuilder(). - Select("*"). - From("Preferences"). + query := s.preferenceSelectQuery. Where(sq.Eq{"UserId": userId}). Where(sq.Eq{"Category": category}). - Where(sq.Eq{"Name": name}). - ToSql() + Where(sq.Eq{"Name": name}) - if err != nil { - return nil, errors.Wrap(err, "could not build sql query to get preference") - } - if err = s.GetReplica().Get(&preference, query, args...); err != nil { + if err := s.GetReplica().GetBuilder(&preference, query); err != nil { return nil, errors.Wrapf(err, "failed to find Preference with userId=%s, category=%s, name=%s", userId, category, name) } @@ -141,50 +141,35 @@ func (s SqlPreferenceStore) Get(userId string, category string, name string) (*m func (s SqlPreferenceStore) GetCategoryAndName(category string, name string) (model.Preferences, error) { var preferences model.Preferences - query, args, err := s.getQueryBuilder(). - Select("*"). - From("Preferences"). + query := s.preferenceSelectQuery. Where(sq.Eq{"Category": category}). - Where(sq.Eq{"Name": name}). - ToSql() - if err != nil { - return nil, errors.Wrap(err, "could not build sql query to get preference") - } - if err = s.GetReplica().Select(&preferences, query, args...); err != nil { - return nil, errors.Wrapf(err, "failed to find Preference with category=%s, name=%s", category, name) + Where(sq.Eq{"Name": name}) + + if err := s.GetReplica().SelectBuilder(&preferences, query); err != nil { + return nil, errors.Wrapf(err, "failed to find Preferences with category=%s, name=%s", category, name) } return preferences, nil } func (s SqlPreferenceStore) GetCategory(userId string, category string) (model.Preferences, error) { var preferences model.Preferences - query, args, err := s.getQueryBuilder(). - Select("*"). - From("Preferences"). + query := s.preferenceSelectQuery. Where(sq.Eq{"UserId": userId}). - Where(sq.Eq{"Category": category}). - ToSql() - if err != nil { - return nil, errors.Wrap(err, "could not build sql query to get preference") - } - if err = s.GetReplica().Select(&preferences, query, args...); err != nil { - return nil, errors.Wrapf(err, "failed to find Preference with userId=%s, category=%s", userId, category) + Where(sq.Eq{"Category": category}) + + if err := s.GetReplica().SelectBuilder(&preferences, query); err != nil { + return nil, errors.Wrapf(err, "failed to find Preferences with userId=%s, category=%s", userId, category) } return preferences, nil } func (s SqlPreferenceStore) GetAll(userId string) (model.Preferences, error) { var preferences model.Preferences - query, args, err := s.getQueryBuilder(). - Select("*"). - From("Preferences"). - Where(sq.Eq{"UserId": userId}). - ToSql() - if err != nil { - return nil, errors.Wrap(err, "could not build sql query to get preference") - } - if err = s.GetReplica().Select(&preferences, query, args...); err != nil { - return nil, errors.Wrapf(err, "failed to find Preference with userId=%s", userId) + query := s.preferenceSelectQuery. + Where(sq.Eq{"UserId": userId}) + + if err := s.GetReplica().SelectBuilder(&preferences, query); err != nil { + return nil, errors.Wrapf(err, "failed to find Preferences with userId=%s", userId) } return preferences, nil } @@ -208,7 +193,6 @@ func (s SqlPreferenceStore) Delete(userId, category, name string) error { Where(sq.Eq{"UserId": userId}). Where(sq.Eq{"Category": category}). Where(sq.Eq{"Name": name}).ToSql() - if err != nil { return errors.Wrap(err, "could not build sql query to get delete preference") } @@ -225,7 +209,6 @@ func (s SqlPreferenceStore) DeleteCategory(userId string, category string) error Delete("Preferences"). Where(sq.Eq{"UserId": userId}). Where(sq.Eq{"Category": category}).ToSql() - if err != nil { return errors.Wrap(err, "could not build sql query to get delete preference by category") } @@ -242,7 +225,6 @@ func (s SqlPreferenceStore) DeleteCategoryAndName(category string, name string) Delete("Preferences"). Where(sq.Eq{"Name": name}). Where(sq.Eq{"Category": category}).ToSql() - if err != nil { return errors.Wrap(err, "could not build sql query to get delete preference by category and name") } @@ -299,7 +281,6 @@ func (s SqlPreferenceStore) CleanupFlagsBatch(limit int64) (int64, error) { Where(sq.Eq{"Category": model.PreferenceCategoryFlaggedPost}). Where(sq.Expr("name IN ("+nameInQ+")", nameInArgs...)). ToSql() - if err != nil { return int64(0), errors.Wrap(err, "could not build sql query to delete preference") } diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 7e77e80698..4683e52aea 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -666,7 +666,7 @@ type CommandWebhookStore interface { type PreferenceStore interface { Save(preferences model.Preferences) error GetCategory(userID string, category string) (model.Preferences, error) - GetCategoryAndName(category string, nane string) (model.Preferences, error) + GetCategoryAndName(category string, name string) (model.Preferences, error) Get(userID string, category string, name string) (*model.Preference, error) GetAll(userID string) (model.Preferences, error) Delete(userID, category, name string) error diff --git a/server/channels/store/storetest/mocks/PreferenceStore.go b/server/channels/store/storetest/mocks/PreferenceStore.go index 719860e852..81b7173aee 100644 --- a/server/channels/store/storetest/mocks/PreferenceStore.go +++ b/server/channels/store/storetest/mocks/PreferenceStore.go @@ -242,9 +242,9 @@ func (_m *PreferenceStore) GetCategory(userID string, category string) (model.Pr return r0, r1 } -// GetCategoryAndName provides a mock function with given fields: category, nane -func (_m *PreferenceStore) GetCategoryAndName(category string, nane string) (model.Preferences, error) { - ret := _m.Called(category, nane) +// GetCategoryAndName provides a mock function with given fields: category, name +func (_m *PreferenceStore) GetCategoryAndName(category string, name string) (model.Preferences, error) { + ret := _m.Called(category, name) if len(ret) == 0 { panic("no return value specified for GetCategoryAndName") @@ -253,10 +253,10 @@ func (_m *PreferenceStore) GetCategoryAndName(category string, nane string) (mod var r0 model.Preferences var r1 error if rf, ok := ret.Get(0).(func(string, string) (model.Preferences, error)); ok { - return rf(category, nane) + return rf(category, name) } if rf, ok := ret.Get(0).(func(string, string) model.Preferences); ok { - r0 = rf(category, nane) + r0 = rf(category, name) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(model.Preferences) @@ -264,7 +264,7 @@ func (_m *PreferenceStore) GetCategoryAndName(category string, nane string) (mod } if rf, ok := ret.Get(1).(func(string, string) error); ok { - r1 = rf(category, nane) + r1 = rf(category, name) } else { r1 = ret.Error(1) } diff --git a/server/channels/store/storetest/preference_store.go b/server/channels/store/storetest/preference_store.go index c9d0280b89..97099c6bfe 100644 --- a/server/channels/store/storetest/preference_store.go +++ b/server/channels/store/storetest/preference_store.go @@ -18,6 +18,7 @@ func TestPreferenceStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlSt t.Run("PreferenceSave", func(t *testing.T) { testPreferenceSave(t, rctx, ss) }) t.Run("PreferenceGet", func(t *testing.T) { testPreferenceGet(t, rctx, ss) }) t.Run("PreferenceGetCategory", func(t *testing.T) { testPreferenceGetCategory(t, rctx, ss) }) + t.Run("PreferenceGetCategoryAndName", func(t *testing.T) { testPreferenceGetCategoryAndName(t, rctx, ss) }) t.Run("PreferenceGetAll", func(t *testing.T) { testPreferenceGetAll(t, rctx, ss) }) t.Run("PreferenceDeleteByUser", func(t *testing.T) { testPreferenceDeleteByUser(t, rctx, ss) }) t.Run("PreferenceDelete", func(t *testing.T) { testPreferenceDelete(t, rctx, ss) }) @@ -27,7 +28,7 @@ func TestPreferenceStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlSt t.Run("PreferenceDeleteInvalidVisibleDmsGms", func(t *testing.T) { testDeleteInvalidVisibleDmsGms(t, rctx, ss, s) }) } -func testPreferenceSave(t *testing.T, rctx request.CTX, ss store.Store) { +func testPreferenceSave(t *testing.T, _ request.CTX, ss store.Store) { id := model.NewId() preferences := model.Preferences{ @@ -63,7 +64,7 @@ func testPreferenceSave(t *testing.T, rctx request.CTX, ss store.Store) { } } -func testPreferenceGet(t *testing.T, rctx request.CTX, ss store.Store) { +func testPreferenceGet(t *testing.T, _ request.CTX, ss store.Store) { userId := model.NewId() category := model.PreferenceCategoryDirectChannelShow name := model.NewId() @@ -103,7 +104,71 @@ func testPreferenceGet(t *testing.T, rctx request.CTX, ss store.Store) { require.Error(t, err, "no error on getting a missing preference") } -func testPreferenceGetCategory(t *testing.T, rctx request.CTX, ss store.Store) { +func testPreferenceGetCategoryAndName(t *testing.T, _ request.CTX, ss store.Store) { + userId := model.NewId() + category := model.PreferenceCategoryGroupChannelShow + name := model.NewId() + + preferences := model.Preferences{ + { + UserId: userId, + Category: category, + Name: name, + Value: "user1", + }, + // same category/name, different user + { + UserId: model.NewId(), + Category: category, + Name: name, + Value: "otherUser", + }, + // same user/name, different category + { + UserId: userId, + Category: model.NewId(), + Name: name, + Value: "", + }, + // same user/category, different name + { + UserId: userId, + Category: category, + Name: model.NewId(), + Value: "", + }, + } + + err := ss.Preference().Save(preferences) + require.NoError(t, err) + + actualPreferences, err := ss.Preference().GetCategoryAndName(category, name) + require.NoError(t, err) + assert.Equal(t, 2, len(actualPreferences), "got the wrong number of preferences") + + for _, preference := range preferences { + // Preferences with empty values aren't expected. + if preference.Value == "" { + continue + } + + found := false + for _, actualPreference := range actualPreferences { + if actualPreference == preference { + found = true + } + } + + assert.True(t, found, "didnt find preference with value %s", preference.Value) + } + + // make sure getting a missing preference category and name doesn't fail + actualPreferences, err = ss.Preference().GetCategoryAndName(model.NewId(), model.NewId()) + require.NoError(t, err) + require.Equal(t, 0, len(actualPreferences), "shouldn't have got any preferences") +} + +func testPreferenceGetCategory(t *testing.T, _ request.CTX, ss store.Store) { userId := model.NewId() category := model.PreferenceCategoryDirectChannelShow name := model.NewId() @@ -152,7 +217,7 @@ func testPreferenceGetCategory(t *testing.T, rctx request.CTX, ss store.Store) { require.Equal(t, 0, len(preferencesByCategory), "shouldn't have got any preferences") } -func testPreferenceGetAll(t *testing.T, rctx request.CTX, ss store.Store) { +func testPreferenceGetAll(t *testing.T, _ request.CTX, ss store.Store) { userId := model.NewId() category := model.PreferenceCategoryDirectChannelShow name := model.NewId() @@ -195,7 +260,7 @@ func testPreferenceGetAll(t *testing.T, rctx request.CTX, ss store.Store) { } } -func testPreferenceDeleteByUser(t *testing.T, rctx request.CTX, ss store.Store) { +func testPreferenceDeleteByUser(t *testing.T, _ request.CTX, ss store.Store) { userId := model.NewId() category := model.PreferenceCategoryDirectChannelShow name := model.NewId() @@ -233,7 +298,7 @@ func testPreferenceDeleteByUser(t *testing.T, rctx request.CTX, ss store.Store) require.NoError(t, err) } -func testPreferenceDelete(t *testing.T, rctx request.CTX, ss store.Store) { +func testPreferenceDelete(t *testing.T, _ request.CTX, ss store.Store) { preference := model.Preference{ UserId: model.NewId(), Category: model.PreferenceCategoryDirectChannelShow, @@ -255,7 +320,7 @@ func testPreferenceDelete(t *testing.T, rctx request.CTX, ss store.Store) { assert.Empty(t, preferences, "should've returned no preferences") } -func testPreferenceDeleteCategory(t *testing.T, rctx request.CTX, ss store.Store) { +func testPreferenceDeleteCategory(t *testing.T, _ request.CTX, ss store.Store) { category := model.NewId() userId := model.NewId() @@ -288,7 +353,7 @@ func testPreferenceDeleteCategory(t *testing.T, rctx request.CTX, ss store.Store assert.Empty(t, preferences, "should've returned no preferences") } -func testPreferenceDeleteCategoryAndName(t *testing.T, rctx request.CTX, ss store.Store) { +func testPreferenceDeleteCategoryAndName(t *testing.T, _ request.CTX, ss store.Store) { category := model.NewId() name := model.NewId() userId := model.NewId() @@ -404,7 +469,7 @@ func testPreferenceDeleteOrphanedRows(t *testing.T, rctx request.CTX, ss store.S assert.NoError(t, nErr, "newer preference should not have been deleted") } -func testDeleteInvalidVisibleDmsGms(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { +func testDeleteInvalidVisibleDmsGms(t *testing.T, _ request.CTX, ss store.Store, s SqlStore) { userId1 := model.NewId() userId2 := model.NewId() userId3 := model.NewId() diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 51d8d95975..4d4a8b6580 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -6993,10 +6993,10 @@ func (s *TimerLayerPreferenceStore) GetCategory(userID string, category string) return result, err } -func (s *TimerLayerPreferenceStore) GetCategoryAndName(category string, nane string) (model.Preferences, error) { +func (s *TimerLayerPreferenceStore) GetCategoryAndName(category string, name string) (model.Preferences, error) { start := time.Now() - result, err := s.PreferenceStore.GetCategoryAndName(category, nane) + result, err := s.PreferenceStore.GetCategoryAndName(category, name) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil {