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

@DataObject does not handle json in String format when generating Converters #312

Open
pendula95 opened this issue Sep 16, 2020 · 5 comments
Labels

Comments

@pendula95
Copy link

pendula95 commented Sep 16, 2020

Version

3.9.2

Context

When @'DataObject' is generated with its fromJson method objects that are import io.vertx.core.json.JsonObject only accept JsonObject as valid option for mapping. Example generated constructor:

private JsonObject attributes;

For this property following code is generated:

case "attributes":
          if (member.getValue() instanceof JsonObject) {
            obj.setAttributes(((JsonObject)member.getValue()).copy());
          }
          break;

Generated code should be like:

case "attributes":
          if (member.getValue() instanceof JsonObject) {
            obj.setAttributes(((JsonObject)member.getValue()).copy());
          } else if (member.getValue() instanceof String) {
            obj.setAttributes(new JsonObject(member.getValue()));
          }
          break;

If for example from DB I get a String field that has JSON formating this property will never get mapped. On the other hand if this POJO is generated by Jackson function this field with get mapped.
SomeDTO someDTO = json.mapTo(SomeDTO.class);

Extra

Is this something that was intended or just wasn't considered. In my eyes this is a standard approach as vertx json wrapper accepts String as valid for when generating Json objects. I can submit a PR if we agree to go with this.

@pendula95 pendula95 added the bug label Sep 16, 2020
@pendula95 pendula95 changed the title @DataObject does not take json in String format when generating Converters @DataObject does not handle json in String format when generating Converters Sep 16, 2020
@vietj
Copy link
Member

vietj commented Sep 18, 2020

this is the expected behavior.

@pendula95
Copy link
Author

This looks like the place where the fix should be applied. https://github.com/vert-x3/vertx-codegen/blob/ca12c22dc7ae8ef1017a24d9fde06be0447801e0/src/main/java/io/vertx/codegen/generators/dataobjecthelper/DataObjectHelperGen.java#L300
Is there any other place/thing I should care about?

@vietj
Copy link
Member

vietj commented Sep 21, 2020

we are not looking to change this behavior @pendula95 as this is the expected one

@pendula95
Copy link
Author

Than I would change this from bug to enhancement as I see a need for this.
As vertx release on jackson and whole json handling logic is tightly wrapped around it I don't see why it would not be supported by codegen. I mean I can use Jackson to map this value but than I am having a problem as this object can no longer be easily passed via eventbus and I need to create hybrid logic half codegen half jackson in order to get desired functionality.

@vietj
Copy link
Member

vietj commented Oct 21, 2020

Consider the case where somebody does not want this behavior: e.g the application does not want to parse a string containing when it receives invalid JSON

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

No branches or pull requests

2 participants