HibernateJpaVendorAdapter preserves connection release mode for JTA

Issue: SPR-16162

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

@@ -55,7 +55,6 @@ import org.springframework.beans.factory.InitializingBean;
 import org.springframework.core.task.AsyncTaskExecutor;
 import org.springframework.dao.DataAccessException;
 import org.springframework.dao.support.PersistenceExceptionTranslator;
-import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.CollectionUtils;
 
@@ -264,8 +263,8 @@ public abstract class AbstractEntityManagerFactoryBean implements
 	}
 
 	/**
-	 * Return the JpaVendorAdapter implementation for this
+	 * Return the JpaVendorAdapter implementation for this EntityManagerFactory,
-	 * EntityManagerFactory, or {@code null} if not known.
+	 * or {@code null} if not known.
 	 */
 	public JpaVendorAdapter getJpaVendorAdapter() {
 		return this.jpaVendorAdapter;
@@ -317,13 +316,26 @@ public abstract class AbstractEntityManagerFactoryBean implements
 
 
 	@Override
-	public final void afterPropertiesSet() throws PersistenceException {
+	public void afterPropertiesSet() throws PersistenceException {
-		if (this.jpaVendorAdapter != null) {
+		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();
+			PersistenceUnitInfo pui = getPersistenceUnitInfo();
-			if (vendorPropertyMap != null) {
+			Map<String, ?> vendorPropertyMap = null;
+			if (pui != null) {
+				try {
+					vendorPropertyMap = jpaVendorAdapter.getJpaPropertyMap(pui);
+				}
+				catch (AbstractMethodError err) {
+					// Spring 4.3.13 getJpaPropertyMap(PersistenceUnitInfo) not implemented
+				}
+			}
+			if (vendorPropertyMap == null) {
+				vendorPropertyMap = jpaVendorAdapter.getJpaPropertyMap();
+			}
+			if (!CollectionUtils.isEmpty(vendorPropertyMap)) {
 				for (Map.Entry<String, ?> entry : vendorPropertyMap.entrySet()) {
 					if (!this.jpaPropertyMap.containsKey(entry.getKey())) {
 						this.jpaPropertyMap.put(entry.getKey(), entry.getValue());
@@ -331,19 +343,19 @@ 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();
 			}
 		}
 
@@ -372,8 +384,9 @@ public abstract class AbstractEntityManagerFactoryBean implements
 			throw new IllegalStateException(
 					"JPA PersistenceProvider returned null EntityManagerFactory - check your JPA provider setup!");
 		}
-		if (this.jpaVendorAdapter != null) {
+		JpaVendorAdapter jpaVendorAdapter = getJpaVendorAdapter();
-			this.jpaVendorAdapter.postProcessEntityManagerFactory(emf);
+		if (jpaVendorAdapter != null) {
+			jpaVendorAdapter.postProcessEntityManagerFactory(emf);
 		}
 		if (logger.isInfoEnabled()) {
 			logger.info("Initialized JPA EntityManagerFactory for persistence unit '" + getPersistenceUnitName() + "'");
@@ -390,8 +403,9 @@ public abstract class AbstractEntityManagerFactoryBean implements
 	 */
 	protected EntityManagerFactory createEntityManagerFactoryProxy(EntityManagerFactory emf) {
 		Set<Class<?>> ifcs = new LinkedHashSet<Class<?>>();
-		if (this.entityManagerFactoryInterface != null) {
+		Class<?> entityManagerFactoryInterface = this.entityManagerFactoryInterface;
-			ifcs.add(this.entityManagerFactoryInterface);
+		if (entityManagerFactoryInterface != null) {
+			ifcs.add(entityManagerFactoryInterface);
 		}
 		else if (emf != null) {
 			ifcs.addAll(ClassUtils.getAllInterfacesForClassAsSet(emf.getClass(), this.beanClassLoader));
@@ -406,8 +420,8 @@ public abstract class AbstractEntityManagerFactoryBean implements
 					new ManagedEntityManagerFactoryInvocationHandler(this));
 		}
 		catch (IllegalArgumentException ex) {
-			if (this.entityManagerFactoryInterface != null) {
+			if (entityManagerFactoryInterface != null) {
-				throw new IllegalStateException("EntityManagerFactory interface [" + this.entityManagerFactoryInterface +
+				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);
 			}
@@ -485,7 +499,8 @@ public abstract class AbstractEntityManagerFactoryBean implements
 	 */
 	@Override
 	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.
@@ -20,6 +20,7 @@ import java.util.Map;
 import javax.persistence.EntityManager;
 import javax.persistence.EntityManagerFactory;
 import javax.persistence.spi.PersistenceProvider;
+import javax.persistence.spi.PersistenceUnitInfo;
 
 /**
  * SPI interface that allows to plug in vendor-specific behavior
@@ -46,17 +47,40 @@ public interface JpaVendorAdapter {
 	 */
 	String getPersistenceProviderRootPackage();
 
+	/**
+	 * 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.
+	 * <p><b>NOTE:</b> This variant will only be invoked in case of Java EE style
+	 * container bootstrapping where a {@link PersistenceUnitInfo} is present
+	 * (i.e. {@link LocalContainerEntityManagerFactoryBean}. In case of simple
+	 * Java SE style bootstrapping via {@link javax.persistence.Persistence}
+	 * (i.e. {@link LocalEntityManagerFactoryBean}), the parameter-less
+	 * {@link #getJpaPropertyMap()} variant will be called directly.
+	 * @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)
+	 */
+	Map<String, ?> getJpaPropertyMap(PersistenceUnitInfo pui);
+
 	/**
 	 * 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
+	 * <p>Note that there might be further JPA properties defined on the
-	 * the EntityManagerFactory bean, which might potentially override
+	 * EntityManagerFactory bean, which might potentially override individual
-	 * individual JPA property values specified here.
+	 * JPA property values specified here.
-	 * @return a Map of JPA properties, as accepted by the standard
+	 * @return a Map of JPA properties, as accepted by the standard JPA bootstrap
-	 * JPA bootstrap facilities, or {@code null} or an empty Map
+	 * facilities, or an empty Map if there are no properties to expose
-	 * if there are no such properties to expose
+	 * @see javax.persistence.Persistence#createEntityManagerFactory(String, Map)
-	 * @see javax.persistence.Persistence#createEntityManagerFactory(String, java.util.Map)
-	 * @see javax.persistence.spi.PersistenceProvider#createContainerEntityManagerFactory(javax.persistence.spi.PersistenceUnitInfo, java.util.Map)
 	 */
 	Map<String, ?> getJpaPropertyMap();
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 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.
@@ -34,6 +34,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;
 
 /**
@@ -91,8 +92,7 @@ public class LocalContainerEntityManagerFactoryBean extends AbstractEntityManage
 
 	private PersistenceUnitManager persistenceUnitManager;
 
-	private final DefaultPersistenceUnitManager internalPersistenceUnitManager =
+	private final DefaultPersistenceUnitManager internalPersistenceUnitManager = new DefaultPersistenceUnitManager();
-			new DefaultPersistenceUnitManager();
 
 	private PersistenceUnitInfo persistenceUnitInfo;
 
@@ -319,7 +319,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();
@@ -333,6 +333,13 @@ public class LocalContainerEntityManagerFactoryBean extends AbstractEntityManage
 					jpaVendorAdapter.getPersistenceProviderRootPackage());
 		}
 
+		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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 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,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.orm.jpa.JpaDialect;
 import org.springframework.orm.jpa.JpaVendorAdapter;
@@ -119,9 +121,14 @@ public abstract class AbstractJpaVendorAdapter implements JpaVendorAdapter {
 		return null;
 	}
 
+	@Override
+	public Map<String, ?> getJpaPropertyMap(PersistenceUnitInfo pui) {
+		return getJpaPropertyMap();
+	}
+
 	@Override
 	public Map<String, ?> getJpaPropertyMap() {
-		return null;
+		return Collections.emptyMap();
 	}
 
 	@Override
@@ -139,10 +146,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.Environment;
 import org.hibernate.dialect.DB2Dialect;
@@ -113,13 +115,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
+	 * <p><b>NOTE: For a persistence unit with transaction type JTA e.g. on WebLogic,
-	 * when using Hibernate in a JTA environment, e.g. on WebLogic.</b> Alternatively,
+	 * the connection release mode will never be altered from its provider default,
-	 * set Hibernate 5.2's "hibernate.connection.handling_mode" property to
+	 * i.e. not be forced to {@code DELAYED_ACQUISITION_AND_HOLD} by this flag.</b>
-	 * "DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION" or even
+	 * 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) {
@@ -137,8 +141,18 @@ public class HibernateJpaVendorAdapter extends AbstractJpaVendorAdapter {
 		return "org.hibernate";
 	}
 
+	@Override
+	public Map<String, Object> getJpaPropertyMap(PersistenceUnitInfo pui) {
+		return buildJpaPropertyMap(this.jpaDialect.prepareConnection &&
+				pui.getTransactionType() != PersistenceUnitTransactionType.JTA);
+	}
+
 	@Override
 	public Map<String, Object> getJpaPropertyMap() {
+		return buildJpaPropertyMap(this.jpaDialect.prepareConnection);
+	}
+
+	private Map<String, Object> buildJpaPropertyMap(boolean connectionReleaseOnClose) {
 		Map<String, Object> jpaProperties = new HashMap<String, Object>();
 
 		if (getDatabasePlatform() != null) {