Skip to content

[Spring] put spring-mvc and spring-boot under the same language gen#3133

Merged
wing328 merged 4 commits intoswagger-api:masterfrom
cbornet:spring_unite
Jun 21, 2016
Merged

[Spring] put spring-mvc and spring-boot under the same language gen#3133
wing328 merged 4 commits intoswagger-api:masterfrom
cbornet:spring_unite

Conversation

@cbornet
Copy link
Contributor

@cbornet cbornet commented Jun 14, 2016

This PR does the following:

  • put spring-mvc and spring-boot as libraries of a "spring" language
  • set spring-boot as default lib
  • remove spring-boot and spring-mvc languages
  • adds joda and jsr310 date support to spring-mvc
  • remove generation timestamps

@cbornet
Copy link
Contributor Author

cbornet commented Jun 14, 2016

@diyfr this is a pretty big one, can you help reviewing ?

@diyfr
Copy link
Contributor

diyfr commented Jun 20, 2016

I try to watch it this week @cbornet

@pbowness
Copy link

My apologies if this is not the right place to ask, but I’m interested to know if there’s any timeframe when swagger-codegen 2.2.0 might be released?

The most recent version I can find is 2.1.6, but this does not have the “springboot” language support.

Thanks, -Piers

Piers Bowness – piers.bowness@rsa.com
Senior Consulting Software Engineer, RSA , the Security Division of EMC.
174 Middlesex Turnpike, Bedford, MA. 01730
W: 781-515-7021 M: 978-808-8072

