From e78c28403baef61e3560ce3c07b7cb802e40befb Mon Sep 17 00:00:00 2001 From: Gavin King Date: Tue, 5 Aug 2025 09:38:08 +1000 Subject: [PATCH] some refactoring to DriverManagerConnectionProvider - clean up initialization logic which was a mess - use ServiceRegistry instead of ServiceRegistryImplementor --- .../internal/BasicConnectionCreator.java | 6 +- .../internal/ConnectionCreatorFactory.java | 4 +- .../ConnectionCreatorFactoryImpl.java | 4 +- .../internal/DatabaseConnectionInfoImpl.java | 3 +- .../internal/DriverConnectionCreator.java | 4 +- .../DriverManagerConnectionCreator.java | 4 +- .../DriverManagerConnectionProviderImpl.java | 133 +++++++++--------- .../internal/log/ConnectionInfoLogger.java | 4 + 8 files changed, 81 insertions(+), 81 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/BasicConnectionCreator.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/BasicConnectionCreator.java index 48751bc2a4a3..4462db97b1f1 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/BasicConnectionCreator.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/BasicConnectionCreator.java @@ -16,8 +16,8 @@ import org.hibernate.exception.internal.SQLStateConversionDelegate; import org.hibernate.exception.spi.SQLExceptionConversionDelegate; import org.hibernate.internal.util.ValueHolder; +import org.hibernate.service.ServiceRegistry; import org.hibernate.service.spi.ServiceException; -import org.hibernate.service.spi.ServiceRegistryImplementor; /** * Template (as in template pattern) support for {@link ConnectionCreator} implementors. @@ -25,7 +25,7 @@ * @author Steve Ebersole */ public abstract class BasicConnectionCreator implements ConnectionCreator { - private final ServiceRegistryImplementor serviceRegistry; + private final ServiceRegistry serviceRegistry; private final String url; private final Properties connectionProps; @@ -35,7 +35,7 @@ public abstract class BasicConnectionCreator implements ConnectionCreator { private final String initSql; public BasicConnectionCreator( - ServiceRegistryImplementor serviceRegistry, + ServiceRegistry serviceRegistry, String url, Properties connectionProps, boolean autocommit, diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/ConnectionCreatorFactory.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/ConnectionCreatorFactory.java index 858c0ed9073a..87cb03b6de99 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/ConnectionCreatorFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/ConnectionCreatorFactory.java @@ -8,7 +8,7 @@ import java.util.Map; import java.util.Properties; -import org.hibernate.service.spi.ServiceRegistryImplementor; +import org.hibernate.service.ServiceRegistry; /** * A factory for {@link ConnectionCreator}. @@ -19,7 +19,7 @@ public interface ConnectionCreatorFactory { ConnectionCreator create( Driver driver, - ServiceRegistryImplementor serviceRegistry, + ServiceRegistry serviceRegistry, String url, Properties connectionProps, Boolean autocommit, diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/ConnectionCreatorFactoryImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/ConnectionCreatorFactoryImpl.java index 8db8d0f4c7c6..7b9507123327 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/ConnectionCreatorFactoryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/ConnectionCreatorFactoryImpl.java @@ -8,7 +8,7 @@ import java.util.Map; import java.util.Properties; -import org.hibernate.service.spi.ServiceRegistryImplementor; +import org.hibernate.service.ServiceRegistry; /** * The default factory for {@link ConnectionCreator} instances. @@ -25,7 +25,7 @@ private ConnectionCreatorFactoryImpl() { @Override public ConnectionCreator create( Driver driver, - ServiceRegistryImplementor serviceRegistry, + ServiceRegistry serviceRegistry, String url, Properties connectionProps, Boolean autoCommit, diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DatabaseConnectionInfoImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DatabaseConnectionInfoImpl.java index c51a98129539..5a9d96110b7a 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DatabaseConnectionInfoImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DatabaseConnectionInfoImpl.java @@ -52,7 +52,8 @@ public DatabaseConnectionInfoImpl( Class connectionProviderClass, String jdbcUrl, String jdbcDriver, - Class dialectClass, DatabaseVersion dialectVersion, + Class dialectClass, + DatabaseVersion dialectVersion, boolean hasSchema, boolean hasCatalog, String schema, diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverConnectionCreator.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverConnectionCreator.java index 4530ed4954a6..fbc41f111843 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverConnectionCreator.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverConnectionCreator.java @@ -9,7 +9,7 @@ import java.sql.SQLException; import java.util.Properties; -import org.hibernate.service.spi.ServiceRegistryImplementor; +import org.hibernate.service.ServiceRegistry; /** * A specialized {@link ConnectionCreator} which uses {@link Driver#connect(String, Properties)} @@ -22,7 +22,7 @@ public class DriverConnectionCreator extends BasicConnectionCreator { public DriverConnectionCreator( Driver driver, - ServiceRegistryImplementor serviceRegistry, + ServiceRegistry serviceRegistry, String url, Properties connectionProps, Boolean autocommit, diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverManagerConnectionCreator.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverManagerConnectionCreator.java index 06211da272af..2f70618fb657 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverManagerConnectionCreator.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverManagerConnectionCreator.java @@ -9,7 +9,7 @@ import java.sql.SQLException; import java.util.Properties; -import org.hibernate.service.spi.ServiceRegistryImplementor; +import org.hibernate.service.ServiceRegistry; /** * A specialized {@link ConnectionCreator} which uses {@link DriverManager#getConnection(String, Properties)} @@ -19,7 +19,7 @@ */ public class DriverManagerConnectionCreator extends BasicConnectionCreator { public DriverManagerConnectionCreator( - ServiceRegistryImplementor serviceRegistry, + ServiceRegistry serviceRegistry, String url, Properties connectionProps, Boolean autocommit, diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverManagerConnectionProviderImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverManagerConnectionProviderImpl.java index b581a687bb3a..b011b96d43e5 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverManagerConnectionProviderImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/connections/internal/DriverManagerConnectionProviderImpl.java @@ -19,6 +19,7 @@ import org.hibernate.engine.jdbc.connections.spi.ConnectionProviderConfigurationException; import org.hibernate.engine.jdbc.connections.spi.DatabaseConnectionInfo; import org.hibernate.exception.JDBCConnectionException; +import org.hibernate.service.ServiceRegistry; import org.hibernate.service.UnknownUnwrapTypeException; import org.hibernate.service.spi.Configurable; import org.hibernate.service.spi.ServiceException; @@ -44,6 +45,7 @@ import static org.hibernate.internal.util.config.ConfigurationHelper.getBoolean; import static org.hibernate.internal.util.config.ConfigurationHelper.getInt; import static org.hibernate.internal.util.config.ConfigurationHelper.getLong; +import static org.hibernate.internal.util.config.ConfigurationHelper.getString; /** * A connection provider that uses the {@link DriverManager} directly to open connections and provides @@ -70,7 +72,7 @@ public class DriverManagerConnectionProviderImpl // create the pool ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - private volatile ServiceRegistryImplementor serviceRegistry; + private volatile ServiceRegistry serviceRegistry; @Override public void injectServices(ServiceRegistryImplementor serviceRegistry) { @@ -80,67 +82,33 @@ public void injectServices(ServiceRegistryImplementor serviceRegistry) { @Override public void configure(Map configurationValues) { ConnectionInfoLogger.INSTANCE.usingHibernateBuiltInConnectionPool(); - PooledConnections pool = buildPool( configurationValues, serviceRegistry ); + final PooledConnections pool = buildPool( configurationValues, serviceRegistry ); final long validationInterval = getLong( VALIDATION_INTERVAL, configurationValues, 30 ); state = new PoolState( pool, validationInterval ); } - private PooledConnections buildPool(Map configuration, ServiceRegistryImplementor serviceRegistry) { - final var creator = buildCreator( configuration, serviceRegistry ); + private PooledConnections buildPool(Map configuration, ServiceRegistry serviceRegistry) { + // connection settings + final String url = jdbcUrl( configuration ); + final String driverClassName = getString( DRIVER, configuration ); + final Properties connectionProps = getConnectionProperties( configuration ); final boolean autoCommit = getBoolean( AUTOCOMMIT, configuration ); // default autocommit to false + final Integer isolation = extractIsolation( configuration ); + final String initSql = getString( INIT_SQL, configuration ); + + // pool settings final int minSize = getInt( MIN_SIZE, configuration, 1 ); final int maxSize = getInt( POOL_SIZE, configuration, 20 ); final int initialSize = getInt( INITIAL_SIZE, configuration, minSize ); - return new PooledConnections.Builder( creator ) - .autoCommit( autoCommit ) - .initialSize( initialSize ) - .minSize( minSize ) - .maxSize( maxSize ) - .validator( this ) - .build(); - } - - private static ConnectionCreator buildCreator( - Map configurationValues, ServiceRegistryImplementor serviceRegistry) { - final String url = jdbcUrl( configurationValues ); - String driverClassName = (String) configurationValues.get( DRIVER ); - boolean success = false; - Driver driver = null; - if ( driverClassName != null ) { - driver = loadDriverIfPossible( driverClassName, serviceRegistry ); - success = true; - } - else { - //try to guess the driver class from the JDBC URL - for ( var database: Database.values() ) { - if ( database.matchesUrl( url ) ) { - driverClassName = database.getDriverClassName( url ); - if ( driverClassName != null ) { - try { - loadDriverIfPossible( driverClassName, serviceRegistry ); - success = true; - } - catch (Exception e) { - //swallow it, since this was not - //an explicit setting by the user - } - break; - } - } - } + final Driver driver = loadDriver( driverClassName, serviceRegistry, url ); + if ( driver == null ) { + //we're hoping that the driver is already loaded + logAvailableDrivers(); } - final String driverList = success ? driverClassName : driverList(); - - final Properties connectionProps = getConnectionProperties( configurationValues ); - - final boolean autoCommit = getBoolean( AUTOCOMMIT, configurationValues ); - final Integer isolation = extractIsolation( configurationValues ); - final String initSql = (String) configurationValues.get( INIT_SQL ); - final var connectionCreator = - getConnectionCreatorFactory( configurationValues, serviceRegistry ) + getConnectionCreatorFactory( configuration, serviceRegistry ) .create( driver, serviceRegistry, @@ -149,15 +117,14 @@ private static ConnectionCreator buildCreator( autoCommit, isolation, initSql, - configurationValues + configuration ); - try ( var connection = connectionCreator.createConnection() ) { dbInfo = new DatabaseConnectionInfoImpl( DriverManagerConnectionProviderImpl.class, url, - driverList, + driver == null ? null : driver.getClass().getName(), null, SimpleDatabaseVersion.ZERO_VERSION, hasSchema( connection ), @@ -168,8 +135,8 @@ private static ConnectionCreator buildCreator( isolation != null ? toIsolationNiceName( isolation ) : toIsolationNiceName( getIsolation( connection ) ), - getInt( MIN_SIZE, configurationValues, 1 ), - getInt( POOL_SIZE, configurationValues, 20 ), + minSize, + maxSize, getFetchSize( connection ) ); if ( !connection.getAutoCommit() ) { @@ -180,11 +147,39 @@ private static ConnectionCreator buildCreator( throw new JDBCConnectionException( "Could not create connection", e ); } - return connectionCreator; + return new PooledConnections.Builder( connectionCreator ) + .autoCommit( autoCommit ) + .initialSize( initialSize ) + .minSize( minSize ) + .maxSize( maxSize ) + .validator( this ) + .build(); + } + + private static Driver loadDriver(String driverClassName, ServiceRegistry serviceRegistry, String url) { + if ( driverClassName != null ) { + return loadDriverIfPossible( driverClassName, serviceRegistry ); + } + else { + // try to guess the driver class from the JDBC URL + for ( var database: Database.values() ) { + if ( database.matchesUrl( url ) ) { + final String databaseDriverClassName = database.getDriverClassName( url ); + if ( databaseDriverClassName != null ) { + try { + return loadDriverIfPossible( databaseDriverClassName, serviceRegistry ); + } + catch (Exception e) { + // swallow it, since this was not an explicit setting by the user + } + } + } + } + return null; + } } - private static String driverList() { - //we're hoping that the driver is already loaded + private static void logAvailableDrivers() { ConnectionInfoLogger.INSTANCE.jdbcDriverNotSpecified(); final var list = new StringBuilder(); DriverManager.drivers() @@ -194,11 +189,11 @@ private static String driverList() { } list.append( driver.getClass().getName() ); } ); - return list.toString(); + ConnectionInfoLogger.INSTANCE.availableJdbcDrivers( list.toString() ); } - private static String jdbcUrl(Map configurationValues) { - final String url = (String) configurationValues.get( URL ); + private static String jdbcUrl(Map configuration) { + final String url = (String) configuration.get( URL ); if ( url == null ) { throw new ConnectionProviderConfigurationException( "No JDBC URL specified by property '" + JAKARTA_JDBC_URL + "'" ); } @@ -206,8 +201,8 @@ private static String jdbcUrl(Map configurationValues) { } private static ConnectionCreatorFactory getConnectionCreatorFactory( - Map configurationValues, ServiceRegistryImplementor serviceRegistry) { - final Object connectionCreatorFactory = configurationValues.get( CONNECTION_CREATOR_FACTORY ); + Map configuration, ServiceRegistry serviceRegistry) { + final Object connectionCreatorFactory = configuration.get( CONNECTION_CREATOR_FACTORY ); final ConnectionCreatorFactory factory; if ( connectionCreatorFactory instanceof ConnectionCreatorFactory instance ) { factory = instance; @@ -221,7 +216,7 @@ else if ( connectionCreatorFactory != null ) { return factory == null ? ConnectionCreatorFactoryImpl.INSTANCE : factory; } - private static Driver loadDriverIfPossible(String driverClassName, ServiceRegistryImplementor serviceRegistry) { + private static Driver loadDriverIfPossible(String driverClassName, ServiceRegistry serviceRegistry) { if ( driverClassName == null ) { ConnectionInfoLogger.INSTANCE.debug( "No driver class specified" ); return null; @@ -248,7 +243,7 @@ else if ( serviceRegistry != null ) { } private static ConnectionCreatorFactory loadConnectionCreatorFactory( - String connectionCreatorFactoryClassName, ServiceRegistryImplementor serviceRegistry) { + String connectionCreatorFactoryClassName, ServiceRegistry serviceRegistry) { if ( connectionCreatorFactoryClassName == null ) { ConnectionInfoLogger.INSTANCE.debug( "No connection creator factory class specified" ); return null; @@ -261,7 +256,8 @@ else if ( serviceRegistry != null ) { return factoryClass.newInstance(); } catch ( Exception e ) { - throw new ServiceException( "Specified ConnectionCreatorFactory " + connectionCreatorFactoryClassName + " could not be loaded", e ); + throw new ServiceException( "Specified ConnectionCreatorFactory " + connectionCreatorFactoryClassName + + " could not be loaded", e ); } } else { @@ -269,9 +265,8 @@ else if ( serviceRegistry != null ) { return (ConnectionCreatorFactory) Class.forName( connectionCreatorFactoryClassName ).newInstance(); } catch (Exception e1) { - throw new ServiceException( - "Specified ConnectionCreatorFactory " + connectionCreatorFactoryClassName + " could not be loaded", - e1 ); + throw new ServiceException( "Specified ConnectionCreatorFactory " + connectionCreatorFactoryClassName + + " could not be loaded", e1 ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/log/ConnectionInfoLogger.java b/hibernate-core/src/main/java/org/hibernate/internal/log/ConnectionInfoLogger.java index 1896e4599048..394cc2ff3057 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/log/ConnectionInfoLogger.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/log/ConnectionInfoLogger.java @@ -55,6 +55,10 @@ public interface ConnectionInfoLogger extends BasicLogger { + JdbcSettings.DRIVER + "'") void jdbcDriverNotSpecified(); + @LogMessage(level = INFO) + @Message(id = 10001007, value = "Available JDBC drivers: [%s]") + void availableJdbcDrivers(String availableDrivers); + @LogMessage(level = DEBUG) @Message(value = "Cleaning up connection pool [%s]", id = 10001008) void cleaningUpConnectionPool(String info);