An OpenFeign Client for Dapr Invokes #1294

lony2003 · 2025-04-02T12:53:01Z

Description

A OpenFeign Client and it's AutoConfiguration using Dapr Invoke APIs.

Splited into two librarys, dapr-openfeign-client is the pure client of OpenFeign, and dapr-spring-openfeign is the plugin of Spring Cloud OpenFeign.

Currently only HTTP Invoke Method and Invoke Binding supported, and Invoke Binding is not always recommend to use this client, depends on what binding component is.

Issue reference

this PR will close: #1181

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Extended the documentation

A OpenFeign Client and it's AutoConfiguration using Dapr Invoke APIs. Splited into two librarys, dapr-openfeign-client is the pure client of OpenFeign, and dapr-spring-openfeign is the plugin of Spring Cloud OpenFeign. Currently only HTTP Invoke Method and Invoke Binding supported, and Invoke Binding is not always recommend to use this client, depends on what binding component is. Signed-off-by: lony2003 <[email protected]>

this example can redirect order request to producer-app. It needs producer-app to run properly. Signed-off-by: lony2003 <[email protected]>

this test tests the PostgreSQL binding of dapr. Signed-off-by: lony2003 <[email protected]>

Signed-off-by: lony2003 <[email protected]>

salaboy · 2025-04-02T14:24:25Z

I haven't reviewed this yet @lony2003 , but I am more convinced that we need this .. @artur-ciocanu @mcruzdev can you help me to check this PR ?

...n/src/main/java/io/dapr/spring/openfeign/autoconfigure/DaprFeignClientAutoConfiguration.java

Fix the example doc to make openfeign app works in mm.py Signed-off-by: lony2003 <[email protected]>

salaboy · 2025-04-07T07:44:00Z

@lony2003 I've just reviewed this PR and it is looking awesome..
I am currently working on testing multiple sidecars using Testcontainers (for service to service invocation) scenarios, so I will probably tag you in those examples when I have them running. I think this PR is ok (as soon as there are no bean conflicts), then we can create some examples in spring-boot-examples showing the feign clients when we can test service to service invocations with multiple sidecars.

…riginal Targeter beans instead of override them. Signed-off-by: lony2003 <[email protected]>

lony2003 · 2025-04-07T10:27:38Z

@salaboy I have changed the Targeter part to using BeanPostProcesser to handle them, so now no confliects will happen.

...gn-app/src/main/java/io/dapr/springboot/examples/openfeign/ProducerClientRestController.java

Signed-off-by: lony2003 <[email protected]>

…ingMessagingIT works Signed-off-by: lony2003 <[email protected]>

…oot 3.3.x Signed-off-by: lony2003 <[email protected]>

…gIT works Signed-off-by: lony2003 <[email protected]>

lony2003 · 2025-04-08T06:12:38Z

@cicoyle I have checked all files and add headers for them, and I have solved the problem that makes IT failed. Please re-trigger the CI.

cicoyle · 2025-04-22T20:33:36Z

Hey @lony2003 - thanks, before I can retrigger CI it says there are conflicts needing to be resolved. Mind resolving them locally and pushing? Then we'll be able to proceed 🙏🏻

Signed-off-by: lony2003 <[email protected]>

…R_RUNTIME_IMAGE_TAG Signed-off-by: lony2003 <[email protected]>

… client bean autowired Job SDK ITs have a daprPreviewClient bean defined, as SpringBoot bean search will first get beans that impl the interface and get all beans that matches the bean's class, when openfeign autowire DaprClient, there is a conflict between daprClient and daprPreviewClient. Change the AutoConfiguration behavior to make a more strict rule for making AutoConfiguration run Signed-off-by: lony2003 <[email protected]>

change APP_PORT to 8082 Signed-off-by: lony2003 <[email protected]>

lony2003 · 2025-04-23T06:44:29Z

@cicoyle conflict solved.

salaboy · 2025-05-07T06:57:43Z

@lony2003 I am happy to see the CI happy for this PR, can you check the codecov?

salaboy

LGTM

salaboy · 2025-05-11T19:12:25Z

@dapr/maintainers-java-sdk I believe that we can merge this PR, we will incrementally improve the quality of this PR, but I want this module to be present in the SDK. Please approve, we can fix any findings in smaller PRs.

Ds2994 · 2025-05-26T13:02:47Z

Hi @salaboy @cicoyle Can we get this enhancement merged. This will significantly enhace the code readability and usability for DaprClients.

salaboy · 2025-05-26T13:03:50Z

I really want this merged too, but I am not a maintainer

