Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to ignore methods on provided interface #686

Open
dmarkhas opened this issue Apr 24, 2018 · 13 comments
Open

Allow to ignore methods on provided interface #686

dmarkhas opened this issue Apr 24, 2018 · 13 comments
Labels
proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration

Comments

@dmarkhas
Copy link

When the Feign annotated interface (or class passed to the builder) contains methods that are not intended to be invoked by a web request (i.e. not annotated with RequestMapping), Feign (actually the BaseContract) throws an exception saying the method is not annotated with HTTP method type.

While this is legitimate, I'd like to request an enhancement to allow the contract to ignore methods on demand (kinda like it ignores static methods), for example via some annotation ("@FeignIgnore"?).

The motivation for this is that the same interface class can be used in different contexts, not just for http based communication. Specifically, we use the same interface class both for Feign and for sending messages to remote services via a queue - if there is a method that should only be invoked through a queue, we'd expect it to not be annotated with RequestMapping, which causes Feign to fail to process the class entirely.

@rage-shadowman
Copy link
Contributor

Would it make more sense to ignore all non-default methods not annotated with @RequestLine (or the equivalent in the Contract that is being used) and throw UnsupportedOperationException if those non-implemented methods are called on the feign target?

@rage-shadowman
Copy link
Contributor

IMO, the cleaner solution would be to make multiple interfaces (your concrete class on the server side can implement them both). The one for Feign, the other for your queue.

But if you are unable to do that for some (business-related?) reason, then it is likely you do not have control of the interface and thus can't add any sort of @FeignIgnore annotation either.

So if Feign wants to support this, then (again, IMO -- YMMV) I think it should ignore all methods it doesn't recognize as supported.

@dmarkhas
Copy link
Author

We are doing exactly that just now - splitting the interface :)
It has some drawbacks for us though, since using a single interface would have allowed us to clean up our own APIs.
Your suggestion to ignore anything that is not annotated with RequestLine, RequestMapping, etc. sounds OK, I proposed the annotation simply because it is probably easier to implement and gives the developer finer control over what gets picked up by Feign, but both approaches are OK really.

@kdavisk6
Copy link
Member

kdavisk6 commented Apr 25, 2018

Complete untested and unverified suggestion, but has anyone tried to extend the interface ala Spring Data?

https://docs.spring.io/spring-data/jpa/docs/2.0.6.RELEASE/reference/html/#repositories.custom-implementations

interface MyCustomInterface {
  void myCustomMethod();
}

class MyCustomIntefaceImpl implements MyCustomInterface {
   public void myCustomMethod() {
       ...
   }
}
interface FeignClient extends MyCustomInterface {
   @RequestLine()
   public Foo myServiceCall();
}

I have no idea if there is some Spring magic going on here however.

@kdavisk6 kdavisk6 added the enhancement For recommending new capabilities label Jul 16, 2018
@kdavisk6
Copy link
Member

kdavisk6 commented Dec 5, 2018

From #615, we may want to consider either adding an explicit ignore annotation or update the default Contract to only intercept methods annotated. Thoughts?

@dmarkhas
Copy link
Author

dmarkhas commented Dec 5, 2018

As a user, having Feign only proxy annotated methods makes more sense and addresses the issue in this request and in #615.

@kdavisk6 kdavisk6 added proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration and removed enhancement For recommending new capabilities labels May 28, 2019
@XhstormR
Copy link

XhstormR commented Jan 8, 2022

I want to ignore some method on api interface too.

@hbprotoss
Copy link

Any progress here?

@XhstormR
Copy link

4 years have passed

@ashakirin
Copy link

Would be very practical to ignore default methods especilly for generated interfaces, for instance, from Open API

@velo
Copy link
Member

velo commented May 1, 2022 via email

@DanielLiu1123
Copy link

Is there any updates for this proposal?

@velo
Copy link
Member

velo commented Jul 6, 2022

Hi @DanielLiu1123 well, still waiting of someone to take charge and make it happen. I can review and guide as needed, just don't have the time to do it myself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration
Projects
None yet
Development

No branches or pull requests

8 participants