From 8f558e7c73fd885676a3e9bfe77c9c24da59f30a Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Sun, 3 May 2015 12:45:47 -0400 Subject: [PATCH] Add methods to (un)register HandlerMethod mappings Issue: SPR-11541 --- .../handler/AbstractHandlerMethodMapping.java | 252 +++++++++++++----- .../handler/HandlerMethodMappingTests.java | 23 +- 2 files changed, 196 insertions(+), 79 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java index 727f989c08..4050407f9d 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.ReentrantReadWriteLock; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -89,7 +90,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap private HandlerMethodMappingNamingStrategy namingStrategy; - private final MappingDefinitionRegistry mappingRegistry = new MappingDefinitionRegistry(); + private final MappingRegistry mappingRegistry = new MappingRegistry(); /** @@ -123,10 +124,16 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } /** - * Return a read-only map with all mapped HandlerMethod's. + * Return a (read-only) map with all mappings and HandlerMethod's. */ public Map getHandlerMethods() { - return this.mappingRegistry.getMappings(); + this.mappingRegistry.acquireReadLock(); + try { + return Collections.unmodifiableMap(this.mappingRegistry.getMappings()); + } + finally { + this.mappingRegistry.releaseReadLock(); + } } /** @@ -221,16 +228,27 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap /** * Register a handler method and its unique mapping. + *

Invoked at startup for each detected handler method. May also be + * invoked at runtime after initialization is complete. * @param handler the bean name of the handler or the handler instance * @param method the method to register * @param mapping the mapping conditions associated with the handler method * @throws IllegalStateException if another method was already registered * under the same mapping */ - protected void registerHandlerMethod(Object handler, Method method, T mapping) { + public void registerHandlerMethod(Object handler, Method method, T mapping) { this.mappingRegistry.register(handler, method, mapping); } + /** + * Un-register a handler method. + *

This method may be invoked at runtime after initialization has completed. + * @param handlerMethod the handler method to be unregistered + */ + public void unregisterHandlerMethod(HandlerMethod handlerMethod) { + this.mappingRegistry.unregister(handlerMethod); + } + /** * Create the HandlerMethod instance. * @param handler either a bean name or an actual handler instance @@ -279,16 +297,22 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap if (logger.isDebugEnabled()) { logger.debug("Looking up handler method for path " + lookupPath); } - HandlerMethod handlerMethod = lookupHandlerMethod(lookupPath, request); - if (logger.isDebugEnabled()) { - if (handlerMethod != null) { - logger.debug("Returning handler method [" + handlerMethod + "]"); - } - else { - logger.debug("Did not find handler method for [" + lookupPath + "]"); + this.mappingRegistry.acquireReadLock(); + try { + HandlerMethod handlerMethod = lookupHandlerMethod(lookupPath, request); + if (logger.isDebugEnabled()) { + if (handlerMethod != null) { + logger.debug("Returning handler method [" + handlerMethod + "]"); + } + else { + logger.debug("Did not find handler method for [" + lookupPath + "]"); + } } + return (handlerMethod != null ? handlerMethod.createWithResolvedBean() : null); + } + finally { + this.mappingRegistry.releaseReadLock(); } - return (handlerMethod != null ? handlerMethod.createWithResolvedBean() : null); } /** @@ -308,7 +332,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } if (matches.isEmpty()) { // No choice but to go through all mappings... - addMatchingMappings(this.mappingRegistry.getMappingKeys(), matches, request); + addMatchingMappings(this.mappingRegistry.getMappings().keySet(), matches, request); } if (!matches.isEmpty()) { @@ -335,7 +359,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap return bestMatch.handlerMethod; } else { - return handleNoMatch(this.mappingRegistry.getMappingKeys(), lookupPath, request); + return handleNoMatch(this.mappingRegistry.getMappings().keySet(), lookupPath, request); } } @@ -404,96 +428,126 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } - private class MappingDefinitionRegistry { - - private final Map> mappingDefinitions = - new ConcurrentHashMap>(); + private class MappingRegistry { private final Map mappingLookup = new LinkedHashMap(); private final MultiValueMap urlLookup = new LinkedMultiValueMap(); - private final Map> nameLookup = + private final Map> mappingNameLookup = new ConcurrentHashMap>(); + private final Map> methodLookup = + new ConcurrentHashMap>(); + + + private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); + /** - * Return a read-only copy of all mappings. - * Safe for concurrent use. + * Return all mappings and handler methods. Not thread-safe. + * @see #acquireReadLock() */ public Map getMappings() { - return Collections.unmodifiableMap(this.mappingLookup); + return this.mappingLookup; } + /** + * Return matches for the given URL path. Not thread-safe. + * @see #acquireReadLock() + */ public List getMappingKeysByUrl(String urlPath) { return this.urlLookup.get(urlPath); } - public Set getMappingKeys() { - return this.mappingLookup.keySet(); - } - + /** + * Return the handler method for the mapping key. Not thread-safe. + * @see #acquireReadLock() + */ public HandlerMethod getHandlerMethod(T mapping) { return this.mappingLookup.get(mapping); } /** - * Return HandlerMethod matches for the given mapping name. - * Safe for concurrent use. + * Return handler methods by mapping name. Thread-safe for concurrent use. */ public List getHandlerMethodsByMappingName(String mappingName) { - return this.nameLookup.get(mappingName); + return this.mappingNameLookup.get(mappingName); } /** - * Return the CorsConfiguration for the given HandlerMethod, if any. - * Safe for concurrent use. + * Return CORS configuration. Thread-safe for concurrent use. */ public CorsConfiguration getCorsConfiguration(HandlerMethod handlerMethod) { Method method = handlerMethod.getMethod(); - MappingDefinition definition = this.mappingDefinitions.get(method); + MappingDefinition definition = this.methodLookup.get(method); return (definition != null ? definition.getCorsConfiguration() : null); } + /** + * Acquire the read lock when using getMappings and getMappingKeysByUrl. + */ + public void acquireReadLock() { + this.readWriteLock.readLock().lock(); + } + + /** + * Release the read lock after using getMappings and getMappingKeysByUrl. + */ + public void releaseReadLock() { + this.readWriteLock.readLock().unlock(); + } + public void register(Object handler, Method method, T mapping) { - HandlerMethod handlerMethod = createHandlerMethod(handler, method); - assertUniqueMapping(handlerMethod, mapping); + this.readWriteLock.writeLock().lock(); + try { + HandlerMethod handlerMethod = createHandlerMethod(handler, method); + assertUniqueMethodMapping(handlerMethod, mapping); - if (logger.isInfoEnabled()) { - logger.info("Mapped \"" + mapping + "\" onto " + handlerMethod); - } - this.mappingLookup.put(mapping, handlerMethod); + if (logger.isInfoEnabled()) { + logger.info("Mapped \"" + mapping + "\" onto " + handlerMethod); + } + this.mappingLookup.put(mapping, handlerMethod); - List directUrls = extractDirectUrls(mapping); - for (String url : directUrls) { - this.urlLookup.add(url, mapping); - } + List directUrls = getDirectUrls(mapping); + for (String url : directUrls) { + this.urlLookup.add(url, mapping); + } - String name = null; - if (getNamingStrategy() != null) { - name = getNamingStrategy().getName(handlerMethod, mapping); - addName(name, handlerMethod); - } + String name = null; + if (getNamingStrategy() != null) { + name = getNamingStrategy().getName(handlerMethod, mapping); + addMappingName(name, handlerMethod); + } - CorsConfiguration corsConfig = initCorsConfiguration(handler, method, mapping); + CorsConfiguration corsConfig = initCorsConfiguration(handler, method, mapping); - this.mappingDefinitions.put(method, - new MappingDefinition(mapping, handlerMethod, directUrls, name, corsConfig)); + this.methodLookup.put(method, + new MappingDefinition(mapping, handlerMethod, directUrls, name, corsConfig)); + } + finally { + this.readWriteLock.writeLock().unlock(); + } } - private void assertUniqueMapping(HandlerMethod handlerMethod, T mapping) { - HandlerMethod existing = this.mappingLookup.get(mapping); - if (existing != null && !existing.equals(handlerMethod)) { + private void assertUniqueMethodMapping(HandlerMethod newHandlerMethod, T mapping) { + HandlerMethod handlerMethod = this.mappingLookup.get(mapping); + if (handlerMethod != null && !handlerMethod.equals(newHandlerMethod)) { throw new IllegalStateException( - "Ambiguous mapping. Cannot map '" + handlerMethod.getBean() + "' method \n" + - handlerMethod + "\nto " + mapping + ": There is already '" + - existing.getBean() + "' bean method\n" + existing + " mapped."); + "Ambiguous mapping. Cannot map '" + newHandlerMethod.getBean() + "' method \n" + + newHandlerMethod + "\nto " + mapping + ": There is already '" + + handlerMethod.getBean() + "' bean method\n" + handlerMethod + " mapped."); + } + MappingDefinition definition = this.methodLookup.get(newHandlerMethod.getMethod()); + if (definition != null) { + throw new IllegalStateException("Cannot map " + newHandlerMethod.getMethod() + + "\nto " + mapping + ".\n It is already mapped to " + definition.getMapping()); } } - private List extractDirectUrls(T mapping) { + private List getDirectUrls(T mapping) { List urls = new ArrayList(1); for (String path : getMappingPathPatterns(mapping)) { if (!getPathMatcher().isPattern(path)) { @@ -503,13 +557,13 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap return urls; } - private void addName(String name, HandlerMethod handlerMethod) { + private void addMappingName(String name, HandlerMethod handlerMethod) { - List oldHandlerMethods = this.nameLookup.containsKey(name) ? - this.nameLookup.get(name) : Collections.emptyList(); + List oldList = this.mappingNameLookup.containsKey(name) ? + this.mappingNameLookup.get(name) : Collections.emptyList(); - for (HandlerMethod oldHandlerMethod : oldHandlerMethods) { - if (oldHandlerMethod.getMethod().equals(handlerMethod.getMethod())) { + for (HandlerMethod current : oldList) { + if (handlerMethod.getMethod().equals(current.getMethod())) { return; } } @@ -518,19 +572,69 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap logger.trace("Mapping name=" + name); } - int size = oldHandlerMethods.size() + 1; - List definitions = new ArrayList(size); - definitions.addAll(oldHandlerMethods); - definitions.add(handlerMethod); - this.nameLookup.put(name, definitions); + List newList = new ArrayList(oldList.size() + 1); + newList.addAll(oldList); + newList.add(handlerMethod); + this.mappingNameLookup.put(name, newList); - if (definitions.size() > 1) { + if (newList.size() > 1) { if (logger.isDebugEnabled()) { - logger.debug("Mapping name clash for handlerMethods=" + definitions + + logger.debug("Mapping name clash for handlerMethods=" + newList + ". Consider assigning explicit names."); } } } + + public void unregister(HandlerMethod handlerMethod) { + this.readWriteLock.writeLock().lock(); + try { + MappingDefinition definition = this.methodLookup.remove(handlerMethod.getMethod()); + if (definition == null) { + return; + } + + this.mappingLookup.remove(definition.getMapping()); + + for (String url : definition.getDirectUrls()) { + List list = this.urlLookup.get(url); + if (list != null) { + list.remove(definition.getMapping()); + if (list.isEmpty()) { + this.urlLookup.remove(url); + } + } + } + + removeMappingName(definition); + } + finally { + this.readWriteLock.writeLock().unlock(); + } + } + + private void removeMappingName(MappingDefinition definition) { + String name = definition.getName(); + if (name == null) { + return; + } + HandlerMethod handlerMethod = definition.getHandlerMethod(); + List oldList = this.mappingNameLookup.get(name); + if (oldList == null) { + return; + } + if (oldList.size() <= 1) { + this.mappingNameLookup.remove(name); + return; + } + List newList = new ArrayList(oldList.size() - 1); + for (HandlerMethod current : oldList) { + if (!current.equals(handlerMethod)) { + newList.add(current); + } + } + this.mappingNameLookup.put(name, newList); + } + } private static class MappingDefinition { @@ -539,14 +643,14 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap private final HandlerMethod handlerMethod; - private final List urls; + private final List directUrls; private final String name; private final CorsConfiguration corsConfiguration; - public MappingDefinition(T mapping, HandlerMethod handlerMethod, List urls, + public MappingDefinition(T mapping, HandlerMethod handlerMethod, List directUrls, String name, CorsConfiguration corsConfiguration) { Assert.notNull(mapping); @@ -554,7 +658,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap this.mapping = mapping; this.handlerMethod = handlerMethod; - this.urls = (urls != null ? urls : Collections.emptyList()); + this.directUrls = (directUrls != null ? directUrls : Collections.emptyList()); this.name = name; this.corsConfiguration = corsConfiguration; } @@ -568,8 +672,8 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap return this.handlerMethod; } - public List getUrls() { - return this.urls; + public List getDirectUrls() { + return this.directUrls; } public String getName() { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java index cf265a4528..61d5b18bd1 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2015 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. @@ -16,10 +16,13 @@ package org.springframework.web.servlet.handler; +import static org.junit.Assert.*; + import java.lang.reflect.Method; import java.util.Comparator; import java.util.HashSet; import java.util.Set; + import javax.servlet.http.HttpServletRequest; import org.junit.Before; @@ -34,8 +37,6 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.method.HandlerMethod; import org.springframework.web.util.UrlPathHelper; -import static org.junit.Assert.*; - /** * Test for {@link AbstractHandlerMethodMapping}. * @@ -77,7 +78,7 @@ public class HandlerMethodMappingTests { @Test public void patternMatch() throws Exception { mapping.registerHandlerMethod(handler, method1, "/fo*"); - mapping.registerHandlerMethod(handler, method1, "/f*"); + mapping.registerHandlerMethod(handler, method2, "/f*"); HandlerMethod result = mapping.getHandlerInternal(new MockHttpServletRequest("GET", "/foo")); assertEquals(method1, result.getMethod()); @@ -92,7 +93,7 @@ public class HandlerMethodMappingTests { } @Test - public void testDetectHandlerMethodsInAncestorContexts() { + public void detectHandlerMethodsInAncestorContexts() { StaticApplicationContext cxt = new StaticApplicationContext(); cxt.registerSingleton("myHandler", MyHandler.class); @@ -110,6 +111,18 @@ public class HandlerMethodMappingTests { assertEquals(2, mapping2.getHandlerMethods().size()); } + @Test + public void unregister() throws Exception { + String key = "foo"; + mapping.registerHandlerMethod(handler, method1, key); + + HandlerMethod handlerMethod = mapping.getHandlerInternal(new MockHttpServletRequest("GET", key)); + assertEquals(method1, handlerMethod.getMethod()); + + mapping.unregisterHandlerMethod(handlerMethod); + assertNull(mapping.getHandlerInternal(new MockHttpServletRequest("GET", key))); + } + private static class MyHandlerMethodMapping extends AbstractHandlerMethodMapping { -- GitLab