Add method-based doWithSpring(BeanBuilder) plugin lifecycle hook#15939
Merged
jdaugherty merged 3 commits intoJul 9, 2026
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #15939 +/- ##
==================================================
- Coverage 49.3909% 49.3797% -0.0112%
- Complexity 16808 16811 +3
==================================================
Files 1993 1993
Lines 93495 93504 +9
Branches 16360 16362 +2
==================================================
- Hits 46178 46172 -6
- Misses 40180 40197 +17
+ Partials 7137 7135 -2
🚀 New features to boost your workflow:
|
codeconsole
force-pushed
the
feat/plugin-dowithspring-method
branch
from
8b95c4c to
21a937d
Compare
jdaugherty
reviewed
Jul 8, 2026
|
It is recommended to throw an error if both forms are defined to prevent ambiguous configuration. Documenting the closure form as legacy and issuing a deprecation warning is a standard practice to guide users toward the preferred approach while maintaining backward compatibility. |
Member
|
(Analysis assisted by Claude Code — I reviewed and am posting the findings below, but flagging the tooling for transparency.) Nice feature — the method-based However, the PR description claims broader coverage than the diff actually delivers, and I think that gap is worth closing (or the description narrowing) before merge. The hook isn't wired into
|
codeconsole
force-pushed
the
feat/plugin-dowithspring-method
branch
from
21a937d to
1a1b9e8
Compare
Introduce a statically-compilable alternative to the closure-returning
doWithSpring() hook on the Plugin base class. Plugins can now register
beans by overriding doWithSpring(BeanBuilder) and calling the builder
directly, instead of returning a closure whose delegate the container
wires up at runtime.
The new hook is a no-op on the Plugin base class, and DefaultGrailsPlugin
dispatches it for Plugin descriptors. A plugin may override either
doWithSpring() or doWithSpring(BeanBuilder), but not both: the two forms
are alternatives, so defining both throws a PluginException and fails
plugin load rather than silently combining or preferring one. The legacy
closure-returning form and the legacy closure-property form on non-Plugin
descriptors remain fully supported.
The hook is intentionally scoped to the Plugin class and is NOT added to
the GrailsApplicationLifeCycle interface: that interface is map-coerced in
the testing support (GrailsApplicationBuilder builds
`[doWithSpring: {...}] as GrailsApplicationLifeCycle`), and Groovy's
map-to-interface proxies do not honour Java default methods, so an
interface-level hook would break every unit test that builds an
application context.
Adds PluginDoWithSpringMethodSpec covering the Plugin no-op, direct
method-form registration, the DefaultGrailsPlugin dispatch for the method
form and the legacy closure form, and the PluginException raised when both
forms are defined. Documents the new hook in the plugin runtime-
configuration guide.
codeconsole
force-pushed
the
feat/plugin-dowithspring-method
branch
from
1a1b9e8 to
029aa5e
Compare
Contributor
Author
|
Thanks — you're right, the description had drifted ahead of the diff. I took your option 2 and narrowed both the description and the scope: the hook lives on For the record on why not option 1 (wire it into // GrailsApplicationBuilder
super([doWithSpring: { -> doWithSpringClosure }] as GrailsApplicationLifeCycle, ...)and Groovy's map-to-interface proxies don't honour Java Separately, per @jdaugherty's review, defining both hook forms is now a hard error ( |
jdaugherty
approved these changes
Jul 9, 2026
🚨 TestLens detected 1 failed test 🚨Here is what you can do:
Test SummaryCI / Functional Tests (Java 21, indy=true) > :grails-test-examples-app1:integrationTest
🏷️ Commit: 13bc85c Test FailuresErrorsFunctionalSpec > Test Interceptor.afterView() has access to the exception if thrown (:grails-test-examples-app1:integrationTest in CI / Functional Tests (Java 21, indy=true))
Muted TestsNote Checks are currently running using the configuration below. Select tests to mute in this pull request: 🔲 ErrorsFunctionalSpec > Test Interceptor.afterView() has access to the exception if thrown Reuse successful test results: 🔲 ♻️ Only rerun the tests that failed or were muted before Click the checkbox to trigger a rerun: 🔲 Rerun jobs Learn more about TestLens at testlens.app. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a method-based
doWithSpring(BeanBuilder)hook to thegrails.plugins.Pluginbase class, as an alternative to the closure-returningdoWithSpring(). This is the plugin analog of the method-based TagLib syntax added in #15465, applied to plugin Spring configuration.Instead of returning a closure whose delegate the container wires up at runtime, a plugin can register beans directly against the supplied builder:
Scope
The hook is on the
grails.plugins.Pluginbase class and is dispatched byDefaultGrailsPluginfor plugin descriptors. It is intentionally not added to theGrailsApplicationLifeCycleinterface: that interface is map-coerced in the testing support (GrailsApplicationBuilderbuilds[doWithSpring: {…}] as GrailsApplicationLifeCycle), and Groovy's map-to-interface proxies do not honour Java default methods, so an interface-level hook would break every unit test that builds an application context. This applies to plugin descriptors (Pluginsubclasses), not to the application class or arbitraryGrailsApplicationLifeCycleimplementors.Backwards compatibility
Fully preserved. The new hook is a no-op on
Plugin, so existing plugins are unaffected.DefaultGrailsPlugininvokes both hooks for aPlugindescriptor, so a plugin may overridedoWithSpring(),doWithSpring(BeanBuilder), or both. The legacy closure-returning form and the legacydef doWithSpring = { }closure-property form on non-Plugindescriptors remain fully supported.Defining both forms
A plugin must define at most one of
doWithSpring()ordoWithSpring(BeanBuilder). Both forms are explicit overrides and are alternatives, so defining both throws aPluginExceptionand fails plugin load rather than silently combining or preferring one — surfacing what is almost always an incomplete migration between the two forms. (This deliberately differs from the auto-detected TagLib case in #15465, where method-tags are convention-discovered; see the discussion thread.)Tests
PluginDoWithSpringMethodSpeccovers: thePluginno-op default, direct method-form registration against aBeanBuilder, theDefaultGrailsPlugindispatch for the method form / both hooks / legacy closure form, and the warning emitted (and not emitted) depending on whether both forms are defined../gradlew clean aggregateViolationsreports no Checkstyle / CodeNarc / PMD / SpotBugs violations.Docs
Documents the new hook and the both-forms behavior in the plugin runtime-configuration guide (
grails-doc).