diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a4b964b..823a38a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,7 +17,7 @@ Thanks for your interest in improving `openapi-parser`. When contributing to thi ## Development setup -You need Go 1.26 or later. +You need Go 1.27 or later. ```sh git clone https://github.com/indykite/openapi-parser diff --git a/gen/constrain.go b/gen/constrain.go index 0e7c265..def3906 100644 --- a/gen/constrain.go +++ b/gen/constrain.go @@ -79,22 +79,52 @@ func enumTarget(s *Schema) *Schema { return s } +// isRequiredRule reports whether a validator rule list makes the field itself +// mandatory. Only rules before the first `dive` apply to the field: everything +// after it constrains the elements (or, between `keys` and `endkeys`, the map +// keys), so `omitempty,dive,keys,required,endkeys,required` on a map means +// "if present, every key and value must be non-empty", not "field required". +func isRequiredRule(rules string) bool { + for rule := range strings.SplitSeq(rules, ",") { + switch strings.TrimSpace(rule) { + case "dive": + return false + case "required": + return true + } + } + return false +} + // applyValidationRules maps the widely-used go-playground/validator rules to // schema constraints. `required` is handled by the caller (it belongs to the // parent object); unknown/custom validators are ignored. `dive` redirects the // remaining rules to the element schema, matching the validator's semantics -// (`min=1,dive,min=8` = at least one element, each at least 8 long). +// (`min=1,dive,min=8` = at least one element, each at least 8 long); on a map +// the elements are the values (additionalProperties). Rules between `keys` +// and `endkeys` constrain map keys and are skipped. func applyValidationRules(s *Schema, rules string) { if s == nil || rules == "" { return } + inKeys := false for rule := range strings.SplitSeq(rules, ",") { name, val, _ := strings.Cut(strings.TrimSpace(rule), "=") - if name == "dive" { - if s = s.Items; s == nil { + switch name { + case "dive": + if s = elementSchema(s); s == nil { return } continue + case "keys": + inKeys = true + continue + case "endkeys": + inKeys = false + continue + } + if inKeys { + continue } switch name { case "oneof": @@ -126,6 +156,14 @@ func applyValidationRules(s *Schema, rules string) { } } +// elementSchema is what `dive` descends into: array items, or map values. +func elementSchema(s *Schema) *Schema { + if s.Items != nil { + return s.Items + } + return s.AdditionalProperties +} + type sizeBound bool const ( diff --git a/gen/gen_test.go b/gen/gen_test.go index 9b7ed2e..927e75f 100644 --- a/gen/gen_test.go +++ b/gen/gen_test.go @@ -326,6 +326,20 @@ func TestEscapedTagsUnexportedDiveRequired(t *testing.T) { t.Errorf("dive should put min=8 on items.minLength: %+v", hosts.Items) } + // `required` after dive (and inside keys/endkeys) constrains map keys and + // values, not the field: params stays optional. + if slices.Contains(acct.Required, "params") { + t.Errorf("params must not be required, got %v", acct.Required) + } + params := acct.Properties["params"] + values := params.AdditionalProperties + if values == nil || values.MaxLength == nil || *values.MaxLength != 64 { + t.Errorf("dive on a map should put max=64 on additionalProperties.maxLength: %+v", values) + } + if params.MaxLength != nil || params.MinLength != nil { + t.Errorf("map itself must carry no string constraints: %+v", params) + } + // unexported fields are never marshaled. if _, ok := acct.Properties["hidden"]; ok { t.Error("unexported field must not appear in the schema") diff --git a/gen/internal_test.go b/gen/internal_test.go index e0bc31e..33bb10d 100644 --- a/gen/internal_test.go +++ b/gen/internal_test.go @@ -177,6 +177,31 @@ func TestApplyValidationRulesTable(t *testing.T) { } applyValidationRules(&Schema{Type: []string{"array"}}, "dive,min=1") // nil items: no panic + // dive on a map descends into additionalProperties; keys..endkeys rules + // constrain map keys and must not leak onto the values. + m := &Schema{Type: []string{"object"}, AdditionalProperties: &Schema{Type: []string{"string"}}} + applyValidationRules(m, "omitempty,dive,keys,required,max=3,endkeys,required,max=64") + if m.MaxLength != nil || m.AdditionalProperties.MaxLength == nil || *m.AdditionalProperties.MaxLength != 64 { + t.Errorf("map dive rules: %+v values %+v", m, m.AdditionalProperties) + } + applyValidationRules(&Schema{Type: []string{"object"}}, "dive,keys,required,endkeys,min=1") // nil values: no panic + + // required only counts before dive: after it, it applies to elements. + for rules, want := range map[string]bool{ + "required": true, + "required,dive,min=1": true, + " required , max=3": true, + "omitempty,dive,required": false, + "omitempty,dive,keys,required,endkeys,required": false, + "min=1,dive,required": false, + "": false, + "required_if=Other x": false, + } { + if got := isRequiredRule(rules); got != want { + t.Errorf("isRequiredRule(%q) = %v, want %v", rules, got, want) + } + } + // custom validators must be ignored without panicking ignored := &Schema{Type: []string{"string"}} applyValidationRules(ignored, "required,gid=PROJECT,node_type,omitempty") diff --git a/gen/schema.go b/gen/schema.go index 8f6abc7..893b1c8 100644 --- a/gen/schema.go +++ b/gen/schema.go @@ -323,8 +323,10 @@ func (r *resolver) addField(schema *Schema, field *ast.Field, ctx refCtx, subst schema.Properties[name] = fieldSchema // like swag, `required` wins even over json omitempty - a request field - // can be mandatory while the response marshaler omits empty values - if hasTag(tag, "validate", "required") || hasTag(tag, "binding", "required") { + // can be mandatory while the response marshaler omits empty values. + // Only a top-level `required` counts: one after `dive` (or inside + // keys/endkeys) constrains the elements, not the field. + if isRequiredRule(tagValue(tag, "validate")) || isRequiredRule(tagValue(tag, "binding")) { schema.Required = append(schema.Required, name) // A required pointer field rejects JSON null at validation time, // so drop the null branch the pointer type added. diff --git a/go.mod b/go.mod index a0338a6..c43fd6d 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/indykite/openapi-parser -go 1.26.4 +go 1.27.1 // Stdlib only by design. Parsing uses go/parser + go/ast; emission uses // encoding/json plus a minimal internal YAML encoder (gen/yaml.go). No diff --git a/testdata/sample.go b/testdata/sample.go index 13c2e65..a011ffa 100644 --- a/testdata/sample.go +++ b/testdata/sample.go @@ -71,6 +71,10 @@ type Account struct { Code string `json:"code,omitempty" validate:"required"` // Hosts: validator dive scopes min=8 to each element, not the array. Hosts []string `json:"hosts" binding:"min=1,dive,min=8"` + // Params: optional map whose keys and values, when present, must be + // non-empty. The `required` rules after dive must not make the field + // itself required; max=64 applies to each value. + Params map[string]string `json:"params" binding:"omitempty,dive,keys,required,endkeys,required,max=64"` // Contact is a required pointer: validation rejects null, so no null type. Contact *string `json:"contact" validate:"required"` // Parent account, required: the null branch is dropped, this doc survives.