Use bean name for servlet and filter registrations if name is not set

If the name is set, it is used. If not, the bean name is used. If that's
not set, the convention based name is taken.

Registration failures now throw an IllegalStateException instead of
being logged. DynamicRegistrationBean.setIgnoreRegistrationFailure
can be used to restore the old behavior.

See 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();