Support parameterized base apis about feign HOT 23 CLOSED

I've done this is another library. Inheritance makes some things more
complex, so would have to be thought through.

How would we handle the inevitable overload/covariance concern? Would we
support multiple inheritance? What if someone defined a parameterized
supertype?

I'm interested in getting to a point where we can either do errorprone
checks or actual compiled implementations (like auto). The former could
help define rules, the latter usually constrains them. These advances may
not happen, but are considerations.

from feign.

ps not saying this we shouldn't do this, just we'd need to cover test cases that include parameterized interfaces and overloads (including covariance). It may be fine to throw on those edge cases.

wanna give it a shot?

from feign.

Has there been any movement on this? This would be an exceptionally useful feature for my use cases. Anything I can do to help?

from feign.

I personally don't want this feature as it will complicate the code
significantly. That's why I haven't moved it.

That said, if you or someone else are willing to implement it,
including tests, I'd be open to helping review it and earnestly try to
get it merged.

Here are some scenarios to keep in mind while implementing this:

• a feign method with a generic return value exists in a parameterized parent
• a feign method with a generic parameter value exists in a
  parameterized parent
• a feign method exists in the grandparent
• you override the annotations on a feign method in the parent
• you covariantly override the return value of a feign method in the parent

There are probably more, but these are the ones on the top of my head.

The code affected would be at least Contract.Default, ReflectiveFeign,
DefaultContractTest and FeignTest

The Contract.Default and ReflectiveFeign.newInstance will need to be
changed to look at parent classes and deal with potential
parameterization if you choose to support that. Particularly watch out
for bridge methods.

All of the scenarios should be covered in tests even if that's saying
something is unsupported (i.e. throwing illegal state exceptions in
Contract.Default parsing).
Some of the edge case will come at runtime, when the methods are
looked up and dispatched (ReflectiveFeign).

from feign.

My specific use cases actually cover HEAVILY type parameterized parent interfaces with parameterized return types (and even generic return types with further parameterization). I'll clone the code and fiddle with it a little bit, and see what I discover, and see if I feel like I can get some of these cases to work. Thanks for the information.

from feign.

from feign.

I'm quite interested in this use case as well. @jmgrimes any chance you have looked into this. My use case would also be involving parametrized parent interfaces.

from feign.

ok, so @alexeos @zampettim @jmgrimes you are starting to convince me that I should help. It will make things more complex, but we can do our best to limit that. Will you help with testing and/or bug fixes to code or docs?

@jdamick @spencergibb (+ any others) you against adding support for this? If not, I'm thinking about rolling up sleeves and implementing it (maybe next week).

from feign.

👍

from feign.

I can definitely help test and vet any solution. I'm using Feign via the Spring Cloud implementation and integration, in case that matters.

from feign.

starting on this, join https://gitter.im/Netflix/feign for updates that aren't here

from feign.

Inheritance without generics is nuanced, but ok. The primary concern is Feign.configKey, which assumes a flat hierarchy. This stitches together contract parsing to actual method invocation.

• Ex. If Target.type is Child extends Super, a method on the Super named get will end up being the config key Super#get(), not SubType#get.

In simplest case, promote all config keys to the subtype. Care must be taken when a child type overrides metadata or narrows its return value.

Dealing with parameterized types is way more nuanced than dealing with inheritance, particularly wrt keeping feign small and dep free. Here are a small smattering of problems inherit if we support generics.

• feign config keys are based on the type data. a method with a type variable ends up being Type#put(Object), not Type#put(YourType). Games can be played to equate these.
• decoders need to know the return type. It is complex to resolve List<T> into List<YourType>

Doing both inheritance and generics will cost as much or more time than other sought after features such as hystrix. It will certainly complicate the code. I'd recommend we limit this to strict inheritance w/o generics. If that's still a step forward, ping back with your interest.

We can table generics to a different issue: it may actually be easier to deal with via a compile-time approach similar to auto-value.

from feign.

We're using a small in-house platform for standing up services that have the same base API, and the payloads are the only thing that differ between different services. To that end, the feature we're hoping to achieve is one where we can define the client interface (with generics) one time, and then have the sub interface not add anything new (typically), but completely bind the generic parameters. For example:

public interface Client<M> {
    public M get(final String key);
}

public interface MyClient extends Client<MyObject> { }

This would be my primary need of such a feature. I don't need multiple inheritance types of cases personally, but I would be using generics in the base interface, and then binding those generics completely in the sub interface.

from feign.

@jmgrimes how about opening an issue for what you describe. It is contained enough to not be too much headache (ex particularly single parent and single type variable). Maybe title it Base Api Interfaces. Go ahead and put an example including list and mutation, so that any tests cases are as relevant as possible.

from feign.

oh wait.. we can just reuse this. The initial example is about base api inheritance. We can just highly specify the rules

• no more than one type variable, and that is only up to one level deep (ex. X and List<X> is ok, but List<List<X>> isn't)
• no more than one ancestor
• no overloading
• config keys are always made from the child

do these constraints work?

from feign.

For us, it tends to be multiple type variables at one level. A truer version of the base api contains these things:

public interface ServiceClient<K, M> {
    public Entity<K, M> get(K key) // GET url param
    public Entities<K, M> getAll(Keys<K> keys) // POST body
}

public class Entity<K,M> {
    @Getter @Setter private K key;
    @Getter @Setter private M model;
}

public class Entities<K,M> {
    @Getter @Setter private List<Entity<K,M>> entities;
}

This would always be extended for us as:

public interface PersonServiceClient extends ServiceClient<UUID, Person> { }

We pretty much never go beyond one level of type variables, but we do use multiple type variables in the base. They are always bound when the base is extended, and there is never any overloading. We don't make use of config keys, but I'm thinking that makes perfect sense for those to be from the child.

Pretty much, we do this so we can vary the model and key types of various entities, but use the same REST API for every entity. It would be exceptionally useful to be able to write one service client interface that can be extended and have the key and model types "plugged in" at extension time.

from feign.

I'm using the spring cloud based stuff for this as well, FWIW.

from feign.

from feign.

So, I'm glad I chose the name Contract.. it is one of the few naming decisions I've ever made that has worked out over time :) I'll make a contract test out of your example, clarifying the rules I mentioned earlier as 'up to two type variables'. If folks need more than that, well we can always update, but the essence of this feature makes sense with this constraint, and even hints at a design (ex. key and value parameterization). As with all things, will be a work in progress, but I think we've reached a workable design!

from feign.

actually 'up to two type variables' isn't needed. pull request will happen today!

from feign.

Thanks! Rolling in a few changes in dev this week, so I'll check this out and try it in our dev environment.

from feign.

@adriancole, and possibly others, I don't mean to hijack this closed issue but I was sent here on behalf of a recommendation by a colleague in hopes that we might replace our existing libraries, which are using jclouds-core as the backend, with feign as the backend as it's lighter weight and not tied to the cloud abstraction. Our libraries are mainly using jclouds-core for its powerful http abstraction and making things stupid-simple. However, we like the use of AutoValue that jclouds provides and searching online for references to feign and its potential use of AutoValue brought me here. It appears that feign is shying away from AutoValue, at least from what this thread is implying, so I was just wondering the technical details around the decision if you guys don't mind explaining and/or pointing me in the direction where this is documented (maybe its not).

from feign.

from feign.