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

[Kotlin][allOf] Generated Code, Data class extends Data class #6080

Closed
donpaul120 opened this issue Apr 27, 2020 · 21 comments
Closed

[Kotlin][allOf] Generated Code, Data class extends Data class #6080

donpaul120 opened this issue Apr 27, 2020 · 21 comments

Comments

@donpaul120
Copy link

components:
  schemas:
    Pet:
      allOf:
        - $ref: '#/components/schemas/NewPet'
        - type: object
          required:
          - id
          properties:
            id:
              type: integer
              format: int64

    NewPet:
      type: object
      required:
        - name  
      properties:
        name:
          type: string
        tag:
          type: string  

The above spec generates:

data class NewPet (
   @SerializedName("name")
   val name: kotlin.String,
   @SerializedName("type")
   val type: kotlin.String,
)


data class Pet (
   @SerializedName("id")
   val id: kotlin.Int
) : Pet

In Kotlin it's impossible for a data class to extend a data class.

@wrssmi
Copy link

wrssmi commented Apr 28, 2020

This happens also in java models since version 4.3.0

public class NewPet {
}

public class Pet extends NewPet  {
}

with 4.2.3 following result is generated

public class NewPet {
...fields NewPet
}

public class Pet   {
...fields Pet + NewPet
}

@donpaul120
Copy link
Author

@wrssmi I believe the result is fine in Java.
data classes in kotlin are final classes thus it can't be inherited, however in the generated model classes for java, models aren't declared as final, which make inheritance fine and allowed.

Except i don't fully understand what you mean.

@wrssmi
Copy link

wrssmi commented Apr 28, 2020

i only want the behavior of extending models if I define it with discriminator

    Pet:
      type: object
      required:
        - petType
      properties:
        petType:
          type: string
      discriminator:
        propertyName: petType
    Dog:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            bark:
              type: boolean
            breed:
              type: string
              enum: [Dingo, Husky, Retriever, Shepherd]

than this is ok:

public class Pet {
}
public class Dog extends Pet {
}

As I mentioned with generator 4.2.3 it works in that way

@wing328
Copy link
Member

wing328 commented Apr 29, 2020

@wrssmi that's a bug (regression) in 4.3.0 and we've fixed it in the master.

Please give it a try and let us know how it goes.

@wrssmi
Copy link

wrssmi commented Apr 29, 2020

4.3.1-SNAPSHOT is working fine for my regression tests!

@prabu-ramaraj
Copy link

This issue is related to Kotlin codegen extending data class. As @donpaul120 mentioned, data class cannot be extended in Kotlin. Looks like this discussion got hijaked into a Java codegen issue.

@donpaul120
Copy link
Author

@prabu-ramaraj exactly, it was forcefully hijacked. I mean you could create an issue relating to Java and that's fine but the codegen issue relating to java that @wrssmi talks about doesn't relate to what the issue refers to in Kotlin. They are not even close.

@wrssmi
Copy link

wrssmi commented May 4, 2020

give it a try with 4.3.1-SNAPSHOT related fix

@prabu-ramaraj
Copy link

@wrssmi - Yes, the latest Snapshot fixes this issue in Kotlin as well. This is great. When can we expect the snaphsot to be published? I think it's supposed to be published 2 weeks back right?

@wrssmi
Copy link

wrssmi commented May 4, 2020 via email

@prabu-ramaraj
Copy link

prabu-ramaraj commented May 4, 2020

@wrssmi - I tested all the way down to 4.2.1, I am still seeing this issue in older versions. I am using Gradle plugin. In 4.2.1, "override" is gone, but still it is extending from parent data class. I can't go below 4.2.1 without updating my build script.

Is there anyway to get an ETA for 4.3.1 release? I have downloaded the Gradle plugin JAR for 4.3.1-SNAPSHOT version. I will use this for my development now

@asm0dey
Copy link

asm0dey commented Sep 15, 2021

@donpaul120 issue still exists in 5.2.0

@torsjens
Copy link

@wing328: Any plans for fixing this if you have multiple layer inheritance?

openapi-generator --version
openapi-generator-cli 5.3.0
  commit : bb124e1
  built  : 2021-10-24T14:50:41Z
  source : https://github.com/openapitools/openapi-generator
  docs   : https://openapi-generator.tech/
openapi: 3.0.3
info:
  description: "Animanimal spec"
  version: "v1"
  title: "Animanimal"
  license:
    name: "test"
    url: "http://www.apache.org/licenses/LICENSE-2.0.html"
