Skip to content

Commit

Permalink
Modify config loader to load resources as a LinkedHashMap<String, Con…
Browse files Browse the repository at this point in the history
…fig> instead of being inconsistent with merging Configs when multiple resources are loaded

Move @next processing to PropertiesConfigReader instead of forcing it on all ConfigReader's since this actually breaks Typesafe's loading
Add addConfigs and replaceConfigs to CompositeConfig
Fix bug where application cascade loading wasn't done correctly
  • Loading branch information
elandau committed May 26, 2015
1 parent 57c2d17 commit 5ec75aa
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 114 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.io.File;
import java.net.URL;
import java.util.LinkedHashMap;
import java.util.Properties;

import com.netflix.archaius.exceptions.ConfigException;
Expand Down Expand Up @@ -69,7 +70,7 @@ public static interface Loader {
* @param resourceName
* @return
*/
Config load(String resourceName) throws ConfigException;
LinkedHashMap<String, Config> load(String resourceName) throws ConfigException;

/**
* Load configuration from a specific URL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,21 @@
import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArraySet;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.netflix.archaius.StrInterpolator.Lookup;
import com.netflix.archaius.cascade.NoCascadeStrategy;
import com.netflix.archaius.config.CompositeConfig;
import com.netflix.archaius.config.MapConfig;
import com.netflix.archaius.exceptions.ConfigException;
import com.netflix.archaius.interpolate.CommonsStrInterpolator;
import com.netflix.archaius.interpolate.ConfigStrLookup;
import com.netflix.archaius.loaders.PropertiesConfigReader;
import com.netflix.archaius.readers.PropertiesConfigReader;

/**
* DefaultConfigLoader provides a DSL to load configurations.
Expand All @@ -47,7 +43,7 @@
*/
public class DefaultConfigLoader implements ConfigLoader {

private static final Logger LOGGER = LoggerFactory.getLogger(DefaultConfigLoader.class);
private static final Logger LOG = LoggerFactory.getLogger(DefaultConfigLoader.class);

private static final NoCascadeStrategy DEFAULT_CASCADE_STRATEGY = new NoCascadeStrategy();
private static final Lookup DEFAULT_LOOKUP = new Lookup() {
Expand All @@ -62,7 +58,6 @@ public static class Builder {
private List<ConfigReader> loaders = new ArrayList<ConfigReader>();
private CascadeStrategy defaultStrategy = DEFAULT_CASCADE_STRATEGY;
private boolean failOnFirst = true;
private String includeKey = "@next";
private StrInterpolator interpolator = DEFAULT_INTERPOLATOR;
private Lookup lookup = DEFAULT_LOOKUP;

Expand Down Expand Up @@ -111,12 +106,6 @@ public Builder withConfigReader(List<ConfigReader> loaders) {
return this;
}

public Builder withIncludeKey(String key) {
if (key != null)
this.includeKey = key;
return this;
}

public DefaultConfigLoader build() {
if (loaders.isEmpty()) {
loaders.add(new PropertiesConfigReader());
Expand All @@ -131,18 +120,14 @@ public static Builder builder() {

private final List<ConfigReader> loaders;
private final CascadeStrategy defaultStrategy;
private final String includeKey;
private final StrInterpolator interpolator;
private final Lookup lookup;
private final CopyOnWriteArraySet<String> alreadyLoaded;
private final Lookup lookup;
private final boolean defaultFailOnFirst;

public DefaultConfigLoader(Builder builder) {
this.includeKey = builder.includeKey;
this.loaders = builder.loaders;
this.defaultStrategy = builder.defaultStrategy;
this.interpolator = builder.interpolator;
this.alreadyLoaded = new CopyOnWriteArraySet<String>();
this.defaultFailOnFirst = builder.failOnFirst;
this.lookup = builder.lookup;
}
Expand Down Expand Up @@ -180,65 +165,32 @@ public Loader withOverrides(Properties props) {
}

@Override
public Config load(String resourceName) throws ConfigException {
public LinkedHashMap<String, Config> load(String resourceName) throws ConfigException {
LinkedHashMap<String, Config> configs = new LinkedHashMap<String, Config>();
boolean failIfNotLoaded = failOnFirst;
if (overrides != null && !overrides.isEmpty()) {
configs.put(resourceName, new MapConfig(overrides));
}

for (String permutationName : strategy.generate(resourceName, interpolator, lookup)) {
LOGGER.info("Attempting to load {}", permutationName);
LOG.info("Attempting to load {}", permutationName);
for (ConfigReader loader : loaders) {
if (loader.canLoad(classLoader, permutationName)) {
Config config;
String fileToLoad = permutationName;
do {
try {
config = loader.load(classLoader, permutationName);
try {
fileToLoad = config.getString(includeKey);
}
catch (Exception e) {
// TODO:
fileToLoad = null;
}
configs.put(permutationName, config);
} catch (ConfigException e) {
LOGGER.debug("could not load config '{}'. '{}'", new Object[]{fileToLoad, e.getMessage()});
break;
try {
configs.put(permutationName, loader.load(classLoader, permutationName));
failIfNotLoaded = false;
}
catch (ConfigException e) {
if (failIfNotLoaded == true) {
throw new ConfigException("Failed to load configuration resource '" + resourceName + "'");
}
} while (fileToLoad != null);
}
break;
}
}

if (failIfNotLoaded == true) {
if (configs.isEmpty())
throw new ConfigException("Failed to load configuration resource '" + resourceName + "'");
failIfNotLoaded = false;
}
}

if (overrides != null && !overrides.isEmpty()) {
configs.put(resourceName, new MapConfig(overrides));
}

// none
if (configs.isEmpty()) {
return null;
}
// single
else if (configs.size() == 1) {
return configs.values().iterator().next();
}
// multiple
else {
CompositeConfig cConfig = new CompositeConfig();
ArrayList<String> names = new ArrayList<String>();
names.addAll(configs.keySet());
Collections.reverse(names);

for (String name : names) {
cConfig.addConfig(name, configs.get(name));
}
return cConfig;
}
return configs;
}

@Override
Expand All @@ -248,8 +200,7 @@ public Config load(URL url) {
try {
return loader.load(classLoader, url);
} catch (ConfigException e) {
// TODO Auto-generated catch block
e.printStackTrace();
LOG.debug("Unable to load file '{}'", new Object[]{url, e.getMessage()});
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,21 @@ public void onError(Throwable error, Config config) {
* @throws ConfigException
*/
public synchronized void addConfig(String name, Config child) throws ConfigException {
internalAddConfig(name, child);
}

private synchronized void internalAddConfig(String name, Config child) throws ConfigException {
LOG.trace("Adding config {} to {}", name, hashCode());

if (name == null) {
throw new ConfigException("Child configuration must be named");
}

if (child == null) {
// TODO: Log a warning?
return;
}

if (name == null) {
throw new ConfigException("Child configuration must be named");
}

if (lookup.containsKey(name)) {
throw new ConfigException(String.format("Configuration with name '%s' already exists", name));
}
Expand All @@ -148,6 +152,18 @@ public synchronized void addConfig(String name, Config child) throws ConfigExcep
postConfigAdded(child);
}

public void addConfigs(LinkedHashMap<String, Config> configs) throws ConfigException {
for (Entry<String, Config> entry : configs.entrySet()) {
addConfig(entry.getKey(), entry.getValue());
}
}

public void replaceConfigs(LinkedHashMap<String, Config> configs) throws ConfigException {
for (Entry<String, Config> entry : configs.entrySet()) {
replaceConfig(entry.getKey(), entry.getValue());
}
}

public synchronized Collection<String> getConfigNames() {
List<String> result = new ArrayList<String>();
result.addAll(this.lookup.keySet());
Expand All @@ -171,14 +187,8 @@ protected void postConfigAdded(Config child) {
* @throws ConfigException
*/
public synchronized void replaceConfig(String name, Config child) throws ConfigException {
if (lookup.containsKey(name)) {
lookup.put(name, child);
child.removeListener(listener);
postConfigAdded(child);
}
else {
addConfig(name, child);
}
internalRemoveConfig(name);
internalAddConfig(name, child);
}

/**
Expand All @@ -190,6 +200,10 @@ public synchronized void replaceConfig(String name, Config child) throws ConfigE
* @return
*/
public synchronized Config removeConfig(String name) {
return internalRemoveConfig(name);
}

public synchronized Config internalRemoveConfig(String name) {
Config child = this.lookup.remove(name);
if (child != null) {
this.children.remove(child);
Expand Down Expand Up @@ -289,4 +303,12 @@ public synchronized void accept(Visitor visitor) {
}
}
}

public static CompositeConfig from(LinkedHashMap<String, Config> load) throws ConfigException {
Builder builder = builder();
for (Entry<String, Config> config : load.entrySet()) {
builder().withConfig(config.getKey(), config.getValue());
}
return builder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,23 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.netflix.archaius.loaders;
package com.netflix.archaius.readers;

import java.io.File;
import java.io.IOException;
import java.net.URL;
import java.net.URLDecoder;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Properties;

import com.netflix.archaius.Config;
import com.netflix.archaius.ConfigReader;
import com.netflix.archaius.config.MapConfig;
import com.netflix.archaius.exceptions.ConfigException;
import com.netflix.archaius.readers.URLConfigReader;

public class PropertiesConfigReader implements ConfigReader {
private String includeKey = "@next";

@Override
public Config load(ClassLoader loader, String resourceName) throws ConfigException {
Expand All @@ -39,10 +42,26 @@ public Config load(ClassLoader loader, String resourceName) throws ConfigExcepti

@Override
public Config load(ClassLoader loader, URL url) throws ConfigException {
Properties props = new Properties();

URL urlToLoad = url;
try {
return new MapConfig(new URLConfigReader(url).call().getToAdd());
} catch (IOException e) {
throw new ConfigException("Unable to load URL " + url.toString(), e);
do {
Map<String, String> p = new URLConfigReader(urlToLoad).call().getToAdd();
for (Entry<String, String> entry : p.entrySet()) {
props.put(entry.getKey(), entry.getValue());
}

String next = p.get(includeKey);
urlToLoad = next != null
? new URL(next)
: null;
} while (urlToLoad != null);

return new MapConfig(props);
}
catch (IOException e) {
throw new ConfigException("Unable to load configuration " + url, e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ public void basicTest() throws ConfigException {
.withDefaultCascadingStrategy(ConcatCascadeStrategy.from("${env}"))
.build();

application.addConfig("app", loader.newLoader().load("application"));
application.replaceConfigs(loader.newLoader().load("application"));

Assert.assertTrue(config.getBoolean("application.loaded"));
Assert.assertTrue(config.getBoolean("application-prod.loaded", false));

Assert.assertFalse(config.getBoolean("libA.loaded", false));

libraries.addConfig("libA", loader.newLoader().load("libA"));
libraries.replaceConfigs(loader.newLoader().load("libA"));
libraries.accept(new PrintStreamVisitor());

config.accept(new PrintStreamVisitor());
Expand All @@ -61,7 +61,7 @@ public void basicTest() throws ConfigException {
Assert.assertFalse(config.getBoolean("libB.loaded", false));
Assert.assertEquals("libA", config.getString("libA.overrideA"));

libraries.addConfig("libB", loader.newLoader().load("libB"));
libraries.replaceConfigs(loader.newLoader().load("libB"));

System.out.println(config.toString());
Assert.assertTrue(config.getBoolean("libA.loaded"));
Expand Down Expand Up @@ -90,14 +90,14 @@ public void basicReversedTest() throws ConfigException {
.withDefaultCascadingStrategy(ConcatCascadeStrategy.from("${env}"))
.build();

application.addConfig("app", loader.newLoader().load("application"));
application.replaceConfigs(loader.newLoader().load("application"));

Assert.assertTrue(config.getBoolean("application.loaded"));
Assert.assertTrue(config.getBoolean("application-prod.loaded", false));

Assert.assertFalse(config.getBoolean("libA.loaded", false));

libraries.addConfig("libA", loader.newLoader().load("libA"));
libraries.replaceConfigs(loader.newLoader().load("libA"));
libraries.accept(new PrintStreamVisitor());

config.accept(new PrintStreamVisitor());
Expand All @@ -106,7 +106,7 @@ public void basicReversedTest() throws ConfigException {
Assert.assertFalse(config.getBoolean("libB.loaded", false));
Assert.assertEquals("libA", config.getString("libA.overrideA"));

libraries.addConfig("libB", loader.newLoader().load("libB"));
libraries.replaceConfigs(loader.newLoader().load("libB"));

System.out.println(config.toString());
Assert.assertTrue(config.getBoolean("libA.loaded"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.netflix.archaius.Config;
import com.netflix.archaius.exceptions.ConfigException;
import com.netflix.archaius.readers.PropertiesConfigReader;

public class PropertyConfigReaderTest {
@Test
Expand Down
Loading

0 comments on commit 5ec75aa

Please sign in to comment.