Skip to content

Commit

Permalink
HHH-7305 - NPE in LogicalConnectionImpl when multi tenancy is used wi…
Browse files Browse the repository at this point in the history
…thout providing a release mode manually

(cherry picked from commit a385792)
  • Loading branch information
sebersole committed Aug 7, 2012
1 parent f5e6ada commit 98e7fc5
Show file tree
Hide file tree
Showing 14 changed files with 219 additions and 36 deletions.
Expand Up @@ -36,7 +36,6 @@
* @author Steve Ebersole
*/
public enum MultiTenancyStrategy {

/**
* Multi-tenancy implemented by use of discriminator columns.
*/
Expand All @@ -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 ) {
Expand Down
27 changes: 17 additions & 10 deletions hibernate-core/src/main/java/org/hibernate/cfg/SettingsFactory.java
Expand Up @@ -47,6 +47,8 @@
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;

Expand Down Expand Up @@ -152,6 +154,12 @@ public Settings buildSettings(Properties props, ServiceRegistry serviceRegistry)
}
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 );
Expand All @@ -162,10 +170,15 @@ public Settings buildSettings(Properties props, ServiceRegistry serviceRegistry)
}
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 );
Expand Down Expand Up @@ -324,12 +337,6 @@ public Settings buildSettings(Properties props, ServiceRegistry serviceRegistry)
}
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() );

Expand Down
Expand Up @@ -246,6 +246,11 @@ public Connection obtainConnection() throws SQLException {
public void releaseConnection(Connection connection) throws SQLException {
connectionProvider.closeConnection( connection );
}

@Override
public boolean supportsAggressiveRelease() {
return connectionProvider.supportsAggressiveRelease();
}
}

private static class MultiTenantConnectionProviderJdbcConnectionAccess implements JdbcConnectionAccess {
Expand All @@ -264,6 +269,11 @@ public Connection obtainConnection() throws SQLException {
public void releaseConnection(Connection connection) throws SQLException {
connectionProvider.releaseAnyConnection( connection );
}

@Override
public boolean supportsAggressiveRelease() {
return connectionProvider.supportsAggressiveRelease();
}
}


Expand Down
Expand Up @@ -98,7 +98,7 @@ private LogicalConnectionImpl(
boolean isClosed,
List<ConnectionObserver> observers) {
this.connectionReleaseMode = determineConnectionReleaseMode(
jdbcServices, isUserSuppliedConnection, connectionReleaseMode
jdbcConnectionAccess, isUserSuppliedConnection, connectionReleaseMode
);
this.jdbcServices = jdbcServices;
this.jdbcConnectionAccess = jdbcConnectionAccess;
Expand All @@ -110,14 +110,14 @@ private LogicalConnectionImpl(
}

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;
}
Expand Down
@@ -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.
Expand Down Expand Up @@ -34,6 +34,30 @@
* @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();
}
Expand Up @@ -23,7 +23,6 @@
*/
package org.hibernate.engine.jdbc.spi;

import java.sql.Connection;
import java.sql.ResultSet;

import org.hibernate.dialect.Dialect;
Expand All @@ -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.
*/
Expand Down
Expand Up @@ -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
Expand Down
Expand Up @@ -29,13 +29,13 @@
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
Expand All @@ -52,8 +52,8 @@ public JdbcIsolationDelegate(TransactionCoordinator transactionCoordinator) {
this.transactionCoordinator = transactionCoordinator;
}

protected ConnectionProvider connectionProvider() {
return transactionCoordinator.getJdbcCoordinator().getLogicalConnection().getJdbcServices().getConnectionProvider();
protected JdbcConnectionAccess jdbcConnectionAccess() {
return transactionCoordinator.getTransactionContext().getJdbcConnectionAccess();
}

protected SqlExceptionHelper sqlExceptionHelper() {
Expand All @@ -65,7 +65,7 @@ public <T> T delegateWork(WorkExecutorVisitable<T> work, boolean transacted) thr
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() ) {
Expand Down Expand Up @@ -112,7 +112,7 @@ else if ( e instanceof SQLException ) {
}
}
try {
connectionProvider().closeConnection( connection );
jdbcConnectionAccess().releaseConnection( connection );
}
catch ( Exception ignore ) {
LOG.unableToReleaseIsolatedConnection( ignore );
Expand Down
Expand Up @@ -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.
Expand All @@ -63,11 +63,8 @@ protected TransactionManager transactionManager() {
.retrieveTransactionManager();
}

protected ConnectionProvider connectionProvider() {
return transactionCoordinator.getTransactionContext()
.getTransactionEnvironment()
.getJdbcServices()
.getConnectionProvider();
protected JdbcConnectionAccess jdbcConnectionAccess() {
return transactionCoordinator.getTransactionContext().getJdbcConnectionAccess();
}

protected SqlExceptionHelper sqlExceptionHelper() {
Expand Down Expand Up @@ -120,15 +117,15 @@ public <T> T delegateWork(WorkExecutorVisitable<T> work, boolean transacted) thr
}

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 {
Expand All @@ -146,7 +143,6 @@ private <T> T doTheWorkInNewTransaction(WorkExecutorVisitable<T> work, Transacti
catch ( NotSupportedException e ) {
throw new HibernateException( "Unable to start isolated transaction", e );
}
return result;
}

private <T> T doTheWorkInNoTransaction(WorkExecutorVisitable<T> work) {
Expand All @@ -156,7 +152,7 @@ private <T> T doTheWorkInNoTransaction(WorkExecutorVisitable<T> work) {
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 );
Expand All @@ -170,7 +166,7 @@ private <T> T doTheWork(WorkExecutorVisitable<T> work) {
finally {
try {
// no matter what, release the connection (handle)
connectionProvider().closeConnection( connection );
jdbcConnectionAccess().releaseConnection( connection );
}
catch ( Throwable ignore ) {
LOG.unableToReleaseIsolatedConnection( ignore );
Expand Down
Expand Up @@ -285,6 +285,11 @@ public Connection obtainConnection() throws SQLException {
public void releaseConnection(Connection connection) throws SQLException {
connectionProvider.closeConnection( connection );
}

@Override
public boolean supportsAggressiveRelease() {
return connectionProvider.supportsAggressiveRelease();
}
}

private class ContextualJdbcConnectionAccess implements JdbcConnectionAccess, Serializable {
Expand All @@ -309,5 +314,10 @@ public void releaseConnection(Connection connection) throws SQLException {
}
connectionProvider.releaseConnection( tenantIdentifier, connection );
}

@Override
public boolean supportsAggressiveRelease() {
return connectionProvider.supportsAggressiveRelease();
}
}
}
@@ -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;
Expand Down
Expand Up @@ -58,4 +58,9 @@ public Connection obtainConnection() throws SQLException {
public void releaseConnection(Connection connection) throws SQLException {
connectionProvider.closeConnection( connection );
}

@Override
public boolean supportsAggressiveRelease() {
return connectionProvider.supportsAggressiveRelease();
}
}

0 comments on commit 98e7fc5

Please sign in to comment.