Merge pull request #15550 from apache/8.0.x-hibernate7-dev

Fixed regressions from Hibernate 5

Author: jdaugherty

AGENTS.md

@@ -51,6 +51,8 @@ export GRADLE_OPTS="-Xms2G -Xmx5G"
 8. **No internal APIs in docs** - Only document public APIs; never reference internal or package-private classes and methods in user-facing documentation
 9. **Test via public APIs** - Tests must exercise behavior through the same APIs an end user calls; never invoke internal implementations, package-private methods, or bypass the public surface directly
 10. **Always review and extend tests** - Review existing unit and functional tests before making changes; every code change must include new or enhanced tests that cover the affected behavior
+11. **Every code touch must update all tests for the changed class** - When a class is modified, find and update every test that covers it — unit, integration, and TCK. Do not leave any existing test out of sync with the new code.
+12. **Clean violations before commit** - Before every automated commit, run `./gradlew clean test aggregateTestFailures --continue` from the root and ensure that `TEST_FAILURES.md` reports no issues and is removed.
 
 ## Available Skills
 
@@ -229,8 +231,9 @@ class MyService { }
 1. **Fork & branch** from the target release branch (e.g., `7.0.x`)
 2. **Run tests** before submitting: `./gradlew build --rerun-tasks`
 3. **Run code style checks**: `./gradlew codeStyle`
-4. **Squash commits** into a single meaningful commit message
-5. **Reference issues** in PR description (e.g., "Fixes #1234")
+4. **Clean style violations**: Before committing, run `./gradlew clean aggregateStyleViolations` from the root and ensure that `CHECKSTYLE_VIOLATIONS.md`, `CODENARC_VIOLATIONS.md`, and `PMD_VIOLATIONS.md` have no issues.
+5. **Squash commits** into a single meaningful commit message
+6. **Reference issues** in PR description (e.g., "Fixes #1234")
 
 ### Review Process
 

build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsTestPlugin.groovy

@@ -50,9 +50,10 @@ class GrailsTestPlugin implements Plugin<Project> {
                 slurper.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false)
                 slurper.setFeature("http://xml.org/sax/features/namespaces", false)
 
