fix(form): correctly handle Map and POJO types

form/src/main/java/feign/form/util/PojoUtil.java

@@ -21,6 +21,7 @@
 
 import feign.form.FormProperty;
 import java.lang.reflect.Field;
+import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.rmi.UnexpectedException;
 import java.security.PrivilegedAction;
@@ -41,14 +42,30 @@
 public final class PojoUtil {
 
   public static boolean isUserPojo(@NonNull Object object) {
-    val type = object.getClass();
-    val packageName = type.getPackage().getName();
-    return !packageName.startsWith("java.");
+    return isUserPojo(object.getClass());
   }
 
   public static boolean isUserPojo(@NonNull Type type) {
-    val typeName = type.toString();
-    return !typeName.startsWith("class java.");
+    if (type instanceof Class<?>) {
+      return isUserPojo((Class<?>) type);
+    }
+
+    if (type instanceof ParameterizedType) {
+      ParameterizedType parameterizedType = (ParameterizedType) type;
+      return isUserPojo(parameterizedType.getRawType());
+    }
+
+    return false;
+  }
+
+  private static boolean isUserPojo(@NonNull Class<?> type) {
+    if (type.isPrimitive() || type.isArray()) {
+      return false;
+    }
+
+    Package pkg = type.getPackage();
+
+    return pkg != null && !pkg.getName().startsWith("java.");
   }
 
   @SneakyThrows

form/src/test/java/feign/form/WildCardMapTest.java

@@ -17,36 +17,34 @@
 
 import static feign.Logger.Level.FULL;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.DEFINED_PORT;
 
 import feign.Feign;
 import feign.Headers;
 import feign.Logger.JavaLogger;
 import feign.RequestLine;
 import feign.Response;
-import java.nio.file.Path;
+import feign.codec.EncodeException;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.io.TempDir;
 import org.springframework.boot.test.context.SpringBootTest;
 
 @SpringBootTest(webEnvironment = DEFINED_PORT, classes = Server.class)
 class WildCardMapTest {
 
   private static FormUrlEncodedApi api;
 
-  @TempDir static Path logDir;
-
   @BeforeAll
   static void configureClient() {
-    var logFile = logDir.resolve("log.txt").toString();
-
     api =
         Feign.builder()
             .encoder(new FormEncoder())
-            .logger(new JavaLogger(WildCardMapTest.class).appendToFile(logFile))
+            .logger(new JavaLogger(WildCardMapTest.class))
             .logLevel(FULL)
             .target(FormUrlEncodedApi.class, "http://localhost:8080");
   }
@@ -83,10 +81,39 @@ void testBadRequest() {
     assertThat(api.wildCardMap(param)).isNotNull().extracting(Response::status).isEqualTo(418);
   }
 
+  @Test
+  void testMapStringString() {
+    Map<String, String> param = new HashMap<>();
+
+    param.put("key1", "1");
+    param.put("key2", "1");
+
+    assertThat(api.mapStringString(param)).isNotNull().extracting(Response::status).isEqualTo(200);
+  }
+
+  @Test
+  void testListIsDelegatedToDefaultEncoder() {
+    List<String> param = new ArrayList<>();
+    param.add("key1");
+    param.add("key2");
+
+    assertThatThrownBy(() -> api.list(param))
+        .isInstanceOf(EncodeException.class)
+        .hasMessageContaining("ArrayList is not a type supported by this encoder.");
+  }
+
   interface FormUrlEncodedApi {
 
     @RequestLine("POST /wild-card-map")
     @Headers("Content-Type: application/x-www-form-urlencoded")
     Response wildCardMap(Map<String, ?> param);
+
+    @RequestLine("POST /wild-card-map")
+    @Headers("Content-Type: application/x-www-form-urlencoded")
+    Response mapStringString(Map<String, String> param);
+
+    @RequestLine("POST /wild-card-map")
+    @Headers("Content-Type: application/x-www-form-urlencoded")
+    Response list(List<String> param);
   }
 }

form/src/test/java/feign/form/util/PojoUtilTest.java

@@ -0,0 +1,243 @@
+/*
+ * Copyright © 2012 The Feign Authors (feign@commonhaus.dev)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package feign.form.util;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import feign.form.FormProperty;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.junit.jupiter.api.Test;
+
+class PojoUtilTest {
+
+  @Test
+  void shouldIdentifyUserPojoFromObject() {
+    var pojo = new UserPojo();
+
+    assertThat(PojoUtil.isUserPojo(pojo)).isTrue();
+  }
+
+  @Test
+  void shouldNotIdentifyJavaObjectAsUserPojo() {
+    var object = new HashMap<String, Object>();
+
+    assertThat(PojoUtil.isUserPojo(object)).isFalse();
+  }
+
+  @Test
+  void shouldIdentifyUserPojoFromClassType() {
+    Type type = UserPojo.class;
+
+    assertThat(PojoUtil.isUserPojo(type)).isTrue();
+  }
+
+  @Test
+  void shouldNotIdentifyJavaClassAsUserPojo() {
+    Type type = HashMap.class;
+
+    assertThat(PojoUtil.isUserPojo(type)).isFalse();
+  }
+
+  @Test
+  void shouldConvertPojoFieldsToMap() {
+    var pojo = new UserPojo();
+    pojo.name = "Eduardo";
+    pojo.age = 30;
+
+    assertThat(PojoUtil.toMap(pojo))
+        .containsExactlyInAnyOrderEntriesOf(Map.of("custom_name", "Eduardo", "age", 30));
+  }
+
+  @Test
+  void shouldIgnoreNullFields() {
+    var pojo = new UserPojo();
+    pojo.name = "Eduardo";
+
+    assertThat(PojoUtil.toMap(pojo)).containsExactly(Map.entry("custom_name", "Eduardo"));
+  }
+
+  @Test
+  void shouldIgnoreStaticFields() {
+    var pojo = new UserPojo();
+    pojo.name = "Eduardo";
+
+    assertThat(PojoUtil.toMap(pojo)).doesNotContainKey("staticField");
+  }
+
+  @Test
+  void shouldIgnoreFinalFields() {
+    var pojo = new UserPojo();
+    pojo.name = "Eduardo";
+
+    assertThat(PojoUtil.toMap(pojo)).doesNotContainKey("finalField");
+  }
+
+  @Test
+  void shouldUseFormPropertyAsMapKey() {
+    var pojo = new UserPojo();
+    pojo.name = "Eduardo";
+
+    assertThat(PojoUtil.toMap(pojo))
+        .containsEntry("custom_name", "Eduardo")
+        .doesNotContainKey("name");
+  }
+
+  @Test
+  void shouldReadPrivateFields() {
+    var pojo = new PrivateFieldsPojo("Eduardo");
+
+    assertThat(PojoUtil.toMap(pojo)).containsEntry("name", "Eduardo");
+  }
+
+  @Test
+  void shouldNotConvertInheritedFields() {
+    var pojo = new ChildPojo();
+    pojo.child = "child";
+
+    assertThat(PojoUtil.toMap(pojo)).containsEntry("child", "child").doesNotContainKey("parent");
+  }
+
+  @Test
+  void shouldReturnEmptyMapWhenPojoHasNoEligibleFields() {
+    var pojo = new EmptyPojo();
+
+    assertThat(PojoUtil.toMap(pojo)).isEmpty();
+  }
+
+  @Test
+  void shouldNotIdentifyParameterizedMapAsUserPojo() {
+    Type type = new TypeReference<Map<String, String>>() {}.getType();
+
+    assertThat(PojoUtil.isUserPojo(type)).isFalse();
+  }
+
+  @Test
+  void shouldNotIdentifyParameterizedListAsUserPojo() {
+    Type type = new TypeReference<List<String>>() {}.getType();
+
+    assertThat(PojoUtil.isUserPojo(type)).isFalse();
+  }
+
+  @Test
+  void shouldNotIdentifyParameterizedHashMapAsUserPojo() {
+    Type type = new TypeReference<HashMap<String, String>>() {}.getType();
+
+    assertThat(PojoUtil.isUserPojo(type)).isFalse();
+  }
+
+  @Test
+  void shouldNotIdentifyParameterizedCollectionAsUserPojo() {
+    Type type = new TypeReference<Collection<String>>() {}.getType();
+
+    assertThat(PojoUtil.isUserPojo(type)).isFalse();
+  }
+
+  @Test
+  void shouldIdentifyParameterizedUserPojoAsUserPojo() {
+    Type type = new TypeReference<UserPojo<String>>() {}.getType();
+
+    assertThat(PojoUtil.isUserPojo(type)).isTrue();
+  }
+
+  @Test
+  void shouldNotIdentifyTypeVariableAsUserPojo() {
+    Type type = UserPojo.class.getTypeParameters()[0];
+
+    assertThat(PojoUtil.isUserPojo(type)).isFalse();
+  }
+
+  @Test
+  void shouldNotIdentifyWildcardTypeAsUserPojo() {
+    Type type = new TypeReference<List<? extends UserPojo>>() {}.getType();
+
+    Type wildcard = ((ParameterizedType) type).getActualTypeArguments()[0];
+
+    assertThat(PojoUtil.isUserPojo(wildcard)).isFalse();
+  }
+
+  @Test
+  void shouldNotIdentifyPrimitiveAsUserPojo() {
+    assertThat(PojoUtil.isUserPojo(int.class)).isFalse();
+  }
+
+  @Test
+  void shouldNotIdentifyArrayAsUserPojo() {
+    assertThat(PojoUtil.isUserPojo(byte[].class)).isFalse();
+  }
+
+  @Test
+  void shouldNotIdentifyObjectArrayAsUserPojo() {
+    assertThat(PojoUtil.isUserPojo(String[].class)).isFalse();
+  }
+
+  static class UserPojo<T> {
+
+    @FormProperty("custom_name")
+    private String name;
+
+    private Integer age;
+
+    private T value;
+
+    private static String staticField;
+
+    private final String finalField = "ignored";
+  }
+
+  private static class PrivateFieldsPojo {
+
+    private String name;
+
+    private PrivateFieldsPojo(String name) {
+      this.name = name;
+    }
+  }
+
+  static class ParentPojo {
+
+    private String parent;
+  }
+
+  static class ChildPojo extends ParentPojo {
+
+    private String child;
+  }
+
+  static class EmptyPojo {
+
+    private static final String STATIC = "ignored";
+
+    private final String FINAL = "ignored";
+  }
+
+  private abstract static class TypeReference<T> {
+
+    private final Type type;
+
+    protected TypeReference() {
+      type = ((ParameterizedType) getClass().getGenericSuperclass()).getActualTypeArguments()[0];
+    }
+
+    Type getType() {
+      return type;
+    }
+  }
+}