Do not warn if first parameter of GetMapping is of type URL#1327
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes incorrect startup warning emitted by SpringMvcContract for @GetMapping methods whose only parameter is a dynamic java.net.URI (the case described in #1326).
Changes:
- Adjust GET/unannotated-parameter warning logic to skip a leading
URIparameter when deciding whether to warn. - Add contract tests that capture log output to verify warning/no-warning behavior for
URIand non-URIparameters.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java | Refactors warning condition into shouldWarnAboutUnannotatedGetParameters() and suppresses warnings for single-URI GET methods. |
| spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java | Adds OutputCaptureExtension and new tests asserting warning behavior for URI vs. unannotated params. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private boolean shouldWarnAboutUnannotatedGetParameters(Method method) { | ||
| Parameter[] params = method.getParameters(); | ||
| if (params.length == 1 && params[0].getType() == URI.class) { | ||
| return false; | ||
| } | ||
| int fromIndex = (params.length > 0 && params[0].getType() == URI.class) ? 1 : 0; | ||
| return !hasHttpAnnotations(method, fromIndex); |
There was a problem hiding this comment.
PR title mentions the first parameter being of type URL, but the implementation and tests only special-case java.net.URI. Either update the PR title (and/or issue reference) to say URI, or extend the implementation to also treat java.net.URL (if that’s the intended supported dynamic-target parameter type) the same way to avoid user confusion and incomplete coverage of the reported behavior.
ryanjbaxter
force-pushed
the
deal-with-warning-uri-param
branch
from
ed3f41d to
0b1397f
Compare
Fixes #1326