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

OpenAPI and automatic generation of REST API docs #1965

Merged
merged 10 commits into from
Jan 19, 2019
Merged

Conversation

bentiss
Copy link
Contributor

@bentiss bentiss commented Oct 24, 2018

This is more of a RFC, as I am not sure you'll want this in the scope of the project itself.

The first patch uses esprima to parse the current code base and generate an OpenAPI specification. It's in Python, but should be run only when there is a change in the API.

I'd like to have the output of the script in the public folder so clients can directly connect to the server and retrieve the current API from there.

The second part is a nice to have feature. Instead of relying on lazy coders (like myself) to write down the current API in the doc, we could use widdershins to create the doc for us based on the OpenAPI spec generated above. I had to duplicate the template for openapi of widdershins in the sources to be able to tweak it to remove a little bit of extra boilerplate as the openapi spec is not the primary input source (the code is). Widdershins is under MIT, so I kept the license file there but it should not be a problem as wekan is also MIT.

The docs are also stored in the git tree, this way the wiki can just point at it.

Finally, the doc is missing a bunch of explanations, but that's because the data is not there in the code or is simply not processed.

Note: Apache I-CLA signed


This change is Reviewable

@bentiss bentiss mentioned this pull request Oct 24, 2018
@xet7
Copy link
Member

xet7 commented Oct 24, 2018

What commands you use to generate documentation?

Please separate API to different pages by topic, like at Wekan GitHub wiki. Now they are at one very long page, it's very hard to find info fast.

It would be very nice to generate Wekan documentation website that had well organized navigation.

@bentiss
Copy link
Contributor Author

bentiss commented Oct 24, 2018

What commands you use to generate documentation?

I think I have it written in the openapi/README.md:

widdershins -e ./widdershins_env.json ../public/wekan_api.yml -o wekan_api.md

Please separate API to different pages by topic, like at Wekan GitHub wiki. Now they are at one very long page, it's very hard to find info fast.

I wished I was able to do it before submitting the PR, but couldn't really get to it.
Either the openapi specification I am producing is not good enough, or the tool (widdershins) is not the right one. Other tools I had a look at today produce a different output, but some are standalone servers, so not ideal. I wanted to produce a markdown set of pages, but maybe other solutions might fit better (like asciidoc, doxygen or sphinx).

It would be very nice to generate Wekan documentation website that had well organized navigation.

agree :)

@xet7
Copy link
Member

xet7 commented Oct 24, 2018

One possible way to present documentation would be like Sandstorm does:
https://docs.sandstorm.io/en/latest/
https://github.com/sandstorm-io/sandstorm/tree/master/docs

Wekan docs were originally moved from GitHub repo to GitHub wiki so it would be easier for Wekan contributors to update documentation. Not many documentation contributors are familiar with adding PRs.

@xet7
Copy link
Member

xet7 commented Oct 24, 2018

Current way to contribute documentation is to add text to wiki, and add PR to Wekan website to host images or animgifs. It would be nice to make contributing even easier.

@bentiss
Copy link
Contributor Author

bentiss commented Oct 25, 2018

One possible way to present documentation would be like Sandstorm does:
https://docs.sandstorm.io/en/latest/
https://github.com/sandstorm-io/sandstorm/tree/master/docs

This is using mkdocs, and also relies on markdown. I am fine with this if you prefer it that way, but processing the OpenAPI file into this will be more costly (IMO, we need a custom script that needs to be maintained).

