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

Downgrade to Openapi v3.0.3 for better tooling support #263

Merged
merged 2 commits into from
Jul 5, 2022

Conversation

joodie
Copy link
Contributor

@joodie joodie commented Jul 4, 2022

Current tooling & libraries generally do not yet support openapi
3.1.0. For instance, none of the data validation libraries at
https://openapi.tools/#data-validators supports it. 3.0.3 support is
widespread.

This change sets the version to 3.0.3 and fixes an issue with the
default values for the sort parameter, which should be an array, so
its default should also be an array.

Further consequences: the duration format is not explicitly
supported in 3.0.3, so strictly conforming validators will allow any
string value for duration fields. This patch replaces the duration
format with an explicit pattern matching the ISO8601 duration format.

Current tooling & libraries generally do not yet support openapi
3.1.0. For instance, none of the data validation libraries at
https://openapi.tools/#data-validators supports it. 3.0.3 support is
widespread.

This change sets the version to 3.0.3 and fixes an issue with the
default values for the `sort` parameter, which should be an array, so
its default should also be an array.

Further consequences: the `duration` format is not explicitly
supported in 3.0.3, so strictly conforming validators will allow any
string value for `duration` fields. This patch replaces the `duration`
format with an explicit pattern matching the ISO8601 duration format.
hamrt
hamrt previously requested changes Jul 5, 2022
Copy link
Contributor

@hamrt hamrt left a comment

Choose a reason for hiding this comment

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

Waarom zijn de voorbeelden in de vorm van een array? Volgens mij zou het één van de waarden uit de lijst moeten zijn. Als het nodig is voor tooling om naar 3.0.3 te gaan lijkt me de oplossing die voorgesteld wordt voor duration prima

Copy link
Contributor

@jelmerderonde jelmerderonde left a comment

Choose a reason for hiding this comment

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

@hamrt die arrays kloppen technisch gezien wel, maar het rendert alleen niet heel lekker in de documentatie. We speccen desort parameter als een array of strings, want clients mogen meerdere velden opgeven om op te sorteren. Omdat het een query parameter is zou dat met komma's gaan: ?sort=name,-date. De default waarde is dan ook een array met één sort parameter. Jammer genoeg rendert redoc dat als ["startDate"]

@joodie
Copy link
Contributor Author

joodie commented Jul 5, 2022

@jelmerderonde ik weet niet zeker of het correct is om bij een array een enkele scalar value als default te hebben, de spec zegt:

https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#schemaObject

default - The default value represents what would be assumed by the consumer of the input
as the value of the schema if one is not provided. Unlike JSON Schema, the value MUST
conform to the defined type for the Schema Object defined at the same level. For example,
if type is string, then default can be "foo" but cannot be 1.

De de library die wij gebruiken accepteert ook geen scalar defaults voor array waarden.

Voor de duidelijkheid: het gaat hier om arrays van enums.

@joodie
Copy link
Contributor Author

joodie commented Jul 5, 2022

Wat betreft redoc rendering, misschien werkt het mooier als we ipv:

default: [ foo ]

DIt doen:

   default:
      - foo

@jelmerderonde
Copy link
Contributor

@joodie net geprobeerd, helaas rendert dat op dezelfde manier:

Screenshot 2022-07-05 at 11 58 23

@jelmerderonde
Copy link
Contributor

jelmerderonde commented Jul 5, 2022

Dit lijkt het issue te zijn in Redoc: Redocly/redoc#1806

Staat al open sinds 26 nov 2021, maar lijkt dus nog niet gefixt te zijn.

Mijn voorstel: wel deze PR mergen als dat nodig is voor compatibiliteit met 3.0.3 tooling. Eventueel kunnen we wel wel een example toevoegen om het duidelijker te maken, dat rendert wel goed:

Screenshot 2022-07-05 at 12 14 59

@jelmerderonde jelmerderonde self-requested a review July 5, 2022 12:20
@jelmerderonde jelmerderonde dismissed hamrt’s stale review July 5, 2022 13:31

issue was resolved

@jelmerderonde jelmerderonde merged commit 6c24dcc into open-education-api:master Jul 5, 2022
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