Skip to content
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

Closed
joscarsson opened this issue Jan 28, 2014 · 58 comments
Closed

Add support for JAX-RS @BeanParam #446

joscarsson opened this issue Jan 28, 2014 · 58 comments
Labels
Milestone

Comments

@joscarsson
Copy link

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.

@avengerpenguin
Copy link

+1 for me. We're making a lot of use of @BeanParam now.

@gschmutz
Copy link

+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!

@brianheineman
Copy link

+1 this is a feature our group needs as well

@mwwickline
Copy link

+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.

@emilesvt
Copy link

emilesvt commented Apr 1, 2014

+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.

@fehguy
Copy link
Contributor

fehguy commented Apr 2, 2014

Yes, not supported but we can target this for 1.3.5

@fehguy fehguy added this to the v1.3.5 milestone Apr 2, 2014
@ncolomer
Copy link

ncolomer commented Apr 6, 2014

+1 would be nice to have

@Paave
Copy link

Paave commented Apr 7, 2014

+1

@shuz
Copy link
Contributor

shuz commented Apr 16, 2014

I made a simple fix but it's based on my own modified branch. You can refer to the commit here.

shuz@5213b4f
shuz@ee67580

@apemberton
Copy link

+1 (sad that @emilesvt and I both found and +1'ed this... me on a Friday night, but hey... who's judging?)

@apemberton
Copy link

This is awesome! Which milestone / release is this scheduled for? 1.3.5?

Any target date to ship 1.3.5?

@fehguy
Copy link
Contributor

fehguy commented Apr 28, 2014

Hi, just pushed to maven central, and documented here:

https://groups.google.com/forum/#!topic/swagger-swaggersocket/cXCnE3cyKBA

@fehguy fehguy closed this as completed Apr 28, 2014
@ncolomer
Copy link

That's just great! Thanks a lot 👍

@apemberton
Copy link

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?

@fehguy
Copy link
Contributor

fehguy commented Apr 28, 2014

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.

@fehguy
Copy link
Contributor

fehguy commented Apr 28, 2014

see also #543

@apemberton
Copy link

Odd... still seeing them come across as:

paramType: "body"

Still looking into it.

@fehguy
Copy link
Contributor

fehguy commented Apr 28, 2014

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.

@shuz
Copy link
Contributor

shuz commented Apr 29, 2014

@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.

@apemberton
Copy link

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:

UNKNOWN TYPE: com.someplace.model.CollectionRequest
java.lang.ClassNotFoundException: class UNKNOWN[com.someplace.model.Search] not found

@shuz
Copy link
Contributor

shuz commented Apr 29, 2014

@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
It's possible there are official effort on this already, I'm not sure.

@giovannibotta
Copy link
Contributor

@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?

@fehguy
Copy link
Contributor

fehguy commented May 1, 2014

Not a big change, no. I wanted to get initial support out, we can round it out in the next dot release.

@giovannibotta
Copy link
Contributor

By the way, in JerseyApiReader in the readRecursive method, I was looking at this line:

// look for method-level annotated properties
val parentParams: List[Parameter] = getAllParamsFromFields(cls)

Here the parameters are extracted from resource fields that might be annotated with one of QueryParam, etc. However, the docs state that a QueryParam annotation is legal in a "resource class bean property". Similarly for PathParam and HeaderParam.

So this should be supported by Swagger as well. I see an easy way of doing it by modifying (and possibly renaming) the getAllParamsFromFields to go off annotations on fields and bean properties.

@enriquedelpino
Copy link

Do you guys know if this will work with the ApacheCXF implementation of JAX-RS?

@webron
Copy link
Contributor

webron commented Sep 17, 2014

@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.

@enriquedelpino
Copy link

@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
is to write a json (even if my api is not designed to consume json), stating the values as you can see in the picture.

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.

screenshot from 2014-09-18 17 04 25

@fehguy
Copy link
Contributor

fehguy commented Sep 18, 2014

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

