diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/samples/spr/ControllerAdviceIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/spr/ControllerAdviceIntegrationTests.java new file mode 100644 index 0000000000000000000000000000000000000000..b62795d2b9f15b7a84fafa8f5255f0a923ea9c7c --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/spr/ControllerAdviceIntegrationTests.java @@ -0,0 +1,128 @@ +/* + * 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.test.web.servlet.samples.spr; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.stereotype.Controller; +import org.springframework.test.context.junit.jupiter.web.SpringJUnitWebConfig; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.ui.Model; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.ModelAttribute; +import org.springframework.web.context.WebApplicationContext; +import org.springframework.web.context.annotation.RequestScope; +import org.springframework.web.servlet.config.annotation.EnableWebMvc; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.forwardedUrl; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import static org.springframework.test.web.servlet.setup.MockMvcBuilders.webAppContextSetup; + +/** + * Integration tests for {@link ControllerAdvice @ControllerAdvice}. + * + *

Introduced in conjunction with + * gh-24017. + * + * @author Sam Brannen + * @since 5.1.12 + */ +@SpringJUnitWebConfig +class ControllerAdviceIntegrationTests { + + MockMvc mockMvc; + + @BeforeEach + void setUpMockMvc(WebApplicationContext wac) { + this.mockMvc = webAppContextSetup(wac).build(); + } + + @Test + void controllerAdviceIsAppliedOnlyOnce() throws Exception { + assertThat(SingletonControllerAdvice.counter).hasValue(0); + assertThat(RequestScopedControllerAdvice.counter).hasValue(0); + + this.mockMvc.perform(get("/test"))// + .andExpect(status().isOk())// + .andExpect(forwardedUrl("singleton:1;request-scoped:1")); + + assertThat(SingletonControllerAdvice.counter).hasValue(1); + assertThat(RequestScopedControllerAdvice.counter).hasValue(1); + } + + @Configuration + @EnableWebMvc + static class Config { + + @Bean + TestController testController() { + return new TestController(); + } + + @Bean + SingletonControllerAdvice singletonControllerAdvice() { + return new SingletonControllerAdvice(); + } + + @Bean + @RequestScope + RequestScopedControllerAdvice requestScopedControllerAdvice() { + return new RequestScopedControllerAdvice(); + } + } + + @ControllerAdvice + static class SingletonControllerAdvice { + + static final AtomicInteger counter = new AtomicInteger(); + + @ModelAttribute + void initModel(Model model) { + model.addAttribute("singleton", counter.incrementAndGet()); + } + } + + @ControllerAdvice + static class RequestScopedControllerAdvice { + + static final AtomicInteger counter = new AtomicInteger(); + + @ModelAttribute + void initModel(Model model) { + model.addAttribute("request-scoped", counter.incrementAndGet()); + } + } + + @Controller + static class TestController { + + @GetMapping("/test") + String get(Model model) { + return "singleton:" + model.getAttribute("singleton") + ";request-scoped:" + + model.getAttribute("request-scoped"); + } + } + +} diff --git a/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java b/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java index 8ec98e0ee7b0ee6d63375f3015e546694f39fb39..e66bb4473091a42e6b10ccca97272395fe225ba4 100644 --- a/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java +++ b/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java @@ -252,11 +252,13 @@ public class ControllerAdviceBean implements Ordered { public static List findAnnotatedBeans(ApplicationContext context) { List adviceBeans = new ArrayList<>(); for (String name : BeanFactoryUtils.beanNamesForTypeIncludingAncestors(context, Object.class)) { - ControllerAdvice controllerAdvice = context.findAnnotationOnBean(name, ControllerAdvice.class); - if (controllerAdvice != null) { - // Use the @ControllerAdvice annotation found by findAnnotationOnBean() - // in order to avoid a subsequent lookup of the same annotation. - adviceBeans.add(new ControllerAdviceBean(name, context, controllerAdvice)); + if (!ScopedProxyUtils.isScopedTarget(name)) { + ControllerAdvice controllerAdvice = context.findAnnotationOnBean(name, ControllerAdvice.class); + if (controllerAdvice != null) { + // Use the @ControllerAdvice annotation found by findAnnotationOnBean() + // in order to avoid a subsequent lookup of the same annotation. + adviceBeans.add(new ControllerAdviceBean(name, context, controllerAdvice)); + } } } OrderComparator.sort(adviceBeans); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestScopedControllerAdviceIntegrationTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestScopedControllerAdviceIntegrationTests.java index 14b812987afa5d62289db26a7381eb15378dadbb..8cb79c50d26c4ceb407bf269af15b63820684954 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestScopedControllerAdviceIntegrationTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestScopedControllerAdviceIntegrationTests.java @@ -43,7 +43,6 @@ import static org.assertj.core.api.Assertions.assertThatCode; class RequestScopedControllerAdviceIntegrationTests { @Test // gh-23985 - @SuppressWarnings({ "rawtypes", "unchecked" }) void loadContextWithRequestScopedControllerAdvice() { AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); context.setServletContext(new MockServletContext()); @@ -51,17 +50,11 @@ class RequestScopedControllerAdviceIntegrationTests { assertThatCode(context::refresh).doesNotThrowAnyException(); - // Until gh-24017 is fixed, we expect the RequestScopedControllerAdvice to show up twice. - Class[] expectedTypes = { RequestScopedControllerAdvice.class, RequestScopedControllerAdvice.class }; - List adviceBeans = ControllerAdviceBean.findAnnotatedBeans(context); - assertThat(adviceBeans)// - .extracting(ControllerAdviceBean::getBeanType)// - .containsExactly(expectedTypes); - - assertThat(adviceBeans)// - .extracting(ControllerAdviceBean::getOrder)// - .containsExactly(42, 42); + assertThat(adviceBeans).size().isEqualTo(1); + assertThat(adviceBeans.get(0))// + .returns(RequestScopedControllerAdvice.class, ControllerAdviceBean::getBeanType)// + .returns(42, ControllerAdviceBean::getOrder); context.close(); }