Skip to content

Commit

Permalink
Add MVC support for resolving request body bounds on generic parameters
Browse files Browse the repository at this point in the history
Before this commit, Spring MVC was unable to resolve upper or lower
bounds on Spring MVC request body parameter when generic controller
inheritance was involved. This use case is common with Kotlin
which uses declaration-site variance for List<T>.

This commit adds a related GenericTypeResolver#resolveGenericBounds
used in HttpEntityMethodProcessor and RequestResponseBodyMethodProcessor
to resolve properly generic parameters.

Closes spring-projectsgh-24033
  • Loading branch information
sdeleuze committed Mar 6, 2023
1 parent 9548101 commit 1600a05
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
* @author Rob Harrop
* @author Sam Brannen
* @author Phillip Webb
* @author Sebastien Deleuze
* @since 2.5.2
*/
public final class GenericTypeResolver {
Expand Down Expand Up @@ -195,6 +196,34 @@ else if (genericType instanceof ParameterizedType parameterizedType) {
return genericType;
}

/**
* Resolve the generic parameters of the specified type against their upper or lower bounds.
* @param genericType the (potentially) generic type
* @return the resolved type (possibly the given generic type as-is)
* @since 6.0.7
*/
public static ResolvableType resolveGenericBounds(ResolvableType genericType) {
Class<?> resolved = genericType.resolve();
if (resolved != null) {
ResolvableType[] generics = genericType.getGenerics();
boolean hasBoundToResolve = false;
for (int i = 0; i < generics.length; i++) {
ResolvableType generic = generics[i];
if (generic.getType() instanceof WildcardType) {
ResolvableType resolvedGeneric = generic.resolveType();
if (resolvedGeneric != ResolvableType.NONE) {
generics[i] = resolvedGeneric;
hasBoundToResolve = true;
}
}
}
if (hasBoundToResolve) {
return ResolvableType.forClassWithGenerics(resolved, generics);
}
}
return genericType;
}

private static ResolvableType resolveVariable(TypeVariable<?> typeVariable, ResolvableType contextType) {
ResolvableType resolvedType;
if (contextType.hasGenerics()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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.
Expand Down Expand Up @@ -29,6 +29,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.core.GenericTypeResolver.getTypeVariableMap;
import static org.springframework.core.GenericTypeResolver.resolveGenericBounds;
import static org.springframework.core.GenericTypeResolver.resolveReturnTypeArgument;
import static org.springframework.core.GenericTypeResolver.resolveType;
import static org.springframework.core.GenericTypeResolver.resolveTypeArgument;
Expand All @@ -37,6 +38,7 @@
/**
* @author Juergen Hoeller
* @author Sam Brannen
* @author Sebastien Deleuze
*/
@SuppressWarnings({"unchecked", "rawtypes"})
class GenericTypeResolverTests {
Expand Down Expand Up @@ -174,17 +176,52 @@ void resolveIncompleteTypeVariables() {
}

@Test
public void resolvePartiallySpecializedTypeVariables() {
void resolvePartiallySpecializedTypeVariables() {
Type resolved = resolveType(BiGenericClass.class.getTypeParameters()[0], TypeFixedBiGenericClass.class);
assertThat(resolved).isEqualTo(D.class);
}

@Test
public void resolveTransitiveTypeVariableWithDifferentName() {
void resolveTransitiveTypeVariableWithDifferentName() {
Type resolved = resolveType(BiGenericClass.class.getTypeParameters()[1], TypeFixedBiGenericClass.class);
assertThat(resolved).isEqualTo(E.class);
}

@Test
void resolveUpperBound() {
Method method = findMethod(MySimpleSuperclassType.class, "upperBound", List.class);
ResolvableType resolvedType = resolveGenericBounds(ResolvableType.forMethodParameter(method, 0, MySimpleSuperclassType.class));
assertThat(resolvedType.resolveGenerics()).containsExactly(String.class);
}

@Test
void resolveLowerBound() {
Method method = findMethod(MySimpleSuperclassType.class, "lowerBound", List.class);
ResolvableType resolvedType = resolveGenericBounds(ResolvableType.forMethodParameter(method, 0, MySimpleSuperclassType.class));
assertThat(resolvedType.resolveGenerics()).containsExactly(String.class);
}

@Test
void skipBoundResolutionForUnbounded() {
Method method = findMethod(MySimpleSuperclassType.class, "unbounded", List.class);
ResolvableType resolvableType = ResolvableType.forMethodParameter(method, 0, MySimpleSuperclassType.class);
assertThat(resolveGenericBounds(resolvableType)).isEqualTo(resolvableType);
}

@Test
void skipBoundResolutionForResolved() {
Method method = findMethod(MySimpleSuperclassType.class, "resolved", List.class);
ResolvableType resolvableType = ResolvableType.forMethodParameter(method, 0, MySimpleSuperclassType.class);
assertThat(resolveGenericBounds(resolvableType)).isEqualTo(resolvableType);
}

@Test
void skipBoundResolutionForNonGeneric() {
Method method = findMethod(MySimpleSuperclassType.class, "string", String.class);
ResolvableType resolvableType = ResolvableType.forMethodParameter(method, 0, MySimpleSuperclassType.class);
assertThat(resolveGenericBounds(resolvableType)).isEqualTo(resolvableType);
}

public interface MyInterfaceType<T> {
}

Expand All @@ -195,6 +232,21 @@ public class MyCollectionInterfaceType implements MyInterfaceType<Collection<Str
}

public abstract class MySuperclassType<T> {

public void upperBound(List<? extends T> list) {
}

public void lowerBound(List<? super T> list) {
}

public void unbounded(List<?> list) {
}

public void resolved(List<String> list) {
}

public void string(String value) {
}
}

public class MySimpleSuperclassType extends MySuperclassType<String> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

import org.springframework.core.GenericTypeResolver;
import org.springframework.core.MethodParameter;
import org.springframework.core.ResolvableType;
import org.springframework.http.HttpEntity;
Expand Down Expand Up @@ -152,7 +153,8 @@ public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewC
@Nullable
private Type getHttpEntityType(MethodParameter parameter) {
Assert.isAssignable(HttpEntity.class, parameter.getParameterType());
Type parameterType = parameter.getGenericParameterType();
ResolvableType resolvableType = ResolvableType.forMethodParameter(parameter);
Type parameterType = GenericTypeResolver.resolveGenericBounds(resolvableType).getType();
if (parameterType instanceof ParameterizedType type) {
if (type.getActualTypeArguments().length != 1) {
throw new IllegalArgumentException("Expected single generic parameter on '" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
import jakarta.servlet.http.HttpServletRequest;

import org.springframework.core.Conventions;
import org.springframework.core.GenericTypeResolver;
import org.springframework.core.MethodParameter;
import org.springframework.core.ResolvableType;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.http.HttpStatusCode;
import org.springframework.http.ProblemDetail;
Expand Down Expand Up @@ -133,7 +135,9 @@ public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewC
NativeWebRequest webRequest, @Nullable WebDataBinderFactory binderFactory) throws Exception {

parameter = parameter.nestedIfOptional();
Object arg = readWithMessageConverters(webRequest, parameter, parameter.getNestedGenericParameterType());
ResolvableType resolvableType = ResolvableType.forMethodParameter(parameter);
Type parameterType = GenericTypeResolver.resolveGenericBounds(resolvableType).getType();
Object arg = readWithMessageConverters(webRequest, parameter, parameterType);
String name = Conventions.getVariableNameForParameter(parameter);

if (binderFactory != null) {
Expand Down

0 comments on commit 1600a05

Please sign in to comment.