Skip to content

GH-1240 - Add support for @ModelAttribute. - #2360

Open
snowykte0426 wants to merge 1 commit into
spring-projects:mainfrom
snowykte0426:feature/1240-modelattribute-support
Open

GH-1240 - Add support for @ModelAttribute.#2360
snowykte0426 wants to merge 1 commit into
spring-projects:mainfrom
snowykte0426:feature/1240-modelattribute-support

Conversation

@snowykte0426

@snowykte0426 snowykte0426 commented Sep 8, 2025

Copy link
Copy Markdown

Adds support for Spring MVC's @ModelAttribute in link building, resolving #1240.

Spring MVC binds a @ModelAttribute parameter from the individual request parameters matching the properties of its type. Link building now mirrors that, using RFC6570's form-style query expansion, with unpopulated properties left as template variables:

linkTo(methodOn(C.class).search(null))                ->  /search{?category,sortBy,tags}
linkTo(methodOn(C.class).search(partiallyPopulated))  ->  /search?category=books{&sortBy,tags}
linkTo(methodOn(C.class).search(fullyPopulated))      ->  /search?category=books&sortBy=name&tags=a&tags=b

Implemented through the existing extension points — @ModelAttribute is registered in WebHandler's HandlerMethodParameters.ANNOTATIONS and HandlerMethodParameter.FACTORY alongside @RequestParam and @PathVariable. Since WebHandler backs both stacks, WebFlux is covered too. No new public API.

Only parameters explicitly annotated with @ModelAttribute are considered, and only the form-style expansion is implemented. See the discussion below for the reasoning.

@snowykte0426
snowykte0426 marked this pull request as ready for review September 8, 2025 15:13
@snowykte0426
snowykte0426 force-pushed the feature/1240-modelattribute-support branch 2 times, most recently from 33d9dbe to f07181f Compare August 19, 2026 06:16
Method parameters annotated with @ModelAttribute now contribute to the links built for the handler method: every bindable property renders its own request parameter, following RFC6570's form-style query expansion, with unpopulated ones left as template variables.

Only explicitly annotated parameters are considered, as link building has no argument resolver chain to defer to and therefore cannot tell an unannotated command object apart from the likes of HttpServletRequest or Pageable.

Signed-off-by: Kim Tae Eun <snowykte0426@naver.com>
@snowykte0426
snowykte0426 force-pushed the feature/1240-modelattribute-support branch from f07181f to 2156dc0 Compare August 19, 2026 07:56
@snowykte0426 snowykte0426 changed the title Add @ModelAttribute support to QueryParameter GH-1240 - Add support for @ModelAttribute. Aug 19, 2026
@snowykte0426

Copy link
Copy Markdown
Author

This has been rebased onto current main and the implementation reworked from scratch. The original version of this PR was wrong in ways worth stating plainly: it only touched QueryParameter, so the link URI template never actually changed; the exploded flag it added had no production readers, so the {?myClass*} form it advertised was never rendered; it swept every method parameter and treated unannotated complex types as implicit model attributes, which pulled HttpServletRequest, BindingResult and friends into the affordance query parameters; and its own SpringAffordanceBuilderModelAttributeTest never passed. Apologies for that — none of it is on the branch any more.

What it does now

Spring MVC binds a @ModelAttribute parameter from the individual request parameters matching the properties of its type. Link building mirrors that, using RFC6570 form-style query expansion, with unpopulated properties left as template variables:

linkTo(methodOn(C.class).search(null))                ->  /search{?category,sortBy,tags}
linkTo(methodOn(C.class).search(partiallyPopulated))  ->  /search?category=books{&sortBy,tags}
linkTo(methodOn(C.class).search(fullyPopulated))      ->  /search?category=books&sortBy=name&tags=a&tags=b

I verified the round trip end to end: expanding the rendered template via Link.expand(...) and replaying the result against a live MVC handler binds every property back into the command object, collections included.

Approach

Rather than a parallel mechanism, this uses the extension points that were already there:

  • WebHandler@ModelAttribute is registered in HandlerMethodParameters.ANNOTATIONS and HandlerMethodParameter.FACTORY alongside @RequestParam and @PathVariable, with a ModelAttributeParameter implementation. This is what makes the URI template actually change. Since WebHandler backs both stacks, WebFlux is covered too.
  • ModelAttributeProperties (new, package private) — detects bindable properties, cached by ResolvableType.
  • QueryParameter and SpringAffordanceBuilder are untouched, so there is no new public API.

On the affordance side: I did have SpringAffordanceBuilder expanding @ModelAttribute into QueryParameters, and dropped it after checking what it produced in serialized output. It is inert in HAL-FORMS (HalFormsTemplateBuilder skips GET affordances and builds properties from getInput(), never from getQueryMethodParameters()); dead in Collection+JSON (findQueries admits only non-GET while determineQueryProperties() returns values only for GET, so queries[].data is always empty); and in UBER it caused a regression, because UberData.mergeDeclaredLinksIntoAffordanceLinks inner-joins declared and affordance links on the URL string and templating the declared href broke the match, dropping the link from the output. Inert in two of three consumers and net-negative in the third, so it is gone.

Behaviour worth knowing

A property is bindable if it is writable (or a record component, as those bind through the canonical constructor) and resolves to a simple type or a collection of such. Type variables are resolved against the owning type, so T value on Form<T> renders according to the type argument actually used. Derived read-only properties and nested objects are skipped, as is the whole attribute under @ModelAttribute(binding = false).

A property whose name is already taken by a @RequestParam or @PathVariable on the same method is skipped, so the declared parameter wins. Without that, a bound name could also be advertised as a template variable (?category=fromForm{&category}), which cannot round-trip. Worth a second opinion if you would rather the attribute won instead.

A failing getter is treated as an absent value rather than propagating, so a form object with a lazily-computing or defensive getter cannot turn link assembly into an exception.

Two limitations are documented rather than fixed. A primitive property cannot express "unset", so int page always renders as page=0 once the attribute is present; the reference docs say so and suggest the boxed type. Array properties render comma-joined rather than exploded, which matches existing @RequestParam String[] behaviour and parses back correctly.

Deliberately out of scope

Implicit @ModelAttribute. The issue asks for it, but Spring MVC can resolve unannotated complex parameters only because ModelAttributeMethodProcessor is registered last, after every other argument resolver has had its chance. Link building has no such chain to defer to, so an unannotated parameter cannot be told apart from HttpServletRequest, Pageable, BindingResult and anything a custom resolver claims — which is precisely how the first version of this PR broke. A denylist cannot fix that; Pageable in particular already has correct handling via a UriComponentsContributor, so a miss would double-render it. Requiring the explicit annotation seems like the only defensible option, and the reference docs explain why.

The composite {?myClass*} form. Only the form-style expansion is implemented. TemplateVariable already carries Cardinality.COMPOSITE, so it could be layered on later, but the composite form names a server-side type the client cannot enumerate, cannot express partial population (there is no equivalent of ?category=books{&sortBy}), and would collapse the affordance to a single useless parameter. Happy to add an opt-in if you disagree.

Tests and docs

WebMvcLinkBuilderUnitTest covers the rendered link templates including records, generics, name collisions, failing getters, arrays and the primitive limitation; ModelAttributePropertiesUnitTest covers the property detection rules; WebFluxLinkBuilderTest gains @ModelAttribute cases, having had none. Regression guards assert that unannotated parameters and binding = false contribute nothing. Full suite green at 1004, and the nullaway profile is back to the single pre-existing error on main (LinkBuilderSupport:114).

server.adoc has a new section under "Building links that point to methods" documenting the behaviour and both limitations.

@odrotbohm — this has been sitting a while and no longer resembles what was originally submitted, so it may be worth a fresh look rather than reading the old diff. Two things I would especially value your call on: whether the declared @RequestParam/@PathVariable should win a name collision the way it does here, and whether requiring the explicit annotation instead of also supporting implicit model attributes is an acceptable scope for a first cut. Happy to keep iterating.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant