HHH-12858 - integration overrides during JPA bootstrap ought to override all logically related settings

- e.g. a datasource passed in the integration overrides map ought to effectively override JDBC-connection settings in `persistence.xml` (and vice-versa)

HHH-13432 - Have EntityManagerFactory expose persistence.xml `jta-data-source` element as a `javax.persistence.nonJtaDataSource` property

hibernate-core/src/main/java/org/hibernate/boot/cfgxml/spi/LoadedConfig.java

@@ -47,7 +47,7 @@ public class LoadedConfig {
 	private List<MappingReference> mappingReferences;
 	private Map<EventType,Set<String>> eventListenerMap;
 
-	private LoadedConfig(String sessionFactoryName) {
+	public LoadedConfig(String sessionFactoryName) {
 		this.sessionFactoryName = sessionFactoryName;
 	}
 
@@ -259,7 +259,7 @@ public class LoadedConfig {
 	}
 
 	@SuppressWarnings("unchecked")
-	private void addConfigurationValues(Map configurationValues) {
+	protected void addConfigurationValues(Map configurationValues) {
 		if ( configurationValues == null ) {
 			return;
 		}

hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/FilterSourceImpl.java

@@ -14,6 +14,7 @@ import org.hibernate.boot.MappingException;
 import org.hibernate.boot.jaxb.hbm.spi.JaxbHbmFilterAliasMappingType;
 import org.hibernate.boot.jaxb.hbm.spi.JaxbHbmFilterType;
 import org.hibernate.boot.model.source.spi.FilterSource;
+import org.hibernate.internal.util.NullnessHelper;
 import org.hibernate.internal.util.StringHelper;
 
 /**
@@ -63,7 +64,7 @@ public class FilterSourceImpl
 			}
 		}
 
-		this.condition = Helper.coalesce( conditionContent, conditionAttribute );
+		this.condition = NullnessHelper.coalesce( conditionContent, conditionAttribute );
 		this.autoAliasInjection = StringHelper.isNotEmpty( explicitAutoAliasInjectionSetting )
 				? Boolean.valueOf( explicitAutoAliasInjectionSetting )
 				: true;

hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/Helper.java

@@ -123,33 +123,6 @@ public class Helper {
 		return params;
 	}
 
-	/**
-	 * Operates like SQL coalesce expression, except empty strings are treated as null.  Return the first non-empty value
-	 *
-	 * @param values The list of values.
-	 * @param <T> Generic type of values to coalesce
-	 *
-	 * @return The first non-empty value, or null if all values were empty
-	 */
-	public static <T> T coalesce(T... values) {
-		if ( values == null ) {
-			return null;
-		}
-		for ( T value : values ) {
-			if ( value != null ) {
-				if ( String.class.isInstance( value ) ) {
-					if ( StringHelper.isNotEmpty( (String) value ) ) {
-						return value;
-					}
-				}
-				else {
-					return value;
-				}
-			}
-		}
-		return null;
-	}
-
 	static ToolingHintContext collectToolingHints(
 			ToolingHintContext baseline,
 			ToolingHintContainer toolingHintContainer) {

hibernate-core/src/main/java/org/hibernate/boot/registry/StandardServiceRegistryBuilder.java

@@ -36,6 +36,35 @@ import org.hibernate.service.spi.ServiceContributor;
  * @see org.hibernate.boot.registry.BootstrapServiceRegistryBuilder
  */
 public class StandardServiceRegistryBuilder {
+	/**
+	 * Intended only for use from {@link org.hibernate.jpa.boot.internal.EntityManagerFactoryBuilderImpl}.
+	 *
+	 * Creates a StandardServiceRegistryBuilder specific to the needs of JPA bootstrapping.
+	 * Specifically we ignore properties found in `cfg.xml` files in terms of adding them to
+	 * the builder immediately.  EntityManagerFactoryBuilderImpl handles collecting these
+	 * properties itself.
+	 */
+	public static StandardServiceRegistryBuilder forJpa(BootstrapServiceRegistry bootstrapServiceRegistry) {
+		final LoadedConfig loadedConfig = new LoadedConfig( null ) {
+			@Override
+			protected void addConfigurationValues(Map configurationValues) {
+				// here, do nothing
+			}
+		};
+		return new StandardServiceRegistryBuilder(
+				bootstrapServiceRegistry,
+				new HashMap(),
+				loadedConfig
+		) {
+			@Override
+			public StandardServiceRegistryBuilder configure(LoadedConfig loadedConfig) {
+				getAggregatedCfgXml().merge( loadedConfig );
+				// super also collects the properties - here we skip that part
+				return this;
+			}
+		};
+	}
+
 	/**
 	 * The default resource name for a hibernate configuration xml file.
 	 */
@@ -43,7 +72,7 @@ public class StandardServiceRegistryBuilder {
 
 	private final Map settings;
 	private final List<StandardServiceInitiator> initiators = standardInitiatorList();
-	private final List<ProvidedService> providedServices = new ArrayList<ProvidedService>();
+	private final List<ProvidedService> providedServices = new ArrayList<>();
 
 	private boolean autoCloseRegistry = true;
 
@@ -67,6 +96,21 @@ public class StandardServiceRegistryBuilder {
 		this( bootstrapServiceRegistry, LoadedConfig.baseline() );
 	}
 
+	/**
+	 * Intended for use exclusively from JPA boot-strapping.
+	 *
+	 * @see #forJpa
+	 */
+	private StandardServiceRegistryBuilder(
+			BootstrapServiceRegistry bootstrapServiceRegistry,
+			Map settings,
+			LoadedConfig loadedConfig) {
+		this.bootstrapServiceRegistry = bootstrapServiceRegistry;
+		this.configLoader = new ConfigLoader( bootstrapServiceRegistry );
+		this.settings = settings;
+		this.aggregatedCfgXml = loadedConfig;
+	}
+
 	/**
 	 * Create a builder with the specified bootstrap services.
 	 *
@@ -81,6 +125,10 @@ public class StandardServiceRegistryBuilder {
 		this.aggregatedCfgXml = loadedConfigBaseline;
 	}
 
+	public ConfigLoader getConfigLoader() {
+		return configLoader;
+	}
+
 	/**
 	 * Intended for internal testing use only!!
 	 */
@@ -94,11 +142,12 @@ public class StandardServiceRegistryBuilder {
 	 * @return List of standard initiators
 	 */
 	private static List<StandardServiceInitiator> standardInitiatorList() {
-		final List<StandardServiceInitiator> initiators = new ArrayList<StandardServiceInitiator>( StandardServiceInitiators.LIST.size() );
+		final List<StandardServiceInitiator> initiators = new ArrayList<>( StandardServiceInitiators.LIST.size() );
 		initiators.addAll( StandardServiceInitiators.LIST );
 		return initiators;
 	}
 
+	@SuppressWarnings("unused")
 	public BootstrapServiceRegistry getBootstrapServiceRegistry() {
 		return bootstrapServiceRegistry;
 	}

hibernate-core/src/main/java/org/hibernate/internal/util/NullnessHelper.java

@@ -0,0 +1,60 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.internal.util;
+
+import java.util.function.Supplier;
+
+/**
+ * @author Steve Ebersole
+ */
+public class NullnessHelper {
+
+	/**
+	 * Operates like SQL coalesce expression, except empty strings are treated as null.  Return the first non-empty value
+	 *
+	 * @param values The list of values.
+	 * @param <T> Generic type of values to coalesce
+	 *
+	 * @return The first non-empty value, or null if all values were empty
+	 */
+	public static <T> T coalesce(T... values) {
+		if ( values == null ) {
+			return null;
+		}
+		for ( T value : values ) {
+			if ( value != null ) {
+				if ( String.class.isInstance( value ) ) {
+					if ( !( (String) value ).isEmpty() ) {
+						return value;
+					}
+				}
+				else {
+					return value;
+				}
+			}
+		}
+		return null;
+	}
+
+	/**
+	 * Find the first non-null value supplied by the given suppliers
+	 */
+	public static <T> T coalesceSuppliedValues(Supplier<T>... valueSuppliers) {
+		if ( valueSuppliers == null ) {
+			return null;
+		}
+
+		for ( Supplier<T> valueSupplier : valueSuppliers ) {
+			final T value = valueSupplier.get();
+			if ( value != null ) {
+					return value;
+			}
+		}
+
+		return null;
+	}
+}

hibernate-core/src/main/java/org/hibernate/jpa/boot/internal/EntityManagerFactoryBuilderImpl.java

@@ -27,11 +27,9 @@ import javax.sql.DataSource;
 import org.hibernate.SessionFactory;
 import org.hibernate.SessionFactoryObserver;
 import org.hibernate.boot.CacheRegionDefinition;
-import org.hibernate.boot.MetadataBuilder;
 import org.hibernate.boot.MetadataSources;
 import org.hibernate.boot.SessionFactoryBuilder;
 import org.hibernate.boot.archive.scan.internal.StandardScanOptions;
-import org.hibernate.boot.cfgxml.internal.ConfigLoader;
 import org.hibernate.boot.cfgxml.spi.CfgXmlAccessService;
 import org.hibernate.boot.cfgxml.spi.LoadedConfig;
 import org.hibernate.boot.cfgxml.spi.MappingReference;
@@ -60,7 +58,9 @@ import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.id.factory.spi.MutableIdentifierGeneratorFactory;
 import org.hibernate.integrator.spi.Integrator;
 import org.hibernate.internal.EntityManagerMessageLogger;
+import org.hibernate.internal.util.NullnessHelper;
 import org.hibernate.internal.util.StringHelper;
+import org.hibernate.internal.util.config.ConfigurationHelper;
 import org.hibernate.jpa.AvailableSettings;
 import org.hibernate.jpa.boot.spi.EntityManagerFactoryBuilder;
 import org.hibernate.jpa.boot.spi.IntegratorProvider;
@@ -73,6 +73,7 @@ import org.hibernate.jpa.spi.IdentifierGeneratorStrategyProvider;
 import org.hibernate.proxy.EntityNotFoundDelegate;
 import org.hibernate.resource.transaction.backend.jdbc.internal.JdbcResourceLocalTransactionCoordinatorBuilderImpl;
 import org.hibernate.resource.transaction.backend.jta.internal.JtaTransactionCoordinatorBuilderImpl;
+import org.hibernate.resource.transaction.spi.TransactionCoordinatorBuilder;
 import org.hibernate.secure.spi.GrantedPermission;
 import org.hibernate.secure.spi.JaccPermissionDeclarations;
 import org.hibernate.service.ServiceRegistry;
@@ -199,24 +200,31 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 		final BootstrapServiceRegistry bsr = buildBootstrapServiceRegistry( integrationSettings, providedClassLoader, providedClassLoaderService);
 
 		// merge configuration sources and build the "standard" service registry
-		final StandardServiceRegistryBuilder ssrBuilder = new StandardServiceRegistryBuilder( bsr );
+		final StandardServiceRegistryBuilder ssrBuilder = StandardServiceRegistryBuilder.forJpa( bsr );
+
 		final MergedSettings mergedSettings = mergeSettings( persistenceUnit, integrationSettings, ssrBuilder );
+
+		// flush before completion validation
+		if ( "true".equals( mergedSettings.configurationValues.get( Environment.FLUSH_BEFORE_COMPLETION ) ) ) {
+			LOG.definingFlushBeforeCompletionIgnoredInHem( Environment.FLUSH_BEFORE_COMPLETION );
+			mergedSettings.configurationValues.put( Environment.FLUSH_BEFORE_COMPLETION, "false" );
+		}
+
+		// keep the merged config values for phase-2
 		this.configurationValues = mergedSettings.getConfigurationValues();
 
 		// Build the "standard" service registry
 		ssrBuilder.applySettings( configurationValues );
-		configure( ssrBuilder );
+
 		this.standardServiceRegistry = ssrBuilder.build();
-		configure( standardServiceRegistry, mergedSettings );
+
+		configureIdentifierGenerators( standardServiceRegistry );
 
 		final MetadataSources metadataSources = new MetadataSources( bsr );
-		List<AttributeConverterDefinition> attributeConverterDefinitions = populate(
+		List<AttributeConverterDefinition> attributeConverterDefinitions = applyMappingResources( metadataSources );
-				metadataSources,
+
-				mergedSettings,
-				standardServiceRegistry
-		);
 		this.metamodelBuilder = (MetadataBuilderImplementor) metadataSources.getMetadataBuilder( standardServiceRegistry );
-		populate( metamodelBuilder, mergedSettings, standardServiceRegistry, attributeConverterDefinitions );
+		applyMetamodelBuilderSettings( mergedSettings, attributeConverterDefinitions );
 
 		applyMetadataBuilderContributor();
 
@@ -307,7 +315,6 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 
 	// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 	// temporary!
-	@SuppressWarnings("unchecked")
 	public Map getConfigurationValues() {
 		return Collections.unmodifiableMap( configurationValues );
 	}
@@ -326,7 +333,10 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 	 * @param associationManagementEnabled To enable association management feature
 	 * @return An enhancement context for classes managed by this EM
 	 */
-	protected EnhancementContext getEnhancementContext(final boolean dirtyTrackingEnabled, final boolean lazyInitializationEnabled, final boolean associationManagementEnabled ) {
+	protected EnhancementContext getEnhancementContext(
+			final boolean dirtyTrackingEnabled,
+			final boolean lazyInitializationEnabled,
+			final boolean associationManagementEnabled ) {
 		return new DefaultEnhancementContext() {
 
 			@Override
@@ -454,21 +464,12 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 		return bsrBuilder.build();
 	}
 
-	@SuppressWarnings("unchecked")
 	private MergedSettings mergeSettings(
 			PersistenceUnitDescriptor persistenceUnit,
 			Map<?,?> integrationSettings,
 			StandardServiceRegistryBuilder ssrBuilder) {
 		final MergedSettings mergedSettings = new MergedSettings();
-
+		mergedSettings.processPersistenceUnitDescriptorProperties( persistenceUnit );
-		// first, apply persistence.xml-defined settings
-		if ( persistenceUnit.getProperties() != null ) {
-			mergedSettings.configurationValues.putAll( persistenceUnit.getProperties() );
-		}
-
-		mergedSettings.configurationValues.put( PERSISTENCE_UNIT_NAME, persistenceUnit.getName() );
-
-		final ConfigLoader configLoader = new ConfigLoader( ssrBuilder.getBootstrapServiceRegistry() );
 
 		// see if the persistence.xml settings named a Hibernate config file....
 		String cfgXmlResourceName = (String) mergedSettings.configurationValues.remove( CFG_FILE );
@@ -478,35 +479,10 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 		}
 
 		if ( StringHelper.isNotEmpty( cfgXmlResourceName ) ) {
-			final LoadedConfig loadedCfg = configLoader.loadConfigXmlResource( cfgXmlResourceName );
+			processHibernateConfigXmlResources( ssrBuilder, mergedSettings, cfgXmlResourceName );
-			processConfigXml( loadedCfg, mergedSettings, ssrBuilder );
 		}
 
-		// finally, apply integration-supplied settings (per JPA spec, integration settings should override other sources)
+		normalizeSettings( persistenceUnit, integrationSettings, mergedSettings );
-		for ( Map.Entry<?,?> entry : integrationSettings.entrySet() ) {
-			if ( entry.getKey() == null ) {
-				continue;
-			}
-
-			if ( entry.getValue() == null ) {
-				mergedSettings.configurationValues.remove( entry.getKey() );
-			}
-			else {
-				mergedSettings.configurationValues.put( entry.getKey(), entry.getValue() );
-			}
-		}
-
-		if ( !mergedSettings.configurationValues.containsKey( JPA_VALIDATION_MODE ) ) {
-			if ( persistenceUnit.getValidationMode() != null ) {
-				mergedSettings.configurationValues.put( JPA_VALIDATION_MODE, persistenceUnit.getValidationMode() );
-			}
-		}
-
-		if ( !mergedSettings.configurationValues.containsKey( JPA_SHARED_CACHE_MODE ) ) {
-			if ( persistenceUnit.getSharedCacheMode() != null ) {
-				mergedSettings.configurationValues.put( JPA_SHARED_CACHE_MODE, persistenceUnit.getSharedCacheMode() );
-			}
-		}
 
 		final String jaccContextId = (String) mergedSettings.configurationValues.get( JACC_CONTEXT_ID );
 
@@ -567,22 +543,446 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 		return mergedSettings;
 	}
 
+	/**
+	 * Handles normalizing the settings coming from multiple sources, applying proper precedences
+	 */
 	@SuppressWarnings("unchecked")
-	private void processConfigXml(
+	private void normalizeSettings(
-			LoadedConfig loadedConfig,
+			PersistenceUnitDescriptor persistenceUnit,
-			MergedSettings mergedSettings,
+			Map<?, ?> integrationSettings,
-			StandardServiceRegistryBuilder ssrBuilder) {
+			MergedSettings mergedSettings) {
-		if ( ! mergedSettings.configurationValues.containsKey( SESSION_FACTORY_NAME ) ) {
+		// make a copy so we can remove things as we process them
-			// there is not already a SF-name in the merged settings
+		final HashMap<?, ?> integrationSettingsCopy = new HashMap<>( integrationSettings );
-			final String sfName = loadedConfig.getSessionFactoryName();
+
-			if ( sfName != null ) {
+		normalizeConnectionAccessUserAndPass( integrationSettingsCopy, mergedSettings );
-				// but the cfg.xml file we are processing named one..
+
-				mergedSettings.configurationValues.put( SESSION_FACTORY_NAME, sfName );
+		normalizeTransactionCoordinator( persistenceUnit, integrationSettingsCopy, mergedSettings );
+
+		normalizeDataAccess( integrationSettingsCopy, mergedSettings, persistenceUnit );
+
+		// normalize ValidationMode
+		final Object intgValidationMode = integrationSettingsCopy.remove( JPA_VALIDATION_MODE );
+		if ( intgValidationMode != null ) {
+			mergedSettings.configurationValues.put( JPA_VALIDATION_MODE, intgValidationMode );
+		}
+		else if ( persistenceUnit.getValidationMode() != null ) {
+			mergedSettings.configurationValues.put( JPA_VALIDATION_MODE, persistenceUnit.getValidationMode() );
+		}
+
+		// normalize SharedCacheMode
+		final Object intgCacheMode = integrationSettingsCopy.remove( JPA_SHARED_CACHE_MODE );
+		if ( intgCacheMode != null ) {
+			mergedSettings.configurationValues.put( JPA_SHARED_CACHE_MODE, intgCacheMode );
+		}
+		else if ( persistenceUnit.getSharedCacheMode() != null ) {
+			mergedSettings.configurationValues.put( JPA_SHARED_CACHE_MODE, persistenceUnit.getSharedCacheMode() );
+		}
+
+		// Apply all "integration overrides" as the last step.  By specification,
+		// these should have precedence.
+		//
+		// NOTE that this occurs after the specialized normalize calls above which remove
+		// any specially-handled settings.
+		for ( Map.Entry<?,?> entry : integrationSettingsCopy.entrySet() ) {
+			if ( entry.getKey() == null ) {
+				continue;
+			}
+
+			if ( entry.getValue() == null ) {
+				mergedSettings.configurationValues.remove( entry.getKey() );
+			}
+			else {
+				mergedSettings.configurationValues.put( entry.getKey(), entry.getValue() );
+			}
 		}
 	}
 
-		mergedSettings.configurationValues.putAll( loadedConfig.getConfigurationValues() );
+	/**
-		ssrBuilder.configure( loadedConfig );
+	 * Because a DataSource can be secured (requiring Hibernate to pass the USER/PASSWORD when accessing the DataSource)
+	 * we apply precedence to the USER and PASS separately
+	 */
+	private void normalizeConnectionAccessUserAndPass(
+			HashMap<?, ?> integrationSettingsCopy,
+			MergedSettings mergedSettings) {
+		//noinspection unchecked
+		final Object effectiveUser = NullnessHelper.coalesceSuppliedValues(
+				() -> integrationSettingsCopy.remove( USER ),
+				() -> integrationSettingsCopy.remove( JPA_JDBC_USER ),
+				() -> extractPuProperty( persistenceUnit, USER ),
+				() -> extractPuProperty( persistenceUnit, JPA_JDBC_USER )
+		);
+
+		//noinspection unchecked
+		final Object effectivePass = NullnessHelper.coalesceSuppliedValues(
+				() -> integrationSettingsCopy.remove( PASS ),
+				() -> integrationSettingsCopy.remove( JPA_JDBC_PASSWORD ),
+				() -> extractPuProperty( persistenceUnit, PASS ),
+				() -> extractPuProperty( persistenceUnit, JPA_JDBC_PASSWORD )
+		);
+
+		if ( effectiveUser != null || effectivePass != null ) {
+			applyUserAndPass( effectiveUser, effectivePass, mergedSettings );
+		}
+	}
+
+	private <T> T extractPuProperty(PersistenceUnitDescriptor persistenceUnit, String propertyName) {
+		//noinspection unchecked
+		return persistenceUnit.getProperties() == null ? null : (T) persistenceUnit.getProperties().get( propertyName );
+	}
+
+	@SuppressWarnings("unchecked")
+	private void applyUserAndPass(Object effectiveUser, Object effectivePass, MergedSettings mergedSettings) {
+		if ( effectiveUser != null ) {
+			mergedSettings.configurationValues.put( USER, effectiveUser );
+			mergedSettings.configurationValues.put( JPA_JDBC_USER, effectiveUser );
+		}
+
+		if ( effectivePass != null ) {
+			mergedSettings.configurationValues.put( PASS, effectivePass );
+			mergedSettings.configurationValues.put( JPA_JDBC_PASSWORD, effectivePass );
+		}
+	}
+
+	private static final String IS_JTA_TXN_COORD = "local.setting.IS_JTA_TXN_COORD";
+
+	@SuppressWarnings("unchecked")
+	private void normalizeTransactionCoordinator(
+			PersistenceUnitDescriptor persistenceUnit,
+			HashMap<?, ?> integrationSettingsCopy,
+			MergedSettings mergedSettings) {
+		PersistenceUnitTransactionType txnType = null;
+
+		final Object intgTxnType = integrationSettingsCopy.remove( JPA_TRANSACTION_TYPE );
+
+		if ( intgTxnType != null ) {
+			txnType = PersistenceUnitTransactionTypeHelper.interpretTransactionType( intgTxnType );
+		}
+		else if ( persistenceUnit.getTransactionType() != null ) {
+			txnType = persistenceUnit.getTransactionType();
+		}
+		else {
+			final Object puPropTxnType = mergedSettings.configurationValues.get( JPA_TRANSACTION_TYPE );
+			if ( puPropTxnType != null ) {
+				txnType = PersistenceUnitTransactionTypeHelper.interpretTransactionType( puPropTxnType );
+			}
+		}
+
+		if ( txnType == null ) {
+			// is it more appropriate to have this be based on bootstrap entry point (EE vs SE)?
+			LOG.debugf( "PersistenceUnitTransactionType not specified - falling back to RESOURCE_LOCAL" );
+			txnType = PersistenceUnitTransactionType.RESOURCE_LOCAL;
+		}
+
+		boolean hasTxStrategy = mergedSettings.configurationValues.containsKey( TRANSACTION_COORDINATOR_STRATEGY );
+		final Boolean definiteJtaCoordinator;
+
+		if ( hasTxStrategy ) {
+			LOG.overridingTransactionStrategyDangerous( TRANSACTION_COORDINATOR_STRATEGY );
+
+			// see if we can tell whether it is a JTA coordinator
+			final Object strategy = mergedSettings.configurationValues.get( TRANSACTION_COORDINATOR_STRATEGY );
+			if ( strategy instanceof TransactionCoordinatorBuilder ) {
+				definiteJtaCoordinator = ( (TransactionCoordinatorBuilder) strategy ).isJta();
+			}
+			else {
+				definiteJtaCoordinator = false;
+			}
+		}
+		else {
+			if ( txnType == PersistenceUnitTransactionType.JTA ) {
+				mergedSettings.configurationValues.put( TRANSACTION_COORDINATOR_STRATEGY, JtaTransactionCoordinatorBuilderImpl.class );
+				definiteJtaCoordinator = true;
+			}
+			else if ( txnType == PersistenceUnitTransactionType.RESOURCE_LOCAL ) {
+				mergedSettings.configurationValues.put( TRANSACTION_COORDINATOR_STRATEGY, JdbcResourceLocalTransactionCoordinatorBuilderImpl.class );
+				definiteJtaCoordinator = false;
+			}
+			else {
+				throw new IllegalStateException( "Could not determine TransactionCoordinator strategy to use" );
+			}
+		}
+
+		mergedSettings.configurationValues.put( IS_JTA_TXN_COORD, definiteJtaCoordinator );
+	}
+
+	private void normalizeDataAccess(
+			HashMap<?, ?> integrationSettingsCopy,
+			MergedSettings mergedSettings,
+			PersistenceUnitDescriptor persistenceUnit) {
+		if ( dataSource != null ) {
+			applyDataSource(
+					dataSource,
+					// we don't explicitly know
+					null,
+					integrationSettingsCopy,
+					mergedSettings
+			);
+
+			// EARLY EXIT!!
+			return;
+		}
+
+		if ( integrationSettingsCopy.containsKey( DATASOURCE ) ) {
+			final Object dataSourceRef = integrationSettingsCopy.remove( DATASOURCE );
+			if ( dataSourceRef != null ) {
+				applyDataSource(
+						dataSourceRef,
+						null,
+						integrationSettingsCopy,
+						mergedSettings
+				);
+
+				// EARLY EXIT!!
+				return;
+			}
+		}
+
+		if ( integrationSettingsCopy.containsKey( JPA_JTA_DATASOURCE ) ) {
+			final Object dataSourceRef = integrationSettingsCopy.remove( JPA_JTA_DATASOURCE );
+			if ( dataSourceRef != null ) {
+				applyDataSource(
+						dataSourceRef,
+						true,
+						integrationSettingsCopy,
+						mergedSettings
+				);
+
+				// EARLY EXIT!!
+				return;
+			}
+		}
+
+		if ( integrationSettingsCopy.containsKey( JPA_NON_JTA_DATASOURCE ) ) {
+			final Object dataSourceRef = integrationSettingsCopy.remove( JPA_NON_JTA_DATASOURCE );
+
+			applyDataSource(
+					dataSourceRef,
+					false,
+					integrationSettingsCopy,
+					mergedSettings
+			);
+
+			// EARLY EXIT!!
+			return;
+		}
+
+		if ( integrationSettingsCopy.containsKey( URL ) ) {
+			// these have precedence over the JPA ones
+			final Object integrationJdbcUrl = integrationSettingsCopy.get( URL );
+			if ( integrationJdbcUrl != null ) {
+				//noinspection unchecked
+				applyJdbcSettings(
+						integrationJdbcUrl,
+						NullnessHelper.coalesceSuppliedValues(
+								() -> ConfigurationHelper.getString( DRIVER, integrationSettingsCopy ),
+								() -> ConfigurationHelper.getString( JPA_JDBC_DRIVER, integrationSettingsCopy ),
+								() -> ConfigurationHelper.getString( DRIVER, mergedSettings.configurationValues ),
+								() -> ConfigurationHelper.getString( JPA_JDBC_DRIVER, mergedSettings.configurationValues )
+						),
+						integrationSettingsCopy,
+						mergedSettings
+				);
+
+				// EARLY EXIT!!
+				return;
+			}
+		}
+
+		if ( integrationSettingsCopy.containsKey( JPA_JDBC_URL ) ) {
+			final Object integrationJdbcUrl = integrationSettingsCopy.get( JPA_JDBC_URL );
+
+			if ( integrationJdbcUrl != null ) {
+				//noinspection unchecked
+				applyJdbcSettings(
+						integrationJdbcUrl,
+						NullnessHelper.coalesceSuppliedValues(
+								() -> ConfigurationHelper.getString( JPA_JDBC_DRIVER, integrationSettingsCopy ),
+								() -> ConfigurationHelper.getString( JPA_JDBC_DRIVER, mergedSettings.configurationValues )
+						),
+						integrationSettingsCopy,
+						mergedSettings
+				);
+
+				// EARLY EXIT!!
+				return;
+			}
+		}
+
+		if ( persistenceUnit.getJtaDataSource() != null ) {
+			applyDataSource(
+					persistenceUnit.getJtaDataSource(),
+					true,
+					integrationSettingsCopy,
+					mergedSettings
+			);
+
+			// EARLY EXIT!!
+			return;
+		}
+
+		if ( persistenceUnit.getNonJtaDataSource() != null ) {
+			applyDataSource(
+					persistenceUnit.getNonJtaDataSource(),
+					false,
+					integrationSettingsCopy,
+					mergedSettings
+			);
+
+			// EARLY EXIT!!
+			return;
+		}
+
+		if ( mergedSettings.configurationValues.containsKey( URL ) ) {
+			final Object url = mergedSettings.configurationValues.get( URL );
+
+			if ( url != null && ( ! ( url instanceof String ) || StringHelper.isNotEmpty( (String) url ) ) ) {
+				applyJdbcSettings(
+						url,
+						ConfigurationHelper.getString( DRIVER, mergedSettings.configurationValues ),
+						integrationSettingsCopy,
+						mergedSettings
+				);
+
+				// EARLY EXIT!!
+				return;
+			}
+		}
+
+		if ( mergedSettings.configurationValues.containsKey( JPA_JDBC_URL ) ) {
+			final Object url = mergedSettings.configurationValues.get( JPA_JDBC_URL );
+
+			if ( url != null && ( ! ( url instanceof String ) || StringHelper.isNotEmpty( (String) url ) ) ) {
+				applyJdbcSettings(
+						url,
+						ConfigurationHelper.getString( JPA_JDBC_DRIVER, mergedSettings.configurationValues ),
+						integrationSettingsCopy,
+						mergedSettings
+				);
+
+				// EARLY EXIT!!
+				return;
+			}
+		}
+
+		// any other conditions to account for?
+	}
+
+	@SuppressWarnings("unchecked")
+	private void applyDataSource(
+			Object dataSourceRef,
+			Boolean useJtaDataSource,
+			HashMap<?, ?> integrationSettingsCopy,
+			MergedSettings mergedSettings) {
+
+		// `IS_JTA_TXN_COORD` is a value set during `#normalizeTransactionCoordinator` to indicate whether
+		// the execution environment "is JTA" as best as it can tell..
+		//
+		// we use this value when JTA was not explicitly specified in regards the DataSource
+		final boolean isJtaTransactionCoordinator = (boolean) mergedSettings.configurationValues.remove( IS_JTA_TXN_COORD );
+		final boolean isJta = useJtaDataSource == null ? isJtaTransactionCoordinator : useJtaDataSource;
+
+		// add to EMF properties (questionable - see HHH-13432)
+		final String emfKey;
+		final String inverseEmfKey;
+		if ( isJta ) {
+			emfKey = JPA_JTA_DATASOURCE;
+			inverseEmfKey = JPA_NON_JTA_DATASOURCE;
+		}
+		else {
+			emfKey = JPA_NON_JTA_DATASOURCE;
+			inverseEmfKey = JPA_JTA_DATASOURCE;
+		}
+		mergedSettings.configurationValues.put( emfKey, dataSourceRef );
+
+		// clear any settings logically overridden by this datasource
+		cleanUpConfigKeys(
+				integrationSettingsCopy,
+				mergedSettings,
+				inverseEmfKey,
+				JPA_JDBC_DRIVER,
+				DRIVER,
+				JPA_JDBC_URL,
+				URL
+		);
+
+
+		// clean-up the entries in the "integration overrides" so they do not get get picked
+		// up in the general "integration overrides" handling
+		cleanUpConfigKeys( integrationSettingsCopy, DATASOURCE, JPA_JTA_DATASOURCE, JPA_NON_JTA_DATASOURCE );
+
+		// add under Hibernate's DATASOURCE setting where the ConnectionProvider will find it
+		mergedSettings.configurationValues.put( DATASOURCE, dataSourceRef );
+	}
+
+	private void cleanUpConfigKeys(HashMap<?, ?> integrationSettingsCopy, MergedSettings mergedSettings, String... keys) {
+		for ( String key : keys ) {
+			final Object removedIntgSetting = integrationSettingsCopy.remove( key );
+			if ( removedIntgSetting != null ) {
+				LOG.debugf( "Removed integration override setting [%s] due to normalization", key );
+			}
+
+			final Object removedMergedSetting = mergedSettings.configurationValues.remove( key );
+			if ( removedMergedSetting != null ) {
+				LOG.debugf( "Removed merged setting [%s] due to normalization", key );
+			}
+		}
+	}
+
+	private void cleanUpConfigKeys(Map<?, ?> settings, String... keys) {
+		for ( String key : keys ) {
+			settings.remove( key );
+		}
+	}
+
+	@SuppressWarnings("unchecked")
+	private void applyJdbcSettings(
+			Object url,
+			String driver,
+			HashMap<?, ?> integrationSettingsCopy,
+			MergedSettings mergedSettings) {
+		mergedSettings.configurationValues.put( URL, url );
+		mergedSettings.configurationValues.put( JPA_JDBC_URL, url );
+
+		if ( driver != null ) {
+			mergedSettings.configurationValues.put( DRIVER, driver );
+			mergedSettings.configurationValues.put( JPA_JDBC_DRIVER, driver );
+		}
+		else {
+			mergedSettings.configurationValues.remove( DRIVER );
+			mergedSettings.configurationValues.remove( JPA_JDBC_DRIVER );
+		}
+
+		// clean up the integration-map values
+		cleanUpConfigKeys(
+				integrationSettingsCopy,
+				DRIVER,
+				JPA_JDBC_DRIVER,
+				URL,
+				JPA_JDBC_URL,
+				USER,
+				JPA_JDBC_USER,
+				PASS,
+				JPA_JDBC_PASSWORD
+		);
+
+		cleanUpConfigKeys(
+				integrationSettingsCopy,
+				mergedSettings,
+				DATASOURCE,
+				JPA_JTA_DATASOURCE,
+				JPA_NON_JTA_DATASOURCE
+		);
+	}
+
+	private void processHibernateConfigXmlResources(
+			StandardServiceRegistryBuilder ssrBuilder,
+			MergedSettings mergedSettings,
+			String cfgXmlResourceName) {
+		final LoadedConfig loadedConfig = ssrBuilder.getConfigLoader().loadConfigXmlResource( cfgXmlResourceName );
+
+		mergedSettings.processHibernateConfigXmlResources( loadedConfig );
+
+		ssrBuilder.getAggregatedCfgXml().merge( loadedConfig );
 	}
 
 	private GrantedPermission parseJaccConfigEntry(String keyString, String valueString) {
@@ -638,90 +1038,7 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 		return new CacheRegionDefinition( cacheType, role, usage, region, lazyProperty );
 	}
 
-	private void configure(StandardServiceRegistryBuilder ssrBuilder) {
+	private void configureIdentifierGenerators(StandardServiceRegistry ssr) {
-
-		applyJdbcConnectionProperties( ssrBuilder );
-		applyTransactionProperties( ssrBuilder );
-
-		// flush before completion validation
-		if ( "true".equals( configurationValues.get( Environment.FLUSH_BEFORE_COMPLETION ) ) ) {
-			ssrBuilder.applySetting( Environment.FLUSH_BEFORE_COMPLETION, "false" );
-			LOG.definingFlushBeforeCompletionIgnoredInHem( Environment.FLUSH_BEFORE_COMPLETION );
-		}
-
-//		final StrategySelector strategySelector = ssrBuilder.getBootstrapServiceRegistry().getService( StrategySelector.class );
-//		final Object interceptorSetting = configurationValues.remove( AvailableSettings.SESSION_INTERCEPTOR );
-//		if ( interceptorSetting != null ) {
-//			settings.setSessionInterceptorClass(
-//					loadSessionInterceptorClass( interceptorSetting, strategySelector )
-//			);
-//		}
-	}
-
-	private void applyJdbcConnectionProperties(StandardServiceRegistryBuilder ssrBuilder) {
-		if ( dataSource != null ) {
-			ssrBuilder.applySetting( DATASOURCE, dataSource );
-		}
-		else if ( persistenceUnit.getJtaDataSource() != null ) {
-			if ( ! ssrBuilder.getSettings().containsKey( DATASOURCE ) ) {
-				ssrBuilder.applySetting( DATASOURCE, persistenceUnit.getJtaDataSource() );
-				// HHH-8121 : make the PU-defined value available to EMF.getProperties()
-				configurationValues.put( JPA_JTA_DATASOURCE, persistenceUnit.getJtaDataSource() );
-			}
-		}
-		else if ( persistenceUnit.getNonJtaDataSource() != null ) {
-			if ( ! ssrBuilder.getSettings().containsKey( DATASOURCE ) ) {
-				ssrBuilder.applySetting( DATASOURCE, persistenceUnit.getNonJtaDataSource() );
-				// HHH-8121 : make the PU-defined value available to EMF.getProperties()
-				configurationValues.put( JPA_NON_JTA_DATASOURCE, persistenceUnit.getNonJtaDataSource() );
-			}
-		}
-		else {
-			final String driver = (String) configurationValues.get( JPA_JDBC_DRIVER );
-			if ( StringHelper.isNotEmpty( driver ) ) {
-				ssrBuilder.applySetting( DRIVER, driver );
-			}
-			final String url = (String) configurationValues.get( JPA_JDBC_URL );
-			if ( StringHelper.isNotEmpty( url ) ) {
-				ssrBuilder.applySetting( URL, url );
-			}
-			final String user = (String) configurationValues.get( JPA_JDBC_USER );
-			if ( StringHelper.isNotEmpty( user ) ) {
-				ssrBuilder.applySetting( USER, user );
-			}
-			final String pass = (String) configurationValues.get( JPA_JDBC_PASSWORD );
-			if ( StringHelper.isNotEmpty( pass ) ) {
-				ssrBuilder.applySetting( PASS, pass );
-			}
-		}
-	}
-
-	private void applyTransactionProperties(StandardServiceRegistryBuilder ssrBuilder) {
-		PersistenceUnitTransactionType txnType = PersistenceUnitTransactionTypeHelper.interpretTransactionType(
-				configurationValues.get( JPA_TRANSACTION_TYPE )
-		);
-		if ( txnType == null ) {
-			txnType = persistenceUnit.getTransactionType();
-		}
-		if ( txnType == null ) {
-			// is it more appropriate to have this be based on bootstrap entry point (EE vs SE)?
-			txnType = PersistenceUnitTransactionType.RESOURCE_LOCAL;
-		}
-		boolean hasTxStrategy = configurationValues.containsKey( TRANSACTION_COORDINATOR_STRATEGY );
-		if ( hasTxStrategy ) {
-			LOG.overridingTransactionStrategyDangerous( TRANSACTION_COORDINATOR_STRATEGY );
-		}
-		else {
-			if ( txnType == PersistenceUnitTransactionType.JTA ) {
-				ssrBuilder.applySetting( TRANSACTION_COORDINATOR_STRATEGY, JtaTransactionCoordinatorBuilderImpl.class );
-			}
-			else if ( txnType == PersistenceUnitTransactionType.RESOURCE_LOCAL ) {
-				ssrBuilder.applySetting( TRANSACTION_COORDINATOR_STRATEGY, JdbcResourceLocalTransactionCoordinatorBuilderImpl.class );
-			}
-		}
-	}
-
-	private void configure(StandardServiceRegistry ssr, MergedSettings mergedSettings) {
 		final StrategySelector strategySelector = ssr.getService( StrategySelector.class );
 
 		// apply id generators
@@ -743,10 +1060,9 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 	}
 
 	@SuppressWarnings("unchecked")
-	protected List<AttributeConverterDefinition> populate(
+	private List<AttributeConverterDefinition> applyMappingResources(MetadataSources metadataSources) {
-			MetadataSources metadataSources,
+		// todo : where in the heck are `org.hibernate.jpa.boot.spi.PersistenceUnitDescriptor.getManagedClassNames` handled?!?
-			MergedSettings mergedSettings,
+
-			StandardServiceRegistry ssr) {
 //		final ClassLoaderService classLoaderService = ssr.getService( ClassLoaderService.class );
 //
 //		// todo : make sure MetadataSources/Metadata are capable of handling duplicate sources
@@ -823,12 +1139,10 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 		return attributeConverterDefinitions;
 	}
 
-	protected void populate(
+	private void applyMetamodelBuilderSettings(
-			MetadataBuilder metamodelBuilder,
 			MergedSettings mergedSettings,
-			StandardServiceRegistry ssr,
 			List<AttributeConverterDefinition> attributeConverterDefinitions) {
-		( (MetadataBuilderImplementor) metamodelBuilder ).getBootstrapContext().markAsJpaBootstrap();
+		metamodelBuilder.getBootstrapContext().markAsJpaBootstrap();
 
 		if ( persistenceUnit.getTempClassLoader() != null ) {
 			metamodelBuilder.applyTempClassLoader( persistenceUnit.getTempClassLoader() );
@@ -912,7 +1226,7 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 		// Metamodel will clean this up...
 		try {
 			SessionFactoryBuilder sfBuilder = metadata().getSessionFactoryBuilder();
-			populate( sfBuilder, standardServiceRegistry );
+			populateSfBuilder( sfBuilder, standardServiceRegistry );
 
 			SchemaManagementToolCoordinator.process(
 					metadata, standardServiceRegistry, configurationValues, DelayedDropRegistryNotAvailableImpl.INSTANCE
@@ -926,10 +1240,10 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 		cancel();
 	}
 
-	@SuppressWarnings("unchecked")
+	@Override
 	public EntityManagerFactory build() {
-		SessionFactoryBuilder sfBuilder = metadata().getSessionFactoryBuilder();
+		final SessionFactoryBuilder sfBuilder = metadata().getSessionFactoryBuilder();
-		populate( sfBuilder, standardServiceRegistry );
+		populateSfBuilder( sfBuilder, standardServiceRegistry );
 
 		try {
 			return sfBuilder.build();
@@ -939,7 +1253,7 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 		}
 	}
 
-	protected void populate(SessionFactoryBuilder sfBuilder, StandardServiceRegistry ssr) {
+	protected void populateSfBuilder(SessionFactoryBuilder sfBuilder, StandardServiceRegistry ssr) {
 
 		final StrategySelector strategySelector = ssr.getService( StrategySelector.class );
 
@@ -1024,7 +1338,36 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 		private Map<String, JaccPermissionDeclarations> jaccPermissionsByContextId;
 		private List<CacheRegionDefinition> cacheRegionDefinitions;
 
+		/**
+		 * 	MergedSettings is initialized with hibernate.properties
+		 */
 		private MergedSettings() {
+			configurationValues.putAll( Environment.getProperties() );
+		}
+
+		public void processPersistenceUnitDescriptorProperties(PersistenceUnitDescriptor persistenceUnit) {
+			if ( persistenceUnit.getProperties() != null ) {
+				configurationValues.putAll( persistenceUnit.getProperties() );
+			}
+
+			configurationValues.put( PERSISTENCE_UNIT_NAME, persistenceUnit.getName() );
+
+		}
+
+		public void processHibernateConfigXmlResources(LoadedConfig loadedConfig){
+			if ( ! configurationValues.containsKey( SESSION_FACTORY_NAME ) ) {
+				// there is not already a SF-name in the merged settings
+				final String sfName = loadedConfig.getSessionFactoryName();
+				if ( sfName != null ) {
+					// but the cfg.xml file we are processing named one..
+					configurationValues.put( SESSION_FACTORY_NAME, sfName );
+				}
+			}
+			else {
+				// make sure they match?
+			}
+
+			configurationValues.putAll( loadedConfig.getConfigurationValues() );
 		}
 
 		public Map getConfigurationValues() {

hibernate-core/src/test/java/org/hibernate/internal/MaskSensitiveInformationTest.java

@@ -34,10 +34,8 @@ public class MaskSensitiveInformationTest extends BaseEntityManagerFunctionalTes
 	}
 
 	@Override
-	@SuppressWarnings("unchecked")
 	protected void addConfigOptions(Map options) {
-		options.put( AvailableSettings.JPA_JDBC_USER, options.get( AvailableSettings.USER ) );
+		super.addConfigOptions( options );
-		options.put( AvailableSettings.JPA_JDBC_PASSWORD, options.get( AvailableSettings.PASS ) );
 	}
 
 	@Test

hibernate-core/src/test/java/org/hibernate/jpa/test/connection/DataSourceInjectionTest.java

@@ -9,54 +9,106 @@
 
 package org.hibernate.jpa.test.connection;
 
-import java.io.File;
+import java.net.URL;
-import javax.persistence.EntityManagerFactory;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Consumer;
 import javax.persistence.PersistenceException;
+import javax.sql.DataSource;
 
+import org.hibernate.cfg.AvailableSettings;
+import org.hibernate.dialect.H2Dialect;
+import org.hibernate.engine.spi.SessionFactoryImplementor;
+import org.hibernate.engine.spi.SessionImplementor;
 import org.hibernate.jpa.HibernatePersistenceProvider;
+import org.hibernate.jpa.test.Distributor;
+import org.hibernate.jpa.test.Item;
+import org.hibernate.jpa.test.xml.Light;
+import org.hibernate.jpa.test.xml.Lighter;
 
-import org.hibernate.testing.util.ExceptionUtil;
+import org.hibernate.testing.util.jpa.PersistenceUnitInfoAdapter;
 import org.junit.Assert;
-import org.junit.Ignore;
 import org.junit.Test;
 
 /**
  * @author Emmanuel Bernard
  */
 public class DataSourceInjectionTest {
-	EntityManagerFactory emf;
     @Test
 	public void testDatasourceInjection() throws Exception {
-		File current = new File(".");
+    	withPuRoot(
-		File sub = new File(current, "puroot");
+				puRootUrl -> {
-		sub.mkdir();
+					final PersistenceUnitInfoAdapter persistenceUnitInfo = createPuDescriptor( puRootUrl, new FakeDataSource() );
-		PersistenceUnitInfoImpl info = new PersistenceUnitInfoImpl( sub.toURI().toURL(), new String[]{} );
+
-		try {
+					// otherwise the FakeDataSourceException will be eaten trying to resolve the Dialect
-			emf = new HibernatePersistenceProvider().createContainerEntityManagerFactory( info, null );
+					final Map<String, Object> intgOverrides = Collections.singletonMap(
-			try {
+							AvailableSettings.DIALECT,
-				emf.createEntityManager().createQuery( "select i from Item i" ).getResultList();
+							H2Dialect.class
-			}
+					);
-			finally {
+
-				try {
+					final HibernatePersistenceProvider provider = new HibernatePersistenceProvider();
-					emf.close();
+					try ( final SessionFactoryImplementor sf = provider.createContainerEntityManagerFactory(
-				}
+							persistenceUnitInfo,
-				catch (Exception ignore) {
+							intgOverrides
-					int i = 0;
+					).unwrap( SessionFactoryImplementor.class ) ) {
-				}
+
-			}
+						try ( final SessionImplementor session = sf.openSession().unwrap( SessionImplementor.class ) ) {
-			Assert.fail( "FakeDatasource should have been used" );
+							session.createQuery( "select i from Item i" ).list();
+							Assert.fail( "Expecting FakeDataSourceException" );
 						}
 						catch (PersistenceException pe) {
-			if(emf != null){
+							try {
-				emf.close();
+								throw (RuntimeException) pe.getCause();
-			}
-			Assert.assertTrue( ExceptionUtil.rootCause( pe ) instanceof FakeDataSourceException );
 							}
 							catch (FakeDataSourceException fde) {
 								//success
 							}
+						}
+						catch (FakeDataSourceException fde) {
+							//success
+						}
+					}
+				}
+		);
+	}
+
+	protected PersistenceUnitInfoAdapter createPuDescriptor(URL puRootUrl, DataSource dataSource) {
+		return new PersistenceUnitInfoAdapter() {
+			@Override
+			public DataSource getNonJtaDataSource() {
+				return dataSource;
+			}
+
+			@Override
+			public URL getPersistenceUnitRootUrl() {
+				return puRootUrl;
+			}
+
+			public List<String> getManagedClassNames() {
+				List<String> classes = new ArrayList<>();
+				classes.add( Item.class.getName() );
+				classes.add( Distributor.class.getName() );
+				classes.add( Light.class.getName() );
+				classes.add( Lighter.class.getName() );
+				return classes;
+			}
+		};
+	}
+
+	private void withPuRoot(Consumer<URL> puRootUrlConsumer) throws Exception {
+		// create a temporary directory to serve as the "PU root URL"
+		final Path puroot = Files.createTempDirectory( "puroot" );
+		final URL puRootUrl = puroot.toUri().toURL();
+
+		try {
+			puRootUrlConsumer.accept( puRootUrl );
+		}
 		finally {
-			sub.delete();
+			Files.deleteIfExists( puroot );
 		}
 	}
 }

hibernate-core/src/test/java/org/hibernate/jpa/test/connection/PersistenceUnitInfoImpl.java

@@ -37,7 +37,7 @@ public class PersistenceUnitInfoImpl implements PersistenceUnitInfo {
 	private URL puRoot;
 
 	public PersistenceUnitInfoImpl(URL puRoot, String[] mappingFiles) {
-		this.mappingFiles = new ArrayList<String>( mappingFiles.length );
+		this.mappingFiles = new ArrayList<>( mappingFiles.length );
 		this.mappingFiles.addAll( Arrays.asList( mappingFiles ) );
 		this.puRoot = puRoot;
 	}

hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitInfoTests.java

@@ -1,54 +0,0 @@
-/*
- * Hibernate, Relational Persistence for Idiomatic Java
- *
- * License: GNU Lesser General Public License (LGPL), version 2.1 or later
- * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
- */
-package org.hibernate.orm.test.bootstrap;
-
-import java.util.Collections;
-import java.util.Map;
-import javax.persistence.EntityManagerFactory;
-import javax.persistence.spi.PersistenceProvider;
-import javax.sql.DataSource;
-
-import org.hibernate.cfg.AvailableSettings;
-import org.hibernate.jpa.HibernatePersistenceProvider;
-
-import org.hibernate.testing.FailureExpected;
-import org.hibernate.testing.TestForIssue;
-import org.hibernate.testing.junit4.BaseUnitTestCase;
-import org.hibernate.testing.util.jpa.PersistenceUnitInfoAdapter;
-import org.junit.Test;
-
-import static org.junit.Assert.assertEquals;
-
-/**
- * @author Steve Ebersole
- */
-public class PersistenceUnitInfoTests extends BaseUnitTestCase {
-	@Test
-	@TestForIssue( jiraKey = "HHH-13432" )
-	@FailureExpected( jiraKey = "HHH-13432" )
-	public void testJtaDataExposedAsProperty() {
-		final DataSource puDataSource = new DataSourceStub( "puDataSource" );
-		final PersistenceUnitInfoAdapter info = new PersistenceUnitInfoAdapter() {
-
-			@Override
-			public DataSource getNonJtaDataSource() {
-				return puDataSource;
-			}
-		};
-
-		final PersistenceProvider provider = new HibernatePersistenceProvider();
-
-		final EntityManagerFactory emf = provider.createContainerEntityManagerFactory(
-				info,
-				Collections.emptyMap()
-		);
-
-		final Map<String, Object> properties = emf.getProperties();
-		final Object o = properties.get( AvailableSettings.JPA_JTA_DATASOURCE );
-		assertEquals( o, puDataSource );
-	}
-}

hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/jpa/PersistenceUnitInfoTests.java

@@ -0,0 +1,105 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.bootstrap.jpa;
+
+import java.util.Collections;
+import java.util.Map;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.spi.PersistenceProvider;
+import javax.sql.DataSource;
+
+import org.hibernate.cfg.AvailableSettings;
+import org.hibernate.engine.jdbc.connections.internal.DatasourceConnectionProviderImpl;
+import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider;
+import org.hibernate.engine.spi.SessionFactoryImplementor;
+import org.hibernate.jpa.HibernatePersistenceProvider;
+import org.hibernate.testing.jdbc.DataSourceStub;
+
+import org.hibernate.testing.FailureExpected;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseUnitTestCase;
+import org.hibernate.testing.util.jpa.PersistenceUnitInfoAdapter;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author Steve Ebersole
+ */
+public class PersistenceUnitInfoTests extends BaseUnitTestCase {
+	@Test
+	@TestForIssue( jiraKey = "HHH-13432" )
+	public void testNonJtaDataExposedAsProperty() {
+		final DataSource puDataSource = new DataSourceStub( "puDataSource" );
+		final PersistenceUnitInfoAdapter info = new PersistenceUnitInfoAdapter() {
+
+			@Override
+			public DataSource getNonJtaDataSource() {
+				return puDataSource;
+			}
+		};
+
+		final PersistenceProvider provider = new HibernatePersistenceProvider();
+
+		final EntityManagerFactory emf = provider.createContainerEntityManagerFactory(
+				info,
+				Collections.emptyMap()
+		);
+
+		// first let's check the DataSource used in the EMF...
+		final ConnectionProvider connectionProvider = emf.unwrap( SessionFactoryImplementor.class )
+				.getServiceRegistry()
+				.getService( ConnectionProvider.class );
+		assertThat( connectionProvider, instanceOf( DatasourceConnectionProviderImpl.class ) );
+		final DatasourceConnectionProviderImpl dsCp = (DatasourceConnectionProviderImpl) connectionProvider;
+		assertThat( dsCp.getDataSource(), is( puDataSource ) );
+
+		// now let's check that it is exposed via the EMF properties
+		//		- note : the spec does not indicate that this should work, but
+		//			it worked this way in previous versions
+		final Object o = emf.getProperties().get( AvailableSettings.JPA_NON_JTA_DATASOURCE );
+		assertThat( o, is( puDataSource ) );
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-13432" )
+	public void testJtaDataExposedAsProperty() {
+		final DataSource puDataSource = new DataSourceStub( "puDataSource" );
+		final PersistenceUnitInfoAdapter info = new PersistenceUnitInfoAdapter() {
+
+			@Override
+			public DataSource getJtaDataSource() {
+				return puDataSource;
+			}
+		};
+
+		final PersistenceProvider provider = new HibernatePersistenceProvider();
+
+		final EntityManagerFactory emf = provider.createContainerEntityManagerFactory(
+				info,
+				Collections.emptyMap()
+		);
+
+		// first let's check the DataSource used in the EMF...
+		final ConnectionProvider connectionProvider = emf.unwrap( SessionFactoryImplementor.class )
+				.getServiceRegistry()
+				.getService( ConnectionProvider.class );
+		assertThat( connectionProvider, instanceOf( DatasourceConnectionProviderImpl.class ) );
+		final DatasourceConnectionProviderImpl dsCp = (DatasourceConnectionProviderImpl) connectionProvider;
+		assertThat( dsCp.getDataSource(), is( puDataSource ) );
+
+		// now let's check that it is exposed via the EMF properties
+		//		- again, the spec does not indicate that this should work, but
+		//			it worked this way in previous versions
+		final Map<String, Object> properties = emf.getProperties();
+		final Object o = properties.get( AvailableSettings.JPA_JTA_DATASOURCE );
+		assertEquals( puDataSource, o );
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/jpa/PersistenceUnitOverridesTests.java

@@ -4,8 +4,9 @@
  * License: GNU Lesser General Public License (LGPL), version 2.1 or later
  * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
  */
-package org.hibernate.orm.test.bootstrap;
+package org.hibernate.orm.test.bootstrap.jpa;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
@@ -21,18 +22,21 @@ import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.jpa.HibernatePersistenceProvider;
 
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.jdbc.DataSourceStub;
+
 import org.hibernate.testing.FailureExpected;
 import org.hibernate.testing.env.ConnectionProviderBuilder;
 import org.hibernate.testing.junit4.BaseUnitTestCase;
 import org.hibernate.testing.util.jpa.DelegatingPersistenceUnitInfo;
 import org.hibernate.testing.util.jpa.PersistenceUnitInfoAdapter;
-import org.hibernate.testing.util.jpa.PersistenceUnitInfoPropertiesWrapper;
 import org.junit.Test;
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 
 /**
@@ -41,31 +45,19 @@ import static org.hamcrest.MatcherAssert.assertThat;
 public class PersistenceUnitOverridesTests extends BaseUnitTestCase {
 
 	@Test
-	public void testCustomProviderPassingIntegrationJpaJdbcOverrides() {
+	public void testPassingIntegrationJpaJdbcOverrides() {
-		PersistenceProvider provider = new HibernatePersistenceProvider() {
-			@Override
-			public EntityManagerFactory createContainerEntityManagerFactory(PersistenceUnitInfo info, Map integrationOverrides) {
-				return super.createContainerEntityManagerFactory(
-						new DelegatingPersistenceUnitInfo( info ) {
-							@Override
-							public Properties getProperties() {
-								// use the "db1" connection settings keyed by the JPA property names (org.hibernate.cfg.AvailableSettings.JPA_JDBC_DRIVER, e.g.)
-								final Properties properties = new Properties();
-								properties.putAll( info.getProperties() );
-								properties.putAll( ConnectionProviderBuilder.getJpaConnectionProviderProperties( "db1" ) );
-								return properties;
-							}
-						},
-						integrationOverrides
-				);
-			}
-		};
 
-		// however, use the "db2" JPA connection settings which should override the persistence unit values
+		// the integration overrides say to use the "db2" JPA connection settings (which should override the persistence unit values)
 		final Map integrationOverrides = ConnectionProviderBuilder.getJpaConnectionProviderProperties( "db2" );
 
-		final EntityManagerFactory emf = provider.createContainerEntityManagerFactory(
+		final EntityManagerFactory emf = new HibernatePersistenceProvider().createContainerEntityManagerFactory(
-				new PersistenceUnitInfoPropertiesWrapper(),
+				new PersistenceUnitInfoAdapter() {
+					@Override
+					public Properties getProperties() {
+						// effectively, the `persistence.xml` defines `db1` as the connection settings
+						return ConnectionProviderBuilder.getJpaConnectionProviderProperties( "db1" );
+					}
+				},
 				integrationOverrides
 		);
 
@@ -84,7 +76,45 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase {
 	}
 
 	@Test
-	public void testPassingIntegrationJpaJdbcOverridesForJtaDataSourceProperty() {
+	public void testPassingIntegrationJtaDataSourceOverrideForJpaJdbcSettings() {
+		final PersistenceUnitInfoAdapter puInfo = new PersistenceUnitInfoAdapter(
+				ConnectionProviderBuilder.getJpaConnectionProviderProperties( "db2" )
+		);
+
+		final DataSource integrationDataSource = new DataSourceStub( "integrationDataSource" );
+
+		final HibernatePersistenceProvider provider = new HibernatePersistenceProvider();
+
+		final EntityManagerFactory emf = provider.createContainerEntityManagerFactory(
+				puInfo,
+				Collections.singletonMap( AvailableSettings.JPA_JTA_DATASOURCE, integrationDataSource )
+		);
+
+		// first let's check the DataSource used in the EMF...
+		final ConnectionProvider connectionProvider = emf.unwrap( SessionFactoryImplementor.class )
+				.getServiceRegistry()
+				.getService( ConnectionProvider.class );
+		assertThat( connectionProvider, instanceOf( DatasourceConnectionProviderImpl.class ) );
+		final DatasourceConnectionProviderImpl dsCp = (DatasourceConnectionProviderImpl) connectionProvider;
+		assertThat( dsCp.getDataSource(), is( integrationDataSource ) );
+
+		// now let's check that it is exposed via the EMF properties
+		//		- note : the spec does not indicate that this should work, but
+		//			it worked this way in previous versions
+		final Object jtaDs = emf.getProperties().get( AvailableSettings.JPA_JTA_DATASOURCE );
+		assertThat( jtaDs, is( integrationDataSource ) );
+
+		// Additionally, we should have set Hibernate's DATASOURCE setting
+		final Object hibDs = emf.getProperties().get( AvailableSettings.JPA_JTA_DATASOURCE );
+		assertThat( hibDs, is( integrationDataSource ) );
+
+		// Make sure the non-jta-data-source setting was cleared or otherwise null
+		final Object nonJtaDs = emf.getProperties().get( AvailableSettings.JPA_NON_JTA_DATASOURCE );
+		assertThat( nonJtaDs, nullValue() );
+	}
+
+	@Test
+	public void testPassingIntegrationJpaJdbcOverrideForJtaDataSourceProperty() {
 		PersistenceProvider provider = new HibernatePersistenceProvider() {
 			@Override
 			public EntityManagerFactory createContainerEntityManagerFactory(PersistenceUnitInfo info, Map integrationOverrides) {
@@ -103,6 +133,16 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase {
 								puProperties.put( AvailableSettings.JPA_JTA_DATASOURCE, puDataSource );
 							}
 
+							@Override
+							public DataSource getJtaDataSource() {
+								return null;
+							}
+
+							@Override
+							public DataSource getNonJtaDataSource() {
+								return null;
+							}
+
 							@Override
 							public Properties getProperties() {
 								return puProperties;
@@ -142,11 +182,11 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase {
 	}
 
 	@Test
-	@FailureExpected(
+//	@FailureExpected(
-			jiraKey = "HHH-12858",
+//			jiraKey = "HHH-12858",
-			message = "Even though the JDBC settings override a DataSource *property*, it" +
+//			message = "Even though the JDBC settings override a DataSource *property*, it" +
-					" does not override a DataSource defined using the dedicated persistence.xml element"
+//					" does not override a DataSource defined using the dedicated persistence.xml element"
-	)
+//	)
 	public void testPassingIntegrationJpaJdbcOverridesForJtaDataSourceElement() {
 		PersistenceProvider provider = new HibernatePersistenceProvider() {
 			@Override
@@ -154,11 +194,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase {
 				return super.createContainerEntityManagerFactory(
 						new DelegatingPersistenceUnitInfo( info ) {
 							// inject a JPA JTA DataSource setting into the PU
-							final DataSource puDataSource;
+							final DataSource puDataSource = new DataSourceStub( "puDataSource" );
-
-							{
-								puDataSource = new DataSourceStub( "puDataSource" );
-							}
 
 							@Override
 							public DataSource getJtaDataSource() {
@@ -199,11 +235,11 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase {
 	}
 
 	@Test
-	@FailureExpected(
+//	@FailureExpected(
-			jiraKey = "HHH-12858",
+//			jiraKey = "HHH-12858",
-			message = "So it appears any use of the persistence.xml `jta-data-source` or `non-jta-data-source` " +
+//			message = "So it appears any use of the persistence.xml `jta-data-source` or `non-jta-data-source` " +
-					"have precedence over integration settings, which is also incorrect"
+//					"have precedence over integration settings, which is also incorrect"
-	)
+//	)
 	public void testPassingIntegrationJpaDataSourceOverrideForJtaDataSourceElement() {
 		final DataSource puDataSource = new DataSourceStub( "puDataSource" );
 		final DataSource integrationDataSource = new DataSourceStub( "integrationDataSource" );
@@ -255,7 +291,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase {
 	}
 
 	@Test
-	@FailureExpected( jiraKey = "HHH-12858", message = "regression - fix" )
+	@TestForIssue( jiraKey = "HHH-13640" )
 	public void testIntegrationOverridesOfPersistenceXmlDataSource() {
 
 		// mimics a DataSource defined in the persistence.xml
@@ -300,7 +336,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase {
 	}
 
 	@Test
-	@FailureExpected( jiraKey = "HHH-12858", message = "regression - fix" )
+	@TestForIssue( jiraKey = "HHH-13640" )
 	public void testIntegrationOverridesOfPersistenceXmlDataSourceWithDriverManagerInfo() {
 
 		// mimics a DataSource defined in the persistence.xml

hibernate-testing/src/main/java/org/hibernate/testing/jdbc/DataSourceStub.java

@@ -4,7 +4,7 @@
  * License: GNU Lesser General Public License (LGPL), version 2.1 or later
  * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
  */
-package org.hibernate.orm.test.bootstrap;
+package org.hibernate.testing.jdbc;
 
 import java.io.PrintWriter;
 import java.sql.Connection;
@@ -24,7 +24,7 @@ public class DataSourceStub implements DataSource {
 	private final DriverManagerConnectionProviderImpl connectionProvider;
 	private PrintWriter printWriter;
 
-	DataSourceStub(String id) {
+	public DataSourceStub(String id) {
 		this.id = id;
 		connectionProvider = new DriverManagerConnectionProviderImpl();
 		connectionProvider.configure( ConnectionProviderBuilder.getConnectionProviderProperties() );