OTOH, widdershins has been designed with slate in mind. This produces a nice single we page that has a sidebar which allows to quickly jump between sections (see https://lord.github.io/slate/).

So we could convert the openapi spec into a slate webpage that is pushed on github pages and the wiki only references it.

Wekan docs were originally moved from GitHub repo to GitHub wiki so it would be easier for Wekan contributors to update documentation. Not many documentation contributors are familiar with adding PRs.
Current way to contribute documentation is to add text to wiki, and add PR to Wekan website to host images or animgifs. It would be nice to make contributing even easier.

I completely understand, and I think having the wiki is probably the best. I don't have an easy solution for the images however. You could rely on a workaround like https://github.com/RWTH-EBC/AixLib/wiki/How-to:-Add-images-to-the-Wiki but having the images properly hosted would be better.

@xet7
Copy link
Member

xet7 commented Oct 25, 2018

@bentiss

Slate looks very nice.

Could this PR go to Wekan website?
https://github.com/wekan/wekan.github.io

And then generated documentation would be at api directory, https://wekan.github.io/api ?

@bentiss
Copy link
Contributor Author

bentiss commented Oct 25, 2018

Slate looks very nice.

Yep. I'll try to come up with a prototype for making the decision on whether we should use it or not.

Could this PR go to Wekan website?
https://github.com/wekan/wekan.github.io

Sure. I still think the openapi spec should be shipped in the public directory of the current wekan code. This way, when you connect to a wekan instance, you get the actual embedded API and can use it properly as it is intended to be used.

So when releasing a new version, I guess the doc should be generated in the wekan website and the spec copied over (if there has been API changes).

And then generated documentation would be at api directory, https://wekan.github.io/api ?

It would be even better to see if we can version it (https://wekan.github.io/api/v1.68, https://wekan.github.io/api/1.70, etc...), so we can actually refer to the correct documentation directly.

@xet7
Copy link
Member

xet7 commented Oct 25, 2018

@bentiss

Ok, so maybe you could add generating openapi to public directory to these Wekan GitHub org repo files. You can presume all repos are cloned to same directory with this script, and generate docs to all directories where they need to be.

wekan/wekan

  • Dockerfile
  • snapcraft.yaml

wekan/wekan-maintainer

  • releases/rebuild-wekan.sh
  • releases/release-wekan-snap.sh
  • releases/release-wekan-sanstorm.sh
  • virtualbox/rebuild-wekan.sh

wekan/wekan.github.io

  • api/v1.68 etc

@bentiss
Copy link
Contributor Author

bentiss commented Oct 26, 2018

@xet7

OK, I'll do what I can

Meanwhile, I just pushed a new update of this branch. I don't think we want to merge it yet as the resulting API spec is still in git, and there is no automation.

However, I used this latest version (with tags and some JSDoc parsing) to publish a prototype here: https://bentiss.github.io/wekan-slate

There are a bunch of things to fix (like why the list on the left is almost alphabetically sorted), we probably need more doc, but this is a start.

@xet7
Copy link
Member

xet7 commented Oct 26, 2018

@bentiss

Thanks! It looks very nice!

@bentiss
Copy link
Contributor Author

bentiss commented Nov 16, 2018

Small update here to say that this has not fell through the cracks.
I spent a good amount of time adding JSDocs to the REST API so we get something nicer in the end:
https://people.freedesktop.org/~tissoire/wekan_api.html

I think this is mostly it, but there are a few functions that are not properly documented in term of parameter typing:

  • Board.add_board_label -> the label parameter is not a string but a collection of {color, name}
  • Card.edit_card -> the parameters labelIds, customFields and members are not a string but collections too.

I can have a local workaround, but I think it would be better to have a correct mapping of the API, especially because this can be used to generate client codes from it.

I also need to make sure python3.6 is used as I use the f-string quite a lot.

The Dockerfile is now updated to generate the openapi spec during the build.
You can generate the HTML file I linked above by using the api2html node module:

api2html -c public/wekan-logo-header.png -o /tmp/api.html public/wekan_api.yml

So I guess adding this to the maintainers script should be easy now.

I have now 2 question @xet7 :

  • for generating the snaps, which debian/ubuntu version are you using? The container on dockerhub to generate snapcrafts is using Ubuntu 16.04 IIRC, and this doesn't ship with python 3.6
  • Can you have a look at the JSDocs I put and tell me if this is acceptable? (especially the @return_type tag)

@xet7
Copy link
Member

xet7 commented Nov 16, 2018

I have now 2 question @xet7 :

  • for generating the snaps, which debian/ubuntu version are you using? The container on dockerhub to generate snapcrafts is using Ubuntu 16.04 IIRC, and this doesn't ship with python 3.6

I have Ubuntu 18.04 on bare metal server, where downloads are hosted, there is Python 3.6.6, so I can generate documentation there, and have docs at for example https://docs.wekan.team/v1.70/...

I build Snap and Sandstorm versions on Ubuntu 16.04 VM on server, because Snap build service is slower.

Docker is built automatically at https://quay.io/wekan/wekan

There could be cron job that builds docs and for example .tar.gz of those docs, and Dockerfile and snapcraft.yaml could download those to some directory into package itself, if those docs needs to be embedded into package.

  • Can you have a look at the JSDocs I put and tell me if this is acceptable? (especially the @return_type tag)

What do you mean? Where?

@xet7
Copy link
Member

xet7 commented Nov 16, 2018

I mean https://api.wekan.team/v1.70/...

@bentiss
Copy link
Contributor Author

bentiss commented Nov 16, 2018

  • Can you have a look at the JSDocs I put and tell me if this is acceptable? (especially the @return_type tag)

What do you mean? Where?

For instance, in models/cards.js I have:

  /**
   * @operation get_all_cards
   * @summary get all cards attached to a list
   *
   * @param {string} boardId the board ID
   * @param {string} listId the list ID
   * @return_type [{_id: string,
   *                title: string,
   *                description: string}]
   */

The @return_type tag is definitively not standard, but I couldn't find a way to express the various fields in the returned dict.

A proper standard JSDoc tag would be:

  /**
   * ...
   * @returns {Array} an array of {_id, title and description}
   */

The problem here is that the description of the items in the dict is free form, so rather hard to parse in the end.

An other solution might be to use sphinx as the type can be more defined as far as I can tell, but there is no guarantees I'll get to the same results :(

I mean https://api.wekan.team/v1.70/

Hmm, I can't seem to access the website just yet. Maybe I need to wait for the DNS entry to get refreshed?

Regarding embedding the API in snaps/Dockers, the solution of an external service is nice, but I would personalty prefer the api to be generated and embedded as they are build. This would allow to spot earlier the issues.

Maybe the solution would be to write down the documentation directly in the openapi language. The previous example looks like the following, but I find it harder to write down than JSDocs:

  /api/boards/{board}/lists/{list}/cards:
    get:
      operationId: get_all_cards
      summary: get all cards attached to a list
      tags:
        - Cards
      parameters:
        - name: board
          in: path
          description: |
            the board ID
          type: string
          required: true
        - name: list
          in: path
          description: |
            the list ID
          type: string
          required: true
      produces:
        - application/json
      security:
          - UserSecurity: []
      responses:
        '200':
          description: |-
            200 response
          schema:
            type: array
            items:
              type: object
              properties:
                _id:
                  type: string
                title:
                  type: string
                description:
                  type: string

Anyway, I might just replace the 61 occurrences of f-strings in generate_openapi.py into python 3.0 compatibles ones to solve the issue entirely.

@xet7
Copy link
Member

xet7 commented Nov 16, 2018

I have not created that https://api.wekan.team subdomain yet.

I don't know is embedding API docs good idea. If someone is running many Wekan Docker containers to use multiple cores of server, space usage really adds up.

@xet7
Copy link
Member

xet7 commented Nov 16, 2018

I don't know is there any problem with return_type tag etc, because I'm not so familiar with OpenAPI. If there is sometime later some improvement, then that can be added.

I would think there should be just a script to run and have these at https://api.wekan.team/... for a start.

Also some hosting providers run multiple instances of Wekan on same server for different customers, space usage does add up.

@bentiss
Copy link
Contributor Author

bentiss commented Nov 16, 2018

I have not created that https://api.wekan.team subdomain yet.

heh :)

I don't know is embedding API docs good idea. If someone is running many Wekan Docker containers to use multiple cores of server, space usage really adds up.

Well, we only need to embed the openapi yaml file, not the html. The YAML is 73K once generated. And having it embedded means a client can retrieve it on the service directly and can use the accurate API directly.
The other point of having it embedded is because most of the openapi clients expects it to be the case, and they automatically replace the host in the client object by the domain you used to retrieve the API.

Actually, what could be done is to have only a small almost static embedded file that just references the yaml in the proper api.wekan.team folder. This way, we win on all sides: generating the embedded file is just a cat away, and we don't need the python 3.6 requirement everywhere (and don't need to install/remove python in Dockerfile/snapcraft.yml)

