Validator invokes hashCode() with null mandatory field
Description
Environment
Activity

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?
Details
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




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
invokeshashCode()
withnull
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!