fix: panic on oneof validation (#86)

* test for illustration * add error on empty oneoff * add required check for validation * use custom validator only for nilable case
contiamo · Dec 6, 2022 · 317299d · 317299d
1 parent 3ef0880
commit 317299d
Show file tree

Hide file tree

Showing 15 changed files with 455 additions and 0 deletions.
diff --git a/pkg/generators/models/models.go b/pkg/generators/models/models.go
@@ -99,6 +99,8 @@ type PropSpec struct {
 	IsRequired bool
 	// IsEnum is true when the property is a enum item
 	IsEnum bool
+	// IsOneOf is true when the property is a `oneof` schema
+	IsOneOf bool
 
 	// Validation stuff
 	Pattern                    string
@@ -507,6 +509,7 @@ func structPropsFromRef(ref *openapi3.SchemaRef) (specs []PropSpec, imports []st
 			IsRequired:  checkIfRequired(name, ref.Value.Required),
 			IsEnum:      len(prop.Value.Enum) > 0,
 			IsNullable:  prop.Value.Nullable,
+			IsOneOf:     prop.Value.OneOf != nil && len(prop.Value.OneOf) > 0,
 		}
 
 		if spec.GoType == "time.Time" || spec.GoType == "*time.Time" {

diff --git a/pkg/generators/models/templates/model.gotpl b/pkg/generators/models/templates/model.gotpl
@@ -65,6 +65,10 @@ func (m {{$modelName}}) Validate() error {
 					{{- if .IsIPv6 }}is.IPv6,{{ end }}
 					{{- if .IsIP }}is.IP,{{ end }}
 					{{- if .Pattern }}validation.Match({{ $modelName | firstLower }}{{.Name}}Pattern){{if .PatternErrorMsg}}.ErrorObject({{ $modelName}}{{.Name}}PatternError){{end}},{{ end }}
+					{{- if and .IsOneOf (not .IsRequiredInValidation) }}
+					{{.GoType}}NilableRule{},
+					validation.Skip,
+					{{- end }}
 				),
 			{{- end }}
 		{{- end }}

diff --git a/pkg/generators/models/templates/oneof.gotpl b/pkg/generators/models/templates/oneof.gotpl
@@ -47,6 +47,22 @@ type {{$modelName}} struct {
 	{{- end }}
 }
 
+{{- if .Discriminator.Field }}
+var Empty{{$modelName}}Error = fmt.Errorf("empty data is not an {{$modelName}}")
+var Not{{$modelName}}Error = fmt.Errorf("could not convert to type {{$modelName}}")
+
+type {{$modelName}}NilableRule struct{}
+
+func (nn {{$modelName}}NilableRule) Validate(v interface{}) error {
+	if m, ok := v.({{$modelName}}); !ok {
+		return Not{{$modelName}}Error
+	} else if m.data == nil {
+		return nil
+	} else {
+		return m.Validate()
+	}
+}
+{{- end }}
 
 // MarshalJSON implements the json.Marshaller interface
 func (m {{$modelName}}) MarshalJSON() ([]byte, error) {
@@ -102,12 +118,22 @@ func (m *{{$modelName}}) From{{$convert.TargetGoType | typeDisplayName}}(data {{
 
 // As converts {{$modelName}} to a user defined structure.
 func (m {{$modelName}}) As(target interface{}) error {
+	{{- if .Discriminator.Field }}
+	if m.data == nil {
+		return Empty{{$modelName}}Error
+	}
+	{{- end }}
 	return mapstructure.Decode(m.data, target)
 }
 
 {{- range $convert := .ConvertSpecs }}
 // As{{firstUpper $convert.TargetGoType}} converts {{$modelName}} to a {{$convert.TargetGoType}}
 func (m {{$modelName}}) As{{$convert.TargetGoType | typeDisplayName }}() (result {{$convert.TargetGoType}}, err error) {
+	{{- if $.Discriminator.Field }}
+	if m.data == nil {
+		return result, Empty{{$modelName}}Error
+	}
+	{{- end }}
 	return result, mapstructure.Decode(m.data, &result)
 }
 {{- end}}
@@ -121,6 +147,11 @@ func (m {{$modelName}}) Discriminator() {{$modelName}}Discriminator {
 
 // Validate implements basic validation for this model
 func (m {{$modelName}}) Validate() error {
+	{{- if .Discriminator.Field }}
+	if m.data == nil {
+		return Empty{{$modelName}}Error
+	}
+	{{- end }}
 	discriminator := m.data.{{$modelName}}Discriminator()
 	switch discriminator {
 	{{- range $value, $type := .Discriminator.Map }}

diff --git a/pkg/generators/models/testdata/cases/nested_inline_arrays/expected/model_geometry.go b/pkg/generators/models/testdata/cases/nested_inline_arrays/expected/model_geometry.go
@@ -34,6 +34,21 @@ type Geometry struct {
 	data geometryer
 }
 
+var EmptyGeometryError = fmt.Errorf("empty data is not an Geometry")
+var NotGeometryError = fmt.Errorf("could not convert to type Geometry")
+
+type GeometryNilableRule struct{}
+
+func (nn GeometryNilableRule) Validate(v interface{}) error {
+	if m, ok := v.(Geometry); !ok {
+		return NotGeometryError
+	} else if m.data == nil {
+		return nil
+	} else {
+		return m.Validate()
+	}
+}
+
 // MarshalJSON implements the json.Marshaller interface
 func (m Geometry) MarshalJSON() ([]byte, error) {
 	return json.Marshal(m.data)
@@ -91,16 +106,25 @@ func (m *Geometry) FromShape(data Shape) {
 
 // As converts Geometry to a user defined structure.
 func (m Geometry) As(target interface{}) error {
+	if m.data == nil {
+		return EmptyGeometryError
+	}
 	return mapstructure.Decode(m.data, target)
 }
 
 // AsLine converts Geometry to a Line
 func (m Geometry) AsLine() (result Line, err error) {
+	if m.data == nil {
+		return result, EmptyGeometryError
+	}
 	return result, mapstructure.Decode(m.data, &result)
 }
 
 // AsShape converts Geometry to a Shape
 func (m Geometry) AsShape() (result Shape, err error) {
+	if m.data == nil {
+		return result, EmptyGeometryError
+	}
 	return result, mapstructure.Decode(m.data, &result)
 }
 
@@ -111,6 +135,9 @@ func (m Geometry) Discriminator() GeometryDiscriminator {
 
 // Validate implements basic validation for this model
 func (m Geometry) Validate() error {
+	if m.data == nil {
+		return EmptyGeometryError
+	}
 	discriminator := m.data.GeometryDiscriminator()
 	switch discriminator {
 	case GeometryDiscriminatorLine:

diff --git a/pkg/generators/models/testdata/cases/nested_inline_arrays/generated/model_geometry.go b/pkg/generators/models/testdata/cases/nested_inline_arrays/generated/model_geometry.go
@@ -34,6 +34,21 @@ type Geometry struct {
 	data geometryer
 }
 
+var EmptyGeometryError = fmt.Errorf("empty data is not an Geometry")
+var NotGeometryError = fmt.Errorf("could not convert to type Geometry")
+
+type GeometryNilableRule struct{}
+
+func (nn GeometryNilableRule) Validate(v interface{}) error {
+	if m, ok := v.(Geometry); !ok {
+		return NotGeometryError
+	} else if m.data == nil {
+		return nil
+	} else {
+		return m.Validate()
+	}
+}
+
 // MarshalJSON implements the json.Marshaller interface
 func (m Geometry) MarshalJSON() ([]byte, error) {
 	return json.Marshal(m.data)
@@ -91,16 +106,25 @@ func (m *Geometry) FromShape(data Shape) {
 
 // As converts Geometry to a user defined structure.
 func (m Geometry) As(target interface{}) error {
+	if m.data == nil {
+		return EmptyGeometryError
+	}
 	return mapstructure.Decode(m.data, target)
 }
 
 // AsLine converts Geometry to a Line
 func (m Geometry) AsLine() (result Line, err error) {
+	if m.data == nil {
+		return result, EmptyGeometryError
+	}
 	return result, mapstructure.Decode(m.data, &result)
 }
 
 // AsShape converts Geometry to a Shape
 func (m Geometry) AsShape() (result Shape, err error) {
+	if m.data == nil {
+		return result, EmptyGeometryError
+	}
 	return result, mapstructure.Decode(m.data, &result)
 }
 
@@ -111,6 +135,9 @@ func (m Geometry) Discriminator() GeometryDiscriminator {
 
 // Validate implements basic validation for this model
 func (m Geometry) Validate() error {
+	if m.data == nil {
+		return EmptyGeometryError
+	}
 	discriminator := m.data.GeometryDiscriminator()
 	switch discriminator {
 	case GeometryDiscriminatorLine:

diff --git a/pkg/generators/models/testdata/cases/oneof_mapped_discriminator/api.yaml b/pkg/generators/models/testdata/cases/oneof_mapped_discriminator/api.yaml
@@ -90,3 +90,17 @@ components:
           field: '#/components/schemas/FieldError'
           external: '#/components/schemas/ExternalError'
           auth: '#/components/schemas/GenericError'
+
+    Container:
+      type: object
+      properties:
+        error:
+          $ref: "#/components/schemas/Error"
+
+    NonEmptyContainer:
+      type: object
+      properties:
+        error:
+          $ref: "#/components/schemas/Error"
+      required:
+        - error
diff --git a/...rs/models/testdata/cases/oneof_mapped_discriminator/expected/container_validation_test.go b/...rs/models/testdata/cases/oneof_mapped_discriminator/expected/container_validation_test.go
@@ -0,0 +1,81 @@
+package generatortest
+
+import (
+	"encoding/json"
+	"testing"
+
+	validation "github.com/go-ozzo/ozzo-validation/v4"
+	"github.com/stretchr/testify/require"
+)
+
+func TestContainerValidation(t *testing.T) {
+	t.Run("validation succeeds for missing property", func(t *testing.T) {
+		rawBytes := `{}`
+		var container Container
+
+		err := json.Unmarshal([]byte(rawBytes), &container)
+		require.NoError(t, err)
+
+		err = container.Validate()
+		require.NoError(t, err)
+	})
+
+	t.Run("validation succeeds for valid property", func(t *testing.T) {
+		rawBytes := `{"error":{"kind": "external", "service": "", "traceId": "12345"}}`
+		var container Container
+
+		err := json.Unmarshal([]byte(rawBytes), &container)
+		require.NoError(t, err)
+
+		err = container.Validate()
+		require.NoError(t, err)
+	})
+
+	t.Run("validation fails for invalid property", func(t *testing.T) {
+		rawBytes := `{"error":{"kind": "external", "service": "", "traceId": ""}}`
+		var container Container
+
+		err := json.Unmarshal([]byte(rawBytes), &container)
+		require.NoError(t, err)
+
+		err = container.Validate()
+		require.EqualError(t, err, "error: (traceId: cannot be blank.).")
+	})
+}
+
+func TestNonEmptyContainerValidation(t *testing.T) {
+	t.Run("validation fails for empty payload", func(t *testing.T) {
+		rawBytes := `{}`
+		var container NonEmptyContainer
+
+		err := json.Unmarshal([]byte(rawBytes), &container)
+		require.NoError(t, err)
+
+		err = container.Validate()
+		require.Equal(t, err, validation.Errors{
+			"error": EmptyErrorError,
+		})
+	})
+
+	t.Run("validation succeeds for valid property", func(t *testing.T) {
+		rawBytes := `{"error":{"kind": "external", "service": "", "traceId": "12345"}}`
+		var container NonEmptyContainer
+
+		err := json.Unmarshal([]byte(rawBytes), &container)
+		require.NoError(t, err)
+
+		err = container.Validate()
+		require.NoError(t, err)
+	})
+
+	t.Run("validation succeeds for valid property", func(t *testing.T) {
+		rawBytes := `{"error":{"kind": "external", "service": "", "traceId": ""}}`
+		var container NonEmptyContainer
+
+		err := json.Unmarshal([]byte(rawBytes), &container)
+		require.NoError(t, err)
+
+		err = container.Validate()
+		require.EqualError(t, err, "error: (traceId: cannot be blank.).")
+	})
+}
diff --git a/pkg/generators/models/testdata/cases/oneof_mapped_discriminator/expected/model_container.go b/pkg/generators/models/testdata/cases/oneof_mapped_discriminator/expected/model_container.go
@@ -0,0 +1,38 @@
+// This file is auto-generated, DO NOT EDIT.
+//
+// Source:
+//
+//	Title: Test oneOf discriminator support
+//	Version: 0.1.0
+package generatortest
+
+import (
+	validation "github.com/go-ozzo/ozzo-validation/v4"
+)
+
+// Container is an object.
+type Container struct {
+	// Error:
+	Error Error `json:"error,omitempty"`
+}
+
+// Validate implements basic validation for this model
+func (m Container) Validate() error {
+	return validation.Errors{
+		"error": validation.Validate(
+			m.Error,
+			ErrorNilableRule{},
+			validation.Skip,
+		),
+	}.Filter()
+}
+
+// GetError returns the Error property
+func (m Container) GetError() Error {
+	return m.Error
+}
+
+// SetError sets the Error property
+func (m *Container) SetError(val Error) {
+	m.Error = val
+}