-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Optional BeanParam support for JAXRS2Contract #1935
Merged
velo
merged 13 commits into
OpenFeign:master
from
JKomoroski:issue/1264/jaxrs2_support_bean_param
Feb 18, 2023
Merged
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
d9dd5cd
Add Optional BeanParam support for JAXRS2Contract
JKomoroski cd44762
Fix Code Style
JKomoroski a11b583
Commit Build Code Format Changes
JKomoroski f6cf42b
Roll Back Removal of braces
JKomoroski d3e5c25
Merge branch 'master' into issue/1264/jaxrs2_support_bean_param
velo 4011cc1
Merge branch 'master' into issue/1264/jaxrs2_support_bean_param
velo 635dbbe
Make sure jaxrs1 dependencies not avaiable on jaxrs2
velo 0bf4201
Merge branch 'master' into issue/1264/jaxrs2_support_bean_param
velo b866ffe
Update JAXRS2ContractWithBeanParamSupportTest.java
velo 9f5b484
Always Use Enable BeanParam Support
JKomoroski 365cbe5
Roll Back Test Changes
JKomoroski 524fb6d
Merge branch 'master' into issue/1264/jaxrs2_support_bean_param
velo 944b3f6
Merge branch 'master' into issue/1264/jaxrs2_support_bean_param
velo File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
94 changes: 94 additions & 0 deletions
94
jaxrs2/src/test/java/feign/jaxrs2/JAXRS2ContractWithBeanParamSupportTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
/* | ||
* Copyright 2012-2023 The Feign Authors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except | ||
* in compliance with the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software distributed under the License | ||
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express | ||
* or implied. See the License for the specific language governing permissions and limitations under | ||
* the License. | ||
*/ | ||
package feign.jaxrs2; | ||
|
||
import feign.MethodMetadata; | ||
import feign.jaxrs.JAXRSContract; | ||
import feign.jaxrs.JAXRSContractTest; | ||
import feign.jaxrs2.JAXRS2ContractWithBeanParamSupportTest.Jaxrs2Internals.BeanParamInput; | ||
import org.junit.Test; | ||
import javax.ws.rs.*; | ||
import javax.ws.rs.container.AsyncResponse; | ||
import javax.ws.rs.container.Suspended; | ||
import javax.ws.rs.core.Context; | ||
import javax.ws.rs.core.UriInfo; | ||
import static feign.assertj.FeignAssertions.assertThat; | ||
import static java.util.Arrays.asList; | ||
import static org.assertj.core.data.MapEntry.entry; | ||
|
||
/** | ||
* Tests interfaces defined per {@link JAXRS2Contract} are interpreted into expected | ||
* {@link feign .RequestTemplate template} instances. | ||
*/ | ||
public class JAXRS2ContractWithBeanParamSupportTest extends JAXRSContractTest { | ||
|
||
@Override | ||
protected JAXRSContract createContract() { | ||
return new JAXRS2Contract(true); | ||
} | ||
|
||
@Test | ||
public void injectJaxrsInternals() throws Exception { | ||
final MethodMetadata methodMetadata = | ||
parseAndValidateMetadata(Jaxrs2Internals.class, "inject", AsyncResponse.class, | ||
UriInfo.class); | ||
assertThat(methodMetadata.template()) | ||
.noRequestBody(); | ||
} | ||
|
||
@Test | ||
public void injectBeanParam() throws Exception { | ||
final MethodMetadata methodMetadata = | ||
parseAndValidateMetadata(Jaxrs2Internals.class, "beanParameters", BeanParamInput.class); | ||
assertThat(methodMetadata.template()) | ||
.noRequestBody(); | ||
|
||
assertThat(methodMetadata.template()) | ||
.hasHeaders(entry("X-Custom-Header", asList("{X-Custom-Header}"))); | ||
assertThat(methodMetadata.template()) | ||
.hasQueries(entry("query", asList("{query}"))); | ||
assertThat(methodMetadata.formParams()) | ||
.isNotEmpty() | ||
.containsExactly("form"); | ||
|
||
} | ||
|
||
|
||
@Path("/") | ||
public interface Jaxrs2Internals { | ||
@GET | ||
void inject(@Suspended AsyncResponse ar, @Context UriInfo info); | ||
|
||
@Path("/{path}") | ||
@POST | ||
void beanParameters(@BeanParam BeanParamInput beanParam); | ||
|
||
public class BeanParamInput { | ||
|
||
@PathParam("path") | ||
String path; | ||
|
||
@QueryParam("query") | ||
String query; | ||
|
||
@FormParam("form") | ||
String form; | ||
|
||
@HeaderParam("X-Custom-Header") | ||
String header; | ||
|
||
} | ||
} | ||
|
||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, before made sense to ignore any
@BeanParam
cause it was not implemented.Now that you fixed that shortcoming, I think it's fine to have it enabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if you would like me to make this change or if you plan to make it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think is a nice to have, but your call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed this change. Jdk8 appears to be failing again in CI. Not able to duplicate on my local