HHH-10018 - Change default ImplicitNamingStrategy to use the JPA compliant one + ImplicitNamingStrategy short-naming

Author: sebersole

hibernate-core/src/main/java/org/hibernate/boot/internal/MetadataBuilderImpl.java

@@ -12,6 +12,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.Callable;
 import javax.persistence.AttributeConverter;
 import javax.persistence.SharedCacheMode;
 
@@ -39,6 +40,7 @@
 import org.hibernate.boot.model.TypeContributions;
 import org.hibernate.boot.model.TypeContributor;
 import org.hibernate.boot.model.naming.ImplicitNamingStrategy;
+import org.hibernate.boot.model.naming.ImplicitNamingStrategyJpaCompliantImpl;
 import org.hibernate.boot.model.naming.ImplicitNamingStrategyLegacyJpaImpl;
 import org.hibernate.boot.model.naming.PhysicalNamingStrategy;
 import org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl;
@@ -672,7 +674,16 @@ public AccessType convert(Object value) {
 			implicitNamingStrategy = strategySelector.resolveDefaultableStrategy(
 					ImplicitNamingStrategy.class,
 					configService.getSettings().get( AvailableSettings.IMPLICIT_NAMING_STRATEGY ),
-					ImplicitNamingStrategyLegacyJpaImpl.INSTANCE
+					new Callable<ImplicitNamingStrategy>() {
+						@Override
+						public ImplicitNamingStrategy call() throws Exception {
+							return strategySelector.resolveDefaultableStrategy(
+									ImplicitNamingStrategy.class,
+									"default",
+									ImplicitNamingStrategyJpaCompliantImpl.INSTANCE
+							);
+						}
+					}
 			);
 
 			physicalNamingStrategy = strategySelector.resolveDefaultableStrategy(

hibernate-core/src/main/java/org/hibernate/boot/registry/selector/internal/StrategySelectorBuilder.java

@@ -9,6 +9,11 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.hibernate.boot.model.naming.ImplicitNamingStrategy;
+import org.hibernate.boot.model.naming.ImplicitNamingStrategyComponentPathImpl;
+import org.hibernate.boot.model.naming.ImplicitNamingStrategyJpaCompliantImpl;
+import org.hibernate.boot.model.naming.ImplicitNamingStrategyLegacyHbmImpl;
+import org.hibernate.boot.model.naming.ImplicitNamingStrategyLegacyJpaImpl;
 import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
 import org.hibernate.boot.registry.selector.SimpleStrategyRegistrationImpl;
 import org.hibernate.boot.registry.selector.StrategyRegistration;
@@ -151,6 +156,7 @@ public StrategySelector buildSelector(ClassLoaderService classLoaderService) {
 		addTransactionCoordinatorBuilders( strategySelector );
 		addMultiTableBulkIdStrategies( strategySelector );
 		addEntityCopyObserverStrategies( strategySelector );
+		addImplicitNamingStrategies( strategySelector );
 
 		// apply auto-discovered registrations
 		for ( StrategyRegistrationProvider provider : classLoaderService.loadJavaServices( StrategyRegistrationProvider.class ) ) {
@@ -385,4 +391,32 @@ private void addEntityCopyObserverStrategies(StrategySelectorImpl strategySelect
 				EntityCopyAllowedLoggedObserver.class
 		);
 	}
+
+	private void addImplicitNamingStrategies(StrategySelectorImpl strategySelector) {
+		strategySelector.registerStrategyImplementor(
+				ImplicitNamingStrategy.class,
+				"default",
+				ImplicitNamingStrategyJpaCompliantImpl.class
+		);
+		strategySelector.registerStrategyImplementor(
+				ImplicitNamingStrategy.class,
+				"jpa",
+				ImplicitNamingStrategyJpaCompliantImpl.class
+		);
+		strategySelector.registerStrategyImplementor(
+				ImplicitNamingStrategy.class,
+				"legacy-jpa",
+				ImplicitNamingStrategyLegacyJpaImpl.class
+		);
+		strategySelector.registerStrategyImplementor(
+				ImplicitNamingStrategy.class,
+				"legacy-hbm",
+				ImplicitNamingStrategyLegacyHbmImpl.class
+		);
+		strategySelector.registerStrategyImplementor(
+				ImplicitNamingStrategy.class,
+				"component-path",
+				ImplicitNamingStrategyComponentPathImpl.class
+		);
+	}
 }

hibernate-core/src/main/java/org/hibernate/boot/registry/selector/internal/StrategySelectorImpl.java

@@ -8,6 +8,7 @@
 
 import java.util.Iterator;
 import java.util.Map;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
@@ -116,14 +117,37 @@ public <T> Class<? extends T> selectStrategyImplementor(Class<T> strategy, Strin
 
 	@Override
 	public <T> T resolveStrategy(Class<T> strategy, Object strategyReference) {
-		return resolveDefaultableStrategy( strategy, strategyReference, null );
+		return resolveDefaultableStrategy( strategy, strategyReference, (T) null );
 	}
 
 	@Override
 	@SuppressWarnings("unchecked")
-	public <T> T resolveDefaultableStrategy(Class<T> strategy, Object strategyReference, T defaultValue) {
+	public <T> T resolveDefaultableStrategy(Class<T> strategy, Object strategyReference, final T defaultValue) {
+		return resolveDefaultableStrategy(
+				strategy,
+				strategyReference,
+				new Callable<T>() {
+					@Override
+					public T call() {
+						return defaultValue;
+					}
+				}
+		);
+	}
+
+	@Override
+	@SuppressWarnings("unchecked")
+	public <T> T resolveDefaultableStrategy(
+			Class<T> strategy,
+			Object strategyReference,
+			Callable<T> defaultResolver) {
 		if ( strategyReference == null ) {
-			return defaultValue;
+			try {
+				return defaultResolver.call();
+			}
+			catch (Exception e) {
+				throw new StrategySelectionException( "Default-resolver threw exception", e );
+			}
 		}
 
 		if ( strategy.isInstance( strategyReference ) ) {

hibernate-core/src/main/java/org/hibernate/boot/registry/selector/spi/StrategySelector.java

@@ -6,6 +6,8 @@
  */
 package org.hibernate.boot.registry.selector.spi;
 
+import java.util.concurrent.Callable;
+
 import org.hibernate.service.Service;
 
 /**
@@ -40,7 +42,7 @@ public interface StrategySelector extends Service {
 	 * @param <T> The type of the strategy.  Used to make sure that the strategy and implementation are type
 	 * compatible.
 	 */
-	public <T> void registerStrategyImplementor(Class<T> strategy, String name, Class<? extends T> implementation);
+	<T> void registerStrategyImplementor(Class<T> strategy, String name, Class<? extends T> implementation);
 
 	/**
 	 * Un-registers a named implementor of a particular strategy contract.  Un-registers all named registrations
@@ -51,7 +53,7 @@ public interface StrategySelector extends Service {
 	 * @param <T> The type of the strategy.  Used to make sure that the strategy and implementation are type
 	 * compatible.
 	 */
-	public <T> void unRegisterStrategyImplementor(Class<T> strategy, Class<? extends T> implementation);
+	<T> void unRegisterStrategyImplementor(Class<T> strategy, Class<? extends T> implementation);
 
 	/**
 	 * Locate the named strategy implementation.
@@ -63,7 +65,7 @@ public interface StrategySelector extends Service {
 	 *
 	 * @return The named strategy implementation class.
 	 */
-	public <T> Class<? extends T> selectStrategyImplementor(Class<T> strategy, String name);
+	<T> Class<? extends T> selectStrategyImplementor(Class<T> strategy, String name);
 
 	/**
 	 * Resolve strategy instances. See discussion on {@link #resolveDefaultableStrategy}.
@@ -76,7 +78,7 @@ public interface StrategySelector extends Service {
 	 *
 	 * @return The strategy instance
 	 */
-	public <T> T resolveStrategy(Class<T> strategy, Object strategyReference);
+	<T> T resolveStrategy(Class<T> strategy, Object strategyReference);
 
 	/**
 	 * Resolve strategy instances. The incoming reference might be:<ul>
@@ -104,5 +106,33 @@ public interface StrategySelector extends Service {
 	 *
 	 * @return The strategy instance
 	 */
-	public <T> T resolveDefaultableStrategy(Class<T> strategy, Object strategyReference, T defaultValue);
+	<T> T resolveDefaultableStrategy(Class<T> strategy, Object strategyReference, T defaultValue);
+
+	/**
+	 * Resolve strategy instances. The incoming reference might be:<ul>
+	 *     <li>
+	 *         {@code null} - in which case defaultValue is returned.
+	 *     </li>
+	 *     <li>
+	 *         An actual instance of the strategy type - it is returned, as is
+	 *     </li>
+	 *     <li>
+	 *         A reference to the implementation {@link Class} - an instance is created by calling
+	 *         {@link Class#newInstance()} (aka, the class's no-arg ctor).
+	 *     </li>
+	 *     <li>
+	 *         The name of the implementation class - First the implementation's {@link Class} reference
+	 *         is resolved, and then an instance is created by calling {@link Class#newInstance()}
+	 *     </li>
+	 * </ul>
+	 *
+	 * @param strategy The type (interface) of the strategy to be resolved.
+	 * @param strategyReference The reference to the strategy for which we need to resolve an instance.
+	 * @param defaultResolver A strategy for resolving the default value strategyReference resolves to null.
+	 * @param <T> The type of the strategy.  Used to make sure that the strategy and implementation are type
+	 * compatible.
+	 *
+	 * @return The strategy instance
+	 */
+	<T> T resolveDefaultableStrategy(Class<T> strategy, Object strategyReference, Callable<T> defaultResolver);
 }

hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java

@@ -358,7 +358,17 @@ public interface AvailableSettings {
 	String SCANNER_DISCOVERY = "hibernate.archive.autodetection";
 
 	/**
-	 * Used to specify the {@link org.hibernate.boot.model.naming.ImplicitNamingStrategy} class to use.
+	 * Used to specify the {@link org.hibernate.boot.model.naming.ImplicitNamingStrategy} class to use.  The following
+	 * short-names are defined for this setting:<ul>
+	 *     <li>"default" -> {@link org.hibernate.boot.model.naming.ImplicitNamingStrategyJpaCompliantImpl}</li>
+	 *     <li>"jpa" -> {@link org.hibernate.boot.model.naming.ImplicitNamingStrategyJpaCompliantImpl}</li>
+	 *     <li>"legacy-jpa" -> {@link org.hibernate.boot.model.naming.ImplicitNamingStrategyLegacyJpaImpl}</li>
+	 *     <li>"legacy-hbm" -> {@link org.hibernate.boot.model.naming.ImplicitNamingStrategyLegacyHbmImpl}</li>
+	 *     <li>"component-path" -> {@link org.hibernate.boot.model.naming.ImplicitNamingStrategyComponentPathImpl}</li>
+	 * </ul>
+	 *
+	 * The default is defined by the ImplicitNamingStrategy registered under the "default" key.  If that happens to
+	 * be empty, the fallback is to use {@link org.hibernate.boot.model.naming.ImplicitNamingStrategyJpaCompliantImpl}.
 	 *
 	 * @see MetadataBuilder#applyImplicitNamingStrategy
 	 *

hibernate-core/src/test/java/org/hibernate/test/schematoolsnaming/StandaloneSchemaToolsNamingTest.java

@@ -29,38 +29,50 @@
 @TestForIssue( jiraKey = "HHH-9467" )
 public class StandaloneSchemaToolsNamingTest extends BaseUnitTestCase {
 	@Test
-	public void testDefaultNamingStrategy() throws Exception {
+	public void testDefaultImplicitNamingStrategy() throws Exception {
 		checkNamingStrategies(
 				new String[] {
 						"--text",
 						"--config=org/hibernate/test/schematoolsnaming/hibernate.cfg.xml"
 				},
-				ImplicitNamingStrategyLegacyJpaImpl.class
+				ImplicitNamingStrategyJpaCompliantImpl.class
 		);
 	}
 
 	@Test
-	public void testDeprecatedNamingStrategy() throws Exception {
+	public void testDeprecatedNamingSetting() throws Exception {
 		// output should be the same as above test, --naming should simply produce a logged warning
 		checkNamingStrategies(
 				new String[] {
 						"--text",
 						"--config=org/hibernate/test/schematoolsnaming/hibernate.cfg.xml",
 						"--naming=DoesNotExist",
 				},
+				ImplicitNamingStrategyJpaCompliantImpl.class
+		);
+	}
+
+	@Test
+	public void testImplicitNamingStrategySettingFullName() throws Exception {
+		checkNamingStrategies(
+				new String[] {
+						"--text",
+						"--config=org/hibernate/test/schematoolsnaming/hibernate.cfg.xml",
+						"--implicit-naming=" + ImplicitNamingStrategyLegacyJpaImpl.class.getName(),
+				},
 				ImplicitNamingStrategyLegacyJpaImpl.class
 		);
 	}
 
 	@Test
-	public void testJpaCompliantNamingStrategy() throws Exception {
+	public void testImplicitNamingStrategySettingShortName() throws Exception {
 		checkNamingStrategies(
 				new String[] {
 						"--text",
 						"--config=org/hibernate/test/schematoolsnaming/hibernate.cfg.xml",
-						"--implicit-naming=" + ImplicitNamingStrategyJpaCompliantImpl.class.getName(),
+						"--implicit-naming=legacy-jpa",
 				},
-				ImplicitNamingStrategyJpaCompliantImpl.class
+				ImplicitNamingStrategyLegacyJpaImpl.class
 		);
 	}
 

working-5.0-migration-guide.md

@@ -39,7 +39,9 @@ Working list of changes for 5.0
     the configuration must explicitly state that using either the `useNamed` (true) setting or by specifying the `type`
     setting set to the value 12 (VARCHAR JDBC type code).
 * Default value for `hibernate.id.new_generator_mappings` setting changed to true for 5.0.  See 
-    `org.hibernate.cfg.AvailableSettings#USE_NEW_ID_GENERATOR_MAPPINGS` javadocs. 
+    `org.hibernate.cfg.AvailableSettings#USE_NEW_ID_GENERATOR_MAPPINGS` javadocs.
+* The default ImplicitNamingStrategy (`hibernate.implicit_naming_strategy`) has changed to the JPA-compliant one.  See 
+    `org.hibernate.cfg.AvailableSettings.IMPLICIT_NAMING_STRATEGY` javadocs for details.
 
 
 TODOs
@@ -67,7 +69,7 @@ Blog items
     * integrated with hibernate-envers
     * collection values, map keys
 * scanning support for non-JPA usage
-* naming strategy
+* naming strategy split.  default is now to be jpa-compliant
 * OSGi improvements, Karaf feature file published