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

codegen: Support top level nullables on json responses #4351

Merged

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Feb 18, 2025

Use case for this is being able to migrate an endpoint that returns an 'Option[Foo]'. It seemed to me that adding nullable: true is the most natural way to declare this in openapi 3.0; although I'm aware that this was removed in openapi 3.1, so I've also added support for declaring nullability with multiple types (i.e. type: ['null', 'object']). I'm not sure what long-term plans really are for version support, but figured it'd be better to support the feature for both 3.0 and 3.1 specifications

Edit: this might not really be what I want actually. Might want to be able to declare optionality by specifying that one status code (e.g. 204) doesn't return a body, and derive it for a oneOf...

@adamw
Copy link
Member

adamw commented Feb 18, 2025

There's probably a couple of ways to represent nullalbility in OpenAPI, neither being "the best". Ideally we would support all of them, so I guess this PR makes sense?

It would be most convenient to just focus on 3.1, but that's of course not always possible.

@hughsimpson
Copy link
Contributor Author

hughsimpson commented Feb 18, 2025

I figure I'll probably try to implement support for optional oneOfs separately from this - as it stands I think it's doing the right thing now for 'nullable' types, but I'd rather we were able to generate the optional for a 'oneOf' with e.g. 200: some content, 204: no content. That's not currently working either, and I find it neater.

Edit: that other pr is up. I think it's a stronger usecase, although I guess both might be nice... 🤷

@adamw adamw merged commit 7c82cdf into softwaremill:master Feb 19, 2025
27 checks passed
@adamw
Copy link
Member

adamw commented Feb 19, 2025

Let's pull in both since they both add functionality

@hughsimpson hughsimpson deleted the support_top_level_nullables_on_req_resps branch February 19, 2025 11:04
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.

2 participants