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 #2218

Merged
merged 2 commits into from
Oct 28, 2023

Conversation

gromspys
Copy link
Contributor

@gromspys gromspys commented Oct 27, 2023

Issue: #686

Summary by CodeRabbit

  • New Feature: Introduced the @FeignIgnore annotation. This allows developers to mark certain methods to be ignored during processing, enhancing control over the application's behavior.
  • New Feature: Added error handling for attempts to invoke ignored methods. If a method marked with @FeignIgnore is called, the system will now throw an UnsupportedOperationException, improving the robustness of the application.
  • Test: Added new test cases to ensure the correct functioning of the @FeignIgnore annotation and the associated error handling mechanism.

@coderabbitai
Copy link

coderabbitai bot commented Oct 27, 2023

Walkthrough

The changes introduce a new annotation @FeignIgnore to the Feign library, allowing methods to be marked for exclusion from processing. If an ignored method is invoked, an UnsupportedOperationException is thrown. Tests have been added to ensure the functionality works as expected.

Changes

File Path Summary
core/src/main/java/feign/... Introduced @FeignIgnore annotation and added conditions to skip methods marked with it.
core/src/test/java/feign/FeignTest.java Added new methods and tests to verify the @FeignIgnore functionality.
reactive/src/main/java/feign/... Added conditions to throw an exception when an ignored method is invoked.
reactive/src/test/java/feign/... Updated imports, added new methods and tests for the @FeignIgnore functionality.

🐇💻

In the land of code, where the brackets lie,

A rabbit hopped, with a twinkle in its eye.

With each key press, a new world unfurls,

"Ignore the noise," whispers the bunny, "and watch your code swirls."

A new annotation, a skip and a hop,

With @FeignIgnore, unwanted methods, we stop.

So here's to the changes, may they run without a hitch,

Coding with joy, isn't life a rich! 🎉🥕


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai help to get help.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.json

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between bc8b4b1 and df58fc4.
Files selected for processing (3)
  • core/src/main/java/feign/Contract.java (1} hunks)
  • core/src/main/java/feign/FeignIgnore.java (1} hunks)
  • core/src/test/java/feign/FeignTest.java (1} hunks)
Files skipped from review due to trivial changes (2)
  • core/src/main/java/feign/Contract.java
  • core/src/main/java/feign/FeignIgnore.java
Additional comments: 1
core/src/test/java/feign/FeignTest.java (1)
  • 1275-1276: The new @FeignIgnore annotation is being used here to ignore the ignore() method during processing. This change seems to be in line with the PR summary and doesn't appear to introduce any issues. However, ensure that the ignore() method is not being called anywhere in the codebase, as it will not be processed due to the @FeignIgnore annotation.

Copy link
Member

@velo velo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the change, and makes lots of sense. But I think it should fire a clear exception if a ignored method is invoked.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between df58fc4 and 799364c.
Files selected for processing (4)
  • core/src/main/java/feign/ReflectiveFeign.java (1 hunks)
  • core/src/test/java/feign/FeignTest.java (3 hunks)
  • reactive/src/main/java/feign/reactive/ReactiveInvocationHandler.java (1 hunks)
  • reactive/src/test/java/feign/reactive/ReactiveFeignIntegrationTest.java (4 hunks)
Additional comments: 9
core/src/main/java/feign/ReflectiveFeign.java (1)
  • 94-95: The addition of a check for the method in the dispatch map before invoking it is a good practice. It prevents potential NullPointerException if the method is not found in the map. Throwing an UnsupportedOperationException is a reasonable approach to handle such cases. However, ensure that this exception is properly handled in the upstream code to avoid unexpected application crashes.
reactive/src/main/java/feign/reactive/ReactiveInvocationHandler.java (1)
  • 51-52: The new code checks if the method is present in the dispatch map and throws an UnsupportedOperationException if it's not. This is a good way to handle methods that are not supposed to be called, but it might be helpful to add a comment explaining why this check is necessary, especially since it's a change in behavior that could potentially break existing code if not handled properly.
reactive/src/test/java/feign/reactive/ReactiveFeignIntegrationTest.java (4)
  • 26-28: The import statements have been reorganized and some unused imports have been removed. Ensure that this does not affect other parts of the code that might be using these imports.

  • 44-44: The import for org.junit.Assert has been added. This is used for the assertion in the new test method testCallIgnoredMethod.

  • 66-78: A new test method testCallIgnoredMethod has been added. This test method creates a TestReactorService instance and tries to call the ignore method, which is annotated with @FeignIgnore. The test expects an UnsupportedOperationException to be thrown when the ignore method is called. This test validates the new @FeignIgnore annotation functionality.

  • 344-345: A new method ignore has been added to the TestReactorService interface and is annotated with @FeignIgnore. This method is used in the new test method testCallIgnoredMethod.

core/src/test/java/feign/FeignTest.java (3)
  • 36-36: The import org.junit.Assert has been added. Ensure that it is used in the code.

  • 1171-1183: A new test method testCallIgnoredMethod has been added. This test method creates an instance of TestInterface and calls the ignore method. It expects an UnsupportedOperationException to be thrown. This test validates the new @FeignIgnore annotation functionality.

  • 1290-1292: The ignore method has been added to the TestInterface and is annotated with @FeignIgnore. This method is used in the testCallIgnoredMethod test.

@velo velo merged commit 757de66 into OpenFeign:master Oct 28, 2023
1 check passed
ghost referenced this pull request in camunda/camunda Nov 15, 2023
15205: deps(maven): Update dependency io.github.openfeign:feign-bom to v13.1 (main) r=github-actions[bot] a=renovate[bot]

[![Mend Renovate logo banner](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [io.github.openfeign:feign-bom](https://github.com/openfeign/feign) | `13.0` -> `13.1` | [![age](https://developer.mend.io/api/mc/badges/age/maven/io.github.openfeign:feign-bom/13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/io.github.openfeign:feign-bom/13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/io.github.openfeign:feign-bom/13.0/13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/io.github.openfeign:feign-bom/13.0/13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency Dashboard for more information.

---

### Release Notes

<details>
<summary>openfeign/feign (io.github.openfeign:feign-bom)</summary>

### [`v13.1`](https://github.com/OpenFeign/feign/releases/tag/13.1): OpenFeign 13.1

[Compare Source](https://github.com/openfeign/feign/compare/13.0...13.1)

##### What's Changed

-   Support Conditional Parameter Expansion by [`@&#8203;gromspys](https://github.com/gromspys)` in [https://github.com/OpenFeign/feign/pull/2216](https://github.com/OpenFeign/feign/pull/2216)
-   Add typed response by [`@&#8203;gromspys](https://github.com/gromspys)` in [https://github.com/OpenFeign/feign/pull/2206](https://github.com/OpenFeign/feign/pull/2206)
-   Allow to ignore methods on provided interface by [`@&#8203;gromspys](https://github.com/gromspys)` in [https://github.com/OpenFeign/feign/pull/2218](https://github.com/OpenFeign/feign/pull/2218)
-   support method option and add UT by [`@&#8203;TaoJing96](https://github.com/TaoJing96)` in [https://github.com/OpenFeign/feign/pull/1881](https://github.com/OpenFeign/feign/pull/1881)
-   Do not decode URL encoding while setting up RequestTemplate by [`@&#8203;Breina](https://github.com/Breina)` in [https://github.com/OpenFeign/feign/pull/2228](https://github.com/OpenFeign/feign/pull/2228)

***

<details>
<summary>List of PRs that updated libraries versions</summary>
<br />
* build(deps): bump jakarta.xml.ws:jakarta.xml.ws-api from 4.0.0 to 4.0.1 by `@&#8203;dependabot` in OpenFeign/feign#2211
* build(deps-dev): bump org.glassfish.jersey.core:jersey-client from 2.40 to 2.41 by `@&#8203;dependabot` in OpenFeign/feign#2210
* build(deps-dev): bump org.glassfish.jersey.inject:jersey-hk2 from 2.40 to 2.41 by `@&#8203;dependabot` in OpenFeign/feign#2209
* build(deps): bump jakarta.xml.soap:jakarta.xml.soap-api from 3.0.0 to 3.0.1 by `@&#8203;dependabot` in OpenFeign/feign#2208
* build(deps): bump com.sun.xml.bind:jaxb-impl from 2.3.8 to 2.3.9 by `@&#8203;dependabot` in OpenFeign/feign#2207
* build(deps): bump io.sundr:sundr-maven-plugin from 0.101.0 to 0.101.1 by `@&#8203;dependabot` in OpenFeign/feign#2213
* build(deps): bump maven-surefire-plugin.version from 3.1.2 to 3.2.1 by `@&#8203;dependabot` in OpenFeign/feign#2212
* build(deps): bump io.sundr:sundr-maven-plugin from 0.101.1 to 0.101.2 by `@&#8203;dependabot` in OpenFeign/feign#2215
* build(deps): bump kotlin.version from 1.9.10 to 1.9.20 by `@&#8203;dependabot` in OpenFeign/feign#2219
* build(deps): bump io.sundr:sundr-maven-plugin from 0.101.2 to 0.101.3 by `@&#8203;dependabot` in OpenFeign/feign#2220
* build(deps): bump org.mockito:mockito-core from 5.6.0 to 5.7.0 by `@&#8203;dependabot` in OpenFeign/feign#2221
* build(deps): bump org.moditect:moditect-maven-plugin from 1.0.0.Final to 1.1.0 by `@&#8203;dependabot` in OpenFeign/feign#2224
* build(deps): bump io.dropwizard.metrics:metrics-core from 4.2.21 to 4.2.22 by `@&#8203;dependabot` in OpenFeign/feign#2222
* build(deps): bump org.junit:junit-bom from 5.10.0 to 5.10.1 by `@&#8203;dependabot` in OpenFeign/feign#2223
* build(deps): bump org.apache.maven.plugins:maven-javadoc-plugin from 3.6.0 to 3.6.2 by `@&#8203;dependabot` in OpenFeign/feign#2226
* build(deps): bump maven-surefire-plugin.version from 3.2.1 to 3.2.2 by `@&#8203;dependabot` in OpenFeign/feign#2225
</details>

##### New Contributors
* `@&#8203;TaoJing96` made their first contributi[https://github.com/OpenFeign/feign/pull/1881](https://github.com/OpenFeign/feign/pull/1881)l/1881
* `@&#8203;Breina` made their first contributi[https://github.com/OpenFeign/feign/pull/2228](https://github.com/OpenFeign/feign/pull/2228)l/2228

**Full Changelog**: OpenFeign/feign@13.0...13.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "every weekday" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/camunda/zeebe).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40Ni4wIiwidXBkYXRlZEluVmVyIjoiMzcuNDYuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->


Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
velo pushed a commit that referenced this pull request Oct 7, 2024
* Allow to ignore methods on provided interface

* Throw an UnsupportedOperationException if an ignored method was called
velo pushed a commit that referenced this pull request Oct 8, 2024
* Allow to ignore methods on provided interface

* Throw an UnsupportedOperationException if an ignored method was called
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.

2 participants