ConstraintViolation#unwrap won't expose implementation

brentdouglas · **Joined:** Thu Feb 09, 2012 8:31 pm **Posts:** 2

I'm using validation 5.1.2.Final.

Trying this code throws a ValidationException, which I was a little surprised about after reading the javadoc.

Code:

                    @SuppressWarnings("unchecked")
                    final ConstraintViolationImpl<Object> impl = violation.unwrap(ConstraintViolationImpl.class);

The 5.1.2.Final sources say that method is:

Code:

   @Override
   public <C> C unwrap(Class<C> type) {
      if ( type.isAssignableFrom( ConstraintViolation.class ) ) {
         return type.cast( this );
      }
      throw log.getTypeNotSupportedForUnwrappingException( type );
   }

Which looks like a bug to me. That condition is essentially

Code:

type.equals( ConstraintViolation.class )

which doesn't make much sense to me, you already have it as a ConstraintViolation in order to get access to that method. It looks like it was intended to be the other way around:

Code:

ConstraintViolation.class.isAssignableFrom( type )

I can see its changed in master but it still looks backwards. Wouldn't it make more sense to be something like this?

Code:

   @Override
   public <C> C unwrap(Class<C> type) {
      if ( type.isInstance( this ) ) {
         return type.cast( this );
      }
      throw log.getTypeNotSupportedForUnwrappingException( type );
   }

Is this a conscious decision not to expose the impl class or a bug? It doesn't really make much difference in my use case as `violation` is always the impl so I can just cast it but I would have though the above usage would have been the intention of that method.

Gunnar · **Joined:** Sat Jan 24, 2009 12:46 pm **Posts:** 388

Hi Brent,

Yes, it has been a conscious decision to not support unwrapping into the implementation class.

As of 5.3, there is a public extension of ConstraintViolation, HibernateConstraintViolation, which would be a sensible target for unwrap(). In general, we are very keen on separating public API/SPI and private implementation in Hibernate Validator, so you should never work with internal code such as ConstraintViolationImpl.

Is there anything you need from ConstraintViolationImpl (we should then consider adding this to HibernateConstraintViolation), or was it more just curiosity about the design?

Cheers,

--Gunnar

brentdouglas · **Joined:** Thu Feb 09, 2012 8:31 pm **Posts:** 2

The piece of code that I was looking at when I noticed this was originally casting the violation (so if it was a proxy it would have failed) and then reflectioning out the field 'elementType' and writes some error message based on that, the cast that made me notice it is actually wholly redundant. I'm going to remove the whole chunk though as the logic is it performing really serves no purpose.

Not exposing the implementation makes sense, though it seems to me that if you are looking at #unwrap you have already decided to do something non-standard. I guess if you really want to get access to the impl in a reliable manner you can unwrap it before casting.

Gunnar · **Joined:** Sat Jan 24, 2009 12:46 pm **Posts:** 388

Sure, by using unwrap(), you leave the field of what's specified.

But still there is a difference between using a non-standard, vendor-specific API and using an implementation class. When using the former, you are save from going through breaking changes in non-major release, whereas implementation classes can change as needed even in micro updates.

--Gunnar