Skip to content

[scala-http4s-server] Use the JSON name for the discriminator and fail decoding on unknown values - #25308

Merged
wing328 merged 1 commit into
OpenAPITools:masterfrom
bartosz822:scala-http4s-server-discriminator-base-name
Oct 19, 2026
Merged

wing328 merged 1 commit into
OpenAPITools:masterfrom
bartosz822:scala-http4s-server-discriminator-base-name

Conversation

@bartosz822

@bartosz822 bartosz822 commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

This fixes two problems with the discriminator codecs in scala-http4s-server/types.mustache. Both branches are affected: with and without discriminator mapping.

1. The JSON key is the Scala identifier instead of the property name. The template uses propertyName. That's the Scala identifier, which is backtick-escaped for reserved words and then HTML-escaped by Mustache. With a discriminator on type, the encoder writes the key `type` and the decoder looks it up with cursor.downField("`type`"), so the polymorphic decoders never match a spec-conformant body. The template now uses propertyBaseName. #23853 made the same change in the scala-http4s client template.

2. An unknown discriminator value throws a MatchError. The flatMap over the discriminator value has no default case, so an unexpected value throws, and the server returns a 500 instead of rejecting the request body. Both decoders now end with the same default case the client template uses:

case discriminatorValue =>
  Left(io.circe.DecodingFailure(s"Unknown discriminator value: $discriminatorValue", cursor.history))

DecodingFailure is fully qualified so the shared import io.circe.{ Decoder, Encoder } stays unchanged for traits without a discriminator.

Tests. I added ScalaHttp4sServerCodegenTest with a spec that has two oneOf unions discriminated on type, one with mapping and one without. The test checks that the keys are "type" in the encoders and decoders, that no HTML-encoded backticks are left, and that both decoders have the default case. It fails on master and passes with this change. I also compiled the server generated from that spec with sbt.

The petstore sample has no discriminator, so ./bin/generate-samples.sh bin/configs/scala-http4s-server.yaml produces no changes.

PR checklist

  • Read the contribution guidelines.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    Commit all changed files.
    (Ran the modules/openapi-generator build with its Scala http4s codegen tests and regenerated the scala-http4s-server sample, the only one using this template. No generator options changed, so the generator docs are unaffected.)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@clasnake (2017/07), @shijinkui (2018/01), @ramzimaalej (2018/03), @chameleon82 (2020/03), @Bouillie (2020/04) @Fish86 (2023/06)


Summary by cubic

Fixes two problems with discriminator codecs in scala-http4s-server/types.mustache: the JSON key now uses the property name instead of the escaped Scala identifier, and unknown discriminator values return a DecodingFailure instead of throwing a MatchError.

  • Encoders and decoders now use propertyBaseName, so reserved words like type are no longer emitted as HTML-escaped backtick identifiers.
  • Both decoders (with and without mapping) now handle unknown discriminator values with a default case returning DecodingFailure.
  • Adds ScalaHttp4sServerCodegenTest with a spec covering discriminated oneOf unions on the reserved word type, with and without mapping.

Written for commit 0fb9214. Summary will update on new commits.

View guided diff

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="modules/openapi-generator/src/main/resources/scala-http4s-server/types.mustache">

<violation number="1" location="modules/openapi-generator/src/main/resources/scala-http4s-server/types.mustache:56">
P2: The new JSON key interpolation still HTML-escapes raw discriminator names, so valid keys such as `a&b` become `a&amp;b` in generated codecs. Use the generator’s JSON/Scala-string-safe raw interpolation for all discriminator key occurrences, including the mapped branch.</violation>

<violation number="2" location="modules/openapi-generator/src/main/resources/scala-http4s-server/types.mustache:67">
P3: The fallback `DecodingFailure` uses `cursor.history`, which is the parent object's history and omits the discriminator field. Use `cursor.downField("{{discriminator.propertyBaseName}}").history` (or the equivalent mapped-section name) so the error path points at the discriminator.</violation>
</file>

Reply to a comment to ask cubic a question or push back. It learns from your replies.

View guided diff | Re-trigger cubic

