Hibernate 7 - Step 1 by jdaugherty · Pull Request #15654 · apache/grails-core

jdaugherty · 2026-05-11T21:06:39Z

creating this request by cloning the existing hibernate five modules so that we can easily review the hibernate seven changes between hibernate five and seven

Copy all source, test examples, BOMs, and build config from the
hibernate5 namespace to hibernate7 so that the real hibernate7 PR
can be reviewed as a true delta rather than a sea of new files.

Modules added:

grails-hibernate7-bom (copy of grails-hibernate5-bom)
grails-data-hibernate7-core, spring-orm, grails-plugin, dbmigration, spring-boot, docs
grails-test-examples/hibernate7 (12 projects mirroring hibernate5)
gradle/hibernate7-test-config.gradle (skipHibernate7Tests flag)

Build infrastructure:

publish-root-config.gradle: register hibernate7 modules for publishing
SbomPlugin.groovy: add LGPL exemptions for hibernate5 artifacts used by hibernate7 staging modules
settings.gradle: include all hibernate7 projects

Copy all source, test examples, BOMs, and build config from the hibernate5 namespace to hibernate7 so that the real hibernate7 PR can be reviewed as a true delta rather than a sea of new files. Modules added: - grails-hibernate7-bom (copy of grails-hibernate5-bom) - grails-data-hibernate7-core, spring-orm, grails-plugin, dbmigration, spring-boot, docs - grails-test-examples/hibernate7 (12 projects mirroring hibernate5) - gradle/hibernate7-test-config.gradle (skipHibernate7Tests flag) Build infrastructure: - publish-root-config.gradle: register hibernate7 modules for publishing - SbomPlugin.groovy: add LGPL exemptions for hibernate5 artifacts used by hibernate7 staging modules - settings.gradle: include all hibernate7 projects

jdaugherty · 2026-05-12T01:40:51Z

This PR is #1 for the hibernate PR. Basically, I cloned the hibernate5 projects and then created this PR as if they were the hibernate 7 artifacts. This then allows the actual hibernate PR to be meaningful since we can better see what changed. It's not perfect, but it did cut out about 400+ files to review this way.

jdaugherty · 2026-05-12T01:45:42Z

I went ahead and renamed hibernate6.gorm.suite to hibernate7.gorm.suite in this initial PR since otherwise the tests will fail that are ignored.

matrei

Thank you @jdaugherty !

There are some TCK tests that are failing because they only have
@IgnoreIf({ System.getProperty('hibernate5.gorm.suite') })
and not
@IgnoreIf({ System.getProperty('hibernate7.gorm.suite') })

jdaugherty · 2026-05-12T14:17:46Z

Per discussion, I've pulled forward the addAllDomainClasses, the common base spec for mongo, & the package renames from tests -> specs

matrei · 2026-05-13T11:51:00Z

Should commit 06fe15d be on this branch?

jdaugherty · 2026-05-13T18:46:44Z

@matrei i had removed it from this branch since it's to fix the datastore changes in the other branch.

jdaugherty · 2026-05-13T18:47:56Z

@matrei Actually, it looks like it got readded somehow, let me try to take it off again

jdaugherty · 2026-05-13T18:59:07Z

@matrei I believe I have removed this now.

jdaugherty · 2026-05-17T15:42:09Z

I've added tests to show how to extend the criteria & hibernate criteria builder.
I've pulled forward the test lens fix
code style is no longer a kitchen sink / it's been split up properly

jdaugherty · 2026-05-17T15:42:28Z

@jamesfredley @borinquenkid @matrei assuming the tests all pass, this is ready to review now

jamesfredley · 2026-05-20T15:05:49Z

@sbglasius @codeconsole @borinquenkid @matrei I think this step 1 base PR for Hibernate 7.2 is ready to merge, which sets us up to dive deeper into the final reviews of #15568. We need 1 more approver.

matrei · 2026-05-21T06:07:50Z

+        if (closures == null) {
+            return new PromiseList<T>()
+        }


What happened here?

matrei · 2026-05-21T06:09:27Z

+        if (successCallbacks != null) {
+            synchronized (successCallbacks) {
+                for (FutureTaskChildPromise callback : successCallbacks) {
+                    callback.accept(t)
+                }


Why was this changed?

matrei · 2026-05-21T06:09:41Z

+        if (failureCallbacks != null) {
+            synchronized (failureCallbacks) {
+                for (FutureTaskChildPromise callback : failureCallbacks) {
+                    callback.accept(t)
+                }


Why was this changed?

matrei · 2026-05-21T06:10:32Z

                    ClassNode promiseNode = ClassHelper.make(Promise.class).getPlainNodeReference();
                    ClassNode originalReturnType = m.getReturnType();
-                    if (!originalReturnType.getNameWithoutPackage().equals(VOID)) {
+                    if (!VOID.equals(originalReturnType.getNameWithoutPackage())) {


Why was this changed?

matrei · 2026-05-21T06:10:51Z

            Class<?> transformerClass = getClass().getClassLoader().loadClass("org.grails.async.transform.internal.DefaultDelegateAsyncTransactionalMethodTransformer");
            return (DelegateAsyncTransactionalMethodTransformer) transformerClass.getDeclaredConstructor().newInstance();
-        } catch (Throwable e) {
+        } catch (Exception ignored) {


Why was this changed?

One explanation is that it's bad coding style to catch Throwables. And ignored is an IntelliJ thing 🤔

matrei · 2026-05-21T06:19:47Z

 import org.junit.runners.Suite
 import org.junit.runner.RunWith
-import grails.gorm.tests.*
+import grails.gorm.specs.*


Why was this changed?

agreed. It's noise

matrei · 2026-05-21T06:30:44Z

 def CONSOLE_LOG_PATTERN = '%d{HH:mm:ss.SSS} [%t] %highlight(%p) %cyan(\\(%logger{39}\\)) %m%n'

 appender('STDOUT', ConsoleAppender) {
+    follow = true


This is redundant now?

Is logback.groovy even supported?

matrei · 2026-05-21T06:47:03Z

I think we should revert any changes that are not directly related to Hibernate 7 compatibility.

Any other changes that we still want to keep should be split out into separate, focused PRs targeting the appropriate version branch.

If we, as a project, discuss and agree that we want to add PMD and Jacoco, those should also be introduced in separate PRs.

That’s my two cents.

sbglasius

This is an approval with a "Why?" and a caveat.

Why are there so many unrelated changes. It makes it impossible to get through all files.

The caveat is, that I have assumed, that all files in grails-data-hibernate7 are a plain copy of that in grails-data-hibernate5

Looking through each and every single file in this humongous PR would take more than the couple of hours that I have already spent on it.

There are none of my comments that requires code changes, they are more there out of frustration.

You can accept my approval if you feel it's valid.

sbglasius · 2026-05-21T07:23:00Z

            Class<?> transformerClass = getClass().getClassLoader().loadClass("org.grails.async.transform.internal.DefaultDelegateAsyncTransactionalMethodTransformer");
            return (DelegateAsyncTransactionalMethodTransformer) transformerClass.getDeclaredConstructor().newInstance();
-        } catch (Throwable e) {
+        } catch (Exception ignored) {


One explanation is that it's bad coding style to catch Throwables. And ignored is an IntelliJ thing 🤔

sbglasius · 2026-05-21T07:24:32Z

    id 'org.apache.grails.buildsrc.sbom'
    id 'org.apache.grails.gradle.grails-code-style'
+    id 'org.apache.grails.gradle.grails-code-analysis'
+    id 'org.apache.grails.gradle.grails-jacoco'


I'm commenting on this once: Why did this have to go into this PR. It's all noise making it almost impossible to review.

sbglasius · 2026-05-21T07:25:04Z


-import grails.persistence.support.PersistenceContextInterceptorExecutor
 import grails.async.decorator.PromiseDecorator
+import grails.persistence.support.PersistenceContextInterceptorExecutor


Why the reformat in this PR.

sbglasius · 2026-05-21T07:25:43Z

 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.function.Consumer

+import groovy.transform.CompileStatic


Import order noise unrelated to this PR changes.

sbglasius · 2026-05-21T07:27:43Z

 import org.junit.runners.Suite
 import org.junit.runner.RunWith
-import grails.gorm.tests.*
+import grails.gorm.specs.*


agreed. It's noise

sbglasius · 2026-05-21T07:38:54Z

+ * under the License.
+ */
+// See http://logback.qos.ch/manual/groovy.html for details on configuration
+def CONSOLE_LOG_PATTERN = '%d{HH:mm:ss.SSS} [%t] %highlight(%p) %cyan(\\(%logger{39}\\)) %m%n'


Is logback.groovy even supported?

sbglasius · 2026-05-21T07:44:33Z


-class MongoCascadeSpec extends GrailsDataTckSpec<GrailsDataMongoTckManager> {
+class MongoCascadeSpec extends MongoDatastoreSpec {


Was a refactor of GrailsDataTckSpec<GrailsDataMongoTckManager> to MongoDatastoreSpec needed? If yes, ok, if no, then it's just another source of noise.

sbglasius · 2026-05-21T07:44:52Z

 import org.grails.datastore.mapping.query.Query
+import org.slf4j.LoggerFactory


Import re-order noise

sbglasius · 2026-05-21T07:45:26Z

+
+import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
+
+abstract class MongoDatastoreSpec extends GrailsDataTckSpec<GrailsDataMongoTckManager> {


This refactor should have been done in a later PR. Adding to the size of the PR.

sbglasius · 2026-05-21T07:53:13Z

    void 'Test the list method returns a PagedResultList with pagination arguments'() {
        given: 'A bunch of people'
        createPeople()

        when: 'A detached criteria instance is created and the list method used with the max parameter'
        def criteria = new DetachedCriteria(Person)
        criteria.with {
-            eq('lastName', 'Simpson')
+            eq 'lastName', 'Simpson'


This is an unneeded change for this PR. Removing parentheses should have been done in a clean-up PR.

github-project-automation Bot added this to Apache Grails May 11, 2026

jdaugherty added 2 commits May 11, 2026 19:53

Add missing hibernate7 test flags

4084938

rename spring orm source from hibernate5 -> hibernate7

ec859bf

jdaugherty requested a review from jamesfredley May 12, 2026 01:40

jdaugherty mentioned this pull request May 12, 2026

Hibernate 7 - Step 2 #15568

Open

hibernate6.gorm.suite -> hibernate7.gorm.suite

214a291

jdaugherty changed the title ~~stage: clone hibernate5 modules to hibernate7 as baseline~~ Hibernate 7 - Step 1 May 12, 2026

matrei requested changes May 12, 2026

View reviewed changes

Comment thread .../spring-orm/src/main/java/org/grails/orm/hibernate/support/hibernate7/HibernateTemplate.java Outdated

Comment thread ...n/java/org/grails/orm/hibernate/support/hibernate7/support/OpenSessionInViewInterceptor.java Outdated

jdaugherty added 4 commits May 12, 2026 08:52

Add missing checks for hibernate7.gorm.suite

5ac6e2c

Remove import / switch back to javadoc

caa32c4

Remove import / switch back to javadoc

5112d89

Pull forward manager.addAllDomainClasses change

ed0916d

matrei reviewed May 12, 2026

View reviewed changes

Comment thread ...tamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/ValidationSpec.groovy Outdated

jdaugherty added 2 commits May 12, 2026 09:53

Pull forward package changes

01c27f9

Pull forward base tck test

c8cb43f

fix codenarc violations

f18465a

jdaugherty requested a review from borinquenkid May 12, 2026 14:32

jdaugherty force-pushed the 8.0.x-stage-hibernate7 branch from 06fe15d to f18465a Compare May 12, 2026 15:20

Fix import

4c5c563

jdaugherty force-pushed the 8.0.x-stage-hibernate7 branch from 6783934 to 4c5c563 Compare May 13, 2026 18:58

jamesfredley reviewed May 13, 2026

View reviewed changes

Comment thread gradle/functional-test-config.gradle Outdated

Fix negative check

fbe6d04

jdaugherty requested review from jamesfredley and matrei May 17, 2026 16:34

Fix bad merge

263f7ed

jdaugherty force-pushed the 8.0.x-stage-hibernate7 branch from 26950bc to 263f7ed Compare May 17, 2026 17:08

jdaugherty added 5 commits May 17, 2026 13:42

Remove codememory skill reference for now

caeb724

Remove slf4j-simple

4eca4ca

Copy hibernate5 test to hibernate7

ada5f2a

Copy hibernate5 test to hibernate7

3fa03ac

Copy hibernate5 test to hibernate7

b04a50f

This comment has been minimized.

Sign in to view

jdaugherty mentioned this pull request May 18, 2026

Hibernate 7.x.x support #14956

Open

jamesfredley approved these changes May 20, 2026

View reviewed changes

Merge branch '8.0.x' into 8.0.x-stage-hibernate7

1c61325

jamesfredley requested review from codeconsole and sbglasius May 20, 2026 14:48

jamesfredley moved this to In Progress in Apache Grails May 20, 2026

jamesfredley added this to the grails:8.0.0-M2 milestone May 20, 2026

jamesfredley mentioned this pull request May 20, 2026

Groovy 5.0.x support for Grails 8 + Spring Boot 4 #15557

Draft

matrei reviewed May 21, 2026

View reviewed changes

sbglasius approved these changes May 21, 2026

View reviewed changes


		class MongoCascadeSpec extends GrailsDataTckSpec<GrailsDataMongoTckManager> {
		class MongoCascadeSpec extends MongoDatastoreSpec {

		import org.grails.datastore.mapping.query.Query
		import org.slf4j.LoggerFactory


		import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec

		abstract class MongoDatastoreSpec extends GrailsDataTckSpec<GrailsDataMongoTckManager> {

Uh oh!

Conversation

jdaugherty commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdaugherty commented May 12, 2026

Uh oh!

jdaugherty commented May 12, 2026

Uh oh!

matrei left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jdaugherty commented May 12, 2026

Uh oh!

matrei commented May 13, 2026

Uh oh!

jdaugherty commented May 13, 2026

Uh oh!

jdaugherty commented May 13, 2026

Uh oh!

jdaugherty commented May 13, 2026

Uh oh!

Uh oh!

jdaugherty commented May 17, 2026

Uh oh!

jdaugherty commented May 17, 2026

Uh oh!

This comment has been minimized.

jamesfredley commented May 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matrei commented May 21, 2026

Uh oh!

sbglasius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

jdaugherty commented May 11, 2026 •

edited

Loading