API reference generated from Open API spec file #40
Conversation
…/ConvivioTeam/tech-docs-gem into feature/GTD-30-api-reference-new * 'feature/GTD-30-api-reference-new' of https://github.com/ConvivioTeam/tech-docs-gem: GTD-30 Add openapi3_parser to gemspec and update api reference parsing code GTD-30 Add support for loading the config file and checking api_path is set GTD-30 initial commit of api reference
…and print their basic info
Schema reference is WIP
Also convert the class into a Middleman extension. This means it has access to the Middleman app instance. We can use this to reference in the custom markdown engine and also get the config without having to pass the config yml file. Performance bonus: The class is only initialised on start up, not on every paragraph
This reverts commit 0baf0cb.
Also for some reason whitespace in erb files is being output in the html. This is documented behaviour, but I'm not sure why this is happening now
|
I've got an experimental branch attempting to add in some unit tests but it requires a big restructure as it seems to be very difficult to unit test a Middleman extension. I'm not even sure which functions are worth unit testing that isn't tested in the integration test as this extension doesn't have complex logic. |
|
I haven't looked at the code yet, but here's some initial feedback from looking at the pet store example. I'll take a deeper look later today or tomorrow and check it against some different specs. With the previous solution I had a lot of trouble documenting responses with nested objects and arrays, so I'm keen to try out that use case. Things that don't work wellOn the "API /Pets" page where you embed the endpoint docs, the page doesn't include the schema section, which means that the schema links in the responses table don't work (the "Swagger Petstore v1.0.0" pages render these sections so the links work there). I think we also need a page in the manual that explains the In the schema section, the headings are a bit confusing. There is an empty section rendered for "Pets" and the Error and Pet schemas have a subheading "properties" which could be removed. CustomisabilityIt seems like at the moment you have to choose whether you want everything autogenerated from the spec, or you have to create a markdown file for every endpoint, which is fine, but the ideal for me is probably somewhere in the middle. For example, with the registers tech docs we moved "Rate limits" and "HTTP status codes" into the API reference section, and I'm not sure we'd still be able to do this if we were generating all the endpoint documentation from the spec. When I tried out dapperdox a while back it was easier to customise, because the page is divided into multiple components and you can choose which bits you want to override. Personally I would prefer if there was a separate page for every resource by default, rather than having it all on one page, because it feels quite information-dense. For a real API, the property tables can get very long, and I find it a bit annoying when if I scroll a bit too far I see information for some unrelated endpoint. This could be addressed in other ways though - Stripe do the single page thing very well, but they also have a lot of vertical space between stuff, which makes it easier to read. |
|
@MatMoore Thanks for the feedback. There are a few bugs you've pointed out that we will fix today. The customisability stuff, I think I understand what you're saying. Initially I was leaning towards single page as the primary use case, I think we got some initial feedback preferring having all the API information on one page for quicker searching. You're right we could do a lot more make the content render from the spec more flexible. Are you able to give some examples of the types of parameters you would pass into I think if we wanted to move towards a more customisable system similar to Dapperdox then we could figure out a way to dynamically search for templates so you could override templates per-node. |
|
@lewisnyman I don't know how viable this is, but I was thinking that if I was using
and be able to render them inline. But I would definitely consider this a 'nice to have' as I don't know whether I'd actually use the edit: you can ignore my comment above about having to choose between autogenerating everything and creating a markdown file for everything. I was assuming that by default it would create the API reference page automatically, but after looking through the code I realised you still provide the source file with |
|
|
||
| ## `api_path` | ||
|
|
||
| Define a path to an Open API V3 spec file. This can be a relative file path or a URI to a raw file. |
There was a problem hiding this comment.
This should say what gets generated when you provide this.
From the example it looks like you need to create a source file like this as well as specifying the api path to get any output?
---
title: Example API Petstore
---
api>
| require 'erb' | ||
| require 'openapi3_parser' | ||
| require 'uri' | ||
| require 'pry' |
There was a problem hiding this comment.
I think this pry can be removed
| require 'pry' | ||
|
|
||
| module GovukTechDocs | ||
| class ApiReference < Middleman::Extension |
There was a problem hiding this comment.
Do you think this extension would be usable by other middleman projects not using the tech-docs-gem?
If so it would be great if we could package this up separately at some point, to contribute back to the wider ecosystem of openapi tools.
|
|
||
| # If no api path then just return. | ||
| if @config['api_path'].to_s.empty? | ||
| # @TODO Throw a middleman error? |
There was a problem hiding this comment.
Can this raise an error that tells you what you need to add?
| end | ||
|
|
||
| # Is the api_path a url or path? | ||
| if uri?@config['api_path'] |
There was a problem hiding this comment.
Minor style thing but it looks weird that there's no space between the method call and its argument here, can you change it to use the parentheses style?
| @api_parser = true | ||
| @document = Openapi3Parser.load_file(@config['api_path']) | ||
| else | ||
| # @TODO Throw a middleman error? |
| def api(text) | ||
| if @api_parser == true | ||
|
|
||
| map = { |
There was a problem hiding this comment.
can you give this a more descriptive name? This makes the following line a bit harder to parse (map.map)
There was a problem hiding this comment.
Renamed to keywords in a new commit.
| text.gsub!(/#{ Regexp.escape(key) }\s+?/, '') | ||
|
|
||
| # Strip paragraph tags from text | ||
| text = text.gsub(/<\/?[^>]*>/, '') |
There was a problem hiding this comment.
This would also strip other tags from the text, if there are any. Is it valid for other stuff to appear in the paragraph besides the api> line?
There was a problem hiding this comment.
Not at the moment, it only works if that's the only thing on the line. Is there a use case where you'd want to include this in a paragraph?
There was a problem hiding this comment.
I don't think so - requiring it to be in its own paragraph makes sense I think.
| end | ||
|
|
||
| def api_path_render(text) | ||
| if text == 'api>' |
There was a problem hiding this comment.
does this forgive trailing whitespace after the api>?
There was a problem hiding this comment.
Yes it does from my testing.
…der" This reverts commit b235f2a.
There was a problem hiding this comment.
Other than the comment on the documentation, I don't think there is anything blocking this being merged once the tests pass.
I didn't get a chance to try out any other specs, but the pet store example looks fine, and we'll get more feedback once people start trying this with real APIs.
|
@MatMoore travis check failed :( |
This branch allows you to define a path to an Open API file in tech-docs.yml:
api_path: source/pets.yml. This can be a relative file path or a URI to a raw file elsewhere.To output the generated content in the page of your choice, write
api>in any markdown parsed file (.md, .html.md, .html.md.erb). You can also specify to just print a specific path:api> /petsSee the example project on how to set this up.