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

@JsonUnwrapped fails for HibernateProxy instances #97

Closed
jwgmeligmeyling opened this issue Sep 30, 2016 · 13 comments
Closed

@JsonUnwrapped fails for HibernateProxy instances #97

jwgmeligmeyling opened this issue Sep 30, 2016 · 13 comments

Comments

@jwgmeligmeyling
Copy link

@JsonUnwrapped fails for HibernateProxy instances. They are serialized as if the @JsonUnwrapped annotation was missing.

Work around:

Replace the object with its underlying implementation:

Hibernate.initialize(client);
this.client = (Client) ((HibernateProxy) client)
      .getHibernateLazyInitializer()
      .getImplementation();

Tested on the following dependencies:

[INFO] +- com.fasterxml.jackson.core:jackson-databind:jar:2.8.0:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.8.0:compile
[INFO] |  \- com.fasterxml.jackson.core:jackson-core:jar:2.8.0:compile
[INFO] +- com.fasterxml.jackson.datatype:jackson-datatype-jsr310:jar:2.8.0:compile
[INFO] +- com.fasterxml.jackson.datatype:jackson-datatype-hibernate5:jar:2.8.0:compile
[INFO] +- com.fasterxml.jackson.datatype:jackson-datatype-jdk8:jar:2.8.0:compile
@cowtowncoder
Copy link
Member

I don't doubt this happens, but a reproducible unit test would be very helpful.

@jwgmeligmeyling
Copy link
Author

I am working on a test case. Meanwhile I am also trying to step through the code a bit.

@shakuzen
Copy link

shakuzen commented Apr 3, 2017

@jwgmeligmeyling Isn't @JsonUnwrapped not working when HibernateProxySerializer is used the expected behavior as per the note in the JsonUnwrapped JavaDoc:

Also note that annotation only applies if

  • Value is serialized as JSON Object (can not unwrap JSON arrays using this mechanism)
  • Serialization is done using BeanSerializer, not a custom serializer
  • No type information is added; if type information needs to be added, structure can not be altered regardless of inclusion strategy; so annotation is basically ignored.

@jwgmeligmeyling
Copy link
Author

jwgmeligmeyling commented Apr 3, 2017

I see why the JsonUnwrapped serializer does not support HibernateProxies by default, but imho the HibernateProxySerializer should work around that limitation as working around Hibernate proxy quirks is quite the goal of this module.

@cowtowncoder
Copy link
Member

PRs welcome!

Note: due to lack of development it is possible and perhaps likely this module will be deprecated and not maintained in future.

@odrotbohm
Copy link

We're running into this in DATAREST-1494. The ultimate place tthigs start to diverge compared to non-proxy rendering is UnwrappingBeanPropertyWriter.serializeAsField(…) in this clause:

if (!ser.isUnwrappingSerializer()) {
  gen.writeFieldName(_name);
}

HibernateProxySerializer doesn't override this method. I am assuming that call is made to not unwrap in situations described by @shakuzen (👋, by the way). Looking at the callers of this method, it feels like the method is not returning the definite decision whether the implementation will unwrap in this moment but rather whether it's generally capable of unwrapping. If that's correct, couldn't the HibernateProxySerializer override the method and return true?

Happy to provide a pull request.

@jwgmeligmeyling
Copy link
Author

@odrotbohm that fix actually makes a lot of sense. I never had the time to finish this myself (probably have some old branch from years ago with a reproducer if I dig deep...). I'd say go ahead and see if the fix works! It sounds promising to me. I think we find out soon enough if unwrapped serialization is really limited to BeanSerializer instances, or just serializers that return true for isUnwrappingSerializer.

@odrotbohm
Copy link

I'd still wait for some input by @cowtowncoder to whether my interpretation is correct. 🙃

@cowtowncoder
Copy link
Member

Without having (had time to) look(ed), that does sound reasonable. I'll add this issue to my "to evalute" list to have a look before finalizing 2.11.0.

@cowtowncoder
Copy link
Member

@odrotbohm Ok. So, meaning of isUnwrappingSerializer() is not about whether serializer supports it as much as try to support unwrapping (bean) serializer. Changing that would not help here. However, this method:

 public JsonSerializer<T> unwrappingSerializer(NameTransformer unwrapper) { }

is what would be needed to "change" proxy serializer to unwrapping one. It would be relatively easy to change that to create standard unwrapping bean serializer; however, that would then not handle proxy part of handling. Instead Hibernate module would either add handling of unwrapped properties into proxy serializer itself, or, subclass UnwrappingBeanSerializer to do so.

This is doable but not a trivial change.

If anyone wants to tackle it, I can probably help. I think some other module does this but I am not sure which one (Afterburner module, perhaps).

tsachev added a commit to tsachev/jackson-datatype-hibernate that referenced this issue Jul 11, 2020
tsachev added a commit to tsachev/jackson-datatype-hibernate that referenced this issue Jul 11, 2020
tsachev added a commit to tsachev/jackson-datatype-hibernate that referenced this issue Jul 12, 2020
@cowtowncoder cowtowncoder changed the title JsonUnwrapped fails for HibernateProxy instances @JsonUnwrapped fails for HibernateProxy instances Jul 12, 2020
cowtowncoder added a commit that referenced this issue Jul 12, 2020
@ptahchiev
Copy link

Just wanted to mention that the PR for this issue was sponsored by Nemesis Software (http://nemesis.io) :)

@cowtowncoder
Copy link
Member

Excellent, thank you to Nemesis Software then!

Would you like me to add a note to VERSION? (so in addition to contributed by Xxx I can add "Xxx / Nemesis Software")

@ptahchiev
Copy link

Yes, sure

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

No branches or pull requests

5 participants