From 8f0e6bd5f0ce681e72cb81e096a207292eb05877 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Fri, 11 Nov 2011 15:59:38 -0500 Subject: [PATCH] Make merging work properly with substitutions involved. If we saw a ConfigSubstitution value, we would then ignore any objects after it. But in fact, the substitution might expand to an object, and then we would need to merge it with the objects after it. If we had an object and merged a substitution with it, we were previously ignoring the substitution. But in fact, the substitution might expand to an object, and we would need to merge that object in. So in both cases now we create a ConfigDelayedMerge or ConfigDelayedMergeObject object instead. As part of this, the merge() code was refactored to use a withFallback() method, which is now handy and public. --- .../com/typesafe/config/ConfigException.java | 19 ++ .../com/typesafe/config/ConfigObject.java | 3 + .../java/com/typesafe/config/ConfigValue.java | 16 ++ .../config/impl/AbstractConfigObject.java | 97 +++---- .../config/impl/AbstractConfigValue.java | 9 + .../config/impl/ConfigDelayedMerge.java | 149 +++++++++++ .../config/impl/ConfigDelayedMergeObject.java | 157 +++++++++++ .../com/typesafe/config/impl/ConfigImpl.java | 6 +- .../config/impl/ConfigSubstitution.java | 40 ++- .../config/impl/TransformedConfigObject.java | 4 + .../com/typesafe/config/impl/Unresolved.java | 13 + .../com/typesafe/config/impl/ConfigTest.scala | 249 +++++++++++++++++- .../config/impl/ConfigValueTest.scala | 68 ++++- 13 files changed, 764 insertions(+), 66 deletions(-) create mode 100644 src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java create mode 100644 src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java create mode 100644 src/main/java/com/typesafe/config/impl/Unresolved.java diff --git a/src/main/java/com/typesafe/config/ConfigException.java b/src/main/java/com/typesafe/config/ConfigException.java index ec49782d..397baf2d 100644 --- a/src/main/java/com/typesafe/config/ConfigException.java +++ b/src/main/java/com/typesafe/config/ConfigException.java @@ -198,4 +198,23 @@ public class ConfigException extends RuntimeException { this(origin, message, null); } } + + /** + * Exception indicating that you tried to use a function that requires + * substitutions to be resolved, but substitutions have not been resolved. + * This is always a bug in either application code or the library; it's + * wrong to write a handler for this exception because you should be able to + * fix the code to avoid it. + */ + public static class NotResolved extends BugOrBroken { + private static final long serialVersionUID = 1L; + + public NotResolved(String message, Throwable cause) { + super(message, cause); + } + + public NotResolved(String message) { + this(message, null); + } + } } diff --git a/src/main/java/com/typesafe/config/ConfigObject.java b/src/main/java/com/typesafe/config/ConfigObject.java index 88afa9ca..1371c751 100644 --- a/src/main/java/com/typesafe/config/ConfigObject.java +++ b/src/main/java/com/typesafe/config/ConfigObject.java @@ -32,6 +32,9 @@ public interface ConfigObject extends ConfigValue, Map { @Override Map unwrapped(); + @Override + ConfigObject withFallback(ConfigValue other); + boolean getBoolean(String path); Number getNumber(String path); diff --git a/src/main/java/com/typesafe/config/ConfigValue.java b/src/main/java/com/typesafe/config/ConfigValue.java index 3dd40a53..0e0152fc 100644 --- a/src/main/java/com/typesafe/config/ConfigValue.java +++ b/src/main/java/com/typesafe/config/ConfigValue.java @@ -1,5 +1,6 @@ package com.typesafe.config; + /** * Interface implemented by any configuration value. From the perspective of * users of this interface, the object is immutable. It is therefore safe to use @@ -26,4 +27,19 @@ public interface ConfigValue { * a ConfigObject or ConfigList, it is recursively unwrapped. */ Object unwrapped(); + + /** + * Returns a new value computed by merging this value with another, with + * keys in this value "winning" over the other one. Only ConfigObject has + * anything to do in this method (it merges the fallback keys into itself). + * All other values just return the original value, since they automatically + * override any fallback. + * + * @param other + * an object whose keys should be used if the keys are not + * present in this one + * @return a new object (or the original one, if the fallback doesn't get + * used) + */ + ConfigValue withFallback(ConfigValue other); } diff --git a/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java b/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java index 38b7a75d..51c51406 100644 --- a/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java +++ b/src/main/java/com/typesafe/config/impl/AbstractConfigObject.java @@ -3,8 +3,10 @@ package com.typesafe.config.impl; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import com.typesafe.config.Config; @@ -100,16 +102,12 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements return ConfigValueType.OBJECT; } - static AbstractConfigObject transformed(AbstractConfigObject obj, - ConfigTransformer transformer) { - if (obj.transformer != transformer) - return new TransformedConfigObject(transformer, obj); + @Override + AbstractConfigObject transformed(ConfigTransformer newTransformer) { + if (this.transformer != newTransformer) + return new TransformedConfigObject(newTransformer, this); else - return obj; - } - - private AbstractConfigObject transformed(AbstractConfigObject obj) { - return transformed(obj, transformer); + return this; } static private AbstractConfigValue resolve(AbstractConfigObject self, @@ -162,6 +160,41 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements originalPath); } + @Override + public AbstractConfigObject withFallback(ConfigValue other) { + if (other instanceof Unresolved) { + List stack = new ArrayList(); + stack.add(this); + stack.addAll(((Unresolved) other).unmergedValues()); + return new ConfigDelayedMergeObject(origin(), transformer, stack); + } else if (other instanceof AbstractConfigObject) { + AbstractConfigObject fallback = (AbstractConfigObject) other; + if (fallback.isEmpty()) { + return this; // nothing to do + } else { + Map merged = new HashMap(); + Set allKeys = new HashSet(); + allKeys.addAll(this.keySet()); + allKeys.addAll(fallback.keySet()); + for (String key : allKeys) { + AbstractConfigValue first = this.peek(key); + AbstractConfigValue second = fallback.peek(key); + if (first == null) + merged.put(key, second); + else if (second == null) + merged.put(key, first); + else + merged.put(key, first.withFallback(second)); + } + return new SimpleConfigObject(origin(), transformer, merged); + } + } else { + // falling back to a non-object has no effect, we just override + // primitive values. + return this; + } + } + /** * Stack should be from overrides to fallbacks (earlier items win). Objects * have their keys combined into a new object, while other kinds of value @@ -174,45 +207,13 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements return new SimpleConfigObject(origin, transformer, Collections. emptyMap()); } else if (stack.size() == 1) { - return transformed(stack.get(0), transformer); + return stack.get(0).transformed(transformer); } else { - // for non-objects, we just take the first value; but for objects we - // have to do work to combine them. - Map merged = new HashMap(); - Map> objects = new HashMap>(); - for (AbstractConfigObject obj : stack) { - for (String key : obj.keySet()) { - AbstractConfigValue v = obj.peek(key); - if (!merged.containsKey(key)) { - if (v.valueType() == ConfigValueType.OBJECT) { - // requires recursive merge and transformer fixup - List stackForKey = null; - if (objects.containsKey(key)) { - stackForKey = objects.get(key); - } else { - stackForKey = new ArrayList(); - objects.put(key, stackForKey); - } - stackForKey.add(transformed( - (AbstractConfigObject) v, - transformer)); - } else { - if (!objects.containsKey(key)) { - merged.put(key, v); - } - } - } - } + AbstractConfigObject merged = stack.get(0); + for (int i = 1; i < stack.size(); ++i) { + merged = merged.withFallback(stack.get(i)); } - - for (Map.Entry> entry : objects - .entrySet()) { - List stackForKey = entry.getValue(); - AbstractConfigObject obj = merge(origin, stackForKey, transformer); - merged.put(entry.getKey(), obj); - } - - return new SimpleConfigObject(origin, transformer, merged); + return merged.transformed(transformer); } } @@ -300,7 +301,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements public AbstractConfigObject getObject(String path) { AbstractConfigObject obj = (AbstractConfigObject) find(path, ConfigValueType.OBJECT, path); - return transformed(obj); + return obj.transformed(this.transformer); } @Override @@ -414,7 +415,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements if (v.valueType() != ConfigValueType.OBJECT) throw new ConfigException.WrongType(v.origin(), path, ConfigValueType.OBJECT.name(), v.valueType().name()); - l.add(transformed((AbstractConfigObject) v)); + l.add(((AbstractConfigObject) v).transformed(this.transformer)); } return l; } diff --git a/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java b/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java index 1c3cf7cd..aee30fa2 100644 --- a/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java +++ b/src/main/java/com/typesafe/config/impl/AbstractConfigValue.java @@ -34,6 +34,15 @@ abstract class AbstractConfigValue implements ConfigValue { return this; } + @Override + public AbstractConfigValue withFallback(ConfigValue other) { + return this; + } + + AbstractConfigValue transformed(ConfigTransformer transformer) { + return this; + } + protected boolean canEqual(Object other) { return other instanceof ConfigValue; } diff --git a/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java b/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java new file mode 100644 index 00000000..d42f27f6 --- /dev/null +++ b/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java @@ -0,0 +1,149 @@ +package com.typesafe.config.impl; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import com.typesafe.config.ConfigException; +import com.typesafe.config.ConfigOrigin; +import com.typesafe.config.ConfigValue; +import com.typesafe.config.ConfigValueType; + +/** + * The issue here is that we want to first merge our stack of config files, and + * then we want to evaluate substitutions. But if two substitutions both expand + * to an object, we might need to merge those two objects. Thus, we can't ever + * "override" a substitution when we do a merge; instead we have to save the + * stack of values that should be merged, and resolve the merge when we evaluate + * substitutions. + */ +final class ConfigDelayedMerge extends AbstractConfigValue implements + Unresolved { + + // earlier items in the stack win + final private List stack; + + ConfigDelayedMerge(ConfigOrigin origin, List stack) { + super(origin); + this.stack = stack; + if (stack.isEmpty()) + throw new ConfigException.BugOrBroken( + "creating empty delayed merge value"); + + } + + @Override + public ConfigValueType valueType() { + throw new ConfigException.NotResolved( + "called valueType() on value with unresolved substitutions, need to resolve first"); + } + + @Override + public Object unwrapped() { + throw new ConfigException.NotResolved( + "called unwrapped() on value with unresolved substitutions, need to resolve first"); + } + + @Override + AbstractConfigValue resolveSubstitutions(SubstitutionResolver resolver, + int depth, boolean withFallbacks) { + return resolveSubstitutions(stack, resolver, depth, withFallbacks); + } + + // static method also used by ConfigDelayedMergeObject + static AbstractConfigValue resolveSubstitutions( + List stack, SubstitutionResolver resolver, + int depth, boolean withFallbacks) { + // to resolve substitutions, we need to recursively resolve + // the stack of stuff to merge, and then merge the stack. + List toMerge = new ArrayList(); + + for (AbstractConfigValue v : stack) { + AbstractConfigValue resolved = resolver.resolve(v, depth, + withFallbacks); + + if (resolved instanceof AbstractConfigObject) { + toMerge.add((AbstractConfigObject) resolved); + } else { + if (toMerge.isEmpty()) { + // done, we'll ignore any objects anyway since we + // now have a non-object value. There is a semantic + // effect to this optimization though: we won't detect + // any cycles or other errors in the objects we are + // not resolving. In other cases (without substitutions + // involved) we might detect those errors. + return resolved; + } else { + // look for more objects to merge, since once we have + // an object we merge all objects even if there are + // intervening non-objects. + continue; + } + } + } + + return AbstractConfigObject.merge(toMerge.get(0).origin(), toMerge, + toMerge.get(0).transformer); + } + + @Override + public AbstractConfigValue withFallback(ConfigValue other) { + if (other instanceof AbstractConfigObject + || other instanceof Unresolved) { + // if we turn out to be an object, and the fallback also does, + // then a merge may be required; delay until we resolve. + List newStack = new ArrayList(); + newStack.addAll(stack); + if (other instanceof Unresolved) + newStack.addAll(((Unresolved) other).unmergedValues()); + else + newStack.add((AbstractConfigValue) other); + return new ConfigDelayedMerge(origin(), newStack); + } 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; + } + } + + @Override + public Collection unmergedValues() { + return stack; + } + + @Override + protected boolean canEqual(Object other) { + return other instanceof ConfigDelayedMerge; + } + + @Override + public boolean equals(Object other) { + // note that "origin" is deliberately NOT part of equality + if (other instanceof ConfigDelayedMerge) { + return canEqual(other) + && this.stack.equals(((ConfigDelayedMerge) other).stack); + } else { + return false; + } + } + + @Override + public int hashCode() { + // note that "origin" is deliberately NOT part of equality + return stack.hashCode(); + } + + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + sb.append("DELAYED_MERGE"); + sb.append("("); + for (Object s : stack) { + sb.append(s.toString()); + sb.append(","); + } + sb.setLength(sb.length() - 1); // chop comma + sb.append(")"); + return sb.toString(); + } +} diff --git a/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java b/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java new file mode 100644 index 00000000..1a1a5df8 --- /dev/null +++ b/src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java @@ -0,0 +1,157 @@ +package com.typesafe.config.impl; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import com.typesafe.config.ConfigException; +import com.typesafe.config.ConfigOrigin; +import com.typesafe.config.ConfigValue; + +// This is just like ConfigDelayedMerge except we know statically +// that it will turn out to be an object. +final class ConfigDelayedMergeObject extends AbstractConfigObject implements + Unresolved { + + final private List stack; + + ConfigDelayedMergeObject(ConfigOrigin origin, + ConfigTransformer transformer, List stack) { + super(origin, transformer); + this.stack = stack; + if (stack.isEmpty()) + throw new ConfigException.BugOrBroken( + "creating empty delayed merge object"); + if (!(stack.get(0) instanceof AbstractConfigObject)) + throw new ConfigException.BugOrBroken( + "created a delayed merge object not guaranteed to be an object"); + } + + @Override + AbstractConfigObject resolveSubstitutions(SubstitutionResolver resolver, + int depth, boolean withFallbacks) { + AbstractConfigValue merged = ConfigDelayedMerge.resolveSubstitutions( + stack, resolver, depth, + withFallbacks); + if (merged instanceof AbstractConfigObject) { + return (AbstractConfigObject) merged; + } else { + throw new ConfigException.BugOrBroken( + "somehow brokenly merged an object and didn't get an object"); + } + } + + @Override + public AbstractConfigObject withFallback(ConfigValue other) { + if (other instanceof AbstractConfigObject + || other instanceof Unresolved) { + // since we are an object, and the fallback could be, + // then a merge may be required; delay until we resolve. + List newStack = new ArrayList(); + newStack.addAll(stack); + if (other instanceof Unresolved) + newStack.addAll(((Unresolved) other).unmergedValues()); + else + newStack.add((AbstractConfigValue) other); + return new ConfigDelayedMergeObject(origin(), transformer, newStack); + } else { + // if the other is not an object, there won't be anything + // to merge with. + return this; + } + } + + @Override + public Collection unmergedValues() { + return stack; + } + + @Override + protected boolean canEqual(Object other) { + return other instanceof ConfigDelayedMergeObject; + } + + @Override + public boolean equals(Object other) { + // note that "origin" is deliberately NOT part of equality + if (other instanceof ConfigDelayedMergeObject) { + return canEqual(other) + && this.stack + .equals(((ConfigDelayedMergeObject) other).stack); + } else { + return false; + } + } + + @Override + public int hashCode() { + // note that "origin" is deliberately NOT part of equality + return stack.hashCode(); + } + + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + sb.append("DELAYED_MERGE_OBJECT"); + sb.append("("); + for (Object s : stack) { + sb.append(s.toString()); + sb.append(","); + } + sb.setLength(sb.length() - 1); // chop comma + sb.append(")"); + return sb.toString(); + } + + private static ConfigException notResolved() { + return new ConfigException.NotResolved( + "bug: this object has not had substitutions resolved, so can't be used"); + } + + @Override + public Map unwrapped() { + throw notResolved(); + } + + @Override + public boolean containsKey(Object key) { + throw notResolved(); + } + + @Override + public boolean containsValue(Object value) { + throw notResolved(); + } + + @Override + public Set> entrySet() { + throw notResolved(); + } + + @Override + public boolean isEmpty() { + throw notResolved(); + } + + @Override + public Set keySet() { + throw notResolved(); + } + + @Override + public int size() { + throw notResolved(); + } + + @Override + public Collection values() { + throw notResolved(); + } + + @Override + protected AbstractConfigValue peek(String key) { + throw notResolved(); + } +} diff --git a/src/main/java/com/typesafe/config/impl/ConfigImpl.java b/src/main/java/com/typesafe/config/impl/ConfigImpl.java index 2e2c49ad..f37e13ff 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigImpl.java +++ b/src/main/java/com/typesafe/config/impl/ConfigImpl.java @@ -46,15 +46,13 @@ public class ConfigImpl { static ConfigObject getEnvironmentAsConfig() { // This should not need to create a new config object // as long as the transformer is just the default transformer. - return AbstractConfigObject.transformed(envVariablesConfig(), - withExtraTransformer(null)); + return envVariablesConfig().transformed(withExtraTransformer(null)); } static ConfigObject getSystemPropertiesAsConfig() { // This should not need to create a new config object // as long as the transformer is just the default transformer. - return AbstractConfigObject.transformed(systemPropertiesConfig(), - withExtraTransformer(null)); + return systemPropertiesConfig().transformed(withExtraTransformer(null)); } private static ConfigTransformer withExtraTransformer( diff --git a/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java b/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java index d55d185f..5c7f4082 100644 --- a/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java +++ b/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java @@ -1,5 +1,8 @@ package com.typesafe.config.impl; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.List; import com.typesafe.config.ConfigException; @@ -12,7 +15,8 @@ import com.typesafe.config.ConfigValueType; * it can resolve to a value of any type, though if the substitution has more * than one piece it always resolves to a string via value concatenation. */ -final class ConfigSubstitution extends AbstractConfigValue { +final class ConfigSubstitution extends AbstractConfigValue implements + Unresolved { // this is a list of String and Path where the Path // have to be resolved to values, then if there's more @@ -26,14 +30,40 @@ final class ConfigSubstitution extends AbstractConfigValue { @Override public ConfigValueType valueType() { - throw new ConfigException.BugOrBroken( - "tried to get value type on a ConfigSubstitution; need to resolve substitution first"); + throw new ConfigException.NotResolved( + "tried to get value type on an unresolved substitution: " + + this); } @Override public Object unwrapped() { - throw new ConfigException.BugOrBroken( - "tried to unwrap a ConfigSubstitution; need to resolve substitution first"); + throw new ConfigException.NotResolved( + "tried to unwrap an unresolved substitution: " + this); + } + + @Override + public AbstractConfigValue withFallback(ConfigValue other) { + if (other instanceof AbstractConfigObject + || other instanceof Unresolved) { + // if we turn out to be an object, and the fallback also does, + // then a merge may be required; delay until we resolve. + List newStack = new ArrayList(); + newStack.add(this); + if (other instanceof Unresolved) + newStack.addAll(((Unresolved) other).unmergedValues()); + else + newStack.add((AbstractConfigValue) other); + return new ConfigDelayedMerge(origin(), newStack); + } 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; + } + } + + @Override + public Collection unmergedValues() { + return Collections.singleton(this); } List pieces() { diff --git a/src/main/java/com/typesafe/config/impl/TransformedConfigObject.java b/src/main/java/com/typesafe/config/impl/TransformedConfigObject.java index d56b8ae5..f4b96e29 100644 --- a/src/main/java/com/typesafe/config/impl/TransformedConfigObject.java +++ b/src/main/java/com/typesafe/config/impl/TransformedConfigObject.java @@ -4,6 +4,7 @@ import java.util.Collection; import java.util.Map; import java.util.Set; +import com.typesafe.config.ConfigException; import com.typesafe.config.ConfigValue; final class TransformedConfigObject extends AbstractConfigObject { @@ -14,6 +15,9 @@ final class TransformedConfigObject extends AbstractConfigObject { AbstractConfigObject underlying) { super(underlying.origin(), transformer); this.underlying = underlying; + if (transformer == underlying.transformer) + throw new ConfigException.BugOrBroken( + "Created unnecessary TransformedConfigObject"); } @Override diff --git a/src/main/java/com/typesafe/config/impl/Unresolved.java b/src/main/java/com/typesafe/config/impl/Unresolved.java new file mode 100644 index 00000000..3bd1d1d4 --- /dev/null +++ b/src/main/java/com/typesafe/config/impl/Unresolved.java @@ -0,0 +1,13 @@ +package com.typesafe.config.impl; + +import java.util.Collection; + +/** + * Interface that tags a ConfigValue that is inherently not resolved; i.e. a + * subclass of ConfigValue that will not appear in a resolved tree of values. + * Types such as ConfigObject may not be resolved _yet_, but they are not + * inherently unresolved. + */ +interface Unresolved { + Collection unmergedValues(); +} diff --git a/src/test/scala/com/typesafe/config/impl/ConfigTest.scala b/src/test/scala/com/typesafe/config/impl/ConfigTest.scala index 08505647..3acb2317 100644 --- a/src/test/scala/com/typesafe/config/impl/ConfigTest.scala +++ b/src/test/scala/com/typesafe/config/impl/ConfigTest.scala @@ -12,6 +12,59 @@ import com.typesafe.config.ConfigConfig class ConfigTest extends TestUtils { + def merge(toMerge: AbstractConfigObject*) = { + AbstractConfigObject.merge(fakeOrigin(), toMerge.toList.asJava, null) + } + + // In many cases, we expect merging to be associative. It is + // not, however, when an object value is the first value, + // a non-object value follows, and then an object after that; + // in that case, if an association starts with the non-object + // value, then the value after the non-object value gets lost. + private def associativeMerge(allObjects: Seq[AbstractConfigObject])(assertions: AbstractConfigObject => Unit) { + def m(toMerge: AbstractConfigObject*) = merge(toMerge: _*) + + def makeTrees(objects: Seq[AbstractConfigObject]): Iterator[AbstractConfigObject] = { + objects.length match { + case 0 => Iterator.empty + case 1 => { + Iterator(objects(0)) + } + case 2 => { + Iterator(m(objects(0), objects(1))) + } + case 3 => { + Seq(m(m(objects(0), objects(1)), objects(2)), + m(objects(0), m(objects(1), objects(2))), + m(objects(0), objects(1), objects(2))).iterator + } + case n => { + // obviously if n gets very high we will be sad ;-) + val trees = for { + i <- (1 until n) + val pair = objects.splitAt(i) + first <- makeTrees(pair._1) + second <- makeTrees(pair._2) + } yield m(first, second) + Iterator(m(objects: _*)) ++ trees.iterator + } + } + } + + // the extra m(allObjects: _*) here is redundant but want to + // be sure we do it first, and guard against makeTrees + // being insane. + val trees = Seq(m(allObjects: _*)) ++ makeTrees(allObjects) + for (tree <- trees) { + // if this fails, we were not associative. + assertEquals(trees(0), tree) + } + + for (tree <- trees) { + assertions(tree) + } + } + @Test def mergeTrivial() { val obj1 = parseObject("""{ "a" : 1 }""") @@ -99,6 +152,21 @@ class ConfigTest extends TestUtils { assertEquals(3, merged.getObject("root").keySet().size) } + @Test + def mergeWithEmpty() { + val obj1 = parseObject("""{ "a" : 1 }""") + val obj2 = parseObject("""{ }""") + val merged = AbstractConfigObject.merge(fakeOrigin(), List(obj1, obj2).asJava, null) + + assertEquals(1, merged.getInt("a")) + assertEquals(1, merged.keySet().size) + + val merged2 = AbstractConfigObject.merge(fakeOrigin(), List(obj2, obj1).asJava, null) + + assertEquals(1, merged2.getInt("a")) + assertEquals(1, merged2.keySet().size) + } + @Test def mergeOverrideObjectAndPrimitive() { val obj1 = parseObject("""{ "a" : 1 }""") @@ -118,27 +186,192 @@ class ConfigTest extends TestUtils { @Test def mergeObjectThenPrimitiveThenObject() { + // the semantic here is that the primitive gets ignored, because + // it can't be merged with the object. But potentially it should + // throw an exception even, or warn. val obj1 = parseObject("""{ "a" : { "b" : 42 } }""") val obj2 = parseObject("""{ "a" : 2 }""") - val obj3 = parseObject("""{ "a" : { "b" : 43 } }""") - - val merged = AbstractConfigObject.merge(fakeOrigin(), List(obj1, obj2, obj3).asJava, null) + val obj3 = parseObject("""{ "a" : { "b" : 43, "c" : 44 } }""") + val merged = merge(obj1, obj2, obj3) assertEquals(42, merged.getInt("a.b")) - assertEquals(1, merged.keySet().size) - assertEquals(1, merged.getObject("a").keySet().size()) + assertEquals(1, merged.size) + assertEquals(44, merged.getInt("a.c")) + assertEquals(2, merged.getObject("a").size()) } @Test def mergePrimitiveThenObjectThenPrimitive() { + // the primitive should override the object val obj1 = parseObject("""{ "a" : 1 }""") val obj2 = parseObject("""{ "a" : { "b" : 42 } }""") val obj3 = parseObject("""{ "a" : 3 }""") - val merged = AbstractConfigObject.merge(fakeOrigin(), List(obj1, obj2, obj3).asJava, null) + associativeMerge(Seq(obj1, obj2, obj3)) { merged => + assertEquals(1, merged.getInt("a")) + assertEquals(1, merged.keySet().size) + } + } - assertEquals(1, merged.getInt("a")) - assertEquals(1, merged.keySet().size) + @Test + def mergeSubstitutedValues() { + val obj1 = parseObject("""{ "a" : { "x" : 1, "z" : 4 }, "c" : ${a} }""") + val obj2 = parseObject("""{ "b" : { "y" : 2, "z" : 5 }, "c" : ${b} }""") + + val merged = AbstractConfigObject.merge(fakeOrigin(), List(obj1, obj2).asJava, null) + val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match { + case x: ConfigObject => x + } + + assertEquals(3, resolved.getObject("c").size()) + assertEquals(1, resolved.getInt("c.x")) + assertEquals(2, resolved.getInt("c.y")) + assertEquals(4, resolved.getInt("c.z")) + } + + @Test + def mergeObjectWithSubstituted() { + val obj1 = parseObject("""{ "a" : { "x" : 1, "z" : 4 }, "c" : { "z" : 42 } }""") + val obj2 = parseObject("""{ "b" : { "y" : 2, "z" : 5 }, "c" : ${b} }""") + + val merged = AbstractConfigObject.merge(fakeOrigin(), List(obj1, obj2).asJava, null) + val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match { + case x: ConfigObject => x + } + + assertEquals(2, resolved.getObject("c").size()) + assertEquals(2, resolved.getInt("c.y")) + assertEquals(42, resolved.getInt("c.z")) + + val merged2 = AbstractConfigObject.merge(fakeOrigin(), List(obj2, obj1).asJava, null) + val resolved2 = SubstitutionResolver.resolveWithoutFallbacks(merged2, merged2) match { + case x: ConfigObject => x + } + + assertEquals(2, resolved2.getObject("c").size()) + assertEquals(2, resolved2.getInt("c.y")) + assertEquals(5, resolved2.getInt("c.z")) + } + + private val cycleObject = { + parseObject(""" +{ + "foo" : ${bar}, + "bar" : ${a.b.c}, + "a" : { "b" : { "c" : ${foo} } } +} +""") + } + + @Test + def mergeHidesCycles() { + // 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 v = SubstitutionResolver.resolveWithoutFallbacks(subst("foo"), cycleObject) + } + assertTrue(e.getMessage().contains("cycle")) + + val fixUpCycle = parseObject(""" { "a" : { "b" : { "c" : 57 } } } """) + val merged = AbstractConfigObject.merge(fakeOrigin(), List(fixUpCycle, cycleObject).asJava, null) + val v = SubstitutionResolver.resolveWithoutFallbacks(subst("foo"), merged) + assertEquals(intValue(57), v); + } + + @Test + def mergeWithObjectInFrontKeepsCycles() { + // 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 v = SubstitutionResolver.resolveWithoutFallbacks(subst("foo"), cycleObject) + } + assertTrue(e.getMessage().contains("cycle")) + + val fixUpCycle = parseObject(""" { "a" : { "b" : { "c" : { "q" : "u" } } } } """) + val merged = AbstractConfigObject.merge(fakeOrigin(), List(fixUpCycle, cycleObject).asJava, null) + val e2 = intercept[ConfigException.BadValue] { + val v = SubstitutionResolver.resolveWithoutFallbacks(subst("foo"), merged) + } + assertTrue(e2.getMessage().contains("cycle")) + } + + @Test + def mergeSeriesOfSubstitutions() { + val obj1 = parseObject("""{ "a" : { "x" : 1, "q" : 4 }, "j" : ${a} }""") + val obj2 = parseObject("""{ "b" : { "y" : 2, "q" : 5 }, "j" : ${b} }""") + val obj3 = parseObject("""{ "c" : { "z" : 3, "q" : 6 }, "j" : ${c} }""") + + associativeMerge(Seq(obj1, obj2, obj3)) { merged => + val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match { + case x: ConfigObject => x + } + + assertEquals(4, resolved.getObject("j").size()) + assertEquals(1, resolved.getInt("j.x")) + assertEquals(2, resolved.getInt("j.y")) + assertEquals(3, resolved.getInt("j.z")) + assertEquals(4, resolved.getInt("j.q")) + } + } + + @Test + def mergePrimitiveAndTwoSubstitutions() { + val obj1 = parseObject("""{ "j" : 42 }""") + val obj2 = parseObject("""{ "b" : { "y" : 2, "q" : 5 }, "j" : ${b} }""") + val obj3 = parseObject("""{ "c" : { "z" : 3, "q" : 6 }, "j" : ${c} }""") + + associativeMerge(Seq(obj1, obj2, obj3)) { merged => + val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match { + case x: ConfigObject => x + } + + assertEquals(3, resolved.size()) + assertEquals(42, resolved.getInt("j")); + assertEquals(2, resolved.getInt("b.y")) + assertEquals(3, resolved.getInt("c.z")) + } + } + + @Test + def mergeObjectAndTwoSubstitutions() { + val obj1 = parseObject("""{ "j" : { "x" : 1, "q" : 4 } }""") + val obj2 = parseObject("""{ "b" : { "y" : 2, "q" : 5 }, "j" : ${b} }""") + val obj3 = parseObject("""{ "c" : { "z" : 3, "q" : 6 }, "j" : ${c} }""") + + associativeMerge(Seq(obj1, obj2, obj3)) { merged => + val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match { + case x: ConfigObject => x + } + + assertEquals(4, resolved.getObject("j").size()) + assertEquals(1, resolved.getInt("j.x")) + assertEquals(2, resolved.getInt("j.y")) + assertEquals(3, resolved.getInt("j.z")) + assertEquals(4, resolved.getInt("j.q")) + } + } + + @Test + def mergeObjectSubstitutionObjectSubstitution() { + val obj1 = parseObject("""{ "j" : { "w" : 1, "q" : 5 } }""") + val obj2 = parseObject("""{ "b" : { "x" : 2, "q" : 6 }, "j" : ${b} }""") + val obj3 = parseObject("""{ "j" : { "y" : 3, "q" : 7 } }""") + val obj4 = parseObject("""{ "c" : { "z" : 4, "q" : 8 }, "j" : ${c} }""") + + associativeMerge(Seq(obj1, obj2, obj3, obj4)) { merged => + val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match { + case x: ConfigObject => x + } + + assertEquals(5, resolved.getObject("j").size()) + assertEquals(1, resolved.getInt("j.w")) + assertEquals(2, resolved.getInt("j.x")) + assertEquals(3, resolved.getInt("j.y")) + assertEquals(4, resolved.getInt("j.z")) + assertEquals(5, resolved.getInt("j.q")) + } } @Test diff --git a/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala b/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala index a8762b98..52cc686f 100644 --- a/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala +++ b/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala @@ -7,6 +7,8 @@ import java.util.Collections import scala.collection.JavaConverters._ import com.typesafe.config.ConfigObject import com.typesafe.config.ConfigList +import com.typesafe.config.ConfigException +import com.typesafe.config.ConfigValueType class ConfigValueTest extends TestUtils { @@ -91,6 +93,33 @@ class ConfigValueTest extends TestUtils { checkNotEqualObjects(a, b) } + @Test + def configDelayedMergeEquality() { + val s1 = subst("foo") + val s2 = subst("bar") + val a = new ConfigDelayedMerge(fakeOrigin(), List[AbstractConfigValue](s1, s2).asJava) + val sameAsA = new ConfigDelayedMerge(fakeOrigin(), List[AbstractConfigValue](s1, s2).asJava) + val b = new ConfigDelayedMerge(fakeOrigin(), List[AbstractConfigValue](s2, s1).asJava) + + checkEqualObjects(a, a) + checkEqualObjects(a, sameAsA) + checkNotEqualObjects(a, b) + } + + @Test + def configDelayedMergeObjectEquality() { + val empty = new SimpleConfigObject(fakeOrigin(), null, Collections.emptyMap[String, AbstractConfigValue]()) + val s1 = subst("foo") + val s2 = subst("bar") + val a = new ConfigDelayedMergeObject(fakeOrigin(), null, List[AbstractConfigValue](empty, s1, s2).asJava) + val sameAsA = new ConfigDelayedMergeObject(fakeOrigin(), null, List[AbstractConfigValue](empty, s1, s2).asJava) + val b = new ConfigDelayedMergeObject(fakeOrigin(), null, List[AbstractConfigValue](empty, s2, s1).asJava) + + checkEqualObjects(a, a) + checkEqualObjects(a, sameAsA) + checkNotEqualObjects(a, b) + } + @Test def valuesToString() { // just check that these don't throw, the exact output @@ -101,10 +130,15 @@ class ConfigValueTest extends TestUtils { stringValue("hi").toString() nullValue().toString() boolValue(true).toString() - (new SimpleConfigObject(fakeOrigin(), null, Collections.emptyMap[String, AbstractConfigValue]())).toString() + val emptyObj = new SimpleConfigObject(fakeOrigin(), null, Collections.emptyMap[String, AbstractConfigValue]()) + emptyObj.toString() (new SimpleConfigList(fakeOrigin(), Collections.emptyList[AbstractConfigValue]())).toString() subst("a").toString() substInString("b").toString() + val dm = new ConfigDelayedMerge(fakeOrigin(), List[AbstractConfigValue](subst("a"), subst("b")).asJava) + dm.toString() + val dmo = new ConfigDelayedMergeObject(fakeOrigin(), null, List[AbstractConfigValue](emptyObj, subst("a"), subst("b")).asJava) + dmo.toString() } private def unsupported(body: => Unit) { @@ -222,4 +256,36 @@ class ConfigValueTest extends TestUtils { unsupported { l.retainAll(List[ConfigValue](intValue(1)).asJava) } unsupported { l.set(0, intValue(42)) } } + + private def unresolved(body: => Unit) { + intercept[ConfigException.NotResolved] { + body + } + } + + @Test + def notResolvedThrown() { + // ConfigSubstitution + unresolved { subst("foo").valueType() } + unresolved { subst("foo").unwrapped() } + + // ConfigDelayedMerge + val dm = new ConfigDelayedMerge(fakeOrigin(), List[AbstractConfigValue](subst("a"), subst("b")).asJava) + unresolved { dm.valueType() } + unresolved { dm.unwrapped() } + + // ConfigDelayedMergeObject + val emptyObj = new SimpleConfigObject(fakeOrigin(), null, Collections.emptyMap[String, AbstractConfigValue]()) + val dmo = new ConfigDelayedMergeObject(fakeOrigin(), null, List[AbstractConfigValue](emptyObj, subst("a"), subst("b")).asJava) + assertEquals(ConfigValueType.OBJECT, dmo.valueType()) + unresolved { dmo.unwrapped() } + unresolved { dmo.containsKey(null) } + unresolved { dmo.containsValue(null) } + unresolved { dmo.entrySet() } + unresolved { dmo.isEmpty() } + unresolved { dmo.keySet() } + unresolved { dmo.size() } + unresolved { dmo.values() } + unresolved { dmo.getInt("foo") } + } }