Merge pull request kubernetes-sigs#286 from liggitt/omitzero

Add omitzero support

Author: k8s-ci-robot

Diff for: value/jsontagutil.go

@@ -22,22 +22,77 @@ import (
 	"strings"
 )
 
+type isZeroer interface {
+	IsZero() bool
+}
+
+var isZeroerType = reflect.TypeOf((*isZeroer)(nil)).Elem()
+
+func reflectIsZero(dv reflect.Value) bool {
+	return dv.IsZero()
+}
+
+// OmitZeroFunc returns a function for a type for a given struct field
+// which determines if the value for that field is a zero value, matching
+// how the stdlib JSON implementation.
+func OmitZeroFunc(t reflect.Type) func(reflect.Value) bool {
+	// Provide a function that uses a type's IsZero method.
+	// This matches the go 1.24 custom IsZero() implementation matching
+	switch {
+	case t.Kind() == reflect.Interface && t.Implements(isZeroerType):
+		return func(v reflect.Value) bool {
+			// Avoid panics calling IsZero on a nil interface or
+			// non-nil interface with nil pointer.
+			return safeIsNil(v) ||
+				(v.Elem().Kind() == reflect.Pointer && v.Elem().IsNil()) ||
+				v.Interface().(isZeroer).IsZero()
+		}
+	case t.Kind() == reflect.Pointer && t.Implements(isZeroerType):
+		return func(v reflect.Value) bool {
+			// Avoid panics calling IsZero on nil pointer.
+			return safeIsNil(v) || v.Interface().(isZeroer).IsZero()
+		}
+	case t.Implements(isZeroerType):
+		return func(v reflect.Value) bool {
+			return v.Interface().(isZeroer).IsZero()
+		}
+	case reflect.PointerTo(t).Implements(isZeroerType):
+		return func(v reflect.Value) bool {
+			if !v.CanAddr() {
+				// Temporarily box v so we can take the address.
+				v2 := reflect.New(v.Type()).Elem()
+				v2.Set(v)
+				v = v2
+			}
+			return v.Addr().Interface().(isZeroer).IsZero()
+		}
+	default:
+		// default to the reflect.IsZero implementation
+		return reflectIsZero
+	}
+}
+
 // TODO: This implements the same functionality as https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go#L236
 // but is based on the highly efficient approach from https://golang.org/src/encoding/json/encode.go
 
