diff --git a/src/main/java/com/typesafe/config/ConfigMergeable.java b/src/main/java/com/typesafe/config/ConfigMergeable.java index 9c3b30bb..13fa336a 100644 --- a/src/main/java/com/typesafe/config/ConfigMergeable.java +++ b/src/main/java/com/typesafe/config/ConfigMergeable.java @@ -21,35 +21,16 @@ public interface ConfigMergeable { * Config instances do anything in this method (they need to merge the * fallback keys into themselves). All other values just return the original * value, since they automatically override any fallback. - * + * * The semantics of merging are described in * https://github.com/havocp/config/blob/master/HOCON.md - * - * Prefer <code>withFallbacks()</code>, listing all your fallbacks at once, - * over this method. - * - * <i>When using this method, there is an easy way to write a wrong - * loop.</i> Even if you don't loop, it's easy to do the equivalent wrong - * thing. - * - * <code> - * // WRONG - * for (ConfigMergeable fallback : fallbacks) { - * // DO NOT DO THIS - * result = result.withFallback(fallback); - * } - * </code> - * - * This is wrong because when <code>result</code> is an object and - * <code>fallback</code> is a non-object, - * <code>result.withFallback(fallback)</code> returns an object. Then if - * there are more objects, they are merged into that object. But the correct - * semantics are that a non-object will block merging any more objects later - * in the list. To get it right, you need to iterate backward. Simplest - * solution: prefer <code>withFallbacks()</code> which is harder to get - * wrong, and merge all your fallbacks in one call to - * <code>withFallbacks()</code>. - * + * + * Note that objects do not merge "across" non-objects; if you do + * <code>object.withFallback(nonObject).withFallback(otherObject)</code>, + * then <code>otherObject</code> will simply be ignored. This is an + * intentional part of how merging works. Both non-objects, and any object + * which has fallen back to a non-object, block subsequent fallbacks. + * * @param other * an object whose keys should be used if the keys are not * present in this one diff --git a/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java b/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java index 8b0f1602..becdc73a 100644 --- a/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java +++ b/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java @@ -102,7 +102,8 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements return ConfigValueType.OBJECT; } - protected abstract AbstractConfigObject newCopy(ResolveStatus status); + protected abstract AbstractConfigObject newCopy(ResolveStatus status, + boolean ignoresFallbacks); @Override public AbstractConfigObject withFallbacks(ConfigMergeable... others) { @@ -113,7 +114,9 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements public AbstractConfigObject withFallback(ConfigMergeable mergeable) { ConfigValue other = mergeable.toValue(); - if (other instanceof Unmergeable) { + if (ignoresFallbacks()) { + return this; + } else if (other instanceof Unmergeable) { List<AbstractConfigValue> stack = new ArrayList<AbstractConfigValue>(); stack.add(this); stack.addAll(((Unmergeable) other).unmergedValues()); @@ -143,12 +146,14 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements allResolved = false; } return new SimpleConfigObject(mergeOrigins(this, fallback), - merged, ResolveStatus.fromBoolean(allResolved)); + merged, ResolveStatus.fromBoolean(allResolved), + ignoresFallbacks()); } } else { - // falling back to a non-object has no effect, we just override - // primitive values. - return this; + // falling back to a non-object doesn't merge anything, and also + // prohibits merging any objects that we fall back to later. + // so we have to switch to ignoresFallbacks mode. + return newCopy(resolveStatus(), true /* ignoresFallbacks */); } } @@ -209,7 +214,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements } } if (changes == null) { - return newCopy(newResolveStatus); + return newCopy(newResolveStatus, ignoresFallbacks()); } else { Map<String, AbstractConfigValue> modified = new HashMap<String, AbstractConfigValue>(); for (String k : keySet()) { @@ -219,8 +224,8 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements modified.put(k, peek(k)); } } - return new SimpleConfigObject(origin(), modified, - newResolveStatus); + return new SimpleConfigObject(origin(), modified, newResolveStatus, + ignoresFallbacks()); } } @@ -314,7 +319,8 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements @Override public boolean equals(Object other) { - // note that "origin" is deliberately NOT part of equality + // note that "origin" is deliberately NOT part of equality. + // neither are other "extras" like ignoresFallbacks or resolve status. if (other instanceof ConfigObject) { // optimization to avoid unwrapped() for two ConfigObject, // which is what AbstractConfigValue does. @@ -327,6 +333,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements @Override public int hashCode() { // note that "origin" is deliberately NOT part of equality + // neither are other "extras" like ignoresFallbacks or resolve status. return mapHash(this); } diff --git a/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java b/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java index 63cb0d57..b05adf4a 100644 --- a/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java +++ b/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java @@ -75,6 +75,13 @@ abstract class AbstractConfigValue implements ConfigValue { return this; } + // this is virtualized rather than a field because only some subclasses + // really need to store the boolean, and they may be able to pack it + // with another boolean to save space. + protected boolean ignoresFallbacks() { + return true; + } + @Override public AbstractConfigValue withFallback(ConfigMergeable other) { return this; diff --git a/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java b/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java index 0a8a7055..3b93b56f 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java +++ b/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java @@ -27,16 +27,23 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements // earlier items in the stack win final private List<AbstractConfigValue> stack; + final private boolean ignoresFallbacks; - ConfigDelayedMerge(ConfigOrigin origin, List<AbstractConfigValue> stack) { + private ConfigDelayedMerge(ConfigOrigin origin, + List<AbstractConfigValue> stack, boolean ignoresFallbacks) { super(origin); this.stack = stack; + this.ignoresFallbacks = ignoresFallbacks; if (stack.isEmpty()) throw new ConfigException.BugOrBroken( "creating empty delayed merge value"); } + ConfigDelayedMerge(ConfigOrigin origin, List<AbstractConfigValue> stack) { + this(origin, stack, false /* ignoresFallbacks */); + } + @Override public ConfigValueType valueType() { throw new ConfigException.NotResolved( @@ -85,14 +92,21 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements for (AbstractConfigValue o : stack) { newStack.add(o.relativized(prefix)); } - return new ConfigDelayedMerge(origin(), newStack); + return new ConfigDelayedMerge(origin(), newStack, ignoresFallbacks); + } + + @Override + protected boolean ignoresFallbacks() { + return ignoresFallbacks; } @Override public AbstractConfigValue withFallback(ConfigMergeable mergeable) { ConfigValue other = mergeable.toValue(); - if (other instanceof AbstractConfigObject + if (ignoresFallbacks) { + return this; + } else if (other instanceof AbstractConfigObject || other instanceof Unmergeable) { // if we turn out to be an object, and the fallback also does, // then a merge may be required; delay until we resolve. @@ -103,11 +117,12 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements else newStack.add((AbstractConfigValue) other); return new ConfigDelayedMerge( - AbstractConfigObject.mergeOrigins(newStack), newStack); + AbstractConfigObject.mergeOrigins(newStack), newStack, + ignoresFallbacks); } else { // if the other is not an object, there won't be anything // to merge with, so we are it even if we are an object. - return this; + return new ConfigDelayedMerge(origin(), stack, true /* ignoresFallbacks */); } } diff --git a/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java b/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java index c0a15f4d..5c4e8ead 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java +++ b/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java @@ -21,11 +21,19 @@ class ConfigDelayedMergeObject extends AbstractConfigObject implements Unmergeable { final private List<AbstractConfigValue> stack; + final private boolean ignoresFallbacks; ConfigDelayedMergeObject(ConfigOrigin origin, List<AbstractConfigValue> stack) { + this(origin, stack, false /* ignoresFallbacks */); + } + + private ConfigDelayedMergeObject(ConfigOrigin origin, + List<AbstractConfigValue> stack, boolean ignoresFallbacks) { super(origin); this.stack = stack; + this.ignoresFallbacks = ignoresFallbacks; + if (stack.isEmpty()) throw new ConfigException.BugOrBroken( "creating empty delayed merge object"); @@ -35,11 +43,12 @@ class ConfigDelayedMergeObject extends AbstractConfigObject implements } @Override - public ConfigDelayedMergeObject newCopy(ResolveStatus status) { + public ConfigDelayedMergeObject newCopy(ResolveStatus status, + boolean ignoresFallbacks) { if (status != resolveStatus()) throw new ConfigException.BugOrBroken( "attempt to create resolved ConfigDelayedMergeObject"); - return new ConfigDelayedMergeObject(origin(), stack); + return new ConfigDelayedMergeObject(origin(), stack, ignoresFallbacks); } @Override @@ -67,14 +76,22 @@ class ConfigDelayedMergeObject extends AbstractConfigObject implements for (AbstractConfigValue o : stack) { newStack.add(o.relativized(prefix)); } - return new ConfigDelayedMergeObject(origin(), newStack); + return new ConfigDelayedMergeObject(origin(), newStack, + ignoresFallbacks); + } + + @Override + protected boolean ignoresFallbacks() { + return ignoresFallbacks; } @Override public ConfigDelayedMergeObject withFallback(ConfigMergeable mergeable) { ConfigValue other = mergeable.toValue(); - if (other instanceof AbstractConfigObject + if (ignoresFallbacks) { + return this; + } else if (other instanceof AbstractConfigObject || other instanceof Unmergeable) { // since we are an object, and the fallback could be, // then a merge may be required; delay until we resolve. @@ -85,12 +102,13 @@ class ConfigDelayedMergeObject extends AbstractConfigObject implements else newStack.add((AbstractConfigValue) other); return new ConfigDelayedMergeObject( - AbstractConfigObject.mergeOrigins(newStack), - newStack); + AbstractConfigObject.mergeOrigins(newStack), newStack, + ignoresFallbacks); } else { // if the other is not an object, there won't be anything - // to merge with. - return this; + // to merge with but we do need to ignore any fallbacks + // that occur after it. + return newCopy(resolveStatus(), true /* ignoresFallbacks */); } } diff --git a/src/main/java/com/typesafe/config/impl/ConfigImpl.java b/src/main/java/com/typesafe/config/impl/ConfigImpl.java index e9fff371..a8ee038f 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigImpl.java +++ b/src/main/java/com/typesafe/config/impl/ConfigImpl.java @@ -10,7 +10,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.List; -import java.util.ListIterator; import java.util.Map; import com.typesafe.config.Config; @@ -37,22 +36,11 @@ public class ConfigImpl { static <T extends ConfigMergeable> T merge(Class<T> klass, T first, List<? extends ConfigMergeable> stack) { - if (stack.isEmpty()) { - return first; - } else { - // to be consistent with the semantics of duplicate keys - // in the same file, we have to go backward like this. - // importantly, a primitive value always permanently - // hides a previous object value. - ListIterator<? extends ConfigMergeable> i = stack - .listIterator(stack.size()); - ConfigMergeable merged = i.previous(); - while (i.hasPrevious()) { - merged = i.previous().withFallback(merged); - } - merged = first.withFallback(merged); - return klass.cast(merged); + ConfigMergeable merged = first; + for (ConfigMergeable fallback : stack) { + merged = merged.withFallback(fallback); } + return klass.cast(merged); } private interface NameSource { @@ -428,7 +416,7 @@ public class ConfigImpl { new SimpleConfigOrigin("env var " + key), entry.getValue())); } return new SimpleConfigObject(new SimpleConfigOrigin("env variables"), - m, ResolveStatus.RESOLVED); + m, ResolveStatus.RESOLVED, false /* ignoresFallbacks */); } /** For use ONLY by library internals, DO NOT TOUCH not guaranteed ABI */ diff --git a/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java b/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java index 7f294d66..87492e94 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java +++ b/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java @@ -28,17 +28,19 @@ final class ConfigSubstitution extends AbstractConfigValue implements // than one piece everything is stringified and concatenated final private List<Object> pieces; // the length of any prefixes added with relativized() - final int prefixLength; + final private int prefixLength; + final private boolean ignoresFallbacks; ConfigSubstitution(ConfigOrigin origin, List<Object> pieces) { - this(origin, pieces, 0); + this(origin, pieces, 0, false); } private ConfigSubstitution(ConfigOrigin origin, List<Object> pieces, - int prefixLength) { + int prefixLength, boolean ignoresFallbacks) { super(origin); this.pieces = pieces; this.prefixLength = prefixLength; + this.ignoresFallbacks = ignoresFallbacks; } @Override @@ -58,7 +60,9 @@ final class ConfigSubstitution extends AbstractConfigValue implements public AbstractConfigValue withFallback(ConfigMergeable mergeable) { ConfigValue other = mergeable.toValue(); - if (other instanceof AbstractConfigObject + if (ignoresFallbacks) { + return this; + } else if (other instanceof AbstractConfigObject || other instanceof Unmergeable) { // if we turn out to be an object, and the fallback also does, // then a merge may be required; delay until we resolve. @@ -73,7 +77,7 @@ final class ConfigSubstitution extends AbstractConfigValue implements } else { // if the other is not an object, there won't be anything // to merge with, so we are it even if we are an object. - return this; + return new ConfigSubstitution(origin(), pieces, prefixLength, true /* ignoresFallbacks */); } } @@ -201,7 +205,7 @@ final class ConfigSubstitution extends AbstractConfigValue implements } } return new ConfigSubstitution(origin(), newPieces, prefixLength - + prefix.length()); + + prefix.length(), ignoresFallbacks); } @Override diff --git a/src/main/java/com/typesafe/config/impl/PropertiesParser.java b/src/main/java/com/typesafe/config/impl/PropertiesParser.java index 5dd36e4b..bd822e65 100644 --- a/src/main/java/com/typesafe/config/impl/PropertiesParser.java +++ b/src/main/java/com/typesafe/config/impl/PropertiesParser.java @@ -180,11 +180,12 @@ final class PropertiesParser { .get(parentPath) : root; AbstractConfigObject o = new SimpleConfigObject(origin, scope, - ResolveStatus.RESOLVED); + ResolveStatus.RESOLVED, false /* ignoresFallbacks */); parent.put(scopePath.last(), o); } // return root config object - return new SimpleConfigObject(origin, root, ResolveStatus.RESOLVED); + return new SimpleConfigObject(origin, root, ResolveStatus.RESOLVED, + false /* ignoresFallbacks */); } } diff --git a/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java b/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java index 23fadc06..3534fca0 100644 --- a/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java +++ b/src/main/java/com/typesafe/config/impl/SimpleConfigObject.java @@ -20,21 +20,23 @@ final class SimpleConfigObject extends AbstractConfigObject { // this map should never be modified - assume immutable final private Map<String, AbstractConfigValue> value; final private boolean resolved; + final private boolean ignoresFallbacks; SimpleConfigObject(ConfigOrigin origin, - Map<String, AbstractConfigValue> value, ResolveStatus status) { + Map<String, AbstractConfigValue> value, ResolveStatus status, + boolean ignoresFallbacks) { super(origin); if (value == null) throw new ConfigException.BugOrBroken( "creating config object with null map"); this.value = value; this.resolved = status == ResolveStatus.RESOLVED; + this.ignoresFallbacks = ignoresFallbacks; } SimpleConfigObject(ConfigOrigin origin, Map<String, AbstractConfigValue> value) { - this(origin, value, ResolveStatus.fromValues(value - .values())); + this(origin, value, ResolveStatus.fromValues(value.values()), false /* ignoresFallbacks */); } @Override @@ -43,9 +45,10 @@ final class SimpleConfigObject extends AbstractConfigObject { } @Override - public SimpleConfigObject newCopy(ResolveStatus newStatus) { - return new SimpleConfigObject(origin(), value, - newStatus); + public SimpleConfigObject newCopy(ResolveStatus newStatus, + boolean ignoresFallbacks) { + return new SimpleConfigObject(origin(), value, newStatus, + ignoresFallbacks); } @Override @@ -53,6 +56,11 @@ final class SimpleConfigObject extends AbstractConfigObject { return ResolveStatus.fromBoolean(resolved); } + @Override + protected boolean ignoresFallbacks() { + return ignoresFallbacks; + } + @Override public Map<String, Object> unwrapped() { Map<String, Object> m = new HashMap<String, Object>(); diff --git a/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala b/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala index c01e3179..61228eaa 100644 --- a/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala +++ b/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala @@ -327,7 +327,8 @@ class ConfigValueTest extends TestUtils { // ConfigDelayedMergeObject val emptyObj = SimpleConfigObject.empty() - val dmo = new ConfigDelayedMergeObject(fakeOrigin(), List[AbstractConfigValue](emptyObj, subst("a"), subst("b")).asJava) + val dmo = new ConfigDelayedMergeObject(fakeOrigin(), + List[AbstractConfigValue](emptyObj, subst("a"), subst("b")).asJava) assertEquals(ConfigValueType.OBJECT, dmo.valueType()) unresolved { dmo.unwrapped() } unresolved { dmo.containsKey(null) }