djechelon commented 2 years ago

Affects: 5.3.10

I am experiencing a problem with MVC method argument resolving in GET invocations. I have already reduced my problem to a bunch of classes and a reproducible example using Spring Test.

The idea is that I have a Filter Framework that heavily relies on generics. The first part of the FF, the one we are dealing with, is MVC argument resolution. The second part is translation into ORM predicates, which is out of scope.

@Data
public class SimpleFilter<T> {

    /**
     * Requires the property to be equal to the value
     */
    protected T eq;
    /**
     * Requires the property to be different from the value
     */
    protected T ne;
    /**
     * Requires the property to be any of the supplied values
     */
    protected T[] in;
    /**
     * Requires the property not to be any of the supplied values
     */
    protected T[] notIn;

    /**
     * Requires the property to be either not null or null.
     * The property can be used as:
     * <p>
     * - Unspecified: null check is ignored
     * - True: the property must be null
     * - False: the property must be not null
     */
    protected Boolean isNull;

}

Let's not talk about IntegerFilter and LongFilter, let's focus on a generic EnumFilter

public class EnumFilter<T extends Enum<T>> extends SimpleFilter<T> {
}

I know that Java erases generic types to the lower bound (java.lang.Enum), but the generic information is preserved in Java 11 when applied to a field.

Consider this particular filter

@Data
public class ExampleTestFilter implements Filter {

    private AmlObjectTypeFilter exampleObjectTypeEnum;

    private EnumFilter<AmlObjectType> exampleObjectTypeEnumGeneric;

    private SimpleFilter<AmlObjectType> exampleObjectTypeSimple;

    public static class AmlObjectTypeFilter extends SimpleFilter<AmlObjectType> {

    }
}

Consider the following controller:

@RestController
@RequestMapping("/exampleFiltering")
@Data
public class FilteringArgumentResolverTestController {

    private ExampleTestFilter filter;

    /**
     * Sample no-op function
     */
    @GetMapping
    @ResponseStatus(HttpStatus.NO_CONTENT)
    public void handleExampleTestFilter(@ModelAttribute("filter") ExampleTestFilter filter) {
        this.filter = filter;
    }
}

Now consider the following tests

@SpringBootTest(classes = AmlControlliBeApplication.class)
@ComponentScan(basePackageClasses = FilterArgumentResolverWeb2Test.class)
class FilterArgumentResolverWeb2Test extends AbstractOrbitE2eTest {

    private final FilteringArgumentResolverTestController controller;

    @Autowired
    FilterArgumentResolverWeb2Test(ConfigurableApplicationContext applicationContext, PlatformTransactionManager transactionManager, DataSource dataSource, MockMvc mockMvc, ObjectMapper objectMapper, FilteringArgumentResolverTestController controller) {
        super(applicationContext, transactionManager, dataSource, mockMvc, objectMapper);
        this.controller = controller;
    }

    @Test
    void resolveArgument_amlObjectTypeEnum_ok() {
        String value = "CONTROL";

        assertDoesNotThrow(() -> getMockMvc().perform(get("/exampleFiltering")
                        .queryParam("exampleObjectTypeEnum.eq", value))
                .andExpect(status().isNoContent())
        );

        assertAll(
                () -> assertThat(controller.getFilter(), is(notNullValue())),
                () -> assertThat(controller.getFilter(), is(instanceOf(ExampleTestFilter.class))),
                () -> assertThat(controller.getFilter(), hasProperty("exampleObjectTypeEnum", is(notNullValue()))),
                () -> assertThat(controller.getFilter(), hasProperty("exampleObjectTypeEnum", hasProperty("eq", equalTo(AmlObjectType.CONTROL))))
        );
    }

    @Test
    void resolveArgument_amlObjectTypeSimple_ok() {
        String value = "CONTROL";

        assertDoesNotThrow(() -> getMockMvc().perform(get("/exampleFiltering")
                        .queryParam("exampleObjectTypeSimple.eq", value))
                .andExpect(status().isNoContent())
        );
        assertAll(
                () -> assertThat(controller.getFilter(), is(notNullValue())),
                () -> assertThat(controller.getFilter(), is(instanceOf(ExampleTestFilter.class))),
                () -> assertThat(controller.getFilter(), hasProperty("exampleObjectTypeSimple", is(notNullValue()))),
                () -> assertThat(controller.getFilter(), hasProperty("exampleObjectTypeSimple", hasProperty("eq", equalTo(AmlObjectType.CONTROL))))
        );
    }
}

The problems here

If I declare (even as a static class) a boilerplate filter class bound to the AmlObjectType enum, it works. But if I declare a field as EnumFilter<AmlObjectType> Spring is unable to resolve the property as it thinks the property value should be cast to Enum, which is the generic bound of EnumFilter<T extends Enum<T>>.

E.g. using GET /api?exampleObjectTypeEnum.eq=CONTROL works, using GET /api?exampleObjectTypeEnumGeneric.eq=CONTROL or GET /api?exampleObjectTypeSimple.eq=CONTROL does not.

It would not happen using JSON POST with Jackson Object Mapper.

  assertDoesNotThrow(() -> getMockMvc().perform(get("/exampleFiltering")
                        .queryParam("exampleObjectTypeSimple.eq", value))
                .andExpect(status().isNoContent())
        );

Caused by: java.lang.AssertionError: 
Expected: hasProperty("exampleObjectTypeSimple", hasProperty("eq", <CONTROL>))
     but:  property 'exampleObjectTypeSimple'  property 'eq' was "CONTROL"
    at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
    at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
    at it.orbit.common.web.filtering.FilterArgumentResolverWeb2Test.lambda$resolveArgument_amlObjectTypeSimple_ok$44(FilterArgumentResolverWeb2Test.java:195)
    at org.junit.jupiter.api.AssertAll.lambda$assertAll$1(AssertAll.java:68)
    at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
    at java.base/java.util.stream.ReferencePipeline$11$1.accept(ReferencePipeline.java:442)
    at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
    at org.junit.jupiter.api.AssertAll.assertAll(AssertAll.java:77)
    ... 87 more

In this case, using SimpleFilter<AmlObjectType> results in Spring setting the value CONTROL (String) as an Object into the property, which fails the assertion because I am comparing an enum with a string.

  assertDoesNotThrow(() -> getMockMvc().perform(get("/exampleFiltering")
                        .queryParam("exampleObjectTypeEnumGeneric.eq", value))
                .andExpect(status().isNoContent())
        );

2021-12-02 16:40:48.514 DEBUG 2248 --- [    Test worker] o.s.t.web.servlet.TestDispatcherServlet  : GET "/exampleFiltering?exampleObjectTypeEnumGeneric.eq=CONTROL", parameters={masked}
2021-12-02 16:40:48.520  WARN 2248 --- [    Test worker] .w.s.m.s.DefaultHandlerExceptionResolver : Resolved [org.springframework.validation.BindException: org.springframework.validation.BeanPropertyBindingResult: 1 errors
Field error in object 'filter' on field 'exampleObjectTypeEnumGeneric.eq': rejected value [CONTROL]; codes [typeMismatch.filter.exampleObjectTypeEnumGeneric.eq,typeMismatch.exampleObjectTypeEnumGeneric.eq,typeMismatch.eq,typeMismatch.java.lang.Enum,typeMismatch]; arguments [org.springframework.context.support.DefaultMessageSourceResolvable: codes [filter.exampleObjectTypeEnumGeneric.eq,exampleObjectTypeEnumGeneric.eq]; arguments []; default message [exampleObjectTypeEnumGeneric.eq]]; default message [Failed to convert property value of type 'java.lang.String' to required type 'java.lang.Enum' for property 'exampleObjectTypeEnumGeneric.eq'; nested exception is java.lang.IllegalArgumentException: The target type java.lang.Enum does not refer to an enum]]

In this case, Spring detects the target type as Enum, and can't cast CONTROL to the root Enum class

  assertDoesNotThrow(() -> getMockMvc().perform(get("/exampleFiltering")
                        .queryParam("exampleObjectTypeEnum.eq", value))
                .andExpect(status().isNoContent())
        );

This succeeds because target property is of type AmlObjectTypeFilter which is bound directly to AmlObjectType

