some cleanup to DatasourceConnectionProviderImpl and multi-tenant version

Signed-off-by: Gavin King <[email protected]>

Author: gavinking

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

@@ -526,8 +526,9 @@ public interface JdbcSettings extends C3p0Settings, ProxoolSettings, AgroalSetti
 	/**
 	 * @see javax.sql.DataSource
 	 *
-	 * @deprecated The JPA-standard {@link #JAKARTA_JTA_DATASOURCE} or {@link #JAKARTA_JTA_DATASOURCE} setting
-	 * is now preferred.
+	 * @deprecated The JPA-standard {@value #JAKARTA_JTA_DATASOURCE} or
+	 *             {@value #JAKARTA_NON_JTA_DATASOURCE} setting are now
+	 *             preferred.
 	 */
 	@Deprecated
 	String DATASOURCE = "hibernate.connection.datasource";

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

@@ -12,7 +12,8 @@
 import javax.sql.DataSource;
 
 import org.hibernate.HibernateException;
-import org.hibernate.cfg.Environment;
+import org.hibernate.cfg.AvailableSettings;
+import org.hibernate.cfg.JdbcSettings;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider;
 import org.hibernate.engine.jdbc.connections.spi.DatabaseConnectionInfo;
@@ -22,13 +23,17 @@
 import org.hibernate.service.spi.InjectService;
 import org.hibernate.service.spi.Stoppable;
 