salaboy · 2025-05-26T13:33:47Z

@Ds2994 are you using this integration already in your project ?

Ds2994 · 2025-05-26T13:53:42Z

Yes @salaboy I am currently using this in an internal project and having a lot of readability trouble with the overloaded methods in DaprClient. If this declarative approach gets merged will make a lot of my development cleaner.

salaboy · 2025-05-26T13:55:00Z

@artur-ciocanu @cicoyle do you have any more concerns on this one ? I think this still need to be rebased by @lony2003

Signed-off-by: lony2003 <[email protected]>

lony2003 · 2025-06-11T13:55:31Z

@salaboy @artur-ciocanu @cicoyle I have merged the latest base for this PR.

salaboy · 2025-06-11T13:59:28Z

@dapr/maintainers-java-sdk please lets merge this when you can.

codecov · 2025-06-11T14:05:16Z

Codecov Report

Attention: Patch coverage is 65.58140% with 74 lines in your changes missing coverage. Please review.

Project coverage is 77.86%. Comparing base (d759c53) to head (3b9b9d4).
Report is 166 commits behind head on master.

Files with missing lines	Patch %	Lines
...main/java/io/dapr/feign/DaprInvokeFeignClient.java	74.49%	28 Missing and 10 partials ⚠️
.../spring/openfeign/targeter/DaprClientTargeter.java	0.00%	20 Missing ⚠️
.../targeter/DaprClientTargeterBeanPostProcessor.java	0.00%	6 Missing ⚠️
...feign/autoconfigure/DaprFeignClientProperties.java	55.55%	4 Missing ⚠️
...onfigure/FeignClientAnnoationEnabledCondition.java	50.00%	2 Missing and 1 partial ⚠️
...gboot/examples/openfeign/OpenFeignApplication.java	33.33%	2 Missing ⚠️
...a/io/dapr/springboot/examples/openfeign/Order.java	91.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1294      +/-   ##
============================================
+ Coverage     76.91%   77.86%   +0.95%     
- Complexity     1592     1888     +296     
============================================
  Files           145      235      +90     
  Lines          4843     5933    +1090     
  Branches        562      622      +60     
============================================
+ Hits           3725     4620     +895     
- Misses          821      982     +161     
- Partials        297      331      +34

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

cicoyle · 2025-06-11T14:11:43Z

waiting on CI to ensure a 🟢 run 👍🏻

lony2003 · 2025-06-11T14:37:27Z

@cicoyle CI have passed. Could you please merge it?

lony2003 · 2025-06-11T15:08:39Z

@salaboy @cicoyle @artur-ciocanu is there any problem remains? It'a simple patch, so I think it's easier to review than cloud config. Could you please merge it before a breaking change happened?

artur-ciocanu

@lony2003 thanks a lot for your contribution. I see the value of this PR and will be a great addition to Dapr Spring portfolio.

I have left quite a few comments. Please take a look and let me know your thoughts.

A few thing that I want to highlight:

I don't think OpenFeign makes a lot of sense outside Spring ecosystem, so having a dedicated module named "dapr-openfeign-client" is not really required and I would move it to "dapr-spring-openfeign"
Using "UseDaprClient" annotation I would argue that a more ergonomic approach is to have a single annotation named "DaprFeignClient" that looks like "FeignClient" but leverages DaprClient behind the scenes.
In the PR we have Feign and OpenFeign in classes, properties file and artifacts. We should have a single naming approach. Since this is part of Spring Cloud we should probably have names like:
- dapr.spring.cloud.openfeign.client.... - for properties
- io.dapr.spring.cloud.openfeign... - for package names
- DaprOpenFeign... - for class names
  This way we can have a more consistent naming scheme.

artur-ciocanu · 2025-06-12T20:15:02Z

dapr-spring/dapr-openfeign-client/pom.xml

+    <dependencies>
+        <dependency>
+            <groupId>io.github.openfeign</groupId>
+            <artifactId>feign-core</artifactId>


Please move version to parent POM, so we can control all the upgrades from a single place.

artur-ciocanu · 2025-06-12T20:16:03Z

dapr-spring/dapr-openfeign-client/pom.xml

+        </dependency>
+        <dependency>
+            <groupId>io.github.openfeign</groupId>
+            <artifactId>feign-jaxrs</artifactId>


Please move version to parent POM. This is a test dependency, so it should be moved closer to other test dependencies.

We try to keep prod dependencies first, followed by test dependencies.

artur-ciocanu · 2025-06-12T20:16:11Z

dapr-spring/dapr-openfeign-client/pom.xml

