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/02/14 18:54:06 UTC
(grails-core) 12/18: Refactor BasicValueIdCreator and SimpleIdBinder for improved statelessness and testability.
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 d33fb6b2740316c2d84f0eca29dfe6c1046a0cba
Author: Walter Duque de Estrada <wb...@mac.com>
AuthorDate: Fri Feb 13 23:25:28 2026 -0600
Refactor BasicValueIdCreator and SimpleIdBinder for improved statelessness and testability.
- BasicValueIdCreator: Removed BasicValue from state; it's now passed into getBasicValueId.
- SimpleIdBinder: Updated protected constructor to accept MetadataBuildingContext and JdbcEnvironment, avoiding NPEs in tests.
- Updated binder logic to instantiate BasicValue at the point of use and pass it to the creator.
- Adjusted test specifications to align with new signatures.
---
.../cfg/domainbinding/binder/SimpleIdBinder.java | 14 +++++++++-----
.../cfg/domainbinding/util/BasicValueIdCreator.java | 10 ++--------
.../domainbinding/BasicValueIdCreatorSpec.groovy | 12 ++++++------
.../cfg/domainbinding/SimpleIdBinderSpec.groovy | 21 +++++++++++++++------
4 files changed, 32 insertions(+), 25 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 17137b288f..2fc0853ebd 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
@@ -36,9 +36,13 @@ public class SimpleIdBinder {
this.basicValueIdCreator = null;
}
- protected SimpleIdBinder(BasicValueIdCreator basicValueIdCreator, SimpleValueBinder simpleValueBinder, PropertyBinder propertyBinder) {
- this.metadataBuildingContext = null;
- this.jdbcEnvironment = null;
+ public MetadataBuildingContext getMetadataBuildingContext() {
+ return metadataBuildingContext;
+ }
+
+ protected SimpleIdBinder(MetadataBuildingContext metadataBuildingContext, JdbcEnvironment jdbcEnvironment, BasicValueIdCreator basicValueIdCreator, SimpleValueBinder simpleValueBinder, PropertyBinder propertyBinder) {
+ this.metadataBuildingContext = metadataBuildingContext;
+ this.jdbcEnvironment = jdbcEnvironment;
this.simpleValueBinder = simpleValueBinder;
this.propertyBinder = propertyBinder;
this.basicValueIdCreator = basicValueIdCreator;
@@ -51,8 +55,8 @@ public class SimpleIdBinder {
boolean useSequence = result != null && result.isTablePerConcreteClass();
// create the id value
- BasicValueIdCreator idCreator = this.basicValueIdCreator != null ? this.basicValueIdCreator : new BasicValueIdCreator(metadataBuildingContext, jdbcEnvironment, table);
- BasicValue id = idCreator.getBasicValueId(mappedId, domainClass, useSequence);
+ BasicValueIdCreator idCreator = this.basicValueIdCreator != null ? this.basicValueIdCreator : new BasicValueIdCreator(jdbcEnvironment);
+ BasicValue id = idCreator.getBasicValueId(mappedId, domainClass, new BasicValue(metadataBuildingContext, table), useSequence);
var identifier = domainClass.getIdentity();
if (mappedId != null) {
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 75090e6339..d2c2cbd01d 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
@@ -15,13 +15,9 @@ import org.grails.orm.hibernate.cfg.domainbinding.generator.GrailsSequenceWrappe
public class BasicValueIdCreator {
private final JdbcEnvironment jdbcEnvironment;
- @SuppressWarnings("unused") // kept for tests that want to provide a prototype BasicValue
- private final BasicValue id;
private final GrailsSequenceWrapper grailsSequenceWrapper;
- public BasicValueIdCreator(MetadataBuildingContext metadataBuildingContext, JdbcEnvironment jdbcEnvironment, Table table) {
- // create a prototype BasicValue (table will be set per-entity when creating the actual BasicValue)
- this.id = new BasicValue(metadataBuildingContext, table);
+ public BasicValueIdCreator(JdbcEnvironment jdbcEnvironment) {
this.jdbcEnvironment = jdbcEnvironment;
this.grailsSequenceWrapper = new GrailsSequenceWrapper();
}
@@ -29,15 +25,13 @@ public class BasicValueIdCreator {
protected BasicValueIdCreator(JdbcEnvironment jdbcEnvironment
- , BasicValue prototypeBasicValue
, GrailsSequenceWrapper grailsSequenceWrapper) {
this.jdbcEnvironment = jdbcEnvironment;
- this.id = prototypeBasicValue;
this.grailsSequenceWrapper = grailsSequenceWrapper;
}
- public BasicValue getBasicValueId(Identity mappedId, GrailsHibernatePersistentEntity domainClass, boolean useSequence) {
+ public BasicValue getBasicValueId(Identity mappedId, GrailsHibernatePersistentEntity domainClass, BasicValue id, boolean useSequence) {
// create a BasicValue for the specific entity table (do not reuse the prototype directly because table differs)
String generatorName = Identity.determineGeneratorName(mappedId, useSequence);
id.setCustomIdGeneratorCreator(context -> createGenerator(mappedId, domainClass, context, generatorName));
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 0446618c62..0f92b72999 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
@@ -36,7 +36,7 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
entity.setTable(table)
// Use a real BasicValue to test that the generator creator lambda is correctly set and executable
basicValue = new BasicValue(metadataBuildingContext, table)
- creator = new BasicValueIdCreator(jdbcEnvironment, basicValue, grailsSequenceWrapper)
+ creator = new BasicValueIdCreator(jdbcEnvironment, grailsSequenceWrapper)
}
@Unroll
@@ -49,7 +49,7 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
def context = Mock(GeneratorCreationContext)
when:
- BasicValue id = creator.getBasicValueId(mappedId, domainClass, useSequence)
+ BasicValue id = creator.getBasicValueId(mappedId, domainClass, basicValue, useSequence)
def generatorCreator = id.getCustomIdGeneratorCreator()
Generator generator = generatorCreator.createGenerator(context)
@@ -78,7 +78,7 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
def context = Mock(GeneratorCreationContext)
when:
- BasicValue id = creator.getBasicValueId(null, domainClass, false)
+ BasicValue id = creator.getBasicValueId(null, domainClass, basicValue, false)
def generatorCreator = id.getCustomIdGeneratorCreator()
Generator generator = generatorCreator.createGenerator(context)
@@ -94,7 +94,7 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
def context = Mock(GeneratorCreationContext)
when:
- BasicValue id = creator.getBasicValueId(null, domainClass, true)
+ BasicValue id = creator.getBasicValueId(null, domainClass, basicValue, true)
def generatorCreator = id.getCustomIdGeneratorCreator()
Generator generator = generatorCreator.createGenerator(context)
@@ -112,7 +112,7 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
def context = Mock(GeneratorCreationContext)
when:
- BasicValue id = creator.getBasicValueId(mappedId, domainClass, true)
+ BasicValue id = creator.getBasicValueId(mappedId, domainClass, basicValue, true)
def generatorCreator = id.getCustomIdGeneratorCreator()
Generator generator = generatorCreator.createGenerator(context)
@@ -129,7 +129,7 @@ class BasicValueIdCreatorSpec extends HibernateGormDatastoreSpec {
def context = Mock(GeneratorCreationContext)
when:
- BasicValue id = creator.getBasicValueId(mappedId, domainClass, false)
+ BasicValue id = creator.getBasicValueId(mappedId, domainClass, basicValue, false)
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 5cc6b3115c..9a5b103935 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
@@ -30,20 +30,28 @@ class SimpleIdBinderSpec extends HibernateGormDatastoreSpec {
def simpleIdBinder
def setup() {
- metadataBuildingContext = getGrailsDomainBinder().getMetadataBuildingContext()
- jdbcEnvironment = getGrailsDomainBinder().getJdbcEnvironment()
+ def domainBinder = getGrailsDomainBinder()
+ def metadataCollector = domainBinder.getMetadataBuildingContext().getMetadataCollector()
+ metadataBuildingContext = new org.hibernate.boot.internal.MetadataBuildingContextRootImpl(
+ "default",
+ metadataCollector.getBootstrapContext(),
+ metadataCollector.getMetadataBuildingOptions(),
+ metadataCollector,
+ null
+ )
+ jdbcEnvironment = domainBinder.getJdbcEnvironment()
// Use a Mock for BasicValueIdCreator and return a BasicValue based on the currentTable
basicValueIdCreator = Mock(BasicValueIdCreator)
- basicValueIdCreator.getBasicValueId(_, _, _) >> { Identity id, GrailsHibernatePersistentEntity domainClass, boolean useSeq ->
- new BasicValue(metadataBuildingContext, currentTable)
+ basicValueIdCreator.getBasicValueId(_, _, _, _) >> { Identity id, GrailsHibernatePersistentEntity domainClass, BasicValue val, boolean useSeq ->
+ return val
}
// Mock the collaborators that can be safely mocked
simpleValueBinder = Mock(SimpleValueBinder)
propertyBinder = Spy(PropertyBinder)
- simpleIdBinder = new SimpleIdBinder(basicValueIdCreator, simpleValueBinder, propertyBinder)
+ simpleIdBinder = new SimpleIdBinder(metadataBuildingContext, jdbcEnvironment, basicValueIdCreator, simpleValueBinder, propertyBinder)
}
def "bindSimpleId with identity generator"() {
@@ -114,9 +122,10 @@ class SimpleIdBinderSpec extends HibernateGormDatastoreSpec {
getIdentity() >> Mock(GrailsHibernatePersistentProperty)
}
def rootClass = new RootClass(metadataBuildingContext)
+ def table = Mock(Table)
when:
- simpleIdBinder.bindSimpleId(domainClass, rootClass, new Identity(name: "nonExistent"), rootClass.getTable())
+ simpleIdBinder.bindSimpleId(domainClass, rootClass, new Identity(name: "nonExistent"), table)
then:
thrown(org.hibernate.MappingException)