[GH-1319] Add Support for Path Style Parameter Expansion (#1537)

* [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
```

* Export Path Style Expression as an Expander for use with custom contracts

* Added example to ReadMe

* Additional Test Cases.

Author: kdavisk6

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<Contributor> contributors(@Param("owners") List<String> 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.

core/src/main/java/feign/template/Expressions.java

@@ -13,32 +13,18 @@
  */
 package feign.template;
 
+import feign.Param.Expander;
+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<Pattern, Class<? extends Expression>> 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 +34,15 @@ public static Expression create(final String value) {
       throw new IllegalArgumentException("an expression is required.");
     }
 
-    Optional<Entry<Pattern, Class<? extends Expression>>> matchedExpressionEntry =
-        expressions.entrySet()
-            .stream()
-            .filter(entry -> entry.getKey().matcher(expression).matches())
-            .findFirst();
-
-    if (!matchedExpressionEntry.isPresent()) {
-      /* not a valid expression */
-      return null;
-    }
-
-    Entry<Pattern, Class<? extends Expression>> 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 +59,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 +84,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<String, ?>) variable));
       } else {
+        if (this.nameRequired) {
+          expanded.append(this.encode(this.getName()))
+              .append("=");
+        }
         expanded.append((encode) ? encode(variable) : variable);
       }
 
@@ -128,8 +127,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,19 +139,71 @@ 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);
         }
       }
 
       /* return the expanded value */
       return result.toString();
     }
+
+    protected String expandMap(Map<String, ?> values) {
+      StringBuilder result = new StringBuilder();
+
+      for (Entry<String, ?> 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();
+    }
+  }
+
+  public static class PathStyleExpression extends SimpleExpression implements Expander {
+
+    public PathStyleExpression(String name, String pattern) {
+      super(name, pattern, ";", true);
+    }
+
+    @Override
+    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);
+    }
+
+    @Override
+    public String getValue() {
+      if (this.getPattern() != null) {
+        return "{" + this.separator + this.getName() + ":" + this.getName() + "}";
+      }
+      return "{" + this.separator + this.getName() + "}";
+    }
   }
 }

core/src/test/java/feign/FeignTest.java

@@ -689,6 +689,7 @@ public void whenReturnTypeIsResponseNoErrorHandling() {
   }
 
   private static class MockRetryer implements Retryer {
+
     boolean tripped;
 
     @Override
@@ -952,6 +953,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<String> 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<String, Object> 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 /")
@@ -1037,6 +1071,12 @@ void queryMapWithQueryParams(@Param("name") String name,
     @RequestLine("GET /")
     void queryMapPropertyInheritence(@QueryMap ChildPojo object);
 
+    @RequestLine("GET /owners{;owners}")
+    void matrixParameters(@Param("owners") List<String> owners);
+
+    @RequestLine("GET /settings{;props}")
+    void matrixParametersWithMap(@Param("props") Map<String, Object> owners);
+
     class DateToMillis implements Param.Expander {
 
       @Override
@@ -1070,6 +1110,7 @@ public void setGrade(String grade) {
   }
 
   class TestInterfaceException extends Exception {
+
     TestInterfaceException(String message) {
       super(message);
     }

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<String> 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<String, Object> parameters = new LinkedHashMap<>();
+    parameters.put("account", "a");
+    parameters.put("name", "n");
+
+    Map<String, Object> 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");
+  }
 }