Add OpenAPI json file#137
Conversation
✅ Deploy Preview for opensubsonic ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
As explained before IMO
Should be done first, PR merging, rebase and everything will be a pain with such a single large file. There's tons of tools to rebuild the large file from the split version. |
For the purposes of this PR it will probably be done last, breaking it up now will slow down making sweeping changes (if requested) and testing it's validity. |
|
Your future work was not clear if it was tied to the PR or for future PR. If the split is made before it's ready to merge then no problem. |
It's a PR, it's meant to be reviewed and changed to fit the needs of the project. Although yes, I hoped I won't need to write GitHub actions and a build step for the file for the first version as it probably won't be that simple if we want client (and server) generation work nicely (which is my priority). |
The priority is actually that anything that is merged is stable and maintainable, this is a public documentation of a public API used by many servers and clients, breaking changes must be avoided at all costs. If we go single file, but can't split because it would trigger some small incompatible changes we are doomed. One solution could be to publish the OpenAPI spec as alpha / unsupported until we are ready to commit to it, but the priority is stability not rushing. The generation does not need to be a github action, the project is already provided with docker and it can be a simple command in it to generate the json, this comes with the added benefits of content validation at that step to avoid PR with errors in the files, this also avoids all formatting issues, greatly simplifies future reviews, ... |
|
You missed the part where I changed the PR description to require your requested changes. I can reiterate that I wrote before.
My motivation and priorities are not something that will change (for this API to be easier to use and adopt) but it also won't affect this PR.
I simply meant some form of pipeline step is needed on every PR to preserve the structural validity of the final generated JSON. |
In all cases we need a way to validate the file(s) if there's one or many that does not change the need, else how to you review the PRs? Humans can't spot a typo or a missing comma in a large commit. |
|
@Gr3q Do you intend to still work on this ? |
|
Yes, when I have time. What about the huge checklists below the "Required work to be done before merge" section? I can take the lack of comments on them that they are not important for you guys for this PR? |
|
Most don't care about the doc part details so it's hard to get engagement until it's finished. For my case, this work was planned to be done by me to unlock the new json endpoints so it's very important, and your list of required correspond to what I requested here and would have done so not much to comment. Edit: Sorry for close missclick |
|
@Tolriq which folder structure and target folder do you prefer for the broken out schemas? Contextual requirements/options
OptionsOptions below are using Subsonic Response, Option one: flat list placed directly into the Option 2: Supplementary schemas go into subfolder of the same name Option 3: All json shemas go into a subfolder of the same name |
|
IMO it should be under an openapi folder to not mix with the .md files. Because for now it's beta stuff and in the far future as you said we could generate the md files from the openapi files and so it's better not to mix them in the hierarchy. For the folders under openapi I have no strong opinion except that it's better if we have some kind of minimal organization. I don't know the best practices here as I still have not had found time to fully investigate this stuff. |
|
Thanks we'll still need to find a way to update docsy to a more recent version too (Dark mode :) ) and fix the not visible versions in white over white. |
You will still need the fix for swagger but here you go #147 |
Yes but it would be a small update. I'll now be able to work on the new APIs without having to deal with all those and it's a massive headache removed. |
|
Ok so @Gr3q IMO it looks good and works well enough to have this as a basis for the future. I think that to merge in current status we just need to open a discussion and move the TODO list there. And add an red important box in the https://deploy-preview-137--opensubsonic.netlify.app/docs/openapi/ page saying that this is still a WIP as there's some inconsistencies and users should still check the actual docs and servers results until this is finished and a link to the discussion TODO list. WDYT ? |
|
Should we extract the todo from this PR to GitHub discussions to simplify future management and not be editing a closed PR ? |
|
Seems swagger / redoc does not work correctly in dark mode :( |
Adding a full swagger theme is overkill in my opinion, but I couldn't get hugo use dart-sass no matter what I tried (to use The alternative is to remove the dark theme for swagger and add minimal styling (white bg) it looks the same in dark mode too. I'm not sure why no-one is complaining in the docsy github repo about this. |
|
Yes no need to make something complicated, just that it's legible for quick checks when needed. People can and will use directly the json in their preferred tool for extended usage. I think it's now in a working shape to be merged since it's labelled as WIP, this will make other react more on the small details / differences. Do you see anything missing ? |
|
I think it's ready. I would love to resolve the TODOs, otherwise I consider the schema ready for this PR. If I forgot to write something down in the dev docs for it I wouldn't know...so for now that is also ready, |
|
Don't worry when building the first version of this docs, I made a couple mistakes as it was a huge dumb work, and at some point the brain no more print :) We'll fix with reports. |
|
@opensubsonic/servers @opensubsonic/clients can we have some love here ? This is the basis for a better documentation and precise API definition for all the future new endpoints. |
lachlan-00
left a comment
There was a problem hiding this comment.
a bit beyond me but swagger is def a good focus point
This is a side effect, the main thing is very precise API definition, allowing to verify that endpoints and clients are compliant without trying to check a webpage with markdown tables and find what mistake we made :) Will merge as it's WIP but will ensure that there's no need to rebase non stop and we can fix the small details over time. This also means people needs to update the OpenAPI part for new PRs. If we can't fix redoc we can remove it too, the important part is the actual OpenAPI json definition, people can use other tools with the file. |




This PR attempts to add an
openapi.jsonto the project for client library/documentation generation. This is a big one.The file should be usable but I will try generating a Python client library soon to see how well it works, but work on the PR can be started even now. Ideally all extension should be added the schema. So far I added extension endpoints, parameters, responses, http post endpoints.
It's Github, there is a live URL to the json file, so technically it can be used for stuff straight away...
Everything below will need to be resolved one way or another, they need decisions by the maintainers of the API specification..
Required work to be done before merge
openapi.json#/...format (final place in theopenapi.jsonshould be inferred from the file path alone, e.g./content/en/docs/Responses/ArtistID3.jsongoes to#/components/schemas/ArtistID3)openapi.jsonduring build and for PRsWork checklist(s)
Decide to Approve/Change/Fix every difference between
openapi.jsonand OpenSubsonic API specminimum: 0to integer types where it's implied (count,offset, unix timestamp,position)jsonformat on every endpoint.xmlresponse formats should not be a big hassle but client library generation will probably become problematic.examples, opted to useexternalDocsonly because it made the Swagger and Redoc generated documents harder to read. I might add them if they make sense for generated client code.description fieldHTTP form POSTextension support might be stricter than what's allowed (global params - auth and format params - only allowed as query params, endpoint specific params are the only ones allowed in request body)Decide to Schedule/Postpone/Fix/Resolve all issues/inconsistencies found in spec
They might need to be resolved before/along with this PR or they can be resolved at a later time. It would be nice if all of them could be answered.
ItemDatedescription and spec doesn't match (in required fields)getAlbumInfo2return type incorrect (based onAlbumList)?downloadPodcastEpisodereturn type incorrect?deleteBookmarksummary and description is wrongdownloaderror payload schema missing completelygetArtistspage firstartistslink is brokengetAvatarusernameparameter is "required" and has a "default" as well. it should be one or the othergetCaptionsreturn payload not cleargetCoverArtsizeparam type not specified, I assumeintas pixelsIndexesignoredArticlespattern is unclear, it was comma-separated list as string beforeinternetRadioStationhomePageUrlhas wrong detailsgetLyricsartistandtitlequery parameters are really optional?getLyricsreturns only one element?PodcastEpisodedescription "PodcastEntry extends Child". Wrong name used?NowPlayingEntryminutesAgoformat is unclearPlayQueuedescription incorrectPodcastChannelerrorMessageexistence could depend onstatusin documentation?GetSimilarSongsandGetSimilarSongs2response type is the same in the spec (not the names)?Userexample uses booleans and integers as strings, but they are typed as booleans and integers.VideoInfois not documentedVideosis not documentedhlsendpoint summary matchesdownloadsearchalbumparameters's commentsearchanyparameter type unclearSearchResultis not documentedSearchResult2all field details are incorrectstreamtimeOffsetparam's description typo, "Transcode Offet"unstarsummary is incorrectupdateuseris missingplaylistRoleparameter?createuseris missingmaxBitRateparameter?Schedule/Require/Postpone/Request future improvements for
openapi.jsongetPodcastspayload dependent on query paramincludeEpisodes, but it doesn't indicate that structurally.jukeboxControlpayload dependent on query paramaction: get, but it doesn't indicate that structurally.getAlbumListandgetAlbumList2POST params can be a tagged unionjukeboxcontrolPOST params can be a tagged unionReview generated OpenAPI documentation to verify they are OK
Verify this OpenAPI schema against servers
Using at least one of the tools from here theopenapi.jsonshould be tested against all endpoints request/response payloads on the servers below. Any mismatches/errors should be fixed ifopenapi.jsondoesn't follow the spec. Ideally will be no behaviors implemented in servers that are different in the official API spec.Way out of scope.
Client libraries generated from this
openapi.jsonare generated OKPython
pydanticfor response payload validation - no such library foundProposed client generation libraries for testing
Typescript
zodfor response payload validationProposed client generation libraries for testing
Kotlin
Proposed client generation libraries for testing
Any other language required by maintainers
Major Future Work
openapi.json(in its current form)