Merge pull request #429 from alexzielenski/implicit-struct-defaults

account for types with custom unmarshalling during default enfocement

pkg/generators/openapi.go

@@ -594,11 +594,43 @@ func defaultFromComments(comments []string, commentPath string, t *types.Type) (
 	return i, nil, nil
 }
 
+func implementsCustomUnmarshalling(t *types.Type) bool {
+	switch t.Kind {
+	case types.Pointer:
+		unmarshaller, isUnmarshaller := t.Elem.Methods["UnmarshalJSON"]
+		return isUnmarshaller && unmarshaller.Signature.Receiver.Kind == types.Pointer
+	case types.Struct:
+		_, isUnmarshaller := t.Methods["UnmarshalJSON"]
+		return isUnmarshaller
+	default:
+		return false
+	}
+}
+
 func mustEnforceDefault(t *types.Type, omitEmpty bool) (interface{}, error) {
+	// Treat types with custom unmarshalling as a value
+	// (Can be alias, struct, or pointer)
+	if implementsCustomUnmarshalling(t) {
+		// Since Go JSON deserializer always feeds `null` when present
+		// to structs with custom UnmarshalJSON, the zero value for
+		// these structs is also null.
+		//
+		// In general, Kubernetes API types with custom marshalling should
+		// marshal their empty values to `null`.
+		return nil, nil
+	}
+
 	switch t.Kind {
+	case types.Alias:
+		return mustEnforceDefault(t.Underlying, omitEmpty)
 	case types.Pointer, types.Map, types.Slice, types.Array, types.Interface:
 		return nil, nil
 	case types.Struct:
+		if len(t.Members) == 1 && t.Members[0].Embedded {
+			// Treat a struct with a single embedded member the same as an alias
+			return mustEnforceDefault(t.Members[0].Type, omitEmpty)
+		}
+
 		return map[string]interface{}{}, nil
 	case types.Builtin:
 		if !omitEmpty {
@@ -619,7 +651,7 @@ func (g openAPITypeWriter) generateDefault(comments []string, t *types.Type, omi
 	if err != nil {
 		return err
 	}
-	if enforced, err := mustEnforceDefault(resolveAliasAndEmbeddedType(t), omitEmpty); err != nil {
+	if enforced, err := mustEnforceDefault(t, omitEmpty); err != nil {
 		return err
 	} else if enforced != nil {
 		if def == nil {
@@ -751,22 +783,6 @@ func (g openAPITypeWriter) generateReferenceProperty(t *types.Type) {
 	g.Do("Ref: ref(\"$.$\"),\n", t.Name.String())
 }
 
-func resolveAliasAndEmbeddedType(t *types.Type) *types.Type {
-	var prev *types.Type
-	for prev != t {
-		prev = t
-		if t.Kind == types.Alias {
-			t = t.Underlying
-		}
-		if t.Kind == types.Struct {
-			if len(t.Members) == 1 && t.Members[0].Embedded {
-				t = t.Members[0].Type
-			}
-		}
-	}
-	return t
-}
-
 func resolveAliasAndPtrType(t *types.Type) *types.Type {
 	var prev *types.Type
 	for prev != t {

pkg/generators/openapi_test.go

@@ -1785,3 +1785,118 @@ type Blah struct {
 	}
 
 }
+
+// Show that types with unmarshalJSON in their hierarchy do not have struct
+// defaults enforced, and that aliases and embededd types are respected
+func TestMustEnforceDefaultStruct(t *testing.T) {
+	callErr, funcErr, assert, _, funcBuffer, imports := testOpenAPITypeWriter(t, `
+package foo
+
+type Time struct {
+	value interface{}
+}
+
+
+type TimeWithoutUnmarshal struct {
+	value interface{}
+}
+
+func (_ TimeWithoutUnmarshal) OpenAPISchemaType() []string { return []string{"string"} }
+func (_ TimeWithoutUnmarshal) OpenAPISchemaFormat() string { return "date-time" }
+
+func (_ Time) UnmarshalJSON([]byte) error {
+	return nil
+}
+
+
+func (_ Time) OpenAPISchemaType() []string { return []string{"string"} }
+func (_ Time) OpenAPISchemaFormat() string { return "date-time" }
+
+// Time with UnmarshalJSON defined on pointer instead of struct
+type MicroTime struct {
+	value interface{}
+}
+
+func (t *MicroTime) UnmarshalJSON([]byte) error {
+	return nil
+}
+
+func (_ MicroTime) OpenAPISchemaType() []string { return []string{"string"} }
+func (_ MicroTime) OpenAPISchemaFormat() string { return "date-time" }
+
+type Int64 int64
+
+type Duration struct {
+	Int64
+}
+
+func (_ Duration) OpenAPISchemaType() []string { return []string{"string"} }
+func (_ Duration) OpenAPISchemaFormat() string { return "" }
+
+type NothingSpecial struct {
+	Field string
+}
+
+// +k8s:openapi-gen=true
+type Blah struct {
+	Embedded Duration
+	PointerUnmarshal MicroTime
+	StructUnmarshal Time
+	NoUnmarshal TimeWithoutUnmarshal
+	Regular NothingSpecial
+}
+	`)
+	assert.NoError(funcErr)
+	assert.NoError(callErr)
+	assert.ElementsMatch(imports, []string{`foo "base/foo"`, `common "k8s.io/kube-openapi/pkg/common"`, `spec "k8s.io/kube-openapi/pkg/validation/spec"`})
+
+	if formatted, err := format.Source(funcBuffer.Bytes()); err != nil {
+		t.Fatal(err)
+	} else {
+		assert.Equal(string(formatted), `func schema_base_foo_Blah(ref common.ReferenceCallback) common.OpenAPIDefinition {
+	return common.OpenAPIDefinition{
+		Schema: spec.Schema{
+			SchemaProps: spec.SchemaProps{
+				Type: []string{"object"},
+				Properties: map[string]spec.Schema{
+					"Embedded": {
+						SchemaProps: spec.SchemaProps{
+							Default: 0,
+							Ref:     ref("base/foo.Duration"),
+						},
+					},
+					"PointerUnmarshal": {
+						SchemaProps: spec.SchemaProps{
+							Ref: ref("base/foo.MicroTime"),
+						},
+					},
+					"StructUnmarshal": {
+						SchemaProps: spec.SchemaProps{
+							Ref: ref("base/foo.Time"),
+						},
+					},
+					"NoUnmarshal": {
+						SchemaProps: spec.SchemaProps{
+							Default: map[string]interface{}{},
+							Ref:     ref("base/foo.TimeWithoutUnmarshal"),
+						},
+					},
+					"Regular": {
+						SchemaProps: spec.SchemaProps{
+							Default: map[string]interface{}{},
+							Ref:     ref("base/foo.NothingSpecial"),
+						},
+					},
+				},
+				Required: []string{"Embedded", "PointerUnmarshal", "StructUnmarshal", "NoUnmarshal", "Regular"},
+			},
+		},
+		Dependencies: []string{
+			"base/foo.Duration", "base/foo.MicroTime", "base/foo.NothingSpecial", "base/foo.Time", "base/foo.TimeWithoutUnmarshal"},
+	}
+}
+
+`)
+	}
+
+}