Adding @ServiceConnection spring boot support with Testcontainers #1118

salaboy · 2024-09-03T09:20:04Z

Description

This PR implements the Testcontainers @Serviceconnection to simplify how applications can connect to DaprContainers.
This PR also aligns Spring Boot dependencies to reduce the amount of dependencies that users need to add to their projects.

Issue reference

Fixes: #1117

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

eddumelendez

Just leaving some suggestions and improvements

.../src/main/java/io/dapr/spring/boot/autoconfigure/client/PropertiesDaprConnectionDetails.java

...gure/src/main/java/io/dapr/spring/boot/autoconfigure/client/DaprClientAutoConfiguration.java

...oot-tests/src/main/java/io/dapr/spring/boot/tests/DaprContainerConnectionDetailsFactory.java

sdk-tests/src/test/java/io/dapr/it/spring/messaging/DaprSpringMessagingIT.java

dapr-spring/dapr-spring-boot-tests/src/main/resources/META-INF/spring.factories

artur-ciocanu · 2024-09-04T18:49:27Z

@salaboy could you please rebase your PR. Thank you!

salaboy · 2024-09-05T10:28:30Z

@artursouza @cicoyle this is the last PR to get the Documentation working with an example. Please review and I am happy to have a call if you want to make the review faster.

artur-ciocanu

@salaboy awesome job! I have left a few tiny comments, mostly for renaming and cleanup.

artur-ciocanu · 2024-09-05T10:29:08Z

...toconfigure/src/main/java/io/dapr/spring/boot/autoconfigure/client/DaprClientProperties.java

+package io.dapr.spring.boot.autoconfigure.client;
+
+import org.springframework.boot.context.properties.ConfigurationProperties;
+


Just a small question, should we use the old Java style Java Beans or should we recommend users to go with Java records?

both should be fine right?

IMO, I would go with java beans in order to add javadoc and could generate the metadata, the configuration processor is already in the classpath. See https://docs.spring.io/spring-boot/specification/configuration-metadata/annotation-processor.html

artur-ciocanu · 2024-09-05T10:29:46Z

...apr/spring/boot/testcontainers/service/connection/DaprContainerConnectionDetailsFactory.java

+
+public class DaprContainerConnectionDetailsFactory
+        extends ContainerConnectionDetailsFactory<DaprContainer, DaprConnectionDetails> {
+


I don't think this constructor provides any value

artur-ciocanu · 2024-09-05T10:30:47Z

...apr/spring/boot/testcontainers/service/connection/DaprContainerConnectionDetailsFactory.java

+import io.dapr.testcontainers.DaprContainer;
+import org.springframework.boot.testcontainers.service.connection.ContainerConnectionDetailsFactory;
+import org.springframework.boot.testcontainers.service.connection.ContainerConnectionSource;
+


We could rename this to DaprConnectionDetailsFactory so it is similar to other connection details factory like Redis, Mongo, Kafka, etc

@artur-ciocanu I am following testcontainers conventions as the connectiondetails for this will be extracted from the DaprContainer

artur-ciocanu · 2024-09-05T10:32:18Z

...apr/spring/boot/testcontainers/service/connection/DaprContainerConnectionDetailsFactory.java

+  protected DaprConnectionDetails getContainerConnectionDetails(ContainerConnectionSource<DaprContainer> source) {
+    return new DaprContainerConnectionDetails(source);
+  }
+


Could we rename this to DefaultDaprConnectionDetails, DaprContainer seems redundant. We want to connect to Dapr, irrespective where it is running, container or not.

The naming convention comes from this: DaprContainer paramter

artur-ciocanu · 2024-09-05T10:33:04Z

sdk-tests/src/test/java/io/dapr/it/spring/data/MySQLDaprKeyValueTemplateIT.java

@@ -93,6 +95,8 @@ public class MySQLDaprKeyValueTemplateIT {
  static void daprProperties(DynamicPropertyRegistry registry) {


We should remove the daprProperties method, we are using @ServiceConnection

eddumelendez · 2024-09-05T17:26:59Z

...toconfigure/src/main/java/io/dapr/spring/boot/autoconfigure/client/DaprClientProperties.java

+package io.dapr.spring.boot.autoconfigure.client;
+
+import org.springframework.boot.context.properties.ConfigurationProperties;
+


IMO, I would go with java beans in order to add javadoc and could generate the metadata, the configuration processor is already in the classpath. See https://docs.spring.io/spring-boot/specification/configuration-metadata/annotation-processor.html

dapr-spring/dapr-spring-boot-starters/dapr-spring-boot-starter-test/pom.xml

...apr/spring/boot/testcontainers/service/connection/DaprContainerConnectionDetailsFactory.java

...toconfigure/src/main/java/io/dapr/spring/boot/autoconfigure/client/DaprClientProperties.java

artur-ciocanu · 2024-09-06T13:10:40Z

@salaboy I have added a small comment, the issue with the build that you have faced is related to DaprClientProperties. You have two options:

Remove the constructor and leave the setters
Remove the setters and ensure that we preserve parameter names by leveraging the -parameter compiler flag. In order to preserver the parameter name you should add the <parameters>true</parameters> to Maven Compiler Plugin in the Parent POM. Something like this:

          <configuration>
            <source>${maven.compiler.source}</source>
            <target>${maven.compiler.target}</target>
            <release>${maven.compiler.release}</release>
            <parameters>true</parameters>
          </configuration>

Let me know your thoughts.

Signed-off-by: salaboy <Salaboy@gmail.com>

…pr/spring/boot/autoconfigure/client/DaprClientAutoConfiguration.java Co-authored-by: Eddú Meléndez Gonzales <eddu.melendez@gmail.com> Signed-off-by: salaboy <Salaboy@gmail.com>

…pr/spring/boot/autoconfigure/client/PropertiesDaprConnectionDetails.java Co-authored-by: Eddú Meléndez Gonzales <eddu.melendez@gmail.com> Signed-off-by: salaboy <Salaboy@gmail.com>

Signed-off-by: salaboy <Salaboy@gmail.com>

salaboy · 2024-09-06T16:43:53Z

@salaboy I have added a small comment, the issue with the build that you have faced is related to DaprClientProperties. You have two options:

Remove the constructor and leave the setters

Remove the setters and ensure that we preserve parameter names by leveraging the -parameter compiler flag. In order to preserver the parameter name you should add the <parameters>true</parameters> to Maven Compiler Plugin in the Parent POM. Something like this:
          <configuration>
            <source>${maven.compiler.source}</source>
            <target>${maven.compiler.target}</target>
            <release>${maven.compiler.release}</release>
            <parameters>true</parameters>
          </configuration>
Let me know your thoughts.

This is solved now

salaboy requested review from a team as code owners September 3, 2024 09:20

salaboy force-pushed the service-conn branch from 8d23d1a to 13673ab Compare September 3, 2024 09:21

salaboy marked this pull request as draft September 3, 2024 09:22

salaboy force-pushed the service-conn branch 2 times, most recently from 1850c5b to b6bd2ee Compare September 3, 2024 10:50

eddumelendez reviewed Sep 3, 2024

View reviewed changes

salaboy force-pushed the service-conn branch 2 times, most recently from 1307148 to cbced54 Compare September 4, 2024 14:23

artur-ciocanu reviewed Sep 4, 2024

View reviewed changes

sdk-tests/src/test/java/io/dapr/it/spring/messaging/DaprSpringMessagingIT.java Outdated Show resolved Hide resolved

artur-ciocanu reviewed Sep 4, 2024

View reviewed changes

dapr-spring/dapr-spring-boot-tests/src/main/resources/META-INF/spring.factories Outdated Show resolved Hide resolved

salaboy force-pushed the service-conn branch 3 times, most recently from 7867cc6 to 2e1eb11 Compare September 5, 2024 08:24

salaboy marked this pull request as ready for review September 5, 2024 08:51

salaboy changed the title ~~[WIP] Adding @ServiceConnection spring boot support with Testcontainers~~ Adding @ServiceConnection spring boot support with Testcontainers Sep 5, 2024

salaboy requested review from eddumelendez and artur-ciocanu September 5, 2024 08:52

salaboy force-pushed the service-conn branch 4 times, most recently from a3cf5ec to c85cd22 Compare September 5, 2024 09:46

artur-ciocanu approved these changes Sep 5, 2024

View reviewed changes

salaboy force-pushed the service-conn branch from 691d852 to 697c471 Compare September 5, 2024 13:47

eddumelendez reviewed Sep 5, 2024

View reviewed changes

salaboy force-pushed the service-conn branch 2 times, most recently from 9f130eb to 73a1f55 Compare September 6, 2024 10:35

artur-ciocanu reviewed Sep 6, 2024

View reviewed changes

...toconfigure/src/main/java/io/dapr/spring/boot/autoconfigure/client/DaprClientProperties.java Show resolved Hide resolved

salaboy and others added 17 commits September 6, 2024 14:16

adding service connection plumbing

1548595

Signed-off-by: salaboy <Salaboy@gmail.com>

fixing style

d70bc0d

Signed-off-by: salaboy <Salaboy@gmail.com>

updating sdk-tests

227283d

Signed-off-by: salaboy <Salaboy@gmail.com>

Update dapr-spring/dapr-spring-boot-autoconfigure/src/main/java/io/da…

23d111e

…pr/spring/boot/autoconfigure/client/DaprClientAutoConfiguration.java Co-authored-by: Eddú Meléndez Gonzales <eddu.melendez@gmail.com> Signed-off-by: salaboy <Salaboy@gmail.com>

Update dapr-spring/dapr-spring-boot-autoconfigure/src/main/java/io/da…

4c4a25e

…pr/spring/boot/autoconfigure/client/PropertiesDaprConnectionDetails.java Co-authored-by: Eddú Meléndez Gonzales <eddu.melendez@gmail.com> Signed-off-by: salaboy <Salaboy@gmail.com>

fixing details

841d603

Signed-off-by: salaboy <Salaboy@gmail.com>

adding @Serviceconnection to Dapr

b98240e

Signed-off-by: salaboy <Salaboy@gmail.com>

fixing tests and style

8cd1bb7

Signed-off-by: salaboy <Salaboy@gmail.com>

removing test that is not needed anymore

78d6849

Signed-off-by: salaboy <Salaboy@gmail.com>

updating starter dependencies

9f507c3

Signed-off-by: salaboy <Salaboy@gmail.com>

adding juniper testcontainers support

8a52c4b

Signed-off-by: salaboy <Salaboy@gmail.com>

adding new testing module

2720f91

Signed-off-by: salaboy <Salaboy@gmail.com>

cleaning sdk-tests deps

bec5a9f

Signed-off-by: salaboy <Salaboy@gmail.com>

removing dead code

c16f5af

Signed-off-by: salaboy <Salaboy@gmail.com>

removing core that is not needed

8c052ec

Signed-off-by: salaboy <Salaboy@gmail.com>

adding setters

f23514a

Signed-off-by: salaboy <Salaboy@gmail.com>

default constructor

82eb498

Signed-off-by: salaboy <Salaboy@gmail.com>

salaboy force-pushed the service-conn branch from a1247d0 to 82eb498 Compare September 6, 2024 13:16

artursouza approved these changes Sep 6, 2024

View reviewed changes

artursouza merged commit 4b83da6 into dapr:master Sep 6, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding @ServiceConnection spring boot support with Testcontainers #1118

Adding @ServiceConnection spring boot support with Testcontainers #1118

salaboy commented Sep 3, 2024 •

edited

Loading

eddumelendez left a comment

artur-ciocanu commented Sep 4, 2024

salaboy commented Sep 5, 2024

artur-ciocanu left a comment

artur-ciocanu Sep 5, 2024

salaboy Sep 5, 2024

eddumelendez Sep 5, 2024

artur-ciocanu Sep 5, 2024

artur-ciocanu Sep 5, 2024

salaboy Sep 5, 2024

artur-ciocanu Sep 5, 2024

salaboy Sep 5, 2024

artur-ciocanu Sep 5, 2024

eddumelendez Sep 5, 2024

artur-ciocanu commented Sep 6, 2024

salaboy commented Sep 6, 2024

		package io.dapr.spring.boot.autoconfigure.client;

		import org.springframework.boot.context.properties.ConfigurationProperties;


		public class DaprContainerConnectionDetailsFactory
		extends ContainerConnectionDetailsFactory<DaprContainer, DaprConnectionDetails> {

		@@ -93,6 +95,8 @@ public class MySQLDaprKeyValueTemplateIT {
		static void daprProperties(DynamicPropertyRegistry registry) {

Adding @ServiceConnection spring boot support with Testcontainers #1118

Adding @ServiceConnection spring boot support with Testcontainers #1118

Conversation

salaboy commented Sep 3, 2024 • edited Loading

Description

Issue reference

Checklist

eddumelendez left a comment

Choose a reason for hiding this comment

artur-ciocanu commented Sep 4, 2024

salaboy commented Sep 5, 2024

artur-ciocanu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artur-ciocanu commented Sep 6, 2024

salaboy commented Sep 6, 2024

salaboy commented Sep 3, 2024 •

edited

Loading