make resolveSubstitutions() do nothing on already-resolved objects.

This way if people resolve() gratuitously, there's no performance
issue. Also, tracking resolved state gives us the option to complain
about usage of unresolved objects.
This commit is contained in:
Havoc Pennington 2011-11-11 23:51:41 -05:00
parent 10babb5e8c
commit df3b3792da
13 changed files with 134 additions and 29 deletions

View File

@ -129,13 +129,13 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
@Override
AbstractConfigObject transformed(ConfigTransformer newTransformer) {
if (newTransformer != transformer)
return newCopy(newTransformer);
return newCopy(newTransformer, resolveStatus());
else
return this;
}
protected abstract AbstractConfigObject newCopy(
ConfigTransformer newTransformer);
ConfigTransformer newTransformer, ResolveStatus status);
static private AbstractConfigValue resolve(AbstractConfigObject self,
String pathExpression,
@ -199,6 +199,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
if (fallback.isEmpty()) {
return this; // nothing to do
} else {
boolean allResolved = true;
Map<String, AbstractConfigValue> merged = new HashMap<String, AbstractConfigValue>();
Set<String> allKeys = new HashSet<String>();
allKeys.addAll(this.keySet());
@ -206,15 +207,19 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
for (String key : allKeys) {
AbstractConfigValue first = this.peek(key);
AbstractConfigValue second = fallback.peek(key);
AbstractConfigValue kept;
if (first == null)
merged.put(key, second);
kept = second;
else if (second == null)
merged.put(key, first);
kept = first;
else
merged.put(key, first.withFallback(second));
kept = first.withFallback(second);
merged.put(key, kept);
if (kept.resolveStatus() == ResolveStatus.UNRESOLVED)
allResolved = false;
}
return new SimpleConfigObject(mergeOrigins(this, fallback),
merged);
merged, ResolveStatus.fromBoolean(allResolved));
}
} else {
// falling back to a non-object has no effect, we just override
@ -269,17 +274,22 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
AbstractConfigObject resolveSubstitutions(SubstitutionResolver resolver,
int depth,
boolean withFallbacks) {
Map<String, AbstractConfigValue> changes = new HashMap<String, AbstractConfigValue>();
if (resolveStatus() == ResolveStatus.RESOLVED)
return this;
Map<String, AbstractConfigValue> changes = null;
for (String k : keySet()) {
AbstractConfigValue v = peek(k);
AbstractConfigValue resolved = resolver.resolve(v, depth,
withFallbacks);
if (resolved != v) {
if (changes == null)
changes = new HashMap<String, AbstractConfigValue>();
changes.put(k, resolved);
}
}
if (changes.isEmpty()) {
return this;
if (changes == null) {
return newCopy(transformer, ResolveStatus.RESOLVED);
} else {
Map<String, AbstractConfigValue> resolved = new HashMap<String, AbstractConfigValue>();
for (String k : keySet()) {
@ -289,7 +299,8 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
resolved.put(k, peek(k));
}
}
return new SimpleConfigObject(origin(), resolved);
return new SimpleConfigObject(origin(), transformer, resolved,
ResolveStatus.RESOLVED);
}
}

View File

@ -34,6 +34,10 @@ abstract class AbstractConfigValue implements ConfigValue {
return this;
}
ResolveStatus resolveStatus() {
return ResolveStatus.RESOLVED;
}
@Override
public AbstractConfigValue withFallback(ConfigValue other) {
return this;

View File

@ -85,6 +85,11 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements
return AbstractConfigObject.merge(toMerge);
}
@Override
ResolveStatus resolveStatus() {
return ResolveStatus.UNRESOLVED;
}
@Override
public AbstractConfigValue withFallback(ConfigValue other) {
if (other instanceof AbstractConfigObject

View File

@ -64,7 +64,11 @@ class ConfigDelayedMergeObject extends AbstractConfigObject implements
}
@Override
public ConfigDelayedMergeObject newCopy(ConfigTransformer newTransformer) {
public ConfigDelayedMergeObject newCopy(ConfigTransformer newTransformer,
ResolveStatus status) {
if (status != resolveStatus())
throw new ConfigException.BugOrBroken(
"attempt to create resolved ConfigDelayedMergeObject");
return new ConfigDelayedMergeObject(origin(), newTransformer, stack);
}
@ -82,6 +86,11 @@ class ConfigDelayedMergeObject extends AbstractConfigObject implements
}
}
@Override
ResolveStatus resolveStatus() {
return ResolveStatus.UNRESOLVED;
}
@Override
public ConfigDelayedMergeObject withFallback(ConfigValue other) {
if (other instanceof AbstractConfigObject

View File

@ -128,6 +128,6 @@ public class ConfigImpl {
new SimpleConfigOrigin("env var " + key), entry.getValue()));
}
return new SimpleConfigObject(new SimpleConfigOrigin("env variables"),
m);
m, ResolveStatus.RESOLVED);
}
}

