Added possibility to leave slash encoded in path parameters

unknown · Adrian Cole · commit e32cdf48581f · 2015-06-18T19:11:21.000-07:00
diff --git a/core/src/main/java/feign/Contract.java b/core/src/main/java/feign/Contract.java
@@ -166,6 +166,9 @@ protected void processAnnotationOnMethod(MethodMetadata data, Annotation methodA
           data.template().append(
               requestLine.substring(requestLine.indexOf(' ') + 1, requestLine.lastIndexOf(' ')));
         }
+        
+        data.template().decodeSlash(RequestLine.class.cast(methodAnnotation).decodeSlash());
+        
       } else if (annotationType == Body.class) {
         String body = Body.class.cast(methodAnnotation).value();
         checkState(emptyToNull(body) != null, "Body annotation was empty on method %s.",
diff --git a/core/src/main/java/feign/RequestLine.java b/core/src/main/java/feign/RequestLine.java
@@ -47,4 +47,5 @@
 public @interface RequestLine {
 
   String value();
+  boolean decodeSlash() default true;
 }
diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java
@@ -56,6 +56,7 @@ public final class RequestTemplate implements Serializable {
   private transient Charset charset;
   private byte[] body;
   private String bodyTemplate;
+  private boolean decodeSlash = true;
 
   public RequestTemplate() {
 
@@ -71,6 +72,7 @@ public RequestTemplate(RequestTemplate toCopy) {
     this.charset = toCopy.charset;
     this.body = toCopy.body;
     this.bodyTemplate = toCopy.bodyTemplate;
+    this.decodeSlash = toCopy.decodeSlash;
   }
 
   private static String urlDecode(String arg) {
@@ -200,7 +202,10 @@ public RequestTemplate resolve(Map<String, ?> unencoded) {
     for (Entry<String, ?> entry : unencoded.entrySet()) {
       encoded.put(entry.getKey(), urlEncode(String.valueOf(entry.getValue())));
     }
-    String resolvedUrl = expand(url.toString(), encoded).replace("%2F", "/").replace("+", "%20");
+    String resolvedUrl = expand(url.toString(), encoded).replace("+", "%20");
+    if (decodeSlash) {
+    	resolvedUrl = resolvedUrl.replace("%2F", "/");
+    }
     url = new StringBuilder(resolvedUrl);
 
     Map<String, Collection<String>>
@@ -246,12 +251,21 @@ public RequestTemplate method(String method) {
     this.method = checkNotNull(method, "method");
     return this;
   }
-
+  
   /* @see Request#method() */
   public String method() {
     return method;
   }
 
+  public RequestTemplate decodeSlash(boolean decodeSlash) {
+	  this.decodeSlash = decodeSlash;
+	  return this;
+  }
+  
+  public boolean decodeSlash() {
+	  return decodeSlash;
+  }
+  
   /* @see #url() */
   public RequestTemplate append(CharSequence value) {
     url.append(value);
diff --git a/core/src/test/java/feign/DefaultContractTest.java b/core/src/test/java/feign/DefaultContractTest.java
@@ -293,6 +293,22 @@ public void customExpander() throws Exception {
         .containsExactly(entry(0, DateToMillis.class));
   }
 
+  @Test
+  public void slashAreEncodedWhenNeeded() throws Exception {
+    MethodMetadata
+        md =
+        contract.parseAndValidatateMetadata(
+            SlashNeedToBeEncoded.class.getDeclaredMethod("getQueues", String.class));
+
+    assertThat(md.template().decodeSlash()).isFalse();
+
+    md = contract.parseAndValidatateMetadata(
+        SlashNeedToBeEncoded.class.getDeclaredMethod("getZone", String.class));
+
+    assertThat(md.template().decodeSlash()).isTrue();
+
+  }
+
   interface Methods {
 
     @RequestLine("POST /")
@@ -405,4 +421,12 @@ public String expand(Object value) {
       return String.valueOf(((Date) value).getTime());
     }
   }
+
+  interface SlashNeedToBeEncoded {
+    @RequestLine(value = "GET /api/queues/{vhost}", decodeSlash = false)
+    String getQueues(@Param("vhost") String vhost);
+
+    @RequestLine("GET /api/{zoneId}")
+    String getZone(@Param("ZoneId") String vhost);
+  }
 }
diff --git a/core/src/test/java/feign/FeignBuilderTest.java b/core/src/test/java/feign/FeignBuilderTest.java
@@ -185,6 +185,21 @@ public InvocationHandler create(Target target, Map<Method, MethodHandler> dispat
     assertThat(server.takeRequest())
         .hasBody("request data");
   }
+  
+  @Test
+  public void testSlashIsEncodedInPathParams() throws Exception {
+    server.enqueue(new MockResponse().setBody("response data"));
+
+    String url = "http://localhost:" + server.getPort();
+
+    TestInterface
+        api =
+        Feign.builder().target(TestInterface.class, url);
+    api.getQueues("/");
+
+    assertThat(server.takeRequest())
+        .hasPath("/api/queues/%2F");
+  }
 
   interface TestInterface {
     @RequestLine("GET")
@@ -201,5 +216,8 @@ interface TestInterface {
 
     @RequestLine("POST /")
     String decodedPost();
+    
+    @RequestLine(value = "GET /api/queues/{vhost}", decodeSlash = false)
+    String getQueues(@Param("vhost") String vhost);
   }
 }
diff --git a/core/src/test/java/feign/RequestTemplateTest.java b/core/src/test/java/feign/RequestTemplateTest.java
@@ -241,4 +241,16 @@ public void spaceEncodingInUrlParam() {
     assertThat(template.request().url())
         .isEqualTo("/api/ABC%20123?key=XYZ+123");
   }
+
+  @Test
+  public void encodeSlashTest() throws Exception {
+    RequestTemplate template = new RequestTemplate().method("GET")
+	    .append("/api/{vhost}")
+	    .decodeSlash(false);
+
+    template.resolve(mapOf("vhost", "/"));
+
+    assertThat(template)
+        .hasUrl("/api/%2F");
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -47,4 +47,5 @@`
`47`	`47`	`public @interface RequestLine {`
`48`	`48`
`49`	`49`	`String value();`
	`50`	`+ boolean decodeSlash() default true;`
`50`	`51`	`}`