Merge pull request #32064 from awmeinema

* pr/32064:
  Polish "Allow AbstractUrlHandlerMapping to add/remote handlers"
  Allow AbstractUrlHandlerMapping to add/remote handlers

Closes gh-32064

spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -137,6 +137,116 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping i
 		this.lazyInitHandlers = lazyInitHandlers;
 	}
 
+	/**
+	 * Register the specified handler for the given URL paths.
+	 * @param urlPaths the URLs that the bean should be mapped to
+	 * @param beanName the name of the handler bean
+	 * @throws BeansException if the handler couldn't be registered
+	 * @throws IllegalStateException if there is a conflicting handler registered
+	 */
+	public void registerHandler(String[] urlPaths, String beanName) throws BeansException, IllegalStateException {
+		Assert.notNull(urlPaths, "URL path array must not be null");
+		for (String urlPath : urlPaths) {
+			registerHandler(urlPath, beanName);
+		}
+	}
+
+	/**
+	 * Register the specified handler for the given URL path.
+	 * @param urlPath the URL the bean should be mapped to
+	 * @param handler the handler instance or handler bean name String
+	 * (a bean name will automatically be resolved into the corresponding handler bean)
+	 * @throws BeansException if the handler couldn't be registered
+	 * @throws IllegalStateException if there is a conflicting handler registered
+	 */
+	public void registerHandler(String urlPath, Object handler) throws BeansException, IllegalStateException {
+		Assert.notNull(urlPath, "URL path must not be null");
+		Assert.notNull(handler, "Handler object must not be null");
+		Object resolvedHandler = handler;
+
+		// Eagerly resolve handler if referencing singleton via name.
+		if (!this.lazyInitHandlers && handler instanceof String handlerName) {
+			ApplicationContext applicationContext = obtainApplicationContext();
+			if (applicationContext.isSingleton(handlerName)) {
+				resolvedHandler = applicationContext.getBean(handlerName);
+			}
+		}
+
+		Object mappedHandler = this.handlerMap.get(urlPath);
+		if (mappedHandler != null) {
+			if (mappedHandler != resolvedHandler) {
+				throw new IllegalStateException(
+						"Cannot map " + getHandlerDescription(handler) + " to URL path [" + urlPath +
+								"]: There is already " + getHandlerDescription(mappedHandler) + " mapped.");
+			}
+		}
+		else {
+			if (urlPath.equals("/")) {
+				if (logger.isTraceEnabled()) {
+					logger.trace("Root mapping to " + getHandlerDescription(handler));
+				}
+				setRootHandler(resolvedHandler);
+			}
+			else if (urlPath.equals("/*")) {
+				if (logger.isTraceEnabled()) {
+					logger.trace("Default mapping to " + getHandlerDescription(handler));
+				}
+				setDefaultHandler(resolvedHandler);
+			}
+			else {
+				this.handlerMap.put(urlPath, resolvedHandler);
+				if (getPatternParser() != null) {
+					this.pathPatternHandlerMap.put(getPatternParser().parse(urlPath), resolvedHandler);
+				}
+				if (logger.isTraceEnabled()) {
+					logger.trace("Mapped [" + urlPath + "] onto " + getHandlerDescription(handler));
+				}
+			}
+		}
+	}
+
+	/**
+	 * Remove the mapping for the handler registered for the given URL path.
+	 * @param urlPath the mapping to remove
+	 */
+	public void unregisterHandler(String urlPath) {
+		Assert.notNull(urlPath, "URL path must not be null");
+		if (urlPath.equals("/")) {
+			if (logger.isTraceEnabled()) {
+				logger.trace("Removing root mapping: " + getRootHandler());
+			}
+			setRootHandler(null);
+		}
+		else if (urlPath.equals("/*")) {
+			if (logger.isTraceEnabled()) {
+				logger.trace("Removing default mapping: " + getDefaultHandler());
+			}
+			setDefaultHandler(null);
+		}
+		else {
+			Object mappedHandler = this.handlerMap.get(urlPath);
+			if (mappedHandler == null) {
+				if (logger.isTraceEnabled()) {
+					logger.trace("No mapping for [" + urlPath + "]");
+				}
+			}
+			else {
+				if (logger.isTraceEnabled()) {
+					logger.trace("Removing mapping \"" + urlPath + "\": " + getHandlerDescription(mappedHandler));
+				}
+				this.handlerMap.remove(urlPath);
+				if (getPatternParser() != null) {
+					this.pathPatternHandlerMap.remove(getPatternParser().parse(urlPath));
+				}
+			}
+		}
+	}
+
+	private String getHandlerDescription(Object handler) {
+		return (handler instanceof String ? "'" + handler + "'" : handler.toString());
+	}
+
+
 	/**
 	 * Look up a handler for the URL path of the given request.
 	 * @param request current HTTP request
@@ -388,78 +498,6 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping i
 		return null;
 	}
 
-	/**
-	 * Register the specified handler for the given URL paths.
-	 * @param urlPaths the URLs that the bean should be mapped to
-	 * @param beanName the name of the handler bean
-	 * @throws BeansException if the handler couldn't be registered
-	 * @throws IllegalStateException if there is a conflicting handler registered
-	 */
-	protected void registerHandler(String[] urlPaths, String beanName) throws BeansException, IllegalStateException {
-		Assert.notNull(urlPaths, "URL path array must not be null");
-		for (String urlPath : urlPaths) {
-			registerHandler(urlPath, beanName);
-		}
-	}
-
-	/**
-	 * Register the specified handler for the given URL path.
-	 * @param urlPath the URL the bean should be mapped to
-	 * @param handler the handler instance or handler bean name String
-	 * (a bean name will automatically be resolved into the corresponding handler bean)
-	 * @throws BeansException if the handler couldn't be registered
-	 * @throws IllegalStateException if there is a conflicting handler registered
-	 */
-	protected void registerHandler(String urlPath, Object handler) throws BeansException, IllegalStateException {
-		Assert.notNull(urlPath, "URL path must not be null");
-		Assert.notNull(handler, "Handler object must not be null");
-		Object resolvedHandler = handler;
-
-		// Eagerly resolve handler if referencing singleton via name.
-		if (!this.lazyInitHandlers && handler instanceof String handlerName) {
-			ApplicationContext applicationContext = obtainApplicationContext();
-			if (applicationContext.isSingleton(handlerName)) {
-				resolvedHandler = applicationContext.getBean(handlerName);
-			}
-		}
-
-		Object mappedHandler = this.handlerMap.get(urlPath);
-		if (mappedHandler != null) {
-			if (mappedHandler != resolvedHandler) {
-				throw new IllegalStateException(
-						"Cannot map " + getHandlerDescription(handler) + " to URL path [" + urlPath +
-						"]: There is already " + getHandlerDescription(mappedHandler) + " mapped.");
-			}
-		}
-		else {
-			if (urlPath.equals("/")) {
-				if (logger.isTraceEnabled()) {
-					logger.trace("Root mapping to " + getHandlerDescription(handler));
-				}
-				setRootHandler(resolvedHandler);
-			}
-			else if (urlPath.equals("/*")) {
-				if (logger.isTraceEnabled()) {
-					logger.trace("Default mapping to " + getHandlerDescription(handler));
-				}
-				setDefaultHandler(resolvedHandler);
-			}
-			else {
-				this.handlerMap.put(urlPath, resolvedHandler);
-				if (getPatternParser() != null) {
-					this.pathPatternHandlerMap.put(getPatternParser().parse(urlPath), resolvedHandler);
-				}
-				if (logger.isTraceEnabled()) {
-					logger.trace("Mapped [" + urlPath + "] onto " + getHandlerDescription(handler));
-				}
-			}
-		}
-	}
-
-	private String getHandlerDescription(Object handler) {
-		return (handler instanceof String ? "'" + handler + "'" : handler.toString());
-	}
-
 
 	/**
 	 * Return the handler mappings as a read-only Map, with the registered path

spring-webmvc/src/test/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMappingTests.java

@@ -0,0 +1,143 @@
+/*
+ * Copyright 2002-2024 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.web.servlet.handler;
+
+import java.util.Map;
+import java.util.function.Consumer;
+
+import org.junit.jupiter.api.Test;
+
+import org.springframework.context.support.StaticApplicationContext;
+import org.springframework.lang.Nullable;
+
+import static java.util.Map.entry;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+
+/**
+ * Tests for {@link AbstractUrlHandlerMapping}.
+ *
+ * @author Stephane Nicoll
+ */
+class AbstractUrlHandlerMappingTests {
+
+	private final AbstractUrlHandlerMapping mapping = new AbstractUrlHandlerMapping() {};
+
+	@Test
+	void registerRootHandler() {
+		TestController rootHandler = new TestController();
+		mapping.registerHandler("/", rootHandler);
+		assertThat(mapping).satisfies(hasMappings(rootHandler, null, Map.of()));
+	}
+
+	@Test
+	void registerDefaultHandler() {
+		TestController defaultHandler = new TestController();
+		mapping.registerHandler("/*", defaultHandler);
+		assertThat(mapping).satisfies(hasMappings(null, defaultHandler, Map.of()));
+	}
+
+	@Test
+	void registerSpecificMapping() {
+		TestController testHandler = new TestController();
+		mapping.registerHandler("/test", testHandler);
+		assertThat(mapping).satisfies(hasMappings(null, null, Map.of("/test", testHandler)));
+	}
+
+	@Test
+	void registerSpecificMappingWithBeanName() {
+		StaticApplicationContext context = new StaticApplicationContext();
+		context.registerSingleton("controller", TestController.class);
+		mapping.setApplicationContext(context);
+		mapping.registerHandler("/test", "controller");
+		assertThat(mapping.getHandlerMap().get("/test")).isSameAs(context.getBean("controller"));
+	}
+
+	@Test
+	void unregisterRootHandler() {
+		TestController rootHandler = new TestController();
+		TestController defaultHandler = new TestController();
+		TestController specificHandler = new TestController();
+		mapping.registerHandler("/", rootHandler);
+		mapping.registerHandler("/*", defaultHandler);
+		mapping.registerHandler("/test", specificHandler);
+		assertThat(mapping).satisfies(hasMappings(rootHandler, defaultHandler, Map.of("/test", specificHandler)));
+
+		mapping.unregisterHandler("/");
+		assertThat(mapping).satisfies(hasMappings(null, defaultHandler, Map.of("/test", specificHandler)));
+	}
+
+	@Test
+	void unregisterDefaultHandler() {
+		TestController rootHandler = new TestController();
+		TestController defaultHandler = new TestController();
+		TestController specificHandler = new TestController();
+		mapping.registerHandler("/", rootHandler);
+		mapping.registerHandler("/*", defaultHandler);
+		mapping.registerHandler("/test", specificHandler);
+		assertThat(mapping).satisfies(hasMappings(rootHandler, defaultHandler, Map.of("/test", specificHandler)));
+
+		mapping.unregisterHandler("/*");
+		assertThat(mapping).satisfies(hasMappings(rootHandler, null, Map.of("/test", specificHandler)));
+	}
+
+	@Test
+	void unregisterSpecificHandler() {
+		TestController rootHandler = new TestController();
+		TestController defaultHandler = new TestController();
+		TestController specificHandler = new TestController();
+		mapping.registerHandler("/", rootHandler);
+		mapping.registerHandler("/*", defaultHandler);
+		mapping.registerHandler("/test", specificHandler);
+		assertThat(mapping).satisfies(hasMappings(rootHandler, defaultHandler, Map.of("/test", specificHandler)));
+
+		mapping.unregisterHandler("/test");
+		assertThat(mapping).satisfies(hasMappings(rootHandler, defaultHandler, Map.of()));
+	}
+
+	@Test
+	void unregisterUnsetRootHandler() {
+		assertThatNoException().isThrownBy(() -> mapping.unregisterHandler("/"));
+	}
+
+	@Test
+	void unregisterUnsetDefaultHandler() {
+		assertThatNoException().isThrownBy(() -> mapping.unregisterHandler("/*"));
+	}
+
+	@Test
+	void unregisterUnknownHandler() {
+		TestController specificHandler = new TestController();
+		mapping.registerHandler("/test", specificHandler);
+
+		mapping.unregisterHandler("/test/*");
+		assertThat(mapping.getHandlerMap()).containsExactly(entry("/test", specificHandler));
+	}
+
+
+	private Consumer<AbstractUrlHandlerMapping> hasMappings(@Nullable Object rootHandler,
+			@Nullable Object defaultHandler, Map<String, Object> handlerMap) {
+		return actual -> {
+			assertThat(actual.getRootHandler()).isEqualTo(rootHandler);
+			assertThat(actual.getDefaultHandler()).isEqualTo(defaultHandler);
+			assertThat(actual.getHandlerMap()).containsExactlyEntriesOf(handlerMap);
+		};
+	}
+
+	private static class TestController {}
+
+}