From 5cecf1afcd1d8e561a5b2d840e5bd1b588a74a27 Mon Sep 17 00:00:00 2001 From: hmhealey Date: Fri, 9 Oct 2015 13:47:11 -0400 Subject: Made structural changes to server-side Preference code based on feedback --- api/preference.go | 37 +++-- api/preference_test.go | 48 +++---- model/client.go | 10 +- model/preference.go | 20 --- model/preferences.go | 31 +++++ store/sql_preference_store.go | 161 +++++++++------------- store/sql_preference_store_test.go | 270 ++++++++++++------------------------- store/sql_store.go | 8 -- store/store.go | 4 +- 9 files changed, 227 insertions(+), 362 deletions(-) create mode 100644 model/preferences.go diff --git a/api/preference.go b/api/preference.go index aa19ee071..855e9ad36 100644 --- a/api/preference.go +++ b/api/preference.go @@ -5,7 +5,6 @@ package api import ( l4g "code.google.com/p/log4go" - "encoding/json" "github.com/gorilla/mux" "github.com/mattermost/platform/model" "net/http" @@ -15,29 +14,27 @@ func InitPreference(r *mux.Router) { l4g.Debug("Initializing preference api routes") sr := r.PathPrefix("/preferences").Subrouter() - sr.Handle("/set", ApiAppHandler(setPreferences)).Methods("POST") + sr.Handle("/save", ApiAppHandler(savePreferences)).Methods("POST") sr.Handle("/{category:[A-Za-z0-9_]+}/{name:[A-Za-z0-9_]+}", ApiAppHandler(getPreferencesByName)).Methods("GET") } -func setPreferences(c *Context, w http.ResponseWriter, r *http.Request) { - var preferences []*model.Preference - - decoder := json.NewDecoder(r.Body) - if err := decoder.Decode(&preferences); err != nil { - c.Err = model.NewAppError("setPreferences", "Unable to decode preferences from request", err.Error()) +func savePreferences(c *Context, w http.ResponseWriter, r *http.Request) { + preferences, err := model.PreferencesFromJson(r.Body) + if err != nil { + c.Err = model.NewAppError("savePreferences", "Unable to decode preferences from request", err.Error()) c.Err.StatusCode = http.StatusBadRequest return } for _, preference := range preferences { if c.Session.UserId != preference.UserId { - c.Err = model.NewAppError("setPreferences", "Unable to set preferences for other user", "session.user_id="+c.Session.UserId+", preference.user_id="+preference.UserId) + c.Err = model.NewAppError("savePreferences", "Unable to set preferences for other user", "session.user_id="+c.Session.UserId+", preference.user_id="+preference.UserId) c.Err.StatusCode = http.StatusUnauthorized return } } - if result := <-Srv.Store.Preference().SaveOrUpdate(preferences...); result.Err != nil { + if result := <-Srv.Store.Preference().Save(&preferences); result.Err != nil { c.Err = result.Err return } @@ -54,7 +51,7 @@ func getPreferencesByName(c *Context, w http.ResponseWriter, r *http.Request) { c.Err = result.Err return } else { - data := result.Data.([]*model.Preference) + data := result.Data.(model.Preferences) if len(data) == 0 { if category == model.PREFERENCE_CATEGORY_DIRECT_CHANNELS && name == model.PREFERENCE_NAME_SHOW { @@ -63,11 +60,11 @@ func getPreferencesByName(c *Context, w http.ResponseWriter, r *http.Request) { } } - w.Write([]byte(model.PreferenceListToJson(data))) + w.Write([]byte(data.ToJson())) } } -func addDirectChannels(userId, teamId string) []*model.Preference { +func addDirectChannels(userId, teamId string) model.Preferences { var profiles map[string]*model.User if result := <-Srv.Store.User().GetProfiles(teamId); result.Err != nil { l4g.Error("Failed to add direct channel preferences for user user_id=%s, team_id=%s, err=%v", userId, teamId, result.Err.Error()) @@ -76,7 +73,7 @@ func addDirectChannels(userId, teamId string) []*model.Preference { profiles = result.Data.(map[string]*model.User) } - var preferences []*model.Preference + var preferences model.Preferences for id := range profiles { if id == userId { @@ -93,11 +90,6 @@ func addDirectChannels(userId, teamId string) []*model.Preference { Value: "true", } - if result := <-Srv.Store.Preference().Save(preference); result.Err != nil { - l4g.Error("Failed to add direct channel preferences for user user_id=%s, alt_id=%s, team_id=%s, err=%v", userId, profile.Id, teamId, result.Err.Error()) - continue - } - preferences = append(preferences, preference) if len(preferences) >= 10 { @@ -105,5 +97,10 @@ func addDirectChannels(userId, teamId string) []*model.Preference { } } - return preferences + if result := <-Srv.Store.Preference().Save(&preferences); result.Err != nil { + l4g.Error("Failed to add direct channel preferences for user user_id=%s, eam_id=%s, err=%v", userId, teamId, result.Err.Error()) + return model.Preferences{} + } else { + return preferences + } } diff --git a/api/preference_test.go b/api/preference_test.go index 77562bb06..a0fc19896 100644 --- a/api/preference_test.go +++ b/api/preference_test.go @@ -22,7 +22,7 @@ func TestSetPreferences(t *testing.T) { Client.LoginByEmail(team.Name, user1.Email, "pwd") // save 10 preferences - var preferences []*model.Preference + var preferences model.Preferences for i := 0; i < 10; i++ { preference := model.Preference{ UserId: user1.Id, @@ -33,7 +33,7 @@ func TestSetPreferences(t *testing.T) { preferences = append(preferences, &preference) } - if _, err := Client.SetPreferences(preferences); err != nil { + if _, err := Client.SetPreferences(&preferences); err != nil { t.Fatal(err) } @@ -42,7 +42,7 @@ func TestSetPreferences(t *testing.T) { preference.Value = "1234garbage" } - if _, err := Client.SetPreferences(preferences); err != nil { + if _, err := Client.SetPreferences(&preferences); err != nil { t.Fatal(err) } @@ -53,7 +53,7 @@ func TestSetPreferences(t *testing.T) { Client.LoginByEmail(team.Name, user2.Email, "pwd") - if _, err := Client.SetPreferences(preferences); err == nil { + if _, err := Client.SetPreferences(&preferences); err == nil { t.Fatal("shouldn't have been able to update another user's preferences") } } @@ -72,7 +72,7 @@ func TestGetPreferencesByName(t *testing.T) { user2 = Client.Must(Client.CreateUser(user2, "")).Data.(*model.User) store.Must(Srv.Store.User().VerifyEmail(user2.Id)) - preferences1 := []*model.Preference{ + preferences1 := model.Preferences{ { UserId: user1.Id, Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, @@ -100,13 +100,13 @@ func TestGetPreferencesByName(t *testing.T) { } Client.LoginByEmail(team.Name, user1.Email, "pwd") - Client.Must(Client.SetPreferences(preferences1)) + Client.Must(Client.SetPreferences(&preferences1)) Client.LoginByEmail(team.Name, user1.Email, "pwd") if result, err := Client.GetPreferencesByName(model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, model.PREFERENCE_NAME_SHOW); err != nil { t.Fatal(err) - } else if data := result.Data.([]*model.Preference); len(data) != 2 { + } else if data := result.Data.(model.Preferences); len(data) != 2 { t.Fatal("received the wrong number of preferences") } else if !((*data[0] == *preferences1[0] && *data[1] == *preferences1[1]) || (*data[0] == *preferences1[1] && *data[1] == *preferences1[0])) { t.Fatal("received incorrect preferences") @@ -117,7 +117,7 @@ func TestGetPreferencesByName(t *testing.T) { // note that user2 will start with a preference to show user1 in the sidebar by default if result, err := Client.GetPreferencesByName(model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, model.PREFERENCE_NAME_SHOW); err != nil { t.Fatal(err) - } else if data := result.Data.([]*model.Preference); len(data) != 1 { + } else if data := result.Data.(model.Preferences); len(data) != 1 { t.Fatal("received the wrong number of preferences") } } @@ -134,32 +134,34 @@ func TestSetAndGetProperties(t *testing.T) { Client.LoginByEmail(team.Name, user.Email, "pwd") - p := model.Preference{ - UserId: user.Id, - Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOW, - AltId: model.NewId(), - Value: model.NewId(), + preferences := model.Preferences{ + { + UserId: user.Id, + Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, + Name: model.PREFERENCE_NAME_SHOW, + AltId: model.NewId(), + Value: model.NewId(), + }, } - Client.Must(Client.SetPreferences([]*model.Preference{&p})) + Client.Must(Client.SetPreferences(&preferences)) - if result, err := Client.GetPreferencesByName(p.Category, p.Name); err != nil { + if result, err := Client.GetPreferencesByName(preferences[0].Category, preferences[0].Name); err != nil { t.Fatal(err) - } else if data := result.Data.([]*model.Preference); len(data) != 1 { + } else if data := result.Data.(model.Preferences); len(data) != 1 { t.Fatal("received too many preferences") - } else if *data[0] != p { + } else if *data[0] != *preferences[0] { t.Fatal("preference saved incorrectly") } - p.Value = model.NewId() - Client.Must(Client.SetPreferences([]*model.Preference{&p})) + preferences[0].Value = model.NewId() + Client.Must(Client.SetPreferences(&preferences)) - if result, err := Client.GetPreferencesByName(p.Category, p.Name); err != nil { + if result, err := Client.GetPreferencesByName(preferences[0].Category, preferences[0].Name); err != nil { t.Fatal(err) - } else if data := result.Data.([]*model.Preference); len(data) != 1 { + } else if data := result.Data.(model.Preferences); len(data) != 1 { t.Fatal("received too many preferences") - } else if *data[0] != p { + } else if *data[0] != *preferences[0] { t.Fatal("preference updated incorrectly") } } diff --git a/model/client.go b/model/client.go index 892d3e979..304593c5c 100644 --- a/model/client.go +++ b/model/client.go @@ -844,12 +844,12 @@ func (c *Client) ListIncomingWebhooks() (*Result, *AppError) { } } -func (c *Client) SetPreferences(preferences []*Preference) (*Result, *AppError) { - if r, err := c.DoApiPost("/preferences/set", PreferenceListToJson(preferences)); err != nil { +func (c *Client) SetPreferences(preferences *Preferences) (*Result, *AppError) { + if r, err := c.DoApiPost("/preferences/save", preferences.ToJson()); err != nil { return nil, err } else { return &Result{r.Header.Get(HEADER_REQUEST_ID), - r.Header.Get(HEADER_ETAG_SERVER), nil}, nil + r.Header.Get(HEADER_ETAG_SERVER), preferences}, nil } } @@ -857,8 +857,8 @@ func (c *Client) GetPreferencesByName(category string, name string) (*Result, *A if r, err := c.DoApiGet("/preferences/"+category+"/"+name, "", ""); err != nil { return nil, err } else { - return &Result{r.Header.Get(HEADER_REQUEST_ID), - r.Header.Get(HEADER_ETAG_SERVER), PreferenceListFromJson(r.Body)}, nil + preferences, _ := PreferencesFromJson(r.Body) + return &Result{r.Header.Get(HEADER_REQUEST_ID), r.Header.Get(HEADER_ETAG_SERVER), preferences}, nil } } diff --git a/model/preference.go b/model/preference.go index 5a5f0d64e..434f55f40 100644 --- a/model/preference.go +++ b/model/preference.go @@ -32,15 +32,6 @@ func (o *Preference) ToJson() string { } } -func PreferenceListToJson(o []*Preference) string { - b, err := json.Marshal(o) - if err != nil { - return "" - } else { - return string(b) - } -} - func PreferenceFromJson(data io.Reader) *Preference { decoder := json.NewDecoder(data) var o Preference @@ -52,17 +43,6 @@ func PreferenceFromJson(data io.Reader) *Preference { } } -func PreferenceListFromJson(data io.Reader) []*Preference { - decoder := json.NewDecoder(data) - var o []*Preference - err := decoder.Decode(&o) - if err == nil { - return o - } else { - return nil - } -} - func (o *Preference) IsValid() *AppError { if len(o.UserId) != 26 { return NewAppError("Preference.IsValid", "Invalid user id", "user_id="+o.UserId) diff --git a/model/preferences.go b/model/preferences.go new file mode 100644 index 000000000..2c6cbdb98 --- /dev/null +++ b/model/preferences.go @@ -0,0 +1,31 @@ +// Copyright (c) 2015 Spinpunch, Inc. All Rights Reserved. +// See License.txt for license information. + +package model + +import ( + "encoding/json" + "io" +) + +type Preferences []*Preference + +func (o *Preferences) ToJson() string { + b, err := json.Marshal(o) + if err != nil { + return "" + } else { + return string(b) + } +} + +func PreferencesFromJson(data io.Reader) (Preferences, error) { + decoder := json.NewDecoder(data) + var o Preferences + err := decoder.Decode(&o) + if err == nil { + return o, nil + } else { + return nil, err + } +} diff --git a/store/sql_preference_store.go b/store/sql_preference_store.go index 8d4e99109..04bb5e4ee 100644 --- a/store/sql_preference_store.go +++ b/store/sql_preference_store.go @@ -4,6 +4,7 @@ package store import ( + "github.com/go-gorp/gorp" "github.com/mattermost/platform/model" "github.com/mattermost/platform/utils" ) @@ -36,103 +37,36 @@ func (s SqlPreferenceStore) CreateIndexesIfNotExists() { s.CreateIndexIfNotExists("idx_preferences_name", "Preferences", "Name") } -func (s SqlPreferenceStore) Save(preference *model.Preference) StoreChannel { - storeChannel := make(StoreChannel) - - go func() { - storeChannel <- s.save(s.GetMaster(), preference) - close(storeChannel) - }() - - return storeChannel -} - -func (s SqlPreferenceStore) save(queryable Queryable, preference *model.Preference) StoreResult { - result := StoreResult{} - - if result.Err = preference.IsValid(); result.Err != nil { - return result - } - - if err := queryable.Insert(preference); err != nil { - if IsUniqueConstraintError(err.Error(), "UserId", "preferences_pkey") { - result.Err = model.NewAppError("SqlPreferenceStore.Save", "A preference with that user id, category, name, and alt id already exists", - "user_id="+preference.UserId+", category="+preference.Category+", name="+preference.Name+", alt_id="+preference.AltId+", "+err.Error()) - } else { - result.Err = model.NewAppError("SqlPreferenceStore.Save", "We couldn't save the preference", - "user_id="+preference.UserId+", category="+preference.Category+", name="+preference.Name+", alt_id="+preference.AltId+", "+err.Error()) - } - } else { - result.Data = preference - } - - return result -} - -func (s SqlPreferenceStore) Update(preference *model.Preference) StoreChannel { - storeChannel := make(StoreChannel) - - go func() { - storeChannel <- s.update(s.GetMaster(), preference) - close(storeChannel) - }() - - return storeChannel -} - -func (s SqlPreferenceStore) update(queryable Queryable, preference *model.Preference) StoreResult { - result := StoreResult{} - - if result.Err = preference.IsValid(); result.Err != nil { - return result - } - - if count, err := queryable.Update(preference); err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.Update", "We couldn't update the preference", - "user_id="+preference.UserId+", category="+preference.Category+", name="+preference.Name+", alt_id="+preference.AltId+", "+err.Error()) - } else { - result.Data = count - } - - return result -} - -func (s SqlPreferenceStore) SaveOrUpdate(preferences ...*model.Preference) StoreChannel { +func (s SqlPreferenceStore) Save(preferences *model.Preferences) StoreChannel { storeChannel := make(StoreChannel) go func() { result := StoreResult{} - db := s.GetReplica() - - if len(preferences) > 1 { - // wrap in a transaction so that if one fails, everything fails - transaction, err := db.Begin() - if err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.SaveOrUpdateMultiple", "Unable to open transaction to update preferences", err.Error()) - } else { - for _, preference := range preferences { - if upsertResult := s.saveOrUpdate(transaction, preference); upsertResult.Err != nil { - result = upsertResult - break - } + // wrap in a transaction so that if one fails, everything fails + transaction, err := s.GetReplica().Begin() + if err != nil { + result.Err = model.NewAppError("SqlPreferenceStore.Save", "Unable to open transaction to save preferences", err.Error()) + } else { + for _, preference := range *preferences { + if upsertResult := s.save(transaction, preference); upsertResult.Err != nil { + result = upsertResult + break } + } - if result.Err == nil { - if err := transaction.Commit(); err != nil { - // don't need to rollback here since the transaction is already closed - result.Err = model.NewAppError("SqlPreferenceStore.SaveOrUpdateMultiple", "Unable to commit transaction to update preferences", err.Error()) - } else { - result.Data = len(preferences) - } + if result.Err == nil { + if err := transaction.Commit(); err != nil { + // don't need to rollback here since the transaction is already closed + result.Err = model.NewAppError("SqlPreferenceStore.Save", "Unable to commit transaction to save preferences", err.Error()) } else { - if err := transaction.Rollback(); err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.SaveOrUpdateMultiple", "Unable to rollback transaction to update preferences", err.Error()) - } + result.Data = len(*preferences) + } + } else { + if err := transaction.Rollback(); err != nil { + result.Err = model.NewAppError("SqlPreferenceStore.Save", "Unable to rollback transaction to save preferences", err.Error()) } } - } else { - result = s.saveOrUpdate(db, preferences[0]) } storeChannel <- result @@ -142,9 +76,13 @@ func (s SqlPreferenceStore) SaveOrUpdate(preferences ...*model.Preference) Store return storeChannel } -func (s SqlPreferenceStore) saveOrUpdate(queryable Queryable, preference *model.Preference) StoreResult { +func (s SqlPreferenceStore) save(transaction *gorp.Transaction, preference *model.Preference) StoreResult { result := StoreResult{} + if result.Err = preference.IsValid(); result.Err != nil { + return result + } + params := map[string]interface{}{ "UserId": preference.UserId, "Category": preference.Category, @@ -154,7 +92,7 @@ func (s SqlPreferenceStore) saveOrUpdate(queryable Queryable, preference *model. } if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL { - if sqlResult, err := queryable.Exec( + if _, err := transaction.Exec( `INSERT INTO Preferences (UserId, Category, Name, AltId, Value) @@ -162,13 +100,11 @@ func (s SqlPreferenceStore) saveOrUpdate(queryable Queryable, preference *model. (:UserId, :Category, :Name, :AltId, :Value) ON DUPLICATE KEY UPDATE Value = :Value`, params); err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.saveOrUpdate", "We encountered an error while updating preferences", err.Error()) - } else { - result.Data, _ = sqlResult.RowsAffected() + result.Err = model.NewAppError("SqlPreferenceStore.save", "We encountered an error while updating preferences", err.Error()) } } else if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES { - // postgres has no way to upsert values until version 9.5 and trying inserting and then updating causes Transactions to abort - count, err := queryable.SelectInt( + // postgres has no way to upsert values until version 9.5 and trying inserting and then updating causes transactions to abort + count, err := transaction.SelectInt( `SELECT count(0) FROM @@ -179,30 +115,57 @@ func (s SqlPreferenceStore) saveOrUpdate(queryable Queryable, preference *model. AND Name = :Name AND AltId = :AltId`, params) if err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.saveOrUpdate", "We encountered an error while updating preferences", err.Error()) + result.Err = model.NewAppError("SqlPreferenceStore.save", "We encountered an error while updating preferences", err.Error()) return result } if count == 1 { - s.update(queryable, preference) + s.update(transaction, preference) } else { - s.save(queryable, preference) + s.insert(transaction, preference) } } else { - result.Err = model.NewAppError("SqlPreferenceStore.saveOrUpdate", "We encountered an error while updating preferences", + result.Err = model.NewAppError("SqlPreferenceStore.save", "We encountered an error while updating preferences", "Failed to update preference because of missing driver") } return result } +func (s SqlPreferenceStore) insert(transaction *gorp.Transaction, preference *model.Preference) StoreResult { + result := StoreResult{} + + if err := transaction.Insert(preference); err != nil { + if IsUniqueConstraintError(err.Error(), "UserId", "preferences_pkey") { + result.Err = model.NewAppError("SqlPreferenceStore.insert", "A preference with that user id, category, name, and alt id already exists", + "user_id="+preference.UserId+", category="+preference.Category+", name="+preference.Name+", alt_id="+preference.AltId+", "+err.Error()) + } else { + result.Err = model.NewAppError("SqlPreferenceStore.insert", "We couldn't save the preference", + "user_id="+preference.UserId+", category="+preference.Category+", name="+preference.Name+", alt_id="+preference.AltId+", "+err.Error()) + } + } + + return result +} + +func (s SqlPreferenceStore) update(transaction *gorp.Transaction, preference *model.Preference) StoreResult { + result := StoreResult{} + + if _, err := transaction.Update(preference); err != nil { + result.Err = model.NewAppError("SqlPreferenceStore.update", "We couldn't update the preference", + "user_id="+preference.UserId+", category="+preference.Category+", name="+preference.Name+", alt_id="+preference.AltId+", "+err.Error()) + } + + return result +} + func (s SqlPreferenceStore) GetByName(userId string, category string, name string) StoreChannel { storeChannel := make(StoreChannel) go func() { result := StoreResult{} - var preferences []*model.Preference + var preferences model.Preferences if _, err := s.GetReplica().Select(&preferences, `SELECT diff --git a/store/sql_preference_store_test.go b/store/sql_preference_store_test.go index 0574ee954..6d5ec0ecd 100644 --- a/store/sql_preference_store_test.go +++ b/store/sql_preference_store_test.go @@ -8,202 +8,104 @@ import ( "testing" ) -func TestPreferenceStoreSave(t *testing.T) { - Setup() - - p1 := model.Preference{ - UserId: model.NewId(), - Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOW, - AltId: model.NewId(), - } - - if err := (<-store.Preference().Save(&p1)).Err; err != nil { - t.Fatal("couldn't save preference", err) - } - - if err := (<-store.Preference().Save(&p1)).Err; err == nil { - t.Fatal("shouldn't be able to save duplicate preference") - } - - p2 := model.Preference{ - UserId: model.NewId(), - Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOW, - AltId: p1.AltId, - } - - if err := (<-store.Preference().Save(&p2)).Err; err != nil { - t.Fatal("couldn't save preference with duplicate category, name, alternate id", err) - } - - p3 := model.Preference{ - UserId: p1.UserId, - Category: model.PREFERENCE_CATEGORY_TEST, - Name: model.PREFERENCE_NAME_SHOW, - AltId: p1.AltId, - } - - if err := (<-store.Preference().Save(&p3)).Err; err != nil { - t.Fatal("couldn't save preference with duplicate user id, name, alternate id", err) - } - - p4 := model.Preference{ - UserId: p1.UserId, - Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_TEST, - AltId: p1.AltId, - } - - if err := (<-store.Preference().Save(&p4)).Err; err != nil { - t.Fatal("couldn't save preference with duplicate user id, category, alternate id", err) - } - - p5 := model.Preference{ - UserId: p1.UserId, - Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOW, - AltId: model.NewId(), - } - - if err := (<-store.Preference().Save(&p5)).Err; err != nil { - t.Fatal("couldn't save preference with duplicate user id, category, name", err) - } -} - -func TestPreferenceStoreUpdate(t *testing.T) { +func TestPreferenceSave(t *testing.T) { Setup() id := model.NewId() - p1 := model.Preference{ - UserId: id, - Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOW, - } - Must(store.Preference().Save(&p1)) - - p1.Value = "1234garbage" - if result := (<-store.Preference().Update(&p1)); result.Err != nil { - t.Fatal(result.Err) - } else if result.Data.(int64) != 1 { - t.Fatal("update should have changed only 1 row") - } - - p1.UserId = model.NewId() - if result := (<-store.Preference().Update(&p1)); result.Err != nil { - t.Fatal(result.Err) - } else if result.Data.(int64) != 0 { - t.Fatal("update shouldn't have made changes because of changed user id") - } - - p1.UserId = id - p1.Category = model.PREFERENCE_CATEGORY_TEST - if result := (<-store.Preference().Update(&p1)); result.Err != nil { - t.Fatal(result.Err) - } else if result.Data.(int64) != 0 { - t.Fatal("update shouldn't have made changes because of changed category") - } - - p1.Category = model.PREFERENCE_CATEGORY_DIRECT_CHANNELS - p1.Name = model.PREFERENCE_NAME_TEST - if result := (<-store.Preference().Update(&p1)); result.Err != nil { - t.Fatal(result.Err) - } else if result.Data.(int64) != 0 { - t.Fatal("update shouldn't have made changes because of changed name") - } - - p1.Name = model.PREFERENCE_NAME_SHOW - p1.AltId = model.NewId() - if result := (<-store.Preference().Update(&p1)); result.Err != nil { - t.Fatal(result.Err) - } else if result.Data.(int64) != 0 { - t.Fatal("update shouldn't have made changes because of changed alt id") - } -} - -func TestPreferenceSaveOrUpdate(t *testing.T) { - Setup() - - id := model.NewId() - - p1 := model.Preference{ - UserId: id, - Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOW, - Value: "value1", - } - Must(store.Preference().SaveOrUpdate(&p1)) - - if preferences := Must(store.Preference().GetByName(p1.UserId, p1.Category, p1.Name)).([]*model.Preference); len(preferences) != 1 { - t.Fatal("got incorrect number of preferences after SaveOrUpdate") - } else if preferences[0].Value != "value1" { - t.Fatal("should have received value1 after SaveOrUpdate") - } - - p1.Value = "value2" - Must(store.Preference().SaveOrUpdate(&p1)) - - if preferences := Must(store.Preference().GetByName(p1.UserId, p1.Category, p1.Name)).([]*model.Preference); len(preferences) != 1 { - t.Fatal("got incorrect number of preferences after second SaveOrUpdate") - } else if preferences[0].Value != "value2" { - t.Fatal("should have received value2 after SaveOrUpdate") + preferences := model.Preferences{ + { + UserId: id, + Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, + Name: model.PREFERENCE_NAME_SHOW, + Value: "value1a", + }, + { + UserId: id, + Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, + Name: model.PREFERENCE_NAME_TEST, + Value: "value1b", + }, + } + if count := Must(store.Preference().Save(&preferences)); count != 2 { + t.Fatal("got incorrect number of rows saved") + } + + for _, preference := range preferences { + if data := Must(store.Preference().GetByName(preference.UserId, preference.Category, preference.Name)).(model.Preferences); len(data) != 1 { + t.Fatal("got incorrect number of preferences after first Save") + } else if *preference != *data[0] { + t.Fatal("got incorrect preference after first Save") + } + } + + preferences[0].Value = "value2a" + preferences[1].Value = "value2b" + if count := Must(store.Preference().Save(&preferences)); count != 2 { + t.Fatal("got incorrect number of rows saved") + } + + for _, preference := range preferences { + if data := Must(store.Preference().GetByName(preference.UserId, preference.Category, preference.Name)).(model.Preferences); len(data) != 1 { + t.Fatal("got incorrect number of preferences after second Save") + } else if *preference != *data[0] { + t.Fatal("got incorrect preference after second Save") + } } } func TestPreferenceGetByName(t *testing.T) { Setup() - p1 := model.Preference{ - UserId: model.NewId(), - Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOW, - AltId: model.NewId(), - } - - // same user/category/name, different alt id - p2 := model.Preference{ - UserId: p1.UserId, - Category: p1.Category, - Name: p1.Name, - AltId: model.NewId(), - } - - // same user/category/alt id, different name - p3 := model.Preference{ - UserId: p1.UserId, - Category: p1.Category, - Name: model.PREFERENCE_NAME_TEST, - AltId: p1.AltId, - } - - // same user/name/alt id, different category - p4 := model.Preference{ - UserId: p1.UserId, - Category: model.PREFERENCE_CATEGORY_TEST, - Name: p1.Name, - AltId: p1.AltId, - } - - // same name/category/alt id, different user - p5 := model.Preference{ - UserId: model.NewId(), - Category: p1.Category, - Name: p1.Name, - AltId: p1.AltId, - } - - Must(store.Preference().Save(&p1)) - Must(store.Preference().Save(&p2)) - Must(store.Preference().Save(&p3)) - Must(store.Preference().Save(&p4)) - Must(store.Preference().Save(&p5)) - - if result := <-store.Preference().GetByName(p1.UserId, p1.Category, p1.Name); result.Err != nil { + userId := model.NewId() + category := model.PREFERENCE_CATEGORY_DIRECT_CHANNELS + name := model.PREFERENCE_NAME_SHOW + altId := model.NewId() + + preferences := model.Preferences{ + { + UserId: userId, + Category: category, + Name: name, + AltId: altId, + }, + // same user/category/name, different alt id + { + UserId: userId, + Category: category, + Name: name, + AltId: model.NewId(), + }, + // same user/category/alt id, different name + { + UserId: userId, + Category: category, + Name: model.PREFERENCE_NAME_TEST, + AltId: altId, + }, + // same user/name/alt id, different category + { + UserId: userId, + Category: model.PREFERENCE_CATEGORY_TEST, + Name: name, + AltId: altId, + }, + // same name/category/alt id, different user + { + UserId: model.NewId(), + Category: category, + Name: name, + AltId: altId, + }, + } + + Must(store.Preference().Save(&preferences)) + + if result := <-store.Preference().GetByName(userId, category, name); result.Err != nil { t.Fatal(result.Err) - } else if data := result.Data.([]*model.Preference); len(data) != 2 { + } else if data := result.Data.(model.Preferences); len(data) != 2 { t.Fatal("got the wrong number of preferences") - } else if !((*data[0] == p1 && *data[1] == p2) || (*data[0] == p2 && *data[1] == p1)) { + } else if !((*data[0] == *preferences[0] && *data[1] == *preferences[1]) || (*data[0] == *preferences[1] && *data[1] == *preferences[0])) { t.Fatal("got incorrect preferences") } } diff --git a/store/sql_store.go b/store/sql_store.go index c31515a44..4b055e455 100644 --- a/store/sql_store.go +++ b/store/sql_store.go @@ -612,11 +612,3 @@ func decrypt(key []byte, cryptoText string) (string, error) { return fmt.Sprintf("%s", ciphertext), nil } - -// Interface for both gorp.DbMap and gorp.Transaction to allow code for one to be reused with the other -type Queryable interface { - Exec(query string, args ...interface{}) (dbsql.Result, error) - Insert(list ...interface{}) error - SelectInt(query string, args ...interface{}) (int64, error) - Update(list ...interface{}) (int64, error) -} diff --git a/store/store.go b/store/store.go index df55bc6d8..9d9fd6489 100644 --- a/store/store.go +++ b/store/store.go @@ -152,8 +152,6 @@ type WebhookStore interface { } type PreferenceStore interface { - Save(preference *model.Preference) StoreChannel - Update(preference *model.Preference) StoreChannel - SaveOrUpdate(preferences ...*model.Preference) StoreChannel + Save(preferences *model.Preferences) StoreChannel GetByName(userId string, category string, name string) StoreChannel } -- cgit v1.2.3-1-g7c22