github-repos/config
1
0
Fork 0
mirror of https://github.com/lightbend/config.git synced 2025-01-15 23:01:05 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix AbstractConfigValue.withFallbacks and clean up ConfigDelayedMerge withFallback

Browse Source 
Had previously fixed AbstractConfigObject.merge() in the same way. For this
patch, factor out the code from AbstractConfigObject and use it everywhere.

Add some tests for merges with substitutions involved.

Also write some docs for ConfigMergeable explaining the easy way
to do it wrong.
This commit is contained in:
Havoc Pennington 2011-11-17 22:23:06 -05:00
parent f6395d629d
commit d700404242
6 changed files with 149 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/main/java/com/typesafe/config/ConfigMergeable.java
View File

@ -7,7 +7,7 @@ package com.typesafe.config;
public interface ConfigMergeable {
/**
* Converts the mergeable to a ConfigValue to be merged.
*
*
* @return
*/
ConfigValue toValue();
@ -19,6 +19,34 @@ public interface ConfigMergeable {
* 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>.
*
* @param other
* an object whose keys should be used if the keys are not
* present in this one
@ -29,7 +57,8 @@ public interface ConfigMergeable {
/**
* Convenience method just calls withFallback() on each of the values;
* earlier values in the list win over later ones.
* earlier values in the list win over later ones. The semantics of merging
* are described in https://github.com/havocp/config/blob/master/HOCON.md
*
* @param fallbacks
* @return a version of the object with the requested fallbacks merged in

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

@ -7,7 +7,6 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
@ -194,31 +193,6 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
return mergeOrigins(Arrays.asList(stack));
}
/**
* Stack should be from overrides to fallbacks (earlier items win). Objects
* have their keys combined into a new object, while other kinds of value
* are just first-one-wins.
*/
static AbstractConfigObject merge(List<AbstractConfigObject> stack) {
if (stack.isEmpty()) {
return SimpleConfigObject.empty();
} else if (stack.size() == 1) {
return stack.get(0);
} 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<AbstractConfigObject> i = stack.listIterator(stack
.size());
AbstractConfigObject merged = i.previous();
while (i.hasPrevious()) {
merged = i.previous().withFallback(merged);
}
return merged;
}
}
private AbstractConfigObject modify(Modifier modifier,
ResolveStatus newResolveStatus) {
Map<String, AbstractConfigValue> changes = null;

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

@ -82,11 +82,7 @@ abstract class AbstractConfigValue implements ConfigValue {
// note: this is a no-op unless the subclass overrides withFallback().
// But we need to do this because subclass withFallback() may not
// just "return this"
AbstractConfigValue merged = this;
for (ConfigMergeable f : fallbacks) {
merged = merged.withFallback(f.toValue());
}
return merged;
return ConfigImpl.merge(AbstractConfigValue.class, this, fallbacks);
}
protected boolean canEqual(Object other) {

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

@ -58,32 +58,17 @@ final class ConfigDelayedMerge extends AbstractConfigValue implements
int depth, ConfigResolveOptions options) {
// to resolve substitutions, we need to recursively resolve
// the stack of stuff to merge, and then merge the stack.
List<AbstractConfigObject> toMerge = new ArrayList<AbstractConfigObject>();
List<AbstractConfigValue> toMerge = new ArrayList<AbstractConfigValue>();
for (AbstractConfigValue v : stack) {
AbstractConfigValue resolved = resolver.resolve(v, depth, options);
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;
}
}
toMerge.add(resolved);
}
return AbstractConfigObject.merge(toMerge);
// we shouldn't have a delayed merge object with an empty stack, so
// it should be safe to ignore the toMerge.isEmpty case.
return ConfigImpl.merge(AbstractConfigValue.class, toMerge.get(0),
toMerge.subList(1, toMerge.size()));
}
@Override

29
src/main/java/com/typesafe/config/impl/ConfigImpl.java
View File

@ -1,16 +1,19 @@
package com.typesafe.config.impl;
import java.util.ArrayList;
import java.util.Arrays;
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;
import com.typesafe.config.ConfigException;
import com.typesafe.config.ConfigIncludeContext;
import com.typesafe.config.ConfigIncluder;
import com.typesafe.config.ConfigMergeable;
import com.typesafe.config.ConfigObject;
import com.typesafe.config.ConfigOrigin;
import com.typesafe.config.ConfigParseOptions;
@ -22,6 +25,32 @@ import com.typesafe.config.ConfigValue;
/** This is public but is only supposed to be used by the "config" package */
public class ConfigImpl {
static <T extends ConfigMergeable> T merge(Class<T> klass, T first,
ConfigMergeable... others) {
List<ConfigMergeable> stack = Arrays.asList(others);
return merge(klass, first, stack);
}
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);
}
}
private interface NameSource {
ConfigParseable nameToParseable(String name);
}

