This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch 8.0.x-hibernate7 in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 47f425b456b4da77d61ee625419ad4b2fd1c83e3 Author: Walter Duque de Estrada <[email protected]> AuthorDate: Wed Feb 18 17:36:13 2026 -0600 Refactor order by and discriminator logic to HibernateToManyProperty and clean up CollectionSecondPassBinder --- .../hibernate/HibernateToManyProperty.java | 48 +++++++- .../secondpass/CollectionSecondPassBinder.java | 123 +++++---------------- .../secondpass/ListSecondPassBinder.java | 2 +- .../hibernate/HibernateToManyPropertySpec.groovy | 113 ++++++++++++++++++- 4 files changed, 185 insertions(+), 101 deletions(-) diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/hibernate/HibernateToManyProperty.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/hibernate/HibernateToManyProperty.java index 317860cdbb..e6ae6becc5 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/hibernate/HibernateToManyProperty.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/hibernate/HibernateToManyProperty.java @@ -1,21 +1,67 @@ package org.grails.orm.hibernate.cfg.domainbinding.hibernate; +import org.grails.datastore.mapping.model.DatastoreConfigurationException; import org.grails.datastore.mapping.model.types.Association; import org.grails.datastore.mapping.model.types.Basic; import org.grails.datastore.mapping.model.types.mapping.PropertyWithMapping; +import org.grails.orm.hibernate.cfg.GrailsHibernatePersistentEntity; import org.grails.orm.hibernate.cfg.GrailsHibernatePersistentProperty; import org.grails.orm.hibernate.cfg.PropertyConfig; - +import org.grails.orm.hibernate.cfg.domainbinding.util.OrderByClauseBuilder; import org.hibernate.FetchMode; +import org.hibernate.MappingException; +import org.hibernate.mapping.Collection; +import org.hibernate.mapping.PersistentClass; import org.springframework.util.StringUtils; import java.util.Map; +import java.util.Optional; +import java.util.Set; /** * Marker interface for Hibernate to-many associations */ public interface HibernateToManyProperty extends PropertyWithMapping<PropertyConfig>, GrailsHibernatePersistentProperty { + /** + * Binds the order by clause for the collection if configured. + * + * @param collection The Hibernate collection + * @param persistentClasses The map of persistent classes + * @param orderByClauseBuilder The order by clause builder + * @return The associated persistent class. + * @throws MappingException if the association references an unmapped class + */ + default PersistentClass bindOrderBy(Collection collection, Map<?, ?> persistentClasses, OrderByClauseBuilder orderByClauseBuilder) { + return Optional.ofNullable(getHibernateAssociatedEntity()) + .map(referenced -> { + if (referenced.isTablePerHierarchySubclass()) { + String discriminatorColumnName = referenced.getDiscriminatorColumnName(); + Set<String> discSet = referenced.buildDiscriminatorSet(); + String inclause = String.join(",", discSet); + + collection.setWhere(discriminatorColumnName + " in (" + inclause + ")"); + } + + PersistentClass associatedClass = (PersistentClass) persistentClasses.get(referenced.getName()); + if (associatedClass == null) { + throw new MappingException("Association references unmapped class: " + referenced.getName()); + } + + if (hasSort()) { + if (!isBidirectional() && this instanceof HibernateOneToManyProperty) { + throw new DatastoreConfigurationException("Default sort for associations [" + getHibernateOwner().getName() + "->" + getName() + + "] are not supported with unidirectional one to many relationships."); + } + GrailsHibernatePersistentProperty sortBy = (GrailsHibernatePersistentProperty) referenced.getPropertyByName(getSort()); + String order = Optional.ofNullable(getOrder()).orElse("asc"); + collection.setOrderBy(orderByClauseBuilder.buildOrderByClause(sortBy.getName(), associatedClass, collection.getRole(), order)); + } + return associatedClass; + }) + .orElse(null); + } + default boolean hasSort() { return StringUtils.hasText(getMappedForm().getSort()); } diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinder.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinder.java index 3feecc1a17..21745c75bd 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinder.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/CollectionSecondPassBinder.java @@ -1,10 +1,8 @@ package org.grails.orm.hibernate.cfg.domainbinding.secondpass; import jakarta.annotation.Nonnull; -import org.grails.datastore.mapping.model.DatastoreConfigurationException; import org.grails.datastore.mapping.model.config.GormProperties; import org.grails.datastore.mapping.model.types.Association; -import org.grails.datastore.mapping.model.types.Basic; import org.grails.orm.hibernate.cfg.*; import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateManyToManyProperty; import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateOneToManyProperty; @@ -15,7 +13,6 @@ import org.grails.orm.hibernate.cfg.domainbinding.binder.ManyToOneBinder; import org.grails.orm.hibernate.cfg.domainbinding.util.OrderByClauseBuilder; import org.grails.orm.hibernate.cfg.domainbinding.binder.SimpleValueColumnBinder; -import org.hibernate.MappingException; import org.hibernate.boot.spi.InFlightMetadataCollector; import org.hibernate.mapping.*; import org.hibernate.mapping.Collection; @@ -74,143 +71,73 @@ public class CollectionSecondPassBinder { @Nonnull InFlightMetadataCollector mappings, Map<?, ?> persistentClasses, @Nonnull Collection collection) { - PersistentClass associatedClass = null; - - if (LOG.isDebugEnabled()) - LOG.debug("Mapping collection: " - + collection.getRole() - + " -> " - + collection.getCollectionTable().getName()); - - GrailsHibernatePersistentEntity referenced = property.getHibernateAssociatedEntity(); - if (property.hasSort()) { - if (!property.isBidirectional() && (property instanceof HibernateOneToManyProperty)) { - throw new DatastoreConfigurationException("Default sort for associations ["+property.getHibernateOwner().getName()+"->" + property.getName() + - "] are not supported with unidirectional one to many relationships."); - } - if (referenced != null) { - GrailsHibernatePersistentProperty propertyToSortBy = (GrailsHibernatePersistentProperty) referenced.getPropertyByName(property.getSort()); - - String associatedClassName = referenced.getName(); - - associatedClass = (PersistentClass) persistentClasses.get(associatedClassName); - if (associatedClass != null) { - collection.setOrderBy(orderByClauseBuilder.buildOrderByClause(propertyToSortBy.getName(), associatedClass, collection.getRole(), - property.getOrder() != null ? property.getOrder() : "asc")); - } - } + if (LOG.isDebugEnabled()) { + LOG.debug("Mapping collection: {} -> {}", collection.getRole(), collection.getCollectionTable().getName()); } - // Configure one-to-many - if (collection.isOneToMany()) { - - if (referenced != null && referenced.isTablePerHierarchySubclass()) { - String discriminatorColumnName = referenced.getDiscriminatorColumnName(); - //NOTE: this will build the set for the in clause if it has sublcasses - Set<String> discSet = referenced.buildDiscriminatorSet(); - String inclause = String.join(",", discSet); - - collection.setWhere(discriminatorColumnName + " in (" + inclause + ")"); - } - + PersistentClass associatedClass = property.bindOrderBy(collection, persistentClasses, orderByClauseBuilder); + if (collection.isOneToMany()) { OneToMany oneToMany = (OneToMany) collection.getElement(); - String associatedClassName = oneToMany.getReferencedEntityName(); - - associatedClass = (PersistentClass) persistentClasses.get(associatedClassName); - // if there is no persistent class for the association throw exception - if (associatedClass == null) { - throw new MappingException("Association references unmapped class: " + oneToMany.getReferencedEntityName()); - } - oneToMany.setAssociatedClass(associatedClass); if (property.shouldBindWithForeignKey()) { collection.setCollectionTable(associatedClass.getTable()); } - collectionForPropertyConfigBinder.bindCollectionForPropertyConfig(collection, property); } - final boolean isManyToMany = property instanceof HibernateManyToManyProperty; - if(referenced != null && !isManyToMany && referenced.isMultiTenant()) { - String filterCondition = referenced.getMultiTenantFilterCondition(defaultColumnNameFetcher); - if(filterCondition != null) { - if (property.isUnidirectionalOneToMany()) { - collection.addManyToManyFilter(GormProperties.TENANT_IDENTITY, filterCondition, true, Collections.emptyMap(), Collections.emptyMap()); - } else { - collection.addFilter(GormProperties.TENANT_IDENTITY, filterCondition, true, Collections.emptyMap(), Collections.emptyMap()); - } - } - } + Optional.ofNullable(property.getHibernateAssociatedEntity()) + .filter(referenced -> !(property instanceof HibernateManyToManyProperty) && referenced.isMultiTenant()) + .map(referenced -> referenced.getMultiTenantFilterCondition(defaultColumnNameFetcher)) + .ifPresent(filterCondition -> { + if (property.isUnidirectionalOneToMany()) { + collection.addManyToManyFilter(GormProperties.TENANT_IDENTITY, filterCondition, true, Collections.emptyMap(), Collections.emptyMap()); + } else { + collection.addFilter(GormProperties.TENANT_IDENTITY, filterCondition, true, Collections.emptyMap(), Collections.emptyMap()); + } + }); if (property.isSorted()) { collection.setSorted(true); } - // setup the primary key references DependantValue key = primaryKeyValueCreator.createPrimaryKeyValue(collection); + collection.setKey(key); - // link a bidirectional relationship if (property.isBidirectional()) { - - var otherSide = property.getHibernateInverseSide(); - - if ((otherSide instanceof org.grails.datastore.mapping.model.types.ToOne) && property.shouldBindWithForeignKey()) { - - bidirectionalOneToManyLinker.link(collection, associatedClass, key, otherSide); - - } else if ((otherSide instanceof HibernateManyToManyProperty) || java.util.Map.class.isAssignableFrom(property.getType())) { - + if (property.getHibernateInverseSide() instanceof org.grails.datastore.mapping.model.types.ToOne && property.shouldBindWithForeignKey()) { + bidirectionalOneToManyLinker.link(collection, associatedClass, key, property.getHibernateInverseSide()); + } else if (property.getHibernateInverseSide() instanceof HibernateManyToManyProperty || java.util.Map.class.isAssignableFrom(property.getType())) { dependentKeyValueBinder.bind(property, key); - } - } else { - if (property.getMappedForm().hasJoinKeyMapping()) { - - String columnName = property.getMappedForm().getJoinTable().getKey().getName(); - - new SimpleValueColumnBinder().bindSimpleValue(key, "long", columnName, true); - + new SimpleValueColumnBinder().bindSimpleValue(key, "long", property.getMappedForm().getJoinTable().getKey().getName(), true); } else { - dependentKeyValueBinder.bind(property, key); - } - - } - collection.setKey(key); - - // get cache config - CacheConfig cacheConfig = property.getMappedForm().getCache(); - if (cacheConfig != null) { - collection.setCacheConcurrencyStrategy(cacheConfig.getUsage()); } - // if we have a many-to-many - if (isManyToMany || property.isBidirectionalOneToManyMap()) { - var otherSide = property.getHibernateInverseSide(); + Optional.ofNullable(property.getMappedForm().getCache()) + .ifPresent(cacheConfig -> collection.setCacheConcurrencyStrategy(cacheConfig.getUsage())); + if (property instanceof HibernateManyToManyProperty || property.isBidirectionalOneToManyMap()) { if (property.isBidirectional()) { - if (LOG.isDebugEnabled()) - LOG.debug("[CollectionSecondPassBinder] Mapping other side " + otherSide.getHibernateOwner().getName() + "." + otherSide.getName() + " -> " + collection.getCollectionTable().getName() + " as ManyToOne"); - ManyToOne element = manyToOneBinder.bindManyToOne((Association)otherSide, collection.getCollectionTable(), EMPTY_PATH); + var otherSide = property.getHibernateInverseSide(); + ManyToOne element = manyToOneBinder.bindManyToOne((Association) otherSide, collection.getCollectionTable(), EMPTY_PATH); element.setReferencedEntityName(otherSide.getOwner().getName()); collection.setElement(element); collectionForPropertyConfigBinder.bindCollectionForPropertyConfig(collection, property); if (property.isCircular()) { collection.setInverse(false); } - } else { - // TODO support unidirectional many-to-many } } else if (property.isUnidirectionalOneToMany()) { unidirectionalOneToManyBinder.bind((HibernateOneToManyProperty) property, mappings, collection); } else if (property.supportsJoinColumnMapping()) { collectionWithJoinTableBinder.bindCollectionWithJoinTable(property, mappings, collection); } - collectionKeyColumnUpdater.forceNullableAndCheckUpdatable(key, property); // Use the injected service + collectionKeyColumnUpdater.forceNullableAndCheckUpdatable(key, property); } diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/ListSecondPassBinder.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/ListSecondPassBinder.java index a618bb87b6..36a0634b79 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/ListSecondPassBinder.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/ListSecondPassBinder.java @@ -58,7 +58,7 @@ public class ListSecondPassBinder { Table collectionTable = list.getCollectionTable(); SimpleValue iv = new BasicValue(metadataBuildingContext, collectionTable); - String type = ((GrailsHibernatePersistentProperty) property).getIndexColumnType("integer"); + String type = property.getIndexColumnType("integer"); new SimpleValueColumnBinder().bindSimpleValue(iv, type, columnName, true); iv.setTypeName(type); list.setIndex(iv); diff --git a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/hibernate/HibernateToManyPropertySpec.groovy b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/hibernate/HibernateToManyPropertySpec.groovy index 17c687b270..f65f76cc68 100644 --- a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/hibernate/HibernateToManyPropertySpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/hibernate/HibernateToManyPropertySpec.groovy @@ -3,7 +3,7 @@ package org.grails.orm.hibernate.cfg.domainbinding.hibernate import grails.gorm.annotation.Entity import grails.gorm.specs.HibernateGormDatastoreSpec import org.grails.orm.hibernate.cfg.CompositeIdentity -import org.grails.orm.hibernate.cfg.Identity // Added this import +import org.grails.orm.hibernate.cfg.Identity import org.grails.orm.hibernate.cfg.GrailsHibernatePersistentEntity import org.grails.orm.hibernate.cfg.GrailsHibernatePersistentProperty import org.grails.orm.hibernate.cfg.Mapping @@ -14,6 +14,10 @@ import org.grails.datastore.mapping.model.PropertyMapping import org.grails.datastore.mapping.reflect.EntityReflector import org.grails.datastore.mapping.model.types.Association import org.grails.datastore.mapping.model.types.Basic +import org.grails.orm.hibernate.cfg.domainbinding.util.OrderByClauseBuilder +import org.hibernate.mapping.Bag +import org.hibernate.mapping.RootClass +import org.grails.datastore.mapping.model.DatastoreConfigurationException import java.util.Optional import java.util.Map @@ -26,8 +30,108 @@ class HibernateToManyPropertySpec extends HibernateGormDatastoreSpec { return property } + def "test bindOrderBy with sort configured"() { + given: + def property = createTestHibernateToManyProperty(TestEntityWithMany, "items") as HibernateToManyProperty + def collection = new Bag(getGrailsDomainBinder().getMetadataBuildingContext(), null) + collection.setRole("TestEntityWithMany.items") + + def persistentClasses = [:] + def associatedPersistentClass = new RootClass(getGrailsDomainBinder().getMetadataBuildingContext()) + associatedPersistentClass.setEntityName(AssociatedItem.name) + persistentClasses[AssociatedItem.name] = associatedPersistentClass + def orderByClauseBuilder = Mock(OrderByClauseBuilder) + + property.getMappedForm().setSort("value") + property.getMappedForm().setOrder("desc") + when: + def result = property.bindOrderBy(collection, persistentClasses, orderByClauseBuilder) + then: + 1 * orderByClauseBuilder.buildOrderByClause("value", associatedPersistentClass, "TestEntityWithMany.items", "desc") >> "order by value desc" + collection.getOrderBy() == "order by value desc" + result == associatedPersistentClass + } + + def "test bindOrderBy with unidirectional one-to-many throws exception"() { + given: + def property = createTestHibernateToManyProperty(UnidirectionalEntity, "items") as HibernateToManyProperty + def collection = new Bag(getGrailsDomainBinder().getMetadataBuildingContext(), null) + def persistentClasses = [:] + def associatedPersistentClass = new RootClass(getGrailsDomainBinder().getMetadataBuildingContext()) + associatedPersistentClass.setEntityName(AssociatedItem.name) + persistentClasses[AssociatedItem.name] = associatedPersistentClass + def orderByClauseBuilder = Mock(OrderByClauseBuilder) + + property.getMappedForm().setSort("value") + + when: + property.bindOrderBy(collection, persistentClasses, orderByClauseBuilder) + + then: + thrown(DatastoreConfigurationException) + } + + def "test bindOrderBy returns associatedClass even without sort"() { + given: + def property = createTestHibernateToManyProperty(TestEntityWithMany, "items") as HibernateToManyProperty + def collection = new Bag(getGrailsDomainBinder().getMetadataBuildingContext(), null) + def persistentClasses = [:] + def associatedPersistentClass = new RootClass(getGrailsDomainBinder().getMetadataBuildingContext()) + associatedPersistentClass.setEntityName(AssociatedItem.name) + persistentClasses[AssociatedItem.name] = associatedPersistentClass + def orderByClauseBuilder = Mock(OrderByClauseBuilder) + + when: + def result = property.bindOrderBy(collection, persistentClasses, orderByClauseBuilder) + + then: + collection.getOrderBy() == null + result == associatedPersistentClass + } + + def "test bindOrderBy throws MappingException when class is unmapped"() { + given: + def property = createTestHibernateToManyProperty(TestEntityWithMany, "items") as HibernateToManyProperty + def collection = new Bag(getGrailsDomainBinder().getMetadataBuildingContext(), null) + def persistentClasses = [:] // Empty map, so AssociatedItem will be missing + def orderByClauseBuilder = Mock(OrderByClauseBuilder) + + when: + property.bindOrderBy(collection, persistentClasses, orderByClauseBuilder) + + then: + thrown(org.hibernate.MappingException) + } + + def "test bindOrderBy with table per hierarchy subclass"() { + given: + def property = createTestHibernateToManyProperty(TestEntityWithMany, "items") as HibernateToManyProperty + def collection = new Bag(getGrailsDomainBinder().getMetadataBuildingContext(), null) + def persistentClasses = [:] + def associatedPersistentClass = new RootClass(getGrailsDomainBinder().getMetadataBuildingContext()) + associatedPersistentClass.setEntityName(AssociatedItem.name) + persistentClasses[AssociatedItem.name] = associatedPersistentClass + def orderByClauseBuilder = Mock(OrderByClauseBuilder) + + // Mock GrailsHibernatePersistentEntity behavior for table per hierarchy + def referencedEntity = property.getHibernateAssociatedEntity() + def spiedReferencedEntity = Spy(referencedEntity) + spiedReferencedEntity.isTablePerHierarchySubclass() >> true + spiedReferencedEntity.getDiscriminatorColumnName() >> "item_type" + spiedReferencedEntity.buildDiscriminatorSet() >> (["'A'", "'B'"] as Set) + + // Inject the spy if possible, or mock the getter on property + def spiedProperty = Spy(property) + spiedProperty.getHibernateAssociatedEntity() >> spiedReferencedEntity + + when: + spiedProperty.bindOrderBy(collection, persistentClasses, orderByClauseBuilder) + + then: + collection.getWhere() == "item_type in ('A','B')" + } def "test getCompositeIdentity returns CompositeIdentity when conditions met"() { given: @@ -130,3 +234,10 @@ class AssociatedItem { TestEntityWithMany parent // Bidirectional for association property testing static belongsTo = [parent: TestEntityWithMany] } + +@Entity +class UnidirectionalEntity { + Long id + Set<AssociatedItem> items + static hasMany = [items: AssociatedItem] +}