View File

@ -168,6 +168,11 @@ final class ConfigSubstitution extends AbstractConfigValue implements
return resolved;
}
@Override
ResolveStatus resolveStatus() {
return ResolveStatus.UNRESOLVED;
}
@Override
protected boolean canEqual(Object other) {
return other instanceof ConfigSubstitution;

View File

@ -16,16 +16,18 @@ abstract class DelegatingConfigObject extends AbstractConfigObject {
}
@Override
protected DelegatingConfigObject newCopy(ConfigTransformer newTransformer) {
AbstractConfigObject transformed = underlying
.transformed(newTransformer);
if (transformed != underlying)
return newCopy(transformed);
else
return this;
protected DelegatingConfigObject newCopy(ConfigTransformer newTransformer,
ResolveStatus newStatus) {
return newCopy(underlying, newTransformer, newStatus);
}
abstract DelegatingConfigObject newCopy(AbstractConfigObject underlying);
abstract DelegatingConfigObject newCopy(AbstractConfigObject underlying,
ConfigTransformer newTransformer, ResolveStatus newStatus);
@Override
ResolveStatus resolveStatus() {
return underlying.resolveStatus();
}
@Override
public boolean containsKey(Object key) {

View File

@ -155,9 +155,12 @@ final class Loader {
// provide to SimpleConfigObject, which is not allowed by
// its contract, but since we know nobody has a ref to this
// SimpleConfigObject yet we can get away with it.
// Also we assume here that any info based on the map that
// SimpleConfigObject computes and caches in its constructor
// will not change. Basically this is a bad hack.
AbstractConfigObject o = new SimpleConfigObject(
new SimpleConfigOrigin(originPrefix + " " + path),
scopes.get(path));
scopes.get(path), ResolveStatus.RESOLVED);
String basename = lastElement(path);
parent.put(basename, o);
}
@ -171,6 +174,6 @@ final class Loader {
// return root config object
return new SimpleConfigObject(new SimpleConfigOrigin(originPrefix),
root);
root, ResolveStatus.RESOLVED);
}
}

View File

@ -0,0 +1,23 @@
package com.typesafe.config.impl;
import java.util.Collection;
/**
* Status of substitution resolution.
*/
enum ResolveStatus {
UNRESOLVED, RESOLVED;
final static ResolveStatus fromValues(
Collection<? extends AbstractConfigValue> values) {
for (AbstractConfigValue v : values) {
if (v.resolveStatus() == ResolveStatus.UNRESOLVED)
return ResolveStatus.UNRESOLVED;
}
return ResolveStatus.RESOLVED;
}
final static ResolveStatus fromBoolean(boolean resolved) {
return resolved ? ResolveStatus.RESOLVED : ResolveStatus.UNRESOLVED;
}
}

View File

