Repository: getkin/kin-openapi
Pull request #148 (2019-11-28 02:36:46)

Fix issue with x-* fields Unmarshal
Addressed https://github.com/getkin/kin-openapi/issues/147

fenollp: Could you add a test for this? Could you also return on non-nil `err` before mutating `props`?
sayboras: > Could you add a test for this? Could you also return on non-nil `err` before mutating `props`?

Thanks for your feedback. Let me make an update :)
sayboras: @fenollp I have added unit test cases, which I think is required.

Patch:
diff --git a/openapi3/extension.go b/openapi3/extension.go
index 5a545f7..f6b7ef9 100644
--- a/openapi3/extension.go
+++ b/openapi3/extension.go
@@ -25,13 +25,14 @@ func (props *ExtensionProps) EncodeWith(encoder *jsoninfo.ObjectEncoder, value i
 
 // DecodeWith will be invoked by package "jsoninfo"
 func (props *ExtensionProps) DecodeWith(decoder *jsoninfo.ObjectDecoder, value interface{}) error {
+	if err := decoder.DecodeStructFieldsAndExtensions(value); err != nil {
+		return err
+	}
 	source := decoder.DecodeExtensionMap()
-	if len(source) > 0 {
-		result := make(map[string]interface{}, len(source))
-		for k, v := range source {
-			result[k] = v
-		}
-		props.Extensions = result
+	result := make(map[string]interface{}, len(source))
+	for k, v := range source {
+		result[k] = v
 	}
-	return decoder.DecodeStructFieldsAndExtensions(value)
+	props.Extensions = result
+	return nil
 }


Test patch:
diff --git a/jsoninfo/unmarshal_test.go b/jsoninfo/unmarshal_test.go
new file mode 100644
index 0000000..85f199e
--- /dev/null
+++ b/jsoninfo/unmarshal_test.go
@@ -0,0 +1,156 @@
+package jsoninfo_test
+
+import (
+	"fmt"
+	"github.com/getkin/kin-openapi/jsoninfo"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestNewObjectDecoder(t *testing.T) {
+	data := []byte(`
+	{
+		"field1": 1,
+		"field2": 2
+	}
+`)
+	t.Run("test new object decoder", func(t *testing.T) {
+		decoder, err := jsoninfo.NewObjectDecoder(data)
+		assert.Nil(t, err)
+		assert.NotNil(t, decoder)
+		assert.Equal(t, data, decoder.Data)
+		assert.Equal(t, 2, len(decoder.DecodeExtensionMap()))
+	})
+}
+
+type mockStrictStruct struct {
+	EncodeWithFn func(encoder *jsoninfo.ObjectEncoder, value interface{}) error
+	DecodeWithFn func(decoder *jsoninfo.ObjectDecoder, value interface{}) error
+}
+
+func (m *mockStrictStruct) EncodeWith(encoder *jsoninfo.ObjectEncoder, value interface{}) error {
+	return m.EncodeWithFn(encoder, value)
+}
+
+func (m *mockStrictStruct) DecodeWith(decoder *jsoninfo.ObjectDecoder, value interface{}) error {
+	return m.DecodeWithFn(decoder, value)
+}
+
+func TestUnmarshalStrictStruct(t *testing.T) {
+	data := []byte(`
+			{
+				"field1": 1,
+				"field2": 2
+			}
+		`)
+
+	t.Run("test unmarshal with StrictStruct without err", func(t *testing.T) {
+		decodeWithFnCalled := 0
+		mockStruct := &mockStrictStruct{
+			EncodeWithFn: func(encoder *jsoninfo.ObjectEncoder, value interface{}) error {
+				return nil
+			},
+			DecodeWithFn: func(decoder *jsoninfo.ObjectDecoder, value interface{}) error {
+				decodeWithFnCalled++
+				return nil
+			},
+		}
+		err := jsoninfo.UnmarshalStrictStruct(data, mockStruct)
+		assert.Nil(t, err)
+		assert.Equal(t, 1, decodeWithFnCalled)
+	})
+
+	t.Run("test unmarshal with StrictStruct with err", func(t *testing.T) {
+		decodeWithFnCalled := 0
+		mockStruct := &mockStrictStruct{
+			EncodeWithFn: func(encoder *jsoninfo.ObjectEncoder, value interface{}) error {
+				return nil
+			},
+			DecodeWithFn: func(decoder *jsoninfo.ObjectDecoder, value interface{}) error {
+				decodeWithFnCalled++
+				return fmt.Errorf("unable to decode the value")
+			},
+		}
+		err := jsoninfo.UnmarshalStrictStruct(data, mockStruct)
+		assert.NotNil(t, err)
+		assert.Equal(t, 1, decodeWithFnCalled)
+	})
+}
+
+func TestDecodeStructFieldsAndExtensions(t *testing.T) {
+	data := []byte(`
+	{
+		"field1": "field1",
+		"field2": "field2"
+	}
+`)
+	decoder, err := jsoninfo.NewObjectDecoder(data)
+	assert.Nil(t, err)
+	assert.NotNil(t, decoder)
+
+	t.Run("value is not pointer", func(t *testing.T) {
+		var value interface{}
+		assert.Panics(t, func() {
+			_ = decoder.DecodeStructFieldsAndExtensions(value)
+		}, "value is not a pointer")
+	})
+
+	t.Run("value is nil", func(t *testing.T) {
+		var value *string = nil
+		assert.Panics(t, func() {
+			_ = decoder.DecodeStructFieldsAndExtensions(value)
+		}, "value is nil")
+	})
+
+	t.Run("value is not struct", func(t *testing.T) {
+		var value = "simple string"
+		assert.Panics(t, func() {
+			_ = decoder.DecodeStructFieldsAndExtensions(&value)
+		}, "value is not struct")
+	})
+
+	t.Run("successfully decoded with all fields", func(t *testing.T) {
+		d, err := jsoninfo.NewObjectDecoder(data)
+		assert.Nil(t, err)
+		assert.NotNil(t, d)
+
+		var value = struct {
+			Field1 string `json:"field1"`
+			Field2 string `json:"field2"`
+		}{}
+		err = d.DecodeStructFieldsAndExtensions(&value)
+		assert.Nil(t, err)
+		assert.Equal(t, "field1", value.Field1)
+		assert.Equal(t, "field2", value.Field2)
+		assert.Equal(t, 0, len(d.DecodeExtensionMap()))
+	})
+
+	t.Run("successfully decoded with renaming field", func(t *testing.T) {
+		d, err := jsoninfo.NewObjectDecoder(data)
+		assert.Nil(t, err)
+		assert.NotNil(t, d)
+
+		var value = struct {
+			Field1 string `json:"field1"`
+		}{}
+		err = d.DecodeStructFieldsAndExtensions(&value)
+		assert.Nil(t, err)
+		assert.Equal(t, "field1", value.Field1)
+		assert.Equal(t, 1, len(d.DecodeExtensionMap()))
+	})
+
+	t.Run("un-successfully decoded due to data mismatch", func(t *testing.T) {
+		d, err := jsoninfo.NewObjectDecoder(data)
+		assert.Nil(t, err)
+		assert.NotNil(t, d)
+
+		var value = struct {
+			Field1 int `json:"field1"`
+		}{}
+		err = d.DecodeStructFieldsAndExtensions(&value)
+		assert.NotNil(t, err)
+		assert.EqualError(t, err, "Error while unmarshalling property 'field1' (*int): json: cannot unmarshal string into Go value of type int")
+		assert.Equal(t, 0, value.Field1)
+		assert.Equal(t, 2, len(d.DecodeExtensionMap()))
+	})
+}
diff --git a/openapi3/extension_test.go b/openapi3/extension_test.go
new file mode 100644
index 0000000..ba2b688
--- /dev/null
+++ b/openapi3/extension_test.go
@@ -0,0 +1,100 @@
+package openapi3_test
+
+import (
+	"github.com/getkin/kin-openapi/jsoninfo"
+	"github.com/getkin/kin-openapi/openapi3"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestExtensionProps_EncodeWith(t *testing.T) {
+	t.Run("successfully encoded", func(t *testing.T) {
+		encoder := jsoninfo.NewObjectEncoder()
+		var extensionProps = openapi3.ExtensionProps{
+			Extensions: map[string]interface{}{
+				"field1": "value1",
+			},
+		}
+
+		var value = struct {
+			Field1 string `json:"field1"`
+			Field2 string `json:"field2"`
+		}{}
+
+		err := extensionProps.EncodeWith(encoder, &value)
+		assert.Nil(t, err)
+	})
+}
+
+func TestExtensionProps_DecodeWith(t *testing.T) {
+	data := []byte(`
+	{
+		"field1": "value1",
+		"field2": "value2"
+	}
+`)
+	t.Run("successfully decode all the fields", func(t *testing.T) {
+		decoder, err := jsoninfo.NewObjectDecoder(data)
+		assert.Nil(t, err)
+		var extensionProps = &openapi3.ExtensionProps{
+			Extensions: map[string]interface{}{
+				"field1": "value1",
+				"field2": "value1",
+			},
+		}
+
+		var value = struct {
+			Field1 string `json:"field1"`
+			Field2 string `json:"field2"`
+		}{}
+
+		err = extensionProps.DecodeWith(decoder, &value)
+		assert.Nil(t, err)
+		assert.Equal(t, 0, len(extensionProps.Extensions))
+		assert.Equal(t, "value1", value.Field1)
+		assert.Equal(t, "value2", value.Field2)
+	})
+
+	t.Run("successfully decode some of the fields", func(t *testing.T) {
+		decoder, err := jsoninfo.NewObjectDecoder(data)
+		assert.Nil(t, err)
+		var extensionProps = &openapi3.ExtensionProps{
+			Extensions: map[string]interface{}{
+				"field1": "value1",
+				"field2": "value2",
+			},
+		}
+
+		var value = &struct {
+			Field1 string `json:"field1"`
+		}{}
+
+		err = extensionProps.DecodeWith(decoder, value)
+		assert.Nil(t, err)
+		assert.Equal(t, 1, len(extensionProps.Extensions))
+		assert.Equal(t, "value1", value.Field1)
+	})
+
+	t.Run("successfully decode none of the fields", func(t *testing.T) {
+		decoder, err := jsoninfo.NewObjectDecoder(data)
+		assert.Nil(t, err)
+
+		var extensionProps = &openapi3.ExtensionProps{
+			Extensions: map[string]interface{}{
+				"field1": "value1",
+				"field2": "value2",
+			},
+		}
+
+		var value = struct {
+			Field3 string `json:"field3"`
+			Field4 string `json:"field4"`
+		}{}
+
+		err = extensionProps.DecodeWith(decoder, &value)
+		assert.Nil(t, err)
+		assert.Equal(t, 2, len(extensionProps.Extensions))
+		assert.Empty(t,  value.Field3)
+		assert.Empty(t,  value.Field4)
+	})
+}