From d927d64c40decba86e1ec44cedda7ddecf9c0aad Mon Sep 17 00:00:00 2001
From: Brian Clozel <brian.clozel@broadcom.com>
Date: Thu, 2 Jan 2025 20:00:38 +0100
Subject: [PATCH] Improve query params in uri KeyValue with HTTP interface
 client

Prior to this commit, the HTTP interface client would create URI
templates and name query params like so:
"?{queryParam0}={queryParam0[0]}".
While technically correct, the URI template is further used in
observations as a KeyValue. This means that several service methods
could result in having the exact same URI template even if they produce
a different set of query params.

This commit improves the naming of query params in the generated URI
templates for better observability integration.

Closes gh-34176
---
 .../service/invoker/HttpRequestValues.java    |  6 ++--
 .../invoker/HttpRequestValuesTests.java       | 28 +++++++++++--------
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java
index 7bf09599ae2b..e2c95915dcab 100644
--- a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java
+++ b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2024 the original author or authors.
+ * Copyright 2002-2025 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -492,16 +492,14 @@ private String appendQueryParams(
 				String uriTemplate, Map<String, String> uriVars, MultiValueMap<String, String> requestParams) {
 
 			UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(uriTemplate);
-			int i = 0;
 			for (Map.Entry<String, List<String>> entry : requestParams.entrySet()) {
-				String nameVar = "queryParam" + i;
+				String nameVar = entry.getKey();
 				uriVars.put(nameVar, entry.getKey());
 				for (int j = 0; j < entry.getValue().size(); j++) {
 					String valueVar = nameVar + "[" + j + "]";
 					uriVars.put(valueVar, entry.getValue().get(j));
 					uriComponentsBuilder.queryParam("{" + nameVar + "}", "{" + valueVar + "}");
 				}
-				i++;
 			}
 			return uriComponentsBuilder.build().toUriString();
 		}
diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java
index 481da00d5f7e..f929e0c72bbe 100644
--- a/spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java
+++ b/spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2024 the original author or authors.
+ * Copyright 2002-2025 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -79,17 +79,17 @@ void queryParamsWithUriTemplate() {
 
 		assertThat(uriTemplate)
 				.isEqualTo("/path?" +
-						"{queryParam0}={queryParam0[0]}&" +
-						"{queryParam1}={queryParam1[0]}&" +
-						"{queryParam1}={queryParam1[1]}");
+						"{param1}={param1[0]}&" +
+						"{param2}={param2[0]}&" +
+						"{param2}={param2[1]}");
 
 		assertThat(requestValues.getUriVariables())
-				.containsOnlyKeys("queryParam0", "queryParam1", "queryParam0[0]", "queryParam1[0]", "queryParam1[1]")
-				.containsEntry("queryParam0", "param1")
-				.containsEntry("queryParam1", "param2")
-				.containsEntry("queryParam0[0]", "1st value")
-				.containsEntry("queryParam1[0]", "2nd value A")
-				.containsEntry("queryParam1[1]", "2nd value B");
+				.containsOnlyKeys("param1", "param2", "param1[0]", "param2[0]", "param2[1]")
+				.containsEntry("param1", "param1")
+				.containsEntry("param2", "param2")
+				.containsEntry("param1[0]", "1st value")
+				.containsEntry("param2[0]", "2nd value A")
+				.containsEntry("param2[1]", "2nd value B");
 
 		URI uri = UriComponentsBuilder.fromUriString(uriTemplate)
 				.encode()
@@ -144,7 +144,13 @@ void requestPartAndRequestParam() {
 		String uriTemplate = requestValues.getUriTemplate();
 		assertThat(uriTemplate).isNotNull();
 
-		assertThat(uriTemplate).isEqualTo("/path?{queryParam0}={queryParam0[0]}");
+		assertThat(uriTemplate).isEqualTo("/path?{query param}={query param[0]}");
+
+		URI uri = UriComponentsBuilder.fromUriString(uriTemplate)
+				.encode()
+				.build(requestValues.getUriVariables());
+		assertThat(uri.toString())
+				.isEqualTo("/path?query%20param=query%20value");
 
 		@SuppressWarnings("unchecked")
 		MultiValueMap<String, Object> map = (MultiValueMap<String, Object>) requestValues.getBodyValue();