Skip to content

Commit 7087915

Browse files
wxiaoguangGiteaBot
wxiaoguang
authored and
GiteaBot
committed
Refactor system setting (go-gitea#27000)
This PR reduces the complexity of the system setting system. It only needs one line to introduce a new option, and the option can be used anywhere out-of-box. It is still high-performant (and more performant) because the config values are cached in the config system.
1 parent a9d547f commit 7087915

File tree

21 files changed

+411
-507
lines changed

21 files changed

+411
-507
lines changed

‎models/avatars/avatar.go

Lines changed: 58 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,20 @@ package avatars
55

66
import (
77
"context"
8+
"fmt"
89
"net/url"
910
"path"
1011
"strconv"
1112
"strings"
12-
"sync"
13+
"sync/atomic"
1314

1415
"code.gitea.io/gitea/models/db"
15-
system_model "code.gitea.io/gitea/models/system"
1616
"code.gitea.io/gitea/modules/base"
1717
"code.gitea.io/gitea/modules/cache"
1818
"code.gitea.io/gitea/modules/log"
1919
"code.gitea.io/gitea/modules/setting"
20+
21+
"strk.kbt.io/projects/go/libravatar"
2022
)
2123

2224
const (
@@ -36,24 +38,54 @@ func init() {
3638
db.RegisterModel(new(EmailHash))
3739
}
3840

39-
var (
41+
type avatarSettingStruct struct {
4042
defaultAvatarLink string
41-
once sync.Once
42-
)
43+
gravatarSource string
44+
gravatarSourceURL *url.URL
45+
libravatar *libravatar.Libravatar
46+
}
4347

44-
// DefaultAvatarLink the default avatar link
45-
func DefaultAvatarLink() string {
46-
once.Do(func() {
48+
var avatarSettingAtomic atomic.Pointer[avatarSettingStruct]
49+
50+
func loadAvatarSetting() (*avatarSettingStruct, error) {
51+
s := avatarSettingAtomic.Load()
52+
if s == nil || s.gravatarSource != setting.GravatarSource {
53+
s = &avatarSettingStruct{}
4754
u, err := url.Parse(setting.AppSubURL)
4855
if err != nil {
49-
log.Error("Can not parse AppSubURL: %v", err)
50-
return
56+
return nil, fmt.Errorf("unable to parse AppSubURL: %w", err)
5157
}
5258

5359
u.Path = path.Join(u.Path, "/assets/img/avatar_default.png")
54-
defaultAvatarLink = u.String()
55-
})
56-
return defaultAvatarLink
60+
s.defaultAvatarLink = u.String()
61+
62+
s.gravatarSourceURL, err = url.Parse(setting.GravatarSource)
63+
if err != nil {
64+
return nil, fmt.Errorf("unable to parse GravatarSource %q: %w", setting.GravatarSource, err)
65+
}
66+
67+
s.libravatar = libravatar.New()
68+
if s.gravatarSourceURL.Scheme == "https" {
69+
s.libravatar.SetUseHTTPS(true)
70+
s.libravatar.SetSecureFallbackHost(s.gravatarSourceURL.Host)
71+
} else {
72+
s.libravatar.SetUseHTTPS(false)
73+
s.libravatar.SetFallbackHost(s.gravatarSourceURL.Host)
74+
}
75+
76+
avatarSettingAtomic.Store(s)
77+
}
78+
return s, nil
79+
}
80+
81+
// DefaultAvatarLink the default avatar link
82+
func DefaultAvatarLink() string {
83+
a, err := loadAvatarSetting()
84+
if err != nil {
85+
log.Error("Failed to loadAvatarSetting: %v", err)
86+
return ""
87+
}
88+
return a.defaultAvatarLink
5789
}
5890

5991
// HashEmail hashes email address to MD5 string. https://en.gravatar.com/site/implement/hash/
@@ -76,7 +108,11 @@ func GetEmailForHash(md5Sum string) (string, error) {
76108
// LibravatarURL returns the URL for the given email. Slow due to the DNS lookup.
77109
// This function should only be called if a federated avatar service is enabled.
78110
func LibravatarURL(email string) (*url.URL, error) {
79-
urlStr, err := system_model.LibravatarService.FromEmail(email)
111+
a, err := loadAvatarSetting()
112+
if err != nil {
113+
return nil, err
114+
}
115+
urlStr, err := a.libravatar.FromEmail(email)
80116
if err != nil {
81117
log.Error("LibravatarService.FromEmail(email=%s): error %v", email, err)
82118
return nil, err
@@ -153,15 +189,13 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
153189
return DefaultAvatarLink()
154190
}
155191

156-
disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar,
157-
setting.GetDefaultDisableGravatar(),
158-
)
159-
160-
enableFederatedAvatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureEnableFederatedAvatar,
161-
setting.GetDefaultEnableFederatedAvatar(disableGravatar))
192+
avatarSetting, err := loadAvatarSetting()
193+
if err != nil {
194+
return DefaultAvatarLink()
195+
}
162196

163-
var err error
164-
if enableFederatedAvatar && system_model.LibravatarService != nil {
197+
enableFederatedAvatar := setting.Config().Picture.EnableFederatedAvatar.Value(ctx)
198+
if enableFederatedAvatar {
165199
emailHash := saveEmailHash(email)
166200
if final {
167201
// for final link, we can spend more time on slow external query
@@ -179,9 +213,10 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
179213
return urlStr
180214
}
181215

216+
disableGravatar := setting.Config().Picture.DisableGravatar.Value(ctx)
182217
if !disableGravatar {
183218
// copy GravatarSourceURL, because we will modify its Path.
184-
avatarURLCopy := *system_model.GravatarSourceURL
219+
avatarURLCopy := *avatarSetting.gravatarSourceURL
185220
avatarURLCopy.Path = path.Join(avatarURLCopy.Path, HashEmail(email))
186221
return generateRecognizedAvatarURL(avatarURLCopy, size)
187222
}

‎models/avatars/avatar_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,26 +10,24 @@ import (
1010
"code.gitea.io/gitea/models/db"
1111
system_model "code.gitea.io/gitea/models/system"
1212
"code.gitea.io/gitea/modules/setting"
13+
"code.gitea.io/gitea/modules/setting/config"
1314

1415
"github.com/stretchr/testify/assert"
1516
)
1617

1718
const gravatarSource = "https://secure.gravatar.com/avatar/"
1819

1920
func disableGravatar(t *testing.T) {
20-
err := system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureEnableFederatedAvatar, "false")
21+
err := system_model.SetSettings(db.DefaultContext, map[string]string{setting.Config().Picture.EnableFederatedAvatar.DynKey(): "false"})
2122
assert.NoError(t, err)
22-
err = system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureDisableGravatar, "true")
23+
err = system_model.SetSettings(db.DefaultContext, map[string]string{setting.Config().Picture.DisableGravatar.DynKey(): "true"})
2324
assert.NoError(t, err)
24-
system_model.LibravatarService = nil
2525
}
2626

2727
func enableGravatar(t *testing.T) {
28-
err := system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureDisableGravatar, "false")
28+
err := system_model.SetSettings(db.DefaultContext, map[string]string{setting.Config().Picture.DisableGravatar.DynKey(): "false"})
2929
assert.NoError(t, err)
3030
setting.GravatarSource = gravatarSource
31-
err = system_model.Init(db.DefaultContext)
32-
assert.NoError(t, err)
3331
}
3432

3533
func TestHashEmail(t *testing.T) {
@@ -47,10 +45,12 @@ func TestSizedAvatarLink(t *testing.T) {
4745
setting.AppSubURL = "/testsuburl"
4846

4947
disableGravatar(t)
48+
config.GetDynGetter().InvalidateCache()
5049
assert.Equal(t, "/testsuburl/assets/img/avatar_default.png",
5150
avatars_model.GenerateEmailAvatarFastLink(db.DefaultContext, "[email protected]", 100))
5251

5352
enableGravatar(t)
53+
config.GetDynGetter().InvalidateCache()
5454
assert.Equal(t,
5555
"https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon&s=100",
5656
avatars_model.GenerateEmailAvatarFastLink(db.DefaultContext, "[email protected]", 100),

‎models/migrations/v1_18/v227.go

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@
44
package v1_18 //nolint
55

66
import (
7-
"fmt"
8-
"strconv"
9-
10-
"code.gitea.io/gitea/modules/setting"
117
"code.gitea.io/gitea/modules/timeutil"
128

139
"xorm.io/xorm"
@@ -22,42 +18,6 @@ type SystemSetting struct {
2218
Updated timeutil.TimeStamp `xorm:"updated"`
2319
}
2420

25-
func insertSettingsIfNotExist(x *xorm.Engine, sysSettings []*SystemSetting) error {
26-
sess := x.NewSession()
27-
defer sess.Close()
28-
if err := sess.Begin(); err != nil {
29-
return err
30-
}
31-
for _, setting := range sysSettings {
32-
exist, err := sess.Table("system_setting").Where("setting_key=?", setting.SettingKey).Exist()
33-
if err != nil {
34-
return err
35-
}
36-
if !exist {
37-
if _, err := sess.Insert(setting); err != nil {
38-
return err
39-
}
40-
}
41-
}
42-
return sess.Commit()
43-
}
44-
4521
func CreateSystemSettingsTable(x *xorm.Engine) error {
46-
if err := x.Sync(new(SystemSetting)); err != nil {
47-
return fmt.Errorf("sync2: %w", err)
48-
}
49-
50-
// migrate xx to database
51-
sysSettings := []*SystemSetting{
52-
{
53-
SettingKey: "picture.disable_gravatar",
54-
SettingValue: strconv.FormatBool(setting.DisableGravatar),
55-
},
56-
{
57-
SettingKey: "picture.enable_federated_avatar",
58-
SettingValue: strconv.FormatBool(setting.EnableFederatedAvatar),
59-
},
60-
}
61-
62-
return insertSettingsIfNotExist(x, sysSettings)
22+
return x.Sync(new(SystemSetting))
6323
}

‎models/repo.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,14 @@ import (
1616
issues_model "code.gitea.io/gitea/models/issues"
1717
access_model "code.gitea.io/gitea/models/perm/access"
1818
repo_model "code.gitea.io/gitea/models/repo"
19-
system_model "code.gitea.io/gitea/models/system"
2019
"code.gitea.io/gitea/models/unit"
2120
user_model "code.gitea.io/gitea/models/user"
2221
"code.gitea.io/gitea/modules/log"
2322
)
2423

2524
// Init initialize model
2625
func Init(ctx context.Context) error {
27-
if err := unit.LoadUnitConfig(); err != nil {
28-
return err
29-
}
30-
return system_model.Init(ctx)
26+
return unit.LoadUnitConfig()
3127
}
3228

3329
type repoChecker struct {

0 commit comments

Comments
 (0)