Upgrade to Gradle 9.8.0 - #16403
Upgrade to Gradle 9.8.0#16403codeconsole wants to merge 116 commits into
Conversation
Replace the Jerry-based spec assertions with direct output checks and remove the unused jodd-wot test dependency wiring. Assisted-by: opencode:gpt-5.5
Register a OncePerRequestFilter for X-Content-Type-Options, X-Frame-Options, Referrer-Policy, optional HSTS on secure requests, and optional CSP. Configurable via grails.security.headers.* with opt-out and ConditionalOnMissingBean. Assisted-by: Sisyphus:xai/grok-4.5 [gpt-coding]
The Copilot suggestion to apply the headers in a post-chain finally block would skip every security header on responses that commit during the chain (sendRedirect, sendError, flushBuffer, streaming), which is exactly where the headers are needed. Keep applying the headers eagerly before the filter chain (values already present still win, explicit downstream setHeader still replaces) and add a regression test proving the headers are present on a redirect-committed response. Assisted-by: Sisyphus:openai/gpt-5.6-terra [gpt-coding]
OncePerRequestFilter.shouldNotFilterErrorDispatch() defaults to true, so the filter was skipped on ERROR redispatches even though the ERROR dispatcher type is registered, leaving error responses without the security headers. Override it to return false and cover the behavior with a DispatcherType.ERROR test. Assisted-by: Sisyphus:openai/gpt-5.6-terra [gpt-coding]
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
Assisted-by: opencode:gpt-5.6-sol
…n spec Copilot's 5 inline comments claiming the acronym naturalName expectations were wrong (e.g. 'HTMLC ontroller') are false positives - verified by running the spec: GrailsNameUtils.getNaturalName genuinely produces those values, and jdaugherty's review already confirmed this. No change needed there. jdaugherty's own feedback was substantive and is addressed here: - Rename ArtefactNamePrecomputationSpec -> ArtefactNamingContractSpec: "precomputed" was aspirational, since nothing in the spec exercises actual precomputation, only naming stability. - Add a comment above the acronym-heavy naturalName assertions explaining the quirky-but-intentional GrailsNameUtils splitting behavior they pin, so a future reader doesn't "fix" the expectations or the algorithm without realizing this spec exists to catch exactly that change. - Add three cases exercising the artefact detection contract (ArtefactHandler#isArtefactClass), which the original spec bypassed entirely by constructing GrailsClass wrappers directly: an abstract controller is rejected (ControllerArtefactHandler's allowAbstract is false), a suffix-matching concrete controller is accepted, and a domain-named class with no @Entity/@ArteFact annotation is rejected by DomainClassArtefactHandler - this is the part a naming precomputation refactor is most likely to disturb, and the prior spec gave it no coverage at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Revert the unresolved-dependency log from WARN back to ERROR, per
jdaugherty and davydotcom (davydotcom revised an initial approval to
CHANGES_REQUESTED specifically for this): unresolved dependencies are
a real load failure - the plugin already lands in failedPlugins - and
the prior code logged it at ERROR. WARN risked being filtered out of
environments that only surface errors.
- Fix logUnresolvedDependencies() misclassifying a dependency as
"missing" whenever it isn't yet registered, even when the dependency
plugin actually exists but itself failed to load or is still waiting
in the delayed-load queue (flagged independently by Copilot's inline
review and bito-code-review's bot comment; verified by tracing
loadDelayedPlugins()'s processing order). Now distinguishes three
cases: registered with an incompatible version (unchanged), found in
failedPlugins ("failed to load"), found in delayedLoadPlugins ("is
still pending load"), and only "is missing" when none of the above.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… gaps
Addresses jdaugherty's 3 unresolved review findings, all confirmed
against the current code before fixing:
- Spring Security precedence (verified inverted doc claim): this filter
registers at GrailsFilters.LAST (order -110), before Spring Security's
chain (-100), and applied its defaults eagerly - so Spring Security's
header writers (which skip a header that's already present) never got
a chance to win, contrary to what the upgrade notes claimed. Fixed by
adding @ConditionalOnMissingClass("...HeaderWriterFilter") to
GrailsSecurityHeadersAutoConfiguration so the whole auto-configuration
backs off when Spring Security's header-writing infrastructure is on
the classpath, per jdaugherty's suggested option - Spring Security
already ships its own configurable header defaults. Added a
spring-security-web test-only dependency and a
FilteredClassLoader-based helper so the existing "Spring Security
absent" tests keep exercising that path explicitly, plus a new test
confirming the back-off.
- HSTS silently never sent behind a TLS-terminating reverse proxy
(request.isSecure() is false unless server.forward-headers-strategy
is configured): this isn't a code bug - request.isSecure() is the
correct check, and Spring Boot's forward-headers-strategy is the
standard way to make it proxy-aware - so documented the proxy case
and the mitigation in the security guide instead of changing behavior.
- Reverse-proxy header duplication (nginx add_header appends rather than
replaces, so the client can receive a header twice once the app also
sends it): also not something a code change here can generally detect
or fix, since the proxy operates entirely outside the request
pipeline. Documented the risk (with the concrete Chrome/CSP/
Referrer-Policy duplicate-header failure modes jdaugherty identified)
and the mitigation (disable the affected header(s) at the app and let
the proxy own them, or strip the app's copy at the proxy).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jdaugherty flagged the string-based assertions this PR introduced as fragile (exact-serialization matching instead of semantic checks), and Copilot independently raised the same concern per-assertion. jamesfredley proposed the trade-off directly: stay dependency-free with tightened (regex) assertions, or trade the removed jodd-wot for a real, modern parser (Jsoup). jdaugherty chose Jsoup explicitly, anticipating more HTML-structure-sensitive tests from planned refactoring. Reimplements the three DefaultFieldTemplateSpec assertions against org.jsoup instead of raw strings, restoring the exact semantic checks the original jodd-wot/Jerry version had (root div has the fieldcontain/ error/required class tokens - order and other-attributes insensitive - label text/for-attribute, label immediately precedes the input, and the required-indicator span's text) rather than the newly-added brittle literal-markup comparisons. jsoup has no existing version management in this repo; added jsoupVersion to gradle.properties following the same pattern already used for javassistVersion/jnrPosixVersion, since it's a single-module, test-only dependency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…m.err DefaultStackTraceFilterer.STACK_LOG routes through commons-logging, which resolves to a jcl-over-slf4j binding on this classpath -- so its output never touches System.err, regardless of test ordering or timing. Swapping System.err therefore never observes the emitted message, making GrailsUtilStackFiltererSpec and GrailsBootstrapRegistryInitializerSpec fail deterministically. Attach a ListAppender directly to the public STACK_LOG_NAME logger instead, which is unaffected by which commons-logging backend wins the classpath. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ystem.err capture Address jamesfredley's review on apache#16067: this was the one remaining latent instance of the same fragility -- three sites asserting on rendered System.err output for the STACK_LOG and GrailsExceptionResolver loggers, including one match on the literal console layout string 'ERROR StackTrace '. It passed only because grails-web-mvc's test classpath happened to carry slf4j-simple, whose SYS_ERR output choice re-reads System.err per call rather than caching it -- the same trap that broke these tests in grails-core once that module got a deterministic logback-test.xml. Swap the module's test logging binding from slf4j-simple to grails-core's test fixtures (which bring logback-classic transitively), and rewrite the three specs to assert on captured ILoggingEvents instead of console text -- including inspecting each event's throwableProxy stack frames directly rather than counting substrings in rendered output. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tribute copy constructors Mirrors the CustomizableRollbackTransactionAttribute fix (4042a87) split out of PR apache#15779 review, applied to the web-tier twin in org.grails.transaction. The RuleBasedTransactionAttribute overload now delegates to Spring's own copy constructor (super(other)), which snapshots the rule list from the field without invoking the source's lazy getRollbackRules() (which would mutate the source by assigning a new list into it). The TransactionDefinition and TransactionAttribute overloads recover the dynamic type and snapshot rules through a temporary Spring copy, so the source is never mutated on any path. All paths now explicitly carry the attribute-level state that Spring 7's DefaultTransactionAttribute copy constructor does not: descriptor, timeoutString, qualifier, and labels (defensively copied, since setLabels stores the given reference), plus inheritRollbackOnly. Also fixes GString-style placeholders ("$ex", "$winner") in trace logging that never interpolated in this .java source, and guards the remaining trace call behind isTraceEnabled(). Covered by GrailsTransactionAttributeSpec (copy independence and state preservation for every constructor dispatch path, including statically dispatched entries). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…kTransactionAttribute copy constructors
The RuleBasedTransactionAttribute and CustomizableRollbackTransactionAttribute
overloads now delegate to Spring's own copy constructor (super(other)),
which snapshots the rule list from the field without invoking the
source's lazy getRollbackRules() (which would mutate the source by
assigning a new list into it). The TransactionDefinition and
TransactionAttribute overloads recover the dynamic type and snapshot
rules through a temporary Spring copy, so the source is never mutated
on any path.
All paths now explicitly carry the attribute-level state that
Spring 7's DefaultTransactionAttribute copy constructor does not:
descriptor, timeoutString, qualifier, and labels (defensively copied,
since setLabels stores the given reference), plus connection and
inheritRollbackOnly.
Also fixes GString-style placeholders ("$ex", "$winner") in trace
logging that never interpolated in this .java source, and guards the
remaining trace call behind isTraceEnabled().
Covered by CustomizableRollbackTransactionAttributeSpec (copy
independence and state preservation for every constructor dispatch
path) and TransactionRollbackRulePropagationSpec (behavior through
GrailsTransactionTemplate and DefaultTransactionService, verifying
NoRollbackRuleAttribute rules survive the conversion).
Split out of the GormRegistry consolidation per review on apache#15779.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tribute copy constructors The TransactionAttribute and TransactionDefinition overloads previously copied only the five TransactionDefinition fields (propagation, isolation, timeout, readOnly, name), silently dropping rollback rules, qualifier, labels, descriptor and timeoutString. The RuleBasedTransactionAttribute overload delegated to super(other), which carries the rules but still loses the attribute-level state because Spring 7's DefaultTransactionAttribute(TransactionAttribute) copy constructor only copies the TransactionDefinition fields. The TransactionAttribute overload now delegates to the TransactionDefinition overload, which recovers the dynamic type via instanceof and snapshots rollback rules through a temporary RuleBasedTransactionAttribute copy - reading the source's rule field without invoking its lazy getRollbackRules(), which would mutate the source by assigning a new list into it. All paths now explicitly carry descriptor, timeoutString, qualifier and labels (defensively copied, since setLabels stores the given reference), plus inheritRollbackOnly when the source is a GrailsTransactionAttribute. Mirrors the CustomizableRollbackTransactionAttribute fix split out of the GormRegistry consolidation per review on apache#15779. Covered by GrailsTransactionAttributeSpec (copy independence and state preservation for every constructor dispatch path, including the statically dispatched TransactionDefinition/TransactionAttribute entries). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Spring Framework 7 no longer converts a configuration Map into a type annotated @builder(builderStrategy = SimpleStrategy), so nested settings failed to bind with ConverterNotFoundException. ConfigurationBuilder now instantiates the target type and populates it from the Map. The gap is demonstrable on this branch: with the previous ConfigurationBuilder and only the new spec applied, six scenarios fail with "Expected exception of type 'ConfigurationException', but got 'ConverterNotFoundException'". 9.0.x resolves spring-core 7.0.8 via Spring Boot 4.1.0. The fallback is deliberately narrow, and every guard below exists because removing it produced an observable failure: - It engages only when the cause chain contains ConverterNotFoundException, so a converter that deliberately rejects a Map is not bypassed. - ConfigurationException is never suppressed, so unknown-key and malformed-value failures still surface instead of being masked by the original conversion exception. - A failure while resolving the raw value throws rather than silently falling back, so configuration whose lookup failed is not quietly accepted. - The instance inherits from the fallback before overrides are applied, and each nested level receives its own fallback child, so overriding one field does not discard the rest. - Values are converted to the target property type, including the case-insensitive enum path, so multiTenancy.mode: database still binds. - Class-typed entries resolve through the thread context class loader, the same route the top-level Class handling uses, because the resolver's converter resolves against the framework class loader and would leave an application class such as hibernate.configClass unbound. - Types that are themselves a Map keep arbitrary entries. HibernateSettings extends LinkedHashMap precisely to carry keys like hibernate.hbm2ddl.auto, which strict property-only binding would have rejected. - Flattened descendant keys are bound once through their parent rather than rejected, since the resolver flattens nested configuration; a dotted key whose first segment is unknown is still rejected. - Setters are invoked with an explicit single-element argument array so an explicit null clears an inherited value. ConfigurationBuilderSpec grows from 10 to 22 specs covering each of the above. Known limitation: a PropertyResolver that exposes only an aggregate map, and not its entries as dotted properties, can still yield null for a configured scalar. Grails' own DatastoreUtils.createPropertyResolver flattens and is unaffected. Binding the raw value unconditionally was rejected as a fix because it would bypass the type conversion above. Assisted-by: claude-code:claude-opus-5
A controller's views live under GrailsNameUtils' logical property name, which keeps a name beginning with two capitals unchanged, so APIController resolves API/show.gsp. Decapitalizing wrote aPI/ instead - precompiled, never rendered - and pointed both the application-view and plugin-index checks at the wrong key. The domain's property name is bound through GrailsNameUtils too, as the runtime model builder binds it.
BinaryGrailsPlugin takes views.properties beside the plugin descriptor first and falls back to gsp/views.properties, reading one per plugin; probe the same two in the same order. The index key prefix now comes from GroovyPagePlugin.VIEWS_SERVER_PATH, the value compileGroovyPages is given, so the two cannot drift apart.
Inherited namespaces are looked up on controllerClasspath rather than templateClasspath, so scoping where templates are read from cannot silently stop them being found. A superclass the bundled ASM cannot read is treated as declaring no namespace instead of failing the build, and each superclass is read once however many controllers share it. Document why the namespace rule stays broad and why an empty namespace still counts.
ConfigurationMetadataPluginSpec built its expected paths from the @tempdir project directory, which on macOS is reached through the /var -> /private/var symlink. Gradle resolves that symlink, so the plugin printed /private/var/... while the spec expected /var/..., and two tests failed for every macOS contributor while staying green on CI, where /tmp is not a symlink. Compare canonical paths instead. On Linux they are identical to what the tests asserted before, so CI behaviour is unchanged. (cherry picked from commit 55ee95e)
Covers plugin descriptor metadata, MongoDB datastore bean registration, the Hibernate-secondary-datastore alias behavior, and the transactionManager alias, none of which had any test coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… the request URL The response wrapper compared writer output, counted in characters, against the container's buffer and Content-Length, which are both in bytes. Three-byte UTF-8 text therefore filled and committed an enlarged buffer before the callback fired, and the headers were dropped. Tomcat keeps an enlarged buffer on the recycled processor, so later requests were affected even on new connections. Writer output is now counted as its encoded length: exactly for UTF-8 and single-byte encodings, and at the encoding's maximum bytes per character otherwise. HSTS resolved the forwarded scheme through ServletServerHttpRequest.getURI(), which throws for request paths that java.net.URI rejects but Tomcat accepts through relaxedPathChars, turning those requests into 500s. The forwarded headers are now applied to the request's scheme alone. The upgrade note moves to its own section, the explicit-configuration wording in the security guide matches the behavior, and the docs no longer place the Grails character-encoding filter outside the security headers filter. REVERSE_PROXY_REQUEST_HEADERS is no longer public API. The Tomcat spec checks every default header value, covers multi-byte writer output, and uses @tempdir.
…iled-view-collisions Preserve view precedence when precompiling scaffolds
…warded header catch The response wrapper now hands back the same writer and output stream on every call, as the container does, instead of building a new wrapper (and resolving the charset again) each time. reset() drops the cached writer, since the character encoding can change after a reset. The HSTS scheme resolution catch covers any malformed forwarded header, not only a non-numeric port; the comment and the test now say so.
…ders Add default configurable Grails security response headers
Moves grails-core's own build from 9.7.1 to 9.8.0: `.sdkmanrc`, `gradleToolingApiVersion`, and the wrapper for every Gradle build in the repository — root, build-logic, grails-gradle, grails-forge, end-to-end — plus the grails-shell-cli gradle-sample fixture. The wrappers were regenerated with `gradle -p gradle-bootstrap` on 9.8.0, which brings a new wrapper jar and a reworked `gradlew.bat`; `gradlew` is unchanged. build-logic and the gradle-sample fixture, which bootstrap does not reach, were refreshed from the same output, so the launcher scripts and wrapper jar stay byte-identical everywhere. The jar matches Gradle's published gradle-9.8.0-wrapper.jar.sha256. `end-to-end/legacy-g7-command-plugin` stays on Gradle 8.14.5, the version Grails 7 pins for that fixture; bootstrap's legacyG7Wrapper still refreshes its launcher jar and `gradlew.bat`, as it did for 9.7.0. The wrapper task regenerates the properties files from scratch, so the "keep this synced" comments were restored afterwards.
Applications created by the forge and by the profile CLIs now get a 9.8.0 wrapper instead of 9.7.1. For the forge, that is the `gradleWrapperProperties` template plus the `gradlew.bat` and `gradle/wrapper/gradle-wrapper.jar` that `Gradle.java` copies onto the generated project; `gradlew` is unchanged in 9.8.0. For the profile CLIs, only the `base` and `profile` skeletons carry wrapper assets; every other profile inherits them from `base`.
sbglasius
left a comment
There was a problem hiding this comment.
Approved, given the CI does not show any errors
|
9.8 release notes: https://docs.gradle.org/9.8.0/release-notes.html Explicitly mention publish changes. I have been trying really hard to get the bug fixes merged so we can release 8.0.0 and the publish plugin has not been confirmed working. we should defer this to 8.1. |
@jdaugherty apache/grails-gradle-publish#39 |
Gradle 9.8 starts warning on `Configuration.setVisible(boolean)`, which is scheduled for removal in Gradle 11. The property has had no effect since Gradle 9.0, but GrailsCliGradlePlugin still set it on the grailsCliDetect configuration, so on 9.8 every build that applies a Grails Gradle plugin reported deprecated Gradle features. CliAutoDiscoverySpec now configures every task of a grails-web project with `--warning-mode=fail`, so a deprecation the plugins introduce fails the test.
|
@jdaugherty I added the publishing changes to the description. To check the publish plugin on 9.8.0, I ran the release job's assemble and publish steps for grails-gradle, grails-core and grails-forge twice, on
Not exercised: the Nexus staging calls themselves ( |
The `--features` completion candidates, and with them the bash completion script in the CLI distribution and the "Possible values" in the create-* help, came out in a different order on every build. BaseAvailableFeatures kept the order of the injected features, which are ordered only by `Feature.getOrder()`. Most features share one order, and those ties come out differently from run to run: three `buildCompletion` runs from the same sources produced three different scripts, so the CLI zip was not reproducible. BaseAvailableFeatures now iterates the names sorted, as `--list-features` already lists them. The order in which features are applied is unchanged.
✅ All tests passed ✅🏷️ Commit: 7462e25 Learn more about TestLens at testlens.app/docs. |
|
@jdaugherty I am running all Grails apps now using Gradle 9.8.0 and I published SiteMesh 3.3.0-RC2 to Maven Central using Gradle 9.8.0 Thoughts? |
Gradle 9.8.0
Grails builds on Gradle 9.8.0:
.sdkmanrc,gradleToolingApiVersion, and the wrapper for root,build-logic,grails-gradle,grails-forge,end-to-end, plus thegrails-shell-cligradle-sample fixture.end-to-end/legacy-g7-command-pluginstays on Gradle 8.14.5, the version Grails 7 pins for that fixture.Applications created by the forge and the profile CLIs get a 9.8.0 wrapper as well.
9.8.0 ships a new wrapper jar and a reworked
gradlew.bat;gradlewis unchanged. Every copy in the repository was regenerated from the samegradle-bootstrapoutput, so all ten are byte-identical, and the jar matches Gradle's publishedgradle-9.8.0-wrapper.jar.sha256.Grails 8 still requires Gradle 9.7 or later, so the upgrade guide's minimum stays as it is. Its
./gradlew wrapper --gradle-version {gradleVersion}example takes the version from the Gradle that builds the docs, so it picks up 9.8.0 without an edit.Publishing changes in 9.8.0
What the release notes and upgrade guide change for publishing, against grails-publish 1.0.0-RC1 (grails-core, grails-gradle), 1.0.0-M1 (grails-forge) and build-logic's
PublishPlugin:GenerateMavenPomis up-to-date checked. Tracking stays off for a publication that registerspom.withXml, which grails-publish does for every publication it creates, so those POMs are still generated on every build. The plugin marker POMs of the Grails Gradle plugins have nowithXmland are now tracked; they come out identical.Signatureas aPublishArtifact, thesignaturesconfiguration,SigningExtension.getConfiguration(),SignOperation.getSignatures(), the classifiersign(...)overloads and theSignatureconstructors. The release signs withSigningExtension.sign(publications)anduseGpgCmd(), neither of which is affected.CopyandSyncget a lazydestinationDirectory. Only task types that overridegetDestinationDir()orsetDestinationDir()are affected; build-logic, grails-gradle and grails-publish have none.Published artifacts
Built from the same sources with release settings, the published artifacts and release zips differ from a 9.7.1 build only in:
.modulefiles, which record"version": "9.8.0"undercreatedBy.grails-shell-cli, which depends ongradle-tooling-api9.8.0. The version also appears in the embeddedMETA-INF/sbom.jsonof the jars that depend on it and in the tooling API classes shaded intograils-cli-all.grails.bat,grails-shell-cli.bat,grails-forge-cli.bat,grailsw.bat), which use the 9.8 template: errors are written with1>&2 echo, and every exit path ends ingoto exitWithErrorLevel. The Unix scripts are unchanged.grails-forge-core, thebaseandprofileprofiles and thegrails-shell-clisources jar.grails_forge_cli_completionin the CLI zip, which now lists the--featuresvalues in name order (see below).All other jars and POMs are byte-identical.
Configuration.visibleGradle 9.8 warns on
Configuration.setVisible(boolean), which has had no effect since 9.0 and is removed in Gradle 11.GrailsCliGradlePluginset it ongrailsCliDetect, so every build applying a Grails Gradle plugin reported deprecated Gradle features on 9.8. The call is removed, andCliAutoDiscoverySpecconfigures a grails-web project with--warning-mode=fail.Forge completion order
The
--featurescompletion candidates, and with themgrails_forge_cli_completionin the CLI zip and the "Possible values" in thecreate-*help, came out in a different order on every build, so the CLI zip was not reproducible.BaseAvailableFeaturesnow lists the feature names in name order, as--list-featuresalready does. The order in which features are applied is unchanged.