From 75d2ada4d8f0a16acb78745ae4655ed8359d3f16 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Tue, 17 Aug 2021 18:30:31 +0200 Subject: [PATCH] Fix component and foreign- as well as primary-key properties/columns ordering --- .../source/internal/hbm/ModelBinder.java | 5 ++- .../cfg/SimpleToOneFkSecondPass.java | 36 ++++++++++++++++++ .../cfg/annotations/TableBinder.java | 11 ++++++ .../java/org/hibernate/mapping/Component.java | 38 +++++++++++++++---- .../java/org/hibernate/mapping/ManyToOne.java | 8 ++++ .../java/org/hibernate/mapping/OneToOne.java | 2 + .../org/hibernate/mapping/SimpleValue.java | 17 +++++++++ .../java/org/hibernate/mapping/ToOne.java | 34 ++++++++++++++++- .../internal/MappingModelCreationHelper.java | 11 +++++- 9 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/cfg/SimpleToOneFkSecondPass.java diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/ModelBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/ModelBinder.java index 2a14d0b0fe..f7bdea2261 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/ModelBinder.java +++ b/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 ); diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/SimpleToOneFkSecondPass.java b/hibernate-core/src/main/java/org/hibernate/cfg/SimpleToOneFkSecondPass.java new file mode 100644 index 0000000000..7b66966a72 --- /dev/null +++ b/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 . + */ +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(); + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/annotations/TableBinder.java b/hibernate-core/src/main/java/org/hibernate/cfg/annotations/TableBinder.java index b226d854ff..4527dc8103 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/annotations/TableBinder.java +++ b/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 ); + } } } } diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java b/hibernate-core/src/main/java/org/hibernate/mapping/Component.java index 32ac703b4c..077d8538bb 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java +++ b/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 columns = primaryKey.getColumns(); + columns.clear(); + for ( int i = 0; i < properties.size(); i++ ) { + final Iterator 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; } } diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java b/hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java index 7fef292691..581c3c95ca 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java +++ b/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(); diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/OneToOne.java b/hibernate-core/src/main/java/org/hibernate/mapping/OneToOne.java index 6fde83f482..959e63a49b 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/OneToOne.java +++ b/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() ); diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java index 4b9b444610..ffe4337aef 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java +++ b/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(); diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/ToOne.java b/hibernate-core/src/main/java/org/hibernate/mapping/ToOne.java index 42ba133465..cecc2e445c 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/ToOne.java +++ b/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 ); + } + } + } } diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/MappingModelCreationHelper.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/MappingModelCreationHelper.java index e7e9bc70c6..f525aad172 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/MappingModelCreationHelper.java +++ b/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