diff --git a/form/src/main/java/feign/form/util/PojoUtil.java b/form/src/main/java/feign/form/util/PojoUtil.java index 195d176704..795779db34 100644 --- a/form/src/main/java/feign/form/util/PojoUtil.java +++ b/form/src/main/java/feign/form/util/PojoUtil.java @@ -19,6 +19,7 @@ import static java.lang.reflect.Modifier.isStatic; import static lombok.AccessLevel.PRIVATE; +import feign.Types; import feign.form.FormProperty; import java.lang.reflect.Field; import java.lang.reflect.Type; @@ -41,14 +42,12 @@ 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."); + val raw = Types.getRawType(type); + return !raw.isPrimitive() && !raw.isArray() && !raw.getName().startsWith("java."); } @SneakyThrows diff --git a/form/src/test/java/feign/form/WildCardMapTest.java b/form/src/test/java/feign/form/WildCardMapTest.java index 3dca096c8f..514b68d276 100644 --- a/form/src/test/java/feign/form/WildCardMapTest.java +++ b/form/src/test/java/feign/form/WildCardMapTest.java @@ -17,6 +17,7 @@ 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; @@ -24,8 +25,11 @@ import feign.Logger.JavaLogger; import feign.RequestLine; import feign.Response; +import feign.codec.EncodeException; import java.nio.file.Path; +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; @@ -83,10 +87,42 @@ void testBadRequest() { assertThat(api.wildCardMap(param)).isNotNull().extracting(Response::status).isEqualTo(418); } + @Test + void testMapStringStringIsFormUrlEncoded() { + Map formFields = new HashMap<>(); + + formFields.put("key1", "1"); + formFields.put("key2", "1"); + + assertThat(api.postFormFields(formFields)) + .isNotNull() + .extracting(Response::status) + .isEqualTo(200); + } + + @Test + void testListIsDelegatedToDefaultEncoder() { + List keys = new ArrayList<>(); + keys.add("key1"); + keys.add("key2"); + + assertThatThrownBy(() -> api.postFormList(keys)) + .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 param); + + @RequestLine("POST /wild-card-map") + @Headers("Content-Type: application/x-www-form-urlencoded") + Response postFormFields(Map formFields); + + @RequestLine("POST /wild-card-map") + @Headers("Content-Type: application/x-www-form-urlencoded") + Response postFormList(List keys); } } diff --git a/form/src/test/java/feign/form/util/PojoUtilTest.java b/form/src/test/java/feign/form/util/PojoUtilTest.java new file mode 100644 index 0000000000..43eac59fb6 --- /dev/null +++ b/form/src/test/java/feign/form/util/PojoUtilTest.java @@ -0,0 +1,247 @@ +/* + * 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 FormFieldsPojo(); + + assertThat(PojoUtil.isUserPojo(pojo)).isTrue(); + } + + @Test + void shouldNotIdentifyJavaObjectAsFormFieldsPojo() { + var javaHashMap = new HashMap(); + + assertThat(PojoUtil.isUserPojo(javaHashMap)).isFalse(); + } + + @Test + void shouldIdentifyUserPojoFromClassType() { + Type type = FormFieldsPojo.class; + + assertThat(PojoUtil.isUserPojo(type)).isTrue(); + } + + @Test + void shouldNotIdentifyJavaClassAsFormFieldsPojo() { + Type type = HashMap.class; + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldConvertPojoFieldsToMap() { + var pojo = new FormFieldsPojo(); + pojo.name = "Eduardo"; + pojo.age = 30; + + assertThat(PojoUtil.toMap(pojo)) + .containsExactlyInAnyOrderEntriesOf(Map.of("custom_name", "Eduardo", "age", 30)); + } + + @Test + void shouldIgnoreNullFields() { + var pojo = new FormFieldsPojo(); + pojo.name = "Eduardo"; + + assertThat(PojoUtil.toMap(pojo)).containsExactly(Map.entry("custom_name", "Eduardo")); + } + + @Test + void shouldIgnoreStaticFields() { + var pojo = new FormFieldsPojo(); + pojo.name = "Eduardo"; + + assertThat(PojoUtil.toMap(pojo)).doesNotContainKey("staticField"); + } + + @Test + void shouldIgnoreFinalFields() { + var pojo = new FormFieldsPojo(); + pojo.name = "Eduardo"; + + assertThat(PojoUtil.toMap(pojo)).doesNotContainKey("finalField"); + } + + @Test + void shouldUseFormPropertyAsMapKey() { + var pojo = new FormFieldsPojo(); + 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 shouldNotIdentifyParameterizedMapAsFormFieldsPojo() { + Type type = new TypeReference>() {}.getType(); + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldNotIdentifyParameterizedListAsFormFieldsPojo() { + Type type = new TypeReference>() {}.getType(); + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldNotIdentifyParameterizedHashMapAsFormFieldsPojo() { + Type type = new TypeReference>() {}.getType(); + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldNotIdentifyParameterizedCollectionAsFormFieldsPojo() { + Type type = new TypeReference>() {}.getType(); + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldIdentifyParameterizedUserPojoAsFormFieldsPojo() { + Type type = new TypeReference>() {}.getType(); + + assertThat(PojoUtil.isUserPojo(type)).isTrue(); + } + + @Test + void shouldNotIdentifyTypeVariableAsFormFieldsPojo() { + Type typeVariable = GenericPojo.class.getTypeParameters()[0]; + + assertThat(PojoUtil.isUserPojo(typeVariable)).isFalse(); + } + + @Test + void shouldIdentifyWildcardBoundedByUserPojoAsFormFieldsPojo() { + Type parameterizedListType = new TypeReference>() {}.getType(); + + Type wildcard = ((ParameterizedType) parameterizedListType).getActualTypeArguments()[0]; + + assertThat(PojoUtil.isUserPojo(wildcard)).isTrue(); + } + + @Test + void shouldNotIdentifyPrimitiveAsFormFieldsPojo() { + assertThat(PojoUtil.isUserPojo(int.class)).isFalse(); + } + + @Test + void shouldNotIdentifyByteArrayAsFormFieldsPojo() { + assertThat(PojoUtil.isUserPojo(byte[].class)).isFalse(); + } + + @Test + void shouldNotIdentifyObjectArrayAsFormFieldsPojo() { + assertThat(PojoUtil.isUserPojo(String[].class)).isFalse(); + } + + static class FormFieldsPojo { + + @FormProperty("custom_name") + private String name; + + private Integer age; + + private static String staticField; + + private final String finalField = "ignored"; + } + + static class GenericPojo { + + private T typeVariableField; + } + + 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 IGNORED_STATIC_FIELD = "ignored"; + + private final String ignoredFinalField = "ignored"; + } + + private abstract static class TypeReference { + + private final Type capturedType; + + protected TypeReference() { + capturedType = + ((ParameterizedType) getClass().getGenericSuperclass()).getActualTypeArguments()[0]; + } + + Type getType() { + return capturedType; + } + } +} diff --git a/spring/src/main/java/feign/spring/SpringContract.java b/spring/src/main/java/feign/spring/SpringContract.java index 4c44fbde9c..b2f0caa058 100755 --- a/spring/src/main/java/feign/spring/SpringContract.java +++ b/spring/src/main/java/feign/spring/SpringContract.java @@ -22,7 +22,6 @@ import feign.Request; import feign.Util; import java.lang.reflect.Parameter; -import java.lang.reflect.Type; import java.util.*; import org.springframework.web.bind.annotation.*; @@ -193,9 +192,8 @@ private String parameterName(String firstPriority, String secondPriority, Parame }; } - private boolean isUserPojo(Type type) { - String typeName = type.toString(); - return !typeName.startsWith("class java."); + private boolean isUserPojo(Class type) { + return !type.isPrimitive() && !type.isArray() && !type.getName().startsWith("java."); } private void appendMappings(MethodMetadata data, String[] mappings) { diff --git a/spring/src/test/java/feign/spring/SpringContractTest.java b/spring/src/test/java/feign/spring/SpringContractTest.java index 4f0277d70e..b45b00c4d2 100755 --- a/spring/src/test/java/feign/spring/SpringContractTest.java +++ b/spring/src/test/java/feign/spring/SpringContractTest.java @@ -36,6 +36,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.MissingResourceException; import java.util.Optional; @@ -85,6 +86,7 @@ void setup() throws IOException { .noContent(HttpMethod.GET, "/health/header") .noContent(HttpMethod.GET, "/health/header/map") .noContent(HttpMethod.GET, "/health/header/pojo") + .noContent(HttpMethod.GET, "/health/header/jdk") .ok(HttpMethod.GET, "/health/generic", "{}") .add(HttpMethod.POST, "/health/text", response); resource = @@ -236,6 +238,15 @@ void requestHeaderPojo() { assertThat(request.headers()).containsEntry("grade1", Arrays.asList("6")); } + @Test + void requestHeaderOfJdkTypeIsNotAHeaderMap() { + resource.checkRequestHeaderJdkTypes(Arrays.asList("a", "b"), 6); + + final Request request = mockClient.verifyOne(HttpMethod.GET, "/health/header/jdk"); + assertThat(request.headers()).containsEntry("ids", Arrays.asList("a,b")); + assertThat(request.headers()).containsEntry("grade1", Arrays.asList("6")); + } + @Test void requestParam() { resource.check("1", true); @@ -351,6 +362,10 @@ void checkRequestHeader( @RequestMapping(value = "/header/pojo", method = RequestMethod.GET) void checkRequestHeaderPojo(@RequestHeader HeaderMapUserObject object); + + @RequestMapping(value = "/header/jdk", method = RequestMethod.GET) + void checkRequestHeaderJdkTypes( + @RequestHeader(name = "ids") List ids, @RequestHeader(name = "grade1") int grade); } class UserObject {