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

basePath specified in swagger file is ignored by spring code generator #5244

Closed
Frank-Stephan opened this issue Mar 28, 2017 · 30 comments · Fixed by OpenAPITools/openapi-generator#104

Comments

@Frank-Stephan
Copy link

Frank-Stephan commented Mar 28, 2017

Description

A basePath specified in a swagger file is ignored by the spring code generator. Annotation @RequestMapping doesn't contain it.

Swagger-codegen version

2.2.2 (swagger-codegen-maven-plugin)

Swagger declaration file content or url
################################################################################
#                             Metadata                                         #
################################################################################
# this is an example API to illustrate the usage of Swagger 

swagger: '2.0'
info:
  title: Person API
  description: Create, maintain, and update a person database as personal contacts' list.
  version: "1.0.0"

# array of all schemes that your API supports
schemes:
  - https
  
# will be prefixed to all paths
basePath: /v1
produces:
  - application/json

################################################################################
#                                 Paths                                        #
################################################################################  
paths:

  /persons/{id}:              
    delete:
      summary: Deletes a person.
      description: |
        Deletes a person from the database.
      tags:
        - Person
      parameters:
        - name: id
          in: path
          description: The ID of the person to be deleted.
          type: integer
          format: int64
          required: true
      responses:
        204:
          description: Person deleted.

generates:

@javax.annotation.Generated(value = "io.swagger.codegen.languages.SpringCodegen", date = "2017-03-28T19:13:06.141+02:00")

@Api(value = "persons", description = "the persons API")
public interface PersonsApi {

    @ApiOperation(value = "Deletes a person.", notes = "Deletes a person from the database. ", response = Void.class, tags={ "Person", })
    @ApiResponses(value = { 
        @ApiResponse(code = 204, message = "Person deleted.", response = Void.class) })
    @RequestMapping(value = "/persons/{id}",
        produces = { "application/json" }, 
        method = RequestMethod.DELETE)
    default ResponseEntity<Void> personsIdDelete(@ApiParam(value = "The ID of the person to be deleted.",required=true ) @PathVariable("id") Long id) {
        // do some magic!
        return new ResponseEntity<Void>(HttpStatus.OK);
    }

}
Command line used for generation
Steps to reproduce
Related issues
Suggest a Fix
@ngaya-ll
Copy link

Is this the same as #3720?

@Frank-Stephan
Copy link
Author

Yes it is the same problem.

@wing328
Copy link
Contributor

wing328 commented Mar 31, 2017

@Frank-Stephan so did the following resolve the issue for you?

#3720 (comment)

@Frank-Stephan
Copy link
Author

In 3720 was suggested to change the base path in the spring configuration file (server.contextPath=/v2). This is not feasible for me, as it moves all rest endpoints including spring boot health endpoints. In addition I see then no way to provide two API versions out of one micro service.

@wing328
Copy link
Contributor

wing328 commented Apr 1, 2017

@Frank-Stephan so your requirement is to change contextPath for some but not all of the endpoints?

@Frank-Stephan
Copy link
Author

No, my expectation was that the code generator for spring takes the basePath specified in the swagger file into account. The contextPath of spring has another purpose. It moves all rest endpoints including those provided by spring itself. We want to use the basePath to put a version number into it that is prepended to all rest endpoints specified in the swagger file. Of course if there were a spring feature with which we can prepend all endpoints of our swagger api with a version number we would have used it as a workaround.

@erreobi
Copy link

erreobi commented Apr 5, 2017

I have the same issue. Do you know when the fix is planned?

@fehguy
Copy link
Contributor

fehguy commented Apr 5, 2017

@Frank-Stephan I think what the generator is doing with basePath is per design. If you want your operation path to be /v1/persons/{id}, for example, you would put that in your path:

paths:
  '/v1/persons/{id}':
    ### stuff

@erreobi
Copy link

erreobi commented Apr 12, 2017

I found a workaround that it works for me: I copied the application properties from the generated code to the src/main.resources.

@wing328 wing328 closed this as completed Apr 18, 2017
@nisheethagarwal
Copy link

nisheethagarwal commented Jun 6, 2017

@fehguy I have a similar problem, but following the approach you suggest consolidates all my endpoints into a single api file, which apparently can then only to mapped to a single controller. Any suggestions on how one could get the base bath as Frank wanted and yet keep different controller implementations? I tried multiple controllers implementing different methods of the same single api interface, but that resulted in an ambiguous mapping exception.

@ok11
Copy link

ok11 commented Feb 14, 2018

