Merge pull request square#307 from square/jw/fix-encoded-path-cast

Ensure @EncodedPath is not cast to @path when processing method.

retrofit/src/main/java/retrofit/RestMethodInfo.java

@@ -308,34 +308,25 @@ private void parseParameters() {
         for (Annotation parameterAnnotation : parameterAnnotations) {
           Class<? extends Annotation> annotationType = parameterAnnotation.annotationType();
 
-          if (annotationType == Path.class || annotationType == EncodedPath.class) {
+          if (annotationType == Path.class) {
             String name = ((Path) parameterAnnotation).value();
+            validatePathName(name);
 
-            if (!PARAM_NAME_REGEX.matcher(name).matches()) {
-              throw new IllegalStateException("Path parameter name is not valid: "
-                  + name
-                  + ". Must match "
-                  + PARAM_URL_REGEX.pattern());
-            }
-            // Verify URL replacement name is actually present in the URL path.
-            if (!requestUrlParamNames.contains(name)) {
-              throw new IllegalStateException(
-                  "Method URL \"" + requestUrl + "\" does not contain {" + name + "}.");
-            }
+            paramNames[i] = name;
+            paramUsage[i] = ParamUsage.PATH;
+          } else if (annotationType == EncodedPath.class) {
+            String name = ((EncodedPath) parameterAnnotation).value();
+            validatePathName(name);
 
             paramNames[i] = name;
-            if (annotationType == EncodedPath.class) {
-              paramUsage[i] = ParamUsage.ENCODED_PATH;
-            } else {
-              paramUsage[i] = ParamUsage.PATH;
-            }
+            paramUsage[i] = ParamUsage.ENCODED_PATH;
           } else if (annotationType == Query.class) {
             String name = ((Query) parameterAnnotation).value();
 
             paramNames[i] = name;
             paramUsage[i] = ParamUsage.QUERY;
           } else if (annotationType == EncodedQuery.class) {
-            String name = ((Query) parameterAnnotation).value();
+            String name = ((EncodedQuery) parameterAnnotation).value();
 
             paramNames[i] = name;
             paramUsage[i] = ParamUsage.ENCODED_QUERY;
@@ -402,6 +393,20 @@ private void parseParameters() {
     }
   }
 
+  private void validatePathName(String name) {
+    if (!PARAM_NAME_REGEX.matcher(name).matches()) {
+      throw new IllegalStateException("Path parameter name is not valid: "
+          + name
+          + ". Must match "
+          + PARAM_URL_REGEX.pattern());
+    }
+    // Verify URL replacement name is actually present in the URL path.
+    if (!requestUrlParamNames.contains(name)) {
+      throw new IllegalStateException(
+          "Method URL \"" + requestUrl + "\" does not contain {" + name + "}.");
+    }
+  }
+
   /**
    * Gets the set of unique path parameters used in the given URI. If a parameter is used twice
    * in the URI, it will only show up once in the set.

retrofit/src/test/java/retrofit/RestMethodInfoTest.java

@@ -13,6 +13,8 @@
 import org.junit.Test;
 import retrofit.http.Body;
 import retrofit.http.DELETE;
+import retrofit.http.EncodedPath;
+import retrofit.http.EncodedQuery;
 import retrofit.http.Field;
 import retrofit.http.FormUrlEncoded;
 import retrofit.http.GET;
@@ -34,6 +36,8 @@
 import static org.fest.assertions.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 import static retrofit.RestMethodInfo.ParamUsage.BODY;
+import static retrofit.RestMethodInfo.ParamUsage.ENCODED_PATH;
+import static retrofit.RestMethodInfo.ParamUsage.ENCODED_QUERY;
 import static retrofit.RestMethodInfo.ParamUsage.HEADER;
 import static retrofit.RestMethodInfo.ParamUsage.PATH;
 import static retrofit.RestMethodInfo.ParamUsage.QUERY;
@@ -95,7 +99,7 @@ class Example {
 
   @Test public void concreteCallbackTypesWithParams() {
     class Example {
-      @GET("/foo") void a(@Path("id") String id, ResponseCallback cb) {
+      @GET("/foo") void a(@Query("id") String id, ResponseCallback cb) {
       }
     }
 
@@ -119,7 +123,7 @@ class Example {
 
   @Test public void genericCallbackTypesWithParams() {
     class Example {
-      @GET("/foo") void a(@Path("id") String id, Callback<Response> c) {
+      @GET("/foo") void a(@Query("id") String id, Callback<Response> c) {
       }
     }
 
@@ -203,7 +207,7 @@ class Example {
   @Test(expected = IllegalArgumentException.class)
   public void missingCallbackTypes() {
     class Example {
-      @GET("/foo") void a(@Path("id") String id) {
+      @GET("/foo") void a(@Query("id") String id) {
       }
     }
 
@@ -408,6 +412,38 @@ class Example {
     assertThat(methodInfo.requestType).isEqualTo(SIMPLE);
   }
 
+  @Test public void singlePathParam() {
+    class Example {
+      @GET("/{a}") Response a(@Path("a") String a) {
+        return null;
+      }
+    }
+
+    Method method = TestingUtils.getMethod(Example.class, "a");
+    RestMethodInfo methodInfo = new RestMethodInfo(method);
+    methodInfo.init();
+
+    assertThat(methodInfo.requestParamNames).hasSize(1).containsExactly("a");
+    assertThat(methodInfo.requestParamUsage).hasSize(1).containsExactly(PATH);
+    assertThat(methodInfo.requestType).isEqualTo(SIMPLE);
+  }
+
+  @Test public void singleEncodedPathParam() {
+    class Example {
+      @GET("/{a}") Response a(@EncodedPath("a") String a) {
+        return null;
+      }
+    }
+
+    Method method = TestingUtils.getMethod(Example.class, "a");
+    RestMethodInfo methodInfo = new RestMethodInfo(method);
+    methodInfo.init();
+
+    assertThat(methodInfo.requestParamNames).hasSize(1).containsExactly("a");
+    assertThat(methodInfo.requestParamUsage).hasSize(1).containsExactly(ENCODED_PATH);
+    assertThat(methodInfo.requestType).isEqualTo(SIMPLE);
+  }
+
   @Test public void singleQueryParam() {
     class Example {
       @GET("/") Response a(@Query("a") String a) {
@@ -424,6 +460,22 @@ class Example {
     assertThat(methodInfo.requestType).isEqualTo(SIMPLE);
   }
 
+  @Test public void singleEncodedQueryParam() {
+    class Example {
+      @GET("/") Response a(@EncodedQuery("a") String a) {
+        return null;
+      }
+    }
+
+    Method method = TestingUtils.getMethod(Example.class, "a");
+    RestMethodInfo methodInfo = new RestMethodInfo(method);
+    methodInfo.init();
+
+    assertThat(methodInfo.requestParamNames).hasSize(1).containsExactly("a");
+    assertThat(methodInfo.requestParamUsage).hasSize(1).containsExactly(ENCODED_QUERY);
+    assertThat(methodInfo.requestType).isEqualTo(SIMPLE);
+  }
+
   @Test public void multipleQueryParams() {
     class Example {
       @GET("/") Response a(@Query("a") String a, @Query("b") String b, @Query("c") String c) {