[scala-sttp4] Circe codecs do not preserve original JSON property names - #24681
Conversation
d8d51b9 to
6e9c6bb
Compare
There was a problem hiding this comment.
1 issue found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache:75">
P2: Inline (exclusive) oneOf members get a new explicit decoder that references enum properties with the raw `{{dataType}}` instead of the enum-aware type (`{{classname}}Enums.{{datatypeWithEnum}}`) that the regular-model codecs in this same change use. If an exclusive oneOf member carries an enum-typed property, it won't be decoded/validated through the generated enum type and may not even reference a resolvable type, unlike every regular model. For consistency with the rest of this PR, the inline-member decoder (and its case-class field type) should apply the same `{{#isEnum}}...{{classname}}Enums.{{datatypeWithEnum}}...{{/isEnum}}` mapping used for regular models.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| implicit val decoder: Decoder[{{classname}}] = Decoder.instance { c => | ||
| for { | ||
| {{/-first}} | ||
| {{{name}}} <- c.downField("{{baseName}}").as[{{^required}}Option[{{/required}}{{dataType}}{{^required}}]{{/required}}] |
There was a problem hiding this comment.
P2: Inline (exclusive) oneOf members get a new explicit decoder that references enum properties with the raw {{dataType}} instead of the enum-aware type ({{classname}}Enums.{{datatypeWithEnum}}) that the regular-model codecs in this same change use. If an exclusive oneOf member carries an enum-typed property, it won't be decoded/validated through the generated enum type and may not even reference a resolvable type, unlike every regular model. For consistency with the rest of this PR, the inline-member decoder (and its case-class field type) should apply the same {{#isEnum}}...{{classname}}Enums.{{datatypeWithEnum}}...{{/isEnum}} mapping used for regular models.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache, line 75:
<comment>Inline (exclusive) oneOf members get a new explicit decoder that references enum properties with the raw `{{dataType}}` instead of the enum-aware type (`{{classname}}Enums.{{datatypeWithEnum}}`) that the regular-model codecs in this same change use. If an exclusive oneOf member carries an enum-typed property, it won't be decoded/validated through the generated enum type and may not even reference a resolvable type, unlike every regular model. For consistency with the rest of this PR, the inline-member decoder (and its case-class field type) should apply the same `{{#isEnum}}...{{classname}}Enums.{{datatypeWithEnum}}...{{/isEnum}}` mapping used for regular models.</comment>
<file context>
@@ -49,10 +49,41 @@ case class {{classname}}(
+ implicit val decoder: Decoder[{{classname}}] = Decoder.instance { c =>
+ for {
+{{/-first}}
+ {{{name}}} <- c.downField("{{baseName}}").as[{{^required}}Option[{{/required}}{{dataType}}{{^required}}]{{/required}}]
+{{#-last}}
+ } yield {{classname}}(
</file context>
There was a problem hiding this comment.
Not introduced here, and the narrow version of this fix would break the build.
The inline-member case class already uses raw {{dataType}} on master — this PR doesn't touch that line:
{{{name}}}: {{^required}}Option[{{/required}}{{dataType}}{{^required}}] = None{{/required}}{{^-last}},{{/-last}}So the new decoder deliberately mirrors the field it has to construct. Changing only the decoder to {{classname}}Enums.{{datatypeWithEnum}} would make the two disagree and fail to compile.
Changing both wouldn't work either: the object {{classname}}Enums wrapper is only emitted in the x-isRegularModel branch, so it doesn't exist for inline oneOf members. For this spec:
ClickEvent:
properties:
button_state: { type: string, enum: [pressed, released] }master and this branch both generate buttonState: Option[String] with no ClickEventEnums object anywhere. Typed enums on inline oneOf members simply aren't supported yet — a pre-existing gap, and adding that support means emitting the enums object in the sealed-trait branch too, which is out of scope for a codec-naming fix.
Verified the generated client for the spec above compiles on this branch.
The circe branch of scala-sttp4 derives codecs with io.circe.generic.semiauto, which keys JSON members off the Scala field names. Any property whose name is not already camelCase - snake_case, kebab-case, PascalCase - is silently renamed on the wire. Replace the derived codecs with explicit Encoder/Decoder instances keyed on baseName, the original property name from the spec, following the pattern already used by scala-http4s and adopted for scala-sttp in OpenAPITools#23465. This also fixes the discriminated oneOf encoder. deriveConfiguredEncoder only applies withDiscriminator when each member's implicit encoder is an Encoder.AsObject; .mapJson(_.dropNullValues) downgraded them to plain Encoder, so the encoder silently emitted a wrapper object while the decoder expected the flat form, making discriminated oneOf impossible to round-trip. Two related codegen fixes: the discriminator property was matched by getPropertyName() (the escaped Scala identifier) against a property's baseName, so it was only stripped from members when the two happened to coincide; and the property name is now exposed to the template so the sealed trait's encoder emits it. Optional fields set to None remain omitted rather than serialized as null, preserving OpenAPITools#24362.
6e9c6bb to
fbf90f3
Compare
Fixes #24679.
The circe branch of
scala-sttp4derives its codecs withio.circe.generic.semiauto:Plain derivation keys JSON members off the Scala field names, so every property whose spec name is not already camelCase is silently renamed on the wire —
assignment_keygoes out asassignmentKey,first-nameasfirstName,ZipCodeaszipCode. The client compiles and only fails against the server.This is the same bug reported for
scala-sttpin #23464 and fixed in #23465. This PR applies that fix toscala-sttp4: explicitEncoder.instance/Decoder.instancekeyed onbaseName, the original property name from the spec, asscala-http4sandscala-sttpalready do.Scala identifiers are untouched — only the JSON keys change — so specs whose properties are already camelCase generate byte-identical output. Optional fields set to
Noneremain omitted rather than serialized asnull— the encoder builds its field list with.flatten— preserving #24362.This also fixes discriminated
oneOfderiveConfiguredEncoderonly applieswithDiscriminatorwhen each member's implicit encoder is anEncoder.AsObject..mapJson(_.dropNullValues)downgrades them to a plainEncoder, so circe-generic-extras silently fell back to wrapper encoding while the decoder still expected the flat form:Replacing the derived sealed-trait codec with the explicit form used elsewhere in this template removes the problem at the root.
Two related codegen fixes were required:
discriminator.getPropertyName()— the escaped Scala identifier, e.g.`type`orpaymentType— against each property'sbaseName(type,payment_type), so it was only stripped from members when the two happened to coincide. It now matches ongetPropertyBaseName().x-discriminator-propertyso the sealed trait's encoder can emit it; previously it was unreachable from inside thex-oneOfMembersloop and rendered empty.Also changed
Two further output changes fall out of replacing the derived codecs:
oneOfwithout a discriminator loses its wrapper. Deriving a sealed trait produced a constructor-name envelope,{"Circle":{"radius_cm":2.0}}; it now encodes the bare member,{"radius_cm":2.0}. Encoder and decoder changed together, so round-tripping is preserved. The previous form is circe's ADT convention rather than whatoneOfmeans in OpenAPI, and thex-hasWrappedOneOfMembersbranch of this same template already emitted the bare member.type,payment_type). Consumers passing it explicitly —CreditCardPayment(paymentType = Some("credit_card"))— will need to drop that argument; the value is written by the sealed trait's encoder.Models with no properties are emitted as
Encoder.instance(_ => Json.obj())/Decoder.const(X()), since an emptyfor {} yield X()is not valid Scala.Tests:
Sttp4CodegenTest#verifyCirceCodecsUseOriginalJsonPropertyNameswith a new fixture (sttp4-mixed-case-fields.yaml) covering snake_case, kebab-case, PascalCase and already-camelCase properties, an inlineoneOfwith a snake_case discriminator, and an inline enum. Three existing tests were updated where they asserted the old derived codecs. Verified end-to-end by compiling the regenerated sample and round-tripping:Also generated and compiled 13 real-world specs (FastAPI-produced, all-snake_case, inline discriminated
oneOf) against this change; all compile and their wire format matches what the server expects.Samples regenerated via
./bin/generate-samples.sh bin/configs/scala-sttp4*.yaml— only the circe sample changes;json4sandscala-sttp4-jsoniterare unaffected, confirmed by zero-diff regeneration. No generator options changed, so no doc export changes.PR checklist
master.