Keep GORM entities on the DevTools restart classloader

[jamesfredley]

Description

Fixes #16287.

This is a real compatibility bug, not user error. With Spring Boot DevTools restart active, Grails 8 loads application domain classes in DevTools' RestartClassLoader while GORM and Hibernate remain on the base loader. Hibernate's JPA metamodel keys entities by Class identity, so GORM Criteria calls such as count() / list() and save() fail with IllegalArgumentException: Not an entity while get() and HQL still work.

What was actually wrong on Hibernate 5

The Hibernate 5 connection-source factory looked up a Spring bean named default (ConnectionSource.DEFAULT). The DataSource bean is dataSource, so setApplicationContext never ran and CLASSLOADERS stayed on connectionSource.getClass().getClassLoader() (the base loader). Hibernate 7 already computed dataSource / dataSource_<name>.

Hibernate then re-resolves entity Class objects by name during SessionFactory construction (ReflectHelper.classForName uses the thread context class loader). Setting AvailableSettings.CLASSLOADERS is not enough if TCCL is still the base loader at bootstrap.

What this PR does

• Hibernate 5 factory looks up dataSource / dataSource_<name> and sets dataSourceName before setApplicationContext, so the application-context class loader is used. Named sources inject dataSource_<name>, not the default DataSource.
• Both Hibernate 5 and 7 call DevToolsClassLoaders.preferRestartClassLoader(...) at SessionFactory build time and temporarily set TCCL around super.buildSessionFactory.
• CLASSLOADERS is left unset when the application context loader is null and DevTools is not active, preserving the old Hibernate 7 fallback.
• The helper matches DevTools' RestartClassLoader by FQCN first (simple-name fallback for tests/shaded copies), keeps a descendant-loader guard, and deprecates resolve() in favor of preferRestartClassLoader().
• Framework jars stay on the base loader; moving GORM/Hibernate onto the restart loader would split Grails types such as MappingContext across loaders.

Contributor Checklist

Please review the following checklist before submitting your pull request. Pull requests that do not meet these requirements may be closed without review.

Issue and Scope

• This PR is linked to an existing issue that has been acknowledged or approved by the project team. If no approved issue exists, please give background on why this change is necessary. Tickets are preferred for release change log history.
• This PR addresses the complete scope of the linked issue. Partial implementations or unfinished work should not be submitted for review.
• This PR contains a single, focused change. Unrelated changes should be submitted as separate pull requests.
• This PR targets the correct branch for the type of change:
  • Patch release branches (e.g., 7.0.x): Bug fixes only. No new features or API changes.
  • Minor release branches (e.g., 7.1.x): New features are welcome, but breaking existing APIs must be avoided.
  • Major release branches (e.g., 8.0.x): Reserved for major changes. Breaking API changes are permitted.

Code Quality

• I have added or updated tests that cover the changes introduced in this PR. All code contributions are expected to include appropriate test coverage.
• I have verified that all existing tests pass by running ./gradlew build --rerun-tasks.
• My code follows the project's code style guidelines. I have run ./gradlew codeStyle and resolved any violations. See Code Style for details.
• This PR does not include mass reformatting, style-only changes, or large-scale refactoring unless it was explicitly approved in the linked issue. Unsolicited reformatting will not be accepted.
• If generative AI tooling was used in preparing this contribution, a quality model was used to ensure contributions are consistent with the project's quality standards.

Licensing and Attribution

• All contributed code is provided under the Apache License 2.0, and new source files include the appropriate Apache license header.
• I have the necessary rights to submit this contribution and confirm it is my own original work (see Legal Notice).
• If generative AI tooling was used in preparing this contribution, I have followed the Apache Software Foundation's policy on generative tooling and have properly attributed its use.

Documentation

• If this PR introduces user-facing changes, I have included or updated the relevant documentation.
• If this PR adds a new feature, I have updated the What's New section of the Grails Guide.
• If this PR introduces breaking changes or changes that require user action during an upgrade, I have updated the Upgrade Notes for the corresponding version in the Grails Guide.
• The PR description clearly explains what was changed and why.

Generative AI attribution

Generative AI tooling (Cursor Grok 4.6 with GPT review) was used to draft the implementation, tests, and documentation. The change was reviewed, tested in the affected modules (grails-datastore-core, grails-data-hibernate5-core, grails-data-hibernate7-core), and edited before submission.

[Copilot]

🟡 Changes recommended

The added DevTools documentation overstates behavior when DevTools is merely present on the classpath, and should be narrowed to cases where the restart classloader is actually active.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes a Grails 8 + Spring Boot DevTools compatibility issue where domain classes can be loaded in DevTools’ RestartClassLoader while Hibernate/GORM stay on the base loader, causing class-identity mismatches (e.g. IllegalArgumentException: Not an entity) for criteria-style operations.

Changes:

• Introduces a shared DevToolsClassLoaders utility to consistently prefer the DevTools restart thread context class loader when present.
• Updates Hibernate 5 and Hibernate 7 HibernateMappingContextConfiguration to use the resolved class loader for AvailableSettings.CLASSLOADERS.
• Adds targeted specs in datastore + Hibernate modules and documents the behavior in the guide.

File summaries

File	Description
grails-doc/src/en/guide/gettingStarted/developmentReloading.adoc	Documents how DevTools restart class loading interacts with Hibernate/GORM entity identity.
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/reflect/DevToolsClassLoaders.java	Adds a reusable classloader-resolution helper for DevTools restart scenarios.
grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/reflect/DevToolsClassLoadersSpec.groovy	Adds unit tests for restart-loader detection and resolution behavior.
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/HibernateMappingContextConfiguration.java	Uses DevToolsClassLoaders.resolve(...) when setting/reading Hibernate CLASSLOADERS.
grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/HibernateMappingContextConfigurationSpec.groovy	Adds a regression spec ensuring restart TCCL is preferred over the app context loader.
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/HibernateMappingContextConfiguration.java	Aligns Hibernate 5 config with the same restart-aware classloader resolution.
grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/cfg/HibernateMappingContextConfigurationSpec.groovy	Adds coverage for Hibernate 5 behavior with and without restart TCCL.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bito-code-review]

The user's observation is accurate. The documentation should be updated to clarify that the RestartClassLoader is only preferred when the thread context class loader is an instance of RestartClassLoader, rather than implying it is always active when DevTools is on the classpath. This ensures the documentation accurately reflects the conditional logic implemented in the DevToolsClassLoaders.resolve method.

[codecov]

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 54.7325%. Comparing base (e8d33a9) to head (a137709).

Files with missing lines	Patch %	Lines
...atastore/mapping/reflect/DevToolsClassLoaders.java	95.6522%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #16299        +/-   ##
==================================================
+ Coverage     54.7242%   54.7325%   +0.0083%     
- Complexity      20531      20543        +12     
==================================================
  Files            2104       2105         +1     
  Lines          101149     101172        +23     
  Branches        17966      17973         +7     
==================================================
+ Hits            55353      55374        +21     
- Misses          37775      37776         +1     
- Partials         8021       8022         +1     

Files with missing lines	Coverage Δ
...atastore/mapping/reflect/DevToolsClassLoaders.java	95.6522% <95.6522%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jdaugherty]

AI did the below review and seems to think the hibernate 5 is still broken. Did you test to confirm these changes fix it? See below for it's review.

Clean consolidation of the duplicated RestartClassLoader sniffing, and the new helper is well covered. But tracing it against the reported reproduction, I don't think it changes behavior on the path that actually fails.

The change appears to be a no-op on the reporter's path

The issue is Grails 8.0.0-M5 from start.grails.org with Hibernate 5.6.15. Forge's default is GormImpl.DEFAULT_OPTION = HIBERNATE5 (GormImpl.java:31), so the failing path is hibernate5:

1. grails-data-hibernate5/core/.../connections/HibernateConnectionSourceFactory.java:142 branches on applicationContext.containsBean(dataSourceConnectionSource.getName()). ConnectionSource.DEFAULT is "default" (ConnectionSource.java:34) and there is no bean named default — the DataSource bean is dataSource. So this always takes the else branch, setDataSourceConnectionSource(...). (hibernate7 computes the real bean name "dataSource" and therefore takes the setApplicationContext branch — the two modules genuinely diverge here.)

2. setDataSourceConnectionSource has preferred a restart TCCL since 56df99d587 ("Support DevTools RestartClassLoader. Fixes #11159"). That is exactly the code this PR replaces with an equivalent DevToolsClassLoaders.resolve(...) call.

3. create() (HibernateConnectionSourceFactory.java:107-109) calls buildConfiguration(...) and then configuration.buildSessionFactory() back to back on the same thread. So the new resolve(storedClassLoader) in buildSessionFactory reads the same Thread.currentThread().getContextClassLoader() the setter just read, and can only return the same loader.

So on the hibernate5 default path the resolved loader is identical before and after this PR. Either the failure has a root cause other than AvailableSettings.CLASSLOADERS, or the TCCL was not a RestartClassLoader at connection-source creation — and in that second case this PR doesn't help either, for the same-thread reason above.

Earlier investigation of this issue confirmed via identityHashCode/loader diagnostics that the JPA metamodel held AppClassLoader copies while GORM held restart-loader copies. Since the restart-loader preference was already active on that path, something else is re-resolving those classes by name.

The one genuine behavior change here is hibernate7's setApplicationContext, which now prefers a restart TCCL over applicationContext.getClassLoader(). Under devtools those normally agree, since AbstractApplicationContext extends DefaultResourceLoader and captures the TCCL at construction on restartedMain.

Ask: how was the fix verified? Ideally a generated app with gorm-hibernate5 + spring-security-core + devtools that failed before and boots after. The checklist also has "verified that all existing tests pass" unchecked. If verification was against hibernate7, the "Fixes #16287" claim should probably be re-scoped, since the reporter's stack is hibernate5.

CI is green apart from the known-flaky UserControllerSpec > User list (#16030).

[jamesfredley]

Independent check of the review feedback (not taking the review text as given):

The Hibernate 5 factory was looking up a bean named default (ConnectionSource.DEFAULT). The DataSource bean is dataSource, so setApplicationContext never ran on the reporter's Hibernate 5 path. That claim was valid. setDataSourceConnectionSource already preferred a restart TCCL, so swapping CLASSLOADERS alone on that else-branch was equivalent. The real Hibernate 5 hole was taking the wrong setter, which left CLASSLOADERS on connectionSource.getClass().getClassLoader() (the base loader) whenever TCCL was not already a RestartClassLoader.

Hibernate also re-resolves entity Class objects by name during SessionFactory construction (ReflectHelper.classForName uses TCCL). That is why this follow-up sets TCCL around super.buildSessionFactory, not only AvailableSettings.CLASSLOADERS.

Updates in c8945fd:

• Hibernate 5 factory looks up dataSource / dataSource_<name> and sets dataSourceName before setApplicationContext, so named sources inject dataSource_secondary rather than the default DataSource.
• Hibernate 5 and 7 use preferRestartClassLoader, re-resolve at SessionFactory build, and wrap TCCL around super.buildSessionFactory (restored in finally).
• CLASSLOADERS is left unset when the application context loader is null and DevTools is not active.
• Helper: FQCN match first, case-sensitive simple-name fallback, descendant-loader guard, resolve() deprecated.
• Tests for factory bean wiring, named DataSource injection, loader identity, null-loader fallback, and resolveSessionFactoryClassLoader.
• Docs: dropped the Role.count() symptom sentence.

Targeted tests passed in grails-datastore-core, grails-data-hibernate5-core, and grails-data-hibernate7-core. Module codeStyle is clean. Review threads from that pass are resolved.

I did not run a generated Forge app with gorm-hibernate5 + spring-security-core + DevTools end-to-end in this follow-up; the factory bean-name miss and TCCL wrap are covered by unit tests against the actual bootstrap path.

[jdaugherty]

@jamesfredley it looks like the tests aren't running on this PR b/c of the infrastructure unapproving the actions.

[jdaugherty]

I opened #16324 to address all of my feedback on this review.

[testlens-app]

🚨 TestLens detected 2 failed tests 🚨

Here is what you can do:

1. Inspect the test failures carefully.
2. If you are convinced that some of the tests are flaky, you can mute them below.
3. Finally, trigger a rerun by checking the rerun checkbox.

Test Summary

CI / Functional Tests (Java 25, indy=false, shard 1) > :grails-test-examples-app1:integrationTest

Test	Runs	Flakiness
RedirectWithAndWithoutParamsFunctionalSpec > Params are not added to the url after a redirect even if they are passed to the redirect	❌	1% 🟡

Groovy Snapshot Canary Build / Build Grails (shard 2) > :grails-test-examples-scaffolding:integrationTest

Test	Runs	Flakiness
UserControllerSpec > User list	❌	5% 🟠

🏷️ Commit: a137709
▶️ Tests: 70978 executed
⚪️ Checks: 89/89 completed

Test Failures

UserControllerSpec > User list (:grails-test-examples-scaffolding:integrationTest in Groovy Snapshot Canary Build / Build Grails (shard 2))

geb.waiting.WaitTimeoutException: condition did not pass in 30 seconds (failed with exception)
	at geb.waiting.Wait.waitFor(Wait.groovy:128)
	at geb.waiting.DefaultWaitingSupport.doWaitFor(DefaultWaitingSupport.groovy:55)
	at geb.waiting.DefaultWaitingSupport.waitFor(DefaultWaitingSupport.groovy:41)
	at geb.Page.waitFor(Page.groovy:120)
	at com.example.pages.LoginPage.login(LoginPage.groovy:39)
	at com.example.UserControllerSpec.User list(UserControllerSpec.groovy:48)
Caused by: Assertion failed: 

title != pageTitle && $('input', name: 'username').empty
|     |  |         |
|     |  |         false
|     |  'Please sign in'
|     false
'Please sign in'

	at com.example.pages.LoginPage.login_closure1(LoginPage.groovy:39)
	at com.example.pages.LoginPage.login_closure1(LoginPage.groovy)
	at geb.waiting.Wait.waitFor(Wait.groovy:117)
	... 5 more

RedirectWithAndWithoutParamsFunctionalSpec > Params are not added to the url after a redirect even if they are passed to the redirect (:grails-test-examples-app1:integrationTest in CI / Functional Tests (Java 25, indy=false, shard 1))

Condition not satisfied:

pageSource.contains('"id":')
|          |
|          false
<html><head></head><body><form action="save" method="post">
    <input type="text" name="name">
    <input type="submit">
</form>
</body></html>

	at functionaltests.RedirectWithAndWithoutParamsFunctionalSpec.$tt__$spock_feature_1_0(RedirectWithAndWithoutParamsFunctionalSpec.groovy:41)
	at functionaltests.RedirectWithAndWithoutParamsFunctionalSpec.Params are not added to the url after a redirect even if they are passed to the redirect_closure1(RedirectWithAndWithoutParamsFunctionalSpec.groovy)
	at grails.gorm.transactions.GrailsTransactionTemplate$1.doInTransaction(GrailsTransactionTemplate.groovy:76)
	at org.springframework.transaction.support.TransactionTemplate.execute(TransactionTemplate.java:137)
	at grails.gorm.transactions.GrailsTransactionTemplate.executeAndRollback(GrailsTransactionTemplate.groovy:73)
	at functionaltests.RedirectWithAndWithoutParamsFunctionalSpec.Params are not added to the url after a redirect even if they are passed to the redirect(RedirectWithAndWithoutParamsFunctionalSpec.groovy)

Rerun Controls

Select tests to mute in this pull request:

• RedirectWithAndWithoutParamsFunctionalSpec > Params are not added to the url after a redirect even if they are passed to the redirect
• UserControllerSpec > User list

Reuse successful test results:

• ♻️ Only rerun the tests that failed or were muted before

Click the checkbox to trigger a rerun:

• Rerun jobs

---

Learn more about TestLens at testlens.app/docs.