@webron
Copy link
Contributor

webron commented Sep 18, 2014

@enriquedelpino - can you share the class used as @BeanParam?

@enriquedelpino
Copy link

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:

@ApiModel("Theme Parameters")
public class ThemeParams {
    public static final String URI_PARAM_NAME = "uri";
    public static final String SERVICE_PARAM_NAME = "service";

    @ApiModelProperty(value = "URIs", required = true)
    @QueryParam(URI_PARAM_NAME)
    private List<String> uris;

    @ApiModelProperty(value = "service", required = false)
    @QueryParam(SERVICE_PARAM_NAME)
    private String serviceUri;

    public List<String> getUris() {
        return uris;
    }

    public ThemeParams setUris(final List<String> uris) {
        this.uris = uris;
        return this;
    }

    public String getServiceUri() {
        return serviceUri;
    }

   public ThemeParams setServiceUri(final String serviceUri) {
       this.serviceUri = serviceUri;
       return this;
   }
}

And this is the method declaration:

@GET
@Produces({SemanticMediaType.JSON_LD, MediaType.APPLICATION_JSON_VALUE})
@ApiOperation(value = "Retrieves BCA Themes", position = 0)
@ApiResponses(value = {@ApiResponse(code = 200,
        message = "Retrieval was successful"), @ApiResponse(code = 403,
        message = "Missing or invalid x-business-group-id header"), @ApiResponse(code = 500,
        message = "Internal server error")})
public Response get(final @ApiParam(value = "Theme parameters") @Valid @BeanParam ThemeParams themeParams) {
...
}

@webron
Copy link
Contributor

webron commented Sep 19, 2014

Can you also share your method declaration? No need for the body.

@enriquedelpino
Copy link

I've updated the previous comment.

@webron
Copy link
Contributor

webron commented Sep 19, 2014

Do you use ThemeParams as a model too? Or only as a container for @BeanParam? If only as a container, can you try removing the @ApiModel and @ApiModelProperty annotations?

@enriquedelpino
Copy link

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.

@webron
Copy link
Contributor

webron commented Sep 19, 2014

Actually, no, that's not the expected behavior. Swagger-core is currently treating your @BeanParam as a body parameter instead of unfolding and extracting it to the two @QueryParams you defined in it. You should use @ApiParam on them to add additional meta data and not the model annotations. It may be what's causing this bug to take place.

@HughePaul's fork treats an issue with the swagger-ui, but this is unrelated to the UI. The output of swagger-core is wrong.

@enriquedelpino
Copy link

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

@webron
Copy link
Contributor

webron commented Sep 19, 2014

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
1.3.9.
On Sep 19, 2014 1:34 PM, "enriquedelpino" [email protected] wrote:

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


Reply to this email directly or view it on GitHub
#446 (comment)
.

@enriquedelpino
Copy link

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.

@webron
Copy link
Contributor

webron commented Sep 19, 2014

Wait, which dependency do you use?
On Sep 19, 2014 1:54 PM, "enriquedelpino" [email protected] wrote:

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 https://github.com/fehguy mentions the configuration of
the jersey servlet. #446 (comment)
#446 (comment)

In my case I am using Apache CXF, so I wonder if I would need to do a
similar configuration.


Reply to this email directly or view it on GitHub
#446 (comment)
.

@enriquedelpino
Copy link

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

@webron
Copy link
Contributor

webron commented Sep 19, 2014

Okay, none of those offer support for @BeanParam. Please use
swagger-jersey2-jaxrs, even if you're using cxf.
On Sep 19, 2014 2:07 PM, "enriquedelpino" [email protected] wrote:

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


Reply to this email directly or view it on GitHub
#446 (comment)
.

@enriquedelpino
Copy link

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:
#618 (comment)

@jayzaw
Copy link

