From 21112f657244c3ecfa6afd8fafe7bc212e04bd0b Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Mon, 8 Nov 2021 14:23:12 -0500 Subject: [PATCH 1/4] [GH-1319] Add Support for Path Style Parameter Expansion Fixes #1319 This change adds limited Path Style support to Feign URI template-style templates. Variable expressions that start with a semi-colon `;` are now expanded in accordance to [RFC 6570 Section 3.2.7](https://datatracker.ietf.org/doc/html/rfc6570#section-3.2.7) with the following modifications: * Maps and Lists are expanded by default. * Only Single variable templates are supported. Examples: ``` {;who} ;who=fred {;half} ;half=50%25 {;empty} ;empty {;list} ;list=red;list=green;list=blue {;keys} ;semi=%3B;dot=.;comma=%2C ``` --- .../main/java/feign/template/Expressions.java | 123 ++++++++++++------ .../java/feign/template/UriTemplateTest.java | 47 +++++++ 2 files changed, 127 insertions(+), 43 deletions(-) diff --git a/core/src/main/java/feign/template/Expressions.java b/core/src/main/java/feign/template/Expressions.java index 8dc9f108d0..6e5842d68c 100644 --- a/core/src/main/java/feign/template/Expressions.java +++ b/core/src/main/java/feign/template/Expressions.java @@ -13,32 +13,17 @@ */ package feign.template; +import feign.Util; import java.util.LinkedHashMap; import java.util.Map; import java.util.Map.Entry; -import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; -import feign.Util; public final class Expressions { - private static Map> expressions; - - static { - expressions = new LinkedHashMap<>(); - - /* - * basic pattern for variable names. this is compliant with RFC 6570 Simple Expressions ONLY - * with the following additional values allowed without required pct-encoding: - * - * - brackets - dashes - * - * see https://tools.ietf.org/html/rfc6570#section-2.3 for more information. - */ - - expressions.put(Pattern.compile("^([+#./;?&]?)(.*)$"), - SimpleExpression.class); - } + + private static final String PATH_STYLE_MODIFIER = ";"; + private static final Pattern EXPRESSION_PATTERN = Pattern.compile("^([+#./;?&]?)(.*)$"); public static Expression create(final String value) { @@ -48,25 +33,15 @@ public static Expression create(final String value) { throw new IllegalArgumentException("an expression is required."); } - Optional>> matchedExpressionEntry = - expressions.entrySet() - .stream() - .filter(entry -> entry.getKey().matcher(expression).matches()) - .findFirst(); - - if (!matchedExpressionEntry.isPresent()) { - /* not a valid expression */ - return null; - } - - Entry> matchedExpression = matchedExpressionEntry.get(); - Pattern expressionPattern = matchedExpression.getKey(); - /* create a new regular expression matcher for the expression */ String variableName = null; String variablePattern = null; - Matcher matcher = expressionPattern.matcher(expression); + String modifier = null; + Matcher matcher = EXPRESSION_PATTERN.matcher(expression); if (matcher.matches()) { + /* grab the modifier */ + modifier = matcher.group(1).trim(); + /* we have a valid variable expression, extract the name from the first group */ variableName = matcher.group(2).trim(); if (variableName.contains(":")) { @@ -83,6 +58,12 @@ public static Expression create(final String value) { } } + /* check for a modifier */ + if (PATH_STYLE_MODIFIER.equalsIgnoreCase(modifier)) { + return new PathStyleExpression(variableName, variablePattern); + } + + /* default to simple */ return new SimpleExpression(variableName, variablePattern); } @@ -102,20 +83,37 @@ private static String stripBraces(String expression) { */ static class SimpleExpression extends Expression { - SimpleExpression(String expression, String pattern) { - super(expression, pattern); + private static final String DEFAULT_SEPARATOR = ","; + protected String separator = DEFAULT_SEPARATOR; + private boolean nameRequired = false; + + SimpleExpression(String name, String pattern) { + super(name, pattern); + } + + SimpleExpression(String name, String pattern, String separator, boolean nameRequired) { + this(name, pattern); + this.separator = separator; + this.nameRequired = nameRequired; } - String encode(Object value) { + protected String encode(Object value) { return UriUtils.encode(value.toString(), Util.UTF_8); } + @SuppressWarnings("unchecked") @Override - String expand(Object variable, boolean encode) { + protected String expand(Object variable, boolean encode) { StringBuilder expanded = new StringBuilder(); if (Iterable.class.isAssignableFrom(variable.getClass())) { expanded.append(this.expandIterable((Iterable) variable)); + } else if (Map.class.isAssignableFrom(variable.getClass())) { + expanded.append(this.expandMap((Map) variable)); } else { + if (this.nameRequired) { + expanded.append(this.encode(this.getName())) + .append("="); + } expanded.append((encode) ? encode(variable) : variable); } @@ -128,8 +126,7 @@ String expand(Object variable, boolean encode) { return result; } - - private String expandIterable(Iterable values) { + protected String expandIterable(Iterable values) { StringBuilder result = new StringBuilder(); for (Object value : values) { if (value == null) { @@ -141,13 +138,17 @@ private String expandIterable(Iterable values) { String expanded = this.encode(value); if (expanded.isEmpty()) { /* always append the separator */ - result.append(","); + result.append(this.separator); } else { if (result.length() != 0) { - if (!result.toString().equalsIgnoreCase(",")) { - result.append(","); + if (!result.toString().equalsIgnoreCase(this.separator)) { + result.append(this.separator); } } + if (this.nameRequired) { + result.append(this.encode(this.getName())) + .append("="); + } result.append(expanded); } } @@ -155,5 +156,41 @@ private String expandIterable(Iterable values) { /* return the expanded value */ return result.toString(); } + + protected String expandMap(Map values) { + StringBuilder result = new StringBuilder(); + + for (Entry entry : values.entrySet()) { + StringBuilder expanded = new StringBuilder(); + String name = this.encode(entry.getKey()); + String value = this.encode(entry.getValue().toString()); + + expanded.append(name) + .append("="); + if (!value.isEmpty()) { + expanded.append(value); + } + + if (result.length() != 0) { + result.append(this.separator); + } + + result.append(expanded); + } + return result.toString(); + } + } + + static class PathStyleExpression extends SimpleExpression { + + PathStyleExpression(String name, String pattern) { + super(name, pattern, ";", true); + } + + @Override + protected String expand(Object variable, boolean encode) { + return this.separator + super.expand(variable, encode); + } + } } diff --git a/core/src/test/java/feign/template/UriTemplateTest.java b/core/src/test/java/feign/template/UriTemplateTest.java index 916d7a7ea8..395db442ef 100644 --- a/core/src/test/java/feign/template/UriTemplateTest.java +++ b/core/src/test/java/feign/template/UriTemplateTest.java @@ -17,6 +17,7 @@ import static org.assertj.core.api.Assertions.fail; import feign.Util; import java.net.URI; +import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; @@ -299,4 +300,50 @@ public void encodeReserved() { String expanded = uriTemplate.expand(Collections.singletonMap("url", "https://www.google.com")); assertThat(expanded).isEqualToIgnoringCase("/get?url=https%3A%2F%2Fwww.google.com"); } + + @Test + public void pathStyleExpansionSupported() { + String template = "{;who}"; + UriTemplate uriTemplate= UriTemplate.create(template, Util.UTF_8); + String expanded = uriTemplate.expand(Collections.singletonMap("who", "fred")); + assertThat(expanded).isEqualToIgnoringCase(";who=fred"); + } + + @Test + public void pathStyleExpansionEncodesReservedCharacters() { + String template = "{;half}"; + UriTemplate uriTemplate= UriTemplate.create(template, Util.UTF_8); + String expanded = uriTemplate.expand(Collections.singletonMap("half", "50%")); + assertThat(expanded).isEqualToIgnoringCase(";half=50%25"); + } + + @Test + public void pathStyleExpansionSupportedWithLists() { + String template = "{;list}"; + UriTemplate uriTemplate= UriTemplate.create(template, Util.UTF_8); + + List values = new ArrayList<>(); + values.add("red"); + values.add("green"); + values.add("blue"); + + String expanded = uriTemplate.expand(Collections.singletonMap("list", values)); + assertThat(expanded).isEqualToIgnoringCase(";list=red;list=green;list=blue"); + + } + + @Test + public void pathStyleExpansionSupportedWithMap() { + String template = "/server/matrixParams{;parameters}"; + Map parameters = new LinkedHashMap<>(); + parameters.put("account", "a"); + parameters.put("name", "n"); + + Map values = new LinkedHashMap<>(); + values.put("parameters", parameters); + + UriTemplate uriTemplate = UriTemplate.create(template, Util.UTF_8); + String expanded = uriTemplate.expand(values); + assertThat(expanded).isEqualToIgnoringCase("/server/matrixParams;account=a;name=n"); + } } From 964ab4a657d996484d0e6ff7d8388411c597a81f Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Mon, 8 Nov 2021 15:08:44 -0500 Subject: [PATCH 2/4] Export Path Style Expression as an Expander for use with custom contracts --- core/src/main/java/feign/template/Expressions.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/feign/template/Expressions.java b/core/src/main/java/feign/template/Expressions.java index 6e5842d68c..72b80f9c9b 100644 --- a/core/src/main/java/feign/template/Expressions.java +++ b/core/src/main/java/feign/template/Expressions.java @@ -13,6 +13,7 @@ */ package feign.template; +import feign.Param.Expander; import feign.Util; import java.util.LinkedHashMap; import java.util.Map; @@ -181,7 +182,7 @@ protected String expandMap(Map values) { } } - static class PathStyleExpression extends SimpleExpression { + public static class PathStyleExpression extends SimpleExpression implements Expander { PathStyleExpression(String name, String pattern) { super(name, pattern, ";", true); @@ -192,5 +193,9 @@ protected String expand(Object variable, boolean encode) { return this.separator + super.expand(variable, encode); } + @Override + public String expand(Object value) { + return this.expand(value, true); + } } } From 82413fa4fcfdc14f3595e2a72d3ceb02ee138d34 Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Mon, 8 Nov 2021 15:18:05 -0500 Subject: [PATCH 3/4] Added example to ReadMe --- README.md | 32 +++++++++++++++++++ .../java/feign/template/UriTemplateTest.java | 6 ++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 02d5ab155c..7166770eb4 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,38 @@ resolved values. *Example* `owner` must be alphabetic. `{owner:[a-zA-Z]*}` * Unresolved expressions are omitted. * All literals and variable values are pct-encoded, if not already encoded or marked `encoded` via a `@Param` annotation. +We also have limited support for Level 3, Path Style Expressions, with the following restrictions: + +* Maps and Lists are expanded by default. +* Only Single variable templates are supported. + +*Examples:* + +``` +{;who} ;who=fred +{;half} ;half=50%25 +{;empty} ;empty +{;list} ;list=red;list=green;list=blue +{;map} ;semi=%3B;dot=.;comma=%2C +``` + +```java +public interface MatrixService { + + @RequestLine("GET /repos{;owners}") + List contributors(@Param("owners") List owners); + + class Contributor { + String login; + int contributions; + } +} +``` + +If `owners` in the above example is defined as `Matt, Jeff, Susan`, the uri will expand to `/repos;owners=Matt;owners=Jeff;owners=Susan` + +For more information see [RFC 6570, Section 3.2.7](https://datatracker.ietf.org/doc/html/rfc6570#section-3.2.7) + #### Undefined vs. Empty Values #### Undefined expressions are expressions where the value for the expression is an explicit `null` or no value is provided. diff --git a/core/src/test/java/feign/template/UriTemplateTest.java b/core/src/test/java/feign/template/UriTemplateTest.java index 395db442ef..0a589a36be 100644 --- a/core/src/test/java/feign/template/UriTemplateTest.java +++ b/core/src/test/java/feign/template/UriTemplateTest.java @@ -304,7 +304,7 @@ public void encodeReserved() { @Test public void pathStyleExpansionSupported() { String template = "{;who}"; - UriTemplate uriTemplate= UriTemplate.create(template, Util.UTF_8); + UriTemplate uriTemplate = UriTemplate.create(template, Util.UTF_8); String expanded = uriTemplate.expand(Collections.singletonMap("who", "fred")); assertThat(expanded).isEqualToIgnoringCase(";who=fred"); } @@ -312,7 +312,7 @@ public void pathStyleExpansionSupported() { @Test public void pathStyleExpansionEncodesReservedCharacters() { String template = "{;half}"; - UriTemplate uriTemplate= UriTemplate.create(template, Util.UTF_8); + UriTemplate uriTemplate = UriTemplate.create(template, Util.UTF_8); String expanded = uriTemplate.expand(Collections.singletonMap("half", "50%")); assertThat(expanded).isEqualToIgnoringCase(";half=50%25"); } @@ -320,7 +320,7 @@ public void pathStyleExpansionEncodesReservedCharacters() { @Test public void pathStyleExpansionSupportedWithLists() { String template = "{;list}"; - UriTemplate uriTemplate= UriTemplate.create(template, Util.UTF_8); + UriTemplate uriTemplate = UriTemplate.create(template, Util.UTF_8); List values = new ArrayList<>(); values.add("red"); From c3cc97d05339c74b9dd6b2d84ac9afca82fbf9a4 Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Mon, 8 Nov 2021 15:40:51 -0500 Subject: [PATCH 4/4] Additional Test Cases. --- .../main/java/feign/template/Expressions.java | 10 ++++- core/src/test/java/feign/FeignTest.java | 41 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/feign/template/Expressions.java b/core/src/main/java/feign/template/Expressions.java index 72b80f9c9b..1f7b891979 100644 --- a/core/src/main/java/feign/template/Expressions.java +++ b/core/src/main/java/feign/template/Expressions.java @@ -184,7 +184,7 @@ protected String expandMap(Map values) { public static class PathStyleExpression extends SimpleExpression implements Expander { - PathStyleExpression(String name, String pattern) { + public PathStyleExpression(String name, String pattern) { super(name, pattern, ";", true); } @@ -197,5 +197,13 @@ protected String expand(Object variable, boolean encode) { public String expand(Object value) { return this.expand(value, true); } + + @Override + public String getValue() { + if (this.getPattern() != null) { + return "{" + this.separator + this.getName() + ":" + this.getName() + "}"; + } + return "{" + this.separator + this.getName() + "}"; + } } } diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java index ad7e5deea7..7bf98cb7d8 100644 --- a/core/src/test/java/feign/FeignTest.java +++ b/core/src/test/java/feign/FeignTest.java @@ -642,6 +642,7 @@ public void whenReturnTypeIsResponseNoErrorHandling() { } private static class MockRetryer implements Retryer { + boolean tripped; @Override @@ -905,6 +906,39 @@ public void beanQueryMapEncoderWithEmptyParams() throws Exception { .hasQueryParams("/"); } + @Test + public void matrixParametersAreSupported() throws Exception { + TestInterface api = new TestInterfaceBuilder() + .target("http://localhost:" + server.getPort()); + + server.enqueue(new MockResponse()); + + List owners = new ArrayList<>(); + owners.add("Mark"); + owners.add("Jeff"); + owners.add("Susan"); + api.matrixParameters(owners); + assertThat(server.takeRequest()) + .hasPath("/owners;owners=Mark;owners=Jeff;owners=Susan"); + + } + + @Test + public void matrixParametersAlsoSupportMaps() throws Exception { + TestInterface api = new TestInterfaceBuilder() + .target("http://localhost:" + server.getPort()); + + server.enqueue(new MockResponse()); + Map properties = new LinkedHashMap<>(); + properties.put("account", "a"); + properties.put("name", "n"); + + api.matrixParametersWithMap(properties); + assertThat(server.takeRequest()) + .hasPath("/settings;account=a;name=n"); + + } + interface TestInterface { @RequestLine("POST /") @@ -990,6 +1024,12 @@ void queryMapWithQueryParams(@Param("name") String name, @RequestLine("GET /") void queryMapPropertyInheritence(@QueryMap ChildPojo object); + @RequestLine("GET /owners{;owners}") + void matrixParameters(@Param("owners") List owners); + + @RequestLine("GET /settings{;props}") + void matrixParametersWithMap(@Param("props") Map owners); + class DateToMillis implements Param.Expander { @Override @@ -1000,6 +1040,7 @@ public String expand(Object value) { } class TestInterfaceException extends Exception { + TestInterfaceException(String message) { super(message); }