-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add configuration validation after it is loaded (#175)
* Remove unused previous iteration of configuration validation * Add configuration validation after it is loaded * Expand comment in Validate function * Add tests for validation function * Fix false positive validation constraint for Required Together * Show the option as required in the CLI description * Add missing helper function to create Configuration structs * Panic if the user attempts to make required a boolean field * Mark flags as required if they are
- Loading branch information
Showing
7 changed files
with
257 additions
and
68 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| package field | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "reflect" | ||
| "strings" | ||
|
|
||
| "github.com/spf13/viper" | ||
| ) | ||
|
|
||
| type ErrConfigurationMissingFields struct { | ||
| errors []error | ||
| } | ||
|
|
||
| func (e *ErrConfigurationMissingFields) Error() string { | ||
| var messages []string | ||
|
|
||
| for _, err := range e.errors { | ||
| messages = append(messages, err.Error()) | ||
| } | ||
|
|
||
| return fmt.Sprintf("errors found:\n%s", strings.Join(messages, "\n")) | ||
| } | ||
|
|
||
| func (e *ErrConfigurationMissingFields) Push(err error) { | ||
| e.errors = append(e.errors, err) | ||
| } | ||
|
|
||
| // Validate perform validation of field requirement and constraints | ||
| // relationships after the configuration is read. | ||
| // We don't check the following: | ||
| // - if required fields are mutually exclusive | ||
| // - repeated fields (by name) are defined | ||
| // - if sets of fields are mutually exclusive and required | ||
| // together at the same time | ||
| func Validate(c Configuration, v *viper.Viper) error { | ||
| present := make(map[string]int) | ||
| missingFieldsError := &ErrConfigurationMissingFields{} | ||
|
|
||
| // check if required fields are present | ||
| for _, f := range c.Fields { | ||
| isNonZero := false | ||
| switch f.FieldType { | ||
| case reflect.Bool: | ||
| isNonZero = v.GetBool(f.FieldName) | ||
| case reflect.Int: | ||
| isNonZero = v.GetInt(f.FieldName) != 0 | ||
| case reflect.String: | ||
| isNonZero = v.GetString(f.FieldName) != "" | ||
| default: | ||
| return fmt.Errorf("field %s has unsupported type %s", f.FieldName, f.FieldType) | ||
| } | ||
|
|
||
| if isNonZero { | ||
| present[f.FieldName] = 1 | ||
| } | ||
|
|
||
| if f.Required && !isNonZero { | ||
| missingFieldsError.Push(fmt.Errorf("field %s of type %s is marked as required but it has a zero-value", f.FieldName, f.FieldType)) | ||
| } | ||
| } | ||
|
|
||
| if len(missingFieldsError.errors) > 0 { | ||
| return missingFieldsError | ||
| } | ||
|
|
||
| // check constraints | ||
| return validateConstraints(present, c.Constraints) | ||
| } | ||
|
|
||
| func validateConstraints(fieldsPresent map[string]int, relationships []SchemaFieldRelationship) error { | ||
| for _, relationship := range relationships { | ||
| var present int | ||
| for _, f := range relationship.Fields { | ||
| present += fieldsPresent[f.FieldName] | ||
| } | ||
| if present > 1 && relationship.Kind == MutuallyExclusive { | ||
| return makeMutuallyExclusiveError(fieldsPresent, relationship) | ||
| } | ||
| if present > 0 && present < len(relationship.Fields) && relationship.Kind == RequiredTogether { | ||
| return makeNeededTogetherError(fieldsPresent, relationship) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func makeMutuallyExclusiveError(fields map[string]int, relation SchemaFieldRelationship) error { | ||
| var found []string | ||
| for _, f := range relation.Fields { | ||
| if fields[f.FieldName] == 1 { | ||
| found = append(found, f.FieldName) | ||
| } | ||
| } | ||
|
|
||
| return fmt.Errorf("fields marked as mutually exclusive were set: %s", strings.Join(found, ", ")) | ||
| } | ||
|
|
||
| func makeNeededTogetherError(fields map[string]int, relation SchemaFieldRelationship) error { | ||
| var found []string | ||
| for _, f := range relation.Fields { | ||
| if fields[f.FieldName] == 0 { | ||
| found = append(found, f.FieldName) | ||
| } | ||
| } | ||
|
|
||
| return fmt.Errorf("fields marked as needed together are missing: %s", strings.Join(found, ", ")) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package field | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/spf13/viper" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestValidateRequiredFieldsNotFound(t *testing.T) { | ||
| carrier := Configuration{ | ||
| Fields: []SchemaField{ | ||
| StringField("foo", WithRequired(true)), | ||
| StringField("bar", WithRequired(false)), | ||
| }, | ||
| } | ||
|
|
||
| // create configuration using viper | ||
| v := viper.New() | ||
| v.Set("foo", "") | ||
| v.Set("bar", "") | ||
|
|
||
| err := Validate(carrier, v) | ||
| require.Error(t, err) | ||
| require.EqualError(t, err, "errors found:\nfield foo of type string is marked as required but it has a zero-value") | ||
| } | ||
|
|
||
| func TestValidateRelationshipMutuallyExclusiveAllPresent(t *testing.T) { | ||
| foo := StringField("foo") | ||
| bar := StringField("bar") | ||
|
|
||
| carrier := Configuration{ | ||
| Fields: []SchemaField{ | ||
| foo, | ||
| bar, | ||
| }, | ||
| Constraints: []SchemaFieldRelationship{ | ||
| FieldsMutuallyExclusive(foo, bar), | ||
| }, | ||
| } | ||
|
|
||
| // create configuration using viper | ||
| v := viper.New() | ||
| v.Set("foo", "hello") | ||
| v.Set("bar", "world") | ||
|
|
||
| err := Validate(carrier, v) | ||
| require.Error(t, err) | ||
| require.EqualError(t, err, "fields marked as mutually exclusive were set: foo, bar") | ||
| } | ||
|
|
||
| func TestValidationRequiredTogetherOneMissing(t *testing.T) { | ||
| foo := StringField("foo") | ||
| bar := StringField("bar") | ||
|
|
||
| carrier := Configuration{ | ||
| Fields: []SchemaField{ | ||
| foo, | ||
| bar, | ||
| }, | ||
| Constraints: []SchemaFieldRelationship{ | ||
| FieldsRequiredTogether(foo, bar), | ||
| }, | ||
| } | ||
|
|
||
| // create configuration using viper | ||
| v := viper.New() | ||
| v.Set("foo", "hello") | ||
| v.Set("bar", "") | ||
|
|
||
| err := Validate(carrier, v) | ||
| require.Error(t, err) | ||
| require.EqualError(t, err, "fields marked as needed together are missing: bar") | ||
| } | ||
|
|
||
| func TestValidationRequiredTogetherAllMissing(t *testing.T) { | ||
| foo := StringField("foo") | ||
| bar := StringField("bar") | ||
|
|
||
| carrier := Configuration{ | ||
| Fields: []SchemaField{ | ||
| foo, | ||
| bar, | ||
| }, | ||
| Constraints: []SchemaFieldRelationship{ | ||
| FieldsRequiredTogether(foo, bar), | ||
| }, | ||
| } | ||
|
|
||
| // create configuration using viper | ||
| v := viper.New() | ||
| v.Set("foo", "") | ||
| v.Set("bar", "") | ||
|
|
||
| err := Validate(carrier, v) | ||
| require.NoError(t, err) | ||
| } |