@xet7
Copy link
Member

xet7 commented Nov 16, 2018

Ok, then yaml needs to be included into Wekan, because Wekan does not load any content from outside links by default, that is core Wekan security feature for offline use.

@xet7
Copy link
Member

xet7 commented Nov 16, 2018

Hmm, probably it would be good to have documentation added to translatable strings and have all that in embedded into wekan as help buttons (or something similar) so that every feature would documented, with descriptions what would be there. Also Wekan website text could be added to translatable strings and have Wekan website in many languages.

xet7 and others added 7 commits January 15, 2019 15:47
This reverts commit f61942e.

Adding a member is actually already handled by
POST', '/api/boards/:boardId/members/:userId/add'

So this function is purely duplicated.

Not to mention that the '/add' one allows to set permissions
so this one in this commit is less interesting.
The API is generated by a custom script that parses the models directory.
Once the API is generated, tools like https://editor.swagger.io/ or
Python bravado can parse the file and generate a language friendly API.

Note that the tool generate an OpenAPI 2.0 version because bravado
doesn't handle OpenAPI 3.0.

The script also parses the JSDoc with a custom parser to allow
customization of the description of the fields.
So we can have a decent REST API documentation generated.
When we build the docker container, we need to generate the openapi
description in it so the geenrated API actually matches the code
the container is running.
It is common to use Ubuntu 16.04 to build snaps. For example,
the official docker container to build snaps is using this old
distribution.

