more minor cleanups in EntityBinder

and stricter enforcement of @DiscriminatorValue requirement
(I don't think this is a substantive change to behavior, but it might be).

Author: gavinking

hibernate-core/src/main/java/org/hibernate/boot/model/internal/EntityBinder.java

@@ -521,9 +521,9 @@ private boolean mapAsIdClass(
 	}
 
 	private ClassDetails idClassDetails(InheritanceState inheritanceState, ClassDetails classWithIdClass) {
-		final var idClassAnn = classWithIdClass.getDirectAnnotationUsage( IdClass.class );
+		final var idClass = classWithIdClass.getDirectAnnotationUsage( IdClass.class );
 		final var classDetailsRegistry = modelsContext().getClassDetailsRegistry();
-		if ( idClassAnn == null ) {
+		if ( idClass == null ) {
 			try {
 				// look for an Id class generated by Hibernate Processor as an inner class of static metamodel
 				final var javaClass = inheritanceState.getClassDetails().toJavaClass();
@@ -535,7 +535,7 @@ private ClassDetails idClassDetails(InheritanceState inheritanceState, ClassDeta
 			}
 		}
 		else {
-			return classDetailsRegistry.resolveClassDetails( idClassAnn.value().getName() );
+			return classDetailsRegistry.resolveClassDetails( idClass.value().getName() );
 		}
 	}
 
@@ -1030,7 +1030,7 @@ private boolean useDiscriminatorColumnForJoined(DiscriminatorColumn discriminato
 			if ( ignore ) {
 				if ( log.isTraceEnabled() ) {
 					log.trace( "Ignoring explicit @DiscriminatorColumn annotation on: "
-							+ annotatedClass.getName() );
+								+ annotatedClass.getName() );
 				}
 			}
 			return !ignore;
@@ -1057,7 +1057,7 @@ private void processIdPropertiesIfNotAlready(
 			Map<ClassDetails, InheritanceState> inheritanceStates) {
 		final Set<String> missingIdProperties = new HashSet<>( idPropertiesIfIdClass );
 		final Set<String> missingEntityProperties = new HashSet<>();
-		for ( PropertyData propertyAnnotatedElement : elementsToProcess.getElements() ) {
+		for ( var propertyAnnotatedElement : elementsToProcess.getElements() ) {
 			final String propertyName = propertyAnnotatedElement.getPropertyName();
 			if ( !idPropertiesIfIdClass.contains( propertyName ) ) {
 				final var memberDetails = propertyAnnotatedElement.getAttributeMember();
@@ -1224,28 +1224,25 @@ public boolean isPropertyDefinedInSuperHierarchy(String name) {
 	}
 
 	private void bindRowManagement() {
-		final var ontext = modelsContext();
-		final var dynamicInsertAnn = annotatedClass.getAnnotationUsage( DynamicInsert.class, ontext );
-		persistentClass.setDynamicInsert( dynamicInsertAnn != null );
-
-		final var dynamicUpdateAnn = annotatedClass.getAnnotationUsage( DynamicUpdate.class, ontext );
-		persistentClass.setDynamicUpdate( dynamicUpdateAnn != null );
+		final var modelsContext = modelsContext();
+		final var dynamicInsert = annotatedClass.hasAnnotationUsage( DynamicInsert.class, modelsContext );
+		final var dynamicUpdate = annotatedClass.hasAnnotationUsage( DynamicUpdate.class, modelsContext );
+		persistentClass.setDynamicInsert( dynamicInsert );
+		persistentClass.setDynamicUpdate( dynamicUpdate );
 
-		if ( persistentClass.useDynamicInsert()
-				&& annotatedClass.hasAnnotationUsage( SQLInsert.class, ontext ) ) {
+		if ( dynamicInsert && annotatedClass.hasAnnotationUsage( SQLInsert.class, modelsContext ) ) {
 			throw new AnnotationException( "Entity '" + name + "' is annotated both '@DynamicInsert' and '@SQLInsert'" );
 		}
-		if ( persistentClass.useDynamicUpdate()
-				&& annotatedClass.hasAnnotationUsage( SQLUpdate.class, ontext ) ) {
+		if ( dynamicUpdate && annotatedClass.hasAnnotationUsage( SQLUpdate.class, modelsContext ) ) {
 			throw new AnnotationException( "Entity '" + name + "' is annotated both '@DynamicUpdate' and '@SQLUpdate'" );
 		}
 	}
 
 	private void bindOptimisticLocking() {
-		final var optimisticLockingAnn = annotatedClass.getAnnotationUsage( OptimisticLocking.class, modelsContext() );
-		persistentClass.setOptimisticLockStyle( fromLockType( optimisticLockingAnn == null
+		final var optimisticLocking = annotatedClass.getAnnotationUsage( OptimisticLocking.class, modelsContext() );
+		persistentClass.setOptimisticLockStyle( fromLockType( optimisticLocking == null
 				? OptimisticLockType.VERSION
-				: optimisticLockingAnn.type() ) );
+				: optimisticLocking.type() ) );
 	}
 
 	private void bindEntityAnnotation() {
@@ -1560,27 +1557,26 @@ private NamedEntityGraphDefinition namedEntityGraphDefinition(org.hibernate.anno
 
 	private void bindDiscriminatorValue() {
 		final var discriminatorValue = annotatedClass.getAnnotationUsage( DiscriminatorValue.class, modelsContext() );
+		persistentClass.setDiscriminatorValue( discriminatorValueString( discriminatorValue ) );
+	}
+
+	private String discriminatorValueString(DiscriminatorValue discriminatorValue) {
 		if ( discriminatorValue == null ) {
-			final Value discriminator = persistentClass.getDiscriminator();
+			final var discriminator = persistentClass.getDiscriminator();
 			if ( discriminator == null ) {
-				persistentClass.setDiscriminatorValue( name );
+				return name;
 			}
 			else {
-				switch ( discriminator.getType().getName() ) {
-					case "character":
-						throw new AnnotationException( "Entity '" + name
-								+ "' has a discriminator of character type and must specify its '@DiscriminatorValue'" );
-					case "integer":
-						// TODO: pretty nasty, should we just deprecate/disallow this?
-						persistentClass.setDiscriminatorValue( String.valueOf( name.hashCode() ) );
-						break;
-					default:
-						persistentClass.setDiscriminatorValue( name ); //Spec compliant
-				}
+				final String discriminatorTypeName = discriminator.getType().getName();
+				return switch ( discriminatorTypeName ) {
+					case "string" -> name; //Spec compliant
+					case "integer"-> String.valueOf( name.hashCode() ); // TODO: pretty nasty, should we just deprecate/disallow this?
+					default -> throw new MappingException( "Entity '" + name + "' must explicitly specify its '@DiscriminatorValue'" );
+				};
 			}
 		}
 		else {
-			persistentClass.setDiscriminatorValue( discriminatorValue.value() );
+			return discriminatorValue.value();
 		}
 	}
 
@@ -1665,7 +1661,7 @@ private void bindSubclassCache() {
 					+" (only root classes may define second-level caching semantics)");
 		}
 
-		final Cacheable cacheable = annotatedClass.getAnnotationUsage( Cacheable.class, modelsContext() );
+		final var cacheable = annotatedClass.getAnnotationUsage( Cacheable.class, modelsContext() );
 		isCached = cacheable == null && persistentClass.getSuperclass() != null
 				// we should inherit the root class caching config
 				? persistentClass.getSuperclass().isCached()