-func lookupJsonTags(f reflect.StructField) (name string, omit bool, inline bool, omitempty bool) {
+func lookupJsonTags(f reflect.StructField) (name string, omit bool, inline bool, omitempty bool, omitzero func(reflect.Value) bool) {
 	tag := f.Tag.Get("json")
 	if tag == "-" {
-		return "", true, false, false
+		return "", true, false, false, nil
 	}
 	name, opts := parseTag(tag)
 	if name == "" {
 		name = f.Name
 	}
-	return name, false, opts.Contains("inline"), opts.Contains("omitempty")
+
+	if opts.Contains("omitzero") {
+		omitzero = OmitZeroFunc(f.Type)
+	}
+
+	return name, false, opts.Contains("inline"), opts.Contains("omitempty"), omitzero
 }
 
-func isZero(v reflect.Value) bool {
+func isEmpty(v reflect.Value) bool {
 	switch v.Kind() {
 	case reflect.Array, reflect.Map, reflect.Slice, reflect.String:
 		return v.Len() == 0

Diff for: value/reflectcache.go

@@ -59,6 +59,8 @@ type FieldCacheEntry struct {
 	JsonName string
 	// isOmitEmpty is true if the field has the json 'omitempty' tag.
 	isOmitEmpty bool
+	// omitzero is set if the field has the json 'omitzero' tag.
+	omitzero func(reflect.Value) bool
 	// fieldPath is a list of field indices (see FieldByIndex) to lookup the value of
 	// a field in a reflect.Value struct. The field indices in the list form a path used
 	// to traverse through intermediary 'inline' fields.
@@ -69,7 +71,13 @@ type FieldCacheEntry struct {
 }
 
 func (f *FieldCacheEntry) CanOmit(fieldVal reflect.Value) bool {
-	return f.isOmitEmpty && (safeIsNil(fieldVal) || isZero(fieldVal))
+	if f.isOmitEmpty && (safeIsNil(fieldVal) || isEmpty(fieldVal)) {
+		return true
+	}
+	if f.omitzero != nil && f.omitzero(fieldVal) {
+		return true
+	}
+	return false
 }
 
 // GetFrom returns the field identified by this FieldCacheEntry from the provided struct.
@@ -147,7 +155,7 @@ func typeReflectEntryOf(cm reflectCacheMap, t reflect.Type, updates reflectCache
 func buildStructCacheEntry(t reflect.Type, infos map[string]*FieldCacheEntry, fieldPath [][]int) {
 	for i := 0; i < t.NumField(); i++ {
 		field := t.Field(i)
-		jsonName, omit, isInline, isOmitempty := lookupJsonTags(field)
+		jsonName, omit, isInline, isOmitempty, omitzero := lookupJsonTags(field)
 		if omit {
 			continue
 		}
@@ -161,7 +169,7 @@ func buildStructCacheEntry(t reflect.Type, infos map[string]*FieldCacheEntry, fi
 			}
 			continue
 		}
-		info := &FieldCacheEntry{JsonName: jsonName, isOmitEmpty: isOmitempty, fieldPath: append(fieldPath, field.Index), fieldType: field.Type}
+		info := &FieldCacheEntry{JsonName: jsonName, isOmitEmpty: isOmitempty, omitzero: omitzero, fieldPath: append(fieldPath, field.Index), fieldType: field.Type}
 		infos[jsonName] = info
 	}
 }

Diff for: value/reflectcache_test.go

@@ -144,6 +144,7 @@ func TestTimeToUnstructured(t *testing.T) {
 
 func TestTypeReflectEntryOf(t *testing.T) {
 	testString := ""
+	testCustomType := customOmitZeroType{}
 	tests := map[string]struct {
 		arg  interface{}
 		want *TypeReflectCacheEntry
@@ -196,6 +197,62 @@ func TestTypeReflectEntryOf(t *testing.T) {
 				},
 			},
 		},
+		"StructWith*StringFieldOmitzero": {
+			arg: struct {
+				F1 *string `json:"f1,omitzero"`
+			}{},
+			want: &TypeReflectCacheEntry{
+				structFields: map[string]*FieldCacheEntry{
+					"f1": {
+						JsonName:  "f1",
+						omitzero:  func(v reflect.Value) bool { return v.IsZero() },
+						fieldPath: [][]int{{0}},
+						fieldType: reflect.TypeOf(&testString),
+						TypeEntry: &TypeReflectCacheEntry{},
+					},
+				},
+				orderedStructFields: []*FieldCacheEntry{
+					{
+						JsonName:  "f1",
+						omitzero:  func(v reflect.Value) bool { return v.IsZero() },
+						fieldPath: [][]int{{0}},
+						fieldType: reflect.TypeOf(&testString),
+						TypeEntry: &TypeReflectCacheEntry{},
+					},
+				},
+			},
+		},
+		"StructWith*CustomFieldOmitzero": {
+			arg: struct {
+				F1 customOmitZeroType `json:"f1,omitzero"`
+			}{},
+			want: &TypeReflectCacheEntry{
+				structFields: map[string]*FieldCacheEntry{
+					"f1": {
+						JsonName:  "f1",
+						omitzero:  func(v reflect.Value) bool { return false },
+						fieldPath: [][]int{{0}},
+						fieldType: reflect.TypeOf(testCustomType),
+						TypeEntry: &TypeReflectCacheEntry{
+							structFields:        map[string]*FieldCacheEntry{},
+							orderedStructFields: []*FieldCacheEntry{},
+						},
+					},
+				},
+				orderedStructFields: []*FieldCacheEntry{
+					{
+						JsonName:  "f1",
+						omitzero:  func(v reflect.Value) bool { return false },
+						fieldPath: [][]int{{0}},
+						fieldType: reflect.TypeOf(testCustomType),
+						TypeEntry: &TypeReflectCacheEntry{
+							structFields:        map[string]*FieldCacheEntry{},
+							orderedStructFields: []*FieldCacheEntry{},
+						},
+					},
+				},
+			},
+		},
 		"StructWithInlinedField": {
 			arg: struct {
 				F1 string `json:",inline"`
@@ -208,13 +265,55 @@ func TestTypeReflectEntryOf(t *testing.T) {
 	}
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
-			if got := TypeReflectEntryOf(reflect.TypeOf(tt.arg)); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("TypeReflectEntryOf() = %v, want %v", got, tt.want)
+			got := TypeReflectEntryOf(reflect.TypeOf(tt.arg))
+
+			// evaluate non-comparable omitzero functions
+			for k, v := range got.structFields {
+				compareOmitZero(t, v.fieldType, v.omitzero, tt.want.structFields[k].omitzero)
+			}
+			for i, v := range got.orderedStructFields {
+				compareOmitZero(t, v.fieldType, v.omitzero, tt.want.orderedStructFields[i].omitzero)
+			}
+
+			// clear non-comparable omitzero functions
+			for k, v := range got.structFields {
+				v.omitzero = nil
+				tt.want.structFields[k].omitzero = nil
+			}
+			for i, v := range got.orderedStructFields {
+				v.omitzero = nil
+				tt.want.orderedStructFields[i].omitzero = nil
+			}
+
+			// compare remaining fields
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("TypeReflectEntryOf() got\n%#v\nwant\n%#v", got, tt.want)
 			}
 		})
 	}
 }
 
+type customOmitZeroType struct {
+}
+
+func (c *customOmitZeroType) IsZero() bool {
+	return false
+}
+
+func compareOmitZero(t *testing.T, fieldType reflect.Type, got, want func(reflect.Value) bool) {
+	t.Helper()
+	if (want == nil) != (got == nil) {
+		t.Fatalf("wanted omitzero=%v, got omitzero=%v", (want == nil), (got == nil))
+	}
+	if want == nil {
+		return
+	}
+	v := reflect.New(fieldType).Elem()
+	if e, a := want(v), got(v); e != a {
+		t.Fatalf("wanted omitzero()=%v, got omitzero()=%v", e, a)
+	}
+}
+
 func TestUnmarshal(t *testing.T) {
 	for _, tc := range []struct {
 		JSON      string