Form-encoded parameters

[stromnet]

Hi,

new to Feign here. Got basic GET to work fine, but trying to make a POST with form-encoded parameters work. I have a basic interface:

    public interface SomeService {
        @RequestLine("POST /service")
        public SomeResponse someCall(@Param("param") String param, @Param("otherparam") String otherParam);
    }

    Feign.builder()
            .decoder(new JacksonDecoder())
            .target(SomeService.class, "http://...");

Judging from the code, having params which does not match {} fields in URL, expands them as "formParams" which then makes use of BuildFormEncodedTemplateFromArgs to transform them into a LinkedHashMap.
But, using the default encoder for this, I get feign.codec.EncodeException: class java.util.LinkedHashMap is not a type supported by this encoder., and I cannot seem to find any other encoder suitable for this.

Am I missing something, or is this a non-complete feature?

Thanks

[codefromthecrypt]

if you have a method which has no @Body anotation, and the parameters are annotated with @Param, they will be passed into your encoder as a Map<String, ?> type. This would allow you to make a multipart form, but there's no implementation at the moment.

We can teach the default encoder to do multipart form and/or there could be a wrapper. The wrapper could be used to mix in form encoding with gson, for example.

class FormAwareEncoder implements Encoder {
  private final Encoder delegate;

  public void encode(Object object, Type bodyType, RequestTemplate template) {
    if (bodyType.equals(Types.MAP_STRING_WILDCARD)) {
      // something like https://github.com/square/okhttp/blob/master/okhttp/src/main/java/com/squareup/okhttp/FormEncodingBuilder.java or this, but not using guava https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/io/payloads/MultipartForm.java
    } else {
      delegate.encode(object, bodyType, template);
    }
  }
}

cc @jmnarloch

[stromnet]

Hi,

yeah, that was the way I understood it too.
I solved it with the following (very) quick-n-dirty encoder:

    private static class FormEncoder implements Encoder {

        @Override
        public void encode(Object o, Type type, RequestTemplate rt) throws EncodeException {
            if(!(o instanceof Map))
                throw new EncodeException("Can only encode Map data");

            Map m = (Map)o;

            // XXX: quick n dirty!
            StringBuilder sb = new StringBuilder();

            for(Object k: m.keySet()) {
                if(!(k instanceof String))
                    throw new EncodeException("Can only encode String keys");

                if(sb.length() > 0)
                    sb.append("&");

                Object v = m.get(k);
                if(!(v instanceof String))
                    throw new EncodeException("Can only encode String values");

                try {
                    sb.append(URLEncoder.encode((String)k, "UTF-8"))
                        .append("=")
                        .append(URLEncoder.encode((String)v, "UTF-8"));
                }catch(UnsupportedEncodingException ex) {
                    throw new EncodeException("Invalid encoding", ex);
                }
            }

            rt.header("Content-Type", "application/x-www-form-urlencoded");
            rt.body(sb.toString());
        }

    }

[codefromthecrypt]

Thanks for posting how you did it!

[pcan]

Hi, I'm currently facing the same issues with multipart requests.. I created a sample project that allows multipart form submission through Feign, but it uses a really ugly approach to detect the form request (Types class is package-private). I would be glad to submit a PR in order to add something like following:

package feign;

public abstract class AbstractFormEncoder implements Encoder {

    private final Encoder delegate;

    public AbstractFormEncoder(Encoder delegate) {
        this.delegate = delegate;
    }

    @Override
    public final void encode(Object object, Type bodyType, RequestTemplate template) {
        if (bodyType.equals(Types.MAP_STRING_WILDCARD)) {
            formEncode(object, bodyType, template);
        } else {
            delegate.encode(object, bodyType, template);
        }
    }

    public abstract void formEncode(Object object, Type bodyType, RequestTemplate template);

}

@adriancole you already proposed this a few posts ago.. what do you think about this?

[codefromthecrypt]

kindof swamped to think about a form encoder right now, but maybe I can help make the hack less ugly.

Something like this should be less gnarly. Ex I don't think you need to manually construct the type.. then you can do equals on it directly.

class Foo {
  static final Map<String, ?> MAP_STRING_WILDCARD = null;
  final Type formType;

 Foo(){
    this.formType = getClass().getField("MAP_STRING_WILDCARD").getGenericType()
 }

[pcan]

@adriancole I tried this approach and it seems to work (but i had to use getDeclaredField). The point is that this approach breaks open-closed principle, since I need to know an internal detail of the library to make my code work. Furthermore, if this detail changes in the future (and the developers are free to do it, since they assume nobody can see it from outside), my code will be suddenly broken for new Feign versions. My proposal does not try to cover form encoding explicitly in Feign library, but it only aims to provide a well defined entry point to allow users build an encoder by themselves, without having to do any assumption about the internal details of Feign code.

[codefromthecrypt]

Which detail are you referring to when raising a concern about knowledge of internals?

[pcan]

Ok, I'll try to clarify a bit more. At some point in ReflectiveFeign there is a check that decides the building of a Map<String, ?> that represents a Form, instead of a simple request body (like a POJO, for example).
Since the only way to know (in the Encoder.encode method) if the request type is a Form one or a regular one, is through the bodyType parameter, and since here we have an hardcoded value (MAP_STRING_WILDCARD) that is currently package private, there is no other 'safe' way to decide if the request is a Form one or not in Encoder, but we can only compare the bodyType with MAP_STRING_WILDCARD.
Since we have to build a multipart request (that still have to be implemented in a custom Encoder), we need to distinguish non-form requests (that are serialized as JSON for example) from multipart ones that are serialized by the custom Encoder.encode method.

[codefromthecrypt]

Oh OK. So the map wildcard type is actually documented on the Encoder for use in forms so I wouldn't call it internal or that you need to peek inside to figure out this. It doesn't include instructions of how to construct the type reference, and neither does feign provide a type reference api (like guava, Jackson, gson, moshi etc do). Probably lack of explanation on this is a problem. Ex not everyone will have a type library handy, and then peek inside feign and see it has an internal one then wonder as you have. I think I would rather move the constant type to Encoder and make it public. I can revise the javadoc with a link to it there, too. Hope this helps and thanks for explaining.

[pcan]

Thank you. A public reference to MAP_STRING_WILDCARD gives us a clear way to do that check. 👍

[codefromthecrypt]

#355

[MohammedIdris]

@adriancole How would I then use feign to upload multiple files? I am still getting NullPointerException at isUserPojo What was the use of exposing the MAP_STRING_WILDCARD? @pcan How do we make use of this change? Is there any sample code?
Also, your Encoder has an empty constructor which does nothing, is there a new implementation which is not pushed to git?
Please help me resolve this error and successfully implement multiple multipart file uploads.