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

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

Merged
merged 2 commits into from Aug 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -165,20 +165,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) {
Expand Down
Expand Up @@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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"));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was never supposed to be disabled in classpath' file, but some users were misusing it and setting it "for everyone".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems sensible 👍

}

public String getDockerClientStrategyClassName() {
return (String) properties.get("docker.client.strategy");
return (String) environmentProperties.get("docker.client.strategy");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does not make sense to have it "for everyone" as well since it is strictly environment specific

}

/**
*
* @deprecated we no longer have different transport types
*/
@Deprecated
public String getTransportType() {
return properties.getProperty("transport.type", "okhttp");
}
Expand All @@ -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;
}
}
@@ -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);
}
}