Skip to content

Commit

Permalink
🩹 fix: make SetValWithStruct set zero values and support more types #…
Browse files Browse the repository at this point in the history
…3167 (#3227)

* 🩹 fix: make SetValWithStruct set zero values and support more types

* 🚨 test: check zero in int_slice

* fix: SetValWithStruct does not exist in fiber v2

* 🩹fix: restrict supported types in SetValWithStruct

* 🩹fix: golangci-lint

---------

Co-authored-by: Juan Calderon-Perez <[email protected]>
  • Loading branch information
ksw2000 and gaby authored Dec 13, 2024
1 parent a63bd34 commit 1134e1f
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 59 deletions.
30 changes: 18 additions & 12 deletions client/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -1062,11 +1062,14 @@ 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 stores the fields of `v` into `p`.
// `tagName` specifies the key used to store into `p`. If not specified,
// the field name is used by default.
// `v` is a struct or a pointer to a struct containing some data.
// Fields in `v` should be string, int, int8, int16, int32, int64, uint,
// uint8, uint16, uint32, uint64, float32, float64, complex64,
// complex128 or bool. Arrays or slices are inserted sequentially with the
// same key. Other types are ignored.
func SetValWithStruct(p WithStruct, tagName string, v any) {
valueOfV := reflect.ValueOf(v)
typeOfV := reflect.TypeOf(v)
Expand All @@ -1080,25 +1083,31 @@ 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.Float32, reflect.Float64:
p.Add(name, strconv.FormatFloat(val.Float(), 'f', -1, 64))
case reflect.Complex64, reflect.Complex128:
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())
case reflect.Float32, reflect.Float64:
p.Add(name, strconv.FormatFloat(val.Float(), 'f', -1, 64))
case reflect.Slice, reflect.Array:
for i := 0; i < val.Len(); i++ {
setVal(name, val.Index(i))
}
default:
return
}
}

Expand All @@ -1113,9 +1122,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
83 changes: 36 additions & 47 deletions client/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1530,14 +1530,16 @@ func Test_Request_MaxRedirects(t *testing.T) {
func Test_SetValWithStruct(t *testing.T) {
t.Parallel()

// test SetValWithStruct vai QueryParam struct.
// test SetValWithStruct via QueryParam struct.
type args struct {
TString string
TSlice []string
TIntSlice []int `param:"int_slice"`
unexport int
TInt int
TUint uint
TFloat float64
TComplex complex128
TBool bool
}

Expand All @@ -1550,18 +1552,22 @@ 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},
})

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.True(t, func() bool {
for _, v := range p.PeekMulti("TSlice") {
if string(v) == "foo" {
Expand All @@ -1580,6 +1586,15 @@ func Test_SetValWithStruct(t *testing.T) {
return false
}())

require.True(t, func() bool {
for _, v := range p.PeekMulti("int_slice") {
if string(v) == "0" {
return true
}
}
return false
}())

require.True(t, func() bool {
for _, v := range p.PeekMulti("int_slice") {
if string(v) == "1" {
Expand Down Expand Up @@ -1655,24 +1670,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 @@ -1684,14 +1681,16 @@ func Test_SetValWithStruct(t *testing.T) {
}

func Benchmark_SetValWithStruct(b *testing.B) {
// test SetValWithStruct vai QueryParam struct.
// test SetValWithStruct via QueryParam struct.
type args struct {
TString string
TSlice []string
TIntSlice []int `param:"int_slice"`
unexport int
TInt int
TUint uint
TFloat float64
TComplex complex128
TBool bool
}

Expand All @@ -1707,19 +1706,23 @@ 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},
})
}

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.True(b, func() bool {
for _, v := range p.PeekMulti("TSlice") {
if string(v) == "foo" {
Expand All @@ -1738,6 +1741,15 @@ func Benchmark_SetValWithStruct(b *testing.B) {
return false
}())

require.True(b, func() bool {
for _, v := range p.PeekMulti("int_slice") {
if string(v) == "0" {
return true
}
}
return false
}())

require.True(b, func() bool {
for _, v := range p.PeekMulti("int_slice") {
if string(v) == "1" {
Expand Down Expand Up @@ -1817,29 +1829,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

1 comment on commit 1134e1f

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 1134e1f Previous: 56ff2de Ratio
Benchmark_Ctx_Send 7.122 ns/op 0 B/op 0 allocs/op 4.729 ns/op 0 B/op 0 allocs/op 1.51
Benchmark_Ctx_Send - ns/op 7.122 ns/op 4.729 ns/op 1.51
Benchmark_Utils_GetOffer/1_parameter 226 ns/op 0 B/op 0 allocs/op 133.7 ns/op 0 B/op 0 allocs/op 1.69
Benchmark_Utils_GetOffer/1_parameter - ns/op 226 ns/op 133.7 ns/op 1.69
Benchmark_Middleware_BasicAuth - B/op 80 B/op 48 B/op 1.67
Benchmark_Middleware_BasicAuth - allocs/op 5 allocs/op 3 allocs/op 1.67
Benchmark_Middleware_BasicAuth_Upper - B/op 80 B/op 48 B/op 1.67
Benchmark_Middleware_BasicAuth_Upper - allocs/op 5 allocs/op 3 allocs/op 1.67
Benchmark_CORS_NewHandler - B/op 16 B/op 0 B/op +∞
Benchmark_CORS_NewHandler - allocs/op 1 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerSingleOrigin - B/op 16 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerSingleOrigin - allocs/op 1 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflight - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflight - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflightSingleOrigin - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflightSingleOrigin - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflightWildcard - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflightWildcard - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_Middleware_CSRF_GenerateToken - B/op 519 B/op 329 B/op 1.58
Benchmark_Middleware_CSRF_GenerateToken - allocs/op 10 allocs/op 6 allocs/op 1.67

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.