[kotlin] [client] Implement inheritance and oneOf support in kotlin-client

[vlsergey]

trafficstars

Add example based on oneof_polymorphism_and_inheritance.yaml schema and fix all problems that prevented to correctly implement it:

• Change kotlin-client generation to support three different model types: data class, open class and interface
• Correctly implement parent constructor call (from open / data class to parent open class)
• Add implemented interfaces list (i.e. supports oneOf interfaces)
• Add override flags so kotlin code can be compiled now
• Add inheritedVars collection to CodegenModel thus we can correctly handle case when property specified both in current and parent model

I do believe that it is a non-breaking change for compilable code generation, but it surely will change how non-compilable kotlin code generated (and makes it compilable)

PR checklist

• [x] Read the contribution guidelines.
• [x] Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• [x] Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.
• [x] File the PR against the correct branch: master (6.1.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
• [x] If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request. @jimschubert, @dr4ke616, @karismann, @Zomzog, @andrewemery, @4brunu, @yutaka0m

[4brunu]

Hey @vlsergey first of all, thanks for taking the time to create this PR. Could you please run the following commands and commit the changes?

./mvnw clean package
./bin/generate-samples.sh ./bin/configs/kotlin*

Thanks

[vlsergey]

Hi @4brunu. I did clean / package / generate-samples multiple times (all samples, not only kotlin ones), but locally i have no new/changes files. I have no idea why CI reports changes in swift examples.

Neverless i've updates own branch with latest master and tried to push again.

[4brunu]

This PR touches in a lot of places that I don't have a lot of knowable and it quick extensive, so I have a hard time reviewing it. Maybe @wing328 can help here with the review? Thanks

[4brunu]

cc @OpenAPITools/generator-core-team can someone please help to review this PR, because it touches in parts that I don't know? Thanks

[scottkennedy]

Any updates on this? It would be really useful.

[vibbix]

Will this interfere with #12966 ?

[vibbix]

When using this with in kotlin multiplatform, i noticed the generated "AnyOf" classes attached a @Serializable annotation, leading to this error: @Serializable annotation without arguments can be used only on sealed interfaces.Non-sealed interfaces are polymorphically serializable by default. For example:

@Serializable
interface PaginatedAwardAllOf {

    @SerialName(value = "data") val `data`: kotlin.collections.List<Award>?
}

[stuebingerb]

Is this PR still alive? What needs to be done to get it merged?

[stuebingerb]

I'm actually not sure if this PR works as expected.

The initial schema is (excerpt):

    FooRefOrValue:
      type: object
      oneOf:
        - $ref: "#/components/schemas/Foo"
        - $ref: "#/components/schemas/FooRef"
      discriminator:
        propertyName: "@type"
    Foo:
      type: object
      properties:
        fooPropA:
          type: string
        fooPropB:
          type: string
      allOf:
        - $ref: '#/components/schemas/Entity'
    FooRef:
      type: object
      properties:
        foorefPropA:
          type: string
      allOf:
        - $ref: '#/components/schemas/EntityRef'

So Foo and FooRef are objects, and FooRefOrValue is either the one or the other.

The models created for that are

interface Foo
interface FooRef
interface FooRefOrValue : Foo, FooRef

Which reads to me as: FooRefOrValue is a Foo and a FooRef (and none of the models is actually able to store data as there is no implementation).

Shouldn't this rather be something like

data class Foo : FooRefOrValue
data class FooRef : FooRefOrValue
interface FooRefOrValue

(or possibly even a sealed class/interface FooRefOrValue)?

[vlsergey]

Shouldn't this rather be something like

data class Foo : FooRefOrValue
data class FooRef : FooRefOrValue
interface FooRefOrValue

?

Interesting, i will look into it. May be there is a mismatch between allOf and oneOf logic, that need to be handled.

[vlsergey]

WIP:

data class Foo /* ... */ : Entity
data class Bar /* ... */ : Entity
interface FooRefOrValue

Now looking how to make Bar and BarRef implements BarRefOrValue interface

[vlsergey]

Done:

data class Foo /* ... */ : Entity, FooRefOrValue
data class Bar /* ... */ : Entity, BarRefOrValue
interface FooRefOrValue

Change note: i've removed inherited variables processing from DefaultCodegen and implemented kotlin-only processing in AbstractKotlinCodegen. So it should be a bit easier to review.

[vlsergey]

• fixed kotlin-allOff-discriminator example
• fixed non-deterministric interfaces order (now explicitly sorted)

[vlsergey]

@4brunu, can you check PR again, now it doesn't change a lot non-kotlin internals.

[4brunu]

This PR is beyond my knowledge of openapi. @wing328 could you please help to review this PR? Thanks 🙂

[trunneml]

Is there any chance that this PR is reviewed and merged soon? oneOf support would be a good thing. :)

[4brunu]

@stuebingerb could you please test to see if this PR now works as expected? Thanks

[stuebingerb]

Sorry, I have been busy in the past days.

I had a look into the generated Kotlin models and they definitely look better than at the time of my previous comment. So far I would say that everything seems to be as expected, good work! Unfortunately, we have moved away from inheritance due to other issues (unrelated to the generator) so I don't have a real use case anymore and won't be able to do thorough testing.

I'm wondering why a Kotlin implementation changes lines in Java models, though, but did not look into that further. Possibly this will be resolved by a rebase.

[wing328]

thanks for the PR. looks like a major change so i'll test it out later and target v7.0.0 release.

[vfouqueron]

Great work! I've used your PR to implement oneOf for kotlin-spring generator, it works great! Looking forward this to be merged so I can push my own PR :)

[richardvanduijn]

We see this pr did not make it to 7.0.0-beta release. Are you still planning to add it to 7.0.0 release? I know many people are waiting for this one-of kotlin support to be in the generator lib :D

[thejeff77]

thanks for the PR. looks like a major change so i'll test it out later and target v7.0.0 release.

@wing328 any comment on this? Thanks for any consideration for a late merge. @vlsergey it looks like some conflicts have arisen.

[thejeff77]

I tried copying the mustache templates over and using them myself.

Unfortunately this doesn't compile for me when I remove the jvm-ktor library option and the jackson serialization library option.

Furthermore, I didn't see any hierarchy in the classes generated that used the oneOf annotation..

[wing328]

Furthermore, I didn't see any hierarchy in the classes generated that used the oneOf annotation..

@thejeff77 I don't think oneOf should result any hierarchy in the classes generated.

[wing328]

Please try the java client generator and review its oneOf implementation.

For the implementation in this PR using interface, I don't think it can handle the following:

IntOrString:
  oneOf:
    - type: string
    - type: integer

[amorozov]

@vfouqueron can you please publish your work?

BTW. I have updated the original patch to the v7.x state, though pretty "mechanically", without deep understanding how things have changed since v6.x.

[amorozov]

Also I have discovered the the original patch incorrectly handles the "Fruit/Apple/Banana" testcase from v7.x and master.

The generated code is:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "fruitType", visible = true)
@JsonSubTypes(
    JsonSubTypes.Type(value = Apple::class, name = "APPLE"),
    JsonSubTypes.Type(value = Banana::class, name = "BANANA")
)

interface Fruit {

    @get:JsonProperty("fruitType")
    val fruitType: FruitType
    @get:JsonProperty("seeds")
    val seeds: kotlin.Int
    @get:JsonProperty("length")
    val length: kotlin.Int
}

data class Apple (

    @field:JsonProperty("seeds")
    override val seeds: kotlin.Int

) : Fruit

data class Banana (

    @field:JsonProperty("length")
    override val length: kotlin.Int

) : Fruit

and, obviously, it can't be compiled due to:

e: .../Apple.kt: (28, 6): Class 'Apple' is not abstract and does not implement abstract member public abstract val fruitType: FruitType defined in org.openapitools.client.models.Fruit
e: .../Banana.kt: (28, 6): Class 'Banana' is not abstract and does not implement abstract member public abstract val fruitType: FruitType defined in org.openapitools.client.models.Fruit

@vlsergey, can you take a look at this? I'd suppose the generated interface shouldn't have all properties of descendant classes, only a minimal common subset or just those which are defined right in the base type. So for the given testcase interface Fruit would have had only fruitType and the data classes are to override the fruitType (supposedly with a constant value) and to define their own properties.

[vfouqueron]

@vfouqueron can you please publish your work?

BTW. I have updated the original patch to the v7.x state, though pretty "mechanically", without deep understanding how things have changed since v6.x.

@amorozov Sorry for the late reply, it's available on my repo and on that branch https://github.com/vfouqueron/openapi-generator/tree/release/vf-7.0.x

I've done a bunch of other improvements that I need to push back here. It solved all my issues, but some are really impacting and I have no idea if I've broken something else in another generator.

I should take time to push everything back here.

[olsavmic]

Finishing this would be very much welcome! What's the reason behind closing this PR, please?