Named Query Parameters on GET requests about feign HOT 18 CLOSED

mind posting the stack trace? Seems more likely an issue for the returnval than the name param.

FeignTest has a test named iterableQueryParams which seems to show query in use and not failing..

It is very strange that this is converted to a POST, as the above test proves at least that scenario it is a GET. Maybe we could replicate the test in the gson module to ensure nothing weird is going on when gson is present.

from feign.

Sure I can get you the stack trace.

I'll try and get you a simple test case that demonstrates the problem when I get in to work in the morning as well.

from feign.

Inevitably when you simplify the error it no longer happens. It looks like this is an issue when you combine named query parameters and named header values.

There's a test case at https://gist.github.com/michael-oliver-pearson/7351503 with the errors they generate.

I would have raised a pull request with the test case but I ran into problems building feign:
Artifact 'org.codehaus.plexus:plexus-utils:2.0.5@jar' not found.

from feign.

Can you verify that this failure happens when the Named annotation doesn't
include curly braces?

I suspect this doesnt matter, but just to ensure we are testing valid
syntax.

from feign.

I've updated the gist. I receive an identical exception with the valid syntax as well.
I was rushing a little earlier.

from feign.

I attempted to reproduce with the test in your gist. I was unable to do so. When I run it against the latest in master, my version of the test works fine as far as I can tell. Maybe it's only broken in some previous version?

https://gist.github.com/davidmc24/7364413

from feign.

I was running 5.3.0. I'll update to 6.0 and see if that resolves it.

from feign.

I tried that. It still doesn't work.
Thanks to your suggestion of commenting out mavenLocal I got feign building at work. The test still fails for me against latest master. Could we be looking at a behaviour difference in JVMs?
What version of Java are you running?

from feign.

FYI, here's the init script snippet I use.

https://gist.github.com/davidmc24/7371821

I'm at a different machine at the moment than where I initially ran the test. Regardless, the tests are still passing for me.

./gradlew --version

------------------------------------------------------------
Gradle 1.5
------------------------------------------------------------

Gradle build time: Wednesday, March 27, 2013 1:51:06 PM UTC
Groovy: 1.8.6
Ant: Apache Ant(TM) version 1.8.4 compiled on May 22 2012
Ivy: 2.2.0
JVM: 1.7.0_45 (Oracle Corporation 24.45-b08)
OS: Mac OS X 10.9 x86_64

from feign.

I'm having same issue. Feign 6.0.0, JacksonModule.

GET with query parameters is being converted into a POST -- even without parameterization.

public interface LocationClient {
    @RequestLine("GET /locations?tripStart=2013-11-19T00:37:43.000+0000")
    List<LocationUpdate> tripByDate();

    @RequestLine("POST /locations")
    Response appendLocations(List<LocationUpdate> locationUpdates);

    @RequestLine("GET /trips")
    List<Trip> tripList();
}

I have tried to remove the appendLocations(List) method as well, but to no avail.

    @RequestLine("GET /locations?tripStart={date}")
    List<LocationUpdate> tripByDate(@Named("date") String date);

Behaves the same.

From debugging Client and HttpUrlConnection it's set to "GET" all the way up to the request being made. Somehow Request#body is being set (and thus the below is being triggered). In the JDK, when getOutputStream is called it flips the method to POST (http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/sun/net/www/protocol/http/HttpURLConnection.java#827) and goes on ahead.

if (request.body() != null) {
        if (contentLength != null) {
          connection.setFixedLengthStreamingMode(contentLength);
        } else {
          connection.setChunkedStreamingMode(8196);
        }
        connection.setDoOutput(true);
        OutputStream out = connection.getOutputStream();
        if (gzipEncodedRequest) {
          out = new GZIPOutputStream(out);
        }
        try {
          out.write(request.body());
        } finally {
          try {
            out.close();
          } catch (IOException suppressed) { // NOPMD
          }
        }
      }

The response body:
groovy:000> [123,34,100,97,116,101,34,58,34,50,48,49,51,45,49,49,45,49,57,84,48,48,58,51,55,58,52,51,46,48,48,48,43,48,48,48,48,34,125].collect { Character.toChars(it)[0] }
===> [{, ", d, a, t, e, ", :, ", 2, 0, 1, 3, -, 1, 1, -, 1, 9, T, 0, 0, :, 3, 7, :, 4, 3, ., 0, 0, 0, +, 0, 0, 0, 0, ", }]

I'll keep debugging and see if I can't make a PR.

from feign.

I've seen things like this in jclouds, and it can often be a jvm version behaviour. Make sure you use a MockWebServer test and note the jre version that failed! thanks

from feign.

The string date (the same as the @nAmed above) makes it into MethodMetadata#formParams, leading Feign to choose BuildFormEncodedTemplateFromArgs and then write a body for it. RequestTemplate#queries does not appear to get populated, allowing:

          if (data.template().url().indexOf('{' + name + '}') == -1 && //
              !(data.template().queries().containsKey(name)
                  || data.template().headers().containsKey(name))) {
            data.formParams().add(name);
          }

To add the form params.

from feign.

@adriancole I'm using Feign from Android 4.3. I'll see if I can't get a test case here...

from feign.

The logic you pasted above looks wrong… Not sure why it passed.
https://github.com/Netflix/feign/blob/master/core/src/main/java/feign/Contract.java#L169

I think we should be comparing the value, not the key here!

It will be a little programming exercise, but basically, we should be testing for an index of '{' + name + '}' for each header or query value before assuming it is a form param.

from feign.

I ran into it Contract comparing by value rather than key once and was surprised by it. I would support a change in that regard. Another thing I was surprised by is named query parameters only working if the parameter is the entirety of the value (no prefixes or suffixes supported, or multiple parameters joined together into a single value). Perhaps someday I'll get down my list of potential enhancements to do something about it.

https://github.com/Netflix/feign/blob/master/core/src/main/java/feign/RequestTemplate.java#L530

from feign.

Here is an implementation searching the values of the relevant queries and headers. Fixes the problem for me.

from feign.

Any chance we can do a release so that the bugfix is not just in the snapshots?

from feign.

this has been released. Note 7.1.0 is releasing today, which implies transitioning from @Named to @Param which supports custom parameter expansion.

from feign.