You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jamesfredley (via GitHub)" <gi...@apache.org> on 2026/10/22 01:47:38 UTC
[PR] Unwrap every Groovy proxy layer when resolving a GORM API [grails-core]
jamesfredley opened a new pull request, #16721:
URL: https://github.com/apache/grails-core/pull/16721
Fixes #16714.
`NameUtils.getClassName` walked only one dollar-named superclass. The extra Groovy proxy wrapper therefore resolved to the Hibernate proxy class, and GORM API lookup missed the entity. It now walks every proxy layer. A normal class and a single-layer Hibernate proxy still resolve to the entity.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Unwrap every Groovy proxy layer when resolving a GORM API [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16721:
URL: https://github.com/apache/grails-core/pull/16721#issuecomment-6331469016
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16721?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `50.00000%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 62.1456%. Comparing base ([`75a355a`](https://app.codecov.io/gh/apache/grails-core/commit/75a355a3aeb30e393178b85e121a3748e603b7d7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`9961dfb`](https://app.codecov.io/gh/apache/grails-core/commit/9961dfb160c7a92bba5c3d3840f5409d8abe2335?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16721?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...rg/grails/datastore/mapping/reflect/NameUtils.java](https://app.codecov.io/gh/apache/grails-core/pull/16721?src=pr&el=tree&filepath=grails-datastore-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fmapping%2Freflect%2FNameUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFzdG9yZS1jb3JlL3NyYy9tYWluL2dyb292eS9vcmcvZ3JhaWxzL2RhdGFzdG9yZS9tYXBwaW5nL3JlZmxlY3QvTmFtZVV0aWxzLmphdmE=) | 50.0000% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16721?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16721?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 8.0.x #16721 +/- ##
==================================================
- Coverage 62.1568% 62.1456% -0.0112%
+ Complexity 26860 26859 -1
==================================================
Files 2237 2237
Lines 110588 110587 -1
Branches 20189 20188 -1
==================================================
- Hits 68738 68725 -13
- Misses 32597 32611 +14
+ Partials 9253 9251 -2
```
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16721?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...rg/grails/datastore/mapping/reflect/NameUtils.java](https://app.codecov.io/gh/apache/grails-core/pull/16721?src=pr&el=tree&filepath=grails-datastore-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fmapping%2Freflect%2FNameUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFzdG9yZS1jb3JlL3NyYy9tYWluL2dyb292eS9vcmcvZ3JhaWxzL2RhdGFzdG9yZS9tYXBwaW5nL3JlZmxlY3QvTmFtZVV0aWxzLmphdmE=) | `83.7209% <50.0000%> (-2.6427%)` | :arrow_down: |
... and [3 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16721/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Unwrap every Groovy proxy layer when resolving a GORM API [grails-core]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16721:
URL: https://github.com/apache/grails-core/pull/16721#discussion_r4343849973
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/reflect/NameUtils.java:
##########
@@ -104,9 +104,8 @@ public static String getGetterName(String propertyName, boolean useBooleanPrefix
* @return The class name
*/
public static String getClassName(Class clazz) {
- final String sn = clazz.getSimpleName();
- if (sn.contains(DOLLAR_SEPARATOR)) {
- return clazz.getSuperclass().getName();
+ while (clazz.getSimpleName().contains(DOLLAR_SEPARATOR) && clazz.getSuperclass() != null) {
+ clazz = clazz.getSuperclass();
}
return clazz.getName();
Review Comment:
This method uses the raw Class type, which triggers unnecessary rawtype warnings in Java. Since you’re already modifying this method, consider changing the signature to use Class<?> for better type safety without affecting runtime behavior.
##########
grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/proxy/ByteBuddyGroovyProxyFactorySpec.groovy:
##########
@@ -41,6 +43,29 @@ class ByteBuddyGroovyProxyFactorySpec extends Specification {
ByteBuddyState state = new ByteBuddyState()
ByteBuddyProxyHelper helper = new ByteBuddyProxyHelper(state)
+ void "a wrapped Hibernate proxy resolves the entity name and validation API without initialization"() {
+ given:
+ Session session = datastore.sessionFactory.openSession()
+ Class entityAlias = GroovyProxyFactoryEntity
+
+ when:
+ def proxy = session.getReference(entityAlias, 1L)
+ Class proxyClass = proxy.getClass()
+
+ then:
+ proxyClass.superclass.superclass == entityAlias
Review Comment:
This assertion depends on Hibernate/ByteBuddy’s exact generated superclass chain depth, which is an implementation detail and may change across versions/configurations. To reduce brittleness while still validating the regression, prefer asserting only the observable contract: that the proxy class is not the entity class, and that NameUtils.getClassName(proxyClass) resolves to the entity name (optionally also assert that unwrapping works for proxyClass.superclass if you want a minimal intermediate check).
##########
grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/proxy/ByteBuddyGroovyProxyFactorySpec.groovy:
##########
@@ -39,6 +41,29 @@ class ByteBuddyGroovyProxyFactorySpec extends Specification {
ByteBuddyProxyHelper helper = new GrailsBytecodeProvider().getByteBuddyProxyHelper()
+ void "a wrapped Hibernate proxy resolves the entity name and validation API without initialization"() {
+ given:
+ Session session = datastore.sessionFactory.openSession()
+ Class entityAlias = GroovyProxyFactoryEntity
+
+ when:
+ def proxy = session.getReference(entityAlias, 1L)
+ Class proxyClass = proxy.getClass()
+
+ then:
+ proxyClass.superclass.superclass == entityAlias
+ NameUtils.getClassName(entityAlias) == entityAlias.name
+ NameUtils.getClassName(proxyClass.superclass) == entityAlias.name
+ NameUtils.getClassName(proxyClass) == entityAlias.name
+ GormEnhancer.findValidationApi(proxyClass) != null
+ GormEnhancer.findValidationApi(proxyClass).is(GormEnhancer.findValidationApi(entityAlias))
Review Comment:
The test calls GormEnhancer.findValidationApi(proxyClass) twice (and also calls findValidationApi(entityAlias) inline), which can repeat reflection/introspection work and makes failures a bit harder to read. Store these results in local variables (e.g., proxyValidationApi/entityValidationApi) and assert against them.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Unwrap every Groovy proxy layer when resolving a GORM API [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16721:
URL: https://github.com/apache/grails-core/pull/16721#discussion_r4348541740
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/reflect/NameUtils.java:
##########
@@ -104,9 +104,8 @@ public static String getGetterName(String propertyName, boolean useBooleanPrefix
* @return The class name
*/
public static String getClassName(Class clazz) {
- final String sn = clazz.getSimpleName();
- if (sn.contains(DOLLAR_SEPARATOR)) {
- return clazz.getSuperclass().getName();
+ while (clazz.getSimpleName().contains(DOLLAR_SEPARATOR) && clazz.getSuperclass() != null) {
Review Comment:
`DomainClassArtefactHandler.isDomainClass(Class, boolean)` in grails-core (lines 111-116) has the same one-level unwrap and regressed the same way with #16625: `@Artefact` and both `@Entity` annotations are not `@Inherited`, so neither generated layer carries them (the probe prints an empty annotation array for the wrapper and for the stock proxy), and the single `getSuperclass()` step now lands on the stock proxy, which fails the check where the entity used to pass it. It is reached from `DefaultLinkGenerator` and `AbstractLinkingRenderer` only when there is no mapping context to answer first, so the impact is small, but it is the same bug. Loop there too, or open a follow-up so it is not lost.
##########
grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/proxy/ByteBuddyGroovyProxyFactorySpec.groovy:
##########
@@ -41,6 +43,29 @@ class ByteBuddyGroovyProxyFactorySpec extends Specification {
ByteBuddyState state = new ByteBuddyState()
ByteBuddyProxyHelper helper = new ByteBuddyProxyHelper(state)
+ void "a wrapped Hibernate proxy resolves the entity name and validation API without initialization"() {
+ given:
+ Session session = datastore.sessionFactory.openSession()
+ Class entityAlias = GroovyProxyFactoryEntity
+
+ when:
+ def proxy = session.getReference(entityAlias, 1L)
+ Class proxyClass = proxy.getClass()
+
+ then:
+ proxyClass.superclass.superclass == entityAlias
+ NameUtils.getClassName(entityAlias) == entityAlias.name
+ NameUtils.getClassName(proxyClass.superclass) == entityAlias.name
+ NameUtils.getClassName(proxyClass) == entityAlias.name
+ GormEnhancer.findValidationApi(proxyClass) != null
Review Comment:
#16714 lists the instance and static API lookups as well. The probe shows `GormEnhancer.findStaticApi(proxyClass)` and `findInstanceApi(proxyClass)` resolve to the entity's now, so two more `.is(...)` lines here, and in the H5 twin, would close the issue's regression list.
##########
grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/reflect/NameUtilsSpec.groovy:
##########
@@ -23,6 +23,24 @@ import spock.lang.Unroll
class NameUtilsSpec extends Specification {
+ @Unroll
+ void "getClassName unwraps every proxy layer for #type.simpleName"() {
+ expect:
+ NameUtils.getClassName(type) == NameUtilsEntity.name
+
+ where:
+ type << [NameUtilsEntity, NameUtilsEntity$Proxy, NameUtilsEntity$Proxy$Wrapper,
+ NameUtilsEntity$Proxy$Wrapper$Outer]
+ }
+
+ void "getClassName preserves a real entity referenced by a local alias"() {
Review Comment:
This feature repeats the first `where:` row: a `Class` local is the same object as the literal, so nothing new reaches `getClassName`. Drop it. The `entityAlias` locals in the two Hibernate specs can be the class literal for the same reason.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Unwrap every Groovy proxy layer when resolving a GORM API [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16721:
URL: https://github.com/apache/grails-core/pull/16721#issuecomment-6332435216
## 🚨 All tests passed but jobs failed 🚨
### Failed Jobs without Test Failures
❌ [Running Copilot Code Review / copilot-pull-request-reviewer](https://github.com/apache/grails-core/actions/runs/39707729303/job/119591603877?pr=16721)
🏷️ Commit: 9961dfb160c7a92bba5c3d3840f5409d8abe2335
▶️ Tests: 38648 executed
🟡 Checks: 78/86 completed
### Rerun Controls
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org