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: limited support for oneOfVariant in responses #3993

Merged
merged 1 commit into from
Sep 2, 2024

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Aug 20, 2024

Adding a bit of support for oneOfVariants. Where the previous code would've generated

.errorOut(jsonBody[MessageResponse].description("Bad Request").and(statusCode(sttp.model.StatusCode(400))))
.errorOut(jsonBody[MessageResponse].description("Forbidden").and(statusCode(sttp.model.StatusCode(403))))
.errorOut(jsonBody[MessageResponse].description("Not Found").and(statusCode(sttp.model.StatusCode(404))))
.errorOut(jsonBody[MessageResponse].description("Unauthorized").and(statusCode(sttp.model.StatusCode(401))))

we would now generate

.errorOut(oneOf(oneOfVariant(sttp.model.StatusCode(400), jsonBody[MessageResponse].description("Bad Request")), oneOfVariant(sttp.model.StatusCode(404), jsonBody[MessageResponse].description("Not Found")), oneOfVariant(sttp.model.StatusCode(403), jsonBody[MessageResponse].description("Forbidden")), oneOfVariant(sttp.model.StatusCode(401), jsonBody[MessageResponse].description("Unauthorized"))))

given

      responses:
        '400':
          description: Bad Request
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/MessageResponse'
        '401':
          description: Unauthorized
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/MessageResponse'
        '403':
          description: Forbidden
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/MessageResponse'
        '404':
          description: Not Found
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/MessageResponse'

else throw new NotImplementedError("any not implemented for json libs other than circe")
else
throw new NotImplementedError(
s"any not implemented for json libs other than circe (problematic models: ${schemasWithAny.map(_._1)})"
Copy link
Contributor Author

@hughsimpson hughsimpson Aug 20, 2024

Choose a reason for hiding this comment

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

Completely unrelated, but this can be triggered by surprising things (e.g. an object containing a property that's an enum without a type) so this helps avoid banging heads on walls or keyboards.

@adamw adamw merged commit 8995b1c into softwaremill:master Sep 2, 2024
22 checks passed
@adamw
Copy link
Member

adamw commented Sep 2, 2024

Thank you :)

@hughsimpson hughsimpson deleted the codegen_oneOfVariant branch September 2, 2024 15:14
case many =>
if (many.map(_.code).distinct.size != many.size) bail("Cannot construct schema for multiple responses with same status code")
val oneOfs = many.map { m =>
val code = if (m.code == "default") "400" else m.code
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this could've been better I think

Copy link
Member

Choose a reason for hiding this comment

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

you mean 200?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I mean, default should encompass other status codes; there's an implicit contract binding we take in our choice of code, but really this is explicitly an 'open' sort of specification.... I don't know how to express it well I'm afraid.

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