This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch merge-hibernate6 in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 3f57064d8f033e005a78c89f9a63f56ae6dda438 Author: Walter Duque de Estrada <[email protected]> AuthorDate: Sat Sep 27 21:08:45 2025 -0500 more refactoring --- .../orm/hibernate/cfg/GrailsDomainBinder.java | 83 ++++------------- .../domainbinding/DefaultColumnNameFetcher.java | 59 ++++++++++++ .../domainbinding/UniqueKeyForColumnsCreator.java | 43 +++++++++ .../DefaultColumnNameFetcherSpec.groovy | 100 +++++++++++++++++++++ .../UniqueKeyForColumnsCreatorSpec.groovy | 33 +++++++ .../mapping/model/PersistentProperty.java | 8 ++ 6 files changed, 261 insertions(+), 65 deletions(-) diff --git a/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java index 4585678f47..9bdc3c158a 100644 --- a/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java +++ b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java @@ -79,7 +79,6 @@ import org.hibernate.mapping.SingleTableSubclass; import org.hibernate.mapping.Subclass; import org.hibernate.mapping.Table; import org.hibernate.mapping.UnionSubclass; -import org.hibernate.mapping.UniqueKey; import org.hibernate.mapping.Value; import org.hibernate.metamodel.mapping.JdbcMapping; import org.hibernate.persister.entity.UnionSubclassEntityPersister; @@ -124,7 +123,7 @@ public class GrailsDomainBinder implements MetadataContributor { public static final String FOREIGN_KEY_SUFFIX = "_id"; private static final String STRING_TYPE = "string"; private static final String EMPTY_PATH = ""; - private static final char UNDERSCORE = '_'; + public static final char UNDERSCORE = '_'; public static final String CASCADE_ALL = "all"; public static final String CASCADE_SAVE_UPDATE = "save-update"; public static final String CASCADE_NONE = "none"; @@ -567,7 +566,8 @@ public class GrailsDomainBinder implements MetadataContributor { private String getMultiTenantFilterCondition(String sessionFactoryBeanName, PersistentEntity referenced) { TenantId tenantId = referenced.getTenantId(); if(tenantId != null) { - String defaultColumnName = getDefaultColumnName(tenantId, sessionFactoryBeanName); + + String defaultColumnName = new DefaultColumnNameFetcher(getNamingStrategyWrapper()).getDefaultColumnName(tenantId); return ":tenantId = " + defaultColumnName; } else { @@ -1213,7 +1213,7 @@ public class GrailsDomainBinder implements MetadataContributor { return tableName; } - private PhysicalNamingStrategy getPhysicalNamingStrategy(String sessionFactoryBeanName) { + public PhysicalNamingStrategy getPhysicalNamingStrategy(String sessionFactoryBeanName) { return NAMING_STRATEGY_PROVIDER.getPhysicalNamingStrategy(sessionFactoryBeanName); } @@ -1312,9 +1312,7 @@ public class GrailsDomainBinder implements MetadataContributor { * @param sessionFactoryBeanName the session factory bean name */ protected void bindRoot(HibernatePersistentEntity entity, InFlightMetadataCollector mappings, String sessionFactoryBeanName) { - this.namingStrategyWrapper = new NamingStrategyWrapper( - getPhysicalNamingStrategy(sessionFactoryBeanName) - ,getJdbcEnvironment()); + this.namingStrategyWrapper = getNamingStrategyWrapper(); if (mappings.getEntityBinding(entity.getName()) != null) { LOG.info("[GrailsDomainBinder] Class [" + entity.getName() + "] is already mapped, skipping.. "); return; @@ -1347,6 +1345,10 @@ public class GrailsDomainBinder implements MetadataContributor { mappings.addEntityBinding(root); } + public NamingStrategyWrapper getNamingStrategyWrapper() { + return new NamingStrategyWrapper(getPhysicalNamingStrategy(sessionFactoryName), getJdbcEnvironment()); + } + /** * Add a Hibernate filter for multitenancy if the persistent class is multitenant * @@ -1932,10 +1934,6 @@ public class GrailsDomainBinder implements MetadataContributor { return userType; } - private boolean isBidirectionalManyToOne(PersistentProperty currentGrailsProp) { - return ((currentGrailsProp instanceof org.grails.datastore.mapping.model.types.ManyToOne) && ((Association)currentGrailsProp).isBidirectional()); - } - /** * Binds a Hibernate component type using the given GrailsDomainClassProperty instance * @@ -2161,7 +2159,8 @@ public class GrailsDomainBinder implements MetadataContributor { // for each property of a composite id by default we use the table name and the property name as a prefix String string = namingStrategyWrapper.getColumnName(referencedProperty.getName()); String compositeIdPrefix = new BackticksRemover().apply(prefix) + UNDERSCORE + new BackticksRemover().apply(string); - String suffix = getDefaultColumnName(cip, sessionFactoryBeanName); + + String suffix = new DefaultColumnNameFetcher(getNamingStrategyWrapper()).getDefaultColumnName(cip); String finalColumnName = new BackticksRemover().apply(compositeIdPrefix) + UNDERSCORE + new BackticksRemover().apply(suffix); cc = new ColumnConfig(); cc.setName(finalColumnName); @@ -2171,7 +2170,7 @@ public class GrailsDomainBinder implements MetadataContributor { } } - String suffix = getDefaultColumnName(referencedProperty, sessionFactoryBeanName); + String suffix = new DefaultColumnNameFetcher(getNamingStrategyWrapper()).getDefaultColumnName(referencedProperty); String finalColumnName = new BackticksRemover().apply(prefix) + UNDERSCORE + new BackticksRemover().apply(suffix); cc.setName(finalColumnName); columns.add(cc); @@ -2553,25 +2552,11 @@ public class GrailsDomainBinder implements MetadataContributor { String otherColumnName = getColumnNameForPropertyAndPath(otherProp, path, null, sessionFactoryBeanName); keyList.add(new Column(otherColumnName)); } - createUniqueKeyForColumns(table, columnName, keyList); - } - private void createUniqueKeyForColumns(Table table, String columnName, List<Column> columns) { - Collections.reverse(columns); - - UniqueKey uk = new UniqueKey(); - uk.setTable(table); - for(Column column : columns) { - uk.addColumn(column); - } - - if(LOG.isDebugEnabled()) { - LOG.debug("create unique key for " + table.getName() + " columns = " + columns); - } - new UniqueNameGenerator().setGeneratedUniqueName(uk); - table.addUniqueKey(uk); + new UniqueKeyForColumnsCreator().createUniqueKeyForColumns(table, keyList); } + private String getColumnNameForPropertyAndPath(PersistentProperty grailsProp, String path, ColumnConfig cc, String sessionFactoryBeanName) { // First try the column config. @@ -2609,10 +2594,12 @@ public class GrailsDomainBinder implements MetadataContributor { if (columnName == null) { if (isNotEmpty(path)) { String s1 = namingStrategyWrapper.getColumnName(path); - String s2 = getDefaultColumnName(grailsProp, sessionFactoryBeanName); + + String s2 = new DefaultColumnNameFetcher(getNamingStrategyWrapper()).getDefaultColumnName(grailsProp); columnName = new BackticksRemover().apply(s1) + UNDERSCORE + new BackticksRemover().apply(s2); } else { - columnName = getDefaultColumnName(grailsProp, sessionFactoryBeanName); + + columnName = new DefaultColumnNameFetcher(getNamingStrategyWrapper()).getDefaultColumnName(grailsProp); } } return columnName; @@ -2626,40 +2613,6 @@ public class GrailsDomainBinder implements MetadataContributor { return grailsProp instanceof ManyToMany || ofNullable(grailsProp).map(PersistentProperty::isUnidirectionalOneToMany).orElse(false) || grailsProp instanceof Basic; } - private String getDefaultColumnName(PersistentProperty property, String sessionFactoryBeanName) { - - String columnName = namingStrategyWrapper.getColumnName(property.getName()); - if (property instanceof Association) { - Association association = (Association) property; - boolean isBasic = property instanceof Basic; - if (isBasic && (new PersistentPropertyToPropertyConfig().apply(property)).getType() != null) { - return columnName; - } - - if (isBasic) { - return namingStrategyWrapper.getForeignKeyForPropertyDomainClass(property); - } - - if (property instanceof ManyToMany) { - return namingStrategyWrapper.getForeignKeyForPropertyDomainClass(property); - } - - if (!association.isBidirectional() && association instanceof org.grails.datastore.mapping.model.types.OneToMany) { - String prefix = namingStrategyWrapper.getTableName(property.getOwner().getName()); - return new BackticksRemover().apply(prefix) + UNDERSCORE + new BackticksRemover().apply(columnName) + FOREIGN_KEY_SUFFIX; - } - - if (property.isInherited() && isBidirectionalManyToOne(property)) { - return namingStrategyWrapper.getColumnName(property.getOwner().getName()) + '_'+ columnName + FOREIGN_KEY_SUFFIX; - } - - return columnName + FOREIGN_KEY_SUFFIX; - } - - - return columnName; - } - private String getIndexColumnName(PersistentProperty property, String sessionFactoryBeanName) { PropertyConfig pc = new PersistentPropertyToPropertyConfig().apply(property); if (pc != null && pc.getIndexColumn() != null && pc.getIndexColumn().getColumn() != null) { diff --git a/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/DefaultColumnNameFetcher.java b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/DefaultColumnNameFetcher.java new file mode 100644 index 0000000000..8a1dc3f2a5 --- /dev/null +++ b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/DefaultColumnNameFetcher.java @@ -0,0 +1,59 @@ +package org.grails.orm.hibernate.cfg.domainbinding; + +import org.grails.datastore.mapping.model.PersistentProperty; +import org.grails.datastore.mapping.model.types.Association; +import org.grails.datastore.mapping.model.types.Basic; +import org.grails.datastore.mapping.model.types.ManyToMany; + +public class DefaultColumnNameFetcher { + + private static final String FOREIGN_KEY_SUFFIX = "_id"; + private static final String UNDERSCORE = "_"; + + private final NamingStrategyWrapper namingStrategyWrapper; + private final BackticksRemover backticksRemover; + + public DefaultColumnNameFetcher( NamingStrategyWrapper namingStrategyWrapper) { + this.namingStrategyWrapper = namingStrategyWrapper; + this.backticksRemover = new BackticksRemover(); + } + + protected DefaultColumnNameFetcher(NamingStrategyWrapper namingStrategyWrapper , BackticksRemover backticksRemover) { + this.namingStrategyWrapper = namingStrategyWrapper; + this.backticksRemover = backticksRemover; + } + + public String getDefaultColumnName(PersistentProperty property) { + + String columnName = namingStrategyWrapper.getColumnName(property.getName()); + if (property instanceof Association) { + Association association = (Association) property; + boolean isBasic = property instanceof Basic; + if (isBasic && (new PersistentPropertyToPropertyConfig().apply(property)).getType() != null) { + return columnName; + } + + if (isBasic) { + return namingStrategyWrapper.getForeignKeyForPropertyDomainClass(property); + } + + if (property instanceof ManyToMany) { + return namingStrategyWrapper.getForeignKeyForPropertyDomainClass(property); + } + + if (!association.isBidirectional() && association instanceof org.grails.datastore.mapping.model.types.OneToMany) { + String prefix = namingStrategyWrapper.getTableName(property.getOwner().getName()); + return backticksRemover.apply(prefix) + UNDERSCORE + backticksRemover.apply(columnName) + FOREIGN_KEY_SUFFIX; + } + + if (property.isInherited() && property.isBidirectionalManyToOne()) { + return namingStrategyWrapper.getColumnName(property.getOwner().getName()) + '_' + columnName + FOREIGN_KEY_SUFFIX; + } + + return columnName + FOREIGN_KEY_SUFFIX; + } + + + return columnName; + } +} diff --git a/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/UniqueKeyForColumnsCreator.java b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/UniqueKeyForColumnsCreator.java new file mode 100644 index 0000000000..b906939f72 --- /dev/null +++ b/grails-data-hibernate6/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/UniqueKeyForColumnsCreator.java @@ -0,0 +1,43 @@ +package org.grails.orm.hibernate.cfg.domainbinding; + +import java.util.Collections; +import java.util.List; + +import org.hibernate.mapping.Column; +import org.hibernate.mapping.Table; +import org.hibernate.mapping.UniqueKey; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.grails.orm.hibernate.cfg.GrailsDomainBinder; + +public class UniqueKeyForColumnsCreator { + + private final UniqueNameGenerator uniqueNameGenerator; + + public UniqueKeyForColumnsCreator() { + uniqueNameGenerator = new UniqueNameGenerator(); + } + + protected UniqueKeyForColumnsCreator(UniqueNameGenerator uniqueNameGenerator) { + this.uniqueNameGenerator = uniqueNameGenerator; + } + + private static final Logger LOG = LoggerFactory.getLogger(UniqueKeyForColumnsCreator.class); + + public void createUniqueKeyForColumns(Table table, List<Column> columns) { + Collections.reverse(columns); + + UniqueKey uk = new UniqueKey(); + uk.setTable(table); + for(Column column : columns) { + uk.addColumn(column); + } + + if(LOG.isDebugEnabled()) { + LOG.debug("create unique key for {} columns = {}", table.getName(), columns); + } + uniqueNameGenerator.setGeneratedUniqueName(uk); + table.addUniqueKey(uk); + } +} diff --git a/grails-data-hibernate6/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/DefaultColumnNameFetcherSpec.groovy b/grails-data-hibernate6/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/DefaultColumnNameFetcherSpec.groovy new file mode 100644 index 0000000000..768267005f --- /dev/null +++ b/grails-data-hibernate6/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/DefaultColumnNameFetcherSpec.groovy @@ -0,0 +1,100 @@ +package org.grails.orm.hibernate.cfg.domainbinding + +import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment + +import grails.gorm.annotation.Entity +import grails.gorm.specs.HibernateGormDatastoreSpec +import org.grails.datastore.mapping.model.PersistentProperty +import spock.lang.Unroll + +class DefaultColumnNameFetcherSpec extends HibernateGormDatastoreSpec { + + @Unroll + void "Test getDefaultColumnName for #description"() { + given: + def namingStrategy =grailsDomainBinder.getNamingStrategyWrapper() + def backticksRemover = new BackticksRemover() + def fetcher = new DefaultColumnNameFetcher(namingStrategy, backticksRemover) + + // Setup related entities that might be needed by the main entity + createPersistentEntity(AssociatedEntity, grailsDomainBinder) + createPersistentEntity(SpecBaseEntity, grailsDomainBinder) + createPersistentEntity(AManyToManyEntity, grailsDomainBinder) // Add the new clean + createPersistentEntity(BManyToManyEntity, grailsDomainBinder) // A// entity + createPersistentEntity(DefaultColumnNameFetcherSpecEntity, grailsDomainBinder) + createPersistentEntity(InheritedEntity, grailsDomainBinder) + + def persistentEntity = createPersistentEntity(entityClass, grailsDomainBinder) + PersistentProperty property = persistentEntity.getPropertyByName(propertyName) + + + when: + String columnName = fetcher.getDefaultColumnName(property) + + then: + columnName == expectedColumnName + + where: + description | entityClass | propertyName | expectedColumnName + "a simple property" | DefaultColumnNameFetcherSpecEntity | "name" | "name" + "a unidirectional one-to-many" | DefaultColumnNameFetcherSpecUnidirectionalOwner | "children" | "org_grails_orm_hibernate_cfg_domainbinding_default_column_name_fetcher_spec_unidirectional_owner_children_id" + "a bidirectional many-to-one" | DefaultColumnNameFetcherSpecEntity | "bidirectionalManyToOne" | "bidirectional_many_to_one_id" + "a many-to-many" | AManyToManyEntity | "manyToMany" | "amany_to_many_entity_id" + "an inherited bidirectional m-t-o" | InheritedEntity | "bidirectionalManyToOne" | "bidirectional_many_to_one_id" + "a basic collection" | DefaultColumnNameFetcherSpecEntity | "basicCollection" | "default_column_name_fetcher_spec_entity_id" + "a basic collection with type" | DefaultColumnNameFetcherSpecEntity | "basicCollectionWithMapping" | "basic_collection_with_mapping" + } +} + +// --- Test Domain Classes --- + +@Entity +class AssociatedEntity { + static belongsTo = [entity: SpecBaseEntity] +} + +@Entity +class SpecBaseEntity { + AssociatedEntity bidirectionalManyToOne +} + +@Entity +class AManyToManyEntity { + String name + static hasMany = [manyToMany: BManyToManyEntity] +} + + +@Entity +class BManyToManyEntity { + String name + static hasMany = [manyToMany: AManyToManyEntity] +} + +@Entity +class DefaultColumnNameFetcherSpecEntity extends SpecBaseEntity { + String name + List<String> basicCollection + List<String> basicCollectionWithMapping + + static hasMany = [unidirectionalOneToMany: AssociatedEntity] // Point to the clean entity + + static mapping = { + basicCollectionWithMapping type: 'text' + } +} + +@Entity +class InheritedEntity extends SpecBaseEntity { + String anotherProperty +} + +@Entity +class DefaultColumnNameFetcherSpecUnidirectionalChild { + String name +} + +@Entity +class DefaultColumnNameFetcherSpecUnidirectionalOwner { + static hasMany = [children: DefaultColumnNameFetcherSpecUnidirectionalChild] +} diff --git a/grails-data-hibernate6/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/UniqueKeyForColumnsCreatorSpec.groovy b/grails-data-hibernate6/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/UniqueKeyForColumnsCreatorSpec.groovy new file mode 100644 index 0000000000..a10c4bdd0a --- /dev/null +++ b/grails-data-hibernate6/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/UniqueKeyForColumnsCreatorSpec.groovy @@ -0,0 +1,33 @@ +package org.grails.orm.hibernate.cfg.domainbinding + +import org.hibernate.mapping.Column +import org.hibernate.mapping.Table +import org.hibernate.mapping.UniqueKey +import spock.lang.Specification + +class UniqueKeyForColumnsCreatorSpec extends Specification { + + def "Test that createUniqueKeyForColumns adds a unique key to the table"() { + given: + UniqueNameGenerator mockUniqueNameGenerator = Mock() + Table mockTable = Mock() + def creator = new UniqueKeyForColumnsCreator(mockUniqueNameGenerator) + def columns = [new Column("col1"), new Column("col2")] + + when: + creator.createUniqueKeyForColumns(mockTable, columns) + + then: + 1 * mockTable.addUniqueKey({ UniqueKey uk -> + uk.table == mockTable + uk.columns.size() == 2 + // The creator reverses the list + uk.columns.get(0).name == "col2" + uk.columns.get(1).name == "col1" + }) + 1 * mockUniqueNameGenerator.setGeneratedUniqueName({ UniqueKey uk -> + uk.table == mockTable + uk.columns.size() == 2 + }) + } +} diff --git a/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/model/PersistentProperty.java b/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/model/PersistentProperty.java index 30c7f96948..fff567779a 100644 --- a/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/model/PersistentProperty.java +++ b/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/model/PersistentProperty.java @@ -22,6 +22,7 @@ package org.grails.datastore.mapping.model; import org.grails.datastore.mapping.config.Property; import org.grails.datastore.mapping.model.types.Association; import org.grails.datastore.mapping.model.types.Embedded; +import org.grails.datastore.mapping.model.types.ManyToOne; import org.grails.datastore.mapping.model.types.OneToMany; import org.grails.datastore.mapping.model.types.ToOne; import org.grails.datastore.mapping.reflect.EntityReflector; @@ -104,4 +105,11 @@ public interface PersistentProperty<T extends Property> { !(this instanceof Association) && !this.equals(this.getOwner().getIdentity()); } + default boolean isBidirectionalManyToOne() { + if (this instanceof ManyToOne manyToOne) { + return manyToOne.isBidirectional(); + } + return false; + } + }
