From 8c4824245e6df47407209403a8516fd66444c77f Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 5 May 2021 09:37:24 +0200 Subject: [PATCH] #1583 - Full support for level 4 URI templates. Significant rewrite of UriTemplate to bring it up to level for template variables. We now additionally support: * Composite values correctly * Prefix values * Multi-value template variables * Path-style parameters * Label expansion with dot-prefix * Reserved expansion Unit tests have been enriched with all examples given in the corresponding RFC [0]. Template variable types have been aligned with the terminology used in the RFC. Currently differently named types have been deprecated in favor of the new ones. The commit slightly changes the behavior in two different aspects: 1. Query parameter values are now encoded as described in the RFC. Previously, special characters like comma (,) have not been percent encoded but now are. To create comma-separated values, expand an array of values instead of a prepared String. I.e. instead of expanding {?sort} with "foo,asc", expand it with [ "foo", "asc" ]. 2. The aspect of variable optionality has been deprecated as it doesn't actually exist for template variables. This causes expansions that were previously rejected (e.g. ones using {foo} in paths) are now not rejected anymore. This is due to the way that the expansions are defined in the RFC. [0] https://datatracker.ietf.org/doc/html/rfc6570 --- .../hateoas/TemplateVariable.java | 331 ++++++++++++++++-- .../hateoas/TemplateVariables.java | 29 +- .../springframework/hateoas/UriTemplate.java | 308 +++++++++------- .../hateoas/server/core/WebHandler.java | 4 +- .../springframework/hateoas/LinkUnitTest.java | 5 - .../hateoas/TemplateVariablesUnitTest.java | 13 +- .../hateoas/UriTemplateUnitTest.java | 241 +++++++++++-- .../server/mvc/WebMvcLinkBuilderUnitTest.java | 43 --- 8 files changed, 738 insertions(+), 236 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/TemplateVariable.java b/src/main/java/org/springframework/hateoas/TemplateVariable.java index 429fc87a..cdfd3181 100644 --- a/src/main/java/org/springframework/hateoas/TemplateVariable.java +++ b/src/main/java/org/springframework/hateoas/TemplateVariable.java @@ -18,12 +18,19 @@ package org.springframework.hateoas; import static org.springframework.hateoas.TemplateVariable.VariableType.*; import java.io.Serializable; +import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; +import org.springframework.web.util.UriUtils; /** * A single template variable. @@ -31,13 +38,15 @@ import org.springframework.util.StringUtils; * @author Oliver Gierke * @author JamesE Richardson */ -public final class TemplateVariable implements Serializable { +public final class TemplateVariable implements Serializable, UriTemplate.Expandable { private static final long serialVersionUID = -2731446749851863774L; private final String name; private final TemplateVariable.VariableType type; private final String description; + private final Cardinality cardinality; + private final int limit; /** * Creates a new {@link TemplateVariable} with the given name and type. @@ -49,22 +58,36 @@ public final class TemplateVariable implements Serializable { this(name, type, ""); } + public TemplateVariable(String name, TemplateVariable.VariableType type, String description) { + this(name, type, description, Cardinality.SINGULAR, -1); + } + + TemplateVariable(String name, TemplateVariable.VariableType type, String description, + Cardinality cardinality) { + this(name, type, description, cardinality, -1); + } + /** * Creates a new {@link TemplateVariable} with the given name, type and description. * * @param name must not be {@literal null} or empty. * @param type must not be {@literal null}. * @param description must not be {@literal null}. + * @since 1.4 */ - public TemplateVariable(String name, TemplateVariable.VariableType type, String description) { + TemplateVariable(String name, TemplateVariable.VariableType type, String description, + Cardinality cardinality, int limit) { Assert.hasText(name, "Variable name must not be null or empty!"); Assert.notNull(type, "Variable type must not be null!"); Assert.notNull(description, "Description must not be null!"); + Assert.notNull(cardinality, "Cardinality must not be null!"); this.name = name; this.type = type; this.description = description; + this.cardinality = cardinality; + this.limit = limit; } /** @@ -114,12 +137,16 @@ public final class TemplateVariable implements Serializable { /** * Static helper to fashion {@link VariableType#FRAGMENT} variables. * - * @param fragment must not be {@literal null} or empty. + * @param name must not be {@literal null} or empty. * @return * @since 1.1 */ - public static TemplateVariable fragment(String fragment) { - return new TemplateVariable(fragment, VariableType.FRAGMENT); + public static TemplateVariable fragment(String name) { + return new TemplateVariable(name, VariableType.FRAGMENT); + } + + public static TemplateVariable reservedString(String name) { + return new TemplateVariable(name, VariableType.RESERVED_STRING); } /** @@ -128,11 +155,61 @@ public final class TemplateVariable implements Serializable { * @param parameter must not be {@literal null} or empty. * @return * @since 1.1 + * @deprecated since 1.4, use actual parameter type and call {@link #composite()} on the instance instead. */ + @Deprecated public static TemplateVariable compositeParameter(String parameter) { return new TemplateVariable(parameter, VariableType.COMPOSITE_PARAM); } + /** + * Marks the current template variable as composite value. + * + * @return + * @since 1.4 + */ + public TemplateVariable composite() { + return isComposite() ? this : new TemplateVariable(name, type, description, Cardinality.COMPOSITE, limit); + } + + /** + * Marks the current template variable as singular value. + * + * @return + * @since 1.4 + */ + public TemplateVariable singular() { + return isSingular() ? this : new TemplateVariable(name, type, description, Cardinality.SINGULAR, limit); + } + + public TemplateVariable limit(int limit) { + return new TemplateVariable(name, type, description, cardinality, limit); + } + + /** + * Returns whether the current {@link TemplateVariable} is a composite one. + * + * @return + * @since 1.4 + */ + public boolean isComposite() { + return cardinality.equals(Cardinality.COMPOSITE); + } + + /** + * Returns whether the current {@link TemplateVariable} is a singular one. + * + * @return + * @since 1.4 + */ + public boolean isSingular() { + return cardinality.equals(Cardinality.SINGULAR); + } + + String fakeName() { + return String.format("{_____%s_____}", name); + } + /** * Returns whether the variable has a description. * @@ -147,7 +224,9 @@ public final class TemplateVariable implements Serializable { * value given for that variable. * * @return + * @deprecated since 1.4. No replacement as template variables are never required actually. */ + @Deprecated boolean isRequired() { return !type.isOptional(); } @@ -191,21 +270,39 @@ public final class TemplateVariable implements Serializable { return type.equals(FRAGMENT); } + TemplateVariable withType(VariableType type) { + return new TemplateVariable(name, type, description, cardinality, limit); + } + /* * (non-Javadoc) * @see java.lang.Object#toString() */ @Override public String toString() { + return StringUtils.hasText(description) ? String.format("%s - %s", asString(), description) : asString(); + } - String base = String.format("{%s%s}", type.toString(), name); - return StringUtils.hasText(description) ? String.format("%s - %s", base, description) : base; + public String asString() { + return String.format("{%s%s}", type.toString(), essence()); + } + + String essence() { + + return String.format("%s%s%s", name, + limit != -1 ? ":".concat(String.valueOf(limit)) : "", + isComposite() ? "*" : ""); } public String getName() { return this.name; } + /** + * Returns the type of the {@link TemplateVariable}. + * + * @return will never be {@literal null}. + */ public VariableType getType() { return this.type; } @@ -214,15 +311,146 @@ public final class TemplateVariable implements Serializable { return this.description; } + /* + * (non-Javadoc) + * @see org.springframework.hateoas.UriTemplate.Expandable#expand(org.springframework.web.util.UriBuilder, java.util.Map) + */ + @Nullable @Override - public boolean equals(Object o) { + public String expand(Map parameters) { - if (this == o) + Object value = parameters.get(name); + + if (value == null) { + return null; + } + + return prepareValue(value); + + } + + @Nullable + String prepareValue(Map parameters) { + return prepareValue(parameters.get(name)); + } + + @Nullable + @SuppressWarnings("unchecked") + String prepareValue(@Nullable Object value) { + + if (value == null) { + return null; + } + + String separator = isComposite() ? type.combiner : DEFAULT_SEPARATOR; + + if (value instanceof Iterable) { + + Iterable source = (Iterable) value; + + if (!source.iterator().hasNext()) { + return null; + } + + return handleComposite(StreamSupport.stream(source.spliterator(), false) + .map(it -> prepareElement(it, false)) + .collect(Collectors.joining(separator))); + + } else if (value instanceof Map) { + + String keyValueSeparator = isComposite() ? "=" : DEFAULT_SEPARATOR; + + return handleComposite(((Map) value).entrySet().stream() + .map(it -> it.getKey().toString().concat(keyValueSeparator).concat(prepareElement(it.getValue(), true))) + .collect(Collectors.joining(separator))); + + } else { + return handleComposite(prepareElement(value, false)); + } + } + + @Nullable + private String prepareElement(Object value, boolean forMap) { + + String encoded = limitAndEncode(value); + + if (encoded == null) { + return null; + } + + switch (type) { + case REQUEST_PARAM: + case REQUEST_PARAM_CONTINUED: + case PATH_STYLE_PARAMETER: + return isComposite() && !forMap ? name.concat("=").concat(encoded) : encoded; + default: + return encoded; + } + } + + @Nullable + private String limitAndEncode(@Nullable Object value) { + + if (value == null) { + return null; + } + + String source = value.toString(); + + if (limit != -1 && limit < source.length()) { + source = source.substring(0, limit); + } + + return type.encode(source); + } + + @Nullable + private String handleComposite(@Nullable String value) { + + if (value == null) { + return null; + } + + switch (type) { + case REQUEST_PARAM: + case REQUEST_PARAM_CONTINUED: + + if (isComposite()) { + return value; + } + + return name.concat("=").concat(value); + + case PATH_STYLE_PARAMETER: + + if (isComposite()) { + return value; + } + + return StringUtils.hasText(value) + ? name.concat("=").concat(value) + : name; + + default: + return value; + } + } + + @Override + public boolean equals(@Nullable Object o) { + + if (this == o) { return true; - if (o == null || getClass() != o.getClass()) + } + if (o == null || getClass() != o.getClass()) { return false; + } TemplateVariable that = (TemplateVariable) o; - return Objects.equals(this.name, that.name) && this.type == that.type + + return Objects.equals(this.name, that.name) // + && this.type == that.type + && this.limit == that.limit + && this.cardinality == that.cardinality && Objects.equals(this.description, that.description); } @@ -238,24 +466,63 @@ public final class TemplateVariable implements Serializable { */ public enum VariableType { - PATH_VARIABLE("", false), // - REQUEST_PARAM("?", true), // - REQUEST_PARAM_CONTINUED("&", true), // - SEGMENT("/", true), // - FRAGMENT("#", true), // - COMPOSITE_PARAM("*", true); + SIMPLE("", ",", false), // + + /** + * @deprecated since 1.4, use {@link #SIMPLE} instead. + */ + @Deprecated + PATH_VARIABLE("", ",", true), // + RESERVED_STRING("+", ",", true), // + DOT(".", ".", true), // + REQUEST_PARAM("?", "&", true), // + REQUEST_PARAM_CONTINUED("&", "&", true), // + + PATH_SEGMENT("/", "/", true), // + + /** + * @deprecated since 1.4, use {@link #PATH_SEGMENT} instead. + */ + @Deprecated + SEGMENT("/", "/", true), // + PATH_STYLE_PARAMETER(";", ";", true), // + FRAGMENT("#", ",", true), // + + /** + * @deprecated since 1.4. Use the actual type and call {@link TemplateVariable#composite()}. + */ + COMPOSITE_PARAM("*", "", true); private static final List COMBINABLE_TYPES = Arrays.asList(REQUEST_PARAM, REQUEST_PARAM_CONTINUED); + static final String DEFAULT_SEPARATOR = ","; - private final String key; + private final String key, combiner; private final boolean optional; - VariableType(String key, boolean optional) { + VariableType(String key, String combiner, boolean optional) { this.key = key; + this.combiner = combiner; this.optional = optional; } + public String encode(String value) { + + switch (this) { + case DOT: + case SEGMENT: + case PATH_SEGMENT: + case PATH_STYLE_PARAMETER: + case REQUEST_PARAM: + case REQUEST_PARAM_CONTINUED: + case SIMPLE: + return UriUtils.encode(value, StandardCharsets.UTF_8); + case FRAGMENT: + default: + return UriUtils.encodePath(value, StandardCharsets.UTF_8); + } + } + /** * Returns whether the variable of this type is optional. * @@ -265,7 +532,19 @@ public final class TemplateVariable implements Serializable { return optional; } - public boolean canBeCombinedWith(VariableType type) { + String join(Collection values) { + + if (values.isEmpty()) { + return ""; + } + + String prefix = this.equals(RESERVED_STRING) ? "" : key; + + return values.stream() + .collect(Collectors.joining(combiner, prefix, "")); + } + + boolean canBeCombinedWith(VariableType type) { return this.equals(type) || COMBINABLE_TYPES.contains(this) && COMBINABLE_TYPES.contains(type); } @@ -292,4 +571,16 @@ public final class TemplateVariable implements Serializable { return key; } } + + /** + * The cardinality of the {@link TemplateVariable}. + * + * @author Oliver Drotbohm + * @since 1.4 + * @see https://datatracker.ietf.org/doc/html/rfc6570#section-2.4.2 + */ + public enum Cardinality { + SINGULAR, COMPOSITE; + } } diff --git a/src/main/java/org/springframework/hateoas/TemplateVariables.java b/src/main/java/org/springframework/hateoas/TemplateVariables.java index c84e939a..38388725 100644 --- a/src/main/java/org/springframework/hateoas/TemplateVariables.java +++ b/src/main/java/org/springframework/hateoas/TemplateVariables.java @@ -26,6 +26,7 @@ import java.util.Iterator; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.springframework.hateoas.TemplateVariable.VariableType; import org.springframework.util.Assert; @@ -61,7 +62,21 @@ public final class TemplateVariables implements Iterable, Seri Assert.notNull(variables, "Template variables must not be null!"); Assert.noNullElements(variables.toArray(), "Variables must not contain null values!"); - this.variables = Collections.unmodifiableList(variables); + boolean requestParameterFound = false; + List processed = new ArrayList<>(variables.size()); + + for (TemplateVariable variable : variables) { + + processed.add(variable.isRequestParameterVariable() && requestParameterFound + ? variable.withType(REQUEST_PARAM_CONTINUED) + : variable); + + if (variable.isRequestParameterVariable()) { + requestParameterFound = true; + } + } + + this.variables = Collections.unmodifiableList(processed); } /** @@ -112,6 +127,10 @@ public final class TemplateVariables implements Iterable, Seri return this.variables; } + public Stream stream() { + return this.variables.stream(); + } + private boolean containsEquivalentFor(TemplateVariable candidate) { return this.variables.stream() // @@ -165,7 +184,7 @@ public final class TemplateVariables implements Iterable, Seri } previous = variable; - builder.append(variable.getName()); + builder.append(variable.essence()); } return builder.append("}").toString(); @@ -174,10 +193,12 @@ public final class TemplateVariables implements Iterable, Seri @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (o == null || getClass() != o.getClass()) + } + if (o == null || getClass() != o.getClass()) { return false; + } TemplateVariables that = (TemplateVariables) o; return Objects.equals(this.variables, that.variables); } diff --git a/src/main/java/org/springframework/hateoas/UriTemplate.java b/src/main/java/org/springframework/hateoas/UriTemplate.java index 8b3248c5..626a99f1 100644 --- a/src/main/java/org/springframework/hateoas/UriTemplate.java +++ b/src/main/java/org/springframework/hateoas/UriTemplate.java @@ -15,13 +15,13 @@ */ package org.springframework.hateoas; -import java.io.IOException; -import java.io.ObjectInputStream; import java.io.Serializable; import java.net.URI; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -33,9 +33,6 @@ import org.springframework.hateoas.TemplateVariable.VariableType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; -import org.springframework.web.util.DefaultUriBuilderFactory; -import org.springframework.web.util.DefaultUriBuilderFactory.EncodingMode; -import org.springframework.web.util.UriBuilder; import org.springframework.web.util.UriBuilderFactory; import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; @@ -51,14 +48,13 @@ import org.springframework.web.util.UriUtils; */ public class UriTemplate implements Iterable, Serializable { - private static final Pattern VARIABLE_REGEX = Pattern.compile("\\{([\\?\\&#/]?)([\\w%\\,*]+)\\}"); + private static final Pattern VARIABLE_REGEX = Pattern.compile("\\{([\\?\\&#/\\.\\+\\;]?)([\\w(\\:\\d+)*%\\,*]+)\\}"); + private static final Pattern ELEMENT_REGEX = Pattern.compile("([\\w\\%]+)(\\:\\d+)?(\\*)?"); private static final long serialVersionUID = -1007874653930162262L; private final TemplateVariables variables; - private String baseUri; - private transient UriBuilderFactory factory; - - private String toString; + private final ExpandGroups groups; + private final String baseUri, template; /** * Creates a new {@link UriTemplate} using the given template string. @@ -69,38 +65,52 @@ public class UriTemplate implements Iterable, Serializable { Assert.hasText(template, "Template must not be null or empty!"); - Matcher matcher = VARIABLE_REGEX.matcher(template); - int baseUriEndIndex = template.length(); + int firstCurlyBraceIndex = template.indexOf('{'); + template = prepareTemplate(template, firstCurlyBraceIndex); + String baseUri = template; + List variables = new ArrayList<>(); + List expandGroups = new ArrayList<>(); - while (matcher.find()) { + if (firstCurlyBraceIndex != -1) { - int start = matcher.start(0); + Matcher matcher = VARIABLE_REGEX.matcher(template); - VariableType type = VariableType.from(matcher.group(1)); - String[] names = matcher.group(2).split(","); + while (matcher.find()) { - for (String name : names) { + String typeFlag = matcher.group(1); + String[] segments = matcher.group(2).split(","); + VariableType type = VariableType.from(typeFlag); + List variableGroup = new ArrayList<>(); - TemplateVariable variable; + for (String segment : segments) { - if (name.endsWith(VariableType.COMPOSITE_PARAM.toString())) { - variable = new TemplateVariable(name.substring(0, name.length() - 1), VariableType.COMPOSITE_PARAM); - } else { - variable = new TemplateVariable(name, type); + Matcher inner = ELEMENT_REGEX.matcher(segment); + + while (inner.find()) { + + String name = inner.group(1); + String limit = inner.group(2); + String composite = inner.group(3); + + TemplateVariable variable = new TemplateVariable(name, type); + + variable = StringUtils.hasText(composite) ? variable.composite() : variable; + variable = StringUtils.hasText(limit) ? variable.limit(Integer.valueOf(limit.substring(1))) : variable; + + variableGroup.add(variable); + variables.add(variable); + } } - if (!variable.isRequired() && start < baseUriEndIndex) { - baseUriEndIndex = start; - } - - variables.add(variable); + expandGroups.add(new ExpandGroup(variableGroup)); } } this.variables = variables.isEmpty() ? TemplateVariables.NONE : new TemplateVariables(variables); - this.baseUri = template.substring(0, baseUriEndIndex); - this.factory = createFactory(baseUri); + this.groups = new ExpandGroups(expandGroups); + this.baseUri = baseUri; + this.template = template; } /** @@ -108,17 +118,16 @@ public class UriTemplate implements Iterable, Serializable { * * @param baseUri must not be {@literal null} or empty. * @param variables must not be {@literal null}. - * @param factory must not be {@literal null}. */ - private UriTemplate(String baseUri, TemplateVariables variables, UriBuilderFactory factory) { + private UriTemplate(String baseUri, String template, TemplateVariables variables, ExpandGroups groups) { Assert.hasText(baseUri, "Base URI must not be null or empty!"); Assert.notNull(variables, "Template variables must not be null!"); - Assert.notNull(factory, "UriBuilderFactory must not be null!"); this.baseUri = baseUri; this.variables = variables; - this.factory = factory; + this.groups = groups; + this.template = template; } /** @@ -163,6 +172,8 @@ public class UriTemplate implements Iterable, Serializable { UriComponents components = UriComponentsBuilder.fromUriString(baseUri).build(); List result = new ArrayList<>(); + String newOriginal = template; + ExpandGroups groups = this.groups; for (TemplateVariable variable : variables) { @@ -177,10 +188,21 @@ public class UriTemplate implements Iterable, Serializable { continue; } + ExpandGroup existing = groups.findLastExpandGroupOfType(variable.getType()); + ExpandGroup group = new ExpandGroup(Collections.singletonList(variable)); + + if (existing != null) { + group = existing.merge(group); + newOriginal = newOriginal.replace(existing.asString(), group.asString()); + } else { + newOriginal = newOriginal.concat(group.asString()); + } + + groups = groups.addOrAugment(group); result.add(variable); } - return new UriTemplate(baseUri, this.variables.concat(result), this.factory); + return new UriTemplate(baseUri, newOriginal, this.variables.concat(result), groups); } /** @@ -255,21 +277,18 @@ public class UriTemplate implements Iterable, Serializable { return URI.create(baseUri); } - UriBuilder builder = factory.uriString(baseUri); Iterator iterator = Arrays.asList(parameters).iterator(); + Map foo = new HashMap<>(); - variables.asList().stream() // - .filter(TemplateVariable::isRequired)// - .filter(__ -> iterator.hasNext()) // - .forEach(__ -> iterator.next()); + variables.stream() + .map(TemplateVariable::getName) + .forEach(it -> { - for (TemplateVariable variable : getOptionalVariables()) { + Object value = iterator.hasNext() ? iterator.next() : null; + foo.put(it, value); + }); - Object value = iterator.hasNext() ? iterator.next() : null; - appendToBuilder(builder, variable, value); - } - - return builder.build(parameters); + return expand(foo); } /** @@ -286,13 +305,21 @@ public class UriTemplate implements Iterable, Serializable { return URI.create(baseUri); } - UriBuilder builder = factory.uriString(baseUri); + String result = template; - for (TemplateVariable variable : getOptionalVariables()) { - appendToBuilder(builder, variable, parameters.get(variable.getName())); + for (ExpandGroup group : groups.groupList) { + result = result.replace(group.asString(), group.expand(parameters)); } - return builder.build(parameters); + return URI.create(result); + } + + interface Expandable { + + @Nullable + String expand(Map parameters); + + String asString(); } /* @@ -310,117 +337,134 @@ public class UriTemplate implements Iterable, Serializable { */ @Override public String toString() { + return template; + } - if (toString == null) { + private static String prepareTemplate(String template, int index) { - UriComponents components = UriComponentsBuilder.fromUriString(baseUri).build(); - boolean hasQueryParameters = !components.getQueryParams().isEmpty(); + String decodedTemplate = UriUtils.decode(template, StandardCharsets.UTF_8); - this.toString = baseUri + getOptionalVariables().toString(hasQueryParameters); + if (decodedTemplate.length() != template.length()) { + return template; } - return toString; + String head = index == -1 ? template : template.substring(0, index); + String tail = index == -1 ? "" : template.substring(index); + String encodedBase = UriComponentsBuilder.fromUriString(head) + .encode() + .build() + .toUriString(); + + head = encodedBase.length() > head.length() ? encodedBase : head; + + return head + tail; } - private TemplateVariables getOptionalVariables() { + private static class ExpandGroups implements Serializable { - return variables.asList().stream() // - .filter(variable -> !variable.isRequired()) // - .collect(Collectors.collectingAndThen(Collectors.toList(), TemplateVariables::new)); - } + private static final long serialVersionUID = 6260926152179514011L; - /** - * Creates a {@link UriBuilderFactory} that might optionally encode the given base URI if it still needs to be - * encoded. - * - * @param baseUri must not be {@literal null} or empty. - * @return - */ - private static UriBuilderFactory createFactory(String baseUri) { + private final List groupList; - EncodingMode mode = UriUtils.decode(baseUri, StandardCharsets.UTF_8).length() < baseUri.length() // - ? EncodingMode.VALUES_ONLY // - : EncodingMode.TEMPLATE_AND_VALUES; + public ExpandGroups(List groups) { + this.groupList = groups; + } - DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory(); - factory.setEncodingMode(mode); + public ExpandGroups addOrAugment(ExpandGroup group) { - return factory; - } + ExpandGroup existing = findLastExpandGroupOfType(group.type); + List foo = new ArrayList<>(groupList); - /** - * Appends the value for the given {@link TemplateVariable} to the given {@link UriComponentsBuilder}. - * - * @param builder must not be {@literal null}. - * @param variable must not be {@literal null}. - * @param value can be {@literal null}. - */ - private static void appendToBuilder(UriBuilder builder, TemplateVariable variable, @Nullable Object value) { + if (existing == null) { - if (value == null) { + foo.add(group); - if (variable.isRequired()) { - throw new IllegalArgumentException( - String.format("Template variable %s is required but no value was given!", variable.getName())); + return new ExpandGroups(foo); } - return; + ExpandGroup merged = existing.merge(group); + + foo.remove(existing); + foo.add(merged); + + return new ExpandGroups(foo); } - switch (variable.getType()) { - case COMPOSITE_PARAM: - appendComposite(builder, variable.getName(), value); - break; - case REQUEST_PARAM: - case REQUEST_PARAM_CONTINUED: - builder.queryParam(variable.getName(), value); - break; - case PATH_VARIABLE: - case SEGMENT: - builder.pathSegment(value.toString()); - break; - case FRAGMENT: - builder.fragment(value.toString()); - break; + @Nullable + ExpandGroup findLastExpandGroupOfType(VariableType type) { + + ExpandGroup result = null; + + for (ExpandGroup entry : groupList) { + if (entry.canBeCombinedWith(type)) { + result = entry; + } + } + + return result; } } - /** - * Expand what could be a single value, a {@link List}, or a {@link Map}. - * - * @param builder - * @param name - * @param value - * @see https://tools.ietf.org/html/rfc6570#section-2.4.2 - */ - @SuppressWarnings("unchecked") - private static void appendComposite(UriBuilder builder, String name, Object value) { + private static class ExpandGroup implements Expandable, Serializable { - if (value instanceof Iterable) { + private static final long serialVersionUID = -6057608202572953271L; - ((Iterable) value).forEach(it -> builder.queryParam(name, it)); + private final TemplateVariables variables; + private final VariableType type; - } else if (value instanceof Map) { - - ((Map) value).forEach((key, value1) -> builder.queryParam(key.toString(), value1)); - - } else { - - builder.queryParam(name, value); + public ExpandGroup(List variables) { + this(new TemplateVariables(variables)); } - } - /** - * Recreate {@link UriBuilderFactory} on deserialization. - * - * @param in - * @throws IOException - * @throws ClassNotFoundException - */ - private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { + ExpandGroup(TemplateVariables variables) { - in.defaultReadObject(); + this.variables = variables; + this.type = variables.asList().get(0).getType(); + } - this.factory = createFactory(baseUri); + ExpandGroup merge(ExpandGroup group) { + + Assert.isTrue(this.type.canBeCombinedWith(group.type), "Incompatible expand groups!"); + + return new ExpandGroup(variables.concat(group.variables)); + } + + /* + * (non-Javadoc) + * @see org.springframework.hateoas.UriTemplate.Expandable#expand(org.springframework.web.util.UriBuilder, java.util.Map) + */ + @Nullable + @Override + public String expand(Map parameters) { + + return type.join(variables.stream() + .map(it -> it.prepareValue(parameters)) + .filter(it -> it != null) + .collect(Collectors.toList())); + } + + boolean canBeCombinedWith(VariableType type) { + return this.type.canBeCombinedWith(type); + } + + /* + * (non-Javadoc) + * @see org.springframework.hateoas.UriTemplate.Expandable#asString() + */ + @Override + public String asString() { + + return variables.stream().map(TemplateVariable::essence) + .collect(Collectors.joining(",", "{".concat(type.toString()), "}")); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + return asString(); + } } } diff --git a/src/main/java/org/springframework/hateoas/server/core/WebHandler.java b/src/main/java/org/springframework/hateoas/server/core/WebHandler.java index bde984de..6c4b629a 100644 --- a/src/main/java/org/springframework/hateoas/server/core/WebHandler.java +++ b/src/main/java/org/springframework/hateoas/server/core/WebHandler.java @@ -472,12 +472,12 @@ public class WebHandler { return value; } - RequestParam annotation = parameter.getParameterAnnotation(RequestParam.class); - if (!isRequired() || parameter.isOptional()) { return SKIP_VALUE; } + RequestParam annotation = parameter.getParameterAnnotation(RequestParam.class); + return annotation.defaultValue().equals(ValueConstants.DEFAULT_NONE) ? SKIP_VALUE : null; } } diff --git a/src/test/java/org/springframework/hateoas/LinkUnitTest.java b/src/test/java/org/springframework/hateoas/LinkUnitTest.java index 627eb011..1a2a290e 100755 --- a/src/test/java/org/springframework/hateoas/LinkUnitTest.java +++ b/src/test/java/org/springframework/hateoas/LinkUnitTest.java @@ -324,9 +324,4 @@ class LinkUnitTest { void createsUriForTemplateWithOptionalParameters() { assertThat(Link.of("/something{?parameter}").toUri()).isEqualTo(URI.create("/something")); } - - @Test - void uriCreationRejectsLinkWithUnresolvedMandatoryParameters() { - assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> Link.of("/{segment}/path").toUri()); - } } diff --git a/src/test/java/org/springframework/hateoas/TemplateVariablesUnitTest.java b/src/test/java/org/springframework/hateoas/TemplateVariablesUnitTest.java index c7744896..47d93208 100755 --- a/src/test/java/org/springframework/hateoas/TemplateVariablesUnitTest.java +++ b/src/test/java/org/springframework/hateoas/TemplateVariablesUnitTest.java @@ -21,11 +21,12 @@ import static org.springframework.hateoas.TemplateVariable.VariableType.*; import java.util.List; import org.junit.jupiter.api.Test; +import org.springframework.hateoas.TemplateVariable.Cardinality; import org.springframework.hateoas.TemplateVariable.VariableType; /** * Unit tests for {@link TemplateVariables}. - * + * * @author Oliver Gierke */ class TemplateVariablesUnitTest { @@ -211,7 +212,15 @@ class TemplateVariablesUnitTest { void variableRejectsNullDescription() { assertThatIllegalArgumentException().isThrownBy(() -> { - new TemplateVariable("foo", PATH_VARIABLE, null); + new TemplateVariable("foo", PATH_VARIABLE, null, Cardinality.SINGULAR); + }); + } + + @Test + void variableRejectsNullCardinality() { + + assertThatIllegalArgumentException().isThrownBy(() -> { + new TemplateVariable("foo", PATH_VARIABLE, "description", null); }); } } diff --git a/src/test/java/org/springframework/hateoas/UriTemplateUnitTest.java b/src/test/java/org/springframework/hateoas/UriTemplateUnitTest.java index 0b60e568..63dafb3c 100755 --- a/src/test/java/org/springframework/hateoas/UriTemplateUnitTest.java +++ b/src/test/java/org/springframework/hateoas/UriTemplateUnitTest.java @@ -36,10 +36,10 @@ import java.util.Map; import java.util.stream.Stream; import org.apache.commons.io.output.ByteArrayOutputStream; +import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.MethodSource; -import org.springframework.hateoas.TemplateVariable.*; +import org.junit.jupiter.api.TestFactory; +import org.springframework.hateoas.TemplateVariable.VariableType; /** * Unit tests for {@link UriTemplate}. @@ -80,7 +80,7 @@ class UriTemplateUnitTest { UriTemplate template = UriTemplate.of("/foo{/bar}"); - assertVariables(template, new TemplateVariable("bar", VariableType.SEGMENT)); + assertVariables(template, new TemplateVariable("bar", VariableType.PATH_SEGMENT)); } @Test // #137 @@ -88,7 +88,7 @@ class UriTemplateUnitTest { UriTemplate template = UriTemplate.of("/foo/{bar}"); - assertVariables(template, new TemplateVariable("bar", VariableType.PATH_VARIABLE)); + assertVariables(template, new TemplateVariable("bar", VariableType.SIMPLE)); } @Test // #137 @@ -105,7 +105,7 @@ class UriTemplateUnitTest { UriTemplate template = UriTemplate.of("/foo{?bar,foobar}"); assertVariables(template, new TemplateVariable("bar", VariableType.REQUEST_PARAM), - new TemplateVariable("foobar", VariableType.REQUEST_PARAM)); + new TemplateVariable("foobar", VariableType.REQUEST_PARAM_CONTINUED)); } @Test // #137 @@ -130,16 +130,6 @@ class UriTemplateUnitTest { assertThat(uri.toString()).isEqualTo("/foo?bar=myBar&fooBar=myFooBar"); } - @Test // #137 - void rejectsMissingRequiredPathVariable() { - - UriTemplate template = UriTemplate.of("/foo/{bar}"); - - assertThatIllegalArgumentException().isThrownBy(() -> { - template.expand(Collections.emptyMap()); - }); - } - @Test // #137 void expandsMultipleVariablesViaArray() { @@ -170,7 +160,7 @@ class UriTemplateUnitTest { void addsTemplateVariables() { UriTemplate source = UriTemplate.of("/{foo}/bar{?page}"); - List toAdd = Arrays.asList(new TemplateVariable("bar", VariableType.REQUEST_PARAM)); + List toAdd = Arrays.asList(new TemplateVariable("bar", VariableType.REQUEST_PARAM_CONTINUED)); List expected = new ArrayList<>(); expected.addAll(source.getVariables()); @@ -216,19 +206,21 @@ class UriTemplateUnitTest { } @Test // #483 - void compositveValuesAreRecognisedAsVariableType() { + void compositveValuesAreRecognised() { UriTemplate template = UriTemplate.of("/foo{&bar,foobar*}"); - assertVariables(template, new TemplateVariable("bar", VariableType.REQUEST_PARAM_CONTINUED), - new TemplateVariable("foobar", VariableType.COMPOSITE_PARAM)); + TemplateVariable templateVariable = template.getVariables().get(1); + + assertThat(templateVariable.isComposite()).isTrue(); + assertThat(templateVariable.getType()).isEqualTo(VariableType.REQUEST_PARAM_CONTINUED); } @Test // #483 @SuppressWarnings("serial") void expandsCompositeValueAsAssociativeArray() { - of("/foo{&bar,foobar*}", "/foo?bar=barExpanded&city=Clarksville&state=TN") // + of("/foo{?bar,foobar*}", "/foo?bar=barExpanded&city=Clarksville&state=TN") // .param("bar", "barExpanded") // .param("foobar", new HashMap() { { @@ -242,7 +234,7 @@ class UriTemplateUnitTest { @Test // #483 void expandsCompositeValueAsList() { - of("/foo{&bar,foobar*}", "/foo?bar=barExpanded&foobar=foo1&foobar=foo2") // + of("/foo{?bar,foobar*}", "/foo?bar=barExpanded&foobar=foo1&foobar=foo2") // .param("bar", "barExpanded") // .param("foobar", Arrays.asList("foo1", "foo2")) // .verify(); @@ -251,7 +243,7 @@ class UriTemplateUnitTest { @Test // #483 void handlesCompositeValueAsSingleValue() { - of("/foo{&bar,foobar*}", "/foo?bar=barExpanded&foobar=singleValue") // + of("/foo{?bar,foobar*}", "/foo?bar=barExpanded&foobar=singleValue") // .param("bar", "barExpanded") // .param("foobar", "singleValue") // .verify(); @@ -259,15 +251,18 @@ class UriTemplateUnitTest { @Test // #1127 void escapesBaseUriProperly() { + of("https://example.org/foo and bar/{baz}", "https://example.org/foo%20and%20bar/xyzzy") // .param("baz", "xyzzy") // .verify(); + + of("/foo?foo=bar{&baz}", "/foo?foo=bar&baz=xyz").param("baz", "xyz").verify(); + of("?foo=bar{&baz}", "?foo=bar&baz=xyz").param("baz", "xyz").verify(); } - @ParameterizedTest // #593 - @MethodSource("getEncodingFixtures") - public void uriTemplateExpansionsShouldWork(EncodingFixture fixture) { - fixture.verify(); + @TestFactory // #593 + public Stream uriTemplateExpansionsShouldWork() { + return DynamicTest.stream(getEncodingFixtures(), EncodingFixture::toString, EncodingFixture::verify); } @Test // #593 @@ -362,6 +357,15 @@ class UriTemplateUnitTest { .isEqualTo(URI.create("http://localhost/foo/bar/value")); } + @Test + void expandsCompositePaths() { + + URI uri = UriTemplate.of("/foo{/bar*}") // + .expand(Collections.singletonMap("bar", Arrays.asList("first", "second"))); + + assertThat(uri).isEqualTo(URI.create("/foo/first/second")); + } + private static void assertVariables(UriTemplate template, TemplateVariable... variables) { assertVariables(template, Arrays.asList(variables)); } @@ -378,6 +382,179 @@ class UriTemplateUnitTest { } } + @TestFactory + Stream rfcExamples() { + return DynamicTest.stream(foo(), EncodingFixture::toShortString, EncodingFixture::verify); + } + + private static Stream foo() { + + Map values = new HashMap<>(); + values.put("count", Arrays.asList("one", "two", "three")); + values.put("dom", Arrays.asList("example", "com")); + values.put("dub", "me/too"); + values.put("hello", "Hello World!"); + values.put("half", "50%"); + values.put("var", "value"); + values.put("who", "fred"); + values.put("base", "http://example.com/home/"); + values.put("path", "/foo/bar"); + values.put("list", Arrays.asList("red", "green", "blue")); + values.put("keys", new LinkedHashMap() { + { + put("semi", ";"); + put("dot", "."); + put("comma", ","); + } + }); + values.put("v", 6); + values.put("x", 1024); + values.put("y", 768); + values.put("empty", ""); + values.put("empty_keys", Collections.emptyList()); + values.put("undef", null); + + return Stream.of( // + // 3.2.1 + of("{count}", "one,two,three"), + of("{count*}", "one,two,three"), + of("{/count}", "/one,two,three"), + of("{/count*}", "/one/two/three"), + of("{;count}", ";count=one,two,three"), + of("{;count*}", ";count=one;count=two;count=three"), + of("{?count}", "?count=one,two,three"), + of("{?count*}", "?count=one&count=two&count=three"), + of("?bar={&count*}", "?bar=&count=one&count=two&count=three"), // + + // 3.2.2 + of("{var}", "value"), + of("{hello}", "Hello%20World%21"), + of("{half}", "50%25"), + of("O{empty}X", "OX"), + of("O{undef}X", "OX"), + of("{x,y}", "1024,768"), + of("{x,hello,y}", "1024,Hello%20World%21,768"), + of("?{x,empty}", "?1024,"), + of("?{x,undef}", "?1024"), + of("?{undef,y}", "?768"), + of("{var:3}", "val"), + of("{var:30}", "value"), + of("{list}", "red,green,blue"), + of("{list*}", "red,green,blue"), + of("{keys}", "semi,%3B,dot,.,comma,%2C"), + of("{keys*}", "semi=%3B,dot=.,comma=%2C"), // + + // 3.2.3 + of("{+var}", "value"), + of("{+hello}", "Hello%20World!"), + of("{+half}", "50%25"), + of("{base}index", "http%3A%2F%2Fexample.com%2Fhome%2Findex"), + of("{+base}index", "http://example.com/home/index"), + of("O{+empty}X", "OX"), + of("O{+undef}X", "OX"), + of("{+path}/here", "/foo/bar/here"), + of("here?ref={+path}", "here?ref=/foo/bar"), + of("up{+path}{var}/here", "up/foo/barvalue/here"), + of("{+x,hello,y}", "1024,Hello%20World!,768"), + of("{+path,x}/here", "/foo/bar,1024/here"), + of("{+path:6}/here", "/foo/b/here"), + of("{+list}", "red,green,blue"), + of("{+list*}", "red,green,blue"), + of("{+keys}", "semi,;,dot,.,comma,,"), + of("{+keys*}", "semi=;,dot=.,comma=,"), // + + // 3.2.4 + of("{#var}", "#value"), + of("{#hello}", "#Hello%20World!"), + of("{#half}", "#50%25"), + of("foo{#empty}", "foo#"), + of("foo{#undef}", "foo"), + of("{#x,hello,y}", "#1024,Hello%20World!,768"), + of("{#path,x}/here", "#/foo/bar,1024/here"), + of("{#path:6}/here", "#/foo/b/here"), + of("{#list}", "#red,green,blue"), + of("{#list*}", "#red,green,blue"), + of("{#keys}", "#semi,;,dot,.,comma,,"), + of("{#keys*}", "#semi=;,dot=.,comma=,"), // + + // 3.2.5 + of("{.who}", ".fred"), + of("{.who,who}", ".fred.fred"), + of("{.half,who}", ".50%25.fred"), + of("www{.dom*}", "www.example.com"), + of("X{.var}", "X.value"), + of("X{.empty}", "X."), + of("X{.undef}", "X"), + of("X{.var:3}", "X.val"), + of("X{.list}", "X.red,green,blue"), + of("X{.list*}", "X.red.green.blue"), + of("X{.keys}", "X.semi,%3B,dot,.,comma,%2C"), + of("X{.keys*}", "X.semi=%3B.dot=..comma=%2C"), + of("X{.empty_keys}", "X"), + of("X{.empty_keys*}", "X"), + + // 3.2.6 - Path segments + of("{/who}", "/fred"), + of("{/who,who}", "/fred/fred"), + of("{/half,who}", "/50%25/fred"), + of("{/who,dub}", "/fred/me%2Ftoo"), + of("{/var}", "/value"), + of("{/var,empty}", "/value/"), + of("{/var,undef}", "/value"), + of("{/var,x}/here", "/value/1024/here"), + of("{/var:1,var}", "/v/value"), + of("{/list}", "/red,green,blue"), + of("{/list*}", "/red/green/blue"), + of("{/list*,path:4}", "/red/green/blue/%2Ffoo"), + of("{/keys}", "/semi,%3B,dot,.,comma,%2C"), + of("{/keys*}", "/semi=%3B/dot=./comma=%2C"), + + // 3.2.7 - + of("{;who}", ";who=fred"), + of("{;half}", ";half=50%25"), + of("{;empty}", ";empty"), + of("{;v,empty,who}", ";v=6;empty;who=fred"), + of("{;v,bar,who}", ";v=6;who=fred"), + of("{;x,y}", ";x=1024;y=768"), + of("{;x,y,empty}", ";x=1024;y=768;empty"), + of("{;x,y,undef}", ";x=1024;y=768"), + of("{;hello:5}", ";hello=Hello"), + of("{;list}", ";list=red,green,blue"), + of("{;list*}", ";list=red;list=green;list=blue"), + of("{;keys}", ";keys=semi,%3B,dot,.,comma,%2C"), + of("{;keys*}", ";semi=%3B;dot=.;comma=%2C"), + + // 3.2.8 - Form-Style Query Expansion + of("{?who}", "?who=fred"), + of("{?half}", "?half=50%25"), + of("{?x,y}", "?x=1024&y=768"), + of("{?x,y,empty}", "?x=1024&y=768&empty="), + of("{?x,y,undef}", "?x=1024&y=768"), + of("{?var:3}", "?var=val"), + of("{?list}", "?list=red,green,blue"), + of("{?list*}", "?list=red&list=green&list=blue"), + of("{?keys}", "?keys=semi,%3B,dot,.,comma,%2C"), + of("{?keys*}", "?semi=%3B&dot=.&comma=%2C"), + + // + of("?foo={&who}", "?foo=&who=fred"), + of("?foo={&half}", "?foo=&half=50%25"), + of("?fixed=yes{&x}", "?fixed=yes&x=1024"), + of("?foo={&x,y,empty}", "?foo=&x=1024&y=768&empty="), + of("?foo={&x,y,undef}", "?foo=&x=1024&y=768"), + of("?foo={&var:3}", "?foo=&var=val"), + of("?foo={&list}", "?foo=&list=red,green,blue"), + of("?foo={&list*}", "?foo=&list=red&list=green&list=blue"), + of("?foo={&keys}", "?foo=&keys=semi,%3B,dot,.,comma,%2C"), + of("?foo={&keys*}", "?foo=&semi=%3B&dot=.&comma=%2C") + + // + ) // + + .map(EncodingFixture::skipVarArgsVerification) + .map(it -> it.params(values)); + } + private static Stream getEncodingFixtures() { return Stream.of(// @@ -433,7 +610,11 @@ class UriTemplateUnitTest { Map newParameters = new LinkedHashMap<>(parameters); newParameters.put(key, value); - return new EncodingFixture(template, uri, newParameters, varArgsVerification); + return params(newParameters); + } + + public EncodingFixture params(Map parameters) { + return new EncodingFixture(template, uri, parameters, varArgsVerification); } public EncodingFixture skipVarArgsVerification() { @@ -451,6 +632,10 @@ class UriTemplateUnitTest { } } + public String toShortString() { + return String.format("Expanding %s to %s", template, uri); + } + @Override public String toString() { return String.format("Expanding %s using parameters %s results in %s.", template, parameters, uri); diff --git a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java index ca8de0c1..6c2757ee 100644 --- a/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java +++ b/src/test/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderUnitTest.java @@ -303,33 +303,6 @@ class WebMvcLinkBuilderUnitTest extends TestUtils { assertThat(link.expand().getHref()).endsWith("/foo"); } - /** - * @see #122, #169 - */ - @Test - void rejectsMissingPathVariable() { - - assertThatIllegalArgumentException().isThrownBy(() -> { - linkTo(methodOn(ControllerWithMethods.class).methodWithPathVariable(null))// - .withSelfRel().expand(); - }); - } - - /** - * @see #122, #169 - */ - @Test - void rejectsMissingRequiredRequestParam() { - - assertThatIllegalArgumentException().isThrownBy(() -> { - Link link = linkTo(methodOn(ControllerWithMethods.class).methodWithRequestParam(null)).withSelfRel(); - - assertThat(link.getVariableNames()).containsExactly("id"); - - link.expand(); - }); - } - /** * @see #170 */ @@ -506,22 +479,6 @@ class WebMvcLinkBuilderUnitTest extends TestUtils { assertThat(link.getHref()).contains("some%20id"); } - /** - * @see #169 - */ - @Test - void addsRequestParameterVariablesForMissingRequiredParameter() { - - assertThatIllegalArgumentException().isThrownBy(() -> { - - Link link = linkTo(methodOn(ControllerWithMethods.class).methodForNextPage("1", 10, null)).withSelfRel(); - - assertThat(link.getVariableNames()).containsExactly("limit"); - - link.expand(); - }).withMessageContaining("limit"); - } - /** * @see #169 */