Skip to content

Commit 91fa0eb

Browse files
lunnylafriks
lunny
and
lafriks
authored
Avoid warning for system setting when start up (#23054)
Partially fix #23050 After #22294 merged, it always has a warning log like `cannot get context cache` when starting up. This should not affect any real life but it's annoying. This PR will fix the problem. That means when starting up, getting the system settings will not try from the cache but will read from the database directly. --------- Co-authored-by: Lauris BH <[email protected]>
1 parent edf98a2 commit 91fa0eb

File tree

11 files changed

+43
-34
lines changed

11 files changed

+43
-34
lines changed

Diff for: ‎models/avatars/avatar.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
153153
return DefaultAvatarLink()
154154
}
155155

156-
enableFederatedAvatar := system_model.GetSettingBool(ctx, system_model.KeyPictureEnableFederatedAvatar)
156+
enableFederatedAvatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureEnableFederatedAvatar)
157157

158158
var err error
159159
if enableFederatedAvatar && system_model.LibravatarService != nil {
@@ -174,7 +174,7 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
174174
return urlStr
175175
}
176176

177-
disableGravatar := system_model.GetSettingBool(ctx, system_model.KeyPictureDisableGravatar)
177+
disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar)
178178
if !disableGravatar {
179179
// copy GravatarSourceURL, because we will modify its Path.
180180
avatarURLCopy := *system_model.GravatarSourceURL

Diff for: ‎models/avatars/avatar_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func enableGravatar(t *testing.T) {
2828
err := system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureDisableGravatar, "false")
2929
assert.NoError(t, err)
3030
setting.GravatarSource = gravatarSource
31-
err = system_model.Init()
31+
err = system_model.Init(db.DefaultContext)
3232
assert.NoError(t, err)
3333
}
3434

