Validator invokes hashCode() with null mandatory field

Description

Hello,

I have mandatory field in object which I want to validate:

But when I'm trying to validate it using the Hibernate Validator I receive an exception:

Problem is that Validator invokes hashCode() with null mandatory field. But purpose of validation is to check nullability of this field and produce a validation error. Current behavior is conceptually wrong from my point of view.

Thanks in advance!

Environment

Hibernate 4.2.12.Final

Activity

Show:

Gunnar Morling August 24, 2016 at 9:25 AM

Also see where we already use the identity hash code. Should be the right thing to do in the traversable resolver, too.

Gunnar Morling August 24, 2016 at 9:15 AM

thanks for sharing these insights!

I see where you are coming from and feel it's a rather unfortunate corner case you are hitting here. I also believe that hashCode() implementations should be safe towards null-valued properties if those can actual occur (which they do in your case, although only in the special case of deserialization).

I think your case would ideally be solved by having Jackson use the builder during deserialization, enforcing the null check during construction of the object (no idea though whether Jackson is flexible enough to achieve that). Then the invariant would be enforced, and HV would never see such illegally constructed object.

That being said, I'm wondering whether we couldn't change the code in CachingTraversableResolverForSingleValidation to use System#identityHashCode() instead of invoking the object's hashCode() method. Actually, the identity of the object and the traversed property should be enough as the identity for the cache key type TraversableHolder. , WDYT?

Rafael Tedin Alvarez March 10, 2016 at 11:33 AM

BTW, we are using Jackson for deserialization and Hibernate Validator 5.2.2.Final. For generating the immutables we use http://immutables.github.io/.

Rafael Tedin Alvarez March 10, 2016 at 11:30 AM

Thanks for the quick reply!

What I wanted to say is that the validation relies on the hashCode() and hashCode() (at least in our use case) only works if the instance is already valid. So in the use case I mentioned the validation only works if the instance is already valid, defeating the purpose of validation.

JSON deserialization is working fine. When we receive a JSON string, we can create an instance of our immutable class without problems. The thing is that the deserialized instance may not follow the class invariant (since it was not created through the builder that would ensure it). Then we need validation to ensure that the deserialized instance follows indeed the class invariant (or preconditions/assumptions if you prefer).

Take the following (made up) example where Son is our automatically generated immutable class and SonBuilder the automatically generated builder. We cannot (for the most part) change their code.

If we use the builder, we can be sure that the resulting Son instance is valid. However if someone sends a JSON (e.g. as part of a web service request), the deserialized instance may be invalid since deserialization does not use the builder. If the JSON does not contain father, the instance is deserialized just fine, but the field father is set to null. Nevertheless the web service has to ensure that it is dealing with correct instances. Hence at this point we need the additional bean validation. But since the field is null hashCode() will throw and we cannot properly validate the instance.

Changing the hashCode() to circumvent the class invariant might be possible in some cases, but not in others. We think it's a not so good idea in our use case, since this could mask problems and errors in our automatically generated builders. In other words we like that hashCode() fails if the instance does not follow its invariant as this gives us the opportunity to discover errors.

I think that the behavior of the validation is currently conceptually wrong because it requires immunity to invalid inner state from some methods (from hashCode() at least). But it's because of invalid inner state that we use validation: to be sure that we can safely use all the methods of the class.

I hope I succeeded to clarify the context a bit more and thanks again!

Hardy Ferentschik March 10, 2016 at 10:06 AM

But if the validation process itself assumes that an instance has to valid

It does not do that by any means.

So you are running into this as well as part of your JSON deserialization? Can you provide more context?

Fixed

Details

Assignee

Reporter

Bug Testcase Reminder (view)

Bug reports should generally be accompanied by a test case!

Bug Testcase Reminder (edit)

Bug reports should generally be accompanied by a test case!

Participants

Gunnar Morling
Hardy Ferentschik
oleg
Rafael Tedin Alvarez

Components

Fix versions

Affects versions

Priority

Created August 20, 2015 at 8:50 AM
Updated September 1, 2016 at 8:58 PM
Resolved August 30, 2016 at 7:23 AM