Skip to content

Commit 1f11e18

Browse files
SVilgelmSVilgelm
authored
Benchmarks and Improvements for parseRequestURL function (#711)
* Benchmarks for applying PathParams in parseRequestURL function ```shell % go test -benchmem -bench=. -run=^Benchmark goos: darwin goarch: amd64 pkg: github.com/go-resty/resty/v2 cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz Benchmark_parseRequestURL_PathParams-16 524658 2260 ns/op 448 B/op 9 allocs/op PASS ok github.com/go-resty/resty/v2 2.327s ``` * Benchmarks for applying QueryParams in parseRequestURL function ```shell % go test -benchmem -bench=. -run=^Benchmark goos: darwin goarch: amd64 pkg: github.com/go-resty/resty/v2 cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz Benchmark_parseRequestURL_QueryParams-16 865923 1371 ns/op 416 B/op 13 allocs/op PASS ok github.com/go-resty/resty/v2 2.491s ``` * improve the performance of applying the path parameters * Use the map to collect all replacements and use replace all path parameters using O(1) logic * Add additional unit tests to cover empty `{}` and not closed `{bar` path parameters ```shell % go test -benchmem -bench=. -run=^Benchmark goos: darwin goarch: amd64 pkg: github.com/go-resty/resty/v2 cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz Benchmark_parseRequestURL_PathParams-16 785971 1410 ns/op 320 B/op 6 allocs/op PASS ok github.com/go-resty/resty/v2 1.445s ``` * improve the performance of applying the query parameters * improve the loging by adding the query parameters from the request first, then adding the parameters from the client and skip if already exists * additional unit tests for the query parameters ```shell % go test -benchmem -bench=. -run=^Benchmark goos: darwin goarch: amd64 pkg: github.com/go-resty/resty/v2 cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz Benchmark_parseRequestURL_QueryParams-16 1000000 1158 ns/op 352 B/op 9 allocs/op PASS ok github.com/go-resty/resty/v2 2.473s ``` * using acquireBuffer reusing a buffer from the pool decreases the allocs and memory usage ```shell % go test -benchmem -bench=. -run=^Benchmark goos: darwin goarch: amd64 pkg: github.com/go-resty/resty/v2 cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz Benchmark_parseRequestURL_PathParams-16 753834 1367 ns/op 256 B/op 5 allocs/op Benchmark_parseRequestURL_QueryParams-16 1000000 1167 ns/op 352 B/op 9 allocs/op PASS ok github.com/go-resty/resty/v2 2.373s ``` * using reflect.DeepEqual to compare the expected and actual QueryParams * update r.QueryParam isntead of creating new variable * remove unneeded if
1 parent 4604150 commit 1f11e18

File tree

2 files changed

+183
-52
lines changed

2 files changed

+183
-52
lines changed

‎middleware.go

+79-36
Original file line numberDiff line numberDiff line change
@@ -27,27 +27,76 @@ const debugRequestLogKey = "__restyDebugRequestLog"
2727
//_______________________________________________________________________
2828

2929
func parseRequestURL(c *Client, r *Request) error {
30-
// GitHub #103 Path Params
31-
if len(r.PathParams) > 0 {
30+
if l := len(c.PathParams) + len(c.RawPathParams) + len(r.PathParams) + len(r.RawPathParams); l > 0 {
31+
params := make(map[string]string, l)
32+
33+
// GitHub #103 Path Params
3234
for p, v := range r.PathParams {
33-
r.URL = strings.Replace(r.URL, "{"+p+"}", url.PathEscape(v), -1)
35+
params[p] = url.PathEscape(v)
3436
}
35-
}
36-
if len(c.PathParams) > 0 {
3737
for p, v := range c.PathParams {
38-
r.URL = strings.Replace(r.URL, "{"+p+"}", url.PathEscape(v), -1)
38+
if _, ok := params[p]; !ok {
39+
params[p] = url.PathEscape(v)
40+
}
3941
}
40-
}
4142

42-
// GitHub #663 Raw Path Params
43-
if len(r.RawPathParams) > 0 {
43+
// GitHub #663 Raw Path Params
4444
for p, v := range r.RawPathParams {
45-
r.URL = strings.Replace(r.URL, "{"+p+"}", v, -1)
45+
if _, ok := params[p]; !ok {
46+
params[p] = v
47+
}
4648
}
47-
}
48-
if len(c.RawPathParams) > 0 {
4949
for p, v := range c.RawPathParams {
50-
r.URL = strings.Replace(r.URL, "{"+p+"}", v, -1)
50+
if _, ok := params[p]; !ok {
51+
params[p] = v
52+
}
53+
}
54+
55+
if len(params) > 0 {
56+
var prev int
57+
buf := acquireBuffer()
58+
defer releaseBuffer(buf)
59+
// search for the next or first opened curly bracket
60+
for curr := strings.Index(r.URL, "{"); curr > prev; curr = prev + strings.Index(r.URL[prev:], "{") {
61+
// write everything form the previous position up to the current
62+
if curr > prev {
63+
buf.WriteString(r.URL[prev:curr])
64+
}
65+
// search for the closed curly bracket from current position
66+
next := curr + strings.Index(r.URL[curr:], "}")
67+
// if not found, then write the remainder and exit
68+
if next < curr {
69+
buf.WriteString(r.URL[curr:])
70+
prev = len(r.URL)
71+
break
72+
}
73+
// special case for {}, without parameter's name
74+
if next == curr+1 {
75+
buf.WriteString("{}")
76+
} else {
77+
// check for the replacement
78+
key := r.URL[curr+1 : next]
79+
value, ok := params[key]
80+
/// keep the original string if the replacement not found
81+
if !ok {
82+
value = r.URL[curr : next+1]
83+
}
84+
buf.WriteString(value)
85+
}
86+
87+
// set the previous position after the closed curly bracket
88+
prev = next + 1
89+
if prev >= len(r.URL) {
90+
break
91+
}
92+
}
93+
if buf.Len() > 0 {
94+
// write remainder
95+
if prev < len(r.URL) {
96+
buf.WriteString(r.URL[prev:])
97+
}
98+
r.URL = buf.String()
99+
}
51100
}
52101
}
53102

@@ -82,32 +131,26 @@ func parseRequestURL(c *Client, r *Request) error {
82131
}
83132

84133
// Adding Query Param
85-
query := make(url.Values)
86-
for k, v := range c.QueryParam {
87-
for _, iv := range v {
88-
query.Add(k, iv)
89-
}
90-
}
91-
92-
for k, v := range r.QueryParam {
93-
// remove query param from client level by key
94-
// since overrides happens for that key in the request
95-
query.Del(k)
134+
if len(c.QueryParam)+len(r.QueryParam) > 0 {
135+
for k, v := range c.QueryParam {
136+
// skip query parameter if it was set in request
137+
if _, ok := r.QueryParam[k]; ok {
138+
continue
139+
}
96140

97-
for _, iv := range v {
98-
query.Add(k, iv)
141+
r.QueryParam[k] = v[:]
99142
}
100-
}
101143

102-
// GitHub #123 Preserve query string order partially.
103-
// Since not feasible in `SetQuery*` resty methods, because
104-
// standard package `url.Encode(...)` sorts the query params
105-
// alphabetically
106-
if len(query) > 0 {
107-
if IsStringEmpty(reqURL.RawQuery) {
108-
reqURL.RawQuery = query.Encode()
109-
} else {
110-
reqURL.RawQuery = reqURL.RawQuery + "&" + query.Encode()
144+
// GitHub #123 Preserve query string order partially.
145+
// Since not feasible in `SetQuery*` resty methods, because
146+
// standard package `url.Encode(...)` sorts the query params
147+
// alphabetically
148+
if len(r.QueryParam) > 0 {
149+
if IsStringEmpty(reqURL.RawQuery) {
150+
reqURL.RawQuery = r.QueryParam.Encode()
151+
} else {
152+
reqURL.RawQuery = reqURL.RawQuery + "&" + r.QueryParam.Encode()
153+
}
111154
}
112155
}
113156

‎middleware_test.go

+104-16
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,46 @@ func Test_parseRequestURL(t *testing.T) {
105105
},
106106
expectedURL: "https://example.com/4%2F5/6/7",
107107
},
108+
{
109+
name: "empty path parameter in URL",
110+
init: func(c *Client, r *Request) {
111+
r.SetPathParams(map[string]string{
112+
"bar": "4",
113+
})
114+
r.URL = "https://example.com/{}/{bar}"
115+
},
116+
expectedURL: "https://example.com/%7B%7D/4",
117+
},
118+
{
119+
name: "not closed path parameter in URL",
120+
init: func(c *Client, r *Request) {
121+
r.SetPathParams(map[string]string{
122+
"foo": "4",
123+
})
124+
r.URL = "https://example.com/{foo}/{bar/1"
125+
},
126+
expectedURL: "https://example.com/4/%7Bbar/1",
127+
},
128+
{
129+
name: "extra path parameter in URL",
130+
init: func(c *Client, r *Request) {
131+
r.SetPathParams(map[string]string{
132+
"foo": "1",
133+
})
134+
r.URL = "https://example.com/{foo}/{bar}"
135+
},
136+
expectedURL: "https://example.com/1/%7Bbar%7D",
137+
},
138+
{
139+
name: " path parameter with remainder",
140+
init: func(c *Client, r *Request) {
141+
r.SetPathParams(map[string]string{
142+
"foo": "1",
143+
})
144+
r.URL = "https://example.com/{foo}/2"
145+
},
146+
expectedURL: "https://example.com/1/2",
147+
},
108148
{
109149
name: "using BaseURL with absolute URL in request",
110150
init: func(c *Client, r *Request) {
@@ -189,13 +229,32 @@ func Test_parseRequestURL(t *testing.T) {
189229
"foo": "1", // ignored, because of the "foo" parameter in request
190230
"bar": "2",
191231
})
192-
c.SetQueryParams(map[string]string{
232+
r.SetQueryParams(map[string]string{
193233
"foo": "3",
194234
})
195235
r.URL = "https://example.com/"
196236
},
197237
expectedURL: "https://example.com/?foo=3&bar=2",
198238
},
239+
{
240+
name: "adding query parameters by request to URL with existent",
241+
init: func(c *Client, r *Request) {
242+
r.SetQueryParams(map[string]string{
243+
"bar": "2",
244+
})
245+
r.URL = "https://example.com/?foo=1"
246+
},
247+
expectedURL: "https://example.com/?foo=1&bar=2",
248+
},
249+
{
250+
name: "adding query parameters by request with multiple values",
251+
init: func(c *Client, r *Request) {
252+
r.QueryParam.Add("foo", "1")
253+
r.QueryParam.Add("foo", "2")
254+
r.URL = "https://example.com/"
255+
},
256+
expectedURL: "https://example.com/?foo=1&foo=2",
257+
},
199258
} {
200259
t.Run(tt.name, func(t *testing.T) {
201260
c := New()
@@ -216,27 +275,55 @@ func Test_parseRequestURL(t *testing.T) {
216275
if expectedURL.String() != actualURL.String() {
217276
t.Errorf("r.URL = %q does not match expected %q", r.URL, tt.expectedURL)
218277
}
219-
if len(expectedQuery) != len(actualQuery) {
278+
if !reflect.DeepEqual(expectedQuery, actualQuery) {
220279
t.Errorf("r.URL = %q does not match expected %q", r.URL, tt.expectedURL)
221280
}
222-
for name, expected := range expectedQuery {
223-
actual, ok := actualQuery[name]
224-
if !ok {
225-
t.Errorf("r.URL = %q does not match expected %q", r.URL, tt.expectedURL)
226-
}
227-
if len(expected) != len(actual) {
228-
t.Errorf("r.URL = %q does not match expected %q", r.URL, tt.expectedURL)
229-
}
230-
for i, v := range expected {
231-
if v != actual[i] {
232-
t.Errorf("r.URL = %q does not match expected %q", r.URL, tt.expectedURL)
233-
}
234-
}
235-
}
236281
})
237282
}
238283
}
239284

285+
func Benchmark_parseRequestURL_PathParams(b *testing.B) {
286+
c := New().SetPathParams(map[string]string{
287+
"foo": "1",
288+
"bar": "2",
289+
}).SetRawPathParams(map[string]string{
290+
"foo": "3",
291+
"xyz": "4",
292+
})
293+
r := c.R().SetPathParams(map[string]string{
294+
"foo": "5",
295+
"qwe": "6",
296+
}).SetRawPathParams(map[string]string{
297+
"foo": "7",
298+
"asd": "8",
299+
})
300+
b.ResetTimer()
301+
for i := 0; i < b.N; i++ {
302+
r.URL = "https://example.com/{foo}/{bar}/{xyz}/{qwe}/{asd}"
303+
if err := parseRequestURL(c, r); err != nil {
304+
b.Errorf("parseRequestURL() error = %v", err)
305+
}
306+
}
307+
}
308+
309+
func Benchmark_parseRequestURL_QueryParams(b *testing.B) {
310+
c := New().SetQueryParams(map[string]string{
311+
"foo": "1",
312+
"bar": "2",
313+
})
314+
r := c.R().SetQueryParams(map[string]string{
315+
"foo": "5",
316+
"qwe": "6",
317+
})
318+
b.ResetTimer()
319+
for i := 0; i < b.N; i++ {
320+
r.URL = "https://example.com/"
321+
if err := parseRequestURL(c, r); err != nil {
322+
b.Errorf("parseRequestURL() error = %v", err)
323+
}
324+
}
325+
}
326+
240327
func Test_parseRequestHeader(t *testing.T) {
241328
for _, tt := range []struct {
242329
name string
@@ -865,6 +952,7 @@ func Benchmark_parseRequestBody_reader_with_SetContentLength(b *testing.B) {
865952
}
866953
}
867954
}
955+
868956
func Benchmark_parseRequestBody_reader_without_SetContentLength(b *testing.B) {
869957
c := New()
870958
r := c.R()

0 commit comments

Comments
 (0)