+        </dependency>
+    </dependencies>
+
+</project>


Please add new line.

artur-ciocanu · 2025-06-12T20:17:21Z

dapr-spring/dapr-openfeign-client/spotbugs-exclude.xml

@@ -0,0 +1,11 @@
+<FindBugsFilter>


Could you please provide more details, why we need these exclusions and why we can't fix the code so we don't need these exclusions.

artur-ciocanu · 2025-06-12T20:18:54Z

dapr-spring/dapr-openfeign-client/src/main/java/io/dapr/feign/DaprInvokeFeignClient.java

+ * See the License for the specific language governing permissions and
+limitations under the License.
+*/
+


The package is artifact is named dapr-openfeign-client, I think it is a good idea to be consistent and name the package: io.dapr.openfeign

artur-ciocanu · 2025-06-12T21:24:28Z

...r-spring-openfeign/src/main/resources/META-INF/additional-spring-configuration-metadata.json

+      "description": "retries for invoke fails."
+    }
+  ]
+}


Add new line

artur-ciocanu · 2025-06-12T21:24:34Z

...n/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports

@@ -0,0 +1 @@
+io.dapr.spring.openfeign.autoconfigure.DaprFeignClientAutoConfiguration


Add new line

artur-ciocanu · 2025-06-12T21:27:00Z

...-spring-openfeign/src/test/java/io/dapr/spring/openfeign/client/DaprInvokeBindingClient.java

+import org.springframework.cloud.openfeign.FeignClient;
+import org.springframework.web.bind.annotation.GetMapping;
+
+


From a design perspective wouldn't it be better to have a composite annotation like:

@DaprFeignClient(name = "invoke-binding", url = "http://binding.democlient/")

Instead of always using two annotations like:

FeignClient(name = "invoke-binding", url = "http://binding.democlient/") @UseDaprClient

Actually, FeignClient annotation is from Spring Cloud Openfeign, and I think this two annotation means that this interface is a FeignClient and it uses DaprClient to send RPC requests. So I would say it's not a good idea to have a composite annotation.

And I should say that we use Spring Cloud Openfeign is for it's automatic features in application and IDE, and a composite annotation will definitely break these features, at least in IDE, features provided by Spring Boot plugin would not available while using a DaprFeignClient

artur-ciocanu · 2025-06-12T21:27:52Z

dapr-spring/dapr-spring-openfeign/src/test/resources/application.properties

@@ -0,0 +1 @@
+dapr.feign.enabled=true


As I mentioned let's use dapr.openfeign.client

artur-ciocanu · 2025-06-12T21:28:19Z

dapr-spring/spotbugs-exclude.xml

@@ -8,4 +8,9 @@
        <Package name="~io\.dapr\.spring.*"/>
        <Bug pattern="EI_EXPOSE_REP2"/>
    </Match>
+


Could you please make sure to fix the code and remove this exclusion

lony2003 · 2025-06-13T04:08:27Z

@artur-ciocanu about dapr-openfeign-client, I think since Openfeign is developed by OpenFeign Community, and is actively developing while Spring Cloud Openfeign is inactived, there should be other usage besides Spring Cloud Openfeign. So I think it's a good idea to have separate library as there should be some users who don't use SpringBoot but uses OpenFeign.

salaboy · 2025-06-13T08:18:44Z

I agree with @lony2003 , this would feel like using spring cloud open feign, it doesn't need to expose any Dapr specific interface or annotation

lony2003 added 6 commits April 1, 2025 21:58

docs(springboot openfeign): add an example to springboot-examples

665c299

this example can redirect order request to producer-app. It needs producer-app to run properly. Signed-off-by: lony2003 <[email protected]>

test(springboot openfeign): A new test that tests PostgreSQL bindings.

d7831b0

this test tests the PostgreSQL binding of dapr. Signed-off-by: lony2003 <[email protected]>

docs(springboot openfeign): Update doc for Dapr OpenFeign

6498a09

Signed-off-by: lony2003 <[email protected]>

lony2003 requested review from a team as code owners April 2, 2025 12:53

lony2003 commented Apr 2, 2025

View reviewed changes

...n/src/main/java/io/dapr/spring/openfeign/autoconfigure/DaprFeignClientAutoConfiguration.java Outdated Show resolved Hide resolved

docs(springboot openfeign): A fix of example docs.

fd89e6f

Fix the example doc to make openfeign app works in mm.py Signed-off-by: lony2003 <[email protected]>

refactor(springboot openfeign): Use BeanPostProcessor to modify the o…

46a9f65

…riginal Targeter beans instead of override them. Signed-off-by: lony2003 <[email protected]>

cicoyle reviewed Apr 7, 2025

View reviewed changes

...gn-app/src/main/java/io/dapr/springboot/examples/openfeign/ProducerClientRestController.java Show resolved Hide resolved

lony2003 added 4 commits April 8, 2025 11:16

style(springboot openfeign): add headers, clean import, refresh style

2877e86

Signed-off-by: lony2003 <[email protected]>

fix(springboot openfeign): delete pubsub subscription to make DaprSpr…

ee6f761

…ingMessagingIT works Signed-off-by: lony2003 <[email protected]>

fix(springboot openfeign): fix spring cloud version error for springb…

bb9e2fe

…oot 3.3.x Signed-off-by: lony2003 <[email protected]>

fix(springboot openfeign): change IT ports to make DaprSpringMassagin…

475835c

…gIT works Signed-off-by: lony2003 <[email protected]>

lony2003 requested a review from cicoyle April 8, 2025 05:53

Merge branch 'master' into development-openfeign

7b8bba5

Signed-off-by: lony2003 <[email protected]>

lony2003 force-pushed the development-openfeign branch from 8580167 to a330ef2 Compare April 23, 2025 01:29

fix(springcloud openfeign): fix IT failed because of reflactor of DAP…

a330ef2

…R_RUNTIME_IMAGE_TAG Signed-off-by: lony2003 <[email protected]>

lony2003 force-pushed the development-openfeign branch 5 times, most recently from 6d1e70a to 1767276 Compare April 23, 2025 04:40

lony2003 force-pushed the development-openfeign branch from 3eaa83e to c381e44 Compare April 23, 2025 05:10

lony2003 force-pushed the development-openfeign branch 4 times, most recently from 00fae6b to bc6d98c Compare April 23, 2025 06:10

lony2003 added 2 commits April 23, 2025 14:11

fix(springboot openfeign): fix IT failure because of APP_PORT

e8a35cc

change APP_PORT to 8082 Signed-off-by: lony2003 <[email protected]>

salaboy previously approved these changes May 7, 2025

View reviewed changes

Merge branch 'master' into development-openfeign

1ae5ace

Signed-off-by: lony2003 <[email protected]>

lony2003 dismissed salaboy’s stale review via 1ae5ace June 11, 2025 12:00

fix(springcloud openfeign): Fix poms to match the upstream

3b9b9d4

Signed-off-by: lony2003 <[email protected]>

lony2003 force-pushed the development-openfeign branch from 6addcf4 to 3b9b9d4 Compare June 11, 2025 13:11

artur-ciocanu requested changes Jun 12, 2025

View reviewed changes

		@@ -0,0 +1 @@
		io.dapr.spring.openfeign.autoconfigure.DaprFeignClientAutoConfiguration

		import org.springframework.cloud.openfeign.FeignClient;
		import org.springframework.web.bind.annotation.GetMapping;

		@@ -0,0 +1 @@
		dapr.feign.enabled=true

An OpenFeign Client for Dapr Invokes #1294

Are you sure you want to change the base?

An OpenFeign Client for Dapr Invokes #1294

Conversation

lony2003 commented Apr 2, 2025

Description

Issue reference

Checklist

Uh oh!

salaboy commented Apr 2, 2025

Uh oh!

Uh oh!

salaboy commented Apr 7, 2025

Uh oh!

lony2003 commented Apr 7, 2025

Uh oh!

Uh oh!

lony2003 commented Apr 8, 2025

Uh oh!

cicoyle commented Apr 22, 2025

Uh oh!

lony2003 commented Apr 23, 2025

Uh oh!

salaboy commented May 7, 2025

Uh oh!

salaboy left a comment

Choose a reason for hiding this comment

Uh oh!

salaboy commented May 11, 2025

Uh oh!

Ds2994 commented May 26, 2025

Uh oh!

salaboy commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

salaboy commented May 26, 2025

Uh oh!

Ds2994 commented May 26, 2025

Uh oh!

salaboy commented May 26, 2025

Uh oh!

lony2003 commented Jun 11, 2025

Uh oh!

salaboy commented Jun 11, 2025

Uh oh!

codecov bot commented Jun 11, 2025

Codecov Report

Uh oh!

cicoyle commented Jun 11, 2025

Uh oh!

lony2003 commented Jun 11, 2025

Uh oh!

lony2003 commented Jun 11, 2025

Uh oh!

artur-ciocanu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

salaboy commented May 26, 2025 •

edited

Loading