Debugging

org.springframework.beans.PropertyBatchUpdateException; nested PropertyAccessException details (1) are:
PropertyAccessException 1:
org.springframework.beans.TypeMismatchException: Failed to convert property value of type 'java.lang.String' to required type 'java.lang.Enum' for property 'exampleObjectTypeEnumGeneric.eq'; nested exception is java.lang.IllegalArgumentException: The target type java.lang.Enum does not refer to an enum
    at org.springframework.beans.AbstractNestablePropertyAccessor.convertIfNecessary(AbstractNestablePropertyAccessor.java:600)
    at org.springframework.beans.AbstractNestablePropertyAccessor.convertForProperty(AbstractNestablePropertyAccessor.java:609)
    at org.springframework.beans.AbstractNestablePropertyAccessor.processLocalProperty(AbstractNestablePropertyAccessor.java:458)
    at org.springframework.beans.AbstractNestablePropertyAccessor.setPropertyValue(AbstractNestablePropertyAccessor.java:278)
    at org.springframework.beans.AbstractNestablePropertyAccessor.setPropertyValue(AbstractNestablePropertyAccessor.java:266)
    at org.springframework.beans.AbstractPropertyAccessor.setPropertyValues(AbstractPropertyAccessor.java:104)
    at org.springframework.validation.DataBinder.applyPropertyValues(DataBinder.java:851)
    at org.springframework.validation.DataBinder.doBind(DataBinder.java:747)
    at org.springframework.web.bind.WebDataBinder.doBind(WebDataBinder.java:198)
    at org.springframework.web.bind.ServletRequestDataBinder.bind(ServletRequestDataBinder.java:118)
    at org.springframework.web.servlet.mvc.method.annotation.ServletModelAttributeMethodProcessor.bindRequestParameters(ServletModelAttributeMethodProcessor.java:158)
    at org.springframework.web.method.annotation.ModelAttributeMethodProcessor.resolveArgument(ModelAttributeMethodProcessor.java:171)
    at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:121)
    at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:179)
    at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:146)
    at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:117)
    at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:895)
    at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:808)
    at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
    at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1067)
    at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:963)
    at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)
    at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:655)
    at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)
    at org.springframework.test.web.servlet.TestDispatcherServlet.service(TestDispatcherServlet.java:72)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:764)
    at org.springframework.mock.web.MockFilterChain$ServletFilterProxy.doFilter(MockFilterChain.java:167)
    at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:134)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:327)
    at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.invoke(FilterSecurityInterceptor.java:115)
    at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.doFilter(FilterSecurityInterceptor.java:81)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:121)
    at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:115)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.security.web.authentication.AnonymousAuthenticationFilter.doFilter(AnonymousAuthenticationFilter.java:105)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter.doFilter(SecurityContextHolderAwareRequestFilter.java:149)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.security.web.savedrequest.RequestCacheAwareFilter.doFilter(RequestCacheAwareFilter.java:63)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.security.oauth2.server.resource.web.BearerTokenAuthenticationFilter.doFilterInternal(BearerTokenAuthenticationFilter.java:121)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:103)
    at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:89)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.web.filter.CorsFilter.doFilterInternal(CorsFilter.java:91)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.security.web.header.HeaderWriterFilter.doHeadersAfter(HeaderWriterFilter.java:90)
    at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:75)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:110)
    at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:80)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter.doFilterInternal(WebAsyncManagerIntegrationFilter.java:55)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
    at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:211)
    at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:183)
    at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:358)
    at org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:271)
    at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:134)
    at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:134)
    at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)
    at org.springframew

I also found that Spring could be aware of the actual generic type of a field at some point. I was spelunking deep into Introspector and found the below

Screenshot debugging

This shows that the generic type is known for the field, but then looks like Spring ignores the property generic binding and works only on the (raw....) type.

At AbstractNestablePropertyAccessors# variable ph has now lost any information about the generic binding of the filter class's property.

And at line 590 the "target class" is now just Enum.

Porkarounds

Use HTTP POST to deserialize the filter object
Define a subclass for each Enum you want to use in a filter

djechelon commented 2 years ago

Thank you for the attention. Is it worth investigating a way to determine the target class based on the field's declaration?

poutsma commented 2 years ago

Thank you for the detailed report. However, we would prefer to have that code in a complete sample with minimal dependencies (i.e. no lombok, Spring Security, or other unrelated libraries). Something that we can unzip or git clone, build, and deploy, and that reproduces the problem.

rstoyanchev commented 2 years ago

If you want your issue to be considered, please try to provide a more focused description. A snippet of a test class with 10 methods and 200 lines of code is just not helpful.

spring-projects-issues commented 2 years ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

djechelon commented 2 years ago

Repository available at https://github.com/djechelon/spring-poc-27760

The repository contains a test controller (which basically does nothing) and a test class FilterArgumentResolverWebTest. I used only the testing class to operate the project. I actually never cared about starting the thing (in a real TDD approach).

Please note that while discussing about enums, I have also implemented an interesting test about OffsetDateTime unparseable.

As stated in the original post, you will find that:

If you define public static class AmlObjectTypeFilter extends SimpleFilter<AmlObjectType> then Spring successfully takes AmlObjectType as type of the property and decodes CONTROL string properly
If you rely on the generic SimpleFilter<AmlObjectType> the property is set with a String value

About item 2, there is yet another test resolveArgument_lookAtTheCast where the invocation succeeds (i.e. 204 code) but then you can't get the item as an enum. It is particularly required for the real case of the filter framework when these parameters need to be passed to the persistence layer.

The ultimate goal of the SimpleFilter class & company is to allow table filtering at some degree of dynamicity, e.g. it's cool to invoke ?author.eq=djechelon in an MVC controller

snicoll commented 10 months ago

@djechelon I've looked at the repository and played a little bit with it and I am afraid we're in the same state as what Rossen described above. It looks like the sample as a lot of unnecessary noise. Can you please trim it down so that it's obvious what you're trying to do?

spring-projects-issues commented 10 months ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

djechelon commented 10 months ago

Okay, let me try to summarize and update the repository whith what's important. Since then, I have changed job and I don't have any more business interest in this

Goal is to have Spring read query string parameters into a complex object. In the old business scenario, it was a filter object for queries.

Sample requests include

GET /data?name.eq=John&surname.in=Doe&surname.in=Roe

By design, I leverage a generic SimpleFilter<T> where T can be anything but is very well declared at compile time.

I am having problems with enums in particular. I have added a test case in which you can see that if you POST the filter and let Jackson handle it, it will decode it fine (I have left only String filter and Enum filter in the sample repository). Jackson decodes string literal CONTROL into enum AmlObjectType.CONTROL.

Spring REST does not decode that literal into an enum, but rather reflects "CONTROL" string literal into the target object

Expected: hasProperty("exampleObjectType", hasProperty("eq", an instance of org.zighinetto.springpoc.AmlObjectType))
     but:  property 'exampleObjectType'  property 'eq' "CONTROL" is a java.lang.String

If I try to get the "eq" value for constructing a filter in runtime code, I will get a ClassCastException.

Repo updated

sdeleuze commented 9 months ago

@rstoyanchev @snicoll Maybe the difference of behavior could be related to the fact that in @ModelAttribute + GET use case, GenericTypeResolver#resolveType is not invoked, while for @RequestBody + POST use case, it is via AbstractJackson2HttpMessageConverter#getJavaType.

jhoeller commented 8 months ago

The difference comes from every nested bean being independently bound: Once the outer ExampleTestFilter instance is dereferenced to the inner SimpleFilter field, further binding occurs on that SimpleFilter instance - resolving type variables against its class but not taking into account where that instance came from, so losing the relationship to the field and its generic declaration in the outer class.

We can try to revisit this for 6.2 but it won't be straightforward since standard JavaBeans introspection operates against a Class.

djechelon commented 7 months ago

it won't be straightforward since standard JavaBeans introspection operates against a Class.

Correct. Jackson works against a Type (e.g. TypedReference), so this explains why this works for the POST payload

spring-projects / spring-framework

Resolving generic enum types (Enum<T>) in nested bean properties #27760

The problems here

Debugging

Porkarounds