openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG][JAVA] discriminator ignored during serialization

Open stevenpost opened this issue 3 years ago • 19 comments

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.

stevenpost avatar Jul 06 '22 09:07 stevenpost

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.

ssternal avatar Oct 07 '22 13:10 ssternal

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

  • what was mentioned above:
  • Make the backend agnostic to both types, similar to the generated OpenAPI code
    @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),
        @JsonSubTypes.Type(value = Impl1.class, name ="Impl1"), // <-- added to be agnostic
        @JsonSubTypes.Type(value = Impl2.class, name = "Impl2"),  // <-- added to be agnostic
    })
    @Schema(
        discriminatorProperty = "type",
        discriminatorMapping = {
            @DiscriminatorMapping(schema = Impl1.class, value = Impl1.TYPE_NAME),
            @DiscriminatorMapping(schema = Impl2.class, value = Impl2.TYPE_NAME)
        }
    )
    public abstract class Base {
      // ...
    }
    
    • However, doing so has at least the following disadvantages: - No fun to apply this change in a code base with >100 of such polymorphic models! - Comes with the big risk that this is forgotten with a new model class added

Possible solution by the Open API team

  • At least change the order, so that what is configured by the user in the backend is used for serialization
    @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 = "impl_1"), // <-- this needs to go first as this is what is defined in the source model
      @JsonSubTypes.Type(value = HostAvailabilityRule.class, name = "impl_2"),  // <-- this needs to go first as this is what is defined in the source model
      @JsonSubTypes.Type(value = EntityVerificationRule.class, name = "Impl1"),
      @JsonSubTypes.Type(value = HostAvailabilityRule.class, name = "Impl2")
    })
    public class Base {
      // ...
    }
    ```
- Allow disable / configure the code generation of:
    ´´´java
    @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
    )
    ```

Thank you! 😇 

b3nk4n avatar Nov 02 '22 09:11 b3nk4n

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 avatar Nov 02 '22 11:11 b3nk4n

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:

  • useless?!
  • but surprisingly correct! (or at least exactly how we would like them to be in the base class)

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!

b3nk4n avatar Nov 02 '22 12:11 b3nk4n

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".

alejo-17 avatar Dec 05 '22 20:12 alejo-17

@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.

ssternal avatar Dec 06 '22 08:12 ssternal

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?

Dalganjan avatar Jan 13 '23 13:01 Dalganjan

Hello, still no updates on this ?

trixprod avatar Mar 19 '23 09:03 trixprod

@b3nk4n I applied your suggestion in my PR.

robbertvanwaveren avatar Apr 21 '23 15:04 robbertvanwaveren

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:_

`` /**

  • BaseAnimalDto */

@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; ... /**

  • FishDto */

@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; }

/**

  • Get favoriteSwimmingPosition
  • @return favoriteSwimmingPosition */

@Schema(name = "favoriteSwimmingPosition", requiredMode = Schema.RequiredMode.NOT_REQUIRED) @JsonProperty("favoriteSwimmingPosition") public String getFavoriteSwimmingPosition() { return favoriteSwimmingPosition; }

public void setFavoriteSwimmingPosition(String favoriteSwimmingPosition) { this.favoriteSwimmingPosition = favoriteSwimmingPosition; }

public FishDto name(String name) { super.setName(name); return this; }

public FishDto animalType(AnimalTypeEnum animalType) { super.setAnimalType(animalType); 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?_

johnfburnett avatar Jul 03 '23 15:07 johnfburnett

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.

anatoliy-balakirev avatar Jul 05 '23 07:07 anatoliy-balakirev

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

johnfburnett avatar Jul 05 '23 17:07 johnfburnett

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

wing328 avatar Sep 12 '23 02:09 wing328

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.

stevenpost avatar Sep 12 '23 18:09 stevenpost

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

Qkyrie avatar Feb 23 '24 08:02 Qkyrie

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 avatar Feb 28 '24 13:02 jbeullen

@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 avatar Feb 29 '24 09:02 jbeullen

@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.

jbeullen avatar Mar 04 '24 14:03 jbeullen

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

danlz avatar May 14 '24 13:05 danlz