diff --git a/config/config_test.go b/config/config_test.go index 97cd8679..acacf5d0 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -3,6 +3,7 @@ package config import ( "fmt" "reflect" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -63,6 +64,7 @@ func TestLoad(t *testing.T) { AuthorizedValues: []any{}, Type: reflect.String, IsSlice: false, + Required: true, } assert.Equal(t, expected, cfg.config["app"].(object)["name"]) }) @@ -106,6 +108,7 @@ func TestLoad(t *testing.T) { AuthorizedValues: []any{}, Type: reflect.String, IsSlice: false, + Required: true, } assert.Equal(t, expected, cfg.config["app"].(object)["name"]) }) @@ -130,6 +133,7 @@ func TestLoad(t *testing.T) { AuthorizedValues: []any{}, Type: reflect.String, IsSlice: false, + Required: true, } assert.Equal(t, expected, cfg.config["app"].(object)["name"]) }) @@ -670,6 +674,7 @@ func TestConfig(t *testing.T) { AuthorizedValues: []any{}, Type: reflect.String, IsSlice: false, + Required: true, } loader.register("app.name", entry) assert.NotSame(t, &entry, loader.defaults["app"].(object)["name"]) @@ -681,6 +686,7 @@ func TestConfig(t *testing.T) { AuthorizedValues: []any{"a", "b"}, Type: reflect.String, IsSlice: true, + Required: true, }) }) @@ -691,6 +697,7 @@ func TestConfig(t *testing.T) { AuthorizedValues: []any{}, Type: reflect.Int, IsSlice: false, + Required: true, }) }) @@ -701,7 +708,81 @@ func TestConfig(t *testing.T) { AuthorizedValues: []any{}, Type: reflect.String, IsSlice: false, + Required: true, }) }) }) } + +func TestRequiredConfig(t *testing.T) { + loader := loader{ + defaults: make(object), + } + + loader.register("testCategory.nullValueNotRequired", Entry{ + Value: nil, + AuthorizedValues: []any{}, + Type: reflect.String, + Required: false, + }) + + loader.register("testCategory.nullValueRequired", Entry{ + Value: nil, + AuthorizedValues: []any{}, + Type: reflect.String, + Required: true, + }) + + loader.register("testCategory.valueCompletelyMissing", Entry{ + Type: reflect.String, + Required: true, + }) + + loader.register("testCategory.valueValidAndDefined", Entry{ + Type: reflect.Int, + Required: true, + }) + + var nilPointer *string + loader.register("testCategory.nilPointerRequired", Entry{ + Value: nilPointer, + Type: reflect.Ptr, + Required: true, + }) + + validPointer := new(string) + *validPointer = "valid" + loader.register("testCategory.validPointer", Entry{ + Value: validPointer, + Type: reflect.Ptr, + Required: true, + }) + + cfgJSON := `{ + "testCategory": { + "nullValueNotRequired": null, + "nullValueRequired": null, + "valueValidAndDefined": 123, + "valueValidAndDefined": 123, + "validPointer": "valid" + } + }` + + _, err := loader.loadJSON(cfgJSON) + require.Error(t, err) + + expectedErrors := []string{ + "- \"testCategory.valueCompletelyMissing\" is required", + "- \"testCategory.nullValueRequired\" is required", + "- \"testCategory.nilPointerRequired\" is required", + } + + actualErrors := strings.Split(err.Error(), "\n") + for i, line := range actualErrors { + actualErrors[i] = strings.TrimSpace(line) + } + + for _, expectedMessage := range expectedErrors { + assert.Contains(t, actualErrors, expectedMessage) + } +} diff --git a/config/default.go b/config/default.go index 3fa548e5..ec40b549 100644 --- a/config/default.go +++ b/config/default.go @@ -4,49 +4,49 @@ import "reflect" var configDefaults = object{ "app": object{ - "name": &Entry{"goyave", []any{}, reflect.String, false}, - "environment": &Entry{"localhost", []any{}, reflect.String, false}, - "debug": &Entry{true, []any{}, reflect.Bool, false}, - "defaultLanguage": &Entry{"en-US", []any{}, reflect.String, false}, + "name": &Entry{"goyave", []any{}, reflect.String, false, true}, + "environment": &Entry{"localhost", []any{}, reflect.String, false, true}, + "debug": &Entry{true, []any{}, reflect.Bool, false, true}, + "defaultLanguage": &Entry{"en-US", []any{}, reflect.String, false, true}, }, "server": object{ - "host": &Entry{"127.0.0.1", []any{}, reflect.String, false}, - "domain": &Entry{"", []any{}, reflect.String, false}, - "port": &Entry{8080, []any{}, reflect.Int, false}, - "writeTimeout": &Entry{10, []any{}, reflect.Int, false}, - "readTimeout": &Entry{10, []any{}, reflect.Int, false}, - "readHeaderTimeout": &Entry{10, []any{}, reflect.Int, false}, - "idleTimeout": &Entry{20, []any{}, reflect.Int, false}, - "websocketCloseTimeout": &Entry{10, []any{}, reflect.Int, false}, - "maxUploadSize": &Entry{10.0, []any{}, reflect.Float64, false}, + "host": &Entry{"127.0.0.1", []any{}, reflect.String, false, true}, + "domain": &Entry{"", []any{}, reflect.String, false, true}, + "port": &Entry{8080, []any{}, reflect.Int, false, true}, + "writeTimeout": &Entry{10, []any{}, reflect.Int, false, true}, + "readTimeout": &Entry{10, []any{}, reflect.Int, false, true}, + "readHeaderTimeout": &Entry{10, []any{}, reflect.Int, false, true}, + "idleTimeout": &Entry{20, []any{}, reflect.Int, false, true}, + "websocketCloseTimeout": &Entry{10, []any{}, reflect.Int, false, true}, + "maxUploadSize": &Entry{10.0, []any{}, reflect.Float64, false, true}, "proxy": object{ - "protocol": &Entry{"http", []any{"http", "https"}, reflect.String, false}, - "host": &Entry{nil, []any{}, reflect.String, false}, - "port": &Entry{80, []any{}, reflect.Int, false}, - "base": &Entry{"", []any{}, reflect.String, false}, + "protocol": &Entry{"http", []any{"http", "https"}, reflect.String, false, true}, + "host": &Entry{nil, []any{}, reflect.String, false, false}, + "port": &Entry{80, []any{}, reflect.Int, false, true}, + "base": &Entry{"", []any{}, reflect.String, false, true}, }, }, "database": object{ - "connection": &Entry{"none", []any{}, reflect.String, false}, - "host": &Entry{"127.0.0.1", []any{}, reflect.String, false}, - "port": &Entry{0, []any{}, reflect.Int, false}, - "name": &Entry{"", []any{}, reflect.String, false}, - "username": &Entry{"", []any{}, reflect.String, false}, - "password": &Entry{"", []any{}, reflect.String, false}, - "options": &Entry{"", []any{}, reflect.String, false}, - "maxOpenConnections": &Entry{20, []any{}, reflect.Int, false}, - "maxIdleConnections": &Entry{20, []any{}, reflect.Int, false}, - "maxLifetime": &Entry{300, []any{}, reflect.Int, false}, - "defaultReadQueryTimeout": &Entry{20000, []any{}, reflect.Int, false}, - "defaultWriteQueryTimeout": &Entry{40000, []any{}, reflect.Int, false}, + "connection": &Entry{"none", []any{}, reflect.String, false, true}, + "host": &Entry{"127.0.0.1", []any{}, reflect.String, false, true}, + "port": &Entry{0, []any{}, reflect.Int, false, true}, + "name": &Entry{"", []any{}, reflect.String, false, true}, + "username": &Entry{"", []any{}, reflect.String, false, true}, + "password": &Entry{"", []any{}, reflect.String, false, true}, + "options": &Entry{"", []any{}, reflect.String, false, true}, + "maxOpenConnections": &Entry{20, []any{}, reflect.Int, false, true}, + "maxIdleConnections": &Entry{20, []any{}, reflect.Int, false, true}, + "maxLifetime": &Entry{300, []any{}, reflect.Int, false, true}, + "defaultReadQueryTimeout": &Entry{20000, []any{}, reflect.Int, false, true}, + "defaultWriteQueryTimeout": &Entry{40000, []any{}, reflect.Int, false, true}, "config": object{ - "skipDefaultTransaction": &Entry{false, []any{}, reflect.Bool, false}, - "dryRun": &Entry{false, []any{}, reflect.Bool, false}, - "prepareStmt": &Entry{true, []any{}, reflect.Bool, false}, - "disableNestedTransaction": &Entry{false, []any{}, reflect.Bool, false}, - "allowGlobalUpdate": &Entry{false, []any{}, reflect.Bool, false}, - "disableAutomaticPing": &Entry{false, []any{}, reflect.Bool, false}, - "disableForeignKeyConstraintWhenMigrating": &Entry{false, []any{}, reflect.Bool, false}, + "skipDefaultTransaction": &Entry{false, []any{}, reflect.Bool, false, true}, + "dryRun": &Entry{false, []any{}, reflect.Bool, false, true}, + "prepareStmt": &Entry{true, []any{}, reflect.Bool, false, true}, + "disableNestedTransaction": &Entry{false, []any{}, reflect.Bool, false, true}, + "allowGlobalUpdate": &Entry{false, []any{}, reflect.Bool, false, true}, + "disableAutomaticPing": &Entry{false, []any{}, reflect.Bool, false, true}, + "disableForeignKeyConstraintWhenMigrating": &Entry{false, []any{}, reflect.Bool, false, true}, }, }, } @@ -70,7 +70,7 @@ func loadDefaults(src object, dst object) { } value = slice.Interface() } - dst[k] = &Entry{value, entry.AuthorizedValues, entry.Type, entry.IsSlice} + dst[k] = &Entry{value, entry.AuthorizedValues, entry.Type, entry.IsSlice, entry.Required} } } } diff --git a/config/entry.go b/config/entry.go index 844c18dc..c4ad1c29 100644 --- a/config/entry.go +++ b/config/entry.go @@ -19,6 +19,7 @@ type Entry struct { AuthorizedValues []any // Leave empty for "any" Type reflect.Kind IsSlice bool + Required bool } func makeEntryFromValue(value any) *Entry { @@ -29,18 +30,23 @@ func makeEntryFromValue(value any) *Entry { kind = t.Elem().Kind() isSlice = true } - return &Entry{value, []any{}, kind, isSlice} + return &Entry{value, []any{}, kind, isSlice, false} } func (e *Entry) validate(key string) error { - if e.Value == nil { // nil values means unset - return nil - } - if err := e.tryEnvVarConversion(key); err != nil { return err } + + v := reflect.ValueOf(e.Value) + if e.Required && (!v.IsValid() || e.Value == nil || (v.Kind() == reflect.Pointer && v.IsNil())) { + return errors.Errorf("%q is required", key) + } + t := reflect.TypeOf(e.Value) + if t == nil { + return nil // Can't determine type, is 'zero' value. + } kind := t.Kind() if e.IsSlice && kind == reflect.Slice { kind = t.Elem().Kind() @@ -60,10 +66,9 @@ func (e *Entry) validate(key string) error { if e.IsSlice { // Accepted values for slices define the values that can be used inside the slice // It doesn't represent the value of the slice itself (content and order) - list := reflect.ValueOf(e.Value) - length := list.Len() + length := v.Len() for i := 0; i < length; i++ { - if !lo.Contains(e.AuthorizedValues, list.Index(i).Interface()) { + if !lo.Contains(e.AuthorizedValues, v.Index(i).Interface()) { return errors.Errorf("%q elements must have one of the following values: %v", key, e.AuthorizedValues) } }