From 515311b70f4bf337a3f40c478689d89fb5342762 Mon Sep 17 00:00:00 2001 From: Matti R Date: Fri, 27 Aug 2021 02:02:48 -0400 Subject: [PATCH 01/47] add user settings k/v DB table --- models/error.go | 30 +++++++++++++ models/models.go | 1 + models/user.go | 1 + models/user_setting.go | 98 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 130 insertions(+) create mode 100644 models/user_setting.go diff --git a/models/error.go b/models/error.go index fd8f2771ae25d..568dfe6586301 100644 --- a/models/error.go +++ b/models/error.go @@ -325,6 +325,36 @@ func (err ErrReachLimitOfRepo) Error() string { return fmt.Sprintf("user has reached maximum limit of repositories [limit: %d]", err.Limit) } +// ErrUserSettingExists represents a "setting already exists for user" error. +type ErrUserSettingExists struct { + Setting setting +} + +// IsErrUserSettingExists checks if an error is a ErrUserSettingExists. +func IsErrUserSettingExists(err error) bool { + _, ok := err.(ErrUserSettingExists) + return ok +} + +func (err ErrUserSettingExists) Error() string { + return fmt.Sprintf("setting already exists for user [uid: %d, key: %s]", err.Setting.UserID, err.Setting.Key) +} + +// ErrUserSettingNotExists represents a "setting already exists for user" error. +type ErrUserSettingNotExists struct { + Setting setting +} + +// IsErrUserSettingNotExists checks if an error is a ErrUserSettingNotExists. +func IsErrUserSettingExists(err error) bool { + _, ok := err.(ErrUserSettingNotExists) + return ok +} + +func (err ErrUserSettingNotExists) Error() string { + return fmt.Sprintf("setting already exists for user [uid: %d, key: %s]", err.Setting.UserID, err.Setting.Key) +} + // __ __.__ __ .__ // / \ / \__| | _|__| // \ \/\/ / | |/ / | diff --git a/models/models.go b/models/models.go index 4e1448241af86..5d6f782a233e7 100755 --- a/models/models.go +++ b/models/models.go @@ -139,6 +139,7 @@ func init() { new(PushMirror), new(RepoArchiver), new(ProtectedTag), + new(UserSetting), ) gonicNames := []string{"SSL", "UID"} diff --git a/models/user.go b/models/user.go index a2d185de39d22..4024b66be2499 100644 --- a/models/user.go +++ b/models/user.go @@ -1201,6 +1201,7 @@ func deleteUser(e Engine, u *User) error { &TeamUser{UID: u.ID}, &Collaboration{UserID: u.ID}, &Stopwatch{UserID: u.ID}, + &UserSetting{UserID: u.ID}, ); err != nil { return fmt.Errorf("deleteBeans: %v", err) } diff --git a/models/user_setting.go b/models/user_setting.go new file mode 100644 index 0000000000000..f81907633df9e --- /dev/null +++ b/models/user_setting.go @@ -0,0 +1,98 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +type UserSettings struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"index"` // to load all of someone's settings + Key string `xorm:"varchar(255) index"` // ensure key is always lowercase + Value string `xorm:"text"` +} + +// BeforeInsert will be invoked by XORM before inserting a record +func (setting *UserSettings) BeforeInsert() { + setting.Key = strings.ToLower(setting.Key) +} + +// GetUserSetting returns specific settings from user +func GetUserSetting(uid int64, keys []string) ([]*UserSettings, error) { + settings := make([]*UserSettings, 0, 5) + if err := x. + Where("uid=?", uid). + And(builder.In("key", keys)). + Asc("id"). + Find(&settings); err != nil { + return nil, err + } + return settings, nil +} + +// GetUserAllSettings returns all settings from user +func GetUserAllSettings(uid int64) ([]*UserSettings, error) { + settings := make([]*UserSettings, 0, 5) + if err := x. + Where("uid=?", uid). + Asc("id"). + Find(&settings); err != nil { + return nil, err + } + return settings, nil +} + +// AddUserSetting adds a specific setting for a user +func AddUserSetting(setting *UserSettings) error { + return addUserSetting(x, setting) +} + +func addUserSetting(e Engine, setting *UserSettings) error { + used, err := settingExists(e, setting.UserID, setting.Key) + if err != nil { + return err + } else if used { + return ErrUserSettingExists{setting} + } + _, err = e.Insert(setting) + return err +} + +func settingExists(e Engine, uid int64, key string) (bool, error) { + if len(key) == 0 { + return true, nil + } + + return e.Where("key=?", strings.ToLower(key)).And("user_id = ?", uid).Get(&UserSettings{}) +} + +// DeleteUserSetting deletes a specific setting for a user +func DeleteUserSetting(setting *UserSettings) error { + sess := x.NewSession() + defer sess.Close() + if err = sess.Begin(); err != nil { + return err + } + + if _, err = sess.Delete(setting); err != nil { + return err + } + + return sess.Commit() +} + +// UpdateUserSetting updates a users' setting for a specific key +func UpdateUserSetting(setting *UserSettings) error { + return updateUserSetting(x, setting) +} + +func updateUserSetting(e Engine, setting *UserSettings) error { + used, err := settingExists(e, setting.UserID, setting.Key) + if err != nil { + return err + } else if !used { + return ErrUserSettingNotExists{setting} + } + + _, err := e.ID(u.ID).Cols("value").Update(setting) + return err +} From 40c45f4e2d5ea0c2fd66b371d96b135de48064b5 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sat, 28 Aug 2021 21:21:24 -0400 Subject: [PATCH 02/47] resolve lint issues --- models/error.go | 6 +++--- models/user_setting.go | 41 ++++++++++++++++++++++++----------------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/models/error.go b/models/error.go index 568dfe6586301..93155aa287d47 100644 --- a/models/error.go +++ b/models/error.go @@ -327,7 +327,7 @@ func (err ErrReachLimitOfRepo) Error() string { // ErrUserSettingExists represents a "setting already exists for user" error. type ErrUserSettingExists struct { - Setting setting + Setting *UserSetting } // IsErrUserSettingExists checks if an error is a ErrUserSettingExists. @@ -342,11 +342,11 @@ func (err ErrUserSettingExists) Error() string { // ErrUserSettingNotExists represents a "setting already exists for user" error. type ErrUserSettingNotExists struct { - Setting setting + Setting *UserSetting } // IsErrUserSettingNotExists checks if an error is a ErrUserSettingNotExists. -func IsErrUserSettingExists(err error) bool { +func IsErrUserSettingNotExists(err error) bool { _, ok := err.(ErrUserSettingNotExists) return ok } diff --git a/models/user_setting.go b/models/user_setting.go index f81907633df9e..6e506ba93b6e6 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -4,7 +4,14 @@ package models -type UserSettings struct { +import ( + "strings" + + "xorm.io/builder" +) + +// UserSetting is a key value store of user settings +type UserSetting struct { ID int64 `xorm:"pk autoincr"` UserID int64 `xorm:"index"` // to load all of someone's settings Key string `xorm:"varchar(255) index"` // ensure key is always lowercase @@ -12,13 +19,13 @@ type UserSettings struct { } // BeforeInsert will be invoked by XORM before inserting a record -func (setting *UserSettings) BeforeInsert() { +func (setting *UserSetting) BeforeInsert() { setting.Key = strings.ToLower(setting.Key) } // GetUserSetting returns specific settings from user -func GetUserSetting(uid int64, keys []string) ([]*UserSettings, error) { - settings := make([]*UserSettings, 0, 5) +func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { + settings := make([]*UserSetting, 0, 5) if err := x. Where("uid=?", uid). And(builder.In("key", keys)). @@ -30,8 +37,8 @@ func GetUserSetting(uid int64, keys []string) ([]*UserSettings, error) { } // GetUserAllSettings returns all settings from user -func GetUserAllSettings(uid int64) ([]*UserSettings, error) { - settings := make([]*UserSettings, 0, 5) +func GetUserAllSettings(uid int64) ([]*UserSetting, error) { + settings := make([]*UserSetting, 0, 5) if err := x. Where("uid=?", uid). Asc("id"). @@ -42,11 +49,11 @@ func GetUserAllSettings(uid int64) ([]*UserSettings, error) { } // AddUserSetting adds a specific setting for a user -func AddUserSetting(setting *UserSettings) error { +func AddUserSetting(setting *UserSetting) error { return addUserSetting(x, setting) } -func addUserSetting(e Engine, setting *UserSettings) error { +func addUserSetting(e Engine, setting *UserSetting) error { used, err := settingExists(e, setting.UserID, setting.Key) if err != nil { return err @@ -62,30 +69,30 @@ func settingExists(e Engine, uid int64, key string) (bool, error) { return true, nil } - return e.Where("key=?", strings.ToLower(key)).And("user_id = ?", uid).Get(&UserSettings{}) + return e.Where("key=?", strings.ToLower(key)).And("user_id = ?", uid).Get(&UserSetting{}) } // DeleteUserSetting deletes a specific setting for a user -func DeleteUserSetting(setting *UserSettings) error { +func DeleteUserSetting(setting *UserSetting) error { sess := x.NewSession() defer sess.Close() - if err = sess.Begin(); err != nil { + if err := sess.Begin(); err != nil { return err } - if _, err = sess.Delete(setting); err != nil { + if _, err := sess.Delete(setting); err != nil { return err } return sess.Commit() } -// UpdateUserSetting updates a users' setting for a specific key -func UpdateUserSetting(setting *UserSettings) error { - return updateUserSetting(x, setting) +// UpdateUserSettingValue updates a users' setting for a specific key +func UpdateUserSettingValue(setting *UserSetting) error { + return updateUserSettingValue(x, setting) } -func updateUserSetting(e Engine, setting *UserSettings) error { +func updateUserSettingValue(e Engine, setting *UserSetting) error { used, err := settingExists(e, setting.UserID, setting.Key) if err != nil { return err @@ -93,6 +100,6 @@ func updateUserSetting(e Engine, setting *UserSettings) error { return ErrUserSettingNotExists{setting} } - _, err := e.ID(u.ID).Cols("value").Update(setting) + _, err = e.ID(setting.ID).Cols("value").Update(setting) return err } From a0a1dacbebfc20acb26cb6fccde6a9d4c9c5cec8 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 29 Aug 2021 00:44:11 -0400 Subject: [PATCH 03/47] add migration & tests --- models/migrations/migrations.go | 2 ++ models/migrations/v193.go | 25 ++++++++++++++++++++ models/user_setting_test.go | 41 +++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 models/migrations/v193.go create mode 100644 models/user_setting_test.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 79b1e90ecd734..8c2774f76b2c7 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -338,6 +338,8 @@ var migrations = []Migration{ NewMigration("Alter issue/comment table TEXT fields to LONGTEXT", alterIssueAndCommentTextFieldsToLongText), // v192 -> v193 NewMigration("RecreateIssueResourceIndexTable to have a primary key instead of an unique index", recreateIssueResourceIndexTable), + // v193 -> v194 + NewMigration("Create key/value table for user settings", createUserSettingsTable), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v193.go b/models/migrations/v193.go new file mode 100644 index 0000000000000..a2d45364b899e --- /dev/null +++ b/models/migrations/v193.go @@ -0,0 +1,25 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "fmt" + + "xorm.io/xorm" +) + +func createUserSettingsTable(x *xorm.Engine) error { + type UserSetting struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"index"` // to load all of someone's settings + Key string `xorm:"varchar(255) index"` // ensure key is always lowercase + Value string `xorm:"text"` + } + if err := x.Sync2(new(UserSetting)); err != nil { + return fmt.Errorf("sync2: %v", err) + } + return nil + +} diff --git a/models/user_setting_test.go b/models/user_setting_test.go new file mode 100644 index 0000000000000..8f4e336a89d0d --- /dev/null +++ b/models/user_setting_test.go @@ -0,0 +1,41 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestUserSettings(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + newSetting := &UserSetting{UserID: 99, Key: "test_user_setting", Value: "Gitea User Setting Test"} + + // create setting + err := AddUserSetting(newSetting) + assert.NoError(t, err) + + // get specific setting + userSetting, err := GetUserSetting(99, "test_user_setting") + assert.NoError(t, err) + assert.EqualValues(t, newSetting.Value, userSetting.Value) + + // updated setting + updatedSetting := &UserSetting{UserID: 99, Key: "test_user_setting", Value: "Updated", ID: userSetting.ID} + err = UpdateUserSettingValue(updatedSetting) + assert.NoError(t, err) + + // get all settings + userSettings, err := GetUserAllSettings(99) + assert.NoError(t, err) + assert.Len(t, userSettings, 1) + assert.EqualValues(t, userSettings[0].Value, updatedSetting.Value) + + // delete setting + err = DeleteUserSetting(updatedSetting) + assert.NoError(t, err) +} From d6c169c39b968ddfbbd38d14a4a05636b6d8416d Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 29 Aug 2021 00:50:39 -0400 Subject: [PATCH 04/47] fix lint --- models/user_setting_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/models/user_setting_test.go b/models/user_setting_test.go index 8f4e336a89d0d..467f96e472c81 100644 --- a/models/user_setting_test.go +++ b/models/user_setting_test.go @@ -20,17 +20,17 @@ func TestUserSettings(t *testing.T) { assert.NoError(t, err) // get specific setting - userSetting, err := GetUserSetting(99, "test_user_setting") + userSettings, err := GetUserSetting(99, []string{"test_user_setting"}) assert.NoError(t, err) - assert.EqualValues(t, newSetting.Value, userSetting.Value) + assert.EqualValues(t, newSetting.Value, userSettings[0].Value) // updated setting - updatedSetting := &UserSetting{UserID: 99, Key: "test_user_setting", Value: "Updated", ID: userSetting.ID} + updatedSetting := &UserSetting{UserID: 99, Key: "test_user_setting", Value: "Updated", ID: userSettings[0].ID} err = UpdateUserSettingValue(updatedSetting) assert.NoError(t, err) // get all settings - userSettings, err := GetUserAllSettings(99) + userSettings, err = GetUserAllSettings(99) assert.NoError(t, err) assert.Len(t, userSettings, 1) assert.EqualValues(t, userSettings[0].Value, updatedSetting.Value) From 9614a3f0ea18a493c2d0248a980d772df01c5b80 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 29 Aug 2021 01:03:17 -0400 Subject: [PATCH 05/47] fix sql query --- models/user_setting.go | 4 ++-- models/user_setting_test.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/models/user_setting.go b/models/user_setting.go index 6e506ba93b6e6..ea2f54202ed17 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -27,7 +27,7 @@ func (setting *UserSetting) BeforeInsert() { func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { settings := make([]*UserSetting, 0, 5) if err := x. - Where("uid=?", uid). + Where("user_id=?", uid). And(builder.In("key", keys)). Asc("id"). Find(&settings); err != nil { @@ -40,7 +40,7 @@ func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { func GetUserAllSettings(uid int64) ([]*UserSetting, error) { settings := make([]*UserSetting, 0, 5) if err := x. - Where("uid=?", uid). + Where("user_id=?", uid). Asc("id"). Find(&settings); err != nil { return nil, err diff --git a/models/user_setting_test.go b/models/user_setting_test.go index 467f96e472c81..639afa7b1281f 100644 --- a/models/user_setting_test.go +++ b/models/user_setting_test.go @@ -22,6 +22,7 @@ func TestUserSettings(t *testing.T) { // get specific setting userSettings, err := GetUserSetting(99, []string{"test_user_setting"}) assert.NoError(t, err) + assert.Len(t, userSettings, 1) assert.EqualValues(t, newSetting.Value, userSettings[0].Value) // updated setting From 147a52cec72b1a03292d6cd72d4e9a09a5445b9f Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 29 Aug 2021 22:40:09 -0400 Subject: [PATCH 06/47] update per Lunny's feedback --- models/user_setting.go | 24 ++++++++++++++---------- models/user_setting_test.go | 4 ++-- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/models/user_setting.go b/models/user_setting.go index ea2f54202ed17..226c41cad7548 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -1,4 +1,4 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. +// Copyright 2021 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. @@ -23,6 +23,11 @@ func (setting *UserSetting) BeforeInsert() { setting.Key = strings.ToLower(setting.Key) } +// BeforeUpdate will be invoked by XORM before updating a record +func (setting *UserSetting) BeforeUpdate() { + setting.Key = strings.ToLower(setting.Key) +} + // GetUserSetting returns specific settings from user func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { settings := make([]*UserSetting, 0, 5) @@ -48,11 +53,6 @@ func GetUserAllSettings(uid int64) ([]*UserSetting, error) { return settings, nil } -// AddUserSetting adds a specific setting for a user -func AddUserSetting(setting *UserSetting) error { - return addUserSetting(x, setting) -} - func addUserSetting(e Engine, setting *UserSetting) error { used, err := settingExists(e, setting.UserID, setting.Key) if err != nil { @@ -69,7 +69,7 @@ func settingExists(e Engine, uid int64, key string) (bool, error) { return true, nil } - return e.Where("key=?", strings.ToLower(key)).And("user_id = ?", uid).Get(&UserSetting{}) + return e.Table(&UserSetting{}).Exist(&UserSetting{UserID: uid, Key: strings.ToLower(key)}) } // DeleteUserSetting deletes a specific setting for a user @@ -87,9 +87,13 @@ func DeleteUserSetting(setting *UserSetting) error { return sess.Commit() } -// UpdateUserSettingValue updates a users' setting for a specific key -func UpdateUserSettingValue(setting *UserSetting) error { - return updateUserSettingValue(x, setting) +// SetUserSetting updates a users' setting for a specific key +func SetUserSetting(setting *UserSetting) error { + err := addUserSetting(x, setting) + if err != nil && IsErrUserSettingExists(err) { + return updateUserSettingValue(x, setting) + } + return err } func updateUserSettingValue(e Engine, setting *UserSetting) error { diff --git a/models/user_setting_test.go b/models/user_setting_test.go index 639afa7b1281f..12d020d5730d1 100644 --- a/models/user_setting_test.go +++ b/models/user_setting_test.go @@ -16,7 +16,7 @@ func TestUserSettings(t *testing.T) { newSetting := &UserSetting{UserID: 99, Key: "test_user_setting", Value: "Gitea User Setting Test"} // create setting - err := AddUserSetting(newSetting) + err := SetUserSetting(newSetting) assert.NoError(t, err) // get specific setting @@ -27,7 +27,7 @@ func TestUserSettings(t *testing.T) { // updated setting updatedSetting := &UserSetting{UserID: 99, Key: "test_user_setting", Value: "Updated", ID: userSettings[0].ID} - err = UpdateUserSettingValue(updatedSetting) + err = SetUserSetting(updatedSetting) assert.NoError(t, err) // get all settings From ebae68e865675e6c66f9735df25656dd344db56a Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Thu, 30 Sep 2021 19:55:51 -0400 Subject: [PATCH 07/47] Update models/user_setting.go Co-authored-by: zeripath --- models/user_setting.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/user_setting.go b/models/user_setting.go index 226c41cad7548..22e952bf7350b 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -13,8 +13,8 @@ import ( // UserSetting is a key value store of user settings type UserSetting struct { ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"index"` // to load all of someone's settings - Key string `xorm:"varchar(255) index"` // ensure key is always lowercase + UserID int64 `xorm:"index unique(key_userid)"` // to load all of someone's settings + Key string `xorm:"varchar(255) index unique(key_userid)"` // ensure key is always lowercase Value string `xorm:"text"` } From 9f72e70152d5da7b39879ef74afeb1bf6cdc620c Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sat, 2 Oct 2021 21:57:45 -0400 Subject: [PATCH 08/47] Rename v193.go to v196.go --- models/migrations/{v193.go => v196.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename models/migrations/{v193.go => v196.go} (100%) diff --git a/models/migrations/v193.go b/models/migrations/v196.go similarity index 100% rename from models/migrations/v193.go rename to models/migrations/v196.go From a3a82716d72bd1c631c7e1684542f50685b0ef4e Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sat, 2 Oct 2021 21:58:06 -0400 Subject: [PATCH 09/47] Rename v196.go to v198.go --- models/migrations/{v196.go => v198.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename models/migrations/{v196.go => v198.go} (100%) diff --git a/models/migrations/v196.go b/models/migrations/v198.go similarity index 100% rename from models/migrations/v196.go rename to models/migrations/v198.go From caaf87309986a1895a278192aea97f96c6fd1ce7 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sat, 2 Oct 2021 22:07:06 -0400 Subject: [PATCH 10/47] refactor PR from recent models/db refactor --- models/migrations/{v198.go => v197.go} | 0 models/user_setting.go | 16 +++++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) rename models/migrations/{v198.go => v197.go} (100%) diff --git a/models/migrations/v198.go b/models/migrations/v197.go similarity index 100% rename from models/migrations/v198.go rename to models/migrations/v197.go diff --git a/models/user_setting.go b/models/user_setting.go index 22e952bf7350b..2b62663636ab6 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -7,13 +7,15 @@ package models import ( "strings" + "code.gitea.io/gitea/models/db" + "xorm.io/builder" ) // UserSetting is a key value store of user settings type UserSetting struct { ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"index unique(key_userid)"` // to load all of someone's settings + UserID int64 `xorm:"index unique(key_userid)"` // to load all of someone's settings Key string `xorm:"varchar(255) index unique(key_userid)"` // ensure key is always lowercase Value string `xorm:"text"` } @@ -28,10 +30,14 @@ func (setting *UserSetting) BeforeUpdate() { setting.Key = strings.ToLower(setting.Key) } +func init() { + db.RegisterModel(new(UserSetting)) +} + // GetUserSetting returns specific settings from user func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { settings := make([]*UserSetting, 0, 5) - if err := x. + if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). And(builder.In("key", keys)). Asc("id"). @@ -44,7 +50,7 @@ func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { // GetUserAllSettings returns all settings from user func GetUserAllSettings(uid int64) ([]*UserSetting, error) { settings := make([]*UserSetting, 0, 5) - if err := x. + if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). Asc("id"). Find(&settings); err != nil { @@ -74,7 +80,7 @@ func settingExists(e Engine, uid int64, key string) (bool, error) { // DeleteUserSetting deletes a specific setting for a user func DeleteUserSetting(setting *UserSetting) error { - sess := x.NewSession() + sess := db.GetEngine(db.DefaultContext).NewSession() defer sess.Close() if err := sess.Begin(); err != nil { return err @@ -89,7 +95,7 @@ func DeleteUserSetting(setting *UserSetting) error { // SetUserSetting updates a users' setting for a specific key func SetUserSetting(setting *UserSetting) error { - err := addUserSetting(x, setting) + err := addUserSetting(db.GetEngine(db.DefaultContext), setting) if err != nil && IsErrUserSettingExists(err) { return updateUserSettingValue(x, setting) } From f04598cdd302f75e2c330843cb213d66df771d0d Mon Sep 17 00:00:00 2001 From: Matti R Date: Sat, 2 Oct 2021 22:10:15 -0400 Subject: [PATCH 11/47] refactor PR from recent models/db refactor --- models/user_setting.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/models/user_setting.go b/models/user_setting.go index 2b62663636ab6..2ee16d9bd0e68 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -59,7 +59,7 @@ func GetUserAllSettings(uid int64) ([]*UserSetting, error) { return settings, nil } -func addUserSetting(e Engine, setting *UserSetting) error { +func addUserSetting(e db.Engine, setting *UserSetting) error { used, err := settingExists(e, setting.UserID, setting.Key) if err != nil { return err @@ -70,7 +70,7 @@ func addUserSetting(e Engine, setting *UserSetting) error { return err } -func settingExists(e Engine, uid int64, key string) (bool, error) { +func settingExists(e db.Engine, uid int64, key string) (bool, error) { if len(key) == 0 { return true, nil } @@ -97,12 +97,12 @@ func DeleteUserSetting(setting *UserSetting) error { func SetUserSetting(setting *UserSetting) error { err := addUserSetting(db.GetEngine(db.DefaultContext), setting) if err != nil && IsErrUserSettingExists(err) { - return updateUserSettingValue(x, setting) + return updateUserSettingValue(db.GetEngine(db.DefaultContext), setting) } return err } -func updateUserSettingValue(e Engine, setting *UserSetting) error { +func updateUserSettingValue(e db.Engine, setting *UserSetting) error { used, err := settingExists(e, setting.UserID, setting.Key) if err != nil { return err From d82b4de21f03f806c162fca586cf557e5b418b95 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sat, 2 Oct 2021 22:14:13 -0400 Subject: [PATCH 12/47] use correct way to start new session --- models/user_setting.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/user_setting.go b/models/user_setting.go index 2ee16d9bd0e68..b6483eac1ba68 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -80,7 +80,7 @@ func settingExists(e db.Engine, uid int64, key string) (bool, error) { // DeleteUserSetting deletes a specific setting for a user func DeleteUserSetting(setting *UserSetting) error { - sess := db.GetEngine(db.DefaultContext).NewSession() + sess := db.NewSession(db.DefaultContext) defer sess.Close() if err := sess.Begin(); err != nil { return err From d48c711a1282782bea9d2712ce88410116ceeb32 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sat, 2 Oct 2021 22:16:32 -0400 Subject: [PATCH 13/47] fix tests --- models/user_setting_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models/user_setting_test.go b/models/user_setting_test.go index 12d020d5730d1..629b4c1b8ec70 100644 --- a/models/user_setting_test.go +++ b/models/user_setting_test.go @@ -7,11 +7,13 @@ package models import ( "testing" + "code.gitea.io/gitea/models/db" + "github.com/stretchr/testify/assert" ) func TestUserSettings(t *testing.T) { - assert.NoError(t, PrepareTestDatabase()) + assert.NoError(t, db.PrepareTestDatabase()) newSetting := &UserSetting{UserID: 99, Key: "test_user_setting", Value: "Gitea User Setting Test"} From 0314c5637b93cd9afeca20de4e08d4ce55050d4e Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Wed, 6 Oct 2021 22:42:24 -0400 Subject: [PATCH 14/47] Apply suggestions from code review --- models/migrations/v197.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/migrations/v197.go b/models/migrations/v197.go index a2d45364b899e..3844126aeb01f 100644 --- a/models/migrations/v197.go +++ b/models/migrations/v197.go @@ -13,7 +13,8 @@ import ( func createUserSettingsTable(x *xorm.Engine) error { type UserSetting struct { ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"index"` // to load all of someone's settings + UserID int64 `xorm:"index unique(key_userid)"` // to load all of someone's settings + Key string `xorm:"varchar(255) index unique(key_userid)"` // ensure key is always lowercase Key string `xorm:"varchar(255) index"` // ensure key is always lowercase Value string `xorm:"text"` } From 2890b7bf1152da314d715c51fc3ba3d01f073b88 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Wed, 6 Oct 2021 22:42:50 -0400 Subject: [PATCH 15/47] Update models/migrations/v197.go --- models/migrations/v197.go | 1 - 1 file changed, 1 deletion(-) diff --git a/models/migrations/v197.go b/models/migrations/v197.go index 3844126aeb01f..92dddb3cd1405 100644 --- a/models/migrations/v197.go +++ b/models/migrations/v197.go @@ -15,7 +15,6 @@ func createUserSettingsTable(x *xorm.Engine) error { ID int64 `xorm:"pk autoincr"` UserID int64 `xorm:"index unique(key_userid)"` // to load all of someone's settings Key string `xorm:"varchar(255) index unique(key_userid)"` // ensure key is always lowercase - Key string `xorm:"varchar(255) index"` // ensure key is always lowercase Value string `xorm:"text"` } if err := x.Sync2(new(UserSetting)); err != nil { From e7bce0c45fbaa7521d27e0e05e8e70be2ce2c9de Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Thu, 7 Oct 2021 12:19:55 -0400 Subject: [PATCH 16/47] Update models/error.go Co-authored-by: delvh --- models/error.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/error.go b/models/error.go index f0abe6a87dc19..789e97ed643af 100644 --- a/models/error.go +++ b/models/error.go @@ -340,7 +340,7 @@ func (err ErrUserSettingExists) Error() string { return fmt.Sprintf("setting already exists for user [uid: %d, key: %s]", err.Setting.UserID, err.Setting.Key) } -// ErrUserSettingNotExists represents a "setting already exists for user" error. +// ErrUserSettingNotExists represents a "setting does not exist for user" error. type ErrUserSettingNotExists struct { Setting *UserSetting } From 851eb3d44f7cb38f95800af830232e2426e1f0c0 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Thu, 7 Oct 2021 12:20:14 -0400 Subject: [PATCH 17/47] Update models/user_setting.go Co-authored-by: delvh --- models/user_setting.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/user_setting.go b/models/user_setting.go index b6483eac1ba68..429b81071a00e 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -36,7 +36,7 @@ func init() { // GetUserSetting returns specific settings from user func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { - settings := make([]*UserSetting, 0, 5) + settings := make([]*UserSetting, 0, len(keys)) if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). And(builder.In("key", keys)). From 4c56f95f17b442b189283ea60dc9026c39836185 Mon Sep 17 00:00:00 2001 From: Matti R Date: Thu, 7 Oct 2021 12:44:47 -0400 Subject: [PATCH 18/47] update per delvh feedback --- models/user_setting.go | 9 +++++++-- models/user_setting_test.go | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/models/user_setting.go b/models/user_setting.go index 429b81071a00e..f8071376875e4 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -30,6 +30,11 @@ func (setting *UserSetting) BeforeUpdate() { setting.Key = strings.ToLower(setting.Key) } +// BeforeDelete will be invoked by XORM before updating a record +func (setting *UserSetting) BeforeDelete() { + setting.Key = strings.ToLower(setting.Key) +} + func init() { db.RegisterModel(new(UserSetting)) } @@ -47,8 +52,8 @@ func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { return settings, nil } -// GetUserAllSettings returns all settings from user -func GetUserAllSettings(uid int64) ([]*UserSetting, error) { +// GetAllUserSettings returns all settings from user +func GetAllUserSettings(uid int64) ([]*UserSetting, error) { settings := make([]*UserSetting, 0, 5) if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). diff --git a/models/user_setting_test.go b/models/user_setting_test.go index 629b4c1b8ec70..ce6b78ae02565 100644 --- a/models/user_setting_test.go +++ b/models/user_setting_test.go @@ -33,7 +33,7 @@ func TestUserSettings(t *testing.T) { assert.NoError(t, err) // get all settings - userSettings, err = GetUserAllSettings(99) + userSettings, err = GetAllUserSettings(99) assert.NoError(t, err) assert.Len(t, userSettings, 1) assert.EqualValues(t, userSettings[0].Value, updatedSetting.Value) From ee8104ca978ca8bd2a844fe9d17dc6c17786a9db Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 10 Oct 2021 17:05:55 -0400 Subject: [PATCH 19/47] rename to 198 --- models/migrations/{v197.go => v198.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename models/migrations/{v197.go => v198.go} (100%) diff --git a/models/migrations/v197.go b/models/migrations/v198.go similarity index 100% rename from models/migrations/v197.go rename to models/migrations/v198.go From f5fe983f71ce6a71f8a02880fb17f1ee704b900d Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 10 Oct 2021 17:24:12 -0400 Subject: [PATCH 20/47] upsert & add const to test --- models/user_setting.go | 80 ++++++++++++++++++------------------- models/user_setting_test.go | 7 ++-- 2 files changed, 42 insertions(+), 45 deletions(-) diff --git a/models/user_setting.go b/models/user_setting.go index f8071376875e4..b234c8f7c36cd 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -5,9 +5,11 @@ package models import ( + "fmt" "strings" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/setting" "xorm.io/builder" ) @@ -21,18 +23,18 @@ type UserSetting struct { } // BeforeInsert will be invoked by XORM before inserting a record -func (setting *UserSetting) BeforeInsert() { - setting.Key = strings.ToLower(setting.Key) +func (userSetting *UserSetting) BeforeInsert() { + userSetting.Key = strings.ToLower(userSetting.Key) } // BeforeUpdate will be invoked by XORM before updating a record -func (setting *UserSetting) BeforeUpdate() { - setting.Key = strings.ToLower(setting.Key) +func (userSetting *UserSetting) BeforeUpdate() { + userSetting.Key = strings.ToLower(userSetting.Key) } // BeforeDelete will be invoked by XORM before updating a record -func (setting *UserSetting) BeforeDelete() { - setting.Key = strings.ToLower(setting.Key) +func (userSetting *UserSetting) BeforeDelete() { + userSetting.Key = strings.ToLower(userSetting.Key) } func init() { @@ -64,34 +66,15 @@ func GetAllUserSettings(uid int64) ([]*UserSetting, error) { return settings, nil } -func addUserSetting(e db.Engine, setting *UserSetting) error { - used, err := settingExists(e, setting.UserID, setting.Key) - if err != nil { - return err - } else if used { - return ErrUserSettingExists{setting} - } - _, err = e.Insert(setting) - return err -} - -func settingExists(e db.Engine, uid int64, key string) (bool, error) { - if len(key) == 0 { - return true, nil - } - - return e.Table(&UserSetting{}).Exist(&UserSetting{UserID: uid, Key: strings.ToLower(key)}) -} - // DeleteUserSetting deletes a specific setting for a user -func DeleteUserSetting(setting *UserSetting) error { +func DeleteUserSetting(userSetting *UserSetting) error { sess := db.NewSession(db.DefaultContext) defer sess.Close() if err := sess.Begin(); err != nil { return err } - if _, err := sess.Delete(setting); err != nil { + if _, err := sess.Delete(userSetting); err != nil { return err } @@ -99,22 +82,35 @@ func DeleteUserSetting(setting *UserSetting) error { } // SetUserSetting updates a users' setting for a specific key -func SetUserSetting(setting *UserSetting) error { - err := addUserSetting(db.GetEngine(db.DefaultContext), setting) - if err != nil && IsErrUserSettingExists(err) { - return updateUserSettingValue(db.GetEngine(db.DefaultContext), setting) - } - return err +func SetUserSetting(userSetting *UserSetting) error { + return upsertUserSettingValue(db.GetEngine(db.DefaultContext), userSetting.UserID, userSetting.Key, userSetting.Value) } -func updateUserSettingValue(e db.Engine, setting *UserSetting) error { - used, err := settingExists(e, setting.UserID, setting.Key) - if err != nil { - return err - } else if !used { - return ErrUserSettingNotExists{setting} +func upsertUserSettingValue(e db.Engine, userID int64, key string, value string) (err error) { + // Intentionally lowercase key here as XORM may not pick it up via Before* actions + key = strings.ToLower(key) + // An atomic UPSERT operation (INSERT/UPDATE) is the only operation + // that ensures that the key is actually locked. + switch { + case setting.Database.UseSQLite3 || setting.Database.UsePostgreSQL: + _, err = e.Exec("INSERT INTO `user_setting` (user_id, key, value) "+ + "VALUES (?,?,?) ON CONFLICT (user_id,key) DO UPDATE SET value = ?", + userID, key, value, value) + case setting.Database.UseMySQL: + _, err = e.Exec("INSERT INTO `user_setting` (user_id, key, value) "+ + "VALUES (?,?,?) ON DUPLICATE KEY UPDATE value = ?", + userID, key, value, value) + case setting.Database.UseMSSQL: + // https://weblogs.sqlteam.com/dang/2009/01/31/upsert-race-condition-with-merge/ + _, err = e.Exec("MERGE `user_setting` WITH (HOLDLOCK) as target "+ + "USING (SELECT ? AS user_id, ? AS key) AS src "+ + "ON src.user_id = target.user_id AND src.key = target.key "+ + "WHEN MATCHED THEN UPDATE SET target.value = ? "+ + "WHEN NOT MATCHED THEN INSERT (user_id, key, value) "+ + "VALUES (src.user_id, src.key, ?);", + userID, key, value, value) + default: + return fmt.Errorf("database type not supported") } - - _, err = e.ID(setting.ID).Cols("value").Update(setting) - return err + return } diff --git a/models/user_setting_test.go b/models/user_setting_test.go index ce6b78ae02565..7602579ecdfed 100644 --- a/models/user_setting_test.go +++ b/models/user_setting_test.go @@ -13,22 +13,23 @@ import ( ) func TestUserSettings(t *testing.T) { + keyName := "test_user_setting" assert.NoError(t, db.PrepareTestDatabase()) - newSetting := &UserSetting{UserID: 99, Key: "test_user_setting", Value: "Gitea User Setting Test"} + newSetting := &UserSetting{UserID: 99, Key: keyName, Value: "Gitea User Setting Test"} // create setting err := SetUserSetting(newSetting) assert.NoError(t, err) // get specific setting - userSettings, err := GetUserSetting(99, []string{"test_user_setting"}) + userSettings, err := GetUserSetting(99, []string{keyName}) assert.NoError(t, err) assert.Len(t, userSettings, 1) assert.EqualValues(t, newSetting.Value, userSettings[0].Value) // updated setting - updatedSetting := &UserSetting{UserID: 99, Key: "test_user_setting", Value: "Updated", ID: userSettings[0].ID} + updatedSetting := &UserSetting{UserID: 99, Key: keyName, Value: "Updated", ID: userSettings[0].ID} err = SetUserSetting(updatedSetting) assert.NoError(t, err) From 70df32f2f56edb004eec54c2b2866ad4007075f5 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 10 Oct 2021 17:26:30 -0400 Subject: [PATCH 21/47] rm dead code --- models/error.go | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/models/error.go b/models/error.go index 789e97ed643af..1179fa6eb7515 100644 --- a/models/error.go +++ b/models/error.go @@ -325,36 +325,6 @@ func (err ErrReachLimitOfRepo) Error() string { return fmt.Sprintf("user has reached maximum limit of repositories [limit: %d]", err.Limit) } -// ErrUserSettingExists represents a "setting already exists for user" error. -type ErrUserSettingExists struct { - Setting *UserSetting -} - -// IsErrUserSettingExists checks if an error is a ErrUserSettingExists. -func IsErrUserSettingExists(err error) bool { - _, ok := err.(ErrUserSettingExists) - return ok -} - -func (err ErrUserSettingExists) Error() string { - return fmt.Sprintf("setting already exists for user [uid: %d, key: %s]", err.Setting.UserID, err.Setting.Key) -} - -// ErrUserSettingNotExists represents a "setting does not exist for user" error. -type ErrUserSettingNotExists struct { - Setting *UserSetting -} - -// IsErrUserSettingNotExists checks if an error is a ErrUserSettingNotExists. -func IsErrUserSettingNotExists(err error) bool { - _, ok := err.(ErrUserSettingNotExists) - return ok -} - -func (err ErrUserSettingNotExists) Error() string { - return fmt.Sprintf("setting already exists for user [uid: %d, key: %s]", err.Setting.UserID, err.Setting.Key) -} - // __ __.__ __ .__ // / \ / \__| | _|__| // \ \/\/ / | |/ / | From d18b2f9c5ef52843a942fb02c564cd6e92fba94d Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 10 Oct 2021 17:28:46 -0400 Subject: [PATCH 22/47] mk fmt --- models/migrations/migrations.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index f835eac1c29ad..7a1efc1dee2e0 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -348,8 +348,8 @@ var migrations = []Migration{ NewMigration("Add Color to ProjectBoard table", addColorColToProjectBoard), // v197 -> v198 NewMigration("Add renamed_branch table", addRenamedBranchTable), - // v198 -> v199 - NewMigration("Create key/value table for user settings", createUserSettingsTable), + // v198 -> v199 + NewMigration("Create key/value table for user settings", createUserSettingsTable), } // GetCurrentDBVersion returns the current db version From d2e9d1c81301c41763fbd3d67b0f5bd59763e37a Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Fri, 15 Oct 2021 09:41:24 -0400 Subject: [PATCH 23/47] Rename v198.go to v199.go --- models/migrations/{v198.go => v199.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename models/migrations/{v198.go => v199.go} (100%) diff --git a/models/migrations/v198.go b/models/migrations/v199.go similarity index 100% rename from models/migrations/v198.go rename to models/migrations/v199.go From 595abd0b5fb3b82b1095830b6da38e28e95a6856 Mon Sep 17 00:00:00 2001 From: Matti R Date: Mon, 8 Nov 2021 18:34:31 -0500 Subject: [PATCH 24/47] temp move migration to a higher number to make merge conflict easier --- models/migrations/{v199.go => v211.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename models/migrations/{v199.go => v211.go} (100%) diff --git a/models/migrations/v199.go b/models/migrations/v211.go similarity index 100% rename from models/migrations/v199.go rename to models/migrations/v211.go From 261fcbbf5732e60b6e334a407907a3dca830ce29 Mon Sep 17 00:00:00 2001 From: Matti R Date: Mon, 8 Nov 2021 19:25:43 -0500 Subject: [PATCH 25/47] update per feedback --- models/user_setting.go | 52 ++++++++++++++++--------------------- models/user_setting_test.go | 10 +++---- 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/models/user_setting.go b/models/user_setting.go index b234c8f7c36cd..f8bf459cd02b3 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -16,25 +16,25 @@ import ( // UserSetting is a key value store of user settings type UserSetting struct { - ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"index unique(key_userid)"` // to load all of someone's settings - Key string `xorm:"varchar(255) index unique(key_userid)"` // ensure key is always lowercase - Value string `xorm:"text"` + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"index unique(key_userid)"` // to load all of someone's settings + SettingKey string `xorm:"varchar(255) index unique(key_userid)"` // ensure key is always lowercase + SettingValue string `xorm:"text"` } // BeforeInsert will be invoked by XORM before inserting a record func (userSetting *UserSetting) BeforeInsert() { - userSetting.Key = strings.ToLower(userSetting.Key) + userSetting.SettingKey = strings.ToLower(userSetting.SettingKey) } // BeforeUpdate will be invoked by XORM before updating a record func (userSetting *UserSetting) BeforeUpdate() { - userSetting.Key = strings.ToLower(userSetting.Key) + userSetting.SettingKey = strings.ToLower(userSetting.SettingKey) } // BeforeDelete will be invoked by XORM before updating a record func (userSetting *UserSetting) BeforeDelete() { - userSetting.Key = strings.ToLower(userSetting.Key) + userSetting.SettingKey = strings.ToLower(userSetting.SettingKey) } func init() { @@ -46,8 +46,7 @@ func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { settings := make([]*UserSetting, 0, len(keys)) if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). - And(builder.In("key", keys)). - Asc("id"). + And(builder.In("setting_key", keys)). Find(&settings); err != nil { return nil, err } @@ -55,7 +54,7 @@ func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { } // GetAllUserSettings returns all settings from user -func GetAllUserSettings(uid int64) ([]*UserSetting, error) { +func GetUserAllSettings(uid int64) ([]*UserSetting, error) { settings := make([]*UserSetting, 0, 5) if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). @@ -68,22 +67,15 @@ func GetAllUserSettings(uid int64) ([]*UserSetting, error) { // DeleteUserSetting deletes a specific setting for a user func DeleteUserSetting(userSetting *UserSetting) error { - sess := db.NewSession(db.DefaultContext) - defer sess.Close() - if err := sess.Begin(); err != nil { - return err - } - - if _, err := sess.Delete(userSetting); err != nil { - return err - } + sess := db.GetEngine(db.DefaultContext) - return sess.Commit() + _, err := sess.Delete(userSetting) + return err } // SetUserSetting updates a users' setting for a specific key func SetUserSetting(userSetting *UserSetting) error { - return upsertUserSettingValue(db.GetEngine(db.DefaultContext), userSetting.UserID, userSetting.Key, userSetting.Value) + return upsertUserSettingValue(db.GetEngine(db.DefaultContext), userSetting.UserID, userSetting.SettingKey, userSetting.SettingValue) } func upsertUserSettingValue(e db.Engine, userID int64, key string, value string) (err error) { @@ -93,21 +85,21 @@ func upsertUserSettingValue(e db.Engine, userID int64, key string, value string) // that ensures that the key is actually locked. switch { case setting.Database.UseSQLite3 || setting.Database.UsePostgreSQL: - _, err = e.Exec("INSERT INTO `user_setting` (user_id, key, value) "+ - "VALUES (?,?,?) ON CONFLICT (user_id,key) DO UPDATE SET value = ?", + _, err = e.Exec("INSERT INTO `user_setting` (user_id, setting_key, setting_value) "+ + "VALUES (?,?,?) ON CONFLICT (user_id,setting_key) DO UPDATE SET setting_value = ?", userID, key, value, value) case setting.Database.UseMySQL: - _, err = e.Exec("INSERT INTO `user_setting` (user_id, key, value) "+ - "VALUES (?,?,?) ON DUPLICATE KEY UPDATE value = ?", + _, err = e.Exec("INSERT INTO `user_setting` (user_id, setting_key, setting_value) "+ + "VALUES (?,?,?) ON DUPLICATE KEY UPDATE setting_value = ?", userID, key, value, value) case setting.Database.UseMSSQL: // https://weblogs.sqlteam.com/dang/2009/01/31/upsert-race-condition-with-merge/ _, err = e.Exec("MERGE `user_setting` WITH (HOLDLOCK) as target "+ - "USING (SELECT ? AS user_id, ? AS key) AS src "+ - "ON src.user_id = target.user_id AND src.key = target.key "+ - "WHEN MATCHED THEN UPDATE SET target.value = ? "+ - "WHEN NOT MATCHED THEN INSERT (user_id, key, value) "+ - "VALUES (src.user_id, src.key, ?);", + "USING (SELECT ? AS user_id, ? AS setting_key) AS src "+ + "ON src.user_id = target.user_id AND src.setting_key = target.setting_key "+ + "WHEN MATCHED THEN UPDATE SET target.setting_value = ? "+ + "WHEN NOT MATCHED THEN INSERT (user_id, setting_key, setting_value) "+ + "VALUES (src.user_id, src.setting_key, ?);", userID, key, value, value) default: return fmt.Errorf("database type not supported") diff --git a/models/user_setting_test.go b/models/user_setting_test.go index 7602579ecdfed..1dd7bd6a5ffac 100644 --- a/models/user_setting_test.go +++ b/models/user_setting_test.go @@ -16,7 +16,7 @@ func TestUserSettings(t *testing.T) { keyName := "test_user_setting" assert.NoError(t, db.PrepareTestDatabase()) - newSetting := &UserSetting{UserID: 99, Key: keyName, Value: "Gitea User Setting Test"} + newSetting := &UserSetting{UserID: 99, SettingKey: keyName, SettingValue: "Gitea User Setting Test"} // create setting err := SetUserSetting(newSetting) @@ -26,18 +26,18 @@ func TestUserSettings(t *testing.T) { userSettings, err := GetUserSetting(99, []string{keyName}) assert.NoError(t, err) assert.Len(t, userSettings, 1) - assert.EqualValues(t, newSetting.Value, userSettings[0].Value) + assert.EqualValues(t, newSetting.SettingValue, userSettings[0].SettingValue) // updated setting - updatedSetting := &UserSetting{UserID: 99, Key: keyName, Value: "Updated", ID: userSettings[0].ID} + updatedSetting := &UserSetting{UserID: 99, SettingKey: keyName, SettingValue: "Updated", ID: userSettings[0].ID} err = SetUserSetting(updatedSetting) assert.NoError(t, err) // get all settings - userSettings, err = GetAllUserSettings(99) + userSettings, err = GetUserAllSettings(99) assert.NoError(t, err) assert.Len(t, userSettings, 1) - assert.EqualValues(t, userSettings[0].Value, updatedSetting.Value) + assert.EqualValues(t, userSettings[0].SettingValue, updatedSetting.SettingValue) // delete setting err = DeleteUserSetting(updatedSetting) From 1c1d85a9aa6686fc55979d7354b16cc4c7431c72 Mon Sep 17 00:00:00 2001 From: Matti R Date: Mon, 8 Nov 2021 19:38:06 -0500 Subject: [PATCH 26/47] swap to map --- models/user_setting.go | 17 +++++++++++++---- models/user_setting_test.go | 6 +++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/models/user_setting.go b/models/user_setting.go index f8bf459cd02b3..c43210e1236f4 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -42,7 +42,8 @@ func init() { } // GetUserSetting returns specific settings from user -func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { +// func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { +func GetUserSetting(uid int64, keys []string) (map[string]*UserSetting, error) { settings := make([]*UserSetting, 0, len(keys)) if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). @@ -50,11 +51,15 @@ func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { Find(&settings); err != nil { return nil, err } - return settings, nil + settingsMap := make(map[string]*UserSetting) + for _, s := range settings { + settingsMap[s.SettingKey] = s + } + return settingsMap, nil } // GetAllUserSettings returns all settings from user -func GetUserAllSettings(uid int64) ([]*UserSetting, error) { +func GetUserAllSettings(uid int64) (map[string]*UserSetting, error) { settings := make([]*UserSetting, 0, 5) if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). @@ -62,7 +67,11 @@ func GetUserAllSettings(uid int64) ([]*UserSetting, error) { Find(&settings); err != nil { return nil, err } - return settings, nil + settingsMap := make(map[string]*UserSetting) + for _, s := range settings { + settingsMap[s.SettingKey] = s + } + return settingsMap, nil } // DeleteUserSetting deletes a specific setting for a user diff --git a/models/user_setting_test.go b/models/user_setting_test.go index 1dd7bd6a5ffac..803c7d7717a14 100644 --- a/models/user_setting_test.go +++ b/models/user_setting_test.go @@ -26,10 +26,10 @@ func TestUserSettings(t *testing.T) { userSettings, err := GetUserSetting(99, []string{keyName}) assert.NoError(t, err) assert.Len(t, userSettings, 1) - assert.EqualValues(t, newSetting.SettingValue, userSettings[0].SettingValue) + assert.EqualValues(t, newSetting.SettingValue, userSettings[keyName].SettingValue) // updated setting - updatedSetting := &UserSetting{UserID: 99, SettingKey: keyName, SettingValue: "Updated", ID: userSettings[0].ID} + updatedSetting := &UserSetting{UserID: 99, SettingKey: keyName, SettingValue: "Updated", ID: userSettings[keyName].ID} err = SetUserSetting(updatedSetting) assert.NoError(t, err) @@ -37,7 +37,7 @@ func TestUserSettings(t *testing.T) { userSettings, err = GetUserAllSettings(99) assert.NoError(t, err) assert.Len(t, userSettings, 1) - assert.EqualValues(t, userSettings[0].SettingValue, updatedSetting.SettingValue) + assert.EqualValues(t, userSettings[updatedSetting.SettingKey].SettingValue, updatedSetting.SettingValue) // delete setting err = DeleteUserSetting(updatedSetting) From 5d98572f92455fc8383e2b4ce4b68ec48962ddac Mon Sep 17 00:00:00 2001 From: Matti R Date: Tue, 9 Nov 2021 20:07:34 -0500 Subject: [PATCH 27/47] return error when key not lower case fix lint issue --- models/user_setting.go | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/models/user_setting.go b/models/user_setting.go index c43210e1236f4..ee550a3c25e97 100644 --- a/models/user_setting.go +++ b/models/user_setting.go @@ -22,21 +22,6 @@ type UserSetting struct { SettingValue string `xorm:"text"` } -// BeforeInsert will be invoked by XORM before inserting a record -func (userSetting *UserSetting) BeforeInsert() { - userSetting.SettingKey = strings.ToLower(userSetting.SettingKey) -} - -// BeforeUpdate will be invoked by XORM before updating a record -func (userSetting *UserSetting) BeforeUpdate() { - userSetting.SettingKey = strings.ToLower(userSetting.SettingKey) -} - -// BeforeDelete will be invoked by XORM before updating a record -func (userSetting *UserSetting) BeforeDelete() { - userSetting.SettingKey = strings.ToLower(userSetting.SettingKey) -} - func init() { db.RegisterModel(new(UserSetting)) } @@ -58,7 +43,7 @@ func GetUserSetting(uid int64, keys []string) (map[string]*UserSetting, error) { return settingsMap, nil } -// GetAllUserSettings returns all settings from user +// GetUserAllSettings returns all settings from user func GetUserAllSettings(uid int64) (map[string]*UserSetting, error) { settings := make([]*UserSetting, 0, 5) if err := db.GetEngine(db.DefaultContext). @@ -84,6 +69,9 @@ func DeleteUserSetting(userSetting *UserSetting) error { // SetUserSetting updates a users' setting for a specific key func SetUserSetting(userSetting *UserSetting) error { + if strings.ToLower(userSetting.SettingKey) != userSetting.SettingKey { + return fmt.Errorf("setting key should be lowercase") + } return upsertUserSettingValue(db.GetEngine(db.DefaultContext), userSetting.UserID, userSetting.SettingKey, userSetting.SettingValue) } From cbb145cb267f4cc490c8c6e51ec7eb40b80d3f0f Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Tue, 9 Nov 2021 21:46:46 -0500 Subject: [PATCH 28/47] match migration struct to user setting struct --- models/migrations/v202.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/models/migrations/v202.go b/models/migrations/v202.go index 92dddb3cd1405..3e023543e4543 100644 --- a/models/migrations/v202.go +++ b/models/migrations/v202.go @@ -12,10 +12,10 @@ import ( func createUserSettingsTable(x *xorm.Engine) error { type UserSetting struct { - ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"index unique(key_userid)"` // to load all of someone's settings - Key string `xorm:"varchar(255) index unique(key_userid)"` // ensure key is always lowercase - Value string `xorm:"text"` + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"index unique(key_userid)"` // to load all of someone's settings + SettingKey string `xorm:"varchar(255) index unique(key_userid)"` // ensure key is always lowercase + SettingValue string `xorm:"text"` } if err := x.Sync2(new(UserSetting)); err != nil { return fmt.Errorf("sync2: %v", err) From d8aeec7f5898619dc1f1714cf89fa783756c460e Mon Sep 17 00:00:00 2001 From: Matti R Date: Wed, 10 Nov 2021 16:43:25 -0500 Subject: [PATCH 29/47] mv files --- models/{user_setting.go => user/setting.go} | 0 models/{user_setting_test.go => user/setting_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename models/{user_setting.go => user/setting.go} (100%) rename models/{user_setting_test.go => user/setting_test.go} (100%) diff --git a/models/user_setting.go b/models/user/setting.go similarity index 100% rename from models/user_setting.go rename to models/user/setting.go diff --git a/models/user_setting_test.go b/models/user/setting_test.go similarity index 100% rename from models/user_setting_test.go rename to models/user/setting_test.go From d8e1c18581a285ba34b175056fd2e7f507774cb0 Mon Sep 17 00:00:00 2001 From: Matti R Date: Wed, 10 Nov 2021 16:50:54 -0500 Subject: [PATCH 30/47] fix lint --- models/user.go | 3 ++- models/user/setting.go | 2 +- models/user/setting_test.go | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/models/user.go b/models/user.go index 4d1cff90f4d20..c26c2acc79862 100644 --- a/models/user.go +++ b/models/user.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/login" "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" @@ -1217,7 +1218,7 @@ func deleteUser(e db.Engine, u *User) error { &TeamUser{UID: u.ID}, &Collaboration{UserID: u.ID}, &Stopwatch{UserID: u.ID}, - &UserSetting{UserID: u.ID}, + &user.UserSetting{UserID: u.ID}, ); err != nil { return fmt.Errorf("deleteBeans: %v", err) } diff --git a/models/user/setting.go b/models/user/setting.go index ee550a3c25e97..cf5da519dcfac 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package models +package user import ( "fmt" diff --git a/models/user/setting_test.go b/models/user/setting_test.go index 803c7d7717a14..54aac8179f803 100644 --- a/models/user/setting_test.go +++ b/models/user/setting_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package models +package user import ( "testing" From 4489d3965b2e9a21eb3e950a789b3aa3c7766482 Mon Sep 17 00:00:00 2001 From: Matti R Date: Wed, 10 Nov 2021 17:01:42 -0500 Subject: [PATCH 31/47] change struct name --- models/user.go | 2 +- models/user/setting.go | 42 ++++++++++++++++++++----------------- models/user/setting_test.go | 24 ++++++++++----------- 3 files changed, 36 insertions(+), 32 deletions(-) diff --git a/models/user.go b/models/user.go index c26c2acc79862..758db618902c2 100644 --- a/models/user.go +++ b/models/user.go @@ -1218,7 +1218,7 @@ func deleteUser(e db.Engine, u *User) error { &TeamUser{UID: u.ID}, &Collaboration{UserID: u.ID}, &Stopwatch{UserID: u.ID}, - &user.UserSetting{UserID: u.ID}, + &user.Setting{UserID: u.ID}, ); err != nil { return fmt.Errorf("deleteBeans: %v", err) } diff --git a/models/user/setting.go b/models/user/setting.go index cf5da519dcfac..8834cfb680291 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -14,29 +14,33 @@ import ( "xorm.io/builder" ) -// UserSetting is a key value store of user settings -type UserSetting struct { +// Setting is a key value store of user settings +type Setting struct { ID int64 `xorm:"pk autoincr"` UserID int64 `xorm:"index unique(key_userid)"` // to load all of someone's settings SettingKey string `xorm:"varchar(255) index unique(key_userid)"` // ensure key is always lowercase SettingValue string `xorm:"text"` } +func (Setting) TableName() string { + return "user_setting" +} + func init() { - db.RegisterModel(new(UserSetting)) + db.RegisterModel(new(Setting)) } -// GetUserSetting returns specific settings from user -// func GetUserSetting(uid int64, keys []string) ([]*UserSetting, error) { -func GetUserSetting(uid int64, keys []string) (map[string]*UserSetting, error) { - settings := make([]*UserSetting, 0, len(keys)) +// GetSetting returns specific settings from user +// func GetSetting(uid int64, keys []string) ([]*Setting, error) { +func GetSetting(uid int64, keys []string) (map[string]*Setting, error) { + settings := make([]*Setting, 0, len(keys)) if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). And(builder.In("setting_key", keys)). Find(&settings); err != nil { return nil, err } - settingsMap := make(map[string]*UserSetting) + settingsMap := make(map[string]*Setting) for _, s := range settings { settingsMap[s.SettingKey] = s } @@ -44,38 +48,38 @@ func GetUserSetting(uid int64, keys []string) (map[string]*UserSetting, error) { } // GetUserAllSettings returns all settings from user -func GetUserAllSettings(uid int64) (map[string]*UserSetting, error) { - settings := make([]*UserSetting, 0, 5) +func GetUserAllSettings(uid int64) (map[string]*Setting, error) { + settings := make([]*Setting, 0, 5) if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). Asc("id"). Find(&settings); err != nil { return nil, err } - settingsMap := make(map[string]*UserSetting) + settingsMap := make(map[string]*Setting) for _, s := range settings { settingsMap[s.SettingKey] = s } return settingsMap, nil } -// DeleteUserSetting deletes a specific setting for a user -func DeleteUserSetting(userSetting *UserSetting) error { +// DeleteSetting deletes a specific setting for a user +func DeleteSetting(Setting *Setting) error { sess := db.GetEngine(db.DefaultContext) - _, err := sess.Delete(userSetting) + _, err := sess.Delete(Setting) return err } -// SetUserSetting updates a users' setting for a specific key -func SetUserSetting(userSetting *UserSetting) error { - if strings.ToLower(userSetting.SettingKey) != userSetting.SettingKey { +// SetSetting updates a users' setting for a specific key +func SetSetting(Setting *Setting) error { + if strings.ToLower(Setting.SettingKey) != Setting.SettingKey { return fmt.Errorf("setting key should be lowercase") } - return upsertUserSettingValue(db.GetEngine(db.DefaultContext), userSetting.UserID, userSetting.SettingKey, userSetting.SettingValue) + return upsertSettingValue(db.GetEngine(db.DefaultContext), Setting.UserID, Setting.SettingKey, Setting.SettingValue) } -func upsertUserSettingValue(e db.Engine, userID int64, key string, value string) (err error) { +func upsertSettingValue(e db.Engine, userID int64, key string, value string) (err error) { // Intentionally lowercase key here as XORM may not pick it up via Before* actions key = strings.ToLower(key) // An atomic UPSERT operation (INSERT/UPDATE) is the only operation diff --git a/models/user/setting_test.go b/models/user/setting_test.go index 54aac8179f803..c53d4fab40366 100644 --- a/models/user/setting_test.go +++ b/models/user/setting_test.go @@ -12,34 +12,34 @@ import ( "github.com/stretchr/testify/assert" ) -func TestUserSettings(t *testing.T) { +func TestSettings(t *testing.T) { keyName := "test_user_setting" assert.NoError(t, db.PrepareTestDatabase()) - newSetting := &UserSetting{UserID: 99, SettingKey: keyName, SettingValue: "Gitea User Setting Test"} + newSetting := &Setting{UserID: 99, SettingKey: keyName, SettingValue: "Gitea User Setting Test"} // create setting - err := SetUserSetting(newSetting) + err := SetSetting(newSetting) assert.NoError(t, err) // get specific setting - userSettings, err := GetUserSetting(99, []string{keyName}) + Settings, err := GetSetting(99, []string{keyName}) assert.NoError(t, err) - assert.Len(t, userSettings, 1) - assert.EqualValues(t, newSetting.SettingValue, userSettings[keyName].SettingValue) + assert.Len(t, Settings, 1) + assert.EqualValues(t, newSetting.SettingValue, Settings[keyName].SettingValue) // updated setting - updatedSetting := &UserSetting{UserID: 99, SettingKey: keyName, SettingValue: "Updated", ID: userSettings[keyName].ID} - err = SetUserSetting(updatedSetting) + updatedSetting := &Setting{UserID: 99, SettingKey: keyName, SettingValue: "Updated", ID: Settings[keyName].ID} + err = SetSetting(updatedSetting) assert.NoError(t, err) // get all settings - userSettings, err = GetUserAllSettings(99) + Settings, err = GetUserAllSettings(99) assert.NoError(t, err) - assert.Len(t, userSettings, 1) - assert.EqualValues(t, userSettings[updatedSetting.SettingKey].SettingValue, updatedSetting.SettingValue) + assert.Len(t, Settings, 1) + assert.EqualValues(t, Settings[updatedSetting.SettingKey].SettingValue, updatedSetting.SettingValue) // delete setting - err = DeleteUserSetting(updatedSetting) + err = DeleteSetting(updatedSetting) assert.NoError(t, err) } From cb50d4842eff07661d3e10370162551191f44286 Mon Sep 17 00:00:00 2001 From: Matti R Date: Wed, 10 Nov 2021 17:23:49 -0500 Subject: [PATCH 32/47] fix caps --- models/user/setting.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/models/user/setting.go b/models/user/setting.go index 8834cfb680291..69565680cab04 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -64,19 +64,19 @@ func GetUserAllSettings(uid int64) (map[string]*Setting, error) { } // DeleteSetting deletes a specific setting for a user -func DeleteSetting(Setting *Setting) error { +func DeleteSetting(setting *Setting) error { sess := db.GetEngine(db.DefaultContext) - _, err := sess.Delete(Setting) + _, err := sess.Delete(setting) return err } // SetSetting updates a users' setting for a specific key -func SetSetting(Setting *Setting) error { - if strings.ToLower(Setting.SettingKey) != Setting.SettingKey { +func SetSetting(setting *Setting) error { + if strings.ToLower(setting.SettingKey) != setting.SettingKey { return fmt.Errorf("setting key should be lowercase") } - return upsertSettingValue(db.GetEngine(db.DefaultContext), Setting.UserID, Setting.SettingKey, Setting.SettingValue) + return upsertSettingValue(db.GetEngine(db.DefaultContext), setting.UserID, setting.SettingKey, setting.SettingValue) } func upsertSettingValue(e db.Engine, userID int64, key string, value string) (err error) { From 5f719fc7a3f289936c2111ac7929e573a5193b86 Mon Sep 17 00:00:00 2001 From: Matti R Date: Wed, 10 Nov 2021 17:28:35 -0500 Subject: [PATCH 33/47] add comment to placate lint --- models/user/setting.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/user/setting.go b/models/user/setting.go index 69565680cab04..84df0a23f6211 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -22,7 +22,8 @@ type Setting struct { SettingValue string `xorm:"text"` } -func (Setting) TableName() string { +// TableName sets the table name for the settings struct +func (s *Setting) TableName() string { return "user_setting" } From ba399006c50cbaeaf703d99f3babce33c788c4e9 Mon Sep 17 00:00:00 2001 From: Matti R Date: Thu, 11 Nov 2021 17:49:19 -0500 Subject: [PATCH 34/47] pass tests --- models/user.go | 2 +- models/user/main_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/user.go b/models/user.go index cba251cc6fa12..a6166180908c0 100644 --- a/models/user.go +++ b/models/user.go @@ -1221,7 +1221,7 @@ func deleteUser(e db.Engine, u *User) error { &TeamUser{UID: u.ID}, &Collaboration{UserID: u.ID}, &Stopwatch{UserID: u.ID}, - &user.Setting{UserID: u.ID}, + &user_model.Setting{UserID: u.ID}, ); err != nil { return fmt.Errorf("deleteBeans: %v", err) } diff --git a/models/user/main_test.go b/models/user/main_test.go index 2999c4c81db53..dbac3cee08bfa 100644 --- a/models/user/main_test.go +++ b/models/user/main_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. +// Copyright 2021 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. From 09841e6bd61756d7998d87cb649f826d3b5211c4 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Fri, 19 Nov 2021 03:43:26 -0500 Subject: [PATCH 35/47] Update setting_test.go --- models/user/setting_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/user/setting_test.go b/models/user/setting_test.go index c53d4fab40366..6c91b7c19f339 100644 --- a/models/user/setting_test.go +++ b/models/user/setting_test.go @@ -7,14 +7,14 @@ package user import ( "testing" - "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" "github.com/stretchr/testify/assert" ) func TestSettings(t *testing.T) { keyName := "test_user_setting" - assert.NoError(t, db.PrepareTestDatabase()) + assert.NoError(t, unittest.PrepareTestDatabase()) newSetting := &Setting{UserID: 99, SettingKey: keyName, SettingValue: "Gitea User Setting Test"} From 43b32d0e03d728fca121cbb24ebadea7b4f06b15 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 21 Nov 2021 01:40:34 -0500 Subject: [PATCH 36/47] swap upsert logic --- cmd/web_https.go | 1 + models/user/setting.go | 45 ++++++++-------------- modules/avatar/identicon/identicon_test.go | 2 +- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/cmd/web_https.go b/cmd/web_https.go index b939dcab8a119..b0910ca04000f 100644 --- a/cmd/web_https.go +++ b/cmd/web_https.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "github.com/klauspost/cpuid/v2" ) diff --git a/models/user/setting.go b/models/user/setting.go index 84df0a23f6211..defd0878ed7aa 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -5,11 +5,11 @@ package user import ( + "context" "fmt" "strings" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/modules/setting" "xorm.io/builder" ) @@ -80,31 +80,20 @@ func SetSetting(setting *Setting) error { return upsertSettingValue(db.GetEngine(db.DefaultContext), setting.UserID, setting.SettingKey, setting.SettingValue) } -func upsertSettingValue(e db.Engine, userID int64, key string, value string) (err error) { - // Intentionally lowercase key here as XORM may not pick it up via Before* actions - key = strings.ToLower(key) - // An atomic UPSERT operation (INSERT/UPDATE) is the only operation - // that ensures that the key is actually locked. - switch { - case setting.Database.UseSQLite3 || setting.Database.UsePostgreSQL: - _, err = e.Exec("INSERT INTO `user_setting` (user_id, setting_key, setting_value) "+ - "VALUES (?,?,?) ON CONFLICT (user_id,setting_key) DO UPDATE SET setting_value = ?", - userID, key, value, value) - case setting.Database.UseMySQL: - _, err = e.Exec("INSERT INTO `user_setting` (user_id, setting_key, setting_value) "+ - "VALUES (?,?,?) ON DUPLICATE KEY UPDATE setting_value = ?", - userID, key, value, value) - case setting.Database.UseMSSQL: - // https://weblogs.sqlteam.com/dang/2009/01/31/upsert-race-condition-with-merge/ - _, err = e.Exec("MERGE `user_setting` WITH (HOLDLOCK) as target "+ - "USING (SELECT ? AS user_id, ? AS setting_key) AS src "+ - "ON src.user_id = target.user_id AND src.setting_key = target.setting_key "+ - "WHEN MATCHED THEN UPDATE SET target.setting_value = ? "+ - "WHEN NOT MATCHED THEN INSERT (user_id, setting_key, setting_value) "+ - "VALUES (src.user_id, src.setting_key, ?);", - userID, key, value, value) - default: - return fmt.Errorf("database type not supported") - } - return +func upsertSettingValue(e db.Engine, userID int64, key string, value string) error { + return db.WithTx(func(ctx context.Context) error { + sess := db.GetEngine(db.DefaultContext) + res, err := sess.Exec("UPDATE user_setting SET setting_value=? WHERE setting_key=?", value, key) + if err != nil { + return err + } + rows, _ := res.RowsAffected() + if rows != 0 { + // the existing row is updated, so we can return + return nil + } + // if no existing row, insert a new row + _, err = sess.Insert(&Setting{SettingKey: key, SettingValue: value}) + return err + }) } diff --git a/modules/avatar/identicon/identicon_test.go b/modules/avatar/identicon/identicon_test.go index ab54183a46ba8..ee44c95139a71 100644 --- a/modules/avatar/identicon/identicon_test.go +++ b/modules/avatar/identicon/identicon_test.go @@ -3,7 +3,7 @@ // license that can be found in the LICENSE file. //go:build test_avatar_identicon -// +build test_avatar_identicon +// +build test_avatar_identicon package identicon From 00950c6767c74fc22476651e2fd6619bc5b9f603 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 21 Nov 2021 02:52:36 -0500 Subject: [PATCH 37/47] woops, only update key for specific user --- models/user/setting.go | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/models/user/setting.go b/models/user/setting.go index defd0878ed7aa..36e7cf1757e12 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -5,7 +5,6 @@ package user import ( - "context" "fmt" "strings" @@ -80,20 +79,22 @@ func SetSetting(setting *Setting) error { return upsertSettingValue(db.GetEngine(db.DefaultContext), setting.UserID, setting.SettingKey, setting.SettingValue) } -func upsertSettingValue(e db.Engine, userID int64, key string, value string) error { - return db.WithTx(func(ctx context.Context) error { - sess := db.GetEngine(db.DefaultContext) - res, err := sess.Exec("UPDATE user_setting SET setting_value=? WHERE setting_key=?", value, key) - if err != nil { - return err - } - rows, _ := res.RowsAffected() - if rows != 0 { - // the existing row is updated, so we can return - return nil - } - // if no existing row, insert a new row - _, err = sess.Insert(&Setting{SettingKey: key, SettingValue: value}) +func upsertSettingValue(e db.Engine, userID int64, key string, value string) (err error) { + sess := db.NewSession(db.DefaultContext) + defer sess.Close() + if err = sess.Begin(); err != nil { return err - }) + } + res, err := sess.Exec("UPDATE user_setting SET setting_value=? WHERE setting_key=? AND user_id=?", value, key, userID) + if err != nil { + return err + } + rows, _ := res.RowsAffected() + if rows != 0 { + // the existing row is updated, so we can return + return nil + } + // if no existing row, insert a new row + _, err = sess.Insert(&Setting{SettingKey: key, SettingValue: value}) + return sess.Commit() } From cef5e77b151546f32f92909e1733c0857fa43300 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sun, 21 Nov 2021 14:07:17 -0500 Subject: [PATCH 38/47] update per lunny feedback --- models/user/setting.go | 7 +++---- models/user/setting_test.go | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/models/user/setting.go b/models/user/setting.go index 36e7cf1757e12..56ff8a0dd2c54 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -30,9 +30,8 @@ func init() { db.RegisterModel(new(Setting)) } -// GetSetting returns specific settings from user -// func GetSetting(uid int64, keys []string) ([]*Setting, error) { -func GetSetting(uid int64, keys []string) (map[string]*Setting, error) { +// GetSettings returns specific settings from user +func GetSettings(uid int64, keys []string) (map[string]*Setting, error) { settings := make([]*Setting, 0, len(keys)) if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). @@ -90,7 +89,7 @@ func upsertSettingValue(e db.Engine, userID int64, key string, value string) (er return err } rows, _ := res.RowsAffected() - if rows != 0 { + if rows > 0 { // the existing row is updated, so we can return return nil } diff --git a/models/user/setting_test.go b/models/user/setting_test.go index 6c91b7c19f339..9db95c266686c 100644 --- a/models/user/setting_test.go +++ b/models/user/setting_test.go @@ -23,7 +23,7 @@ func TestSettings(t *testing.T) { assert.NoError(t, err) // get specific setting - Settings, err := GetSetting(99, []string{keyName}) + Settings, err := GetSettings(99, []string{keyName}) assert.NoError(t, err) assert.Len(t, Settings, 1) assert.EqualValues(t, newSetting.SettingValue, Settings[keyName].SettingValue) From 63f71fe5f5ae80a34de8771bec5f40f62ba6bd1c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 22 Nov 2021 03:43:34 +0800 Subject: [PATCH 39/47] Update setting.go --- models/user/setting.go | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/models/user/setting.go b/models/user/setting.go index 56ff8a0dd2c54..e5f1c019b24eb 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -64,9 +64,7 @@ func GetUserAllSettings(uid int64) (map[string]*Setting, error) { // DeleteSetting deletes a specific setting for a user func DeleteSetting(setting *Setting) error { - sess := db.GetEngine(db.DefaultContext) - - _, err := sess.Delete(setting) + _, err := db.GetEngine(db.DefaultContext).Delete(setting) return err } @@ -75,16 +73,17 @@ func SetSetting(setting *Setting) error { if strings.ToLower(setting.SettingKey) != setting.SettingKey { return fmt.Errorf("setting key should be lowercase") } - return upsertSettingValue(db.GetEngine(db.DefaultContext), setting.UserID, setting.SettingKey, setting.SettingValue) + return upsertSettingValue(setting.UserID, setting.SettingKey, setting.SettingValue) } -func upsertSettingValue(e db.Engine, userID int64, key string, value string) (err error) { - sess := db.NewSession(db.DefaultContext) - defer sess.Close() - if err = sess.Begin(); err != nil { +func upsertSettingValue(userID int64, key string, value string) (err error) { + ctx, committer, err := db.TxContext() + if err != nil { return err } - res, err := sess.Exec("UPDATE user_setting SET setting_value=? WHERE setting_key=? AND user_id=?", value, key, userID) + defer committer.Close() + e := db.GetEngine(ctx) + res, err := e.Exec("UPDATE user_setting SET setting_value=? WHERE setting_key=? AND user_id=?", value, key, userID) if err != nil { return err } @@ -93,7 +92,17 @@ func upsertSettingValue(e db.Engine, userID int64, key string, value string) (er // the existing row is updated, so we can return return nil } + + // in case the value isn't changed, update would return 0 rows changed, so we need this check + has, err := e.Exist(&Setting{UserID: userID, SettingKey: key}) + if err != nil { + return err + } + if has { + return nil + } + // if no existing row, insert a new row - _, err = sess.Insert(&Setting{SettingKey: key, SettingValue: value}) - return sess.Commit() + _, err = e.Insert(&Setting{UserID: userID, SettingKey: key, SettingValue: value}) + return committer.Commit() } From 83fa71e8b6f4999d10b61a5fbacde5eb7afc38f3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 22 Nov 2021 03:44:44 +0800 Subject: [PATCH 40/47] Update setting_test.go --- models/user/setting_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/models/user/setting_test.go b/models/user/setting_test.go index 9db95c266686c..b0b531b668572 100644 --- a/models/user/setting_test.go +++ b/models/user/setting_test.go @@ -21,6 +21,8 @@ func TestSettings(t *testing.T) { // create setting err := SetSetting(newSetting) assert.NoError(t, err) + err = SetSetting(newSetting) // test about saving unchanged values + assert.NoError(t, err) // get specific setting Settings, err := GetSettings(99, []string{keyName}) From c81f0b88790a2bfa04e7e14a1b414150ec864402 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 22 Nov 2021 03:52:03 +0800 Subject: [PATCH 41/47] Update setting.go comments --- models/user/setting.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/models/user/setting.go b/models/user/setting.go index e5f1c019b24eb..a26645a28667e 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -83,6 +83,17 @@ func upsertSettingValue(userID int64, key string, value string) (err error) { } defer committer.Close() e := db.GetEngine(ctx) + + // here we use a general method to do a safe upsert for different databases (and most transaction levels) + // 1. try to UPDATE the record and acquire the transaction write lock + // if UPDATE returns non-zero rows are changed, OK, the setting is saved correctly + // if UPDATE returns "0 rows changed", two possibilities: (a) record doesn't exist (b) value is not changed + // 2. do a SELECT to check if the row exists or not (we already have the transaction lock) + // 3. if the row doesn't exist, do an INSERT (we are still protected by the transaction lock, so it's safe) + // + // to optimize the SELECT in step 2, we can use an extra column like `revision=revision+1` + // to make sure the UPDATE always returns a non-zero value for existing (unchanged) records. + res, err := e.Exec("UPDATE user_setting SET setting_value=? WHERE setting_key=? AND user_id=?", value, key, userID) if err != nil { return err From 84aac13fb004e126334bea7db0e2c7577d960e71 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 22 Nov 2021 03:55:16 +0800 Subject: [PATCH 42/47] Update setting.go --- models/user/setting.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/models/user/setting.go b/models/user/setting.go index a26645a28667e..306aa9e3c96b8 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -115,5 +115,9 @@ func upsertSettingValue(userID int64, key string, value string) (err error) { // if no existing row, insert a new row _, err = e.Insert(&Setting{UserID: userID, SettingKey: key, SettingValue: value}) + if err != nil { + return err + } + return committer.Commit() } From ee57a241f306a54d51287601f8e05f7e9805da00 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sun, 21 Nov 2021 15:10:16 -0500 Subject: [PATCH 43/47] proper casing --- models/user/setting_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/user/setting_test.go b/models/user/setting_test.go index b0b531b668572..6af12c59fbad4 100644 --- a/models/user/setting_test.go +++ b/models/user/setting_test.go @@ -25,7 +25,7 @@ func TestSettings(t *testing.T) { assert.NoError(t, err) // get specific setting - Settings, err := GetSettings(99, []string{keyName}) + settings, err := GetSettings(99, []string{keyName}) assert.NoError(t, err) assert.Len(t, Settings, 1) assert.EqualValues(t, newSetting.SettingValue, Settings[keyName].SettingValue) @@ -36,10 +36,10 @@ func TestSettings(t *testing.T) { assert.NoError(t, err) // get all settings - Settings, err = GetUserAllSettings(99) + settings, err = GetUserAllSettings(99) assert.NoError(t, err) assert.Len(t, Settings, 1) - assert.EqualValues(t, Settings[updatedSetting.SettingKey].SettingValue, updatedSetting.SettingValue) + assert.EqualValues(t, settings[updatedSetting.SettingKey].SettingValue, updatedSetting.SettingValue) // delete setting err = DeleteSetting(updatedSetting) From 25fbcae8b4b61f27de4003c54fa914ca5ec9b438 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sun, 21 Nov 2021 15:13:43 -0500 Subject: [PATCH 44/47] more casing changes --- models/user/setting_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/models/user/setting_test.go b/models/user/setting_test.go index 6af12c59fbad4..d7c1af33e1eca 100644 --- a/models/user/setting_test.go +++ b/models/user/setting_test.go @@ -27,18 +27,18 @@ func TestSettings(t *testing.T) { // get specific setting settings, err := GetSettings(99, []string{keyName}) assert.NoError(t, err) - assert.Len(t, Settings, 1) - assert.EqualValues(t, newSetting.SettingValue, Settings[keyName].SettingValue) + assert.Len(t, settings, 1) + assert.EqualValues(t, newSetting.SettingValue, settings[keyName].SettingValue) // updated setting - updatedSetting := &Setting{UserID: 99, SettingKey: keyName, SettingValue: "Updated", ID: Settings[keyName].ID} + updatedSetting := &Setting{UserID: 99, SettingKey: keyName, SettingValue: "Updated", ID: settings[keyName].ID} err = SetSetting(updatedSetting) assert.NoError(t, err) // get all settings settings, err = GetUserAllSettings(99) assert.NoError(t, err) - assert.Len(t, Settings, 1) + assert.Len(t, settings, 1) assert.EqualValues(t, settings[updatedSetting.SettingKey].SettingValue, updatedSetting.SettingValue) // delete setting From 73a869eb3fcaca618ffc231ef2f87ffc1114c12c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 22 Nov 2021 09:31:21 +0800 Subject: [PATCH 45/47] fix transaction session and unit test --- models/user/setting.go | 15 +++++++++------ models/user/setting_test.go | 12 ++++++++---- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/models/user/setting.go b/models/user/setting.go index 306aa9e3c96b8..55e7169ee5bdd 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -81,7 +81,14 @@ func upsertSettingValue(userID int64, key string, value string) (err error) { if err != nil { return err } - defer committer.Close() + defer func() { + if err != nil { + _ = committer.Close() + } else { + err = committer.Commit() + } + }() + e := db.GetEngine(ctx) // here we use a general method to do a safe upsert for different databases (and most transaction levels) @@ -115,9 +122,5 @@ func upsertSettingValue(userID int64, key string, value string) (err error) { // if no existing row, insert a new row _, err = e.Insert(&Setting{UserID: userID, SettingKey: key, SettingValue: value}) - if err != nil { - return err - } - - return committer.Commit() + return err } diff --git a/models/user/setting_test.go b/models/user/setting_test.go index d7c1af33e1eca..81445a9f6f782 100644 --- a/models/user/setting_test.go +++ b/models/user/setting_test.go @@ -21,7 +21,8 @@ func TestSettings(t *testing.T) { // create setting err := SetSetting(newSetting) assert.NoError(t, err) - err = SetSetting(newSetting) // test about saving unchanged values + // test about saving unchanged values + err = SetSetting(newSetting) assert.NoError(t, err) // get specific setting @@ -31,7 +32,7 @@ func TestSettings(t *testing.T) { assert.EqualValues(t, newSetting.SettingValue, settings[keyName].SettingValue) // updated setting - updatedSetting := &Setting{UserID: 99, SettingKey: keyName, SettingValue: "Updated", ID: settings[keyName].ID} + updatedSetting := &Setting{UserID: 99, SettingKey: keyName, SettingValue: "Updated"} err = SetSetting(updatedSetting) assert.NoError(t, err) @@ -39,9 +40,12 @@ func TestSettings(t *testing.T) { settings, err = GetUserAllSettings(99) assert.NoError(t, err) assert.Len(t, settings, 1) - assert.EqualValues(t, settings[updatedSetting.SettingKey].SettingValue, updatedSetting.SettingValue) + assert.EqualValues(t, updatedSetting.SettingValue, settings[updatedSetting.SettingKey].SettingValue) // delete setting - err = DeleteSetting(updatedSetting) + err = DeleteSetting(&Setting{UserID: 99, SettingKey: keyName}) assert.NoError(t, err) + settings, err = GetUserAllSettings(99) + assert.NoError(t, err) + assert.Len(t, settings, 0) } From 5d289492a073a126edcac2a136d1552a3160c689 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 22 Nov 2021 09:40:15 +0800 Subject: [PATCH 46/47] remove unnecessary SQL sorting --- models/user/setting.go | 1 - 1 file changed, 1 deletion(-) diff --git a/models/user/setting.go b/models/user/setting.go index 55e7169ee5bdd..22f7cd643a9ec 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -51,7 +51,6 @@ func GetUserAllSettings(uid int64) (map[string]*Setting, error) { settings := make([]*Setting, 0, 5) if err := db.GetEngine(db.DefaultContext). Where("user_id=?", uid). - Asc("id"). Find(&settings); err != nil { return nil, err } From 162bcbe68252dba92b95685dc0d3630da80f27ae Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 22 Nov 2021 13:27:43 +0800 Subject: [PATCH 47/47] use WithTx instead of TxContext --- models/user/setting.go | 77 +++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 43 deletions(-) diff --git a/models/user/setting.go b/models/user/setting.go index 22f7cd643a9ec..c5a3d482b5ffa 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -5,6 +5,7 @@ package user import ( + "context" "fmt" "strings" @@ -75,51 +76,41 @@ func SetSetting(setting *Setting) error { return upsertSettingValue(setting.UserID, setting.SettingKey, setting.SettingValue) } -func upsertSettingValue(userID int64, key string, value string) (err error) { - ctx, committer, err := db.TxContext() - if err != nil { - return err - } - defer func() { +func upsertSettingValue(userID int64, key string, value string) error { + return db.WithTx(func(ctx context.Context) error { + e := db.GetEngine(ctx) + + // here we use a general method to do a safe upsert for different databases (and most transaction levels) + // 1. try to UPDATE the record and acquire the transaction write lock + // if UPDATE returns non-zero rows are changed, OK, the setting is saved correctly + // if UPDATE returns "0 rows changed", two possibilities: (a) record doesn't exist (b) value is not changed + // 2. do a SELECT to check if the row exists or not (we already have the transaction lock) + // 3. if the row doesn't exist, do an INSERT (we are still protected by the transaction lock, so it's safe) + // + // to optimize the SELECT in step 2, we can use an extra column like `revision=revision+1` + // to make sure the UPDATE always returns a non-zero value for existing (unchanged) records. + + res, err := e.Exec("UPDATE user_setting SET setting_value=? WHERE setting_key=? AND user_id=?", value, key, userID) if err != nil { - _ = committer.Close() - } else { - err = committer.Commit() + return err + } + rows, _ := res.RowsAffected() + if rows > 0 { + // the existing row is updated, so we can return + return nil } - }() - - e := db.GetEngine(ctx) - - // here we use a general method to do a safe upsert for different databases (and most transaction levels) - // 1. try to UPDATE the record and acquire the transaction write lock - // if UPDATE returns non-zero rows are changed, OK, the setting is saved correctly - // if UPDATE returns "0 rows changed", two possibilities: (a) record doesn't exist (b) value is not changed - // 2. do a SELECT to check if the row exists or not (we already have the transaction lock) - // 3. if the row doesn't exist, do an INSERT (we are still protected by the transaction lock, so it's safe) - // - // to optimize the SELECT in step 2, we can use an extra column like `revision=revision+1` - // to make sure the UPDATE always returns a non-zero value for existing (unchanged) records. - - res, err := e.Exec("UPDATE user_setting SET setting_value=? WHERE setting_key=? AND user_id=?", value, key, userID) - if err != nil { - return err - } - rows, _ := res.RowsAffected() - if rows > 0 { - // the existing row is updated, so we can return - return nil - } - // in case the value isn't changed, update would return 0 rows changed, so we need this check - has, err := e.Exist(&Setting{UserID: userID, SettingKey: key}) - if err != nil { - return err - } - if has { - return nil - } + // in case the value isn't changed, update would return 0 rows changed, so we need this check + has, err := e.Exist(&Setting{UserID: userID, SettingKey: key}) + if err != nil { + return err + } + if has { + return nil + } - // if no existing row, insert a new row - _, err = e.Insert(&Setting{UserID: userID, SettingKey: key, SettingValue: value}) - return err + // if no existing row, insert a new row + _, err = e.Insert(&Setting{UserID: userID, SettingKey: key, SettingValue: value}) + return err + }) }