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

[BUG][KOTLIN-SPRING] Constructor call generated for interface supertypes if using inheritance/discriminator #8366

Open
5 of 6 tasks
notizklotz opened this issue Jan 7, 2021 · 5 comments · May be fixed by #17008
Open
5 of 6 tasks

Comments

@notizklotz
Copy link

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

If the parent of a type is generated as interface the subtype is generated with a constructor call to the parent interface which doesn't compile:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type", visible = true)
@JsonSubTypes(
      JsonSubTypes.Type(value = SubtypeA::class, name = "subtypeA"),
      JsonSubTypes.Type(value = SubtypeB::class, name = "subtypeB")
)
interface ParentSchema{
        val id: kotlin.String
        val type: DiscriminatingType
}

data class SubtypeA(

    @get:NotNull  
    @field:JsonProperty("id") override val id: kotlin.String,

    @get:NotNull  
    @field:Valid
    @field:JsonProperty("type") override val type: DiscriminatingType,

    @field:JsonProperty("subtypeAproperty") val subtypeAproperty: kotlin.Int? = null
) : ParentSchema(){
  
}

Note: ParentSchema(). The () prevent compilation

openapi-generator version

openapi-generator-maven-plugin:5.0.0

OpenAPI declaration file content or url

Sample spec

Related issues/PRs

Similar: #3587

Suggest a fix

I worked around it by removing the () from dataClass.mustache. This wouldn't work if the parent is a class.

@ena1106
Copy link

ena1106 commented Aug 10, 2021

Thank you for the workaround! 🥳
If others are using openapi-generator-maven-plugin:

  • copy dataClass.mustache and remove ().
  • In the project root add a folder called openapi-generator-templates with the corrected mustache template.
  • In the plugin configuration (NOT configOptions) add: <templateDirectory>${project.basedir}/openapi-generator-templates</templateDirectory>

@nicolaes
Copy link

nicolaes commented Aug 3, 2022

This is fixed in 5.4.0. See #11166 and #8687.

@petermetz
Copy link
Contributor

The problem with the fix is that it doesn't seem to work for when "additionalProperties": true makes it so that the parent class is kotlin.collections.HashMap<String, kotlin.Any> which does need the () in order to compile.

openapi.json

{
  "openapi": "3.0.3",
  "info": {
    "title": "Hyperledger Cactus Plugin - Connector Corda",
    "description": "Can perform basic tasks on a Corda ledger",
    "version": "v2.0.0-alpha.2",
    "license": {
      "name": "Apache-2.0",
      "url": "https://www.apache.org/licenses/LICENSE-2.0.html"
    }
  },
  "paths": {},
  "components": {
    "schemas": {
      "JarFile": {
        "type": "object",
        "required": ["filename", "contentBase64", "hasDbMigrations"],
        "additionalProperties": true,
        "properties": {
          "filename": {
            "type": "string",
            "nullable": false,
            "minLength": 1,
            "maxLength": 255
          },
          "hasDbMigrations": {
            "description": "Indicates whether the cordapp jar in question contains any embedded migrations that Cactus can/should execute between copying the jar into the cordapp directory and starting the node back up.",
            "type": "boolean",
            "nullable": false
          },
          "contentBase64": {
            "type": "string",
            "format": "base64",
            "nullable": false,
            "minLength": 1,
            "maxLength": 1073741824
          }
        }
      }
    }
  }
}

JarFile.kt

package org.hyperledger.cactus.plugin.ledger.connector.corda.server.model

import java.util.Objects
import com.fasterxml.jackson.annotation.JsonProperty
import javax.validation.constraints.DecimalMax
import javax.validation.constraints.DecimalMin
import javax.validation.constraints.Email
import javax.validation.constraints.Max
import javax.validation.constraints.Min
import javax.validation.constraints.NotNull
import javax.validation.constraints.Pattern
import javax.validation.constraints.Size
import javax.validation.Valid
import io.swagger.v3.oas.annotations.media.Schema

/**
 * 
 * @param filename 
 * @param hasDbMigrations Indicates whether the cordapp jar in question contains any embedded migrations that Cactus can/should execute between copying the jar into the cordapp directory and starting the node back up.
 * @param contentBase64 
 */
data class JarFile(

    @get:Size(min=1,max=255)
    @Schema(example = "null", required = true, description = "")
    @get:JsonProperty("filename", required = true) val filename: kotlin.String,

    @Schema(example = "null", required = true, description = "Indicates whether the cordapp jar in question contains any embedded migrations that Cactus can/should execute between copying the jar into the cordapp directory and starting the node back up.")
    @get:JsonProperty("hasDbMigrations", required = true) val hasDbMigrations: kotlin.Boolean,

    @get:Size(min=1,max=1073741824)
    @Schema(example = "null", required = true, description = "")
    @get:JsonProperty("contentBase64", required = true) val contentBase64: kotlin.String
) : kotlin.collections.HashMap<String, kotlin.Any>{

}

Resulting compiler error

/kotlin-spring/src/main/kotlin/org/hyperledger/cactus/plugin/ledger/connector/corda/server/model/JarFile.kt: (34, 5): This type has a constructor, and thus must be initialized here


I'm not an expert in the generator's internals. My naive idea for a more robust solution is to have an additional context variable added by the generator before the template rendering happens. Something that could be called parentCtorCallNeeded or similar which then would allow us to update dataClass.mustache to use that boolean to render () or not with 100% certainty. I'm assuming here that the generator internally should be able to figure out whether the parent class is an interface or not, but this also might be a non-trivial thing to determine, I wouldn't know.

petermetz added a commit to petermetz/openapi-generator that referenced this issue Nov 8, 2023
The new parentCtorCallNeeded property can be used by the template to
determine if there should be a constructor invocation on the parent type.

Right now its value is computed in a way that could probably use some
improvement because it is language specifically checking if the base type
is a kotlin map or not.
This might seem overly brittle, but was the only way I could differentiate
between the two different edge cases that are present in the repro spec.

This was the previous commit that changed the default behavior from one
edge case to the other (both are wrong because we can only know for sure
at generation time whether a constructor call will be necessary or not).

OpenAPITools@0bb08a7

Below is a yaml OpenAPI spec document that can be used to test/verify
the currently known edge cases (which were also mentioned in the GH
issue tracker's comments)

```yaml
openapi: 3.0.3
info:
  version: "1.0.0"
  title: ""
paths:
  /documents/v1:
    get:
      responses:
        200:
          description: lorem
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: "#/components/schemas/ParentSchema"
components:
  schemas:
    JarFiles:
      type: array
      items:
        $ref: "#/components/schemas/JarFile"
    JarFile:
      type: object
      required:
      - filename
      - contentBase64
      - hasDbMigrations
      additionalProperties: true
      properties:
        filename:
          type: string
          nullable: false
          minLength: 1
          maxLength: 255
        hasDbMigrations:
          description: Indicates whether the cordapp jar in question contains any
            embedded migrations that Cactus can/should execute between copying the
            jar into the cordapp directory and starting the node back up.
          type: boolean
          nullable: false
        contentBase64:
          type: string
          format: base64
          nullable: false
          minLength: 1
          maxLength: 1073741824

    ParentSchema:
      type: object
      properties:
        id:
          type: string
        type:
          $ref: "#/components/schemas/DiscriminatingType"
      required:
        - id
        - type
      discriminator:
        propertyName: type
        mapping:
          subtypeA: "#/components/schemas/SubtypeA"
          subtypeB: "#/components/schemas/SubtypeB"
          subtypeC: "#/components/schemas/SubtypeC"
    SubtypeA:
      type: object
      allOf:
        - $ref: '#/components/schemas/ParentSchema'
        - type: object
          properties:
            subtypeAproperty:
              type: integer
    SubtypeB:
      type: object
      allOf:
        - $ref: '#/components/schemas/ParentSchema'
        - type: object
          properties:
            subtypeBproperty:
              type: string
    SubtypeC:
      type: object
      additionalProperties: true
      allOf:
        - $ref: '#/components/schemas/ParentSchema'
        - type: object
          properties:
            subtypeAproperty:
              type: integer

    DiscriminatingType:
      type: string
      enum:
        - subtypeA
        - subtypeB
        - subtypeC
```

Fixes OpenAPITools#8366

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz added a commit to petermetz/cacti that referenced this issue Nov 11, 2023
…ler errors

1. Started overriding a specific template (dataClass.mustache) in the
main-server sub-package of the corda connector because of issues that
are further explained here [1] and here [2].
2. Also had to update generator configuration to specifically exclude
spring-doc generation because it seems to be broken within the template
as well: it does not provide updated dependencies for the grandle and
maven manifests and so the `io.swagger.core.v3:swagger-annotations` package
was missing and failing the build in a second way.
3. The example value for the return array of `ListFlowsV1Response` in
the openapi.json spec file of the corda connector was containing dollar
signs ($) which ended up being appended to the kotlin code's annotations
as documentation, but the dollar signs have a special meaning in kotlin
and lead to syntax errors. Updating the examples to not have dollar signs
in the openapi.json specification document resulted in fixing this issue.
4. Also updated the artifact version in the openapi generator configuration
file. This is just a temporary fix, what we really need is scripts bumping
this up as part of the automated release process.

[1] OpenAPITools/openapi-generator#8366 (comment)
[2] OpenAPITools/openapi-generator#17008

Fixes hyperledger-cacti#2662

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz added a commit to petermetz/cacti that referenced this issue Nov 12, 2023
…ler errors

1. Started overriding a specific template (dataClass.mustache) in the
main-server sub-package of the corda connector because of issues that
are further explained here [1] and here [2].
2. Also had to update generator configuration to specifically exclude
spring-doc generation because it seems to be broken within the template
as well: it does not provide updated dependencies for the grandle and
maven manifests and so the `io.swagger.core.v3:swagger-annotations` package
was missing and failing the build in a second way.
3. The example value for the return array of `ListFlowsV1Response` in
the openapi.json spec file of the corda connector was containing dollar
signs ($) which ended up being appended to the kotlin code's annotations
as documentation, but the dollar signs have a special meaning in kotlin
and lead to syntax errors. Updating the examples to not have dollar signs
in the openapi.json specification document resulted in fixing this issue.
4. Also updated the artifact version in the openapi generator configuration
file. This is just a temporary fix, what we really need is scripts bumping
this up as part of the automated release process.

[1] OpenAPITools/openapi-generator#8366 (comment)
[2] OpenAPITools/openapi-generator#17008

Fixes hyperledger-cacti#2662

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz added a commit to petermetz/cacti that referenced this issue Nov 14, 2023
…ler errors

1. Started overriding a specific template (dataClass.mustache) in the
main-server sub-package of the corda connector because of issues that
are further explained here [1] and here [2].
2. Also had to update generator configuration to specifically exclude
spring-doc generation because it seems to be broken within the template
as well: it does not provide updated dependencies for the grandle and
maven manifests and so the `io.swagger.core.v3:swagger-annotations` package
was missing and failing the build in a second way.
3. The example value for the return array of `ListFlowsV1Response` in
the openapi.json spec file of the corda connector was containing dollar
signs ($) which ended up being appended to the kotlin code's annotations
as documentation, but the dollar signs have a special meaning in kotlin
and lead to syntax errors. Updating the examples to not have dollar signs
in the openapi.json specification document resulted in fixing this issue.
4. Also updated the artifact version in the openapi generator configuration
file. This is just a temporary fix, what we really need is scripts bumping
this up as part of the automated release process.

[1] OpenAPITools/openapi-generator#8366 (comment)
[2] OpenAPITools/openapi-generator#17008

Fixes hyperledger-cacti#2662

Signed-off-by: Peter Somogyvari <[email protected]>
(cherry picked from commit 99ec2ff)
petermetz added a commit to petermetz/cacti that referenced this issue Nov 14, 2023
…ler errors

1. Started overriding a specific template (dataClass.mustache) in the
main-server sub-package of the corda connector because of issues that
are further explained here [1] and here [2].
2. Also had to update generator configuration to specifically exclude
spring-doc generation because it seems to be broken within the template
as well: it does not provide updated dependencies for the grandle and
maven manifests and so the `io.swagger.core.v3:swagger-annotations` package
was missing and failing the build in a second way.
3. The example value for the return array of `ListFlowsV1Response` in
the openapi.json spec file of the corda connector was containing dollar
signs ($) which ended up being appended to the kotlin code's annotations
as documentation, but the dollar signs have a special meaning in kotlin
and lead to syntax errors. Updating the examples to not have dollar signs
in the openapi.json specification document resulted in fixing this issue.
4. Also updated the artifact version in the openapi generator configuration
file. This is just a temporary fix, what we really need is scripts bumping
this up as part of the automated release process.

[1] OpenAPITools/openapi-generator#8366 (comment)
[2] OpenAPITools/openapi-generator#17008

Fixes hyperledger-cacti#2662

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz added a commit to hyperledger-cacti/cacti that referenced this issue Nov 14, 2023
…ler errors

1. Started overriding a specific template (dataClass.mustache) in the
main-server sub-package of the corda connector because of issues that
are further explained here [1] and here [2].
2. Also had to update generator configuration to specifically exclude
spring-doc generation because it seems to be broken within the template
as well: it does not provide updated dependencies for the grandle and
maven manifests and so the `io.swagger.core.v3:swagger-annotations` package
was missing and failing the build in a second way.
3. The example value for the return array of `ListFlowsV1Response` in
the openapi.json spec file of the corda connector was containing dollar
signs ($) which ended up being appended to the kotlin code's annotations
as documentation, but the dollar signs have a special meaning in kotlin
and lead to syntax errors. Updating the examples to not have dollar signs
in the openapi.json specification document resulted in fixing this issue.
4. Also updated the artifact version in the openapi generator configuration
file. This is just a temporary fix, what we really need is scripts bumping
this up as part of the automated release process.

[1] OpenAPITools/openapi-generator#8366 (comment)
[2] OpenAPITools/openapi-generator#17008

Fixes #2662

Signed-off-by: Peter Somogyvari <[email protected]>
sandeepnRES pushed a commit to sandeepnRES/cacti that referenced this issue Dec 21, 2023
…ler errors

1. Started overriding a specific template (dataClass.mustache) in the
main-server sub-package of the corda connector because of issues that
are further explained here [1] and here [2].
2. Also had to update generator configuration to specifically exclude
spring-doc generation because it seems to be broken within the template
as well: it does not provide updated dependencies for the grandle and
maven manifests and so the `io.swagger.core.v3:swagger-annotations` package
was missing and failing the build in a second way.
3. The example value for the return array of `ListFlowsV1Response` in
the openapi.json spec file of the corda connector was containing dollar
signs ($) which ended up being appended to the kotlin code's annotations
as documentation, but the dollar signs have a special meaning in kotlin
and lead to syntax errors. Updating the examples to not have dollar signs
in the openapi.json specification document resulted in fixing this issue.
4. Also updated the artifact version in the openapi generator configuration
file. This is just a temporary fix, what we really need is scripts bumping
this up as part of the automated release process.

[1] OpenAPITools/openapi-generator#8366 (comment)
[2] OpenAPITools/openapi-generator#17008

Fixes hyperledger-cacti#2662

Signed-off-by: Peter Somogyvari <[email protected]>
@timurgen
Copy link

timurgen commented Sep 3, 2024

This is still an issue with incorrectly generated constructor , would it be fixed someday?

@petermetz
Copy link
Contributor

petermetz commented Sep 4, 2024

@timurgen Sorry, I'm struggling with workload. I'll get around to it eventually but not sure when I'll have time to update the PR. If you'd like I can invite you as a collaborator to my fork and you can take it all the way based on the most recent feedback (which was reasonable change requests IMO I just don't have the time right now).

Oh wait this is a different PR that is not mine, nvm. :-)

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

Successfully merging a pull request may close this issue.

5 participants