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.

Sign up for GitHub

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

Jump to bottom

Generate swagger json #1402

Merged
merged 4 commits into from
May 2, 2017
Merged

Generate swagger json #1402

merged 4 commits into from
May 2, 2017

Conversation

sapk
Copy link
Member

@sapk sapk commented Mar 28, 2017 •
edited
Loading

Use https://github.com/go-swagger/go-swagger with go generate to (auto-)build public/swagger.json

Related : #194 go-gitea/go-sdk#53

I will fill missing API descriptions. For the responses descriptions, tags must be add to sdk repo (so in a other PR).

You can test rendering at http://editor.swagger.io and copy-paste swagger.json.

Documented API :
  • GET /version (getVersion)
  • POST /markdown (renderMarkdown)
  • POST /markdown/raw (renderMarkdownRaw)
  • GET /users/search (userSearch)
  • GET /users/:username (userGet)
  • GET /user (userGetCurrent)
  • GET /users/:username/tokens (userGetTokens)
  • POST /users/:username/tokens (userCreateToken)
  • GET /users/:username/repos (userListRepos)
  • GET /user/repos (userCurrentListRepos)
  • GET /user/keys userCurrentListKeys
  • GET /users/:username/keys userListKeys
  • GET /user/keys/:id userCurrentGetKey
  • POST /user/keys userCurrentPostKey
  • DELETE /user/keys/:id userCurrentDeleteKey
  • GET /user/gpg_keys userCurrentListGPGKeys
  • GET /users/:username/gpg_keys userListGPGKeys
  • GET /user/gpg_keys/:id userCurrentGetGPGKey
  • POST /user/gpg_keys userCurrentPostGPGKey
  • DELETE /user/gpg_keys/:id userCurrentDeleteGPGKey
  • GET /user/followers userCurrentListFollowers
  • GET /users/:username/followers userListFollowers
  • GET /user/following userCurrentListFollowing
  • GET /users/:username/following userListFollowing
  • GET /user/following/:username userCurrentCheckFollowing
  • GET /users/:username/following/:target userCheckFollowing
  • PUT /user/following/:username userCurrentPutFollow
  • DELETE /user/following/:username userCurrentDeleteFollow
  • GET /users/:username/starred userListStarred
  • GET /user/starred userCurrentListStarred
  • GET /user/starred/:username/:reponame userCurrentCheckStarring
  • PUT /user/starred/:username/:reponame userCurrentPutStar
  • DELETE /user/starred/:username/:reponame userCurrentDeleteStar
  • GET /users/:username/subscriptions userListSubscriptions
  • GET /user/subscriptions userCurrentListSubscriptions
  • GET /repos/:username/:reponame/subscription userCurrentCheckSubscription
  • PUT /repos/:username/:reponame/subscription userCurrentPutSubscription
  • DELETE /repos/:username/:reponame/subscription userCurrentDeleteSubscription
  • POST /org/:org/repos createOrgRepo
  • ..

@lunny lunny added this to the 1.2.0 milestone Mar 29, 2017
@lunny lunny added the type/docs This PR mainly updates/creates documentation label Mar 29, 2017
@sapk sapk force-pushed the generate-swagger branch 4 times, most recently from 5f70d74 to dfdf6fa Compare April 14, 2017 00:56
@lunny
Copy link
Member

lunny commented Apr 14, 2017

Maybe we need an API docs site? Or a link on docs.gitea.io?

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 14, 2017
@sapk
Copy link
Member Author

sapk commented Apr 14, 2017

Once fully implented we could use swagger ui for presentation. There is two choice possible :

  • We use a remote one. This require to have CORS enable to be able to execute test request from ui. (example : http://petstore.swagger.io/)
  • We use expose a swagger ui inside gitea and we have complete access to api and docs is accessible alongside gitea.

As global documentation we could start a docker containing swagger ui (allready avalaibe) for try.gitea.io : https://github.com/swagger-api/swagger-ui

@sapk
Copy link
Member Author

sapk commented Apr 14, 2017 •
edited
Loading

I think that I will simply copy https://github.com/swagger-api/swagger-ui/tree/master/dist into public/assets. This will provide swagger-ui on all instance.

@lunny
Copy link
Member

lunny commented Apr 14, 2017

@sapk so add new make subcommand is enough?

@sapk
Copy link
Member Author

sapk commented Apr 14, 2017 •
edited
Loading

@lunny For generating swagger.json, I added a go generate here :
https://github.com/go-gitea/gitea/pull/1402/files#diff-2620fdbeeb83ce43692534e6c2c39452R5

So make generate will update it and place it under public folder.

For swagger-ui, I am thinking of adding a other step to make assets

@sapk
Copy link
Member Author

sapk commented Apr 14, 2017

In parallel, I will PR to fix little things like go-gitea/go-sdk#50 (it will need a PR in gitea also) to put all data structure under go-sdk and add missing api request. A other PR to go-gitea/go-sdk will be added to add swagger comment to api response and params.

This PR is a long running one to complete ^^. It will maybe need a second pass to adjust comments but the base will be there.

@lunny
Copy link
Member

lunny commented Apr 14, 2017

Or maybe we can add a link on the bottom or top on Gitea named API to the API docs home page.

@sapk sapk force-pushed the generate-swagger branch 3 times, most recently from 9d6d4f2 to cefa415 Compare April 14, 2017 23:03
bkcsoft
bkcsoft suggested changes Apr 18, 2017
View reviewed changes
Makefile Outdated
git clone --depth=10 https://github.com/swagger-api/swagger-ui.git /tmp/swagger-ui
mv /tmp/swagger-ui/dist public/assets/swagger-ui
rm -Rf /tmp/swagger-ui
sed -i "s/http:\/\/petstore.swagger.io\/v2\/swagger.json/..\/..\/swagger.v1.json/g" public/assets/swagger-ui/index.html
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use "s;https://...;...;g" instead for readability?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a big expert in sed. ^^

Makefile Outdated
rm -Rf public/assets/swagger-ui
git clone --depth=10 https://github.com/swagger-api/swagger-ui.git /tmp/swagger-ui
mv /tmp/swagger-ui/dist public/assets/swagger-ui
rm -Rf /tmp/swagger-ui
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just download a tar.gz release instead. We shouldn't depend on latest master being functional

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use git tag now to fix the version. (less things to change ^^)

routers/api/v1/misc/markdown.go Outdated
@@ -15,6 +15,18 @@ import (
// Markdown render markdown document to HTML
// see https://github.com/gogits/go-gogs-client/wiki/Miscellaneous#render-an-arbitrary-markdown-document
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe update the doc links while you're at it :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or remove them since they now become redundant 😅

vendor/vendor.json Outdated
@@ -9,10 +9,13 @@
"revisionTime": "2017-04-07T07:44:04Z"
},
{
"checksumSHA1": "32qRX47gRmdBW4l4hCKGRZbuIJk=",
"checksumSHA1": "oUPCyOhO8e61S2KVMJzM8sDnFdA=",
"origin": "github.com/sapk-fork/gitea-go-sdk/gitea",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be fixed before merge :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this temporary and will be added in a PR in sdk. All the input and output format (used by api) are in sdk and I have already done one PR to move a struct in gitea to sdk.

@bkcsoft
Copy link
Member

bkcsoft commented Apr 18, 2017

Actually, would it make sense to have this in go-gitea/sdk instead?

@lunny
Copy link
Member

lunny commented Apr 19, 2017

Is this still WIP?

@sapk
Copy link
Member Author

sapk commented Apr 19, 2017

@lunny yes because it is missing some api url and it need a PR in sdk for all format response and request. But you can review tooling around it like Makefile and the only changes that will append next are swagger comment so It will be ready to merge when complete.

@lunny
Copy link
Member

lunny commented Apr 29, 2017

@sapk maybe move this to v1.3?

@sapk
Copy link
Member Author

sapk commented Apr 29, 2017

@lunny There is two possibility :

  • Push it in v1.2, everything is working it just need to doc (add comment) to the not yet documented API in other(s) PR.
  • Wait v1.3 to have complete documentation.

@bkcsoft
Copy link
Member

bkcsoft commented Apr 29, 2017

@lunny @sapk For API-docs I'm actually fine with having partial docs for a while. Better than to have no docs at all 😅

@bkcsoft
Copy link
Member

bkcsoft commented Apr 29, 2017

This will have my LG-TM once the gogs docs links are gone 🙂

@jonasfranz
Copy link
Member

The swagger file is not complete. Some parts of the API are missing like the issue management.

@sapk
Copy link
Member Author

sapk commented Apr 29, 2017

@JonasFranzDEV That what I say ^^
@bkcsoft ok I will clean this up for fully documented in swagger

@sapk sapk mentioned this pull request Apr 29, 2017
Merged
@sapk
Fully working base
7b895fe 
Generate swagger.json into public/
Add swagger-ui auto-installation

Add footer link to local swagger-ui
Add /swagger url for using app url.
Fix Swagger-UI version via git tag
Use a easier to understand sed syntax
Vendor temporary PR for sdk
@sapk
Copy link
Member Author

sapk commented Apr 29, 2017

I rebase/squash all changes into a fully working version. I vendor go-gitea/go-sdk#53 in the wait of it validation so that this PR is working for validation also. I will revert it back as soon that go-gitea/go-sdk/pull/53 is merged.

For testing, just build gitea like always, start it, and go to http://localhost:3000/swagger

Fix/Hack that I have implemented : (This is if someone has a better/cleaner idea)

  • Swagger-ui doesn't support recursive deps like GPGKey that contain sub-GPGKey that contain .... -> I use a sedin go generate to set sub-GPGKey as object.

Improvements to be done in other PR :

  • Add all missing API
  • Improve documentation of parameters and responses. (For example : add a regex for armored key params)
  • Add params for in url/path params. As of now they are displayed but we need struct to link them to params in code and be able to set them in swagger-ui.

Over all, this PR should not have an big impact on code functionality as it is mostly comments.

@sapk sapk changed the title [WIP] Generate swagger json Generate swagger json Apr 29, 2017
@lunny
Copy link
Member

lunny commented Apr 30, 2017

@sapk I like first chosen. So is it ready to review and merge?

@sapk
Copy link
Member Author

sapk commented Apr 30, 2017 •
edited
Loading

@lunny Yes, the only thing that bother me is the little fix/hack for recursive structure not supported by swagger-ui but we need it. (describe in my previous comment)

@KangoV
Copy link

KangoV commented Apr 30, 2017

Which version are you using? We've just switched to using swagger-ui v3.0.7. Has a much nicer look. This is getting ready for OpenAPI v3.0 later in the year: http://swagger.io/new-swagger-ui-and-swagger-editor/.
It's also much easier to integrate.

@sapk
Copy link
Member Author

sapk commented Apr 30, 2017

@KangoV I have added swagger-ui v3.0.7 (get via makefile). I used go-swagger to generate the JSON from code comments.

@lunny
Copy link
Member

lunny commented May 2, 2017

Is this ready for review?

@sapk
Copy link
Member Author

sapk commented May 2, 2017 •
edited
Loading

@lunny yes like I said two days ago ^^
and since go-gitea/go-sdk#53 is merged so nothing else to do.

@lunny
Copy link
Member

lunny commented May 2, 2017

LGTM

@tboerger tboerger 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 May 2, 2017
@bkcsoft
Copy link
Member

bkcsoft commented May 2, 2017

LGTM

@bkcsoft
Copy link
Member

bkcsoft commented May 2, 2017

make LG-TM work

@tboerger tboerger 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 May 2, 2017
@bkcsoft bkcsoft merged commit 3edb0c5 into go-gitea:master May 2, 2017
@sapk sapk deleted the generate-swagger branch June 17, 2017 17:14
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkcsoft bkcsoft bkcsoft approved these changes

Assignees
No one assigned
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
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@sapk @lunny @bkcsoft @jonasfranz @KangoV @tboerger