jamesfredley opened a new pull request, #16101:
URL: https://github.com/apache/grails-core/pull/16101
## Summary
This change restores the Grails 7 lifecycle behavior that made the promoted
`GrailsApplication` available through `Holders` while plugin `doWithSpring`
callbacks execute.
It allows existing Grails 7-era plugins, including audit-logging 6.0.0, to
start under Grails 8 early plugin registration without requiring plugin-side
changes or `legacyCommandSupport`.
The implementation also makes failure rollback ownership-aware so a failed
application context cannot clear or overwrite a newer application published by
another context.
## User-visible failure
Applications using audit-logging 6.0.0 fail during startup on Grails
8.0.0-M5 with this path:
```text
IllegalArgumentException: GrailsApplication not found
at grails.util.Holders.getGrailsApplication
at grails.plugins.auditlogging.AuditLoggingConfigUtils.reloadAuditConfig
at grails.plugins.auditlogging.AuditLoggingGrailsPlugin.doWithSpring
at grails.boot.config.GrailsEarlyPluginRegistrationPostProcessor
```
audit-logging 6.0.0 resets and reloads its configuration from
`doWithSpring`. Its configuration helper resolves the application through
`Holders.grailsApplication`, which was valid during the equivalent Grails 7
plugin lifecycle phase.
Relevant upstream implementation:
-
[`AuditLoggingGrailsPlugin#doWithSpring`](https://github.com/apache/grails-audit-logging-plugin/blob/v6.0.0/plugin/src/main/groovy/grails/plugins/auditlogging/AuditLoggingGrailsPlugin.groovy)
-
[`AuditLoggingConfigUtils#reloadAuditConfig`](https://github.com/apache/grails-audit-logging-plugin/blob/v6.0.0/plugin/src/main/groovy/grails/plugins/auditlogging/AuditLoggingConfigUtils.groovy)
- [`ReflectionUtils` Holder-based application
lookup](https://github.com/apache/grails-audit-logging-plugin/blob/v6.0.0/plugin/src/main/groovy/grails/plugins/auditlogging/ReflectionUtils.groovy)
`legacyCommandSupport` cannot affect this failure because command
compatibility is unrelated to plugin Spring configuration timing.
## Root cause
Commit `a853ddaf6f2aafe7b8e9dfc5e661dab2c37c88e0` from #15934 introduced
`GrailsEarlyPluginRegistrationPostProcessor` so plugin beans are registered
before Spring Boot auto-configuration evaluates `@ConditionalOnMissingBean`
conditions.
That move changed the observable lifecycle order:
1. Grails 8 M5 creates and initializes the promoted
`DefaultGrailsApplication`.
2. The early post-processor calls
`pluginManager.doRuntimeConfiguration(...)`.
3. Plugin `doWithSpring` callbacks execute.
4. Only after all callbacks finish does the post-processor call
`Holders.setGrailsApplication(...)`.
Step 4 is too late for legacy plugins that access the Holder from step 3.
Grails 7 used the opposite order in `GrailsApplicationPostProcessor`: it
assigned `Holders.grailsApplication` before invoking `doRuntimeConfiguration`.
The M5 ordering therefore introduced a backward-compatibility regression even
though the exact Holder timing was a de facto plugin contract rather than a
formally documented API guarantee.
## Implementation
### Publish before plugin runtime configuration
`GrailsEarlyPluginRegistrationPostProcessor` now publishes the fully
initialized, promoted `GrailsApplication` fallback immediately before
`pluginManager.doRuntimeConfiguration(...)`.
This is the narrowest compatibility point:
- The application has already been initialized.
- Artefacts have already been registered.
- Legacy `doWithSpring` callbacks can resolve the same promoted application
that later becomes the context singleton.
- Discovery-strategy precedence remains unchanged from Grails 7.
- Plugin ordering and early bean-definition timing remain unchanged.
### Preserve failure semantics
Publishing earlier means failures can occur while a new application is
visible globally. The post-processor therefore records the exact fallback it
replaces and restores it if early registration fails.
The cleanup catches `Throwable` because Groovy closures can throw checked
exceptions without declaring them at the Java call site. Runtime exceptions and
errors retain their original identity. Checked failures are wrapped in Spring's
`BeanInitializationException` after process-wide state is restored.
`Environment.initializing` is reset on every failure path.
### Make rollback ownership-aware
An unconditional restore is unsafe when application contexts initialize
concurrently:
1. Context A publishes application A.
2. Context B publishes application B.
3. Context A fails.
4. A must not restore its prior value over B.
The private Holder fallback now uses `AtomicReference<GrailsApplication>`:
- Existing reads use `get()`.
- Existing writes and clear operations use `set()`.
- Early publication uses `getAndSet()` to capture the exact overwritten
fallback without invoking discovery strategies.
- Failure rollback uses `compareAndSet(expected, previous)` so only the
context that still owns the fallback may restore it.
Existing public Holder methods retain source and binary compatibility.
## Regression coverage
`EarlyPluginRegistrationOrderingSpec` now exercises the real public startup
path through `GrailsPluginLifecycleInitializer` and
`AnnotationConfigApplicationContext.refresh()`.
Added coverage verifies:
- A Grails 7-style plugin can read the promoted application from `Holders`
inside `doWithSpring`.
- The object exposed through `Holders` is the same application promoted into
the context.
- Runtime failures reset `Environment.initializing` and restore the prior
Holder fallback.
- Fallback exchange and restoration do not invoke application discovery
strategies.
- Checked exceptions from Groovy `doWithSpring` closures are wrapped only
after global state is restored.
- A failed context does not overwrite a newer publisher.
- A real two-thread race between conditional rollback and a concurrent
setter always leaves the newer publication installed.
- Existing early-registration ordering, bean override, single-manager-pass,
and control scenarios remain green.
The first new compatibility test was run before the production change and
reproduced the reported `IllegalArgumentException: GrailsApplication not found`
at `GrailsEarlyPluginRegistrationPostProcessor#doRuntimeConfiguration`. It
passed after the publication order changed.
## Verification
The following checks pass on Java 21 and Gradle 9.6.0:
```text
.\gradlew.bat :grails-core:test --tests
"grails.boot.config.EarlyPluginRegistrationOrderingSpec" --rerun-tasks
--offline --max-workers=7
.\gradlew.bat :grails-core:test --offline --max-workers=7
.\gradlew.bat :grails-core:codeStyle --offline --max-workers=7
```
The focused specification passes all 14 lifecycle scenarios. The complete
`grails-core` test and CLI test tasks pass.
The repository-required aggregate verification was also attempted with:
```text
.\gradlew.bat clean aggregateViolations :grails-test-report:check --continue
--max-workers=7
```
All generated violation summaries report no violations:
- Checkstyle: clean
- CodeNarc: clean
- PMD: clean
- SpotBugs: clean
The aggregate test gate has one unrelated Windows-only failure in
`:grails-test-examples-mail:integrationTest`: `MailServiceSpec.should handle
newlines in text GSP views` expects LF but receives CRLF. Rerunning that single
feature serially with `--rerun-tasks` reproduces the same CRLF mismatch. The
affected `grails-core` module is green and this change does not touch the mail
test example.
## Review evidence
The final cumulative diff was reviewed against `origin/8.0.x` by both
mandatory reviewers:
- Impact-aware Oracle: GREEN
- Codex CLI review: GREEN
The review loop specifically found and drove fixes for checked-exception
cleanup, discovery-strategy side effects, and concurrent rollback ownership
before the branch was pushed.
## Compatibility and scope
This is intentionally a framework compatibility fix rather than an
audit-logging-specific workaround. Other Grails 7-era plugins that access
`Holders.grailsApplication` during `doWithSpring` receive the same restored
lifecycle behavior.
No configuration property, command compatibility mode, dependency, or public
plugin hook changes. No documentation update is needed because this restores
prior behavior rather than introducing a new user-facing option.
--
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]
