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

Issue with generics model and return type #498

Closed
shuz opened this issue Mar 19, 2014 · 34 comments
Closed

Issue with generics model and return type #498

shuz opened this issue Mar 19, 2014 · 34 comments
Labels
Milestone

Comments

@shuz
Copy link
Contributor

shuz commented Mar 19, 2014

The reflection can't find proper generic model property.

public class ResultList<T> {
    private List<T> results;
    public List<T> getResults() {
        return results;
    }
    public void setResults(List<T> results) {
        this.results = results;
    }
}
class UserResultList extends ResultList<User> {
}

The reflection can find the ResultList results property, but it shows as an array of objects. To work around, I have to override the methods:

class OfferResultList extends ResultList<Offer> {
    @Override
    public List<Offer> getResults() {
        return super.getResults();
    }
    @Override
    public void setResults(List<Offer> results) {
        super.setResults(results);
    }
}

The issue also exists in the method return types. I looked at the code and seems it's not trivial to make a fix. All reflection functions are expecting a Class[_] rather than Type.

@shuz
Copy link
Contributor Author

shuz commented Mar 30, 2014

Just FYI, I made some changes to enable this. The change might be too big to merge into your development. There are basically two changes:

  1. I added a ClassWrapper to help parse generic types and load generic types from string.
  2. I changed the swagger model and api parsers to use the ClassWrapper to handle types.

The change pass all unit tests. I added unit tests for ClassWrapper I didn't add a model test case to verify, but it worked well in my project. Now the APIs can return generic types:

class OfferService {
    public ResultWrapper<Offer> getAllOffers();
}

Here are the commits.
shuz@a4b6cd0
shuz@207307d

@apemberton
Copy link

👍

@kmadel
Copy link

kmadel commented Jul 27, 2014

Just wondering why this was closed? Will there be support for generic types for responses?

@webron
Copy link
Contributor

webron commented Jul 27, 2014

It was never closed.

@fehguy fehguy modified the milestone: v1.5.0 Sep 19, 2014
@fehguy
Copy link
Contributor

fehguy commented Dec 20, 2014

support for generics is limited, but I believe this is going to be handled with the jackson mapper in 1.5.0

@fehguy fehguy added the Bug label Dec 20, 2014
@fehguy fehguy modified the milestones: v1.5.0-M1, v1.5.0 Dec 20, 2014
@fehguy fehguy added the P3 label Dec 23, 2014
@fehguy
Copy link
Contributor

fehguy commented Jan 28, 2015

For now, there is the opportunity to override datatypes with the @ApiModelProperty annotation, like such: https://github.com/swagger-api/swagger-core/blob/develop_2.0/modules/swagger-core/src/test/scala/1_3/models/ModelWithModelPropertyOverrides.java#L7-L18

public class ModelWithModelPropertyOverrides {
  @ApiModelProperty(dataType = "List[models.Children]")
  private String children;

  public String getChildren() {
    return children;
  }

  public void setChildren(String children) {
    this.children = children;
  }
}

where you override the datatype with whatever you like, which means generics can be expressed in any concrete class.

@fehguy
Copy link
Contributor

fehguy commented Jan 28, 2015

Partial solution, moving to M2 for more investigation and more use cases.

@fehguy fehguy modified the milestones: v1.5.0-M2, v1.5.0-M1 Jan 28, 2015
@ovstetun
Copy link

Will there be a general solution to this problem? I am exposing an API with a general "ResultList" implementation supporting paging:

public class ListResult<T> {
    public long size = 0;
    public long rangeFrom = 0;
    public long rangeSize = 0;
    public List<T> entries = Collections.emptyList();
}

And I should really have the T-type as part of my Swagger documentation.

@fehguy
Copy link
Contributor

fehguy commented Feb 10, 2015

It is technically impossible to support all forms of generics. The general solution is to manually map generics to an interface or type if runtime reflection is not possible. There will be guidelines for M2, which we're working on.

@ovstetun
Copy link

I found a "reasonable" way for my case to give the correct output to work in swagger ui, by subclassing the return type and overriding the generic parts. Note that my private class is not used in code, only as parameter for swagger and the @ApiOperation-annotation.

