sbglasius opened a new issue, #16326:
URL: https://github.com/apache/grails-core/issues/16326

   Follow-up to https://github.com/apache/grails-core/pull/16296, where 
@jdaugherty pointed out that the repo carries several `ReflectionUtils`-shaped 
classes and asked for one shared home rather than another copy.
   
   That PR added `org.apache.grails.common.reflect.ReflectionUtils` in 
**grails-common** for the non-public bean access it needed, and deliberately 
stopped there. This issue covers the rest of the consolidation, which is a 
cross-module public-API change and was kept out of a bug fix on the 8.0.0 
timeline.
   
   ## What exists today
   
   | Class | Module | Notes |
   |---|---|---|
   | `org.grails.datastore.mapping.reflect.ReflectionUtils` | 
grails-datastore-core | **public, `@since 1.0`, published.** 
`makeAccessible(Field)`, `makeAccessible(Method)`, `isAssignableFrom`, 
`instantiate`, `getPropertiesOfType`, `isGetter`, `isSetter`, `forName`. 8 
internal call sites |
   | `org.grails.datastore.gorm.utils.ReflectionUtils` | 
grails-datamapping-core | one method: `isMethodOverriddenFromParent(Method)` |
   | `grails.plugin.cache.util.ClassUtils` | grails-cache | 
`getPropertyOrFieldValue(Object, String)`, delegates to Spring |
   | `grails.util.GrailsClassUtils` | grails-core | public `grails.*` API; 
carries the property/field accessor helpers |
   | `org.apache.grails.common.reflect.ReflectionUtils` | grails-common | added 
by #16296 |
   
   The 8 call sites of the grails-datastore-core one live in 
grails-datastore-core, grails-datamapping-core and grails-datamapping-rx. No 
test references it — every `ReflectionUtils` in `src/test` resolves to Spring's.
   
   ## Proposed
   
   1. Move `org.grails.datastore.mapping.reflect.ReflectionUtils` into 
`org.apache.grails.common.reflect`, merging it with the class added by #16296.
   2. Leave a deprecated delegating class at the old FQCN. It is published and 
public since 1.0, so external GORM plugins may well be compiling against it; 
removing it outright is a binary-compatibility break.
   3. Repoint the 8 internal call sites.
   4. Fold in the others where it makes sense: 
`org.grails.datastore.gorm.utils.ReflectionUtils`, 
`grails.plugin.cache.util.ClassUtils`, and the accessibility helpers on 
`grails.util.GrailsClassUtils` (that one is `grails.*` public API, so it needs 
the same deprecation treatment rather than a move).
   5. Note that grails-common must not depend on grails-core (its build file 
says so), and exposes `spring-core` as `implementation` only — so anything 
moved there must not put a Spring type in a method signature.
   
   ## Also worth revisiting here
   
   `grails-views-core` (`WritableScriptTemplate`) and `grails-views-gson` 
(`DefaultHalViewHelper`) do their own `setAccessible` widening. If the shared 
utility is the one home for this, those are candidates to repoint too.
   
   ## Removal of the workaround
   
   Separately, 
`org.apache.grails.common.reflect.ReflectionUtils.warnOnNonPublicClass` exists 
so that the non-public-class handling added in #16296 is visible and can be 
withdrawn if Groovy stops compiling anonymous inner classes as non-public. 
Whoever picks this up should keep that seam intact rather than quietly 
absorbing it.


-- 
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]

Reply via email to