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][JAVA] discriminator ignored during serialization #12777

Open
stevenpost opened this issue Jul 6, 2022 · 19 comments
Open

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

stevenpost opened this issue Jul 6, 2022 · 19 comments

Comments

@stevenpost
Copy link

stevenpost commented Jul 6, 2022

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
Copy link

ssternal commented Oct 7, 2022

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

- @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
Copy link

b3nk4n commented Nov 2, 2022

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
Copy link

b3nk4n commented Nov 2, 2022

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
Copy link

b3nk4n commented Nov 2, 2022

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!

@alejo-17
Copy link

alejo-17 commented Dec 5, 2022

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
Copy link

ssternal commented Dec 6, 2022

@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
Copy link

Dalganjan commented Jan 13, 2023

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
Copy link

Hello, still no updates on this ?

@robbertvanwaveren
Copy link
Contributor

@b3nk4n I applied your suggestion in my PR.

@johnfburnett
Copy link

johnfburnett commented Jul 3, 2023

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

@anatoliy-balakirev
Copy link

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
Copy link

@rpost I noticed you pushed the commit c22997b - could you please help here?

@wing328
Copy link
Member

wing328 commented Sep 12, 2023

Can someone please share a spec to reproduce the issue? I tested with the one in #12777 (comment) but the output remains the same with #15284

@stevenpost
Copy link
Author

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.

wing328 pushed a commit that referenced this issue Sep 27, 2023
…sue #12777) (#15284)

* prioritize mapped discriminators over generated

* update samples with new ordering

* explain reason behind discriminator prioritization

* add new samples

* prioritize explicit mapping over any generated mappings

* update examples to reflect new logic

* update tests to reflect explicit mappings
AlanCitrix pushed a commit to AlanCitrix/openapi-generator that referenced this issue Oct 26, 2023
…sue OpenAPITools#12777) (OpenAPITools#15284)

* prioritize mapped discriminators over generated

* update samples with new ordering

* explain reason behind discriminator prioritization

* add new samples

* prioritize explicit mapping over any generated mappings

* update examples to reflect new logic

* update tests to reflect explicit mappings
@Qkyrie
Copy link

Qkyrie commented Feb 23, 2024

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

@jbeullen
Copy link

jbeullen commented Feb 28, 2024

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
Copy link

@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
Copy link

jbeullen commented Mar 4, 2024

@wing328
Any news on this?

I found a discussion on Open API stating that Many-To-One is allowed in de specification.
OAI/OpenAPI-Specification#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
Copy link

danlz commented May 14, 2024

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

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

No branches or pull requests