From 9aa9707f3798f0f0da4f5fefd4b16e31f54bf5a1 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Thu, 10 Sep 2020 12:12:45 -0400 Subject: [PATCH] GH-3366: Return null from HTTP handleNoMatch Fixes: https://github.com/spring-projects/spring-integration/issues/3366 When the same path is mapped for integration HTTP endpoint and MVC method mapping, but different other mapping options (e.g. method) and one of them fails to match, there is no way to try other `RequestMapping` from the `DispatcherServlet` because `RequestMappingHandlerMapping.handleNoMatch()` throws an exception when no match instead of `null` according chain of responsibility logic in the `DispatcherServlet` * Rework `IntegrationRequestMappingHandlerMapping.handleNoMatch()` to catch all the super exception and return `null` to the `DispatcherServlet` to let it to try other `RequestMapping` from the configuration * Change an order for `IntegrationRequestMappingHandlerMapping` to `-1` to let it to be tried first before regular MVC `RequestMappingHandlerMapping` * Add a test-case to ensure that mix-in Integration HTTP and MVC for the same path works as expected without failing on first try --- ...tpIntegrationConfigurationInitializer.java | 9 +++--- ...tegrationRequestMappingHandlerMapping.java | 14 +++++++++ .../HttpInboundChannelAdapterParserTests.java | 13 ++------ .../integration/http/dsl/HttpDslTests.java | 30 ++++++++++++++++++- 4 files changed, 50 insertions(+), 16 deletions(-) diff --git a/spring-integration-http/src/main/java/org/springframework/integration/http/config/HttpIntegrationConfigurationInitializer.java b/spring-integration-http/src/main/java/org/springframework/integration/http/config/HttpIntegrationConfigurationInitializer.java index 3c7d01990e..b1959e3cb8 100644 --- a/spring-integration-http/src/main/java/org/springframework/integration/http/config/HttpIntegrationConfigurationInitializer.java +++ b/spring-integration-http/src/main/java/org/springframework/integration/http/config/HttpIntegrationConfigurationInitializer.java @@ -55,8 +55,7 @@ public class HttpIntegrationConfigurationInitializer implements IntegrationConfi * which could also be overridden by the user by simply registering * a {@link IntegrationRequestMappingHandlerMapping} {@code } with 'id' * {@link HttpContextUtils#HANDLER_MAPPING_BEAN_NAME}. - *

- * In addition, checks if the {@code javax.servlet.Servlet} class is present on the classpath. + *

In addition, checks if the {@code javax.servlet.Servlet} class is present on the classpath. * When Spring Integration HTTP is used only as an HTTP client, there is no reason to use and register * the HTTP server components. */ @@ -64,9 +63,9 @@ public class HttpIntegrationConfigurationInitializer implements IntegrationConfi if (HttpContextUtils.WEB_MVC_PRESENT && !registry.containsBeanDefinition(HttpContextUtils.HANDLER_MAPPING_BEAN_NAME)) { BeanDefinitionBuilder requestMappingBuilder = - BeanDefinitionBuilder.genericBeanDefinition(IntegrationRequestMappingHandlerMapping.class); - requestMappingBuilder.setRole(BeanDefinition.ROLE_INFRASTRUCTURE); - requestMappingBuilder.addPropertyValue(IntegrationNamespaceUtils.ORDER, 0); + BeanDefinitionBuilder.genericBeanDefinition(IntegrationRequestMappingHandlerMapping.class) + .setRole(BeanDefinition.ROLE_INFRASTRUCTURE) + .addPropertyValue(IntegrationNamespaceUtils.ORDER, -1); registry.registerBeanDefinition(HttpContextUtils.HANDLER_MAPPING_BEAN_NAME, requestMappingBuilder.getBeanDefinition()); } diff --git a/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/IntegrationRequestMappingHandlerMapping.java b/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/IntegrationRequestMappingHandlerMapping.java index b353e02069..12accdc7b2 100644 --- a/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/IntegrationRequestMappingHandlerMapping.java +++ b/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/IntegrationRequestMappingHandlerMapping.java @@ -21,8 +21,10 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -177,6 +179,18 @@ public final class IntegrationRequestMappingHandlerMapping extends RequestMappin return null; } + @Override + protected HandlerMethod handleNoMatch(Set infos, String path, HttpServletRequest request) { + try { + return super.handleNoMatch(infos, path, request); + } + catch (ServletException ex) { + // Since this component has a higher precedence the 'null' return allows a + // 'DispatcherServlet' to try other 'HandlerMapping' + return null; + } + } + private static CorsConfiguration buildCorsConfiguration(CrossOrigin crossOrigin, RequestMappingInfo mappingInfo) { CorsConfiguration config = new CorsConfiguration(); for (RequestMethod requestMethod : crossOrigin.getMethod()) { diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/config/HttpInboundChannelAdapterParserTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/config/HttpInboundChannelAdapterParserTests.java index dc5e9a85ca..365e365447 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/config/HttpInboundChannelAdapterParserTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/config/HttpInboundChannelAdapterParserTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -17,7 +17,6 @@ package org.springframework.integration.http.config; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.willReturn; @@ -56,7 +55,6 @@ import org.springframework.test.context.junit4.SpringRunner; import org.springframework.util.AntPathMatcher; import org.springframework.util.MultiValueMap; import org.springframework.validation.Validator; -import org.springframework.web.HttpRequestMethodNotSupportedException; import org.springframework.web.servlet.HandlerMapping; @@ -195,18 +193,13 @@ public class HttpInboundChannelAdapterParserTests extends AbstractHttpInboundTes } @Test - public void getRequestNotAllowed() { + public void getRequestNotAllowed() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("GET"); request.setParameter("foo", "bar"); request.setRequestURI("/postOnly"); - assertThatExceptionOfType(HttpRequestMethodNotSupportedException.class) - .isThrownBy(() -> this.integrationRequestMappingHandlerMapping.getHandler(request)) - .satisfies((ex) -> { - assertThat(ex.getMethod()).isEqualTo("GET"); - assertThat(ex.getSupportedMethods()).containsExactly("POST"); - }); + assertThat(this.integrationRequestMappingHandlerMapping.getHandler(request)).isNull(); } @Test diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/dsl/HttpDslTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/dsl/HttpDslTests.java index b7eb2ceb21..9dcebc44a9 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/dsl/HttpDslTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/dsl/HttpDslTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2019 the original author or authors. + * Copyright 2016-2020 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. @@ -78,6 +78,8 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.util.StringUtils; import org.springframework.validation.Errors; import org.springframework.validation.Validator; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; import org.springframework.web.client.DefaultResponseErrorHandler; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.context.WebApplicationContext; @@ -85,6 +87,7 @@ import org.springframework.web.multipart.MultipartResolver; import org.springframework.web.multipart.support.StandardServletMultipartResolver; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.config.annotation.EnableWebMvc; /** * @author Artem Bilan @@ -208,6 +211,7 @@ public class HttpDslTests { } @Autowired + @Qualifier("customValidator") private Validator validator; @Test @@ -293,9 +297,20 @@ public class HttpDslTests { flowRegistration.destroy(); } + @Test + public void testMixWithMvcRequestMapping() throws Exception { + this.mockMvc.perform( + get("/mvcRequest") + .with(httpBasic("user", "user"))) + .andExpect(status().isOk()) + .andExpect(content().string("MVC reply")); + } + @Configuration @EnableWebSecurity @EnableIntegration + @EnableWebMvc + @RestController public static class ContextConfiguration extends WebSecurityConfigurerAdapter { @Override @@ -409,6 +424,19 @@ public class HttpDslTests { return new TestModelValidator(); } + + @GetMapping("/mvcRequest") + ResponseEntity mvcGet() { + return ResponseEntity.ok("MVC reply"); + } + + @Bean + HttpRequestHandlerEndpointSpec mvcHandler() { + return Http.inboundChannelAdapter("/mvcRequest") + .requestMapping((mapping) -> mapping.methods(HttpMethod.POST)); + } + + } public static class HttpProxyResponseErrorHandler extends DefaultResponseErrorHandler {