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: permit parent properties in one of children #3958

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Jul 26, 2024

It turns out that children of oneOf declarations with a discriminator should have the discriminator declared as a property (cf https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/), and some other openapi tooling gets broken if the children do not have that property declaration. We cannot simply declare the discriminator property on a child to be a standard field since that breaks jsoniter adt codecs (for jsoniter, the discriminator must not be a field on the child class). I also didn't like the idea of dropping the property entirely from the child if it's declared in the openapi yaml, since that feels like a violation of the declared construct (even if it does accord with the declaration in over-the-wire formats).

As a compromise between these different requirements, the approach I've taken here is that, if a property is declared on a schema that's a child of a oneOf, and the property has the same name as the descriminator, then we produce a def discriminatorPropertyName: String = "expectedValue" on the child case class. This means that if a case class is the child of two separate oneOfs with the same discriminator name but different values, the generated code will fail; but this feels A) very edge-casey, B) probably bad api design, and C) I can't actually think of a better solution that doesn't have this issue.

Example:
Input openapi:

    ADTWithDiscriminator:
      type: object
      oneOf:
        - $ref: '#/components/schemas/SubtypeWithD1'
        - $ref: '#/components/schemas/SubtypeWithD2'
      discriminator:
        propertyName: type
        mapping:
          'SubA': '#/components/schemas/SubtypeWithD1'
          'SubB': '#/components/schemas/SubtypeWithD2'
...
    SubtypeWithD2:
      type: object
      required:
        - type
        - s
      properties:
        type:
          type: string
        s:
          type: string
        a:
          type: array
          items:
            type: string

Output scala:

case class SubtypeWithD2 (
    s: String,
    a: Option[Seq[String]] = None
  ) extends ADTWithDiscriminator {
    def `type`: String = "SubB"
  }

@adamw
Copy link
Member

adamw commented Jul 26, 2024

Sounds good, lets go with that solution, and if somebody encounters the edge-case, for sure they'll report here :)

@adamw adamw merged commit 1a8d332 into softwaremill:master Jul 26, 2024
22 checks passed
@hughsimpson hughsimpson deleted the codegen_permit_parent_properties_in_oneOf_children branch July 26, 2024 13:56
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