Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,10 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)

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

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

// Remove discriminator field from child
// (circe-generic-extras adds it automatically)
// (the sealed trait's encoder writes it)
childModel.vars.removeIf(prop -> prop.baseName.equals(discriminatorName));
childModel.allVars.removeIf(prop -> prop.baseName.equals(discriminatorName));
childModel.requiredVars.removeIf(prop -> prop.baseName.equals(discriminatorName));
Expand Down Expand Up @@ -423,7 +426,7 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
// Remove discriminator property from models that extend a oneOf parent
// (circe-generic-extras adds it automatically)
if (cModel.parent != null && cModel.parentModel != null && cModel.parentModel.discriminator != null) {
String discriminatorName = cModel.parentModel.discriminator.getPropertyName();
String discriminatorName = cModel.parentModel.discriminator.getPropertyBaseName();
cModel.vars.removeIf(prop -> prop.baseName.equals(discriminatorName));
cModel.allVars.removeIf(prop -> prop.baseName.equals(discriminatorName));
cModel.requiredVars.removeIf(prop -> prop.baseName.equals(discriminatorName));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,41 @@ case class {{classname}}(
object {{classname}} {
import io.circe._
import io.circe.syntax._
import io.circe.generic.semiauto._

implicit val encoder: Encoder[{{classname}}] = deriveEncoder[{{classname}}].mapJson(_.dropNullValues)
implicit val decoder: Decoder[{{classname}}] = deriveDecoder
{{^allVars}}
implicit val encoder: Encoder[{{classname}}] = Encoder.instance(_ => Json.obj())
implicit val decoder: Decoder[{{classname}}] = Decoder.const({{classname}}())
{{/allVars}}
{{#allVars}}
{{#-first}}
implicit val encoder: Encoder[{{classname}}] = Encoder.instance { t =>
Json.fromFields(
Seq(
{{/-first}}
{{#required}}Some("{{baseName}}" -> t.{{{name}}}.asJson){{/required}}{{^required}}t.{{{name}}}.map(v => "{{baseName}}" -> v.asJson){{/required}}{{^-last}},{{/-last}}
{{#-last}}
).flatten
)
}
{{/-last}}
{{/allVars}}
{{#allVars}}
{{#-first}}
implicit val decoder: Decoder[{{classname}}] = Decoder.instance { c =>
for {
{{/-first}}
{{{name}}} <- c.downField("{{baseName}}").as[{{^required}}Option[{{/required}}{{dataType}}{{^required}}]{{/required}}]

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: 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>

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.

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.

{{#-last}}
} yield {{classname}}(
{{/-last}}
{{/allVars}}
{{#allVars}}
{{{name}}} = {{{name}}}{{^-last}},{{/-last}}
{{#-last}}
)
}
{{/-last}}
{{/allVars}}
}
{{/circe}}

Expand Down Expand Up @@ -176,30 +207,39 @@ object {{classname}} {
{{#circe}}
{{^vendorExtensions.x-hasWrappedOneOfMembers}}
{{^vendorExtensions.x-use-discr}}
// oneOf without discriminator - using semiauto derivation
// oneOf without discriminator - try each member in turn
import io.circe.{Encoder, Decoder}
import io.circe.generic.semiauto._
import io.circe.syntax._

implicit val encoder: Encoder[{{classname}}] = deriveEncoder[{{classname}}].mapJson(_.dropNullValues)
implicit val decoder: Decoder[{{classname}}] = deriveDecoder
implicit val encoder: Encoder[{{classname}}] = Encoder.instance {
{{#vendorExtensions.x-oneOfMembers}}
case obj: {{classname}} => obj.asJson
{{/vendorExtensions.x-oneOfMembers}}
}
implicit val decoder: Decoder[{{classname}}] = List[Decoder[{{classname}}]](
{{#vendorExtensions.x-oneOfMembers}}
Decoder[{{classname}}].map(x => x: {{vendorExtensions.x-oneOfParent}}),
{{/vendorExtensions.x-oneOfMembers}}
).reduceLeft(_ or _)
{{/vendorExtensions.x-use-discr}}
{{#vendorExtensions.x-use-discr}}
// oneOf with discriminator - using semiauto derivation with Configuration
import io.circe.{Encoder, Decoder}
import io.circe.generic.extras._
import io.circe.generic.extras.semiauto._
// oneOf with discriminator
import io.circe.{Encoder, Decoder, DecodingFailure}
import io.circe.syntax._

private implicit val config: Configuration = Configuration.default.withDiscriminator("{{discriminator.propertyBaseName}}")
.copy(
transformConstructorNames = {
implicit val encoder: Encoder[{{classname}}] = Encoder.instance {
{{#vendorExtensions.x-oneOfMembers}}
case "{{classname}}" => "{{vendorExtensions.x-discriminator-value}}"
case obj: {{classname}} => obj.asJson.mapObject(("{{vendorExtensions.x-discriminator-property}}" -> "{{vendorExtensions.x-discriminator-value}}".asJson) +: _)
{{/vendorExtensions.x-oneOfMembers}}
case other => sys.error(s"Invalid {{classname}} discriminant: ${other}")
}
)
implicit val encoder: Encoder[{{classname}}] = deriveConfiguredEncoder[{{classname}}].mapJson(_.dropNullValues)
implicit val decoder: Decoder[{{classname}}] = deriveConfiguredDecoder
}
implicit val decoder: Decoder[{{classname}}] = Decoder.instance { c =>
c.downField("{{discriminator.propertyBaseName}}").as[String].flatMap {
{{#vendorExtensions.x-oneOfMembers}}
case "{{vendorExtensions.x-discriminator-value}}" => c.as[{{classname}}].map(x => x: {{vendorExtensions.x-oneOfParent}})
{{/vendorExtensions.x-oneOfMembers}}
case other => Left(DecodingFailure(s"Unknown {{discriminator.propertyBaseName}}: $other", c.history))
}
}
{{/vendorExtensions.x-use-discr}}
{{/vendorExtensions.x-hasWrappedOneOfMembers}}
{{#vendorExtensions.x-hasWrappedOneOfMembers}}
Expand Down Expand Up @@ -239,7 +279,7 @@ object {{classname}} {
implicit val decoder: Decoder[{{classname}}] = Decoder.instance { c =>
c.get[String]("{{discriminator.propertyBaseName}}").flatMap {
{{#vendorExtensions.x-oneOfMembers}}
case "{{vendorExtensions.x-discriminator-value}}" => c.as[{{classname}}]({{classname}}.decoder).map(x => x: {{parentClassname}})
case "{{vendorExtensions.x-discriminator-value}}" => c.as[{{classname}}]({{classname}}.decoder).map(x => x: {{vendorExtensions.x-oneOfParent}})
{{/vendorExtensions.x-oneOfMembers}}
{{#vendorExtensions.x-wrappedOneOfMembers}}
case "{{discriminatorValue}}" => c.as[{{classname}}]({{classname}}.decoder).map({{wrapperClassname}}.apply)
Expand Down Expand Up @@ -324,10 +364,41 @@ case class {{classname}}(
object {{classname}} {
import io.circe._
import io.circe.syntax._
import io.circe.generic.semiauto._

implicit val encoder: Encoder[{{classname}}] = deriveEncoder[{{classname}}].mapJson(_.dropNullValues)
implicit val decoder: Decoder[{{classname}}] = deriveDecoder
{{^vars}}
implicit val encoder: Encoder[{{classname}}] = Encoder.instance(_ => Json.obj())
implicit val decoder: Decoder[{{classname}}] = Decoder.const({{classname}}())
{{/vars}}
{{#vars}}
{{#-first}}
implicit val encoder: Encoder[{{classname}}] = Encoder.instance { t =>
Json.fromFields(
Seq(
{{/-first}}
{{#required}}Some("{{baseName}}" -> t.{{{name}}}.asJson){{/required}}{{^required}}t.{{{name}}}.map(v => "{{baseName}}" -> v.asJson){{/required}}{{^-last}},{{/-last}}
{{#-last}}
).flatten
)
}
{{/-last}}
{{/vars}}
{{#vars}}
{{#-first}}
implicit val decoder: Decoder[{{classname}}] = Decoder.instance { c =>
for {
{{/-first}}
{{{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}}]
{{#-last}}
} yield {{classname}}(
{{/-last}}
{{/vars}}
{{#vars}}
{{{name}}} = {{{name}}}{{^-last}},{{/-last}}
{{#-last}}
)
}
{{/-last}}
{{/vars}}
}
{{/circe}}
{{#hasEnums}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,20 +151,22 @@ public void verifyOneOfSupportWithCirce() throws IOException {
Path vehiclePath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/Vehicle.scala");
assertFileContains(vehiclePath, "sealed trait Vehicle");
assertFileContains(vehiclePath, "object Vehicle {");
assertFileContains(vehiclePath, "// oneOf with discriminator - using semiauto derivation with Configuration");
assertFileContains(vehiclePath, "// oneOf with discriminator");
assertFileContains(vehiclePath,
"private implicit val config: Configuration = Configuration.default.withDiscriminator(\"vehicleType\")");
assertFileContains(vehiclePath, "\"Car\" => \"car\"");
assertFileContains(vehiclePath, "\"Truck\" => \"truck\"");
"case obj: Car => obj.asJson.mapObject((\"vehicleType\" -> \"car\".asJson) +: _)");
assertFileContains(vehiclePath,
"case obj: Truck => obj.asJson.mapObject((\"vehicleType\" -> \"truck\".asJson) +: _)");
assertFileContains(vehiclePath, "c.downField(\"vehicleType\").as[String].flatMap {");
assertFileContains(vehiclePath, "case \"car\" => c.as[Car].map(x => x: Vehicle)");
assertFileContains(vehiclePath, "case \"truck\" => c.as[Truck].map(x => x: Vehicle)");

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

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

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

// Verify the discriminator property was removed from inline members
// ClickEvent and ViewEvent should have NO properties at all
Expand Down Expand Up @@ -408,6 +410,70 @@ public void verifyOptionalFieldsOmittedWhenNone() throws IOException {
// not serialized as null: strict servers reject explicit null for
// non-nullable optional properties.
Path petPath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/Pet.scala");
assertFileContains(petPath, "implicit val encoder: Encoder[Pet] = deriveEncoder[Pet].mapJson(_.dropNullValues)");
assertFileContains(petPath, "t.tag.map(v => \"tag\" -> v.asJson)");
assertFileNotContains(petPath, "deriveEncoder");
}

@Test
public void verifyCirceCodecsUseOriginalJsonPropertyNames() throws IOException {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();
String outputPath = output.getAbsolutePath().replace('\\', '/');

OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/3_0/scala/sttp4-mixed-case-fields.yaml", null, new ParseOptions())
.getOpenAPI();

ScalaSttp4ClientCodegen codegen = new ScalaSttp4ClientCodegen();
codegen.setOutputDir(output.getAbsolutePath());
codegen.additionalProperties().put("jsonLibrary", "circe");

ClientOptInput input = new ClientOptInput();
input.openAPI(openAPI);
input.config(codegen);

DefaultGenerator generator = new DefaultGenerator();

generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_TESTS, "false");
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_DOCS, "false");
generator.setGeneratorPropertyDefault(CodegenConstants.APIS, "false");
generator.setGeneratorPropertyDefault(CodegenConstants.SUPPORTING_FILES, "false");
generator.opts(input).generate();

// Scala identifiers stay camelCase; JSON keys are the original spec property names
Path modelPath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/MixedCaseModel.scala");
assertFileContains(modelPath, "assignmentKey: String");
assertFileContains(modelPath, "addressLine2: Option[String] = None");
assertFileContains(modelPath, "Some(\"assignment_key\" -> t.assignmentKey.asJson)");
assertFileContains(modelPath, "t.firstName.map(v => \"first-name\" -> v.asJson)");
assertFileContains(modelPath, "t.zipCode.map(v => \"ZipCode\" -> v.asJson)");
// an already-camelCase property must keep its name, so this stays byte-identical
assertFileContains(modelPath, "t.lastName.map(v => \"lastName\" -> v.asJson)");
assertFileContains(modelPath, "lastName <- c.downField(\"lastName\").as[Option[String]]");
assertFileContains(modelPath, "t.addressLine2.map(v => \"address_line_2\" -> v.asJson)");
assertFileContains(modelPath, "assignmentKey <- c.downField(\"assignment_key\").as[String]");
assertFileContains(modelPath, "addressLine2 <- c.downField(\"address_line_2\").as[Option[String]]");
assertFileContains(modelPath,
"bookingStatus <- c.downField(\"booking_status\").as[Option[MixedCaseModelEnums.BookingStatus]]");
assertFileNotContains(modelPath, "deriveEncoder");
assertFileNotContains(modelPath, "deriveDecoder");

// Inline oneOf members: original names too, and the discriminator property is
// written by the sealed trait rather than carried as a member field
Path paymentPath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/PaymentMethod.scala");
assertFileContains(paymentPath, "Some(\"card_holder_name\" -> t.cardHolderName.asJson)");
assertFileContains(paymentPath, "t.lastFourDigits.map(v => \"last-four-digits\" -> v.asJson)");
assertFileContains(paymentPath,
"case obj: CreditCardPayment => obj.asJson.mapObject((\"payment_type\" -> \"credit_card\".asJson) +: _)");
assertFileContains(paymentPath, "c.downField(\"payment_type\").as[String].flatMap {");
assertFileNotContains(paymentPath, "paymentType");

// oneOf without a discriminator encodes the bare member. Deriving it produced a
// constructor-name wrapper ({"Circle":{..}}), which is not what oneOf means.
Path shapePath = Paths.get(outputPath + "/src/main/scala/org/openapitools/client/model/Shape.scala");
assertFileContains(shapePath, "case obj: Circle => obj.asJson");
assertFileContains(shapePath, "Decoder[Circle].map(x => x: Shape)");
assertFileNotContains(shapePath, "deriveEncoder");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
openapi: 3.0.0
info:
title: sttp4 mixed case fields
version: 1.0.0
paths:
/bookings:
post:
operationId: createBooking
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/MixedCaseModel'
responses:
'200':
description: OK
content:
application/json:
schema:
$ref: '#/components/schemas/PaymentMethod'
components:
schemas:
MixedCaseModel:
type: object
required:
- assignment_key
properties:
assignment_key:
type: string
first-name:
type: string
phone_number:
type: string
lastName:
type: string
ZipCode:
type: string
address_line_2:
type: string
trip_summary:
$ref: '#/components/schemas/TripSummary'
booking_status:
type: string
enum:
- pending
- confirmed
TripSummary:
type: object
properties:
departure_airport_code:
type: string
PaymentMethod:
oneOf:
- $ref: '#/components/schemas/CreditCardPayment'
- $ref: '#/components/schemas/BankTransferPayment'
discriminator:
propertyName: payment_type
mapping:
credit_card: '#/components/schemas/CreditCardPayment'
bank_transfer: '#/components/schemas/BankTransferPayment'
CreditCardPayment:
type: object
required:
- card_holder_name
properties:
payment_type:
type: string
card_holder_name:
type: string
last-four-digits:
type: string
BankTransferPayment:
type: object
properties:
payment_type:
type: string
account_holder_name:
type: string
Shape:
oneOf:
- $ref: '#/components/schemas/Circle'
- $ref: '#/components/schemas/Square'
Circle:
type: object
required:
- radius_cm
properties:
radius_cm:
type: number
Square:
type: object
properties:
side_length:
type: number
Loading
Loading