Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feign bug when use @RequestParam but not have value #1482

Closed
924060929 opened this issue Nov 20, 2016 · 9 comments
Closed

Feign bug when use @RequestParam but not have value #1482

924060929 opened this issue Nov 20, 2016 · 9 comments

Comments

@924060929
Copy link

924060929 commented Nov 20, 2016

The jar is spring-cloud-netflix-core-1.2.2.RELEASE-sources.jar

In the code org\springframework\cloud\netflix\feign\annotation\RequestParamParameterProcessor.java, the method "processArgument" treat @RequestParam param as Map when the value is empty, but in spring mvc, when @RequestParam's value is null, reflect the field name as the value. This diffrent action can make misunderstand.

As follows:

@Override
public boolean processArgument(AnnotatedParameterContext context,
		Annotation annotation) {
	RequestParam requestParam = ANNOTATION.cast(annotation);
	String name = requestParam.value();
	if (emptyToNull(name) != null) {
		context.setParameterName(name);

		MethodMetadata data = context.getMethodMetadata();
		Collection<String> query = context.setTemplateParameter(name,
				data.template().queries().get(name));
		data.template().query(name, query);
	} else {
		// supports `Map` types
		MethodMetadata data = context.getMethodMetadata();
		data.queryMapIndex(context.getParameterIndex());
	}
	return true;
}

Please fix it, :)

@lowzj
Copy link
Contributor

lowzj commented Nov 21, 2016

Spring Cloud Feign supports simple and map request parameter.
It reflects the field name as the value in another method SpringMvcContract#synthesizeWithMethodParameterNameAsFallbackValue. And it synthesizes RequestParam's value when RequestParam's value is null and the parameter is a simple property(checked by using BeanUtils#isSimpleProperty).

related issue: #1097

@ryanjbaxter
Copy link
Contributor

@lowzj so if the @RequestParam is a Map the value will be an empty Map but if the @RequestParam is a "simple property" the value will be null?

@lowzj
Copy link
Contributor

lowzj commented Nov 22, 2016

@ryanjbaxter I'm sorry for my unclear description. For examples:

  • if the parameter is a Map, the value of @RequestParam will be empty(default value ""). And we should not set the value.

    User getUser(@RequestParam Map<String, String> query);
  • if the parameter is a "simple property" and we don't set the @RequestParam value, SpringMvcContract will synthesizes with the parameter's name as fallback value, so the value of @RequestParam will not be empty.

    // the value of @RequestParam will be "name"
    User getUser(@RequestParam String name);
  • if the the parameter is a "simple property" and we give a value , the value of @RequestParam will not be empty.

    // the value of @RequestParam will be "name"
    User getUser(@RequestParam("name") String name);

@ryanjbaxter
Copy link
Contributor

@lowzj and this is also the same behavior as Spring MVC? It sounds like @924060929 is saying it is not the same behavior.

@924060929
Copy link
Author

924060929 commented Nov 22, 2016

@ryanjbaxter @lowzj Sorry, forgot to show my code.

Feign client interface code:

@FeignClient(serviceId="Do-authority")
public interface IDoAuthorityService {
    @RequestMapping(path = "hasUser", method = RequestMethod.GET)
    LinkedHashMap hasUser(@RequestParam String username);
}

Program throw exception when startup:

Caused by: java.lang.IllegalStateException: QueryMap parameter must be a Map: class java.lang.String
at feign.Util.checkState(Util.java:128) ~[feign-core-9.3.1.jar:na]
at feign.Contract$BaseContract.parseAndValidateMetadata(Contract.java:126) ~[feign-core-9.3.1.jar:na]
at org.springframework.cloud.netflix.feign.support.SpringMvcContract.parseAndValidateMetadata(SpringMvcContract.java:133) ~[spring-cloud-netflix-core-1.2.2.RELEASE.jar:1.2.2.RELEASE]
at feign.Contract$BaseContract.parseAndValidatateMetadata(Contract.java:64) ~[feign-core-9.3.1.jar:na]
at feign.hystrix.HystrixDelegatingContract.parseAndValidatateMetadata(HystrixDelegatingContract.java:34) ~[feign-hystrix-9.3.1.jar:na]
at feign.ReflectiveFeign$ParseHandlersByName.apply(ReflectiveFeign.java:146) ~[feign-core-9.3.1.jar:na]
at feign.ReflectiveFeign.newInstance(ReflectiveFeign.java:53) ~[feign-core-9.3.1.jar:na]
at feign.Feign$Builder.target(Feign.java:209) ~[feign-core-9.3.1.jar:na]
at org.springframework.cloud.netflix.feign.HystrixTargeter.target(HystrixTargeter.java:48) ~[spring-cloud-netflix-core-1.2.2.RELEASE.jar:1.2.2.RELEASE]
at org.springframework.cloud.netflix.feign.FeignClientFactoryBean.loadBalance(FeignClientFactoryBean.java:146) ~[spring-cloud-netflix-core-1.2.2.RELEASE.jar:1.2.2.RELEASE]
at org.springframework.cloud.netflix.feign.FeignClientFactoryBean.getObject(FeignClientFactoryBean.java:167) ~[spring-cloud-netflix-core-1.2.2.RELEASE.jar:1.2.2.RELEASE]
at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.doGetObjectFromFactoryBean(FactoryBeanRegistrySupport.java:168) ~[spring-beans-4.3.4.RELEASE.jar:4.3.4.RELEASE]
... 29 common frames omitted

@ryanjbaxter
Copy link
Contributor

I think I understand the problem better now. Correct me if I am wrong...

It seems like Spring MVC allows you to specify query parameters in a GET request using @RequestMapping and a basic type. However Feign expects query parameters to be a type Map. Since we are using Spring MVC to create Feign clients Feign is complaining because it is expecting the query parameters in the method signature to be a type Map. Does that basically describe the problem?

@924060929
Copy link
Author

Yes, that's what you said

@ryanjbaxter
Copy link
Contributor

If that is the case maybe you can open an issue in OpenFeign to see if this is something they can help support. I don't really see a way to change the behavior from our side. Maybe in Feign they can do something similar to Spring MVC so that if the method signature contains basic types it takes the parameters and converts them to a Map with keys being the parameter names.

@lowzj
Copy link
Contributor

lowzj commented Nov 23, 2016

@924060929 I guess that you're using java8 to compile your code. Java8 has a different way to preserve method parameter names and make them available at runtime by using javac compiler argument -parameters.

more information may be helpful:
#1102
Obtaining Names of Method Parameters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants