Fixes Map Based Parameter Checking for Generic Subclasses (OpenFeign#689)

kdavisk6 · velo · commit 7ce0727e0994 · 2018-05-01T08:33:59.000+12:00
Fixes OpenFeign#665 When verifying that any of th `@*Map` annotations are in fact `Map` instances, we were assumping that all values are direct extension of a `Map` with generic type information intact. When using frameworks like Spring, it is possible to have `Map` objects that do not expose type information, like `HttpHeaders`, which directly extend from a `Map` with the type information static. This added additional checking to the `checkMapKeys` function to accomodate for `Map` subclasses without type information. If the map key information cannot be validated, we simply pass it through.
diff --git a/core/src/main/java/feign/Contract.java b/core/src/main/java/feign/Contract.java
@@ -138,12 +138,35 @@ private static void checkMapString(String name, Class<?> type, Type genericType)
     }
 
     private static void checkMapKeys(String name, Type genericType) {
-      Type[] parameterTypes = ((ParameterizedType) genericType).getActualTypeArguments();
-      Class<?> keyClass = (Class<?>) parameterTypes[0];
-      checkState(String.class.equals(keyClass),
-              "%s key must be a String: %s", name, keyClass.getSimpleName());
+      Class<?> keyClass = null;
+
+      // assume our type parameterized
+      if (ParameterizedType.class.isAssignableFrom(genericType.getClass())) {
+        Type[] parameterTypes = ((ParameterizedType) genericType).getActualTypeArguments();
+        keyClass = (Class<?>) parameterTypes[0];
+      } else if (genericType instanceof Class<?>) {
+        // raw class, type parameters cannot be inferred directly, but we can scan any extended
+        // interfaces looking for any explict types
+        Type[] interfaces = ((Class) genericType).getGenericInterfaces();
+        if (interfaces != null) {
+          for (Type extended : interfaces) {
+            if (ParameterizedType.class.isAssignableFrom(extended.getClass())) {
+              // use the first extended interface we find.
+              Type[] parameterTypes = ((ParameterizedType) extended).getActualTypeArguments();
+              keyClass = (Class<?>) parameterTypes[0];
+              break;
+            }
+          }
+        }
+      }
+
+      if (keyClass != null) {
+        checkState(String.class.equals(keyClass),
+            "%s key must be a String: %s", name, keyClass.getSimpleName());
+      }
     }
 
+
     /**
      * Called by parseAndValidateMetadata twice, first on the declaring class, then on the
      * target type (unless they are the same).
diff --git a/core/src/test/java/feign/DefaultContractTest.java b/core/src/test/java/feign/DefaultContractTest.java
@@ -371,6 +371,12 @@ public void onlyOneHeaderMapAnnotationPermitted() throws Exception {
     }
   }
 
+  @Test
+  public void headerMapSubclass() throws Exception {
+    MethodMetadata md = parseAndValidateMetadata(HeaderMapInterface.class, "headerMapSubClass", SubClassHeaders.class);
+    assertThat(md.headerMapIndex()).isEqualTo(0);
+  }
+
   interface Methods {
 
     @RequestLine("POST /")
@@ -470,6 +476,9 @@ interface HeaderMapInterface {
 
     @RequestLine("POST /")
     void multipleHeaderMap(@HeaderMap Map<String, String> headers, @HeaderMap Map<String,String> queries);
+
+    @RequestLine("POST /")
+    void headerMapSubClass(@HeaderMap SubClassHeaders httpHeaders);
   }
 
   interface HeaderParams {
@@ -627,6 +636,12 @@ static class Entities<K, M> {
     private List<Entity<K, M>> entities;
   }
 
+
+  interface SubClassHeaders extends Map<String, String> {
+
+  }
+
+
   @Headers("Version: 1")
   interface ParameterizedApi extends ParameterizedBaseApi<String, Long> {
 
