Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GenericTypeResolver does not resolve nested generics - causes issues with Jackson & Spring MVC #31690

Closed
harmonic-ben opened this issue Nov 26, 2023 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@harmonic-ben
Copy link

harmonic-ben commented Nov 26, 2023

This affects spring 6.1.1

The following test passes on 6.0.14 but fails on 6.1.1

import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.util.ReflectionUtils.findMethod;

import java.lang.reflect.Method;
import java.lang.reflect.Type;
import java.util.List;
import java.util.Map;

import org.junit.jupiter.api.Test;
import org.springframework.core.GenericTypeResolver;
import org.springframework.core.MethodParameter;
import org.springframework.core.ParameterizedTypeReference;

public class GenericConverterTypeTest {

    @Test
    public void foo() {
        Method method = findMethod(Bar.class, "accept", (Class<?>[])null);
        MethodParameter intMessageMethodParam = new MethodParameter(method, 0);
        Type resolveType = GenericTypeResolver.resolveType(intMessageMethodParam.getGenericParameterType(), Bar.class);
        ParameterizedTypeReference<List<Map<String, Integer>>> reference = new ParameterizedTypeReference<List<Map<String, Integer>>>() {
        };
        assertThat(resolveType).isEqualTo(reference.getType());
    }

    public class Bar {
        public void accept(List<Map<String, Integer>> input) {

        }
    }
}

Failure message:

org.opentest4j.AssertionFailedError: 
expected: java.util.List<java.util.Map<java.lang.String, java.lang.Integer>>
 but was: java.util.List<java.util.Map>
	at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502)
	at app.hs.platform.server.GenericConverterTypeTest.foo(GenericConverterTypeTest.java:25)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

I believe this was caused by f075120

This causes nested generic deserialisation to fail from jackson due to the use of GenericTypeResolver in org.springframework.http.converter.json.AbstractJackson2HttpMessageConverter.getJavaType(Type, Class<?>)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 26, 2023
@quaff
Copy link
Contributor

quaff commented Nov 27, 2023

It's duplicate of #24963
It should be fixed by #30079

@snicoll
Copy link
Member

snicoll commented Nov 27, 2023

Thanks @quaff, I also assumed your work would fix this. However I don't think this is a duplicate as this used to work in 6.0.x and I'd like to understand why it broke.

@harmonic-ben
Copy link
Author

Hadn't really had time to look at this properly hence the poorly formatted test and believe line but it seems GenericTypeResolver.resolveType purpose is for using the context class to fill in additional type information. In this case it seems the type arrives will all type information supplied and just needs to be passed back. i.e.
#30079 seems to be doing a lot of work for a type that needs no enhancement.

@sdeleuze sdeleuze self-assigned this Nov 29, 2023
@sdeleuze sdeleuze added type: regression A bug that is also a regression in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 29, 2023
@sdeleuze sdeleuze added this to the 6.1.2 milestone Nov 29, 2023
@sdeleuze
Copy link
Contributor

That's a bit embarrassing, but it looks like I shipped involuntarily a commit related to #22313 which is still opened while working on another issue, sorry about that. When I worked deeper on that issue, I found that by design Kotlin reflection is required to solve the Kotlin issue, and I was afraid of potential regressions on Java side, and chose to document the behavior via #31370. As a consequence, I plan to revert f075120.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

5 participants