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

Swagger 2 integration #1508

Closed
cbornet opened this issue May 22, 2015 · 31 comments
Closed

Swagger 2 integration #1508

cbornet opened this issue May 22, 2015 · 31 comments
Milestone

Comments

@cbornet
Copy link
Member

cbornet commented May 22, 2015

Since springfox-swagger2 v2.0.0 got released, it would be great to use it to have a swagger 2.0 api-docs running in parallel of the swagger 1.2 version as it provides a lot more functionalities (for instance support of map objects).

Note : Swagger UI v2.1 will support 2.0 spec and could be used in the future for the on-line doc.

@jdubois
Copy link
Member

jdubois commented May 22, 2015

Is SpringFox the new name of Swagger Spring MVC, which we are using?
Yes, if there is a new version, we should upgrade

@cbornet
Copy link
Member Author

cbornet commented May 23, 2015

Yes, springfox is the new name of swagger spring mvc for version 2.0.

@gmarziou
Copy link
Contributor

gmarziou commented Jun 5, 2015

Just trying to migrate my jhipster app to SpingFox 2.0.1: the API has completely changed!

@dilipkrish
Copy link

Let me know if you need help with this. Swagger core Api is changing as well (package names are moving to io.swagger from com.wordnik, so you might want to hold off until next week or wait for 2.0.2, which will be based on the released version of swagger-core.

The memory footprint is orders of magnitude smaller (no more scala related bloat). There are no more dependencies on swagger-core other than the models and annotations. Also, there are a few of performance improvements planned for 2.1 that should make swagger even snappier.

Also I might add that, we have a really simple way to pull in the swagger-ui as a resource jar.

@gmarziou
Copy link
Contributor

gmarziou commented Jun 6, 2015

Thanks @dilipkrish with the examples, I was able to make it work.

I noted that URLs are different from jhipster had:

http://localhost:8080/v2/api-docs for the JSON description

http://localhost:8080/swagger-ui.html for the UI

springfox

@gmarziou
Copy link
Contributor

gmarziou commented Jun 6, 2015

I have to do some cleanup and then I'll submit a PR

@gmarziou
Copy link
Contributor

gmarziou commented Jun 7, 2015

Here is a commit showing changes required on jhipster sample app.
Please review and let me know if you want a PR on generator or if you prefer to wait for springfox 2.1 re-packaging as announced by @dilipkrish

@cbornet
Copy link
Member Author

cbornet commented Jun 7, 2015

@dilipkrish the swagger ui webjar is marked as WIP in your docs. Do you think it is safe to use in jhipster or is it preferable to stay for now with the bower version ?
BTW, swagger-ui 2.1.0 just got released with support for v2.0 specs !

@dilipkrish
Copy link

@cbornet we haven't updated the readme for the module since 2.0.0-SNAPSHOT 😮 thanks for pointing that out! Here is the official documentation. I'll make a note to fix that!

We've had couple of fixes since and I'd like to think its stable as of 2.0.2-SNAPSHOT. Also there other reason for not advertising it as stable is because up until today the web jar was built on a prerelease swagger-ui library. We should be releasing 2.0.2 that pretty soon with the latest swagger-ui (2.1.0) and latest swagger-core (1.5.0 note: that that they changed the package names from com.wordnik.swagger to io.swagger).

@gmarziou
Copy link
Contributor

gmarziou commented Jun 8, 2015

I my commit, you'll note that I have removed the bower dependency on swagger-ui

@gmarziou
Copy link
Contributor

gmarziou commented Jun 9, 2015

@dilipkrish in dev on localhost:8081, I am getting CORS message "Can't read from server. It may not have the appropriate access-control-origin settings." at the top of the page. I found this issue springfox/springfox#77 but it is for old version and I could not find what is done to workaround this in recent demos.

@cbornet
Copy link
Member Author

cbornet commented Jun 9, 2015

@gmarziou Since the webapp and the API are on the same domain this is probably a problem different than CORS hell.

@dilipkrish
Copy link

@gmarziou Is that happening when you actually hit the "try out" button?

@gmarziou
Copy link
Contributor

gmarziou commented Jun 9, 2015

It's happening from the beginning and I get this in the browser's console:

XMLHttpRequest cannot load http://petstore.swagger.io/v2/swagger.json. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://localhost:8081' is therefore not allowed access.

This triggers another error at swagger-ui.html:51

Uncaught ReferenceError: log is not defined

@dilipkrish
Copy link

@gmarziou that would make sense that you have a CORS issue there, since it reaches out to http://petstore.swagger.io/v2/swagger.json. Once the page is loaded tho' you should see the dropdown with the swagger urls served out of the application.

@gmarziou
Copy link
Contributor

gmarziou commented Jun 9, 2015

Yes I can see my api but it's not clean to see this error message like below:

2015-06-09_16h35_03

@dilipkrish
Copy link

@gmarziou could you create an issue for that! that is definitely not desirable! 😬

@gmarziou
Copy link
Contributor

gmarziou commented Jun 9, 2015

Here it is springfox/springfox#781

@dilipkrish
Copy link

Just released 2.0.2 and also polished up the swagger-ui integration. Thanks for your feedback @gmarziou!

@gmarziou
Copy link
Contributor

Great, I will update my jhipster sample project and if it's OK for jhipster team, I will propose a PR including migration to SpringFox and static doc generation.

@gmarziou
Copy link
Contributor

Here is a commit for sample webapp generated by jhipster 2.16.1 that shows the required changes.
@jdubois could you have a look and let me know if you want a PR.

It brings:

  • no more swagger-ui directory under webapp
  • no more swagger dependency in bower.json
  • version of the API in application.yml
  • static docs generated in HTML and PDF under target/api-docs
  • possibility to customize docs by editing src/docs/asciidoc/index.adoc

Static doc generation currently works with maven only but could be easily adapted to gradle as explained here

@gmarziou
Copy link
Contributor

If you prefer I could create 2 PRs: one for the springfox migration and one the static doc generation.

I'd be curious to know whether people really uses swagger web UI in prod profile, in my projects I enable it only in dev profile and if someone asked for it, I would give the static doc in PDF or HTML.

@jdubois
Copy link
Member

jdubois commented Jun 23, 2015

@gmarziou this is the next issue I'll have a look at, hopefully tonight.
Yes it could only work in dev profile, but as it's secured I don't think it makes a problem in production. PDF is great, but you can't execute it. Let's first migrate our current features as they currently work, and improve them afterwards.

@gmarziou
Copy link
Contributor

OK, I'll wait for your feedback before working on the PR

@gmarziou
Copy link
Contributor

I mean I can submit a PR just for migrating to spingfox and then we can work on next steps

@jdubois
Copy link
Member

jdubois commented Jun 23, 2015

@gmarziou yes please do the SpringFox PR first, it looks like we have a lot of issues with Swagger at the moment

@gmarziou
Copy link
Contributor

@dilipkrish : about performance, I was wondering whether we could initialise SpringFox on-demand rather than on application startup. So the price for introspection would be paid only on first request to /v2/api-docs URL.

@dilipkrish
Copy link

That's definitely next on the cards ... And caching of operations, parameters, return types and models.

@gmarziou
Copy link
Contributor

Great, keep up the good work 👍

@cbornet
Copy link
Member Author

cbornet commented Jun 26, 2015

👍 for introspection on first request.

@jdubois
Copy link
Member

jdubois commented Jul 3, 2015

Fixed by #1635

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

No branches or pull requests

4 participants