You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@grails.apache.org by bo...@apache.org on 2026/04/02 15:40:13 UTC
(grails-core) 03/06: refactor: encapsulate identifier resolution in BasicValueIdCreator
This is an automated email from the ASF dual-hosted git repository.
borinquenkid pushed a commit to branch 8.0.x-hibernate7-dev
in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 1a7eb80bc837a449907f1a8f002aa8b518aa9770
Author: Walter Duque de Estrada <wb...@mac.com>
AuthorDate: Wed Apr 1 14:09:37 2026 -0500
refactor: encapsulate identifier resolution in BasicValueIdCreator
- Move identifier property resolution and sequence detection logic from SimpleIdBinder to BasicValueIdCreator.
- Simplify SimpleIdBinder.bindSimpleId by using the new encapsulated methods.
- Update relevant unit tests to verify the refactored logic.
---
.../cfg/domainbinding/binder/SimpleIdBinder.java | 36 +++-------------------
.../domainbinding/util/BasicValueIdCreator.java | 35 ++++++++++++++++++++-
.../domainbinding/BasicValueIdCreatorSpec.groovy | 21 ++++++++++---
.../cfg/domainbinding/SimpleIdBinderSpec.groovy | 6 +++-
4 files changed, 60 insertions(+), 38 deletions(-)
diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/SimpleIdBinder.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/SimpleIdBinder.java
index cb75ead4e7..41a0c0bb91 100644
--- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/SimpleIdBinder.java
+++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/SimpleIdBinder.java
@@ -20,7 +20,6 @@ package org.grails.orm.hibernate.cfg.domainbinding.binder;
import jakarta.annotation.Nonnull;
-import org.hibernate.MappingException;
import org.hibernate.boot.spi.MetadataBuildingContext;
import org.hibernate.mapping.BasicValue;
import org.hibernate.mapping.PrimaryKey;
@@ -28,24 +27,20 @@ import org.hibernate.mapping.Property;
import org.hibernate.mapping.RootClass;
import org.hibernate.mapping.Table;
-import org.grails.datastore.mapping.model.DefaultPropertyMapping;
-import org.grails.datastore.mapping.model.config.GormProperties;
import org.grails.orm.hibernate.cfg.Identity;
-import org.grails.orm.hibernate.cfg.Mapping;
-import org.grails.orm.hibernate.cfg.PropertyConfig;
-import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateIdentityProperty;
import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentEntity;
import org.grails.orm.hibernate.cfg.domainbinding.util.BasicValueIdCreator;
import static org.grails.orm.hibernate.cfg.domainbinding.binder.GrailsDomainBinder.EMPTY_PATH;
+/** The simple id binder class. */
@SuppressWarnings("PMD.DataflowAnomalyAnalysis")
public class SimpleIdBinder {
private final MetadataBuildingContext metadataBuildingContext;
+ private final BasicValueIdCreator basicValueIdCreator;
private final SimpleValueBinder simpleValueBinder;
private final PropertyBinder propertyBinder;
- private final BasicValueIdCreator basicValueIdCreator;
public SimpleIdBinder(
MetadataBuildingContext metadataBuildingContext,
@@ -53,9 +48,9 @@ public class SimpleIdBinder {
SimpleValueBinder simpleValueBinder,
PropertyBinder propertyBinder) {
this.metadataBuildingContext = metadataBuildingContext;
+ this.basicValueIdCreator = basicValueIdCreator;
this.simpleValueBinder = simpleValueBinder;
this.propertyBinder = propertyBinder;
- this.basicValueIdCreator = basicValueIdCreator;
}
public MetadataBuildingContext getMetadataBuildingContext() {
@@ -65,32 +60,11 @@ public class SimpleIdBinder {
public void bindSimpleId(
@Nonnull HibernatePersistentEntity domainClass, RootClass entity, Identity mappedId, Table table) {
- Mapping result = domainClass.getHibernateMappedForm();
- boolean useSequence = result != null && result.isTablePerConcreteClass();
// create the id value
- BasicValue id = basicValueIdCreator.bindBasicValue(table, mappedId, domainClass, useSequence);
+ BasicValue id = basicValueIdCreator.bindBasicValue(table, mappedId, domainClass);
- var identifier = domainClass.getIdentity();
- if (identifier == null) {
- var syntheticId = new HibernateIdentityProperty(
- domainClass, domainClass.getMappingContext(), GormProperties.IDENTITY, Long.class);
- syntheticId.setMapping(new DefaultPropertyMapping<>(domainClass.getMapping(), new PropertyConfig()));
- identifier = syntheticId;
- }
- if (mappedId != null) {
- String propertyName = mappedId.getName();
- if (propertyName != null && !propertyName.equals(domainClass.getName())) {
- var namedIdentityProp = domainClass.getHibernatePropertyByName(propertyName);
- if (namedIdentityProp == null) {
- throw new MappingException(
- "Mapping specifies an identifier property name that doesn't exist [" + propertyName + "]");
- }
- if (!namedIdentityProp.equals(identifier)) {
- identifier = namedIdentityProp;
- }
- }
- }
+ var identifier = basicValueIdCreator.resolveIdentifierProperty(domainClass, mappedId);
Property idProperty = new Property();
idProperty.setName(identifier.getName());
diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/BasicValueIdCreator.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/BasicValueIdCreator.java
index b4887660c0..ba6e451405 100644
--- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/BasicValueIdCreator.java
+++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/BasicValueIdCreator.java
@@ -18,6 +18,7 @@
*/
package org.grails.orm.hibernate.cfg.domainbinding.util;
+import org.hibernate.MappingException;
import org.hibernate.boot.spi.MetadataBuildingContext;
import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
import org.hibernate.generator.Generator;
@@ -25,10 +26,15 @@ import org.hibernate.generator.GeneratorCreationContext;
import org.hibernate.mapping.BasicValue;
import org.hibernate.mapping.Table;
+import org.grails.datastore.mapping.model.DefaultPropertyMapping;
+import org.grails.datastore.mapping.model.config.GormProperties;
import org.grails.orm.hibernate.cfg.Identity;
import org.grails.orm.hibernate.cfg.PersistentEntityNamingStrategy;
+import org.grails.orm.hibernate.cfg.PropertyConfig;
import org.grails.orm.hibernate.cfg.domainbinding.generator.GrailsSequenceWrapper;
import org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity;
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateIdentityProperty;
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty;
/** The basic value id creator class. */
@SuppressWarnings("PMD.DataflowAnomalyAnalysis")
@@ -64,7 +70,9 @@ public class BasicValueIdCreator {
/** Gets the basic value id. */
public BasicValue bindBasicValue(
- Table table, Identity mappedId, GrailsHibernatePersistentEntity domainClass, boolean useSequence) {
+ Table table, Identity mappedId, GrailsHibernatePersistentEntity domainClass) {
+ org.grails.orm.hibernate.cfg.Mapping result = domainClass.getHibernateMappedForm();
+ boolean useSequence = result != null && result.isTablePerConcreteClass();
BasicValue basicValue = new BasicValue(metadataBuildingContext, table);
// create a BasicValue for the specific entity table (do not reuse the prototype directly
// because table differs)
@@ -77,6 +85,31 @@ public class BasicValueIdCreator {
return basicValue;
}
+ public HibernatePersistentProperty resolveIdentifierProperty(
+ GrailsHibernatePersistentEntity domainClass, Identity mappedId) {
+ var identifier = domainClass.getIdentity();
+ if (identifier == null) {
+ var syntheticId = new HibernateIdentityProperty(
+ domainClass, domainClass.getMappingContext(), GormProperties.IDENTITY, Long.class);
+ syntheticId.setMapping(new DefaultPropertyMapping<>(domainClass.getMapping(), new PropertyConfig()));
+ identifier = syntheticId;
+ }
+ if (mappedId != null) {
+ String propertyName = mappedId.getName();
+ if (propertyName != null && !propertyName.equals(domainClass.getName())) {
+ var namedIdentityProp = domainClass.getHibernatePropertyByName(propertyName);
+ if (namedIdentityProp == null) {
+ throw new MappingException(
+ "Mapping specifies an identifier property name that doesn't exist [" + propertyName + "]");
+ }
+ if (!namedIdentityProp.equals(identifier)) {
+ identifier = namedIdentityProp;
+ }
+ }
+ }
+ return identifier;
+ }
+
private Generator createGenerator(
Identity mappedId,
GrailsHibernatePersistentEntity domainClass,
diff --git a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/BasicValueIdCreatorSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/BasicValueIdCreatorSpec.groovy
index 23d43616de..85fab360c4 100644
--- a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/BasicValueIdCreatorSpec.groovy
+++ b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/BasicValueIdCreatorSpec.groovy
@@ -66,12 +66,15 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
given:
Identity mappedId = new Identity()
mappedId.setGenerator(generatorName)
+ def mapping = Mock(org.grails.orm.hibernate.cfg.Mapping)
+ mapping.isTablePerConcreteClass() >> useSequence
def domainClass = Mock(HibernatePersistentEntity)
+ domainClass.getHibernateMappedForm() >> mapping
def mockGenerator = Mock(Generator)
def context = Mock(GeneratorCreationContext)
when:
- BasicValue id = creator.bindBasicValue(table, mappedId, domainClass, useSequence)
+ BasicValue id = creator.bindBasicValue(table, mappedId, domainClass)
def generatorCreator = id.getCustomIdGeneratorCreator()
Generator generator = generatorCreator.createGenerator(context)
@@ -97,10 +100,11 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
given:
def mockGenerator = Mock(Generator)
def domainClass = Mock(HibernatePersistentEntity)
+ domainClass.getHibernateMappedForm() >> null
def context = Mock(GeneratorCreationContext)
when:
- BasicValue id = creator.bindBasicValue(table, null, domainClass, false)
+ BasicValue id = creator.bindBasicValue(table, null, domainClass)
def generatorCreator = id.getCustomIdGeneratorCreator()
Generator generator = generatorCreator.createGenerator(context)
@@ -112,11 +116,14 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
def "should default to sequence-identity when mappedId is null and useSequence is true"() {
given:
def mockGenerator = Mock(Generator)
+ def mapping = Mock(org.grails.orm.hibernate.cfg.Mapping)
+ mapping.isTablePerConcreteClass() >> true
def domainClass = Mock(HibernatePersistentEntity)
+ domainClass.getHibernateMappedForm() >> mapping
def context = Mock(GeneratorCreationContext)
when:
- BasicValue id = creator.bindBasicValue(table, null, domainClass, true)
+ BasicValue id = creator.bindBasicValue(table, null, domainClass)
def generatorCreator = id.getCustomIdGeneratorCreator()
Generator generator = generatorCreator.createGenerator(context)
@@ -130,11 +137,14 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
Identity mappedId = new Identity()
mappedId.setGenerator(GrailsSequenceGeneratorEnum.NATIVE.toString())
def mockGenerator = Mock(Generator)
+ def mapping = Mock(org.grails.orm.hibernate.cfg.Mapping)
+ mapping.isTablePerConcreteClass() >> true
def domainClass = Mock(HibernatePersistentEntity)
+ domainClass.getHibernateMappedForm() >> mapping
def context = Mock(GeneratorCreationContext)
when:
- BasicValue id = creator.bindBasicValue(table, mappedId, domainClass, true)
+ BasicValue id = creator.bindBasicValue(table, mappedId, domainClass)
def generatorCreator = id.getCustomIdGeneratorCreator()
Generator generator = generatorCreator.createGenerator(context)
@@ -148,10 +158,11 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
Identity mappedId = new Identity()
mappedId.setGenerator("custom")
def domainClass = Mock(HibernatePersistentEntity)
+ domainClass.getHibernateMappedForm() >> null
def context = Mock(GeneratorCreationContext)
when:
- BasicValue id = creator.bindBasicValue(table, mappedId, domainClass, false)
+ BasicValue id = creator.bindBasicValue(table, mappedId, domainClass)
def generatorCreator = id.getCustomIdGeneratorCreator()
generatorCreator.createGenerator(context)
diff --git a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/SimpleIdBinderSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/SimpleIdBinderSpec.groovy
index 9181ec4ec7..5f56de7568 100644
--- a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/SimpleIdBinderSpec.groovy
+++ b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/SimpleIdBinderSpec.groovy
@@ -62,9 +62,12 @@ class SimpleIdBinderSpec extends HibernateGormDatastoreSpec {
// Use a Mock for BasicValueIdCreator and return a BasicValue based on the currentTable
basicValueIdCreator = Mock(BasicValueIdCreator)
- basicValueIdCreator.bindBasicValue(_, _, _, _) >> { Table table, Identity id, HibernatePersistentEntity domainClass, boolean useSeq ->
+ basicValueIdCreator.bindBasicValue(_, _, _) >> { Table table, Identity id, HibernatePersistentEntity domainClass ->
return new BasicValue(metadataBuildingContext, table)
}
+ basicValueIdCreator.resolveIdentifierProperty(_, _) >> { HibernatePersistentEntity domainClass, Identity mappedId ->
+ return domainClass.getIdentity() ?: Mock(HibernatePersistentProperty) { getName() >> "id" }
+ }
// Mock the collaborators that can be safely mocked
simpleValueBinder = Mock(SimpleValueBinder)
@@ -147,6 +150,7 @@ class SimpleIdBinderSpec extends HibernateGormDatastoreSpec {
simpleIdBinder.bindSimpleId(domainClass, rootClass, new Identity(name: "nonExistent"), table)
then:
+ 1 * basicValueIdCreator.resolveIdentifierProperty(domainClass, _) >> { throw new org.hibernate.MappingException("Mapping specifies an identifier property name that doesn't exist [nonExistent]") }
thrown(org.hibernate.MappingException)
}