-                project.subprojects.each { subproject ->
-                    def testResultsDir = subproject.layout.buildDirectory.dir("test-results").get().asFile
-                    if (testResultsDir.exists()) {
+                def processedDirs = [] as Set
+
+                def processTestResults = { File testResultsDir, String moduleName ->
+                    if (testResultsDir.exists() && processedDirs.add(testResultsDir.absolutePath)) {
                         testResultsDir.eachFileRecurse { file ->
                             if (file.name.endsWith('.xml') && file.name.startsWith('TEST-')) {
                                 try {
@@ -61,7 +62,7 @@ class GrailsTestPlugin implements Plugin<Project> {
                                         if (testcase.failure.size() > 0 || testcase.error.size() > 0) {
                                             def failure = testcase.failure.size() > 0 ? testcase.failure[0] : testcase.error[0]
                                             failures << [
-                                                module: subproject.name,
+                                                module: moduleName,
                                                 className: testcase.@classname.text(),
                                                 testName: testcase.@name.text(),
                                                 message: failure.@message.text() ?: failure.text().take(200),
@@ -77,6 +78,24 @@ class GrailsTestPlugin implements Plugin<Project> {
                     }
                 }
 
+                // 1. All subprojects
+                project.subprojects.each { subproject ->
+                    def testResultsDir = subproject.layout.buildDirectory.dir("test-results").get().asFile
+                    processTestResults(testResultsDir, subproject.name)
+                }
+
+                // 2. Any other directory in root that might have test results (like grails-docs or build-logic)
+                project.layout.projectDirectory.asFile.eachDir { dir ->
+                    if (!dir.name.startsWith('.') && dir.name != 'build' && dir.name != 'node_modules') {
+                        def testResultsDir = new File(dir, "build/test-results")
+                        processTestResults(testResultsDir, dir.name)
+                        
+                        // Also check for results in the directory itself (if it's a build output)
+                        def directTestResultsDir = new File(dir, "test-results")
+                        processTestResults(directTestResultsDir, dir.name)
+                    }
+                }
+
                 writeReport(project, failures)
             }
         }

grails-data-hibernate5/core/src/test/groovy/grails/gorm/specs/WhereQueryOldIssueVerificationSpec.groovy

@@ -18,9 +18,6 @@
  */
 package grails.gorm.tests
 
-import spock.lang.IgnoreIf
-import spock.lang.PendingFeatureIf
-
 import grails.gorm.annotation.Entity
 import grails.gorm.hibernate.HibernateEntity
 import grails.gorm.transactions.Rollback
@@ -160,7 +157,6 @@ class WhereQueryOldIssueVerificationSpec extends Specification {
 
     @Rollback
     @Issue('https://github.com/apache/grails-core/issues/14636')
-    @PendingFeatureIf({ System.getProperty('hibernate5.gorm.suite') })
     def "many-to-many queries with sorting do not throw exception"() {
         given: "users and roles in a many-to-many relationship"
         def role1 = new WqRole(name: "ADMIN").save(flush: true)

grails-data-hibernate7/HIBERNATE7-BINDING.md

@@ -82,7 +82,7 @@ The `bindProperty` method in `GrailsPropertyBinder.java` has been successfully r
 *   **IdentityBinder**: Main coordinator for binding entity identifiers (simple or composite).
 *   **SimpleIdBinder**: Binds simple primary keys.
 *   **CompositeIdBinder**: Binds composite primary keys.
-*   **BasicValueIdCreator**: Factory for creating identifier `Value` objects and their generators.
+*   **BasicValueCreator**: Factory for creating identifier `Value` objects and their generators.
 *   **VersionBinder**: Binds the version property used for optimistic locking.
 *   **NaturalIdentifierBinder**: Binds properties marked as `naturalId`.
 
@@ -163,7 +163,7 @@ The `bindProperty` method in `GrailsPropertyBinder.java` has been successfully r
 
 | Class | Status | Notes |
 | :--- | :--- | :--- |
-| `CollectionSecondPassBinder` | Migrated | Contains TODOs for unidirectional many-to-many. |
+| `CollectionSecondPassBinder` | Migrated | Unidirectional many-to-many support implemented. |
 | `GrailsSecondPass` | Migrated | |
 | `ListSecondPass` | Migrated | |
 | `ListSecondPassBinder` | Migrated | |
@@ -197,7 +197,7 @@ The `bindProperty` method in `GrailsPropertyBinder.java` has been successfully r
 | `TableForManyCalculator` | Migrated | |
 | `UniqueNameGenerator` | Migrated | |
 | `BackticksRemover` | Migrated | |
-| `BasicValueIdCreator` | Migrated | |
+| `BasicValueCreator` | Migrated | |
 
 ## Utility Class Refactoring & Mock Compatibility
 
@@ -216,6 +216,9 @@ The `bindProperty` method in `GrailsPropertyBinder.java` has been successfully r
 
 ## Remaining Known Issues / TODOs
 
-- `CollectionSecondPassBinder`: TODO support unidirectional many-to-many.
 - `GrailsIncrementGenerator`: Reflection hacks for Hibernate 7 (scheduled for removal in Hibernate 8).
-- **Multitenancy & CompositeId:** While many tests are passing, some complex scenarios in `MultiTenancyBidirectionalManyToManySpec` and `GlobalConstraintWithCompositeIdSpec` may still require attention or further validation in a full application context.
+
+## Resolved Issues
+
+- `CollectionSecondPassBinder`: Unidirectional many-to-many support implemented.
+- **Multitenancy & CompositeId:** `MultiTenancyBidirectionalManyToManySpec` and `GlobalConstraintWithCompositeIdSpec` have been fixed and validated.

grails-data-hibernate7/HIBERNATE_7_MIGRATION.md

@@ -156,13 +156,7 @@ class HibernateGormInstanceApi<D> extends GormInstanceApi<D> {
         validateable.skipValidation(true)
 
         try {
-            String idPropertyName = domainClass.identity?.name ?: 'id'
-            Object idVal = InvokerHelper.getProperty(target, idPropertyName)
-            if (idVal == null) {
-                return performPersist(target, shouldFlush)
-            } else {
-                return performMerge(target, shouldFlush)
-            }
+            return performUpsert(target, shouldFlush)
         } finally {
             validateable.skipValidation(false)
         }
@@ -240,14 +234,33 @@ class HibernateGormInstanceApi<D> extends GormInstanceApi<D> {
         return instance
     }
 
+    protected D performUpsert(D target, boolean shouldFlush) {
+        PersistentEntity entity = persistentEntity
+        String idPropertyName = entity.identity?.name ?: 'id'
+        Object idVal = InvokerHelper.getProperty(target, idPropertyName)
+        if (idVal == null) {
+            return performPersist(target, shouldFlush)
+        } else {
+            return performMerge(target, shouldFlush)
+        }
+    }
+
     protected D performMerge(final D target, final boolean flush) {
         hibernateTemplate.execute { Session session ->
-            Object merged = session.merge(target)
+            D merged = (D) session.merge(target)
             session.lock(merged, LockModeType.NONE)
+            // Sync id back immediately so target has an identity
+            String idProp = persistentEntity.identity?.name ?: 'id'
+            InvokerHelper.setProperty(target, idProp, InvokerHelper.getProperty(merged, idProp))
             if (flush) {
                 flushSession session
             }
-            return (D) merged
+            // Sync version after flush so the incremented value is captured
+            PersistentProperty versionProperty = persistentEntity.version
+            if (versionProperty != null) {
+                InvokerHelper.setProperty(target, versionProperty.name, InvokerHelper.getProperty(merged, versionProperty.name))
+            }
+            return target
         }
     }
 

grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormInstanceApi.groovy

@@ -123,6 +123,11 @@ class HibernateGormStaticApi<D> extends GormStaticApi<D> {
         (GormStaticApi<D>) HibernateGormEnhancer.findStaticApi(persistentClass, qualifier)
     }
 
+    @Override
+    D merge(D d) {
+        instanceApi.merge(d)
+    }
+
     @Override
     <T> T withNewSession(Closure<T> callable) {
         if (persistentEntity.isMultiTenant()) {

grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy

@@ -42,7 +42,7 @@ import org.hibernate.MappingException
 
 import org.grails.datastore.mapping.config.Property
 import org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity
-import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateIdentity
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePropertyIdentity
 import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty
 
 /**
@@ -54,7 +54,7 @@ import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentP
 @AutoClone
 @Builder(builderStrategy = SimpleStrategy, prefix = '')
 @CompileStatic
-class CompositeIdentity extends Property implements HibernateIdentity {
+class HibernateCompositeIdentity extends Property implements HibernatePropertyIdentity {
 
     /**
      * The property names that make up the custom identity
@@ -74,7 +74,7 @@ class CompositeIdentity extends Property implements HibernateIdentity {
      * @param naturalIdDef The callable
      * @return This id
      */
-    CompositeIdentity naturalId(@DelegatesTo(NaturalId) Closure naturalIdDef) {
+    HibernateCompositeIdentity naturalId(@DelegatesTo(NaturalId) Closure naturalIdDef) {
         this.natural = new NaturalId()
         naturalIdDef.setDelegate(this.natural)
         naturalIdDef.setResolveStrategy(Closure.DELEGATE_ONLY)

…m/hibernate/cfg/CompositeIdentity.groovy …te/cfg/HibernateCompositeIdentity.groovygrails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/CompositeIdentity.groovy renamed to grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/HibernateCompositeIdentity.groovy grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/CompositeIdentity.groovy renamed to grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/HibernateCompositeIdentity.groovy

@@ -42,7 +42,7 @@ import org.springframework.validation.DataBinder
 
 import org.grails.datastore.mapping.config.Property
 import org.grails.orm.hibernate.cfg.domainbinding.generator.GrailsSequenceGeneratorEnum
-import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateIdentity
+import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePropertyIdentity
 
 /**
  * Defines the identity generation strategy. In the case of a 'composite' identity the properties
@@ -53,7 +53,7 @@ import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateIdentity
  */
 @CompileStatic
 @Builder(builderStrategy = SimpleStrategy, prefix = '')
-class Identity extends Property implements HibernateIdentity {
+class HibernateSimpleIdentity extends Property implements HibernatePropertyIdentity {
 
     /**
      * The generator to use
@@ -92,7 +92,7 @@ class Identity extends Property implements HibernateIdentity {
         return useSequence ? GrailsSequenceGeneratorEnum.SEQUENCE_IDENTITY.toString() : GrailsSequenceGeneratorEnum.NATIVE.toString()
     }
 
-    static String determineGeneratorName(Identity mappedId, boolean useSequence) {
+    static String determineGeneratorName(HibernateSimpleIdentity mappedId, boolean useSequence) {
         if (mappedId != null) {
             return mappedId.determineGeneratorName(useSequence)
         }
@@ -104,7 +104,7 @@ class Identity extends Property implements HibernateIdentity {
      * @param naturalIdDef The callable
      * @return This id
      */
-    Identity naturalId(@DelegatesTo(NaturalId) Closure naturalIdDef) {
+    HibernateSimpleIdentity naturalId(@DelegatesTo(NaturalId) Closure naturalIdDef) {
         this.natural = new NaturalId()
         naturalIdDef.setDelegate(this.natural)
         naturalIdDef.setResolveStrategy(Closure.DELEGATE_ONLY)
@@ -120,8 +120,8 @@ class Identity extends Property implements HibernateIdentity {
      * @param config The configuration
      * @return The new instance
      */
-    static Identity configureNew(@DelegatesTo(Identity) Closure config) {
-        Identity property = new Identity()
+    static HibernateSimpleIdentity configureNew(@DelegatesTo(HibernateSimpleIdentity) Closure config) {
+        HibernateSimpleIdentity property = new HibernateSimpleIdentity()
         return configureExisting(property, config)
     }
 
@@ -131,7 +131,7 @@ class Identity extends Property implements HibernateIdentity {
      * @param config The configuration
      * @return The new instance
      */
-    static Identity configureExisting(Identity property, Map config) {
+    static HibernateSimpleIdentity configureExisting(HibernateSimpleIdentity property, Map config) {
         DataBinder dataBinder = new DataBinder(property)
         dataBinder.bind(new MutablePropertyValues(config))
         return property
@@ -142,7 +142,7 @@ class Identity extends Property implements HibernateIdentity {
      * @param config The configuration
      * @return The new instance
      */
-    static Identity configureExisting(Identity property, @DelegatesTo(Identity) Closure config) {
+    static HibernateSimpleIdentity configureExisting(HibernateSimpleIdentity property, @DelegatesTo(HibernateSimpleIdentity) Closure config) {
         config.setDelegate(property)
         config.setResolveStrategy(Closure.DELEGATE_ONLY)
         config.call()