drop the ConfigRoot interface.

This was intended to make resolve() typesafe (only allowed on root objects).
However, in practice it was not safe because it was too easy to create a
root by merging a non-root object into a root object. To fix it, we'd
have to prohibit that (root objects can only merge with other roots).

But it seems simpler to just document that resolving on subtrees
will do the wrong thing, and hopefully it won't be an issue in practice.
This commit is contained in:
Havoc Pennington 2011-11-25 10:34:51 -05:00
parent afff7c61ef
commit 7420a3da33
9 changed files with 75 additions and 178 deletions

View File

@ -90,6 +90,58 @@ public interface Config extends ConfigMergeable {
@Override
ConfigObject toValue();
/**
* Returns a replacement config with all substitutions (the
* <code>${foo.bar}</code> syntax, see <a
* href="https://github.com/havocp/config/blob/master/HOCON.md">the
* spec</a>) resolved. Substitutions are looked up using this
* <code>Config</code> as the root object, that is, a substitution
* <code>${foo.bar}</code> will be replaced with the result of
* <code>getValue("foo.bar")</code>.
*
* <p>
* This method uses {@link ConfigResolveOptions#defaults()}, there is
* another variant {@link Config#resolve(ConfigResolveOptions)} which lets
* you specify non-default options.
*
* <p>
* A given {@link Config} must be resolved before using it to retrieve
* config values, but ideally should be resolved one time for your entire
* stack of fallbacks (see {@link Config#withFallback}). Otherwise, some
* substitutions that could have resolved with all fallbacks available may
* not resolve, which will be a user-visible oddity.
*
* <p>
* <code>resolve()</code> should be invoked on root config objects, rather
* than on a subtree (a subtree is the result of something like
* <code>config.getConfig("foo")</code>). The problem with
* <code>resolve()</code> on a subtree is that substitutions are relative to
* the root of the config and the subtree will have no way to get values
* from the root. For example, if you did
* <code>config.getConfig("foo").resolve()</code> on the below config file,
* it would not work:
*
* <pre>
* common-value = 10
* foo {
* whatever = ${common-value}
* }
* </pre>
*
* @return an immutable object with substitutions resolved
*/
Config resolve();
/**
* Like {@link Config#resolve()} but allows you to specify non-default
* options.
*
* @param options
* resolve options
* @return the resolved <code>Config</code>
*/
Config resolve(ConfigResolveOptions options);
/**
* Checks whether a value is present and non-null at the given path. This
* differs in two ways from {@code Map.containsKey()} as implemented by
@ -213,7 +265,7 @@ public interface Config extends ConfigMergeable {
* Gets the value at the path as an unwrapped Java boxed value (
* {@link java.lang.Boolean Boolean}, {@link java.lang.Integer Integer}, and
* so on - see {@link ConfigValue#unwrapped()}).
*
*
* @param path
* path expression
* @return the unwrapped value at the requested path

View File

@ -53,31 +53,31 @@ public final class ConfigFactory {
* the configuration "domain"
* @return configuration for the requested root path
*/
public static ConfigRoot load(String rootPath) {
public static Config load(String rootPath) {
return loadWithoutResolving(rootPath).resolve();
}
public static ConfigRoot load(String rootPath,
public static Config load(String rootPath,
ConfigParseOptions parseOptions, ConfigResolveOptions resolveOptions) {
return loadWithoutResolving(rootPath, parseOptions).resolve(
resolveOptions);
}
/**
* Like load() but does not resolve the object, so you can go ahead and add
* Like load() but does not resolve the config, so you can go ahead and add
* more fallbacks and stuff and have them seen by substitutions when you do
* call {@link ConfigRoot#resolve}.
* call {@link Config#resolve()}.
*
* @param rootPath
* @return configuration for the requested root path
*/
public static ConfigRoot loadWithoutResolving(String rootPath) {
public static Config loadWithoutResolving(String rootPath) {
return loadWithoutResolving(rootPath, ConfigParseOptions.defaults());
}
public static ConfigRoot loadWithoutResolving(String rootPath,
public static Config loadWithoutResolving(String rootPath,
ConfigParseOptions options) {
ConfigRoot system = systemPropertiesRoot(rootPath);
Config system = ConfigImpl.systemPropertiesWithPrefix(rootPath);
Config mainFiles = parseResourcesForPath(rootPath, options);
Config referenceFiles = parseResourcesForPath(rootPath + ".reference",
@ -86,26 +86,14 @@ public final class ConfigFactory {
return system.withFallback(mainFiles).withFallback(referenceFiles);
}
public static ConfigRoot emptyRoot(String rootPath) {
return emptyRoot(rootPath, null);
}
public static Config empty() {
return empty(null);
}
public static ConfigRoot emptyRoot(String rootPath, String originDescription) {
return ConfigImpl.emptyRoot(rootPath, originDescription);
}
public static Config empty(String originDescription) {
return ConfigImpl.emptyConfig(originDescription);
}
public static ConfigRoot systemPropertiesRoot(String rootPath) {
return ConfigImpl.systemPropertiesRoot(rootPath);
}
public static Config systemProperties() {
return ConfigImpl.systemPropertiesAsConfig();
}
@ -177,7 +165,7 @@ public final class ConfigFactory {
/**
* Same behavior as {@link #parseFileAnySyntax(File,ConfigParseOptions)} but
* for classpath resources instead.
*
*
* @param klass
* @param resourceBasename
* @param options

View File

@ -1,48 +0,0 @@
/**
* Copyright (C) 2011 Typesafe Inc. <http://typesafe.com>
*/
package com.typesafe.config;
/**
* Subtype of {@link Config} which is a root rather than nested object and
* supports {@link ConfigRoot#resolve resolving substitutions}.
*
* <p>
* <em>Do not implement this interface</em>; it should only be implemented by
* the config library. Arbitrary implementations will not work because the
* library internals assume a specific concrete implementation. Also, this
* interface is likely to grow new methods over time, so third-party
* implementations will break.
*/
public interface ConfigRoot extends Config {
/**
* Returns a replacement root object with all substitutions (the
* <code>${foo.bar}</code> syntax, see <a
* href="https://github.com/havocp/config/blob/master/HOCON.md">the
* spec</a>) resolved. Substitutions are looked up in this root object. A
* {@link Config} must be resolved before you can use it. This method uses
* {@link ConfigResolveOptions#defaults()}.
*
* @return an immutable object with substitutions resolved
*/
ConfigRoot resolve();
/**
* Like {@link ConfigRoot#resolve()} but allows you to specify options.
*
* @param options
* resolve options
* @return the resolved root config
*/
ConfigRoot resolve(ConfigResolveOptions options);
@Override
ConfigRoot withFallback(ConfigMergeable fallback);
/**
* Gets the global app name that this root represents.
*
* @return the app's root config path
*/
String rootPath();
}

View File

@ -19,7 +19,6 @@ import com.typesafe.config.ConfigObject;
import com.typesafe.config.ConfigOrigin;
import com.typesafe.config.ConfigParseOptions;
import com.typesafe.config.ConfigParseable;
import com.typesafe.config.ConfigRoot;
import com.typesafe.config.ConfigSyntax;
import com.typesafe.config.ConfigValue;
@ -139,13 +138,6 @@ public class ConfigImpl {
return fromBasename(source, basename.getPath(), baseOptions);
}
/** For use ONLY by library internals, DO NOT TOUCH not guaranteed ABI */
public static ConfigRoot emptyRoot(String rootPath, String originDescription) {
String desc = originDescription != null ? originDescription : rootPath;
return emptyObject(desc).toConfig().asRoot(
Path.newPath(rootPath));
}
static AbstractConfigObject emptyObject(String originDescription) {
ConfigOrigin origin = originDescription != null ? new SimpleConfigOrigin(
originDescription) : null;
@ -291,13 +283,12 @@ public class ConfigImpl {
}
/** For use ONLY by library internals, DO NOT TOUCH not guaranteed ABI */
public static ConfigRoot systemPropertiesRoot(String rootPath) {
Path path = Parser.parsePath(rootPath);
/* FIXME this is broken and will go away when Config.load() doesn't use it. */
public static Config systemPropertiesWithPrefix(String rootPath) {
try {
return systemPropertiesAsConfigObject().toConfig().getConfig(rootPath)
.asRoot(path);
return systemPropertiesAsConfigObject().toConfig().getConfig(rootPath);
} catch (ConfigException.Missing e) {
return emptyObject("system properties").toConfig().asRoot(path);
return emptyObject("system properties").toConfig();
}
}

View File

@ -1,61 +0,0 @@
/**
* Copyright (C) 2011 Typesafe Inc. <http://typesafe.com>
*/
package com.typesafe.config.impl;
import com.typesafe.config.ConfigMergeable;
import com.typesafe.config.ConfigResolveOptions;
import com.typesafe.config.ConfigRoot;
final class RootConfig extends SimpleConfig implements ConfigRoot {
final private Path rootPath;
RootConfig(AbstractConfigObject underlying, Path rootPath) {
super(underlying);
this.rootPath = rootPath;
}
@Override
protected RootConfig asRoot(AbstractConfigObject underlying,
Path newRootPath) {
if (newRootPath.equals(this.rootPath))
return this;
else
return new RootConfig(underlying, newRootPath);
}
@Override
public RootConfig resolve() {
return resolve(ConfigResolveOptions.defaults());
}
@Override
public RootConfig resolve(ConfigResolveOptions options) {
// if the object is already resolved then we should end up returning
// "this" here, since asRoot() should return this if the path
// is unchanged.
AbstractConfigObject resolved = resolvedObject(options);
return newRootIfObjectChanged(this, resolved);
}
@Override
public RootConfig withFallback(ConfigMergeable value) {
// this can return "this" if the withFallback does nothing
return newRootIfObjectChanged(this, super.withFallback(value).toObject());
}
Path rootPathObject() {
return rootPath;
}
@Override
public String rootPath() {
return rootPath.render();
}
@Override
public String toString() {
return "Root" + super.toString();
}
}

View File

@ -43,35 +43,22 @@ class SimpleConfig implements Config {
return object.origin();
}
/**
* Returns a version of this config that implements the ConfigRoot
* interface.
*
* @return a config root
*/
RootConfig asRoot(Path rootPath) {
return asRoot(object, rootPath);
@Override
public SimpleConfig resolve() {
return resolve(ConfigResolveOptions.defaults());
}
// RootConfig overrides this to avoid a new object on unchanged path.
protected RootConfig asRoot(AbstractConfigObject underlying,
Path newRootPath) {
return new RootConfig(underlying, newRootPath);
}
static protected RootConfig newRootIfObjectChanged(RootConfig self, AbstractConfigObject underlying) {
if (underlying == self.object)
return self;
else
return new RootConfig(underlying, self.rootPathObject());
}
protected AbstractConfigObject resolvedObject(ConfigResolveOptions options) {
@Override
public SimpleConfig resolve(ConfigResolveOptions options) {
AbstractConfigValue resolved = SubstitutionResolver.resolve(object,
object, options);
return (AbstractConfigObject) resolved;
if (resolved == object)
return this;
else
return new SimpleConfig((AbstractConfigObject) resolved);
}
@Override
public boolean hasPath(String pathExpression) {
Path path = Path.newPath(pathExpression);

View File

@ -499,10 +499,6 @@ class ConfigTest extends TestUtils {
def ignoredMergesDoNothing() {
val conf = parseConfig("{ a : 1 }")
testIgnoredMergesDoNothing(conf)
// ConfigRoot mode uses a little different codepath
val root = conf.asRoot(path("whatever"))
testIgnoredMergesDoNothing(root)
}
@Test

View File

@ -101,12 +101,6 @@ class ConfigValueTest extends TestUtils {
// configs are not equal to objects
checkNotEqualObjects(a, a.toConfig())
checkNotEqualObjects(b, b.toConfig())
// configs are equal to the same config as a root
val root = config.asRoot(path("foo"))
checkEqualObjects(config, root)
checkNotEqualObjects(b.toConfig(), root)
checkNotEqualObjects(c.toConfig(), root)
}
@Test

View File

@ -87,8 +87,6 @@ class PublicApiTest extends TestUtils {
assertEquals("empty config", ConfigFactory.empty().origin().description())
assertTrue(ConfigFactory.empty("foo").isEmpty())
assertEquals("foo", ConfigFactory.empty("foo").origin().description())
assertTrue(ConfigFactory.emptyRoot("foo.bar").isEmpty())
assertEquals("foo.bar", ConfigFactory.emptyRoot("foo.bar").origin().description())
}
private val defaultValueDesc = "hardcoded value";