HHH-12749 - Avoid setting the isolation level to the same value in C3P0ConnectionProvider

hibernate-c3p0/hibernate-c3p0.gradle

@@ -14,6 +14,8 @@ dependencies {
     compile( libraries.c3p0 )
 
     testCompile project( ':hibernate-testing' )
+    testCompile( libraries.mockito )
+    testCompile( libraries.mockito_inline )
 
     testCompile( libraries.validator ) {
         // for test runtime

hibernate-c3p0/src/main/java/org/hibernate/c3p0/internal/C3P0ConnectionProvider.java

@@ -70,7 +70,7 @@ public class C3P0ConnectionProvider
 	@SuppressWarnings("UnnecessaryUnboxing")
 	public Connection getConnection() throws SQLException {
 		final Connection c = ds.getConnection();
-		if ( isolation != null ) {
+		if ( isolation != null && !isolation.equals( c.getTransactionIsolation() ) ) {
 			c.setTransactionIsolation( isolation.intValue() );
 		}
 		if ( c.getAutoCommit() != autocommit ) {

hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/C3P0DefaultIsolationLevelTest.java

@@ -0,0 +1,100 @@
+package org.hibernate.test.c3p0;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.Map;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+
+import org.hibernate.boot.SessionFactoryBuilder;
+import org.hibernate.cfg.AvailableSettings;
+import org.hibernate.dialect.H2Dialect;
+
+import org.hibernate.testing.RequiresDialect;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.jdbc.SQLStatementInterceptor;
+import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
+import org.junit.Test;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+/**
+ * @author Vlad Mihalcea
+ */
+@TestForIssue(jiraKey = "HHH-12749")
+@RequiresDialect(H2Dialect.class)
+public class C3P0DefaultIsolationLevelTest extends
+		BaseNonConfigCoreFunctionalTestCase {
+
+	private C3P0ProxyConnectionProvider connectionProvider;
+	private SQLStatementInterceptor sqlStatementInterceptor;
+
+	@Override
+	protected void configureSessionFactoryBuilder(SessionFactoryBuilder sfb) {
+		sqlStatementInterceptor = new SQLStatementInterceptor( sfb );
+	}
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] {
+				Person.class,
+		};
+	}
+
+	@Override
+	protected void addSettings(Map settings) {
+		connectionProvider = new C3P0ProxyConnectionProvider();
+		settings.put( AvailableSettings.CONNECTION_PROVIDER, connectionProvider );
+		settings.put( AvailableSettings.ISOLATION, "READ_COMMITTED" );
+	}
+
+	@Test
+	public void testStoredProcedureOutParameter() throws SQLException {
+		clearSpies();
+
+		doInHibernate( this::sessionFactory, session -> {
+			Person person = new Person();
+			person.id = 1L;
+			person.name = "Vlad Mihalcea";
+
+			session.persist( person );
+		} );
+
+		assertEquals( 1, sqlStatementInterceptor.getSqlQueries().size() );
+		assertTrue( sqlStatementInterceptor.getSqlQueries().get( 0 ).toLowerCase().startsWith( "insert into" ) );
+		Connection connectionSpy = connectionProvider.getConnectionSpyMap().keySet().iterator().next();
+		verify( connectionSpy, never() ).setTransactionIsolation( Connection.TRANSACTION_READ_COMMITTED );
+
+		clearSpies();
+
+		doInHibernate( this::sessionFactory, session -> {
+			Person person = session.find( Person.class, 1L );
+
+			assertEquals( "Vlad Mihalcea", person.name );
+		} );
+
+		assertEquals( 1, sqlStatementInterceptor.getSqlQueries().size() );
+		assertTrue( sqlStatementInterceptor.getSqlQueries().get( 0 ).toLowerCase().startsWith( "select" ) );
+		connectionSpy = connectionProvider.getConnectionSpyMap().keySet().iterator().next();
+		verify( connectionSpy, never() ).setTransactionIsolation( Connection.TRANSACTION_READ_COMMITTED );
+	}
+
+	private void clearSpies() {
+		sqlStatementInterceptor.getSqlQueries().clear();
+		connectionProvider.clear();
+	}
+
+	@Entity(name = "Person")
+	public static class Person {
+
+		@Id
+		private Long id;
+
+		private String name;
+	}
+
+}

hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/C3P0DifferentIsolationLevelTest.java

@@ -0,0 +1,101 @@
+package org.hibernate.test.c3p0;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.Map;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+
+import org.hibernate.boot.SessionFactoryBuilder;
+import org.hibernate.cfg.AvailableSettings;
+import org.hibernate.dialect.H2Dialect;
+
+import org.hibernate.testing.RequiresDialect;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.jdbc.SQLStatementInterceptor;
+import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
+import org.junit.Test;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+/**
+ * @author Vlad Mihalcea
+ */
+@TestForIssue(jiraKey = "HHH-12749")
+@RequiresDialect(H2Dialect.class)
+public class C3P0DifferentIsolationLevelTest extends
+		BaseNonConfigCoreFunctionalTestCase {
+
+	private C3P0ProxyConnectionProvider connectionProvider;
+	private SQLStatementInterceptor sqlStatementInterceptor;
+
+	@Override
+	protected void configureSessionFactoryBuilder(SessionFactoryBuilder sfb) {
+		sqlStatementInterceptor = new SQLStatementInterceptor( sfb );
+	}
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] {
+				Person.class,
+		};
+	}
+
+	@Override
+	protected void addSettings(Map settings) {
+		connectionProvider = new C3P0ProxyConnectionProvider();
+		settings.put( AvailableSettings.CONNECTION_PROVIDER, connectionProvider );
+		settings.put( AvailableSettings.ISOLATION, "REPEATABLE_READ" );
+	}
+
+	@Test
+	public void testStoredProcedureOutParameter() throws SQLException {
+		clearSpies();
+
+		doInHibernate( this::sessionFactory, session -> {
+			Person person = new Person();
+			person.id = 1L;
+			person.name = "Vlad Mihalcea";
+
+			session.persist( person );
+		} );
+
+		assertEquals( 1, sqlStatementInterceptor.getSqlQueries().size() );
+		assertTrue( sqlStatementInterceptor.getSqlQueries().get( 0 ).toLowerCase().startsWith( "insert into" ) );
+		Connection connectionSpy = connectionProvider.getConnectionSpyMap().keySet().iterator().next();
+		verify( connectionSpy, times(1) ).setTransactionIsolation( Connection.TRANSACTION_REPEATABLE_READ );
+
+		clearSpies();
+
+		doInHibernate( this::sessionFactory, session -> {
+			Person person = session.find( Person.class, 1L );
+
+			assertEquals( "Vlad Mihalcea", person.name );
+		} );
+
+		assertEquals( 1, sqlStatementInterceptor.getSqlQueries().size() );
+		assertTrue( sqlStatementInterceptor.getSqlQueries().get( 0 ).toLowerCase().startsWith( "select" ) );
+		connectionSpy = connectionProvider.getConnectionSpyMap().keySet().iterator().next();
+		verify( connectionSpy, times(1) ).setTransactionIsolation( Connection.TRANSACTION_REPEATABLE_READ );
+	}
+
+	private void clearSpies() {
+		sqlStatementInterceptor.getSqlQueries().clear();
+		connectionProvider.clear();
+	}
+
+	@Entity(name = "Person")
+	public static class Person {
+
+		@Id
+		private Long id;
+
+		private String name;
+	}
+
+}

hibernate-c3p0/src/test/java/org/hibernate/test/c3p0/C3P0ProxyConnectionProvider.java

@@ -0,0 +1,65 @@
+package org.hibernate.test.c3p0;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.HashMap;
+import java.util.Map;
+import javax.sql.DataSource;
+
+import org.hibernate.c3p0.internal.C3P0ConnectionProvider;
+
+import org.hibernate.testing.util.ReflectionUtil;
+
+import org.mockito.MockSettings;
+import org.mockito.Mockito;
+
+/**
+ * @author Vlad Mihalcea
+ */
+public class C3P0ProxyConnectionProvider extends C3P0ConnectionProvider {
+
+	private static final MockSettings VERIFIEABLE_MOCK_SETTINGS = Mockito.withSettings()
+			.defaultAnswer( org.mockito.Answers.CALLS_REAL_METHODS );
+
+	private final Map<Connection, Connection> connectionSpyMap = new HashMap<>();
+
+	private static <T> T spy(T subject) {
+		return Mockito.mock( (Class<T>) subject.getClass(), VERIFIEABLE_MOCK_SETTINGS.spiedInstance( subject ) );
+	}
+
+	@Override
+	public void configure(Map props) {
+		super.configure( props );
+		DataSource ds = unwrap( DataSource.class );
+		DataSource dataSource = spy( ds );
+
+		try {
+			Mockito.doAnswer( invocation -> {
+				Connection connection = (Connection) invocation.callRealMethod();
+				Connection connectionSpy = spy( connection );
+				connectionSpyMap.put( connectionSpy, connection );
+				return connectionSpy;
+			} ).when( dataSource ).getConnection();
+		}
+		catch (SQLException e) {
+			throw new IllegalStateException( e );
+		}
+
+		ReflectionUtil.setField( C3P0ConnectionProvider.class.cast( this ), "ds", dataSource );
+	}
+
+	@Override
+	public void closeConnection(Connection conn) throws SQLException {
+		Connection originalConnection = connectionSpyMap.get( conn );
+
+		super.closeConnection( originalConnection != null ? originalConnection : conn );
+	}
+
+	public Map<Connection, Connection> getConnectionSpyMap() {
+		return connectionSpyMap;
+	}
+
+	public void clear() {
+		connectionSpyMap.clear();
+	}
+}

hibernate-c3p0/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker

@@ -0,0 +1 @@
+mock-maker-inline

hibernate-core/src/test/java/org/hibernate/boot/model/naming/NamingHelperTest.java

@@ -11,7 +11,7 @@
 
 import org.hibernate.testing.TestForIssue;
 import org.hibernate.testing.junit4.BaseUnitTestCase;
-import org.hibernate.test.util.ReflectionUtil;
+import org.hibernate.testing.util.ReflectionUtil;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExternalResource;

hibernate-core/src/test/java/org/hibernate/test/dialect/functional/OracleFollowOnLockingTest.java

@@ -28,7 +28,7 @@
 import org.hibernate.testing.RequiresDialect;
 import org.hibernate.testing.TestForIssue;
 import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
-import org.hibernate.test.util.jdbc.SQLStatementInterceptor;
+import org.hibernate.testing.jdbc.SQLStatementInterceptor;
 import org.junit.Before;
 import org.junit.Test;
 

hibernate-core/src/test/java/org/hibernate/test/distinct/SelectDistinctHqlTest.java

@@ -20,7 +20,7 @@
 
 import org.hibernate.testing.TestForIssue;
 import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
-import org.hibernate.test.util.jdbc.SQLStatementInterceptor;
+import org.hibernate.testing.jdbc.SQLStatementInterceptor;
 import org.junit.Test;
 
 import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;

hibernate-core/src/test/java/org/hibernate/test/id/SequenceGeneratorTest.java

@@ -19,7 +19,7 @@
 import org.hibernate.testing.SkipForDialect;
 import org.hibernate.testing.TestForIssue;
 import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
-import org.hibernate.test.util.jdbc.SQLStatementInterceptor;
+import org.hibernate.testing.jdbc.SQLStatementInterceptor;
 import org.junit.Test;
 
 import static org.junit.Assert.assertTrue;

hibernate-core/src/test/java/org/hibernate/test/locking/PessimisticWriteLockTimeoutTest.java

@@ -11,7 +11,7 @@
 
 import org.hibernate.testing.RequiresDialect;
 import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
-import org.hibernate.test.util.jdbc.SQLStatementInterceptor;
+import org.hibernate.testing.jdbc.SQLStatementInterceptor;
 import org.junit.Before;
 import org.junit.Test;
 

hibernate-core/src/test/java/org/hibernate/test/resource/transaction/jdbc/autocommit/H2SkipAutoCommitTest.java

@@ -13,7 +13,7 @@
 import org.hibernate.dialect.H2Dialect;
 
 import org.hibernate.testing.RequiresDialect;
-import org.hibernate.test.util.ReflectionUtil;
+import org.hibernate.testing.util.ReflectionUtil;
 
 /**
  * @author Vlad Mihalcea

hibernate-core/src/test/java/org/hibernate/test/resource/transaction/jdbc/autocommit/MySQLSkipAutoCommitTest.java

@@ -15,7 +15,7 @@
 import org.hibernate.dialect.MySQLDialect;
 
 import org.hibernate.testing.RequiresDialect;
-import org.hibernate.test.util.ReflectionUtil;
+import org.hibernate.testing.util.ReflectionUtil;
 
 
 /**

hibernate-core/src/test/java/org/hibernate/test/resource/transaction/jdbc/autocommit/OracleSkipAutoCommitTest.java

@@ -10,11 +10,10 @@
 
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.cfg.Environment;
-import org.hibernate.dialect.MySQLDialect;
 import org.hibernate.dialect.Oracle8iDialect;
 
 import org.hibernate.testing.RequiresDialect;
-import org.hibernate.test.util.ReflectionUtil;
+import org.hibernate.testing.util.ReflectionUtil;
 
 /**
  * @author Vlad Mihalcea

hibernate-core/src/test/java/org/hibernate/test/resource/transaction/jdbc/autocommit/PostgreSQLSkipAutoCommitTest.java

@@ -10,11 +10,10 @@
 
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.cfg.Environment;
-import org.hibernate.dialect.H2Dialect;
 import org.hibernate.dialect.PostgreSQL81Dialect;
 
 import org.hibernate.testing.RequiresDialect;
-import org.hibernate.test.util.ReflectionUtil;
+import org.hibernate.testing.util.ReflectionUtil;
 
 /**
  * @author Vlad Mihalcea

hibernate-core/src/test/java/org/hibernate/test/resource/transaction/jdbc/autocommit/SQLServerSkipAutoCommitTest.java

@@ -10,11 +10,10 @@
 
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.cfg.Environment;
-import org.hibernate.dialect.PostgreSQL81Dialect;
 import org.hibernate.dialect.SQLServerDialect;
 
 import org.hibernate.testing.RequiresDialect;
-import org.hibernate.test.util.ReflectionUtil;
+import org.hibernate.testing.util.ReflectionUtil;
 
 /**
  * @author Vlad Mihalcea

hibernate-testing/src/main/java/org/hibernate/testing/jdbc/SQLStatementInterceptor.java

@@ -4,7 +4,7 @@
  * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
  * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
  */
-package org.hibernate.test.util.jdbc;
+package org.hibernate.testing.jdbc;
 
 import java.util.LinkedList;
 

hibernate-testing/src/main/java/org/hibernate/testing/util/ReflectionUtil.java

@@ -1,4 +1,4 @@
-package org.hibernate.test.util;
+package org.hibernate.testing.util;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
@@ -24,6 +24,10 @@ public static Field getField(Class clazz, String name) {
 			return field;
 		}
 		catch ( NoSuchFieldException e ) {
+			Class superClass = clazz.getSuperclass();
+			if ( !clazz.equals( superClass ) ) {
+				return getField( superClass, name );
+			}
 			throw new IllegalArgumentException( "Class " + clazz + " does not contain a " + name + " field", e);
 		}
 	}