Diff for: ‎models/repo.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ import (
3939
var ItemsPerPage = 40
4040

4141
// Init initialize model
42-
func Init() error {
42+
func Init(ctx context.Context) error {
4343
unit.LoadUnitConfig()
44-
return system_model.Init()
44+
return system_model.Init(ctx)
4545
}
4646

4747
// DeleteRepository deletes a repository for a user or organization.

Diff for: ‎models/system/setting.go

+29-20
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ func IsErrDataExpired(err error) bool {
7979
return ok
8080
}
8181

82-
// GetSettingNoCache returns specific setting without using the cache
83-
func GetSettingNoCache(ctx context.Context, key string) (*Setting, error) {
82+
// GetSetting returns specific setting without using the cache
83+
func GetSetting(ctx context.Context, key string) (*Setting, error) {
8484
v, err := GetSettings(ctx, []string{key})
8585
if err != nil {
8686
return nil, err
@@ -93,11 +93,11 @@ func GetSettingNoCache(ctx context.Context, key string) (*Setting, error) {
9393

9494
const contextCacheKey = "system_setting"
9595

96-
// GetSetting returns the setting value via the key
97-
func GetSetting(ctx context.Context, key string) (string, error) {
96+
// GetSettingWithCache returns the setting value via the key
97+
func GetSettingWithCache(ctx context.Context, key string) (string, error) {
9898
return cache.GetWithContextCache(ctx, contextCacheKey, key, func() (string, error) {
9999
return cache.GetString(genSettingCacheKey(key), func() (string, error) {
100-
res, err := GetSettingNoCache(ctx, key)
100+
res, err := GetSetting(ctx, key)
101101
if err != nil {
102102
return "", err
103103
}
@@ -110,6 +110,15 @@ func GetSetting(ctx context.Context, key string) (string, error) {
110110
// none existing keys and errors are ignored and result in false
111111
func GetSettingBool(ctx context.Context, key string) bool {
112112
s, _ := GetSetting(ctx, key)
113+
if s == nil {
114+
return false
115+
}
116+
v, _ := strconv.ParseBool(s.SettingValue)
117+
return v
118+
}
119+
120+
func GetSettingWithCacheBool(ctx context.Context, key string) bool {
121+
s, _ := GetSettingWithCache(ctx, key)
113122
v, _ := strconv.ParseBool(s)
114123
return v
115124
}
@@ -120,7 +129,7 @@ func GetSettings(ctx context.Context, keys []string) (map[string]*Setting, error
120129
keys[i] = strings.ToLower(keys[i])
121130
}
122131
settings := make([]*Setting, 0, len(keys))
123-
if err := db.GetEngine(db.DefaultContext).
132+
if err := db.GetEngine(ctx).
124133
Where(builder.In("setting_key", keys)).
125134
Find(&settings); err != nil {
126135
return nil, err
@@ -151,9 +160,9 @@ func (settings AllSettings) GetVersion(key string) int {
151160
}
152161

153162
// GetAllSettings returns all settings from user
154-
func GetAllSettings() (AllSettings, error) {
163+
func GetAllSettings(ctx context.Context) (AllSettings, error) {
155164
settings := make([]*Setting, 0, 5)
156-
if err := db.GetEngine(db.DefaultContext).
165+
if err := db.GetEngine(ctx).
157166
Find(&settings); err != nil {
158167
return nil, err
159168
}
@@ -168,12 +177,12 @@ func GetAllSettings() (AllSettings, error) {
168177
func DeleteSetting(ctx context.Context, setting *Setting) error {
169178
cache.RemoveContextData(ctx, contextCacheKey, setting.SettingKey)
170179
cache.Remove(genSettingCacheKey(setting.SettingKey))
171-
_, err := db.GetEngine(db.DefaultContext).Delete(setting)
180+
_, err := db.GetEngine(ctx).Delete(setting)
172181
return err
173182
}
174183

175184
func SetSettingNoVersion(ctx context.Context, key, value string) error {
176-
s, err := GetSettingNoCache(ctx, key)
185+
s, err := GetSetting(ctx, key)
177186
if IsErrSettingIsNotExist(err) {
178187
return SetSetting(ctx, &Setting{
179188
SettingKey: key,
@@ -189,7 +198,7 @@ func SetSettingNoVersion(ctx context.Context, key, value string) error {
189198

190199
// SetSetting updates a users' setting for a specific key
191200
func SetSetting(ctx context.Context, setting *Setting) error {
192-
if err := upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil {
201+
if err := upsertSettingValue(ctx, strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil {
193202
return err
194203
}
195204

@@ -205,8 +214,8 @@ func SetSetting(ctx context.Context, setting *Setting) error {
205214
return nil
206215
}
207216

208-
func upsertSettingValue(key, value string, version int) error {
209-
return db.WithTx(db.DefaultContext, func(ctx context.Context) error {
217+
func upsertSettingValue(parentCtx context.Context, key, value string, version int) error {
218+
return db.WithTx(parentCtx, func(ctx context.Context) error {
210219
e := db.GetEngine(ctx)
211220

212221
// here we use a general method to do a safe upsert for different databases (and most transaction levels)
@@ -249,9 +258,9 @@ var (
249258
LibravatarService *libravatar.Libravatar
250259
)
251260

252-
func Init() error {
261+
func Init(ctx context.Context) error {
253262
var disableGravatar bool
254-
disableGravatarSetting, err := GetSettingNoCache(db.DefaultContext, KeyPictureDisableGravatar)
263+
disableGravatarSetting, err := GetSetting(ctx, KeyPictureDisableGravatar)
255264
if IsErrSettingIsNotExist(err) {
256265
disableGravatar = setting_module.GetDefaultDisableGravatar()
257266
disableGravatarSetting = &Setting{SettingValue: strconv.FormatBool(disableGravatar)}
@@ -262,7 +271,7 @@ func Init() error {
262271
}
263272

264273
var enableFederatedAvatar bool
265-
enableFederatedAvatarSetting, err := GetSettingNoCache(db.DefaultContext, KeyPictureEnableFederatedAvatar)
274+
enableFederatedAvatarSetting, err := GetSetting(ctx, KeyPictureEnableFederatedAvatar)
266275
if IsErrSettingIsNotExist(err) {
267276
enableFederatedAvatar = setting_module.GetDefaultEnableFederatedAvatar(disableGravatar)
268277
enableFederatedAvatarSetting = &Setting{SettingValue: strconv.FormatBool(enableFederatedAvatar)}
@@ -275,13 +284,13 @@ func Init() error {
275284
if setting_module.OfflineMode {
276285
disableGravatar = true
277286
enableFederatedAvatar = false
278-
if !GetSettingBool(db.DefaultContext, KeyPictureDisableGravatar) {
279-
if err := SetSettingNoVersion(db.DefaultContext, KeyPictureDisableGravatar, "true"); err != nil {
287+
if !GetSettingBool(ctx, KeyPictureDisableGravatar) {
288+
if err := SetSettingNoVersion(ctx, KeyPictureDisableGravatar, "true"); err != nil {
280289
return fmt.Errorf("Failed to set setting %q: %w", KeyPictureDisableGravatar, err)
281290
}
282291
}
283-
if GetSettingBool(db.DefaultContext, KeyPictureEnableFederatedAvatar) {
284-
if err := SetSettingNoVersion(db.DefaultContext, KeyPictureEnableFederatedAvatar, "false"); err != nil {
292+
if GetSettingBool(ctx, KeyPictureEnableFederatedAvatar) {
293+
if err := SetSettingNoVersion(ctx, KeyPictureEnableFederatedAvatar, "false"); err != nil {
285294
return fmt.Errorf("Failed to set setting %q: %w", KeyPictureEnableFederatedAvatar, err)
286295
}
287296
}

Diff for: ‎models/system/setting_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,18 @@ func TestSettings(t *testing.T) {
4040

4141
value, err := system.GetSetting(db.DefaultContext, keyName)
4242
assert.NoError(t, err)
43-
assert.EqualValues(t, updatedSetting.SettingValue, value)
43+
assert.EqualValues(t, updatedSetting.SettingValue, value.SettingValue)
4444

4545
// get all settings
46-
settings, err = system.GetAllSettings()
46+
settings, err = system.GetAllSettings(db.DefaultContext)
4747
assert.NoError(t, err)
4848
assert.Len(t, settings, 3)
4949
assert.EqualValues(t, updatedSetting.SettingValue, settings[strings.ToLower(updatedSetting.SettingKey)].SettingValue)
5050

5151
// delete setting
5252
err = system.DeleteSetting(db.DefaultContext, &system.Setting{SettingKey: strings.ToLower(keyName)})
5353
assert.NoError(t, err)
54-
settings, err = system.GetAllSettings()
54+
settings, err = system.GetAllSettings(db.DefaultContext)
5555
assert.NoError(t, err)
5656
assert.Len(t, settings, 2)
5757
}

Diff for: ‎models/unittest/testdb.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func MainTest(m *testing.M, testOpts *TestOptions) {
113113
if err = storage.Init(); err != nil {
114114
fatalTestError("storage.Init: %v\n", err)
115115
}
116-
if err = system_model.Init(); err != nil {
116+
if err = system_model.Init(db.DefaultContext); err != nil {
117117
fatalTestError("models.Init: %v\n", err)
118118
}
119119

Diff for: ‎models/user/avatar.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string {
6767
useLocalAvatar := false
6868
autoGenerateAvatar := false
6969

70-
disableGravatar := system_model.GetSettingBool(ctx, system_model.KeyPictureDisableGravatar)
70+
disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar)
7171

7272
switch {
7373
case u.UseCustomAvatar:

Diff for: ‎modules/repository/commits_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func enableGravatar(t *testing.T) {
106106
err := system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureDisableGravatar, "false")
107107
assert.NoError(t, err)
108108
setting.GravatarSource = "https://secure.gravatar.com/avatar"
109-
err = system_model.Init()
109+
err = system_model.Init(db.DefaultContext)
110110
assert.NoError(t, err)
111111
}
112112

Diff for: ‎modules/templates/helper.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func NewFuncMap() []template.FuncMap {
9292
return setting.AssetVersion
9393
},
9494
"DisableGravatar": func(ctx context.Context) bool {
95-
return system_model.GetSettingBool(ctx, system_model.KeyPictureDisableGravatar)
95+
return system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar)
9696
},
9797
"DefaultShowFullName": func() bool {
9898
return setting.UI.DefaultShowFullName

Diff for: ‎routers/init.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func GlobalInitInstalled(ctx context.Context) {
150150
mustInit(system.Init)
151151
mustInit(oauth2.Init)
152152

153-
mustInit(models.Init)
153+
mustInitCtx(ctx, models.Init)
154154
mustInit(repo_service.Init)
155155

156156
// Booting long running goroutines.

Diff for: ‎routers/web/admin/config.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func Config(ctx *context.Context) {
103103
ctx.Data["PageIsAdmin"] = true
104104
ctx.Data["PageIsAdminConfig"] = true
105105

106-
systemSettings, err := system_model.GetAllSettings()
106+
systemSettings, err := system_model.GetAllSettings(ctx)
107107
if err != nil {
108108
ctx.ServerError("system_model.GetAllSettings", err)
109109
return

0 commit comments

Comments
 (0)