From da66599fa39ddbff96b0844fabac161e130a2bc4 Mon Sep 17 00:00:00 2001 From: hmhealey Date: Thu, 1 Oct 2015 10:56:07 -0400 Subject: Added Preferences table to store user preferences --- store/sql_preference_store.go | 125 +++++++++++++++++++++++++++ store/sql_preference_store_test.go | 170 +++++++++++++++++++++++++++++++++++++ store/sql_store.go | 30 ++++--- store/store.go | 7 ++ 4 files changed, 321 insertions(+), 11 deletions(-) create mode 100644 store/sql_preference_store.go create mode 100644 store/sql_preference_store_test.go (limited to 'store') diff --git a/store/sql_preference_store.go b/store/sql_preference_store.go new file mode 100644 index 000000000..3e3a91f61 --- /dev/null +++ b/store/sql_preference_store.go @@ -0,0 +1,125 @@ +// Copyright (c) 2015 Spinpunch, Inc. All Rights Reserved. +// See License.txt for license information. + +package store + +import ( + "github.com/mattermost/platform/model" +) + +type SqlPreferenceStore struct { + *SqlStore +} + +func NewSqlPreferenceStore(sqlStore *SqlStore) PreferenceStore { + s := &SqlPreferenceStore{sqlStore} + + for _, db := range sqlStore.GetAllConns() { + table := db.AddTableWithName(model.Preference{}, "Preferences").SetKeys(false, "UserId", "Category", "Name", "AltId") + table.ColMap("UserId").SetMaxSize(26) + table.ColMap("Category").SetMaxSize(32) + table.ColMap("Name").SetMaxSize(32) + table.ColMap("AltId").SetMaxSize(26) + table.ColMap("Value").SetMaxSize(128) + } + + return s +} + +func (s SqlPreferenceStore) UpgradeSchemaIfNeeded() { +} + +func (s SqlPreferenceStore) CreateIndexesIfNotExists() { + s.CreateIndexIfNotExists("idx_preferences_user_id", "Preferences", "UserId") + s.CreateIndexIfNotExists("idx_preferences_category", "Preferences", "Category") + s.CreateIndexIfNotExists("idx_preferences_name", "Preferences", "Name") +} + +func (s SqlPreferenceStore) Save(preference *model.Preference) StoreChannel { + storeChannel := make(StoreChannel) + + go func() { + result := StoreResult{} + + if result.Err = preference.IsValid(); result.Err != nil { + storeChannel <- result + close(storeChannel) + return + } + + if err := s.GetMaster().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 + } + + storeChannel <- result + close(storeChannel) + }() + + return storeChannel +} + +func (s SqlPreferenceStore) Update(preference *model.Preference) StoreChannel { + storeChannel := make(StoreChannel) + + go func() { + result := StoreResult{} + + if result.Err = preference.IsValid(); result.Err != nil { + storeChannel <- result + close(storeChannel) + return + } + + if count, err := s.GetMaster().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 if count != 1 { + 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) + } else { + result.Data = preference + } + + storeChannel <- result + close(storeChannel) + }() + + return storeChannel +} + +func (s SqlPreferenceStore) GetByName(userId string, category string, name string) StoreChannel { + storeChannel := make(StoreChannel) + + go func() { + result := StoreResult{} + + var preferences []*model.Preference + + if _, err := s.GetReplica().Select(&preferences, + `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.GetByName", "We encounted an error while finding preferences", err.Error()) + } else { + result.Data = preferences + } + + storeChannel <- result + close(storeChannel) + }() + + return storeChannel +} diff --git a/store/sql_preference_store_test.go b/store/sql_preference_store_test.go new file mode 100644 index 000000000..d7ef26374 --- /dev/null +++ b/store/sql_preference_store_test.go @@ -0,0 +1,170 @@ +// Copyright (c) 2015 Spinpunch, Inc. All Rights Reserved. +// See License.txt for license information. + +package store + +import ( + "github.com/mattermost/platform/model" + "testing" +) + +func TestPreferenceStoreSave(t *testing.T) { + Setup() + + p1 := model.Preference{ + UserId: model.NewId(), + Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, + Name: model.PREFERENCE_NAME_SHOWHIDE, + 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_SHOWHIDE, + 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_SHOWHIDE, + 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_SHOWHIDE, + 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) { + Setup() + + id := model.NewId() + + p1 := model.Preference{ + UserId: id, + Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, + Name: model.PREFERENCE_NAME_SHOWHIDE, + } + Must(store.Preference().Save(&p1)) + + p1.Value = "1234garbage" + if err := (<-store.Preference().Update(&p1)).Err; err != nil { + t.Fatal(err) + } + + p1.UserId = model.NewId() + if err := (<-store.Preference().Update(&p1)).Err; err == nil { + t.Fatal("update should have failed because of changed user id") + } + + p1.UserId = id + p1.Category = model.PREFERENCE_CATEGORY_TEST + if err := (<-store.Preference().Update(&p1)).Err; err == nil { + t.Fatal("update should have failed because of changed category") + } + + p1.Category = model.PREFERENCE_CATEGORY_DIRECT_CHANNELS + p1.Name = model.PREFERENCE_NAME_TEST + if err := (<-store.Preference().Update(&p1)).Err; err == nil { + t.Fatal("update should have failed because of changed name") + } + + p1.Name = model.PREFERENCE_NAME_SHOWHIDE + p1.AltId = model.NewId() + if err := (<-store.Preference().Update(&p1)).Err; err == nil { + t.Fatal("update should have failed because of changed alternate id") + } +} + +func TestPreferenceGetByName(t *testing.T) { + Setup() + + p1 := model.Preference{ + UserId: model.NewId(), + Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, + Name: model.PREFERENCE_NAME_SHOWHIDE, + 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 { + t.Fatal(result.Err) + } else if data := result.Data.([]*model.Preference); 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)) { + t.Fatal("got incorrect preferences") + } +} diff --git a/store/sql_store.go b/store/sql_store.go index 900543460..4b055e455 100644 --- a/store/sql_store.go +++ b/store/sql_store.go @@ -30,17 +30,18 @@ import ( ) type SqlStore struct { - master *gorp.DbMap - replicas []*gorp.DbMap - team TeamStore - channel ChannelStore - post PostStore - user UserStore - audit AuditStore - session SessionStore - oauth OAuthStore - system SystemStore - webhook WebhookStore + master *gorp.DbMap + replicas []*gorp.DbMap + team TeamStore + channel ChannelStore + post PostStore + user UserStore + audit AuditStore + session SessionStore + oauth OAuthStore + system SystemStore + webhook WebhookStore + preference PreferenceStore } func NewSqlStore() Store { @@ -93,6 +94,7 @@ func NewSqlStore() Store { sqlStore.oauth = NewSqlOAuthStore(sqlStore) sqlStore.system = NewSqlSystemStore(sqlStore) sqlStore.webhook = NewSqlWebhookStore(sqlStore) + sqlStore.preference = NewSqlPreferenceStore(sqlStore) sqlStore.master.CreateTablesIfNotExists() @@ -105,6 +107,7 @@ func NewSqlStore() Store { sqlStore.oauth.(*SqlOAuthStore).UpgradeSchemaIfNeeded() sqlStore.system.(*SqlSystemStore).UpgradeSchemaIfNeeded() sqlStore.webhook.(*SqlWebhookStore).UpgradeSchemaIfNeeded() + sqlStore.preference.(*SqlPreferenceStore).UpgradeSchemaIfNeeded() sqlStore.team.(*SqlTeamStore).CreateIndexesIfNotExists() sqlStore.channel.(*SqlChannelStore).CreateIndexesIfNotExists() @@ -115,6 +118,7 @@ func NewSqlStore() Store { sqlStore.oauth.(*SqlOAuthStore).CreateIndexesIfNotExists() sqlStore.system.(*SqlSystemStore).CreateIndexesIfNotExists() sqlStore.webhook.(*SqlWebhookStore).CreateIndexesIfNotExists() + sqlStore.preference.(*SqlPreferenceStore).CreateIndexesIfNotExists() if model.IsPreviousVersion(schemaVersion) { sqlStore.system.Update(&model.System{Name: "Version", Value: model.CurrentVersion}) @@ -472,6 +476,10 @@ func (ss SqlStore) Webhook() WebhookStore { return ss.webhook } +func (ss SqlStore) Preference() PreferenceStore { + return ss.preference +} + type mattermConverter struct{} func (me mattermConverter) ToDb(val interface{}) (interface{}, error) { diff --git a/store/store.go b/store/store.go index e539bc98a..8a8faae85 100644 --- a/store/store.go +++ b/store/store.go @@ -37,6 +37,7 @@ type Store interface { OAuth() OAuthStore System() SystemStore Webhook() WebhookStore + Preference() PreferenceStore Close() } @@ -149,3 +150,9 @@ type WebhookStore interface { GetIncomingByUser(userId string) StoreChannel DeleteIncoming(webhookId string, time int64) StoreChannel } + +type PreferenceStore interface { + Save(preference *model.Preference) StoreChannel + Update(preference *model.Preference) StoreChannel + GetByName(userId string, category string, name string) StoreChannel +} -- cgit v1.2.3-1-g7c22 From 599644fb2fa75d1760420806c8c821959fc6b645 Mon Sep 17 00:00:00 2001 From: hmhealey Date: Mon, 5 Oct 2015 12:03:27 -0400 Subject: Renamed show_hide preference to show --- store/sql_preference_store_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'store') diff --git a/store/sql_preference_store_test.go b/store/sql_preference_store_test.go index d7ef26374..99b183274 100644 --- a/store/sql_preference_store_test.go +++ b/store/sql_preference_store_test.go @@ -14,7 +14,7 @@ func TestPreferenceStoreSave(t *testing.T) { p1 := model.Preference{ UserId: model.NewId(), Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOWHIDE, + Name: model.PREFERENCE_NAME_SHOW, AltId: model.NewId(), } @@ -29,7 +29,7 @@ func TestPreferenceStoreSave(t *testing.T) { p2 := model.Preference{ UserId: model.NewId(), Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOWHIDE, + Name: model.PREFERENCE_NAME_SHOW, AltId: p1.AltId, } @@ -40,7 +40,7 @@ func TestPreferenceStoreSave(t *testing.T) { p3 := model.Preference{ UserId: p1.UserId, Category: model.PREFERENCE_CATEGORY_TEST, - Name: model.PREFERENCE_NAME_SHOWHIDE, + Name: model.PREFERENCE_NAME_SHOW, AltId: p1.AltId, } @@ -62,7 +62,7 @@ func TestPreferenceStoreSave(t *testing.T) { p5 := model.Preference{ UserId: p1.UserId, Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOWHIDE, + Name: model.PREFERENCE_NAME_SHOW, AltId: model.NewId(), } @@ -79,7 +79,7 @@ func TestPreferenceStoreUpdate(t *testing.T) { p1 := model.Preference{ UserId: id, Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOWHIDE, + Name: model.PREFERENCE_NAME_SHOW, } Must(store.Preference().Save(&p1)) @@ -105,7 +105,7 @@ func TestPreferenceStoreUpdate(t *testing.T) { t.Fatal("update should have failed because of changed name") } - p1.Name = model.PREFERENCE_NAME_SHOWHIDE + p1.Name = model.PREFERENCE_NAME_SHOW p1.AltId = model.NewId() if err := (<-store.Preference().Update(&p1)).Err; err == nil { t.Fatal("update should have failed because of changed alternate id") @@ -118,7 +118,7 @@ func TestPreferenceGetByName(t *testing.T) { p1 := model.Preference{ UserId: model.NewId(), Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOWHIDE, + Name: model.PREFERENCE_NAME_SHOW, AltId: model.NewId(), } -- cgit v1.2.3-1-g7c22 From a6cd2a79612d6d96e0e929ab769ec5e70cd35f5f Mon Sep 17 00:00:00 2001 From: hmhealey Date: Wed, 7 Oct 2015 10:19:02 -0400 Subject: Moved saving multiple user preferences into a database transaction --- store/sql_preference_store.go | 112 +++++++++++++++++++++++++++---------- store/sql_preference_store_test.go | 59 +++++++++++++++---- store/sql_store.go | 6 ++ store/store.go | 1 + 4 files changed, 139 insertions(+), 39 deletions(-) (limited to 'store') diff --git a/store/sql_preference_store.go b/store/sql_preference_store.go index 3e3a91f61..366a63fa6 100644 --- a/store/sql_preference_store.go +++ b/store/sql_preference_store.go @@ -39,53 +39,97 @@ func (s SqlPreferenceStore) Save(preference *model.Preference) StoreChannel { storeChannel := make(StoreChannel) go func() { - result := StoreResult{} + storeChannel <- s.save(s.GetMaster(), preference) + close(storeChannel) + }() - if result.Err = preference.IsValid(); result.Err != nil { - storeChannel <- result - close(storeChannel) - return - } + return storeChannel +} - if err := s.GetMaster().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()) - } +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.Data = preference + 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 + } - storeChannel <- result + 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(preference *model.Preference) 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 { storeChannel := make(StoreChannel) go func() { result := StoreResult{} - if result.Err = preference.IsValid(); result.Err != nil { - storeChannel <- result - close(storeChannel) - return - } + db := s.GetReplica() - if count, err := s.GetMaster().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 if count != 1 { - 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) + 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 err := s.saveOrUpdate(transaction, preference); err != nil { + result.Err = err + 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 { + if err := transaction.Rollback(); err != nil { + result.Err = model.NewAppError("SqlPreferenceStore.SaveOrUpdateMultiple", "Unable to rollback transaction to update preferences", err.Error()) + } + } + } } else { - result.Data = preference + result.Err = s.saveOrUpdate(db, preferences[0]) } storeChannel <- result @@ -95,6 +139,16 @@ func (s SqlPreferenceStore) Update(preference *model.Preference) StoreChannel { return storeChannel } +func (s SqlPreferenceStore) saveOrUpdate(queryable Queryable, preference *model.Preference) *model.AppError { + if result := s.save(queryable, preference); result.Err != nil { + if result := s.update(queryable, preference); result.Err != nil { + return result.Err + } + } + + return nil +} + func (s SqlPreferenceStore) GetByName(userId string, category string, name string) StoreChannel { storeChannel := make(StoreChannel) diff --git a/store/sql_preference_store_test.go b/store/sql_preference_store_test.go index 99b183274..0574ee954 100644 --- a/store/sql_preference_store_test.go +++ b/store/sql_preference_store_test.go @@ -84,31 +84,70 @@ func TestPreferenceStoreUpdate(t *testing.T) { Must(store.Preference().Save(&p1)) p1.Value = "1234garbage" - if err := (<-store.Preference().Update(&p1)).Err; err != nil { - t.Fatal(err) + 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 err := (<-store.Preference().Update(&p1)).Err; err == nil { - t.Fatal("update should have failed because of changed user id") + 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 err := (<-store.Preference().Update(&p1)).Err; err == nil { - t.Fatal("update should have failed because of changed category") + 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 err := (<-store.Preference().Update(&p1)).Err; err == nil { - t.Fatal("update should have failed because of changed name") + 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 err := (<-store.Preference().Update(&p1)).Err; err == nil { - t.Fatal("update should have failed because of changed alternate id") + 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") } } diff --git a/store/sql_store.go b/store/sql_store.go index 4b055e455..f8c585979 100644 --- a/store/sql_store.go +++ b/store/sql_store.go @@ -612,3 +612,9 @@ 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 { + Insert(list ...interface{}) error + Update(list ...interface{}) (int64, error) +} diff --git a/store/store.go b/store/store.go index 8a8faae85..df55bc6d8 100644 --- a/store/store.go +++ b/store/store.go @@ -154,5 +154,6 @@ type WebhookStore interface { type PreferenceStore interface { Save(preference *model.Preference) StoreChannel Update(preference *model.Preference) StoreChannel + SaveOrUpdate(preferences ...*model.Preference) StoreChannel GetByName(userId string, category string, name string) StoreChannel } -- cgit v1.2.3-1-g7c22 From 8a7aa7f66f8f244ac17eda54b2a63b9538b55435 Mon Sep 17 00:00:00 2001 From: hmhealey Date: Thu, 8 Oct 2015 10:52:20 -0400 Subject: Rewrote PreferenceStore.SaveOrUpdate to work on Postgres within a transaction --- store/sql_preference_store.go | 63 +++++++++++++++++++++++++++++++++++++------ store/sql_store.go | 2 ++ 2 files changed, 57 insertions(+), 8 deletions(-) (limited to 'store') diff --git a/store/sql_preference_store.go b/store/sql_preference_store.go index 366a63fa6..8d4e99109 100644 --- a/store/sql_preference_store.go +++ b/store/sql_preference_store.go @@ -5,6 +5,7 @@ package store import ( "github.com/mattermost/platform/model" + "github.com/mattermost/platform/utils" ) type SqlPreferenceStore struct { @@ -111,8 +112,8 @@ func (s SqlPreferenceStore) SaveOrUpdate(preferences ...*model.Preference) Store result.Err = model.NewAppError("SqlPreferenceStore.SaveOrUpdateMultiple", "Unable to open transaction to update preferences", err.Error()) } else { for _, preference := range preferences { - if err := s.saveOrUpdate(transaction, preference); err != nil { - result.Err = err + if upsertResult := s.saveOrUpdate(transaction, preference); upsertResult.Err != nil { + result = upsertResult break } } @@ -121,6 +122,8 @@ func (s SqlPreferenceStore) SaveOrUpdate(preferences ...*model.Preference) Store 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) } } else { if err := transaction.Rollback(); err != nil { @@ -129,7 +132,7 @@ func (s SqlPreferenceStore) SaveOrUpdate(preferences ...*model.Preference) Store } } } else { - result.Err = s.saveOrUpdate(db, preferences[0]) + result = s.saveOrUpdate(db, preferences[0]) } storeChannel <- result @@ -139,14 +142,58 @@ func (s SqlPreferenceStore) SaveOrUpdate(preferences ...*model.Preference) Store return storeChannel } -func (s SqlPreferenceStore) saveOrUpdate(queryable Queryable, preference *model.Preference) *model.AppError { - if result := s.save(queryable, preference); result.Err != nil { - if result := s.update(queryable, preference); result.Err != nil { - return result.Err +func (s SqlPreferenceStore) saveOrUpdate(queryable Queryable, preference *model.Preference) StoreResult { + result := StoreResult{} + + params := map[string]interface{}{ + "UserId": preference.UserId, + "Category": preference.Category, + "Name": preference.Name, + "AltId": preference.AltId, + "Value": preference.Value, + } + + if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL { + if sqlResult, err := queryable.Exec( + `INSERT INTO + Preferences + (UserId, Category, Name, AltId, Value) + VALUES + (: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() + } + } 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( + `SELECT + count(0) + FROM + Preferences + WHERE + UserId = :UserId + AND Category = :Category + 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()) + return result } + + if count == 1 { + s.update(queryable, preference) + } else { + s.save(queryable, preference) + } + } else { + result.Err = model.NewAppError("SqlPreferenceStore.saveOrUpdate", "We encountered an error while updating preferences", + "Failed to update preference because of missing driver") } - return nil + return result } func (s SqlPreferenceStore) GetByName(userId string, category string, name string) StoreChannel { diff --git a/store/sql_store.go b/store/sql_store.go index f8c585979..c31515a44 100644 --- a/store/sql_store.go +++ b/store/sql_store.go @@ -615,6 +615,8 @@ func decrypt(key []byte, cryptoText string) (string, error) { // 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) } -- cgit v1.2.3-1-g7c22 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 --- store/sql_preference_store.go | 161 +++++++++------------- store/sql_preference_store_test.go | 270 ++++++++++++------------------------- store/sql_store.go | 8 -- store/store.go | 4 +- 4 files changed, 149 insertions(+), 294 deletions(-) (limited to 'store') 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 From 2a39e8dbfab8506b09d0d030f87cac4c079b975a Mon Sep 17 00:00:00 2001 From: hmhealey Date: Tue, 13 Oct 2015 11:52:17 -0400 Subject: Removed Preference.AltId --- store/sql_preference_store.go | 51 +++++++++++++++++++------- store/sql_preference_store_test.go | 75 +++++++++++++++++++++++++------------- store/store.go | 3 +- 3 files changed, 89 insertions(+), 40 deletions(-) (limited to 'store') diff --git a/store/sql_preference_store.go b/store/sql_preference_store.go index 04bb5e4ee..59c52d888 100644 --- a/store/sql_preference_store.go +++ b/store/sql_preference_store.go @@ -17,11 +17,10 @@ func NewSqlPreferenceStore(sqlStore *SqlStore) PreferenceStore { s := &SqlPreferenceStore{sqlStore} for _, db := range sqlStore.GetAllConns() { - table := db.AddTableWithName(model.Preference{}, "Preferences").SetKeys(false, "UserId", "Category", "Name", "AltId") + table := db.AddTableWithName(model.Preference{}, "Preferences").SetKeys(false, "UserId", "Category", "Name") table.ColMap("UserId").SetMaxSize(26) table.ColMap("Category").SetMaxSize(32) table.ColMap("Name").SetMaxSize(32) - table.ColMap("AltId").SetMaxSize(26) table.ColMap("Value").SetMaxSize(128) } @@ -49,7 +48,7 @@ func (s SqlPreferenceStore) Save(preferences *model.Preferences) StoreChannel { 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 { + if upsertResult := s.save(transaction, &preference); upsertResult.Err != nil { result = upsertResult break } @@ -87,7 +86,6 @@ func (s SqlPreferenceStore) save(transaction *gorp.Transaction, preference *mode "UserId": preference.UserId, "Category": preference.Category, "Name": preference.Name, - "AltId": preference.AltId, "Value": preference.Value, } @@ -95,9 +93,9 @@ func (s SqlPreferenceStore) save(transaction *gorp.Transaction, preference *mode if _, err := transaction.Exec( `INSERT INTO Preferences - (UserId, Category, Name, AltId, Value) + (UserId, Category, Name, Value) VALUES - (:UserId, :Category, :Name, :AltId, :Value) + (:UserId, :Category, :Name, :Value) ON DUPLICATE KEY UPDATE Value = :Value`, params); err != nil { result.Err = model.NewAppError("SqlPreferenceStore.save", "We encountered an error while updating preferences", err.Error()) @@ -112,8 +110,7 @@ func (s SqlPreferenceStore) save(transaction *gorp.Transaction, preference *mode WHERE UserId = :UserId AND Category = :Category - AND Name = :Name - AND AltId = :AltId`, params) + AND Name = :Name`, params) if err != nil { result.Err = model.NewAppError("SqlPreferenceStore.save", "We encountered an error while updating preferences", err.Error()) return result @@ -137,11 +134,11 @@ func (s SqlPreferenceStore) insert(transaction *gorp.Transaction, preference *mo 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()) + result.Err = model.NewAppError("SqlPreferenceStore.insert", "A preference with that user id, category, and name already exists", + "user_id="+preference.UserId+", category="+preference.Category+", name="+preference.Name+", "+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()) + "user_id="+preference.UserId+", category="+preference.Category+", name="+preference.Name+", "+err.Error()) } } @@ -153,13 +150,13 @@ func (s SqlPreferenceStore) update(transaction *gorp.Transaction, preference *mo 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()) + "user_id="+preference.UserId+", category="+preference.Category+", name="+preference.Name+", "+err.Error()) } return result } -func (s SqlPreferenceStore) GetByName(userId string, category string, name string) StoreChannel { +func (s SqlPreferenceStore) Get(userId string, category string, name string) StoreChannel { storeChannel := make(StoreChannel) go func() { @@ -177,6 +174,34 @@ func (s SqlPreferenceStore) GetByName(userId string, category string, name strin AND Category = :Category AND Name = :Name`, map[string]interface{}{"UserId": userId, "Category": category, "Name": name}); err != nil { result.Err = model.NewAppError("SqlPreferenceStore.GetByName", "We encounted an error while finding preferences", err.Error()) + } else { + result.Data = preferences[0] + } + + storeChannel <- result + close(storeChannel) + }() + + return storeChannel +} + +func (s SqlPreferenceStore) GetCategory(userId string, category string) StoreChannel { + storeChannel := make(StoreChannel) + + go func() { + result := StoreResult{} + + var preferences model.Preferences + + 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", "We encounted an error while finding preferences", err.Error()) } else { result.Data = preferences } diff --git a/store/sql_preference_store_test.go b/store/sql_preference_store_test.go index 6d5ec0ecd..1feda01d9 100644 --- a/store/sql_preference_store_test.go +++ b/store/sql_preference_store_test.go @@ -16,14 +16,14 @@ func TestPreferenceSave(t *testing.T) { preferences := model.Preferences{ { UserId: id, - Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_SHOW, + Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW, + Name: model.NewId(), Value: "value1a", }, { UserId: id, - Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS, - Name: model.PREFERENCE_NAME_TEST, + Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW, + Name: model.NewId(), Value: "value1b", }, } @@ -32,9 +32,7 @@ func TestPreferenceSave(t *testing.T) { } 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] { + if data := Must(store.Preference().Get(preference.UserId, preference.Category, preference.Name)).(model.Preference); preference != data { t.Fatal("got incorrect preference after first Save") } } @@ -46,66 +44,91 @@ func TestPreferenceSave(t *testing.T) { } 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] { + if data := Must(store.Preference().Get(preference.UserId, preference.Category, preference.Name)).(model.Preference); preference != data { t.Fatal("got incorrect preference after second Save") } } } -func TestPreferenceGetByName(t *testing.T) { +func TestPreferenceGet(t *testing.T) { Setup() userId := model.NewId() - category := model.PREFERENCE_CATEGORY_DIRECT_CHANNELS - name := model.PREFERENCE_NAME_SHOW - altId := model.NewId() + category := model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW + name := model.PREFERENCE_NAME_TEST preferences := model.Preferences{ { UserId: userId, Category: category, Name: name, - AltId: altId, }, - // same user/category/name, different alt id + { + UserId: userId, + Category: category, + Name: model.NewId(), + }, + { + UserId: userId, + Category: model.PREFERENCE_CATEGORY_TEST, + Name: name, + }, + { + UserId: model.NewId(), + Category: category, + Name: name, + }, + } + + Must(store.Preference().Save(&preferences)) + + if result := <-store.Preference().Get(userId, category, name); result.Err != nil { + t.Fatal(result.Err) + } else if data := result.Data.(model.Preference); data != preferences[0] { + t.Fatal("got incorrect preference") + } +} + +func TestPreferenceGetCategory(t *testing.T) { + Setup() + + userId := model.NewId() + category := model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW + name := model.NewId() + + preferences := model.Preferences{ { UserId: userId, Category: category, Name: name, - AltId: model.NewId(), }, - // same user/category/alt id, different name + // same user/category, different name { UserId: userId, Category: category, - Name: model.PREFERENCE_NAME_TEST, - AltId: altId, + Name: model.NewId(), }, - // same user/name/alt id, different category + // same user/name, different category { UserId: userId, Category: model.PREFERENCE_CATEGORY_TEST, Name: name, - AltId: altId, }, - // same name/category/alt id, different user + // same name/category, 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 { + if result := <-store.Preference().GetCategory(userId, category); result.Err != nil { t.Fatal(result.Err) } else if data := result.Data.(model.Preferences); len(data) != 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 !((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/store.go b/store/store.go index 9d9fd6489..6e1614ccb 100644 --- a/store/store.go +++ b/store/store.go @@ -153,5 +153,6 @@ type WebhookStore interface { type PreferenceStore interface { Save(preferences *model.Preferences) StoreChannel - GetByName(userId string, category string, name string) StoreChannel + Get(userId string, category string, name string) StoreChannel + GetCategory(userId string, category string) StoreChannel } -- cgit v1.2.3-1-g7c22 From 97b2f6ffe7fa09a2188163740865322582b00b59 Mon Sep 17 00:00:00 2001 From: hmhealey Date: Tue, 13 Oct 2015 15:18:01 -0400 Subject: Made further changes based on feedback --- store/sql_preference_store.go | 8 ++++---- store/sql_preference_store_test.go | 18 +++++++++++++++--- 2 files changed, 19 insertions(+), 7 deletions(-) (limited to 'store') diff --git a/store/sql_preference_store.go b/store/sql_preference_store.go index 59c52d888..46cef38b1 100644 --- a/store/sql_preference_store.go +++ b/store/sql_preference_store.go @@ -162,9 +162,9 @@ func (s SqlPreferenceStore) Get(userId string, category string, name string) Sto go func() { result := StoreResult{} - var preferences model.Preferences + var preference model.Preference - if _, err := s.GetReplica().Select(&preferences, + if err := s.GetReplica().SelectOne(&preference, `SELECT * FROM @@ -173,9 +173,9 @@ func (s SqlPreferenceStore) Get(userId string, category string, name string) Sto UserId = :UserId AND Category = :Category AND Name = :Name`, map[string]interface{}{"UserId": userId, "Category": category, "Name": name}); err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.GetByName", "We encounted an error while finding preferences", err.Error()) + result.Err = model.NewAppError("SqlPreferenceStore.Get", "We encounted an error while finding preferences", err.Error()) } else { - result.Data = preferences[0] + result.Data = preference } storeChannel <- result diff --git a/store/sql_preference_store_test.go b/store/sql_preference_store_test.go index 1feda01d9..76b1bcb17 100644 --- a/store/sql_preference_store_test.go +++ b/store/sql_preference_store_test.go @@ -55,7 +55,7 @@ func TestPreferenceGet(t *testing.T) { userId := model.NewId() category := model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW - name := model.PREFERENCE_NAME_TEST + name := model.NewId() preferences := model.Preferences{ { @@ -70,7 +70,7 @@ func TestPreferenceGet(t *testing.T) { }, { UserId: userId, - Category: model.PREFERENCE_CATEGORY_TEST, + Category: model.NewId(), Name: name, }, { @@ -87,6 +87,11 @@ func TestPreferenceGet(t *testing.T) { } else if data := result.Data.(model.Preference); data != preferences[0] { t.Fatal("got incorrect preference") } + + // make sure getting a missing preference fails + if result := <-store.Preference().Get(model.NewId(), model.NewId(), model.NewId()); result.Err == nil { + t.Fatal("no error on getting a missing preference") + } } func TestPreferenceGetCategory(t *testing.T) { @@ -111,7 +116,7 @@ func TestPreferenceGetCategory(t *testing.T) { // same user/name, different category { UserId: userId, - Category: model.PREFERENCE_CATEGORY_TEST, + Category: model.NewId(), Name: name, }, // same name/category, different user @@ -131,4 +136,11 @@ func TestPreferenceGetCategory(t *testing.T) { } else if !((data[0] == preferences[0] && data[1] == preferences[1]) || (data[0] == preferences[1] && data[1] == preferences[0])) { t.Fatal("got incorrect preferences") } + + // make sure getting a missing preference category doesn't fail + if result := <-store.Preference().GetCategory(model.NewId(), model.NewId()); result.Err != nil { + t.Fatal(result.Err) + } else if data := result.Data.(model.Preferences); len(data) != 0 { + t.Fatal("shouldn't have got any preferences") + } } -- cgit v1.2.3-1-g7c22