root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

AbstractHandlerMethodMapping refactoring 

Remove convenience Map that is to avoid. The only downside is that
getHandlerMethods requires a transformation but that should not be used frequently.

See gh-22961
This commit is contained in:
Rossen Stoyanchev 2020-07-09 12:11:15 +03:00
parent 0584c289ab
commit 4d7418841c
3 changed files with 31 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java
View File

@ -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<T> extends AbstractHandlerMap
public Map<T, HandlerMethod> 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<T> 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<Match> comparator = new MatchComparator(getMappingComparator(exchange));
@ -344,7 +345,7 @@ public abstract class AbstractHandlerMethodMapping<T> 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<T> 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<T> extends AbstractHandlerMap
private final Map<T, MappingRegistration<T>> registry = new HashMap<>();
private final Map<T, HandlerMethod> mappingLookup = new LinkedHashMap<>();
private final MultiValueMap<String, T> pathLookup = new LinkedMultiValueMap<>();
private final Map<HandlerMethod, CorsConfiguration> corsLookup = new ConcurrentHashMap<>();
@ -465,11 +465,11 @@ public abstract class AbstractHandlerMethodMapping<T> 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<T, HandlerMethod> getMappings() {
return this.mappingLookup;
public Map<T, MappingRegistration<T>> getRegistrations() {
return this.registry;
}
/**
@ -511,7 +511,6 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
try {
HandlerMethod handlerMethod = createHandlerMethod(handler, method);
validateMethodMapping(handlerMethod, mapping);
this.mappingLookup.put(mapping, handlerMethod);
Set<String> directPaths = AbstractHandlerMethodMapping.this.getDirectPaths(mapping);
for (String path : directPaths) {
@ -532,8 +531,8 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
}
private void validateMethodMapping(HandlerMethod handlerMethod, T mapping) {
// Assert that the supplied mapping is unique.
HandlerMethod existingHandlerMethod = this.mappingLookup.get(mapping);
MappingRegistration<T> 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<T> extends AbstractHandlerMap
return;
}
this.mappingLookup.remove(registration.getMapping());
for (String path : registration.getDirectPaths()) {
List<T> mappings = this.pathLookup.get(path);
if (mappings != null) {
@ -571,7 +568,7 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
}
private static class MappingRegistration<T> {
static class MappingRegistration<T> {
private final T mapping;

8
spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java
View File

@ -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);
}

31
spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java
View File

@ -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<T> extends AbstractHandlerMap
public Map<T, HandlerMethod> 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<T> 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<T> 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<T> 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<T> extends AbstractHandlerMap
private final Map<T, MappingRegistration<T>> registry = new HashMap<>();
private final Map<T, HandlerMethod> mappingLookup = new LinkedHashMap<>();
private final MultiValueMap<String, T> pathLookup = new LinkedMultiValueMap<>();
private final Map<String, List<HandlerMethod>> nameLookup = new ConcurrentHashMap<>();
@ -559,11 +559,11 @@ public abstract class AbstractHandlerMethodMapping<T> 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<T, HandlerMethod> getMappings() {
return this.mappingLookup;
public Map<T, MappingRegistration<T>> getRegistrations() {
return this.registry;
}
/**
@ -617,7 +617,6 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
try {
HandlerMethod handlerMethod = createHandlerMethod(handler, method);
validateMethodMapping(handlerMethod, mapping);
this.mappingLookup.put(mapping, handlerMethod);
Set<String> directPaths = AbstractHandlerMethodMapping.this.getDirectPaths(mapping);
for (String path : directPaths) {
@ -644,8 +643,8 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
}
private void validateMethodMapping(HandlerMethod handlerMethod, T mapping) {
// Assert that the supplied mapping is unique.
HandlerMethod existingHandlerMethod = this.mappingLookup.get(mapping);
MappingRegistration<T> 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<T> extends AbstractHandlerMap
return;
}
this.mappingLookup.remove(registration.getMapping());
for (String path : registration.getDirectPaths()) {
List<T> mappings = this.pathLookup.get(path);
if (mappings != null) {
@ -726,7 +723,7 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
}
private static class MappingRegistration<T> {
static class MappingRegistration<T> {
private final T mapping;