paths:
  /getMyCockerSpaniel:
    post:
      summary: "get my Cocker Spaniel"
      responses:
        '200':
          description: "a Cocker Spaniel"
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/CockerSpaniel'
components:
  schemas:
    Pet:
      type: object
      discriminator:
        propertyName: petType
      properties:
        petType:
          type: string
      required:
        - petType
    Dog:
      description: A representation of a dog
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            packSize:
              type: integer
              format: int32
    CockerSpaniel:
      description: A representation of a Cocker Spaniel dog
      allOf:
        - $ref: '#/components/schemas/Dog'
        - type: object
          properties:
            earLength:
              type: integer

This generates:

interface Pet {
    @Json(name = "petType")
    val petType: kotlin.String
}
data class Dog (
    @Json(name = "petType")
    override val petType: kotlin.String,

    @Json(name = "packSize")
    val packSize: kotlin.Int? = null
) : Pet
data class CockerSpaniel (
    @Json(name = "petType")
    override val petType: kotlin.String,

    @Json(name = "packSize")
    override val packSize: kotlin.Int? = null,

    @Json(name = "earLength")
    val earLength: kotlin.Int? = null
) : Dog

@tormodatt
Copy link

tormodatt commented Nov 26, 2021

I guess Dog from the example above could have been generated to something like this?

interface Dog: Pet {
    @Json(name = "petType")
    override val petType: kotlin.String

    @Json(name = "packSize")
    val packSize: kotlin.Int?
}

data class DogImpl (
    @Json(name = "petType")
    override val petType: kotlin.String,

    @Json(name = "packSize")
    override val packSize: kotlin.Int? = null
) : Dog

@asm0dey
Copy link

asm0dey commented Nov 26, 2021

@tormodatt for example, but it will prevent system from creation of actual Dog instances which can potentially be desirable

@torsjens
Copy link

torsjens commented Nov 29, 2021

@asm0dey: But you could create a DogImpl instance instead, which would practically be a Dog, except for the name of the class? Or am I missing something? For instance, by having the generator declare the following JsonSubTypes? (when using Jackson as serialization library):

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "petType", visible = true)
@JsonSubTypes(
    JsonSubTypes.Type(value = DogImpl::class, name = "dog")
)
interface Pet {
    @Json(name = "petType")
    val petType: kotlin.String
}

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "petType", visible = true)
@JsonSubTypes(
    JsonSubTypes.Type(value = CockerSpaniel::class, name = "cockerSpaniel")
)
interface Dog: Pet {
    @Json(name = "petType")
    override val petType: kotlin.String

    @Json(name = "packSize")
    val packSize: kotlin.Int?
}

data class DogImpl (
    @Json(name = "petType")
    override val petType: kotlin.String,

    @Json(name = "packSize")
    override val packSize: kotlin.Int? = null
) : Dog

data class CockerSpaniel (
    @Json(name = "petType")
    override val petType: kotlin.String,

    @Json(name = "packSize")
    override val packSize: kotlin.Int? = null,

    @Json(name = "earLength")
    val earLength: kotlin.Int? = null
) : Dog

And when creating an *Impl instance having an understanding that these instances have to be created with discriminator (petType) without the impl part, i.e. "dog" and not "dogImpl":

val dogInstance = DogImpl("dog", 10)

Then, when receiving

{
   petType: "dog",
  packSize: 10
}

you could deserialize it to a DogImpl instance locally, and when sending it back, you would serialize it back to the same json, and a receiver would be able to parse it as a Dog on the other end.

@asm0dey
Copy link

asm0dey commented Nov 29, 2021

@torsjens yes, you're right. If the backend will support such behaviour - it's a working solution!

@enekonieto
Copy link

This is working in 7.0.1. Child data class does not extends the Parent data class.

If you add discriminator to Parent it becomes an Interface and Child data class extends it.

Disclaimer: I didn't test with the provided spec but I had a very similar one.

@wing328
Copy link
Member

wing328 commented Oct 23, 2023

FYI. One can use openapi normalizer (rule: REF_AS_PARENT_IN_ALLOF ) to consider $ref as parent without using discriminator.

@enekonieto
Copy link

Thanks for the info, very useful! Should this issue be closed?

@gcuellar
Copy link

gcuellar commented Mar 26, 2024

This thread should be reopened (or created new one instead) in order to include the solution proposed by @torsjens. It would be great to generate an interface and implementation for each model. This would allow upcasting to any ancestor class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants