some minor refactorings around Column

Author: gavinking

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

@@ -9,6 +9,7 @@
 import java.util.List;
 import java.util.Map;
 
+import jakarta.persistence.AttributeConverter;
 import org.hibernate.AnnotationException;
 import org.hibernate.AssertionFailure;
 import org.hibernate.annotations.ColumnTransformer;
@@ -111,11 +112,11 @@ public ConverterDescriptor<?,?> resolveAttributeConverterDescriptor(MemberDetail
 	}
 
 	protected IllegalStateException buildExceptionFromInstantiationError(AttributeConversionInfo info, Exception e) {
-		if ( void.class.equals( info.getConverterClass() ) ) {
+		if ( AttributeConverter.class.equals( info.getConverterClass() ) ) {
 			// the user forgot to set @Convert.converter
 			// we already know it's not a @Convert.disableConversion
 			return new IllegalStateException(
-					"Unable to instantiate AttributeConverter: you left @Convert.converter to its default value void.",
+					"Unable to instantiate AttributeConverter because the 'converter' member of '@Convert' was not specified",
 					e
 			);
 
@@ -283,7 +284,7 @@ private Column[] getExactOverriddenColumn(String propertyName) {
 	 */
 	@Override
 	public JoinColumn[] getOverriddenJoinColumn(String propertyName) {
-		final JoinColumn[] result = getExactOverriddenJoinColumn( propertyName );
+		final var result = getExactOverriddenJoinColumn( propertyName );
 		if ( result == null && propertyName.contains( ".{element}." ) ) {
 			//support for non map collections where no prefix is needed
 			//TODO cache the underlying regexp
@@ -316,13 +317,15 @@ private JoinColumn[] getExactOverriddenJoinColumn(String propertyName) {
 
 	@Override
 	public ForeignKey getOverriddenForeignKey(String propertyName) {
-		final ForeignKey result = getExactOverriddenForeignKey( propertyName );
+		final var result = getExactOverriddenForeignKey( propertyName );
 		if ( result == null && propertyName.contains( ".{element}." ) ) {
 			//support for non map collections where no prefix is needed
 			//TODO cache the underlying regexp
 			return getExactOverriddenForeignKey( propertyName.replace( ".{element}.", "." ) );
 		}
-		return result;
+		else {
+			return result;
+		}
 	}
 
 	private ForeignKey getExactOverriddenForeignKey(String propertyName) {
@@ -585,7 +588,7 @@ private static Map<String, ColumnTransformer> buildColumnTransformerOverride(Ann
 	private static Map<String, JoinColumn[]> buildJoinColumnOverride(AnnotationTarget element, String path, MetadataBuildingContext context) {
 		final Map<String, JoinColumn[]> columnOverride = new HashMap<>();
 		if ( element != null ) {
-			for ( var override : buildAssociationOverrides( element, path, context ) ) {
+			for ( var override : buildAssociationOverrides( element, context ) ) {
 				columnOverride.put( qualify( path, override.name() ), override.joinColumns() );
 			}
 		}
@@ -595,21 +598,21 @@ private static Map<String, JoinColumn[]> buildJoinColumnOverride(AnnotationTarge
 	private static Map<String, ForeignKey> buildForeignKeyOverride(AnnotationTarget element, String path, MetadataBuildingContext context) {
 		final Map<String, ForeignKey> foreignKeyOverride = new HashMap<>();
 		if ( element != null ) {
-			for ( var override : buildAssociationOverrides( element, path, context ) ) {
+			for ( var override : buildAssociationOverrides( element, context ) ) {
 				foreignKeyOverride.put( qualify( path, override.name() ), override.foreignKey() );
 			}
 		}
 		return foreignKeyOverride;
 	}
 
-	private static AssociationOverride[] buildAssociationOverrides(AnnotationTarget element, String path, MetadataBuildingContext context) {
+	private static AssociationOverride[] buildAssociationOverrides(AnnotationTarget element, MetadataBuildingContext context) {
 		return element.getRepeatedAnnotationUsages( AssociationOverride.class, context.getBootstrapContext().getModelsContext() );
 	}
 
 	private static Map<String, JoinTable> buildJoinTableOverride(AnnotationTarget element, String path, MetadataBuildingContext context) {
 		final Map<String, JoinTable> result = new HashMap<>();
 		if ( element != null ) {
-			for ( var override : buildAssociationOverrides( element, path, context ) ) {
+			for ( var override : buildAssociationOverrides( element, context ) ) {
 				if ( isEmpty( override.joinColumns() ) ) {
 					result.put( qualify( path, override.name() ), override.joinTable() );
 				}

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

@@ -73,7 +73,7 @@ static AnnotatedJoinColumn buildJoinColumn(
 			PropertyHolder propertyHolder,
 			PropertyData inferredData) {
 		final String path = qualify( propertyHolder.getPath(), inferredData.getPropertyName() );
-		final JoinColumn[] overrides = propertyHolder.getOverriddenJoinColumn( path );
+		final var overrides = propertyHolder.getOverriddenJoinColumn( path );
 		if ( overrides != null ) {
 			//TODO: relax this restriction
 			throw new AnnotationException( "Property '" + path
@@ -334,7 +334,7 @@ public void linkValueUsingDefaultColumnNaming(
 		setLogicalColumnName( columnName );
 		setImplicit( true );
 		setReferencedColumn( logicalReferencedColumn );
-		final Column mappingColumn = getMappingColumn();
+		final var mappingColumn = getMappingColumn();
 		initMappingColumn(
 				columnName,
 				null,
@@ -363,7 +363,7 @@ private String defaultColumnName(int columnIndex, PersistentClass referencedEnti
 			// the name of the @MapsId annotation, but
 			// it's better than just having two different
 			// column names which disagree
-			final Column column = parent.resolveMapsId().getValue().getColumns().get( columnIndex );
+			final var column = parent.resolveMapsId().getValue().getColumns().get( columnIndex );
 //			return column.getQuotedName();
 			if ( column.isExplicit() ) {
 				throw new AnnotationException( "Association '" + parent.getPropertyName()
@@ -429,7 +429,7 @@ protected void addColumnBinding(SimpleValue value) {
 	 * @param column the referenced column.
 	 */
 	public void overrideFromReferencedColumnIfNecessary(Column column) {
-		final Column mappingColumn = getMappingColumn();
+		final var mappingColumn = getMappingColumn();
 		if ( mappingColumn != null ) {
 			// columnDefinition can also be specified using @JoinColumn, hence we have to check
 			// whether it is set or not

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

@@ -40,6 +40,7 @@
 import static org.hibernate.boot.model.internal.AnnotatedJoinColumn.buildExplicitJoinTableJoinColumn;
 import static org.hibernate.boot.model.internal.AnnotatedJoinColumn.buildImplicitJoinTableJoinColumn;
 import static org.hibernate.boot.model.internal.AnnotatedJoinColumn.buildJoinColumn;
+import static org.hibernate.boot.model.internal.AnnotatedJoinColumn.buildJoinFormula;
 import static org.hibernate.boot.model.internal.BinderHelper.findReferencedColumnOwner;
 import static org.hibernate.boot.model.internal.BinderHelper.getRelativePath;
 import static org.hibernate.boot.model.internal.ForeignKeyType.EXPLICIT_PRIMARY_KEY_REFERENCE;
@@ -99,7 +100,7 @@ public static AnnotatedJoinColumns buildJoinColumnsOrFormulas(
 			final var column = columnOrFormula.column();
 			final String annotationString = formula.value();
 			if ( isNotBlank( annotationString ) ) {
-				AnnotatedJoinColumn.buildJoinFormula( formula, parent );
+				buildJoinFormula( formula, parent );
 			}
 			else {
 				buildJoinColumn( column, mappedBy, parent, propertyHolder, inferredData );
@@ -134,7 +135,7 @@ static AnnotatedJoinColumns buildJoinColumnsWithFormula(
 		joinColumns.setJoins( secondaryTables );
 		joinColumns.setPropertyHolder( propertyHolder );
 		joinColumns.setPropertyName( getRelativePath( propertyHolder, inferredData.getPropertyName() ) );
-		AnnotatedJoinColumn.buildJoinFormula( joinFormula, joinColumns );
+		buildJoinFormula( joinFormula, joinColumns );
 		handlePropertyRef( inferredData.getAttributeMember(), joinColumns );
 		return joinColumns;
 	}
@@ -168,15 +169,15 @@ public static AnnotatedJoinColumns buildJoinColumnsWithDefaultColumnSuffix(
 		assert mappedBy == null || !mappedBy.isBlank();
 		final String propertyName = inferredData.getPropertyName();
 		final String path = qualify( propertyHolder.getPath(), propertyName );
-		final JoinColumn[] overrides = propertyHolder.getOverriddenJoinColumn( path );
-		final JoinColumn[] actualColumns = overrides == null ? joinColumns : overrides;
+		final var overriddenJoinColumns = propertyHolder.getOverriddenJoinColumn( path );
+		final var actualJoinColumns = overriddenJoinColumns == null ? joinColumns : overriddenJoinColumns;
 		final var parent = new AnnotatedJoinColumns();
 		parent.setBuildingContext( context );
 		parent.setJoins( joins );
 		parent.setPropertyHolder( propertyHolder );
 		parent.setPropertyName( getRelativePath( propertyHolder, propertyName ) );
 		parent.setMappedBy( mappedBy );
-		if ( isEmpty( actualColumns ) ) {
+		if ( isEmpty( actualJoinColumns ) ) {
 			buildJoinColumn(
 					null,
 					mappedBy,
@@ -188,7 +189,7 @@ public static AnnotatedJoinColumns buildJoinColumnsWithDefaultColumnSuffix(
 		}
 		else {
 			parent.setMappedBy( mappedBy );
-			for ( var actualColumn : actualColumns ) {
+			for ( var actualColumn : actualJoinColumns ) {
 				buildJoinColumn(
 						actualColumn,
 						mappedBy,

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

@@ -12,7 +12,6 @@
 import org.hibernate.annotations.EmbeddedTable;
 import org.hibernate.boot.model.naming.Identifier;
 import org.hibernate.boot.models.AnnotationPlacementException;
-import org.hibernate.boot.spi.InFlightMetadataCollector;
 import org.hibernate.boot.spi.MetadataBuildingContext;
 import org.hibernate.boot.spi.PropertyData;
 import org.hibernate.mapping.AggregateColumn;
@@ -118,47 +117,48 @@ public static void applyExplicitTableName(
 			PropertyHolder container,
 			MetadataBuildingContext buildingContext) {
 		Table tableToUse = container.getTable();
-		boolean wasExplicit = false;
-		if ( container instanceof ComponentPropertyHolder componentPropertyHolder ) {
-			wasExplicit = componentPropertyHolder.getComponent().wasTableExplicitlyDefined();
-		}
+		boolean wasExplicit =
+				container instanceof ComponentPropertyHolder componentPropertyHolder
+						&& componentPropertyHolder.getComponent().wasTableExplicitlyDefined();
 
-		if ( propertyData.getAttributeMember() != null ) {
-			final EmbeddedTable embeddedTableAnn = propertyData.getAttributeMember()
-					.getDirectAnnotationUsage( EmbeddedTable.class );
+		final var attributeMember = propertyData.getAttributeMember();
+		if ( attributeMember != null ) {
+			final var embeddedTableAnn = attributeMember.getDirectAnnotationUsage( EmbeddedTable.class );
 			// we only allow this when done for an embedded on an entity or mapped-superclass
 			if ( container instanceof ClassPropertyHolder ) {
 				if ( embeddedTableAnn != null ) {
-					final Identifier tableNameIdentifier = buildingContext.getObjectNameNormalizer().normalizeIdentifierQuoting( embeddedTableAnn.value() );
-					final InFlightMetadataCollector.EntityTableXref entityTableXref = buildingContext
-							.getMetadataCollector()
-							.getEntityTableXref( container.getEntityName() );
-					tableToUse =  entityTableXref.resolveTable( tableNameIdentifier );
+					tableToUse = resolveEmbeddedTable( container, buildingContext, embeddedTableAnn );
 					wasExplicit = true;
 				}
 			}
-			else {
-				if ( embeddedTableAnn != null ) {
-					// not allowed
-					throw new AnnotationPlacementException( "@EmbeddedTable only supported for use on entity or mapped-superclass" );
-				}
+			else if ( embeddedTableAnn != null ) {
+				// not allowed
+				throw new AnnotationPlacementException(
+						"@EmbeddedTable only supported for use on entity or mapped-superclass" );
 			}
 		}
-		if ( propertyData.getAttributeMember() != null && container instanceof ClassPropertyHolder ) {
-			final EmbeddedTable embeddedTableAnn = propertyData.getAttributeMember().getDirectAnnotationUsage( EmbeddedTable.class );
+		if ( attributeMember != null && container instanceof ClassPropertyHolder ) {
+			final var embeddedTableAnn = attributeMember.getDirectAnnotationUsage( EmbeddedTable.class );
 			if ( embeddedTableAnn != null ) {
-				final Identifier tableNameIdentifier = buildingContext.getObjectNameNormalizer().normalizeIdentifierQuoting( embeddedTableAnn.value() );
-				final InFlightMetadataCollector.EntityTableXref entityTableXref = buildingContext
-						.getMetadataCollector()
-						.getEntityTableXref( container.getEntityName() );
-				tableToUse =  entityTableXref.resolveTable( tableNameIdentifier );
+				tableToUse = resolveEmbeddedTable( container, buildingContext, embeddedTableAnn );
 				wasExplicit = true;
 			}
 		}
 
 		component.setTable( tableToUse, wasExplicit );
 	}
 
+	private static Table resolveEmbeddedTable(
+			PropertyHolder container, MetadataBuildingContext buildingContext, EmbeddedTable embeddedTableAnn) {
+		final Identifier tableNameIdentifier =
+				buildingContext.getObjectNameNormalizer()
+						.normalizeIdentifierQuoting( embeddedTableAnn.value() );
+		final var entityTableXref =
+				buildingContext.getMetadataCollector()
+						.getEntityTableXref( container.getEntityName() );
+		return entityTableXref.resolveTable( tableNameIdentifier );
+	}
+
 	/**
 	 * Access to the underlying component
 	 */
@@ -235,21 +235,19 @@ protected String normalizeCompositePathForLogging(String attributeName) {
 	public void startingProperty(MemberDetails propertyMemberDetails) {
 		if ( propertyMemberDetails != null ) {
 			// again: the property coming in here *should* be the property on the embeddable (Address#city in the example),
-			// so we just ignore it if there is already an existing conversion info for that path since they would have
+			// so we just ignore it if there is already an existing ConversionInfo for that path since they would have
 			// precedence
 
 			// technically we should only do this for properties of "basic type"
 
 			final String attributeName = propertyMemberDetails.resolveAttributeName();
 			final String path = embeddedAttributeName + '.' + attributeName;
-			if ( attributeConversionInfoMap.containsKey( path ) ) {
-				return;
+			if ( !attributeConversionInfoMap.containsKey( path ) ) {
+				propertyMemberDetails.forEachAnnotationUsage( Convert.class, getSourceModelContext(), (usage) -> {
+					final var info = new AttributeConversionInfo( usage, propertyMemberDetails );
+					attributeConversionInfoMap.put( attributeName, info );
+				} );
 			}
-
-			propertyMemberDetails.forEachAnnotationUsage( Convert.class, getSourceModelContext(), (usage) -> {
-				final AttributeConversionInfo info = new AttributeConversionInfo( usage, propertyMemberDetails );
-				attributeConversionInfoMap.put( attributeName, info );
-			} );
 		}
 	}
 
@@ -403,7 +401,7 @@ public void setParentProperty(String parentProperty) {
 	@Override
 	public Column[] getOverriddenColumn(String propertyName) {
 		//FIXME this is yukky
-		Column[] result = super.getOverriddenColumn( propertyName );
+		var result = super.getOverriddenColumn( propertyName );
 		if ( result == null ) {
 			final String userPropertyName = extractUserPropertyName( "id", propertyName );
 			if ( userPropertyName != null ) {
@@ -434,6 +432,7 @@ private String extractUserPropertyName(String redundantString, String propertyNa
 
 	@Override
 	public String toString() {
-		return getClass().getSimpleName() + "(" + parent.normalizeCompositePathForLogging( embeddedAttributeName ) + ")";
+		return getClass().getSimpleName()
+			+ "(" + parent.normalizeCompositePathForLogging( embeddedAttributeName ) + ")";
 	}
 }

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

@@ -61,12 +61,14 @@ public interface PropertyHolder {
 	String getPath();
 
 	/**
-	 * return null if the column is not overridden, or an array of column if true
+	 * return null if the column is not overridden,
+	 * or an array of columns if it is
 	 */
 	Column[] getOverriddenColumn(String propertyName);
 
 	/**
-	 * return null if the column is not overridden, or an array of column if true
+	 * return null if the column is not overridden,
+	 * or an array of columns if it is
 	 */
 	JoinColumn[] getOverriddenJoinColumn(String propertyName);