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

Fix generated method toString with inheritance. #1298

Merged

Conversation

altro3
Copy link
Collaborator

@altro3 altro3 commented Nov 9, 2023

Add no args constructor for POJOs, when use jackson-databind.

@altro3
Copy link
Collaborator Author

altro3 commented Nov 9, 2023

About toString: as I see, this is bug in openapi-generator lib. allVars property must contains all properties for class plus all parent properties, but now in some cases it contains only parent properties.

About no-args constructor: need it for jackson-databind, because jackson by default use no-args constructor or trying to find method with @JsonCreator annotation. This is easiest solution. The second solution: add annotation @JsonCreator to required args constructor, but in this case you also need to add @JsonProperty annotation for every constructor argument, because jackson can't automatticaly map json proeprties to method arguments

@altro3 altro3 force-pushed the fix-generator-tostring branch 2 times, most recently from bbbbdd0 to df6feb4 Compare November 9, 2023 09:25
@altro3 altro3 changed the base branch from 5.2.x to master November 9, 2023 10:43
@altro3
Copy link
Collaborator Author

altro3 commented Nov 9, 2023

@andriy-dmytruk Hi! Could you revie wit? And after this PR we can finalize next release

@@ -382,9 +387,9 @@ public class {{classname}} {{#parent}}extends {{{parent}}} {{/parent}}{{#vendorE
@Override
public String toString() {
return "{{classname}}("
{{#allVars}}
{{#vendorExtensions.allVars}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should fix the allVars property instead of creating a new property in vendorExtensions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -131,6 +131,11 @@ public class {{classname}} {{#parent}}extends {{{parent}}} {{/parent}}{{#vendorE
{{/vars}}

{{#requiredPropertiesInConstructor}}
{{^micronaut_serde_jackson}}
public {{classname}}() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better for constructor to be private or package-private. Otherwise the point of required properties is lost.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@sdelamo sdelamo added the type: bug Something isn't working label Nov 9, 2023
@altro3 altro3 force-pushed the fix-generator-tostring branch 2 times, most recently from 474876f to 146b056 Compare November 10, 2023 07:23
@altro3
Copy link
Collaborator Author

altro3 commented Nov 10, 2023

Also removed @Introspected annotation, when we add @Serdeable annotation

Add no args constructor for POJOs, when use jackson-databind
@andriy-dmytruk andriy-dmytruk merged commit d951cea into micronaut-projects:master Nov 10, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants