Separate "environment" and "classpath" properties (for global things) (#1784)

* Separate "environment" and "classpath" properties (for global things)

* Update TestcontainersConfiguration.java

Author: bsideup

core/src/main/java/org/testcontainers/dockerclient/DockerClientProviderStrategy.java

@@ -167,20 +167,10 @@ public DockerClient getClient() {
     }
 
     protected DockerClient getClientForConfig(DockerClientConfig config) {
-        DockerClientBuilder clientBuilder = DockerClientBuilder
-            .getInstance(new AuthDelegatingDockerClientConfig(config));
-
-        String transportType = TestcontainersConfiguration.getInstance().getTransportType();
-        if ("okhttp".equals(transportType)) {
-            clientBuilder
-                .withDockerCmdExecFactory(new OkHttpDockerCmdExecFactory());
-        } else {
-            throw new IllegalArgumentException("Unknown transport type: " + transportType);
-        }
-
-        LOGGER.info("Will use '{}' transport", transportType);
-
-        return clientBuilder.build();
+        return DockerClientBuilder
+            .getInstance(new AuthDelegatingDockerClientConfig(config))
+            .withDockerCmdExecFactory(new OkHttpDockerCmdExecFactory())
+            .build();
     }
 
     protected void ping(DockerClient client, int timeoutInSeconds) {

core/src/main/java/org/testcontainers/utility/TestcontainersConfiguration.java

@@ -5,6 +5,7 @@
 
 import java.io.*;
 import java.net.MalformedURLException;
+import java.net.URL;
 import java.util.Objects;
 import java.util.Properties;
 import java.util.stream.Stream;
@@ -19,12 +20,22 @@ public class TestcontainersConfiguration {
 
     private static String PROPERTIES_FILE_NAME = "testcontainers.properties";
 
-    private static File GLOBAL_CONFIG_FILE = new File(System.getProperty("user.home"), "." + PROPERTIES_FILE_NAME);
+    private static File ENVIRONMENT_CONFIG_FILE = new File(System.getProperty("user.home"), "." + PROPERTIES_FILE_NAME);
 
     @Getter(lazy = true)
     private static final TestcontainersConfiguration instance = loadConfiguration();
 
-    private final Properties properties;
+    @Getter(AccessLevel.NONE)
+    private final Properties environmentProperties;
+
+    private final Properties properties = new Properties();
+
+    TestcontainersConfiguration(Properties environmentProperties, Properties classpathProperties) {
+        this.environmentProperties = environmentProperties;
+
+        this.properties.putAll(classpathProperties);
+        this.properties.putAll(environmentProperties);
+    }
 
     public String getAmbassadorContainerImage() {
         return (String) properties.getOrDefault("ambassador.container.image", "richnorth/ambassador:latest");
@@ -71,13 +82,18 @@ public String getPulsarImage() {
     }
 
     public boolean isDisableChecks() {
-        return Boolean.parseBoolean((String) properties.getOrDefault("checks.disable", "false"));
+        return Boolean.parseBoolean((String) environmentProperties.getOrDefault("checks.disable", "false"));
     }
 
     public String getDockerClientStrategyClassName() {
-        return (String) properties.get("docker.client.strategy");
+        return (String) environmentProperties.get("docker.client.strategy");
     }
 
+    /**
+     * 
+     * @deprecated we no longer have different transport types
+     */
+    @Deprecated
     public String getTransportType() {
         return properties.getProperty("transport.type", "okhttp");
     }
@@ -89,64 +105,55 @@ public Integer getImagePullPauseTimeout() {
     @Synchronized
     public boolean updateGlobalConfig(@NonNull String prop, @NonNull String value) {
         try {
-            Properties globalProperties = new Properties();
-            GLOBAL_CONFIG_FILE.createNewFile();
-            try (InputStream inputStream = new FileInputStream(GLOBAL_CONFIG_FILE)) {
-                globalProperties.load(inputStream);
-            }
-
-            if (value.equals(globalProperties.get(prop))) {
+            if (value.equals(environmentProperties.get(prop))) {
                 return false;
             }
 
-            globalProperties.setProperty(prop, value);
+            environmentProperties.setProperty(prop, value);
 
-            try (OutputStream outputStream = new FileOutputStream(GLOBAL_CONFIG_FILE)) {
-                globalProperties.store(outputStream, "Modified by Testcontainers");
+            ENVIRONMENT_CONFIG_FILE.createNewFile();
+            try (OutputStream outputStream = new FileOutputStream(ENVIRONMENT_CONFIG_FILE)) {
+                environmentProperties.store(outputStream, "Modified by Testcontainers");
             }
 
-            // Update internal state only if global config was successfully updated
+            // Update internal state only if environment config was successfully updated
             properties.setProperty(prop, value);
             return true;
         } catch (Exception e) {
-            log.debug("Can't store global property {} in {}", prop, GLOBAL_CONFIG_FILE);
+            log.debug("Can't store environment property {} in {}", prop, ENVIRONMENT_CONFIG_FILE);
             return false;
         }
     }
 
     @SneakyThrows(MalformedURLException.class)
     private static TestcontainersConfiguration loadConfiguration() {
-        final TestcontainersConfiguration config = new TestcontainersConfiguration(
-                Stream
-                        .of(
-                                TestcontainersConfiguration.class.getClassLoader().getResource(PROPERTIES_FILE_NAME),
-                                Thread.currentThread().getContextClassLoader().getResource(PROPERTIES_FILE_NAME),
-                                GLOBAL_CONFIG_FILE.toURI().toURL()
-                        )
-                        .filter(Objects::nonNull)
-                        .map(it -> {
-                            log.debug("Testcontainers configuration overrides will be loaded from {}", it);
-
-                            final Properties subProperties = new Properties();
-                            try (final InputStream inputStream = it.openStream()) {
-                                subProperties.load(inputStream);
-                            } catch (FileNotFoundException e) {
-                                log.trace("Testcontainers config override was found on " + it + " but the file was not found", e);
-                            } catch (IOException e) {
-                                log.warn("Testcontainers config override was found on " + it + " but could not be loaded", e);
-                            }
-                            return subProperties;
-                        })
-                        .reduce(new Properties(), (a, b) -> {
-                            a.putAll(b);
-                            return a;
-                        })
+        return new TestcontainersConfiguration(
+            readProperties(ENVIRONMENT_CONFIG_FILE.toURI().toURL()),
+            Stream
+                .of(
+                    TestcontainersConfiguration.class.getClassLoader(),
+                    Thread.currentThread().getContextClassLoader()
+                )
+                .map(it -> it.getResource(PROPERTIES_FILE_NAME))
+                .filter(Objects::nonNull)
+                .map(TestcontainersConfiguration::readProperties)
+                .reduce(new Properties(), (a, b) -> {
+                    a.putAll(b);
+                    return a;
+                })
         );
+    }
 
-        if (!config.getProperties().isEmpty()) {
-            log.debug("Testcontainers configuration overrides loaded from {}", config);
+    private static Properties readProperties(URL url) {
+        log.debug("Testcontainers configuration overrides will be loaded from {}", url);
+        Properties properties = new Properties();
+        try (InputStream inputStream = url.openStream()) {
+            properties.load(inputStream);
+        } catch (FileNotFoundException e) {
+            log.trace("Testcontainers config override was found on {} but the file was not found", url, e);
+        } catch (IOException e) {
+            log.warn("Testcontainers config override was found on {} but could not be loaded", url, e);
         }
-
-        return config;
+        return properties;
     }
 }

core/src/test/java/org/testcontainers/utility/TestcontainersConfigurationTest.java

@@ -0,0 +1,43 @@
+package org.testcontainers.utility;
+
+import org.junit.Test;
+
+import java.util.Properties;
+import java.util.UUID;
+
+import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;
+import static org.rnorth.visibleassertions.VisibleAssertions.assertFalse;
+import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue;
+
+public class TestcontainersConfigurationTest {
+
+    final Properties environmentProperties = new Properties();
+
+    final Properties classpathProperties = new Properties();
+
+    @Test
+    public void shouldReadChecksFromEnvironmentOnly() {
+        assertFalse("checks enabled by default", newConfig().isDisableChecks());
+
+        classpathProperties.setProperty("checks.disable", "true");
+        assertFalse("checks are not affected by classpath properties", newConfig().isDisableChecks());
+
+        environmentProperties.setProperty("checks.disable", "true");
+        assertTrue("checks disabled", newConfig().isDisableChecks());
+    }
+
+    @Test
+    public void shouldReadDockerClientStrategyFromEnvironmentOnly() {
+        String currentValue = newConfig().getDockerClientStrategyClassName();
+
+        classpathProperties.setProperty("docker.client.strategy", UUID.randomUUID().toString());
+        assertEquals("Docker client strategy is not affected by classpath properties", currentValue, newConfig().getDockerClientStrategyClassName());
+
+        environmentProperties.setProperty("docker.client.strategy", "foo");
+        assertEquals("Docker client strategy is changed", "foo", newConfig().getDockerClientStrategyClassName());
+    }
+
+    private TestcontainersConfiguration newConfig() {
+        return new TestcontainersConfiguration(environmentProperties, classpathProperties);
+    }
+}