-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 support for JAX-RS @BeanParam #446
Comments
+1 for me. We're making a lot of use of @BeanParam now. |
+1 would be nice as it would help to cleanup the signature of the API, having the annotations in the bean parameter class instead of in the parameter list .... Thanks! |
+1 this is a feature our group needs as well |
+1 we're trying to incorporate swagger for a new project and would like to make use of @BeanParam but still have the benefit of swagger docs and the super-nice "try it now" testability for query params. |
+1 We're using implicit parameters to get this support now, but would be nice to not have to copy and paste boilerplate all over the place. |
Yes, not supported but we can target this for 1.3.5 |
+1 would be nice to have |
+1 |
I made a simple fix but it's based on my own modified branch. You can refer to the commit here. |
+1 (sad that @emilesvt and I both found and +1'ed this... me on a Friday night, but hey... who's judging?) |
This is awesome! Which milestone / release is this scheduled for? 1.3.5? Any target date to ship 1.3.5? |
Hi, just pushed to maven central, and documented here: https://groups.google.com/forum/#!topic/swagger-swaggersocket/cXCnE3cyKBA |
That's just great! Thanks a lot 👍 |
Hi Tony: not seeing where the BeanParam scanning is working yet with 1.3.5. When used on a resource method, I don't see the params added. What's the appropriate way to use this? Is there a sample project? |
hi @apemberton I think the implementation is a bit picky about how the Bean is annotated. The annotations need to be on the property, not the method. For example: public class QueryResultBean {
@QueryParam("skip")
private Integer skip;
@QueryParam("limit")
private Integer limit;
public Integer getSkip() {
return skip;
}
public void setSkip(Integer skip) {
this.skip = skip;
}
public Integer getLimit() {
return limit;
}
public void setLimit(Integer limit) {
this.limit = limit;
}
} The sample project is here: https://github.com/wordnik/swagger-core/tree/master/samples/java-jersey2 And shows the above bean working fine. |
see also #543 |
Odd... still seeing them come across as: paramType: "body" Still looking into it. |
This is your problem then (note the scanner package com.wordnik.swagger.jersey.config.JerseyJaxrsConfig): <servlet>
<servlet-name>Jersey2Config</servlet-name>
<servlet-class>com.wordnik.swagger.jersey.config.JerseyJaxrsConfig</servlet-class>
<init-param>
<param-name>api.version</param-name>
<param-value>1.0.0</param-value>
</init-param>
<init-param>
<param-name>swagger.api.basepath</param-name>
<param-value>http://localhost:8002/api</param-value>
</init-param>
<load-on-startup>2</load-on-startup>
</servlet> The scanner is what inspects the annotations, and unfortunately is very picky to the jersey versions. I'm pretty confident that once you have the correct scanner, it'll work as expected. |
@apemberton Would you also help double check that you are referencing swagger-jersey2-jaxrs? BeanParam is not available in swagger-jersey-jaxrs, so I only added in swagger-jersey2-jaxrs. |
Got it; that was my issue. Given BeanParam isn't a Jersey feature but core to JAX-RS 2.0, was expecting to see it in swagger-jaxrs_2.10, but adding swagger-jersey2-jaxrs_2.10 does the trick. Next issue / question I have is with generics. Does the new BeanParam support work for generics? For the following method : @ApiOperation(value = "Search", response = CollectionResponse.class)
public CollectionResponse getMany(@BeanParam CollectionRequest<Search> search) I'm getting:
|
@apemberton To make generics work it require non-trivial code change. I made it work for models, but not for BeanParam yet. To make it display correctly, you also need changes in swagger-ui. I just hacked my copy of swagger UI and didn't have that uploaded to github. Since there is interface changes, there is no unit test yet, and it's not fully polished, it requires more time to reach the quality for a pull request. Maybe this will happen after I finish this very tight project. If you are interested, the repo is in: https://github.com/shuz/swagger-core/ and in the "junbo" branch. And I opened another issue to track it. #498 |
@fehguy Actually the Jersey docs clearly states that the annotations can be on the bean fields or properties. Would that be a big change to make? |
Not a big change, no. I wanted to get initial support out, we can round it out in the next dot release. |
By the way, in
Here the parameters are extracted from resource fields that might be annotated with one of So this should be supported by Swagger as well. I see an easy way of doing it by modifying (and possibly renaming) the |
Do you guys know if this will work with the ApacheCXF implementation of JAX-RS? |
@enriquedelpino - assuming you're using a CXF version that supports JAX-RS 2, it should work. There may be issues if you use file uploads though. |
@webron - I gave it a try with not the best results. When I try to see the documentation of my resource using a BeanParam, I see a single parameter called "body", and I also see that in DataType there is a description of the BeanParam. So it seems the only way I have to populate the fields of the bean param I was hoping that by implementing documenting the BeanParam I would get each param in a different field, similar to what you get when the parameters are not grouped. |
Hi @enriquedelpino what you're seeing is the expected behavior. Complex objects do not expand into forms in the main swagger-ui--there is a fork that does it, here: https://github.com/HughePaul/swagger-ui/tree/feature-jsonbodyeditor |
@enriquedelpino - can you share the class used as |
I tried using the aforementioned fork, but it seems to be working exactly the same as the original swagger-ui in this case. @webron This is a snippet of the class used as BeanParam:
And this is the method declaration:
|
Can you also share your method declaration? No need for the body. |
I've updated the previous comment. |
Do you use |
As @fehguy mentioned before, the behaviour I am getting (where the BeanParam class is displayed as a Json object in the UI is the expected behavior. ApiModel and ApiModelProperty, allow me to display them as required/not required, but still in the Json format. I was hoping to be able to use @HughePaul's fork to see the parameters displayed individually in the form, instead of grouped, but I still don't see them like that. |
Actually, no, that's not the expected behavior. Swagger-core is currently treating your @HughePaul's fork treats an issue with the swagger-ui, but this is unrelated to the UI. The output of swagger-core is wrong. |
Adding @ApiParam to each of the individual fields inside of the @BeanParam class is one of the first things I tried, and it still keeps treating the parameters as a Body. I'm using swagger-core_2.10 version 1.3.8 |
Is it on top of the @ApiModelParam and @apimodel or instead? Also, don't use 1.3.8, it has a critical bug in it. Use either 1.3.7 or
|
When I used @ApiParam, I removed all the references to @apimodel and @ApiModelParam, as you suggested. Also after reading previous messages in this thread, I saw that in this comment @fehguy mentions the configuration of the jersey servlet. #446 (comment) In my case I am using Apache CXF, so I wonder if I would need to do a similar configuration. |
Wait, which dependency do you use?
|
I first used swagger-core_2.10 and swagger-jaxrs_2.10, and then in a second try I used also swagger-jersey-jaxrs_2.10 (all of them under version 1.3.8) , but still the jaxrs implementation I use is Apache CXF |
Okay, none of those offer support for @BeanParam. Please use
|
I tried using swagger-jersey2-jaxrs_2.10 and swagger-jersey2-jaxrs_2.11, both in version 1.3.9 and still shows the BeanParam class as body, without breaking up into single params. Am I missing some extra configuration of the servlet or something? Edited: I think I might be missing the apiReader configuration as it's said in here: |
In 1.3.9 it "works" like this: if your bean is annotated with QueryParam types, it works. If you use @FormParam however, the workaround is to annotate the FormParams additionally with a @ApiParam annotation. Kind of ugly, but better than having no BeanParam support at all. @fehguy : I tried to find the place where QueryParams are treated differently than FormParams in the swagger 1.3.9 code, class JaxRSApiReader, and I thought to have found it here: def getAllParamsFromFields(cls: Class[_]): List[Parameter] = { In the original code, the "FormParam" check had been missing, I patched it locally to see if that would yield any changes. However, even after introducing the additional checks, FormParams were not treated equal to QueryParams. Any clue on this ? Or where to look ? |
@enriquedelpino - that's quite possible. Try configuring it and see if it works. |
@enriquedelpino - I had the same problem, here is how I solved it It worked when I changed ClassReaders.setReader(new DefaultJaxrsApiReader()); to ClassReaders.setReader(new JerseyApiReader()); |
Has anyone tried using Swagger Core 1.5.x and Swagger UI 2.0 using Jersey2 with no web.xml It doesn't seem to recognize API annotations within BeanParam object and also the path param pattern doesn't get replaced such as /v1/report/{report_type:[aA-zZ]+} ..when executed on UI it uses the pattern as is |
Hi @swarupe04 It is definitely supported. Please take a look at this sample: https://github.com/swagger-api/swagger-core/tree/develop_2.0/samples/java-jaxrs-no-webxml It is a jaxrs 1.x sample but the spirit is the same. All configuration for swagger is done in the Bootstrap.java. |
Hi Tony, It seems to be the UI side, that doesn't seem to support it. Since I can Since, by using swagger 2.0 forma, we're forced to move to the new UI. Is master the latest for UI work, or do u think I should instead using Thanks On Wed, Mar 18, 2015 at 6:15 PM, Tony Tam notifications@github.com wrote:
|
Master should be fine. Can you share the bean property? Email is fine if its private |
Please can you provide the email id to send to. Thanks, Swarup On Wed, Mar 18, 2015 at 9:05 PM, Tony Tam notifications@github.com wrote:
|
sent to your gmail..please let me know if you need more info.. also wanted to mention about the swagger.json out too base class to support all types of Report Requests:
for bean class @apimodel(value = "base class to support all types of Report Requests") shouldn't it using the class name instead? On Wed, Mar 18, 2015 at 9:30 PM, Swarup Eda swarup.eda@teamaol.com wrote:
|
Would it be possible to add support for the @BeanParam (https://jax-rs-spec.java.net/nonav/2.0/apidocs/javax/ws/rs/BeanParam.html) from JAX-RS? Same question as answered at http://stackoverflow.com/questions/19913046/does-swagger-support-the-jax-rs-beanparam-annotation, but didn't find an issue for it here.
The text was updated successfully, but these errors were encountered: