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

WIP: feat: add more OpenAPI specs #3198

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Vito0912
Copy link
Contributor

This PR is a work in progress and may never be "finished."

I will be updating it with commits periodically. This PR should not be merged until the last commit is ready for merging. If the last commit is not ready for merging, I am still adding new specifications. So at that time I did not tested it or generated the OpenApi.json.
I try to test everything somewhat. Please review before merging.

Contributions and suggestions for better practices are appreciated, including naming conventions, references, etc. This is my first time writing OpenAPI specs, and I am doing my best to ensure each component is unique and references existing schemas when possible.

Currently, I am testing a bit, but I wanted to open this PR to show what I am working on so others can help or contribute.

Copy link
Contributor Author

@Vito0912 Vito0912 left a comment

Choose a reason for hiding this comment

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

My language and the generator does not support the oneOf per https://github.com/OpenAPITools/openapi-generator. Therefore, I will focus on endpoints that do not use oneOf.

Vito0912 and others added 8 commits July 29, 2024 21:44
Added missing abridged
Fixed wrong type on start of bookChapter
Added missing libraryFile for LibraryItem
Added missing type to id in media
* Update: formatting

* Fix: libraryItemController `play` endpoint
+ Endpoints cannot have optional path parameters
+ Moved schemas to `components` section

* Fix: MeController optional path parameters

* Fix: Books do not have `episodeId` in `mediaProgress`

* Fix: `PlaybackSession` oneOf between book and podcast

* Update: bundled spec

* Fix: `allowReserved` in LibraryItem include query

* Add: tags to AuthController endpoints

* Fix: summary of play endpoints

* Update: bundled spec
@Vito0912 Vito0912 marked this pull request as ready for review August 4, 2024 18:11
@Vito0912 Vito0912 marked this pull request as draft August 9, 2024 15:36
@unl0ck
Copy link

unl0ck commented Nov 23, 2024

@nichwall Looks good on my site, maybe push current state it is easier small pushes as big pushes

@Vito0912
Copy link
Contributor Author

@unl0ck I’m not sure this is ready for merging just yet.

I’ve encountered several issues with certain routes in my client. Additionally, I believe I added one or two routes to the OpenAPI spec for my client (if you want to take a look): openapi-spec.json.

From what I recall, the search endpoint occasionally crashes. This seems to be related to the series attribute, which can either be a list of Series or just a single Series object. This inconsistency makes parsing tricky and less reliable. Moreover, I suspect there are some incorrect or inconsistent types. For instance, certain attributes (e.g., backup) can have multiple types—null, a boolean, or a string—which complicates handling.

Because of these challenges, I haven’t made much progress. To be honest, given the current state, it doesn’t seem logical to move forward if there are planned route changes. I recall nichwall mentioning the idea of establishing a solid foundation before introducing new endpoints (though I could be mistaken), but I feel that approach is no longer practical.

Lastly, we should avoid relying on oneOf. Nichwall’s PR appears to address this, which is a good direction. Most programming languages have limited or no support for oneOf, as outlined in the OpenAPI Generator documentation: openapi-generator.tech/docs/generators. You will find near to none language supporting this. Dato: The goal of having autogenerated routes for thrid party devs does not exsits (despite having a proper doc ofc).
That just as a comment and my opinion.

I can help merging this into your repo so you can try plaing around with this and fix some things

Also, as some types are wrong (from the current docs) you can take a look at this models I made for my client. They should use proper types (but I merged many Objects together). I only have one Series Object that can have all nullable attributes (As there are even more distinct series models returned by the api as it currently states for example): https://github.com/Vito0912/abs_flutter/tree/main/lib/api

@Vito0912
Copy link
Contributor Author

Vito0912 commented Nov 23, 2024

Just scraped the docs:

Function Anzahl
Simple 75
Composite 74
Polymorphism 39
Union 2
allOf 5
anyOf 5
oneOf 7
not 0

Generators that support oneOf:

  • cpp-restsdk
  • dart-dio
  • go
  • julia-client
  • python
  • python-pydantic-v1
  • rust

And I can say that dart-dio does not support it (at least for my PR) as it is my langauge. I am a bit supprised that this is on that list

@unl0ck
Copy link

unl0ck commented Jan 2, 2025

@Vito0912 you are right swift is also an problem with oneOf and others

@Vito0912
Copy link
Contributor Author

Vito0912 commented Jan 2, 2025

@unl0ck I made a complete (excluding the LibraryitemController - As it should be near to impossible to do that as multiple specs fit in eachother) spec in the Discord.
I will later or sooner close this PR in favour of a new I will start then.
It should include all current docs and some new routes added in the meantime.

It has it's own issues though and will not replace the current version.

This does not fix the oneOf problem though. But there should be some API changes in the future

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