Skip to content

Commit 23dd9fc

Browse files
vivekmahajanvivekmahajan
authored
[scala-sttp4] Circe codecs do not preserve original JSON property names (#24681)
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 #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 #24362.
1 parent 123b428 commit 23dd9fc

10 files changed

Lines changed: 418 additions & 55 deletions

File tree

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/ScalaSttp4ClientCodegen.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,10 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
350350

351351
// Add discriminator mapping value if present
352352
if (cModel.discriminator != null) {
353-
String discriminatorName = cModel.discriminator.getPropertyName();
353+
// baseName, not propertyName: the latter is the escaped Scala
354+
// identifier and never matches a property's baseName
355+
String discriminatorName = cModel.discriminator.getPropertyBaseName();
356+
childModel.getVendorExtensions().put("x-discriminator-property", discriminatorName);
354357

355358
// Find the mapping value for this child model
356359
String discriminatorValue = null;
@@ -368,7 +371,7 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
368371
}
369372

370373
// Remove discriminator field from child
371-
// (circe-generic-extras adds it automatically)
374+
// (the sealed trait's encoder writes it)
372375
childModel.vars.removeIf(prop -> prop.baseName.equals(discriminatorName));
373376
childModel.allVars.removeIf(prop -> prop.baseName.equals(discriminatorName));
374377
childModel.requiredVars.removeIf(prop -> prop.baseName.equals(discriminatorName));
@@ -423,7 +426,7 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
423426
// Remove discriminator property from models that extend a oneOf parent
424427
// (circe-generic-extras adds it automatically)
425428
if (cModel.parent != null && cModel.parentModel != null && cModel.parentModel.discriminator != null) {
426-
String discriminatorName = cModel.parentModel.discriminator.getPropertyName();
429+
String discriminatorName = cModel.parentModel.discriminator.getPropertyBaseName();
427430
cModel.vars.removeIf(prop -> prop.baseName.equals(discriminatorName));
428431
cModel.allVars.removeIf(prop -> prop.baseName.equals(discriminatorName));
429432
cModel.requiredVars.removeIf(prop -> prop.baseName.equals(discriminatorName));

‎modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache‎

Lines changed: 95 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,41 @@ case class {{classname}}(
4949
object {{classname}} {
5050
import io.circe._
5151
import io.circe.syntax._
52-
import io.circe.generic.semiauto._
5352
54-
implicit val encoder: Encoder[{{classname}}] = deriveEncoder[{{classname}}].mapJson(_.dropNullValues)
55-
implicit val decoder: Decoder[{{classname}}] = deriveDecoder
53+
{{^allVars}}
54+
implicit val encoder: Encoder[{{classname}}] = Encoder.instance(_ => Json.obj())
55+
implicit val decoder: Decoder[{{classname}}] = Decoder.const({{classname}}())
56+
{{/allVars}}
57+
{{#allVars}}
58+
{{#-first}}
59+
implicit val encoder: Encoder[{{classname}}] = Encoder.instance { t =>
60+
Json.fromFields(
61+
Seq(
62+
{{/-first}}
63+
{{#required}}Some("{{baseName}}" -> t.{{{name}}}.asJson){{/required}}{{^required}}t.{{{name}}}.map(v => "{{baseName}}" -> v.asJson){{/required}}{{^-last}},{{/-last}}
64+
{{#-last}}
65+
).flatten
66+
)
67+
}
68+
{{/-last}}
69+
{{/allVars}}
70+
{{#allVars}}
71+
{{#-first}}
72+
implicit val decoder: Decoder[{{classname}}] = Decoder.instance { c =>
73+
for {
74+
{{/-first}}
75+
{{{name}}} <- c.downField("{{baseName}}").as[{{^required}}Option[{{/required}}{{dataType}}{{^required}}]{{/required}}]
76+
{{#-last}}
77+
} yield {{classname}}(
78+
{{/-last}}
79+
{{/allVars}}
80+
{{#allVars}}
81+
{{{name}}} = {{{name}}}{{^-last}},{{/-last}}
82+
{{#-last}}
83+
)
84+
}
85+
{{/-last}}
86+
{{/allVars}}
5687
}
5788
{{/circe}}
5889

@@ -176,30 +207,39 @@ object {{classname}} {
176207
{{#circe}}
177208
{{^vendorExtensions.x-hasWrappedOneOfMembers}}
178209
{{^vendorExtensions.x-use-discr}}
179-
// oneOf without discriminator - using semiauto derivation
210+
// oneOf without discriminator - try each member in turn
180211
import io.circe.{Encoder, Decoder}
181-
import io.circe.generic.semiauto._
212+
import io.circe.syntax._
182213
183-
implicit val encoder: Encoder[{{classname}}] = deriveEncoder[{{classname}}].mapJson(_.dropNullValues)
184-
implicit val decoder: Decoder[{{classname}}] = deriveDecoder
214+
implicit val encoder: Encoder[{{classname}}] = Encoder.instance {
215+
{{#vendorExtensions.x-oneOfMembers}}
216+
case obj: {{classname}} => obj.asJson
217+
{{/vendorExtensions.x-oneOfMembers}}
218+
}
219+
implicit val decoder: Decoder[{{classname}}] = List[Decoder[{{classname}}]](
220+
{{#vendorExtensions.x-oneOfMembers}}
221+
Decoder[{{classname}}].map(x => x: {{vendorExtensions.x-oneOfParent}}),
222+
{{/vendorExtensions.x-oneOfMembers}}
223+
).reduceLeft(_ or _)
185224
{{/vendorExtensions.x-use-discr}}
186225
{{#vendorExtensions.x-use-discr}}
187-
// oneOf with discriminator - using semiauto derivation with Configuration
188-
import io.circe.{Encoder, Decoder}
189-
import io.circe.generic.extras._
190-
import io.circe.generic.extras.semiauto._
226+
// oneOf with discriminator
227+
import io.circe.{Encoder, Decoder, DecodingFailure}
228+
import io.circe.syntax._
191229
192-
private implicit val config: Configuration = Configuration.default.withDiscriminator("{{discriminator.propertyBaseName}}")
193-
.copy(
194-
transformConstructorNames = {
230+
implicit val encoder: Encoder[{{classname}}] = Encoder.instance {
195231
{{#vendorExtensions.x-oneOfMembers}}
196-
case "{{classname}}" => "{{vendorExtensions.x-discriminator-value}}"
232+
case obj: {{classname}} => obj.asJson.mapObject(("{{vendorExtensions.x-discriminator-property}}" -> "{{vendorExtensions.x-discriminator-value}}".asJson) +: _)
197233
{{/vendorExtensions.x-oneOfMembers}}
198-
case other => sys.error(s"Invalid {{classname}} discriminant: ${other}")
199-
}
200-
)
201-
implicit val encoder: Encoder[{{classname}}] = deriveConfiguredEncoder[{{classname}}].mapJson(_.dropNullValues)
202-
implicit val decoder: Decoder[{{classname}}] = deriveConfiguredDecoder
234+
}
235+
implicit val decoder: Decoder[{{classname}}] = Decoder.instance { c =>
236+
c.downField("{{discriminator.propertyBaseName}}").as[String].flatMap {
237+
{{#vendorExtensions.x-oneOfMembers}}
238+
case "{{vendorExtensions.x-discriminator-value}}" => c.as[{{classname}}].map(x => x: {{vendorExtensions.x-oneOfParent}})
239+
{{/vendorExtensions.x-oneOfMembers}}
240+
case other => Left(DecodingFailure(s"Unknown {{discriminator.propertyBaseName}}: $other", c.history))
241+
}
242+
}
203243
{{/vendorExtensions.x-use-discr}}
204244
{{/vendorExtensions.x-hasWrappedOneOfMembers}}
205245
{{#vendorExtensions.x-hasWrappedOneOfMembers}}
@@ -239,7 +279,7 @@ object {{classname}} {
239279
implicit val decoder: Decoder[{{classname}}] = Decoder.instance { c =>
240280
c.get[String]("{{discriminator.propertyBaseName}}").flatMap {
241281
{{#vendorExtensions.x-oneOfMembers}}
242-
case "{{vendorExtensions.x-discriminator-value}}" => c.as[{{classname}}]({{classname}}.decoder).map(x => x: {{parentClassname}})
282+
case "{{vendorExtensions.x-discriminator-value}}" => c.as[{{classname}}]({{classname}}.decoder).map(x => x: {{vendorExtensions.x-oneOfParent}})
243283
{{/vendorExtensions.x-oneOfMembers}}
244284
{{#vendorExtensions.x-wrappedOneOfMembers}}
245285
case "{{discriminatorValue}}" => c.as[{{classname}}]({{classname}}.decoder).map({{wrapperClassname}}.apply)
@@ -324,10 +364,41 @@ case class {{classname}}(
324364
object {{classname}} {
325365
import io.circe._
326366
import io.circe.syntax._
327-
import io.circe.generic.semiauto._
328367
329-
implicit val encoder: Encoder[{{classname}}] = deriveEncoder[{{classname}}].mapJson(_.dropNullValues)
330-
implicit val decoder: Decoder[{{classname}}] = deriveDecoder
368+
{{^vars}}
369+
implicit val encoder: Encoder[{{classname}}] = Encoder.instance(_ => Json.obj())
370+
implicit val decoder: Decoder[{{classname}}] = Decoder.const({{classname}}())
371+
{{/vars}}
372+
{{#vars}}
373+
{{#-first}}
374+
implicit val encoder: Encoder[{{classname}}] = Encoder.instance { t =>
375+
Json.fromFields(
376+
Seq(
377+
{{/-first}}
378+
{{#required}}Some("{{baseName}}" -> t.{{{name}}}.asJson){{/required}}{{^required}}t.{{{name}}}.map(v => "{{baseName}}" -> v.asJson){{/required}}{{^-last}},{{/-last}}
379+
{{#-last}}
380+
).flatten
381+
)
382+
}
383+
{{/-last}}
384+
{{/vars}}
385+
{{#vars}}
386+
{{#-first}}
387+
implicit val decoder: Decoder[{{classname}}] = Decoder.instance { c =>
388+
for {
389+
{{/-first}}
390+
{{{name}}} <- c.downField("{{baseName}}").as[{{^required}}Option[{{/required}}{{^isEnum}}{{dataType}}{{/isEnum}}{{#isEnum}}{{^isArray}}{{classname}}Enums.{{datatypeWithEnum}}{{/isArray}}{{#isArray}}Seq[{{classname}}Enums.{{datatypeWithEnum}}]{{/isArray}}{{/isEnum}}{{^required}}]{{/required}}]
391+
{{#-last}}
392+
} yield {{classname}}(
393+
{{/-last}}
394+
{{/vars}}
395+
{{#vars}}
396+
{{{name}}} = {{{name}}}{{^-last}},{{/-last}}
397+
{{#-last}}
398+
)
399+
}
400+
{{/-last}}
401+
{{/vars}}
331402
}
332403
{{/circe}}
333404
{{#hasEnums}}

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/scala/Sttp4CodegenTest.java‎

Lines changed: 76 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -151,20 +151,22 @@ public void verifyOneOfSupportWithCirce() throws IOException {
151151
Path vehiclePath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/Vehicle.scala");
152152
assertFileContains(vehiclePath, "sealed trait Vehicle");
153153
assertFileContains(vehiclePath, "object Vehicle {");
154-
assertFileContains(vehiclePath, "// oneOf with discriminator - using semiauto derivation with Configuration");
154+
assertFileContains(vehiclePath, "// oneOf with discriminator");
155155
assertFileContains(vehiclePath,
156-
"private implicit val config: Configuration = Configuration.default.withDiscriminator(\"vehicleType\")");
157-
assertFileContains(vehiclePath, "\"Car\" => \"car\"");
158-
assertFileContains(vehiclePath, "\"Truck\" => \"truck\"");
156+
"case obj: Car => obj.asJson.mapObject((\"vehicleType\" -> \"car\".asJson) +: _)");
157+
assertFileContains(vehiclePath,
158+
"case obj: Truck => obj.asJson.mapObject((\"vehicleType\" -> \"truck\".asJson) +: _)");
159+
assertFileContains(vehiclePath, "c.downField(\"vehicleType\").as[String].flatMap {");
160+
assertFileContains(vehiclePath, "case \"car\" => c.as[Car].map(x => x: Vehicle)");
161+
assertFileContains(vehiclePath, "case \"truck\" => c.as[Truck].map(x => x: Vehicle)");
159162

160163
// Test oneOf with discriminator that is a Scala keyword ("type")
161164
// The discriminator should use the original wire name, not the backtick-escaped Scala name
162165
Path shapePath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/Shape.scala");
163166
assertFileContains(shapePath, "sealed trait Shape");
164-
assertFileContains(shapePath,
165-
"private implicit val config: Configuration = Configuration.default.withDiscriminator(\"type\")");
167+
assertFileContains(shapePath, "c.downField(\"type\").as[String].flatMap {");
166168
// Discriminator in serialization must not be backtick-escaped
167-
assertFileNotContains(shapePath, "withDiscriminator(\"`type`\")");
169+
assertFileNotContains(shapePath, "`type`");
168170

169171
// Verify regular models are still case classes
170172
Path dogPath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/Dog.scala");
@@ -263,8 +265,8 @@ public void verifyOneOfWithEmptyMembers() throws IOException {
263265
assertFileContains(eventPath, "case class PurchaseEvent(");
264266
assertFileContains(eventPath, "amount: Double");
265267

266-
// Verify discriminator is configured
267-
assertFileContains(eventPath, "Configuration.default.withDiscriminator(\"eventType\")");
268+
// Verify the discriminator is written by the sealed trait's encoder
269+
assertFileContains(eventPath, "c.downField(\"eventType\").as[String].flatMap {");
268270

269271
// Verify the discriminator property was removed from inline members
270272
// ClickEvent and ViewEvent should have NO properties at all
@@ -408,7 +410,71 @@ public void verifyOptionalFieldsOmittedWhenNone() throws IOException {
408410
// not serialized as null: strict servers reject explicit null for
409411
// non-nullable optional properties.
410412
Path petPath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/Pet.scala");
411-
assertFileContains(petPath, "implicit val encoder: Encoder[Pet] = deriveEncoder[Pet].mapJson(_.dropNullValues)");
413+
assertFileContains(petPath, "t.tag.map(v => \"tag\" -> v.asJson)");
414+
assertFileNotContains(petPath, "deriveEncoder");
415+
}
416+
417+
@Test
418+
public void verifyCirceCodecsUseOriginalJsonPropertyNames() throws IOException {
419+
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
420+
output.deleteOnExit();
421+
String outputPath = output.getAbsolutePath().replace('\\', '/');
422+
423+
OpenAPI openAPI = new OpenAPIParser()
424+
.readLocation("src/test/resources/3_0/scala/sttp4-mixed-case-fields.yaml", null, new ParseOptions())
425+
.getOpenAPI();
426+
427+
ScalaSttp4ClientCodegen codegen = new ScalaSttp4ClientCodegen();
428+
codegen.setOutputDir(output.getAbsolutePath());
429+
codegen.additionalProperties().put("jsonLibrary", "circe");
430+
431+
ClientOptInput input = new ClientOptInput();
432+
input.openAPI(openAPI);
433+
input.config(codegen);
434+
435+
DefaultGenerator generator = new DefaultGenerator();
436+
437+
generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
438+
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_TESTS, "false");
439+
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_DOCS, "false");
440+
generator.setGeneratorPropertyDefault(CodegenConstants.APIS, "false");
441+
generator.setGeneratorPropertyDefault(CodegenConstants.SUPPORTING_FILES, "false");
442+
generator.opts(input).generate();
443+
444+
// Scala identifiers stay camelCase; JSON keys are the original spec property names
445+
Path modelPath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/MixedCaseModel.scala");
446+
assertFileContains(modelPath, "assignmentKey: String");
447+
assertFileContains(modelPath, "addressLine2: Option[String] = None");
448+
assertFileContains(modelPath, "Some(\"assignment_key\" -> t.assignmentKey.asJson)");
449+
assertFileContains(modelPath, "t.firstName.map(v => \"first-name\" -> v.asJson)");
450+
assertFileContains(modelPath, "t.zipCode.map(v => \"ZipCode\" -> v.asJson)");
451+
// an already-camelCase property must keep its name, so this stays byte-identical
452+
assertFileContains(modelPath, "t.lastName.map(v => \"lastName\" -> v.asJson)");
453+
assertFileContains(modelPath, "lastName <- c.downField(\"lastName\").as[Option[String]]");
454+
assertFileContains(modelPath, "t.addressLine2.map(v => \"address_line_2\" -> v.asJson)");
455+
assertFileContains(modelPath, "assignmentKey <- c.downField(\"assignment_key\").as[String]");
456+
assertFileContains(modelPath, "addressLine2 <- c.downField(\"address_line_2\").as[Option[String]]");
457+
assertFileContains(modelPath,
458+
"bookingStatus <- c.downField(\"booking_status\").as[Option[MixedCaseModelEnums.BookingStatus]]");
459+
assertFileNotContains(modelPath, "deriveEncoder");
460+
assertFileNotContains(modelPath, "deriveDecoder");
461+
462+
// Inline oneOf members: original names too, and the discriminator property is
463+
// written by the sealed trait rather than carried as a member field
464+
Path paymentPath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/PaymentMethod.scala");
465+
assertFileContains(paymentPath, "Some(\"card_holder_name\" -> t.cardHolderName.asJson)");
466+
assertFileContains(paymentPath, "t.lastFourDigits.map(v => \"last-four-digits\" -> v.asJson)");
467+
assertFileContains(paymentPath,
468+
"case obj: CreditCardPayment => obj.asJson.mapObject((\"payment_type\" -> \"credit_card\".asJson) +: _)");
469+
assertFileContains(paymentPath, "c.downField(\"payment_type\").as[String].flatMap {");
470+
assertFileNotContains(paymentPath, "paymentType");
471+
472+
// oneOf without a discriminator encodes the bare member. Deriving it produced a
473+
// constructor-name wrapper ({"Circle":{..}}), which is not what oneOf means.
474+
Path shapePath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/Shape.scala");
475+
assertFileContains(shapePath, "case obj: Circle => obj.asJson");
476+
assertFileContains(shapePath, "Decoder[Circle].map(x => x: Shape)");
477+
assertFileNotContains(shapePath, "deriveEncoder");
412478
}
413479

414480
@Test

‎modules/openapi-generator/src/test/resources/3_0/scala/sttp4-mixed-case-fields.yaml‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
openapi: 3.0.0
2+
info:
3+
title: sttp4 mixed case fields
4+
version: 1.0.0
5+
paths:
6+
/bookings:
7+
post:
8+
operationId: createBooking
9+
requestBody:
10+
content:
11+
application/json:
12+
schema:
13+
$ref: '#/components/schemas/MixedCaseModel'
14+
responses:
15+
'200':
16+
description: OK
17+
content:
18+
application/json:
19+
schema:
20+
$ref: '#/components/schemas/PaymentMethod'
21+
components:
22+
schemas:
23+
MixedCaseModel:
24+
type: object
25+
required:
26+
- assignment_key
27+
properties:
28+
assignment_key:
29+
type: string
30+
first-name:
31+
type: string
32+
phone_number:
33+
type: string
34+
lastName:
35+
type: string
36+
ZipCode:
37+
type: string
38+
address_line_2:
39+
type: string
40+
trip_summary:
41+
$ref: '#/components/schemas/TripSummary'
42+
booking_status:
43+
type: string
44+
enum:
45+
- pending
46+
- confirmed
47+
TripSummary:
48+
type: object
49+
properties:
50+
departure_airport_code:
51+
type: string
52+
PaymentMethod:
53+
oneOf:
54+
- $ref: '#/components/schemas/CreditCardPayment'
55+
- $ref: '#/components/schemas/BankTransferPayment'
56+
discriminator:
57+
propertyName: payment_type
58+
mapping:
59+
credit_card: '#/components/schemas/CreditCardPayment'
60+
bank_transfer: '#/components/schemas/BankTransferPayment'
61+
CreditCardPayment:
62+
type: object
63+
required:
64+
- card_holder_name
65+
properties:
66+
payment_type:
67+
type: string
68+
card_holder_name:
69+
type: string
70+
last-four-digits:
71+
type: string
72+
BankTransferPayment:
73+
type: object
74+
properties:
75+
payment_type:
76+
type: string
77+
account_holder_name:
78+
type: string
79+
Shape:
80+
oneOf:
81+
- $ref: '#/components/schemas/Circle'
82+
- $ref: '#/components/schemas/Square'
83+
Circle:
84+
type: object
85+
required:
86+
- radius_cm
87+
properties:
88+
radius_cm:
89+
type: number
90+
Square:
91+
type: object
92+
properties:
93+
side_length:
94+
type: number

0 commit comments

Comments
 (0)