Register plugin beans before Spring Boot auto-configuration by retiming doWithSpring#15934
Open
codeconsole wants to merge 18 commits into
Open
Register plugin beans before Spring Boot auto-configuration by retiming doWithSpring#15934codeconsole wants to merge 18 commits into
codeconsole wants to merge 18 commits into
Conversation
Plugin bean definitions previously registered through GrailsApplicationPostProcessor, a registry-discovered BeanDefinitionRegistryPostProcessor that runs after ConfigurationClassPostProcessor has expanded auto-configurations, so plugin beans had to override Boot's auto-configured defaults. Register them ahead of auto-configuration instead so that Boot's @ConditionalOnMissingBean guards back off in favour of plugin beans. GrailsPluginLifecycleInitializer (registered via spring.factories) adds GrailsEarlyPluginRegistrationPostProcessor with addBeanFactoryPostProcessor; manually-added registry post-processors are guaranteed to run before registry-discovered ones, including ConfigurationClassPostProcessor. The early phase builds the one true GrailsApplication and DefaultGrailsPluginManager from the promoted PluginDiscovery singleton, performs artefact discovery (required because core plugins iterate artefacts inside doWithSpring), drains doWithSpring/doWithRuntimeConfiguration into the registry, and promotes both singletons plus a completion marker. Application classes are resolved for early artefact discovery from the source classes GrailsApp now stashes in the context, falling back to GrailsApplicationClass bean definitions already present in the registry when the application is started through a plain SpringApplication. The default scanning logic is extracted from GrailsAutoConfiguration.classes() into ApplicationClassScanner so there is a single implementation. GrailsApplicationPostProcessor reuses the promoted grailsApplication (adopting the application class via the new DefaultGrailsApplication.setApplicationClass) and pluginManager, skips loadPlugins and the initialization sequence when the early phase ran, and no longer re-drains plugin runtime configuration; resources.groovy, resources.xml and the application's own doWithSpring still drain in GrailsApplicationPostProcessor as before. Plugins and the plugin manager are constructed exactly once. DevelopmentModeWatchSpec now supplies its watched plugin through the real PluginDiscovery bootstrap path instead of defining a second plugin manager bean, which would be ambiguous with the promoted singleton.
Introduce beanRegistrar() on GrailsApplicationLifeCycle (interface default returning null) and grails.plugins.Plugin as the modern, Spring-native way for plugins and applications to register beans, built on Spring Framework 7's org.springframework.beans.factory.BeanRegistrar (register(BeanRegistry, Environment)). GrailsPlugin gains a default getBeanRegistrar() accessor, implemented by DefaultGrailsPlugin to expose the plugin instance's registrar, mirroring how doWithSpring reaches the instance today. The early plugin registration phase applies each enabled plugin's registrar to the bean definition registry in plugin order, immediately after the doWithSpring drain and still ahead of Spring Boot auto-configuration, using BeanRegistryAdapter exactly as GenericApplicationContext.register does. The application class's registrar drains in GrailsApplicationPostProcessor at the same point as the application doWithSpring closure. doWithSpring() on GrailsApplicationLifeCycle and Plugin is deprecated since 8.0; the bean builder DSL continues to work unchanged. Documents the retimed lifecycle and the new API in the 8.0 upgrade guide and rewrites the plugin runtime-configuration guide around beanRegistrar() with doWithSpring as the deprecated alternative. Extends the ordering spec to prove a registrar bean wins a @ConditionalOnMissingBean race and that both hooks coexist on one plugin.
…ase coverage Adds an ordering spec feature proving that under the default spring.main.allow-bean-definition-overriding=true an application doWithSpring bean still replaces a plugin bean of the same name after the retimed lifecycle split the two registrations. Ports the end-to-end integration coverage to app3: the loadafter plugin registers a probe bean in plain doWithSpring and the application defines a @ConditionalOnMissingBean default for the same name; PluginBeansBeforeAutoConfigurationSpec asserts the plugin's bean wins through real plugin discovery and a real boot. Extends the 8.0 upgrade notes with the bean-definition-overriding edge case (apps explicitly disabling overriding now see BeanDefinitionOverrideException where shadowing was previously silent) and the pre-auto-configuration registry contract for plugins that inspect the registry inside doWithSpring.
The early plugin registration phase scanned each application source class's own package, but the Grails compiler injects a packageNames() override into the application class that lists every project package. Artefacts in packages other than the application class's own (e.g. a controller in a sibling package) were therefore discovered only later by GrailsApplicationPostProcessor, after the plugins' doWithSpring had already run — leaving controllers without beans and failing requests with NoSuchBeanDefinitionException. Early artefact discovery now resolves the application classes exactly the way GrailsApplicationPostProcessor does: classes() invoked on a GrailsAutoConfiguration instance (created solely to compute the scan), honoring the injected packageNames() as well as user overrides of classes(), packageNames() and limitScanningToApplication(). Sources that are not application classes no longer contribute artefacts, matching the previous lifecycle. Verified by UriMatchingInterceptorFunctionalSpec in the hibernate5 and hibernate7 app1 test examples, which exercises a controller and an interceptor living outside the application class's package.
The GORM initializers decided whether to register the open-session-in- view interceptor by checking for the dispatcherServlet bean definition. That bean is registered by Spring Boot auto-configuration, so with plugin beans now draining before auto-configuration the check always returned false and the interceptor silently stopped registering in web applications (verified: openSessionInViewInterceptor present on 8.0.x, absent on this branch before this fix). AbstractDatastoreInitializer gains isWebApplicationRegistry(), which still honours a dispatcherServlet definition when present but otherwise checks whether the registry is the web application context itself — a signal independent of auto-configuration ordering. The Hibernate 5 and Hibernate 7 initializers and the MongoDB initializer use it in place of the bean-definition check. Note: Neo4jDataStoreSpringInitializer (grails-data-neo4j, a separate Gradle build) has the identical dispatcherServlet check and needs the same one-line change once it builds against this grails-datamapping-core. New OpenSessionInViewSpec integration tests in the hibernate5 and hibernate7 app1 examples pin the interceptor's registration.
This was referenced Jul 7, 2026
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #15934 +/- ##
==================================================
+ Coverage 49.3593% 49.4471% +0.0878%
- Complexity 16777 16846 +69
==================================================
Files 1986 1991 +5
Lines 93336 93504 +168
Branches 16337 16375 +38
==================================================
+ Hits 46070 46235 +165
+ Misses 40140 40127 -13
- Partials 7126 7142 +16
🚀 New features to boost your workflow:
|
Contributor
|
I merged the latest 8.x branch changes, apologies for the test failures initially. They were due to merging up 7 -> 8 + the vulnerability fixes + the spring boot 4 vs 3 mismatches. Will review this after the tests are run. Any failures related to the merge, I'll try to fix and merge into this PR |
The controllers plugin registered its own RequestMappingHandlerMapping (annotationHandlerMapping) and RequestMappingHandlerAdapter (annotationHandlerAdapter) to support @controller annotated beans, a remnant of the pre-Boot era. In a Grails application Spring's MVC configuration always provides the fully configured requestMappingHandlerMapping and requestMappingHandlerAdapter, so the Grails pair was never consulted: the mapping sorts after Spring's order-0 mapping, and the adapter tied with Spring's on order and lost by bean registration order. With plugin beans now registering before auto-configuration, the registration order of the two adapters flipped and the DispatcherServlet began selecting the bare Grails adapter — which lacks the JSON message converters — for every annotated handler method, including the actuator endpoints. Requests to /actuator/env and /actuator/health failed with HttpMessageNotWritableException: 'No converter for EnvironmentDescriptor with preset Content-Type null', surfacing as the issue-10279 functional test regression. Removing the never-consulted duplicates restores the previous effective behavior regardless of registration order. The localeChangeInterceptor wiring on the removed mapping was equally unreachable and had no effect. Documented in the 8.0 upgrade notes. Verified by ActuatorEnvClosureSpec (passes again, differential-checked: passes on 8.0.x, failed on this branch before this change) and the full functional test selection.
…before-autoconfig # Conflicts: # grails-doc/src/en/guide/upgrading/upgrading80x.adoc
codeconsole
requested review from
jamesfredley,
jdaugherty,
matrei and
sbglasius
and removed request for
jamesfredley and
jdaugherty
jdaugherty
reviewed
Jul 8, 2026
jdaugherty
left a comment
jdaugherty
left a comment
Contributor
There was a problem hiding this comment.
I made a first pass at this review. I think this is the right direction overall.
Can we also add a test example that explicitly exercises the new registrar functionality?
- Rename ApplicationClassScanner to ApplicationArtefactScanner to reflect that it scans the application's artefact classes, not the application class - Apply plugin beanRegistrar()s on the GrailsApplicationPostProcessor fallback path so the new API behaves identically in contexts not booted through GrailsApp - Reset the initializing flag if the early registration phase throws, so a failed context does not leak the system property to later contexts - Narrow the application-scan catch to Exception | LinkageError - Resolve String application sources to classes, and recover an application class from the registry when the stash holds none - Drop the consumed source-classes stash singleton - Guard setApplicationClass as set-once - Strengthen the doWithSpring deprecation wording - Replace the linear isArtefact scan with a Set lookup - Add a trailing newline to spring.factories - Docs: lead the beanRegistrar examples with the concise closure form, add a what's new entry, and note the double application-class instantiation - Tests: closure-coercion registrar, setApplicationClass set-once, and an app3 beanRegistrar() end-to-end example
Contributor
Author
Added — the |
This comment has been minimized.
This comment has been minimized.
jdaugherty
approved these changes
Jul 8, 2026
Member
|
Note: This comment was generated by an AI coding assistant (Claude), directed by a human reviewer to verify that this PR's tests actually cover the situations described in its own description and commit messages. Findings below are grounded in specific file/line citations from the diff and the Overall, the core architectural claims are well covered: A few gaps, in rough priority order:
Flagging 1-2 as the highest priority since they mirror a regression class this PR already proved happened once. |
jdaugherty
pushed a commit
that referenced
this pull request
Jul 9, 2026
Neo4jDataStoreSpringInitializer gated the open-session-in-view interceptor on the dispatcherServlet bean definition. That bean is registered by Spring Boot auto-configuration, so under the Grails 8 lifecycle (plugin beans drain before auto-configuration, #15934) the check always returned false and the interceptor silently stopped registering in web applications. The check still honours a dispatcherServlet definition when present but otherwise tests whether the registry is the web application context itself, a signal independent of auto-configuration ordering. The logic mirrors AbstractDatastoreInitializer.isWebApplicationRegistry() added to grails-datamapping-core in #15934 (where it is exercised by the hibernate5/hibernate7 OpenSessionInViewSpec integration tests); it is inlined here because this build resolves grails-datamapping-core from published artifacts that do not yet contain the shared method, and the override becomes removable once it does.
- Assert the actuator /health endpoint also serializes to JSON (the same converter path the /env regression exposed) - Add a MongoDB open-session-in-view registration test mirroring the Hibernate one, covering the identical isWebApplicationRegistry() fix - Cover the bean-definition-overriding-disabled edge with a colliding plugin+application bean pair that must throw - Prove the early phase resets the initializing flag when it throws - Prove the early phase loads plugins with a single manager pass, and correct the upgrade note: Grails always constructs a plugin's reference and real instance, so the guarantee is one manager pass, not a single plugin instantiation
Contributor
Author
|
Thanks — this audit was accurate and genuinely useful; all five gaps are now addressed in e737052.
grails-core full suite, the ordering spec (all 9 cases), and the issue-10279 actuator spec pass locally; the mongo spec is the only one gated on CI infrastructure. |
The Mongo OSIV regression coverage added in e737052 lives in grails-test-examples/mongodb/base, which requires a real Mongo already listening on localhost:27017 and can't be verified without that external dependency. grails-data-mongodb/core already has Testcontainers wired up via AutoStartedMongoSpec (auto-starts Mongo, needs only Docker), so add MongoOpenSessionInViewSpec there, exercising MongoDbDataStoreSpringInitializer.isWebApplicationRegistry() directly against a GenericWebApplicationContext with no dispatcherServlet bean definition present yet — the exact ordering the retimed plugin lifecycle now requires — plus a non-web control case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Member
|
Note: This comment was generated by an AI coding assistant (Claude), directed by a human reviewer, as a follow-up to the coverage audit above. All 5 gaps from the audit are addressed as of e737052, and confirmed locally:
On top of that, |
borinquenkid
approved these changes
Jul 9, 2026
borinquenkid
left a comment
borinquenkid
left a comment
Member
There was a problem hiding this comment.
You did all the fixes required, we added the mongo coverage
…d check The mongodb/base example does not register mongoOpenSessionInViewInterceptor — the open-session-in-view class lives in grails-datastore-web, which that app does not depend on — so the integration test asserted a bean that is never present and failed in the MongoDB CI job. Cover the actual fix instead with a unit test of AbstractDatastoreInitializer.isWebApplicationRegistry(), the shared method the Hibernate and MongoDB initializers both use. The web-application-context branch (which needs spring-web) remains covered end-to-end by the Hibernate open-session-in-view integration tests.
…fig' into feat/plugin-beans-before-autoconfig
Contributor
Author
|
Correcting my Mongo OSIV test (it broke the MongoDB Functional Tests job): the @wbduque's I also kept a small complementary unit test ( |
…before-autoconfig # Conflicts: # grails-core/src/main/groovy/grails/plugins/Plugin.groovy
…fig' into feat/plugin-beans-before-autoconfig
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
Supersedes #15755, implementing the design agreed in its review discussion: instead of adding a new lifecycle method, the existing plugin entry point is retimed. Completes the follow-up scoped out of #15409 ("we could move plugins completely to the autoconfiguration workflow") and delivers the visible half of #14915.
doWithSpring/doWithRuntimeConfigurationnow runs before Spring Boot auto-configuration. AnApplicationContextInitializeradds a manually-registeredBeanDefinitionRegistryPostProcessor, which Spring runs ahead ofConfigurationClassPostProcessor. Plugin beans are in the registry before auto-configuration conditions are evaluated, so Boot's@ConditionalOnMissingBeanbeans defer to plugin beans — no more overriding Boot beans after the fact.GrailsApplication, built once in the early phase fromPluginDiscovery(the 7.x bootstrap infrastructure) and promoted as singletons thatGrailsApplicationPostProcessorreuses — so plugins are loaded by one manager pass rather than by a throwaway pass followed by the real one (the superseded Register plugin beans before Spring Boot auto-configuration (doWithSpringBeforeAutoConfiguration) #15755 approach). (Grails still constructs each plugin'sGrailsClassreference instance plus its real instance, as it always has.)classes()path as before, honoring the compile-timepackageNames()injection), sodoWithSpringclosures that iterate artefacts — controllers, services, interceptors — work unmodified.BeanRegistraris the new native registration API for plugins:beanRegistrar()onGrailsApplicationLifeCycle/Plugin, applied pre-refresh through Spring's ownBeanRegistryAdapter(the mechanismGenericApplicationContext.register(BeanRegistrar...)uses). It is a functional interface, so a plugin can return a closure coerced withas BeanRegistrar— no separate class needed. AOT-friendly by design.@Deprecated(since = '8.0')ondoWithSpring) pointing atbeanRegistrar(). Deprecated DSL closures keep draining through the existingRuntimeSpringConfigurationpath — just earlier — until removal. The DSL internals are deliberately not reimplemented onBeanRegistry: the DSL supports constructs with no clean equivalent (parent/abstract beans, late-resolved references,BeanConfigurationpost-processing), the plumbing is invisible to users, and that translation work would be deleted at DSL removal anyway.resources.groovy/resources.xmland the application class's owndoWithSpringdrain where they always did, so application beans still override plugin beans under the defaultallow-bean-definition-overriding=true. The one behavioral edge — apps explicitly setting itfalsenow getBeanDefinitionOverrideExceptionwhere the old single-drain merge was silent — is documented in the upgrade notes.Regressions found and fixed by verification
The retiming sweep caught two real breakages, both fixed generally in this PR:
containsBeanDefinition('dispatcherServlet')— an auto-configuration bean, always absent at the early drain.AbstractDatastoreInitializer.isWebApplicationRegistry()now also does a registration-order-independentWebApplicationContexttype check. Follow-up:Neo4jDataStoreSpringInitializerneeds the identical one-line change in its own build (separate PR).packageNames()override. Fixed by resolving application classes throughclasses()for exact parity with the previous behavior.Verification
All green, zero failures across grails-core, grails-test-suite-uber, grails-test-suite-web, grails-gsp, grails-controllers, grails-web-boot, sitemesh3, mongodb-core (testcontainers), hibernate5/7, plus the full hibernate7
app1:integrationTestandapp1:integrationTestfunctional suites.Test coverage includes: ordering specs proving a plugin
doWithSpringbean and a pluginbeanRegistrar()bean each win a@ConditionalOnMissingBeanrace (with a no-early-phase control) and that thebeanRegistrar()closure form works; app-over-plugin override under the default settings, and aBeanDefinitionOverrideExceptionwhen overriding is disabled; a single-manager-pass check (no throwaway extra load); the initializing flag resetting when the early phase throws; an end-to-end app3 integration test exercising bothdoWithSpringandbeanRegistrar()in a real booted application; Hibernate and MongoDB OSIV registration; and the actuator/envand/healthendpoints serializing to JSON. Neo4j's suite and the MongoDB example apps run in their dedicated CI jobs (independent Gradle build / require a Mongo instance); noted above.Docs: new upgrade-notes section covering the retiming, the override edge, and the DSL deprecation; a "what's new" entry; the plugin guide's runtime-configuration page documents
beanRegistrar()first (concise closure form) withdoWithSpringas the deprecated-but-working alternative.Related: #15755 (superseded), #15409 / #15408 (7.x groundwork), #14915 (bean DSL deprecation ticket), #13863 / #15916 (independent extractions from #15755).