From 8d88e291736d02c38bb6f068f47535d65e0e7baf Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Wed, 30 Oct 2019 14:52:43 +0100 Subject: [PATCH 1/3] Add qualified injection points for MVC and WebFlux infrastructure Previously, the infrastructure provided by WebMvcConfigurationSupport and WebFluxConfigurationSupport can lead to unexpected results due to the lack of qualifier for certain dependencies. Those configuration classes refer to very specific beans, yet their injection points do not define such qualifiers. As a result, if a candidate exists for the requested type, the context will inject the existing bean and will ignore a most specific one as such constraint it not defined. This can be easily reproduced by having a primary Validator whereas a dedicated "mvcValidator" is expected. Note that a parameter name is in no way a constraint as the name is only used as a fallback when a single candidate cannot be determined. This commit provides explicit @Qualifier metadata for such injection points, renaming the parameter name in the process to clarify that it isn't relevant for the proper bean to be resolved by the context. Closes gh-23887 --- .../config/WebFluxConfigurationSupport.java | 33 +-- ...gWebFluxConfigurationIntegrationTests.java | 199 +++++++++++++++++ .../WebMvcConfigurationSupport.java | 76 ++++--- ...ngWebMvcConfigurationIntegrationTests.java | 201 ++++++++++++++++++ 4 files changed, 460 insertions(+), 49 deletions(-) create mode 100644 spring-webflux/src/test/java/org/springframework/web/reactive/config/DelegatingWebFluxConfigurationIntegrationTests.java create mode 100644 spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/DelegatingWebMvcConfigurationIntegrationTests.java diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java index ab99a8d5d1c5..f2f40e8ea506 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java @@ -24,6 +24,7 @@ import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.BeanInitializationException; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.context.annotation.Bean; @@ -120,10 +121,10 @@ public WebExceptionHandler responseStatusExceptionHandler() { @Bean public RequestMappingHandlerMapping requestMappingHandlerMapping( - RequestedContentTypeResolver webFluxContentTypeResolver) { + @Qualifier("webFluxContentTypeResolver") RequestedContentTypeResolver contentTypeResolver) { RequestMappingHandlerMapping mapping = createRequestMappingHandlerMapping(); mapping.setOrder(0); - mapping.setContentTypeResolver(webFluxContentTypeResolver); + mapping.setContentTypeResolver(contentTypeResolver); mapping.setCorsConfigurations(getCorsConfigurations()); PathMatchConfigurer configurer = getPathMatchConfigurer(); @@ -265,14 +266,14 @@ protected void addResourceHandlers(ResourceHandlerRegistry registry) { @Bean public RequestMappingHandlerAdapter requestMappingHandlerAdapter( - ReactiveAdapterRegistry webFluxAdapterRegistry, + @Qualifier("webFluxAdapterRegistry") ReactiveAdapterRegistry reactiveAdapterRegistry, ServerCodecConfigurer serverCodecConfigurer, - FormattingConversionService webFluxConversionService, - Validator webfluxValidator) { + @Qualifier("webFluxConversionService") FormattingConversionService conversionService, + @Qualifier("webFluxValidator") Validator validator) { RequestMappingHandlerAdapter adapter = createRequestMappingHandlerAdapter(); adapter.setMessageReaders(serverCodecConfigurer.getReaders()); - adapter.setWebBindingInitializer(getConfigurableWebBindingInitializer(webFluxConversionService, webfluxValidator)); - adapter.setReactiveAdapterRegistry(webFluxAdapterRegistry); + adapter.setWebBindingInitializer(getConfigurableWebBindingInitializer(conversionService, validator)); + adapter.setReactiveAdapterRegistry(reactiveAdapterRegistry); ArgumentResolverConfigurer configurer = new ArgumentResolverConfigurer(); configureArgumentResolvers(configurer); @@ -426,30 +427,30 @@ public SimpleHandlerAdapter simpleHandlerAdapter() { @Bean public ResponseEntityResultHandler responseEntityResultHandler( - ReactiveAdapterRegistry webFluxAdapterRegistry, + @Qualifier("webFluxAdapterRegistry") ReactiveAdapterRegistry reactiveAdapterRegistry, ServerCodecConfigurer serverCodecConfigurer, - RequestedContentTypeResolver webFluxContentTypeResolver) { + @Qualifier("webFluxContentTypeResolver") RequestedContentTypeResolver contentTypeResolver) { return new ResponseEntityResultHandler(serverCodecConfigurer.getWriters(), - webFluxContentTypeResolver, webFluxAdapterRegistry); + contentTypeResolver, reactiveAdapterRegistry); } @Bean public ResponseBodyResultHandler responseBodyResultHandler( - ReactiveAdapterRegistry webFluxAdapterRegistry, + @Qualifier("webFluxAdapterRegistry") ReactiveAdapterRegistry reactiveAdapterRegistry, ServerCodecConfigurer serverCodecConfigurer, - RequestedContentTypeResolver webFluxContentTypeResolver) { + @Qualifier("webFluxContentTypeResolver") RequestedContentTypeResolver contentTypeResolver) { return new ResponseBodyResultHandler(serverCodecConfigurer.getWriters(), - webFluxContentTypeResolver, webFluxAdapterRegistry); + contentTypeResolver, reactiveAdapterRegistry); } @Bean public ViewResolutionResultHandler viewResolutionResultHandler( - ReactiveAdapterRegistry webFluxAdapterRegistry, - RequestedContentTypeResolver webFluxContentTypeResolver) { + @Qualifier("webFluxAdapterRegistry") ReactiveAdapterRegistry reactiveAdapterRegistry, + @Qualifier("webFluxContentTypeResolver") RequestedContentTypeResolver contentTypeResolver) { ViewResolverRegistry registry = getViewResolverRegistry(); List resolvers = registry.getViewResolvers(); ViewResolutionResultHandler handler = new ViewResolutionResultHandler( - resolvers, webFluxContentTypeResolver, webFluxAdapterRegistry); + resolvers, contentTypeResolver, reactiveAdapterRegistry); handler.setDefaultViews(registry.getDefaultViews()); handler.setOrder(registry.getOrder()); return handler; diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/config/DelegatingWebFluxConfigurationIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/config/DelegatingWebFluxConfigurationIntegrationTests.java new file mode 100644 index 000000000000..35df96e88bba --- /dev/null +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/config/DelegatingWebFluxConfigurationIntegrationTests.java @@ -0,0 +1,199 @@ +/* + * Copyright 2002-2019 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.web.reactive.config; + +import java.util.function.Consumer; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.core.ReactiveAdapterRegistry; +import org.springframework.format.support.FormattingConversionService; +import org.springframework.validation.Validator; +import org.springframework.web.reactive.accept.RequestedContentTypeResolver; +import org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerAdapter; +import org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerMapping; +import org.springframework.web.reactive.result.method.annotation.ResponseBodyResultHandler; +import org.springframework.web.reactive.result.method.annotation.ResponseEntityResultHandler; +import org.springframework.web.reactive.result.view.ViewResolutionResultHandler; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +/** + * Integration tests for {@link DelegatingWebFluxConfiguration}. + * + * @author Stephane Nicoll + */ +public class DelegatingWebFluxConfigurationIntegrationTests { + + private AnnotationConfigApplicationContext context; + + @AfterEach + public void closeContext() { + if (this.context != null) { + this.context.close(); + } + } + + @Test + void requestMappingHandlerMappingUsesWebFluxInfrastructureByDefault() { + load(context -> { }); + RequestMappingHandlerMapping handlerMapping = this.context.getBean(RequestMappingHandlerMapping.class); + assertThat(handlerMapping.getContentTypeResolver()).isSameAs(this.context.getBean("webFluxContentTypeResolver")); + } + + @Test + void requestMappingHandlerMappingWithPrimaryUsesQualifiedRequestedContentTypeResolver() { + load(registerPrimaryBean("testContentTypeResolver", RequestedContentTypeResolver.class)); + RequestMappingHandlerMapping handlerMapping = this.context.getBean(RequestMappingHandlerMapping.class); + assertThat(handlerMapping.getContentTypeResolver()).isSameAs(this.context.getBean("webFluxContentTypeResolver")); + assertThat(this.context.getBeansOfType(RequestedContentTypeResolver.class)).containsOnlyKeys( + "webFluxContentTypeResolver", "testContentTypeResolver"); + } + + @Test + void requestMappingHandlerAdapterUsesWebFluxInfrastructureByDefault() { + load(context -> { }); + RequestMappingHandlerAdapter mappingHandlerAdapter = this.context.getBean(RequestMappingHandlerAdapter.class); + assertThat(mappingHandlerAdapter.getReactiveAdapterRegistry()).isSameAs(this.context.getBean("webFluxAdapterRegistry")); + assertThat(mappingHandlerAdapter.getWebBindingInitializer()).hasFieldOrPropertyWithValue("conversionService", + this.context.getBean("webFluxConversionService")); + assertThat(mappingHandlerAdapter.getWebBindingInitializer()).hasFieldOrPropertyWithValue("validator", + this.context.getBean("webFluxValidator")); + } + + @Test + void requestMappingHandlerAdapterWithPrimaryUsesQualifiedReactiveAdapterRegistry() { + load(registerPrimaryBean("testReactiveAdapterRegistry", ReactiveAdapterRegistry.class)); + RequestMappingHandlerAdapter mappingHandlerAdapter = this.context.getBean(RequestMappingHandlerAdapter.class); + assertThat(mappingHandlerAdapter.getReactiveAdapterRegistry()).isSameAs(this.context.getBean("webFluxAdapterRegistry")); + assertThat(this.context.getBeansOfType(ReactiveAdapterRegistry.class)).containsOnlyKeys( + "webFluxAdapterRegistry", "testReactiveAdapterRegistry"); + } + + @Test + void requestMappingHandlerAdapterWithPrimaryUsesQualifiedConversionService() { + load(registerPrimaryBean("testConversionService", FormattingConversionService.class)); + RequestMappingHandlerAdapter mappingHandlerAdapter = this.context.getBean(RequestMappingHandlerAdapter.class); + assertThat(mappingHandlerAdapter.getWebBindingInitializer()).hasFieldOrPropertyWithValue("conversionService", + this.context.getBean("webFluxConversionService")); + assertThat(this.context.getBeansOfType(FormattingConversionService.class)).containsOnlyKeys( + "webFluxConversionService", "testConversionService"); + } + + @Test + void requestMappingHandlerAdapterWithPrimaryUsesQualifiedValidator() { + load(registerPrimaryBean("testValidator", Validator.class)); + RequestMappingHandlerAdapter mappingHandlerAdapter = this.context.getBean(RequestMappingHandlerAdapter.class); + assertThat(mappingHandlerAdapter.getWebBindingInitializer()).hasFieldOrPropertyWithValue("validator", + this.context.getBean("webFluxValidator")); + assertThat(this.context.getBeansOfType(Validator.class)).containsOnlyKeys( + "webFluxValidator", "testValidator"); + } + + @Test + void responseEntityResultHandlerUsesWebFluxInfrastructureByDefault() { + load(context -> { }); + ResponseEntityResultHandler responseEntityResultHandler = this.context.getBean(ResponseEntityResultHandler.class); + assertThat(responseEntityResultHandler.getAdapterRegistry()).isSameAs(this.context.getBean("webFluxAdapterRegistry")); + assertThat(responseEntityResultHandler.getContentTypeResolver()).isSameAs(this.context.getBean("webFluxContentTypeResolver")); + } + + @Test + void responseEntityResultHandlerWithPrimaryUsesQualifiedReactiveAdapterRegistry() { + load(registerPrimaryBean("testReactiveAdapterRegistry", ReactiveAdapterRegistry.class)); + ResponseEntityResultHandler responseEntityResultHandler = this.context.getBean(ResponseEntityResultHandler.class); + assertThat(responseEntityResultHandler.getAdapterRegistry()).isSameAs(this.context.getBean("webFluxAdapterRegistry")); + assertThat(this.context.getBeansOfType(ReactiveAdapterRegistry.class)).containsOnlyKeys( + "webFluxAdapterRegistry", "testReactiveAdapterRegistry"); + } + + @Test + void responseEntityResultHandlerWithPrimaryUsesQualifiedRequestedContentTypeResolver() { + load(registerPrimaryBean("testContentTypeResolver", RequestedContentTypeResolver.class)); + ResponseEntityResultHandler responseEntityResultHandler = this.context.getBean(ResponseEntityResultHandler.class); + assertThat(responseEntityResultHandler.getContentTypeResolver()).isSameAs(this.context.getBean("webFluxContentTypeResolver")); + assertThat(this.context.getBeansOfType(RequestedContentTypeResolver.class)).containsOnlyKeys( + "webFluxContentTypeResolver", "testContentTypeResolver"); + } + + @Test + void responseBodyResultHandlerUsesWebFluxInfrastructureByDefault() { + load(context -> { }); + ResponseBodyResultHandler responseBodyResultHandler = this.context.getBean(ResponseBodyResultHandler.class); + assertThat(responseBodyResultHandler.getAdapterRegistry()).isSameAs(this.context.getBean("webFluxAdapterRegistry")); + assertThat(responseBodyResultHandler.getContentTypeResolver()).isSameAs(this.context.getBean("webFluxContentTypeResolver")); + } + + @Test + void responseBodyResultHandlerWithPrimaryUsesQualifiedReactiveAdapterRegistry() { + load(registerPrimaryBean("testReactiveAdapterRegistry", ReactiveAdapterRegistry.class)); + ResponseBodyResultHandler responseBodyResultHandler = this.context.getBean(ResponseBodyResultHandler.class); + assertThat(responseBodyResultHandler.getAdapterRegistry()).isSameAs(this.context.getBean("webFluxAdapterRegistry")); + assertThat(this.context.getBeansOfType(ReactiveAdapterRegistry.class)).containsOnlyKeys( + "webFluxAdapterRegistry", "testReactiveAdapterRegistry"); + } + + @Test + void responseBodyResultHandlerWithPrimaryUsesQualifiedRequestedContentTypeResolver() { + load(registerPrimaryBean("testContentTypeResolver", RequestedContentTypeResolver.class)); + ResponseBodyResultHandler responseBodyResultHandler = this.context.getBean(ResponseBodyResultHandler.class); + assertThat(responseBodyResultHandler.getContentTypeResolver()).isSameAs(this.context.getBean("webFluxContentTypeResolver")); + assertThat(this.context.getBeansOfType(RequestedContentTypeResolver.class)).containsOnlyKeys( + "webFluxContentTypeResolver", "testContentTypeResolver"); + } + + @Test + void viewResolutionResultHandlerUsesWebFluxInfrastructureByDefault() { + load(context -> { }); + ViewResolutionResultHandler viewResolutionResultHandler = this.context.getBean(ViewResolutionResultHandler.class); + assertThat(viewResolutionResultHandler.getAdapterRegistry()).isSameAs(this.context.getBean("webFluxAdapterRegistry")); + assertThat(viewResolutionResultHandler.getContentTypeResolver()).isSameAs(this.context.getBean("webFluxContentTypeResolver")); + } + + @Test + void viewResolutionResultHandlerWithPrimaryUsesQualifiedReactiveAdapterRegistry() { + load(registerPrimaryBean("testReactiveAdapterRegistry", ReactiveAdapterRegistry.class)); + ViewResolutionResultHandler viewResolutionResultHandler = this.context.getBean(ViewResolutionResultHandler.class); + assertThat(viewResolutionResultHandler.getAdapterRegistry()).isSameAs(this.context.getBean("webFluxAdapterRegistry")); + assertThat(this.context.getBeansOfType(ReactiveAdapterRegistry.class)).containsOnlyKeys( + "webFluxAdapterRegistry", "testReactiveAdapterRegistry"); + } + + @Test + void viewResolutionResultHandlerWithPrimaryUsesQualifiedRequestedContentTypeResolver() { + load(registerPrimaryBean("testContentTypeResolver", RequestedContentTypeResolver.class)); + ViewResolutionResultHandler viewResolutionResultHandler = this.context.getBean(ViewResolutionResultHandler.class); + assertThat(viewResolutionResultHandler.getContentTypeResolver()).isSameAs(this.context.getBean("webFluxContentTypeResolver")); + assertThat(this.context.getBeansOfType(RequestedContentTypeResolver.class)).containsOnlyKeys( + "webFluxContentTypeResolver", "testContentTypeResolver"); + } + + private Consumer registerPrimaryBean(String beanName, Class type) { + return context -> context.registerBean(beanName, type, () -> mock(type), definition -> definition.setPrimary(true)); + } + + private void load(Consumer context) { + AnnotationConfigApplicationContext testContext = new AnnotationConfigApplicationContext(); + context.accept(testContext); + testContext.registerBean(DelegatingWebFluxConfiguration.class); + testContext.refresh(); + this.context = testContext; + } +} diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java index e59868bfba25..d8f93bf4e545 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupport.java @@ -29,6 +29,7 @@ import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.BeanInitializationException; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.context.annotation.Bean; @@ -275,13 +276,14 @@ public final ServletContext getServletContext() { */ @Bean public RequestMappingHandlerMapping requestMappingHandlerMapping( - ContentNegotiationManager mvcContentNegotiationManager, - FormattingConversionService mvcConversionService, ResourceUrlProvider mvcResourceUrlProvider) { + @Qualifier("mvcContentNegotiationManager") ContentNegotiationManager contentNegotiationManager, + @Qualifier("mvcConversionService") FormattingConversionService conversionService, + @Qualifier("mvcResourceUrlProvider") ResourceUrlProvider resourceUrlProvider) { RequestMappingHandlerMapping mapping = createRequestMappingHandlerMapping(); mapping.setOrder(0); - mapping.setInterceptors(getInterceptors(mvcConversionService, mvcResourceUrlProvider)); - mapping.setContentNegotiationManager(mvcContentNegotiationManager); + mapping.setInterceptors(getInterceptors(conversionService, resourceUrlProvider)); + mapping.setContentNegotiationManager(contentNegotiationManager); mapping.setCorsConfigurations(getCorsConfigurations()); PathMatchConfigurer configurer = getPathMatchConfigurer(); @@ -447,10 +449,11 @@ protected void configureContentNegotiation(ContentNegotiationConfigurer configur */ @Bean @Nullable - public HandlerMapping viewControllerHandlerMapping(PathMatcher mvcPathMatcher, - UrlPathHelper mvcUrlPathHelper, - FormattingConversionService mvcConversionService, - ResourceUrlProvider mvcResourceUrlProvider) { + public HandlerMapping viewControllerHandlerMapping( + @Qualifier("mvcPathMatcher") PathMatcher pathMatcher, + @Qualifier("mvcUrlPathHelper") UrlPathHelper urlPathHelper, + @Qualifier("mvcConversionService") FormattingConversionService conversionService, + @Qualifier("mvcResourceUrlProvider") ResourceUrlProvider resourceUrlProvider) { ViewControllerRegistry registry = new ViewControllerRegistry(this.applicationContext); addViewControllers(registry); @@ -458,9 +461,9 @@ public HandlerMapping viewControllerHandlerMapping(PathMatcher mvcPathMatcher, if (handlerMapping == null) { return null; } - handlerMapping.setPathMatcher(mvcPathMatcher); - handlerMapping.setUrlPathHelper(mvcUrlPathHelper); - handlerMapping.setInterceptors(getInterceptors(mvcConversionService, mvcResourceUrlProvider)); + handlerMapping.setPathMatcher(pathMatcher); + handlerMapping.setUrlPathHelper(urlPathHelper); + handlerMapping.setInterceptors(getInterceptors(conversionService, resourceUrlProvider)); handlerMapping.setCorsConfigurations(getCorsConfigurations()); return handlerMapping; } @@ -478,11 +481,12 @@ protected void addViewControllers(ViewControllerRegistry registry) { */ @Bean public BeanNameUrlHandlerMapping beanNameHandlerMapping( - FormattingConversionService mvcConversionService, ResourceUrlProvider mvcResourceUrlProvider) { + @Qualifier("mvcConversionService") FormattingConversionService conversionService, + @Qualifier("mvcResourceUrlProvider") ResourceUrlProvider resourceUrlProvider) { BeanNameUrlHandlerMapping mapping = new BeanNameUrlHandlerMapping(); mapping.setOrder(2); - mapping.setInterceptors(getInterceptors(mvcConversionService, mvcResourceUrlProvider)); + mapping.setInterceptors(getInterceptors(conversionService, resourceUrlProvider)); mapping.setCorsConfigurations(getCorsConfigurations()); return mapping; } @@ -500,11 +504,12 @@ public BeanNameUrlHandlerMapping beanNameHandlerMapping( */ @Bean public RouterFunctionMapping routerFunctionMapping( - FormattingConversionService mvcConversionService, ResourceUrlProvider mvcResourceUrlProvider) { + @Qualifier("mvcConversionService") FormattingConversionService conversionService, + @Qualifier("mvcResourceUrlProvider") ResourceUrlProvider resourceUrlProvider) { RouterFunctionMapping mapping = new RouterFunctionMapping(); mapping.setOrder(3); - mapping.setInterceptors(getInterceptors(mvcConversionService, mvcResourceUrlProvider)); + mapping.setInterceptors(getInterceptors(conversionService, resourceUrlProvider)); mapping.setCorsConfigurations(getCorsConfigurations()); mapping.setMessageConverters(getMessageConverters()); return mapping; @@ -517,24 +522,27 @@ public RouterFunctionMapping routerFunctionMapping( */ @Bean @Nullable - public HandlerMapping resourceHandlerMapping(UrlPathHelper mvcUrlPathHelper, - PathMatcher mvcPathMatcher, ContentNegotiationManager mvcContentNegotiationManager, - FormattingConversionService mvcConversionService, ResourceUrlProvider mvcResourceUrlProvider) { + public HandlerMapping resourceHandlerMapping( + @Qualifier("mvcUrlPathHelper") UrlPathHelper urlPathHelper, + @Qualifier("mvcPathMatcher") PathMatcher pathMatcher, + @Qualifier("mvcContentNegotiationManager") ContentNegotiationManager contentNegotiationManager, + @Qualifier("mvcConversionService") FormattingConversionService conversionService, + @Qualifier("mvcResourceUrlProvider") ResourceUrlProvider resourceUrlProvider) { Assert.state(this.applicationContext != null, "No ApplicationContext set"); Assert.state(this.servletContext != null, "No ServletContext set"); ResourceHandlerRegistry registry = new ResourceHandlerRegistry(this.applicationContext, - this.servletContext, mvcContentNegotiationManager, mvcUrlPathHelper); + this.servletContext, contentNegotiationManager, urlPathHelper); addResourceHandlers(registry); AbstractHandlerMapping handlerMapping = registry.getHandlerMapping(); if (handlerMapping == null) { return null; } - handlerMapping.setPathMatcher(mvcPathMatcher); - handlerMapping.setUrlPathHelper(mvcUrlPathHelper); - handlerMapping.setInterceptors(getInterceptors(mvcConversionService, mvcResourceUrlProvider)); + handlerMapping.setPathMatcher(pathMatcher); + handlerMapping.setUrlPathHelper(urlPathHelper); + handlerMapping.setInterceptors(getInterceptors(conversionService, resourceUrlProvider)); handlerMapping.setCorsConfigurations(getCorsConfigurations()); return handlerMapping; } @@ -597,13 +605,14 @@ protected void configureDefaultServletHandling(DefaultServletHandlerConfigurer c */ @Bean public RequestMappingHandlerAdapter requestMappingHandlerAdapter( - ContentNegotiationManager mvcContentNegotiationManager, - FormattingConversionService mvcConversionService, Validator mvcValidator) { + @Qualifier("mvcContentNegotiationManager") ContentNegotiationManager contentNegotiationManager, + @Qualifier("mvcConversionService") FormattingConversionService conversionService, + @Qualifier("mvcValidator") Validator validator) { RequestMappingHandlerAdapter adapter = createRequestMappingHandlerAdapter(); - adapter.setContentNegotiationManager(mvcContentNegotiationManager); + adapter.setContentNegotiationManager(contentNegotiationManager); adapter.setMessageConverters(getMessageConverters()); - adapter.setWebBindingInitializer(getConfigurableWebBindingInitializer(mvcConversionService, mvcValidator)); + adapter.setWebBindingInitializer(getConfigurableWebBindingInitializer(conversionService, validator)); adapter.setCustomArgumentResolvers(getArgumentResolvers()); adapter.setCustomReturnValueHandlers(getReturnValueHandlers()); @@ -898,10 +907,10 @@ else if (jsonbPresent) { */ @Bean public CompositeUriComponentsContributor mvcUriComponentsContributor( - FormattingConversionService mvcConversionService, - RequestMappingHandlerAdapter requestMappingHandlerAdapter) { + @Qualifier("mvcConversionService") FormattingConversionService conversionService, + @Qualifier("requestMappingHandlerAdapter") RequestMappingHandlerAdapter requestMappingHandlerAdapter) { return new CompositeUriComponentsContributor( - requestMappingHandlerAdapter.getArgumentResolvers(), mvcConversionService); + requestMappingHandlerAdapter.getArgumentResolvers(), conversionService); } /** @@ -932,11 +941,11 @@ public SimpleControllerHandlerAdapter simpleControllerHandlerAdapter() { */ @Bean public HandlerExceptionResolver handlerExceptionResolver( - ContentNegotiationManager mvcContentNegotiationManager) { + @Qualifier("mvcContentNegotiationManager") ContentNegotiationManager contentNegotiationManager) { List exceptionResolvers = new ArrayList<>(); configureHandlerExceptionResolvers(exceptionResolvers); if (exceptionResolvers.isEmpty()) { - addDefaultHandlerExceptionResolvers(exceptionResolvers, mvcContentNegotiationManager); + addDefaultHandlerExceptionResolvers(exceptionResolvers, contentNegotiationManager); } extendHandlerExceptionResolvers(exceptionResolvers); HandlerExceptionResolverComposite composite = new HandlerExceptionResolverComposite(); @@ -1026,9 +1035,10 @@ protected ExceptionHandlerExceptionResolver createExceptionHandlerExceptionResol * @since 4.1 */ @Bean - public ViewResolver mvcViewResolver(ContentNegotiationManager mvcContentNegotiationManager) { + public ViewResolver mvcViewResolver( + @Qualifier("mvcContentNegotiationManager") ContentNegotiationManager contentNegotiationManager) { ViewResolverRegistry registry = - new ViewResolverRegistry(mvcContentNegotiationManager, this.applicationContext); + new ViewResolverRegistry(contentNegotiationManager, this.applicationContext); configureViewResolvers(registry); if (registry.getViewResolvers().isEmpty() && this.applicationContext != null) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/DelegatingWebMvcConfigurationIntegrationTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/DelegatingWebMvcConfigurationIntegrationTests.java new file mode 100644 index 000000000000..f1ba8110e438 --- /dev/null +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/DelegatingWebMvcConfigurationIntegrationTests.java @@ -0,0 +1,201 @@ +/* + * Copyright 2002-2019 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.web.servlet.config.annotation; + +import java.util.function.Consumer; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.AnnotationConfigUtils; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.convert.ConversionService; +import org.springframework.mock.web.test.MockServletContext; +import org.springframework.util.PathMatcher; +import org.springframework.validation.Validator; +import org.springframework.web.accept.ContentNegotiationManager; +import org.springframework.web.context.support.GenericWebApplicationContext; +import org.springframework.web.servlet.handler.AbstractHandlerMapping; +import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter; +import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; +import org.springframework.web.util.UrlPathHelper; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +/** + * Integration tests for {@link DelegatingWebMvcConfiguration}. + * + * @author Stephane Nicoll + */ +public class DelegatingWebMvcConfigurationIntegrationTests { + + private ConfigurableApplicationContext context; + + @AfterEach + public void closeContext() { + if (this.context != null) { + this.context.close(); + } + } + + @Test + void requestMappingHandlerMappingUsesMvcInfrastructureByDefault() { + load(context -> { }); + RequestMappingHandlerMapping handlerMapping = this.context.getBean(RequestMappingHandlerMapping.class); + assertThat(handlerMapping.getContentNegotiationManager()).isSameAs(this.context.getBean("mvcContentNegotiationManager")); + } + + @Test + void requestMappingHandlerMappingWithPrimaryUsesQualifiedContentNegotiationManager() { + load(registerPrimaryBean("testContentNegotiationManager", ContentNegotiationManager.class)); + RequestMappingHandlerMapping handlerMapping = this.context.getBean(RequestMappingHandlerMapping.class); + assertThat(handlerMapping.getContentNegotiationManager()).isSameAs(this.context.getBean("mvcContentNegotiationManager")); + assertThat(this.context.getBeansOfType(ContentNegotiationManager.class)).containsOnlyKeys( + "mvcContentNegotiationManager", "testContentNegotiationManager"); + } + + @Test + void viewControllerHandlerMappingUsesMvcInfrastructureByDefault() { + load(context -> context.registerBean(ViewControllerConfiguration.class)); + AbstractHandlerMapping handlerMapping = this.context.getBean("viewControllerHandlerMapping", AbstractHandlerMapping.class); + assertThat(handlerMapping.getPathMatcher()).isSameAs(this.context.getBean("mvcPathMatcher")); + assertThat(handlerMapping.getUrlPathHelper()).isSameAs(this.context.getBean("mvcUrlPathHelper")); + } + + @Test + void viewControllerHandlerMappingWithPrimaryUsesQualifiedPathMatcher() { + load(registerPrimaryBean("testPathMatcher", PathMatcher.class) + .andThen(context -> context.registerBean(ViewControllerConfiguration.class))); + AbstractHandlerMapping handlerMapping = this.context.getBean("viewControllerHandlerMapping", AbstractHandlerMapping.class); + assertThat(handlerMapping.getPathMatcher()).isSameAs(this.context.getBean("mvcPathMatcher")); + assertThat(this.context.getBeansOfType(PathMatcher.class)).containsOnlyKeys( + "mvcPathMatcher", "testPathMatcher"); + } + + @Test + void viewControllerHandlerMappingWithPrimaryUsesQualifiedUrlPathHelper() { + load(registerPrimaryBean("testUrlPathHelper", UrlPathHelper.class) + .andThen(context -> context.registerBean(ViewControllerConfiguration.class))); + AbstractHandlerMapping handlerMapping = this.context.getBean("viewControllerHandlerMapping", AbstractHandlerMapping.class); + assertThat(handlerMapping.getUrlPathHelper()).isSameAs(this.context.getBean("mvcUrlPathHelper")); + assertThat(this.context.getBeansOfType(UrlPathHelper.class)).containsOnlyKeys( + "mvcUrlPathHelper", "testUrlPathHelper"); + } + + @Test + void resourceHandlerMappingUsesMvcInfrastructureByDefault() { + load(context -> context.registerBean(ResourceHandlerConfiguration.class)); + AbstractHandlerMapping handlerMapping = this.context.getBean("resourceHandlerMapping", AbstractHandlerMapping.class); + assertThat(handlerMapping.getPathMatcher()).isSameAs(this.context.getBean("mvcPathMatcher")); + assertThat(handlerMapping.getUrlPathHelper()).isSameAs(this.context.getBean("mvcUrlPathHelper")); + } + + @Test + void resourceHandlerMappingWithPrimaryUsesQualifiedPathMatcher() { + load(registerPrimaryBean("testPathMatcher", PathMatcher.class) + .andThen(context -> context.registerBean(ResourceHandlerConfiguration.class))); + AbstractHandlerMapping handlerMapping = this.context.getBean("resourceHandlerMapping", AbstractHandlerMapping.class); + assertThat(handlerMapping.getPathMatcher()).isSameAs(this.context.getBean("mvcPathMatcher")); + assertThat(this.context.getBeansOfType(PathMatcher.class)).containsOnlyKeys( + "mvcPathMatcher", "testPathMatcher"); + } + + @Test + void resourceHandlerMappingWithPrimaryUsesQualifiedUrlPathHelper() { + load(registerPrimaryBean("testUrlPathHelper", UrlPathHelper.class) + .andThen(context -> context.registerBean(ResourceHandlerConfiguration.class))); + AbstractHandlerMapping handlerMapping = this.context.getBean("resourceHandlerMapping", AbstractHandlerMapping.class); + assertThat(handlerMapping.getUrlPathHelper()).isSameAs(this.context.getBean("mvcUrlPathHelper")); + assertThat(this.context.getBeansOfType(UrlPathHelper.class)).containsOnlyKeys( + "mvcUrlPathHelper", "testUrlPathHelper"); + } + + @Test + void requestMappingHandlerAdapterUsesMvcInfrastructureByDefault() { + load(context -> { }); + RequestMappingHandlerAdapter mappingHandlerAdapter = this.context.getBean(RequestMappingHandlerAdapter.class); + assertThat(mappingHandlerAdapter).hasFieldOrPropertyWithValue( + "contentNegotiationManager", this.context.getBean("mvcContentNegotiationManager")); + assertThat(mappingHandlerAdapter.getWebBindingInitializer()).hasFieldOrPropertyWithValue( + "conversionService", this.context.getBean("mvcConversionService")); + assertThat(mappingHandlerAdapter.getWebBindingInitializer()).hasFieldOrPropertyWithValue( + "validator", this.context.getBean("mvcValidator")); + } + + @Test + void requestMappingHandlerAdapterWithPrimaryUsesQualifiedContentNegotiationManager() { + load(registerPrimaryBean("testContentNegotiationManager", ContentNegotiationManager.class)); + RequestMappingHandlerAdapter mappingHandlerAdapter = this.context.getBean(RequestMappingHandlerAdapter.class); + assertThat(mappingHandlerAdapter).hasFieldOrPropertyWithValue( + "contentNegotiationManager", this.context.getBean("mvcContentNegotiationManager")); + assertThat(this.context.getBeansOfType(ContentNegotiationManager.class)).containsOnlyKeys( + "mvcContentNegotiationManager", "testContentNegotiationManager"); + } + + @Test + void requestMappingHandlerAdapterWithPrimaryUsesQualifiedConversionService() { + load(registerPrimaryBean("testConversionService", ConversionService.class)); + RequestMappingHandlerAdapter mappingHandlerAdapter = this.context.getBean(RequestMappingHandlerAdapter.class); + assertThat(mappingHandlerAdapter.getWebBindingInitializer()).hasFieldOrPropertyWithValue( + "conversionService", this.context.getBean("mvcConversionService")); + assertThat(this.context.getBeansOfType(ConversionService.class)).containsOnlyKeys("mvcConversionService", "testConversionService"); + } + + @Test + void requestMappingHandlerAdapterWithPrimaryUsesQualifiedValidator() { + load(registerPrimaryBean("testValidator", Validator.class)); + RequestMappingHandlerAdapter mappingHandlerAdapter = this.context.getBean(RequestMappingHandlerAdapter.class); + assertThat(mappingHandlerAdapter.getWebBindingInitializer()).hasFieldOrPropertyWithValue( + "validator", this.context.getBean("mvcValidator")); + assertThat(this.context.getBeansOfType(Validator.class)).containsOnlyKeys("mvcValidator", "testValidator"); + } + + private Consumer registerPrimaryBean(String beanName, Class type) { + return context -> context.registerBean(beanName, type, () -> mock(type), definition -> definition.setPrimary(true)); + } + + private void load(Consumer context) { + GenericWebApplicationContext webContext = new GenericWebApplicationContext(); + AnnotationConfigUtils.registerAnnotationConfigProcessors(webContext); + webContext.setServletContext(new MockServletContext()); + + context.accept(webContext); + webContext.registerBean(DelegatingWebMvcConfiguration.class); + webContext.refresh(); + this.context = webContext; + } + + @Configuration + static class ViewControllerConfiguration implements WebMvcConfigurer { + + @Override + public void addViewControllers(ViewControllerRegistry registry) { + registry.addViewController("/test"); + } + } + + @Configuration + static class ResourceHandlerConfiguration implements WebMvcConfigurer { + + @Override + public void addResourceHandlers(ResourceHandlerRegistry registry) { + registry.addResourceHandler("/resources/**"); + } + } +} From cef4478b7b917033a8a01a6e633f6f59aab67e59 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 30 Oct 2019 15:59:44 +0100 Subject: [PATCH 2/3] Treat InvalidPathException like an IOException in MockServletContext Prior to this commit, if MockServletContext was configured with a FileSystemResourceLoader, invocations of the following methods on a Microsoft Windows operating system resulted in an InvalidPathException if the supplied path contained a colon (such as "C:\\temp"). This is inconsistent with the behavior on non-Windows operating systems. In addition, for comparable errors resulting in an IOException, those methods (except getRealPath()) return null instead of throwing the exception. - getResourcePaths() - getResource() - getResourceAsStream() - getRealPath() This commit makes handling of InvalidPathException and IOException consistent for these methods: both exceptions now result in null be returned by these methods. Closes gh-23717 --- .../mock/web/MockServletContext.java | 51 ++- .../mock/web/MockServletContextTests.java | 346 ++++++++++-------- .../mock/web/test/MockServletContext.java | 51 ++- 3 files changed, 265 insertions(+), 183 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java b/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java index 97fed0c5d149..7dab1c8c21b9 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockServletContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -21,6 +21,7 @@ import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; +import java.nio.file.InvalidPathException; import java.util.Collections; import java.util.Enumeration; import java.util.EventListener; @@ -294,8 +295,10 @@ public void addMimeType(String fileExtension, MediaType mimeType) { @Nullable public Set getResourcePaths(String path) { String actualPath = (path.endsWith("/") ? path : path + "/"); - Resource resource = this.resourceLoader.getResource(getResourceLocation(actualPath)); + String resourceLocation = getResourceLocation(actualPath); + Resource resource = null; try { + resource = this.resourceLoader.getResource(resourceLocation); File file = resource.getFile(); String[] fileList = file.list(); if (ObjectUtils.isEmpty(fileList)) { @@ -311,9 +314,10 @@ public Set getResourcePaths(String path) { } return resourcePaths; } - catch (IOException ex) { + catch (InvalidPathException | IOException ex ) { if (logger.isWarnEnabled()) { - logger.warn("Could not get resource paths for " + resource, ex); + logger.warn("Could not get resource paths for " + + (resource != null ? resource : resourceLocation), ex); } return null; } @@ -322,19 +326,22 @@ public Set getResourcePaths(String path) { @Override @Nullable public URL getResource(String path) throws MalformedURLException { - Resource resource = this.resourceLoader.getResource(getResourceLocation(path)); - if (!resource.exists()) { - return null; - } + String resourceLocation = getResourceLocation(path); + Resource resource = null; try { + resource = this.resourceLoader.getResource(resourceLocation); + if (!resource.exists()) { + return null; + } return resource.getURL(); } catch (MalformedURLException ex) { throw ex; } - catch (IOException ex) { + catch (InvalidPathException | IOException ex) { if (logger.isWarnEnabled()) { - logger.warn("Could not get URL for " + resource, ex); + logger.warn("Could not get URL for resource " + + (resource != null ? resource : resourceLocation), ex); } return null; } @@ -343,16 +350,19 @@ public URL getResource(String path) throws MalformedURLException { @Override @Nullable public InputStream getResourceAsStream(String path) { - Resource resource = this.resourceLoader.getResource(getResourceLocation(path)); - if (!resource.exists()) { - return null; - } + String resourceLocation = getResourceLocation(path); + Resource resource = null; try { + resource = this.resourceLoader.getResource(resourceLocation); + if (!resource.exists()) { + return null; + } return resource.getInputStream(); } - catch (IOException ex) { + catch (InvalidPathException | IOException ex) { if (logger.isWarnEnabled()) { - logger.warn("Could not open InputStream for " + resource, ex); + logger.warn("Could not open InputStream for resource " + + (resource != null ? resource : resourceLocation), ex); } return null; } @@ -459,13 +469,16 @@ public void log(String message, Throwable ex) { @Override @Nullable public String getRealPath(String path) { - Resource resource = this.resourceLoader.getResource(getResourceLocation(path)); + String resourceLocation = getResourceLocation(path); + Resource resource = null; try { + resource = this.resourceLoader.getResource(resourceLocation); return resource.getFile().getAbsolutePath(); } - catch (IOException ex) { + catch (InvalidPathException | IOException ex) { if (logger.isWarnEnabled()) { - logger.warn("Could not determine real path of resource " + resource, ex); + logger.warn("Could not determine real path of resource " + + (resource != null ? resource : resourceLocation), ex); } return null; } diff --git a/spring-test/src/test/java/org/springframework/mock/web/MockServletContextTests.java b/spring-test/src/test/java/org/springframework/mock/web/MockServletContextTests.java index 48c8e32ee384..fd26ff1e25ce 100644 --- a/spring-test/src/test/java/org/springframework/mock/web/MockServletContextTests.java +++ b/spring-test/src/test/java/org/springframework/mock/web/MockServletContextTests.java @@ -16,6 +16,8 @@ package org.springframework.mock.web; +import java.io.InputStream; +import java.net.URL; import java.util.Map; import java.util.Set; @@ -23,171 +25,225 @@ import javax.servlet.RequestDispatcher; import javax.servlet.ServletRegistration; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.OS; +import org.springframework.core.io.FileSystemResourceLoader; import org.springframework.http.MediaType; import static org.assertj.core.api.Assertions.assertThat; /** + * Unit tests for {@link MockServletContext}. + * * @author Juergen Hoeller * @author Chris Beams * @author Sam Brannen * @since 19.02.2006 */ +@DisplayName("MockServletContext unit tests") class MockServletContextTests { - private final MockServletContext sc = new MockServletContext("org/springframework/mock"); - - - @Test - void listFiles() { - Set paths = sc.getResourcePaths("/web"); - assertThat(paths).isNotNull(); - assertThat(paths.contains("/web/MockServletContextTests.class")).isTrue(); - } - - @Test - void listSubdirectories() { - Set paths = sc.getResourcePaths("/"); - assertThat(paths).isNotNull(); - assertThat(paths.contains("/web/")).isTrue(); - } - - @Test - void listNonDirectory() { - Set paths = sc.getResourcePaths("/web/MockServletContextTests.class"); - assertThat(paths).isNull(); - } - - @Test - void listInvalidPath() { - Set paths = sc.getResourcePaths("/web/invalid"); - assertThat(paths).isNull(); - } - - @Test - void registerContextAndGetContext() { - MockServletContext sc2 = new MockServletContext(); - sc.setContextPath("/"); - sc.registerContext("/second", sc2); - assertThat(sc.getContext("/")).isSameAs(sc); - assertThat(sc.getContext("/second")).isSameAs(sc2); - } - - @Test - void getMimeType() { - assertThat(sc.getMimeType("test.html")).isEqualTo("text/html"); - assertThat(sc.getMimeType("test.gif")).isEqualTo("image/gif"); - assertThat(sc.getMimeType("test.foobar")).isNull(); - } - - /** - * Introduced to dispel claims in a thread on Stack Overflow: - * Testing Spring managed servlet - */ - @Test - void getMimeTypeWithCustomConfiguredType() { - sc.addMimeType("enigma", new MediaType("text", "enigma")); - assertThat(sc.getMimeType("filename.enigma")).isEqualTo("text/enigma"); - } - - @Test - void servletVersion() { - assertThat(sc.getMajorVersion()).isEqualTo(3); - assertThat(sc.getMinorVersion()).isEqualTo(1); - assertThat(sc.getEffectiveMajorVersion()).isEqualTo(3); - assertThat(sc.getEffectiveMinorVersion()).isEqualTo(1); - - sc.setMajorVersion(4); - sc.setMinorVersion(0); - sc.setEffectiveMajorVersion(4); - sc.setEffectiveMinorVersion(0); - assertThat(sc.getMajorVersion()).isEqualTo(4); - assertThat(sc.getMinorVersion()).isEqualTo(0); - assertThat(sc.getEffectiveMajorVersion()).isEqualTo(4); - assertThat(sc.getEffectiveMinorVersion()).isEqualTo(0); - } - - @Test - void registerAndUnregisterNamedDispatcher() throws Exception { - final String name = "test-servlet"; - final String url = "/test"; - assertThat(sc.getNamedDispatcher(name)).isNull(); - - sc.registerNamedDispatcher(name, new MockRequestDispatcher(url)); - RequestDispatcher namedDispatcher = sc.getNamedDispatcher(name); - assertThat(namedDispatcher).isNotNull(); - MockHttpServletResponse response = new MockHttpServletResponse(); - namedDispatcher.forward(new MockHttpServletRequest(sc), response); - assertThat(response.getForwardedUrl()).isEqualTo(url); - - sc.unregisterNamedDispatcher(name); - assertThat(sc.getNamedDispatcher(name)).isNull(); - } - - @Test - void getNamedDispatcherForDefaultServlet() throws Exception { - final String name = "default"; - RequestDispatcher namedDispatcher = sc.getNamedDispatcher(name); - assertThat(namedDispatcher).isNotNull(); - - MockHttpServletResponse response = new MockHttpServletResponse(); - namedDispatcher.forward(new MockHttpServletRequest(sc), response); - assertThat(response.getForwardedUrl()).isEqualTo(name); - } - - @Test - void setDefaultServletName() throws Exception { - final String originalDefault = "default"; - final String newDefault = "test"; - assertThat(sc.getNamedDispatcher(originalDefault)).isNotNull(); - - sc.setDefaultServletName(newDefault); - assertThat(sc.getDefaultServletName()).isEqualTo(newDefault); - assertThat(sc.getNamedDispatcher(originalDefault)).isNull(); - - RequestDispatcher namedDispatcher = sc.getNamedDispatcher(newDefault); - assertThat(namedDispatcher).isNotNull(); - MockHttpServletResponse response = new MockHttpServletResponse(); - namedDispatcher.forward(new MockHttpServletRequest(sc), response); - assertThat(response.getForwardedUrl()).isEqualTo(newDefault); - } - - /** - * @since 4.1.2 - */ - @Test - void getServletRegistration() { - assertThat(sc.getServletRegistration("servlet")).isNull(); - } + @Nested + @DisplayName("with DefaultResourceLoader") + class MockServletContextWithDefaultResourceLoaderTests { + + private final MockServletContext servletContext = new MockServletContext("org/springframework/mock"); + + @Test + void getResourcePaths() { + Set paths = servletContext.getResourcePaths("/web"); + assertThat(paths).isNotNull(); + assertThat(paths.contains("/web/MockServletContextTests.class")).isTrue(); + } + + @Test + void getResourcePathsWithSubdirectories() { + Set paths = servletContext.getResourcePaths("/"); + assertThat(paths).isNotNull(); + assertThat(paths.contains("/web/")).isTrue(); + } + + @Test + void getResourcePathsWithNonDirectory() { + Set paths = servletContext.getResourcePaths("/web/MockServletContextTests.class"); + assertThat(paths).isNull(); + } + + @Test + void getResourcePathsWithInvalidPath() { + Set paths = servletContext.getResourcePaths("/web/invalid"); + assertThat(paths).isNull(); + } + + @Test + void registerContextAndGetContext() { + MockServletContext sc2 = new MockServletContext(); + servletContext.setContextPath("/"); + servletContext.registerContext("/second", sc2); + assertThat(servletContext.getContext("/")).isSameAs(servletContext); + assertThat(servletContext.getContext("/second")).isSameAs(sc2); + } + + @Test + void getMimeType() { + assertThat(servletContext.getMimeType("test.html")).isEqualTo("text/html"); + assertThat(servletContext.getMimeType("test.gif")).isEqualTo("image/gif"); + assertThat(servletContext.getMimeType("test.foobar")).isNull(); + } + + /** + * Introduced to dispel claims in a thread on Stack Overflow: + * Testing Spring managed servlet + */ + @Test + void getMimeTypeWithCustomConfiguredType() { + servletContext.addMimeType("enigma", new MediaType("text", "enigma")); + assertThat(servletContext.getMimeType("filename.enigma")).isEqualTo("text/enigma"); + } + + @Test + void servletVersion() { + assertThat(servletContext.getMajorVersion()).isEqualTo(3); + assertThat(servletContext.getMinorVersion()).isEqualTo(1); + assertThat(servletContext.getEffectiveMajorVersion()).isEqualTo(3); + assertThat(servletContext.getEffectiveMinorVersion()).isEqualTo(1); + + servletContext.setMajorVersion(4); + servletContext.setMinorVersion(0); + servletContext.setEffectiveMajorVersion(4); + servletContext.setEffectiveMinorVersion(0); + assertThat(servletContext.getMajorVersion()).isEqualTo(4); + assertThat(servletContext.getMinorVersion()).isEqualTo(0); + assertThat(servletContext.getEffectiveMajorVersion()).isEqualTo(4); + assertThat(servletContext.getEffectiveMinorVersion()).isEqualTo(0); + } + + @Test + void registerAndUnregisterNamedDispatcher() throws Exception { + final String name = "test-servlet"; + final String url = "/test"; + assertThat(servletContext.getNamedDispatcher(name)).isNull(); + + servletContext.registerNamedDispatcher(name, new MockRequestDispatcher(url)); + RequestDispatcher namedDispatcher = servletContext.getNamedDispatcher(name); + assertThat(namedDispatcher).isNotNull(); + MockHttpServletResponse response = new MockHttpServletResponse(); + namedDispatcher.forward(new MockHttpServletRequest(servletContext), response); + assertThat(response.getForwardedUrl()).isEqualTo(url); + + servletContext.unregisterNamedDispatcher(name); + assertThat(servletContext.getNamedDispatcher(name)).isNull(); + } + + @Test + void getNamedDispatcherForDefaultServlet() throws Exception { + final String name = "default"; + RequestDispatcher namedDispatcher = servletContext.getNamedDispatcher(name); + assertThat(namedDispatcher).isNotNull(); + + MockHttpServletResponse response = new MockHttpServletResponse(); + namedDispatcher.forward(new MockHttpServletRequest(servletContext), response); + assertThat(response.getForwardedUrl()).isEqualTo(name); + } + + @Test + void setDefaultServletName() throws Exception { + final String originalDefault = "default"; + final String newDefault = "test"; + assertThat(servletContext.getNamedDispatcher(originalDefault)).isNotNull(); + + servletContext.setDefaultServletName(newDefault); + assertThat(servletContext.getDefaultServletName()).isEqualTo(newDefault); + assertThat(servletContext.getNamedDispatcher(originalDefault)).isNull(); + + RequestDispatcher namedDispatcher = servletContext.getNamedDispatcher(newDefault); + assertThat(namedDispatcher).isNotNull(); + MockHttpServletResponse response = new MockHttpServletResponse(); + namedDispatcher.forward(new MockHttpServletRequest(servletContext), response); + assertThat(response.getForwardedUrl()).isEqualTo(newDefault); + } + + /** + * @since 4.1.2 + */ + @Test + void getServletRegistration() { + assertThat(servletContext.getServletRegistration("servlet")).isNull(); + } + + /** + * @since 4.1.2 + */ + @Test + void getServletRegistrations() { + Map servletRegistrations = servletContext.getServletRegistrations(); + assertThat(servletRegistrations).isNotNull(); + assertThat(servletRegistrations.size()).isEqualTo(0); + } + + /** + * @since 4.1.2 + */ + @Test + void getFilterRegistration() { + assertThat(servletContext.getFilterRegistration("filter")).isNull(); + } + + /** + * @since 4.1.2 + */ + @Test + void getFilterRegistrations() { + Map filterRegistrations = servletContext.getFilterRegistrations(); + assertThat(filterRegistrations).isNotNull(); + assertThat(filterRegistrations.size()).isEqualTo(0); + } - /** - * @since 4.1.2 - */ - @Test - void getServletRegistrations() { - Map servletRegistrations = sc.getServletRegistrations(); - assertThat(servletRegistrations).isNotNull(); - assertThat(servletRegistrations.size()).isEqualTo(0); } /** - * @since 4.1.2 + * @since 5.1.11 */ - @Test - void getFilterRegistration() { - assertThat(sc.getFilterRegistration("filter")).isNull(); - } + @Nested + @DisplayName("with FileSystemResourceLoader") + class MockServletContextWithFileSystemResourceLoaderTests { + + private final MockServletContext servletContext = + new MockServletContext( "org/springframework/mock", new FileSystemResourceLoader()); + + @Test + void getResourcePathsWithRelativePathToWindowsCDrive() { + Set paths = servletContext.getResourcePaths("C:\\temp"); + assertThat(paths).isNull(); + } + + @Test + void getResourceWithRelativePathToWindowsCDrive() throws Exception { + URL resource = servletContext.getResource("C:\\temp"); + assertThat(resource).isNull(); + } + + @Test + void getResourceAsStreamWithRelativePathToWindowsCDrive() { + InputStream inputStream = servletContext.getResourceAsStream("C:\\temp"); + assertThat(inputStream).isNull(); + } + + @Test + void getRealPathWithRelativePathToWindowsCDrive() { + String realPath = servletContext.getRealPath("C:\\temp"); + + if (OS.WINDOWS.isCurrentOs()) { + assertThat(realPath).isNull(); + } + else { + assertThat(realPath).isNotNull(); + } + } - /** - * @since 4.1.2 - */ - @Test - void getFilterRegistrations() { - Map filterRegistrations = sc.getFilterRegistrations(); - assertThat(filterRegistrations).isNotNull(); - assertThat(filterRegistrations.size()).isEqualTo(0); } } diff --git a/spring-web/src/test/java/org/springframework/mock/web/test/MockServletContext.java b/spring-web/src/test/java/org/springframework/mock/web/test/MockServletContext.java index 3b78ed50ee6e..32fa355b8662 100644 --- a/spring-web/src/test/java/org/springframework/mock/web/test/MockServletContext.java +++ b/spring-web/src/test/java/org/springframework/mock/web/test/MockServletContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -21,6 +21,7 @@ import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; +import java.nio.file.InvalidPathException; import java.util.Collections; import java.util.Enumeration; import java.util.EventListener; @@ -294,8 +295,10 @@ public void addMimeType(String fileExtension, MediaType mimeType) { @Nullable public Set getResourcePaths(String path) { String actualPath = (path.endsWith("/") ? path : path + "/"); - Resource resource = this.resourceLoader.getResource(getResourceLocation(actualPath)); + String resourceLocation = getResourceLocation(actualPath); + Resource resource = null; try { + resource = this.resourceLoader.getResource(resourceLocation); File file = resource.getFile(); String[] fileList = file.list(); if (ObjectUtils.isEmpty(fileList)) { @@ -311,9 +314,10 @@ public Set getResourcePaths(String path) { } return resourcePaths; } - catch (IOException ex) { + catch (InvalidPathException | IOException ex ) { if (logger.isWarnEnabled()) { - logger.warn("Could not get resource paths for " + resource, ex); + logger.warn("Could not get resource paths for " + + (resource != null ? resource : resourceLocation), ex); } return null; } @@ -322,19 +326,22 @@ public Set getResourcePaths(String path) { @Override @Nullable public URL getResource(String path) throws MalformedURLException { - Resource resource = this.resourceLoader.getResource(getResourceLocation(path)); - if (!resource.exists()) { - return null; - } + String resourceLocation = getResourceLocation(path); + Resource resource = null; try { + resource = this.resourceLoader.getResource(resourceLocation); + if (!resource.exists()) { + return null; + } return resource.getURL(); } catch (MalformedURLException ex) { throw ex; } - catch (IOException ex) { + catch (InvalidPathException | IOException ex) { if (logger.isWarnEnabled()) { - logger.warn("Could not get URL for " + resource, ex); + logger.warn("Could not get URL for resource " + + (resource != null ? resource : resourceLocation), ex); } return null; } @@ -343,16 +350,19 @@ public URL getResource(String path) throws MalformedURLException { @Override @Nullable public InputStream getResourceAsStream(String path) { - Resource resource = this.resourceLoader.getResource(getResourceLocation(path)); - if (!resource.exists()) { - return null; - } + String resourceLocation = getResourceLocation(path); + Resource resource = null; try { + resource = this.resourceLoader.getResource(resourceLocation); + if (!resource.exists()) { + return null; + } return resource.getInputStream(); } - catch (IOException ex) { + catch (InvalidPathException | IOException ex) { if (logger.isWarnEnabled()) { - logger.warn("Could not open InputStream for " + resource, ex); + logger.warn("Could not open InputStream for resource " + + (resource != null ? resource : resourceLocation), ex); } return null; } @@ -459,13 +469,16 @@ public void log(String message, Throwable ex) { @Override @Nullable public String getRealPath(String path) { - Resource resource = this.resourceLoader.getResource(getResourceLocation(path)); + String resourceLocation = getResourceLocation(path); + Resource resource = null; try { + resource = this.resourceLoader.getResource(resourceLocation); return resource.getFile().getAbsolutePath(); } - catch (IOException ex) { + catch (InvalidPathException | IOException ex) { if (logger.isWarnEnabled()) { - logger.warn("Could not determine real path of resource " + resource, ex); + logger.warn("Could not determine real path of resource " + + (resource != null ? resource : resourceLocation), ex); } return null; } From 43a86565cac09de05e50678721cbc83a61a43a6e Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 30 Oct 2019 16:23:37 +0100 Subject: [PATCH 3/3] Consider target transaction manager for reactive transaction decision Closes gh-23832 --- .../interceptor/TransactionAspectSupport.java | 173 ++++++------------ .../TransactionInterceptorTests.java | 26 +-- 2 files changed, 74 insertions(+), 125 deletions(-) diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java index c8150bef556a..c2d9bf5599ab 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java +++ b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java @@ -173,7 +173,11 @@ public static TransactionStatus currentTransactionStatus() throws NoTransactionE @Nullable private BeanFactory beanFactory; - private final ConcurrentMap transactionManagerCache = new ConcurrentReferenceHashMap<>(4); + private final ConcurrentMap transactionManagerCache = + new ConcurrentReferenceHashMap<>(4); + + private final ConcurrentMap transactionSupportCache = + new ConcurrentReferenceHashMap<>(1024); protected TransactionAspectSupport() { @@ -301,7 +305,7 @@ public void afterPropertiesSet() { if (getTransactionManager() == null && this.beanFactory == null) { throw new IllegalStateException( "Set the 'transactionManager' property or make sure to run within a BeanFactory " + - "containing a PlatformTransactionManager bean!"); + "containing a TransactionManager bean!"); } if (getTransactionAttributeSource() == null) { throw new IllegalStateException( @@ -325,26 +329,35 @@ public void afterPropertiesSet() { protected Object invokeWithinTransaction(Method method, @Nullable Class targetClass, final InvocationCallback invocation) throws Throwable { - if (this.reactiveAdapterRegistry != null) { - if (KotlinDetector.isKotlinType(method.getDeclaringClass()) && KotlinDelegate.isSuspend(method)) { - throw new TransactionUsageException("Unsupported annotated transaction on suspending function detected: " - + method + ". Use TransactionalOperator.transactional extensions instead."); - } - ReactiveAdapter adapter = this.reactiveAdapterRegistry.getAdapter(method.getReturnType()); - if (adapter != null) { - return new ReactiveTransactionSupport(adapter).invokeWithinTransaction(method, targetClass, invocation); - } - } - // If the transaction attribute is null, the method is non-transactional. TransactionAttributeSource tas = getTransactionAttributeSource(); final TransactionAttribute txAttr = (tas != null ? tas.getTransactionAttribute(method, targetClass) : null); - final PlatformTransactionManager tm = determineTransactionManager(txAttr); + final TransactionManager tm = determineTransactionManager(txAttr); + + if (this.reactiveAdapterRegistry != null && tm instanceof ReactiveTransactionManager) { + ReactiveTransactionSupport txSupport = this.transactionSupportCache.computeIfAbsent(method, key -> { + if (KotlinDetector.isKotlinType(method.getDeclaringClass()) && KotlinDelegate.isSuspend(method)) { + throw new TransactionUsageException( + "Unsupported annotated transaction on suspending function detected: " + method + + ". Use TransactionalOperator.transactional extensions instead."); + } + ReactiveAdapter adapter = this.reactiveAdapterRegistry.getAdapter(method.getReturnType()); + if (adapter == null) { + throw new IllegalStateException("Cannot apply reactive transaction to non-reactive return type: " + + method.getReturnType()); + } + return new ReactiveTransactionSupport(adapter); + }); + return txSupport.invokeWithinTransaction( + method, targetClass, invocation, txAttr, (ReactiveTransactionManager) tm); + } + + PlatformTransactionManager ptm = asPlatformTransactionManager(tm); final String joinpointIdentification = methodIdentification(method, targetClass, txAttr); - if (txAttr == null || !(tm instanceof CallbackPreferringPlatformTransactionManager)) { + if (txAttr == null || !(ptm instanceof CallbackPreferringPlatformTransactionManager)) { // Standard transaction demarcation with getTransaction and commit/rollback calls. - TransactionInfo txInfo = createTransactionIfNecessary(tm, txAttr, joinpointIdentification); + TransactionInfo txInfo = createTransactionIfNecessary(ptm, txAttr, joinpointIdentification); Object retVal; try { @@ -378,8 +391,8 @@ protected Object invokeWithinTransaction(Method method, @Nullable Class targe // It's a CallbackPreferringPlatformTransactionManager: pass a TransactionCallback in. try { - Object result = ((CallbackPreferringPlatformTransactionManager) tm).execute(txAttr, status -> { - TransactionInfo txInfo = prepareTransactionInfo(tm, txAttr, joinpointIdentification, status); + Object result = ((CallbackPreferringPlatformTransactionManager) ptm).execute(txAttr, status -> { + TransactionInfo txInfo = prepareTransactionInfo(ptm, txAttr, joinpointIdentification, status); try { Object retVal = invocation.proceedWithInvocation(); if (vavrPresent && VavrDelegate.isVavrTry(retVal)) { @@ -446,10 +459,10 @@ protected void clearTransactionManagerCache() { * Determine the specific transaction manager to use for the given transaction. */ @Nullable - protected PlatformTransactionManager determineTransactionManager(@Nullable TransactionAttribute txAttr) { + protected TransactionManager determineTransactionManager(@Nullable TransactionAttribute txAttr) { // Do not attempt to lookup tx manager if no tx attributes are set if (txAttr == null || this.beanFactory == null) { - return asPlatformTransactionManager(getTransactionManager()); + return getTransactionManager(); } String qualifier = txAttr.getQualifier(); @@ -460,12 +473,11 @@ else if (StringUtils.hasText(this.transactionManagerBeanName)) { return determineQualifiedTransactionManager(this.beanFactory, this.transactionManagerBeanName); } else { - PlatformTransactionManager defaultTransactionManager = asPlatformTransactionManager(getTransactionManager()); + TransactionManager defaultTransactionManager = getTransactionManager(); if (defaultTransactionManager == null) { - defaultTransactionManager = asPlatformTransactionManager( - this.transactionManagerCache.get(DEFAULT_TRANSACTION_MANAGER_KEY)); + defaultTransactionManager = this.transactionManagerCache.get(DEFAULT_TRANSACTION_MANAGER_KEY); if (defaultTransactionManager == null) { - defaultTransactionManager = this.beanFactory.getBean(PlatformTransactionManager.class); + defaultTransactionManager = this.beanFactory.getBean(TransactionManager.class); this.transactionManagerCache.putIfAbsent( DEFAULT_TRANSACTION_MANAGER_KEY, defaultTransactionManager); } @@ -474,11 +486,11 @@ else if (StringUtils.hasText(this.transactionManagerBeanName)) { } } - private PlatformTransactionManager determineQualifiedTransactionManager(BeanFactory beanFactory, String qualifier) { - PlatformTransactionManager txManager = asPlatformTransactionManager(this.transactionManagerCache.get(qualifier)); + private TransactionManager determineQualifiedTransactionManager(BeanFactory beanFactory, String qualifier) { + TransactionManager txManager = this.transactionManagerCache.get(qualifier); if (txManager == null) { txManager = BeanFactoryAnnotationUtils.qualifiedBeanOfType( - beanFactory, PlatformTransactionManager.class, qualifier); + beanFactory, TransactionManager.class, qualifier); this.transactionManagerCache.putIfAbsent(qualifier, txManager); } return txManager; @@ -841,33 +853,30 @@ public ReactiveTransactionSupport(ReactiveAdapter adapter) { this.adapter = adapter; } - public Object invokeWithinTransaction(Method method, @Nullable Class targetClass, InvocationCallback invocation) { - // If the transaction attribute is null, the method is non-transactional. - TransactionAttributeSource tas = getTransactionAttributeSource(); - TransactionAttribute txAttr = (tas != null ? tas.getTransactionAttribute(method, targetClass) : null); - ReactiveTransactionManager tm = determineTransactionManager(txAttr); + public Object invokeWithinTransaction(Method method, @Nullable Class targetClass, + InvocationCallback invocation, @Nullable TransactionAttribute txAttr, ReactiveTransactionManager rtm) { + String joinpointIdentification = methodIdentification(method, targetClass, txAttr); // Optimize for Mono if (Mono.class.isAssignableFrom(method.getReturnType())) { return TransactionContextManager.currentContext().flatMap(context -> - createTransactionIfNecessary(tm, txAttr, joinpointIdentification).flatMap(it -> { + createTransactionIfNecessary(rtm, txAttr, joinpointIdentification).flatMap(it -> { try { // Need re-wrapping until we get hold of the exception through usingWhen. - return Mono - .usingWhen( - Mono.just(it), - txInfo -> { - try { - return (Mono) invocation.proceedWithInvocation(); - } - catch (Throwable ex) { - return Mono.error(ex); - } - }, - this::commitTransactionAfterReturning, - (txInfo, err) -> Mono.empty(), - this::commitTransactionAfterReturning) + return Mono.usingWhen( + Mono.just(it), + txInfo -> { + try { + return (Mono) invocation.proceedWithInvocation(); + } + catch (Throwable ex) { + return Mono.error(ex); + } + }, + this::commitTransactionAfterReturning, + (txInfo, err) -> Mono.empty(), + this::commitTransactionAfterReturning) .onErrorResume(ex -> completeTransactionAfterThrowing(it, ex).then(Mono.error(ex))); } @@ -881,7 +890,7 @@ public Object invokeWithinTransaction(Method method, @Nullable Class targetCl // Any other reactive type, typically a Flux return this.adapter.fromPublisher(TransactionContextManager.currentContext().flatMapMany(context -> - createTransactionIfNecessary(tm, txAttr, joinpointIdentification).flatMapMany(it -> { + createTransactionIfNecessary(rtm, txAttr, joinpointIdentification).flatMapMany(it -> { try { // Need re-wrapping until we get hold of the exception through usingWhen. return Flux @@ -909,58 +918,8 @@ public Object invokeWithinTransaction(Method method, @Nullable Class targetCl .subscriberContext(TransactionContextManager.getOrCreateContextHolder())); } - @Nullable - private ReactiveTransactionManager determineTransactionManager(@Nullable TransactionAttribute txAttr) { - // Do not attempt to lookup tx manager if no tx attributes are set - if (txAttr == null || beanFactory == null) { - return asReactiveTransactionManager(getTransactionManager()); - } - - String qualifier = txAttr.getQualifier(); - if (StringUtils.hasText(qualifier)) { - return determineQualifiedTransactionManager(beanFactory, qualifier); - } - else if (StringUtils.hasText(transactionManagerBeanName)) { - return determineQualifiedTransactionManager(beanFactory, transactionManagerBeanName); - } - else { - ReactiveTransactionManager defaultTransactionManager = asReactiveTransactionManager(getTransactionManager()); - if (defaultTransactionManager == null) { - defaultTransactionManager = asReactiveTransactionManager( - transactionManagerCache.get(DEFAULT_TRANSACTION_MANAGER_KEY)); - if (defaultTransactionManager == null) { - defaultTransactionManager = beanFactory.getBean(ReactiveTransactionManager.class); - transactionManagerCache.putIfAbsent( - DEFAULT_TRANSACTION_MANAGER_KEY, defaultTransactionManager); - } - } - return defaultTransactionManager; - } - } - - private ReactiveTransactionManager determineQualifiedTransactionManager(BeanFactory beanFactory, String qualifier) { - ReactiveTransactionManager txManager = asReactiveTransactionManager(transactionManagerCache.get(qualifier)); - if (txManager == null) { - txManager = BeanFactoryAnnotationUtils.qualifiedBeanOfType( - beanFactory, ReactiveTransactionManager.class, qualifier); - transactionManagerCache.putIfAbsent(qualifier, txManager); - } - return txManager; - } - - @Nullable - private ReactiveTransactionManager asReactiveTransactionManager(@Nullable Object transactionManager) { - if (transactionManager == null || transactionManager instanceof ReactiveTransactionManager) { - return (ReactiveTransactionManager) transactionManager; - } - else { - throw new IllegalStateException( - "Specified transaction manager is not a ReactiveTransactionManager: " + transactionManager); - } - } - @SuppressWarnings("serial") - private Mono createTransactionIfNecessary(@Nullable ReactiveTransactionManager tm, + private Mono createTransactionIfNecessary(ReactiveTransactionManager tm, @Nullable TransactionAttribute txAttr, final String joinpointIdentification) { // If no name specified, apply method identification as transaction name. @@ -972,21 +931,9 @@ public String getName() { } }; } - TransactionAttribute attrToUse = txAttr; - - Mono tx = Mono.empty(); - if (txAttr != null) { - if (tm != null) { - tx = tm.getReactiveTransaction(txAttr); - } - else { - if (logger.isDebugEnabled()) { - logger.debug("Skipping transactional joinpoint [" + joinpointIdentification + - "] because no transaction manager has been configured"); - } - } - } + final TransactionAttribute attrToUse = txAttr; + Mono tx = (attrToUse != null ? tm.getReactiveTransaction(attrToUse) : Mono.empty()); return tx.map(it -> prepareTransactionInfo(tm, attrToUse, joinpointIdentification, it)).switchIfEmpty( Mono.defer(() -> Mono.just(prepareTransactionInfo(tm, attrToUse, joinpointIdentification, null)))); } diff --git a/spring-tx/src/test/java/org/springframework/transaction/interceptor/TransactionInterceptorTests.java b/spring-tx/src/test/java/org/springframework/transaction/interceptor/TransactionInterceptorTests.java index 082bc4734277..bc138e813cee 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/interceptor/TransactionInterceptorTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/interceptor/TransactionInterceptorTests.java @@ -28,6 +28,7 @@ import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.TransactionException; +import org.springframework.transaction.TransactionManager; import org.springframework.transaction.TransactionStatus; import org.springframework.util.SerializationTestUtils; @@ -42,12 +43,13 @@ * Mock object based tests for TransactionInterceptor. * * @author Rod Johnson + * @author Juergen Hoeller * @since 16.03.2003 */ public class TransactionInterceptorTests extends AbstractTransactionAspectTests { @Override - protected Object advised(Object target, PlatformTransactionManager ptm, TransactionAttributeSource[] tas) throws Exception { + protected Object advised(Object target, PlatformTransactionManager ptm, TransactionAttributeSource[] tas) { TransactionInterceptor ti = new TransactionInterceptor(); ti.setTransactionManager(ptm); ti.setTransactionAttributeSources(tas); @@ -214,14 +216,14 @@ public void determineTransactionManagerWithQualifierSeveralTimes() { DefaultTransactionAttribute attribute = new DefaultTransactionAttribute(); attribute.setQualifier("fooTransactionManager"); - PlatformTransactionManager actual = ti.determineTransactionManager(attribute); + TransactionManager actual = ti.determineTransactionManager(attribute); assertThat(actual).isSameAs(txManager); // Call again, should be cached - PlatformTransactionManager actual2 = ti.determineTransactionManager(attribute); + TransactionManager actual2 = ti.determineTransactionManager(attribute); assertThat(actual2).isSameAs(txManager); verify(beanFactory, times(1)).containsBean("fooTransactionManager"); - verify(beanFactory, times(1)).getBean("fooTransactionManager", PlatformTransactionManager.class); + verify(beanFactory, times(1)).getBean("fooTransactionManager", TransactionManager.class); } @Test @@ -233,13 +235,13 @@ public void determineTransactionManagerWithBeanNameSeveralTimes() { PlatformTransactionManager txManager = associateTransactionManager(beanFactory, "fooTransactionManager"); DefaultTransactionAttribute attribute = new DefaultTransactionAttribute(); - PlatformTransactionManager actual = ti.determineTransactionManager(attribute); + TransactionManager actual = ti.determineTransactionManager(attribute); assertThat(actual).isSameAs(txManager); // Call again, should be cached - PlatformTransactionManager actual2 = ti.determineTransactionManager(attribute); + TransactionManager actual2 = ti.determineTransactionManager(attribute); assertThat(actual2).isSameAs(txManager); - verify(beanFactory, times(1)).getBean("fooTransactionManager", PlatformTransactionManager.class); + verify(beanFactory, times(1)).getBean("fooTransactionManager", TransactionManager.class); } @Test @@ -248,16 +250,16 @@ public void determineTransactionManagerDefaultSeveralTimes() { TransactionInterceptor ti = simpleTransactionInterceptor(beanFactory); PlatformTransactionManager txManager = mock(PlatformTransactionManager.class); - given(beanFactory.getBean(PlatformTransactionManager.class)).willReturn(txManager); + given(beanFactory.getBean(TransactionManager.class)).willReturn(txManager); DefaultTransactionAttribute attribute = new DefaultTransactionAttribute(); - PlatformTransactionManager actual = ti.determineTransactionManager(attribute); + TransactionManager actual = ti.determineTransactionManager(attribute); assertThat(actual).isSameAs(txManager); // Call again, should be cached - PlatformTransactionManager actual2 = ti.determineTransactionManager(attribute); + TransactionManager actual2 = ti.determineTransactionManager(attribute); assertThat(actual2).isSameAs(txManager); - verify(beanFactory, times(1)).getBean(PlatformTransactionManager.class); + verify(beanFactory, times(1)).getBean(TransactionManager.class); } @@ -299,7 +301,7 @@ private TransactionInterceptor simpleTransactionInterceptor(BeanFactory beanFact private PlatformTransactionManager associateTransactionManager(BeanFactory beanFactory, String name) { PlatformTransactionManager transactionManager = mock(PlatformTransactionManager.class); given(beanFactory.containsBean(name)).willReturn(true); - given(beanFactory.getBean(name, PlatformTransactionManager.class)).willReturn(transactionManager); + given(beanFactory.getBean(name, TransactionManager.class)).willReturn(transactionManager); return transactionManager; }