From 840717006cc6e09a27a6f91896cefec9d881df96 Mon Sep 17 00:00:00 2001 From: Henrik Ilgen Date: Fri, 12 Apr 2024 13:21:24 +0200 Subject: [PATCH] Removes nil check for `required` properties --- pkg/generator/validator.go | 5 +- tests/data/core/date/date.go | 2 +- tests/data/core/dateTime/dateTime.go | 2 +- tests/data/core/ip/ip.go | 2 +- tests/data/core/object/object.go | 2 +- tests/data/core/time/time.go | 2 +- .../cyclicAndRequired1/cyclicAndRequired1.go | 2 +- tests/data/regressions/issue32/issue32.go | 4 +- tests/data/validation/maxLength/maxLength.go | 2 +- tests/data/validation/minLength/minLength.go | 2 +- .../requiredFields/requiredFields.go | 24 +++---- .../requiredFields/requiredNullable.go | 65 +++++++++++++++++++ .../requiredFields/requiredNullable.json | 41 ++++++++++++ tests/validation_test.go | 40 ++++++++++++ 14 files changed, 172 insertions(+), 23 deletions(-) create mode 100644 tests/data/validation/requiredFields/requiredNullable.go create mode 100644 tests/data/validation/requiredFields/requiredNullable.json diff --git a/pkg/generator/validator.go b/pkg/generator/validator.go index bd8d1fba..e260c01e 100644 --- a/pkg/generator/validator.go +++ b/pkg/generator/validator.go @@ -35,7 +35,10 @@ type requiredValidator struct { } func (v *requiredValidator) generate(out *codegen.Emitter) { - out.Printlnf(`if v, ok := %s["%s"]; !ok || v == nil {`, varNameRawMap, v.jsonName) + // The container itself may be null (if the type is ["null", "object"]), in which case + // the map will be nil and none of the properties are present. This shouldn't fail + // the validation, though, as that's allowed as long as the container is allowed to be null. + out.Printlnf(`if _, ok := %s["%s"]; %s != nil && !ok {`, varNameRawMap, v.jsonName, varNameRawMap) out.Indent(1) out.Printlnf(`return fmt.Errorf("field %s in %s: required")`, v.jsonName, v.declName) out.Indent(-1) diff --git a/tests/data/core/date/date.go b/tests/data/core/date/date.go index ce558d92..dcf31db6 100644 --- a/tests/data/core/date/date.go +++ b/tests/data/core/date/date.go @@ -22,7 +22,7 @@ func (j *DateMyObject) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["myDate"]; !ok || v == nil { + if _, ok := raw["myDate"]; raw != nil && !ok { return fmt.Errorf("field myDate in DateMyObject: required") } type Plain DateMyObject diff --git a/tests/data/core/dateTime/dateTime.go b/tests/data/core/dateTime/dateTime.go index cd2df821..271752ed 100644 --- a/tests/data/core/dateTime/dateTime.go +++ b/tests/data/core/dateTime/dateTime.go @@ -22,7 +22,7 @@ func (j *DateTimeMyObject) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["myDateTime"]; !ok || v == nil { + if _, ok := raw["myDateTime"]; raw != nil && !ok { return fmt.Errorf("field myDateTime in DateTimeMyObject: required") } type Plain DateTimeMyObject diff --git a/tests/data/core/ip/ip.go b/tests/data/core/ip/ip.go index 380bbb00..b8d8ea93 100644 --- a/tests/data/core/ip/ip.go +++ b/tests/data/core/ip/ip.go @@ -22,7 +22,7 @@ func (j *IpMyObject) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["myIp"]; !ok || v == nil { + if _, ok := raw["myIp"]; raw != nil && !ok { return fmt.Errorf("field myIp in IpMyObject: required") } type Plain IpMyObject diff --git a/tests/data/core/object/object.go b/tests/data/core/object/object.go index 76652123..83521b77 100644 --- a/tests/data/core/object/object.go +++ b/tests/data/core/object/object.go @@ -21,7 +21,7 @@ func (j *ObjectMyObject) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["myString"]; !ok || v == nil { + if _, ok := raw["myString"]; raw != nil && !ok { return fmt.Errorf("field myString in ObjectMyObject: required") } type Plain ObjectMyObject diff --git a/tests/data/core/time/time.go b/tests/data/core/time/time.go index 4e4b5457..3182fdfb 100644 --- a/tests/data/core/time/time.go +++ b/tests/data/core/time/time.go @@ -22,7 +22,7 @@ func (j *TimeMyObject) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["myTime"]; !ok || v == nil { + if _, ok := raw["myTime"]; raw != nil && !ok { return fmt.Errorf("field myTime in TimeMyObject: required") } type Plain TimeMyObject diff --git a/tests/data/miscWithDefaults/cyclicAndRequired1/cyclicAndRequired1.go b/tests/data/miscWithDefaults/cyclicAndRequired1/cyclicAndRequired1.go index 2f924da5..403b41dd 100644 --- a/tests/data/miscWithDefaults/cyclicAndRequired1/cyclicAndRequired1.go +++ b/tests/data/miscWithDefaults/cyclicAndRequired1/cyclicAndRequired1.go @@ -26,7 +26,7 @@ func (j *Foo) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["refToBar"]; !ok || v == nil { + if _, ok := raw["refToBar"]; raw != nil && !ok { return fmt.Errorf("field refToBar in Foo: required") } type Plain Foo diff --git a/tests/data/regressions/issue32/issue32.go b/tests/data/regressions/issue32/issue32.go index 6dc056be..7e4618ad 100644 --- a/tests/data/regressions/issue32/issue32.go +++ b/tests/data/regressions/issue32/issue32.go @@ -24,10 +24,10 @@ func (j *TestObject) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["name"]; !ok || v == nil { + if _, ok := raw["name"]; raw != nil && !ok { return fmt.Errorf("field name in TestObject: required") } - if v, ok := raw["owner"]; !ok || v == nil { + if _, ok := raw["owner"]; raw != nil && !ok { return fmt.Errorf("field owner in TestObject: required") } type Plain TestObject diff --git a/tests/data/validation/maxLength/maxLength.go b/tests/data/validation/maxLength/maxLength.go index 92cf1241..c4ea0324 100644 --- a/tests/data/validation/maxLength/maxLength.go +++ b/tests/data/validation/maxLength/maxLength.go @@ -19,7 +19,7 @@ func (j *MaxLength) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["myString"]; !ok || v == nil { + if _, ok := raw["myString"]; raw != nil && !ok { return fmt.Errorf("field myString in MaxLength: required") } type Plain MaxLength diff --git a/tests/data/validation/minLength/minLength.go b/tests/data/validation/minLength/minLength.go index 25020340..3f3f7395 100644 --- a/tests/data/validation/minLength/minLength.go +++ b/tests/data/validation/minLength/minLength.go @@ -19,7 +19,7 @@ func (j *MinLength) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["myString"]; !ok || v == nil { + if _, ok := raw["myString"]; raw != nil && !ok { return fmt.Errorf("field myString in MinLength: required") } type Plain MinLength diff --git a/tests/data/validation/requiredFields/requiredFields.go b/tests/data/validation/requiredFields/requiredFields.go index 5d340b0b..534ac4c2 100644 --- a/tests/data/validation/requiredFields/requiredFields.go +++ b/tests/data/validation/requiredFields/requiredFields.go @@ -61,7 +61,7 @@ func (j *RequiredFieldsMyObjectArrayElem) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["myNestedObjectString"]; !ok || v == nil { + if _, ok := raw["myNestedObjectString"]; raw != nil && !ok { return fmt.Errorf("field myNestedObjectString in RequiredFieldsMyObjectArrayElem: required") } type Plain RequiredFieldsMyObjectArrayElem @@ -79,7 +79,7 @@ func (j *RequiredFieldsMyObject) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["myNestedObjectString"]; !ok || v == nil { + if _, ok := raw["myNestedObjectString"]; raw != nil && !ok { return fmt.Errorf("field myNestedObjectString in RequiredFieldsMyObject: required") } type Plain RequiredFieldsMyObject @@ -97,34 +97,34 @@ func (j *RequiredFields) UnmarshalJSON(b []byte) error { if err := json.Unmarshal(b, &raw); err != nil { return err } - if v, ok := raw["myBoolean"]; !ok || v == nil { + if _, ok := raw["myBoolean"]; raw != nil && !ok { return fmt.Errorf("field myBoolean in RequiredFields: required") } - if v, ok := raw["myBooleanArray"]; !ok || v == nil { + if _, ok := raw["myBooleanArray"]; raw != nil && !ok { return fmt.Errorf("field myBooleanArray in RequiredFields: required") } - if v, ok := raw["myNull"]; !ok || v == nil { + if _, ok := raw["myNull"]; raw != nil && !ok { return fmt.Errorf("field myNull in RequiredFields: required") } - if v, ok := raw["myNullArray"]; !ok || v == nil { + if _, ok := raw["myNullArray"]; raw != nil && !ok { return fmt.Errorf("field myNullArray in RequiredFields: required") } - if v, ok := raw["myNumber"]; !ok || v == nil { + if _, ok := raw["myNumber"]; raw != nil && !ok { return fmt.Errorf("field myNumber in RequiredFields: required") } - if v, ok := raw["myNumberArray"]; !ok || v == nil { + if _, ok := raw["myNumberArray"]; raw != nil && !ok { return fmt.Errorf("field myNumberArray in RequiredFields: required") } - if v, ok := raw["myObject"]; !ok || v == nil { + if _, ok := raw["myObject"]; raw != nil && !ok { return fmt.Errorf("field myObject in RequiredFields: required") } - if v, ok := raw["myObjectArray"]; !ok || v == nil { + if _, ok := raw["myObjectArray"]; raw != nil && !ok { return fmt.Errorf("field myObjectArray in RequiredFields: required") } - if v, ok := raw["myString"]; !ok || v == nil { + if _, ok := raw["myString"]; raw != nil && !ok { return fmt.Errorf("field myString in RequiredFields: required") } - if v, ok := raw["myStringArray"]; !ok || v == nil { + if _, ok := raw["myStringArray"]; raw != nil && !ok { return fmt.Errorf("field myStringArray in RequiredFields: required") } type Plain RequiredFields diff --git a/tests/data/validation/requiredFields/requiredNullable.go b/tests/data/validation/requiredFields/requiredNullable.go new file mode 100644 index 00000000..f82acba1 --- /dev/null +++ b/tests/data/validation/requiredFields/requiredNullable.go @@ -0,0 +1,65 @@ +// Code generated by github.com/atombender/go-jsonschema, DO NOT EDIT. + +package test + +import "encoding/json" +import "fmt" + +type RequiredNullable struct { + // MyNullableObject corresponds to the JSON schema field "myNullableObject". + MyNullableObject RequiredNullableMyNullableObject `json:"myNullableObject" yaml:"myNullableObject" mapstructure:"myNullableObject"` + + // MyNullableString corresponds to the JSON schema field "myNullableString". + MyNullableString *string `json:"myNullableString" yaml:"myNullableString" mapstructure:"myNullableString"` + + // MyNullableStringArray corresponds to the JSON schema field + // "myNullableStringArray". + MyNullableStringArray []string `json:"myNullableStringArray" yaml:"myNullableStringArray" mapstructure:"myNullableStringArray"` +} + +type RequiredNullableMyNullableObject struct { + // MyNestedProp corresponds to the JSON schema field "myNestedProp". + MyNestedProp string `json:"myNestedProp" yaml:"myNestedProp" mapstructure:"myNestedProp"` +} + +// UnmarshalJSON implements json.Unmarshaler. +func (j *RequiredNullableMyNullableObject) UnmarshalJSON(b []byte) error { + var raw map[string]interface{} + if err := json.Unmarshal(b, &raw); err != nil { + return err + } + if _, ok := raw["myNestedProp"]; raw != nil && !ok { + return fmt.Errorf("field myNestedProp in RequiredNullableMyNullableObject: required") + } + type Plain RequiredNullableMyNullableObject + var plain Plain + if err := json.Unmarshal(b, &plain); err != nil { + return err + } + *j = RequiredNullableMyNullableObject(plain) + return nil +} + +// UnmarshalJSON implements json.Unmarshaler. +func (j *RequiredNullable) UnmarshalJSON(b []byte) error { + var raw map[string]interface{} + if err := json.Unmarshal(b, &raw); err != nil { + return err + } + if _, ok := raw["myNullableObject"]; raw != nil && !ok { + return fmt.Errorf("field myNullableObject in RequiredNullable: required") + } + if _, ok := raw["myNullableString"]; raw != nil && !ok { + return fmt.Errorf("field myNullableString in RequiredNullable: required") + } + if _, ok := raw["myNullableStringArray"]; raw != nil && !ok { + return fmt.Errorf("field myNullableStringArray in RequiredNullable: required") + } + type Plain RequiredNullable + var plain Plain + if err := json.Unmarshal(b, &plain); err != nil { + return err + } + *j = RequiredNullable(plain) + return nil +} diff --git a/tests/data/validation/requiredFields/requiredNullable.json b/tests/data/validation/requiredFields/requiredNullable.json new file mode 100644 index 00000000..e3edb93f --- /dev/null +++ b/tests/data/validation/requiredFields/requiredNullable.json @@ -0,0 +1,41 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "id": "https://example.com/requiredNullable", + "type": "object", + "properties": { + "myNullableString": { + "type": [ + "string", + "null" + ] + }, + "myNullableStringArray": { + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + } + }, + "myNullableObject": { + "type": [ + "object", + "null" + ], + "properties": { + "myNestedProp": { + "type": "string" + } + }, + "required": [ + "myNestedProp" + ] + } + }, + "required": [ + "myNullableString", + "myNullableStringArray", + "myNullableObject" + ] +} \ No newline at end of file diff --git a/tests/validation_test.go b/tests/validation_test.go index d89b4f1a..d9b34e70 100644 --- a/tests/validation_test.go +++ b/tests/validation_test.go @@ -7,6 +7,7 @@ import ( testMaxLength "github.com/atombender/go-jsonschema/tests/data/validation/maxLength" testMinLength "github.com/atombender/go-jsonschema/tests/data/validation/minLength" + testRequiredFields "github.com/atombender/go-jsonschema/tests/data/validation/requiredFields" "github.com/atombender/go-jsonschema/tests/helpers" ) @@ -112,3 +113,42 @@ func TestMinStringLength(t *testing.T) { }) } } + +func TestRequiredFields(t *testing.T) { + t.Parallel() + + testCases := []struct { + desc string + data string + wantErr error + }{ + { + desc: "object without required property fails validation", + data: `{}`, + wantErr: errors.New("field myNullableObject in RequiredNullable: required"), + }, + { + desc: "required properties may be null", + data: `{ "myNullableObject": null, "myNullableStringArray": null, "myNullableString": null }`, + wantErr: nil, + }, + { + desc: "required properties may have a non-null value", + data: `{ "myNullableObject": { "myNestedProp": "世界" }, "myNullableStringArray": ["hello"], "myNullableString": "world" }`, + wantErr: nil, + }, + } + for _, tC := range testCases { + tC := tC + + t.Run(tC.desc, func(t *testing.T) { + t.Parallel() + + model := testRequiredFields.RequiredNullable{} + + err := json.Unmarshal([]byte(tC.data), &model) + + helpers.CheckError(t, tC.wantErr, err) + }) + } +}