Skip to content

Commit

Permalink
fix: better coverage, fix a few issues
Browse files Browse the repository at this point in the history
  • Loading branch information
danielgtaylor committed Apr 3, 2024
1 parent fb523a6 commit ae44908
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 30 deletions.
26 changes: 10 additions & 16 deletions openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -1544,9 +1544,10 @@ func downgradeSpec(input any) {
if t == "null" {
// The "null" type is a nullable field in 3.0.
m["nullable"] = true
} else {
// Last non-null wins.
m["type"] = t
}
// Last non-null wins.
m["type"] = t
}
continue
}
Expand Down Expand Up @@ -1596,12 +1597,6 @@ func downgradeSpec(input any) {
continue
}

if k == "contentMediaType" && v == "application/octet-stream" {
delete(m, k)
m["format"] = "binary"
continue
}

downgradeSpec(v)
}
case []any:
Expand All @@ -1619,16 +1614,15 @@ func downgradeSpec(input any) {
// https://www.openapis.org/blog/2021/02/16/migrating-from-openapi-3-0-to-3-1-0
func (o OpenAPI) Downgrade() ([]byte, error) {
b, err := o.MarshalJSON()
if err != nil {
return nil, err
}

var v any
json.Unmarshal(b, &v)
if err == nil {
var v any
json.Unmarshal(b, &v)

downgradeSpec(v)
downgradeSpec(v)

return json.Marshal(v)
b, err = json.Marshal(v)
}
return b, err
}

// DowngradeYAML converts this OpenAPI 3.1 spec to OpenAPI 3.0.3, returning the
Expand Down
97 changes: 97 additions & 0 deletions openapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/danielgtaylor/huma/v2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -168,3 +169,99 @@ x-test: 123

require.Equal(t, expected, string(out))
}

func TestDowngrade(t *testing.T) {
// Test that we can downgrade a v3 OpenAPI document to v2.
v31 := &huma.OpenAPI{
OpenAPI: "3.1.0",
Info: &huma.Info{
Title: "Test API",
Version: "1.0.0",
},
Paths: map[string]*huma.PathItem{
"/test": {
Get: &huma.Operation{
Responses: map[string]*huma.Response{
"200": {
Description: "OK",
Content: map[string]*huma.MediaType{
"application/json": {
Schema: &huma.Schema{
Type: "object",
Properties: map[string]*huma.Schema{
"test": {
Type: "integer",
ExclusiveMinimum: Ptr(0.0),
ExclusiveMaximum: Ptr(100.0),
Nullable: true,
Examples: []any{100},
},
"encoding": {
Type: huma.TypeString,
ContentEncoding: "base64",
},
},
},
},
"application/octet-stream": {},
},
},
},
},
},
},
}

v30, err := v31.Downgrade()
require.NoError(t, err)

expected := `{
"openapi": "3.0.3",
"info": {
"title": "Test API",
"version": "1.0.0"
},
"paths": {
"/test": {
"get": {
"responses": {
"200": {
"description": "OK",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"test": {
"type": "integer",
"nullable": true,
"minimum": 0,
"exclusiveMinimum": true,
"maximum": 100,
"exclusiveMaximum": true,
"example": 100
},
"encoding": {
"type": "string",
"format": "base64"
}
}
}
},
"application/octet-stream": {
"schema": {
"type": "string",
"format": "binary"
}
}
}
}
}
}
}
}
}`

// Check that the downgrade worked as expected.
assert.JSONEq(t, expected, string(v30))
}
12 changes: 4 additions & 8 deletions schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,14 +502,14 @@ func SchemaFromField(registry Registry, f reflect.StructField, hint string) *Sch
}

