diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java index f0f705aab3..3523361949 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java @@ -23,7 +23,6 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -107,7 +106,9 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap public Map getHandlerMethods() { this.mappingRegistry.acquireReadLock(); try { - return Collections.unmodifiableMap(this.mappingRegistry.getMappings()); + return Collections.unmodifiableMap( + this.mappingRegistry.getRegistrations().entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().handlerMethod))); } finally { this.mappingRegistry.releaseReadLock(); @@ -318,7 +319,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap addMatchingMappings(directPathMatches, matches, exchange); } if (matches.isEmpty()) { - addMatchingMappings(this.mappingRegistry.getMappings().keySet(), matches, exchange); + addMatchingMappings(this.mappingRegistry.getRegistrations().keySet(), matches, exchange); } if (!matches.isEmpty()) { Comparator comparator = new MatchComparator(getMappingComparator(exchange)); @@ -344,7 +345,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap return bestMatch.handlerMethod; } else { - return handleNoMatch(this.mappingRegistry.getMappings().keySet(), exchange); + return handleNoMatch(this.mappingRegistry.getRegistrations().keySet(), exchange); } } @@ -352,7 +353,8 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap for (T mapping : mappings) { T match = getMatchingMapping(mapping, exchange); if (match != null) { - matches.add(new Match(match, this.mappingRegistry.getMappings().get(mapping))); + matches.add(new Match(match, + this.mappingRegistry.getRegistrations().get(mapping).getHandlerMethod())); } } } @@ -456,8 +458,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap private final Map> registry = new HashMap<>(); - private final Map mappingLookup = new LinkedHashMap<>(); - private final MultiValueMap pathLookup = new LinkedMultiValueMap<>(); private final Map corsLookup = new ConcurrentHashMap<>(); @@ -465,11 +465,11 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); /** - * Return all mappings and handler methods. Not thread-safe. - * @see #acquireReadLock() + * Return all registrations. + * @since 5.3 */ - public Map getMappings() { - return this.mappingLookup; + public Map> getRegistrations() { + return this.registry; } /** @@ -511,7 +511,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap try { HandlerMethod handlerMethod = createHandlerMethod(handler, method); validateMethodMapping(handlerMethod, mapping); - this.mappingLookup.put(mapping, handlerMethod); Set directPaths = AbstractHandlerMethodMapping.this.getDirectPaths(mapping); for (String path : directPaths) { @@ -532,8 +531,8 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } private void validateMethodMapping(HandlerMethod handlerMethod, T mapping) { - // Assert that the supplied mapping is unique. - HandlerMethod existingHandlerMethod = this.mappingLookup.get(mapping); + MappingRegistration registration = this.registry.get(mapping); + HandlerMethod existingHandlerMethod = (registration != null ? registration.getHandlerMethod() : null); if (existingHandlerMethod != null && !existingHandlerMethod.equals(handlerMethod)) { throw new IllegalStateException( "Ambiguous mapping. Cannot map '" + handlerMethod.getBean() + "' method \n" + @@ -550,8 +549,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap return; } - this.mappingLookup.remove(registration.getMapping()); - for (String path : registration.getDirectPaths()) { List mappings = this.pathLookup.get(path); if (mappings != null) { @@ -571,7 +568,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } - private static class MappingRegistration { + static class MappingRegistration { private final T mapping; diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java index 17cc8aa93c..59e9d192c3 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java @@ -112,8 +112,7 @@ public class HandlerMethodMappingTests { this.mapping.registerMapping(key1, this.handler, this.method1); this.mapping.registerMapping(key2, this.handler, this.method2); - assertThat(this.mapping.getMappingRegistry().getMappings()) - .containsKeys(key1, key2); + assertThat(this.mapping.getMappingRegistry().getRegistrations()).containsKeys(key1, key2); } @Test @@ -125,8 +124,7 @@ public class HandlerMethodMappingTests { this.mapping.registerMapping(key1, handler1, this.method1); this.mapping.registerMapping(key2, handler2, this.method1); - assertThat(this.mapping.getMappingRegistry().getMappings()) - .containsKeys(key1, key2); + assertThat(this.mapping.getMappingRegistry().getRegistrations()).containsKeys(key1, key2); } @Test @@ -141,7 +139,7 @@ public class HandlerMethodMappingTests { result = this.mapping.getHandler(MockServerWebExchange.from(MockServerHttpRequest.get(key))); assertThat(result.block()).isNull(); - assertThat(this.mapping.getMappingRegistry().getMappings().keySet()).doesNotContain(key); + assertThat(this.mapping.getMappingRegistry().getRegistrations().keySet()).doesNotContain(key); } 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 03df143c94..b88c11c718 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 @@ -24,7 +24,6 @@ import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -139,7 +138,9 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap public Map getHandlerMethods() { this.mappingRegistry.acquireReadLock(); try { - return Collections.unmodifiableMap(this.mappingRegistry.getMappings()); + return Collections.unmodifiableMap( + this.mappingRegistry.getRegistrations().entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().handlerMethod))); } finally { this.mappingRegistry.releaseReadLock(); @@ -386,7 +387,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap addMatchingMappings(directPathMatches, matches, request); } if (matches.isEmpty()) { - addMatchingMappings(this.mappingRegistry.getMappings().keySet(), matches, request); + addMatchingMappings(this.mappingRegistry.getRegistrations().keySet(), matches, request); } if (!matches.isEmpty()) { Match bestMatch = matches.get(0); @@ -414,7 +415,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap return bestMatch.handlerMethod; } else { - return handleNoMatch(this.mappingRegistry.getMappings().keySet(), lookupPath, request); + return handleNoMatch(this.mappingRegistry.getRegistrations().keySet(), lookupPath, request); } } @@ -422,7 +423,8 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap for (T mapping : mappings) { T match = getMatchingMapping(mapping, request); if (match != null) { - matches.add(new Match(match, this.mappingRegistry.getMappings().get(mapping))); + matches.add(new Match(match, + this.mappingRegistry.getRegistrations().get(mapping).getHandlerMethod())); } } } @@ -548,8 +550,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap private final Map> registry = new HashMap<>(); - private final Map mappingLookup = new LinkedHashMap<>(); - private final MultiValueMap pathLookup = new LinkedMultiValueMap<>(); private final Map> nameLookup = new ConcurrentHashMap<>(); @@ -559,11 +559,11 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); /** - * Return all mappings and handler methods. Not thread-safe. - * @see #acquireReadLock() + * Return all registrations. + * @since 5.3 */ - public Map getMappings() { - return this.mappingLookup; + public Map> getRegistrations() { + return this.registry; } /** @@ -617,7 +617,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap try { HandlerMethod handlerMethod = createHandlerMethod(handler, method); validateMethodMapping(handlerMethod, mapping); - this.mappingLookup.put(mapping, handlerMethod); Set directPaths = AbstractHandlerMethodMapping.this.getDirectPaths(mapping); for (String path : directPaths) { @@ -644,8 +643,8 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } private void validateMethodMapping(HandlerMethod handlerMethod, T mapping) { - // Assert that the supplied mapping is unique. - HandlerMethod existingHandlerMethod = this.mappingLookup.get(mapping); + MappingRegistration registration = this.registry.get(mapping); + HandlerMethod existingHandlerMethod = (registration != null ? registration.getHandlerMethod() : null); if (existingHandlerMethod != null && !existingHandlerMethod.equals(handlerMethod)) { throw new IllegalStateException( "Ambiguous mapping. Cannot map '" + handlerMethod.getBean() + "' method \n" + @@ -680,8 +679,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap return; } - this.mappingLookup.remove(registration.getMapping()); - for (String path : registration.getDirectPaths()) { List mappings = this.pathLookup.get(path); if (mappings != null) { @@ -726,7 +723,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } - private static class MappingRegistration { + static class MappingRegistration { private final T mapping;