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 org.testcontainers.containers.DockerComposeContainer as a source for ContainerConnectionDetailsFactory #43174

Open
reda-alaoui opened this issue Nov 15, 2024 · 2 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@reda-alaoui
Copy link

reda-alaoui commented Nov 15, 2024

I'd like to define a new ConnectionDetails type for https://github.com/eclipse/kapua .
Eclipse Kapua can only be run via Docker compose. To achieve that, I run it successfully via org.testcontainers.containers.DockerComposeContainer.

I added a KapuaConnectionDetails:

public interface KapuaConnectionDetails extends ConnectionDetails {

  String baseHttpUrl();
}

I would have expected to be able to plug DockerComposeContainer as source of ContainerConnectionDetailsFactory.

But the current signature of ContainerConnectionDetailsFactory is:

public abstract class ContainerConnectionDetailsFactory<C extends Container<?>, D extends ConnectionDetails>
		implements ConnectionDetailsFactory<ContainerConnectionSource<C>, D> {}

Because of that, C excludes org.testcontainers.containers.DockerComposeContainer.

By looking at ContainerConnectionDetailsFactory, I didn't find a good reason for why C is restricted to Container<?>. ContainerConnectionDetailsFactory and its dependencies do not use Container<?>'s method. The only used method is from Startable, and it is only used after a cast check:

Could we relax C restriction to allow DockerComposeContainer usage?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 15, 2024
@reda-alaoui
Copy link
Author

With the following patch, Spring Boot project still compiles:

Index: spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ServiceConnectionContextCustomizer.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ServiceConnectionContextCustomizer.java b/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ServiceConnectionContextCustomizer.java
--- a/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ServiceConnectionContextCustomizer.java	(revision f77c3bbd6bddfb09041a63470a10974e09ebf424)
+++ b/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ServiceConnectionContextCustomizer.java	(date 1731667616310)
@@ -20,8 +20,6 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import org.testcontainers.containers.Container;
-
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.boot.autoconfigure.service.connection.ConnectionDetails;
@@ -91,7 +89,7 @@
 	 * Relevant details from {@link ContainerConnectionSource} used as a
 	 * MergedContextConfiguration cache key.
 	 */
-	private record CacheKey(String connectionName, Set<Class<?>> connectionDetailsTypes, Container<?> container) {
+	private record CacheKey(String connectionName, Set<Class<?>> connectionDetailsTypes, Object container) {
 
 		CacheKey(ContainerConnectionSource<?> source) {
 			this(source.getConnectionName(), source.getConnectionDetailsTypes(), source.getContainerSupplier().get());
Index: spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionSource.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionSource.java b/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionSource.java
--- a/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionSource.java	(revision f77c3bbd6bddfb09041a63470a10974e09ebf424)
+++ b/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionSource.java	(date 1731667535088)
@@ -42,7 +42,7 @@
  * @since 3.1.0
  * @see ContainerConnectionDetailsFactory
  */
-public final class ContainerConnectionSource<C extends Container<?>> implements OriginProvider {
+public final class ContainerConnectionSource<C> implements OriginProvider {
 
 	private static final Log logger = LogFactory.getLog(ContainerConnectionSource.class);
 
Index: spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionDetailsFactory.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionDetailsFactory.java b/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionDetailsFactory.java
--- a/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionDetailsFactory.java	(revision f77c3bbd6bddfb09041a63470a10974e09ebf424)
+++ b/spring-boot-project/spring-boot-testcontainers/src/main/java/org/springframework/boot/testcontainers/service/connection/ContainerConnectionDetailsFactory.java	(date 1731667526709)
@@ -53,7 +53,7 @@
  * @author Phillip Webb
  * @since 3.1.0
  */
-public abstract class ContainerConnectionDetailsFactory<C extends Container<?>, D extends ConnectionDetails>
+public abstract class ContainerConnectionDetailsFactory<C, D extends ConnectionDetails>
 		implements ConnectionDetailsFactory<ContainerConnectionSource<C>, D> {
 
 	/**

@philwebb philwebb changed the title Allow org.testcontainers.containers.DockerComposeContainer as a source for ContainerConnectionDetailsFactory Allow org.testcontainers.containers.DockerComposeContainer as a source for ContainerConnectionDetailsFactory Nov 15, 2024
@philwebb philwebb added for: team-meeting An issue we'd like to discuss as a team to make progress type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Nov 15, 2024
@philwebb philwebb added this to the 3.x milestone Nov 18, 2024
@philwebb
Copy link
Member

Thanks for the report. We discussed this today as a team and we would like to support DockerComposeContainer, however, we don't think we can change the generic signature in the way you suggest because we believe that it will break back compatibility.

We'll have to think about how we can support this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants