Merge pull request #33911 from mhalbritter

* mh/19605:
  Use bean name for servlet and filter registrations if name is not set

Closes gh-33911

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jersey/JerseyAutoConfiguration.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2022 the original author or authors.
+ * Copyright 2012-2023 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.
@@ -144,6 +144,7 @@ public class JerseyAutoConfiguration implements ServletContextAware {
 		addInitParameters(registration);
 		registration.setName(getServletRegistrationName());
 		registration.setLoadOnStartup(this.jersey.getServlet().getLoadOnStartup());
+		registration.setIgnoreRegistrationFailure(true);
 		return registration;
 	}
 

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java

@@ -501,7 +501,10 @@ class ServletWebServerFactoryAutoConfigurationTests {
 
 		@Bean(name = DispatcherServletAutoConfiguration.DEFAULT_DISPATCHER_SERVLET_REGISTRATION_BEAN_NAME)
 		ServletRegistrationBean<DispatcherServlet> dispatcherRegistration(DispatcherServlet dispatcherServlet) {
-			return new ServletRegistrationBean<>(dispatcherServlet, "/app/*");
+			ServletRegistrationBean<DispatcherServlet> registration = new ServletRegistrationBean<>(dispatcherServlet,
+					"/app/*");
+			registration.setName(DispatcherServletAutoConfiguration.DEFAULT_DISPATCHER_SERVLET_BEAN_NAME);
+			return registration;
 		}
 
 	}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/DynamicRegistrationBean.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2022 the original author or authors.
+ * Copyright 2012-2023 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.
@@ -24,6 +24,7 @@ import jakarta.servlet.ServletContext;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.beans.factory.BeanNameAware;
 import org.springframework.core.Conventions;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
@@ -34,9 +35,11 @@ import org.springframework.util.StringUtils;
  *
  * @param <D> the dynamic registration result
  * @author Phillip Webb
+ * @author Moritz Halbritter
  * @since 2.0.0
  */
-public abstract class DynamicRegistrationBean<D extends Registration.Dynamic> extends RegistrationBean {
+public abstract class DynamicRegistrationBean<D extends Registration.Dynamic> extends RegistrationBean
+		implements BeanNameAware {
 
 	private static final Log logger = LogFactory.getLog(RegistrationBean.class);
 
@@ -46,6 +49,10 @@ public abstract class DynamicRegistrationBean<D extends Registration.Dynamic> ex
 
 	private Map<String, String> initParameters = new LinkedHashMap<>();
 
+	private String beanName;
+
+	private boolean ignoreRegistrationFailure;
+
 	/**
 	 * Set the name of this registration. If not specified the bean name will be used.
 	 * @param name the name of the registration
@@ -106,12 +113,32 @@ public abstract class DynamicRegistrationBean<D extends Registration.Dynamic> ex
 	protected final void register(String description, ServletContext servletContext) {
 		D registration = addRegistration(description, servletContext);
 		if (registration == null) {
-			logger.info(StringUtils.capitalize(description) + " was not registered (possibly already registered?)");
-			return;
+			if (this.ignoreRegistrationFailure) {
+				logger.info(StringUtils.capitalize(description) + " was not registered (possibly already registered?)");
+				return;
+			}
+			throw new IllegalStateException(
+					"Failed to register '%s' on the servlet context. Possibly already registered?"
+							.formatted(description));
 		}
 		configure(registration);
 	}
 
+	/**
+	 * Sets whether registration failures should be ignored. If set to true, a failure
+	 * will be logged. If set to false, an {@link IllegalStateException} will be thrown.
+	 * @param ignoreRegistrationFailure whether to ignore registration failures
+	 * @since 3.1.0
+	 */
+	public void setIgnoreRegistrationFailure(boolean ignoreRegistrationFailure) {
+		this.ignoreRegistrationFailure = ignoreRegistrationFailure;
+	}
+
+	@Override
+	public void setBeanName(String name) {
+		this.beanName = name;
+	}
+
 	protected abstract D addRegistration(String description, ServletContext servletContext);
 
 	protected void configure(D registration) {
@@ -123,12 +150,19 @@ public abstract class DynamicRegistrationBean<D extends Registration.Dynamic> ex
 
 	/**
 	 * Deduces the name for this registration. Will return user specified name or fallback
-	 * to convention based naming.
+	 * to the bean name. If the bean name is not available, convention based naming is
+	 * used.
 	 * @param value the object used for convention based names
 	 * @return the deduced name
 	 */
 	protected final String getOrDeduceName(Object value) {
-		return (this.name != null) ? this.name : Conventions.getVariableName(value);
+		if (this.name != null) {
+			return this.name;
+		}
+		if (this.beanName != null) {
+			return this.beanName;
+		}
+		return Conventions.getVariableName(value);
 	}
 
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/AbstractFilterRegistrationBeanTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2022 the original author or authors.
+ * Copyright 2012-2023 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.
@@ -32,7 +32,9 @@ import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
@@ -94,6 +96,7 @@ abstract class AbstractFilterRegistrationBeanTests {
 
 	@Test
 	void specificName() throws Exception {
+		given(this.servletContext.addFilter(anyString(), any(Filter.class))).willReturn(this.registration);
 		AbstractFilterRegistrationBean<?> bean = createFilterRegistrationBean();
 		bean.setName("specificName");
 		bean.onStartup(this.servletContext);
@@ -102,6 +105,7 @@ abstract class AbstractFilterRegistrationBeanTests {
 
 	@Test
 	void deducedName() throws Exception {
+		given(this.servletContext.addFilter(anyString(), any(Filter.class))).willReturn(this.registration);
 		AbstractFilterRegistrationBean<?> bean = createFilterRegistrationBean();
 		bean.onStartup(this.servletContext);
 		then(this.servletContext).should().addFilter(eq("mockFilter"), getExpectedFilter());
@@ -198,6 +202,28 @@ abstract class AbstractFilterRegistrationBeanTests {
 		then(this.registration).should().addMappingForUrlPatterns(types, false, "/*");
 	}
 
+	@Test
+	void failsWithDoubleRegistration() {
+		assertThatThrownBy(() -> {
+			AbstractFilterRegistrationBean<?> bean = createFilterRegistrationBean();
+			bean.setName("double-registration");
+			given(this.servletContext.addFilter(anyString(), any(Filter.class))).willReturn(null);
+			bean.onStartup(this.servletContext);
+		}).isInstanceOf(IllegalStateException.class).hasMessage(
+				"Failed to register 'filter double-registration' on the servlet context. Possibly already registered?");
+	}
+
+	@Test
+	void doesntFailIfDoubleRegistrationIsIgnored() {
+		assertThatCode(() -> {
+			AbstractFilterRegistrationBean<?> bean = createFilterRegistrationBean();
+			bean.setName("double-registration");
+			given(this.servletContext.addFilter(anyString(), any(Filter.class))).willReturn(null);
+			bean.setIgnoreRegistrationFailure(true);
+			bean.onStartup(this.servletContext);
+		}).doesNotThrowAnyException();
+	}
+
 	protected abstract Filter getExpectedFilter();
 
 	protected abstract AbstractFilterRegistrationBean<?> createFilterRegistrationBean(

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/DelegatingFilterProxyRegistrationBeanTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2022 the original author or authors.
+ * Copyright 2012-2023 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.
@@ -87,9 +87,8 @@ class DelegatingFilterProxyRegistrationBeanTests extends AbstractFilterRegistrat
 
 	@Test
 	void createServletRegistrationBeanMustNotBeNull() {
-		assertThatIllegalArgumentException()
-				.isThrownBy(
-						() -> new DelegatingFilterProxyRegistrationBean("mockFilter", (ServletRegistrationBean[]) null))
+		assertThatIllegalArgumentException().isThrownBy(
+				() -> new DelegatingFilterProxyRegistrationBean("mockFilter", (ServletRegistrationBean<?>[]) null))
 				.withMessageContaining("ServletRegistrationBeans must not be null");
 	}
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/DynamicRegistrationBeanTests.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2012-2023 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.boot.web.servlet;
+
+import jakarta.servlet.Registration.Dynamic;
+import jakarta.servlet.ServletContext;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link DynamicRegistrationBean}.
+ *
+ * @author Moritz Halbritter
+ */
+class DynamicRegistrationBeanTests {
+
+	@Test
+	void shouldUseNameIfSet() {
+		DynamicRegistrationBean<?> bean = createBean();
+		bean.setName("givenName");
+		assertThat(bean.getOrDeduceName("dummy")).isEqualTo("givenName");
+	}
+
+	@Test
+	void shouldUseBeanNameIfNameIsNotSet() {
+		DynamicRegistrationBean<?> bean = createBean();
+		bean.setBeanName("beanName");
+		assertThat(bean.getOrDeduceName("dummy")).isEqualTo("beanName");
+	}
+
+	@Test
+	void shouldUseConventionBasedNameIfNoNameOrBeanNameIsSet() {
+		DynamicRegistrationBean<?> bean = createBean();
+		assertThat(bean.getOrDeduceName("dummy")).isEqualTo("string");
+	}
+
+	private static DynamicRegistrationBean<?> createBean() {
+		return new DynamicRegistrationBean<Dynamic>() {
+			@Override
+			protected Dynamic addRegistration(String description, ServletContext servletContext) {
+				return null;
+			}
+
+			@Override
+			protected String getDescription() {
+				return null;
+			}
+		};
+	}
+
+}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/FilterRegistrationBeanTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2022 the original author or authors.
+ * Copyright 2012-2023 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.
@@ -41,6 +41,7 @@ import static org.mockito.BDDMockito.then;
  * Tests for {@link FilterRegistrationBean}.
  *
  * @author Phillip Webb
+ * @author Moritz Halbritter
  */
 class FilterRegistrationBeanTests extends AbstractFilterRegistrationBeanTests {
 
@@ -58,6 +59,7 @@ class FilterRegistrationBeanTests extends AbstractFilterRegistrationBeanTests {
 
 	@Test
 	void setFilter() throws Exception {
+		given(this.servletContext.addFilter(anyString(), any(Filter.class))).willReturn(this.registration);
 		FilterRegistrationBean<Filter> bean = new FilterRegistrationBean<>();
 		bean.setFilter(this.filter);
 		bean.onStartup(this.servletContext);

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/ServletRegistrationBeanTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2022 the original author or authors.
+ * Copyright 2012-2023 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.
@@ -22,7 +22,6 @@ import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
 
-import jakarta.servlet.FilterRegistration;
 import jakarta.servlet.Servlet;
 import jakarta.servlet.ServletContext;
 import jakarta.servlet.ServletRegistration;
@@ -34,6 +33,7 @@ import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.boot.web.servlet.mock.MockServlet;
 
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.BDDMockito.given;
@@ -56,9 +56,6 @@ class ServletRegistrationBeanTests {
 	@Mock
 	private ServletRegistration.Dynamic registration;
 
-	@Mock
-	private FilterRegistration.Dynamic filterRegistration;
-
 	@Test
 	void startupWithDefaults() throws Exception {
 		given(this.servletContext.addServlet(anyString(), any(Servlet.class))).willReturn(this.registration);
@@ -70,12 +67,14 @@ class ServletRegistrationBeanTests {
 	}
 
 	@Test
-	void startupWithDoubleRegistration() throws Exception {
-		ServletRegistrationBean<MockServlet> bean = new ServletRegistrationBean<>(this.servlet);
-		given(this.servletContext.addServlet(anyString(), any(Servlet.class))).willReturn(null);
-		bean.onStartup(this.servletContext);
-		then(this.servletContext).should().addServlet("mockServlet", this.servlet);
-		then(this.registration).should(never()).setAsyncSupported(true);
+	void failsWithDoubleRegistration() {
+		assertThatThrownBy(() -> {
+			ServletRegistrationBean<MockServlet> bean = new ServletRegistrationBean<>(this.servlet);
+			bean.setName("double-registration");
+			given(this.servletContext.addServlet(anyString(), any(Servlet.class))).willReturn(null);
+			bean.onStartup(this.servletContext);
+		}).isInstanceOf(IllegalStateException.class).hasMessage(
+				"Failed to register 'servlet double-registration' on the servlet context. Possibly already registered?");
 	}
 
 	@Test
@@ -103,6 +102,7 @@ class ServletRegistrationBeanTests {
 
 	@Test
 	void specificName() throws Exception {
+		given(this.servletContext.addServlet(anyString(), any(Servlet.class))).willReturn(this.registration);
 		ServletRegistrationBean<MockServlet> bean = new ServletRegistrationBean<>();
 		bean.setName("specificName");
 		bean.setServlet(this.servlet);
@@ -112,6 +112,7 @@ class ServletRegistrationBeanTests {
 
 	@Test
 	void deducedName() throws Exception {
+		given(this.servletContext.addServlet(anyString(), any(Servlet.class))).willReturn(this.registration);
 		ServletRegistrationBean<MockServlet> bean = new ServletRegistrationBean<>();
 		bean.setServlet(this.servlet);
 		bean.onStartup(this.servletContext);
@@ -182,6 +183,7 @@ class ServletRegistrationBeanTests {
 
 	@Test
 	void withoutDefaultMappings() throws Exception {
+		given(this.servletContext.addServlet(anyString(), any(Servlet.class))).willReturn(this.registration);
 		ServletRegistrationBean<MockServlet> bean = new ServletRegistrationBean<>(this.servlet, false);
 		bean.onStartup(this.servletContext);
 		then(this.registration).should(never()).addMapping(any(String[].class));

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContextTests.java

@@ -237,13 +237,14 @@ class ServletWebServerApplicationContextTests {
 		OrderedFilter filter = new OrderedFilter();
 		this.context.registerBeanDefinition("filterBean", beanDefinition(filter));
 		FilterRegistrationBean<Filter> registration = new FilterRegistrationBean<>();
+		registration.setName("filterBeanRegistration");
 		registration.setFilter(mock(Filter.class));
 		registration.setOrder(100);
 		this.context.registerBeanDefinition("filterRegistrationBean", beanDefinition(registration));
 		this.context.refresh();
 		MockServletWebServerFactory factory = getWebServerFactory();
 		then(factory.getServletContext()).should().addFilter("filterBean", filter);
-		then(factory.getServletContext()).should().addFilter("object", registration.getFilter());
+		then(factory.getServletContext()).should().addFilter("filterBeanRegistration", registration.getFilter());
 		assertThat(factory.getRegisteredFilter(0).getFilter()).isEqualTo(filter);
 	}
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializerTests.java

@@ -23,6 +23,8 @@ import java.util.Vector;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import jakarta.servlet.DispatcherType;
+import jakarta.servlet.Filter;
+import jakarta.servlet.FilterRegistration.Dynamic;
 import jakarta.servlet.ServletContext;
 import jakarta.servlet.ServletContextEvent;
 import jakarta.servlet.ServletContextListener;
@@ -55,6 +57,7 @@ import org.springframework.web.context.support.StandardServletEnvironment;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.BDDMockito.then;
 import static org.mockito.Mockito.mock;
@@ -189,6 +192,7 @@ class SpringBootServletInitializerTests {
 	@Test
 	void servletContextPropertySourceIsAvailablePriorToRefresh() {
 		ServletContext servletContext = mock(ServletContext.class);
+		given(servletContext.addFilter(any(), any(Filter.class))).willReturn(mock(Dynamic.class));
 		given(servletContext.getInitParameterNames())
 				.willReturn(Collections.enumeration(Collections.singletonList("spring.profiles.active")));
 		given(servletContext.getInitParameter("spring.profiles.active")).willReturn("from-servlet-context");
@@ -202,6 +206,7 @@ class SpringBootServletInitializerTests {
 	@Test
 	void whenServletContextIsDestroyedThenJdbcDriversAreDeregistered() throws ServletException {
 		ServletContext servletContext = mock(ServletContext.class);
+		given(servletContext.addFilter(any(), any(Filter.class))).willReturn(mock(Dynamic.class));
 		given(servletContext.getInitParameterNames()).willReturn(new Vector<String>().elements());
 		given(servletContext.getAttributeNames()).willReturn(new Vector<String>().elements());
 		AtomicBoolean driversDeregistered = new AtomicBoolean();