@ -16,8 +16,10 @@ final class RootConfigObject extends DelegatingConfigObject implements
}
@Override
public RootConfigObject newCopy(AbstractConfigObject underlying) {
return new RootConfigObject(underlying);
public RootConfigObject newCopy(AbstractConfigObject underlying,
ConfigTransformer newTransformer, ResolveStatus newStatus) {
return new RootConfigObject(underlying.newCopy(newTransformer,
newStatus));
}
@Override

View File

@ -15,10 +15,17 @@ import com.typesafe.config.ConfigValueType;
final class SimpleConfigList extends AbstractConfigValue implements ConfigList {
final private List<AbstractConfigValue> value;
final private boolean resolved;
SimpleConfigList(ConfigOrigin origin, List<AbstractConfigValue> value) {
this(origin, value, ResolveStatus.fromValues(value));
}
SimpleConfigList(ConfigOrigin origin, List<AbstractConfigValue> value,
ResolveStatus status) {
super(origin);
this.value = value;
this.resolved = status == ResolveStatus.RESOLVED;
}
@Override
@ -35,9 +42,17 @@ final class SimpleConfigList extends AbstractConfigValue implements ConfigList {
return list;
}
@Override
ResolveStatus resolveStatus() {
return ResolveStatus.fromBoolean(resolved);
}
@Override
SimpleConfigList resolveSubstitutions(SubstitutionResolver resolver, int depth,
boolean withFallbacks) {
if (resolved)
return this;
// lazy-create for optimization
List<AbstractConfigValue> changed = null;
int i = 0;
@ -66,7 +81,8 @@ final class SimpleConfigList extends AbstractConfigValue implements ConfigList {
if (changed.size() != value.size())
throw new ConfigException.BugOrBroken(
"substituted list's size doesn't match");
return new SimpleConfigList(origin(), changed);
return new SimpleConfigList(origin(), changed,
ResolveStatus.RESOLVED);
} else {
return this;
}

View File

@ -19,19 +19,32 @@ final class SimpleConfigObject extends AbstractConfigObject {
// this map should never be modified - assume immutable
final private Map<String, AbstractConfigValue> value;
final private boolean resolved;
SimpleConfigObject(ConfigOrigin origin, ConfigTransformer transformer,
Map<String, AbstractConfigValue> value) {
Map<String, AbstractConfigValue> value, ResolveStatus status) {
super(origin, transformer);
if (value == null)
throw new ConfigException.BugOrBroken(
"creating config object with null map");
this.value = value;
this.resolved = status == ResolveStatus.RESOLVED;
}
SimpleConfigObject(ConfigOrigin origin, ConfigTransformer transformer,
Map<String, AbstractConfigValue> value) {
this(origin, transformer, value, ResolveStatus.fromValues(value
.values()));
}
SimpleConfigObject(ConfigOrigin origin,
Map<String, AbstractConfigValue> value, ResolveStatus status) {
this(origin, ConfigImpl.defaultConfigTransformer(), value, status);
}
SimpleConfigObject(ConfigOrigin origin,
Map<String, AbstractConfigValue> value) {
this(origin, ConfigImpl.defaultConfigTransformer(), value);
this(origin, value, ResolveStatus.fromValues(value.values()));
}
@Override
@ -40,8 +53,15 @@ final class SimpleConfigObject extends AbstractConfigObject {
}
@Override
public SimpleConfigObject newCopy(ConfigTransformer newTransformer) {
return new SimpleConfigObject(origin(), newTransformer, value);
public SimpleConfigObject newCopy(ConfigTransformer newTransformer,
ResolveStatus newStatus) {
return new SimpleConfigObject(origin(), newTransformer, value,
newStatus);
}
@Override
ResolveStatus resolveStatus() {
return ResolveStatus.fromBoolean(resolved);
}
@Override

View File

@ -3,6 +3,8 @@ package com.typesafe.config.impl;
import java.util.IdentityHashMap;
import java.util.Map;
import com.typesafe.config.ConfigException;
/**
* This exists because we have to memoize resolved substitutions as we go
* through the config tree; otherwise we could end up creating multiple copies
@ -26,6 +28,9 @@ final class SubstitutionResolver {
AbstractConfigValue resolved = original.resolveSubstitutions(this,
depth,
withFallbacks);
if (resolved.resolveStatus() != ResolveStatus.RESOLVED)
throw new ConfigException.BugOrBroken(
"resolveSubstitutions() did not give us a resolved object");
memos.put(original, resolved);
return resolved;
}