github-repos/config
1
0
Fork 0
mirror of https://github.com/lightbend/config.git synced 2025-03-26 12:06:02 +08:00
Code Issues Packages Projects Releases Wiki Activity

Reimplement cycle detection with a traversed-node-set

Browse Source 
This is probably not different from the depth counter
in practical situations, but it does guarantee 100%
that we are only detecting true cycles. Since we want
true cycles to have different semantics (in a following
patch), I feel better about this implementation.
This commit is contained in:
Havoc Pennington 2012-03-28 19:50:27 -04:00
parent 16871b597e
commit db85e5ec8a
9 changed files with 124 additions and 84 deletions

26
config/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java
View File

@ -6,8 +6,10 @@ package com.typesafe.config.impl;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import com.typesafe.config.ConfigException;
import com.typesafe.config.ConfigMergeable;
@ -89,9 +91,10 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
*
* @throws NotPossibleToResolve
*/
protected AbstractConfigValue peekPath(Path path, SubstitutionResolver resolver, int depth,
ConfigResolveOptions options) throws NotPossibleToResolve, NeedsFullResolve {
return peekPath(this, path, resolver, depth, options);
protected AbstractConfigValue peekPath(Path path, SubstitutionResolver resolver,
Set<ConfigSubstitution> traversed, ConfigResolveOptions options)
throws NotPossibleToResolve, NeedsFullResolve {
return peekPath(this, path, resolver, traversed, options);
}
/**
@ -100,7 +103,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
*/
AbstractConfigValue peekPathWithExternalExceptions(Path path) {
try {
return peekPath(this, path, null, 0, null);
return peekPath(this, path, null, Collections.<ConfigSubstitution> emptySet(), null);
} catch (NotPossibleToResolve e) {
throw e.exportException(origin(), path.render());
} catch (NeedsFullResolve e) {
@ -113,14 +116,17 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
// child being peeked, but NOT the child itself. Caller has to resolve
// the child itself if needed.
private static AbstractConfigValue peekPath(AbstractConfigObject self, Path path,
SubstitutionResolver resolver, int depth, ConfigResolveOptions options)
SubstitutionResolver resolver, Set<ConfigSubstitution> traversed,
ConfigResolveOptions options)
throws NotPossibleToResolve, NeedsFullResolve {
if (resolver != null) {
// walk down through the path resolving only things along that path,
// and then recursively call ourselves with no resolver.
AbstractConfigValue partiallyResolved = resolver.resolve(self, depth, options, path);
AbstractConfigValue partiallyResolved = resolver
.resolve(self, traversed, options, path);
if (partiallyResolved instanceof AbstractConfigObject) {
return peekPath((AbstractConfigObject) partiallyResolved, path, null, 0, null);
return peekPath((AbstractConfigObject) partiallyResolved, path, null,
traversed, null);
} else {
throw new ConfigException.BugOrBroken("resolved object to non-object " + self
+ " to " + partiallyResolved);
@ -135,7 +141,8 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
return v;
} else {
if (v instanceof AbstractConfigObject) {
return peekPath((AbstractConfigObject) v, next, null, 0, null);
return peekPath((AbstractConfigObject) v, next, null,
Collections.<ConfigSubstitution> emptySet(), null);
} else {
return null;
}
@ -218,7 +225,8 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
@Override
abstract AbstractConfigObject resolveSubstitutions(final SubstitutionResolver resolver,
int depth, ConfigResolveOptions options, Path restrictToChildOrNull)
Set<ConfigSubstitution> traversed, ConfigResolveOptions options,
Path restrictToChildOrNull)
throws NotPossibleToResolve, NeedsFullResolve;
@Override

21
config/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java
View File

@ -4,6 +4,7 @@
package com.typesafe.config.impl;
import java.io.Serializable;
import java.util.Set;
import com.typesafe.config.ConfigException;
import com.typesafe.config.ConfigMergeable;
@ -40,7 +41,7 @@ abstract class AbstractConfigValue implements ConfigValue, MergeableValue, Seria
* resolved. This is a checked exception since it's internal to the library
* and we want to be sure we handle it before passing it out to public API.
*/
static final class NotPossibleToResolve extends Exception {
static class NotPossibleToResolve extends Exception {
private static final long serialVersionUID = 1L;
ConfigOrigin origin;
@ -76,6 +77,14 @@ abstract class AbstractConfigValue implements ConfigValue, MergeableValue, Seria
}
}
static final class SelfReferential extends NotPossibleToResolve {
private static final long serialVersionUID = 1L;
SelfReferential(ConfigOrigin origin, String path) {
super(origin, path, "Substitution ${" + path + "} is part of a cycle of substitutions");
}
}
// thrown if a full rather than partial resolve is needed
static final class NeedsFullResolve extends Exception {
private static final long serialVersionUID = 1L;
@ -93,17 +102,17 @@ abstract class AbstractConfigValue implements ConfigValue, MergeableValue, Seria
*
* @param resolver
* the resolver doing the resolving
* @param depth
* the number of substitutions followed in resolving the current
* one
* @param traversed
* objects which have already been visited, will include this one
* @param options
* whether to look at system props and env vars
* @param restrictToChildOrNull
* if non-null, only recurse into this child path
* @return a new value if there were changes, or this if no changes
*/
AbstractConfigValue resolveSubstitutions(SubstitutionResolver resolver, int depth,
ConfigResolveOptions options, Path restrictToChildOrNull) throws NotPossibleToResolve,
AbstractConfigValue resolveSubstitutions(SubstitutionResolver resolver,
Set<ConfigSubstitution> traversed, ConfigResolveOptions options,
Path restrictToChildOrNull) throws NotPossibleToResolve,
NeedsFullResolve {
return this;
}

13
config/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java
View File

@ -8,6 +8,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import com.typesafe.config.ConfigException;
import com.typesafe.config.ConfigOrigin;
@ -64,15 +65,17 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements
}
@Override
AbstractConfigValue resolveSubstitutions(SubstitutionResolver resolver, int depth,
ConfigResolveOptions options, Path restrictToChildOrNull) throws NotPossibleToResolve,
AbstractConfigValue resolveSubstitutions(SubstitutionResolver resolver,
Set<ConfigSubstitution> traversed, ConfigResolveOptions options,
Path restrictToChildOrNull) throws NotPossibleToResolve,
NeedsFullResolve {
return resolveSubstitutions(stack, resolver, depth, options, restrictToChildOrNull);
return resolveSubstitutions(stack, resolver, traversed, options, restrictToChildOrNull);
}
// static method also used by ConfigDelayedMergeObject
static AbstractConfigValue resolveSubstitutions(List<AbstractConfigValue> stack,
SubstitutionResolver resolver, int depth, ConfigResolveOptions options,
SubstitutionResolver resolver, Set<ConfigSubstitution> traversed,
ConfigResolveOptions options,
Path restrictToChildOrNull) throws NotPossibleToResolve, NeedsFullResolve {
// to resolve substitutions, we need to recursively resolve
// the stack of stuff to merge, and merge the stack so
@ -81,7 +84,7 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements
AbstractConfigValue merged = null;
for (AbstractConfigValue v : stack) {
AbstractConfigValue resolved = resolver.resolve(v, depth, options,
AbstractConfigValue resolved = resolver.resolve(v, traversed, options,
restrictToChildOrNull);
if (resolved != null) {
if (merged == null)

7
config/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java
View File

@ -61,11 +61,12 @@ final class ConfigDelayedMergeObject extends AbstractConfigObject implements
}
@Override
AbstractConfigObject resolveSubstitutions(SubstitutionResolver resolver, int depth,
ConfigResolveOptions options, Path restrictToChildOrNull) throws NotPossibleToResolve,
AbstractConfigObject resolveSubstitutions(SubstitutionResolver resolver,
Set<ConfigSubstitution> traversed, ConfigResolveOptions options,
Path restrictToChildOrNull) throws NotPossibleToResolve,
NeedsFullResolve {
AbstractConfigValue merged = ConfigDelayedMerge.resolveSubstitutions(stack, resolver,
depth, options, restrictToChildOrNull);
traversed, options, restrictToChildOrNull);
if (merged instanceof AbstractConfigObject) {
return (AbstractConfigObject) merged;
} else {

86
config/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java
View File

@ -8,6 +8,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import com.typesafe.config.ConfigException;
import com.typesafe.config.ConfigOrigin;
@ -124,55 +125,61 @@ final class ConfigSubstitution extends AbstractConfigValue implements
return pieces;
}
// larger than anyone would ever want
private static final int MAX_DEPTH = 100;
private AbstractConfigValue findInObject(AbstractConfigObject root,
private static AbstractConfigValue findInObject(AbstractConfigObject root,
SubstitutionResolver resolver, /* null if we should not have refs */
Path subst, int depth, ConfigResolveOptions options) throws NotPossibleToResolve,
NeedsFullResolve {
if (depth > MAX_DEPTH) {
throw new NotPossibleToResolve(origin(), subst.render(), "Substitution ${"
+ subst.render() + "} is part of a cycle of substitutions");
}
Path subst, Set<ConfigSubstitution> traversed, ConfigResolveOptions options)
throws NotPossibleToResolve, NeedsFullResolve {
AbstractConfigValue result = root.peekPath(subst, resolver, depth, options);
AbstractConfigValue result = root.peekPath(subst, resolver, traversed, options);
return result;
}
private AbstractConfigValue resolve(SubstitutionResolver resolver,
SubstitutionExpression subst, int depth, ConfigResolveOptions options,
Path restrictToChildOrNull) throws NotPossibleToResolve, NeedsFullResolve {
// First we look up the full path, which means relative to the
// included file if we were not a root file
AbstractConfigValue result = findInObject(resolver.root(), resolver, subst.path(), depth,
options);
SubstitutionExpression subst, Set<ConfigSubstitution> traversed,
ConfigResolveOptions options, Path restrictToChildOrNull) throws NotPossibleToResolve,
NeedsFullResolve {
if (traversed.contains(this))
throw new SelfReferential(origin(), subst.path().render());
if (result == null) {
// Then we want to check relative to the root file. We don't
// want the prefix we were included at to be used when looking up
// env variables either.
Path unprefixed = subst.path().subPath(prefixLength);
traversed.add(this);
if (result == null && prefixLength > 0) {
result = findInObject(resolver.root(), resolver, unprefixed, depth, options);
try {
// First we look up the full path, which means relative to the
// included file if we were not a root file
AbstractConfigValue result = findInObject(resolver.root(), resolver, subst.path(),
traversed, options);
if (result == null) {
// Then we want to check relative to the root file. We don't
// want the prefix we were included at to be used when looking
// up
// env variables either.
Path unprefixed = subst.path().subPath(prefixLength);
if (result == null && prefixLength > 0) {
result = findInObject(resolver.root(), resolver, unprefixed, traversed, options);
}
if (result == null && options.getUseSystemEnvironment()) {
result = findInObject(ConfigImpl.envVariablesAsConfigObject(), null,
unprefixed, traversed, options);
}
}
if (result == null && options.getUseSystemEnvironment()) {
result = findInObject(ConfigImpl.envVariablesAsConfigObject(), null, unprefixed,
depth, options);
if (result != null) {
result = resolver.resolve(result, traversed, options, restrictToChildOrNull);
}
}
if (result != null) {
result = resolver.resolve(result, depth, options, restrictToChildOrNull);
}
return result;
return result;
} finally {
traversed.remove(this);
}
}
private ConfigValue resolve(SubstitutionResolver resolver, int depth,
private ConfigValue resolve(SubstitutionResolver resolver, Set<ConfigSubstitution> traversed,
ConfigResolveOptions options, Path restrictToChildOrNull) throws NotPossibleToResolve {
if (pieces.size() > 1) {
// need to concat everything into a string
@ -186,7 +193,7 @@ final class ConfigSubstitution extends AbstractConfigValue implements
try {
// to concat into a string we have to do a full resolve,
// so don't pass along restrictToChildOrNull
v = resolve(resolver, exp, depth, options, null);
v = resolve(resolver, exp, traversed, options, null);
} catch (NeedsFullResolve e) {
throw new NotPossibleToResolve(null, exp.path().render(),
"Some kind of loop or interdependency prevents resolving " + exp, e);
@ -220,7 +227,7 @@ final class ConfigSubstitution extends AbstractConfigValue implements
SubstitutionExpression exp = (SubstitutionExpression) pieces.get(0);
ConfigValue v;
try {
v = resolve(resolver, exp, depth, options, restrictToChildOrNull);
v = resolve(resolver, exp, traversed, options, restrictToChildOrNull);
} catch (NeedsFullResolve e) {
throw new NotPossibleToResolve(null, exp.path().render(),
"Some kind of loop or interdependency prevents resolving " + exp, e);
@ -233,11 +240,10 @@ final class ConfigSubstitution extends AbstractConfigValue implements
}
@Override
AbstractConfigValue resolveSubstitutions(SubstitutionResolver resolver, int depth,
ConfigResolveOptions options, Path restrictToChildOrNull) throws NotPossibleToResolve {
// only ConfigSubstitution adds to depth here, because the depth
// is the substitution depth not the recursion depth.
AbstractConfigValue resolved = (AbstractConfigValue) resolve(resolver, depth + 1, options,
AbstractConfigValue resolveSubstitutions(SubstitutionResolver resolver,
Set<ConfigSubstitution> traversed, ConfigResolveOptions options,
Path restrictToChildOrNull) throws NotPossibleToResolve {
AbstractConfigValue resolved = (AbstractConfigValue) resolve(resolver, traversed, options,
restrictToChildOrNull);
return resolved;
}

3
config/src/main/java/com/typesafe/config/impl/SimpleConfig.java
View File

@ -6,6 +6,7 @@ package com.typesafe.config.impl;
import java.io.Serializable;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@ -73,7 +74,7 @@ final class SimpleConfig implements Config, MergeableValue, Serializable {
Path path = Path.newPath(pathExpression);
ConfigValue peeked;
try {
peeked = object.peekPath(path, null, 0, null);
peeked = object.peekPath(path, null, Collections.<ConfigSubstitution> emptySet(), null);
} catch (NotPossibleToResolve e) {
throw e.exportException(origin(), pathExpression);
} catch (NeedsFullResolve e) {

8
config/src/main/java/com/typesafe/config/impl/SimpleConfigList.java
View File

@ -9,6 +9,7 @@ import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
import java.util.Set;
import com.typesafe.config.ConfigException;
import com.typesafe.config.ConfigList;
@ -104,8 +105,9 @@ final class SimpleConfigList extends AbstractConfigValue implements ConfigList {
}
@Override
SimpleConfigList resolveSubstitutions(final SubstitutionResolver resolver, final int depth,
final ConfigResolveOptions options, Path restrictToChildOrNull)
SimpleConfigList resolveSubstitutions(final SubstitutionResolver resolver,
final Set<ConfigSubstitution> traversed, final ConfigResolveOptions options,
Path restrictToChildOrNull)
throws NotPossibleToResolve, NeedsFullResolve {
if (resolved)
return this;
@ -120,7 +122,7 @@ final class SimpleConfigList extends AbstractConfigValue implements ConfigList {
@Override
public AbstractConfigValue modifyChildMayThrow(String key, AbstractConfigValue v)
throws NotPossibleToResolve, NeedsFullResolve {
return resolver.resolve(v, depth, options, null /* restrictToChild */);
return resolver.resolve(v, traversed, options, null /* restrictToChild */);
}
}, ResolveStatus.RESOLVED);

9
config/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java
View File

@ -260,8 +260,9 @@ final class SimpleConfigObject extends AbstractConfigObject {
}
@Override
AbstractConfigObject resolveSubstitutions(final SubstitutionResolver resolver, final int depth,
final ConfigResolveOptions options, final Path restrictToChildOrNull)
AbstractConfigObject resolveSubstitutions(final SubstitutionResolver resolver,
final Set<ConfigSubstitution> traversed, final ConfigResolveOptions options,
final Path restrictToChildOrNull)
throws NotPossibleToResolve, NeedsFullResolve {
if (resolveStatus() == ResolveStatus.RESOLVED)
return this;
@ -276,7 +277,7 @@ final class SimpleConfigObject extends AbstractConfigObject {
if (key.equals(restrictToChildOrNull.first())) {
Path remainder = restrictToChildOrNull.remainder();
if (remainder != null) {
return resolver.resolve(v, depth, options, remainder);
return resolver.resolve(v, traversed, options, remainder);
} else {
// we don't want to resolve the leaf child.
return v;
@ -287,7 +288,7 @@ final class SimpleConfigObject extends AbstractConfigObject {
}
} else {
// no restrictToChild, resolve everything
return resolver.resolve(v, depth, options, null);
return resolver.resolve(v, traversed, options, null);
}
}

35
config/src/main/java/com/typesafe/config/impl/SubstitutionResolver.java
View File

@ -3,8 +3,11 @@
*/
package com.typesafe.config.impl;
import java.util.Collections;
import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Set;
import com.typesafe.config.ConfigException;
import com.typesafe.config.ConfigResolveOptions;
@ -64,12 +67,12 @@ final class SubstitutionResolver {
this.memos = new HashMap<MemoKey, AbstractConfigValue>();
}
AbstractConfigValue resolve(AbstractConfigValue original, int depth,
AbstractConfigValue resolve(AbstractConfigValue original, Set<ConfigSubstitution> traversed,
ConfigResolveOptions options, Path restrictToChildOrNull) throws NotPossibleToResolve,
NeedsFullResolve {
// a fully-resolved (no restrictToChild) object can satisfy a request
// for a restricted object, so always check that first.
// a fully-resolved (no restrictToChild) object can satisfy a
// request for a restricted object, so always check that first.
final MemoKey fullKey = new MemoKey(original, null);
MemoKey restrictedKey = null;
@ -86,18 +89,19 @@ final class SubstitutionResolver {
if (cached != null) {
return cached;
} else {
AbstractConfigValue resolved = original.resolveSubstitutions(this, depth, options,
restrictToChildOrNull);
AbstractConfigValue resolved = original.resolveSubstitutions(this, traversed, options,
restrictToChildOrNull);
if (resolved == null || resolved.resolveStatus() == ResolveStatus.RESOLVED) {
// if the resolved object is fully resolved by resolving only
// the restrictToChildOrNull, then it can be cached under
// fullKey since the child we were restricted to turned out to
// be the only unresolved thing.
// if the resolved object is fully resolved by resolving
// only the restrictToChildOrNull, then it can be cached
// under fullKey since the child we were restricted to
// turned out to be the only unresolved thing.
memos.put(fullKey, resolved);
} else {
// if we have an unresolved object then either we did a partial
// resolve restricted to a certain child, or it's a bug.
// if we have an unresolved object then either we did a
// partial resolve restricted to a certain child, or it's
// a bug.
if (restrictToChildOrNull == null) {
throw new ConfigException.BugOrBroken(
"resolveSubstitutions() did not give us a resolved object");
@ -118,18 +122,23 @@ final class SubstitutionResolver {
return this.root;
}
private static Set<ConfigSubstitution> newTraversedSet() {
return Collections.newSetFromMap(new IdentityHashMap<ConfigSubstitution, Boolean>());
}
static AbstractConfigValue resolve(AbstractConfigValue value, AbstractConfigObject root,
ConfigResolveOptions options, Path restrictToChildOrNull) throws NotPossibleToResolve,
NeedsFullResolve {
SubstitutionResolver resolver = new SubstitutionResolver(root);
return resolver.resolve(value, 0, options, restrictToChildOrNull);
return resolver.resolve(value, newTraversedSet(), options, restrictToChildOrNull);
}
static AbstractConfigValue resolveWithExternalExceptions(AbstractConfigValue value,
AbstractConfigObject root, ConfigResolveOptions options) {
SubstitutionResolver resolver = new SubstitutionResolver(root);
try {
return resolver.resolve(value, 0, options, null /* restrictToChild */);
return resolver.resolve(value, newTraversedSet(), options, null /* restrictToChild */);
} catch (NotPossibleToResolve e) {
throw e.exportException(value.origin(), null);
} catch (NeedsFullResolve e) {