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

Nullable "$refs" in oneOfs with no other options create unnecessary interfaces that break both Java/Kotlin Models at Runtime #1679

Closed
scprek opened this issue Aug 7, 2024 · 10 comments · Fixed by #1732

Comments

@scprek
Copy link
Contributor

scprek commented Aug 7, 2024

Expected Behavior

Building off #1667

No interface? just the exact DTO as nullable

openapi-generator-cli generate -i test.yaml -g kotlin -o output/directory -p serializationLibrary=jackson,generateOneOfAnyOfWrappers=true,library=jvm-ktor

Upstream creates an interface still

ata class FavorDTOMerchant(var actualInstance: Any? = null) {

    class CustomTypeAdapterFactory : TypeAdapterFactory {
        override fun <T> create(gson: Gson, type: TypeToken<T>): TypeAdapter<T>? {
            if (!FavorDTOMerchant::class.java.isAssignableFrom(type.rawType)) {
                return null // this class only serializes 'FavorDTOMerchant' and its subtypes
            }
            val elementAdapter = gson.getAdapter(JsonElement::class.java)
            val adapterMerchantDTO = gson.getDelegateAdapter(this, TypeToken.get(MerchantDTO::class.java))

            @Suppress("UNCHECKED_CAST")
            return object : TypeAdapter<FavorDTOMerchant?>() {
                @Throws(IOException::class)
                override fun write(out: JsonWriter,value: FavorDTOMerchant?) {
                    if (value?.actualInstance == null) {
                        elementAdapter.write(out, null)
                        return
                    }

                    // check if the actual instance is of the type `MerchantDTO`
                    if (value.actualInstance is MerchantDTO) {
                        val element = adapterMerchantDTO.toJsonTree(value.actualInstance as MerchantDTO?)
                        elementAdapter.write(out, element)
                        return
                    }
                    throw IOException("Failed to serialize as the type doesn't match oneOf schemas: MerchantDTO")
                }

                @Throws(IOException::class)
                override fun read(jsonReader: JsonReader): FavorDTOMerchant {
                    val jsonElement = elementAdapter.read(jsonReader)
                    var match = 0
                    val errorMessages = ArrayList<String>()
                    var actualAdapter: TypeAdapter<*> = elementAdapter

                    // deserialize MerchantDTO
                    try {
                        // validate the JSON object to see if any exception is thrown
                        MerchantDTO.validateJsonElement(jsonElement)
                        actualAdapter = adapterMerchantDTO
                        match++
                        //log.log(Level.FINER, "Input data matches schema 'MerchantDTO'")
                    } catch (e: Exception) {
                        // deserialization failed, continue
                        errorMessages.add(String.format("Deserialization for MerchantDTO failed with `%s`.", e.message))
                        //log.log(Level.FINER, "Input data does not match schema 'MerchantDTO'", e)
                    }

                    if (match == 1) {
                        val ret = FavorDTOMerchant()
                        ret.actualInstance = actualAdapter.fromJsonTree(jsonElement)
                        return ret
                    }

                    throw IOException(String.format("Failed deserialization for FavorDTOMerchant: %d classes match result, expected 1. Detailed failure message for oneOf schemas: %s. JSON: %s", match, errorMessages, jsonElement.toString()))
                }
            }.nullSafe() as TypeAdapter<T>
        }
    }
}

And MerchantDTO does not implement the interface

data class MerchantDTO (

    @field:JsonProperty("id")
    val id: kotlin.Any? = null,

    @field:JsonProperty("name")
    val name: kotlin.Any? = null
) {


}

Actual Behaviour

Creates interface with Top level Schema prefix (FavorDTO)

package com.favordelivery.coreapi.model

@Generated("io.micronaut.openapi.generator.KotlinMicronautClientCodegen")
interface FavorDTOMerchant {

}

And in the FavorDTO it has this as the field instead of MerchantDTO directly

    @field:Nullable
    @field:Valid
    @field:JsonProperty(JSON_PROPERTY_MERCHANT)
    @field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
    var merchant: FavorDTOMerchant? = null,

Runtime error

Client 'core-api': Error decoding HTTP response body: Error decoding JSON stream for type [favorDtoResponse]: Cannot construct instance of `com.favordelivery.coreapi.model.FavorDTOMerchant` (no Creators, like default constructor, exist): abstract types either need to be mapped to concrete types, have custom deserializer, or contain additional type information
io.micronaut.http.client.exceptions.HttpClientResponseException: Client 'core-api': Error decoding HTTP response body: Error decoding JSON stream for type [favorDtoResponse]: Cannot construct instance of `com.favordelivery.coreapi.model.FavorDTOMerchant` (no Creators, like default constructor, exist): abstract types either need to be mapped to concrete types, have custom deserializer, or contain additional type information
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 2515] (through reference chain: com.favordelivery.coreapi.model.FavorDtoResponse["favor"]->com.favordelivery.coreapi.model.FavorDTO["merchant"])

Steps To Reproduce

components:
  schemas:
    FavorDTO:
      properties:
        merchant:
          nullable: true
          oneOf:
            - $ref: '#/components/schemas/MerchantDTO'
    MerchantDTO:
      description: 'Class MerchantDTO'
      properties:
        id: {}
        name: {}

Environment Information

No response

Example Application

No response

Version

4.5.1

@altro3
Copy link
Collaborator

altro3 commented Aug 8, 2024

Hm... I checked it with 6.12.0-SNAPSHOT and can't reprodcue it. My swagger file:

openapi: 3.0.0
info:
  title: 'Order API'
  version: v6
servers:
  -
    url: 'https://{environment}.com/api/{apiVersion}'
    variables:
      environment:
        enum:
          - api-dev.test.com
        default: api.dev.test.com
      apiVersion:
        enum:
          - v1
          - v2
          - v3
          - v4
          - v5
          - v6
          - v7
        default: v6
paths:
  '/orders/{id}':
    get:
      tags:
        - Order
      operationId: getOrderById
      parameters:
        -
          $ref: '#/components/parameters/OrderId'
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/FavorDTO'
components:
  parameters:
    OrderId:
      name: id
      in: path
      description: 'The ID'
      required: true
      schema:
        type: string
  schemas:
    FavorDTO:
      properties:
        merchant:
          nullable: true
          oneOf:
            - $ref: '#/components/schemas/MerchantDTO'
    MerchantDTO:
      description: 'Class MerchantDTO'
      properties:
        id: {}
        name: {}

Generated 3 classes:

FavorDTO:

/**
 * FavorDTO
 *
 * @param merchant
 */
@Serdeable
@JsonPropertyOrder(
        FavorDTO.JSON_PROPERTY_MERCHANT
)
@Generated("io.micronaut.openapi.generator.KotlinMicronautClientCodegen")
data class FavorDTO(
    @field:Nullable
    @field:Valid
    @field:JsonProperty(JSON_PROPERTY_MERCHANT)
    @field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
    var merchant: FavorDTOMerchant? = null,
) {

    companion object {

        const val JSON_PROPERTY_MERCHANT = "merchant"
    }
}

Interface FavorDTOMerchant for oneof:

@Generated("io.micronaut.openapi.generator.KotlinMicronautClientCodegen")
interface FavorDTOMerchant {

}

MerchantDTO:

/**
 * Class MerchantDTO
 *
 * @param id
 * @param name
 */
@Serdeable
@JsonPropertyOrder(
        MerchantDTO.JSON_PROPERTY_ID,
        MerchantDTO.JSON_PROPERTY_NAME
)
@Generated("io.micronaut.openapi.generator.KotlinMicronautClientCodegen")
data class MerchantDTO(
    @field:Nullable
    @field:JsonProperty(JSON_PROPERTY_ID)
    @field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
    var id: Any? = null,
    @field:Nullable
    @field:JsonProperty(JSON_PROPERTY_NAME)
    @field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
    var name: Any? = null,
): FavorDTOMerchant {

    companion object {

        const val JSON_PROPERTY_ID = "id"
        const val JSON_PROPERTY_NAME = "name"
    }
}

@scprek
Copy link
Contributor Author

scprek commented Aug 8, 2024

Ok. What was the test you ran? I simply ran the http request and it failed to deserialize the response

@altro3
Copy link
Collaborator

altro3 commented Aug 8, 2024

You wrote that the error is that the FavorDTOMerchant interface is being created, the merchant field is of type FavorDTOMerchant , but here’s the problem, the MerchantDTO class does not implement the MerchantDTO interface.

If this is not the problem, then please make a small reproducer project so that I can understand what the error is

@altro3
Copy link
Collaborator

altro3 commented Aug 8, 2024

Yes, now I understand what the problem is. It turns out that the interface cannot be used without a discriminator if you have not declared a custom binder for the object... Then it’s clear why the standard generator creates a CustomTypeAdapterFactory... hmm, we need to finish this solution.... Ok, I’ll see how the standard generator works and based on it I will complete our solution

@altro3
Copy link
Collaborator

altro3 commented Aug 8, 2024

In general, I see that fixing your situation is not so easy, because the model on which you are trying to generate code is crooked.

I have a request to you, try to generate code with standard java and kotlin generators and, if you get a working code, then show me what should be generated as a result for everything to work. Because I tried this and got non-working code in all three cases. But I need to understand what needs to be generated for it to work correctly.

@scprek
Copy link
Contributor Author

scprek commented Aug 8, 2024

Yeah I agree it's weird. I'll first open an issue on the swagger php library and ask why they do nullable refs that way. I don't need them to be nullable personally so having an option to just allow them to be "not required" would suffice.

https://stackoverflow.com/a/48114924

OAI/OpenAPI-Specification#1368

@altro3
Copy link
Collaborator

altro3 commented Aug 26, 2024

@scprek I think, I fixed all problems with oneOf here: #1732

Now useOneOfInterfaces property is true by default - that's why you see generated interface. But with nex gradle plugin version you will have ability to turn off this feature and with useOneOfInterfaces=false you will see what you want

@scprek
Copy link
Contributor Author

scprek commented Aug 26, 2024

Ok, so I tried it with the plugin snapshot version so I could set useOneOfInterfaces.set(false) and it look

Slightly different example from above, but same concept just different field. So I have a nullable ref generated by the Swagger PHP library.

          "customer_contact_preference": {
            "oneOf": [
              {
                "$ref": "#/components/schemas/CustomerContactPreferenceDTO"
              }
            ],
            "nullable": true
          },

components

      "CustomerContactPreferenceDTO": {
        "properties": {
          "contact_type": {
            "type": "string"
          },
          "message": {
            "type": "string"
          },
          "icon_url": {
            "type": "string"
          }
        },
        "type": "object"
      },

Then Generated Kotlin

    @field:Nullable
    @field:Valid
    @field:JsonProperty(JSON_PROPERTY_CUSTOMER_CONTACT_PREFERENCE)
    @field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
    var customerContactPreference: FavorDTOCustomerContactPreference? = null,

Follow up question on difference between upstream kotlin generator and micronaut. It appears micronaut's is making a super data class with all params and then it has the JsonTypeInfo, JsonSubTypes right?

But in the upstream they don't have any fields in the data class and just a TypeAdapterFactory. Just want to confirm my understanding.

Micronaut OpenAPI

@JsonIgnoreProperties(
        value = ["version"], // ignore manually set version, it will be automatically generated by Jackson during serialization
        allowSetters = true // allows the version to be set during deserialization
)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "version", visible = true)
@JsonSubTypes(
        JsonSubTypes.Type(value = CancellationReasonTypesV1::class, name = "-1"),
        JsonSubTypes.Type(value = CancellationReasonTypesV2::class, name = "0"),
        JsonSubTypes.Type(value = CancellationReasonTypesV2::class, name = "1"),
        JsonSubTypes.Type(value = CancellationReasonTypesV2::class, name = "2"),
        JsonSubTypes.Type(value = CancellationReasonTypesV2::class, name = "3"),
        JsonSubTypes.Type(value = CancellationReasonTypesV3::class, name = "4")
)
data class CancellationReasonTypesDTO(
    @field:Nullable
    @field:JsonProperty(JSON_PROPERTY_REASONS)
    @field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
    var reasons: List<@Valid CancellationTypeDto>? = null,
    @field:Nullable
    @field:JsonProperty(JSON_PROPERTY_TITLE)
    @field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
    var title: String? = null,
    @field:Nullable
    @field:JsonProperty(JSON_PROPERTY_COMMENTS_FIELD_PROMPT)
    @field:JsonInclude(JsonInclude.Include.USE_DEFAULTS)
    var commentsFieldPrompt: String? = null,

Kotlin Generator

-g kotlin -o output/directory -p serializationLibrary=jackson,generateOneOfAnyOfWrappers=false,library=jvm-ktor

 */
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "version", visible = true)
@JsonSubTypes(
    JsonSubTypes.Type(value = CancellationReasonTypesV1::class, name = "-1"),
    JsonSubTypes.Type(value = CancellationReasonTypesV2::class, name = "0"),
    JsonSubTypes.Type(value = CancellationReasonTypesV2::class, name = "1"),
    JsonSubTypes.Type(value = CancellationReasonTypesV2::class, name = "2"),
    JsonSubTypes.Type(value = CancellationReasonTypesV2::class, name = "3"),
    JsonSubTypes.Type(value = CancellationReasonTypesV3::class, name = "4")
)

data class CancellationReasonTypesDTO(var actualInstance: Any? = null) {

    class CustomTypeAdapterFactory : TypeAdapterFactory {
        override fun <T> create(gson: Gson, type: TypeToken<T>): TypeAdapter<T>? {
            if (!CancellationReasonTypesDTO::class.java.isAssignableFrom(type.rawType)) {
                return null // this class only serializes 'CancellationReasonTypesDTO' and its subtypes
            }
            val elementAdapter = gson.getAdapter(JsonElement::class.java)
            val adapterCancellationReasonTypesV1 = gson.getDelegateAdapter(this, TypeToken.get(CancellationReasonTypesV1::class.java))
            val adapterCancellationReasonTypesV2 = gson.getDelegateAdapter(this, TypeToken.get(CancellationReasonTypesV2::class.java))
            val adapterCancellationReasonTypesV3 = gson.getDelegateAdapter(this, TypeToken.get(CancellationReasonTypesV3::class.java))
            

@altro3
Copy link
Collaborator

altro3 commented Aug 26, 2024

Sorry, but I don't understand from your comment what's wrong. You give the example of the Kotlin generator, but the code it generates looks like a crutch. Here, the capabilities of another library are used - gson, but we use jackson.

So can you clearly write to me - what code was generated, and what code should be generated. You shouldn't compare our generator with the Kotlin generator, because we have completely different templates for code generation.

I checked what was generated by our generator - I did not see any problems with the code. So I need a specific description of the problem

@scprek
Copy link
Contributor Author

scprek commented Aug 26, 2024

Ah never mind . I specified Jackson in the options to generate but looks like it still used gson there.

No issues.

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