diff --git a/config/src/main/java/com/typesafe/config/ConfigException.java b/config/src/main/java/com/typesafe/config/ConfigException.java index 3c31d811..69072f1b 100644 --- a/config/src/main/java/com/typesafe/config/ConfigException.java +++ b/config/src/main/java/com/typesafe/config/ConfigException.java @@ -244,12 +244,12 @@ public abstract class ConfigException extends RuntimeException { public static class UnresolvedSubstitution extends Parse { private static final long serialVersionUID = 1L; - public UnresolvedSubstitution(ConfigOrigin origin, String expression, Throwable cause) { - super(origin, "Could not resolve substitution to a value: " + expression, cause); + public UnresolvedSubstitution(ConfigOrigin origin, String detail, Throwable cause) { + super(origin, "Could not resolve substitution to a value: " + detail, cause); } - public UnresolvedSubstitution(ConfigOrigin origin, String expression) { - this(origin, expression, null); + public UnresolvedSubstitution(ConfigOrigin origin, String detail) { + this(origin, detail, null); } } diff --git a/config/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java b/config/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java index 1f970502..0d8b07dd 100644 --- a/config/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java +++ b/config/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java @@ -83,22 +83,24 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements Confi * Looks up the path with no transformation, type conversion, or exceptions * (just returns null if path not found). Does however resolve the path, if * resolver != null. - * + * * @throws NotPossibleToResolve + * if context is not null and resolution fails */ protected AbstractConfigValue peekPath(Path path, ResolveContext context) throws NotPossibleToResolve { return peekPath(this, path, context); } /** - * Looks up the path and throws public API exceptions (ConfigException). - * Doesn't do any resolution, will throw if any is needed. + * Looks up the path. Doesn't do any resolution, will throw if any is + * needed. */ - AbstractConfigValue peekPathWithExternalExceptions(Path path) { + AbstractConfigValue peekPath(Path path) { try { return peekPath(this, path, null); } catch (NotPossibleToResolve e) { - throw e.exportException(origin(), path.render()); + throw new ConfigException.BugOrBroken( + "NotPossibleToResolve happened though we had no ResolveContext in peekPath"); } } diff --git a/config/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java b/config/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java index 2dcbd9f1..703ad100 100644 --- a/config/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java +++ b/config/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java @@ -33,53 +33,29 @@ abstract class AbstractConfigValue implements ConfigValue, MergeableValue, Seria } /** - * This exception means that a value is inherently not resolveable, for - * example because there's a cycle in the substitutions. That's different - * from a ConfigException.NotResolved which just means it hasn't been - * 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. + * This exception means that a value is inherently not resolveable, at the + * moment the only known cause is a cycle of substitutions. 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. This is only + * supposed to be thrown by the target of a cyclic reference and it's + * supposed to be caught by the ConfigReference looking up that reference, + * so it should be impossible for an outermost resolve() to throw this. + * + * Contrast with ConfigException.NotResolved which just means nobody called + * resolve(). */ static class NotPossibleToResolve extends Exception { private static final long serialVersionUID = 1L; - ConfigOrigin origin; - String path; + final private String traceString; - NotPossibleToResolve(String message) { - super(message); - this.origin = null; - this.path = null; + NotPossibleToResolve(ResolveContext context) { + super("was not possible to resolve"); + this.traceString = context.traceString(); } - // use this constructor ONLY if you know the right origin and path - // to describe the problem. - NotPossibleToResolve(ConfigOrigin origin, String path, String message) { - this(origin, path, message, null); - } - - NotPossibleToResolve(ConfigOrigin origin, String path, String message, Throwable cause) { - super(message, cause); - this.origin = origin; - this.path = path; - } - - ConfigException exportException(ConfigOrigin outerOrigin, String outerPath) { - ConfigOrigin o = origin != null ? origin : outerOrigin; - String p = path != null ? path : outerPath; - if (p == null) - path = ""; - if (o != null) - return new ConfigException.BadValue(o, p, getMessage(), this); - else - return new ConfigException.BadValue(p, getMessage(), this); - } - } - - 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"); + String traceString() { + return traceString; } } diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigConcatenation.java b/config/src/main/java/com/typesafe/config/impl/ConfigConcatenation.java index 62841b12..4ae3f24e 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigConcatenation.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigConcatenation.java @@ -102,20 +102,13 @@ final class ConfigConcatenation extends AbstractConfigValue implements Unmergeab return Collections.singleton(this); } - private static ResolveReplacer undefinedReplacer = new ResolveReplacer() { - @Override - protected AbstractConfigValue makeReplacement() throws Undefined { - throw new Undefined(); - } - }; - @Override AbstractConfigValue resolveSubstitutions(ResolveContext context) throws NotPossibleToResolve { List resolved = new ArrayList(pieces.size()); // if you have "foo = ${?foo}bar" then we will // self-referentially look up foo and we need to // get undefined, rather than "bar" - context.replace(this, undefinedReplacer); + context.source().replace(this, ResolveReplacer.cycleResolveReplacer); try { for (AbstractConfigValue p : pieces) { // to concat into a string we have to do a full resolve, @@ -138,7 +131,7 @@ final class ConfigConcatenation extends AbstractConfigValue implements Unmergeab } } } finally { - context.unreplace(this); + context.source().unreplace(this); } // now need to concat everything diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java b/config/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java index 40534113..41344821 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java @@ -12,7 +12,6 @@ import java.util.List; import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigOrigin; import com.typesafe.config.ConfigValueType; -import com.typesafe.config.impl.ResolveReplacer.Undefined; /** * The issue here is that we want to first merge our stack of config files, and @@ -91,7 +90,7 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements Unmergeabl // ConfigDelayedMerge with a value that is only // the remainder of the stack below this one. - context.replace((AbstractConfigValue) replaceable, + context.source().replace((AbstractConfigValue) replaceable, replaceable.makeReplacer(count + 1)); replaced = true; } @@ -101,7 +100,7 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements Unmergeabl resolved = context.resolve(v); } finally { if (replaced) - context.unreplace((AbstractConfigValue) replaceable); + context.source().unreplace((AbstractConfigValue) replaceable); } if (resolved != null) { @@ -120,20 +119,21 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements Unmergeabl public ResolveReplacer makeReplacer(final int skipping) { return new ResolveReplacer() { @Override - protected AbstractConfigValue makeReplacement() throws Undefined { - return ConfigDelayedMerge.makeReplacement(stack, skipping); + protected AbstractConfigValue makeReplacement(ResolveContext context) + throws NotPossibleToResolve { + return ConfigDelayedMerge.makeReplacement(context, stack, skipping); } }; } // static method also used by ConfigDelayedMergeObject - static AbstractConfigValue makeReplacement(List stack, int skipping) - throws Undefined { + static AbstractConfigValue makeReplacement(ResolveContext context, + List stack, int skipping) throws NotPossibleToResolve { List subStack = stack.subList(skipping, stack.size()); if (subStack.isEmpty()) { - throw new ResolveReplacer.Undefined(); + throw new NotPossibleToResolve(context); } else { // generate a new merge stack from only the remaining items AbstractConfigValue merged = null; diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java b/config/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java index bb75368b..580ed5ba 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java @@ -75,8 +75,9 @@ final class ConfigDelayedMergeObject extends AbstractConfigObject implements Unm public ResolveReplacer makeReplacer(final int skipping) { return new ResolveReplacer() { @Override - protected AbstractConfigValue makeReplacement() throws Undefined { - return ConfigDelayedMerge.makeReplacement(stack, skipping); + protected AbstractConfigValue makeReplacement(ResolveContext context) + throws NotPossibleToResolve { + return ConfigDelayedMerge.makeReplacement(context, stack, skipping); } }; } diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigReference.java b/config/src/main/java/com/typesafe/config/impl/ConfigReference.java index 82f976cd..b2718a20 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigReference.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigReference.java @@ -103,14 +103,33 @@ final class ConfigReference extends AbstractConfigValue implements Unmergeable { return Collections.singleton(this); } + // ConfigReference should be a firewall against NotPossibleToResolve going + // further up the stack; it should convert everything to ConfigException. + // This way it's impossible for NotPossibleToResolve to "escape" since + // any failure to resolve has to start with a ConfigReference. @Override - AbstractConfigValue resolveSubstitutions(ResolveContext context) throws NotPossibleToResolve { - AbstractConfigValue v = context.source().lookupSubst(context, this, expr, prefixLength); + AbstractConfigValue resolveSubstitutions(ResolveContext context) { + context.source().replace(this, ResolveReplacer.cycleResolveReplacer); + try { + AbstractConfigValue v; + try { + v = context.source().lookupSubst(context, expr, prefixLength); + } catch (NotPossibleToResolve e) { + if (expr.optional()) + v = null; + else + throw new ConfigException.UnresolvedSubstitution(origin(), expr + + " was part of a cycle of substitutions involving " + e.traceString(), + e); + } - if (v == null && !expr.optional()) { - throw new ConfigException.UnresolvedSubstitution(origin(), expr.toString()); + if (v == null && !expr.optional()) { + throw new ConfigException.UnresolvedSubstitution(origin(), expr.toString()); + } + return v; + } finally { + context.source().unreplace(this); } - return v; } @Override diff --git a/config/src/main/java/com/typesafe/config/impl/ResolveContext.java b/config/src/main/java/com/typesafe/config/impl/ResolveContext.java index 4e67ebca..b2529ba9 100644 --- a/config/src/main/java/com/typesafe/config/impl/ResolveContext.java +++ b/config/src/main/java/com/typesafe/config/impl/ResolveContext.java @@ -1,15 +1,11 @@ package com.typesafe.config.impl; -import java.util.Collections; -import java.util.LinkedList; -import java.util.Set; -import java.util.LinkedHashSet; -import java.util.concurrent.Callable; +import java.util.List; +import java.util.ArrayList; import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigResolveOptions; import com.typesafe.config.impl.AbstractConfigValue.NotPossibleToResolve; -import com.typesafe.config.impl.AbstractConfigValue.SelfReferential; final class ResolveContext { // this is unfortunately mutable so should only be shared among @@ -20,12 +16,6 @@ final class ResolveContext { // ResolveContext in the same traversal. final private ResolveMemos memos; - // Resolves that we have already begun (for cycle detection). - // SubstitutionResolver separately memoizes completed resolves. - // this set is unfortunately mutable and the user of ResolveContext - // has to be sure it's only shared between ResolveContext that - // are in the same traversal. - final private LinkedList> traversedStack; final private ConfigResolveOptions options; // the current path restriction, used to ensure lazy // resolution and avoid gratuitous cycles. without this, @@ -34,83 +24,25 @@ final class ResolveContext { // CAN BE NULL for a full resolve. final private Path restrictToChild; - ResolveContext(ResolveSource source, ResolveMemos memos, - LinkedList> traversedStack, ConfigResolveOptions options, - Path restrictToChild) { + // another mutable unfortunate. This is + // used to make nice error messages when + // resolution fails. + final private List expressionTrace; + + ResolveContext(ResolveSource source, ResolveMemos memos, ConfigResolveOptions options, + Path restrictToChild, List expressionTrace) { this.source = source; this.memos = memos; - this.traversedStack = traversedStack; this.options = options; this.restrictToChild = restrictToChild; + this.expressionTrace = expressionTrace; } ResolveContext(AbstractConfigObject root, ConfigResolveOptions options, Path restrictToChild) { // LinkedHashSet keeps the traversal order which is at least useful // in error messages if nothing else - this(new ResolveSource(root), new ResolveMemos(), new LinkedList>( - Collections.singletonList(new LinkedHashSet())), options, restrictToChild); - } - - private void traverse(ConfigReference value, SubstitutionExpression via) - throws SelfReferential { - Set traversed = traversedStack.peekFirst(); - - MemoKey key = new MemoKey(value, restrictToChild); - if (traversed.contains(key)) { - throw new SelfReferential(value.origin(), via.path().render()); - } - - traversed.add(key); - } - - private void untraverse(ConfigReference value) { - Set traversed = traversedStack.peekFirst(); - - MemoKey key = new MemoKey(value, restrictToChild); - if (!traversed.remove(key)) - throw new ConfigException.BugOrBroken( - "untraverse() did not find the untraversed substitution " + value); - } - - // this just exists to fix the "throws Exception" on Callable - interface Resolver extends Callable { - @Override - AbstractConfigValue call() throws NotPossibleToResolve; - } - - AbstractConfigValue traversing(ConfigReference value, SubstitutionExpression subst, - Resolver callable) throws NotPossibleToResolve { - try { - traverse(value, subst); - } catch (SelfReferential e) { - if (subst.optional()) - return null; - else - throw e; - } - - try { - return callable.call(); - } finally { - untraverse(value); - } - } - - void replace(AbstractConfigValue value, ResolveReplacer replacer) { - source.replace(value, replacer); - - // we have to reset the cycle detection because with the - // replacement, a cycle may not exist anymore. - traversedStack.addFirst(new LinkedHashSet()); - } - - void unreplace(AbstractConfigValue value) { - source.unreplace(value); - - Set oldTraversed = traversedStack.removeFirst(); - if (!oldTraversed.isEmpty()) - throw new ConfigException.BugOrBroken( - "unreplace() with stuff still in the traverse set: " + oldTraversed); + this(new ResolveSource(root), new ResolveMemos(), options, restrictToChild, + new ArrayList()); } ResolveSource source() { @@ -133,15 +65,34 @@ final class ResolveContext { if (restrictTo == restrictToChild) return this; else - return new ResolveContext(source, memos, traversedStack, options, restrictTo); + return new ResolveContext(source, memos, options, restrictTo, expressionTrace); } ResolveContext unrestricted() { return restrict(null); } - AbstractConfigValue resolve(AbstractConfigValue original) - throws NotPossibleToResolve { + void trace(SubstitutionExpression expr) { + expressionTrace.add(expr); + } + + void untrace() { + expressionTrace.remove(expressionTrace.size() - 1); + } + + String traceString() { + String separator = ", "; + StringBuilder sb = new StringBuilder(); + for (SubstitutionExpression expr : expressionTrace) { + sb.append(expr.toString()); + sb.append(separator); + } + if (sb.length() > 0) + sb.setLength(sb.length() - separator.length()); + return sb.toString(); + } + + AbstractConfigValue resolve(AbstractConfigValue original) throws NotPossibleToResolve { // a fully-resolved (no restrictToChild) object can satisfy a // request for a restricted object, so always check that first. @@ -190,20 +141,20 @@ final class ResolveContext { } static AbstractConfigValue resolve(AbstractConfigValue value, AbstractConfigObject root, - ConfigResolveOptions options, Path restrictToChildOrNull) throws NotPossibleToResolve { - ResolveContext context = new ResolveContext(root, options, restrictToChildOrNull); - - return context.resolve(value); - } - - static AbstractConfigValue resolveWithExternalExceptions(AbstractConfigValue value, - AbstractConfigObject root, ConfigResolveOptions options) { + ConfigResolveOptions options, Path restrictToChildOrNull) { ResolveContext context = new ResolveContext(root, options, null /* restrictToChild */); try { return context.resolve(value); } catch (NotPossibleToResolve e) { - throw e.exportException(value.origin(), null); + // ConfigReference was supposed to catch NotPossibleToResolve + throw new ConfigException.BugOrBroken( + "NotPossibleToResolve was thrown from an outermost resolve", e); } } + + static AbstractConfigValue resolve(AbstractConfigValue value, AbstractConfigObject root, + ConfigResolveOptions options) { + return resolve(value, root, options, null); + } } diff --git a/config/src/main/java/com/typesafe/config/impl/ResolveReplacer.java b/config/src/main/java/com/typesafe/config/impl/ResolveReplacer.java index 846d058e..1a5650f0 100644 --- a/config/src/main/java/com/typesafe/config/impl/ResolveReplacer.java +++ b/config/src/main/java/com/typesafe/config/impl/ResolveReplacer.java @@ -1,15 +1,9 @@ package com.typesafe.config.impl; +import com.typesafe.config.impl.AbstractConfigValue.NotPossibleToResolve; + /** Callback that generates a replacement to use for resolving a substitution. */ abstract class ResolveReplacer { - static final class Undefined extends Exception { - private static final long serialVersionUID = 1L; - - Undefined() { - super("No replacement, substitution will resolve to undefined"); - } - } - // this is a "lazy val" in essence (we only want to // make the replacement one time). Making it volatile // is good enough for thread safety as long as this @@ -17,11 +11,20 @@ abstract class ResolveReplacer { // twice has no side effects, which it should not... private volatile AbstractConfigValue replacement = null; - final AbstractConfigValue replace() throws Undefined { + final AbstractConfigValue replace(ResolveContext context) throws NotPossibleToResolve { if (replacement == null) - replacement = makeReplacement(); + replacement = makeReplacement(context); return replacement; } - protected abstract AbstractConfigValue makeReplacement() throws Undefined; + protected abstract AbstractConfigValue makeReplacement(ResolveContext context) + throws NotPossibleToResolve; + + static final ResolveReplacer cycleResolveReplacer = new ResolveReplacer() { + @Override + protected AbstractConfigValue makeReplacement(ResolveContext context) + throws NotPossibleToResolve { + throw new NotPossibleToResolve(context); + } + }; } diff --git a/config/src/main/java/com/typesafe/config/impl/ResolveSource.java b/config/src/main/java/com/typesafe/config/impl/ResolveSource.java index ae1aaeb6..d7196362 100644 --- a/config/src/main/java/com/typesafe/config/impl/ResolveSource.java +++ b/config/src/main/java/com/typesafe/config/impl/ResolveSource.java @@ -1,12 +1,10 @@ package com.typesafe.config.impl; import java.util.IdentityHashMap; -import java.util.LinkedList; import java.util.Map; import com.typesafe.config.ConfigException; import com.typesafe.config.impl.AbstractConfigValue.NotPossibleToResolve; -import com.typesafe.config.impl.ResolveReplacer.Undefined; /** * This class is the source for values for a substitution like ${foo}. @@ -18,83 +16,79 @@ final class ResolveSource { // traversed node and therefore avoid circular dependencies. // We implement it with this somewhat hacky "patch a replacement" // mechanism instead of actually transforming the tree. - final private Map> replacements; + final private Map replacements; ResolveSource(AbstractConfigObject root) { this.root = root; - this.replacements = new IdentityHashMap>(); + this.replacements = new IdentityHashMap(); } - static private AbstractConfigValue findInObject(final AbstractConfigObject obj, - final ResolveContext context, ConfigReference traversed, - final SubstitutionExpression subst) throws NotPossibleToResolve { - return context.traversing(traversed, subst, new ResolveContext.Resolver() { - @Override - public AbstractConfigValue call() throws NotPossibleToResolve { - return obj.peekPath(subst.path(), context); - } - }); + static private AbstractConfigValue findInObject(AbstractConfigObject obj, + ResolveContext context, SubstitutionExpression subst) + throws NotPossibleToResolve { + return obj.peekPath(subst.path(), context); } - AbstractConfigValue lookupSubst(final ResolveContext context, ConfigReference traversed, - final SubstitutionExpression subst, int prefixLength) throws NotPossibleToResolve { - // First we look up the full path, which means relative to the - // included file if we were not a root file - AbstractConfigValue result = findInObject(root, context, traversed, subst); + AbstractConfigValue lookupSubst(ResolveContext context, SubstitutionExpression subst, + int prefixLength) throws NotPossibleToResolve { + context.trace(subst); + 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(root, context, subst); - 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. - SubstitutionExpression unprefixed = subst - .changePath(subst.path().subPath(prefixLength)); + 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. + SubstitutionExpression unprefixed = subst.changePath(subst.path().subPath( + prefixLength)); - if (result == null && prefixLength > 0) { - result = findInObject(root, context, traversed, unprefixed); - } + // replace the debug trace path + context.untrace(); + context.trace(unprefixed); - if (result == null && context.options().getUseSystemEnvironment()) { - result = findInObject(ConfigImpl.envVariablesAsConfigObject(), context, traversed, - unprefixed); - } - } - - if (result != null) { - final AbstractConfigValue unresolved = result; - result = context.traversing(traversed, subst, new ResolveContext.Resolver() { - @Override - public AbstractConfigValue call() throws NotPossibleToResolve { - return context.resolve(unresolved); + if (result == null && prefixLength > 0) { + result = findInObject(root, context, unprefixed); } - }); - } - return result; + if (result == null && context.options().getUseSystemEnvironment()) { + result = findInObject(ConfigImpl.envVariablesAsConfigObject(), context, + unprefixed); + } + } + + if (result != null) { + result = context.resolve(result); + } + + return result; + } finally { + context.untrace(); + } } void replace(AbstractConfigValue value, ResolveReplacer replacer) { - LinkedList stack = replacements.get(value); - if (stack == null) { - stack = new LinkedList(); - replacements.put(value, stack); - } - stack.addFirst(replacer); + ResolveReplacer old = replacements.put(value, replacer); + if (old != null) + throw new ConfigException.BugOrBroken("should not have replaced the same value twice: " + + value); } void unreplace(AbstractConfigValue value) { - LinkedList stack = replacements.get(value); - if (stack == null) + ResolveReplacer replacer = replacements.remove(value); + if (replacer == null) throw new ConfigException.BugOrBroken("unreplace() without replace(): " + value); - - stack.removeFirst(); } - private AbstractConfigValue replacement(AbstractConfigValue value) throws Undefined { - LinkedList stack = replacements.get(value); - if (stack == null || stack.isEmpty()) + private AbstractConfigValue replacement(ResolveContext context, AbstractConfigValue value) + throws NotPossibleToResolve { + ResolveReplacer replacer = replacements.get(value); + if (replacer == null) { return value; - else - return stack.peek().replace(); + } else { + return replacer.replace(context); + } } /** @@ -104,13 +98,8 @@ final class ResolveSource { AbstractConfigValue resolveCheckingReplacement(ResolveContext context, AbstractConfigValue original) throws NotPossibleToResolve { AbstractConfigValue replacement; - boolean forceUndefined = false; - try { - replacement = replacement(original); - } catch (Undefined e) { - replacement = original; - forceUndefined = true; - } + + replacement = replacement(context, original); if (replacement != original) { // start over, checking if replacement was memoized @@ -118,10 +107,7 @@ final class ResolveSource { } else { AbstractConfigValue resolved; - if (forceUndefined) - resolved = null; - else - resolved = original.resolveSubstitutions(context); + resolved = original.resolveSubstitutions(context); return resolved; } diff --git a/config/src/main/java/com/typesafe/config/impl/SimpleConfig.java b/config/src/main/java/com/typesafe/config/impl/SimpleConfig.java index 7b47adde..96238a3d 100644 --- a/config/src/main/java/com/typesafe/config/impl/SimpleConfig.java +++ b/config/src/main/java/com/typesafe/config/impl/SimpleConfig.java @@ -22,7 +22,6 @@ import com.typesafe.config.ConfigOrigin; import com.typesafe.config.ConfigResolveOptions; import com.typesafe.config.ConfigValue; import com.typesafe.config.ConfigValueType; -import com.typesafe.config.impl.AbstractConfigValue.NotPossibleToResolve; /** * One thing to keep in mind in the future: as Collection-like APIs are added @@ -57,8 +56,7 @@ final class SimpleConfig implements Config, MergeableValue, Serializable { @Override public SimpleConfig resolve(ConfigResolveOptions options) { - AbstractConfigValue resolved = ResolveContext.resolveWithExternalExceptions(object, - object, options); + AbstractConfigValue resolved = ResolveContext.resolve(object, object, options); if (resolved == object) return this; @@ -72,9 +70,7 @@ final class SimpleConfig implements Config, MergeableValue, Serializable { Path path = Path.newPath(pathExpression); ConfigValue peeked; try { - peeked = object.peekPath(path, null); - } catch (NotPossibleToResolve e) { - throw e.exportException(origin(), pathExpression); + peeked = object.peekPath(path); } catch (ConfigException.NotResolved e) { throw ConfigImpl.improveNotResolved(pathExpression, e); } @@ -669,7 +665,7 @@ final class SimpleConfig implements Config, MergeableValue, Serializable { } private AbstractConfigValue peekPath(Path path) { - return root().peekPathWithExternalExceptions(path); + return root().peekPath(path); } private static void addProblem(List accumulator, Path path, diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala index e28476d9..6a8e37c5 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala @@ -31,7 +31,7 @@ class ConfParserTest extends TestUtils { // interpolating arrays into strings tree match { case obj: AbstractConfigObject => - ResolveContext.resolveWithExternalExceptions(tree, obj, ConfigResolveOptions.noSystem()) + ResolveContext.resolve(tree, obj, ConfigResolveOptions.noSystem()) case _ => tree } diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala index 033bbd32..17da465f 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala @@ -15,20 +15,20 @@ class ConfigSubstitutionTest extends TestUtils { private def resolveWithoutFallbacks(v: AbstractConfigObject) = { val options = ConfigResolveOptions.noSystem() - ResolveContext.resolveWithExternalExceptions(v, v, options).asInstanceOf[AbstractConfigObject].toConfig + ResolveContext.resolve(v, v, options).asInstanceOf[AbstractConfigObject].toConfig } private def resolveWithoutFallbacks(s: ConfigSubstitution, root: AbstractConfigObject) = { val options = ConfigResolveOptions.noSystem() - ResolveContext.resolveWithExternalExceptions(s, root, options) + ResolveContext.resolve(s, root, options) } private def resolve(v: AbstractConfigObject) = { val options = ConfigResolveOptions.defaults() - ResolveContext.resolveWithExternalExceptions(v, v, options).asInstanceOf[AbstractConfigObject].toConfig + ResolveContext.resolve(v, v, options).asInstanceOf[AbstractConfigObject].toConfig } private def resolve(s: ConfigSubstitution, root: AbstractConfigObject) = { val options = ConfigResolveOptions.defaults() - ResolveContext.resolveWithExternalExceptions(s, root, options) + ResolveContext.resolve(s, root, options) } private val simpleObject = { @@ -97,10 +97,12 @@ class ConfigSubstitutionTest extends TestUtils { @Test def resolveMissingThrows() { - intercept[ConfigException.UnresolvedSubstitution] { + val e = intercept[ConfigException.UnresolvedSubstitution] { val s = subst("bar.missing") val v = resolveWithoutFallbacks(s, simpleObject) } + assertTrue("wrong exception: " + e.getMessage, + !e.getMessage.contains("cycle")) } @Test @@ -218,10 +220,11 @@ class ConfigSubstitutionTest extends TestUtils { @Test def throwOnCycles() { val s = subst("foo") - val e = intercept[ConfigException.BadValue] { + val e = intercept[ConfigException.UnresolvedSubstitution] { val v = resolveWithoutFallbacks(s, substCycleObject) } assertTrue("Wrong exception: " + e.getMessage, e.getMessage().contains("cycle")) + assertTrue("Wrong exception: " + e.getMessage, e.getMessage().contains("${foo}, ${bar}, ${a.b.c}, ${foo}")) } @Test @@ -229,7 +232,7 @@ class ConfigSubstitutionTest extends TestUtils { // we look up ${?foo}, but the cycle has hard // non-optional links in it so still has to throw. val s = subst("foo", optional = true) - val e = intercept[ConfigException.BadValue] { + val e = intercept[ConfigException.UnresolvedSubstitution] { val v = resolveWithoutFallbacks(s, substCycleObject) } assertTrue("Wrong exception: " + e.getMessage, e.getMessage().contains("cycle")) @@ -256,7 +259,7 @@ class ConfigSubstitutionTest extends TestUtils { @Test def throwOnTwoKeyCycle() { val obj = parseObject("""a:${b},b:${a}""") - val e = intercept[ConfigException.BadValue] { + val e = intercept[ConfigException.UnresolvedSubstitution] { resolve(obj) } assertTrue("Wrong exception: " + e.getMessage, e.getMessage().contains("cycle")) @@ -265,7 +268,7 @@ class ConfigSubstitutionTest extends TestUtils { @Test def throwOnFourKeyCycle() { val obj = parseObject("""a:${b},b:${c},c:${d},d:${a}""") - val e = intercept[ConfigException.BadValue] { + val e = intercept[ConfigException.UnresolvedSubstitution] { resolve(obj) } assertTrue("Wrong exception: " + e.getMessage, e.getMessage().contains("cycle")) @@ -1023,7 +1026,7 @@ class ConfigSubstitutionTest extends TestUtils { @Test def substSelfReferenceUndefined() { val obj = parseObject("""a=${a}""") - val e = intercept[ConfigException.BadValue] { + val e = intercept[ConfigException.UnresolvedSubstitution] { resolve(obj) } assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("cycle")) @@ -1039,15 +1042,19 @@ class ConfigSubstitutionTest extends TestUtils { @Test def substSelfReferenceIndirect() { val obj = parseObject("""a=1, b=${a}, a=${b}""") - val resolved = resolve(obj) - assertEquals(1, resolved.getInt("a")) + val e = intercept[ConfigException.UnresolvedSubstitution] { + resolve(obj) + } + assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("cycle")) } @Test def substSelfReferenceDoubleIndirect() { val obj = parseObject("""a=1, b=${c}, c=${a}, a=${b}""") - val resolved = resolve(obj) - assertEquals(1, resolved.getInt("a")) + val e = intercept[ConfigException.UnresolvedSubstitution] { + resolve(obj) + } + assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("cycle")) } @Test @@ -1201,7 +1208,7 @@ class ConfigSubstitutionTest extends TestUtils { @Test def substInChildFieldNotASelfReference3() { // checking that having bar.foo earlier in the merge - // stack doesn't break the behavior + // stack doesn't break the behavior. val obj = parseObject(""" bar : { foo : 43 } bar : { foo : 42, diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigTest.scala index 2c1d1d87..064747db 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfigTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfigTest.scala @@ -20,11 +20,11 @@ import com.typesafe.config.ConfigMergeable class ConfigTest extends TestUtils { private def resolveNoSystem(v: AbstractConfigValue, root: AbstractConfigObject) = { - ResolveContext.resolveWithExternalExceptions(v, root, ConfigResolveOptions.noSystem()) + ResolveContext.resolve(v, root, ConfigResolveOptions.noSystem()) } private def resolveNoSystem(v: SimpleConfig, root: SimpleConfig) = { - ResolveContext.resolveWithExternalExceptions(v.root, root.root, + ResolveContext.resolve(v.root, root.root, ConfigResolveOptions.noSystem()).asInstanceOf[AbstractConfigObject].toConfig } @@ -346,10 +346,10 @@ class ConfigTest extends TestUtils { // the point here is that we should not try to evaluate a substitution // that's been overridden, and thus not end up with a cycle as long // as we override the problematic link in the cycle. - val e = intercept[ConfigException.BadValue] { + val e = intercept[ConfigException.UnresolvedSubstitution] { val v = resolveNoSystem(subst("foo"), cycleObject) } - assertTrue(e.getMessage().contains("cycle")) + assertTrue("wrong exception: " + e.getMessage, e.getMessage().contains("cycle")) val fixUpCycle = parseObject(""" { "a" : { "b" : { "c" : 57 } } } """) val merged = mergeUnresolved(fixUpCycle, cycleObject) @@ -362,10 +362,10 @@ class ConfigTest extends TestUtils { // the point here is that if our eventual value will be an object, then // we have to evaluate the substitution to see if it's an object to merge, // so we don't avoid the cycle. - val e = intercept[ConfigException.BadValue] { + val e = intercept[ConfigException.UnresolvedSubstitution] { val v = resolveNoSystem(subst("foo"), cycleObject) } - assertTrue(e.getMessage().contains("cycle")) + assertTrue("wrong exception: " + e.getMessage, e.getMessage().contains("cycle")) val fixUpCycle = parseObject(""" { "a" : { "b" : { "c" : { "q" : "u" } } } } """) val merged = mergeUnresolved(fixUpCycle, cycleObject) diff --git a/config/src/test/scala/com/typesafe/config/impl/EquivalentsTest.scala b/config/src/test/scala/com/typesafe/config/impl/EquivalentsTest.scala index 2adb270e..cda1e7cb 100644 --- a/config/src/test/scala/com/typesafe/config/impl/EquivalentsTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/EquivalentsTest.scala @@ -34,7 +34,7 @@ class EquivalentsTest extends TestUtils { // for purposes of these tests, substitutions are only // against the same file's root, and without looking at // system prop or env variable fallbacks. - ResolveContext.resolveWithExternalExceptions(v, v, ConfigResolveOptions.noSystem()) + ResolveContext.resolve(v, v, ConfigResolveOptions.noSystem()) case v => v }