From 42d0470d94362041c09fdafe68eb4d1737557e48 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 6 Feb 2014 16:46:28 -0500 Subject: [PATCH] Improve expanding in MvcUriComponentsBuilder Before this change MvcUriComponentsBuilder could not create a UriComponentsBuilder for methods where the mapping has a URI variable and no matching method argument for it. For example a URI variable may be in the type-level mapping but not all methods may have an @PathVariable argument for it. This fix addresses the shortcoming such that MvcUriComponentsBuilder expands the method argument values available to it and leaves remaining URI variables to be further expanded via UriComponents.expand(). Issue: SPR-11391 --- .../web/util/UriComponents.java | 15 +++++++++++- .../web/util/UriComponentsBuilder.java | 11 +++++++-- .../annotation/MvcUriComponentsBuilder.java | 23 ++++++++++++------- .../MvcUriComponentsBuilderTests.java | 19 +++++++++++++++ 4 files changed, 57 insertions(+), 11 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/UriComponents.java b/spring-web/src/main/java/org/springframework/web/util/UriComponents.java index f14d78f96d9..d9edd00658b 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UriComponents.java +++ b/spring-web/src/main/java/org/springframework/web/util/UriComponents.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -218,6 +218,9 @@ public abstract class UriComponents implements Serializable { String match = matcher.group(1); String variableName = getVariableName(match); Object variableValue = uriVariables.getValue(variableName); + if (UriTemplateVariables.SKIP_VALUE.equals(variableValue)) { + continue; + } String variableValueString = getVariableValueAsString(variableValue); String replacement = Matcher.quoteReplacement(variableValueString); matcher.appendReplacement(sb, replacement); @@ -242,6 +245,16 @@ public abstract class UriComponents implements Serializable { */ public interface UriTemplateVariables { + public static final Object SKIP_VALUE = UriTemplateVariables.class; + + /** + * Get the value for the given URI variable name. + * If the value is {@code null}, an empty String is expanded. + * If the value is {@link #SKIP_VALUE}, the URI variable is not expanded. + * + * @param name the variable name + * @return the variable value, possibly {@code null} or {@link #SKIP_VALUE} + */ Object getValue(String name); } diff --git a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java index a5ca916d6c8..1685db1b664 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java @@ -363,7 +363,7 @@ public class UriComponentsBuilder { } /** - * Initializes all components of this URI builder from the given {@link UriComponents}. + * Set all components of this URI builder from the given {@link UriComponents}. * @param uriComponents the UriComponents instance * @return this UriComponentsBuilder */ @@ -385,7 +385,14 @@ public class UriComponentsBuilder { this.port = uriComponents.getPort(); } if (StringUtils.hasLength(uriComponents.getPath())) { - this.pathBuilder = new CompositePathComponentBuilder(uriComponents.getPath()); + List segments = uriComponents.getPathSegments(); + if (segments.isEmpty()) { + // Perhaps "/" + this.pathBuilder.addPath(uriComponents.getPath()); + } + else { + this.pathBuilder.addPathSegments(segments.toArray(new String[segments.size()])); + } } if (!uriComponents.getQueryParams().isEmpty()) { this.queryParams.clear(); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java index a5a8c11cd77..e9a0b9be6d9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java @@ -209,15 +209,14 @@ public class MvcUriComponentsBuilder extends UriComponentsBuilder { */ public static UriComponentsBuilder fromMethod(Method method, Object... argumentValues) { - UriComponentsBuilder builder = ServletUriComponentsBuilder.newInstance().path(getMethodRequestMapping(method)); - UriComponents uriComponents = applyContributors(builder, method, argumentValues); - String typePath = getTypeRequestMapping(method.getDeclaringClass()); - String methodPath = uriComponents.getPath(); + String methodPath = getMethodRequestMapping(method); String path = pathMatcher.combine(typePath, methodPath); - return ServletUriComponentsBuilder.fromCurrentServletMapping().path(path) - .queryParams(uriComponents.getQueryParams()); + UriComponentsBuilder builder = ServletUriComponentsBuilder.fromCurrentServletMapping().path(path); + UriComponents uriComponents = applyContributors(builder, method, argumentValues); + + return ServletUriComponentsBuilder.newInstance().uriComponents(uriComponents); } private static String getMethodRequestMapping(Method method) { @@ -246,13 +245,21 @@ public class MvcUriComponentsBuilder extends UriComponentsBuilder { Assert.isTrue(paramCount == argCount, "Number of method parameters " + paramCount + " does not match number of argument values " + argCount); - Map uriVars = new HashMap(); + final Map uriVars = new HashMap(); for (int i=0; i < paramCount; i++) { MethodParameter param = new MethodParameter(method, i); param.initParameterNameDiscovery(parameterNameDiscoverer); contributor.contributeMethodArgument(param, args[i], builder, uriVars); } - return builder.buildAndExpand(uriVars); + + // We may not have all URI var values, expand only what we have + + return builder.build().expand(new UriComponents.UriTemplateVariables() { + @Override + public Object getValue(String name) { + return uriVars.containsKey(name) ? uriVars.get(name) : UriComponents.UriTemplateVariables.SKIP_VALUE; + } + }); } protected static CompositeUriComponentsContributor getConfiguredUriComponentsContributor() { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java index a4708a6acb9..35b2a79c179 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java @@ -34,6 +34,7 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.context.request.RequestContextHolder; import org.springframework.web.context.request.ServletRequestAttributes; import org.springframework.web.util.UriComponents; +import org.springframework.web.util.UriComponentsBuilder; import java.util.Arrays; import java.util.List; @@ -134,6 +135,15 @@ public class MvcUriComponentsBuilderTests { assertThat(queryParams.get("offset"), contains("10")); } + // SPR-11391 + + @Test + public void testFromMethodNameTypeLevelPathVariableWithoutArgumentValue() throws Exception { + UriComponents uriComponents = fromMethodName(UserContactController.class, "showCreate", 123).build(); + + assertThat(uriComponents.getPath(), is("/user/123/contacts/create")); + } + @Test public void testFromMethodNameNotMapped() throws Exception { UriComponents uriComponents = fromMethodName(UnmappedController.class, "unmappedMethod").build(); @@ -290,4 +300,13 @@ public class MvcUriComponentsBuilderTests { } } + @RequestMapping("/user/{userId}/contacts") + static class UserContactController { + + @RequestMapping("/create") + public String showCreate(@PathVariable Integer userId) { + return null; + } + } + }