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

[java] Combining "properties" and "additionalProperties" in one class generates broken Jackson mapping. #1466

Closed
r-alukhanov opened this issue Nov 16, 2018 · 23 comments · Fixed by #11572 · May be fixed by #10610
Closed

[java] Combining "properties" and "additionalProperties" in one class generates broken Jackson mapping. #1466

r-alukhanov opened this issue Nov 16, 2018 · 23 comments · Fixed by #11572 · May be fixed by #10610

Comments

@r-alukhanov
Copy link

Description

If using both "properties" and "additionalProperties", the generated class extends HashMap and contains plain properties. Like this:

public class Example extends HashMap<...>  {
  @JsonProperty("a")
  private String a;
  ...
}

Such a class cannot be meaningfully deserialised using Jackson. Jackson puts all the fields as key/value pairs into the HashMap ignoring the plain fields. In provided example getA() would return "null", while get("a") contains the value.

openapi-generator version

Used version: openapi-generator-maven-plugin : 3.3.3

OpenAPI declaration file
Example:
  type: object
  properties:
    a:
    type: string
  additionalProperties: true
Suggest a fix/enhancement

I would suggest to generate such a class (which has both "properties" and "additionalProperites") as:

public class Example {
  private String a;
  private HashMap<> otherProperties;
  ...
}

Similar suggestion in "swagger-codegen" project:
swagger-api/swagger-codegen#5187

@cjmamo
Copy link

cjmamo commented Apr 9, 2019

Any updates on this?

@vandeseer
Copy link

We are facing the very same issue using openapi-generator-gradle-plugin : 4.0.1 and the jaxrs-jersey generator.

Are there any workarounds beside not using both (i.e. properties and additionalProperties) at the same time? Are there any plans to fix it?

Thanks!

@FatCash
Copy link

FatCash commented Sep 3, 2019

Also having problems with serialize/deserialize properly because of this.
Desired approach would be to populate a mustache flag like {{hasAdditionalProperties}} on class level (not vars level), similar to {{isMapContainer}} which is currently true if additionalProperties: true
Then we could use it however we want in mustache to add the otherProperties field and necessary json annotations and builders etc..

The automatic extends HashMap<...> should not be there, instead users can add that themselves in mustasche by checking the {{hasAdditionalProperties}} flag.

@wakedeer
Copy link

Any updates on this issue?

@harel-e
Copy link

harel-e commented Aug 13, 2020

I think that having a member of HashMap instead of extending HashMap would address the issue.
@JsonAnyGetter and @JsonAnySetter are designed for this exactly.
Like @FatCash suggested, there could be a feature flag that would allow backward compatibility (though it seems broken right now anyway)

@cgfarmer4
Copy link
Contributor

curious if this was ever fixed?

@B-hamza
Copy link

B-hamza commented Feb 24, 2021

Also having problems with this issue, Any workaround or update ?

@wakedeer
Copy link

Also having problems with this issue, Any workaround or update ?

Hey, Unfortunately not! But you can make customization for it. I have made wrapper over gradle plugin and added some logic. I can share it with you if you are interested in it

@cgfarmer4
Copy link
Contributor

@wakedeer I would be interested to see this.

@harel-e
Copy link

harel-e commented Mar 10, 2021

This is related to issues 6146 and 5515

@benfonty
Copy link
Contributor

Hello, I faced the same problem. I took a look at the code and it happens that if you use the java generator with configOptions.library set to jersey2 (gradle plugin), it works as you expect.

Unfortunately for me I use the spring generator, I don't know why the behavior hasn't been set for all java generators.

I will maybe propose a fix about that.

@ssiegler
Copy link

I tried to adapt the fix for spring to the jaxrs-cxf generator. It seems to work fine for our usecase but I am not sure how to prevent this from breaking existing code which may rely on the current behavior. So I am still hesitant to open a pull request.

@jameswynn
Copy link
Contributor

These changes look really good @benfonty . Is there anything left to get this change accepted?

@jameswynn
Copy link
Contributor

@benfonty can you try merging master to your branch and push again? That should solve the build issue.

benfonty added a commit to benfonty/openapi-generator that referenced this issue Dec 8, 2021
benfonty added a commit to benfonty/openapi-generator that referenced this issue Dec 8, 2021
@benfonty
Copy link
Contributor

benfonty commented Dec 8, 2021

@benfonty can you try merging master to your branch and push again? That should solve the build issue.

@jameswynn CI is indeed OK now.

@sirimantraore
Copy link

Hello @jameswynn / @benfonty ,

Have you a update for this fix please ?

Thank you so much :)

@jameswynn
Copy link
Contributor

Is there anything blocking this from making it into the 6.0 release? @wing328 I can update the PR again.

@loiccara
Copy link

Hi, this issue is blocking my team as well.
@Zomzog , @banlevente , @borsch , @javisst , @cachescrubber , @welshm , @MelleD, @atextor, @manedev79, What is the status on this issue? I tried to contact @jameswynn with no success. I'm happy trying to merge this pull request if need be.

@WeihmannDev
Copy link
Contributor

WeihmannDev commented Oct 18, 2022

Hello, is there an issue with the MR for it not to be merged?

@zsait-clearstreet
Copy link

Is there a reason this fix is not being merged yet?

@welshm
Copy link
Contributor

welshm commented Jun 6, 2023

#11572 - this likely needs to be updated and revived to tackle this issue.

If anyone wants to create a new pull request based on the content, that would be helpful.

@benfonty
Copy link
Contributor

benfonty commented Jun 7, 2023

Hello, I created this pull request a long time ago and did not receive any approval at the time.
If anybody wants to take it over, feel free.

borsch added a commit that referenced this issue Jun 8, 2023
fix #1466)

* fix: #1466 additionalProperties works now in spring generator

* chore: chore: #1466 solved rebase conflicts

* 1466; updated samples

* [Spring] update additionalProperties MR

* [Spring] additionalProperties unit test

---------

Co-authored-by: Your Name <benfonty@gmail.com>
Co-authored-by: Oleh Kurpiak <oleh.kurpiak@gmail.com>
@borsch borsch added this to the 7.0.0 milestone Jun 8, 2023
makru86 pushed a commit to makru86/openapi-generator that referenced this issue Jun 12, 2023
…ator (OpenAPITools#11572) (fix OpenAPITools#1466)

* fix: OpenAPITools#1466 additionalProperties works now in spring generator

* chore: chore: OpenAPITools#1466 solved rebase conflicts

* 1466; updated samples

* [Spring] update additionalProperties MR

* [Spring] additionalProperties unit test

---------

Co-authored-by: Your Name <benfonty@gmail.com>
Co-authored-by: Oleh Kurpiak <oleh.kurpiak@gmail.com>
fmoraespadtec pushed a commit to padteclab/openapi-generator that referenced this issue Jun 26, 2023
…ator (OpenAPITools#11572) (fix OpenAPITools#1466)

* fix: OpenAPITools#1466 additionalProperties works now in spring generator

* chore: chore: OpenAPITools#1466 solved rebase conflicts

* 1466; updated samples

* [Spring] update additionalProperties MR

* [Spring] additionalProperties unit test

---------

Co-authored-by: Your Name <benfonty@gmail.com>
Co-authored-by: Oleh Kurpiak <oleh.kurpiak@gmail.com>
@Mettbrot
Copy link

Mettbrot commented Dec 8, 2023

This issue still persists in the Java generator of the latest version. Does anyone have an update on this?

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