Skip to content

Commit

Permalink
Merge pull request #139 from cfg4j/fail-safe-for-getConfiguration
Browse files Browse the repository at this point in the history 
Fail safe for get configuration
  • Loading branch information
@norbertpotocki
norbertpotocki committed Apr 7, 2016
2 parents 18dd9e7 + 0a7d663 commit dc044d2
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 12 deletions.
9 changes: 9 additions & 0 deletions cfg4j-consul/src/main/java/org/cfg4j/source/consul/ConsulConfigurationSource.java
View file
Expand Up @@ -44,6 +44,7 @@ class ConsulConfigurationSource implements ConfigurationSource {
private Map<String, String> consulValues;
private final String host;
private final int port;
private boolean initialized;

/**
* Note: use {@link ConsulConfigurationSourceBuilder} for building instances of this class.
Expand All @@ -56,12 +57,18 @@ class ConsulConfigurationSource implements ConfigurationSource {
ConsulConfigurationSource(String host, int port) {
this.host = requireNonNull(host);
this.port = port;

initialized = false;
}

@Override
public Properties getConfiguration(Environment environment) {
LOG.trace("Requesting configuration for environment: " + environment.getName());

if (!initialized) {
throw new IllegalStateException("Configuration source has to be successfully initialized before you request configuration.");
}

Properties properties = new Properties();
String path = environment.getName();

Expand Down Expand Up @@ -97,6 +104,7 @@ public void init() {
}

reload();
initialized = true;
}

@Override
Expand All @@ -108,6 +116,7 @@ public void reload() {
LOG.debug("Reloading configuration from Consuls' K-V store");
valueList = kvClient.getValues("/");
} catch (Exception e) {
initialized = false;
throw new SourceCommunicationException("Can't get values from k-v store", e);
}

Expand Down
28 changes: 26 additions & 2 deletions ...onsul/src/test/java/org/cfg4j/source/consul/ConsulConfigurationSourceIntegrationTest.java
View file
Expand Up @@ -52,7 +52,7 @@ private class ModifiableDispatcher extends Dispatcher {

private boolean usWest2Toggle = false;

public void toggleUsWest2() {
void toggleUsWest2() {
usWest2Toggle = !usWest2Toggle;
}

Expand Down Expand Up @@ -124,7 +124,7 @@ public void getConfigurationShouldReturnAllKeysFromGivenEnvironment() throws Exc
}

@Test
public void getConfigurationShouldIgnoreLeadginSlashInGivenEnvironment() throws Exception {
public void getConfigurationShouldIgnoreLeadingSlashInGivenEnvironment() throws Exception {
Environment environment = new ImmutableEnvironment("/us-west-1");

assertThat(source.getConfiguration(environment)).contains(MapEntry.entry("featureA.toggle", "disabled"));
Expand All @@ -140,6 +140,30 @@ public void getConfigurationShouldBeUpdatedByReload() throws Exception {
assertThat(source.getConfiguration(environment)).contains(MapEntry.entry("featureA.toggle", "enabled"));
}

@Test
public void getConfigurationShouldThrowBeforeInitCalled() throws Exception {
source = new ConsulConfigurationSourceBuilder()
.withHost(server.getHostName())
.withPort(server.getPort())
.build();

expectedException.expect(IllegalStateException.class);
source.getConfiguration(new ImmutableEnvironment(""));
}

@Test
public void getConfigurationShouldThrowAfterFailedReload() throws Exception {
server.shutdown();
try {
source.reload();
} catch (Exception e) {
// NOP
}

expectedException.expect(IllegalStateException.class);
source.getConfiguration(new ImmutableEnvironment(""));
}

@Test
public void reloadShouldThrowOnConnectionFailure() throws Exception {
server.shutdown();
Expand Down
10 changes: 10 additions & 0 deletions cfg4j-git/src/main/java/org/cfg4j/source/git/GitConfigurationSource.java
View file
Expand Up @@ -61,6 +61,7 @@ class GitConfigurationSource implements ConfigurationSource, Closeable {
private final String tmpRepoPrefix;
private Git clonedRepo;
private Path clonedRepoPath;
private boolean initialized;

/**
* Note: use {@link GitConfigurationSourceBuilder} for building instances of this class.
Expand Down Expand Up @@ -88,10 +89,16 @@ class GitConfigurationSource implements ConfigurationSource, Closeable {
this.repositoryURI = requireNonNull(repositoryURI);
this.tmpPath = requireNonNull(tmpPath);
this.tmpRepoPrefix = requireNonNull(tmpRepoPrefix);

initialized = false;
}

@Override
public Properties getConfiguration(Environment environment) {
if (!initialized) {
throw new IllegalStateException("Configuration source has to be successfully initialized before you request configuration.");
}

try {
checkoutToBranch(branchResolver.getBranchNameFor(environment));
} catch (GitAPIException e) {
Expand Down Expand Up @@ -143,6 +150,8 @@ public void init() {
} catch (GitAPIException e) {
throw new SourceCommunicationException("Unable to clone repository: " + repositoryURI, e);
}

initialized = true;
}

@Override
Expand All @@ -151,6 +160,7 @@ public void reload() {
LOG.debug("Reloading configuration by pulling changes");
clonedRepo.pull().call();
} catch (GitAPIException e) {
initialized = false;
throw new IllegalStateException("Unable to pull from remote repository", e);
}
}
Expand Down
44 changes: 34 additions & 10 deletions cfg4j-git/src/test/java/org/cfg4j/source/git/GitConfigurationSourceIntegrationTest.java
View file
Expand Up @@ -85,7 +85,7 @@ public void initShouldThrowOnInvalidRemote() throws Exception {
}

@Test
public void getConfiguration2ShouldUseBranchResolver() throws Exception {
public void getConfigurationShouldUseBranchResolver() throws Exception {
class Resolver implements BranchResolver {

@Override
Expand All @@ -102,7 +102,7 @@ public String getBranchNameFor(Environment environment) {
}

@Test
public void getConfiguration2ShouldReadConfigFromGivenBranch() throws Exception {
public void getConfigurationShouldReadConfigFromGivenBranch() throws Exception {
try (GitConfigurationSource gitConfigurationSource = getSourceForRemoteRepoWithDefaults()) {
Environment environment = new ImmutableEnvironment(TEST_ENV_BRANCH);

Expand All @@ -111,7 +111,7 @@ public void getConfiguration2ShouldReadConfigFromGivenBranch() throws Exception
}

@Test
public void getConfiguration2ShouldUsePathResolver() throws Exception {
public void getConfigurationShouldUsePathResolver() throws Exception {
class Resolver implements PathResolver {

@Override
Expand All @@ -128,7 +128,7 @@ public Path getPathFor(Environment environment) {
}

@Test
public void getConfiguration2ShouldReadFromGivenPath() throws Exception {
public void getConfigurationShouldReadFromGivenPath() throws Exception {
try (GitConfigurationSource gitConfigurationSource = getSourceForRemoteRepoWithDefaults()) {
Environment environment = new ImmutableEnvironment("/otherApplicationConfigs/");

Expand All @@ -137,7 +137,7 @@ public void getConfiguration2ShouldReadFromGivenPath() throws Exception {
}

@Test
public void getConfiguration2ShouldReadFromGivenFiles() throws Exception {
public void getConfigurationShouldReadFromGivenFiles() throws Exception {
ConfigFilesProvider configFilesProvider = new ConfigFilesProvider() {
@Override
public Iterable<Path> getConfigFiles() {
Expand All @@ -152,15 +152,15 @@ public Iterable<Path> getConfigFiles() {
}

@Test
public void getConfiguration2ShouldThrowOnMissingBranch() throws Exception {
public void getConfigurationShouldThrowOnMissingBranch() throws Exception {
try (GitConfigurationSource gitConfigurationSource = getSourceForRemoteRepoWithDefaults()) {
expectedException.expect(MissingEnvironmentException.class);
gitConfigurationSource.getConfiguration(new ImmutableEnvironment("nonExistentBranch"));
}
}

@Test
public void getConfiguration2ShouldThrowOnMissingConfigFile() throws Exception {
public void getConfigurationShouldThrowOnMissingConfigFile() throws Exception {
remoteRepo.deleteFile(Paths.get("application.properties"));

try (GitConfigurationSource gitConfigurationSource = getSourceForRemoteRepoWithDefaults()) {
Expand All @@ -170,7 +170,7 @@ public void getConfiguration2ShouldThrowOnMissingConfigFile() throws Exception {
}

@Test
public void getConfiguration2ShouldThrowOnMalformedConfigFile() throws Exception {
public void getConfigurationShouldThrowOnMalformedConfigFile() throws Exception {
ConfigFilesProvider configFilesProvider = new ConfigFilesProvider() {
@Override
public Iterable<Path> getConfigFiles() {
Expand All @@ -183,7 +183,31 @@ public Iterable<Path> getConfigFiles() {
}

@Test
public void reloadShouldUpdateGetConfiguration2OnDefaultBranch() throws Exception {
public void getConfigurationShouldThrowBeforeInitCalled() throws Exception {
try (GitConfigurationSource gitConfigurationSource = getSourceBuilderForRemoteRepoWithDefaults().build()) {
expectedException.expect(IllegalStateException.class);
gitConfigurationSource.getConfiguration(new ImmutableEnvironment(""));
}
}

@Test
public void getConfigurationShouldThrowAfterFailedReload() throws Exception {
try (GitConfigurationSource gitConfigurationSource = getSourceForRemoteRepoWithDefaults()) {
remoteRepo.remove();

try {
gitConfigurationSource.reload();
} catch (Exception e) {
// NOP
}

expectedException.expect(IllegalStateException.class);
gitConfigurationSource.getConfiguration(new ImmutableEnvironment(""));
}
}

@Test
public void reloadShouldUpdateGetConfigurationOnDefaultBranch() throws Exception {
try (GitConfigurationSource gitConfigurationSource = getSourceForRemoteRepoWithDefaults()) {
remoteRepo.changeProperty(Paths.get("application.properties"), "some.setting", "changedValue");
gitConfigurationSource.reload();
Expand All @@ -193,7 +217,7 @@ public void reloadShouldUpdateGetConfiguration2OnDefaultBranch() throws Exceptio
}

@Test
public void reloadShouldUpdateGetConfiguration2OnNonDefaultBranch() throws Exception {
public void reloadShouldUpdateGetConfigurationOnNonDefaultBranch() throws Exception {
try (GitConfigurationSource gitConfigurationSource = getSourceForRemoteRepoWithDefaults()) {
remoteRepo.changeBranchTo(TEST_ENV_BRANCH);
remoteRepo.changeProperty(Paths.get("application.properties"), "some.setting", "changedValue");
Expand Down

0 comments on commit dc044d2

Please sign in to comment.