HHH-7305 - NPE in LogicalConnectionImpl when multi tenancy is used without providing a release mode manually

hibernate-core/src/main/java/org/hibernate/MultiTenancyStrategy.java

@@ -36,7 +36,6 @@ import org.hibernate.internal.CoreMessageLogger;
  * @author Steve Ebersole
  */
 public enum MultiTenancyStrategy {
-
 	/**
 	 * Multi-tenancy implemented by use of discriminator columns.
 	 */
@@ -53,10 +52,16 @@ public enum MultiTenancyStrategy {
 	 * No multi-tenancy
 	 */
 	NONE;
+
 	private static final CoreMessageLogger LOG = Logger.getMessageLogger(
 			CoreMessageLogger.class,
 			MultiTenancyStrategy.class.getName()
 	);
+
+	public boolean requiresMultiTenantConnectionProvider() {
+		return this == DATABASE || this == SCHEMA;
+	}
+
 	public static MultiTenancyStrategy determineMultiTenancyStrategy(Map properties) {
 		final Object strategy = properties.get( Environment.MULTI_TENANT );
 		if ( strategy == null ) {

hibernate-core/src/main/java/org/hibernate/cfg/SettingsFactory.java

@@ -47,6 +47,8 @@ import org.hibernate.internal.util.StringHelper;
 import org.hibernate.internal.util.config.ConfigurationHelper;
 import org.hibernate.service.ServiceRegistry;
 import org.hibernate.service.classloading.spi.ClassLoaderService;
+import org.hibernate.service.jdbc.connections.spi.ConnectionProvider;
+import org.hibernate.service.jdbc.connections.spi.MultiTenantConnectionProvider;
 import org.hibernate.service.jta.platform.spi.JtaPlatform;
 import org.hibernate.tuple.entity.EntityTuplizerFactory;
 
@@ -152,6 +154,12 @@ public class SettingsFactory implements Serializable {
 		}
 		settings.setJdbcFetchSize(statementFetchSize);
 
+		MultiTenancyStrategy multiTenancyStrategy = MultiTenancyStrategy.determineMultiTenancyStrategy( properties );
+		if ( debugEnabled ) {
+			LOG.debugf( "multi-tenancy strategy : %s", multiTenancyStrategy );
+		}
+		settings.setMultiTenancyStrategy( multiTenancyStrategy );
+
 		String releaseModeName = ConfigurationHelper.getString( Environment.RELEASE_CONNECTIONS, properties, "auto" );
 		if ( debugEnabled ) {
 			LOG.debugf( "Connection release mode: %s", releaseModeName );
@@ -162,10 +170,15 @@ public class SettingsFactory implements Serializable {
 		}
 		else {
 			releaseMode = ConnectionReleaseMode.parse( releaseModeName );
-			if ( releaseMode == ConnectionReleaseMode.AFTER_STATEMENT &&
-					! jdbcServices.getConnectionProvider().supportsAggressiveRelease() ) {
-				LOG.unsupportedAfterStatement();
-				releaseMode = ConnectionReleaseMode.AFTER_TRANSACTION;
+			if ( releaseMode == ConnectionReleaseMode.AFTER_STATEMENT ) {
+				// we need to make sure the underlying JDBC connection access supports aggressive release...
+				boolean supportsAgrressiveRelease = multiTenancyStrategy.requiresMultiTenantConnectionProvider()
+						? serviceRegistry.getService( MultiTenantConnectionProvider.class ).supportsAggressiveRelease()
+						: serviceRegistry.getService( ConnectionProvider.class ).supportsAggressiveRelease();
+				if ( ! supportsAgrressiveRelease ) {
+					LOG.unsupportedAfterStatement();
+					releaseMode = ConnectionReleaseMode.AFTER_TRANSACTION;
+				}
 			}
 		}
 		settings.setConnectionReleaseMode( releaseMode );
@@ -324,12 +337,6 @@ public class SettingsFactory implements Serializable {
 		}
 		settings.setCheckNullability(checkNullability);
 
-		MultiTenancyStrategy multiTenancyStrategy = MultiTenancyStrategy.determineMultiTenancyStrategy( properties );
-		if ( debugEnabled ) {
-			LOG.debugf( "multi-tenancy strategy : %s", multiTenancyStrategy );
-		}
-		settings.setMultiTenancyStrategy( multiTenancyStrategy );
-
 		// TODO: Does EntityTuplizerFactory really need to be configurable? revisit for HHH-6383
 		settings.setEntityTuplizerFactory( new EntityTuplizerFactory() );
 

hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcServicesImpl.java

@@ -253,6 +253,11 @@ public class JdbcServicesImpl implements JdbcServices, ServiceRegistryAwareServi
 		public void releaseConnection(Connection connection) throws SQLException {
 			connectionProvider.closeConnection( connection );
 		}
+
+		@Override
+		public boolean supportsAggressiveRelease() {
+			return connectionProvider.supportsAggressiveRelease();
+		}
 	}
 
 	private static class MultiTenantConnectionProviderJdbcConnectionAccess implements JdbcConnectionAccess {
@@ -271,6 +276,11 @@ public class JdbcServicesImpl implements JdbcServices, ServiceRegistryAwareServi
 		public void releaseConnection(Connection connection) throws SQLException {
 			connectionProvider.releaseAnyConnection( connection );
 		}
+
+		@Override
+		public boolean supportsAggressiveRelease() {
+			return connectionProvider.supportsAggressiveRelease();
+		}
 	}
 
 

hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/LogicalConnectionImpl.java

@@ -98,7 +98,7 @@ public class LogicalConnectionImpl implements LogicalConnectionImplementor {
 			boolean isClosed,
 			List<ConnectionObserver> observers) {
 		this.connectionReleaseMode = determineConnectionReleaseMode(
-				jdbcServices, isUserSuppliedConnection, connectionReleaseMode
+				jdbcConnectionAccess, isUserSuppliedConnection, connectionReleaseMode
 		);
 		this.jdbcServices = jdbcServices;
 		this.jdbcConnectionAccess = jdbcConnectionAccess;
@@ -110,14 +110,14 @@ public class LogicalConnectionImpl implements LogicalConnectionImplementor {
 	}
 
 	private static ConnectionReleaseMode determineConnectionReleaseMode(
-			JdbcServices jdbcServices,
+			JdbcConnectionAccess jdbcConnectionAccess,
 			boolean isUserSuppliedConnection,
 			ConnectionReleaseMode connectionReleaseMode) {
 		if ( isUserSuppliedConnection ) {
 			return ConnectionReleaseMode.ON_CLOSE;
 		}
 		else if ( connectionReleaseMode == ConnectionReleaseMode.AFTER_STATEMENT &&
-				! jdbcServices.getConnectionProvider().supportsAggressiveRelease() ) {
+				! jdbcConnectionAccess.supportsAggressiveRelease() ) {
 			LOG.debug( "Connection provider reports to not support aggressive release; overriding" );
 			return ConnectionReleaseMode.AFTER_TRANSACTION;
 		}

hibernate-core/src/main/java/org/hibernate/engine/jdbc/spi/JdbcConnectionAccess.java

@@ -1,7 +1,7 @@
 /*
  * Hibernate, Relational Persistence for Idiomatic Java
  *
- * Copyright (c) 2011, Red Hat Inc. or third-party contributors as
+ * Copyright (c) 2012, Red Hat Inc. or third-party contributors as
  * indicated by the @author tags or express copyright attribution
  * statements applied by the authors.  All third-party contributions are
  * distributed under license by Red Hat Inc.
@@ -34,6 +34,30 @@ import java.sql.SQLException;
  * @author Steve Ebersole
  */
 public interface JdbcConnectionAccess extends Serializable {
+	/**
+	 * Obtain a JDBC connection
+	 *
+	 * @return The obtained connection
+	 *
+	 * @throws SQLException Indicates a problem getting the connection
+	 */
 	public Connection obtainConnection() throws SQLException;
+
+	/**
+	 * Release a previously obtained connection
+	 *
+	 * @param connection The connection to release
+	 *
+	 * @throws SQLException Indicates a problem releasing the connection
+	 */
 	public void releaseConnection(Connection connection) throws SQLException;
+
+	/**
+	 * Does the underlying provider of connections support aggressive releasing of connections (and re-acquisition
+	 * of those connections later, if need be) in JTA environments?
+	 *
+	 * @see org.hibernate.service.jdbc.connections.spi.ConnectionProvider#supportsAggressiveRelease()
+	 * @see org.hibernate.service.jdbc.connections.spi.MultiTenantConnectionProvider#supportsAggressiveRelease()
+	 */
+	public boolean supportsAggressiveRelease();
 }

hibernate-core/src/main/java/org/hibernate/engine/jdbc/spi/JdbcServices.java

@@ -23,7 +23,6 @@
  */
 package org.hibernate.engine.jdbc.spi;
 
-import java.sql.Connection;
 import java.sql.ResultSet;
 
 import org.hibernate.dialect.Dialect;
@@ -42,12 +41,15 @@ public interface JdbcServices extends Service {
 	 * Obtain service for providing JDBC connections.
 	 *
 	 * @return The connection provider.
+	 *
+	 * @deprecated See deprecation notice on {@link org.hibernate.engine.spi.SessionFactoryImplementor#getConnectionProvider()}
+	 * for details
 	 */
+	@Deprecated
 	public ConnectionProvider getConnectionProvider();
 
 	/**
-	 * Obtain the dialect of the database to which {@link Connection connections} from
-	 * {@link #getConnectionProvider()} point.
+	 * Obtain the dialect of the database.
 	 *
 	 * @return The database dialect.
 	 */

hibernate-core/src/main/java/org/hibernate/engine/spi/SessionFactoryImplementor.java

@@ -152,7 +152,13 @@ public interface SessionFactoryImplementor extends Mapping, SessionFactory {
 
 	/**
 	 * Get the connection provider
+	 *
+	 * @deprecated Access to connections via {@link org.hibernate.engine.jdbc.spi.JdbcConnectionAccess} should
+	 * be preferred over access via {@link ConnectionProvider}, whenever possible.
+	 * {@link org.hibernate.engine.jdbc.spi.JdbcConnectionAccess} is tied to the Hibernate Session to
+	 * properly account for contextual information.  See {@link SessionImplementor#getJdbcConnectionAccess()}
 	 */
+	@Deprecated
 	public ConnectionProvider getConnectionProvider();
 	/**
 	 * Get the names of all persistent classes that implement/extend the given interface/class

hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/jdbc/JdbcIsolationDelegate.java

@@ -29,13 +29,13 @@ import java.sql.SQLException;
 import org.jboss.logging.Logger;
 
 import org.hibernate.HibernateException;
+import org.hibernate.engine.jdbc.spi.JdbcConnectionAccess;
 import org.hibernate.engine.jdbc.spi.SqlExceptionHelper;
 import org.hibernate.engine.transaction.spi.IsolationDelegate;
 import org.hibernate.engine.transaction.spi.TransactionCoordinator;
 import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.jdbc.WorkExecutor;
 import org.hibernate.jdbc.WorkExecutorVisitable;
-import org.hibernate.service.jdbc.connections.spi.ConnectionProvider;
 
 /**
  * The isolation delegate for JDBC {@link Connection} based transactions
@@ -52,8 +52,8 @@ public class JdbcIsolationDelegate implements IsolationDelegate {
 		this.transactionCoordinator = transactionCoordinator;
 	}
 
-	protected ConnectionProvider connectionProvider() {
-		return transactionCoordinator.getJdbcCoordinator().getLogicalConnection().getJdbcServices().getConnectionProvider();
+	protected JdbcConnectionAccess jdbcConnectionAccess() {
+		return transactionCoordinator.getTransactionContext().getJdbcConnectionAccess();
 	}
 
 	protected SqlExceptionHelper sqlExceptionHelper() {
@@ -65,7 +65,7 @@ public class JdbcIsolationDelegate implements IsolationDelegate {
 		boolean wasAutoCommit = false;
 		try {
 			// todo : should we use a connection proxy here?
-			Connection connection = connectionProvider().getConnection();
+			Connection connection = jdbcConnectionAccess().obtainConnection();
 			try {
 				if ( transacted ) {
 					if ( connection.getAutoCommit() ) {
@@ -112,7 +112,7 @@ public class JdbcIsolationDelegate implements IsolationDelegate {
 					}
 				}
 				try {
-					connectionProvider().closeConnection( connection );
+					jdbcConnectionAccess().releaseConnection( connection );
 				}
 				catch ( Exception ignore ) {
 					LOG.unableToReleaseIsolatedConnection( ignore );

hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/jta/JtaIsolationDelegate.java

@@ -23,23 +23,23 @@
  */
 package org.hibernate.engine.transaction.internal.jta;
 
-import java.sql.Connection;
-import java.sql.SQLException;
 import javax.transaction.NotSupportedException;
 import javax.transaction.SystemException;
 import javax.transaction.Transaction;
 import javax.transaction.TransactionManager;
+import java.sql.Connection;
+import java.sql.SQLException;
 
 import org.jboss.logging.Logger;
 
 import org.hibernate.HibernateException;
+import org.hibernate.engine.jdbc.spi.JdbcConnectionAccess;
 import org.hibernate.engine.jdbc.spi.SqlExceptionHelper;
 import org.hibernate.engine.transaction.spi.IsolationDelegate;
 import org.hibernate.engine.transaction.spi.TransactionCoordinator;
 import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.jdbc.WorkExecutor;
 import org.hibernate.jdbc.WorkExecutorVisitable;
-import org.hibernate.service.jdbc.connections.spi.ConnectionProvider;
 
 /**
  * An isolation delegate for JTA environments.
@@ -63,11 +63,8 @@ public class JtaIsolationDelegate implements IsolationDelegate {
 				.retrieveTransactionManager();
 	}
 
-	protected ConnectionProvider connectionProvider() {
-		return transactionCoordinator.getTransactionContext()
-				.getTransactionEnvironment()
-				.getJdbcServices()
-				.getConnectionProvider();
+	protected JdbcConnectionAccess jdbcConnectionAccess() {
+		return transactionCoordinator.getTransactionContext().getJdbcConnectionAccess();
 	}
 
 	protected SqlExceptionHelper sqlExceptionHelper() {
@@ -120,15 +117,15 @@ public class JtaIsolationDelegate implements IsolationDelegate {
 	}
 
 	private <T> T doTheWorkInNewTransaction(WorkExecutorVisitable<T> work, TransactionManager transactionManager) {
-		T result = null;
 		try {
 			// start the new isolated transaction
 			transactionManager.begin();
 
 			try {
-				result = doTheWork( work );
+				T result = doTheWork( work );
 				// if everything went ok, commit the isolated transaction
 				transactionManager.commit();
+				return result;
 			}
 			catch ( Exception e ) {
 				try {
@@ -146,7 +143,6 @@ public class JtaIsolationDelegate implements IsolationDelegate {
 		catch ( NotSupportedException e ) {
 			throw new HibernateException( "Unable to start isolated transaction", e );
 		}
-		return result;
 	}
 
 	private <T> T doTheWorkInNoTransaction(WorkExecutorVisitable<T> work) {
@@ -156,7 +152,7 @@ public class JtaIsolationDelegate implements IsolationDelegate {
 	private <T> T doTheWork(WorkExecutorVisitable<T> work) {
 		try {
 			// obtain our isolated connection
-			Connection connection = connectionProvider().getConnection();
+			Connection connection = jdbcConnectionAccess().obtainConnection();
 			try {
 				// do the actual work
 				return work.accept( new WorkExecutor<T>(), connection );
@@ -170,7 +166,7 @@ public class JtaIsolationDelegate implements IsolationDelegate {
 			finally {
 				try {
 					// no matter what, release the connection (handle)
-					connectionProvider().closeConnection( connection );
+					jdbcConnectionAccess().releaseConnection( connection );
 				}
 				catch ( Throwable ignore ) {
 					LOG.unableToReleaseIsolatedConnection( ignore );

hibernate-core/src/main/java/org/hibernate/internal/AbstractSessionImpl.java

@@ -333,6 +333,11 @@ public abstract class AbstractSessionImpl implements Serializable, SharedSession
 		public void releaseConnection(Connection connection) throws SQLException {
 			connectionProvider.closeConnection( connection );
 		}
+
+		@Override
+		public boolean supportsAggressiveRelease() {
+			return connectionProvider.supportsAggressiveRelease();
+		}
 	}
 
 	private class ContextualJdbcConnectionAccess implements JdbcConnectionAccess, Serializable {
@@ -357,5 +362,10 @@ public abstract class AbstractSessionImpl implements Serializable, SharedSession
 			}
 			connectionProvider.releaseConnection( tenantIdentifier, connection );
 		}
+
+		@Override
+		public boolean supportsAggressiveRelease() {
+			return connectionProvider.supportsAggressiveRelease();
+		}
 	}
 }

hibernate-core/src/test/java/org/hibernate/cache/spi/NaturalIdCacheKeyTest.java

@@ -1,3 +1,26 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2012, Red Hat Inc. or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
 package org.hibernate.cache.spi;
 
 import static junit.framework.Assert.assertEquals;

hibernate-core/src/test/java/org/hibernate/test/common/JdbcConnectionAccessImpl.java

@@ -58,4 +58,9 @@ public class JdbcConnectionAccessImpl implements JdbcConnectionAccess {
 	public void releaseConnection(Connection connection) throws SQLException {
 		connectionProvider.closeConnection( connection );
 	}
+
+	@Override
+	public boolean supportsAggressiveRelease() {
+		return connectionProvider.supportsAggressiveRelease();
+	}
 }

hibernate-core/src/test/java/org/hibernate/test/multitenancy/ConfigurationValidationTest.java

@@ -2,13 +2,16 @@ package org.hibernate.test.multitenancy;
 
 import org.junit.Test;
 
+import org.hibernate.ConnectionReleaseMode;
 import org.hibernate.MultiTenancyStrategy;
 import org.hibernate.cfg.Configuration;
 import org.hibernate.cfg.Environment;
 import org.hibernate.service.ServiceRegistryBuilder;
+import org.hibernate.service.jdbc.connections.spi.MultiTenantConnectionProvider;
 import org.hibernate.service.spi.ServiceException;
 import org.hibernate.service.spi.ServiceRegistryImplementor;
 import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.env.ConnectionProviderBuilder;
 import org.hibernate.testing.junit4.BaseUnitTestCase;
 
 /**
@@ -26,4 +29,27 @@ public class ConfigurationValidationTest extends BaseUnitTestCase {
 				.applySettings( cfg.getProperties() ).buildServiceRegistry();
 		cfg.buildSessionFactory( serviceRegistry );
 	}
+
+	@Test
+	public void testReleaseMode() {
+		Configuration cfg = new Configuration();
+		cfg.getProperties().put( Environment.MULTI_TENANT, MultiTenancyStrategy.SCHEMA );
+		cfg.getProperties().put( Environment.RELEASE_CONNECTIONS, ConnectionReleaseMode.AFTER_STATEMENT.name() );
+		cfg.buildMappings();
+
+		ServiceRegistryImplementor serviceRegistry = (ServiceRegistryImplementor) new ServiceRegistryBuilder()
+				.applySettings( cfg.getProperties() )
+				.addService(
+						MultiTenantConnectionProvider.class,
+						new TestingConnectionProvider(
+								new TestingConnectionProvider.NamedConnectionProviderPair(
+										"acme",
+										ConnectionProviderBuilder.buildConnectionProvider( "acme" )
+								)
+						)
+				)
+				.buildServiceRegistry();
+
+		cfg.buildSessionFactory( serviceRegistry );
+	}
 }

hibernate-core/src/test/java/org/hibernate/test/multitenancy/TestingConnectionProvider.java

@@ -0,0 +1,69 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2012, Red Hat Inc. or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.multitenancy;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.hibernate.service.jdbc.connections.spi.AbstractMultiTenantConnectionProvider;
+import org.hibernate.service.jdbc.connections.spi.ConnectionProvider;
+
+/**
+ * @author Steve Ebersole
+ */
+public class TestingConnectionProvider extends AbstractMultiTenantConnectionProvider {
+	private Map<String,ConnectionProvider> connectionProviderMap;
+
+	public TestingConnectionProvider(Map<String, ConnectionProvider> connectionProviderMap) {
+		this.connectionProviderMap = connectionProviderMap;
+	}
+
+	public TestingConnectionProvider(NamedConnectionProviderPair... pairs) {
+		Map<String,ConnectionProvider> map = new HashMap<String, ConnectionProvider>();
+		for ( NamedConnectionProviderPair pair : pairs ) {
+			map.put( pair.name, pair.connectionProvider );
+		}
+		this.connectionProviderMap = map;
+	}
+
+	@Override
+	protected ConnectionProvider getAnyConnectionProvider() {
+		return connectionProviderMap.values().iterator().next();
+	}
+
+	@Override
+	protected ConnectionProvider selectConnectionProvider(String tenantIdentifier) {
+		return connectionProviderMap.get( tenantIdentifier );
+	}
+
+	public static class NamedConnectionProviderPair {
+		private final String name;
+		private final ConnectionProvider connectionProvider;
+
+		public NamedConnectionProviderPair(String name, ConnectionProvider connectionProvider) {
+			this.name = name;
+			this.connectionProvider = connectionProvider;
+		}
+	}
+}