Merge pull request #273 from Yamashou/remove-enableInputJsonOmitemptyTag

Remove EnableInputJsonOmitemptyTag and modernize

Author: Yamashou

clientv2/client.go

@@ -12,6 +12,7 @@ import (
 	"mime/multipart"
 	"net/http"
 	"reflect"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -462,52 +463,13 @@ func (c *Client) unmarshal(data []byte, res any) error {
 	return err
 }
 
-// contextKey is a type for context keys
-type contextKey string
-
-const (
-	// EnableInputJsonOmitemptyTagKey is a context key for EnableInputJsonOmitemptyTag
-	EnableInputJsonOmitemptyTagKey contextKey = "enable_input_json_omitempty_tag"
-)
-
-// WithEnableInputJsonOmitemptyTag returns a new context with EnableInputJsonOmitemptyTag value
-func WithEnableInputJsonOmitemptyTag(ctx context.Context, enable bool) context.Context {
-	return context.WithValue(ctx, EnableInputJsonOmitemptyTagKey, enable)
-}
-
-// getEnableInputJsonOmitemptyTagFromContext retrieves the EnableInputJsonOmitemptyTag value from context
-func getEnableInputJsonOmitemptyTagFromContext(ctx context.Context) bool {
-	enableClientJsonOmitemptyTag := true
-	if ctx != nil {
-		enable, ok := ctx.Value(EnableInputJsonOmitemptyTagKey).(bool)
-		if ok {
-			enableClientJsonOmitemptyTag = enable
-		}
-	}
-	return enableClientJsonOmitemptyTag
-}
-
-func MarshalJSON(ctx context.Context, v any) ([]byte, error) {
-	if v == nil {
-		return []byte("null"), nil
-	}
-
-	val := reflect.ValueOf(v)
-	if !val.IsValid() || (val.Kind() == reflect.Ptr && val.IsNil()) {
-		return []byte("null"), nil
-	}
-
-	encoder := &Encoder{
-		EnableInputJsonOmitemptyTag: getEnableInputJsonOmitemptyTagFromContext(ctx),
-	}
-
-	return encoder.Encode(val)
+func MarshalJSON(_ context.Context, v any) ([]byte, error) {
+	encoder := &Encoder{}
+	return encoder.Encode(reflect.ValueOf(v))
 }
 
 // Encoder is a struct for encoding GraphQL requests to JSON
-type Encoder struct {
-	EnableInputJsonOmitemptyTag bool
-}
+type Encoder struct{}
 
 // fieldInfo holds field information of a struct
 type fieldInfo struct {
@@ -620,17 +582,17 @@ func (e *Encoder) encodeBool(v reflect.Value) ([]byte, error) {
 
 // encodeInt encodes an integer value
 func (e *Encoder) encodeInt(v reflect.Value) ([]byte, error) {
-	return []byte(fmt.Sprintf("%d", v.Int())), nil
+	return fmt.Appendf(nil, "%d", v.Int()), nil
 }
 
 // encodeUint encodes an unsigned integer value
 func (e *Encoder) encodeUint(v reflect.Value) ([]byte, error) {
-	return []byte(fmt.Sprintf("%d", v.Uint())), nil
+	return fmt.Appendf(nil, "%d", v.Uint()), nil
 }
 
 // encodeFloat encodes a floating-point value
 func (e *Encoder) encodeFloat(v reflect.Value) ([]byte, error) {
-	return []byte(fmt.Sprintf("%f", v.Float())), nil
+	return fmt.Appendf(nil, "%f", v.Float()), nil
 }
 
 // encodeString encodes a string value
@@ -650,24 +612,32 @@ func (e *Encoder) trimQuotes(s string) string {
 	return s
 }
 
-func (e *Encoder) isSkipOmitemptyField(v reflect.Value, field fieldInfo) bool {
-	if !e.EnableInputJsonOmitemptyTag {
-		return false
-	}
-
-	if !field.omitempty {
-		return false
-	}
-
+func isSkipField(omitempty bool, v reflect.Value) bool {
 	if !v.IsValid() {
 		return true
 	}
 
-	if v.Kind() == reflect.Ptr && v.IsNil() {
-		return true
+	var skipByOmitEmpty bool
+	if omitempty {
+		skipByOmitEmpty = isEmptyValue(v)
 	}
 
-	return v.IsZero()
+	return skipByOmitEmpty
+}
+
+// https://cs.opensource.google/go/go/+/refs/tags/go1.24.2:src/encoding/json/encode.go;l=318-330
+func isEmptyValue(v reflect.Value) bool {
+	switch v.Kind() {
+	case reflect.Array, reflect.Map, reflect.Slice, reflect.String:
+		return v.Len() == 0
+	case reflect.Bool,
+		reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
+		reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr,
+		reflect.Float32, reflect.Float64,
+		reflect.Interface, reflect.Pointer:
+		return v.IsZero()
+	}
+	return false
 }
 
 // encodeStruct encodes a struct value
@@ -676,7 +646,7 @@ func (e *Encoder) encodeStruct(v reflect.Value) ([]byte, error) {
 	result := make(map[string]json.RawMessage)
 	for _, field := range fields {
 		fieldValue := v.FieldByName(field.name)
-		if e.isSkipOmitemptyField(fieldValue, field) {
+		if isSkipField(field.omitempty, fieldValue) {
 			continue
 		}
 
@@ -773,21 +743,17 @@ func (e *Encoder) prepareFields(t reflect.Type) []fieldInfo {
 		if jsonTag == "-" {
 			continue
 		}
-
-		jsonName := f.Name
-		if jsonTag != "" {
-			parts := strings.Split(jsonTag, ",")
-			jsonName = parts[0]
-		}
-
 		fi := fieldInfo{
 			name:     f.Name,
-			jsonName: jsonName,
+			jsonName: f.Name,
 			typ:      f.Type,
 		}
-
-		if strings.Contains(jsonTag, "omitempty") {
-			fi.omitempty = true
+		if jsonTag != "" {
+			parts := strings.Split(jsonTag, ",")
+			fi.jsonName = parts[0]
+			if len(parts) > 1 {
+				fi.omitempty = slices.Contains(parts[1:], "omitempty")
+			}
 		}
 
 		fields = append(fields, fi)

clientv2/client_test.go

@@ -1038,11 +1038,10 @@ func TestEncoder_encodeStruct(t *testing.T) {
 	email := "test@example.com"
 
 	tests := []struct {
-		name               string
-		input              Person
-		enableOmitemptyTag bool
-		want               map[string]interface{}
-		wantErr            bool
+		name    string
+		input   Person
+		want    map[string]any
+		wantErr bool
 	}{
 		{
 			name: "all fields filled",
@@ -1055,7 +1054,6 @@ func TestEncoder_encodeStruct(t *testing.T) {
 				Nickname: "Johnny",
 				Hobbies:  []string{"reading", "swimming"},
 			},
-			enableOmitemptyTag: true,
 			want: map[string]any{
 				"name":     "John",
 				"age":      int64(30),
@@ -1073,54 +1071,13 @@ func TestEncoder_encodeStruct(t *testing.T) {
 				Name:    "John",
 				Address: Address{City: "Tokyo"},
 			},
-			enableOmitemptyTag: true,
 			want: map[string]any{
 				"name":    "John",
 				"email2":  nil,
 				"address": map[string]any{"city": "Tokyo"},
 				"hobbies": nil,
 			},
 		},
-		{
-			name: "omitempty disabled",
-			input: Person{
-				Name:    "John",
-				Address: Address{City: "Tokyo"},
-			},
-			enableOmitemptyTag: false,
-			want: map[string]any{
-				"name":     "John",
-				"age":      int64(0),
-				"email":    nil,
-				"email2":   nil,
-				"address":  map[string]any{"city": "Tokyo", "country": "", "zip": nil},
-				"tags":     nil,
-				"nickname": "",
-				"hobbies":  nil,
-			},
-		},
-		{
-			name: "nil slice set to null on omitempty disabled",
-			input: Person{
-				Name:    "John",
-				Address: Address{City: "Tokyo"},
-			},
-			enableOmitemptyTag: false,
-			want: map[string]any{
-				"name":    "John",
-				"age":     int64(0),
-				"email":   nil, // omitempty is ignored
-				"email2":  nil,
-				"tags":    nil, // omitempty is ignored
-				"hobbies": nil,
-				"address": map[string]any{
-					"city":    "Tokyo",
-					"country": "", // omitempty is ignored
-					"zip":     nil,
-				},
-				"nickname": "", // omitempty is ignored
-			},
-		},
 		{
 			name: "zero value of slice (i.e. nil slice) dropped on omitempty enabled",
 			input: Person{
@@ -1129,7 +1086,6 @@ func TestEncoder_encodeStruct(t *testing.T) {
 				Hobbies: nil,
 				Tags:    nil, // will be dropped as omitempty is enabled
 			},
-			enableOmitemptyTag: true,
 			want: map[string]any{
 				"name":    "John",
 				"email2":  nil,
@@ -1140,14 +1096,12 @@ func TestEncoder_encodeStruct(t *testing.T) {
 		{
 			name: "nil slice set to null",
 			input: Person{
-				Tags:    []string{}, // this is not zero value, so omitempty will not be applied
+				Tags:    []string{}, // empty slice is empty value but not zero value
 				Hobbies: nil,        // will continue to be nil
 			},
-			enableOmitemptyTag: true,
 			want: map[string]any{
 				"name":    "",
 				"email2":  nil,
-				"tags":    []any{},
 				"hobbies": nil,
 				"address": map[string]any{"city": ""},
 			},
@@ -1156,10 +1110,7 @@ func TestEncoder_encodeStruct(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			encoder := &Encoder{
-				EnableInputJsonOmitemptyTag: tt.enableOmitemptyTag,
-			}
-
+			encoder := &Encoder{}
 			got, err := encoder.encodeStruct(reflect.ValueOf(tt.input))
 			if (err != nil) != tt.wantErr {
 				t.Errorf("encodeStruct() error = %v, wantErr %v", err, tt.wantErr)
@@ -1175,78 +1126,56 @@ func TestEncoder_encodeStruct(t *testing.T) {
 
 			// JSONの文字列として比較
 			if string(got) != string(want) {
-				t.Errorf("encodeStruct() = %s, want %s", got, want)
+				t.Errorf("encodeStruct()\n got: %s\nwant: %s", got, want)
 			}
 		})
 	}
 }
 
 func TestEncoder_isSkipOmitemptyField(t *testing.T) {
-	type testStruct struct {
-		Required string  `json:"required"`
-		Optional string  `json:"optional,omitempty"`
-		Ptr      *string `json:"ptr,omitempty"`
-	}
-
 	str := "test"
 	tests := []struct {
-		name               string
-		value              reflect.Value
-		field              fieldInfo
-		enableOmitemptyTag bool
-		want               bool
+		name  string
+		value reflect.Value
+		field fieldInfo
+		want  bool
 	}{
 		{
-			name:               "non-empty value with omitempty",
-			value:              reflect.ValueOf("test"),
-			field:              fieldInfo{omitempty: true},
-			enableOmitemptyTag: true,
-			want:               false,
+			name:  "non-empty value with omitempty",
+			value: reflect.ValueOf("test"),
+			field: fieldInfo{omitempty: true},
+			want:  false,
 		},
 		{
-			name:               "empty value with omitempty",
-			value:              reflect.ValueOf(""),
-			field:              fieldInfo{omitempty: true},
-			enableOmitemptyTag: true,
-			want:               true,
+			name:  "empty value with omitempty",
+			value: reflect.ValueOf(""),
+			field: fieldInfo{omitempty: true},
+			want:  true,
 		},
 		{
-			name:               "nil pointer with omitempty",
-			value:              reflect.ValueOf((*string)(nil)),
-			field:              fieldInfo{omitempty: true},
-			enableOmitemptyTag: true,
-			want:               true,
+			name:  "nil pointer with omitempty",
+			value: reflect.ValueOf((*string)(nil)),
+			field: fieldInfo{omitempty: true},
+			want:  true,
 		},
 		{
-			name:               "non-nil pointer with omitempty",
-			value:              reflect.ValueOf(&str),
-			field:              fieldInfo{omitempty: true},
-			enableOmitemptyTag: true,
-			want:               false,
+			name:  "non-nil pointer with omitempty",
+			value: reflect.ValueOf(&str),
+			field: fieldInfo{omitempty: true},
+			want:  false,
 		},
 		{
-			name:               "empty value without omitempty",
-			value:              reflect.ValueOf(""),
-			field:              fieldInfo{omitempty: false},
-			enableOmitemptyTag: true,
-			want:               false,
-		},
-		{
-			name:               "omitempty tag disabled",
-			value:              reflect.ValueOf(""),
-			field:              fieldInfo{omitempty: true},
-			enableOmitemptyTag: false,
-			want:               false,
+			name:  "empty value without omitempty",
+			value: reflect.ValueOf(""),
+			field: fieldInfo{omitempty: false},
+			want:  false,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			encoder := &Encoder{
-				EnableInputJsonOmitemptyTag: tt.enableOmitemptyTag,
-			}
-			if got := encoder.isSkipOmitemptyField(tt.value, tt.field); got != tt.want {
-				t.Errorf("isSkipOmitemptyField() = %v, want %v", got, tt.want)
+			if got := isSkipField(tt.field.omitempty, tt.value); got != tt.want {
+				t.Errorf("isSkipField() = %v, want %v", got, tt.want)
 			}
 		})
 	}