Various identifier fixes for not unwrapping Hibernate proxies

[jdaugherty]

Groovy-Aware Hibernate Proxies for grails-data-hibernate5 (and handler fixes for both Hibernate lines)

While working on the Spring Security branch merge to 8.x, I discovered inconsistent property behavior with .id and .getId - the redis plugin imported the Spring version of Rollback so we never noticed this before. This PR ensures domains do not unwrap when calling either form in hibernate 5 or 7.

The bug

Accessing the identifier of an uninitialized GORM/Hibernate proxy — proxy.id, proxy.getId(),
proxy['id'], proxy.ident() — initialized the proxy on grails-data-hibernate5. With an open
session that was a silent eager fetch (defeating the point of load()/lazy associations); on a
detached proxy it threw LazyInitializationException. Identifier access must never unwrap a
proxy.

First observed as two failures in RedisIntegrationSpec (testMemoizeDomainList,
testMemoizeDomainObject) on this branch's CI. The redis plugin caches only entity IDs in redis
and rehydrates with domainClass.load(id) inside short @Transactional methods, so its
cache-hit results are exactly this shape: uninitialized proxies whose transaction has ended.

Root cause

Hibernate's stock ByteBuddy proxy intercepts every virtual method. BasicLazyInitializer
has a tiny no-init allowlist — essentially the identifier getter (getId()) and
getHibernateLazyInitializer() — and initializes on anything else. Dynamic Groovy property
access never reaches getId() directly: the MOP resolves the receiver first, calling
getMetaClass() (and getProperty("id")) on the instance. Those are ordinary virtual methods
woven in by the Groovy compiler, so the proxy intercepts them, they miss the allowlist, and the
proxy initializes before getId() is ever called.

Findings that scoped the fix:

• Not a Groovy 5 regression. A recording fake proxy run under Groovy 4.0.32 and 5.0.7 shows
  byte-identical dispatch: getMetaClass() ×3 then getProperty("id") then getId(). The same
  redis tests also fail on a local run of upstream/7.2.x (Groovy 4) with a real redis — they
  had simply never executed in this repo's CI before the inverted isRedis flag was fixed on
  this branch.
• hibernate7 already solved this in-repo with GrailsBytecodeProvider →
  ByteBuddyGroovyProxyFactory → ByteBuddyGroovyInterceptor + GroovyProxyInterceptorLogic,
  installed by default. hibernate5 had no equivalent — only an opt-in third-party test dependency
  (org.yakworks:hibernate-groovy-proxy) and @PendingFeatureIf-gated tests in
  ByteBuddyProxySpec documenting the gap.
• A separate handler regression on this branch: HibernateProxyHandler (both modules) gained
  a getProxyInstanceMetaClass probe that calls getMetaClass() on the object before the
  instanceof HibernateProxy checks. The probe only exists to detect the simple/in-memory
  datastore's metaclass-based proxies (ProxyInstanceMetaClass), which a Hibernate proxy can
  never be — so for Hibernate proxies it was pure downside: getIdentifier()/isProxy()/
  unwrap() initialized the proxy (or threw when detached) on 7.2.x-and-later code that used to
  answer straight from the LazyInitializer.

The fix

1. Port the hibernate7 proxy stack to grails-data-hibernate5 (new files)

Same classes, same names, same behavior — adapted only where the Hibernate 5.6 SPI differs
(Serializable ids, ByteBuddyProxyFactory/ProxyFactoryFactory signatures):

File (grails-data-hibernate5/core/.../org/grails/orm/hibernate/proxy/)	Purpose
GroovyProxyInterceptorLogic.java	Verbatim copy of the h7 logic: answers getMetaClass/getStaticMetaClass/getProperty("id"|"metaClass")/ident/isDirty/hasChanged/toString for uninitialized proxies without initializing.
ByteBuddyGroovyInterceptor.java	Extends 5.6 ByteBuddyInterceptor. Returns the identifier for getId/getIdentifier/the mapped id getter before any other handling; routes uninitialized calls through the logic above; falls back to stock interception otherwise.
ByteBuddyGroovyProxyFactory.java	Extends 5.6 ByteBuddyProxyFactory; builds the proxy class via ByteBuddyProxyHelper and installs the Groovy-aware interceptor via ProxyConfiguration.$$_hibernate_set_interceptor.
GrailsProxyFactoryFactory.java	ProxyFactoryFactory returning the Groovy-aware factory for entities; basic (non-entity) proxies delegate to the stock implementation.
GrailsBytecodeProvider.java	Extends stock BytecodeProviderImpl so enhancement/reflection optimization are untouched; only getProxyFactoryFactory() is overridden.

Wiring (HibernateMappingContextConfiguration.buildSessionFactory()):

standardServiceRegistryBuilder.addService(ProxyFactoryFactory.class,
        new GrailsBytecodeProvider().getProxyFactoryFactory());

Hibernate 5.6's PojoEntityTuplizer resolves the ProxyFactoryFactory from the service
registry, and a provided service beats any classpath-contributed initiator — so this is
deterministic, default-on, and requires no third-party dependency. (hibernate7 wires the same
provider through HibernateConnectionSourceFactory/hibernate.enhancer.bytecodeprovider.instance;
the SPIs differ, the behavior does not.)

2. HibernateProxyHandler ordering fix (both modules)

• h5: getProxyInstanceMetaClass() returns null immediately for HibernateProxy
  instances, so getIdentifier()/isProxy()/unwrap()/initialize() never dispatch
  getMetaClass() through a Hibernate proxy. getIdentifier() is answered session-free from
  getHibernateLazyInitializer().getIdentifier() (the pre-branch behavior).
• h7: equivalent reorder in getIdentifier() and isProxy() — instanceof HibernateProxy
  before the GroovyProxyInterceptorLogic metaclass probe (the helper itself stays
  Hibernate-decoupled).

This matters independently of the interceptor: the handler must be safe for any
HibernateProxy it is handed, including ones not produced by our factory.

3. Tests

• ByteBuddyProxySpec (h5 + h7) un-gated: the @PendingFeatureIf/runPending machinery and
  the yakworks testImplementation are removed from both modules — all lazy-proxy requirements
  are now hard tests proving the in-repo stacks. 7 features each, identical scenarios:
  • getId()/.id/['id'] (dynamic and @CompileStatic) don't initialize
  • ident() and id access on a detached proxy work with no session and don't initialize
  • association id checks (team.club.id, team.clubId) don't initialize the association
  • truthy checks don't initialize
  • a clean isDirty() doesn't initialize (h5 expectation updated to match h7 — previously
    documented as initializing on h5)
  • new scenario spec: unwrap of a detached uninitialized proxy throws
    LazyInitializationException while getIdentifier()/ident() on the same proxy succeed —
    documents the one operation that legitimately needs a session (unwrap must materialize the
    entity; it exists only in the database).
• HibernateProxyHandlerMetaClassProbeSpec (h5 + h7, new): DB-free unit tripwire — a fake
  HibernateProxy whose getMetaClass() throws proves the handler answers
  getIdentifier/isProxy/isInitialized without dispatching through the proxy. (All probe
  interaction is @CompileStatic; the first version demonstrated live that even dynamic spec
  code touching the object trips the MOP interception.)

Verification (local)

Suite	Result
h5 proxy suites (ByteBuddyProxySpec, HibernateProxyHandler5Spec, MetaClassProbeSpec, SimpleHibernateProxyHandlerSpec, ToOneProxySpec)	32 tests, 0 failures
h7 proxy suites (incl. GrailsBytecodeProviderSpec, ByteBuddyGroovyInterceptorSpec, HibernateProxyHandler7Spec)	85 tests, 0 failures

Behavior changes to watch in CI

• h5 proxies now behave like h7's everywhere: toString() on an uninitialized proxy renders
  Entity:id instead of initializing; a clean isDirty() returns false without initializing;
  load() results stay lazy. Any h5 test/app code that accidentally relied on eager
  initialization will surface in the full suites.
• The grails-test-examples/hibernate5|hibernate7/grails-hibernate-groovy-proxy example apps
  still declare org.yakworks:hibernate-groovy-proxy; the provided service now overrides its
  contributor, so the dependency is redundant there and those apps/tests should be reviewed
  (likely: drop the dependency, keep the tests as extra coverage of the in-repo stack).
• unwrap()/unwrapIfProxy() semantics are unchanged: they initialize by contract. Code that
  only needs the id must use ident()/getProxyIdentifier()/proxy.id.

[jdaugherty]

@jamesfredley & @borinquenkid I created this separate from the plugin merge. This removes the need for the groovy-proxy library from yakworks too (which is built on a very old version of groovy).

[borinquenkid]

Awesome work.

[jdaugherty]

@borinquenkid & @jamesfredley this is finally ready to review.

[bito-code-review]

The request to remove GroovyProxyInterceptorLogic.java cannot be fulfilled as it is a newly introduced file in this pull request, essential for the Groovy-aware entity proxy functionality being implemented. This class provides the core logic for intercepting Groovy-specific methods on Hibernate proxies without triggering full proxy initialization, which is a key requirement for the changes in this PR.

[borinquenkid]

Just a couple of quibbles

[jdaugherty]

I went ahead and completely removed org.yakworks:hibernate-groovy-proxy now that .id and .getId() do not unwrap proxies. I've also added this to the weekly meeting to confirm there's no objection.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.3266%. Comparing base (5e561da) to head (96c0aed).
⚠️ Report is 5 commits behind head on 8.0.x.

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #15841        +/-   ##
==================================================
- Coverage     49.3373%   49.3266%   -0.0107%     
+ Complexity      16775      16770         -5     
==================================================
  Files            1985       1985                
  Lines           93327      93327                
  Branches        16336      16336                
==================================================
- Hits            46045      46035        -10     
- Misses          40158      40167         +9     
- Partials         7124       7125         +1     

see 4 files 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]

Removing the yakworks library revealed a change that's probably significant. Here's what AI said:

I found the root cause. ProfileServiceSpec isn't the real failure — it's collateral damage from a genuine behavioral regression in the PR.

The failure chain

1. RegisterSpec.testRegisterAndForgotPassword fails first (the real failure). It registers a user, creates a profile, then calls ProfileListPage.editProfile(un), which looks up the edit link with the Geb selector $('a', text: "User(username:$username)") — matching on the rendered toString of the user.
2. Because RegisterSpec dies at that step, its own cleanup (delete profile, delete user at the end of the same feature) never runs, leaving a committed extra User and Profile.
3. That leftover row is exactly why ProfileServiceSpec > test count sees 5 instead of 4, and why UserSpec > testFindAll (user search results) fails too.

The regression

The profile list GSP renders the association directly: ${entry.user} (grails-app/views/profile/index.gsp:52), where profile.user is a lazy Hibernate proxy. The User domain has @tostring(includes='username', includeNames=true, includePackage=false), so the page used to render User(username:test_user_...).

This PR's new GroovyProxyInterceptorLogic.handleUninitialized() (grails-data-hibernate5/.../GroovyProxyInterceptorLogic.java:68-70) short-circuits toString() on an uninitialized proxy and returns entityName + ":" + id — e.g. test.User:5 — without initializing the proxy or calling the entity's real toString(). And since HibernateMappingContextConfiguration now auto-registers the Groovy-aware proxy factory for every hibernate5 app, the spring-security extended app silently switched behavior: the page now renders test.User:5, the selector matches nothing, and the spec fails deterministically. The behavior is even pinned by the PR's own test (GroovyProxyInterceptorLogicSpec asserts "Book:1").

My recommendation

Remove (or rethink) the toString() short-circuit. Rendering ${someEntity.association} in a GSP is an extremely common pattern in real Grails apps, and stock Hibernate semantics — initialize the proxy and delegate to the entity's real toString() — is what every existing app depends on. The lazy-toString behavior was inherited from yakworks' design, but yakworks was opt-in; this factory is now the global default. The id/metaClass/ident() interceptions are the valuable part of the fix and don't have this compatibility problem.

The concrete change: drop the TO_STRING branch from handleUninitialized in the h5 GroovyProxyInterceptorLogic (and the equivalent in h7 if it does the same — worth checking GrailsBytecodeProvider/interceptor there), update GroovyProxyInterceptorLogicSpec, and add an assertion that toString() initializes and returns the real value.

[jdaugherty]

I'm going to make the toString behavior opt-in via config and match the historical grails behavior instead.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 96c0aed
▶️ Tests: 32373 executed
⚪️ Checks: 61/61 completed

---

Learn more about TestLens at testlens.app.