jayzaw commented Sep 19, 2014

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] = {
// find getter/setters
(cls.getDeclaredMethods map {
method =>
if (method.getAnnotation(classOf[QueryParam]) != null ||
method.getAnnotation(classOf[FormParam]) != null ||
method.getAnnotation(classOf[HeaderParam]) != null ||
method.getAnnotation(classOf[PathParam]) != null ||
method.getAnnotation(classOf[ApiParam]) != null) {
createParameterFromGetterOrSetter(method).toList
} else Nil
}).flatten.toList ++
(for (field <- getAllFields(cls)) yield {
// only process fields with @ApiParam, @QueryParam, @HeaderParam, @PathParam
if (field.getAnnotation(classOf[QueryParam]) != null ||
field.getAnnotation(classOf[FormParam]) != null ||
field.getAnnotation(classOf[HeaderParam]) != null ||
field.getAnnotation(classOf[PathParam]) != null ||
field.getAnnotation(classOf[ApiParam]) != null) {
val param = new MutableParameter
param.dataType = processDataType(field.getType, field.getGenericType)
Option(field.getAnnotation(classOf[ApiParam])) match {
case Some(annotation) => toAllowableValues(annotation.allowableValues)
case _ =>
}
val annotations = field.getAnnotations
processParamAnnotations(param, annotations)
}
else Nil
}).flatten.toList
}

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 ?

@webron
Copy link
Contributor

webron commented Sep 19, 2014

@enriquedelpino - that's quite possible. Try configuring it and see if it works.

@netzwerch
Copy link

@enriquedelpino - I had the same problem, here is how I solved it
I have a Jersey project, only use swagger-jersey2-jaxrs_2.11 and do the configuration with a RessourceConfig by a combination of these two files:
Application.java
SwaggerExampleGuiceContextListener.java

It worked when I changed

ClassReaders.setReader(new DefaultJaxrsApiReader());

to

ClassReaders.setReader(new JerseyApiReader());

@swarupe04
Copy link

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
On 1.3.x didn't have any of these issues

@fehguy
Copy link
Contributor

fehguy commented Mar 18, 2015

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.

@swarupe04
Copy link

Hi Tony,

It seems to be the UI side, that doesn't seem to support it. Since I can
see api documentation in the swagger.json.

Since, by using swagger 2.0 forma, we're forced to move to the new UI.
Also, I noticed that new UI 2.0 doesn't have the "raw" link anymore.

Is master the latest for UI work, or do u think I should instead using
develop_2.0 for swagger ui too, to get the latest UI modules.

Thanks

On Wed, Mar 18, 2015 at 6:15 PM, Tony Tam [email protected] wrote:

Hi @swarupe04 https://github.com/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.


Reply to this email directly or view it on GitHub
#446 (comment)
.

@fehguy
Copy link
Contributor

fehguy commented Mar 19, 2015

Master should be fine. Can you share the bean property? Email is fine if its private

@swarupe04
Copy link

Please can you provide the email id to send to.

Thanks,

Swarup

On Wed, Mar 18, 2015 at 9:05 PM, Tony Tam [email protected] wrote:

Master should be fine. Can you share the bean property? Email is fine if
its private


Reply to this email directly or view it on GitHub
#446 (comment)
.

@fehguy
Copy link
Contributor

fehguy commented Mar 19, 2015

[email protected]

@swarupe04
Copy link

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:
{

  • required:
    [
    • "active_campaigns",
    • "date_range",
    • "report_type",
    • "time_unit",
    • "view_level"
      ],

for bean class

@apimodel(value = "base class to support all types of Report Requests")
public class ReportRequest {

shouldn't it using the class name instead?

On Wed, Mar 18, 2015 at 9:30 PM, Swarup Eda [email protected] wrote:

Please can you provide the email id to send to.

Thanks,

Swarup

On Wed, Mar 18, 2015 at 9:05 PM, Tony Tam [email protected]
wrote:

Master should be fine. Can you share the bean property? Email is fine if
its private


Reply to this email directly or view it on GitHub
#446 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests