OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.82k stars 6.58k forks source link

[BUG][JAVA] discriminator ignored during serialization #12777

Open stevenpost opened 2 years ago

stevenpost commented 2 years ago

Using version 6.0.0

The generated models contain the following annotation: @JsonIgnoreProperties( value = "type", // ignore manually set type, it will be automatically generated by Jackson during serialization allowSetters = true // allows the type to be set during deserialization ) This is problematic in my case, were a single type has multiple values:

@JsonSubTypes({
  @JsonSubTypes.Type(value = AssignedEntitiesTile.class, name = "APPLICATION"),
  @JsonSubTypes.Type(value = FilterableEntityTile.class, name = "APPLICATIONS"),
  @JsonSubTypes.Type(value = Tile.class, name = "APPLICATIONS_MOST_ACTIVE"),
  @JsonSubTypes.Type(value = AssignedEntitiesTile.class, name = "APPLICATION_METHOD")
...
})

When ignoring this field, the first value is used during serialization, so even when manually setting the type to 'APPLICATION_METHOD' serialization fills in 'APPLICATION'.

This is a regression from 5.1.0 which I used previously.

ssternal commented 2 years ago

These lines were added in v6.0.0 with commit c22997b9b8b314e3304afeb182bf846eed91f356:

- @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, property = "{{{discriminator.propertyBaseName}}}", visible = true)
+ @JsonIgnoreProperties(
+   value = "{{{discriminator.propertyBaseName}}}", // ignore manually set {{{discriminator.propertyBaseName}}}, it will be automatically generated by Jackson during serialization
+   allowSetters = true // allows the {{{discriminator.propertyBaseName}}} to be set during deserialization
+ )
+ @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "{{{discriminator.propertyBaseName}}}", visible = true)

I would assume that the property is ignored regardless if it's explicitly defined field or automatically added during serialization.

As a workaround, we modified the typeInfoAnnotation.mustache and removed the @JsonIgnoreProperties annotation.

b3nk4n commented 2 years ago

Hi! We noticed the same problem today when upgrading from 5.1.0 to 6.2.0.

Example:

Our backed defines the following model:

@JsonIgnoreProperties(ignoreUnknown = true)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type")
@JsonSubTypes({
    @JsonSubTypes.Type(value = Impl1.class, name = Impl1.TYPE_NAME),
    @JsonSubTypes.Type(value = Impl2.class, name = Impl2.TYPE_NAME),
})
@Schema(
    discriminatorProperty = "type",
    discriminatorMapping = {
        @DiscriminatorMapping(schema = Impl1.class, value = Impl1.TYPE_NAME),
        @DiscriminatorMapping(schema = Impl2.class, value = Impl2.TYPE_NAME)
    }
)
public abstract class Base {
  // ...
}

public class Impl1 extends Base {
   static final String TYPE_NAME = "impl_1";
  // ...
}

public class Impl2 extends Base {
  static final String TYPE_NAME = "impl_1";
  // ...
}

What caught our attention was that our end-to-end tests have been failing, because the generated Open API client code started to send a wrong request:

{
    "code": 400,
    "message": "Unable to process JSON",
    "details": "Could not resolve type id 'Impl1' as a subtype of `foo.bar.Base`: known type ids = [impl_1, impl_2] (for POJO property 'type')"
}

And when digging into the generated OpenApi code, as mentioned in the messages above, it was due to the following:

@JsonPropertyOrder({
  AbstractRule.JSON_PROPERTY_RULE_TYPE,
  AbstractRule.JSON_PROPERTY_SEVERITY
})
@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen")
@JsonIgnoreProperties(
  value = "type", // ignore manually set type, it will be automatically generated by Jackson during serialization
  allowSetters = true // allows the type to be set during deserialization
)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type", visible = true)
@JsonSubTypes({
  @JsonSubTypes.Type(value = EntityVerificationRule.class, name = "Impl1"),
  @JsonSubTypes.Type(value = HostAvailabilityRule.class, name = "Impl2"),
  @JsonSubTypes.Type(value = EntityVerificationRule.class, name = "impl_1"),
  @JsonSubTypes.Type(value = HostAvailabilityRule.class, name = "impl_2")
})
public class Base {
  // ...
}

Possible workarounds

Possible solution by the Open API team

Thank you! 😇

b3nk4n commented 2 years ago

One further interesting observation I made related to this: The order of the generated @JsonSubTypes.Type seems to be simply alphabetical:

  @JsonSubTypes.Type(value = MyFilterExpression.class, name = "EXPRESSION"),
  @JsonSubTypes.Type(value = MyFilter.class, name = "FILTER"),
  @JsonSubTypes.Type(value = MyFilter.class, name = "MyFilter"),
  @JsonSubTypes.Type(value = MyFilterExpression.class, name = "MyFilterExpression"),

This has the effect that depending on the naming of the class name / type value, that the order might be fine and backward compatible (as in this example above), while it is not compatible in other cases, such as in the example of my message above.

Question: Any chance you could remove this alphabetical ordering? 🙏 That might already solve the problem!

So instead of the above, use the following order as defined in the openapi.yaml: First as defined in the YAML:

  @JsonSubTypes.Type(value = MyFilterExpression.class, name = "EXPRESSION"),
  @JsonSubTypes.Type(value = MyFilter.class, name = "FILTER"),

and then second the redundant ones using the class name:

  @JsonSubTypes.Type(value = MyFilterExpression.class, name = "MyFilterExpression"),
  @JsonSubTypes.Type(value = MyFilter.class, name = "MyFilter"),

Or would anything speak against that?

b3nk4n commented 2 years ago

One more thing I noticed, because it is related and looks fishy to me: These annotations are not just generated in the base class, but also in the implementations. Where they are:

Base class:

@JsonPropertyOrder({
  TagFilterExpressionElement.JSON_PROPERTY_TYPE
})
@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen")
@JsonIgnoreProperties(
  value = "type", // ignore manually set type, it will be automatically generated by Jackson during serialization
  allowSetters = true // allows the type to be set during deserialization
)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type", visible = true)
@JsonSubTypes({
  @JsonSubTypes.Type(value = TagFilterExpression.class, name = "EXPRESSION"),
  @JsonSubTypes.Type(value = TagFilter.class, name = "TAG_FILTER"),
  @JsonSubTypes.Type(value = TagFilter.class, name = "TagFilter"), // <-- we don't need this, or at least ALWAYS listed last/second (as in this example, due to alphabetical order)
  @JsonSubTypes.Type(value = TagFilterExpression.class, name = "TagFilterExpression"),  // <-- we don't need this, or at least ALWAYS listed last/second (as in this example, due to alphabetical order)
})

public class TagFilterExpressionElement {

One of the implementations:

@JsonPropertyOrder({
  TagFilter.JSON_PROPERTY_BOOLEAN_VALUE,
  TagFilter.JSON_PROPERTY_ENTITY,
  TagFilter.JSON_PROPERTY_KEY,
  TagFilter.JSON_PROPERTY_NAME,
  TagFilter.JSON_PROPERTY_NUMBER_VALUE,
  TagFilter.JSON_PROPERTY_OPERATOR,
  TagFilter.JSON_PROPERTY_STRING_VALUE,
  TagFilter.JSON_PROPERTY_VALUE
})
@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen")
@JsonIgnoreProperties(
  value = "type", // ignore manually set type, it will be automatically generated by Jackson during serialization
  allowSetters = true // allows the type to be set during deserialization
)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type", visible = true)
@JsonSubTypes({ // <-- NOTE: only the two expected @JsonSubTypes.Type are generated here! Even though worthless in the sub-class :-/
  @JsonSubTypes.Type(value = TagFilterExpression.class, name = "EXPRESSION"),
  @JsonSubTypes.Type(value = TagFilter.class, name = "TAG_FILTER"),
})

public class TagFilter extends TagFilterExpressionElement {

Is that expected that the sub-class also has these annotations? And is there a way we could have exactly these also in the base class instead?

Thx in advance!

alejo-17 commented 1 year ago

hi @ssternal I am having the same issue: I have one property defined as discriminator and I want to set a different value to an object created from it, but it always returns the class name. How can I apply the workaround you mentioned before?

Class generation:

@JsonIgnoreProperties(
  value = "propertyDiscriminator", // ignore manually set status, it will be automatically generated by Jackson during serialization
  allowSetters = true // allows the status to be set during deserialization
)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "propertyDiscriminator", visible = true)
@JsonSubTypes({
  @JsonSubTypes.Type(value = Class1.class, name = "Class1"),
  @JsonSubTypes.Type(value = Class2.class, name = "Class2"),
  @JsonSubTypes.Type(value = Class3.class, name = "Class3"),
  @JsonSubTypes.Type(value = Class1.class, name = "class1value"),
  @JsonSubTypes.Type(value = Class3.class, name = "class3value"),
  @JsonSubTypes.Type(value = Class2.class, name = "class2value")
})

@Generated(value = "org.openapitools.codegen.languages.SpringCodegen")
public class MainClass implements Serializable {

I want to override the property value "propertyDiscriminator" during serialization but it always return "MainClass".

ssternal commented 1 year ago

@alejo-17 I'm sorry, but my problem was about the discriminator field not being serialized at all, not about changing the content of it. However, for using different values within the discriminator you can use a mapping:

components:
  schemas:
    MySchema:
      type: object
      properties:
        myProperty:
          type: string
        myDiscriminator:
          type: string
          enum: ["TypeA", "TypeB"]
      discriminator:
        propertyName: myDiscriminator
        mapping:
          TypeA: '#/components/schemas/MySchemaA'
          TypeB: '#/components/schemas/MySchemaB'
    MySchemaA:
      type: object
      allOf:
        - $ref: '#/components/schemas/MySchema'
        - type: object
          properties:
            anotherProp:
              type: string
    MySchemaB:
      type: object
      allOf:
        - $ref: '#/components/schemas/MySchema'
        - type: object
          properties:
            yetAnotherProp:
              type: string

As this is off topic to the problem discussed here, please don't use this issue for further answers regarding your question.

Dalganjan commented 1 year ago

The issue exists for 6.2.1 as well. We created an open API spec with 3.0.3 specification, and the generated model has an issue with mapping the discriminator. Added up the comment, so as to raise concern on the OPEN bugs in the current release as well.

If in any way, do we have a fix on this issue, when the generatedSource is spring?

trixprod commented 1 year ago

Hello, still no updates on this ?

robbertvanwaveren commented 1 year ago

@b3nk4n I applied your suggestion in my PR.

johnfburnett commented 1 year ago

Hello, are there any updates on this bug yet? I tested this with the latest openapi-generator 6.6.0 from May 11 2023 and it still does not appear to be fixed.

I am also using the workaround described above using the typeInfoAnnotation.mustache file with the JsonIgnore removed.

Without the workaround, once I add the discriminator in the base object the @JasonIgnoreProperties attribute is added in the generated Dto, which makes it no longer possible to differentiate subtypes using the value of the differentiator.

Here my example api:_

# animals
BaseAnimal:
  discriminator: 
    propertyName: animalType
  properties:
    name:
      type: string
      description: name of animal
      example: Lucy
    animalType:
      type: string
      enum: ["CAT","FISH"]
      description: Type of animal

MyCat:
  allOf:
    - $ref: '#/components/schemas/BaseAnimal'
    - type: object
      properties:
        preferredMouseType:
          type: string

MyFish:
  allOf:
    - $ref: '#/components/schemas/BaseAnimal'
    - type: object
      properties:
        favoriteSwimmingPosition:
          type: string

`

Here is what is generated:_

`` /**

@JsonIgnoreProperties( value = "animalType", // ignore manually set animalType, it will be automatically generated by Jackson during serialization allowSetters = true // allows the animalType to be set during deserialization ) @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "animalType", visible = true) @JsonSubTypes({ @JsonSubTypes.Type(value = CatDto.class, name = "CatDto"), @JsonSubTypes.Type(value = FishDto.class, name = "FishDto") })

@JsonTypeName("BaseAnimal") @Generated(value = "org.openapitools.codegen.languages.SpringCodegen", date = "2023-07-03T15:49:38.421804200+02:00[Europe/Paris]") public class BaseAnimalDto {

@JsonProperty("name") private String name; ... /**

@JsonTypeName("Fish") @Generated(value = "org.openapitools.codegen.languages.SpringCodegen", date = "2023-07-03T17:27:12.649840100+02:00[Europe/Paris]") public class FishDto extends BaseAnimalDto {

private String favoriteSwimmingPosition;

public FishDto favoriteSwimmingPosition(String favoriteSwimmingPosition) { this.favoriteSwimmingPosition = favoriteSwimmingPosition; return this; }

/**

`` Note that FishDto correctly extends from BaseAnimalDto, but the animal type is ignored, This is what I am doing with the Dtos: BaseAnimalDto fish = new FishDto(); fish.setName("Wanda"); fish.setAnimalType=BaseAnimalDto.AnimalTypeEnum.FISH; fish.setPreferredSwimingPosition="back stroke";

IS CASE with version 6.6.0 when I receive the deserialized object I get: { "MyFish", (The json type name of the fish - incorrect) "Wanda", "back stroke" }

SHOULD BE what I need is (as is the case with version 5.1.x which I used previously): { "FISH", (The animalType differentiator field value - in my case an enum value) "Wanda", "back stroke" }

Can somebody create a fix for this or explain how I can achieve the desired result in a different way?_

anatoliy-balakirev commented 1 year ago

Hi. Any news on this? Maybe there could be some flag introduced, controlling whether this annotation should be added or not? I understand that the combination of those two annotations, which are in place now, are supposed to make it more reliable: the type is derived by the serializer itself and there is no way to mistakenly put the wrong one, but somehow it does not work with the latest Spring Boot (3.1.1) / Jackson dependencies (haven't tested with other versions though). Even more weird: it works in the unit test, which I just introduced exactly for that, but it does not work in the running application somehow. I already spent hours trying to figure out the reason (e.g. potential conflict between Jackson and Jersey, which are both on the classpath, etc.), but wasn't able to find anything so far.

johnfburnett commented 1 year ago

@rpost I noticed you pushed the commit https://github.com/OpenAPITools/openapi-generator/commit/c22997b9b8b314e3304afeb182bf846eed91f356 - could you please help here?

wing328 commented 1 year ago

Can someone please share a spec to reproduce the issue? I tested with the one in https://github.com/OpenAPITools/openapi-generator/issues/12777#issuecomment-1618731277 but the output remains the same with https://github.com/OpenAPITools/openapi-generator/pull/15284

stevenpost commented 1 year ago

Original bug submitter here. The exact file I used when reporting this bug can be found on the following URL: https://culsans-public.s3-eu-west-1.amazonaws.com/dynatrace-config-v1.json

It is rather large (1.3 MB), so I'm not going to inline it here.

Qkyrie commented 8 months ago

any update on this? We're also running into this issue and would like a solution.

jbeullen commented 8 months ago

Can this be expedited?

Following MR introduced a limitation in the OpenAPI specification. The following snippet is valid:

"discriminator": {
  "propertyName": "species",
  "mapping": {
    "Dog": "#/components/schemas/Dog",
    "Cat": "#/components/schemas/Cat",
    "Tiger": "#/components/schemas/Cat"
   }
}

When unmarshalling a Cat Object with species "Tiger", it will generate JSON with species "Cat".

The OpenAPI specification allows for n species to be mapped to 1 component type. This MR has now put the following limitation on the OpenAPI specification, forcing the relation to 1 to 1. This will break all existing contracts that use n to 1 mapping.

@wing328 Can this be reverted/fixed asap? The generator should at all time respect the specification.

jbeullen commented 8 months ago

@wing328 FYI This project contains a valid spec file and a unit test that reproduces the issue. https://github.com/jbeullen/open-api-generator-issue

jbeullen commented 8 months ago

@wing328 Any news on this?

I found a discussion on Open API stating that Many-To-One is allowed in de specification. https://github.com/OAI/OpenAPI-Specification/discussions/3283

Hopes this gives new insight into the severity of this issue.

An elegant solution could be to add a flag to the generator (e.g enable-many-to-one-discriminator-mappings), that falls back to the old style of generation, in order not to break existing clients.

danlz commented 6 months ago

As a workaround you can override the typeInfoAnnotation.mustache template.