codeconsole opened a new pull request, #15650:
URL: https://github.com/apache/grails-core/pull/15650
# fix(grails-datamapping-core): GroovyProxyFactory.getProxiedClass walks
past entity to Object
## Summary
`GroovyProxyFactory.getProxiedClass()` returns `java.lang.Object` for
proxies that this same factory created, instead of the proxied entity class.
Any caller that feeds the result into
`MappingContext.getPersistentEntity(name)` then receives `null` and NPEs on the
next dereference. The most user-visible symptom is a cascade-validation
`NullPointerException` from `UniqueConstraint.processValidate` when saving an
entity whose to-one association is a not-yet-unwrapped proxy — happens in
production on MongoDB / Neo4j stacks (any datastore whose classpath does not
pull Javassist).
## Root cause
`GroovyProxyFactory` and `JavassistProxyFactory` use **different** proxy
strategies, but they ship the same `getProxiedClass()` implementation:
```groovy
Class<?> getProxiedClass(Object o) {
if (isProxy(o)) {
return o.getClass().getSuperclass()
}
return o.getClass()
}
```
| Factory | What `createProxy()` returns | `proxy.getClass()` is |
`proxy.getClass().getSuperclass()` is |
|---|---|---|---|
| `JavassistProxyFactory` | a runtime subclass `Proxy_$$_javassist extends
Entity` | the **proxy** class | the **entity** class — correct |
| `GroovyProxyFactory` | a real `Entity` instance with a
`ProxyInstanceMetaClass` attached | the **entity** class | `java.lang.Object` —
wrong |
The `getSuperclass()` walk is correct for Javassist's runtime-subclass
strategy. Copy-pasted into `GroovyProxyFactory`, whose proxies are *not*
subclasses of the entity (they are real entity instances with a custom
metaclass), the walk steps one level past the entity — to `java.lang.Object`
for any entity extending `Object` directly, which is essentially all of them.
## Why this surfaces on NoSQL stacks
`AbstractMappingContext.getProxyFactory()` picks the factory by classpath
probe:
```java
if (ClassUtils.isPresent("javassist.util.proxy.ProxyFactory", classLoader)) {
proxyFactory = DefaultProxyFactoryCreator.create(); // →
JavassistProxyFactory
}
else if (ClassUtils.isPresent(GROOVY_PROXY_FACTORY_NAME, classLoader)) {
proxyFactory = new GroovyProxyFactory(); // ← fallback
}
```
Hibernate / SQL stacks pull Javassist transitively, so they get
`JavassistProxyFactory` and never exercise the buggy code path. MongoDB and
Neo4j do not pull Javassist, so they fall through to `GroovyProxyFactory` and
hit the bug whenever a proxy reaches a constraint validator (or any other
consumer of `ProxyHandler.getProxiedClass()`) before being unwrapped.
## Concrete reproduction trace
The bug was first surfaced in a Grails 7 application using the MongoDB
plugin. Saving a child entity from a non-request thread (no GORM session bound)
triggered:
```
java.lang.NullPointerException: Cannot invoke
"org.grails.datastore.mapping.model.PersistentEntity.isRoot()"
because the return value of "groovy.lang.Reference.get()" is null
at org.grails.datastore.gorm.validation.constraints.builtin
.UniqueConstraint.processValidate(UniqueConstraint.groovy:85)
at grails.gorm.validation.PersistentEntityValidator
.cascadeValidationToOne(PersistentEntityValidator.groovy:266)
at grails.gorm.validation.PersistentEntityValidator
.cascadeToAssociativeProperty(PersistentEntityValidator.groovy:136)
at grails.gorm.validation.PersistentEntityValidator
.validate(PersistentEntityValidator.groovy:102)
...
at <Domain>.save(...)
```
Diagnostic logging captured at the crash site confirmed the chain end-to-end:
```
proxyHandler.class =
org.grails.datastore.gorm.proxy.GroovyProxyFactory
proxyHandler.isProxy(proj) = true
proj.class = com.example.IssueProject // real
entity instance
proj.class.is(IssueProject) = true
proxiedClass = java.lang.Object // ← bug:
should be IssueProject
ctx.getPersistentEntity(...) = null // ← lookup
misses
ctx.persistentEntities count = 79
registered IssueProject entries = [IssueProject] // entity
IS registered
```
Same data on the request thread (where the proxy gets unwrapped before
reaching the validator) saved cleanly with cascade validation enabled. After
applying this fix, the same async-thread save also succeeds.
## Fix
Drop the bogus superclass walk:
```diff
@Override
- @Override
Class<?> getProxiedClass(Object o) {
- if (isProxy(o)) {
- return o.getClass().getSuperclass()
- }
return o.getClass()
}
```
`o.getClass()` is the correct answer for both branches:
- For metaclass-only proxies: `o.getClass()` *is* the entity class.
- For non-proxy instances: `o.getClass()` is the entity class.
The `isProxy(o)` branch becomes a no-op for the case it's currently meant to
handle, so it has been removed entirely. The duplicate `@Override` annotation
that was already on the method has also been removed (no behavior change, just
cleanup).
## Test coverage
Adds
`grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactorySpec.groovy`
with two cases:
1. **`getProxiedClass` returns the entity class for a proxy created by this
factory** — fails on the unmodified code (`getProxiedClass(proxy)` returns
`java.lang.Object`), passes after the fix.
2. **`getProxiedClass` returns the entity class for a non-proxy instance** —
passes both before and after; regression guard for the non-proxy branch.
Both cases mirror what `GroovyProxyFactory.createProxy()` does internally
(real entity instance + `ProxyInstanceMetaClass` attached), so they exercise
the same shape `MappingContext` produces at runtime.
## Verification
Test totals on the patched code (`groovy-proxy-getproxiedclass-fix` branch
off `7.2.x`):
| Suite | Tests | Failures | Skipped |
|---|---|---|---|
| `grails-datamapping-core` (incl. new `GroovyProxyFactorySpec`) | passing |
0 | — |
| `grails-datamapping-core-test` (TCK against simple datastore — runs
`GroovyProxySpec` for both `useGroovyProxyFactory: true` and `false`) | 474 | 0
| — |
| `grails-datamapping-validation`, `tck`, `support`, `async` | passing | 0 |
— |
| `grails-datastore-core` (incl. `JavassistProxyFactorySpec`) | passing | 0
| — |
| `grails-datastore-web` | passing | 0 | — |
| `grails-data-mongodb-core` (real Mongo, full GORM cascade + proxy paths) |
568 | 0 | 23 |
| `grails-data-mongodb-bson` | 31 | 0 | 0 |
| `grails-data-hibernate5/core` (Javassist path — sanity check that the
alternate proxy strategy is unaffected) | 478 | 0 | 28 |
Zero regressions across the data-mapping, datastore, MongoDB, and Hibernate
test suites. The fix is in the agnostic core (`grails-datamapping-core`), so
all consumers of `GroovyProxyFactory` benefit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
