Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ControllerAdviceBean.findAnnotatedBeans() finds proxies as well as their target beans, resulting in double registration #24017

Closed
1 task done
sbrannen opened this issue Nov 18, 2019 · 1 comment
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@sbrannen
Copy link
Member

sbrannen commented Nov 18, 2019

Overview

While implementing a fix for the regression raised in #23985, it became apparent that methods in a @ControllerAdvice bean end up being registered and invoked twice if the advice is a scoped bean (e.g., request or session scoped). In other words, both the proxy bean and the target bean are wrapped in ControllerAdviceBean instances.

The reason this happens is that ControllerAdviceBean.findAnnotatedBeans() finds all beans in the ApplicationContext that are annotated with @ControllerAdvice.

Deliverables

  • Ensure that the list returned by ControllerAdviceBean.findAnnotatedBeans() does not contain target beans for scoped proxies.
@sbrannen sbrannen added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug labels Nov 18, 2019
@sbrannen sbrannen added this to the 5.2.2 milestone Nov 18, 2019
@sbrannen sbrannen self-assigned this Nov 18, 2019
@sbrannen
Copy link
Member Author

sbrannen commented Nov 18, 2019

The following failing test case demonstrates the undesired behavior.

@SpringJUnitWebConfig
class RequestScopedControllerAdviceIntegrationTests {

	MockMvc mockMvc;

	@BeforeEach
	void setUpMockMvc(WebApplicationContext wac) throws Exception {
		this.mockMvc = webAppContextSetup(wac).build();
	}

	@Test
	void test() throws Exception {
		this.mockMvc.perform(get("/test"))//
				.andExpect(status().isOk())//
				.andExpect(forwardedUrl("count: 1"));

	}

	@Configuration
	@EnableWebMvc
	static class Config {

		@Bean
		TestController testController() {
			return new TestController();
		}

		@Bean
		@RequestScope
		RequestScopedControllerAdvice requestScopedControllerAdvice() {
			return new RequestScopedControllerAdvice();
		}
	}

	@ControllerAdvice
	static class RequestScopedControllerAdvice {

		static final AtomicInteger counter = new AtomicInteger();

		@ModelAttribute
		public void initModel(Model model) {
			model.addAttribute("count", counter.incrementAndGet());
		}
	}

	@Controller
	static class TestController {

		@GetMapping("/test")
		String get(Model model) {
			return "count: " + model.getAttribute("count");
		}
	}

}

@sbrannen sbrannen added the for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x label Nov 18, 2019
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x labels Nov 18, 2019
sbrannen added a commit that referenced this issue Nov 19, 2019
Prior to this commit, methods in a @ControllerAdvice bean were
registered and invoked twice if the advice was a scoped bean (e.g.,
request or session scoped). In other words, both the proxy bean and the
target bean were wrapped in ControllerAdviceBean instances.

This commit fixes this bug by modifying the findAnnotatedBeans() method
in ControllerAdviceBean so that it filters out targets of scoped
proxies.

Closes gh-24017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants