Benchmarks and Improvements for parseRequestURL function

middleware.go

@@ -27,27 +27,76 @@ const debugRequestLogKey = "__restyDebugRequestLog"
 //_______________________________________________________________________
 
 func parseRequestURL(c *Client, r *Request) error {
-	// GitHub #103 Path Params
-	if len(r.PathParams) > 0 {
+	if l := len(c.PathParams) + len(c.RawPathParams) + len(r.PathParams) + len(r.RawPathParams); l > 0 {
+		params := make(map[string]string, l)
+
+		// GitHub #103 Path Params
 		for p, v := range r.PathParams {
-			r.URL = strings.Replace(r.URL, "{"+p+"}", url.PathEscape(v), -1)
+			params[p] = url.PathEscape(v)
 		}
-	}
-	if len(c.PathParams) > 0 {
 		for p, v := range c.PathParams {
-			r.URL = strings.Replace(r.URL, "{"+p+"}", url.PathEscape(v), -1)
+			if _, ok := params[p]; !ok {
+				params[p] = url.PathEscape(v)
+			}
 		}
-	}
 
-	// GitHub #663 Raw Path Params
-	if len(r.RawPathParams) > 0 {
+		// GitHub #663 Raw Path Params
 		for p, v := range r.RawPathParams {
-			r.URL = strings.Replace(r.URL, "{"+p+"}", v, -1)
+			if _, ok := params[p]; !ok {
+				params[p] = v
+			}
 		}
-	}
-	if len(c.RawPathParams) > 0 {
 		for p, v := range c.RawPathParams {
-			r.URL = strings.Replace(r.URL, "{"+p+"}", v, -1)
+			if _, ok := params[p]; !ok {
+				params[p] = v
+			}
+		}
+
+		if len(params) > 0 {
+			var prev int
+			buf := acquireBuffer()
+			defer releaseBuffer(buf)
+			// search for the next or first opened curly bracket
+			for curr := strings.Index(r.URL, "{"); curr > prev; curr = prev + strings.Index(r.URL[prev:], "{") {
+				// write everything form the previous position up to the current
+				if curr > prev {
+					buf.WriteString(r.URL[prev:curr])
+				}
+				// search for the closed curly bracket from current position
+				next := curr + strings.Index(r.URL[curr:], "}")
+				// if not found, then write the remainder and exit
+				if next < curr {
+					buf.WriteString(r.URL[curr:])
+					prev = len(r.URL)
+					break
+				}
+				// special case for {}, without parameter's name
+				if next == curr+1 {
+					buf.WriteString("{}")
+				} else {
+					// check for the replacement
+					key := r.URL[curr+1 : next]
+					value, ok := params[key]
+					/// keep the original string if the replacement not found
+					if !ok {
+						value = r.URL[curr : next+1]
+					}
+					buf.WriteString(value)
+				}
+
+				// set the previous position after the closed curly bracket
+				prev = next + 1
+				if prev >= len(r.URL) {
+					break
+				}
+			}
+			if buf.Len() > 0 {
+				// write remainder
+				if prev < len(r.URL) {
+					buf.WriteString(r.URL[prev:])
+				}
+				r.URL = buf.String()
+			}
 		}
 	}
 
@@ -82,32 +131,26 @@ func parseRequestURL(c *Client, r *Request) error {
 	}
 
 	// Adding Query Param
-	query := make(url.Values)
-	for k, v := range c.QueryParam {
-		for _, iv := range v {
-			query.Add(k, iv)
-		}
-	}
-
-	for k, v := range r.QueryParam {
-		// remove query param from client level by key
-		// since overrides happens for that key in the request
-		query.Del(k)
+	if len(c.QueryParam)+len(r.QueryParam) > 0 {
+		for k, v := range c.QueryParam {
+			// skip query parameter if it was set in request
+			if _, ok := r.QueryParam[k]; ok {
+				continue
+			}
 
-		for _, iv := range v {
-			query.Add(k, iv)
+			r.QueryParam[k] = v[:]
 		}
-	}
 
-	// GitHub #123 Preserve query string order partially.
-	// Since not feasible in `SetQuery*` resty methods, because
-	// standard package `url.Encode(...)` sorts the query params
-	// alphabetically
-	if len(query) > 0 {
-		if IsStringEmpty(reqURL.RawQuery) {
-			reqURL.RawQuery = query.Encode()
-		} else {
-			reqURL.RawQuery = reqURL.RawQuery + "&" + query.Encode()
+		// GitHub #123 Preserve query string order partially.
+		// Since not feasible in `SetQuery*` resty methods, because
+		// standard package `url.Encode(...)` sorts the query params
+		// alphabetically
+		if len(r.QueryParam) > 0 {
+			if IsStringEmpty(reqURL.RawQuery) {
+				reqURL.RawQuery = r.QueryParam.Encode()
+			} else {
+				reqURL.RawQuery = reqURL.RawQuery + "&" + r.QueryParam.Encode()
+			}
 		}
 	}
 

middleware_test.go

@@ -105,6 +105,46 @@ func Test_parseRequestURL(t *testing.T) {
 			},
 			expectedURL: "https://example.com/4%2F5/6/7",
 		},
+		{
+			name: "empty path parameter in URL",
+			init: func(c *Client, r *Request) {
+				r.SetPathParams(map[string]string{
+					"bar": "4",
+				})
+				r.URL = "https://example.com/{}/{bar}"
+			},
+			expectedURL: "https://example.com/%7B%7D/4",
+		},
+		{
+			name: "not closed path parameter in URL",
+			init: func(c *Client, r *Request) {
+				r.SetPathParams(map[string]string{
+					"foo": "4",
+				})
+				r.URL = "https://example.com/{foo}/{bar/1"
+			},
+			expectedURL: "https://example.com/4/%7Bbar/1",
+		},
+		{
+			name: "extra path parameter in URL",
+			init: func(c *Client, r *Request) {
+				r.SetPathParams(map[string]string{
+					"foo": "1",
+				})
+				r.URL = "https://example.com/{foo}/{bar}"
+			},
+			expectedURL: "https://example.com/1/%7Bbar%7D",
+		},
+		{
+			name: " path parameter with remainder",
+			init: func(c *Client, r *Request) {
+				r.SetPathParams(map[string]string{
+					"foo": "1",
+				})
+				r.URL = "https://example.com/{foo}/2"
+			},
+			expectedURL: "https://example.com/1/2",
+		},
 		{
 			name: "using BaseURL with absolute URL in request",
 			init: func(c *Client, r *Request) {
@@ -189,13 +229,32 @@ func Test_parseRequestURL(t *testing.T) {
 					"foo": "1", // ignored, because of the "foo" parameter in request
 					"bar": "2",
 				})
-				c.SetQueryParams(map[string]string{
+				r.SetQueryParams(map[string]string{
 					"foo": "3",
 				})
 				r.URL = "https://example.com/"
 			},
 			expectedURL: "https://example.com/?foo=3&bar=2",
 		},
+		{
+			name: "adding query parameters by request to URL with existent",
+			init: func(c *Client, r *Request) {
+				r.SetQueryParams(map[string]string{
+					"bar": "2",
+				})
+				r.URL = "https://example.com/?foo=1"
+			},
+			expectedURL: "https://example.com/?foo=1&bar=2",
+		},
+		{
+			name: "adding query parameters by request with multiple values",
+			init: func(c *Client, r *Request) {
+				r.QueryParam.Add("foo", "1")
+				r.QueryParam.Add("foo", "2")
+				r.URL = "https://example.com/"
+			},
+			expectedURL: "https://example.com/?foo=1&foo=2",
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			c := New()
@@ -216,27 +275,55 @@ func Test_parseRequestURL(t *testing.T) {
 			if expectedURL.String() != actualURL.String() {
 				t.Errorf("r.URL = %q does not match expected %q", r.URL, tt.expectedURL)
 			}
-			if len(expectedQuery) != len(actualQuery) {
+			if !reflect.DeepEqual(expectedQuery, actualQuery) {
 				t.Errorf("r.URL = %q does not match expected %q", r.URL, tt.expectedURL)
 			}
-			for name, expected := range expectedQuery {
-				actual, ok := actualQuery[name]
-				if !ok {
-					t.Errorf("r.URL = %q does not match expected %q", r.URL, tt.expectedURL)
-				}
-				if len(expected) != len(actual) {
-					t.Errorf("r.URL = %q does not match expected %q", r.URL, tt.expectedURL)
-				}
-				for i, v := range expected {
-					if v != actual[i] {
-						t.Errorf("r.URL = %q does not match expected %q", r.URL, tt.expectedURL)
-					}
-				}
-			}
 		})
 	}
 }
 
+func Benchmark_parseRequestURL_PathParams(b *testing.B) {
+	c := New().SetPathParams(map[string]string{
+		"foo": "1",
+		"bar": "2",
+	}).SetRawPathParams(map[string]string{
+		"foo": "3",
+		"xyz": "4",
+	})
+	r := c.R().SetPathParams(map[string]string{
+		"foo": "5",
+		"qwe": "6",
+	}).SetRawPathParams(map[string]string{
+		"foo": "7",
+		"asd": "8",
+	})
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		r.URL = "https://example.com/{foo}/{bar}/{xyz}/{qwe}/{asd}"
+		if err := parseRequestURL(c, r); err != nil {
+			b.Errorf("parseRequestURL() error = %v", err)
+		}
+	}
+}
+
+func Benchmark_parseRequestURL_QueryParams(b *testing.B) {
+	c := New().SetQueryParams(map[string]string{
+		"foo": "1",
+		"bar": "2",
+	})
+	r := c.R().SetQueryParams(map[string]string{
+		"foo": "5",
+		"qwe": "6",
+	})
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		r.URL = "https://example.com/"
+		if err := parseRequestURL(c, r); err != nil {
+			b.Errorf("parseRequestURL() error = %v", err)
+		}
+	}
+}
+
 func Test_parseRequestHeader(t *testing.T) {
 	for _, tt := range []struct {
 		name           string
@@ -865,6 +952,7 @@ func Benchmark_parseRequestBody_reader_with_SetContentLength(b *testing.B) {
 		}
 	}
 }
+
 func Benchmark_parseRequestBody_reader_without_SetContentLength(b *testing.B) {
 	c := New()
 	r := c.R()