Made structural changes to server-side Preference code based on feedback

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) {
+func savePreferences(c *Context, w http.ResponseWriter, r *http.Request) {
-	var preferences []*model.Preference
+	preferences, err := model.PreferencesFromJson(r.Body)
-
+	if err != nil {
-	decoder := json.NewDecoder(r.Body)
+		c.Err = model.NewAppError("savePreferences", "Unable to decode preferences from request", err.Error())
-	if err := decoder.Decode(&preferences); err != nil {
-		c.Err = model.NewAppError("setPreferences", "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
+	}
 }

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{
+	preferences := model.Preferences{
-		UserId:   user.Id,
+		{
-		Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS,
+			UserId:   user.Id,
-		Name:     model.PREFERENCE_NAME_SHOW,
+			Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS,
-		AltId:    model.NewId(),
+			Name:     model.PREFERENCE_NAME_SHOW,
-		Value:    model.NewId(),
+			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()
+	preferences[0].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 updated incorrectly")
 	}
 }

model/client.go

@@ -844,12 +844,12 @@ func (c *Client) ListIncomingWebhooks() (*Result, *AppError) {
 	}
 }
 
-func (c *Client) SetPreferences(preferences []*Preference) (*Result, *AppError) {
+func (c *Client) SetPreferences(preferences *Preferences) (*Result, *AppError) {
-	if r, err := c.DoApiPost("/preferences/set", PreferenceListToJson(preferences)); err != nil {
+	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),
+		preferences, _ := PreferencesFromJson(r.Body)
-			r.Header.Get(HEADER_ETAG_SERVER), PreferenceListFromJson(r.Body)}, nil
+		return &Result{r.Header.Get(HEADER_REQUEST_ID), r.Header.Get(HEADER_ETAG_SERVER), preferences}, nil
 	}
 }
 

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)

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
+	}
+}

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 {
+func (s SqlPreferenceStore) Save(preferences *model.Preferences) 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 {
 	storeChannel := make(StoreChannel)
 
 	go func() {
 		result := StoreResult{}
 
-		db := s.GetReplica()
+		// wrap in a transaction so that if one fails, everything fails
-
+		transaction, err := s.GetReplica().Begin()
-		if len(preferences) > 1 {
+		if err != nil {
-			// wrap in a transaction so that if one fails, everything fails
+			result.Err = model.NewAppError("SqlPreferenceStore.Save", "Unable to open transaction to save preferences", err.Error())
-			transaction, err := db.Begin()
+		} else {
-			if err != nil {
+			for _, preference := range *preferences {
-				result.Err = model.NewAppError("SqlPreferenceStore.SaveOrUpdateMultiple", "Unable to open transaction to update preferences", err.Error())
+				if upsertResult := s.save(transaction, preference); upsertResult.Err != nil {
-			} else {
+					result = upsertResult
-				for _, preference := range preferences {
+					break
-					if upsertResult := s.saveOrUpdate(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
-				if result.Err == nil {
+					result.Err = model.NewAppError("SqlPreferenceStore.Save", "Unable to commit transaction to save preferences", err.Error())
-					if err := transaction.Commit(); err != nil {
+				} else {
-						// don't need to rollback here since the transaction is already closed
+					result.Data = len(*preferences)
-						result.Err = model.NewAppError("SqlPreferenceStore.SaveOrUpdateMultiple", "Unable to commit transaction to update preferences", err.Error())
+				}
-					} else {
+			} else {
-						result.Data = len(preferences)
+				if err := transaction.Rollback(); err != nil {
-					}
+					result.Err = model.NewAppError("SqlPreferenceStore.Save", "Unable to rollback 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())
-					}
 				}
 			}
-		} 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())
+			result.Err = model.NewAppError("SqlPreferenceStore.save", "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
+		// 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(
+		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

store/sql_preference_store_test.go

@@ -8,202 +8,104 @@ import (
 	"testing"
 )
 
-func TestPreferenceStoreSave(t *testing.T) {
+func TestPreferenceSave(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) {
 	Setup()
 
 	id := model.NewId()
 
-	p1 := model.Preference{
+	preferences := model.Preferences{
-		UserId:   id,
+		{
-		Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS,
+			UserId:   id,
-		Name:     model.PREFERENCE_NAME_SHOW,
+			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",
+		},
 	}
-	Must(store.Preference().Save(&p1))
+	if count := Must(store.Preference().Save(&preferences)); count != 2 {
-
+		t.Fatal("got incorrect number of rows saved")
-	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()
+	for _, preference := range preferences {
-	if result := (<-store.Preference().Update(&p1)); result.Err != nil {
+		if data := Must(store.Preference().GetByName(preference.UserId, preference.Category, preference.Name)).(model.Preferences); len(data) != 1 {
-		t.Fatal(result.Err)
+			t.Fatal("got incorrect number of preferences after first Save")
-	} else if result.Data.(int64) != 0 {
+		} else if *preference != *data[0] {
-		t.Fatal("update shouldn't have made changes because of changed user id")
+			t.Fatal("got incorrect preference after first Save")
+		}
 	}
 
-	p1.UserId = id
+	preferences[0].Value = "value2a"
-	p1.Category = model.PREFERENCE_CATEGORY_TEST
+	preferences[1].Value = "value2b"
-	if result := (<-store.Preference().Update(&p1)); result.Err != nil {
+	if count := Must(store.Preference().Save(&preferences)); count != 2 {
-		t.Fatal(result.Err)
+		t.Fatal("got incorrect number of rows saved")
-	} 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
+	for _, preference := range preferences {
-	p1.Name = model.PREFERENCE_NAME_TEST
+		if data := Must(store.Preference().GetByName(preference.UserId, preference.Category, preference.Name)).(model.Preferences); len(data) != 1 {
-	if result := (<-store.Preference().Update(&p1)); result.Err != nil {
+			t.Fatal("got incorrect number of preferences after second Save")
-		t.Fatal(result.Err)
+		} else if *preference != *data[0] {
-	} else if result.Data.(int64) != 0 {
+			t.Fatal("got incorrect preference after second Save")
-		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")
 	}
 }
 
 func TestPreferenceGetByName(t *testing.T) {
 	Setup()
 
-	p1 := model.Preference{
+	userId := model.NewId()
-		UserId:   model.NewId(),
+	category := model.PREFERENCE_CATEGORY_DIRECT_CHANNELS
-		Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNELS,
+	name := model.PREFERENCE_NAME_SHOW
-		Name:     model.PREFERENCE_NAME_SHOW,
+	altId := model.NewId()
-		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,
+		},
 	}
 
-	// same user/category/name, different alt id
+	Must(store.Preference().Save(&preferences))
-	p2 := model.Preference{
-		UserId:   p1.UserId,
-		Category: p1.Category,
-		Name:     p1.Name,
-		AltId:    model.NewId(),
-	}
 
-	// same user/category/alt id, different name
+	if result := <-store.Preference().GetByName(userId, category, name); result.Err != nil {
-	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 {
+	} 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")
 	}
 }

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)
-}

store/store.go

@@ -152,8 +152,6 @@ type WebhookStore interface {
 }
 
 type PreferenceStore interface {
-	Save(preference *model.Preference) StoreChannel
+	Save(preferences *model.Preferences) StoreChannel
-	Update(preference *model.Preference) StoreChannel
-	SaveOrUpdate(preferences ...*model.Preference) StoreChannel
 	GetByName(userId string, category string, name string) StoreChannel
 }