From: Stéphane [mailto:notifications@github.com]
Sent: Monday, June 20, 2016 12:06 AM
To: swagger-api/swagger-codegen
Cc: Bowness, Piers; Subscribed
Subject: Re: [swagger-api/swagger-codegen] [Spring] put spring-mvc and spring-boot under the same language gen (#3133)

I try to watch it this week @cbornethttps://github.com/cbornet


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHubhttps://github.com//pull/3133#issuecomment-227046515, or mute the threadhttps://github.com/notifications/unsubscribe/ATCU6VSMXwUZ0sTiO3RQxbJZ_FBfHyWIks5qNhG3gaJpZM4I1drZ.

@wing328
Copy link
Contributor

wing328 commented Jun 20, 2016

@pbowness usually it takes 3 to 4 months for a minor release and I would foresee 2.2.0 would take a bit more time. Please pull the latest master to use spring boot generator.

@diyfr
Copy link
Contributor

diyfr commented Jun 21, 2016

I see you've solved the problem by moving the requestMapping on methods rather than on the class. Would not it interesting move implementations in a sub-package "impl" . I'm still on my problematic regeneration ..

@cbornet
Copy link
Contributor Author

cbornet commented Jun 21, 2016

Jersey gen does it by putting the interfaces in /src/gen/java and the impl in /src/main/java.
We could do that in another PR

@cbornet
Copy link
Contributor Author

cbornet commented Jun 21, 2016

And it also doesn't override the impl if it already exists.

@diyfr
Copy link
Contributor

diyfr commented Jun 21, 2016

Can you upgrade dependencies Version ? (1.3.5.RELEASE for springboot Parent , and for spiringfox is (i think) 2.5.0)

@cbornet
Copy link
Contributor Author

cbornet commented Jun 21, 2016

Done. I also upgraded to latest model with better enums, fluent setters, ...

@thebignet
Copy link
Contributor

I have enums for query parameters. Current codegen doesn't seem to translate them to Java Enums. I think the same goes for path parameters. Tried to change datatype to datatypeWithEnum in pathParams.mustache and queryParams.mustache to no luck. Can't seem to figure out what's wrong.

This could go in a separate issue, but since this PR includes better enums, if you know how to solve this.

@wing328
Copy link
Contributor

wing328 commented Jun 22, 2016

Upgrade Note

Starting from 2.2.0, spring-mvc has been renamed to spring to cover both Spring MVC and Spring Boot.

To generate Spring MVC server stub, please supply -l spring --library spring-mvc as command line arguments while the default library for spring is spring-boot

@cbornet cbornet deleted the spring_unite branch June 22, 2016 14:16
@cbornet
Copy link
Contributor Author

cbornet commented Jun 22, 2016

@thebignet see #1347 #2282

@thebignet
Copy link
Contributor

I've added #3198 for these issues

@diyfr
Copy link
Contributor

diyfr commented Jul 11, 2016

@cbornet
I would in some cases be able to use Page (include in ResponseEntity) on get List with Pageable query parameters.
Parameters type in query can be only primitive,
Do you think it is interesting to detect page, size & sort , to generate ResponseEntity<Page> instead of ResponseEntity<List> and replace this 3 parameters to Pageable ?

@cbornet
Copy link
Contributor Author

cbornet commented Jul 11, 2016

@diyfr Detecting a Paged resource would indeed be nice. Do you know if springfox can also detect them ?

@cbornet
Copy link
Contributor Author

cbornet commented Jul 11, 2016

This could also be done easily with a "x-paged" vendor-extension

@diyfr
Copy link
Contributor

diyfr commented Jul 11, 2016

Yes same idea x-java-pageable ?
If we do respect extension name.

@diyfr
Copy link
Contributor

diyfr commented Jul 12, 2016

@wing328 it's very difficult to find this page [https://github.com/swagger-api/swagger-codegen/wiki/Vendor-Extensions]
I create new issue on OpenAPI-Specification , but i don't know if is the good place for this.
And on the swagger.io, same problem.

@cbornet
Copy link
Contributor Author

cbornet commented Jul 12, 2016

@diyfr pagination is more linked to spring than to java.
I think the cleaner would be to check for a "x-spring-pageable" and if present ignore any page/size/sort/limit query param from the input spec and generate a Page resource.
Then the doc generated by Springfox should also manage paging : see springfox/springfox#755 to add the relevant query params (either with generated @ApiImplicitParam or with a ParameterBuilderPlugin ) and add the "x-spring-pageable".

@cbornet
Copy link
Contributor Author

cbornet commented Jul 12, 2016

If you're working with pagination, it would also be useful to add a rfc5988 "Link" header and provide a utility class to generate "next" and "previous" links. See for instance https://github.com/jhipster/generator-jhipster/blob/master/generators/server/templates/src/main/java/package/web/rest/util/_PaginationUtil.java

@diyfr
Copy link
Contributor

diyfr commented Jul 12, 2016

@cbornet Il see this issue (https://github.com/springfox/springfox/issues/755 and today ParameterBuilderPlugin is not merged.
I think make a switch on api.mustache "if x-spring-pageable is present. "replace the code and add method. (@ApiImplicitParams).
It's ok for the generated code, but Springfox doesn't make a same swagger file.....

A "Link" it's a HATEOS format ?
Sample response serialized with ResponseEntity<Page<MonObjet>>

{  
  "content": [  
    {  
      "name": "Mon Objet",  
      "localDate": [  
        2016,  
        7,  
        12  
      ],  
    }  
  ],  
  "totalElements": 1,  
  "totalPages": 1,  
  "last": true,  
  "size": 0,  
  "number": 0,  
  "sort": null,  
  "numberOfElements": 1,  
  "first": true  
}  

@cbornet
Copy link
Contributor Author

cbornet commented Jul 12, 2016

ParameterBuilderPlugin is not merged but we can generate it.
rfc5988 is not HATEOAS but you can use rfc5988 to build a HATEOAS implementation

@cbornet
Copy link
Contributor Author

cbornet commented Jul 12, 2016

And I think with @ApiImplicitParams we can generate the same swagger spec. What bothers you ?

@cbornet
Copy link
Contributor Author

cbornet commented Jul 12, 2016

You shouldn't return Page<MonObjet> but List<MonObjet> objectList = page.getContent(); and put the pagination details in the "Link" header using the PaginationUtil class.

@cbornet
Copy link
Contributor Author

cbornet commented Jul 12, 2016

That way it's the same body format as if the result was not paged

@wing328
Copy link
Contributor

wing328 commented Jul 12, 2016

it's very difficult to find this page [https://github.com/swagger-api/swagger-codegen/wiki/Vendor-Extensions]

@diyfr you want to put it in the README.md instead?

Personally I prefer not making README.md too long and that's why I've moved some of the documentations to the wiki.

@diyfr
Copy link
Contributor

diyfr commented Jul 12, 2016

@wing328 And i don't know why github search engine doesn't get an result to [https://github.com/swagger-api/swagger-codegen/wiki/Vendor-Extensions](Wiki Vendor Extension)
with words "vendor extension"
And in CONTRIBUTING.md the link for Vendor Extension go to OAI but OAI has no section for Vendor Extension.

@diyfr
Copy link
Contributor

diyfr commented Jul 12, 2016

@cbornet work in progress...
Pageable need spring.data.commons and spring.data.commons need to declare a datasource.
h2 ?

@cbornet
Copy link
Contributor Author

cbornet commented Jul 12, 2016

This feels very awkward...

@cbornet
Copy link
Contributor Author

cbornet commented Jul 12, 2016

What makes you need to declare a datasource ? Spring-boot autoconfig ? There may be some configuration to do to disable it.

@cbornet
Copy link
Contributor Author

cbornet commented Jul 12, 2016

I just tried

    public ResponseEntity<List<Pet>> findPetsByTags(@ApiParam(value = "Tags to filter by", required = true) @RequestParam(value = "tags", required = true) List<String> tags, Pageable pageable) {
        ArrayList<Pet> petList = new ArrayList<Pet>();
        Page<Pet> page = new PageImpl(petList, pageable, 0);
        // do some magic!
        return new ResponseEntity<List<Pet>>(page.getContent(), HttpStatus.OK);
    }

in spring-boot after including spring-data-commons and didn't have any issue.
I didn't have to configure any datasource.

@cbornet
Copy link
Contributor Author

cbornet commented Jul 12, 2016

I have opened another issue to track the discussion on this feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants