Remove no-op classes in web-related Java config

rstoyanchev · rstoyanchev · commit a40d25a76024 · 2018-07-11T11:10:03.000-04:00
diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/config/AbstractMessageBrokerConfiguration.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/config/AbstractMessageBrokerConfiguration.java
@@ -28,7 +28,6 @@
 import org.springframework.context.ApplicationContextAware;
 import org.springframework.context.annotation.Bean;
 import org.springframework.lang.Nullable;
-import org.springframework.messaging.Message;
 import org.springframework.messaging.MessageHandler;
 import org.springframework.messaging.converter.ByteArrayMessageConverter;
 import org.springframework.messaging.converter.CompositeMessageConverter;
@@ -290,10 +289,11 @@ protected void addReturnValueHandlers(List<HandlerMethodReturnValueHandler> retu
 	}
 
 	@Bean
+	@Nullable
 	public AbstractBrokerMessageHandler simpleBrokerMessageHandler() {
 		SimpleBrokerMessageHandler handler = getBrokerRegistry().getSimpleBroker(brokerChannel());
 		if (handler == null) {
-			return new NoOpBrokerMessageHandler();
+			return null;
 		}
 		updateUserDestinationResolver(handler);
 		return handler;
@@ -307,10 +307,11 @@ private void updateUserDestinationResolver(AbstractBrokerMessageHandler handler)
 	}
 
 	@Bean
+	@Nullable
 	public AbstractBrokerMessageHandler stompBrokerRelayMessageHandler() {
 		StompBrokerRelayMessageHandler handler = getBrokerRegistry().getStompBrokerRelay(brokerChannel());
 		if (handler == null) {
-			return new NoOpBrokerMessageHandler();
+			return null;
 		}
 		Map<String, MessageHandler> subscriptions = new HashMap<>(1);
 		String destination = getBrokerRegistry().getUserDestinationBroadcast();
@@ -338,9 +339,10 @@ public UserDestinationMessageHandler userDestinationMessageHandler() {
 	}
 
 	@Bean
+	@Nullable
 	public MessageHandler userRegistryMessageHandler() {
 		if (getBrokerRegistry().getUserRegistryBroadcast() == null) {
-			return new NoOpMessageHandler();
+			return null;
 		}
 		SimpUserRegistry userRegistry = userRegistry();
 		Assert.isInstanceOf(MultiServerUserRegistry.class, userRegistry, "MultiServerUserRegistry required");
@@ -424,7 +426,8 @@ public SimpUserRegistry userRegistry() {
 	protected abstract SimpUserRegistry createLocalUserRegistry();
 
 	/**
-	 * Return a {@link org.springframework.validation.Validator org.springframework.validation.Validators} instance for validating
+	 * Return a {@link org.springframework.validation.Validator
+	 * org.springframework.validation.Validators} instance for validating
 	 * {@code @Payload} method arguments.
 	 * <p>In order, this method tries to get a Validator instance:
 	 * <ul>
@@ -477,36 +480,4 @@ public Validator getValidator() {
 		return null;
 	}
 
-
-	private static class NoOpMessageHandler implements MessageHandler {
-
-		@Override
-		public void handleMessage(Message<?> message) {
-		}
-	}
-
-
-	private class NoOpBrokerMessageHandler extends AbstractBrokerMessageHandler {
-
-		public NoOpBrokerMessageHandler() {
-			super(clientInboundChannel(), clientOutboundChannel(), brokerChannel());
-		}
-
-		@Override
-		public void start() {
-		}
-
-		@Override
-		public void stop() {
-		}
-
-		@Override
-		public void handleMessage(Message<?> message) {
-		}
-
-		@Override
-		protected void handleMessageInternal(Message<?> message) {
-		}
-	}
-
 }
diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/config/MessageBrokerConfigurationTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/config/MessageBrokerConfigurationTests.java
@@ -458,9 +458,8 @@ public void userBroadcastsDisabledWithSimpleBroker() {
 		UserDestinationMessageHandler handler = context.getBean(UserDestinationMessageHandler.class);
 		assertNull(handler.getBroadcastDestination());
 
-		String name = "userRegistryMessageHandler";
-		MessageHandler messageHandler = context.getBean(name, MessageHandler.class);
-		assertNotEquals(UserRegistryMessageHandler.class, messageHandler.getClass());
+		Object nullBean = context.getBean("userRegistryMessageHandler");
+		assertTrue(nullBean.equals(null));
 	}
 
 	@Test // SPR-16275
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
@@ -24,7 +24,6 @@
 import java.util.Map;
 import java.util.function.Predicate;
 import javax.servlet.ServletContext;
-import javax.servlet.http.HttpServletRequest;
 
 import org.springframework.beans.BeanUtils;
 import org.springframework.beans.factory.BeanFactoryUtils;
@@ -448,12 +447,15 @@ protected void configureContentNegotiation(ContentNegotiationConfigurer configur
 	 * {@link #addViewControllers}.
 	 */
 	@Bean
+	@Nullable
 	public HandlerMapping viewControllerHandlerMapping() {
 		ViewControllerRegistry registry = new ViewControllerRegistry(this.applicationContext);
 		addViewControllers(registry);
 
 		AbstractHandlerMapping handlerMapping = registry.buildHandlerMapping();
-		handlerMapping = (handlerMapping != null ? handlerMapping : new EmptyHandlerMapping());
+		if (handlerMapping == null) {
+			return null;
+		}
 		handlerMapping.setPathMatcher(mvcPathMatcher());
 		handlerMapping.setUrlPathHelper(mvcUrlPathHelper());
 		handlerMapping.setInterceptors(getInterceptors());
@@ -487,6 +489,7 @@ public BeanNameUrlHandlerMapping beanNameHandlerMapping() {
 	 * {@link #addResourceHandlers}.
 	 */
 	@Bean
+	@Nullable
 	public HandlerMapping resourceHandlerMapping() {
 		Assert.state(this.applicationContext != null, "No ApplicationContext set");
 		Assert.state(this.servletContext != null, "No ServletContext set");
@@ -496,15 +499,13 @@ public HandlerMapping resourceHandlerMapping() {
 		addResourceHandlers(registry);
 
 		AbstractHandlerMapping handlerMapping = registry.getHandlerMapping();
-		if (handlerMapping != null) {
-			handlerMapping.setPathMatcher(mvcPathMatcher());
-			handlerMapping.setUrlPathHelper(mvcUrlPathHelper());
-			handlerMapping.setInterceptors(getInterceptors());
-			handlerMapping.setCorsConfigurations(getCorsConfigurations());
-		}
-		else {
-			handlerMapping = new EmptyHandlerMapping();
+		if (handlerMapping == null) {
+			return null;
 		}
+		handlerMapping.setPathMatcher(mvcPathMatcher());
+		handlerMapping.setUrlPathHelper(mvcUrlPathHelper());
+		handlerMapping.setInterceptors(getInterceptors());
+		handlerMapping.setCorsConfigurations(getCorsConfigurations());
 		return handlerMapping;
 	}
 
@@ -539,13 +540,12 @@ public ResourceUrlProvider mvcResourceUrlProvider() {
 	 * override {@link #configureDefaultServletHandling}.
 	 */
 	@Bean
+	@Nullable
 	public HandlerMapping defaultServletHandlerMapping() {
 		Assert.state(this.servletContext != null, "No ServletContext set");
 		DefaultServletHandlerConfigurer configurer = new DefaultServletHandlerConfigurer(this.servletContext);
 		configureDefaultServletHandling(configurer);
-
-		HandlerMapping handlerMapping = configurer.buildHandlerMapping();
-		return (handlerMapping != null ? handlerMapping : new EmptyHandlerMapping());
+		return configurer.buildHandlerMapping();
 	}
 
 	/**
@@ -645,7 +645,8 @@ public FormattingConversionService mvcConversionService() {
 	}
 
 	/**
-	 * Override this method to add custom {@link Converter}s and {@link Formatter Converter}s and {@link Formatters}.
+	 * Override this method to add custom {@link Converter}s and
+	 * {@link Formatter Converter}s and {@link Formatters}.
 	 */
 	protected void addFormatters(FormatterRegistry registry) {
 	}
@@ -1046,16 +1047,6 @@ public HandlerMappingIntrospector mvcHandlerMappingIntrospector() {
 	}
 
 
-
-	private static final class EmptyHandlerMapping extends AbstractHandlerMapping {
-
-		@Override
-		protected Object getHandlerInternal(HttpServletRequest request) {
-			return null;
-		}
-	}
-
-
 	private static final class NoOpValidator implements Validator {
 
 		@Override
diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java
@@ -67,7 +67,6 @@
 import org.springframework.web.servlet.HandlerExceptionResolver;
 import org.springframework.web.servlet.HandlerExecutionChain;
 import org.springframework.web.servlet.ViewResolver;
-import org.springframework.web.servlet.handler.AbstractHandlerMapping;
 import org.springframework.web.servlet.handler.BeanNameUrlHandlerMapping;
 import org.springframework.web.servlet.handler.ConversionServiceExposingInterceptor;
 import org.springframework.web.servlet.handler.HandlerExceptionResolverComposite;
@@ -85,13 +84,9 @@
 import org.springframework.web.servlet.view.ViewResolverComposite;
 import org.springframework.web.util.UrlPathHelper;
 
-import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
-import static com.fasterxml.jackson.databind.MapperFeature.DEFAULT_VIEW_INCLUSION;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static com.fasterxml.jackson.databind.DeserializationFeature.*;
+import static com.fasterxml.jackson.databind.MapperFeature.*;
+import static org.junit.Assert.*;
 
 /**
  * Integration tests for {@link WebMvcConfigurationSupport} (imported via
@@ -123,14 +118,17 @@ public void requestMappingHandlerMapping() throws Exception {
 	}
 
 	@Test
-	public void emptyViewControllerHandlerMapping() {
+	public void emptyHandlerMappings() {
 		ApplicationContext context = initContext(WebConfig.class);
-		String name = "viewControllerHandlerMapping";
-		AbstractHandlerMapping handlerMapping = context.getBean(name, AbstractHandlerMapping.class);
 
-		assertNotNull(handlerMapping);
-		assertEquals(Integer.MAX_VALUE, handlerMapping.getOrder());
-		assertTrue(handlerMapping.getClass().getName().endsWith("EmptyHandlerMapping"));
+		Object nullBean = context.getBean("viewControllerHandlerMapping");
+		assertTrue(nullBean.equals(null));
+
+		nullBean = context.getBean("resourceHandlerMapping");
+		assertTrue(nullBean.equals(null));
+
+		nullBean = context.getBean("defaultServletHandlerMapping");
+		assertTrue(nullBean.equals(null));
 	}
 
 	@Test
@@ -149,27 +147,6 @@ public void beanNameHandlerMapping() throws Exception {
 		assertEquals(ResourceUrlProviderExposingInterceptor.class, chain.getInterceptors()[2].getClass());
 	}
 
-	@Test
-	public void emptyResourceHandlerMapping() {
-		ApplicationContext context = initContext(WebConfig.class);
-		AbstractHandlerMapping handlerMapping = context.getBean("resourceHandlerMapping", AbstractHandlerMapping.class);
-
-		assertNotNull(handlerMapping);
-		assertEquals(Integer.MAX_VALUE, handlerMapping.getOrder());
-		assertTrue(handlerMapping.getClass().getName().endsWith("EmptyHandlerMapping"));
-	}
-
-	@Test
-	public void emptyDefaultServletHandlerMapping() {
-		ApplicationContext context = initContext(WebConfig.class);
-		String name = "defaultServletHandlerMapping";
-		AbstractHandlerMapping handlerMapping = context.getBean(name, AbstractHandlerMapping.class);
-
-		assertNotNull(handlerMapping);
-		assertEquals(Integer.MAX_VALUE, handlerMapping.getOrder());
-		assertTrue(handlerMapping.getClass().getName().endsWith("EmptyHandlerMapping"));
-	}
-
 	@Test
 	public void requestMappingHandlerAdapter() throws Exception {
 		ApplicationContext context = initContext(WebConfig.class);
