spring-projects / spring-projects/spring-framework
ModelAttributeMethodArgumentResolver should expose validateIfApplicable and isBindExceptionRequired as protected extension points
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 60.2k
- Forks
- 38.8k
- Avg merge
- 5d 2h
- Merged PRs (30d)
- 27
Description
Describe the use case
When migrating from Spring MVC to WebFlux, a common customization pattern is to subclass ModelAttributeMethodProcessor and override validateIfApplicable to apply group-based or context-aware validation — for example, selecting different @Validated groups based on the HTTP method, tenant context, or feature flags. The same need exists in WebFlux, but ModelAttributeMethodArgumentResolver does not support it.
Similarly, isBindExceptionRequired is overridden in MVC subclasses to control whether a binding/validation failure should throw or be silently passed to a BindingResult parameter — this decision sometimes needs to depend on runtime state, not just parameter inspection.
What is missing
ModelAttributeMethodProcessor (MVC) exposes both as protected extension points:
protected void validateIfApplicable(WebDataBinder binder, MethodParameter parameter)
protected boolean isBindExceptionRequired(WebDataBinder binder, MethodParameter parameter)
protected boolean isBindExceptionRequired(MethodParameter parameter)
In ModelAttributeMethodArgumentResolver (WebFlux), validateIfApplicable is private and isBindExceptionRequired does not exist at all — the equivalent logic is inlined as a private hasErrorsArgument helper. Neither can be overridden by subclasses.
Proposed solution
Change validateIfApplicable from private to protected:
protected void validateIfApplicable(WebExchangeDataBinder binder,
MethodParameter parameter,
ServerWebExchange exchange)
Introduce isBindExceptionRequired as a protected method:
protected boolean isBindExceptionRequired(MethodParameter parameter)
The existing private hasErrorsArgument(MethodParameter) can be replaced by this method with no behavioral change.
Precedent in the same class
PR #24947 already established this pattern by making bindRequestParameters protected in ModelAttributeMethodArgumentResolver. The class currently exposes two protected reactive extension points:
protected Mono<Void> constructAttribute(WebExchangeDataBinder binder, ServerWebExchange exchange)
protected Mono<Void> bindRequestParameters(WebExchangeDataBinder binder, ServerWebExchange exchange)
validateIfApplicable sits at the same level in the binding lifecycle and should follow the same convention. The visibility change for validateIfApplicable keeps the synchronous signature consistent with how it is called internally — inside a .doOnSuccess() — which matches the existing contract. A separate reactive variant can be considered later if async validation becomes a requirement.
No behavioral change
Both changes are purely additive. Existing behavior is unaffected for any code that does not subclass ModelAttributeMethodArgumentResolver.
Happy to open a PR about it if the proposed solution is fine and also if there is any other approach you may want to have for this case I will be happy to help
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading ModelAttributeMethodArgumentResolver and comparing its validation and binding-error flow with ModelAttributeMethodProcessor. Trace the existing hasErrorsArgument and validateIfApplicable call sites, then confirm that the requested protected extension points are available without changing behavior for existing subclasses.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, spring
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100