+import static org.hibernate.cfg.JdbcSettings.DATASOURCE;
+
 /**
  * A {@link ConnectionProvider} that manages connections from an underlying {@link DataSource}.
  * <p>
  * The {@link DataSource} to use may be specified by either:<ul>
- * <li>injection using {@link #setDataSource}
- * <li>declaring the {@link DataSource} instance using the {@value Environment#DATASOURCE} config property
- * <li>declaring the JNDI name under which the {@link DataSource} is found via the {@value Environment#DATASOURCE} config property
+ * <li>injection using {@link #setDataSource},
+ * <li>passing the {@link DataSource} instance using {@value JdbcSettings#DATASOURCE},
+ *     {@value JdbcSettings#JAKARTA_JTA_DATASOURCE}, or {@value JdbcSettings#JAKARTA_NON_JTA_DATASOURCE}, or
+ * <li>declaring the JNDI name under which the {@link DataSource} is found via {@value JdbcSettings#DATASOURCE},
+ *     {@value JdbcSettings#JAKARTA_JTA_DATASOURCE}, or {@value JdbcSettings#JAKARTA_NON_JTA_DATASOURCE}.
  * </ul>
  *
  * @author Gavin King
@@ -83,15 +88,15 @@ else if ( DataSource.class.isAssignableFrom( unwrapType ) ) {
 	@Override
 	public void configure(Map<String, Object> configValues) {
 		if ( dataSource == null ) {
-			final Object dataSourceSetting = configValues.get( Environment.DATASOURCE );
+			final Object dataSourceSetting = configValues.get( DATASOURCE );
 			if ( dataSourceSetting instanceof DataSource ) {
 				dataSource = (DataSource) dataSourceSetting;
 			}
 			else {
 				final String dataSourceJndiName = (String) dataSourceSetting;
 				if ( dataSourceJndiName == null ) {
 					throw new HibernateException(
-							"DataSource to use was not injected nor specified by [" + Environment.DATASOURCE
+							"DataSource to use was not injected nor specified by [" + DATASOURCE
 									+ "] configuration property"
 					);
 				}
@@ -106,8 +111,8 @@ public void configure(Map<String, Object> configValues) {
 			throw new HibernateException( "Unable to determine appropriate DataSource to use" );
 		}
 
-		user = (String) configValues.get( Environment.USER );
-		pass = (String) configValues.get( Environment.PASS );
+		user = (String) configValues.get( AvailableSettings.USER );
+		pass = (String) configValues.get( AvailableSettings.PASS );
 		useCredentials = user != null || pass != null;
 		available = true;
 	}
@@ -150,7 +155,7 @@ public DatabaseConnectionInfo getDatabaseConnectionInfo(Dialect dialect) {
 			@Override
 			public String toInfoString() {
 				return dataSourceJndiName != null
-						? "\tDatasource JND name [" + dataSourceJndiName + "]"
+						? "\tDatasource JNDI name [" + dataSourceJndiName + "]"
 						: "\tProvided DataSource";
 			}
 		};

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

@@ -22,19 +22,20 @@
 import org.hibernate.service.spi.ServiceRegistryImplementor;
 import org.hibernate.service.spi.Stoppable;
 
+import static org.hibernate.cfg.JdbcSettings.DATASOURCE;
 import static org.hibernate.cfg.MultiTenancySettings.TENANT_IDENTIFIER_TO_USE_FOR_ANY_KEY;
 
 /**
- * A concrete implementation of the {@link MultiTenantConnectionProvider} contract bases on a number of
- * reasonable assumptions.  We assume that:<ul>
+ * A concrete implementation of the {@link MultiTenantConnectionProvider} contract bases on
+ * a number of reasonable assumptions. We assume that:<ul>
  *     <li>
- *         The {@link DataSource} instances are all available from JNDI named by the tenant identifier relative
- *         to a single base JNDI context
+ *         The {@link DataSource} instances are all available from JNDI named by the tenant
+ *         identifier relative to a single base JNDI context.
  *     </li>
  *     <li>
  *         {@value AvailableSettings#DATASOURCE} is a string naming either the {@literal any}
- *         data source or the base JNDI context.  If the latter, {@link MultiTenancySettings#TENANT_IDENTIFIER_TO_USE_FOR_ANY_KEY} must
- *         also be set.
+ *         data source or the base JNDI context. If the latter,
+ *         {@link MultiTenancySettings#TENANT_IDENTIFIER_TO_USE_FOR_ANY_KEY} must also be set.
  *     </li>
  * </ul>
  *
@@ -44,10 +45,11 @@ public class DataSourceBasedMultiTenantConnectionProviderImpl<T>
 		extends AbstractDataSourceBasedMultiTenantConnectionProviderImpl<T>
 		implements ServiceRegistryAwareService, Stoppable {
 
-	private Map<T, DataSource> dataSourceMap;
+	private final Map<T, DataSource> dataSourceMap = new ConcurrentHashMap<>();
 	private JndiService jndiService;
 	private T tenantIdentifierForAny;
 	private String baseJndiNamespace;
+	private String jndiName;
 
 	@Override
 	protected DataSource selectAnyDataSource() {
@@ -65,21 +67,17 @@ protected DataSource selectDataSource(T tenantIdentifier) {
 	}
 
 	private Map<T, DataSource> dataSourceMap() {
-		if ( dataSourceMap == null ) {
-			dataSourceMap = new ConcurrentHashMap<>();
-		}
 		return dataSourceMap;
 	}
 
 	@Override
 	public void injectServices(ServiceRegistryImplementor serviceRegistry) {
-		final Object dataSourceConfigValue =
-				serviceRegistry.requireService( ConfigurationService.class )
-						.getSettings().get( AvailableSettings.DATASOURCE );
+		final ConfigurationService configurationService = serviceRegistry.requireService( ConfigurationService.class );
+		final Object dataSourceConfigValue = configurationService.getSettings().get( DATASOURCE );
 		if ( !(dataSourceConfigValue instanceof String) ) {
-			throw new HibernateException( "Improper set up of DataSourceBasedMultiTenantConnectionProviderImpl" );
+			throw new HibernateException( "illegal value for configuration setting '" + DATASOURCE + "'" );
 		}
-		final String jndiName = (String) dataSourceConfigValue;
+		jndiName = (String) dataSourceConfigValue;
 
 		jndiService = serviceRegistry.getService( JndiService.class );
 		if ( jndiService == null ) {
@@ -91,17 +89,18 @@ public void injectServices(ServiceRegistryImplementor serviceRegistry) {
 			throw new HibernateException( "JNDI name [" + jndiName + "] could not be resolved" );
 		}
 
-		if ( namedObject instanceof DataSource ) {
+		if ( namedObject instanceof DataSource datasource ) {
 			final int loc = jndiName.lastIndexOf( '/' );
-			this.baseJndiNamespace = jndiName.substring( 0, loc );
-			this.tenantIdentifierForAny = (T) jndiName.substring( loc + 1 );
-			dataSourceMap().put( tenantIdentifierForAny, (DataSource) namedObject );
+			baseJndiNamespace = jndiName.substring( 0, loc );
+			final String prefix = jndiName.substring(loc + 1);
+			tenantIdentifierForAny = (T) prefix;
+			dataSourceMap().put( tenantIdentifierForAny, datasource );
 		}
 		else if ( namedObject instanceof Context ) {
-			this.baseJndiNamespace = jndiName;
-			this.tenantIdentifierForAny = (T) serviceRegistry.requireService( ConfigurationService.class )
-					.getSettings()
-					.get( TENANT_IDENTIFIER_TO_USE_FOR_ANY_KEY );
+			baseJndiNamespace = jndiName;
+			final Object configuredTenantId =
+					configurationService.getSettings().get( TENANT_IDENTIFIER_TO_USE_FOR_ANY_KEY );
+			tenantIdentifierForAny = (T) configuredTenantId;
 			if ( tenantIdentifierForAny == null ) {
 				throw new HibernateException( "JNDI name named a Context, but tenant identifier to use for ANY was not specified" );
 			}
@@ -116,10 +115,7 @@ else if ( namedObject instanceof Context ) {
 
 	@Override
 	public void stop() {
-		if ( dataSourceMap != null ) {
-			dataSourceMap.clear();
-			dataSourceMap = null;
-		}
+		dataSourceMap.clear();
 	}
 
 	@Override
@@ -139,5 +135,4 @@ public String toInfoString() {
 			}
 		};
 	}
-
 }

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

@@ -163,11 +163,12 @@ else if ( explicitDialectConfiguration( explicitDatabaseName, configurationValue
 
 	private DatabaseConnectionInfo buildDbInfo(ServiceRegistryImplementor registry, Dialect dialect) {
 		if ( !isMultiTenancyEnabled( registry ) ) {
-			final ConnectionProvider cp = registry.requireService( ConnectionProvider.class );
-			return cp.getDatabaseConnectionInfo( dialect );
+			return registry.requireService( ConnectionProvider.class )
+					.getDatabaseConnectionInfo( dialect );
 		}
 		else {
-			final MultiTenantConnectionProvider<?> mcp = registry.requireService( MultiTenantConnectionProvider.class );
+			final MultiTenantConnectionProvider<?> mcp =
+					registry.requireService( MultiTenantConnectionProvider.class );
 			return mcp.getDatabaseConnectionInfo( dialect );
 		}
 	}