104
src/test/scala/com/typesafe/config/impl/ConfigTest.scala
View File

@ -15,8 +15,31 @@ import com.typesafe.config.ConfigFactory
class ConfigTest extends TestUtils {
private def resolveNoSystem(v: AbstractConfigValue, root: AbstractConfigObject) = {
SubstitutionResolver.resolve(v, root, ConfigResolveOptions.noSystem())
}
private def resolveNoSystem(v: SimpleConfig, root: SimpleConfig) = {
SubstitutionResolver.resolve(v.toObject, root.toObject,
ConfigResolveOptions.noSystem()).asInstanceOf[AbstractConfigObject].toConfig
}
def mergeUnresolved(toMerge: AbstractConfigObject*) = {
if (toMerge.isEmpty) {
SimpleConfigObject.empty()
} else {
val obj = toMerge(0).withFallbacks(toMerge.slice(1, toMerge.size): _*)
obj match {
case x: AbstractConfigObject => x
}
}
}
def merge(toMerge: AbstractConfigObject*) = {
AbstractConfigObject.merge(toMerge.toList.asJava)
val obj = mergeUnresolved(toMerge: _*)
resolveNoSystem(obj, obj) match {
case x: AbstractConfigObject => x
}
}
// In many cases, we expect merging to be associative. It is
@ -25,7 +48,7 @@ class ConfigTest extends TestUtils {
// 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: SimpleConfig => Unit) {
def m(toMerge: AbstractConfigObject*) = merge(toMerge: _*)
def m(toMerge: AbstractConfigObject*) = mergeUnresolved(toMerge: _*)
def makeTrees(objects: Seq[AbstractConfigObject]): Iterator[AbstractConfigObject] = {
objects.length match {
@ -187,6 +210,23 @@ class ConfigTest extends TestUtils {
assertEquals(1, merged2.getObject("a").size)
}
@Test
def mergeOverrideObjectAndSubstitution() {
val obj1 = parseObject("""{ "a" : 1 }""")
val obj2 = parseObject("""{ "a" : { "b" : ${c} }, "c" : 42 }""")
val merged = merge(obj1, obj2).toConfig
assertEquals(1, merged.getInt("a"))
assertEquals(2, merged.toObject.size)
val merged2 = merge(obj2, obj1).toConfig
assertEquals(42, merged2.getConfig("a").getInt("b"))
assertEquals(42, merged2.getInt("a.b"))
assertEquals(2, merged2.toObject.size)
assertEquals(1, merged2.getObject("a").size)
}
@Test
def mergeObjectThenPrimitiveThenObject() {
// the semantic here is that the primitive blocks the
@ -210,6 +250,29 @@ class ConfigTest extends TestUtils {
assertEquals(2, merged2.getObject("a").size())
}
@Test
def mergeObjectThenSubstitutionThenObject() {
// the semantic here is that the primitive blocks the
// object that occurs at lower priority. This is consistent
// with duplicate keys in the same file.
val obj1 = parseObject("""{ "a" : { "b" : ${f} } }""")
val obj2 = parseObject("""{ "a" : 2 }""")
val obj3 = parseObject("""{ "a" : { "b" : ${d}, "c" : ${e} }, "d" : 43, "e" : 44, "f" : 42 }""")
val merged = merge(obj1, obj2, obj3).toConfig
assertEquals(42, merged.getInt("a.b"))
assertEquals(4, merged.toObject.size)
assertEquals(1, merged.getObject("a").size())
val merged2 = merge(obj3, obj2, obj1).toConfig
assertEquals(43, merged2.getInt("a.b"))
assertEquals(44, merged2.getInt("a.c"))
assertEquals(4, merged2.toObject.size)
assertEquals(2, merged2.getObject("a").size())
}
@Test
def mergePrimitiveThenObjectThenPrimitive() {
// the primitive should override the object
@ -223,13 +286,19 @@ class ConfigTest extends TestUtils {
}
}
private def resolveNoSystem(v: AbstractConfigValue, root: AbstractConfigObject) = {
SubstitutionResolver.resolve(v, root, ConfigResolveOptions.noSystem())
}
@Test
def mergeSubstitutionThenObjectThenSubstitution() {
// the substitution should override the object
val obj1 = parseObject("""{ "a" : ${b}, "b" : 1 }""")
val obj2 = parseObject("""{ "a" : { "b" : 42 } }""")
val obj3 = parseObject("""{ "a" : ${c}, "c" : 2 }""")
private def resolveNoSystem(v: SimpleConfig, root: SimpleConfig) = {
SubstitutionResolver.resolve(v.toObject, root.toObject,
ConfigResolveOptions.noSystem()).asInstanceOf[AbstractConfigObject].toConfig
associativeMerge(Seq(obj1, obj2, obj3)) { merged =>
val resolved = resolveNoSystem(merged, merged)
assertEquals(1, resolved.getInt("a"))
assertEquals(3, resolved.toObject.size)
}
}
@Test
@ -237,10 +306,7 @@ class ConfigTest extends TestUtils {
val obj1 = parseObject("""{ "a" : { "x" : 1, "z" : 4 }, "c" : ${a} }""")
val obj2 = parseObject("""{ "b" : { "y" : 2, "z" : 5 }, "c" : ${b} }""")
val merged = merge(obj1, obj2)
val resolved = (resolveNoSystem(merged, merged) match {
case x: ConfigObject => x
}).toConfig
val resolved = merge(obj1, obj2).toConfig
assertEquals(3, resolved.getObject("c").size())
assertEquals(1, resolved.getInt("c.x"))
@ -253,19 +319,13 @@ class ConfigTest extends TestUtils {
val obj1 = parseObject("""{ "a" : { "x" : 1, "z" : 4 }, "c" : { "z" : 42 } }""")
val obj2 = parseObject("""{ "b" : { "y" : 2, "z" : 5 }, "c" : ${b} }""")
val merged = merge(obj1, obj2)
val resolved = (resolveNoSystem(merged, merged) match {
case x: ConfigObject => x
}).toConfig
val resolved = merge(obj1, obj2).toConfig
assertEquals(2, resolved.getObject("c").size())
assertEquals(2, resolved.getInt("c.y"))
assertEquals(42, resolved.getInt("c.z"))
val merged2 = merge(obj2, obj1)
val resolved2 = (resolveNoSystem(merged2, merged2) match {
case x: ConfigObject => x
}).toConfig
val resolved2 = merge(obj2, obj1).toConfig
assertEquals(2, resolved2.getObject("c").size())
assertEquals(2, resolved2.getInt("c.y"))
@ -293,7 +353,7 @@ class ConfigTest extends TestUtils {
assertTrue(e.getMessage().contains("cycle"))
val fixUpCycle = parseObject(""" { "a" : { "b" : { "c" : 57 } } } """)
val merged = merge(fixUpCycle, cycleObject)
val merged = mergeUnresolved(fixUpCycle, cycleObject)
val v = resolveNoSystem(subst("foo"), merged)
assertEquals(intValue(57), v);
}
@ -309,7 +369,7 @@ class ConfigTest extends TestUtils {
assertTrue(e.getMessage().contains("cycle"))
val fixUpCycle = parseObject(""" { "a" : { "b" : { "c" : { "q" : "u" } } } } """)
val merged = merge(fixUpCycle, cycleObject)
val merged = mergeUnresolved(fixUpCycle, cycleObject)
val e2 = intercept[ConfigException.BadValue] {
val v = resolveNoSystem(subst("foo"), merged)
}