public class ListResult<T> {
    public long size = 0;
    public long rangeFrom = 0;
    public long rangeSize = 0;
    public List<T> entries = Collections.emptyList();
}

@Path("/records")
@Api(value = "/records", description = "")
public class RecordsResource {

    @GET
    @Path("inbox")
    @ApiOperation(value = "/inbox", response = RecordResultList.class)
    public Response inbox() {
      ListResult<Record> result = ListResult.create(records, Record::new);
      return Response.ok().entity(result).build();
    }
    private static class RecordResultList extends ListResult<Record> {
      public List<Record> entries;
    }
}

Hope this helps somebody else, for me this works as an OK workaround.

@fehguy fehguy self-assigned this Mar 13, 2015
@fehguy fehguy modified the milestones: v1.5-M2, v1.5-M3 Mar 29, 2015
@webron webron modified the milestones: v1.5-M3, v1.5 May 4, 2015
@MichaelMcClure
Copy link

+1

@webron webron added the P2 label May 28, 2015
webron added a commit that referenced this issue Jun 1, 2015
Fixed #498: Handling of generic responses has been fixed.
@tomtit
Copy link
Contributor

tomtit commented Jun 1, 2015

It's impossible to pass generic type via the @ApiOperation.response, but we can get the right type from the return type. After the #1119 the definition below should provide the correct output:

@GET
@Path("inbox")
@ApiOperation(value = "/inbox")
public ListResult<Record> inbox() {
  return ListResult.create(records, Record::new);
}

@webron
Copy link
Contributor

webron commented Jun 1, 2015

@tomtit - that would work only if the method returns the type itself. The more common use case with JAX-RS is to return a Response that can also include the HTTP code and so on. That's actually the reason why the response was included in @ApiOperation.

@tomtit
Copy link
Contributor

tomtit commented Jun 1, 2015

In this case the response should be of type java.lang.reflect.Type but not Class.

@webron
Copy link
Contributor

webron commented Jun 1, 2015

And then how would it be represented?

@tomtit
Copy link
Contributor

tomtit commented Jun 1, 2015

Unfortunately there is no way to do this as JLS states:

It is a compile-time error if the return type of a method declared in an annotation type is any type other than one of the following: one of the primitive types, String, Class and any invocation of Class, an enum type (§8.9), an annotation type, or an array (§10) of one of the preceding types.

@tomtit
Copy link
Contributor

tomtit commented Jun 1, 2015

So, we need to use the workaround with a non-generic type (RecordResultList) or change method's return type from Response to generic type.

@webron
Copy link
Contributor

webron commented Jun 1, 2015

Not really up to us but up to the users, but sure, we're limited with our options.

@fehguy fehguy closed this as completed in ea3a390 Jun 6, 2015
@chenjianjx
Copy link

Why is this issue closed? Is there any new issue tracking this problem? @ovstetun 's workaround is fine, but it may cause too many classes to be created.

@fehguy
Copy link
Contributor

fehguy commented Dec 22, 2015

@chenjianjx please see my comment #498 (comment)

@chenjianjx
Copy link

ok, thank you

@tavoda
Copy link

tavoda commented Mar 8, 2016

@fehguy sure you can not support ALL POSSIBLE forms of generic, however you can easily support at least very common patterns which can make life better. Just simple e.g. Result<T, U> and than have possibility to specify for example return type as Result<String,Person>. This is supported by many languages and doesn't require too much changes in spec.

@webron
Copy link
Contributor

webron commented Mar 8, 2016

@tavoda @fehguy was talking about the limitations of Java with type erasure as the ability to read and retrieve the types. Changes to the spec have nothing to do with swagger-core. If you'd like to see changes there, please open a ticket on the spec itself.

@tavoda
Copy link

tavoda commented Mar 8, 2016

Which limitations? Erasures are in generic type (e.g. List), in concrete type you can find it easily. This code produce:
Method getListOfStrings return java.util.List[java.lang.String]
Method getMapOfStringToSample return java.util.Map[java.lang.String, Sample]

public class Sample {
    public List<String> getListOfStrings() {return null;}
    public Map<String, Sample> getMapOfStringToSample() {return null;}

