Describe the bug It loos like MatrixVariables aren't resolved in v2.2.2.RELEASE (feign-core v10.7.4)
Sample If I have the following Feign definition:
@GetMapping(value = "/objects/links")
ObjectLinkList getObjects(@MatrixVariable(name = "matrixVars") Map<String, List<String>> matrixVars);
(note: I had to add name to the MatrixVariable otherwise you get an Caused by: java.lang.IllegalStateException: MatrixVariable annotation was empty on param 0. error on startup)
Calling this method with 2 variables in the matrixVars map, the latest version of spring-cloud-openfeign generates the URL GET /objects/links.
The prevision version,v2.2.1.RELEASE (feign-core v10.4.0) , generates GET /objects/links;level=1;ancestorId=301 as expected.
Comment From: ryanjbaxter
The prevision version,v2.2.RELEASE (feign-core v10.4.0) , generates GET /objects/links;level=1;ancestorId=301 as expected.
Do you mean version 2.2.1.RELEASE?
Comment From: nickcodefresh
yes, sorry. comment updated
Comment From: OlgaMaciaszek
Will check why it's stopped working.
Comment From: OlgaMaciaszek
I have verified on 2.2.1 (Hoxton.SR1) - then there was no support for @MatrixVariable yet, so it has not worked correctly (as one would expect) and it's sent the following request:
GET /objects/links HTTP/1.1
Content-Length: 36
Content-Type: application/json
{"level":["1"],"ancestorId":["301"]}
with the matrixVars in the body.
In v.2.2.2 (Hoxton.SR3), support was added but maybe there's a bug in that - verifying it now.
Comment From: nickcodefresh
Sorry you're right. We were using the following encoder to get support matix vars
import feign.RequestTemplate;
import feign.form.spring.SpringFormEncoder;
import org.apache.commons.collections4.MapUtils;
import org.apache.commons.lang3.StringUtils;
import org.springframework.beans.factory.ObjectFactory;
import org.springframework.boot.autoconfigure.http.HttpMessageConverters;
import org.springframework.cloud.openfeign.support.SpringEncoder;
import org.springframework.web.multipart.MultipartFile;
import java.lang.reflect.Type;
import java.util.List;
import java.util.Map;
/**
* The Encoder implementation will use to add matrix params in request.
*/
public class RequestEncoder extends SpringEncoder {
public RequestEncoder(ObjectFactory<HttpMessageConverters> messageConverters) {
super(messageConverters);
}
@Override
@SuppressWarnings("unchecked")
public void encode(Object object, Type type, RequestTemplate template) {
if (object instanceof Map) {
Map<String, List<String>> matrixParams = (Map) object;
StringBuilder builder = new StringBuilder();
if (MapUtils.isNotEmpty(matrixParams)) {
for (Map.Entry<String, List<String>> entry : matrixParams.entrySet()) {
entry.getValue().forEach(value -> builder.append(";").append(entry.getKey()).append("=")
.append(value));
}
}
template.uri(StringUtils.isNotEmpty(builder.toString()) ? builder.substring(0) : "", true);
}
else if (object instanceof MultipartFile) {
new SpringFormEncoder().encode(object, type, template);
}
else {
super.encode(object, type, template);
}
}
}
Comment From: OlgaMaciaszek
Ok. Np. Your encoder might not work now as support has been added on our side, but will verify anyway to check whether there's a bug in that support.
Comment From: OlgaMaciaszek
In fact, there's a bug - matrix variables are resolved to a correct String but then not appended to the URI. Will fix it.
Comment From: OlgaMaciaszek
@nickcodefresh Have had a closer look at it. What happens is that the existing implementation works only when the @MatrixVariable value() corresponds to a {} segment in the path with the same String, for example:
@GetMapping(value = "/objects/links/{matrixVars}")
Map<String, List<String>> getObjects(@MatrixVariable Map<String, List<String>> matrixVars);
However, it actually does not have to correspond to work on the server-side so it's understandable for the users to expect it.
Unfortunately, there seems to be too much ambiguity in possible resolutions from the feign client levels to make it work, specifically because matrix variables can also be attached to "regular" path variables.
For example, for a method with such signature on server side:
@GetMapping("/owners/{owner}/pets/{pet}")
public void findPet(@MatrixVariable MultiValueMap<String, String> matrixVars)
all these paths would work in an HTTP call:
/owners/42/pets/21;q=11;r=12
/owners/42;q=11;r=12/pets/21;q=11;r=12
/owners/42;q=11;r=12/pets/21
/owners/q=11;r=12/pets/21
Because of that, we will need to stay with the way it currently works and are just going to add information in the documentation on that.
Comment From: nickcodefresh
Raise a futher issue https://github.com/spring-cloud/spring-cloud-openfeign/issues/334