fs.Nullable = boolTag(f, "nullable")
if fs.Nullable && (fs.Type == TypeArray || fs.Type == TypeObject) {
if fs.Nullable && fs.Ref != "" {
// Nullability is only supported for scalar types for now. Objects are
// much more complicated because the `null` type lives within the object
// definition (requiring multiple copies of the object) or needs to use
// `anyOf` or `not` which is not supported by all code generators, or is
// supported poorly & generates hard-to-use code. This is less than ideal
// but a compromise.
panic(fmt.Errorf("nullable is not supported for field '%s' which is type '%s'", f.Name, fs.Type))
// but a compromise for now to support some nullability built-in.
panic(fmt.Errorf("nullable is not supported for field '%s' which is type '%s'", f.Name, fs.Ref))
}

fs.Minimum = floatTag(f, "minimum")
Expand Down Expand Up @@ -734,11 +734,7 @@ func SchemaFromType(r Registry, t reflect.Type) *Schema {
propNames = append(propNames, name)
if fieldRequired {
required = append(required, name)
if !fs.Nullable {
// In Go we can't easily distinguish if `null` was sent, so no
// need to check for a required nullable field in the validator.
requiredMap[name] = true
}
requiredMap[name] = true
}
}
}
Expand Down
44 changes: 44 additions & 0 deletions schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,41 @@ func TestSchema(t *testing.T) {
}
}`,
},
{
name: "field-nullable",
input: struct {
Int *int64 `json:"int" nullable:"true"`
}{},
expected: `{
"type": "object",
"additionalProperties": false,
"properties": {
"int": {
"type": ["integer", "null"],
"format": "int64"
}
}
}`,
},
{
name: "field-nullable-struct",
input: struct {
Field struct {
_ struct{} `json:"-" nullable:"true"`
Foo string `json:"foo"`
} `json:"field"`
}{},
expected: `{
"type": "object",
"additionalProperties": false,
"properties": {
"field": {
"$ref": "#/components/schemas/FieldStruct"
}
},
"required": ["field"]
}`,
},
{
name: "recursive-embedded-structure",
input: RecursiveChild{},
Expand Down Expand Up @@ -752,6 +787,15 @@ func TestSchema(t *testing.T) {
}{},
panics: `dependent field 'missing1' for field 'value1' does not exist; dependent field 'missing2' for field 'value1' does not exist; dependent field 'missing2' for field 'value2' does not exist`,
},
{
name: "panic-nullable-struct",
input: struct {
Value *struct {
Foo string `json:"foo"`
} `json:"value" nullable:"true"`
}{},
panics: `nullable is not supported for field 'Value' which is type '#/components/schemas/ValueStruct'`,
},
}

for _, c := range cases {
Expand Down
10 changes: 6 additions & 4 deletions validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,8 +576,9 @@ func handleMapString(r Registry, s *Schema, path *PathBuffer, mode ValidateMode,
continue
}

if m[k] == nil && s.requiredMap[k] {
// This is a non-required field which is null, so ignore it.
if m[k] == nil && (!s.requiredMap[k] || s.Nullable) {
// This is a non-required field which is null, or a nullable field set
// to null, so ignore it.
continue
}

Expand Down Expand Up @@ -669,8 +670,9 @@ func handleMapAny(r Registry, s *Schema, path *PathBuffer, mode ValidateMode, m
continue
}

if m[k] == nil && s.requiredMap[k] {
// This is a non-required field which is null, so ignore it.
if m[k] == nil && (!s.requiredMap[k] || s.Nullable) {
// This is a non-required field which is null, or a nullable field set
// to null, so ignore it.
continue
}

Expand Down
18 changes: 16 additions & 2 deletions validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,20 @@ var validateTests = []struct {
}{}),
input: map[string]any{},
},
{
name: "optional null success",
typ: reflect.TypeOf(struct {
Value *string `json:"value,omitempty" minLength:"1"`
}{}),
input: map[string]any{"value": nil},
},
{
name: "optional any null success",
typ: reflect.TypeOf(struct {
Value *string `json:"value,omitempty" minLength:"1"`
}{}),
input: map[any]any{"value": nil},
},
{
name: "optional fail",
typ: reflect.TypeOf(struct {
Expand Down Expand Up @@ -1221,14 +1235,14 @@ var validateTests = []struct {
{
name: "pointer required field success",
typ: reflect.TypeOf(struct {
Field *int `json:"field" required:"true"`
Field *int `json:"field" required:"true" nullable:"true"`
}{}),
input: map[string]any{"field": nil},
},
{
name: "pointer required field fail",
typ: reflect.TypeOf(struct {
Field *int `json:"field" required:"true"`
Field *int `json:"field" required:"true" nullable:"true"`
}{}),
input: map[string]any{},
errs: []string{"expected required property field to be present"},
Expand Down

0 comments on commit ae44908

Please sign in to comment.