HHH-19542: Feed-back from Gavin King

Author: itsmoonrack

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

@@ -4,14 +4,16 @@
  */
 package org.hibernate.boot.model.internal;
 
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.concurrent.atomic.AtomicReference;
 
 import org.hibernate.AnnotationException;
 import org.hibernate.boot.spi.MetadataBuildingContext;
 import org.hibernate.boot.spi.PropertyData;
+import org.hibernate.internal.util.StringHelper;
 import org.hibernate.mapping.AggregateColumn;
 import org.hibernate.mapping.Component;
 import org.hibernate.mapping.Join;
@@ -31,7 +33,6 @@
 import static org.hibernate.boot.model.internal.ClassPropertyHolder.handleGenericComponentProperty;
 import static org.hibernate.boot.model.internal.PropertyBinder.hasIdAnnotation;
 import static org.hibernate.internal.util.StringHelper.isEmpty;
-import static org.hibernate.internal.util.StringHelper.nullIfEmpty;
 import static org.hibernate.internal.util.StringHelper.qualifyConditionally;
 import static org.hibernate.spi.NavigablePath.IDENTIFIER_MAPPER_PROPERTY;
 
@@ -71,7 +72,7 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder {
 
 	private final String embeddedAttributeName;
 	private final Map<String,AttributeConversionInfo> attributeConversionInfoMap;
-	private final AtomicReference<AnnotatedColumn> annotatedColumn;
+	private final List<AnnotatedColumn> annotatedColumns;
 
 	public ComponentPropertyHolder(
 			Component component,
@@ -100,9 +101,9 @@ public ComponentPropertyHolder(
 		}
 
 		if ( parent instanceof ComponentPropertyHolder parentHolder ) {
-			this.annotatedColumn = parentHolder.annotatedColumn;
+			this.annotatedColumns = parentHolder.annotatedColumns;
 		} else {
-			this.annotatedColumn = new AtomicReference<>();
+			this.annotatedColumns = new ArrayList<>();
 		}
 	}
 
@@ -236,7 +237,7 @@ public void addProperty(Property property, MemberDetails attributeMemberDetails,
 		// if not, change the component table if no properties are set
 		// if a property is set already the core cannot support that
 		final Table table = property.getValue().getTable();
-		if ( !table.equals( getTable() ) || !columnTableIsConsistent( columns ) ) {
+		if ( !table.equals( getTable() ) ) {
 			if ( component.getPropertySpan() == 0 ) {
 				component.setTable( table );
 			}
@@ -248,21 +249,28 @@ public void addProperty(Property property, MemberDetails attributeMemberDetails,
 				);
 			}
 		}
+		if ( columns != null ) {
+			annotatedColumns.addAll( columns.getColumns() );
+		}
 		addProperty( property, attributeMemberDetails, declaringClass );
 	}
 
-	private boolean columnTableIsConsistent(AnnotatedColumns columns) {
-		if ( columns != null ) {
-			for ( AnnotatedColumn current : columns.getColumns() ) {
-				final AnnotatedColumn previous = annotatedColumn.getAndSet( current );
-				if ( previous != null && !Objects.equals(
-							nullIfEmpty( current.getExplicitTableName() ),
-							nullIfEmpty( previous.getExplicitTableName() ) ) ) {
-					return false;
+	public void checkPropertyConsistency() {
+		if ( annotatedColumns.size() > 1 ) {
+			for ( int currentIndex = 1; currentIndex < annotatedColumns.size(); currentIndex++ ) {
+				final AnnotatedColumn current = annotatedColumns.get( currentIndex );
+				final AnnotatedColumn previous = annotatedColumns.get( currentIndex - 1 );
+				if ( !Objects.equals(
+						StringHelper.nullIfEmpty( current.getExplicitTableName() ),
+						StringHelper.nullIfEmpty( previous.getExplicitTableName() ) ) ) {
+					throw new AnnotationException(
+							"Embeddable class '" + component.getComponentClassName()
+							+ "' has properties mapped to two different tables"
+							+ " (all properties of the embeddable class must map to the same table)"
+					);
 				}
 			}
 		}
-		return true;
 	}
 
 	@Override

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

@@ -455,7 +455,7 @@ static Component fillEmbeddable(
 		if ( LOG.isDebugEnabled() ) {
 			LOG.debug( "Binding component with path: " + subpath );
 		}
-		final PropertyHolder subholder = buildPropertyHolder(
+		final ComponentPropertyHolder subholder = buildPropertyHolder(
 				component,
 				subpath,
 				inferredData,
@@ -579,6 +579,8 @@ else if ( member.hasDirectAnnotationUsage( GeneratedValue.class ) ) {
 			}
 		}
 
+		subholder.checkPropertyConsistency();
+
 		if ( compositeUserType != null ) {
 			processCompositeUserType( component, compositeUserType );
 		}

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

@@ -47,7 +47,7 @@ public static PropertyHolder buildPropertyHolder(
 	 *
 	 * @return PropertyHolder
 	 */
-	public static PropertyHolder buildPropertyHolder(
+	public static ComponentPropertyHolder buildPropertyHolder(
 			Component component,
 			String path,
 			PropertyData inferredData,