Fix component and foreign- as well as primary-key properties/columns ordering

hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/ModelBinder.java

@@ -97,6 +97,7 @@ import org.hibernate.boot.spi.NaturalIdUniqueKeyBinder;
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.cfg.FkSecondPass;
 import org.hibernate.cfg.SecondPass;
+import org.hibernate.cfg.SimpleToOneFkSecondPass;
 import org.hibernate.engine.FetchStyle;
 import org.hibernate.engine.FetchTiming;
 import org.hibernate.engine.config.spi.ConfigurationService;
@@ -2042,7 +2043,9 @@ public class ModelBinder {
 			);
 		}
 
-		oneToOneBinding.createForeignKey();
+		// Defer the creation of the foreign key as we need the associated entity persister to be initialized
+		// so that we can observe the properties/columns of a possible component in the correct order
+		metadataBuildingContext.getMetadataCollector().addSecondPass( new SimpleToOneFkSecondPass( oneToOneBinding ) );
 
 		Property prop = new Property();
 		prop.setValue( oneToOneBinding );

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

@@ -0,0 +1,36 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.cfg;
+
+import org.hibernate.MappingException;
+import org.hibernate.mapping.ToOne;
+
+/**
+ * A simple second pass that just creates the foreign key
+ *
+ * @author Christian Beikov
+ */
+public class SimpleToOneFkSecondPass extends FkSecondPass {
+
+	public SimpleToOneFkSecondPass(ToOne value) {
+		super( value, null );
+	}
+
+	@Override
+	public String getReferencedEntityName() {
+		return ( (ToOne) value ).getReferencedEntityName();
+	}
+
+	@Override
+	public boolean isInPrimaryKey() {
+		return false;
+	}
+
+	public void doSecondPass(java.util.Map persistentClasses) throws MappingException {
+		value.createForeignKey();
+	}
+}

hibernate-core/src/main/java/org/hibernate/cfg/annotations/TableBinder.java

@@ -28,12 +28,15 @@ import org.hibernate.cfg.Ejb3JoinColumn;
 import org.hibernate.cfg.IndexOrUniqueKeySecondPass;
 import org.hibernate.cfg.JPAIndexHolder;
 import org.hibernate.cfg.ObjectNameSource;
+import org.hibernate.cfg.SimpleToOneFkSecondPass;
+import org.hibernate.cfg.ToOneFkSecondPass;
 import org.hibernate.cfg.UniqueConstraintHolder;
 import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.internal.util.StringHelper;
 import org.hibernate.internal.util.collections.CollectionHelper;
 import org.hibernate.mapping.Collection;
 import org.hibernate.mapping.Column;
+import org.hibernate.mapping.Component;
 import org.hibernate.mapping.DependantValue;
 import org.hibernate.mapping.JoinedSubclass;
 import org.hibernate.mapping.PersistentClass;
@@ -656,6 +659,10 @@ public class TableBinder {
 					);
 				}
 				else {
+					// Ensure the component is sorted so that we can simply set sorted to true on the to-one
+					if ( referencedEntity.getKey() instanceof Component ) {
+						( (Component) referencedEntity.getKey() ).sortProperties();
+					}
 					//explicit referencedColumnName
 					Iterator idColItr = referencedEntity.getKey().getColumnIterator();
 					org.hibernate.mapping.Column col;
@@ -698,6 +705,10 @@ public class TableBinder {
 							);
 						}
 					}
+
+					if ( value instanceof ToOne ) {
+						( (ToOne) value ).setSorted( true );
+					}
 				}
 			}
 		}

hibernate-core/src/main/java/org/hibernate/mapping/Component.java

@@ -199,12 +199,15 @@ public class Component extends SimpleValue implements MetaAttributable {
 		if ( localType == null ) {
 			synchronized ( this ) {
 				if ( type == null ) {
+					// Make sure this is sorted which is important especially for synthetic components
+					// Other components should be sorted already
+					sortProperties( true );
+
 					// TODO : temporary initial step towards HHH-1907
 					final ComponentMetamodel metamodel = new ComponentMetamodel(
 							this,
 							getMetadata().getMetadataBuildingOptions()
 					);
-
 					localType = isEmbedded()
 							? new EmbeddedComponentType( getBuildingContext().getBootstrapContext().getTypeConfiguration(), metamodel, originalPropertyOrder )
 							: new ComponentType( getBuildingContext().getBootstrapContext().getTypeConfiguration(), metamodel, originalPropertyOrder );
@@ -528,16 +531,20 @@ public class Component extends SimpleValue implements MetaAttributable {
 		getType();
 	}
 
-	public void sortProperties() {
-		if ( this.originalPropertyOrder != ArrayHelper.EMPTY_INT_ARRAY ) {
-			return;
+	public int[] sortProperties() {
+		return sortProperties( false );
+	}
+
+	private int[] sortProperties(boolean forceRetainOriginalOrder) {
+		if ( originalPropertyOrder != ArrayHelper.EMPTY_INT_ARRAY ) {
+			return originalPropertyOrder;
 		}
 		final int[] originalPropertyOrder;
-		// We need to capture the original property order if this is an alternate unique key
+		// We need to capture the original property order if this is an alternate unique key or embedded component property
 		// to be able to sort the other side of the foreign key accordingly
 		// and also if the source is a XML mapping
 		// because XML mappings might refer to this through the defined order
-		if ( isAlternateUniqueKey() || getBuildingContext() instanceof MappingDocument ) {
+		if ( forceRetainOriginalOrder || isAlternateUniqueKey() || isEmbedded() || getBuildingContext() instanceof MappingDocument ) {
 			final Object[] originalProperties = properties.toArray();
 			properties.sort( Comparator.comparing( Property::getName ) );
 			originalPropertyOrder = new int[originalProperties.length];
@@ -549,7 +556,24 @@ public class Component extends SimpleValue implements MetaAttributable {
 			properties.sort( Comparator.comparing( Property::getName ) );
 			originalPropertyOrder = null;
 		}
-		this.originalPropertyOrder = originalPropertyOrder;
+		if ( isKey ) {
+			final PrimaryKey primaryKey = getOwner().getTable().getPrimaryKey();
+			if ( primaryKey != null ) {
+				// We have to re-order the primary key accordingly
+				final List<Column> columns = primaryKey.getColumns();
+				columns.clear();
+				for ( int i = 0; i < properties.size(); i++ ) {
+					final Iterator<Selectable> columnIterator = properties.get( i ).getColumnIterator();
+					while ( columnIterator.hasNext() ) {
+						final Selectable selectable = columnIterator.next();
+						if ( selectable instanceof Column ) {
+							columns.add( (Column) selectable );
+						}
+					}
+				}
+			}
+		}
+		return this.originalPropertyOrder = originalPropertyOrder;
 	}
 
 }

hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java

@@ -49,6 +49,8 @@ public class ManyToOne extends ToOne {
 	}
 
 	public void createForeignKey() throws MappingException {
+		// Ensure properties are sorted before we create a foreign key
+		sortProperties();
 		// the case of a foreign key to something other than the pk is handled in createPropertyRefConstraints
 		if (referencedPropertyName==null && !hasFormula() ) {
 			createForeignKeyOfEntity( ( (EntityType) getType() ).getAssociatedEntityName() );
@@ -59,6 +61,8 @@ public class ManyToOne extends ToOne {
 
 	public void createPropertyRefConstraints(Map persistentClasses) {
 		if (referencedPropertyName!=null) {
+			// Ensure properties are sorted before we create a foreign key
+			sortProperties();
 			PersistentClass pc = (PersistentClass) persistentClasses.get(getReferencedEntityName() );
 			
 			Property property = pc.getReferencedProperty( getReferencedPropertyName() );
@@ -72,6 +76,10 @@ public class ManyToOne extends ToOne {
 					);
 			} 
 			else {
+				// Make sure synthetic properties are sorted
+				if ( property.getValue() instanceof Component ) {
+					( (Component) property.getValue() ).sortProperties();
+				}
 				// todo : if "none" another option is to create the ForeignKey object still	but to set its #disableCreation flag
 				if ( !hasFormula() && !"none".equals( getForeignKeyName() ) ) {
 					java.util.List refColumns = new ArrayList();

hibernate-core/src/main/java/org/hibernate/mapping/OneToOne.java

@@ -85,6 +85,8 @@ public class OneToOne extends ToOne {
 	}
 
 	public void createForeignKey() throws MappingException {
+		// Ensure properties are sorted before we create a foreign key
+		sortProperties();
 		if ( constrained && referencedPropertyName==null) {
 			//TODO: handle the case of a foreign key to something other than the pk
 			createForeignKeyOfEntity( ( (EntityType) getType() ).getAssociatedEntityName() );

hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java

@@ -43,6 +43,7 @@ import org.hibernate.id.factory.IdentifierGeneratorFactory;
 import org.hibernate.internal.CoreLogging;
 import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.internal.util.ReflectHelper;
+import org.hibernate.internal.util.collections.ArrayHelper;
 import org.hibernate.metamodel.model.convert.spi.JpaAttributeConverter;
 import org.hibernate.resource.beans.spi.ManagedBeanRegistry;
 import org.hibernate.service.ServiceRegistry;
@@ -164,6 +165,22 @@ public abstract class SimpleValue implements KeyValue {
 		updatability.add( false );
 	}
 
+	protected void sortColumns(int[] originalOrder) {
+		final Selectable[] originalColumns = columns.toArray(new Selectable[0]);
+		final boolean[] originalInsertability = ArrayHelper.toBooleanArray( insertability );
+		final boolean[] originalUpdatability = ArrayHelper.toBooleanArray( updatability );
+		for ( int i = 0; i < originalOrder.length; i++ ) {
+			final int originalIndex = originalOrder[i];
+			final Selectable selectable = originalColumns[originalIndex];
+			if ( selectable instanceof Column ) {
+				( (Column) selectable ).setTypeIndex( i );
+			}
+			columns.set( i, selectable );
+			insertability.set( i, originalInsertability[originalIndex] );
+			updatability.set( i, originalUpdatability[originalIndex] );
+		}
+	}
+
 	@Override
 	public boolean hasFormula() {
 		Iterator iter = getColumnIterator();

hibernate-core/src/main/java/org/hibernate/mapping/ToOne.java

@@ -10,9 +10,9 @@ import org.hibernate.FetchMode;
 import org.hibernate.MappingException;
 import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
 import org.hibernate.boot.spi.MetadataBuildingContext;
-import org.hibernate.boot.spi.MetadataImplementor;
 import org.hibernate.engine.spi.Mapping;
 import org.hibernate.internal.util.ReflectHelper;
+import org.hibernate.type.ComponentType;
 import org.hibernate.type.Type;
 
 import java.util.Objects;
@@ -27,6 +27,7 @@ public abstract class ToOne extends SimpleValue implements Fetchable {
 	private String referencedEntityName;
 	private String propertyName;
 	private boolean lazy = true;
+	private boolean sorted;
 	protected boolean unwrapProxy;
 	protected boolean isUnwrapProxyImplicit;
 	protected boolean referenceToPrimaryKey = true;
@@ -143,5 +144,34 @@ public abstract class ToOne extends SimpleValue implements Fetchable {
 	public void setReferenceToPrimaryKey(boolean referenceToPrimaryKey) {
 		this.referenceToPrimaryKey = referenceToPrimaryKey;
 	}
-	
+
+	public boolean isSorted() {
+		return sorted;
+	}
+
+	public void setSorted(boolean sorted) {
+		this.sorted = sorted;
+	}
+
+	public void sortProperties() {
+		if ( sorted ) {
+			return;
+		}
+		sorted = true;
+		final PersistentClass entityBinding = getMetadata().getEntityBinding( getReferencedEntityName() );
+		final Value value;
+		if ( getReferencedPropertyName() == null ) {
+			value = entityBinding.getIdentifier();
+		}
+		else {
+			value = entityBinding.getProperty( getReferencedPropertyName() ).getValue();
+		}
+		if ( value instanceof Component ) {
+			final Component component = (Component) value;
+			final int[] originalPropertyOrder = component.sortProperties();
+			if ( originalPropertyOrder != null ) {
+				sortColumns( originalPropertyOrder );
+			}
+		}
+	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/MappingModelCreationHelper.java

@@ -1184,9 +1184,11 @@ public class MappingModelCreationHelper {
 
 	private static int[] getPropertyOrder(Value bootValueMapping, MappingModelCreationProcess creationProcess) {
 		final ComponentType componentType;
+		final boolean sorted;
 		if ( bootValueMapping instanceof Collection ) {
 			final Collection collectionBootValueMapping = (Collection) bootValueMapping;
 			componentType = (ComponentType) collectionBootValueMapping.getKey().getType();
+			sorted = false;
 		}
 		else {
 			final EntityType entityType = (EntityType) bootValueMapping.getType();
@@ -1194,9 +1196,16 @@ public class MappingModelCreationHelper {
 					creationProcess.getCreationContext().getSessionFactory()
 			);
 			componentType = (ComponentType) identifierOrUniqueKeyType;
+			if ( bootValueMapping instanceof ToOne ) {
+				sorted = ( (ToOne) bootValueMapping ).isSorted();
+			}
+			else {
+				// Assume one-to-many is sorted, because it always uses the primary key value
+				sorted = true;
+			}
 		}
 		// Consider the reordering if available
-		if ( componentType.getOriginalPropertyOrder() != null ) {
+		if ( !sorted && componentType.getOriginalPropertyOrder() != null ) {
 			return componentType.getOriginalPropertyOrder();
 		}
 		// A value that came from the annotation model is already sorted appropriately