Add proxy package test coverage and fix IntelliJ inspections in grails-datamapping-core

[borinquenkid]

Summary

• Adds unit test coverage for GroovyProxyFactory and ProxyInstanceMetaClass in grails-datamapping-core, which previously had none in this module despite implementing the core Groovy-based proxy contract used outside Hibernate/Neo4j.
• Fixes IntelliJ inspection warnings in both classes: methods with no instance-state dependency marked static, replaces deprecated Class.newInstance() with getDeclaredConstructor().newInstance(), makes never-reassigned fields final, suppresses the unchecked-generics warning inherent to MetaClass.getTheClass()'s raw return type, and fills in missing Javadoc tag descriptions.

Test plan

• ./gradlew :grails-datamapping-core:test --tests "org.grails.datastore.gorm.proxy.*" — 29 tests pass
• ./gradlew :grails-datamapping-core:test — full module suite passes
• ./gradlew :grails-datamapping-core:codeStyle — no Checkstyle/CodeNarc violations

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR improves grails-datamapping-core’s Groovy proxy support by adding missing unit coverage for the proxy implementation and applying a set of code adjustments aimed at addressing IntelliJ inspection warnings.

Changes:

• Added new Spock specs covering GroovyProxyFactory and ProxyInstanceMetaClass behavior (lazy resolution, proxy metadata, delegation paths).
• Refactored ProxyInstanceMetaClass (final fields, Javadoc tag descriptions, switch expressions, warning suppression) to reduce inspection noise and modernize code.
• Refactored GroovyProxyFactory to replace deprecated instantiation and adjust helper methods/annotations (with a potential API-compatibility concern noted in comments).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClassSpec.groovy	Adds focused unit coverage for proxy metaclass behavior (method/property/attribute resolution semantics).
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactorySpec.groovy	Adds unit coverage for proxy factory creation/unwrap/identifier/initialization paths.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClass.java	Cleans up implementation details (final fields, switch refactor, warning suppression, Javadoc completeness).
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactory.groovy	Updates proxy factory internals (constructor instantiation, annotation import usage) and changes helper methods to static (flagged).

Suppressed comments (2)

grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactory.groovy:128

• Same concern here: making a protected method static breaks subclasses that might override it to customize proxy metaClass installation. Consider keeping it non-static and suppressing IntelliJ’s “method may be static” inspection instead.

    @CompileDynamic
    protected static void setMetaClassDynamic(Object proxy, MetaClass proxyMc) {
        proxy.setMetaClass(proxyMc)
    }

grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactory.groovy:170

• Making this protected method static is also a potential breaking change for subclasses (cannot be overridden). If the goal is just to satisfy IntelliJ’s inspection, keep it as an instance method and suppress GrMethodMayBeStatic on the method.

    protected static MetaClass unwrapHandleMetaClass(MetaClass mc) {
        if (mc instanceof HandleMetaClass) {
            return ((HandleMetaClass) mc).getAdaptee()
        }
        return mc
    }

---

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

[bito-code-review]

The concern regarding the conversion of protected instance methods to static is valid. In Groovy, changing an instance method to static breaks binary compatibility for any subclasses that override these methods, as static methods are not polymorphic and cannot be overridden.

If these changes were made solely to satisfy static analysis inspections, it is recommended to revert them to instance methods and suppress the inspection, consistent with existing patterns in the repository.

grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactory.groovy

@CompileDynamic
    protected Serializable getIdDynamic(obj) {
        if (obj.respondsTo('getId')) {
            return (Serializable)obj.invokeMethod('getId', null)
        }

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.1846%. Comparing base (df424da) to head (55d4b81).

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.1.x     #16155        +/-   ##
==================================================
+ Coverage     53.1444%   53.1846%   +0.0402%     
- Complexity      19371      19406        +35     
==================================================
  Files            2080       2080                
  Lines           99000      98991         -9     
  Branches        17363      17355         -8     
==================================================
+ Hits            52613      52648        +35     
+ Misses          38828      38792        -36     
+ Partials         7559       7551         -8     

Files with missing lines	Coverage Δ
...ils/datastore/gorm/proxy/GroovyProxyFactory.groovy	80.4348% <100.0000%> (+36.9565%)	⬆️
...s/datastore/gorm/proxy/ProxyInstanceMetaClass.java	100.0000% <100.0000%> (+52.7273%)	⬆️

... and 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.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/proxy/ProxyInstanceMetaClassSpec.groovy:103

• The test name says the call “delegates” for getTarget/initialize, but ProxyInstanceMetaClass.invokeMethod returns getProxyTarget() directly for those method names (no delegate.invokeMethod call). Removing the unused delegate stub and renaming the test avoids misleading future readers.

    void "invokeMethod resolves the target and delegates for getTarget/initialize"() {
        given:
        ProxyInstanceMetaClass metaClass = newMetaClass()
        session.retrieve(ProxyInstanceTestTarget, 11L) >> target
        delegate.invokeMethod(target, methodName, [] as Object[]) >> target

[testlens-app]

✅ All tests passed ✅

⚠️ TestLens detected flakiness ⚠️

Test Summary

CI / Build Grails-Core (macOS JDK 21) > :grails-wrapper:test

Test	Runs	Flakiness
GrailsUpdaterSpec > remote wrapper rejects HTTP redirect responses	❌ ✅	0% 🟢

🏷️ Commit: 55d4b81
▶️ Tests: 23735 executed
⚪️ Checks: 78/78 completed

---

Learn more about TestLens at testlens.app.