Browse Source

Use LinkedHashmap to preserve insert order

In several places in the spring-webmvc module, URL patterns / objects
relationships are kept in `HashMap`s. When matching with actual URLs,
the algorithm uses a pattern comparator to sort the matching patterns
and select the most specific. But the underlying collection
implementation does not keep the original order which can lead to
inconsistencies.

This commit changes the underlying collection implementation to
`LinkedHashmap`s, in order to keep the insert order if the comparator
does not reorder entries.

Issue: SPR-13798
(cherry picked from commit 3be35c0)
pull/941/head
Brian Clozel 10 years ago committed by Juergen Hoeller
parent
commit
46016d090b
  1. 15
      spring-webmvc/src/main/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMapping.java
  2. 23
      spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java
  3. 32
      spring-webmvc/src/main/java/org/springframework/web/servlet/resource/VersionResourceResolver.java

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

@ -1,5 +1,5 @@ @@ -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,7 +16,7 @@ @@ -16,7 +16,7 @@
package org.springframework.web.servlet.handler;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Properties;
@ -41,11 +41,10 @@ import org.springframework.util.CollectionUtils; @@ -41,11 +41,10 @@ import org.springframework.util.CollectionUtils;
* If the path doesn't begin with a slash, one is prepended.
*
* <p>Supports direct matches (given "/test" -> registered "/test") and "*"
* matches (given "/test" -> registered "/t*"). Note that the default is
* to map within the current servlet mapping if applicable; see the
* {@link #setAlwaysUseFullPath "alwaysUseFullPath"} property for details.
* For details on the pattern options, see the
* {@link org.springframework.util.AntPathMatcher} javadoc.
* pattern matches (given "/test" -> registered "/t*"). Note that the default
* is to map within the current servlet mapping if applicable; see the
* {@link #setAlwaysUseFullPath "alwaysUseFullPath"} property. For details on the
* pattern options, see the {@link org.springframework.util.AntPathMatcher} javadoc.
* @author Rod Johnson
* @author Juergen Hoeller
@ -55,7 +54,7 @@ import org.springframework.util.CollectionUtils; @@ -55,7 +54,7 @@ import org.springframework.util.CollectionUtils;
*/
public class SimpleUrlHandlerMapping extends AbstractUrlHandlerMapping {
private final Map<String, Object> urlMap = new HashMap<String, Object>();
private final Map<String, Object> urlMap = new LinkedHashMap<String, Object>();
/**

23
spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java

@ -19,7 +19,7 @@ package org.springframework.web.servlet.resource; @@ -19,7 +19,7 @@ package org.springframework.web.servlet.resource;
import java.util.ArrayList;
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 javax.servlet.http.HttpServletRequest;
@ -55,7 +55,7 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed @@ -55,7 +55,7 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed
private PathMatcher pathMatcher = new AntPathMatcher();
private final Map<String, ResourceHttpRequestHandler> handlerMap = new HashMap<String, ResourceHttpRequestHandler>();
private final Map<String, ResourceHttpRequestHandler> handlerMap = new LinkedHashMap<String, ResourceHttpRequestHandler>();
private boolean autodetect = true;
@ -165,17 +165,17 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed @@ -165,17 +165,17 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed
* URL path to expose for public use.
* @param request the current request
* @param requestUrl the request URL path to resolve
* @return the resolved public URL path or {@code null} if unresolved
* @return the resolved public URL path, or {@code null} if unresolved
*/
public final String getForRequestUrl(HttpServletRequest request, String requestUrl) {
if (logger.isTraceEnabled()) {
logger.trace("Getting resource URL for requestURL=" + requestUrl);
logger.trace("Getting resource URL for request URL \"" + requestUrl + "\"");
}
int index = getLookupPathIndex(request);
String prefix = requestUrl.substring(0, index);
String lookupPath = requestUrl.substring(index);
String resolvedLookupPath = getForLookupPath(lookupPath);
return (resolvedLookupPath != null) ? prefix + resolvedLookupPath : null;
return (resolvedLookupPath != null ? prefix + resolvedLookupPath : null);
}
private int getLookupPathIndex(HttpServletRequest request) {
@ -198,14 +198,16 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed @@ -198,14 +198,16 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed
*/
public final String getForLookupPath(String lookupPath) {
if (logger.isTraceEnabled()) {
logger.trace("Getting resource URL for lookupPath=" + lookupPath);
logger.trace("Getting resource URL for lookup path \"" + lookupPath + "\"");
}
List<String> matchingPatterns = new ArrayList<String>();
for (String pattern : this.handlerMap.keySet()) {
if (getPathMatcher().match(pattern, lookupPath)) {
matchingPatterns.add(pattern);
}
}
if (!matchingPatterns.isEmpty()) {
Comparator<String> patternComparator = getPathMatcher().getPatternComparator(lookupPath);
Collections.sort(matchingPatterns, patternComparator);
@ -213,7 +215,7 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed @@ -213,7 +215,7 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed
String pathWithinMapping = getPathMatcher().extractPathWithinPattern(pattern, lookupPath);
String pathMapping = lookupPath.substring(0, lookupPath.indexOf(pathWithinMapping));
if (logger.isTraceEnabled()) {
logger.trace("Invoking ResourceResolverChain for URL pattern=\"" + pattern + "\"");
logger.trace("Invoking ResourceResolverChain for URL pattern \"" + pattern + "\"");
}
ResourceHttpRequestHandler handler = this.handlerMap.get(pattern);
ResourceResolverChain chain = new DefaultResourceResolverChain(handler.getResourceResolvers());
@ -222,12 +224,15 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed @@ -222,12 +224,15 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed
continue;
}
if (logger.isTraceEnabled()) {
logger.trace("Resolved public resource URL path=\"" + resolved + "\"");
logger.trace("Resolved public resource URL path \"" + resolved + "\"");
}
return pathMapping + resolved;
}
}
logger.debug("No matching resource mapping");
if (logger.isDebugEnabled()) {
logger.debug("No matching resource mapping for lookup path \"" + lookupPath + "\"");
}
return null;
}

32
spring-webmvc/src/main/java/org/springframework/web/servlet/resource/VersionResourceResolver.java

@ -19,7 +19,7 @@ package org.springframework.web.servlet.resource; @@ -19,7 +19,7 @@ package org.springframework.web.servlet.resource;
import java.util.ArrayList;
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 javax.servlet.http.HttpServletRequest;
@ -58,7 +58,7 @@ public class VersionResourceResolver extends AbstractResourceResolver { @@ -58,7 +58,7 @@ public class VersionResourceResolver extends AbstractResourceResolver {
private AntPathMatcher pathMatcher = new AntPathMatcher();
/** Map from path pattern -> VersionStrategy */
private final Map<String, VersionStrategy> versionStrategyMap = new HashMap<String, VersionStrategy>();
private final Map<String, VersionStrategy> versionStrategyMap = new LinkedHashMap<String, VersionStrategy>();
/**
@ -146,14 +146,14 @@ public class VersionResourceResolver extends AbstractResourceResolver { @@ -146,14 +146,14 @@ public class VersionResourceResolver extends AbstractResourceResolver {
String candidateVersion = versionStrategy.extractVersion(requestPath);
if (StringUtils.isEmpty(candidateVersion)) {
if (logger.isTraceEnabled()) {
logger.trace("No version found in path=\"" + requestPath + "\"");
logger.trace("No version found in path \"" + requestPath + "\"");
}
return null;
}
String simplePath = versionStrategy.removeVersion(requestPath, candidateVersion);
if (logger.isTraceEnabled()) {
logger.trace("Extracted version from path, re-resolving without version, path=\"" + simplePath + "\"");
logger.trace("Extracted version from path, re-resolving without version: \"" + simplePath + "\"");
}
Resource baseResource = chain.resolveResource(request, simplePath, locations);
@ -164,14 +164,14 @@ public class VersionResourceResolver extends AbstractResourceResolver { @@ -164,14 +164,14 @@ public class VersionResourceResolver extends AbstractResourceResolver {
String actualVersion = versionStrategy.getResourceVersion(baseResource);
if (candidateVersion.equals(actualVersion)) {
if (logger.isTraceEnabled()) {
logger.trace("resource matches extracted version");
logger.trace("Resource matches extracted version ["+ candidateVersion + "]");
}
return baseResource;
}
else {
if (logger.isTraceEnabled()) {
logger.trace("Potential resource found for [" + requestPath + "], but version [" +
candidateVersion + "] doesn't match.");
logger.trace("Potential resource found for \"" + requestPath + "\", but version [" +
candidateVersion + "] does not match");
}
return null;
}
@ -186,12 +186,12 @@ public class VersionResourceResolver extends AbstractResourceResolver { @@ -186,12 +186,12 @@ public class VersionResourceResolver extends AbstractResourceResolver {
return null;
}
if (logger.isTraceEnabled()) {
logger.trace("Getting the original resource to determine version");
logger.trace("Getting the original resource to determine version for path \"" + resourceUrlPath + "\"");
}
Resource resource = chain.resolveResource(null, baseUrl, locations);
String version = versionStrategy.getResourceVersion(resource);
if (logger.isTraceEnabled()) {
logger.trace("Version=" + version);
logger.trace("Determined version [" + version + "] for " + resource);
}
return versionStrategy.addVersion(baseUrl, version);
}
@ -204,17 +204,17 @@ public class VersionResourceResolver extends AbstractResourceResolver { @@ -204,17 +204,17 @@ public class VersionResourceResolver extends AbstractResourceResolver {
*/
protected VersionStrategy getStrategyForPath(String requestPath) {
String path = "/".concat(requestPath);
List<String> matchingPatterns = new ArrayList<String>();
List<String> matchingPatterns = new ArrayList<String>();
for (String pattern : this.versionStrategyMap.keySet()) {
if (this.pathMatcher.match(pattern, path)) {
matchingPatterns.add(pattern);
matchingPatterns.add(pattern);
}
}
if (!matchingPatterns.isEmpty()) {
Comparator<String> comparator = this.pathMatcher.getPatternComparator(path);
Collections.sort(matchingPatterns, comparator);
return this.versionStrategyMap.get(matchingPatterns.get(0));
}
if (!matchingPatterns.isEmpty()) {
Comparator<String> comparator = this.pathMatcher.getPatternComparator(path);
Collections.sort(matchingPatterns, comparator);
return this.versionStrategyMap.get(matchingPatterns.get(0));
}
return null;
}

Loading…
Cancel
Save