From 724d63220aaebf3b52aac6575b58ec1988c5e1cf Mon Sep 17 00:00:00 2001 From: puppy4c Date: Thu, 4 Apr 2024 14:32:51 +0800 Subject: [PATCH 1/4] SpringMvcContract support parse params --- .../openfeign/support/SpringMvcContract.java | 56 ++++++++++++- .../support/SpringMvcContractTests.java | 79 ++++++++++++++++++- 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java index f4e445342..1cb1c976a 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2023 the original author or authors. + * Copyright 2013-2024 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. @@ -87,6 +87,7 @@ * @author Darren Foong * @author Ram Anaswara * @author Sam Kruglov + * @author Tang Xiong */ public class SpringMvcContract extends Contract.BaseContract implements ResourceLoaderAware { @@ -244,6 +245,9 @@ protected void processAnnotationOnMethod(MethodMetadata data, Annotation methodA // headers parseHeaders(data, method, methodMapping); + // params + parseParams(data, method, methodMapping); + data.indexToExpander(new LinkedHashMap<>()); } @@ -354,6 +358,19 @@ private void parseHeaders(MethodMetadata md, Method method, RequestMapping annot } } + private void parseParams(MethodMetadata data, Method method, RequestMapping methodMapping) { + String[] params = methodMapping.params(); + if (params == null || params.length == 0) { + return; + } + for (String param : params) { + NameValueResolver nameValueResolver = new NameValueResolver(param); + if (!nameValueResolver.isNegated()) { + data.template().query(resolve(nameValueResolver.getName()), resolve(nameValueResolver.getValue())); + } + } + } + private Map, AnnotatedParameterProcessor> toAnnotatedArgumentProcessorMap( List processors) { Map, AnnotatedParameterProcessor> result = new HashMap<>(); @@ -465,4 +482,41 @@ public Collection setTemplateParameter(String name, Collection r } + private static class NameValueResolver { + + private final String name; + + private final String value; + + private final boolean isNegated; + + 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; + } + 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); + } + } + + public String getName() { + return name; + } + + public String getValue() { + return value; + } + + public boolean isNegated() { + return isNegated; + } + + } + } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java index 9f95ba515..a8a2eb8c8 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2023 the original author or authors. + * Copyright 2013-2024 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. @@ -82,6 +82,7 @@ * @author Szymon Linowski * @author Sam Kruglov * @author Bhavya Agrawal + * @author Tang Xiong **/ class SpringMvcContractTests { @@ -462,6 +463,60 @@ void testProcessAnnotations_MapParams() throws Exception { assertThat(data.queryMapIndex().intValue()).isEqualTo(0); } + @Test + void testProcessAnnotations_ParseParams_SingleParam() throws Exception { + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("singleParam"); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test?p1=1"); + assertThat(data.template().method()).isEqualTo("GET"); + } + + @Test + void testProcessAnnotations_ParseParams_MultipleParams() throws Exception { + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("multipleParams"); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test?p1=1&p2=2"); + assertThat(data.template().method()).isEqualTo("GET"); + } + + @Test + void testProcessAnnotations_ParseParams_MixParams() throws Exception { + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("mixParams"); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test?p1=1&p2"); + assertThat(data.template().method()).isEqualTo("GET"); + } + + @Test + void testProcessAnnotations_ParseParams_SingleParamWithoutValue() throws Exception { + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("singleParamWithoutValue"); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test?p1"); + assertThat(data.template().method()).isEqualTo("GET"); + } + + @Test + void testProcessAnnotations_ParseParams_MultipleParamsWithoutValue() throws Exception { + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("multipleParamsWithoutValue"); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test?p1&p2"); + assertThat(data.template().method()).isEqualTo("GET"); + } + + @Test + void testProcessAnnotations_ParseParams_NotEqualParams() throws Exception { + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("notEqualParams"); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test"); + assertThat(data.template().method()).isEqualTo("GET"); + } + @Test void testProcessHeaders() throws Exception { Method method = TestTemplate_Headers.class.getDeclaredMethod("getTest", String.class); @@ -750,6 +805,28 @@ public interface TestTemplate_MapParams { } + public interface TestTemplate_ParseParams { + + @GetMapping(value = "test", params = "p1=1") + ResponseEntity singleParam(); + + @GetMapping(value = "test", params = { "p1=1", "p2=2" }) + ResponseEntity multipleParams(); + + @GetMapping(value = "test", params = { "p1" }) + ResponseEntity singleParamWithoutValue(); + + @GetMapping(value = "test", params = { "p1", "p2" }) + ResponseEntity multipleParamsWithoutValue(); + + @GetMapping(value = "test", params = { "p1=1", "p2" }) + ResponseEntity mixParams(); + + @GetMapping(value = "test", params = { "p1!=1" }) + ResponseEntity notEqualParams(); + + } + public interface TestTemplate_HeaderMap { @GetMapping("/headerMap") From c26d64e5895e306c70339a0be79a3a5e52fc0a51 Mon Sep 17 00:00:00 2001 From: puppy4c Date: Sat, 20 Apr 2024 18:14:32 +0800 Subject: [PATCH 2/4] Address PR Comments --- .../ROOT/pages/spring-cloud-openfeign.adoc | 2 +- .../openfeign/support/SpringMvcContract.java | 16 +++++++++------- .../support/SpringMvcContractTests.java | 15 +++++++++++++-- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc b/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc index b59fbd1c6..266aedd0b 100644 --- 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 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/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java index 1cb1c976a..8686777fa 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java +++ b/spring-cloud-openfeign-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/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java index a8a2eb8c8..1d8197608 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java +++ b/spring-cloud-openfeign-core/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 notEqualParams(); + @GetMapping(value = "test", params = { "p1=1" }) + ResponseEntity paramsAndRequestParam(@RequestParam("p2") String p2); + } public interface TestTemplate_HeaderMap { From 2e44717b672f929b70ac58f2e44b7db67d3cf7c2 Mon Sep 17 00:00:00 2001 From: puppy4c Date: Wed, 24 Apr 2024 21:11:19 +0800 Subject: [PATCH 3/4] Address PR Comments --- .../ROOT/pages/spring-cloud-openfeign.adoc | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc b/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc index 266aedd0b..310621311 100644 --- a/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc +++ b/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc @@ -701,6 +701,36 @@ public interface DemoClient { You can also disable the feature via property `spring.cloud.openfeign.cache.enabled=false`. + +[[spring-requestmapping-support]] +=== Spring @RequestMapping Support + +Spring Cloud OpenFeign provides support for the Spring `@RequestMapping` annotation and its derived annotations (such as `@GetMapping`, `@PostMapping`, etc.) support. +The attributes on the `@RequestMapping` annotation (including `value`, `method`, `params`, `headers`, `consumes`, and `produces`) will be parsed by `SpringMvcContract` as the content of the request. + + +For example: + +Define an interface using the `params` attribute. + +[source,java,indent=0] +---- +@FeignClient("demo") +public interface DemoTemplate { + + @PostMapping(value = "/stores/{storeId}", params = "mode=upsert") + Store update(@PathVariable("storeId") Long storeId, Store store); +} +---- + +In the above example, the request url will be resolved to `/stores/{storeId}?mode=upsert`. + +The params attribute also supports the use of multiple `key=value` or only one `key`. For example: + + +- When `params = { "key1=v1", "key2=v2" }`, the request url will be parsed as `/stores/{storeId}?key1=v1&key2=v2`. +- When `params = "key"`, the request url will be parsed as `/stores/{storeId}?key`. + + + [[feign-querymap-support]] === Feign @QueryMap support From 8a9da4a71a48ccce5f7af9d2f85666330df06d0e Mon Sep 17 00:00:00 2001 From: puppy4c Date: Thu, 9 May 2024 21:00:58 +0800 Subject: [PATCH 4/4] Address PR Comments --- .../modules/ROOT/pages/spring-cloud-openfeign.adoc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc b/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc index 310621311..38e569e7e 100644 --- a/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc +++ b/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc @@ -705,11 +705,11 @@ You can also disable the feature via property `spring.cloud.openfeign.cache.enab [[spring-requestmapping-support]] === Spring @RequestMapping Support -Spring Cloud OpenFeign provides support for the Spring `@RequestMapping` annotation and its derived annotations (such as `@GetMapping`, `@PostMapping`, etc.) support. -The attributes on the `@RequestMapping` annotation (including `value`, `method`, `params`, `headers`, `consumes`, and `produces`) will be parsed by `SpringMvcContract` as the content of the request. +Spring Cloud OpenFeign provides support for the Spring `@RequestMapping` annotation and its derived annotations (such as `@GetMapping`, `@PostMapping`, and others) support. +The attributes on the `@RequestMapping` annotation (including `value`, `method`, `params`, `headers`, `consumes`, and `produces`) are parsed by `SpringMvcContract` as the content of the request. -For example: +Consider the following example: Define an interface using the `params` attribute. @@ -723,11 +723,11 @@ public interface DemoTemplate { } ---- -In the above example, the request url will be resolved to `/stores/{storeId}?mode=upsert`. + -The params attribute also supports the use of multiple `key=value` or only one `key`. For example: + +In the above example, the request url is resolved to `/stores/{storeId}?mode=upsert`. + +The params attribute also supports the use of multiple `key=value` or only one `key`: + -- When `params = { "key1=v1", "key2=v2" }`, the request url will be parsed as `/stores/{storeId}?key1=v1&key2=v2`. -- When `params = "key"`, the request url will be parsed as `/stores/{storeId}?key`. +- When `params = { "key1=v1", "key2=v2" }`, the request url is parsed as `/stores/{storeId}?key1=v1&key2=v2`. +- When `params = "key"`, the request url is parsed as `/stores/{storeId}?key`.