From 69c8e6e2e5b2ae919929937d960661dede64854f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 18 Jan 2023 14:19:33 +0800 Subject: [PATCH 1/6] improvements --- models/migrations/migrations.go | 2 + models/migrations/v1_19/v240.go | 23 +++ models/repo/setting.go | 246 ++++++++++++++++++++++++++++++++ models/repo/setting_keys.go | 4 + models/repo/setting_test.go | 58 ++++++++ 5 files changed, 333 insertions(+) create mode 100644 models/migrations/v1_19/v240.go create mode 100644 models/repo/setting.go create mode 100644 models/repo/setting_keys.go create mode 100644 models/repo/setting_test.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 2058fcec0fe05..f3a670f9f8bd2 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -453,6 +453,8 @@ var migrations = []Migration{ NewMigration("Add updated unix to LFSMetaObject", v1_19.AddUpdatedUnixToLFSMetaObject), // v239 -> v240 NewMigration("Add scope for access_token", v1_19.AddScopeForAccessTokens), + // v240 -> v241 + NewMigration("Create key/value table for repo settings", v1_19.CreateRepoSettingsTable), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_19/v240.go b/models/migrations/v1_19/v240.go new file mode 100644 index 0000000000000..9db0d45b47de5 --- /dev/null +++ b/models/migrations/v1_19/v240.go @@ -0,0 +1,23 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_19 //nolint + +import ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func CreateRepoSettingsTable(x *xorm.Engine) error { + type RepoSetting struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"index unique(key_repoid)"` // to load all of someone's settings + SettingKey string `xorm:"varchar(255) index unique(key_repoid)"` // ensure key is always lowercase + SettingValue string `xorm:"text"` + Version int `xorm:"version"` // prevent to override + Created timeutil.TimeStamp `xorm:"created"` + Updated timeutil.TimeStamp `xorm:"updated"` + } + return x.Sync2(new(RepoSetting)) +} diff --git a/models/repo/setting.go b/models/repo/setting.go new file mode 100644 index 0000000000000..83eaf5f37e8d5 --- /dev/null +++ b/models/repo/setting.go @@ -0,0 +1,246 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "context" + "fmt" + "strconv" + "strings" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/cache" + setting_module "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/builder" +) + +// genSettingCacheKey returns the cache key for some configuration +func genSettingCacheKey(repoID int64, key string) string { + return fmt.Sprintf("repo.setting.%d.%s", repoID, key) +} + +// Setting is a key value store of repo settings +type Setting struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"index unique(key_repoid)"` // to load all of someone's settings + SettingKey string `xorm:"varchar(255) index unique(key_repoid)"` // ensure key is always lowercase + SettingValue string `xorm:"text"` + Version int `xorm:"version"` // prevent to override + Created timeutil.TimeStamp `xorm:"created"` + Updated timeutil.TimeStamp `xorm:"updated"` +} + +// TableName sets the table name for the settings struct +func (s *Setting) TableName() string { + return "repo_setting" +} + +func (s *Setting) GetValueBool() bool { + if s == nil { + return false + } + + b, _ := strconv.ParseBool(s.SettingValue) + return b +} + +func init() { + db.RegisterModel(new(Setting)) +} + +// ErrSettingIsNotExist represents an error that a setting is not exist with special key +type ErrSettingIsNotExist struct { + RepoID int64 + Key string +} + +// Error implements error +func (err ErrSettingIsNotExist) Error() string { + return fmt.Sprintf("Repo[%d] setting[%s] is not exist", err.RepoID, err.Key) +} + +// IsErrSettingIsNotExist return true if err is ErrSettingIsNotExist +func IsErrSettingIsNotExist(err error) bool { + _, ok := err.(ErrSettingIsNotExist) + return ok +} + +// GetSettingNoCache returns specific setting without using the cache +func GetSettingNoCache(repoID int64, key string) (*Setting, error) { + v, err := GetSettings(repoID, []string{key}) + if err != nil { + return nil, err + } + if len(v) == 0 { + return nil, ErrSettingIsNotExist{repoID, key} + } + return v[strings.ToLower(key)], nil +} + +func validateRepoSettingKey(key string) error { + if len(key) == 0 { + return fmt.Errorf("setting key must be set") + } + if strings.ToLower(key) != key { + return fmt.Errorf("setting key should be lowercase") + } + return nil +} + +// GetSetting returns the setting value via the key +func GetSetting(repoID int64, key string) (string, error) { + if err := validateRepoSettingKey(key); err != nil { + return "", err + } + return cache.GetString(genSettingCacheKey(repoID, key), func() (string, error) { + res, err := GetSettingNoCache(repoID, key) + if err != nil { + return "", err + } + return res.SettingValue, nil + }) +} + +// GetSettingBool return bool value of setting, +// none existing keys and errors are ignored and result in false +func GetSettingBool(repoID int64, key string) bool { + s, _ := GetSetting(repoID, key) + v, _ := strconv.ParseBool(s) + return v +} + +// GetSettings returns specific settings +func GetSettings(repoID int64, keys []string) (map[string]*Setting, error) { + for i := 0; i < len(keys); i++ { + keys[i] = strings.ToLower(keys[i]) + } + settings := make([]*Setting, 0, len(keys)) + if err := db.GetEngine(db.DefaultContext). + Where("repo_id=?", repoID). + And(builder.In("setting_key", keys)). + Find(&settings); err != nil { + return nil, err + } + settingsMap := make(map[string]*Setting) + for _, s := range settings { + settingsMap[s.SettingKey] = s + } + return settingsMap, nil +} + +type AllSettings map[string]*Setting + +func (settings AllSettings) Get(key string) Setting { + if v, ok := settings[strings.ToLower(key)]; ok { + return *v + } + return Setting{} +} + +func (settings AllSettings) GetBool(key string) bool { + b, _ := strconv.ParseBool(settings.Get(key).SettingValue) + return b +} + +func (settings AllSettings) GetVersion(key string) int { + return settings.Get(key).Version +} + +// GetAllSettings returns all settings from repo +func GetAllSettings(repoID int64) (AllSettings, error) { + settings := make([]*Setting, 0, 5) + if err := db.GetEngine(db.DefaultContext). + Where("repo_id=?", repoID). + Find(&settings); err != nil { + return nil, err + } + settingsMap := make(map[string]*Setting) + for _, s := range settings { + settingsMap[s.SettingKey] = s + } + return settingsMap, nil +} + +// DeleteSetting deletes a specific setting for a repo +func DeleteSetting(repoID int64, key string) error { + if err := validateRepoSettingKey(key); err != nil { + return err + } + cache.Remove(genSettingCacheKey(repoID, key)) + _, err := db.GetEngine(db.DefaultContext).Delete(&Setting{RepoID: repoID, SettingKey: key}) + return err +} + +func SetSettingNoVersion(repoID int64, key, value string) error { + s, err := GetSettingNoCache(repoID, key) + if IsErrSettingIsNotExist(err) { + return SetSetting(&Setting{ + RepoID: repoID, + SettingKey: key, + SettingValue: value, + }) + } + if err != nil { + return err + } + s.SettingValue = value + return SetSetting(s) +} + +// SetSetting updates a users' setting for a specific key +func SetSetting(setting *Setting) error { + if err := upsertSettingValue(setting.RepoID, strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil { + return err + } + + setting.Version++ + + cc := cache.GetCache() + if cc != nil { + return cc.Put(genSettingCacheKey(setting.RepoID, setting.SettingKey), setting.SettingValue, setting_module.CacheService.TTLSeconds()) + } + + return nil +} + +func upsertSettingValue(repoID int64, key, value string, version int) error { + return db.WithTx(db.DefaultContext, 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 repo_setting SET setting_value=?, version = version+1 WHERE repo_id=? AND setting_key=? AND version=?", value, repoID, key, version) + 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{RepoID: repoID, SettingKey: key}) + if err != nil { + return err + } + if has { + return nil + } + + // if no existing row, insert a new row + _, err = e.Insert(&Setting{RepoID: repoID, SettingKey: key, SettingValue: value}) + return err + }) +} diff --git a/models/repo/setting_keys.go b/models/repo/setting_keys.go new file mode 100644 index 0000000000000..07a943e64cb02 --- /dev/null +++ b/models/repo/setting_keys.go @@ -0,0 +1,4 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo diff --git a/models/repo/setting_test.go b/models/repo/setting_test.go new file mode 100644 index 0000000000000..1e8da17772839 --- /dev/null +++ b/models/repo/setting_test.go @@ -0,0 +1,58 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "testing" + + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestSettings(t *testing.T) { + keyName := "test_repo_setting" + assert.NoError(t, unittest.PrepareTestDatabase()) + + newSetting := &Setting{RepoID: 99, SettingKey: keyName, SettingValue: "Gitea Repo Setting Test"} + + // create setting + err := SetSetting(newSetting) + assert.NoError(t, err) + // test about saving unchanged values + err = SetSetting(newSetting) + assert.NoError(t, err) + + // 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) + + settingValue, err := GetSetting(99, keyName) + assert.NoError(t, err) + assert.EqualValues(t, newSetting.SettingValue, settingValue) + + settingValue, err = GetSetting(99, "no_such") + assert.NoError(t, err) + assert.EqualValues(t, "", settingValue) + + // updated setting + updatedSetting := &Setting{RepoID: 99, SettingKey: keyName, SettingValue: "Updated"} + err = SetSetting(updatedSetting) + assert.NoError(t, err) + + // get all settings + settings, err = GetAllSettings(99) + assert.NoError(t, err) + assert.Len(t, settings, 1) + assert.EqualValues(t, updatedSetting.SettingValue, settings[updatedSetting.SettingKey].SettingValue) + + // delete setting + err = DeleteSetting(99, keyName) + assert.NoError(t, err) + settings, err = GetAllSettings(99) + assert.NoError(t, err) + assert.Len(t, settings, 0) +} From 8cead1a36d49eea4ff711f3b25625687eeef1ebb Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 18 Jan 2023 16:46:42 +0800 Subject: [PATCH 2/6] Use error wrap and fix test --- models/migrations/v1_19/v240.go | 2 +- models/repo.go | 1 + models/repo/setting.go | 25 +++++-------------------- models/repo/setting_keys.go | 7 ++++++- models/repo/setting_test.go | 6 ++++-- 5 files changed, 17 insertions(+), 24 deletions(-) diff --git a/models/migrations/v1_19/v240.go b/models/migrations/v1_19/v240.go index 9db0d45b47de5..6ec44e1db3306 100644 --- a/models/migrations/v1_19/v240.go +++ b/models/migrations/v1_19/v240.go @@ -1,4 +1,4 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. +// Copyright 2023 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package v1_19 //nolint diff --git a/models/repo.go b/models/repo.go index e95887077c955..d78e8899bffed 100644 --- a/models/repo.go +++ b/models/repo.go @@ -152,6 +152,7 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { &repo_model.Watch{RepoID: repoID}, &webhook.Webhook{RepoID: repoID}, &secret_model.Secret{RepoID: repoID}, + &repo_model.Setting{RepoID: repoID}, ); err != nil { return fmt.Errorf("deleteBeans: %w", err) } diff --git a/models/repo/setting.go b/models/repo/setting.go index 83eaf5f37e8d5..3ac36f624bbcb 100644 --- a/models/repo/setting.go +++ b/models/repo/setting.go @@ -1,10 +1,11 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. +// Copyright 2023 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package repo import ( "context" + "errors" "fmt" "strconv" "strings" @@ -13,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/cache" setting_module "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" ) @@ -51,23 +53,6 @@ func init() { db.RegisterModel(new(Setting)) } -// ErrSettingIsNotExist represents an error that a setting is not exist with special key -type ErrSettingIsNotExist struct { - RepoID int64 - Key string -} - -// Error implements error -func (err ErrSettingIsNotExist) Error() string { - return fmt.Sprintf("Repo[%d] setting[%s] is not exist", err.RepoID, err.Key) -} - -// IsErrSettingIsNotExist return true if err is ErrSettingIsNotExist -func IsErrSettingIsNotExist(err error) bool { - _, ok := err.(ErrSettingIsNotExist) - return ok -} - // GetSettingNoCache returns specific setting without using the cache func GetSettingNoCache(repoID int64, key string) (*Setting, error) { v, err := GetSettings(repoID, []string{key}) @@ -75,7 +60,7 @@ func GetSettingNoCache(repoID int64, key string) (*Setting, error) { return nil, err } if len(v) == 0 { - return nil, ErrSettingIsNotExist{repoID, key} + return nil, fmt.Errorf("repo[%d] setting[%s]: %w", repoID, key, util.ErrNotExist) } return v[strings.ToLower(key)], nil } @@ -176,7 +161,7 @@ func DeleteSetting(repoID int64, key string) error { func SetSettingNoVersion(repoID int64, key, value string) error { s, err := GetSettingNoCache(repoID, key) - if IsErrSettingIsNotExist(err) { + if errors.Is(err, util.ErrNotExist) { return SetSetting(&Setting{ RepoID: repoID, SettingKey: key, diff --git a/models/repo/setting_keys.go b/models/repo/setting_keys.go index 07a943e64cb02..f906ddfce8d42 100644 --- a/models/repo/setting_keys.go +++ b/models/repo/setting_keys.go @@ -1,4 +1,9 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. +// Copyright 2023 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package repo + +const ( + // setting keys + _ = "" +) diff --git a/models/repo/setting_test.go b/models/repo/setting_test.go index 1e8da17772839..390fd53ad28c0 100644 --- a/models/repo/setting_test.go +++ b/models/repo/setting_test.go @@ -4,9 +4,11 @@ package repo import ( + "errors" "testing" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -35,11 +37,11 @@ func TestSettings(t *testing.T) { assert.EqualValues(t, newSetting.SettingValue, settingValue) settingValue, err = GetSetting(99, "no_such") - assert.NoError(t, err) + assert.True(t, errors.Is(err, util.ErrNotExist)) assert.EqualValues(t, "", settingValue) // updated setting - updatedSetting := &Setting{RepoID: 99, SettingKey: keyName, SettingValue: "Updated"} + updatedSetting := &Setting{RepoID: 99, SettingKey: keyName, SettingValue: "Updated", Version: 2} // updated twice err = SetSetting(updatedSetting) assert.NoError(t, err) From e5434d4cd718e74ff210dc1ac6b1080523bd3f52 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 18 Jan 2023 16:48:33 +0800 Subject: [PATCH 3/6] Fix copyright year --- models/repo/setting_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo/setting_test.go b/models/repo/setting_test.go index 390fd53ad28c0..9ff51de0d1446 100644 --- a/models/repo/setting_test.go +++ b/models/repo/setting_test.go @@ -1,4 +1,4 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. +// Copyright 2023 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package repo From d6b5d37ffb52d4a9cf96570185f7bcc484ab4c18 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 29 Jan 2023 13:34:29 +0800 Subject: [PATCH 4/6] share the common function --- models/repo/setting.go | 17 ++++------------- models/user/setting.go | 8 ++++---- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/models/repo/setting.go b/models/repo/setting.go index 3ac36f624bbcb..1dc2b6516b4df 100644 --- a/models/repo/setting.go +++ b/models/repo/setting.go @@ -11,6 +11,7 @@ import ( "strings" "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" setting_module "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" @@ -40,7 +41,7 @@ func (s *Setting) TableName() string { return "repo_setting" } -func (s *Setting) GetValueBool() bool { +func (s *Setting) AsBool() bool { if s == nil { return false } @@ -65,19 +66,9 @@ func GetSettingNoCache(repoID int64, key string) (*Setting, error) { return v[strings.ToLower(key)], nil } -func validateRepoSettingKey(key string) error { - if len(key) == 0 { - return fmt.Errorf("setting key must be set") - } - if strings.ToLower(key) != key { - return fmt.Errorf("setting key should be lowercase") - } - return nil -} - // GetSetting returns the setting value via the key func GetSetting(repoID int64, key string) (string, error) { - if err := validateRepoSettingKey(key); err != nil { + if err := user_model.ValidateSettingKey(key); err != nil { return "", err } return cache.GetString(genSettingCacheKey(repoID, key), func() (string, error) { @@ -151,7 +142,7 @@ func GetAllSettings(repoID int64) (AllSettings, error) { // DeleteSetting deletes a specific setting for a repo func DeleteSetting(repoID int64, key string) error { - if err := validateRepoSettingKey(key); err != nil { + if err := user_model.ValidateSettingKey(key); err != nil { return err } cache.Remove(genSettingCacheKey(repoID, key)) diff --git a/models/user/setting.go b/models/user/setting.go index f5cfef5b33945..69d26915a3613 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -106,7 +106,7 @@ func GetUserAllSettings(uid int64) (map[string]*Setting, error) { return settingsMap, nil } -func validateUserSettingKey(key string) error { +func ValidateSettingKey(key string) error { if len(key) == 0 { return fmt.Errorf("setting key must be set") } @@ -118,7 +118,7 @@ func validateUserSettingKey(key string) error { // GetUserSetting gets a specific setting for a user func GetUserSetting(userID int64, key string, def ...string) (string, error) { - if err := validateUserSettingKey(key); err != nil { + if err := ValidateSettingKey(key); err != nil { return "", err } @@ -138,7 +138,7 @@ func GetUserSetting(userID int64, key string, def ...string) (string, error) { // DeleteUserSetting deletes a specific setting for a user func DeleteUserSetting(userID int64, key string) error { - if err := validateUserSettingKey(key); err != nil { + if err := ValidateSettingKey(key); err != nil { return err } @@ -150,7 +150,7 @@ func DeleteUserSetting(userID int64, key string) error { // SetUserSetting updates a users' setting for a specific key func SetUserSetting(userID int64, key, value string) error { - if err := validateUserSettingKey(key); err != nil { + if err := ValidateSettingKey(key); err != nil { return err } From 3ee92eb74ef2ac71b2e7e672e7784ee6e2b24835 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 6 Feb 2023 17:25:19 +0800 Subject: [PATCH 5/6] Use an abstract db.ResourceSetting like what db.ResourceIndex did --- models/db/setting.go | 222 ++++++++++++++++++++++++++++++++ models/migrations/v1_19/v241.go | 12 +- models/repo.go | 2 +- models/repo/setting.go | 207 +++-------------------------- models/repo/setting_keys.go | 9 -- models/repo/setting_test.go | 4 +- models/user/setting.go | 17 +-- 7 files changed, 251 insertions(+), 222 deletions(-) create mode 100644 models/db/setting.go delete mode 100644 models/repo/setting_keys.go diff --git a/models/db/setting.go b/models/db/setting.go new file mode 100644 index 0000000000000..e65a4738173a3 --- /dev/null +++ b/models/db/setting.go @@ -0,0 +1,222 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package db + +import ( + "context" + "errors" + "fmt" + "strconv" + "strings" + + "code.gitea.io/gitea/modules/cache" + setting_module "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" + + "xorm.io/builder" +) + +type ResourceSetting struct { + ID int64 `xorm:"pk autoincr"` + GroupID int64 `xorm:"index unique(key_repoid)"` // to load all of some group's settings + SettingKey string `xorm:"varchar(255) index unique(key_groupid)"` // ensure key is always lowercase + SettingValue string `xorm:"text"` + Version int `xorm:"version"` // prevent to override + Created timeutil.TimeStamp `xorm:"created"` + Updated timeutil.TimeStamp `xorm:"updated"` +} + +func (s *ResourceSetting) AsBool() bool { + if s == nil { + return false + } + + b, _ := strconv.ParseBool(s.SettingValue) + return b +} + +// GetSettings returns specific settings +func GetSettings(ctx context.Context, tableName string, groupID int64, keys []string) (map[string]*ResourceSetting, error) { + for i := 0; i < len(keys); i++ { + keys[i] = strings.ToLower(keys[i]) + } + settings := make([]*ResourceSetting, 0, len(keys)) + if err := GetEngine(ctx). + Table(tableName). + Where("group_id=?", groupID). + And(builder.In("setting_key", keys)). + Find(&settings); err != nil { + return nil, err + } + settingsMap := make(map[string]*ResourceSetting) + for _, s := range settings { + settingsMap[s.SettingKey] = s + } + return settingsMap, nil +} + +func ValidateSettingKey(key string) error { + if len(key) == 0 { + return fmt.Errorf("setting key must be set") + } + if strings.ToLower(key) != key { + return fmt.Errorf("setting key should be lowercase") + } + return nil +} + +// genSettingCacheKey returns the cache key for some configuration +func genSettingCacheKey(tableName string, groupID int64, key string) string { + return fmt.Sprintf("%s.setting.%d.%s", tableName, groupID, key) +} + +// GetSettingNoCache returns specific setting without using the cache +func GetSettingNoCache(ctx context.Context, tableName string, groupID int64, key string) (*ResourceSetting, error) { + v, err := GetSettings(ctx, tableName, groupID, []string{key}) + if err != nil { + return nil, err + } + if len(v) == 0 { + return nil, fmt.Errorf("%s[%d] setting[%s]: %w", tableName, groupID, key, util.ErrNotExist) + } + return v[strings.ToLower(key)], nil +} + +// GetSetting returns the setting value via the key +func GetSetting(ctx context.Context, tableName string, groupID int64, key string) (string, error) { + if err := ValidateSettingKey(key); err != nil { + return "", err + } + return cache.GetString(genSettingCacheKey(tableName, groupID, key), func() (string, error) { + res, err := GetSettingNoCache(ctx, tableName, groupID, key) + if err != nil { + return "", err + } + return res.SettingValue, nil + }) +} + +// GetSettingBool return bool value of setting, +// none existing keys and errors are ignored and result in false +func GetSettingBool(ctx context.Context, tableName string, groupID int64, key string) bool { + s, _ := GetSetting(ctx, tableName, groupID, key) + v, _ := strconv.ParseBool(s) + return v +} + +type AllSettings map[string]*ResourceSetting + +func (settings AllSettings) Get(key string) ResourceSetting { + if v, ok := settings[strings.ToLower(key)]; ok { + return *v + } + return ResourceSetting{} +} + +func (settings AllSettings) GetBool(key string) bool { + b, _ := strconv.ParseBool(settings.Get(key).SettingValue) + return b +} + +func (settings AllSettings) GetVersion(key string) int { + return settings.Get(key).Version +} + +// GetAllSettings returns all settings from repo +func GetAllSettings(ctx context.Context, tableName string, groupID int64) (AllSettings, error) { + settings := make([]*ResourceSetting, 0, 5) + if err := GetEngine(ctx). + Table(tableName). + Where("group_id=?", groupID). + Find(&settings); err != nil { + return nil, err + } + settingsMap := make(map[string]*ResourceSetting) + for _, s := range settings { + settingsMap[s.SettingKey] = s + } + return settingsMap, nil +} + +// DeleteSetting deletes a specific setting for a repo +func DeleteSetting(ctx context.Context, tableName string, groupID int64, key string) error { + if err := ValidateSettingKey(key); err != nil { + return err + } + cache.Remove(genSettingCacheKey(tableName, groupID, key)) + _, err := GetEngine(ctx).Table(tableName).Delete(&ResourceSetting{GroupID: groupID, SettingKey: key}) + return err +} + +func SetSettingNoVersion(ctx context.Context, tableName string, groupID int64, key, value string) error { + s, err := GetSettingNoCache(ctx, tableName, groupID, key) + if errors.Is(err, util.ErrNotExist) { + return SetSetting(ctx, tableName, &ResourceSetting{ + GroupID: groupID, + SettingKey: key, + SettingValue: value, + }) + } + if err != nil { + return err + } + s.SettingValue = value + return SetSetting(ctx, tableName, s) +} + +// SetSetting updates a users' setting for a specific key +func SetSetting(ctx context.Context, tableName string, setting *ResourceSetting) error { + if err := upsertSettingValue(ctx, tableName, setting.GroupID, strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil { + return err + } + + setting.Version++ + + cc := cache.GetCache() + if cc != nil { + return cc.Put(genSettingCacheKey(tableName, setting.GroupID, setting.SettingKey), setting.SettingValue, setting_module.CacheService.TTLSeconds()) + } + + return nil +} + +func upsertSettingValue(ctx context.Context, tableName string, groupID int64, key, value string, version int) error { + return WithTx(ctx, func(ctx context.Context) error { + e := 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(fmt.Sprintf("UPDATE %s SET setting_value=?, version = version+1 WHERE group_id=? AND setting_key=? AND version=?", tableName), value, groupID, key, version) + 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.Table(tableName).Exist(&ResourceSetting{GroupID: groupID, SettingKey: key}) + if err != nil { + return err + } + if has { + return nil + } + + // if no existing row, insert a new row + _, err = e.Table(tableName).Insert(&ResourceSetting{GroupID: groupID, SettingKey: key, SettingValue: value}) + return err + }) +} diff --git a/models/migrations/v1_19/v241.go b/models/migrations/v1_19/v241.go index 6ec44e1db3306..2bc30def0a3bc 100644 --- a/models/migrations/v1_19/v241.go +++ b/models/migrations/v1_19/v241.go @@ -4,20 +4,12 @@ package v1_19 //nolint import ( - "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/models/db" "xorm.io/xorm" ) func CreateRepoSettingsTable(x *xorm.Engine) error { - type RepoSetting struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"index unique(key_repoid)"` // to load all of someone's settings - SettingKey string `xorm:"varchar(255) index unique(key_repoid)"` // ensure key is always lowercase - SettingValue string `xorm:"text"` - Version int `xorm:"version"` // prevent to override - Created timeutil.TimeStamp `xorm:"created"` - Updated timeutil.TimeStamp `xorm:"updated"` - } + type RepoSetting db.ResourceSetting // to generate a table named repo_setting return x.Sync2(new(RepoSetting)) } diff --git a/models/repo.go b/models/repo.go index a3b96d2883a48..b62b08d0836c1 100644 --- a/models/repo.go +++ b/models/repo.go @@ -160,7 +160,7 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { &repo_model.Watch{RepoID: repoID}, &webhook.Webhook{RepoID: repoID}, &secret_model.Secret{RepoID: repoID}, - &repo_model.Setting{RepoID: repoID}, + &repo_model.Setting{GroupID: repoID}, &actions_model.ActionTaskStep{RepoID: repoID}, &actions_model.ActionTask{RepoID: repoID}, &actions_model.ActionRunJob{RepoID: repoID}, diff --git a/models/repo/setting.go b/models/repo/setting.go index 1dc2b6516b4df..6742edea3611b 100644 --- a/models/repo/setting.go +++ b/models/repo/setting.go @@ -4,219 +4,54 @@ package repo import ( - "context" - "errors" - "fmt" - "strconv" - "strings" - "code.gitea.io/gitea/models/db" - user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/cache" - setting_module "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/timeutil" - "code.gitea.io/gitea/modules/util" - - "xorm.io/builder" ) -// genSettingCacheKey returns the cache key for some configuration -func genSettingCacheKey(repoID int64, key string) string { - return fmt.Sprintf("repo.setting.%d.%s", repoID, key) -} +type Setting db.ResourceSetting -// Setting is a key value store of repo settings -type Setting struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"index unique(key_repoid)"` // to load all of someone's settings - SettingKey string `xorm:"varchar(255) index unique(key_repoid)"` // ensure key is always lowercase - SettingValue string `xorm:"text"` - Version int `xorm:"version"` // prevent to override - Created timeutil.TimeStamp `xorm:"created"` - Updated timeutil.TimeStamp `xorm:"updated"` -} +const repoSettingTableName = "repo_setting" // TableName sets the table name for the settings struct func (s *Setting) TableName() string { - return "repo_setting" -} - -func (s *Setting) AsBool() bool { - if s == nil { - return false - } - - b, _ := strconv.ParseBool(s.SettingValue) - return b + return repoSettingTableName } func init() { db.RegisterModel(new(Setting)) } -// GetSettingNoCache returns specific setting without using the cache -func GetSettingNoCache(repoID int64, key string) (*Setting, error) { - v, err := GetSettings(repoID, []string{key}) - if err != nil { - return nil, err - } - if len(v) == 0 { - return nil, fmt.Errorf("repo[%d] setting[%s]: %w", repoID, key, util.ErrNotExist) - } - return v[strings.ToLower(key)], nil -} - -// GetSetting returns the setting value via the key -func GetSetting(repoID int64, key string) (string, error) { - if err := user_model.ValidateSettingKey(key); err != nil { - return "", err - } - return cache.GetString(genSettingCacheKey(repoID, key), func() (string, error) { - res, err := GetSettingNoCache(repoID, key) - if err != nil { - return "", err - } - return res.SettingValue, nil - }) -} - -// GetSettingBool return bool value of setting, -// none existing keys and errors are ignored and result in false -func GetSettingBool(repoID int64, key string) bool { - s, _ := GetSetting(repoID, key) - v, _ := strconv.ParseBool(s) - return v +func SetSetting(s *Setting) error { + return db.SetSetting(db.DefaultContext, repoSettingTableName, (*db.ResourceSetting)(s)) } -// GetSettings returns specific settings func GetSettings(repoID int64, keys []string) (map[string]*Setting, error) { - for i := 0; i < len(keys); i++ { - keys[i] = strings.ToLower(keys[i]) - } - settings := make([]*Setting, 0, len(keys)) - if err := db.GetEngine(db.DefaultContext). - Where("repo_id=?", repoID). - And(builder.In("setting_key", keys)). - Find(&settings); err != nil { + settings := make(map[string]*Setting) + resourceSettings, err := db.GetSettings(db.DefaultContext, repoSettingTableName, repoID, keys) + if err != nil { return nil, err } - settingsMap := make(map[string]*Setting) - for _, s := range settings { - settingsMap[s.SettingKey] = s - } - return settingsMap, nil -} - -type AllSettings map[string]*Setting - -func (settings AllSettings) Get(key string) Setting { - if v, ok := settings[strings.ToLower(key)]; ok { - return *v + for key, setting := range resourceSettings { + settings[key] = (*Setting)(setting) } - return Setting{} + return settings, nil } -func (settings AllSettings) GetBool(key string) bool { - b, _ := strconv.ParseBool(settings.Get(key).SettingValue) - return b -} - -func (settings AllSettings) GetVersion(key string) int { - return settings.Get(key).Version -} - -// GetAllSettings returns all settings from repo -func GetAllSettings(repoID int64) (AllSettings, error) { - settings := make([]*Setting, 0, 5) - if err := db.GetEngine(db.DefaultContext). - Where("repo_id=?", repoID). - Find(&settings); err != nil { - return nil, err - } - settingsMap := make(map[string]*Setting) - for _, s := range settings { - settingsMap[s.SettingKey] = s - } - return settingsMap, nil +func GetSetting(repoID int64, key string) (string, error) { + return db.GetSetting(db.DefaultContext, repoSettingTableName, repoID, key) } -// DeleteSetting deletes a specific setting for a repo func DeleteSetting(repoID int64, key string) error { - if err := user_model.ValidateSettingKey(key); err != nil { - return err - } - cache.Remove(genSettingCacheKey(repoID, key)) - _, err := db.GetEngine(db.DefaultContext).Delete(&Setting{RepoID: repoID, SettingKey: key}) - return err + return db.DeleteSetting(db.DefaultContext, repoSettingTableName, repoID, key) } -func SetSettingNoVersion(repoID int64, key, value string) error { - s, err := GetSettingNoCache(repoID, key) - if errors.Is(err, util.ErrNotExist) { - return SetSetting(&Setting{ - RepoID: repoID, - SettingKey: key, - SettingValue: value, - }) - } +func GetAllSettings(repoID int64) (map[string]*Setting, error) { + settings := make(map[string]*Setting) + resourceSettings, err := db.GetAllSettings(db.DefaultContext, repoSettingTableName, repoID) if err != nil { - return err - } - s.SettingValue = value - return SetSetting(s) -} - -// SetSetting updates a users' setting for a specific key -func SetSetting(setting *Setting) error { - if err := upsertSettingValue(setting.RepoID, strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil { - return err + return nil, err } - - setting.Version++ - - cc := cache.GetCache() - if cc != nil { - return cc.Put(genSettingCacheKey(setting.RepoID, setting.SettingKey), setting.SettingValue, setting_module.CacheService.TTLSeconds()) + for key, setting := range resourceSettings { + settings[key] = (*Setting)(setting) } - - return nil -} - -func upsertSettingValue(repoID int64, key, value string, version int) error { - return db.WithTx(db.DefaultContext, 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 repo_setting SET setting_value=?, version = version+1 WHERE repo_id=? AND setting_key=? AND version=?", value, repoID, key, version) - 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{RepoID: repoID, SettingKey: key}) - if err != nil { - return err - } - if has { - return nil - } - - // if no existing row, insert a new row - _, err = e.Insert(&Setting{RepoID: repoID, SettingKey: key, SettingValue: value}) - return err - }) + return settings, nil } diff --git a/models/repo/setting_keys.go b/models/repo/setting_keys.go deleted file mode 100644 index f906ddfce8d42..0000000000000 --- a/models/repo/setting_keys.go +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright 2023 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package repo - -const ( - // setting keys - _ = "" -) diff --git a/models/repo/setting_test.go b/models/repo/setting_test.go index 9ff51de0d1446..949b1651e9b64 100644 --- a/models/repo/setting_test.go +++ b/models/repo/setting_test.go @@ -17,7 +17,7 @@ func TestSettings(t *testing.T) { keyName := "test_repo_setting" assert.NoError(t, unittest.PrepareTestDatabase()) - newSetting := &Setting{RepoID: 99, SettingKey: keyName, SettingValue: "Gitea Repo Setting Test"} + newSetting := &Setting{GroupID: 99, SettingKey: keyName, SettingValue: "Gitea Repo Setting Test"} // create setting err := SetSetting(newSetting) @@ -41,7 +41,7 @@ func TestSettings(t *testing.T) { assert.EqualValues(t, "", settingValue) // updated setting - updatedSetting := &Setting{RepoID: 99, SettingKey: keyName, SettingValue: "Updated", Version: 2} // updated twice + updatedSetting := &Setting{GroupID: 99, SettingKey: keyName, SettingValue: "Updated", Version: 2} // updated twice err = SetSetting(updatedSetting) assert.NoError(t, err) diff --git a/models/user/setting.go b/models/user/setting.go index 47ffcdc75ec5a..40bd74a8ca9ed 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -6,7 +6,6 @@ package user import ( "context" "fmt" - "strings" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/cache" @@ -107,19 +106,9 @@ func GetUserAllSettings(uid int64) (map[string]*Setting, error) { return settingsMap, nil } -func ValidateSettingKey(key string) error { - if len(key) == 0 { - return fmt.Errorf("setting key must be set") - } - if strings.ToLower(key) != key { - return fmt.Errorf("setting key should be lowercase") - } - return nil -} - // GetUserSetting gets a specific setting for a user func GetUserSetting(userID int64, key string, def ...string) (string, error) { - if err := ValidateSettingKey(key); err != nil { + if err := db.ValidateSettingKey(key); err != nil { return "", err } @@ -139,7 +128,7 @@ func GetUserSetting(userID int64, key string, def ...string) (string, error) { // DeleteUserSetting deletes a specific setting for a user func DeleteUserSetting(userID int64, key string) error { - if err := ValidateSettingKey(key); err != nil { + if err := db.ValidateSettingKey(key); err != nil { return err } @@ -151,7 +140,7 @@ func DeleteUserSetting(userID int64, key string) error { // SetUserSetting updates a users' setting for a specific key func SetUserSetting(userID int64, key, value string) error { - if err := ValidateSettingKey(key); err != nil { + if err := db.ValidateSettingKey(key); err != nil { return err } From 7acebb157fc3c0dbb8795f3a1f86e8b86e92554b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 21 Mar 2023 16:26:51 +0800 Subject: [PATCH 6/6] follow @delvh suggestion --- models/db/setting.go | 31 +++++++++++++++++++++++++------ models/migrations/v1_20/v245.go | 4 ++-- models/repo/setting.go | 4 ++-- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/models/db/setting.go b/models/db/setting.go index e65a4738173a3..2d3bb759cd121 100644 --- a/models/db/setting.go +++ b/models/db/setting.go @@ -20,7 +20,7 @@ import ( type ResourceSetting struct { ID int64 `xorm:"pk autoincr"` - GroupID int64 `xorm:"index unique(key_repoid)"` // to load all of some group's settings + GroupID int64 `xorm:"index unique(key_groupid)"` // to load all of some group's settings SettingKey string `xorm:"varchar(255) index unique(key_groupid)"` // ensure key is always lowercase SettingValue string `xorm:"text"` Version int `xorm:"version"` // prevent to override @@ -39,7 +39,7 @@ func (s *ResourceSetting) AsBool() bool { // GetSettings returns specific settings func GetSettings(ctx context.Context, tableName string, groupID int64, keys []string) (map[string]*ResourceSetting, error) { - for i := 0; i < len(keys); i++ { + for i := range keys { keys[i] = strings.ToLower(keys[i]) } settings := make([]*ResourceSetting, 0, len(keys)) @@ -50,7 +50,7 @@ func GetSettings(ctx context.Context, tableName string, groupID int64, keys []st Find(&settings); err != nil { return nil, err } - settingsMap := make(map[string]*ResourceSetting) + settingsMap := make(map[string]*ResourceSetting, len(settings)) for _, s := range settings { settingsMap[s.SettingKey] = s } @@ -79,7 +79,7 @@ func GetSettingNoCache(ctx context.Context, tableName string, groupID int64, key return nil, err } if len(v) == 0 { - return nil, fmt.Errorf("%s[%d] setting[%s]: %w", tableName, groupID, key, util.ErrNotExist) + return nil, fmt.Errorf("%s[%d][%s]: %w", tableName, groupID, key, util.ErrNotExist) } return v[strings.ToLower(key)], nil } @@ -115,7 +115,7 @@ func (settings AllSettings) Get(key string) ResourceSetting { return ResourceSetting{} } -func (settings AllSettings) GetBool(key string) bool { +func (settings AllSettings) AsBool(key string) bool { b, _ := strconv.ParseBool(settings.Get(key).SettingValue) return b } @@ -133,7 +133,7 @@ func GetAllSettings(ctx context.Context, tableName string, groupID int64) (AllSe Find(&settings); err != nil { return nil, err } - settingsMap := make(map[string]*ResourceSetting) + settingsMap := make(map[string]*ResourceSetting, len(settings)) for _, s := range settings { settingsMap[s.SettingKey] = s } @@ -220,3 +220,22 @@ func upsertSettingValue(ctx context.Context, tableName string, groupID int64, ke return err }) } + +type ResourceSettingKey struct { + ID int64 + KeyName string `xorm:"unique"` +} + +// GetResourceSettingKeyID get key id by key name +func GetResourceSettingKeyID(ctx context.Context, keyname string) (int64, error) { + key := &ResourceSettingKey{KeyName: keyname} + has, err := GetEngine(ctx).Get(key) + if err != nil { + return 0, err + } else if !has { + if err := Insert(ctx, key); err != nil { + return 0, err + } + } + return key.ID, nil +} diff --git a/models/migrations/v1_20/v245.go b/models/migrations/v1_20/v245.go index 56a8917c45f6c..1d783ca7b2642 100644 --- a/models/migrations/v1_20/v245.go +++ b/models/migrations/v1_20/v245.go @@ -12,12 +12,12 @@ import ( func CreateRepoSettingsTable(x *xorm.Engine) error { type RepoSetting struct { ID int64 `xorm:"pk autoincr"` - GroupID int64 `xorm:"index unique(key_repoid)"` // to load all of some group's settings + GroupID int64 `xorm:"index unique(key_groupid)"` // to load all of some group's settings SettingKey string `xorm:"varchar(255) index unique(key_groupid)"` // ensure key is always lowercase SettingValue string `xorm:"text"` Version int `xorm:"version"` // prevent to override Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated"` } - return x.Sync2(new(RepoSetting)) + return x.Sync(new(RepoSetting)) } diff --git a/models/repo/setting.go b/models/repo/setting.go index 6742edea3611b..6de7cecb64dc3 100644 --- a/models/repo/setting.go +++ b/models/repo/setting.go @@ -25,11 +25,11 @@ func SetSetting(s *Setting) error { } func GetSettings(repoID int64, keys []string) (map[string]*Setting, error) { - settings := make(map[string]*Setting) resourceSettings, err := db.GetSettings(db.DefaultContext, repoSettingTableName, repoID, keys) if err != nil { return nil, err } + settings := make(map[string]*Setting, len(resourceSettings)) for key, setting := range resourceSettings { settings[key] = (*Setting)(setting) } @@ -45,11 +45,11 @@ func DeleteSetting(repoID int64, key string) error { } func GetAllSettings(repoID int64) (map[string]*Setting, error) { - settings := make(map[string]*Setting) resourceSettings, err := db.GetAllSettings(db.DefaultContext, repoSettingTableName, repoID) if err != nil { return nil, err } + settings := make(map[string]*Setting, len(resourceSettings)) for key, setting := range resourceSettings { settings[key] = (*Setting)(setting) }