Added ability to expand and encode variables in body annotation and expand request line variables that are not already key pairs

[sjhorn]

Example below serialises complex type "Operation" into the inputData placeholder in @Body.

@RequestLine("POST /sdpapi/request")
@Body("OPERATION_NAME=GET_REQUESTS&TECHNICIAN_KEY={technicalKey}&INPUT_DATA={inputData}")
    public API getRequests(
        @Named("technicalKey") String technicalKey,
        @Named("inputData") Operation operation
    )

The example below automatically expands the key-value pairs for the fields variable.

@RequestLine("PUT /1/cards/{cardIdOrShortLink}?key={key}&token={token}&{fields}")
    public Card updateCardWithFields(
        @Named("key") String key,
        @Named("token") String token,
        @Named("cardIdOrShortLink") String cardIdOrShortLink,
        @Named("fields") Object fields
    )

[cloudbees-pull-request-builder]

feign-pull-requests #144 FAILURE
Looks like there's a problem with this pull request

[sjhorn]

Interesting. If I am reading the console output correctly the build failed because it couldn't resolve a dependency:

* What went wrong:
Could not resolve all dependencies for configuration ':feign-core:compile'.
> Artifact 'javax.inject:javax.inject:1@jar' not found.

But I didn't add/change any dependencies with this request. Am I missing something here?

[cloudbees-pull-request-builder]

feign-pull-requests #145 FAILURE
Looks like there's a problem with this pull request

[cloudbees-pull-request-builder]

feign-pull-requests #146 FAILURE
Looks like there's a problem with this pull request

[codefromthecrypt]

Thanks for the effort here @sjhorn! I'm curious.. I thought body expansion already worked. Can you clarify what the essence of the change is (perhaps adding to the existing test cases?) Ex. we regularly use body expansion in denominator

https://github.com/Netflix/denominator/blob/master/ultradns/src/main/java/denominator/ultradns/UltraDNS.java

[cloudbees-pull-request-builder]

feign-pull-requests #147 FAILURE
Looks like there's a problem with this pull request

[sjhorn]

Cheers @adriancole for the quick response. I have added some unit tests to demonstrate the behaviour. I have also adjusted the title of the pull request - the change expands and encodes the body variables.

The change for the request line annotation expands
{fields} to property1=encodevalue1&property2=encodedvalue2...

This has been useful with the Trello API (eg. https://trello.com/docs/api/card/#put-1-cards-card-id-or-shortlink) that uses the URL for updates and has a variable number of property/value combinations.

[cloudbees-pull-request-builder]

feign-pull-requests #148 FAILURE
Looks like there's a problem with this pull request

[codefromthecrypt]

can you do me a favor and fix this file? there's a combination of multiple space and tab indent, which is distracting. I (think) feign is using 4 space.

[codefromthecrypt]

actually, it should be 2-space :)

[cloudbees-pull-request-builder]

feign-pull-requests #149 FAILURE
Looks like there's a problem with this pull request

[codefromthecrypt]

hmm this looks kindof meh. Seems we should not do something as magical as this, and instead use the type adapter system. feign tries really hard to avoid reflection.

[codefromthecrypt]

The more I look at this, the more I feel like the answer is a custom encoder. Ex. when the structured type is a body param, then an Encoder can splat it out as you've done here. Hopefully this particular pattern is isolated to trello. If I were doing this, I'll have a look at the other encoders and see if I could work something out. Can you try and see how far you get?

[codefromthecrypt]

ex. if this weren't @Named, a custom encoder could be used to add the &INPUT_DATA=splat(operation)

[sjhorn]

Yeah it is challenging. I have even made a JAXBEncoder/Decoder that I will have contribute shortly too, at least it abstracts most of the pain away ;)

The work you have done with feign is great I hope we can contribute more value in the coming months as we use it more.

[codefromthecrypt]

Long delayed summary.

So I think I've parsed this (mentally). I think what you are looking for is a means to control how variables are expanded. As opposed to baking in how to look things up reflectively, this could be a new annotation, like @Expander(SplatAnything.class).

I think that the best way is to see if the existing structures can work, or if this could be punted to a different annotation or similar.

Below is an attempt to do use existing code (mind you haven't tried to compile this).

  @RequestLine("POST /sdpapi/request")
  public API invoke(OperationRequest request);

@AutoValue
static abstract class OperationRequest {
  abstract String name();
  abstract String technicalKey();
  abstract Operation operation();

  static TechnicianAndOperation getRequests(String technicalKey, Operation operation) {
    return new AutoValue_OperationRequest("GET_REQUESTS", technicalKey, operation);
  }
}

@RequestLine("PUT /1/cards/{cardIdOrShortLink}?key={key}&token={token}")
    public Card updateCardWithFields(
        @Named("key") String key,
        @Named("token") String token,
        @Named("cardIdOrShortLink") String cardIdOrShortLink,
        Object fields // note this is now a body param
    )

// stashes gnarliness to only this api. That way, when the api you are using becomes less horrible, you can delete this.
  static class CustomEncoder implements Encoder {
    @Override
    public void encode(Object object, RequestTemplate template) throws EncodeException {
      if (object instanceof OperationRequest) {
       OperationRequest op = (OperationRequest) object;
        if (template.url().endsWith("request")){
           template.body("OPERATION_NAME="+op.name()+"&TECHNICIAN_KEY="+op.technicalKey()+"&INPUT_DATA="+op.operation()");
        }
      } else if (template.url().startsWith("/1/cards/")) {
        template.queries(variableToQueryString(object));
      } else {
        throw new EncodeException("unsupported");
      }
    }
  }

[codefromthecrypt]

mind having a try with the latest @Param(value="name", expander = CustomExpander.class)? Not sure it will work, but worth a shot.

#152

[codefromthecrypt]

hopefully expander addresses this. If not, it will need rework: feel free to open in such case.