HibernateJpaVendorAdapter preserves connection release mode for JTA

Issue: SPR-16162

spring-orm/src/main/java/org/springframework/orm/jpa/AbstractEntityManagerFactoryBean.java

@@ -280,8 +280,8 @@ public abstract class AbstractEntityManagerFactoryBean implements
 	}
 
 	/**
-	 * Return the JpaVendorAdapter implementation for this
-	 * EntityManagerFactory, or {@code null} if not known.
+	 * Return the JpaVendorAdapter implementation for this EntityManagerFactory,
+	 * or {@code null} if not known.
 	 */
 	@Nullable
 	public JpaVendorAdapter getJpaVendorAdapter() {
@@ -335,13 +335,16 @@ public abstract class AbstractEntityManagerFactoryBean implements
 
 
 	@Override
-	public final void afterPropertiesSet() throws PersistenceException {
-		if (this.jpaVendorAdapter != null) {
+	public void afterPropertiesSet() throws PersistenceException {
+		JpaVendorAdapter jpaVendorAdapter = getJpaVendorAdapter();
+		if (jpaVendorAdapter != null) {
 			if (this.persistenceProvider == null) {
-				this.persistenceProvider = this.jpaVendorAdapter.getPersistenceProvider();
+				this.persistenceProvider = jpaVendorAdapter.getPersistenceProvider();
 			}
-			Map<String, ?> vendorPropertyMap = this.jpaVendorAdapter.getJpaPropertyMap();
-			if (vendorPropertyMap != null) {
+			PersistenceUnitInfo pui = getPersistenceUnitInfo();
+			Map<String, ?> vendorPropertyMap = (pui != null ? jpaVendorAdapter.getJpaPropertyMap(pui) :
+					jpaVendorAdapter.getJpaPropertyMap());
+			if (!CollectionUtils.isEmpty(vendorPropertyMap)) {
 				vendorPropertyMap.forEach((key, value) -> {
 					if (!this.jpaPropertyMap.containsKey(key)) {
 						this.jpaPropertyMap.put(key, value);
@@ -349,25 +352,25 @@ public abstract class AbstractEntityManagerFactoryBean implements
 				});
 			}
 			if (this.entityManagerFactoryInterface == null) {
-				this.entityManagerFactoryInterface = this.jpaVendorAdapter.getEntityManagerFactoryInterface();
+				this.entityManagerFactoryInterface = jpaVendorAdapter.getEntityManagerFactoryInterface();
 				if (!ClassUtils.isVisible(this.entityManagerFactoryInterface, this.beanClassLoader)) {
 					this.entityManagerFactoryInterface = EntityManagerFactory.class;
 				}
 			}
 			if (this.entityManagerInterface == null) {
-				this.entityManagerInterface = this.jpaVendorAdapter.getEntityManagerInterface();
+				this.entityManagerInterface = jpaVendorAdapter.getEntityManagerInterface();
 				if (!ClassUtils.isVisible(this.entityManagerInterface, this.beanClassLoader)) {
 					this.entityManagerInterface = EntityManager.class;
 				}
 			}
 			if (this.jpaDialect == null) {
-				this.jpaDialect = this.jpaVendorAdapter.getJpaDialect();
+				this.jpaDialect = jpaVendorAdapter.getJpaDialect();
 			}
 		}
 
-		if (this.bootstrapExecutor != null) {
-			this.nativeEntityManagerFactoryFuture = this.bootstrapExecutor.submit(
-					this::buildNativeEntityManagerFactory);
+		AsyncTaskExecutor bootstrapExecutor = getBootstrapExecutor();
+		if (bootstrapExecutor != null) {
+			this.nativeEntityManagerFactoryFuture = bootstrapExecutor.submit(this::buildNativeEntityManagerFactory);
 		}
 		else {
 			this.nativeEntityManagerFactory = buildNativeEntityManagerFactory();
@@ -382,8 +385,9 @@ public abstract class AbstractEntityManagerFactoryBean implements
 
 	private EntityManagerFactory buildNativeEntityManagerFactory() {
 		EntityManagerFactory emf = createNativeEntityManagerFactory();
-		if (this.jpaVendorAdapter != null) {
-			this.jpaVendorAdapter.postProcessEntityManagerFactory(emf);
+		JpaVendorAdapter jpaVendorAdapter = getJpaVendorAdapter();
+		if (jpaVendorAdapter != null) {
+			jpaVendorAdapter.postProcessEntityManagerFactory(emf);
 		}
 		if (logger.isInfoEnabled()) {
 			logger.info("Initialized JPA EntityManagerFactory for persistence unit '" + getPersistenceUnitName() + "'");
@@ -401,8 +405,9 @@ public abstract class AbstractEntityManagerFactoryBean implements
 	 */
 	protected EntityManagerFactory createEntityManagerFactoryProxy(@Nullable EntityManagerFactory emf) {
 		Set<Class<?>> ifcs = new LinkedHashSet<>();
-		if (this.entityManagerFactoryInterface != null) {
-			ifcs.add(this.entityManagerFactoryInterface);
+		Class<?> entityManagerFactoryInterface = this.entityManagerFactoryInterface;
+		if (entityManagerFactoryInterface != null) {
+			ifcs.add(entityManagerFactoryInterface);
 		}
 		else if (emf != null) {
 			ifcs.addAll(ClassUtils.getAllInterfacesForClassAsSet(emf.getClass(), this.beanClassLoader));
@@ -417,8 +422,8 @@ public abstract class AbstractEntityManagerFactoryBean implements
 					new ManagedEntityManagerFactoryInvocationHandler(this));
 		}
 		catch (IllegalArgumentException ex) {
-			if (this.entityManagerFactoryInterface != null) {
-				throw new IllegalStateException("EntityManagerFactory interface [" + this.entityManagerFactoryInterface +
+			if (entityManagerFactoryInterface != null) {
+				throw new IllegalStateException("EntityManagerFactory interface [" + entityManagerFactoryInterface +
 						"] seems to conflict with Spring's EntityManagerFactoryInfo mixin - consider resetting the "+
 						"'entityManagerFactoryInterface' property to plain [javax.persistence.EntityManagerFactory]", ex);
 			}
@@ -497,7 +502,8 @@ public abstract class AbstractEntityManagerFactoryBean implements
 	@Override
 	@Nullable
 	public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
-		return (this.jpaDialect != null ? this.jpaDialect.translateExceptionIfPossible(ex) :
+		JpaDialect jpaDialect = getJpaDialect();
+		return (jpaDialect != null ? jpaDialect.translateExceptionIfPossible(ex) :
 				EntityManagerFactoryUtils.convertJpaAccessExceptionIfPossible(ex));
 	}
 

spring-orm/src/main/java/org/springframework/orm/jpa/JpaVendorAdapter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,11 +16,12 @@
 
 package org.springframework.orm.jpa;
 
+import java.util.Collections;
 import java.util.Map;
-
 import javax.persistence.EntityManager;
 import javax.persistence.EntityManagerFactory;
 import javax.persistence.spi.PersistenceProvider;
+import javax.persistence.spi.PersistenceUnitInfo;
 
 import org.springframework.lang.Nullable;
 
@@ -48,29 +49,53 @@ public interface JpaVendorAdapter {
 	 * @since 2.5.2
 	 */
 	@Nullable
-	String getPersistenceProviderRootPackage();
+	default String getPersistenceProviderRootPackage() {
+		return null;
+	}
+
+	/**
+	 * Return a Map of vendor-specific JPA properties for the given persistence
+	 * unit, typically based on settings in this JpaVendorAdapter instance.
+	 * <p>Note that there might be further JPA properties defined on the
+	 * EntityManagerFactory bean, which might potentially override individual
+	 * JPA property values specified here.
+	 * <p>This implementation delegates to {@link #getJpaPropertyMap()} for
+	 * non-unit-dependent properties. Effectively, this PersistenceUnitInfo-based
+	 * variant only needs to be implemented if there is an actual need to react
+	 * to unit-specific characteristics such as the transaction type.
+	 * @param pui the PersistenceUnitInfo for the current persistence unit
+	 * @return a Map of JPA properties, as accepted by the standard JPA bootstrap
+	 * facilities, or an empty Map if there are no properties to expose
+	 * @since 4.3.13
+	 * @see PersistenceUnitInfo#getTransactionType()
+	 * @see javax.persistence.spi.PersistenceProvider#createContainerEntityManagerFactory(PersistenceUnitInfo, Map)
+	 */
+	default Map<String, ?> getJpaPropertyMap(PersistenceUnitInfo pui) {
+		return getJpaPropertyMap();
+	}
 
 	/**
 	 * Return a Map of vendor-specific JPA properties,
 	 * typically based on settings in this JpaVendorAdapter instance.
-	 * <p>Note that there might be further JPA properties defined on
-	 * the EntityManagerFactory bean, which might potentially override
-	 * individual JPA property values specified here.
-	 * @return a Map of JPA properties, as accepted by the standard
-	 * JPA bootstrap facilities, or {@code null} or an empty Map
-	 * if there are no such properties to expose
-	 * @see javax.persistence.Persistence#createEntityManagerFactory(String, java.util.Map)
-	 * @see javax.persistence.spi.PersistenceProvider#createContainerEntityManagerFactory(javax.persistence.spi.PersistenceUnitInfo, java.util.Map)
+	 * <p>Note that there might be further JPA properties defined on the
+	 * EntityManagerFactory bean, which might potentially override individual
+	 * JPA property values specified here.
+	 * @return a Map of JPA properties, as accepted by the standard JPA bootstrap
+	 * facilities, or an empty Map if there are no properties to expose
+	 * @see javax.persistence.Persistence#createEntityManagerFactory(String, Map)
 	 */
-	@Nullable
-	Map<String, ?> getJpaPropertyMap();
+	default Map<String, ?> getJpaPropertyMap() {
+		return Collections.emptyMap();
+	}
 
 	/**
 	 * Return the vendor-specific JpaDialect implementation for this
 	 * provider, or {@code null} if there is none.
 	 */
 	@Nullable
-	JpaDialect getJpaDialect();
+	default JpaDialect getJpaDialect() {
+		return null;
+	}
 
 	/**
 	 * Return the vendor-specific EntityManagerFactory interface
@@ -80,7 +105,9 @@ public interface JpaVendorAdapter {
 	 * {@link javax.persistence.EntityManagerFactory} class here.
 	 * @since 2.5.2
 	 */
-	Class<? extends EntityManagerFactory> getEntityManagerFactoryInterface();
+	default Class<? extends EntityManagerFactory> getEntityManagerFactoryInterface() {
+		return EntityManagerFactory.class;
+	}
 
 	/**
 	 * Return the vendor-specific EntityManager interface
@@ -89,7 +116,9 @@ public interface JpaVendorAdapter {
 	 * the adapter should simply return the standard
 	 * {@link javax.persistence.EntityManager} class here.
 	 */
-	Class<? extends EntityManager> getEntityManagerInterface();
+	default Class<? extends EntityManager> getEntityManagerInterface() {
+		return EntityManager.class;
+	}
 
 	/**
 	 * Optional callback for post-processing the native EntityManagerFactory
@@ -98,6 +127,7 @@ public interface JpaVendorAdapter {
 	 * While this is not expected to be used for most providers, it is included
 	 * here as a general extension hook.
 	 */
-	void postProcessEntityManagerFactory(EntityManagerFactory emf);
+	default void postProcessEntityManagerFactory(EntityManagerFactory emf) {
+	}
 
 }

spring-orm/src/main/java/org/springframework/orm/jpa/LocalContainerEntityManagerFactoryBean.java

@@ -35,6 +35,7 @@ import org.springframework.orm.jpa.persistenceunit.DefaultPersistenceUnitManager
 import org.springframework.orm.jpa.persistenceunit.PersistenceUnitManager;
 import org.springframework.orm.jpa.persistenceunit.PersistenceUnitPostProcessor;
 import org.springframework.orm.jpa.persistenceunit.SmartPersistenceUnitInfo;
+import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
 /**
@@ -92,8 +93,7 @@ public class LocalContainerEntityManagerFactoryBean extends AbstractEntityManage
 	@Nullable
 	private PersistenceUnitManager persistenceUnitManager;
 
-	private final DefaultPersistenceUnitManager internalPersistenceUnitManager =
-			new DefaultPersistenceUnitManager();
+	private final DefaultPersistenceUnitManager internalPersistenceUnitManager = new DefaultPersistenceUnitManager();
 
 	@Nullable
 	private PersistenceUnitInfo persistenceUnitInfo;
@@ -322,7 +322,7 @@ public class LocalContainerEntityManagerFactoryBean extends AbstractEntityManage
 
 
 	@Override
-	protected EntityManagerFactory createNativeEntityManagerFactory() throws PersistenceException {
+	public void afterPropertiesSet() throws PersistenceException {
 		PersistenceUnitManager managerToUse = this.persistenceUnitManager;
 		if (this.persistenceUnitManager == null) {
 			this.internalPersistenceUnitManager.afterPropertiesSet();
@@ -338,6 +338,13 @@ public class LocalContainerEntityManagerFactoryBean extends AbstractEntityManage
 			}
 		}
 
+		super.afterPropertiesSet();
+	}
+
+	@Override
+	protected EntityManagerFactory createNativeEntityManagerFactory() throws PersistenceException {
+		Assert.state(this.persistenceUnitInfo != null, "PersistenceUnitInfo not initialized");
+
 		PersistenceProvider provider = getPersistenceProvider();
 		if (provider == null) {
 			String providerClassName = this.persistenceUnitInfo.getPersistenceProviderClassName();

spring-orm/src/main/java/org/springframework/orm/jpa/vendor/AbstractJpaVendorAdapter.java

@@ -16,9 +16,11 @@
 
 package org.springframework.orm.jpa.vendor;
 
+import java.util.Collections;
 import java.util.Map;
 import javax.persistence.EntityManager;
 import javax.persistence.EntityManagerFactory;
+import javax.persistence.spi.PersistenceUnitInfo;
 
 import org.springframework.lang.Nullable;
 import org.springframework.orm.jpa.JpaDialect;
@@ -124,9 +126,13 @@ public abstract class AbstractJpaVendorAdapter implements JpaVendorAdapter {
 	}
 
 	@Override
-	@Nullable
+	public Map<String, ?> getJpaPropertyMap(PersistenceUnitInfo pui) {
+		return getJpaPropertyMap();
+	}
+
+	@Override
 	public Map<String, ?> getJpaPropertyMap() {
-		return null;
+		return Collections.emptyMap();
 	}
 
 	@Override
@@ -145,10 +151,6 @@ public abstract class AbstractJpaVendorAdapter implements JpaVendorAdapter {
 		return EntityManager.class;
 	}
 
-	/**
-	 * Post-process the EntityManagerFactory after it has been initialized.
-	 * @param emf the EntityManagerFactory to process
-	 */
 	@Override
 	public void postProcessEntityManagerFactory(EntityManagerFactory emf) {
 	}

spring-orm/src/main/java/org/springframework/orm/jpa/vendor/HibernateJpaVendorAdapter.java

@@ -21,6 +21,8 @@ import java.util.Map;
 import javax.persistence.EntityManager;
 import javax.persistence.EntityManagerFactory;
 import javax.persistence.spi.PersistenceProvider;
+import javax.persistence.spi.PersistenceUnitInfo;
+import javax.persistence.spi.PersistenceUnitTransactionType;
 
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.dialect.DB2Dialect;
@@ -85,13 +87,15 @@ public class HibernateJpaVendorAdapter extends AbstractJpaVendorAdapter {
 	 * new connection handling mode {@code DELAYED_ACQUISITION_AND_HOLD} in that case
 	 * unless a user-specified connection handling mode property indicates otherwise;
 	 * switch this flag to {@code false} to avoid that interference.
-	 * <p><b>NOTE: Per the explanation above, you may have to turn this flag off
-	 * when using Hibernate in a JTA environment, e.g. on WebLogic.</b> Alternatively,
-	 * set Hibernate 5.2's "hibernate.connection.handling_mode" property to
-	 * "DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION" or even
+	 * <p><b>NOTE: For a persistence unit with transaction type JTA e.g. on WebLogic,
+	 * the connection release mode will never be altered from its provider default,
+	 * i.e. not be forced to {@code DELAYED_ACQUISITION_AND_HOLD} by this flag.</b>
+	 * Alternatively, set Hibernate 5.2's "hibernate.connection.handling_mode"
+	 * property to "DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION" or even
 	 * "DELAYED_ACQUISITION_AND_RELEASE_AFTER_STATEMENT" in such a scenario.
 	 * @since 4.3.1
-	 * @see #getJpaPropertyMap()
+	 * @see PersistenceUnitInfo#getTransactionType()
+	 * @see #getJpaPropertyMap(PersistenceUnitInfo)
 	 * @see HibernateJpaDialect#beginTransaction
 	 */
 	public void setPrepareConnection(boolean prepareConnection) {
@@ -109,6 +113,32 @@ public class HibernateJpaVendorAdapter extends AbstractJpaVendorAdapter {
 		return "org.hibernate";
 	}
 
+	@Override
+	public Map<String, Object> getJpaPropertyMap(PersistenceUnitInfo pui) {
+		Map<String, Object> jpaProperties = getJpaPropertyMap();
+
+		if (this.jpaDialect.prepareConnection && pui.getTransactionType() != PersistenceUnitTransactionType.JTA) {
+			// Hibernate 5.1/5.2: manually enforce connection release mode ON_CLOSE (the former default)
+			try {
+				// Try Hibernate 5.2
+				AvailableSettings.class.getField("CONNECTION_HANDLING");
+				jpaProperties.put("hibernate.connection.handling_mode", "DELAYED_ACQUISITION_AND_HOLD");
+			}
+			catch (NoSuchFieldException ex) {
+				// Try Hibernate 5.1
+				try {
+					AvailableSettings.class.getField("ACQUIRE_CONNECTIONS");
+					jpaProperties.put("hibernate.connection.release_mode", "ON_CLOSE");
+				}
+				catch (NoSuchFieldException ex2) {
+					// on Hibernate 5.0.x or lower - no need to change the default there
+				}
+			}
+		}
+
+		return jpaProperties;
+	}
+
 	@Override
 	public Map<String, Object> getJpaPropertyMap() {
 		Map<String, Object> jpaProperties = new HashMap<>();
@@ -130,25 +160,6 @@ public class HibernateJpaVendorAdapter extends AbstractJpaVendorAdapter {
 			jpaProperties.put(AvailableSettings.SHOW_SQL, "true");
 		}
 
-		if (this.jpaDialect.prepareConnection) {
-			// Hibernate 5.1/5.2: manually enforce connection release mode ON_CLOSE (the former default)
-			try {
-				// Try Hibernate 5.2
-				AvailableSettings.class.getField("CONNECTION_HANDLING");
-				jpaProperties.put("hibernate.connection.handling_mode", "DELAYED_ACQUISITION_AND_HOLD");
-			}
-			catch (NoSuchFieldException ex) {
-				// Try Hibernate 5.1
-				try {
-					AvailableSettings.class.getField("ACQUIRE_CONNECTIONS");
-					jpaProperties.put("hibernate.connection.release_mode", "ON_CLOSE");
-				}
-				catch (NoSuchFieldException ex2) {
-					// on Hibernate 5.0.x or lower - no need to change the default there
-				}
-			}
-		}
-
 		return jpaProperties;
 	}