Address PR Comments

spring-cloud · Apr 20, 2024 · c26d64e · c26d64e
1 parent 724d632
commit c26d64e
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 10 deletions.
diff --git a/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc b/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc
@@ -45,7 +45,7 @@ public interface StoreClient {
 	@GetMapping("/stores")
 	Page<Store> getStores(Pageable pageable);
 
-	@PostMapping(value = "/stores/{storeId}", consumes = "application/json")
+	@PostMapping(value = "/stores/{storeId}", consumes = "application/json", params = "mode=upsert")
 	Store update(@PathVariable("storeId") Long storeId, Store store);
 
 	@DeleteMapping("/stores/{storeId:\\d+}")

diff --git a/...ign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java b/...ign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java
@@ -368,6 +368,9 @@ private void parseParams(MethodMetadata data, Method method, RequestMapping meth
 			if (!nameValueResolver.isNegated()) {
 				data.template().query(resolve(nameValueResolver.getName()), resolve(nameValueResolver.getValue()));
 			}
+			else {
+				throw new IllegalArgumentException("Negated params are not supported: " + param);
+			}
 		}
 	}
 
@@ -493,15 +496,14 @@ private static class NameValueResolver {
 		NameValueResolver(String expression) {
 			int separator = expression.indexOf('=');
 			if (separator == -1) {
-				this.isNegated = expression.startsWith("!");
-				this.name = (this.isNegated ? expression.substring(1) : expression);
-				this.value = null;
+				isNegated = expression.startsWith("!");
+				name = (isNegated ? expression.substring(1) : expression);
+				value = null;
 			}
 			else {
-				this.isNegated = (separator > 0) && (expression.charAt(separator - 1) == '!');
-				this.name = (this.isNegated ? expression.substring(0, separator - 1)
-						: expression.substring(0, separator));
-				this.value = expression.substring(separator + 1);
+				isNegated = (separator > 0) && (expression.charAt(separator - 1) == '!');
+				name = (isNegated ? expression.substring(0, separator - 1) : expression.substring(0, separator));
+				value = expression.substring(separator + 1);
 			}
 		}
 

diff --git a/...ore/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java b/...ore/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java
@@ -510,10 +510,18 @@ void testProcessAnnotations_ParseParams_MultipleParamsWithoutValue() throws Exce
 
 	@Test
 	void testProcessAnnotations_ParseParams_NotEqualParams() throws Exception {
-		Method method = TestTemplate_ParseParams.class.getDeclaredMethod("notEqualParams");
+		assertThatIllegalArgumentException().isThrownBy(() -> {
+			Method method = TestTemplate_ParseParams.class.getDeclaredMethod("notEqualParams");
+			contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
+		});
+	}
+
+	@Test
+	void testProcessAnnotations_ParseParams_ParamsAndRequestParam() throws Exception {
+		Method method = TestTemplate_ParseParams.class.getDeclaredMethod("paramsAndRequestParam", String.class);
 		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
 
-		assertThat(data.template().url()).isEqualTo("/test");
+		assertThat(data.template().url()).isEqualTo("/test?p1=1&p2={p2}");
 		assertThat(data.template().method()).isEqualTo("GET");
 	}
 
@@ -825,6 +833,9 @@ public interface TestTemplate_ParseParams {
 		@GetMapping(value = "test", params = { "p1!=1" })
 		ResponseEntity<TestObject> notEqualParams();
 
+		@GetMapping(value = "test", params = { "p1=1" })
+		ResponseEntity<TestObject> paramsAndRequestParam(@RequestParam("p2") String p2);
+
 	}
 
 	public interface TestTemplate_HeaderMap {