cleanups to the service package

- get rid of some raw types
- remove an obsolete exceptipon type
- use ServiceLogger

Author: gavinking

hibernate-core/src/main/java/org/hibernate/service/NullServiceException.java

@@ -10,14 +10,14 @@
  * @author Andrea Boriero
  */
 public class NullServiceException extends ServiceException {
-	public final Class serviceRole;
+	public final Class<?> serviceRole;
 
-	public NullServiceException(Class serviceRole) {
+	public NullServiceException(Class<?> serviceRole) {
 		super( "Unknown service requested [" + serviceRole.getName() + "]" );
 		this.serviceRole = serviceRole;
 	}
 
-	public Class getServiceRole() {
+	public Class<?> getServiceRole() {
 		return serviceRole;
 	}
 }

hibernate-core/src/main/java/org/hibernate/service/StandardServiceInitiators.java

@@ -59,7 +59,7 @@ private static List<StandardServiceInitiator<?>> buildStandardServiceInitiatorLi
 
 		// Please do not rearrange - it's useful to maintain this particular order
 		// so to simplify comparisons with custom initiator lists in other projects;
-		// for example we customize this list in Hibernate Reactive and Quarkus.
+		// for example, we customize this list in Hibernate Reactive and Quarkus.
 
 		final ArrayList<StandardServiceInitiator<?>> serviceInitiators = new ArrayList<>();
 

hibernate-core/src/main/java/org/hibernate/service/UnknownServiceException.java

@@ -11,14 +11,14 @@
  * @author Steve Ebersole
  */
 public class UnknownServiceException extends ServiceException {
-	public final Class serviceRole;
+	public final Class<?> serviceRole;
 
-	public UnknownServiceException(Class serviceRole) {
+	public UnknownServiceException(Class<?> serviceRole) {
 		super( "Unknown service requested [" + serviceRole.getName() + "]" );
 		this.serviceRole = serviceRole;
 	}
 
-	public Class getServiceRole() {
+	public Class<?> getServiceRole() {
 		return serviceRole;
 	}
 }

hibernate-core/src/main/java/org/hibernate/service/UnknownUnwrapTypeException.java

@@ -10,11 +10,11 @@
  * @author Steve Ebersole
  */
 public class UnknownUnwrapTypeException extends HibernateException {
-	public UnknownUnwrapTypeException(Class unwrapType) {
+	public UnknownUnwrapTypeException(Class<?> unwrapType) {
 		super( "Cannot unwrap to requested type [" + unwrapType.getName() + "]" );
 	}
 
-	public UnknownUnwrapTypeException(Class unwrapType, Throwable root) {
+	public UnknownUnwrapTypeException(Class<?> unwrapType, Throwable root) {
 		this( unwrapType );
 		super.initCause( root );
 	}

hibernate-core/src/main/java/org/hibernate/service/internal/AbstractServiceRegistryImpl.java

@@ -110,12 +110,12 @@ protected <R extends Service> void createServiceBinding(ServiceInitiator<R> init
 	}
 
 	protected <R extends Service> void createServiceBinding(ProvidedService<R> providedService) {
-		var binding = locateServiceBinding( providedService.getServiceRole(), false );
+		var binding = locateServiceBinding( providedService.serviceRole(), false );
 		if ( binding == null ) {
-			binding = new ServiceBinding<>( this, providedService.getServiceRole(), providedService.getService() );
-			serviceBindingMap.put( providedService.getServiceRole(), binding );
+			binding = new ServiceBinding<>( this, providedService.serviceRole(), providedService.service() );
+			serviceBindingMap.put( providedService.serviceRole(), binding );
 		}
-		registerService( binding, providedService.getService() );
+		registerService( binding, providedService.service() );
 	}
 
 	protected void visitServiceBindings(Consumer<ServiceBinding<?>> action) {
@@ -156,18 +156,21 @@ protected void visitServiceBindings(Consumer<ServiceBinding<?>> action) {
 
 		// perform a crawl looking for an alternate registration
 		for ( var binding : serviceBindingMap.values() ) {
-			if ( serviceRole.isAssignableFrom( binding.getServiceRole() ) ) {
+			final Class<?> bindingServiceRole = binding.getServiceRole();
+			if ( serviceRole.isAssignableFrom( bindingServiceRole ) ) {
 				// we found an alternate...
-				SERVICE_LOGGER.alternateServiceRole( serviceRole.getName(), binding.getServiceRole().getName() );
-				registerAlternate( serviceRole, binding.getServiceRole() );
+				SERVICE_LOGGER.alternateServiceRole( serviceRole.getName(), bindingServiceRole.getName() );
+				registerAlternate( serviceRole, bindingServiceRole );
 				return (ServiceBinding<R>) binding;
 			}
-
-			if ( binding.getService() != null && serviceRole.isInstance( binding.getService() ) ) {
-				// we found an alternate...
-				SERVICE_LOGGER.alternateServiceRole( serviceRole.getName(), binding.getServiceRole().getName() );
-				registerAlternate( serviceRole, binding.getServiceRole() );
-				return (ServiceBinding<R>) binding;
+			else {
+				final var bindingService = binding.getService();
+				if ( serviceRole.isInstance( bindingService ) ) {
+					// we found an alternate...
+					SERVICE_LOGGER.alternateServiceRole( serviceRole.getName(), bindingServiceRole.getName() );
+					registerAlternate( serviceRole, bindingServiceRole );
+					return (ServiceBinding<R>) binding;
+				}
 			}
 		}
 
@@ -183,10 +186,8 @@ private void registerAlternate(Class<?> alternate, Class<?> target) {
 		// Fast-path for ClassLoaderService as it's extremely hot during bootstrap
 		// (and after bootstrap service loading performance is less interesting as it's
 		// ideally being cached by long-term consumers)
-		if ( ClassLoaderService.class == serviceRole ) {
-			if ( parent != null ) {
-				return parent.getService( serviceRole );
-			}
+		if ( ClassLoaderService.class == serviceRole && parent != null ) {
+			return parent.getService( serviceRole );
 		}
 		// TODO: should an exception be thrown if active == false???
 		R service = serviceRole.cast( initializedServiceByRole.get( serviceRole ) );
@@ -231,7 +232,7 @@ protected <R extends Service> void registerService(ServiceBinding<R> serviceBind
 		}
 
 		// PHASE 1: create service
-		R service = createService( serviceBinding );
+		final R service = createService( serviceBinding );
 		if ( service == null ) {
 			return null;
 		}
@@ -256,7 +257,7 @@ protected <R extends Service> void registerService(ServiceBinding<R> serviceBind
 		}
 
 		try {
-			R service = serviceBinding.getLifecycleOwner().initiateService( serviceInitiator );
+			final R service = serviceBinding.getLifecycleOwner().initiateService( serviceInitiator );
 			// IMPL NOTE: the register call here is important to avoid potential stack overflow issues
 			//		      from recursive calls through #configureService
 			if ( service != null ) {
@@ -276,18 +277,16 @@ protected <R extends Service> void registerService(ServiceBinding<R> serviceBind
 	@Override
 	public <R extends Service> void injectDependencies(ServiceBinding<R> serviceBinding) {
 		final R service = serviceBinding.getService();
-
 		applyInjections( service );
-
 		if ( service instanceof ServiceRegistryAwareService serviceRegistryAwareService ) {
 			serviceRegistryAwareService.injectServices( this );
 		}
 	}
 
 	private <R extends Service> void applyInjections(R service) {
 		try {
-			for ( Method method : service.getClass().getMethods() ) {
-				final InjectService injectService = method.getAnnotation( InjectService.class );
+			for ( var method : service.getClass().getMethods() ) {
+				final var injectService = method.getAnnotation( InjectService.class );
 				if ( injectService != null ) {
 					processInjection( service, method, injectService );
 				}
@@ -299,23 +298,14 @@ private <R extends Service> void applyInjections(R service) {
 	}
 
 	private <T extends Service> void processInjection(T service, Method injectionMethod, InjectService injectService) {
-		final Class<?>[] parameterTypes = injectionMethod.getParameterTypes();
-		if ( injectionMethod.getParameterCount() != 1 ) {
-			throw new ServiceDependencyException(
-					"Encountered @InjectService on method with unexpected number of parameters"
-			);
-		}
-
-		//noinspection rawtypes
-		Class dependentServiceRole = injectService.serviceRole();
-		if ( dependentServiceRole == null || dependentServiceRole.equals( Void.class ) ) {
-			dependentServiceRole = parameterTypes[0];
-		}
+		injectDependentService( service, injectionMethod, injectService,
+				dependentServiceRole( injectionMethod, injectService ) );
+	}
 
+	private <T extends Service> void injectDependentService(T service, Method injectionMethod, InjectService injectService, Class<? extends Service> dependentServiceRole) {
 		// todo : because of the use of proxies, this is no longer returning null here...
 
-		//noinspection unchecked
-		final Service dependantService = getService( dependentServiceRole );
+		final var dependantService = getService( dependentServiceRole );
 		if ( dependantService == null ) {
 			if ( injectService.required() ) {
 				throw new ServiceDependencyException(
@@ -333,6 +323,25 @@ private <T extends Service> void processInjection(T service, Method injectionMet
 		}
 	}
 
+	private static Class<? extends Service> dependentServiceRole(Method injectionMethod, InjectService injectService) {
+		final var parameterTypes = injectionMethod.getParameterTypes();
+		if ( injectionMethod.getParameterCount() != 1 ) {
+			throw new ServiceDependencyException(
+					"Encountered @InjectService on method with unexpected number of parameters"
+			);
+		}
+
+		final var dependentServiceRole = injectService.serviceRole();
+		if ( dependentServiceRole == null
+				|| Void.class.equals( dependentServiceRole )  // old default value
+				|| Service.class.equals( dependentServiceRole ) ) {  // new default value
+			return (Class<? extends Service>) parameterTypes[0];
+		}
+		else {
+			return dependentServiceRole;
+		}
+	}
+
 	@Override
 	public <R extends Service> void startService(ServiceBinding<R> serviceBinding) {
 		if ( serviceBinding.getService() instanceof Startable startable ) {

hibernate-core/src/main/java/org/hibernate/service/internal/ProvidedService.java

@@ -9,20 +9,5 @@
  *
  * @author Steve Ebersole
  */
-public class ProvidedService<R> {
-	private final Class<R> serviceRole;
-	private final R service;
-
-	public ProvidedService(Class<R> serviceRole, R service) {
-		this.serviceRole = serviceRole;
-		this.service = service;
-	}
-
-	public Class<R> getServiceRole() {
-		return serviceRole;
-	}
-
-	public R getService() {
-		return service;
-	}
+public record ProvidedService<R>(Class<R> serviceRole, R service) {
 }

hibernate-core/src/main/java/org/hibernate/service/internal/ServiceLogger.java

@@ -19,16 +19,17 @@
 import java.lang.invoke.MethodHandles;
 import java.util.ServiceConfigurationError;
 
+import static org.jboss.logging.Logger.Level.DEBUG;
 import static org.jboss.logging.Logger.Level.INFO;
 import static org.jboss.logging.Logger.Level.TRACE;
 import static org.jboss.logging.Logger.Level.WARN;
 
 /**
- * Miscellaneous logging related to Hibernate ORM Core.
+ * Logging related to Hibernate Services.
  */
 @SubSystemLogging(
 		name = SessionLogging.NAME,
-		description = "Miscellaneous Logging related to Hibernate ORM Core"
+		description = "Logging related to Hibernate Services"
 )
 @MessageLogger(projectCode = "HHH")
 @ValidIdRange(min=10002,max = 20000)
@@ -67,4 +68,9 @@ public interface ServiceLogger extends BasicLogger {
 	@LogMessage(level = TRACE)
 	@Message(id = 10453, value = "Skipping destroying bootstrap registry after deregistration of every child ServiceRegistry")
 	void skippingBootstrapRegistryDestruction();
+
+	@LogMessage(level = DEBUG)
+	@Message( id = 10454, value = "EventListenerRegistry access via ServiceRegistry is deprecated - "
+								+ "use 'sessionFactory.getEventEngine().getListenerRegistry()' instead" )
+	void eventListenerRegistryAccessDeprecated();
 }

hibernate-core/src/main/java/org/hibernate/service/internal/ServiceProxyGenerationException.java

@@ -29,14 +29,8 @@ public class SessionFactoryServiceRegistryBuilderImpl implements SessionFactoryS
 	public SessionFactoryServiceRegistryBuilderImpl(ServiceRegistryImplementor parent) {
 		this.parent = parent;
 		if ( parent != null ) {
-			for ( var iterator = initiators.iterator(); iterator.hasNext(); ) {
-				final var initiator = iterator.next();
-				if ( parent.locateServiceBinding( initiator.getServiceInitiated() ) != null ) {
-					// Parent takes precedence over the standard service initiators
-					iterator.remove();
-				}
-			}
-
+			// Parent takes precedence over the standard service initiators
+			initiators.removeIf( initiator -> parent.locateServiceBinding( initiator.getServiceInitiated() ) != null );
 		}
 	}
 

hibernate-core/src/main/java/org/hibernate/service/internal/SessionFactoryServiceRegistryBuilderImpl.java

@@ -19,19 +19,17 @@
 import org.hibernate.service.spi.SessionFactoryServiceInitiatorContext;
 import org.hibernate.service.spi.SessionFactoryServiceRegistry;
 
-import org.jboss.logging.Logger;
-
 import org.checkerframework.checker.nullness.qual.Nullable;
 
+import static org.hibernate.service.internal.ServiceLogger.SERVICE_LOGGER;
+
 /**
  * @author Steve Ebersole
  */
 public class SessionFactoryServiceRegistryImpl
 		extends AbstractServiceRegistryImpl
 		implements SessionFactoryServiceRegistry, SessionFactoryServiceInitiatorContext {
 
-	private static final Logger LOG = Logger.getLogger( SessionFactoryServiceRegistryImpl.class );
-
 	private final SessionFactoryOptions sessionFactoryOptions;
 	private final SessionFactoryImplementor sessionFactory;
 
@@ -101,11 +99,7 @@ public ServiceRegistryImplementor getServiceRegistry() {
 	@Override
 	public <R extends Service> @Nullable R getService(Class<R> serviceRole) {
 		if ( serviceRole.equals( EventListenerRegistry.class ) ) {
-			LOG.debug(
-					"EventListenerRegistry access via ServiceRegistry is deprecated - "
-						+ "use 'sessionFactory.getEventEngine().getListenerRegistry()' instead"
-			);
-
+			SERVICE_LOGGER.eventListenerRegistryAccessDeprecated();
 			//noinspection unchecked
 			return (R) sessionFactory.getEventEngine().getListenerRegistry();
 		}