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

Fix Swagger JSON autogeneration issues. #4845

Merged

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Sep 2, 2018

When autogenerating a swager client from the current swagger.v1.json, at least in Java, there is an issue whereby the use of schema references for the forbidden and empty responses leads to a broken client being created due to swagger believing that these represent an unspecified object. This pull requests removes these unnecessary schema references for the API references where a description for the empty/forbidden responses is provided.

@codecov-io
Copy link

codecov-io commented Sep 2, 2018

Codecov Report

Merging #4845 into master will increase coverage by 0.03%.
The diff coverage is 47.1%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4845      +/-   ##
==========================================
+ Coverage    37.4%   37.43%   +0.03%     
==========================================
  Files         308      308              
  Lines       45612    45711      +99     
==========================================
+ Hits        17060    17113      +53     
- Misses      26095    26138      +43     
- Partials     2457     2460       +3
Impacted Files Coverage Δ
routers/api/v1/org/member.go 0% <ø> (ø) ⬆️
routers/api/v1/org/hook.go 0% <0%> (ø) ⬆️
routers/api/v1/repo/release_attachment.go 0% <0%> (ø) ⬆️
routers/api/v1/repo/issue_tracked_time.go 0% <0%> (ø) ⬆️
routers/api/v1/repo/issue.go 30.17% <0%> (+0.08%) ⬆️
routers/api/v1/repo/label.go 0% <0%> (ø) ⬆️
routers/api/v1/repo/milestone.go 0% <0%> (ø) ⬆️
routers/api/v1/admin/org.go 0% <0%> (ø) ⬆️
routers/api/v1/repo/key.go 20.24% <0%> (-0.26%) ⬇️
routers/api/v1/admin/repo.go 0% <0%> (ø) ⬆️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b2fcad...9d1a4ee. Read the comment docs.

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 2, 2018
@lunny lunny added the type/docs This PR mainly updates/creates documentation label Sep 7, 2018
@zeripath zeripath force-pushed the swagger-json-autogeneration-fixes branch from f617ff1 to f5f2cbe Compare September 7, 2018 09:31
@sapk
Copy link
Member

sapk commented Sep 18, 2018

I remember the opposite problem with go version of swagger but since CI doesn't seems to complain. LGTM

@bkcsoft bkcsoft added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 18, 2018
@bkcsoft bkcsoft added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 18, 2018
@zeripath zeripath force-pushed the swagger-json-autogeneration-fixes branch 3 times, most recently from bdf7f2c to 0ffd663 Compare September 25, 2018 16:59
@zeripath zeripath force-pushed the swagger-json-autogeneration-fixes branch from 0ffd663 to 5bf3306 Compare September 29, 2018 13:55
@zeripath zeripath force-pushed the swagger-json-autogeneration-fixes branch from 273541b to b76928a Compare October 19, 2018 17:19
Remove unnecessary schema references for the forbidden and empty responses

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor Author

My fixes for the CreateAccessToken are only partial see #5126.

It's not obvious where the setting for AccessToken going in to the header is set.

@zeripath zeripath force-pushed the swagger-json-autogeneration-fixes branch from b76928a to ad17f7d Compare October 20, 2018 11:52
@zeripath
Copy link
Contributor Author

Most of the swagger api descriptions incorrectly use the integer type when the underlying go code actually uses int64. I've added further changes to account for this.

@zeripath
Copy link
Contributor Author

I've also noted that the swagger description of GET /repos/{owner}/{repo}/pulls was also inadequate

@zeripath zeripath force-pushed the swagger-json-autogeneration-fixes branch from fb9d4cf to c232452 Compare October 20, 2018 13:07
zeripath added a commit to zeripath/java-gitea-api that referenced this pull request Oct 20, 2018
@zeripath zeripath force-pushed the swagger-json-autogeneration-fixes branch from c232452 to aefbac0 Compare October 20, 2018 13:27
routers/api/v1/admin/org.go Outdated Show resolved Hide resolved
@zeripath zeripath force-pushed the swagger-json-autogeneration-fixes branch from aefbac0 to d7e86e6 Compare October 20, 2018 15:28
@lafriks
Copy link
Member

lafriks commented Oct 20, 2018

@lunny need your approval

zeripath added a commit to zeripath/java-gitea-api that referenced this pull request Oct 20, 2018
@techknowlogick techknowlogick merged commit 43f9233 into go-gitea:master Oct 21, 2018
@zeripath zeripath deleted the swagger-json-autogeneration-fixes branch October 21, 2018 07:19
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/docs This PR mainly updates/creates documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants