From 2156dc0c560f287ae2aa4b25b4d6b8f66256b8d1 Mon Sep 17 00:00:00 2001 From: Kim Tae Eun Date: Wed, 19 Aug 2026 16:56:29 +0900 Subject: [PATCH] GH-1240 - Add support for @ModelAttribute. 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 --- src/main/asciidoc/server.adoc | 55 ++++ .../server/core/ModelAttributeProperties.java | 145 +++++++++ .../hateoas/server/core/WebHandler.java | 176 +++++++++- .../ModelAttributePropertiesUnitTest.java | 232 +++++++++++++ .../server/mvc/WebMvcLinkBuilderUnitTest.java | 308 ++++++++++++++++++ .../reactive/WebFluxLinkBuilderTest.java | 56 ++++ 6 files changed, 970 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/springframework/hateoas/server/core/ModelAttributeProperties.java create mode 100644 src/test/java/org/springframework/hateoas/server/core/ModelAttributePropertiesUnitTest.java diff --git a/src/main/asciidoc/server.adoc b/src/main/asciidoc/server.adoc index 169253472..9ecea262b 100644 --- a/src/main/asciidoc/server.adoc +++ b/src/main/asciidoc/server.adoc @@ -138,6 +138,61 @@ assertThat(link.getHref()).endsWith("/people?names=Matthews,Beauford"); <3> NOTE: The reason we're exposing `@NonComposite` is that the composite way of rendering request parameters is baked into the internals of Spring's `UriComponents` builder and we only introduced that non-composite style in Spring HATEOAS 1.4. If we started from scratch today, we'd probably default to that style and rather let users opt into the composite style explicitly rather than the other way around. +[[server.link-builder.webmvc.methods.model-attributes]] +==== Building links for `@ModelAttribute` parameters + +Spring MVC binds a handler method parameter annotated with `@ModelAttribute` from the individual request parameters that match the properties of the attribute's type. +Link building mirrors that: each bindable property contributes its own request parameter, following https://tools.ietf.org/html/rfc6570#section-3.2.8[RFC6570's form-style query expansion]. +Properties you have not populated are rendered as template variables, so that a client can fill them in. + +==== +[source, java] +---- +class Filter { <1> + + String category; + String sortBy; + + // getters and setters +} + +@Controller +class PersonController { + + @GetMapping("/people") + HttpEntity showAll(@ModelAttribute Filter filter) { … } +} + +var link = linkTo(methodOn(PersonController.class).showAll(null)).withSelfRel(); <2> + +assertThat(link.getHref()).endsWith("/people{?category,sortBy}"); + +var filter = new Filter(); +filter.setCategory("customer"); + +link = linkTo(methodOn(PersonController.class).showAll(filter)).withSelfRel(); <3> + +assertThat(link.getHref()).endsWith("/people?category=customer{&sortBy}"); +---- +<1> A plain form backing object. Java records work just as well, as their components are bound through the canonical constructor. +<2> Invoking the method without an attribute renders all bindable properties as template variables. +<3> Populated properties are rendered as actual request parameters, the remaining ones stay templated. +==== + +A property is considered bindable if it is writable (or a record component) and resolves to a simple type or a collection of such. +`Collection`-valued properties are rendered in the composite style, i.e. the parameter name is repeated for each value; arrays are rendered comma-separated, matching how `@RequestParam` arrays already render. +Derived, read-only properties and nested objects are skipped, as is the entire attribute if it is declared as `@ModelAttribute(binding = false)`. +A property whose name is already taken by a `@RequestParam` or `@PathVariable` of the same method is skipped too, so that the declared parameter wins and the link stays expandable. + +NOTE: A property of a primitive type can never be rendered as a template variable once the attribute itself is present, because a primitive cannot express "unset" — `int page` is indistinguishable from `page = 0`. +Such properties are always rendered as bound request parameters. +Use the boxed type (`Integer`) if you want the property to stay fillable by the client. + +NOTE: Only parameters explicitly annotated with `@ModelAttribute` are considered. +Spring MVC additionally treats unannotated complex parameters as implicit model attributes, but it can only do so because its model attribute resolver runs after every other argument resolver has had a chance to claim the parameter. +Link building has no such chain to defer to, which means an unannotated parameter cannot be told apart from the likes of `HttpServletRequest`, `Pageable` or `BindingResult`. +Annotate the parameter explicitly to have it rendered into the link. + [[server.link-builder.webflux]] == Building links in Spring WebFlux diff --git a/src/main/java/org/springframework/hateoas/server/core/ModelAttributeProperties.java b/src/main/java/org/springframework/hateoas/server/core/ModelAttributeProperties.java new file mode 100644 index 000000000..bcd0235c1 --- /dev/null +++ b/src/main/java/org/springframework/hateoas/server/core/ModelAttributeProperties.java @@ -0,0 +1,145 @@ +/* + * Copyright 2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.hateoas.server.core; + +import java.beans.PropertyDescriptor; +import java.lang.reflect.RecordComponent; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +import org.springframework.beans.BeanUtils; +import org.springframework.core.ResolvableType; +import org.springframework.util.Assert; +import org.springframework.util.ConcurrentReferenceHashMap; +import org.springframework.web.bind.annotation.ModelAttribute; + +/** + * Detects the properties of a {@link ModelAttribute} that Spring MVC's {@literal WebDataBinder} will populate from + * request parameters. Those are the properties that need to show up in a URI template as individual request parameters. + *

+ * A property is considered bindable if it is writable (or a record component, as those are bound through the canonical + * constructor) and resolves to a simple type or a {@link Collection} of such, as those are the ones representable as a + * request parameter. Derived, read-only properties and nested objects are skipped. + *

+ * This deliberately does not reuse {@literal mediatype.PropertyUtils}: that one answers which properties Jackson + * serializes (honoring {@literal @JsonIgnore}, unwrapping {@link org.springframework.hateoas.EntityModel} and + * friends, ignoring writability), which is a different question from which properties a {@literal WebDataBinder} + * binds. + * + * @author Kim Tae Eun + * @since 3.2 + * @see RFC6570 - Form-Style Query Expansion + */ +class ModelAttributeProperties { + + private static final Map> CACHE = new ConcurrentReferenceHashMap<>(); + + private ModelAttributeProperties() {} + + /** + * Returns the names of all bindable properties of the given type, in alphabetical order. The type is keyed with its + * generics intact, so that {@code Form} and {@code Form} are told apart. + * + * @param type must not be {@literal null}. + * @return will never be {@literal null}. + */ + static List getPropertyNames(ResolvableType type) { + + Assert.notNull(type, "Type must not be null!"); + + return CACHE.computeIfAbsent(type, ModelAttributeProperties::detectPropertyNames); + } + + /** + * Introspects the given type for bindable property names. + * + * @param owner must not be {@literal null}. + * @return will never be {@literal null}. + */ + private static List detectPropertyNames(ResolvableType owner) { + + Class type = owner.resolve(); + + if (type == null) { + return Collections.emptyList(); + } + + Set recordComponents = detectRecordComponents(type); + + return List.copyOf(Arrays.stream(BeanUtils.getPropertyDescriptors(type)) // + .filter(it -> !"class".equals(it.getName())) // + .filter(it -> it.getReadMethod() != null) // + .filter(it -> it.getWriteMethod() != null || recordComponents.contains(it.getName())) // + .filter(it -> isBindable(it, owner)) // + .map(PropertyDescriptor::getName) // + .sorted() // + .collect(Collectors.toList())); + } + + /** + * Returns the names of the record components of the given type, or an empty {@link Set} if it is not a record. + * Record components are bound through the canonical constructor and thus do not expose a setter. + * + * @param type must not be {@literal null}. + * @return will never be {@literal null}. + */ + private static Set detectRecordComponents(Class type) { + + if (!type.isRecord()) { + return Collections.emptySet(); + } + + return Arrays.stream(type.getRecordComponents()) // + .map(RecordComponent::getName) // + .collect(Collectors.toSet()); + } + + /** + * Returns whether the given property can be represented as a request parameter, i.e. whether it resolves to a + * simple type or a {@link Collection} of such. Type variables are resolved against the owning type, so that a + * {@code T value} declared on {@code Form} is judged by the type argument actually used. + * + * @param descriptor must not be {@literal null}. + * @param owner must not be {@literal null}. + * @return whether the property binds to a request parameter. + */ + private static boolean isBindable(PropertyDescriptor descriptor, ResolvableType owner) { + + ResolvableType type = ResolvableType.forType(descriptor.getReadMethod().getGenericReturnType(), owner); + Class resolved = type.resolve(); + + if (resolved == null) { + return false; + } + + if (BeanUtils.isSimpleProperty(resolved)) { + return true; + } + + if (!Collection.class.isAssignableFrom(resolved)) { + return false; + } + + Class elementType = type.asCollection().resolveGeneric(0); + + return elementType != null && BeanUtils.isSimpleProperty(elementType); + } +} diff --git a/src/main/java/org/springframework/hateoas/server/core/WebHandler.java b/src/main/java/org/springframework/hateoas/server/core/WebHandler.java index d983c45e6..f3ff8b502 100644 --- a/src/main/java/org/springframework/hateoas/server/core/WebHandler.java +++ b/src/main/java/org/springframework/hateoas/server/core/WebHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2024 the original author or authors. + * Copyright 2019-2026 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -30,6 +30,9 @@ import java.util.stream.Collectors; import org.jspecify.annotations.Nullable; +import org.springframework.beans.BeansException; +import org.springframework.beans.BeanWrapper; +import org.springframework.beans.PropertyAccessorFactory; import org.springframework.core.MethodParameter; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; @@ -45,6 +48,7 @@ import org.springframework.util.MultiValueMap; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; +import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ValueConstants; @@ -58,6 +62,7 @@ * @author Greg Turnquist * @author Oliver Drotbohm * @author Réda Housni Alaoui + * @author Kim Tae Eun */ public class WebHandler { @@ -126,6 +131,14 @@ private static PreparedWebHandler linkTo(Object invoc Object[] arguments = invocation.getArguments(); List optionalEmptyParameters = new ArrayList<>(); + // Names already spoken for, so that model attribute properties do not shadow them. See + // HandlerMethodParameter#contributeTo(…). + Set reservedNames = new HashSet<>(); + + for (MappingVariable variable : mappingVariables) { + reservedNames.add(variable.getKey()); + } + for (HandlerMethodParameter parameter : parameters.getParameterAnnotatedWith(PathVariable.class, arguments)) { MappingVariable mappingVariable = mappingVariables.getVariable(parameter.getVariableName()); @@ -158,6 +171,10 @@ private static PreparedWebHandler linkTo(Object invoc boolean isSkipValue = SKIP_VALUE.equals(parameter.getVerifiedValue(arguments)); boolean isMapParameter = Map.class.isAssignableFrom(parameter.parameter.getParameterType()); + if (!isMapParameter) { + reservedNames.add(parameter.getVariableName()); + } + if (isSkipValue && !isMapParameter) { values.put(parameter.getVariableName(), SKIP_VALUE); @@ -168,6 +185,12 @@ private static PreparedWebHandler linkTo(Object invoc } } + for (HandlerMethodParameter parameter : parameters.getParameterAnnotatedWith(ModelAttribute.class, + arguments)) { + + optionalEmptyParameters.addAll(parameter.contributeTo(builder, arguments, factory, reservedNames)); + } + for (MappingVariable variable : mappingVariables) { if (!values.containsKey(variable.getKey())) { values.put(variable.getKey(), variable.getAbsentValue()); @@ -285,7 +308,7 @@ private static void bindRequestParameters(UriComponentsBuilder builder, HandlerM private static class HandlerMethodParameters { private static final List> ANNOTATIONS = Arrays.asList(RequestParam.class, - PathVariable.class); + PathVariable.class, ModelAttribute.class); private static final Map CACHE = new ConcurrentHashMap(); private final MultiValueMap, HandlerMethodParameter> byAnnotationCache; @@ -342,6 +365,7 @@ private abstract static class HandlerMethodParameter { FACTORY = new HashMap<>(); FACTORY.put(RequestParam.class, RequestParamParameter::new); FACTORY.put(PathVariable.class, PathVariableParameter::new); + FACTORY.put(ModelAttribute.class, ModelAttributeParameter::new); } private final MethodParameter parameter; @@ -402,6 +426,16 @@ Class getAnnotationType() { return attribute.getAnnotationType(); } + /** + * Returns the {@link TypeDescriptor} of the parameter, with a potential {@link Optional} wrapper already + * unwrapped. + * + * @return will never be {@literal null}. + */ + TypeDescriptor getTypeDescriptor() { + return typeDescriptor; + } + /** * Returns whether the * @@ -513,6 +547,23 @@ public Object getVerifiedValue(Object[] values) { } public abstract boolean isRequired(); + + /** + * Contributes the request parameters this parameter binds to the given {@link UriComponentsBuilder} and + * returns the names of the ones no value was available for, so that they can be rendered as template + * variables. Only implemented for parameters expanding into more than one request parameter; single-variable + * ones are handled by {@code bindRequestParameters(…)}. + * + * @param builder must not be {@literal null}. + * @param arguments must not be {@literal null}. + * @param factory must not be {@literal null}. + * @param reservedNames names already bound by other parameters, must not be {@literal null}. + * @return will never be {@literal null}. + */ + public List contributeTo(UriComponentsBuilder builder, Object[] arguments, FormatterFactory factory, + Set reservedNames) { + return Collections.emptyList(); + } } /** @@ -626,6 +677,127 @@ public Object getVerifiedValue(Object[] values) { } } + /** + * {@link HandlerMethodParameter} extension dealing with {@link ModelAttribute} parameters. In contrast to the other + * implementations a {@link ModelAttribute} parameter does not contribute a single request parameter but one per + * bindable property of the attribute's type. + * + * @author Kim Tae Eun + * @since 3.2 + */ + private static class ModelAttributeParameter extends HandlerMethodParameter { + + private final MethodParameter parameter; + private final List propertyNames; + + public ModelAttributeParameter(MethodParameter parameter) { + + super(parameter, new AnnotationAttribute(ModelAttribute.class)); + + ModelAttribute annotation = parameter.getParameterAnnotation(ModelAttribute.class); + + this.parameter = parameter; + this.propertyNames = annotation != null && !annotation.binding() // + ? Collections.emptyList() // + : ModelAttributeProperties.getPropertyNames(getTypeDescriptor().getResolvableType()); + } + + /* + * (non-Javadoc) + * @see org.springframework.hateoas.server.core.WebHandler.HandlerMethodParameter#isRequired() + */ + @Override + public boolean isRequired() { + return false; + } + + /* + * (non-Javadoc) + * @see org.springframework.hateoas.server.core.WebHandler.HandlerMethodParameter#getVerifiedValue(java.lang.Object[]) + */ + @Override + @Nullable + public Object getVerifiedValue(Object[] values) { + + Object value = ObjectUtils.unwrapOptional(values[parameter.getParameterIndex()]); + + return value == null ? SKIP_VALUE : value; + } + + /* + * (non-Javadoc) + * @see org.springframework.hateoas.server.core.WebHandler.HandlerMethodParameter#contributeTo + */ + @Override + public List contributeTo(UriComponentsBuilder builder, Object[] arguments, FormatterFactory factory, + Set reservedNames) { + + List names = new ArrayList<>(); + + for (String property : propertyNames) { + if (reservedNames.add(property)) { + names.add(property); + } + } + + Object value = getVerifiedValue(arguments); + + if (value == null || SKIP_VALUE.equals(value)) { + return names; + } + + BeanWrapper wrapper = PropertyAccessorFactory.forBeanPropertyAccess(value); + List unbound = new ArrayList<>(); + + for (String property : names) { + + Object propertyValue = readProperty(wrapper, property); + + // An empty collection is treated as absent, so that the link stays expandable. Note that an empty + // collection handed to a @RequestParam vanishes entirely instead - see bindRequestParameters(…). + if (propertyValue == null + || propertyValue instanceof Collection && ((Collection) propertyValue).isEmpty()) { + + unbound.add(property); + continue; + } + + TemplateVariable variable = TemplateVariable.requestParameter(property); + Object prepared = prepareValue(propertyValue, factory, wrapper.getPropertyTypeDescriptor(property)); + + if (prepared instanceof Collection) { + + for (Object element : (Collection) prepared) { + builder.queryParam(property, variable.prepareAndEncode(element)); + } + + } else { + builder.queryParam(property, variable.prepareAndEncode(prepared)); + } + } + + return unbound; + } + + /** + * Reads the given property, treating a failing getter as an absent value. Link building must not break + * because a form object exposes a getter that throws. + * + * @param wrapper must not be {@literal null}. + * @param property must not be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + private static Object readProperty(BeanWrapper wrapper, String property) { + + try { + return wrapper.getPropertyValue(property); + } catch (BeansException o_O) { + return null; + } + } + } + /** * {@link HandlerMethodParameter} extension dealing with {@link PathVariable} parameters. * diff --git a/src/test/java/org/springframework/hateoas/server/core/ModelAttributePropertiesUnitTest.java b/src/test/java/org/springframework/hateoas/server/core/ModelAttributePropertiesUnitTest.java new file mode 100644 index 000000000..227f31f56 --- /dev/null +++ b/src/test/java/org/springframework/hateoas/server/core/ModelAttributePropertiesUnitTest.java @@ -0,0 +1,232 @@ +/* + * Copyright 2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.hateoas.server.core; + +import static org.assertj.core.api.Assertions.*; + +import java.time.LocalDate; +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.springframework.core.ResolvableType; + +/** + * Unit tests for {@link ModelAttributeProperties}. + * + * @author Kim Tae Eun + */ +class ModelAttributePropertiesUnitTest { + + @Test // GH-1240 + void detectsBindablePropertiesInAlphabeticalOrder() { + + assertThat(getPropertyNames(SearchForm.class)) // + .containsExactly("category", "includeArchived", "page", "sortBy", "tags"); + } + + @Test // GH-1240 + void skipsNestedObjectProperties() { + assertThat(getPropertyNames(SearchForm.class)).doesNotContain("nested"); + } + + @Test // GH-1240 + void skipsDerivedReadOnlyProperties() { + assertThat(getPropertyNames(SearchForm.class)).doesNotContain("summary"); + } + + @Test // GH-1240 + void skipsCollectionsOfNestedObjects() { + assertThat(getPropertyNames(SearchForm.class)).doesNotContain("children"); + } + + @Test // GH-1240 + void detectsRecordComponents() { + + assertThat(getPropertyNames(SearchRecord.class)) // + .containsExactly("category", "sortBy"); + } + + @Test // GH-1240 + void detectsInheritedProperties() { + + assertThat(getPropertyNames(ChildForm.class)) // + .containsExactly("childProperty", "parentProperty"); + } + + @Test // GH-1240 + void resolvesTypeVariablesAgainstTheActualTypeArgument() { + + ResolvableType bindable = ResolvableType.forClassWithGenerics(GenericForm.class, String.class); + + assertThat(ModelAttributeProperties.getPropertyNames(bindable)).containsExactly("name", "value"); + } + + @Test // GH-1240 + void skipsTypeVariablesResolvingToANonSimpleType() { + + ResolvableType nested = ResolvableType.forClassWithGenerics(GenericForm.class, Nested.class); + + assertThat(ModelAttributeProperties.getPropertyNames(nested)).containsExactly("name"); + } + + @Test // GH-1240 + void tellsParameterisationsOfTheSameRawTypeApart() { + + ResolvableType simple = ResolvableType.forClassWithGenerics(GenericForm.class, LocalDate.class); + ResolvableType nested = ResolvableType.forClassWithGenerics(GenericForm.class, Nested.class); + + assertThat(ModelAttributeProperties.getPropertyNames(simple)).contains("value"); + assertThat(ModelAttributeProperties.getPropertyNames(nested)).doesNotContain("value"); + } + + private static List getPropertyNames(Class type) { + return ModelAttributeProperties.getPropertyNames(ResolvableType.forClass(type)); + } + + record SearchRecord(String category, String sortBy) {} + + static class GenericForm { + + private T value; + private String name; + + public T getValue() { + return value; + } + + public void setValue(T value) { + this.value = value; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + static class ParentForm { + + private String parentProperty; + + public String getParentProperty() { + return parentProperty; + } + + public void setParentProperty(String parentProperty) { + this.parentProperty = parentProperty; + } + } + + static class ChildForm extends ParentForm { + + private String childProperty; + + public String getChildProperty() { + return childProperty; + } + + public void setChildProperty(String childProperty) { + this.childProperty = childProperty; + } + } + + static class SearchForm { + + private String category; + private String sortBy; + private Boolean includeArchived; + private int page; + private List tags; + private List children; + private Nested nested; + + public String getCategory() { + return category; + } + + public void setCategory(String category) { + this.category = category; + } + + public String getSortBy() { + return sortBy; + } + + public void setSortBy(String sortBy) { + this.sortBy = sortBy; + } + + public Boolean getIncludeArchived() { + return includeArchived; + } + + public void setIncludeArchived(Boolean includeArchived) { + this.includeArchived = includeArchived; + } + + public int getPage() { + return page; + } + + public void setPage(int page) { + this.page = page; + } + + public List getTags() { + return tags; + } + + public void setTags(List tags) { + this.tags = tags; + } + + public List getChildren() { + return children; + } + + public void setChildren(List children) { + this.children = children; + } + + public Nested getNested() { + return nested; + } + + public void setNested(Nested nested) { + this.nested = nested; + } + + public String getSummary() { + return category + "/" + sortBy; + } + } + + static class Nested { + + private String value; + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + } +} diff --git a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java index e221f7c7f..5c00e08b8 100644 --- a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java +++ b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java @@ -55,6 +55,7 @@ import org.springframework.test.util.ReflectionTestUtils; import org.springframework.util.MultiValueMap; import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -79,6 +80,7 @@ * @author Oliver Trosien * @author Greg Turnquist * @author Réda Housni Alaoui + * @author Kim Tae Eun */ class WebMvcLinkBuilderUnitTest extends TestUtils { @@ -762,6 +764,136 @@ void doesNotAppendTrailingSlashForEmptyMapping() { .isEqualTo("http://localhost/people"); } + @Test // GH-1240 + void rendersModelAttributePropertiesAsTemplateVariables() { + + assertThat(linkTo(methodOn(ModelAttributeController.class).search(null)).withSelfRel().getHref()) // + .endsWith("/search{?category,sortBy,tags}"); + } + + @Test // GH-1240 + void bindsModelAttributePropertyValues() { + + SearchForm form = new SearchForm(); + form.setCategory("books"); + form.setSortBy("name"); + form.setTags(Arrays.asList("fiction", "classic")); + + assertThat(linkTo(methodOn(ModelAttributeController.class).search(form)).withSelfRel().getHref()) // + .endsWith("/search?category=books&sortBy=name&tags=fiction&tags=classic"); + } + + @Test // GH-1240 + void rendersUnsetModelAttributePropertiesAsTemplateVariables() { + + SearchForm form = new SearchForm(); + form.setCategory("books"); + + assertThat(linkTo(methodOn(ModelAttributeController.class).search(form)).withSelfRel().getHref()) // + .endsWith("/search?category=books{&sortBy,tags}"); + } + + @Test // GH-1240 + void combinesRequestParametersAndModelAttributeProperties() { + + assertThat(linkTo(methodOn(ModelAttributeController.class).mixed("spring", null)).withSelfRel().getHref()) // + .endsWith("/mixed?q=spring{&category,sortBy,tags}"); + } + + @Test // GH-1240 + void rendersModelAttributeExposedAsOptional() { + + var controller = methodOn(ModelAttributeController.class); + + assertThat(linkTo(controller.optional(Optional.empty())).withSelfRel().getHref()) // + .endsWith("/optional{?category,sortBy,tags}"); + } + + @Test // GH-1240 + void ignoresModelAttributeWithBindingDisabled() { + + assertThat(linkTo(methodOn(ModelAttributeController.class).nonBinding(null)).withSelfRel().getHref()) // + .endsWith("/non-binding"); + } + + @Test // GH-1240 + void doesNotTreatUnannotatedParametersAsModelAttributes() { + + assertThat(linkTo(methodOn(ModelAttributeController.class).unannotated(null)).withSelfRel().getHref()) // + .endsWith("/unannotated"); + } + + @Test // GH-1240 + void bindsRecordComponents() { + + var controller = methodOn(ModelAttributeController.class); + + assertThat(linkTo(controller.record(null)).withSelfRel().getHref()) // + .endsWith("/record{?category,sortBy}"); + + assertThat(linkTo(controller.record(new SearchRecord("books", "name"))).withSelfRel().getHref()) // + .endsWith("/record?category=books&sortBy=name"); + } + + @Test // GH-1240 + void resolvesTypeVariablesAgainstTheActualTypeArgument() { + + GenericForm form = new GenericForm<>(); + form.setValue("books"); + + assertThat(linkTo(methodOn(ModelAttributeController.class).generic(form)).withSelfRel().getHref()) // + .endsWith("/generic?value=books"); + } + + @Test // GH-1240 + void doesNotShadowARequestParameterOfTheSameName() { + + SearchForm form = new SearchForm(); + form.setCategory("fromForm"); + form.setSortBy("name"); + + var controller = methodOn(ModelAttributeController.class); + + assertThat(linkTo(controller.collide("fromParam", form)).withSelfRel().getHref()) // + .endsWith("/collide?category=fromParam&sortBy=name{&tags}"); + } + + @Test // GH-1240 + void doesNotShadowAPathVariableOfTheSameName() { + + var controller = methodOn(ModelAttributeController.class); + + assertThat(linkTo(controller.pathCollide(1L, new WithId())).withSelfRel().getHref()) // + .endsWith("/items/1{?note}"); + } + + @Test // GH-1240 + void treatsAFailingGetterAsAnAbsentValue() { + + assertThat(linkTo(methodOn(ModelAttributeController.class).throwing(new Throwing())).withSelfRel().getHref()) // + .endsWith("/throwing?ok=ok{&boom}"); + } + + @Test // GH-1240 + void rendersPrimitivePropertiesUsingTheirDefaultValue() { + + // A primitive cannot express "unset", so it is always bound. See the note in server.adoc. + var controller = methodOn(ModelAttributeController.class); + + assertThat(linkTo(controller.primitives(new Primitives())).withSelfRel().getHref()) // + .endsWith("/primitives?page=0"); + } + + @Test // GH-1240 + void rendersArrayPropertiesLikeRequestParameterArrays() { + + ArrayForm form = new ArrayForm(); + form.setCodes(new String[] { "a", "b" }); + + assertThat(linkTo(methodOn(ModelAttributeController.class).array(form)).withSelfRel().getHref()) // + .endsWith("/array?codes=a%2Cb"); + } + private static UriComponents toComponents(Link link) { return UriComponentsBuilder.fromUriString(link.expand().getHref()).build(); } @@ -987,4 +1119,180 @@ HttpEntity test(@PathVariable String first, @PathVariable String second) { return null; } } + + static class ModelAttributeController { + + @RequestMapping("/search") + HttpEntity search(@ModelAttribute SearchForm form) { + return null; + } + + @RequestMapping("/mixed") + HttpEntity mixed(@RequestParam String q, @ModelAttribute SearchForm form) { + return null; + } + + @RequestMapping("/optional") + HttpEntity optional(@ModelAttribute Optional form) { + return null; + } + + @RequestMapping("/non-binding") + HttpEntity nonBinding(@ModelAttribute(binding = false) SearchForm form) { + return null; + } + + @RequestMapping("/unannotated") + HttpEntity unannotated(SearchForm form) { + return null; + } + + @RequestMapping("/record") + HttpEntity record(@ModelAttribute SearchRecord form) { + return null; + } + + @RequestMapping("/generic") + HttpEntity generic(@ModelAttribute GenericForm form) { + return null; + } + + @RequestMapping("/collide") + HttpEntity collide(@RequestParam String category, @ModelAttribute SearchForm form) { + return null; + } + + @RequestMapping("/items/{id}") + HttpEntity pathCollide(@PathVariable Long id, @ModelAttribute WithId form) { + return null; + } + + @RequestMapping("/throwing") + HttpEntity throwing(@ModelAttribute Throwing form) { + return null; + } + + @RequestMapping("/primitives") + HttpEntity primitives(@ModelAttribute Primitives form) { + return null; + } + + @RequestMapping("/array") + HttpEntity array(@ModelAttribute ArrayForm form) { + return null; + } + } + + public static class ArrayForm { + + private String[] codes; + + public String[] getCodes() { + return codes; + } + + public void setCodes(String[] codes) { + this.codes = codes; + } + } + + record SearchRecord(String category, String sortBy) {} + + public static class GenericForm { + + private T value; + + public T getValue() { + return value; + } + + public void setValue(T value) { + this.value = value; + } + } + + public static class WithId { + + private Long id; + private String note; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getNote() { + return note; + } + + public void setNote(String note) { + this.note = note; + } + } + + public static class Throwing { + + private String ok = "ok"; + + public String getOk() { + return ok; + } + + public void setOk(String ok) { + this.ok = ok; + } + + public String getBoom() { + throw new IllegalStateException("Not available!"); + } + + public void setBoom(String boom) {} + } + + public static class Primitives { + + private int page; + + public int getPage() { + return page; + } + + public void setPage(int page) { + this.page = page; + } + } + + public static class SearchForm { + + private String category; + private String sortBy; + private List tags; + + public String getCategory() { + return category; + } + + public void setCategory(String category) { + this.category = category; + } + + public String getSortBy() { + return sortBy; + } + + public void setSortBy(String sortBy) { + this.sortBy = sortBy; + } + + public List getTags() { + return tags; + } + + public void setTags(List tags) { + this.tags = tags; + } + } } diff --git a/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderTest.java b/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderTest.java index b8dcbf1e7..7916646d2 100644 --- a/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderTest.java +++ b/src/test/java/org/springframework/hateoas/server/reactive/WebFluxLinkBuilderTest.java @@ -37,6 +37,7 @@ import org.springframework.http.HttpEntity; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; @@ -47,6 +48,7 @@ * * @author Greg Turnquist * @author Oliver Drotbohm + * @author Kim Tae Eun */ @ExtendWith(MockitoExtension.class) class WebFluxLinkBuilderTest { @@ -239,6 +241,29 @@ void detectsParameterAnnotationOnInterfaceDeclarations() throws Exception { }); } + @Test // GH-1240 + void rendersModelAttributePropertiesAsTemplateVariables() { + + WebFluxLink link = linkTo(methodOn(ModelAttributeController.class).search(null)).withSelfRel(); + + verify(null, link, it -> { + assertThat(it.getHref()).endsWith("/search{?category,sortBy}"); + }); + } + + @Test // GH-1240 + void bindsModelAttributePropertyValues() { + + SearchForm form = new SearchForm(); + form.setCategory("books"); + + WebFluxLink link = linkTo(methodOn(ModelAttributeController.class).search(form)).withSelfRel(); + + verify(null, link, it -> { + assertThat(it.getHref()).endsWith("/search?category=books{&sortBy}"); + }); + } + private void verify(@Nullable MockServerHttpRequest request, WebFluxLink link, Consumer verifications) { Mono mono = link.toMono(); @@ -297,4 +322,35 @@ public Mono> root(String view) { return Mono.empty(); } } + + @RestController + static class ModelAttributeController { + + @GetMapping("/search") + Mono> search(@ModelAttribute SearchForm form) { + return Mono.empty(); + } + } + + public static class SearchForm { + + private String category; + private String sortBy; + + public String getCategory() { + return category; + } + + public void setCategory(String category) { + this.category = category; + } + + public String getSortBy() { + return sortBy; + } + + public void setSortBy(String sortBy) { + this.sortBy = sortBy; + } + } }