Allow beans to be used as Hibernate naming strategies

Previously, custom Hibernate naming strategies could only be
configured via properties. This allowed a fully-qualified classname to
be specified, but did not allow a naming strategy instance to be used.

This commit updates HibernateJpaConfiguration to use
ImplicitNamingStrategy and PhysicalNamingStrategy beans if they
exist. If both a bean exists and the equivalent property has been set,
the bean wins.

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DataSourceInitializedPublisher.java

@@ -92,8 +92,8 @@ class DataSourceInitializedPublisher implements BeanPostProcessor {
 		}
 		String defaultDdlAuto = (EmbeddedDatabaseConnection.isEmbedded(dataSource)
 				? "create-drop" : "none");
-		Map<String, String> hibernate = this.properties
-				.getHibernateProperties(defaultDdlAuto);
+		Map<String, Object> hibernate = this.properties
+				.getHibernateProperties(new HibernateSettings().ddlAuto(defaultDdlAuto));
 		if (hibernate.containsKey("hibernate.hbm2ddl.auto")) {
 			return true;
 		}

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaConfiguration.java

@@ -26,6 +26,8 @@ import javax.sql.DataSource;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.hibernate.boot.model.naming.ImplicitNamingStrategy;
+import org.hibernate.boot.model.naming.PhysicalNamingStrategy;
 
 import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnSingleCandidate;
@@ -81,17 +83,25 @@ class HibernateJpaConfiguration extends JpaBaseConfiguration {
 
 	private DataSourcePoolMetadataProvider poolMetadataProvider;
 
+	private final PhysicalNamingStrategy physicalNamingStrategy;
+
+	private final ImplicitNamingStrategy implicitNamingStrategy;
+
 	HibernateJpaConfiguration(DataSource dataSource, JpaProperties jpaProperties,
 			ObjectProvider<JtaTransactionManager> jtaTransactionManager,
 			ObjectProvider<TransactionManagerCustomizers> transactionManagerCustomizers,
 			ObjectProvider<Collection<DataSourcePoolMetadataProvider>> metadataProviders,
-			ObjectProvider<List<SchemaManagementProvider>> providers) {
+			ObjectProvider<List<SchemaManagementProvider>> providers,
+			ObjectProvider<PhysicalNamingStrategy> physicalNamingStrategy,
+			ObjectProvider<ImplicitNamingStrategy> implicitNamingStrategy) {
 		super(dataSource, jpaProperties, jtaTransactionManager,
 				transactionManagerCustomizers);
 		this.defaultDdlAutoProvider = new HibernateDefaultDdlAutoProvider(
 				providers.getIfAvailable(Collections::emptyList));
 		this.poolMetadataProvider = new CompositeDataSourcePoolMetadataProvider(
 				metadataProviders.getIfAvailable());
+		this.physicalNamingStrategy = physicalNamingStrategy.getIfAvailable();
+		this.implicitNamingStrategy = implicitNamingStrategy.getIfAvailable();
 	}
 
 	@Override
@@ -104,7 +114,10 @@ class HibernateJpaConfiguration extends JpaBaseConfiguration {
 		Map<String, Object> vendorProperties = new LinkedHashMap<>();
 		String defaultDdlMode = this.defaultDdlAutoProvider
 				.getDefaultDdlAuto(getDataSource());
-		vendorProperties.putAll(getProperties().getHibernateProperties(defaultDdlMode));
+		vendorProperties.putAll(getProperties()
+				.getHibernateProperties(new HibernateSettings().ddlAuto(defaultDdlMode)
+						.implicitNamingStrategy(this.implicitNamingStrategy)
+						.physicalNamingStrategy(this.physicalNamingStrategy)));
 		return vendorProperties;
 	}
 

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateSettings.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2012-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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.autoconfigure.orm.jpa;
+
+import org.hibernate.boot.model.naming.ImplicitNamingStrategy;
+import org.hibernate.boot.model.naming.PhysicalNamingStrategy;
+
+/**
+ * Settings to apply when configuring Hibernate.
+ *
+ * @author Andy Wilkinson
+ * @since 2.0.0
+ */
+public class HibernateSettings {
+
+	private String ddlAuto;
+
+	private ImplicitNamingStrategy implicitNamingStrategy;
+
+	private PhysicalNamingStrategy physicalNamingStrategy;
+
+	public HibernateSettings ddlAuto(String ddlAuto) {
+		this.ddlAuto = ddlAuto;
+		return this;
+	}
+
+	public String getDdlAuto() {
+		return this.ddlAuto;
+	}
+
+	public HibernateSettings implicitNamingStrategy(
+			ImplicitNamingStrategy implicitNamingStrategy) {
+		this.implicitNamingStrategy = implicitNamingStrategy;
+		return this;
+	}
+
+	public ImplicitNamingStrategy getImplicitNamingStrategy() {
+		return this.implicitNamingStrategy;
+	}
+
+	public HibernateSettings physicalNamingStrategy(
+			PhysicalNamingStrategy physicalNamingStrategy) {
+		this.physicalNamingStrategy = physicalNamingStrategy;
+		return this;
+	}
+
+	public PhysicalNamingStrategy getPhysicalNamingStrategy() {
+		return this.physicalNamingStrategy;
+	}
+
+}

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java

@@ -23,6 +23,9 @@ import java.util.Map;
 
 import javax.sql.DataSource;
 
+import org.hibernate.boot.model.naming.ImplicitNamingStrategy;
+import org.hibernate.boot.model.naming.PhysicalNamingStrategy;
+
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.orm.jpa.vendor.Database;
 import org.springframework.util.StringUtils;
@@ -143,11 +146,11 @@ public class JpaProperties {
 	/**
 	 * Get configuration properties for the initialization of the main Hibernate
 	 * EntityManagerFactory.
-	 * @param defaultDdlAuto the default DDL auto (can be {@code null})
+	 * @param settings the settings to apply when determining the configuration properties
 	 * @return some Hibernate properties for configuration
 	 */
-	public Map<String, String> getHibernateProperties(String defaultDdlAuto) {
-		return this.hibernate.getAdditionalProperties(this.properties, defaultDdlAuto);
+	public Map<String, Object> getHibernateProperties(HibernateSettings settings) {
+		return this.hibernate.getAdditionalProperties(this.properties, settings);
 	}
 
 	/**
@@ -205,12 +208,14 @@ public class JpaProperties {
 			return this.naming;
 		}
 
-		private Map<String, String> getAdditionalProperties(Map<String, String> existing,
-				String defaultDdlAuto) {
-			Map<String, String> result = new HashMap<>(existing);
+		private Map<String, Object> getAdditionalProperties(Map<String, String> existing,
+				HibernateSettings settings) {
+			Map<String, Object> result = new HashMap<>(existing);
 			applyNewIdGeneratorMappings(result);
-			getNaming().applyNamingStrategies(result);
-			String ddlAuto = determineDdlAuto(existing, defaultDdlAuto);
+			getNaming().applyNamingStrategies(result,
+					settings.getImplicitNamingStrategy(),
+					settings.getPhysicalNamingStrategy());
+			String ddlAuto = determineDdlAuto(existing, settings.getDdlAuto());
 			if (StringUtils.hasText(ddlAuto) && !"none".equals(ddlAuto)) {
 				result.put("hibernate.hbm2ddl.auto", ddlAuto);
 			}
@@ -220,7 +225,7 @@ public class JpaProperties {
 			return result;
 		}
 
-		private void applyNewIdGeneratorMappings(Map<String, String> result) {
+		private void applyNewIdGeneratorMappings(Map<String, Object> result) {
 			if (this.useNewIdGeneratorMappings != null) {
 				result.put(USE_NEW_ID_GENERATOR_MAPPINGS,
 						this.useNewIdGeneratorMappings.toString());
@@ -277,15 +282,21 @@ public class JpaProperties {
 			this.physicalStrategy = physicalStrategy;
 		}
 
-		private void applyNamingStrategies(Map<String, String> properties) {
-			applyNamingStrategy(properties, "hibernate.implicit_naming_strategy",
-					this.implicitStrategy, DEFAULT_IMPLICIT_STRATEGY);
-			applyNamingStrategy(properties, "hibernate.physical_naming_strategy",
-					this.physicalStrategy, DEFAULT_PHYSICAL_STRATEGY);
+		private void applyNamingStrategies(Map<String, Object> properties,
+				ImplicitNamingStrategy implicitStrategyBean,
+				PhysicalNamingStrategy physicalStrategyBean) {
+			applyNamingStrategy(properties,
+					"hibernate.implicit_naming_strategy", implicitStrategyBean != null
+							? implicitStrategyBean : this.implicitStrategy,
+					DEFAULT_IMPLICIT_STRATEGY);
+			applyNamingStrategy(properties,
+					"hibernate.physical_naming_strategy", physicalStrategyBean != null
+							? physicalStrategyBean : this.physicalStrategy,
+					DEFAULT_PHYSICAL_STRATEGY);
 		}
 
-		private void applyNamingStrategy(Map<String, String> properties, String key,
-				String strategy, String defaultStrategy) {
+		private void applyNamingStrategy(Map<String, Object> properties, String key,
+				Object strategy, Object defaultStrategy) {
 			if (strategy != null) {
 				properties.put(key, strategy);
 			}

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/CustomHibernateJpaAutoConfigurationTests.java

@@ -23,6 +23,10 @@ import java.util.Map;
 
 import javax.sql.DataSource;
 
+import org.hibernate.boot.model.naming.ImplicitNamingStrategy;
+import org.hibernate.boot.model.naming.ImplicitNamingStrategyJpaCompliantImpl;
+import org.hibernate.boot.model.naming.PhysicalNamingStrategy;
+import org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl;
 import org.junit.Test;
 
 import org.springframework.boot.autoconfigure.AutoConfigurations;
@@ -63,13 +67,32 @@ public class CustomHibernateJpaAutoConfigurationTests {
 								+ "org.hibernate.cfg.naming.ImprovedNamingStrategyDelegator")
 				.run((context) -> {
 					JpaProperties bean = context.getBean(JpaProperties.class);
-					Map<String, String> hibernateProperties = bean
-							.getHibernateProperties("create-drop");
+					Map<String, Object> hibernateProperties = bean.getHibernateProperties(
+							new HibernateSettings().ddlAuto("create-drop"));
 					assertThat(hibernateProperties.get("hibernate.ejb.naming_strategy"))
 							.isNull();
 				});
 	}
 
+	@Test
+	public void namingStrategyBeansAreUsed() {
+		this.contextRunner.withUserConfiguration(NamingStrategyConfiguration.class)
+				.withPropertyValues(
+						"spring.datasource.url:jdbc:h2:mem:naming-strategy-beans")
+				.run((context) -> {
+					HibernateJpaConfiguration jpaConfiguration = context
+							.getBean(HibernateJpaConfiguration.class);
+					Map<String, Object> hibernateProperties = jpaConfiguration
+							.getVendorProperties();
+					assertThat(hibernateProperties
+							.get("hibernate.implicit_naming_strategy")).isEqualTo(
+									NamingStrategyConfiguration.implicitNamingStrategy);
+					assertThat(hibernateProperties
+							.get("hibernate.physical_naming_strategy")).isEqualTo(
+									NamingStrategyConfiguration.physicalNamingStrategy);
+				});
+	}
+
 	@Test
 	public void defaultDatabaseForH2() {
 		this.contextRunner.withPropertyValues("spring.datasource.url:jdbc:h2:mem:testdb",
@@ -107,4 +130,23 @@ public class CustomHibernateJpaAutoConfigurationTests {
 
 	}
 
+	@Configuration
+	static class NamingStrategyConfiguration {
+
+		static final ImplicitNamingStrategy implicitNamingStrategy = new ImplicitNamingStrategyJpaCompliantImpl();
+
+		static final PhysicalNamingStrategy physicalNamingStrategy = new PhysicalNamingStrategyStandardImpl();
+
+		@Bean
+		public ImplicitNamingStrategy implicitNamingStrategy() {
+			return implicitNamingStrategy;
+		}
+
+		@Bean
+		public PhysicalNamingStrategy physicalNamingStrategy() {
+			return physicalNamingStrategy;
+		}
+
+	}
+
 }

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/JpaPropertiesTests.java

@@ -23,6 +23,8 @@ import java.util.Map;
 
 import javax.sql.DataSource;
 
+import org.hibernate.boot.model.naming.ImplicitNamingStrategy;
+import org.hibernate.boot.model.naming.PhysicalNamingStrategy;
 import org.hibernate.cfg.AvailableSettings;
 import org.junit.After;
 import org.junit.Test;
@@ -61,8 +63,8 @@ public class JpaPropertiesTests {
 	@Test
 	public void noCustomNamingStrategy() throws Exception {
 		JpaProperties properties = load();
-		Map<String, String> hibernateProperties = properties
-				.getHibernateProperties("none");
+		Map<String, Object> hibernateProperties = properties
+				.getHibernateProperties(new HibernateSettings().ddlAuto("none"));
 		assertThat(hibernateProperties)
 				.doesNotContainKeys("hibernate.ejb.naming_strategy");
 		assertThat(hibernateProperties).containsEntry(
@@ -78,8 +80,8 @@ public class JpaPropertiesTests {
 		JpaProperties properties = load(
 				"spring.jpa.hibernate.naming.implicit-strategy:com.example.Implicit",
 				"spring.jpa.hibernate.naming.physical-strategy:com.example.Physical");
-		Map<String, String> hibernateProperties = properties
-				.getHibernateProperties("none");
+		Map<String, Object> hibernateProperties = properties
+				.getHibernateProperties(new HibernateSettings().ddlAuto("none"));
 		assertThat(hibernateProperties).contains(
 				entry("hibernate.implicit_naming_strategy", "com.example.Implicit"),
 				entry("hibernate.physical_naming_strategy", "com.example.Physical"));
@@ -87,13 +89,48 @@ public class JpaPropertiesTests {
 				.doesNotContainKeys("hibernate.ejb.naming_strategy");
 	}
 
+	@Test
+	public void namingStrategyInstancesCanBeUsed() throws Exception {
+		JpaProperties properties = load();
+		ImplicitNamingStrategy implicitStrategy = mock(ImplicitNamingStrategy.class);
+		PhysicalNamingStrategy physicalStrategy = mock(PhysicalNamingStrategy.class);
+		Map<String, Object> hibernateProperties = properties
+				.getHibernateProperties(new HibernateSettings().ddlAuto("none")
+						.implicitNamingStrategy(implicitStrategy)
+						.physicalNamingStrategy(physicalStrategy));
+		assertThat(hibernateProperties).contains(
+				entry("hibernate.implicit_naming_strategy", implicitStrategy),
+				entry("hibernate.physical_naming_strategy", physicalStrategy));
+		assertThat(hibernateProperties)
+				.doesNotContainKeys("hibernate.ejb.naming_strategy");
+	}
+
+	@Test
+	public void namingStrategyInstancesTakePrecedenceOverNamingStrategyProperties()
+			throws Exception {
+		JpaProperties properties = load(
+				"spring.jpa.hibernate.naming.implicit-strategy:com.example.Implicit",
+				"spring.jpa.hibernate.naming.physical-strategy:com.example.Physical");
+		ImplicitNamingStrategy implicitStrategy = mock(ImplicitNamingStrategy.class);
+		PhysicalNamingStrategy physicalStrategy = mock(PhysicalNamingStrategy.class);
+		Map<String, Object> hibernateProperties = properties
+				.getHibernateProperties(new HibernateSettings().ddlAuto("none")
+						.implicitNamingStrategy(implicitStrategy)
+						.physicalNamingStrategy(physicalStrategy));
+		assertThat(hibernateProperties).contains(
+				entry("hibernate.implicit_naming_strategy", implicitStrategy),
+				entry("hibernate.physical_naming_strategy", physicalStrategy));
+		assertThat(hibernateProperties)
+				.doesNotContainKeys("hibernate.ejb.naming_strategy");
+	}
+
 	@Test
 	public void hibernate5CustomNamingStrategiesViaJpaProperties() throws Exception {
 		JpaProperties properties = load(
 				"spring.jpa.properties.hibernate.implicit_naming_strategy:com.example.Implicit",
 				"spring.jpa.properties.hibernate.physical_naming_strategy:com.example.Physical");
-		Map<String, String> hibernateProperties = properties
-				.getHibernateProperties("none");
+		Map<String, Object> hibernateProperties = properties
+				.getHibernateProperties(new HibernateSettings().ddlAuto("none"));
 		// You can override them as we don't provide any default
 		assertThat(hibernateProperties).contains(
 				entry("hibernate.implicit_naming_strategy", "com.example.Implicit"),
@@ -105,8 +142,8 @@ public class JpaPropertiesTests {
 	@Test
 	public void useNewIdGeneratorMappingsDefault() throws Exception {
 		JpaProperties properties = load();
-		Map<String, String> hibernateProperties = properties
-				.getHibernateProperties("none");
+		Map<String, Object> hibernateProperties = properties
+				.getHibernateProperties(new HibernateSettings().ddlAuto("none"));
 		assertThat(hibernateProperties)
 				.containsEntry(AvailableSettings.USE_NEW_ID_GENERATOR_MAPPINGS, "true");
 	}
@@ -115,8 +152,8 @@ public class JpaPropertiesTests {
 	public void useNewIdGeneratorMappingsFalse() throws Exception {
 		JpaProperties properties = load(
 				"spring.jpa.hibernate.use-new-id-generator-mappings:false");
-		Map<String, String> hibernateProperties = properties
-				.getHibernateProperties("none");
+		Map<String, Object> hibernateProperties = properties
+				.getHibernateProperties(new HibernateSettings().ddlAuto("none"));
 		assertThat(hibernateProperties)
 				.containsEntry(AvailableSettings.USE_NEW_ID_GENERATOR_MAPPINGS, "false");
 	}