implicit val {{#lambda.camelcase}}{{classname}}{{/lambda.camelcase}}Encoder: Encoder[{{classname}}] = Encoder.instance {
{{#oneOf}}
case {{#lambda.camelcase}}{{.}}{{/lambda.camelcase}}: {{.}} => {{#lambda.camelcase}}{{.}}{{/lambda.camelcase}}.asJson.mapObject(("{{discriminator.propertyName}}" -> "{{.}}".asJson) +: _)
case {{#lambda.camelcase}}{{.}}{{/lambda.camelcase}}: {{.}} => {{#lambda.camelcase}}{{.}}{{/lambda.camelcase}}.asJson.mapObject(("{{discriminator.propertyBaseName}}" -> "{{.}}".asJson) +: _)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The new JSON key interpolation still HTML-escapes raw discriminator names, so valid keys such as a&b become a&amp;b in generated codecs. Use the generator’s JSON/Scala-string-safe raw interpolation for all discriminator key occurrences, including the mapped branch.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/openapi-generator/src/main/resources/scala-http4s-server/types.mustache, line 56:

<comment>The new JSON key interpolation still HTML-escapes raw discriminator names, so valid keys such as `a&b` become `a&amp;b` in generated codecs. Use the generator’s JSON/Scala-string-safe raw interpolation for all discriminator key occurrences, including the mapped branch.</comment>

<file context>
@@ -53,16 +53,18 @@ object {{classname}} {
   implicit val {{#lambda.camelcase}}{{classname}}{{/lambda.camelcase}}Encoder: Encoder[{{classname}}] = Encoder.instance {
 {{#oneOf}}
-    case {{#lambda.camelcase}}{{.}}{{/lambda.camelcase}}: {{.}} => {{#lambda.camelcase}}{{.}}{{/lambda.camelcase}}.asJson.mapObject(("{{discriminator.propertyName}}" -> "{{.}}".asJson) +: _)
+    case {{#lambda.camelcase}}{{.}}{{/lambda.camelcase}}: {{.}} => {{#lambda.camelcase}}{{.}}{{/lambda.camelcase}}.asJson.mapObject(("{{discriminator.propertyBaseName}}" -> "{{.}}".asJson) +: _)
 {{/oneOf}}
   }
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This applies to discriminator handling in general, not just this change. The client template (scala-http4s/model.mustache, #23853) uses the same {{discriminator.propertyBaseName}}, and both templates already HTML-escape the discriminator values ("{{.}}", "{{mappingName}}") the same way. Switching to triple braces wouldn't fix it either, since a " or \ in the name would then break the Scala string literal. A proper fix would escape keys and values as Scala string literals in both templates, which belongs in a separate PR. This one fixes the reserved-word case (type), which is the one that breaks decoding in practice.

cursor.as[{{.}}]
{{/oneOf}}
case discriminatorValue =>
Left(io.circe.DecodingFailure(s"Unknown discriminator value: $discriminatorValue", cursor.history))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P3: The fallback DecodingFailure uses cursor.history, which is the parent object's history and omits the discriminator field. Use cursor.downField("{{discriminator.propertyBaseName}}").history (or the equivalent mapped-section name) so the error path points at the discriminator.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/openapi-generator/src/main/resources/scala-http4s-server/types.mustache, line 67:

<comment>The fallback `DecodingFailure` uses `cursor.history`, which is the parent object's history and omits the discriminator field. Use `cursor.downField("{{discriminator.propertyBaseName}}").history` (or the equivalent mapped-section name) so the error path points at the discriminator.</comment>

<file context>
@@ -53,16 +53,18 @@ object {{classname}} {
           cursor.as[{{.}}]
       {{/oneOf}}
+        case discriminatorValue =>
+          Left(io.circe.DecodingFailure(s"Unknown discriminator value: $discriminatorValue", cursor.history))
     }
   }
</file context>
Suggested change
Left(io.circe.DecodingFailure(s"Unknown discriminator value: $discriminatorValue", cursor.history))
Left(io.circe.DecodingFailure(s"Unknown discriminator value: $discriminatorValue", cursor.downField("{{discriminator.propertyBaseName}}").history))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is intentional: the default case matches the one in the client template (scala-http4s/model.mustache), which also uses cursor.history, so client and server report unknown discriminator values the same way. Pointing the history at the discriminator field would be a small improvement, but it should change both templates together, so I'd leave it for a follow-up.

@wing328

wing328 commented Oct 19, 2026

Copy link
Copy Markdown
Member

tested locally and the fix (diff) looks good.

thanks for the contribution.

@wing328
wing328 merged commit c08cf74 into OpenAPITools:master Oct 19, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants