refactor: convert grails-core to Groovy by borinquenkid · Pull Request #16586 · apache/grails-core

borinquenkid · 2026-10-11T01:27:34Z

What

Converts the remaining Java sources of grails-core to Groovy: 191 .java files removed, 241 .groovy files added (268 files changed, 16677 insertions(+), 12145 deletions(-)).

Why

The maintainers asked on #16351 for the Java-to-Groovy migration to land one Gradle project at a time as a stack of PRs, bottom-up in dependency order. #16572 did grails-data-hibernate7; this PR is 8 of 30 in the next stack (base: refactor/java-to-groovy-grails-shell-cli). The end state of every file is taken from the 9.0.x-java-to-groovy branch, which already carries the regression fixes found by CI.

Commits:

80c8329 fix: keep the Java-facing API of the converted grails-core classes
b75e1de refactor: convert grails-core to Groovy

Changed outside the converted sources

M grails-core/build.gradle

Java-facing API sweep

Every deleted .java was compared with its Groovy twin via javap -v -protected (class declaration, public/protected fields, constructors, methods incl. modifiers/throws/generics, interface default methods, nested class modifiers, and the annotations callers see). Result:

grails-core: 191 deleted .java, 9 difference(s) found and fixed in this PR (see the fix: commit), 0 left

See also the notes below. Remaining informational entries (widened visibility, e.g. Groovy property accessors): 32.

Verification

classes and testClasses (plus ast/cli/testFixtures source sets where present) of the converted project(s) and of their direct dependents compile: ['grails-async', 'grails-beans-dsl', 'grails-cache', 'grails-console', 'grails-controllers', 'grails-converters', 'grails-core-cli-legacy', 'grails-data-mongodb', 'grails-databinding', 'grails-datamapping-core', 'grails-datamapping-core-test', 'grails-datamapping-support', 'grails-datamapping-tck', 'grails-datasource', 'grails-domain-class', 'grails-encoder', 'grails-events', 'grails-geb', 'grails-gsp', 'grails-gsp-core', 'grails-layout', 'grails-logging', 'grails-mail', 'grails-openapi', 'grails-quartz', 'grails-redis', 'grails-scaffolding', 'grails-services', 'grails-sitemesh3', 'grails-spring-security', 'grails-spring-security-acl', 'grails-spring-security-oauth2', 'grails-spring-security-rest', 'grails-spring-security-rest-gorm', 'grails-spring-security-rest-grailscache', 'grails-spring-security-rest-memcached', 'grails-spring-security-rest-redis', 'grails-spring-security-ui', 'grails-startup-progress', 'grails-taglib', 'grails-test-suite-base', 'grails-testing-support-datamapping', 'grails-testing-support-views-gson', 'grails-undertow', 'grails-validation', 'grails-views-core', 'grails-views-markup', 'grails-web-common']
No test suites were run locally (the umbrella branch was verified; CI runs them). codeStyle, rat and a whole-repo build -PskipTests are run once on the tip of the stack.

Notes

Informational sweep entries that are not regressions: Groovy enums gain MIN_VALUE/MAX_VALUE/next()/previous(), property accessors appear for formerly package-private members, nested static classes with implicit constructors become public (they were protected), and generic type parameters that Groovy erases in constructors/methods (descriptors identical).
Groovy gotchas that applied (see build: migrate remaining Java sources to Groovy #16351): static final without public creates a property; == calls equals(); Closure/SAM coercions; @CompileStatic with nested classes. All CI regression fixes found on the umbrella branch are included in the end state.

🤖 Generated with Claude Code

Converts the Java sources of grails-core to Groovy. Part of the per-project split requested by the maintainers on #16351; end state taken from the 9.0.x-java-to-groovy branch (origin/9.0.x..e55fd9e). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Public constants and fields declared without 'public' became Groovy properties (DefaultResourceLocator, PluginPathAwareFileSystemResourceLoader, AbstractResourceLocatorPostProcessor, DefaultArtefactInfo.handlerData); the MockFileResource test-fixture constructors lost their 'throws UnsupportedEncodingException'. Restore both. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

codecov · 2026-10-11T02:11:25Z

Codecov Report

❌ Patch coverage is 72.52216% with 682 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.5800%. Comparing base (3a640f2) to head (203429d).

Files with missing lines	Patch %	Lines
...injection/AbstractGrailsArtefactTransformer.groovy	34.7458%	65 Missing and 12 partials ⚠️
.../grails/core/plugins/DefaultPluginDiscovery.groovy	61.8784%	55 Missing and 14 partials ⚠️
...groovy/grails/core/DefaultGrailsApplication.groovy	67.9245%	57 Missing and 11 partials ⚠️
...rc/main/groovy/grails/util/GrailsClassUtils.groovy	82.9384%	28 Missing and 8 partials ⚠️
.../org/apache/grails/core/plugins/PluginUtils.groovy	77.0370%	22 Missing and 9 partials ⚠️
.../injection/DefaultGrailsDomainClassInjector.groovy	63.0137%	19 Missing and 8 partials ⚠️
...vy/org/grails/core/DefaultGrailsDomainClass.groovy	22.2222%	21 Missing ⚠️
.../groovy/org/grails/core/AbstractGrailsClass.groovy	62.9630%	17 Missing and 3 partials ⚠️
...core/src/main/groovy/grails/util/CacheEntry.groovy	70.3125%	17 Missing and 2 partials ⚠️
...groovy/org/grails/config/NavigableMapConfig.groovy	82.2430%	16 Missing and 3 partials ⚠️
... and 56 more

Additional details and impacted files

@@                                Coverage Diff                                 @@
##             refactor/java-to-groovy-grails-shell-cli     #16586        +/-   ##
==================================================================================
+ Coverage                                     65.3008%   65.5800%   +0.2792%     
- Complexity                                      30521      31175       +654     
==================================================================================
  Files                                            2254       2254                
  Lines                                          111190     110680       -510     
  Branches                                        20387      20397        +10     
==================================================================================
- Hits                                            72608      72584        -24     
+ Misses                                          29391      28685       -706     
- Partials                                         9191       9411       +220

Files with missing lines	Coverage Δ
...ore/src/main/groovy/grails/boot/StartupTask.groovy	`100.0000% <100.0000%> (ø)`
...oot/config/GrailsPluginLifecycleInitializer.groovy	`100.0000% <100.0000%> (ø)`
...ls/compiler/ast/GrailsArtefactClassInjector.groovy	`100.0000% <100.0000%> (ø)`
...main/groovy/grails/core/DefaultArtefactInfo.groovy	`93.5484% <100.0000%> (ø)`
...ails/core/support/proxy/DefaultProxyHandler.groovy	`100.0000% <100.0000%> (ø)`
...ls-core/src/main/groovy/grails/util/Holders.groovy	`86.7470% <100.0000%> (ø)`
...ain/groovy/grails/web/CamelCaseUrlConverter.groovy	`100.0000% <100.0000%> (ø)`
...ore/aot/AbstractBeanDefinitionExcludeFilter.groovy	`100.0000% <100.0000%> (ø)`
...ot/AutowireModeBeanRegistrationAotProcessor.groovy	`100.0000% <100.0000%> (ø)`
...he/grails/core/internal/util/TypeConverters.groovy	`81.9048% <100.0000%> (ø)`
... and 127 more

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

…groovy-grails-core

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

sbglasius

Review of the Java→Groovy conversion. The translation itself looks faithful: identity checks moved to .is(), Map-backed config classes use @POJO, and field writes use this.@. The comments below cover a hot-path performance regression, a couple of bugs that were carried over, test-isolation and convention issues, and two public-API shape changes.

sbglasius · 2026-10-13T09:49:39Z

+
+        @Override
+        Object invoke(Object controller) throws Throwable {
+            return handle.invokeWithArguments(controller)


Efficiency: handle.invoke(controller) became handle.invokeWithArguments(controller). That is the generic varargs path: it allocates an array, boxes, and adapts the type on every call. In any non-development environment this runs on every controller action dispatch, so it undoes much of the gain MethodHandles had over the ReflectionInvoker. A cheaper option is to adapt the handle once (e.g. mh.asType(MethodType.methodType(Object, Object))) and use a typed invoker or LambdaMetafactory, or keep this invoker in Java.

sbglasius · 2026-10-13T09:49:39Z

+    static void reset() {
+        setPluginManager(null)
+        setGrailsApplication(null)
+        setServletContext(null)


Correctness: reset() calls setServletContext(null) without a guard, so it throws a NullPointerException whenever servletContexts was never created, i.e. outside a servlet environment. It throws after the plugin manager and application are cleared but before setPluginManagerInCreation(false) and setConfig(null) run, so config is left behind. clear() already guards with servletContexts != null; reset() should do the same. The new HoldersSpec asserts the NPE instead of fixing it.

sbglasius · 2026-10-13T09:49:39Z

+    def <T> T getProperty(String key, Class<T> targetType, T defaultValue, List<T> allowedValues) {
+        T v = getProperty(key, targetType, defaultValue)
+        if (!allowedValues.contains(v)) {
+            throw new GrailsConfigurationException('Invalid configuration value [\$value] for key [\${key}]. Possible values \$allowedValues')


Correctness: These messages are single-quoted with escaped \$value / \${key} / \$allowedValues, so they print the placeholders literally. There is also no value variable here; the local is v. The same issue is at lines 240 and 249 and at NavigableMapConfig.groovy:207. Now that this is Groovy, a GString such as "Invalid configuration value [${v}] for key [${key}]. Possible values ${allowedValues}" would fix it.

sbglasius · 2026-10-13T09:49:39Z

+        // clear(), unlike reset(), guards its servlet-context write and is therefore safe to call
+        // outside a servlet environment (grails-core has no grails.web.context.WebRequestServletHolder
+        // on its classpath, so Holders.servletContexts is never populated here)
+        Holders.clear()


Test isolation: Holders.clear() also empties the static applicationDiscoveryStrategies list, which is loaded once from GrailsFactoriesLoader when the class initialises, and nothing restores it. Forks are reused across specs, so any spec that runs after this one in the same JVM loses every factory-registered strategy and can fail depending on test order. Consider saving and restoring the strategies, or resetting only the holders this spec sets.

sbglasius · 2026-10-13T09:49:39Z

+    }
+
+    private static List<TypeFilter> includeFiltersOf(ClassPathBeanDefinitionScanner scanner) {
+        Field field = ClassPathScanningCandidateComponentProvider.getDeclaredField('includeFilters')


Conventions: This reads Spring's private includeFilters field by reflection, and GrailsContextNamespaceHandlerSpec:40 does the same with NamespaceHandlerSupport.parsers. The spec also calls the protected createScanner directly. CLAUDE.md rule 9 says: "Test via public APIs - ... never invoke internal implementations, package-private methods, or bypass the public surface directly". These tests will also break if Spring renames its private fields.

sbglasius · 2026-10-13T09:49:39Z

+     */
+    @POJO
+    @CompileStatic
+    static class GrailsPluginClass extends AbstractGrailsClass {


API compatibility: GrailsPluginClass was a public non-static inner class in Java and is now static, which changes its constructor signature and binary shape. Precompiled code that creates it through an outer instance would break. Was this intended? If not, it should keep its previous shape, in line with 80c8329, which aimed to keep the Java-facing API.

sbglasius · 2026-10-13T09:49:39Z

+@CompileStatic
+class Pair<A, B> {
+
+    final A aValue


API compatibility: The Java package-private fields aValue / bValue (same in Triple) are now Groovy properties. The fields become private, and Groovy generates public getAValue() / getBValue() next to the hand-written getaValue() / getbValue(). @PackageScope final A aValue, as used in InstanceFactoryBean, would keep the original shape.

sbglasius · 2026-10-13T09:49:39Z

+        def loader = new SimpleMapResourceLoader()
+
+        expect:
+        loader.getClassLoader() == SimpleMapResourceLoader.classLoader


Test coverage: This compares against the same expression the implementation returns (SimpleMapResourceLoader.classLoader), so the assertion can't fail. Comparing against an independent value such as loader.getClass().getClassLoader() would make it meaningful.

- DefaultGrailsControllerClass: adapt each action MethodHandle once to the (Object[])Object shape Groovy emits for invokeExact, so production dispatch is an exact direct invocation again instead of invokeWithArguments - Holders.reset(): guard the servlet-context write like clear() does, so it no longer throws outside a servlet environment and leaves config behind - CompositeConfig / NavigableMapConfig / ArtefactTypeAstTransformation: turn the messages that were literal "$value" placeholders in Java into GStrings - AbstractGrailsPlugin.GrailsPluginClass: back to a package-private, non-static inner class; DefaultGrailsPlugin passes the enclosing instance explicitly - Pair / Triple: keep the package-private fields (@PackageScope) instead of Groovy properties that would add getAValue()/getBValue() next to getaValue() - Specs: HoldersSpec restores the factory-loaded discovery strategies after clear() and asserts reset() works; the component-scan parser and namespace handler specs drive Spring through XML parsing and public APIs instead of reading private fields; SimpleMapResourceLoaderSpec compares against an independent value; new cases cover the message texts, inherited and non-String actions, and the package-private Pair/Triple shape

Faithfulness pass over the 191 converted files of grails-core: - identity comparisons that had become equals(): GrailsConversionServiceConverters (ApplicationContext), DefaultGrailsApplication (applicationClass) - field reads on another instance that had become getter calls: AbstractGrailsPlugin.equals, GrailsTransactionAttribute copy constructor - DefaultResourceLocator: plugin file-system name compared with equals() as in Java - package-private members that had become public: GrailsConversionServiceConverters, SynchronizationManager, SpringTransactionSynchronizationManager, the ChainedTransactionManager test constructor, GrailsClosureRuntimeHints.enclosingLoads, DefaultPluginDiscovery.filterPlugins/populatePlugins, the PluginInfo constructor, PluginMetadata.canRegisterPlugin/supportsEnvironment, PluginXmlHandler, AbstractGrailsPluginManager.getConfigSlurper, ClassRelativeClassLoader, DefaultResourceLocator.PluginResourceInfo, PluginAstReader, EnvironmentAwarePropertySource, GrailsPluginSorter.sortPlugins - GrailsEnvironmentPostProcessorSpec builds its PluginInfos through a same-package test factory now that the constructor is package-private again

jdaugherty

Pushed the fixes for the review feedback in 477cbb0 and a faithfulness pass over all 191 converted files in 05370e9 (identity checks, cross-instance field reads and package-private members the conversion had widened). Two observations below that were left as they are.

jdaugherty · 2026-10-13T10:41:29Z

+            return Collections.emptyList()
+        }
+        if (paramValues.getClass().isArray()) {
+            return Arrays.asList((Object[]) paramValues)


(Object[]) paramValues behaves differently here than it did in Java. The static type of paramValues is Object, so Groovy routes the cast through its coercion rules rather than a plain checkcast: a primitive array such as int[] is boxed element by element into an Object[] and returned as a list, where the Java version threw ClassCastException. Verified with Groovy 5.1.3 and 6.0.0: toList(new int[] {1, 2}) returns [1, 2].

The more lenient result seems fine for a parameter-value helper, so no change requested; this is just to make the divergence a deliberate one. If the Java behaviour should be kept exactly, Object[].class.cast(paramValues) restores the ClassCastException.

Kept deliberately. toList normalises request parameter values, so listing a primitive array is the more useful behaviour than the ClassCastException the Java version threw. TypeConvertersSpec now pins it (1f67931), with a comment saying the difference is on purpose.

jdaugherty · 2026-10-13T10:41:29Z

+"""
+        XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(registry)
+        // the Grails schema is not on the classpath, so parse namespace-aware without XSD validation
+        reader.validating = false


Not introduced by this PR, but recorded here because the spec has to work around it: META-INF/spring.schemas maps http://grails.org/schema/context/grails-context.xsd to org/grails/spring/context/annotation/grails-context.xsd, and that file lives under src/main/groovy, which the build does not treat as a resource directory. It is missing from the published grails-core jar (checked grails-core-8.1.0-SNAPSHOT.jar: only spring.schemas is there), so an XSD-validating parse of <grails:component-scan> cannot resolve its schema. That is why this spec parses namespace-aware with validation off.

Packaging the XSD (for example processResources { from('src/main/groovy') { include '**/*.xsd' } }) is a separate fix and worth its own issue.

Filed as #16635, with the processResources snippet. Once it is packaged the specs that parse <grails:component-scan> can validate against the schema again.

borinquenkid · 2026-10-13T10:50:34Z

@jdaugherty feel free to approve , although I wont merge until the stack is ready

…groovy-grails-core

…e array case The Java original threw ClassCastException for a primitive array; Groovy's cast to Object[] boxes it instead. Keep that on purpose and say so in a spec. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

…nc9/refactor/java-to-groovy-grails-core # Conflicts: # grails-core/src/main/groovy/org/grails/config/yaml/YamlPropertySourceLoader.groovy

…ava-to-groovy-grails-core # Conflicts: # grails-core/src/test/groovy/grails/util/GrailsUtilTests.groovy

…/java-to-groovy-grails-core

Rebuild the grails-core main/test files from their original .java content with semicolons kept and CodeNarc UnnecessarySemicolon suppressed, so GitHub's default-threshold rename detection recognizes these as moves rather than delete+add (see matrei's review on #16576). Fixes a regression the mechanical conversion introduced in LazyMetaPropertyMap.groovy: isExcluded()'s `return` sat alone on its own line with the boolean expression starting on the next line. Java tolerates that (semicolon-terminated), but Groovy treats a bare `return` on its own line as a complete statement, so the method always returned false and let 'class' leak into keySet(). Fixed by keeping the first operand on the same line as `return`, confirmed via grails-core's full test suite (keySet excludes configurational and static properties now passes) and a sweep of all 20 files plus modules 1-7 for the same pattern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@pojo

The stage-2 rebuild from the .java originals (which carry no annotations) silently dropped @CompileStatic on ArtefactHandlerAdapter, Holders, AbstractGrailsApplication, SpringResource, BeanCreationProfilingPostProcessor and GrailsTransactionAttribute, and both @CompileStatic and @pojo on CompositeConfig and PrefixedConfig. Stage 1 had added them on purpose: @pojo keeps the two Config implementations from becoming GroovyObjects (their dynamic property lookups must reach Config, not the metaclass), and the static compilation keeps the self-calls out of the indy call-site path that NPEs when such classes are entered cold via reflection. Restored with their imports; similarity stays at R056-R078. grails-core 1229 tests (3 skipped), 0 failures, codenarc clean. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…/java-to-groovy-grails-core

…groovy-grails-core CompositeConfig.groovy already carried upstream bd67495, so the re-added .java is dropped. GrailsUtilTests takes the 10.0.0-SNAPSHOT version. The five specs upstream added with the same names as this branch's (LazyMetaPropertyMap, GrailsClassUtils, CompositeConfig, PrefixedConfig, StopWatch) are kept verbatim with the feature methods only this branch asserted appended; the ones asserting the old CompositeConfig aggregate semantics are dropped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

borinquenkid and others added 2 commits October 10, 2026 14:27

borinquenkid requested review from jamesfredley and jdaugherty and a balanced review from Copilot October 11, 2026 01:27

github-project-automation Bot added this to Apache Grails Oct 11, 2026

Copilot AI reviewed Oct 11, 2026

jdaugherty added this pull request to stack #16598 October 11, 2026 07:55

borinquenkid added this to the grails:9.0.0-M1 milestone Oct 11, 2026

borinquenkid and others added 2 commits October 11, 2026 15:02

Merge refactor/java-to-groovy-grails-shell-cli into refactor/java-to-…

25df45a

…groovy-grails-core

test(startup-progress): report the answers a timed-out awaitStatus saw

f0e5a54

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

sbglasius reviewed Oct 13, 2026

View reviewed changes

jdaugherty added 2 commits October 13, 2026 11:37

jdaugherty reviewed Oct 13, 2026

View reviewed changes

borinquenkid and others added 2 commits October 13, 2026 06:37

Merge refactor/java-to-groovy-grails-shell-cli into refactor/java-to-…

f3f234b

…groovy-grails-core

borinquenkid mentioned this pull request Oct 13, 2026

grails-core: package grails-context.xsd in the jar so <grails:component-scan> can be schema-validated #16635

Open

Merge branch 'sync9/refactor/java-to-groovy-grails-shell-cli' into sy…

46c2ba3

…nc9/refactor/java-to-groovy-grails-core # Conflicts: # grails-core/src/main/groovy/org/grails/config/yaml/YamlPropertySourceLoader.groovy

borinquenkid modified the milestones: grails:9.0.0-M1, grails:10.0.0-M1 Oct 16, 2026

borinquenkid and others added 7 commits October 15, 2026 21:46

Merge sync10/refactor/java-to-groovy-grails-shell-cli into refactor/j…

66cab19

…ava-to-groovy-grails-core # Conflicts: # grails-core/src/test/groovy/grails/util/GrailsUtilTests.groovy

Merge branch 'refactor/java-to-groovy-grails-shell-cli' into refactor…

cb0dd98

…/java-to-groovy-grails-core

Merge branch 'refactor/java-to-groovy-grails-shell-cli' into refactor…

914b983

…/java-to-groovy-grails-core

Merge branch 'refactor/java-to-groovy-grails-shell-cli' into refactor…

2860836

…/java-to-groovy-grails-core

Merge branch 'refactor/java-to-groovy-grails-shell-cli' into refactor…

aec8161

…/java-to-groovy-grails-core

This comment has been minimized.

Sign in to view

Conversation

borinquenkid commented Oct 11, 2026

What

Why

Changed outside the converted sources

Java-facing API sweep

Verification

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sbglasius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdaugherty left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

borinquenkid commented Oct 13, 2026

Uh oh!

This comment has been minimized.

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Oct 11, 2026 •

edited

Loading