Skip to content

Adopting Spring Boot Multipart Configuration for File Uploads - #16040

Open
gsartori wants to merge 32 commits into
apache:8.0.xfrom
gsartori:use-spring-upload-config
Open

Adopting Spring Boot Multipart Configuration for File Uploads#16040
gsartori wants to merge 32 commits into
apache:8.0.xfrom
gsartori:use-spring-upload-config

Conversation

@gsartori

Copy link
Copy Markdown
Contributor

Description

Fixes #15644.

This change delegates multipart upload configuration to Spring Boot instead of creating and configuring a Grails-specific MultipartConfigElement.
Applications should now use the standard spring.servlet.multipart properties:

spring:
    servlet:
        multipart:
            max-file-size: 200MB
            max-request-size: 200MB

The previous grails.controllers.upload configuration namespace is no longer supported. If legacy properties are detected, application startup fails with migration instructions instead of silently ignoring them.

The change also:

  • Removes the obsolete upload constants from grails.config.Settings.
  • Removes the corresponding Grails configuration metadata.
  • Allows Spring Boot’s multipart auto-configuration to supply the MultipartConfigElement.
  • Makes servlet registration tolerate multipart support being disabled or otherwise unavailable.
  • Adopts Spring Boot’s defaults of 1 MB per file and 10 MB per multipart request.
  • Adds a regression test covering detection of legacy configuration.
  • Updates the uploading-files documentation and threat model with the new properties and defaults.
    GrailsHibernatePersistentEntity is not changed by these commits.

Contributor Checklist

Issue and Scope

  • This PR is linked to an existing issue that has been acknowledged or approved by the project team: #15644.
  • This PR addresses the complete scope of the linked issue.
  • This PR contains a single, focused change.
  • This PR targets the 8.0.x major release branch, where breaking configuration changes are permitted.

Code Quality

  • I have added or updated tests that cover the changes introduced in this PR.
  • I have verified that all existing tests pass by running ./gradlew build --rerun-tasks.
  • I have run ./gradlew codeStyle and resolved any violations.
  • This PR does not include mass reformatting, style-only changes, or unrelated large-scale refactoring.
  • If generative AI tooling was used, a quality model was used to ensure consistency with the project’s quality standards.

Licensing and Attribution

  • All contributed code is provided under the Apache License 2.0.
  • I have the necessary rights to submit this contribution and confirm it is my own original work.
  • If generative AI tooling was used, I have followed the ASF policy and properly attributed its use.

Documentation

  • The relevant user-facing documentation has been updated.
  • The What’s New section of the Grails Guide has been updated.
  • The Grails 8 Upgrade Notes have been updated to document migration from grails.controllers.upload to spring.servlet.multipart.
  • The PR description clearly explains what was changed and why.

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.4781%. Comparing base (5e1acba) to head (90dacd3).
⚠️ Report is 7 commits behind head on 8.0.x.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##                8.0.x     #16040        +/-   ##
==================================================
+ Coverage     52.4207%   52.4781%   +0.0573%     
- Complexity      18319      18329        +10     
==================================================
  Files            2032       2033         +1     
  Lines           96294      96305        +11     
  Branches        16835      16837         +2     
==================================================
+ Hits            50478      50539        +61     
+ Misses          38394      38341        -53     
- Partials         7422       7425         +3     
Files with missing lines Coverage Δ
.../web/controllers/ControllersAutoConfiguration.java 96.5517% <100.0000%> (+1.4698%) ⬆️
...ers/GrailsControllersEnvironmentPostProcessor.java 100.0000% <100.0000%> (ø)
...core/src/main/groovy/grails/config/Settings.groovy 100.0000% <ø> (ø)

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

@gsartori

Copy link
Copy Markdown
Contributor Author

@martei I've added a paragraph in the upgrade guide here as well

Comment thread grails-doc/src/en/guide/upgrading/upgrading80x.adoc Outdated

@jdaugherty jdaugherty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. I am ok with this but I would like to know if @jamesfredley is ok with this

@gsartori

gsartori commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@jdaugherty I think I need help with the failing tests since I didn't manage to understand the link between my code and the failures

@gsartori

gsartori commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@jdaugherty @jamesfredley I've let AI fix the issue with the logger. I need someone to review the changes since this time I let her do the job altogether. Changes are small but I have no experience on that area.

@matrei

matrei commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@gsartori I reverted your last change. The tests are green now ✔️

@jdaugherty jdaugherty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remaining comments are about the migration surface and test coverage.

It looks like cf2e542 was pulled into this PR and then reverted by Mattias - we should be sure to squash this PR when merged.

Comment thread grails-doc/src/en/guide/upgrading/upgrading80x.adoc Outdated
Comment thread grails-doc/src/en/guide/theWebLayer/controllers/uploadingFiles.adoc Outdated
@matrei
matrei force-pushed the use-spring-upload-config branch from c973b31 to 90dacd3 Compare August 11, 2026 06:19
@testlens-app

testlens-app Bot commented Aug 11, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 90dacd3
▶️ Tests: 67640 executed
⚪️ Checks: 77/77 completed


Learn more about TestLens at testlens.app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Adopting Spring Boot Multipart Configuration for File Uploads

5 participants