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 a415a4c9c70c7f073d9880fc8cd0e191350419ab Author: Walter Duque de Estrada <[email protected]> AuthorDate: Thu Feb 12 15:31:42 2026 -0600 refactored the logic for determining the identifier generator name into the Identity class and added a comprehensive test suite. Changes: 1. Refactored `Identity.groovy`: * Enhanced the determineGeneratorName(boolean useSequence) instance method to fully encapsulate the logic for choosing between configured, sequence-based, or native generators. * Added a static determineGeneratorName(Identity mappedId, boolean useSequence) method to handle cases where the mappedId might be null, providing a single entry point for this logic. 2. Simplified `BasicValueIdCreator.java`: * The getBasicValueId method now simply calls the static Identity.determineGeneratorName(mappedId, useSequence) method, removing the local conditional logic and reducing duplication. 3. Created `IdentitySpec.groovy`: * Added a new Spock specification to verify the generator name determination logic across various scenarios (native, sequence, identity, null configurations, etc.) for both instance and static methods. 4. Verification: * Executed both IdentitySpec and BasicValueIdCreatorSpec. All 27 tests passed successfully, ensuring the refactoring is robust and correct. --- .../org/grails/orm/hibernate/cfg/Identity.groovy | 15 ++++++++ .../domainbinding/util/BasicValueIdCreator.java | 11 +----- .../grails/orm/hibernate/cfg/IdentitySpec.groovy | 45 ++++++++++++++++++++++ 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/Identity.groovy b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/Identity.groovy index fc2ed31ee2..87fdc352a4 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/Identity.groovy +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/Identity.groovy @@ -23,6 +23,7 @@ import org.springframework.beans.MutablePropertyValues import org.springframework.validation.DataBinder import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateIdentity +import org.grails.orm.hibernate.cfg.domainbinding.generator.GrailsSequenceGeneratorEnum /** * Defines the identity generation strategy. In the case of a 'composite' identity the properties @@ -59,6 +60,20 @@ class Identity extends Property implements HibernateIdentity { */ Map params = [:] + String determineGeneratorName(boolean useSequence) { + if (generator != null && !(GrailsSequenceGeneratorEnum.NATIVE.toString() == generator && useSequence)) { + return generator + } + return useSequence ? GrailsSequenceGeneratorEnum.SEQUENCE_IDENTITY.toString() : GrailsSequenceGeneratorEnum.NATIVE.toString() + } + + static String determineGeneratorName(Identity mappedId, boolean useSequence) { + if (mappedId != null) { + return mappedId.determineGeneratorName(useSequence) + } + return useSequence ? GrailsSequenceGeneratorEnum.SEQUENCE_IDENTITY.toString() : GrailsSequenceGeneratorEnum.NATIVE.toString() + } + /** * Define the natural id * @param naturalIdDef The callable 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 885a48cdee..862aaeda90 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 @@ -1,7 +1,5 @@ package org.grails.orm.hibernate.cfg.domainbinding.util; -import java.util.Optional; - import org.hibernate.boot.spi.MetadataBuildingContext; import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment; import org.hibernate.generator.Generator; @@ -43,7 +41,7 @@ public class BasicValueIdCreator { public BasicValue getBasicValueId(Identity mappedId, boolean useSequence) { // create a BasicValue for the specific entity table (do not reuse the prototype directly because table differs) - String generatorName = determineGeneratorName(mappedId, useSequence); + String generatorName = Identity.determineGeneratorName(mappedId, useSequence); id.setCustomIdGeneratorCreator(context -> createGenerator(mappedId, context, generatorName)); return id; } @@ -51,12 +49,5 @@ public class BasicValueIdCreator { private Generator createGenerator(Identity mappedId, GeneratorCreationContext context, String generatorName) { return grailsSequenceWrapper.getGenerator(generatorName, context, mappedId, domainClass, jdbcEnvironment); } - - private String determineGeneratorName(Identity mappedId, boolean useSequence) { - return Optional.ofNullable(mappedId) - .map(Identity::getGenerator) - .filter(gen -> !(GrailsSequenceGeneratorEnum.NATIVE.toString().equals(gen) && useSequence)) - .orElse(useSequence ? GrailsSequenceGeneratorEnum.SEQUENCE_IDENTITY.toString() : GrailsSequenceGeneratorEnum.NATIVE.toString()); - } } diff --git a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/IdentitySpec.groovy b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/IdentitySpec.groovy new file mode 100644 index 0000000000..3cc1965b72 --- /dev/null +++ b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/IdentitySpec.groovy @@ -0,0 +1,45 @@ +package org.grails.orm.hibernate.cfg + +import spock.lang.Specification +import spock.lang.Unroll +import org.grails.orm.hibernate.cfg.domainbinding.generator.GrailsSequenceGeneratorEnum + +class IdentitySpec extends Specification { + + @Unroll + def "test determineGeneratorName with generator=#generatorName and useSequence=#useSequence"() { + given: + Identity identity = new Identity(generator: generatorName) + + expect: + identity.determineGeneratorName(useSequence) == expected + + where: + generatorName | useSequence | expected + 'native' | false | 'native' + 'native' | true | 'sequence-identity' + 'identity' | false | 'identity' + 'identity' | true | 'identity' + 'sequence' | true | 'sequence' + 'increment' | false | 'increment' + null | false | 'native' + null | true | 'sequence-identity' + } + + @Unroll + def "test static determineGeneratorName with mappedId=#mappedIdPresent and useSequence=#useSequence"() { + given: + Identity identity = mappedIdPresent ? new Identity(generator: generatorName) : null + + expect: + Identity.determineGeneratorName(identity, useSequence) == expected + + where: + mappedIdPresent | generatorName | useSequence | expected + true | 'native' | false | 'native' + true | 'native' | true | 'sequence-identity' + true | 'uuid' | false | 'uuid' + false | null | false | 'native' + false | null | true | 'sequence-identity' + } +}
