Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow extended config on cron settings #12939

Merged
merged 3 commits into from
Sep 25, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions modules/setting/cron.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,26 @@

package setting

import "reflect"

// GetCronSettings maps the cron subsection to the provided config
func GetCronSettings(name string, config interface{}) (interface{}, error) {
err := Cfg.Section("cron." + name).MapTo(config)
return config, err
if err := Cfg.Section("cron." + name).MapTo(config); err != nil {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have 0 experience in Go, but it was hard for me to read this line. I did some research and potentially this may help to avoid such init statements in if with return inside

https://golang.org/doc/effective_go.html#if

In the Go libraries, you'll find that when an if statement doesn't flow into the next statement—that is, the body ends in break, continue, goto, or return—the unnecessary else is omitted.

f, err := os.Open(name)
if err != nil {
    return err
}
codeUsing(f)

More to read
https://www.calhoun.io/one-liner-if-statements-with-errors/

return config, err
}

typ := reflect.TypeOf(config).Elem()
val := reflect.ValueOf(config).Elem()

for i := 0; i < typ.NumField(); i++ {
field := val.Field(i)
tpField := typ.Field(i)
if tpField.Type.Kind() == reflect.Struct && tpField.Anonymous {
if err := Cfg.Section("cron." + name).MapTo(field.Addr().Interface()); err != nil {
return config, err
}
}
}

return config, nil
}
43 changes: 43 additions & 0 deletions modules/setting/cron_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package setting

lafriks marked this conversation as resolved.
Show resolved Hide resolved
import (
"testing"

"github.com/stretchr/testify/assert"
ini "gopkg.in/ini.v1"
)

func Test_GetCronSettings(t *testing.T) {

type BaseStruct struct {
Base bool
Second string
}

type Extended struct {
BaseStruct
Extend bool
}

iniStr := `
[cron.test]
Base = true
Second = white rabbit
Extend = true
`
Cfg, _ = ini.Load([]byte(iniStr))

extended := &Extended{
BaseStruct: BaseStruct{
Second: "queen of hearts",
},
}

_, err := GetCronSettings("test", extended)

assert.NoError(t, err)
assert.True(t, extended.Base)
assert.EqualValues(t, extended.Second, "white rabbit")
assert.True(t, extended.Extend)

}