Fix pointer receivers handling in unstructured converter

This commit is contained in:
Mikhail Mazurskiy 2017-09-08 21:28:42 +10:00
parent eda3db550b
commit a672b98cc6
No known key found for this signature in database
GPG Key ID: 93551ECC96E2F568
4 changed files with 123 additions and 37 deletions

View File

@ -3,17 +3,6 @@ package(default_visibility = ["//visibility:public"])
load( load(
"@io_bazel_rules_go//go:def.bzl", "@io_bazel_rules_go//go:def.bzl",
"go_library", "go_library",
"go_test",
)
go_test(
name = "go_default_test",
srcs = ["converter_test.go"],
library = ":go_default_library",
deps = [
"//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library",
],
) )
go_library( go_library(
@ -40,6 +29,9 @@ filegroup(
filegroup( filegroup(
name = "all-srcs", name = "all-srcs",
srcs = [":package-srcs"], srcs = [
":package-srcs",
"//staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/testing:all-srcs",
],
tags = ["automanaged"], tags = ["automanaged"],
) )

View File

@ -426,17 +426,29 @@ var (
falseBytes = []byte("false") falseBytes = []byte("false")
) )
func toUnstructured(sv, dv reflect.Value) error { func getMarshaler(v reflect.Value) (encodingjson.Marshaler, bool) {
st, dt := sv.Type(), dv.Type() // Check value receivers if v is not a pointer and pointer receivers if v is a pointer
if v.Type().Implements(marshalerType) {
return v.Interface().(encodingjson.Marshaler), true
}
// Check pointer receivers if v is not a pointer
if v.Kind() != reflect.Ptr && v.CanAddr() {
v = v.Addr()
if v.Type().Implements(marshalerType) {
return v.Interface().(encodingjson.Marshaler), true
}
}
return nil, false
}
func toUnstructured(sv, dv reflect.Value) error {
// Check if the object has a custom JSON marshaller/unmarshaller. // Check if the object has a custom JSON marshaller/unmarshaller.
if st.Implements(marshalerType) { if marshaler, ok := getMarshaler(sv); ok {
if sv.Kind() == reflect.Ptr && sv.IsNil() { if sv.Kind() == reflect.Ptr && sv.IsNil() {
// We're done - we don't need to store anything. // We're done - we don't need to store anything.
return nil return nil
} }
marshaler := sv.Interface().(encodingjson.Marshaler)
data, err := marshaler.MarshalJSON() data, err := marshaler.MarshalJSON()
if err != nil { if err != nil {
return err return err
@ -496,6 +508,7 @@ func toUnstructured(sv, dv reflect.Value) error {
return nil return nil
} }
st, dt := sv.Type(), dv.Type()
switch st.Kind() { switch st.Kind() {
case reflect.String: case reflect.String:
if dt.Kind() == reflect.Interface && dv.NumMethod() == 0 { if dt.Kind() == reflect.Interface && dv.NumMethod() == 0 {

View File

@ -0,0 +1,28 @@
load("@io_bazel_rules_go//go:def.bzl", "go_test")
go_test(
name = "go_default_test",
srcs = ["converter_test.go"],
deps = [
"//vendor/github.com/stretchr/testify/assert:go_default_library",
"//vendor/github.com/stretchr/testify/require:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/conversion/unstructured:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library",
],
)
filegroup(
name = "package-srcs",
srcs = glob(["**"]),
tags = ["automanaged"],
visibility = ["//visibility:private"],
)
filegroup(
name = "all-srcs",
srcs = [":package-srcs"],
tags = ["automanaged"],
visibility = ["//visibility:public"],
)

View File

@ -14,15 +14,25 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
package unstructured // These tests are in a separate package to break cyclic dependency in tests.
// Unstructured type depends on unstructured converter package but we want to test how the converter handles
// the Unstructured type so we need to import both.
package testing
import ( import (
"fmt" "fmt"
"reflect" "reflect"
"strconv"
"testing" "testing"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
conversionunstructured "k8s.io/apimachinery/pkg/conversion/unstructured"
"k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/json"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
) )
// Definte a number of test types. // Definte a number of test types.
@ -72,14 +82,27 @@ type F struct {
} }
type G struct { type G struct {
Custom Custom `json:"custom"` CustomValue1 CustomValue `json:"customValue1"`
CustomValue2 *CustomValue `json:"customValue2"`
CustomPointer1 CustomPointer `json:"customPointer1"`
CustomPointer2 *CustomPointer `json:"customPointer2"`
} }
type Custom struct { type CustomValue struct {
data []byte data []byte
} }
func (c Custom) MarshalJSON() ([]byte, error) { // MarshalJSON has a value receiver on this type.
func (c CustomValue) MarshalJSON() ([]byte, error) {
return c.data, nil
}
type CustomPointer struct {
data []byte
}
// MarshalJSON has a pointer receiver on this type.
func (c *CustomPointer) MarshalJSON() ([]byte, error) {
return c.data, nil return c.data, nil
} }
@ -113,14 +136,14 @@ func doRoundTrip(t *testing.T, item interface{}) {
return return
} }
newUnstr, err := DefaultConverter.ToUnstructured(item) newUnstr, err := conversionunstructured.DefaultConverter.ToUnstructured(item)
if err != nil { if err != nil {
t.Errorf("ToUnstructured failed: %v", err) t.Errorf("ToUnstructured failed: %v", err)
return return
} }
newObj := reflect.New(reflect.TypeOf(item).Elem()).Interface() newObj := reflect.New(reflect.TypeOf(item).Elem()).Interface()
err = DefaultConverter.FromUnstructured(newUnstr, newObj) err = conversionunstructured.DefaultConverter.FromUnstructured(newUnstr, newObj)
if err != nil { if err != nil {
t.Errorf("FromUnstructured failed: %v", err) t.Errorf("FromUnstructured failed: %v", err)
return return
@ -136,6 +159,17 @@ func TestRoundTrip(t *testing.T) {
testCases := []struct { testCases := []struct {
obj interface{} obj interface{}
}{ }{
{
obj: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
"kind": "Foo",
"metadata": map[string]interface{}{
"name": "foo1",
},
},
},
},
{ {
// This (among others) tests nil map, slice and pointer. // This (among others) tests nil map, slice and pointer.
obj: &C{ obj: &C{
@ -230,7 +264,7 @@ func doUnrecognized(t *testing.T, jsonData string, item interface{}, expectedErr
return return
} }
newObj := reflect.New(reflect.TypeOf(item).Elem()).Interface() newObj := reflect.New(reflect.TypeOf(item).Elem()).Interface()
err = DefaultConverter.FromUnstructured(unstr, newObj) err = conversionunstructured.DefaultConverter.FromUnstructured(unstr, newObj)
if (err != nil) != (expectedErr != nil) { if (err != nil) != (expectedErr != nil) {
t.Errorf("Unexpected error in FromUnstructured: %v, expected: %v", err, expectedErr) t.Errorf("Unexpected error in FromUnstructured: %v, expected: %v", err, expectedErr)
} }
@ -434,7 +468,7 @@ func TestFloatIntConversion(t *testing.T) {
unstr := map[string]interface{}{"fd": float64(3)} unstr := map[string]interface{}{"fd": float64(3)}
var obj F var obj F
if err := DefaultConverter.FromUnstructured(unstr, &obj); err != nil { if err := conversionunstructured.DefaultConverter.FromUnstructured(unstr, &obj); err != nil {
t.Errorf("Unexpected error in FromUnstructured: %v", err) t.Errorf("Unexpected error in FromUnstructured: %v", err)
} }
@ -469,18 +503,37 @@ func TestCustomToUnstructured(t *testing.T) {
} }
for _, tc := range testcases { for _, tc := range testcases {
result, err := DefaultConverter.ToUnstructured(&G{Custom: Custom{data: []byte(tc.Data)}}) tc := tc
if err != nil { t.Run(tc.Data, func(t *testing.T) {
t.Errorf("%s: %v", tc.Data, err) t.Parallel()
continue result, err := conversionunstructured.DefaultConverter.ToUnstructured(&G{
} CustomValue1: CustomValue{data: []byte(tc.Data)},
CustomValue2: &CustomValue{data: []byte(tc.Data)},
fieldResult := result["custom"] CustomPointer1: CustomPointer{data: []byte(tc.Data)},
if !reflect.DeepEqual(fieldResult, tc.Expected) { CustomPointer2: &CustomPointer{data: []byte(tc.Data)},
t.Errorf("%s: expected %v, got %v", tc.Data, tc.Expected, fieldResult) })
// t.Log("expected", spew.Sdump(tc.Expected)) require.NoError(t, err)
// t.Log("actual", spew.Sdump(fieldResult)) for field, fieldResult := range result {
continue assert.Equal(t, tc.Expected, fieldResult, field)
} }
})
}
}
func TestCustomToUnstructuredTopLevel(t *testing.T) {
// Only objects are supported at the top level
topLevelCases := []interface{}{
&CustomValue{data: []byte(`{"a":1}`)},
&CustomPointer{data: []byte(`{"a":1}`)},
}
expected := map[string]interface{}{"a": int64(1)}
for i, obj := range topLevelCases {
obj := obj
t.Run(strconv.Itoa(i), func(t *testing.T) {
t.Parallel()
result, err := conversionunstructured.DefaultConverter.ToUnstructured(obj)
require.NoError(t, err)
assert.Equal(t, expected, result)
})
} }
} }