minor cleanups and logging improvements related to logical connections

Author: gavinking

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

@@ -33,9 +33,12 @@
 import jakarta.transaction.Synchronization;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
+import static org.hibernate.cfg.JdbcSettings.CONNECTION_PROVIDER_DISABLES_AUTOCOMMIT;
+import static org.hibernate.cfg.ValidationSettings.JAKARTA_VALIDATION_MODE;
 import static org.jboss.logging.Logger.Level.DEBUG;
 import static org.jboss.logging.Logger.Level.ERROR;
 import static org.jboss.logging.Logger.Level.INFO;
+import static org.jboss.logging.Logger.Level.TRACE;
 import static org.jboss.logging.Logger.Level.WARN;
 
 /**
@@ -562,7 +565,7 @@ void cannotResolveNonNullableTransientDependencies(
 	void explicitSkipLockedLockCombo();
 
 	@LogMessage(level = INFO)
-	@Message(value = "'jakarta.persistence.validation.mode' named multiple values: %s", id = 448)
+	@Message(value = "'" + JAKARTA_VALIDATION_MODE + "' named multiple values: %s", id = 448)
 	void multipleValidationModes(String modes);
 
 	@LogMessage(level = WARN)
@@ -722,22 +725,22 @@ void cannotResolveNonNullableTransientDependencies(
 	void flushAndEvictOnRemove(String entityName);
 
 	@LogMessage(level = ERROR)
-	@Message(value = "Illegal argument on static metamodel field injection : %s#%s; expected type :  %s; encountered type : %s", id = 15007)
+	@Message(value = "Illegal argument on static metamodel field injection: %s#%s; expected type: %s; encountered type: %s", id = 15007)
 	void illegalArgumentOnStaticMetamodelFieldInjection(
 			String name,
 			String name2,
 			String name3,
 			String name4);
 
 	@LogMessage(level = WARN)
-	@Message(value = "Unable to locate static metamodel field : %s#%s; this may or may not indicate a problem with the static metamodel", id = 15011)
+	@Message(value = "Unable to locate static metamodel field: %s#%s; this may or may not indicate a problem with the static metamodel", id = 15011)
 	void unableToLocateStaticMetamodelField(
 			String name,
 			String name2);
 
 	@LogMessage(level = DEBUG)
-	@Message(value = "Returning null (as required by JPA spec) rather than throwing EntityNotFoundException, " +
-			"as the entity (type=%s, id=%s) does not exist", id = 15013)
+	@Message(value = "Returning null (as required by JPA spec) rather than throwing EntityNotFoundException " +
+			"since the entity of type '%s' with id [%s] does not exist", id = 15013)
 	void ignoringEntityNotFound(String entityName, String identifier);
 
 	@LogMessage(level = DEBUG)
@@ -754,6 +757,27 @@ void unableToLocateStaticMetamodelField(
 	)
 	void duplicatedPersistenceUnitName(String name);
 
+	@LogMessage(level = DEBUG)
+	@Message(
+			id = 455,
+			value =
+					"'" + CONNECTION_PROVIDER_DISABLES_AUTOCOMMIT + "' " +
+					"""
+					was enabled. This setting should only be enabled when JDBC Connections obtained by Hibernate \
+					from the ConnectionProvider have auto-commit disabled. Enabling this setting when connections \
+					have auto-commit enabled leads to execution of SQL operations outside of any JDBC transaction.\
+					"""
+	)
+	void connectionProviderDisablesAutoCommitEnabled();
+
+	@LogMessage(level = TRACE)
+	@Message(value = "Closing logical connection", id = 456)
+	void closingLogicalConnection();
+
+	@LogMessage(level = TRACE)
+	@Message(value = "Logical connection closed", id = 457)
+	void logicalConnectionClosed();
+
 	@LogMessage(level = DEBUG)
 	@Message(
 			id = 401,

hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/LogicalConnectionManagedImpl.java

@@ -14,14 +14,14 @@
 import org.hibernate.engine.jdbc.connections.spi.JdbcConnectionAccess;
 import org.hibernate.engine.jdbc.spi.JdbcServices;
 import org.hibernate.engine.jdbc.spi.SqlExceptionHelper;
+import org.hibernate.internal.CoreLogging;
+import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.resource.jdbc.LogicalConnection;
 import org.hibernate.resource.jdbc.ResourceRegistry;
 import org.hibernate.resource.jdbc.spi.JdbcEventHandler;
 import org.hibernate.resource.jdbc.spi.JdbcSessionContext;
 import org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode;
 
-import org.jboss.logging.Logger;
-
 import static org.hibernate.ConnectionAcquisitionMode.IMMEDIATELY;
 import static org.hibernate.ConnectionReleaseMode.AFTER_STATEMENT;
 import static org.hibernate.ConnectionReleaseMode.BEFORE_TRANSACTION_COMPLETION;
@@ -37,7 +37,7 @@
  * @author Steve Ebersole
  */
 public class LogicalConnectionManagedImpl extends AbstractLogicalConnectionImplementor {
-	private static final Logger log = Logger.getLogger( LogicalConnectionManagedImpl.class );
+	private static final CoreMessageLogger log = CoreLogging.messageLogger( LogicalConnectionManagedImpl.class );
 
 	private final transient JdbcConnectionAccess jdbcConnectionAccess;
 	private final transient JdbcEventHandler jdbcEventHandler;
@@ -72,13 +72,7 @@ public LogicalConnectionManagedImpl(
 
 		this.providerDisablesAutoCommit = jdbcSessionContext.doesConnectionProviderDisableAutoCommit();
 		if ( providerDisablesAutoCommit ) {
-			log.debug(
-					"`hibernate.connection.provider_disables_autocommit` was enabled.  This setting should only be " +
-							"enabled when you are certain that the Connections given to Hibernate by the " +
-							"ConnectionProvider have auto-commit disabled.  Enabling this setting when the " +
-							"Connections do not have auto-commit disabled will lead to Hibernate executing " +
-							"SQL operations outside of any JDBC/SQL transaction."
-			);
+			log.connectionProviderDisablesAutoCommitEnabled();
 		}
 	}
 
@@ -98,12 +92,10 @@ public LogicalConnectionManagedImpl(
 	private PhysicalConnectionHandlingMode determineConnectionHandlingMode(
 			PhysicalConnectionHandlingMode connectionHandlingMode,
 			JdbcConnectionAccess jdbcConnectionAccess) {
-		if ( connectionHandlingMode.getReleaseMode() == AFTER_STATEMENT
-				&& !jdbcConnectionAccess.supportsAggressiveRelease() ) {
-			return DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION;
-		}
-
-		return connectionHandlingMode;
+		return connectionHandlingMode.getReleaseMode() == AFTER_STATEMENT
+			&& !jdbcConnectionAccess.supportsAggressiveRelease()
+				? DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION
+				: connectionHandlingMode;
 	}
 
 	private LogicalConnectionManagedImpl(
@@ -159,7 +151,6 @@ public Connection getPhysicalConnection() {
 	@Override
 	public void afterStatement() {
 		super.afterStatement();
-
 		if ( connectionHandlingMode.getReleaseMode() == AFTER_STATEMENT ) {
 			if ( getResourceRegistry().hasRegisteredResources() ) {
 				log.debug( "Skipping aggressive release of JDBC Connection after-statement due to held resources" );
@@ -183,7 +174,6 @@ public void beforeTransactionCompletion() {
 	@Override
 	public void afterTransaction() {
 		super.afterTransaction();
-
 		if ( connectionHandlingMode.getReleaseMode() != ON_CLOSE ) {
 			// NOTE : we check for !ON_CLOSE here (rather than AFTER_TRANSACTION) to also catch:
 			// - AFTER_STATEMENT cases that were circumvented due to held resources
@@ -199,51 +189,48 @@ public Connection manualDisconnect() {
 		if ( closed ) {
 			throw new ResourceClosedException( "Logical connection is closed" );
 		}
-		final Connection c = physicalConnection;
+		final Connection connection = physicalConnection;
 		releaseConnection();
-		return c;
+		return connection;
 	}
 
 	@Override
 	public void manualReconnect(Connection suppliedConnection) {
 		if ( closed ) {
 			throw new ResourceClosedException( "Logical connection is closed" );
 		}
-
 		throw new IllegalStateException( "Cannot manually reconnect unless Connection was originally supplied by user" );
 	}
 
 	private void releaseConnection() {
-		final Connection localVariableConnection = this.physicalConnection;
-		if ( localVariableConnection == null ) {
-			return;
-		}
-
-		// We need to set the connection to null before we release resources,
-		// in order to prevent recursion into this method.
-		// Recursion can happen when we release resources and when batch statements are in progress:
-		// when releasing resources, we'll abort the batch statement,
-		// which will trigger "logicalConnection.afterStatement()",
-		// which in some configurations will release the connection.
-		this.physicalConnection = null;
-		try {
+		final Connection localVariableConnection = physicalConnection;
+		if ( localVariableConnection != null ) {
+			// We need to set the connection to null before we release resources,
+			// in order to prevent recursion into this method.
+			// Recursion can happen when we release resources and when batch statements are in progress:
+			// when releasing resources, we'll abort the batch statement,
+			// which will trigger "logicalConnection.afterStatement()",
+			// which in some configurations will release the connection.
+			physicalConnection = null;
 			try {
-				getResourceRegistry().releaseResources();
-				if ( !localVariableConnection.isClosed() ) {
-					sqlExceptionHelper.logAndClearWarnings( localVariableConnection );
+				try {
+					getResourceRegistry().releaseResources();
+					if ( !localVariableConnection.isClosed() ) {
+						sqlExceptionHelper.logAndClearWarnings( localVariableConnection );
+					}
+				}
+				finally {
+					jdbcEventHandler.jdbcConnectionReleaseStart();
+					jdbcConnectionAccess.releaseConnection( localVariableConnection );
 				}
 			}
+			catch (SQLException e) {
+				throw sqlExceptionHelper.convert( e, "Unable to release JDBC Connection" );
+			}
 			finally {
-				jdbcEventHandler.jdbcConnectionReleaseStart();
-				jdbcConnectionAccess.releaseConnection( localVariableConnection );
+				jdbcEventHandler.jdbcConnectionReleaseEnd();
 			}
 		}
-		catch (SQLException e) {
-			throw sqlExceptionHelper.convert( e, "Unable to release JDBC Connection" );
-		}
-		finally {
-			jdbcEventHandler.jdbcConnectionReleaseEnd();
-		}
 	}
 
 	@Override
@@ -261,20 +248,17 @@ public static LogicalConnectionManagedImpl deserialize(
 
 	@Override
 	public Connection close() {
-		if ( closed ) {
-			return null;
-		}
-
-		getResourceRegistry().releaseResources();
-
-		log.trace( "Closing logical connection" );
-		try {
-			releaseConnection();
-		}
-		finally {
-			// no matter what
-			closed = true;
-			log.trace( "Logical connection closed" );
+		if ( !closed ) {
+			getResourceRegistry().releaseResources();
+			log.closingLogicalConnection();
+			try {
+				releaseConnection();
+			}
+			finally {
+				// no matter what
+				closed = true;
+				log.logicalConnectionClosed();
+			}
 		}
 		return null;
 	}
@@ -300,7 +284,6 @@ public void begin() {
 	protected void afterCompletion() {
 		resetConnection( initiallyAutoCommit );
 		initiallyAutoCommit = false;
-
 		afterTransaction();
 	}
 

hibernate-core/src/main/java/org/hibernate/resource/jdbc/internal/LogicalConnectionProvidedImpl.java

@@ -9,17 +9,19 @@
 import java.io.ObjectOutputStream;
 import java.sql.Connection;
 
+import org.hibernate.internal.CoreLogging;
+import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.resource.jdbc.LogicalConnection;
 import org.hibernate.resource.jdbc.ResourceRegistry;
 import org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode;
 
-import org.jboss.logging.Logger;
+import static org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode.IMMEDIATE_ACQUISITION_AND_HOLD;
 
 /**
  * @author Steve Ebersole
  */
 public class LogicalConnectionProvidedImpl extends AbstractLogicalConnectionImplementor {
-	private static final Logger log = Logger.getLogger( LogicalConnection.class );
+	private static final CoreMessageLogger log = CoreLogging.messageLogger( LogicalConnection.class );
 
 	private transient Connection providedConnection;
 	private final boolean initiallyAutoCommit;
@@ -30,7 +32,6 @@ public LogicalConnectionProvidedImpl(Connection providedConnection, ResourceRegi
 		if ( providedConnection == null ) {
 			throw new IllegalArgumentException( "Provided Connection cannot be null" );
 		}
-
 		this.providedConnection = providedConnection;
 		this.initiallyAutoCommit = determineInitialAutoCommitMode( providedConnection );
 	}
@@ -43,7 +44,7 @@ private LogicalConnectionProvidedImpl(boolean closed, boolean initiallyAutoCommi
 
 	@Override
 	public PhysicalConnectionHandlingMode getConnectionHandlingMode() {
-		return PhysicalConnectionHandlingMode.IMMEDIATE_ACQUISITION_AND_HOLD;
+		return IMMEDIATE_ACQUISITION_AND_HOLD;
 	}
 
 	@Override
@@ -53,17 +54,15 @@ public boolean isOpen() {
 
 	@Override
 	public Connection close() {
-		log.trace( "Closing logical connection" );
-
+		log.closingLogicalConnection();
 		getResourceRegistry().releaseResources();
-
 		try {
 			return providedConnection;
 		}
 		finally {
 			providedConnection = null;
 			closed = true;
-			log.trace( "Logical connection closed" );
+			log.logicalConnectionClosed();
 		}
 	}
 
@@ -99,14 +98,13 @@ public Connection manualDisconnect() {
 			return providedConnection;
 		}
 		finally {
-			this.providedConnection = null;
+			providedConnection = null;
 		}
 	}
 
 	@Override
 	public void manualReconnect(Connection connection) {
 		errorIfClosed();
-
 		if ( connection == null ) {
 			throw new IllegalArgumentException( "cannot reconnect using a null connection" );
 		}
@@ -131,7 +129,6 @@ protected Connection getConnectionForTransactionManagement() {
 	@Override
 	protected void afterCompletion() {
 		afterTransaction();
-
 		resetConnection( initiallyAutoCommit );
 	}
 }

tooling/hibernate-ant/src/main/java/org/hibernate/tool/hbm2ddl/ConnectionHelper.java

@@ -22,22 +22,18 @@ public interface ConnectionHelper {
 	 *
 	 * @param needsAutoCommit Should connection be forced to auto-commit
 	 * if not already.
-	 * @throws SQLException
 	 */
-	public void prepare(boolean needsAutoCommit) throws SQLException;
+	void prepare(boolean needsAutoCommit) throws SQLException;
 
 	/**
 	 * Get a reference to the connection we are using.
 	 *
 	 * @return The JDBC connection.
-	 * @throws SQLException
 	 */
-	public Connection getConnection() throws SQLException;
+	Connection getConnection() throws SQLException;
 
 	/**
 	 * Release any resources held by this helper.
-	 *
-	 * @throws SQLException
 	 */
-	public void release() throws SQLException;
+	void release() throws SQLException;
 }