    public static void main(String[] args) {
        Method[] methods = Sample.class.getDeclaredMethods();
        for (Method m : methods) {
            Type returnType = m.getGenericReturnType();
            if (returnType instanceof ParameterizedType) {
                ParameterizedType pType = (ParameterizedType) returnType;
                StringBuilder params = new StringBuilder();
                String prefix = "";
                for (Type type : pType.getActualTypeArguments()) {
                    params.append(prefix).append(type.getTypeName());
                    prefix = ", ";
                }
                System.out.println("Method " + m.getName() + " return "
                    + pType.getRawType().getTypeName() + "[" + params + "]");
            }
        }
    }
}

@webron
Copy link
Contributor

webron commented Mar 8, 2016

If you feel things can be improved, you're more than welcome to submit PRs with changes.

@tavoda
Copy link

tavoda commented Mar 8, 2016

@webron I'm still confused by spec. Maybe implementation is fine, I would like to have very simple Java return type like PagingResult:

class PagindResult<T> {
    int startRow, endRow, totalRows;
    List<T> data;
}

And than AddressService.find method which return just PagingResult<Address>. What is correct way of specifying PagingResult in Swagger? Do I have to create specific type for each service?
I can investigate in Java all necessary types however I'm not able to write it in swagger. Is it really problem of core or spec?
Thanks for your help

@webron
Copy link
Contributor

webron commented Mar 8, 2016

You'd need to create a specific type for each service, yes. OAI/OpenAPI-Specification#555 covers this as a feature request for the next version as there's no easy way to define wrappers for responses.

@tavoda
Copy link

tavoda commented Mar 8, 2016

Thanks @webron, I will track that issue.

@tomtit
Copy link
Contributor

tomtit commented Mar 8, 2016

@tavoda actually it depends on the way you define your resource:

@Path("/")
public class AddressService {

    // Here you need a specific type
    @GET
    @Path("/foo")
    @ApiOperation(value = "/foo", response = PagingResultOfAddresses.class)
    public Response foo() {
        ...
    }

    // Swagger core will handle return type
    @GET
    @Path("/boo")
    @ApiOperation(value = "/boo")
    public PagingResult<Address> foo() {
        ...
    }
}

@tavoda
Copy link

tavoda commented Mar 8, 2016

@tomtit nice to know, thanks

@Marvel77
Copy link

Marvel77 commented Apr 5, 2016

Hey, guys! Maybe it will be usefull. I had same issues with my Spring based application and I've used alternate type rules in the Swagger config:

@Bean
    Docket api() {
        return new Docket(DocumentationType.SWAGGER_2)
                .select()
                .apis(RequestHandlerSelectors.any())
                .paths(paths())
                .build()
                .apiInfo(apiInfo())
                .alternateTypeRules(
                        newRule(typeResolver.resolve(DeferredResult.class,
                                typeResolver.resolve(ResponseEntity.class, WildcardType.class)),
                                typeResolver.resolve(WildcardType.class)))
                ;
}

This means: Swagger, if you will find DeferredResult<ResponseEntity<Something>> then use <Something> as return type for generating JSON in SwaggerUI.

Here is a controller's method signature:

@ApiOperation(
            value = "Authenticate a user",
            notes = "..."
    )
    @RequestMapping(method = RequestMethod.POST)
    public DeferredResult<ResponseEntity<BaseResponse<AccessTokenResponse>>> authAndGetDeviceInfo(@RequestBody AuthUserRequest authReques...

And now Swagger generates returned JSON description based on generic BaseResponse<AccessTokenResponse>.

Have a nice day!

PS for more info read http://springfox.github.io/springfox/docs/snapshot
2.1.3. Springfox-swagger2 with Spring MVC and Spring Boot

@fehguy
Copy link
Contributor

fehguy commented Apr 6, 2016

@Marvel77 thank you for sharing

@bugproof
Copy link

How is this solved?

@tavoda
Copy link

tavoda commented Jun 19, 2018

It's problem of SPEC. This mean is not possible to specify real generic return type. Core is handling it fine as mentioned by @tomtit . It should be closed.

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