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: add streaming support for application/octet-stream contents #3966

Merged
merged 8 commits into from
Aug 6, 2024

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Aug 5, 2024

Adding support for application/octet-stream content types, via streamBody/streamTextBody. Since the correct output code to produce depends upon the server provider, this also adds the flag openapiStreamingImplementation.

There are more interpreters that this could support, but I figured that the 4 I've added are probably the most common and it should be easy to add support for others down the line.

I'd originally intended to support only Array[Byte] schemas, but I don't think that restriction is actually necessary; given that the implementing service will need to provide the serdes to/from the bytestream independently of the endpoint generation, and the Schema generation already seems to work pretty well, I figure it's probably fine just to use Schema.binary[$theFormat] and support the same range of types as the json endpoints.

This pr also refactors the SBT plugin a bit, since we'd reached the limit of the number of params that could be sustained by the old code.

sourceManaged,
streams,
scalaVersion
) flatMap {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Annoyingly it turns out that this syntax was only supported up to 11 elems for some reason (Why 11? 🤷). Took a while to find a refactor of the sbt plugin that would support aggregation of more setting keys, but I think the solution I've settled on here should be fine for arbitrary N settings.

Copy link
Member

Choose a reason for hiding this comment

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

11 is 22/2, which is the tuple limit in Scala ;). Honestly, no idea where this comes from :)

@hughsimpson hughsimpson marked this pull request as draft August 5, 2024 16:36
@hughsimpson hughsimpson marked this pull request as ready for review August 5, 2024 16:39
@hughsimpson hughsimpson marked this pull request as draft August 5, 2024 17:29
@hughsimpson
Copy link
Contributor Author

hughsimpson commented Aug 5, 2024

I think there might be some issue here with the sbt changes... trying to figure it out...

EDIT: seem to have fixed the issue. Very loose and barely-understood explanation would be something like: I'd misunderstood the semantics of binding a config struct setting from many individual structs, and so put a Def.sequence where there shouldn't've been one. The practical effect was that it somehow messed up the evaluation time of the source generators and meant additional downstream munging was getting ignored (I like to emit an HList instead of a List for the endpoints definitions because reasons, so need source generator composition to work when using this plugin)

@hughsimpson hughsimpson marked this pull request as ready for review August 5, 2024 17:46
@hughsimpson hughsimpson changed the title codegen: add support for application/octet-stream contents codegen: add streaming support for application/octet-stream contents Aug 6, 2024
@adamw adamw merged commit ef22dd9 into softwaremill:master Aug 6, 2024
22 checks passed
@adamw
Copy link
Member

adamw commented Aug 6, 2024

Thanks!

@hughsimpson hughsimpson deleted the octet_stream branch August 6, 2024 17:12
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