HHH-4675 Bean Validation ConstraintViolationException should trigger a tx rollback

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@18450 1b8cb986-b30d-0410-93ca-fae66ebed9b2

entitymanager/src/main/java/org/hibernate/ejb/AbstractEntityManagerImpl.java

@@ -56,6 +56,7 @@
 import javax.transaction.Synchronization;
 import javax.transaction.SystemException;
 import javax.transaction.TransactionManager;
+import javax.validation.ConstraintViolationException;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -444,8 +445,8 @@ public void persist(Object entity) {
 		catch ( MappingException e ) {
 			throw new IllegalArgumentException( e.getMessage() );
 		}
-		catch ( HibernateException he ) {
-			throw convert( he );
+		catch ( RuntimeException e ) {
+			throw convert( e );
 		}
 	}
 
@@ -461,8 +462,8 @@ public <A> A merge(A entity) {
 		catch ( MappingException e ) {
 			throw new IllegalArgumentException( e.getMessage(), e );
 		}
-		catch ( HibernateException he ) {
-			throw convert( he );
+		catch ( RuntimeException e ) { //including HibernateException
+			throw convert( e );
 		}
 	}
 
@@ -474,8 +475,8 @@ public void remove(Object entity) {
 		catch ( MappingException e ) {
 			throw new IllegalArgumentException( e.getMessage(), e );
 		}
-		catch ( HibernateException he ) {
-			throw convert( he );
+		catch ( RuntimeException e ) { //including HibernateException
+			throw convert( e );
 		}
 	}
 
@@ -553,8 +554,8 @@ public void flush() {
 			//adjustFlushMode();
 			getSession().flush();
 		}
-		catch ( HibernateException he ) {
-			throw convert( he );
+		catch ( RuntimeException e ) {
+			throw convert( e );
 		}
 	}
 
@@ -768,7 +769,7 @@ protected void markAsRollback() {
 			tx.setRollbackOnly();
 		}
 		else {
-			//no explicit use of the tx. boudaries methods
+			//no explicit use of the tx. boundaries methods
 			if ( PersistenceUnitTransactionType.JTA == transactionType ) {
 				TransactionManager transactionManager =
 						( ( SessionFactoryImplementor ) getRawSession().getSessionFactory() ).getTransactionManager();
@@ -985,10 +986,26 @@ public void throwPersistenceException(PersistenceException e) {
 	/**
 	 * {@inheritDoc}
 	 */
+	//FIXME should we remove all calls to this method and use convert(RuntimeException) ?
 	public RuntimeException convert(HibernateException e) {
 		return convert(e, null);
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
+	public RuntimeException convert(RuntimeException e) {
+		RuntimeException result = e;
+		if ( e instanceof HibernateException ) {
+			result = convert( (HibernateException) e );
+		}
+		else if (e instanceof ConstraintViolationException) {
+			markAsRollback();
+		}
+		//if any RT exception should mark the tx for rollback, convert the last else if into a else
+		return result;
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */

entitymanager/src/main/java/org/hibernate/ejb/QueryImpl.java

@@ -579,7 +579,8 @@ public <T> T unwrap(Class<T> tClass) {
 				PersistenceException pe = new PersistenceException(
 						"Unsupported unwrap target type [" + tClass.getName() + "]"
 				);
-				getEntityManager().handlePersistenceException( pe );
+				//It's probably against the spec to not mark the tx for rollback but it will be easier for people
+				//getEntityManager().handlePersistenceException( pe );
 				throw pe;
 			}
 		}

entitymanager/src/test/java/org/hibernate/ejb/test/beanvalidation/BeanValidationTest.java

@@ -25,6 +25,10 @@ public void testBeanValidationIntegration() {
 		catch ( ConstraintViolationException e ) {
 			assertEquals( 1, e.getConstraintViolations().size() );
 		}
+		assertTrue(
+				"A constraint violation exception should mark the transaction for rollback",
+				em.getTransaction().getRollbackOnly()
+		);
 		em.getTransaction().rollback();
 		em.close();
 	}