Include image name in RemoteDockerImage#toString (#2558)

* clean up imports in GenericContainer

* include image name in RemoteDockerImage.toString() to fix #2443

* test RemoteDockerImage directly

* handle failures getting the image name

* use completeExceptionally instead of mocking

* tweak assertions

* more assertion tweaks

* add isDone check to RemoteDockerImage.toString -- still figuring out why test fails

* go back to using lombok @tostring, exclude dockerClient while we're at it

* use `Futures.lazyTransform` that also proxies `isDone`

* `imageNameToString` should not be public

* Include the message on error

Co-authored-by: Byron David <[email protected]>
Co-authored-by: David Byron <[email protected]>

Author: 3 people

core/src/main/java/org/testcontainers/images/RemoteDockerImage.java

@@ -3,6 +3,7 @@
 import com.github.dockerjava.api.DockerClient;
 import com.github.dockerjava.api.exception.DockerClientException;
 import com.github.dockerjava.api.exception.InternalServerErrorException;
+import com.google.common.util.concurrent.Futures;
 import lombok.AccessLevel;
 import lombok.AllArgsConstructor;
 import lombok.NonNull;
@@ -28,11 +29,13 @@ public class RemoteDockerImage extends LazyFuture<String> {
 
     private static final Duration PULL_RETRY_TIME_LIMIT = Duration.ofMinutes(2);
 
+    @ToString.Exclude
     private Future<DockerImageName> imageNameFuture;
 
     @Wither
     private ImagePullPolicy imagePullPolicy = PullPolicy.defaultPolicy();
 
+    @ToString.Exclude
     private DockerClient dockerClient = DockerClientFactory.lazyClient();
 
     public RemoteDockerImage(String dockerImageName) {
@@ -44,19 +47,13 @@ public RemoteDockerImage(@NonNull String repository, @NonNull String tag) {
     }
 
     public RemoteDockerImage(@NonNull Future<String> imageFuture) {
-        this.imageNameFuture = new LazyFuture<DockerImageName>() {
-            @Override
-            @SneakyThrows({InterruptedException.class, ExecutionException.class})
-            protected DockerImageName resolve() {
-                return new DockerImageName(imageFuture.get());
-            }
-        };
+        this.imageNameFuture = Futures.lazyTransform(imageFuture, DockerImageName::new);
     }
 
     @Override
     @SneakyThrows({InterruptedException.class, ExecutionException.class})
     protected final String resolve() {
-        final DockerImageName imageName = imageNameFuture.get();
+        final DockerImageName imageName = getImageName();
         Logger logger = DockerLoggerFactory.getLogger(imageName.toString());
         try {
             if (!imagePullPolicy.shouldPull(imageName)) {
@@ -95,4 +92,21 @@ protected final String resolve() {
             throw new ContainerFetchException("Failed to get Docker client for " + imageName, e);
         }
     }
+
+    private DockerImageName getImageName() throws InterruptedException, ExecutionException {
+        return imageNameFuture.get();
+    }
+
+    @ToString.Include(name = "imageName", rank = 1)
+    private String imageNameToString() {
+        if (!imageNameFuture.isDone()) {
+            return "<resolving>";
+        }
+
+        try {
+            return getImageName().toString();
+        } catch (InterruptedException | ExecutionException e) {
+            return e.getMessage();
+        }
+    }
 }

core/src/test/java/org/testcontainers/containers/GenericContainerTest.java

@@ -2,7 +2,6 @@
 
 import com.github.dockerjava.api.DockerClient;
 import com.github.dockerjava.api.command.InspectContainerResponse.ContainerState;
-import com.github.dockerjava.api.model.HostConfig;
 import lombok.RequiredArgsConstructor;
 import lombok.SneakyThrows;
 import lombok.experimental.FieldDefaults;

core/src/test/java/org/testcontainers/images/RemoteDockerImageTest.java

@@ -0,0 +1,47 @@
+package org.testcontainers.images;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+
+import org.junit.Test;
+import org.testcontainers.utility.Base58;
+
+import java.util.concurrent.CompletableFuture;
+
+public class RemoteDockerImageTest {
+
+    @Test
+    public void toStringContainsOnlyImageName() {
+        String imageName = Base58.randomString(8).toLowerCase();
+        RemoteDockerImage remoteDockerImage = new RemoteDockerImage(imageName);
+        assertThat(remoteDockerImage.toString(), containsString("imageName=" + imageName));
+    }
+
+    @Test
+    public void toStringWithExceptionContainsOnlyImageNameFuture()  {
+        CompletableFuture<String> imageNameFuture = new CompletableFuture<>();
+        imageNameFuture.completeExceptionally(new RuntimeException("arbitrary"));
+
+        RemoteDockerImage remoteDockerImage = new RemoteDockerImage(imageNameFuture);
+        assertThat(remoteDockerImage.toString(), containsString("imageName=java.lang.RuntimeException: arbitrary"));
+    }
+
+    @Test(timeout=5000L)
+    public void toStringDoesntResolveImageNameFuture()  {
+        CompletableFuture<String> imageNameFuture = new CompletableFuture<>();
+
+        // verify that we've set up the test properly
+        assertFalse(imageNameFuture.isDone());
+
+        RemoteDockerImage remoteDockerImage = new RemoteDockerImage(imageNameFuture);
+        assertThat(remoteDockerImage.toString(), containsString("imageName=<resolving>"));
+
+        // Make sure the act of calling toString doesn't resolve the imageNameFuture
+        assertFalse(imageNameFuture.isDone());
+
+        String imageName = Base58.randomString(8).toLowerCase();
+        imageNameFuture.complete(imageName);
+        assertThat(remoteDockerImage.toString(), containsString("imageName=" + imageName));
+    }
+}