Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🩹 fix: make SetValWithStruct set zero values and support more types #3167 #3227

Merged
merged 7 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions client/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"errors"
"fmt"
"io"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -929,11 +930,13 @@ func ReleaseFile(f *File) {
filePool.Put(f)
}

// SetValWithStruct Set some values using structs.
// `p` is a structure that implements the WithStruct interface,
// The field name can be specified by `tagName`.
// `v` is a struct include some data.
// Note: This method only supports simple types and nested structs are not currently supported.
// SetValWithStruct sets values using a struct.
// `p` is a structure that implements the WithStruct interface.
// The field name is specified by `tagName`.
// `v` is a struct containing some data.
// Note: This method supports all values that can be converted to an interface.
ksw2000 marked this conversation as resolved.
Show resolved Hide resolved
// In Fiber v2, SetValWithStruct sets non-zero values only. Starting from v3,
// it sets values regardless of whether they are zero or non-zero.
func SetValWithStruct(p WithStruct, tagName string, v any) {
valueOfV := reflect.ValueOf(v)
typeOfV := reflect.TypeOf(v)
Expand All @@ -947,15 +950,20 @@ func SetValWithStruct(p WithStruct, tagName string, v any) {
}

// Boring type judge.
// TODO: cover more types and complex data structure.
var setVal func(name string, value reflect.Value)
var setVal func(name string, val reflect.Value)
setVal = func(name string, val reflect.Value) {
switch val.Kind() {
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
p.Add(name, strconv.Itoa(int(val.Int())))
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
p.Add(name, strconv.FormatUint(val.Uint(), 10))
case reflect.Complex128, reflect.Complex64:
p.Add(name, strconv.FormatComplex(val.Complex(), 'f', -1, 128))
case reflect.Bool:
if val.Bool() {
p.Add(name, "true")
} else {
p.Add(name, "false")
}
case reflect.String:
p.Add(name, val.String())
Expand All @@ -966,6 +974,9 @@ func SetValWithStruct(p WithStruct, tagName string, v any) {
setVal(name, val.Index(i))
}
default:
if val.CanInterface() {
p.Add(name, fmt.Sprintf("%#v", val.Interface()))
}
ksw2000 marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -980,9 +991,6 @@ func SetValWithStruct(p WithStruct, tagName string, v any) {
name = field.Name
}
val := valueOfV.Field(i)
if val.IsZero() {
continue
}
// To cover slice and array, we delete the val then add it.
p.Del(name)
setVal(name, val)
Expand Down
81 changes: 32 additions & 49 deletions client/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1317,15 +1317,19 @@ func Test_Request_MaxRedirects(t *testing.T) {
func Test_SetValWithStruct(t *testing.T) {
t.Parallel()

// test SetValWithStruct vai QueryParam struct.
type args struct {
// test SetValWithStruct via QueryParam struct.
type args struct { //nolint:govet // Aligning the struct fields is not necessary.
TString string
TSlice []string
TIntSlice []int `param:"int_slice"`
unexport int
TInt int
TUint uint
TFloat float64
TComplex complex128
TFunc func()
TBool bool
TMap map[int]int
ksw2000 marked this conversation as resolved.
Show resolved Hide resolved
}

t.Run("the struct should be applied", func(t *testing.T) {
Expand All @@ -1337,18 +1341,26 @@ func Test_SetValWithStruct(t *testing.T) {
SetValWithStruct(p, "param", args{
unexport: 5,
TInt: 5,
TUint: 5,
TString: "string",
TFloat: 3.1,
TComplex: 3 + 4i,
TBool: false,
TSlice: []string{"foo", "bar"},
TIntSlice: []int{1, 2},
TIntSlice: []int{0, 1, 2},
TFunc: func() {},
TMap: map[int]int{1: 2},
ksw2000 marked this conversation as resolved.
Show resolved Hide resolved
})

require.Equal(t, "", string(p.Peek("unexport")))
require.Equal(t, []byte("5"), p.Peek("TInt"))
require.Equal(t, []byte("5"), p.Peek("TUint"))
require.Equal(t, []byte("string"), p.Peek("TString"))
require.Equal(t, []byte("3.1"), p.Peek("TFloat"))
require.Equal(t, "", string(p.Peek("TBool")))
require.Equal(t, []byte("(3+4i)"), p.Peek("TComplex"))
require.Equal(t, []byte("false"), p.Peek("TBool"))
require.Contains(t, string(p.Peek("TFunc")), "(func())")
require.Equal(t, []byte("map[int]int{1:2}"), p.Peek("TMap"))
require.True(t, func() bool {
for _, v := range p.PeekMulti("TSlice") {
if string(v) == "foo" {
Expand Down Expand Up @@ -1442,24 +1454,6 @@ func Test_SetValWithStruct(t *testing.T) {
}())
})

t.Run("the zero val should be ignore", func(t *testing.T) {
t.Parallel()
p := &QueryParam{
Args: fasthttp.AcquireArgs(),
}
SetValWithStruct(p, "param", &args{
TInt: 0,
TString: "",
TFloat: 0.0,
})

require.Equal(t, "", string(p.Peek("TInt")))
require.Equal(t, "", string(p.Peek("TString")))
require.Equal(t, "", string(p.Peek("TFloat")))
require.Empty(t, p.PeekMulti("TSlice"))
require.Empty(t, p.PeekMulti("int_slice"))
})

t.Run("error type should ignore", func(t *testing.T) {
t.Parallel()
p := &QueryParam{
Expand All @@ -1471,15 +1465,19 @@ func Test_SetValWithStruct(t *testing.T) {
}

func Benchmark_SetValWithStruct(b *testing.B) {
// test SetValWithStruct vai QueryParam struct.
type args struct {
// test SetValWithStruct via QueryParam struct.
type args struct { //nolint:govet // Aligning the struct fields is not necessary.
TString string
TSlice []string
TIntSlice []int `param:"int_slice"`
unexport int
TInt int
TUint uint
TFloat float64
TComplex complex128
TBool bool
TFunc func()
TMap map[int]int
}

b.Run("the struct should be applied", func(b *testing.B) {
Expand All @@ -1494,19 +1492,27 @@ func Benchmark_SetValWithStruct(b *testing.B) {
SetValWithStruct(p, "param", args{
unexport: 5,
TInt: 5,
TUint: 5,
TString: "string",
TFloat: 3.1,
TComplex: 3 + 4i,
TBool: false,
TSlice: []string{"foo", "bar"},
TIntSlice: []int{1, 2},
TIntSlice: []int{0, 1, 2},
TFunc: func() {},
TMap: map[int]int{1: 2},
})
}

require.Equal(b, "", string(p.Peek("unexport")))
require.Equal(b, []byte("5"), p.Peek("TInt"))
require.Equal(b, []byte("5"), p.Peek("TUint"))
require.Equal(b, []byte("string"), p.Peek("TString"))
require.Equal(b, []byte("3.1"), p.Peek("TFloat"))
require.Equal(b, "", string(p.Peek("TBool")))
require.Equal(b, []byte("(3+4i)"), p.Peek("TComplex"))
require.Equal(b, []byte("false"), p.Peek("TBool"))
require.Contains(b, string(p.Peek("TFunc")), "(func())")
require.Equal(b, []byte("map[int]int{1:2}"), p.Peek("TMap"))
require.True(b, func() bool {
for _, v := range p.PeekMulti("TSlice") {
if string(v) == "foo" {
Expand Down Expand Up @@ -1604,29 +1610,6 @@ func Benchmark_SetValWithStruct(b *testing.B) {
}())
})

b.Run("the zero val should be ignore", func(b *testing.B) {
p := &QueryParam{
Args: fasthttp.AcquireArgs(),
}

b.ReportAllocs()
b.StartTimer()

for i := 0; i < b.N; i++ {
SetValWithStruct(p, "param", &args{
TInt: 0,
TString: "",
TFloat: 0.0,
})
}

require.Empty(b, string(p.Peek("TInt")))
require.Empty(b, string(p.Peek("TString")))
require.Empty(b, string(p.Peek("TFloat")))
require.Empty(b, p.PeekMulti("TSlice"))
require.Empty(b, p.PeekMulti("int_slice"))
})

b.Run("error type should ignore", func(b *testing.B) {
p := &QueryParam{
Args: fasthttp.AcquireArgs(),
Expand Down