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: Semiauto schema derivation #3671

Merged
merged 3 commits into from
Apr 19, 2024

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Apr 11, 2024

This would address two issues with the code currently being generated:

  1. When using the codegen, until now I've had to add in a sourceGenerator hack to replace import sttp.tapir.generic.auto._ with a implicit def schemaForAny[T]: sttp.tapir.Schema[T] = sttp.tapir.Schema.any[T] stub, since I was hitting javac class size limits (also, compilation was taking a while to fail - I guess because the leaf schemas were being regenerated for everything that needed it? I don't really know enough about how the scala compiler handles memoization of implicit resolution to be sure of that though...). ANYWAY:

  2. This also means that the schemas for ADTs should be more* correct (automatic derivation didn't know about discriminators or lack thereof).

This pr will create explicit schemas for every class model, and emit them to TapirGeneratedEndpointsSchemas (or TapirGeneratedEndpointsSchemas1, TapirGeneratedEndpointsSchemas2 etc if split over multiple files). The maximum number of schemas per file is configurable via the new option openapiMaxSchemasPerFile. Experimentally, a value of 400 worked fine for me, so that's the default.

Generating the correct schemas for objects and maps made me aware of the fact that I'd mucked up circe json serdes for object schemas with fields that were, themselves, inlined object or map declarations; so this pr also fixes that.

* The 'more' qualifier is here because I don't know how to mark an array field as required in the generated JSON schema, so round-tripping the swagger example I've touched fails to retain semantics. I tried adding @sttp.tapir.Schema.annotations.customise((_: sttp.tapir.Schema[_]).copy(isOptional = false)) to Seq fields but that didn't change the output...

@hughsimpson
Copy link
Contributor Author

@adamw @kciesielski Is there any appetite for this one, or do you hold some reservations about where it's going? There are possibly other routes towards what I'm trying to do with this one... although I couldn't think of one that worked better.

@kciesielski
Copy link
Member

kciesielski commented Apr 19, 2024

@hughsimpson I'm okay with proceeding, thank you for this impressive work! One thing that might be useful for future maintenance is probably a few more comments. The algorithm used in generateSchemas consists of steps groupedRings, orderedLayers, and foldedLayers - could you add a textual description of how these steps work together?

@hughsimpson
Copy link
Contributor Author

That's an eminently reasonable request 😅 I should have commented more intially, will push some up this morning

@hughsimpson
Copy link
Contributor Author

Pushed up some comments, hopefully makes it clear enough how it's working now!

@kciesielski
Copy link
Member

Looks great, thanks again!

@kciesielski kciesielski merged commit 20a526c into softwaremill:master Apr 19, 2024
23 checks passed
@hughsimpson hughsimpson deleted the semiauto_schemas branch April 19, 2024 11:52
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