From 951588a6123cdd9e5091080dcda666fa428a6da9 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Thu, 13 Mar 2025 13:25:27 +0100 Subject: [PATCH 1/5] Merge remote-tracking branch 'origin/4.2.x' # Conflicts: # pom.xml --- .../cloud/openfeign/support/SpringMvcContract.java | 7 +++++-- .../openfeign/support/SpringMvcContractTests.java | 13 +++++++------ 2 files changed, 12 insertions(+), 8 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 9d9d7a8c..5bf64653 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-2024 the original author or authors. + * Copyright 2013-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. @@ -369,7 +369,10 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource data.template().query(resolve(nameValueResolver.getName()), resolve(nameValueResolver.getValue())); } else { - throw new IllegalArgumentException("Negated params are not supported: " + param); + if (LOG.isDebugEnabled()) { + LOG.debug("Negated params are not supported by Feign and ignored during parameter processing: " + + param); + } } } } 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 02219e2c..0266a5ec 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-2024 the original author or authors. + * Copyright 2013-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. @@ -68,6 +68,7 @@ import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE; import static feign.CollectionFormat.CSV; import static feign.CollectionFormat.SSV; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -509,11 +510,11 @@ class SpringMvcContractTests { } @Test - void testProcessAnnotations_ParseParams_NotEqualParams() throws Exception { - assertThatIllegalArgumentException().isThrownBy(() -> { - Method method = TestTemplate_ParseParams.class.getDeclaredMethod("notEqualParams"); + void testProcessAnnotations_ParseParams_NegatedParams() { + assertThatCode(() -> { + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("negatedParams"); contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - }); + }).doesNotThrowAnyException(); } @Test @@ -831,7 +832,7 @@ class SpringMvcContractTests { ResponseEntity mixParams(); @GetMapping(value = "test", params = { "p1!=1" }) - ResponseEntity notEqualParams(); + ResponseEntity negatedParams(); @GetMapping(value = "test", params = { "p1=1" }) ResponseEntity paramsAndRequestParam(@RequestParam("p2") String p2); From b822670fdc8fe093beb7fb2c6e1e519e7f0b2f8d Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Thu, 13 Mar 2025 13:25:27 +0100 Subject: [PATCH 2/5] Merge remote-tracking branch 'origin/4.2.x' --- .../cloud/openfeign/support/SpringMvcContract.java | 7 +++++-- .../openfeign/support/SpringMvcContractTests.java | 13 +++++++------ 2 files changed, 12 insertions(+), 8 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 9d9d7a8c..5bf64653 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-2024 the original author or authors. + * Copyright 2013-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. @@ -369,7 +369,10 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource data.template().query(resolve(nameValueResolver.getName()), resolve(nameValueResolver.getValue())); } else { - throw new IllegalArgumentException("Negated params are not supported: " + param); + if (LOG.isDebugEnabled()) { + LOG.debug("Negated params are not supported by Feign and ignored during parameter processing: " + + param); + } } } } 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 02219e2c..0266a5ec 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-2024 the original author or authors. + * Copyright 2013-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. @@ -68,6 +68,7 @@ import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE; import static feign.CollectionFormat.CSV; import static feign.CollectionFormat.SSV; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -509,11 +510,11 @@ class SpringMvcContractTests { } @Test - void testProcessAnnotations_ParseParams_NotEqualParams() throws Exception { - assertThatIllegalArgumentException().isThrownBy(() -> { - Method method = TestTemplate_ParseParams.class.getDeclaredMethod("notEqualParams"); + void testProcessAnnotations_ParseParams_NegatedParams() { + assertThatCode(() -> { + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("negatedParams"); contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - }); + }).doesNotThrowAnyException(); } @Test @@ -831,7 +832,7 @@ class SpringMvcContractTests { ResponseEntity mixParams(); @GetMapping(value = "test", params = { "p1!=1" }) - ResponseEntity notEqualParams(); + ResponseEntity negatedParams(); @GetMapping(value = "test", params = { "p1=1" }) ResponseEntity paramsAndRequestParam(@RequestParam("p2") String p2); From 79a78d0fa3ad96064bc2d0296461251555bee433 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Thu, 13 Mar 2025 13:27:44 +0100 Subject: [PATCH 3/5] Revert "Merge remote-tracking branch 'origin/4.2.x'" This reverts commit b822670fdc8fe093beb7fb2c6e1e519e7f0b2f8d. --- .../cloud/openfeign/support/SpringMvcContract.java | 7 ++----- .../openfeign/support/SpringMvcContractTests.java | 13 ++++++------- 2 files changed, 8 insertions(+), 12 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 5bf64653..9d9d7a8c 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-2025 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. @@ -369,10 +369,7 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource data.template().query(resolve(nameValueResolver.getName()), resolve(nameValueResolver.getValue())); } else { - if (LOG.isDebugEnabled()) { - LOG.debug("Negated params are not supported by Feign and ignored during parameter processing: " - + param); - } + throw new IllegalArgumentException("Negated params are not supported: " + param); } } } 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 0266a5ec..02219e2c 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-2025 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. @@ -68,7 +68,6 @@ import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE; import static feign.CollectionFormat.CSV; import static feign.CollectionFormat.SSV; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -510,11 +509,11 @@ class SpringMvcContractTests { } @Test - void testProcessAnnotations_ParseParams_NegatedParams() { - assertThatCode(() -> { - Method method = TestTemplate_ParseParams.class.getDeclaredMethod("negatedParams"); + void testProcessAnnotations_ParseParams_NotEqualParams() throws Exception { + assertThatIllegalArgumentException().isThrownBy(() -> { + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("notEqualParams"); contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - }).doesNotThrowAnyException(); + }); } @Test @@ -832,7 +831,7 @@ class SpringMvcContractTests { ResponseEntity mixParams(); @GetMapping(value = "test", params = { "p1!=1" }) - ResponseEntity negatedParams(); + ResponseEntity notEqualParams(); @GetMapping(value = "test", params = { "p1=1" }) ResponseEntity paramsAndRequestParam(@RequestParam("p2") String p2); From 3c0c10fe11b538364daaf773656638f830be2d72 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Thu, 13 Mar 2025 13:30:34 +0100 Subject: [PATCH 4/5] Ignore negated params. Fixes gh-1134. --- .../cloud/openfeign/support/SpringMvcContract.java | 7 +++++-- .../openfeign/support/SpringMvcContractTests.java | 13 +++++++------ 2 files changed, 12 insertions(+), 8 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 9d9d7a8c..5bf64653 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-2024 the original author or authors. + * Copyright 2013-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. @@ -369,7 +369,10 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource data.template().query(resolve(nameValueResolver.getName()), resolve(nameValueResolver.getValue())); } else { - throw new IllegalArgumentException("Negated params are not supported: " + param); + if (LOG.isDebugEnabled()) { + LOG.debug("Negated params are not supported by Feign and ignored during parameter processing: " + + param); + } } } } 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 02219e2c..0266a5ec 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-2024 the original author or authors. + * Copyright 2013-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. @@ -68,6 +68,7 @@ import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE; import static feign.CollectionFormat.CSV; import static feign.CollectionFormat.SSV; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -509,11 +510,11 @@ class SpringMvcContractTests { } @Test - void testProcessAnnotations_ParseParams_NotEqualParams() throws Exception { - assertThatIllegalArgumentException().isThrownBy(() -> { - Method method = TestTemplate_ParseParams.class.getDeclaredMethod("notEqualParams"); + void testProcessAnnotations_ParseParams_NegatedParams() { + assertThatCode(() -> { + Method method = TestTemplate_ParseParams.class.getDeclaredMethod("negatedParams"); contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - }); + }).doesNotThrowAnyException(); } @Test @@ -831,7 +832,7 @@ class SpringMvcContractTests { ResponseEntity mixParams(); @GetMapping(value = "test", params = { "p1!=1" }) - ResponseEntity notEqualParams(); + ResponseEntity negatedParams(); @GetMapping(value = "test", params = { "p1=1" }) ResponseEntity paramsAndRequestParam(@RequestParam("p2") String p2); From 535cdc013705760b30ce31e646b749745fcb3bbe Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Thu, 13 Mar 2025 13:43:34 +0100 Subject: [PATCH 5/5] Simplify private method signatures and checks. --- .../openfeign/support/SpringMvcContract.java | 28 ++++++++----------- .../support/SpringMvcContractTests.java | 8 +----- 2 files changed, 12 insertions(+), 24 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 5bf64653..2b536cc0 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 @@ -237,16 +237,16 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource } // produces - parseProduces(data, method, methodMapping); + parseProduces(data, methodMapping); // consumes - parseConsumes(data, method, methodMapping); + parseConsumes(data, methodMapping); // headers - parseHeaders(data, method, methodMapping); + parseHeaders(data, methodMapping); // params - parseParams(data, method, methodMapping); + parseParams(data, methodMapping); data.indexToExpander(new LinkedHashMap<>()); } @@ -329,7 +329,7 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource return false; } - private void parseProduces(MethodMetadata md, Method method, RequestMapping annotation) { + private void parseProduces(MethodMetadata md, RequestMapping annotation) { String[] serverProduces = annotation.produces(); String clientAccepts = serverProduces.length == 0 ? null : emptyToNull(serverProduces[0]); if (clientAccepts != null) { @@ -337,7 +337,7 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource } } - private void parseConsumes(MethodMetadata md, Method method, RequestMapping annotation) { + private void parseConsumes(MethodMetadata md, RequestMapping annotation) { String[] serverConsumes = annotation.consumes(); String clientProduces = serverConsumes.length == 0 ? null : emptyToNull(serverConsumes[0]); if (clientProduces != null) { @@ -345,9 +345,9 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource } } - private void parseHeaders(MethodMetadata md, Method method, RequestMapping annotation) { + private void parseHeaders(MethodMetadata md, RequestMapping annotation) { // TODO: only supports one header value per key - if (annotation.headers() != null && annotation.headers().length > 0) { + if (annotation.headers() != null) { for (String header : annotation.headers()) { int index = header.indexOf('='); if (!header.contains("!=") && index >= 0) { @@ -358,9 +358,9 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource } } - private void parseParams(MethodMetadata data, Method method, RequestMapping methodMapping) { + private void parseParams(MethodMetadata data, RequestMapping methodMapping) { String[] params = methodMapping.params(); - if (params == null || params.length == 0) { + if (params == null) { return; } for (String param : params) { @@ -438,13 +438,7 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource return false; } - private static class ConvertingExpanderFactory { - - private final ConversionService conversionService; - - ConvertingExpanderFactory(ConversionService conversionService) { - this.conversionService = conversionService; - } + private record ConvertingExpanderFactory(ConversionService conversionService) { Param.Expander getExpander(TypeDescriptor typeDescriptor) { return value -> { 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 0266a5ec..67df1df1 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 @@ -1003,13 +1003,7 @@ class SpringMvcContractTests { @Override public String toString() { - return new StringBuilder("TestObject{").append("something='") - .append(something) - .append("', ") - .append("number=") - .append(number) - .append("}") - .toString(); + return "TestObject{" + "something='" + something + "', " + "number=" + number + "}"; } }