Skip to content

Fix NPE when building a client with a query param with no values#192

Closed
jebeaudet wants to merge 1 commit intoOpenFeign:masterfrom
jebeaudet:master
Closed

Fix NPE when building a client with a query param with no values#192
jebeaudet wants to merge 1 commit intoOpenFeign:masterfrom
jebeaudet:master

Conversation

@jebeaudet
Copy link
Contributor

Using feign-core 7.1.0, when creating a feign client with a query param with no values, the builder throws a NPE. I didn't want to include Apache CollectionsUtils so I just added a null check. Thanks for the great library btw, hope this PR is OK!

TestApi.java:

public interface TestApi
{
    @RequestLine("POST /api/{parameter}?in_error")
    void updateInError(@Named("parameter")String parameter);
}

Builder :

Feign.builder()
         .decoder(new GsonDecoder())
         .encoder(new GsonEncoder())
         .logLevel(Level.FULL)
         .target(TestApi.class, "http://localhost:8888");

throws this stackstrace :

java.lang.NullPointerException: null
    at feign.RequestTemplate.allValuesAreNull(RequestTemplate.java:494) ~[feign-core-7.1.0.jar:7.1.0]
    at feign.RequestTemplate.pullAnyQueriesOutOfUrl(RequestTemplate.java:476) ~[feign-core-7.1.0.jar:7.1.0]
    at feign.RequestTemplate.append(RequestTemplate.java:207) ~[feign-core-7.1.0.jar:7.1.0]
    at feign.Contract$Default.processAnnotationOnMethod(Contract.java:137) ~[feign-core-7.1.0.jar:7.1.0]
    at feign.Contract$BaseContract.parseAndValidatateMetadata(Contract.java:62) ~[feign-core-7.1.0.jar:7.1.0]
    at feign.Contract$BaseContract.parseAndValidatateMetadata(Contract.java:48) ~[feign-core-7.1.0.jar:7.1.0]
    at feign.ReflectiveFeign$ParseHandlersByName.apply(ReflectiveFeign.java:143) ~[feign-core-7.1.0.jar:7.1.0]
    at feign.ReflectiveFeign.newInstance(ReflectiveFeign.java:58) ~[feign-core-7.1.0.jar:7.1.0]
    at feign.Feign$Builder.target(Feign.java:265) ~[feign-core-7.1.0.jar:7.1.0]
    at feign.Feign$Builder.target(Feign.java:261) ~[feign-core-7.1.0.jar:7.1.0]

@cloudbees-pull-request-builder

NetflixOSS » feign » feign-pull-requests #60 SUCCESS
This pull request looks good

@codefromthecrypt
Copy link
Contributor

Thanks for the help! Looks good. just might want to add a test in DefaultContractTest to ensure query w/o value continues to work later :)

A couple small notes

  1. most likely, you'll want to put your changes on a branch besides master. that way, things like squashing commits are contained.
  2. when your commit is final, reword it so that it says what it supports. ex.
Supports query params without values

Fixes an NPE when building a client with a query param with no values.

The above Supports query params without values header looks nicer in the commit log, but the details are still present. Reads like "this commit Supports query params without values"

Make sense?

Thanks again!

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Feb 21, 2015 via email

@jebeaudet
Copy link
Contributor Author

Thanks for the tips! I'll close this one and reopen another from a different branch.

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

Successfully merging this pull request may close these issues.

3 participants