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 a8e7a01f1aa269b2c380f1bccea4b691d8e2beb2 Author: Walter Duque de Estrada <[email protected]> AuthorDate: Thu Feb 12 20:52:42 2026 -0600 Refactor GormColumnSnapshotGenerator for better readability and testability. - Extract findPersistentClass, isIdentifier, resolveGormProperty helpers. - Extract applyGormIdentitySettings and applyGormPropertySettings to modularize column mutation. - Flatten snapshot method to use guard clauses and reduce nesting. - Explicitly set identifier columns as non-nullable. --- .../orm/hibernate/cfg/GrailsDomainBinder.java | 3 +- .../domainbinding/binder/GrailsPropertyBinder.java | 29 ++-- .../liquibase/GormColumnSnapshotGenerator.groovy | 184 ++++++++++----------- 3 files changed, 106 insertions(+), 110 deletions(-) diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java index cb85986557..9785aba27e 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/GrailsDomainBinder.java @@ -578,8 +578,7 @@ public class GrailsDomainBinder for (GrailsHibernatePersistentProperty currentGrailsProp : domainClass.getPersistentPropertiesToBind()) { - var value = grailsPropertyBinder.bindProperty(persistentClass, currentGrailsProp, mappings, sessionFactoryBeanName); - persistentClass.addProperty(propertyFromValueCreator.createProperty(value, currentGrailsProp)); + grailsPropertyBinder.bindProperty(persistentClass, currentGrailsProp, mappings, sessionFactoryBeanName); } new NaturalIdentifierBinder().bindNaturalIdentifier(domainClass.getMappedForm(), persistentClass); diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/GrailsPropertyBinder.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/GrailsPropertyBinder.java index e81750ece2..e99c64bbdf 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/GrailsPropertyBinder.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/GrailsPropertyBinder.java @@ -20,6 +20,7 @@ import org.hibernate.mapping.Component; import org.hibernate.mapping.ManyToOne; import org.hibernate.mapping.OneToOne; import org.hibernate.mapping.PersistentClass; +import org.hibernate.mapping.Property; import org.hibernate.mapping.SimpleValue; import org.hibernate.mapping.Table; import org.hibernate.mapping.Value; @@ -41,6 +42,7 @@ public class GrailsPropertyBinder { private final ColumnNameForPropertyAndPathFetcher columnNameForPropertyAndPathFetcher; private final OneToOneBinder oneToOneBinder; private final ManyToOneBinder manyToOneBinder; + private final PropertyFromValueCreator propertyFromValueCreator; public GrailsPropertyBinder( MetadataBuildingContext metadataBuildingContext, @@ -59,7 +61,8 @@ public class GrailsPropertyBinder { new SimpleValueBinder(namingStrategy), new ColumnNameForPropertyAndPathFetcher(namingStrategy), new OneToOneBinder(namingStrategy), - new ManyToOneBinder(namingStrategy)); + new ManyToOneBinder(namingStrategy), + propertyFromValueCreator); } protected GrailsPropertyBinder( @@ -72,7 +75,8 @@ public class GrailsPropertyBinder { SimpleValueBinder simpleValueBinder, ColumnNameForPropertyAndPathFetcher columnNameForPropertyAndPathFetcher, OneToOneBinder oneToOneBinder, - ManyToOneBinder manyToOneBinder) { + ManyToOneBinder manyToOneBinder, + PropertyFromValueCreator propertyFromValueCreator) { this.metadataBuildingContext = metadataBuildingContext; this.collectionHolder = collectionHolder; this.enumTypeBinder = enumTypeBinder; @@ -82,9 +86,10 @@ public class GrailsPropertyBinder { this.columnNameForPropertyAndPathFetcher = columnNameForPropertyAndPathFetcher; this.oneToOneBinder = oneToOneBinder; this.manyToOneBinder = manyToOneBinder; + this.propertyFromValueCreator = propertyFromValueCreator; } - public Value bindProperty(PersistentClass persistentClass + public void bindProperty(PersistentClass persistentClass , @Nonnull GrailsHibernatePersistentProperty currentGrailsProp , @Nonnull InFlightMetadataCollector mappings , String sessionFactoryBeanName) { @@ -104,41 +109,35 @@ public class GrailsPropertyBinder { // 1. Create Value and apply binders (consolidated block) if (currentGrailsProp.isUserButNotCollectionType()) { value = new BasicValue(metadataBuildingContext, table); - // No specific binder call needed for this case per original logic simpleValueBinder.bindSimpleValue(currentGrailsProp, null,(SimpleValue) value, EMPTY_PATH); } else if (collectionType != null) { if (currentGrailsProp.isSerializableType()) { value = new BasicValue(metadataBuildingContext, table); - simpleValueBinder.bindSimpleValue(currentGrailsProp, null,(SimpleValue) value, EMPTY_PATH);// No specific binder call needed + simpleValueBinder.bindSimpleValue(currentGrailsProp, null,(SimpleValue) value, EMPTY_PATH); } else { // Actual Collection Collection collection = collectionType.create((HibernateToManyProperty) currentGrailsProp, persistentClass); collectionBinder.bindCollection((HibernateToManyProperty) currentGrailsProp, collection, persistentClass, mappings, EMPTY_PATH, sessionFactoryBeanName); mappings.addCollectionBinding(collection); value = collection; - // No specific binder for Collection itself in Block 2 originally. } } else if (currentGrailsProp.getType().isEnum()) { value = new BasicValue(metadataBuildingContext, table); - // Apply enumTypeBinder if the created value is a SimpleValue SimpleValue simpleValue = (SimpleValue) value; String columnName = columnNameForPropertyAndPathFetcher.getColumnNameForPropertyAndPath(currentGrailsProp, EMPTY_PATH, null); enumTypeBinder.bindEnumType(currentGrailsProp, currentGrailsProp.getType(), simpleValue, columnName); } else if (currentGrailsProp.isHibernateOneToOne()) { value = new OneToOne(metadataBuildingContext, table, persistentClass); - // Apply OneToOneBinder logic oneToOneBinder.bindOneToOne((org.grails.datastore.mapping.model.types.OneToOne)currentGrailsProp, (OneToOne)value, EMPTY_PATH); } else if(currentGrailsProp.isHibernateManyToOne()) { value = new ManyToOne(metadataBuildingContext, table); - // Apply ManyToOneBinder logic manyToOneBinder.bindManyToOne((Association)currentGrailsProp, (ManyToOne)value, EMPTY_PATH); } else if (currentGrailsProp instanceof Embedded) { value = new Component(metadataBuildingContext, persistentClass); - // Apply ComponentPropertyBinder logic componentPropertyBinder.bindComponent((Component)value, (Embedded)currentGrailsProp, true, mappings, sessionFactoryBeanName); } // work out what type of relationship it is and bind value @@ -147,9 +146,9 @@ public class GrailsPropertyBinder { simpleValueBinder.bindSimpleValue(currentGrailsProp, null,(SimpleValue) value, EMPTY_PATH); } - // After creating the value and applying binders (where applicable), create and add the property. - // This is now done once at the end of the consolidated block. - - return value; + if (value != null) { + Property property = propertyFromValueCreator.createProperty(value, currentGrailsProp); + persistentClass.addProperty(property); + } } -} \ No newline at end of file +} diff --git a/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/liquibase/GormColumnSnapshotGenerator.groovy b/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/liquibase/GormColumnSnapshotGenerator.groovy index 1eb8ba32e9..349e0b8f8a 100644 --- a/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/liquibase/GormColumnSnapshotGenerator.groovy +++ b/grails-data-hibernate7/dbmigration/src/main/groovy/org/grails/plugins/databasemigration/liquibase/GormColumnSnapshotGenerator.groovy @@ -29,6 +29,7 @@ import liquibase.structure.core.Relation import org.grails.datastore.mapping.model.MappingContext import org.grails.datastore.mapping.model.PersistentEntity import org.grails.datastore.mapping.model.PersistentProperty +import org.grails.datastore.mapping.model.types.Association import org.grails.orm.hibernate.cfg.GrailsHibernatePersistentEntity import org.grails.orm.hibernate.cfg.GrailsHibernatePersistentProperty import org.grails.orm.hibernate.cfg.Mapping @@ -64,101 +65,98 @@ class GormColumnSnapshotGenerator implements SnapshotGenerator { public <T extends DatabaseObject> T snapshot(T example, DatabaseSnapshot snapshot, SnapshotGeneratorChain chain) { T snapshotObject = chain.snapshot(example, snapshot) - if (snapshotObject instanceof Column && snapshot.getDatabase() instanceof GormDatabase) { - Column column = (Column) snapshotObject - - Relation relation = column.getRelation() - if (relation == null) return snapshotObject - String tableName = relation.getName() - if (tableName == null) return snapshotObject - - GormDatabase gormDb = (GormDatabase) snapshot.getDatabase() - def gormDatastore = gormDb.getGormDatastore() - if (gormDatastore == null) return snapshotObject - - MappingContext mappingContext = gormDatastore.getMappingContext() - Metadata metadata = gormDb.getMetadata() - if (metadata == null) return snapshotObject - - for (PersistentClass pc : metadata.getEntityBindings()) { - if (pc instanceof RootClass) { - RootClass root = (RootClass) pc - if (tableName.equalsIgnoreCase(root.getTable()?.getName())) { - org.hibernate.mapping.Column hibernateColumn = null - for (org.hibernate.mapping.Column hc : root.getTable().getColumns()) { - if (hc.getName().equalsIgnoreCase(column.getName())) { - hibernateColumn = hc - break - } - } - - if (hibernateColumn != null) { - PersistentEntity entity = mappingContext.getPersistentEntity(pc.getClassName() ?: pc.getEntityName()) - if (entity instanceof GrailsHibernatePersistentEntity) { - GrailsHibernatePersistentEntity gpe = (GrailsHibernatePersistentEntity) entity - - // 1. Check if it is an ID column - boolean isIdColumn = false - if (root.getIdentifier() instanceof SimpleValue) { - SimpleValue sv = (SimpleValue) root.getIdentifier() - for (Selectable s : sv.getColumns()) { - if (s instanceof org.hibernate.mapping.Column) { - if (s.getName().equalsIgnoreCase(column.getName())) { - isIdColumn = true - break - } - } - } - } - - if (isIdColumn) { - // Always set identifiers as non-nullable - column.setNullable(false) - - Mapping m = gpe.getMappedForm() - Object idMapping = m.getIdentity() - if (idMapping instanceof Identity) { - Identity identity = (Identity) idMapping - boolean useSequence = m.isTablePerConcreteClass() - String strategy = identity.determineGeneratorName(useSequence) - if ("identity" == strategy || "native" == strategy || "sequence-identity" == strategy) { - column.setAutoIncrementInformation(new Column.AutoIncrementInformation()) - } - } - } else { - // 2. Fix nullability for non-ID columns using GORM metadata - if (column.isNullable() == null || column.isNullable()) { - boolean gormNullable = true - for (PersistentProperty prop : gpe.getPersistentProperties()) { - String propColumnName = null - if (prop instanceof GrailsHibernatePersistentProperty) { - propColumnName = ((GrailsHibernatePersistentProperty) prop).getMappedColumnName() - } - if (propColumnName == null) { - // Default naming convention - propColumnName = prop.getName() - if (prop instanceof org.grails.datastore.mapping.model.types.Association) { - propColumnName += "_id" - } - } - - if (column.getName().equalsIgnoreCase(propColumnName)) { - gormNullable = prop.isNullable() - break - } - } - - if (!gormNullable) { - column.setNullable(false) - } - } - } - } - } - } - } + if (!(snapshotObject instanceof Column) || !(snapshot.database instanceof GormDatabase)) { + return snapshotObject + } + + Column column = (Column) snapshotObject + String tableName = column.relation?.name + if (!tableName) return snapshotObject + + GormDatabase gormDb = (GormDatabase) snapshot.database + def gormDatastore = gormDb.gormDatastore + if (!gormDatastore) return snapshotObject + + PersistentClass pc = findPersistentClass(gormDb.metadata, tableName) + if (!pc) return snapshotObject + + MappingContext mappingContext = gormDatastore.mappingContext + PersistentEntity entity = mappingContext.getPersistentEntity(pc.className ?: pc.entityName) + if (!(entity instanceof GrailsHibernatePersistentEntity)) return snapshotObject + + GrailsHibernatePersistentEntity gpe = (GrailsHibernatePersistentEntity) entity + + if (isIdentifier(pc, column.name)) { + applyGormIdentitySettings(column, gpe) + } else { + PersistentProperty prop = resolveGormProperty(gpe, column.name) + if (prop) { + applyGormPropertySettings(column, prop) } } + return snapshotObject } + + protected PersistentClass findPersistentClass(Metadata metadata, String tableName) { + for (PersistentClass pc : metadata.entityBindings) { + if (tableName.equalsIgnoreCase(pc.table?.name)) { + return pc + } + } + return null + } + + protected boolean isIdentifier(PersistentClass pc, String columnName) { + if (!(pc instanceof RootClass)) return false + RootClass root = (RootClass) pc + if (!(root.identifier instanceof SimpleValue)) return false + SimpleValue sv = (SimpleValue) root.identifier + return sv.columns.any { Selectable s -> + s instanceof org.hibernate.mapping.Column && s.name.equalsIgnoreCase(columnName) + } + } + + protected PersistentProperty resolveGormProperty(GrailsHibernatePersistentEntity gpe, String columnName) { + for (PersistentProperty prop : gpe.persistentProperties) { + String propColumnName = null + if (prop instanceof GrailsHibernatePersistentProperty) { + propColumnName = ((GrailsHibernatePersistentProperty) prop).mappedColumnName + } + if (propColumnName == null) { + propColumnName = prop.name + if (prop instanceof Association) { + propColumnName += "_id" + } + } + if (columnName.equalsIgnoreCase(propColumnName)) { + return prop + } + } + return null + } + + protected void applyGormIdentitySettings(Column column, GrailsHibernatePersistentEntity gpe) { + // Always set identifiers as non-nullable + column.setNullable(false) + + Mapping m = gpe.mappedForm + Object idMapping = m.identity + if (idMapping instanceof Identity) { + Identity identity = (Identity) idMapping + boolean useSequence = m.isTablePerConcreteClass() + String strategy = identity.determineGeneratorName(useSequence) + if (strategy == "identity" || strategy == "native" || strategy == "sequence-identity") { + column.setAutoIncrementInformation(new Column.AutoIncrementInformation()) + } + } + } + + protected void applyGormPropertySettings(Column column, PersistentProperty prop) { + if (column.isNullable() == null || column.isNullable()) { + if (!prop.isNullable()) { + column.setNullable(false) + } + } + } }