Frankly, I'm not sure why the issue was closed.
I think it is a valid requirement to have the basePath prepended to the resources. Otherwise the only viable workaround is to do it manually for each resource. And this not only violates the DRY principle, but also contradicts the entire REST design paradigm. REST is all about resources and by the workaround we just mix up concerns, making the notion of a resource contaminated with some "configuration" stuff.
Imagine I'm talking about user management and would like to create a namespace for it. Not only to separate versions, but also to separate bounded contexts. In that case it would sound like user-management/v1. Putting it into the basePath looks natural. In that case it is just a prefix, not a part of a resource. The resources are then described as

  /users:
    post:
...

Lacking the ability to handle the basePath by the generator, I have to describe my resources in the way:

  /user-management/v1/users:
    post:
...

But sorry, there is no resource like user-management/v1/users. It is a mix of configuration data and resources.
So, maybe it is still possible to reopen it and fix the issue? I will see if it is possible to do that by just modifying the Spring templates, if yes, will try to create a PR for the issue.

@ok11
Copy link

ok11 commented Feb 14, 2018

And a workaround by @erreobi may not work for everyone. For example, I never check in a generated code to a source repository and never copy parts of it to my sources. It is an endless source of inconsistencies. Just imagine, you changed your Swagger model and the generated code is changed. Who should then know all places, where a part of the generated code has been replicated to? This might work for a one-person project, but in a team environment might be challenging...

@Rampai94
Copy link

Rampai94 commented Mar 1, 2018

This issue still persists. Please let us know when a fix is expected.

@kerkhofsd
Copy link
Contributor

Adding

...
@RequestMapping(value = "{{{contextPath}}}")
...

as annotation to api.mustache in the JavaSpring templates seems a viable solution to me. Or am I missing something here?

@ok11
Copy link

ok11 commented Mar 28, 2018

@kerkhofsd this will produce request mapping with only a base path. What worked for me, is
@RequestMapping(value = "{{{contextPath}}}{{{path}}}",...
I will fire a PR in a couple of days...

@kerkhofsd
Copy link
Contributor

kerkhofsd commented Mar 28, 2018

@ok11
I meant adding it as an annotation on class/interface level:

@RequestMapping(value = "{{{contextPath}}}")
public interface {{classname}} {

@ok11
Copy link

ok11 commented Mar 29, 2018

Right. Sorry, haven't realized what you mean. Yes, your suggestion is definitely better. Would you then initiate a PR with the proposal?

kerkhofsd pushed a commit to kerkhofsd/swagger-codegen that referenced this issue Mar 30, 2018
@imduffy15
Copy link

@kerkhofsd Any chance of a PR for your fork and fix, would be great to see it fixed upstream.

kerkhofsd pushed a commit to kerkhofsd/swagger-codegen that referenced this issue May 3, 2018
kerkhofsd pushed a commit to kerkhofsd/swagger-codegen that referenced this issue May 3, 2018
@kerkhofsd
Copy link
Contributor

#8131
@imduffy15 thanks for the reminder

@imduffy15
Copy link

Awesome! Thank you.

@talaviss
Copy link

talaviss commented Aug 12, 2018

I don't understand if this problem was fixed and merged to the master branch since even though i have basePath i my swagger file I don't see it in the generated interface file

@ryanwalker
Copy link

ryanwalker commented Aug 17, 2018

Any word on this? BasePath is still not being added to my requestMapping in Spring.

Here's the relevant part of my spec file.

host: 'localhost:8080'
basePath: /v1
...
paths:
  /free-busy:
...

Here's the request mapping from the java controller. There is no v1 :(

@RequestMapping(value = "/free-busy",

This should be @RequestMapping(value = "/v1/free-busy")

@kerkhofsd
Copy link
Contributor

kerkhofsd commented Aug 20, 2018

#8131
PR still open...

@samuelAle
Copy link

Ping...

@rycler
Copy link

rycler commented Nov 13, 2018

Any progress on this? Currently we manually have to copy the base path and attach it to hostname in configuration. That should not be the case. Currently the basepath in swagger basically useless...

This is an issue both server( and I can understand that it may be difficult to implement ) but whyfor client as well?

@wilgao
Copy link

wilgao commented Mar 27, 2019

Again this is something that we've run into when developing using Swagger's generated Spring code.

@praseedasathaye
Copy link

when will this fix be merged?

@martedesco
Copy link

Hello, would be interested in an update as well! Thanks

@HugoMario
Copy link
Contributor

hey guys, i'll take a look on this next week.

@rafarolo
Copy link

https://stackoverflow.com/a/59924228/5626568

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

Successfully merging a pull request may close this issue.