From c22e9a5514fa3c6108b34062e27d5a870098fbb7 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Sat, 5 May 2018 07:21:37 +0200 Subject: [PATCH] Make sure that JpaProperties does not have Hibernate imports This commit removes Hibernate imports from JpaProperties so that it can be used with another JPA provider. This commit is a breaking change for an internal, yet public class used to transmit customizations provided by the user. It does not change the external functionality though: naming strategies defined as bean are still taken into account and a customizer has a chance to override it regardless. Closes gh-13043 --- .../orm/jpa/HibernateJpaConfiguration.java | 56 ++++++-- .../orm/jpa/HibernateSettings.java | 27 ---- .../autoconfigure/orm/jpa/JpaProperties.java | 19 +-- .../HibernateJpaAutoConfigurationTests.java | 121 ++++++++++++++++++ .../orm/jpa/JpaPropertiesTests.java | 78 ----------- 5 files changed, 171 insertions(+), 130 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaConfiguration.java index 8c4bf82d3e..d234631853 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaConfiguration.java @@ -19,6 +19,7 @@ package org.springframework.boot.autoconfigure.orm.jpa; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -84,10 +85,6 @@ class HibernateJpaConfiguration extends JpaBaseConfiguration { private DataSourcePoolMetadataProvider poolMetadataProvider; - private final PhysicalNamingStrategy physicalNamingStrategy; - - private final ImplicitNamingStrategy implicitNamingStrategy; - private final List hibernatePropertiesCustomizers; HibernateJpaConfiguration(DataSource dataSource, JpaProperties jpaProperties, @@ -104,10 +101,23 @@ class HibernateJpaConfiguration extends JpaBaseConfiguration { providers.getIfAvailable(Collections::emptyList)); this.poolMetadataProvider = new CompositeDataSourcePoolMetadataProvider( metadataProviders.getIfAvailable()); - this.physicalNamingStrategy = physicalNamingStrategy.getIfAvailable(); - this.implicitNamingStrategy = implicitNamingStrategy.getIfAvailable(); - this.hibernatePropertiesCustomizers = hibernatePropertiesCustomizers - .getIfAvailable(() -> Collections.emptyList()); + this.hibernatePropertiesCustomizers = determineHibernatePropertiesCustomizers( + physicalNamingStrategy.getIfAvailable(), + implicitNamingStrategy.getIfAvailable(), + hibernatePropertiesCustomizers.getIfAvailable(Collections::emptyList)); + } + + private List determineHibernatePropertiesCustomizers( + PhysicalNamingStrategy physicalNamingStrategy, + ImplicitNamingStrategy implicitNamingStrategy, + List hibernatePropertiesCustomizers) { + if (physicalNamingStrategy != null || implicitNamingStrategy != null) { + LinkedList customizers = new LinkedList<>(hibernatePropertiesCustomizers); + customizers.addFirst(new NamingStrategiesHibernatePropertiesCustomizer( + physicalNamingStrategy, implicitNamingStrategy)); + return customizers; + } + return hibernatePropertiesCustomizers; } @Override @@ -121,8 +131,6 @@ class HibernateJpaConfiguration extends JpaBaseConfiguration { .getDefaultDdlAuto(getDataSource()); return new LinkedHashMap<>(getProperties() .getHibernateProperties(new HibernateSettings().ddlAuto(defaultDdlMode) - .implicitNamingStrategy(this.implicitNamingStrategy) - .physicalNamingStrategy(this.physicalNamingStrategy) .hibernatePropertiesCustomizers( this.hibernatePropertiesCustomizers))); } @@ -232,4 +240,32 @@ class HibernateJpaConfiguration extends JpaBaseConfiguration { throw new IllegalStateException("Could not configure JTA platform"); } + private static class NamingStrategiesHibernatePropertiesCustomizer + implements HibernatePropertiesCustomizer { + + private final PhysicalNamingStrategy physicalNamingStrategy; + + private final ImplicitNamingStrategy implicitNamingStrategy; + + NamingStrategiesHibernatePropertiesCustomizer( + PhysicalNamingStrategy physicalNamingStrategy, + ImplicitNamingStrategy implicitNamingStrategy) { + this.physicalNamingStrategy = physicalNamingStrategy; + this.implicitNamingStrategy = implicitNamingStrategy; + } + + @Override + public void customize(Map hibernateProperties) { + if (this.physicalNamingStrategy != null) { + hibernateProperties.put("hibernate.physical_naming_strategy", + this.physicalNamingStrategy); + } + if (this.implicitNamingStrategy != null) { + hibernateProperties.put("hibernate.implicit_naming_strategy", + this.implicitNamingStrategy); + } + } + + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateSettings.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateSettings.java index 052946a1fe..8b21be88db 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateSettings.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateSettings.java @@ -20,9 +20,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.function.Supplier; -import org.hibernate.boot.model.naming.ImplicitNamingStrategy; -import org.hibernate.boot.model.naming.PhysicalNamingStrategy; - /** * Settings to apply when configuring Hibernate. * @@ -33,10 +30,6 @@ public class HibernateSettings { private Supplier ddlAuto; - private ImplicitNamingStrategy implicitNamingStrategy; - - private PhysicalNamingStrategy physicalNamingStrategy; - private Collection hibernatePropertiesCustomizers; public HibernateSettings ddlAuto(Supplier ddlAuto) { @@ -60,26 +53,6 @@ public class HibernateSettings { return (this.ddlAuto != null ? this.ddlAuto.get() : null); } - 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; - } - public HibernateSettings hibernatePropertiesCustomizers( Collection hibernatePropertiesCustomizers) { this.hibernatePropertiesCustomizers = new ArrayList<>( diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java index 0507663b9f..99b198928b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java @@ -25,9 +25,6 @@ import java.util.function.Supplier; 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.ObjectUtils; @@ -215,9 +212,7 @@ public class JpaProperties { HibernateSettings settings) { Map result = new HashMap<>(existing); applyNewIdGeneratorMappings(result); - getNaming().applyNamingStrategies(result, - settings.getImplicitNamingStrategy(), - settings.getPhysicalNamingStrategy()); + getNaming().applyNamingStrategies(result); String ddlAuto = determineDdlAuto(existing, settings::getDdlAuto); if (StringUtils.hasText(ddlAuto) && !"none".equals(ddlAuto)) { result.put("hibernate.hbm2ddl.auto", ddlAuto); @@ -292,17 +287,11 @@ public class JpaProperties { this.physicalStrategy = physicalStrategy; } - private void applyNamingStrategies(Map properties, - ImplicitNamingStrategy implicitStrategyBean, - PhysicalNamingStrategy physicalStrategyBean) { + private void applyNamingStrategies(Map properties) { applyNamingStrategy(properties, "hibernate.implicit_naming_strategy", - implicitStrategyBean != null ? implicitStrategyBean - : this.implicitStrategy, - DEFAULT_IMPLICIT_STRATEGY); + this.implicitStrategy, DEFAULT_IMPLICIT_STRATEGY); applyNamingStrategy(properties, "hibernate.physical_naming_strategy", - physicalStrategyBean != null ? physicalStrategyBean - : this.physicalStrategy, - DEFAULT_PHYSICAL_STRATEGY); + this.physicalStrategy, DEFAULT_PHYSICAL_STRATEGY); } private void applyNamingStrategy(Map properties, String key, diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java index 0d35a46e5a..97943718f4 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java @@ -33,6 +33,8 @@ import javax.transaction.TransactionManager; import javax.transaction.UserTransaction; import com.zaxxer.hikari.HikariDataSource; +import org.hibernate.boot.model.naming.ImplicitNamingStrategy; +import org.hibernate.boot.model.naming.PhysicalNamingStrategy; import org.hibernate.engine.transaction.jta.platform.spi.JtaPlatform; import org.junit.Test; @@ -47,7 +49,10 @@ import org.springframework.boot.autoconfigure.liquibase.LiquibaseAutoConfigurati import org.springframework.boot.autoconfigure.orm.jpa.mapping.NonAnnotatedEntity; import org.springframework.boot.autoconfigure.orm.jpa.test.City; import org.springframework.boot.autoconfigure.transaction.jta.JtaAutoConfiguration; +import org.springframework.boot.orm.jpa.hibernate.SpringImplicitNamingStrategy; import org.springframework.boot.orm.jpa.hibernate.SpringJtaPlatform; +import org.springframework.boot.orm.jpa.hibernate.SpringPhysicalNamingStrategy; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.orm.jpa.JpaTransactionManager; import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; @@ -268,6 +273,83 @@ public class HibernateJpaAutoConfigurationTests }); } + @Test + public void physicalNamingStrategyCanBeUsed() { + contextRunner() + .withUserConfiguration(TestPhysicalNamingStrategyConfiguration.class) + .run((context) -> { + Map hibernateProperties = context.getBean( + HibernateJpaConfiguration.class).getVendorProperties(); + assertThat(hibernateProperties).contains( + entry("hibernate.physical_naming_strategy", + context.getBean("testPhysicalNamingStrategy"))); + assertThat(hibernateProperties) + .doesNotContainKeys("hibernate.ejb.naming_strategy"); + }); + } + + @Test + public void implicitNamingStrategyCanBeUsed() { + contextRunner() + .withUserConfiguration(TestImplicitNamingStrategyConfiguration.class) + .run((context) -> { + Map hibernateProperties = context.getBean( + HibernateJpaConfiguration.class).getVendorProperties(); + assertThat(hibernateProperties).contains( + entry("hibernate.implicit_naming_strategy", + context.getBean("testImplicitNamingStrategy"))); + assertThat(hibernateProperties) + .doesNotContainKeys("hibernate.ejb.naming_strategy"); + }); + } + + @Test + public void namingStrategyInstancesTakePrecedenceOverNamingStrategyProperties() { + contextRunner() + .withUserConfiguration(TestPhysicalNamingStrategyConfiguration.class, + TestImplicitNamingStrategyConfiguration.class) + .withPropertyValues( + "spring.jpa.hibernate.naming.physical-strategy:com.example.Physical", + "spring.jpa.hibernate.naming.implicit-strategy:com.example.Implicit") + .run((context) -> { + Map hibernateProperties = context.getBean( + HibernateJpaConfiguration.class).getVendorProperties(); + assertThat(hibernateProperties).contains( + entry("hibernate.physical_naming_strategy", + context.getBean("testPhysicalNamingStrategy")), + entry("hibernate.implicit_naming_strategy", + context.getBean("testImplicitNamingStrategy"))); + assertThat(hibernateProperties) + .doesNotContainKeys("hibernate.ejb.naming_strategy"); + }); + } + + @Test + public void hibernatePropertiesCustomizerTakePrecedenceOverStrategyInstancesAndNamingStrategyProperties() { + contextRunner() + .withUserConfiguration( + TestHibernatePropertiesCustomizerConfiguration.class, + TestPhysicalNamingStrategyConfiguration.class, + TestImplicitNamingStrategyConfiguration.class) + .withPropertyValues( + "spring.jpa.hibernate.naming.physical-strategy:com.example.Physical", + "spring.jpa.hibernate.naming.implicit-strategy:com.example.Implicit") + .run((context) -> { + Map hibernateProperties = context.getBean( + HibernateJpaConfiguration.class).getVendorProperties(); + TestHibernatePropertiesCustomizerConfiguration configuration = context.getBean( + TestHibernatePropertiesCustomizerConfiguration.class); + assertThat(hibernateProperties).contains( + entry("hibernate.physical_naming_strategy", + configuration.physicalNamingStrategy), + entry("hibernate.implicit_naming_strategy", + configuration.implicitNamingStrategy)); + assertThat(hibernateProperties) + .doesNotContainKeys("hibernate.ejb.naming_strategy"); + }); + } + + @Configuration @TestAutoConfigurationPackage(City.class) static class TestInitializedJpaConfiguration { @@ -288,6 +370,45 @@ public class HibernateJpaAutoConfigurationTests } + @Configuration + static class TestImplicitNamingStrategyConfiguration { + + @Bean + public ImplicitNamingStrategy testImplicitNamingStrategy() { + return new SpringImplicitNamingStrategy(); + } + + } + + @Configuration + static class TestPhysicalNamingStrategyConfiguration { + + @Bean + public PhysicalNamingStrategy testPhysicalNamingStrategy() { + return new SpringPhysicalNamingStrategy(); + } + + } + + @Configuration + static class TestHibernatePropertiesCustomizerConfiguration { + + private final PhysicalNamingStrategy physicalNamingStrategy = new SpringPhysicalNamingStrategy(); + + private final ImplicitNamingStrategy implicitNamingStrategy = new SpringImplicitNamingStrategy(); + + @Bean + public HibernatePropertiesCustomizer testHibernatePropertiesCustomizer() { + return (hibernateProperties) -> { + hibernateProperties.put("hibernate.physical_naming_strategy", + this.physicalNamingStrategy); + hibernateProperties.put("hibernate.implicit_naming_strategy", + this.implicitNamingStrategy); + }; + } + + } + public static class TestJtaPlatform implements JtaPlatform { @Override diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/JpaPropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/JpaPropertiesTests.java index 55204d7de5..a3aedabd53 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/JpaPropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/JpaPropertiesTests.java @@ -19,15 +19,12 @@ package org.springframework.boot.autoconfigure.orm.jpa; import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.SQLException; -import java.util.Collections; import java.util.Map; import java.util.function.Consumer; import java.util.function.Supplier; 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.Before; import org.junit.Test; @@ -102,81 +99,6 @@ public class JpaPropertiesTests { })); } - @Test - public void namingStrategyInstancesCanBeUsed() { - this.contextRunner.run(assertJpaProperties((properties) -> { - ImplicitNamingStrategy implicitStrategy = mock(ImplicitNamingStrategy.class); - PhysicalNamingStrategy physicalStrategy = mock(PhysicalNamingStrategy.class); - Map hibernateProperties = properties.getHibernateProperties( - new HibernateSettings().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() { - this.contextRunner.withPropertyValues( - "spring.jpa.hibernate.naming.implicit-strategy:com.example.Implicit", - "spring.jpa.hibernate.naming.physical-strategy:com.example.Physical") - .run(assertJpaProperties((properties) -> { - ImplicitNamingStrategy implicitStrategy = mock( - ImplicitNamingStrategy.class); - PhysicalNamingStrategy physicalStrategy = mock( - PhysicalNamingStrategy.class); - Map hibernateProperties = properties - .getHibernateProperties(new HibernateSettings() - .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 hibernatePropertiesCustomizerTakePrecedenceOverStrategyInstancesAndNamingStrategyProperties() { - this.contextRunner.withPropertyValues( - "spring.jpa.hibernate.naming.implicit-strategy:com.example.Implicit", - "spring.jpa.hibernate.naming.physical-strategy:com.example.Physical") - .run(assertJpaProperties((properties) -> { - ImplicitNamingStrategy implicitStrategy = mock( - ImplicitNamingStrategy.class); - PhysicalNamingStrategy physicalStrategy = mock( - PhysicalNamingStrategy.class); - ImplicitNamingStrategy effectiveImplicitStrategy = mock( - ImplicitNamingStrategy.class); - PhysicalNamingStrategy effectivePhysicalStrategy = mock( - PhysicalNamingStrategy.class); - HibernatePropertiesCustomizer customizer = (hibernateProperties) -> { - hibernateProperties.put("hibernate.implicit_naming_strategy", - effectiveImplicitStrategy); - hibernateProperties.put("hibernate.physical_naming_strategy", - effectivePhysicalStrategy); - }; - Map hibernateProperties = properties - .getHibernateProperties(new HibernateSettings() - .implicitNamingStrategy(implicitStrategy) - .physicalNamingStrategy(physicalStrategy) - .hibernatePropertiesCustomizers( - Collections.singleton(customizer))); - assertThat(hibernateProperties).contains( - entry("hibernate.implicit_naming_strategy", - effectiveImplicitStrategy), - entry("hibernate.physical_naming_strategy", - effectivePhysicalStrategy)); - assertThat(hibernateProperties) - .doesNotContainKeys("hibernate.ejb.naming_strategy"); - })); - } - @Test public void hibernate5CustomNamingStrategiesViaJpaProperties() { this.contextRunner.withPropertyValues(