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] combining properties and additionalProperties generates broken Model class #17361

Open
4 of 5 tasks
Mettbrot opened this issue Dec 9, 2023 · 8 comments
Open
4 of 5 tasks

Comments

@Mettbrot
Copy link

Mettbrot commented Dec 9, 2023

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

Combining required properties with additionalProperties: true generates a Model class that extends HashMap. On deserialization, the given Properties are not filled, instead all properties (required and additional) are part of the Hashmap. According to FasterXML/jackson-databind#3173 this is expected behavior for Jackson so the generated Model is wrong.
This was fixed for the spring generator in #11572 but its still happening in the java generator.

openapi-generator version

7.1.0

OpenAPI declaration file content or url
    additionalPropertiesWithRequiredData:
      type: object
      additionalProperties: true
      properties:
        propA:
          type: integer
          format: int64
        propB:
          type: string
      required:
        - propA
        - propB
Related issues/PRs

#1466
#11572

Suggest a fix

The model generation should not extend HashMap, instead put additionalProperties as a HashMap inside the class and annotate it with @JsonAnySetter / @JsonAnyGetter.

@throwable
Copy link

Thank you for pointing out that the bug had already been fixed in the Spring generator. This enabled me to create a simple workaround consisting of two steps:

  1. Generate the client as-is using the Java + WebClient generator.
  2. Replace the original model with the one generated by the Spring + Spring HTTP Interface generator.

Ugly, but it happened to work.

@lesteenman
Copy link

It seems worse for me - even setting additionalProperties: false in the model will result in the model explicitly extending HashMap.

We explicitly set this property to false to make sure our validation correctly fails if our examples have unknown types, as the default of openapi 3.1 is to allow additional properties.

Example:

  schemas:
    DemoNested:
      type: object
      required:
        - textField
      properties:
        textField:
          type: string

Result:

/**
 * ReproductionDemoNested
 */
@JsonPropertyOrder({
  ReproductionDemoNested.JSON_PROPERTY_TEXT_FIELD
})
@JsonTypeName("DemoNested")
@jakarta.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2024-09-23T11:41:21.482102+02:00[Europe/Amsterdam]", comments = "Generator version: 7.8.0")
public class ReproductionDemoNested {
  public static final String JSON_PROPERTY_TEXT_FIELD = "textField";
  private String textField;

versus:

  schemas:
    DemoNested:
      type: object
      additionalProperties: false
      required:
        - textField
      properties:
        textField:
          type: string

resulting in

/**
 * ReproductionDemoNested
 */
@JsonPropertyOrder({
  ReproductionDemoNested.JSON_PROPERTY_TEXT_FIELD
})
@JsonTypeName("DemoNested")
@jakarta.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", date = "2024-09-23T11:41:09.715975+02:00[Europe/Amsterdam]", comments = "Generator version: 7.8.0")
public class ReproductionDemoNested extends HashMap<String, Object> {
  public static final String JSON_PROPERTY_TEXT_FIELD = "textField";
  private String textField;

@JoaoBrlt
Copy link

Hey! I'm facing the same issue with the java generator and the resttemplate library.
Which library are you using?
I took a look at the code and it seems to work with the java generator for the following libraries only: native, jersey2, jersey3, okhttp-gson, feign.

@Mettbrot @throwable @lesteenman

@JoaoBrlt
Copy link

JoaoBrlt commented Sep 28, 2024

I've created the pull request #19706 to fix the issue for the resttemplate library.
If you use another library, we can also create a pull request for that.
The fix already exists but it's "opt-in" for each library.

@lesteenman
Copy link

Awesome. We use the feign library - I haven't checked it in enough detail yet, but how hard would it be to port your fix to that library as well?

Edit: I see you mention this should already work with feign? That's odd since we do see an issue still.

@Mettbrot
Copy link
Author

Mettbrot commented Sep 29, 2024 via email

@JoaoBrlt
Copy link

JoaoBrlt commented Sep 29, 2024

Hey @Mettbrot! Alright, I've created the pull request #19711 to fix the same issue for the webclient library. 😉

@JoaoBrlt
Copy link

Hey @lesteenman! I've created the pull request #19713 to fix the same issue for the feign library. It's a bit weird because this library already had the additional Mustache templates to handle additional properties using an attribute and the Jackson annotations. However, the library was not marked as supporting "additional properties with composed schema" which means the model classes were still extending HashMap. I also fixed the equals(), hashCode() and toString() methods to take the additional properties into account (as the other libraries). 😉

I was not able to reproduce the issue with additionalProperties set to false. Maybe you can create another issue for that if it still happens after the fix.

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

4 participants