However, Ubuntu 16.04 ships Python 3.5.X which is not compatible
with the f-strings in generate_openapi.py. This is sad, because
we need to use the `.format()` syntax to make it compatible.
When pulling the docker container snapcore/snapcraft
to build the snap, those 2 packages are not present
by default leading to a failure in the snap creation.

Note: it is good to call `apt-get update` before
`snapcraft` or the build will fail.
Same thing than in the Dockerfile, snaps need
to embed the current openapi yaml file.
@bentiss bentiss changed the title RFC: OpenAPI and automatic generation of REST API docs OpenAPI and automatic generation of REST API docs Jan 18, 2019
@bentiss
Copy link
Contributor Author

bentiss commented Jan 18, 2019

So, after a long period of time, this is hopefully in a better shape:

  • the yaml file corresponding to the REST API is now embedded in /api/wekan.yml for both snaps and docker
  • the html file is also generated and embedded in the snap and docker images in /api/wekan.html
  • there is a custom JSDoc parser that can now provide the same level of details we have in the wiki
  • many improvements in the python script :)

I made sure we can ship the html files in the snap and docker images, but honestly, I think using a https://api.wekan.team subdomain would be better.

Adding the doc generation to your maintainer scripts should be easy as you just need esprima-python, the 2 node modules api2html and mkdirp.

Hmm, probably it would be good to have documentation added to translatable strings and have all that in embedded into wekan as help buttons (or something similar) so that every feature would documented, with descriptions what would be there. Also Wekan website text could be added to translatable strings and have Wekan website in many languages.

I am not sure translating the REST API doc is a good idea. The persons who are using it are developers and English is a requirement. We can always add a post-processing to translate the doc, but is it worth it?

But I agree, translating the website is a good idea though :)

@bentiss
Copy link
Contributor Author

bentiss commented Jan 18, 2019

And an example of generated file (from the snap, with my to-be-submitted card-colors patch) can be found here: https://bentiss.github.io/wekan-slate/wekan.html

Aligning with the requirement to run the container without
external resources: embed the documentation of the REST API
directly in the Docker image.
Same for snap: embed the documentation of the REST API
in the snap.
@bentiss
Copy link
Contributor Author

bentiss commented Jan 18, 2019

grmbl, of course, the html was not correctly generated in the docker image.

Fixed now.

@xet7 xet7 merged commit 048c3cd into wekan:devel Jan 19, 2019
xet7 added a commit that referenced this pull request Jan 19, 2019
@bentiss bentiss deleted the openapi branch January 22, 2019 13:51
@xet7 xet7 mentioned this pull request Jan 23, 2019
@arradoq
Copy link

arradoq commented Jan 23, 2019

Thanks for bring the problem here. My problem is up till now I just able to login and get the token, the rest features gave zero response when curling from terminal. I use wekan source-installation.

@xet7
Copy link
Member

xet7 commented Apr 2, 2019

I have added API docs to https://wekan.github.io/api/

@xet7
Copy link
Member

xet7 commented Apr 2, 2019

Before using API, login to api to get Bearer token as form data, with admin user username and password:
https://github.com/wekan/wekan/wiki/REST-API#example-call---as-form-data

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

Successfully merging this pull request may close these issues.

3 participants