Permalink
Browse files

more fully test origin serialization and clean up code a bit

Mostly removing impossible codepaths.
  • Loading branch information...
1 parent aea95bd commit 985958521d9ed7f080558e91312b4f74fc77e9f7 @havocp havocp committed Apr 12, 2012
View
3 config/src/main/java/com/typesafe/config/impl/SerializedConfigValue.java
@@ -63,7 +63,6 @@
ORIGIN_TYPE,
ORIGIN_URL,
ORIGIN_COMMENTS,
- ORIGIN_NULL_DESCRIPTION,
ORIGIN_NULL_URL,
ORIGIN_NULL_COMMENTS;
@@ -188,7 +187,6 @@ private static void writeOriginField(DataOutput out, SerializedField code, Objec
out.writeUTF(s);
}
break;
- case ORIGIN_NULL_DESCRIPTION: // FALL THRU
case ORIGIN_NULL_URL: // FALL THRU
case ORIGIN_NULL_COMMENTS:
// nothing to write out besides code and length
@@ -248,7 +246,6 @@ private static SimpleConfigOrigin readOrigin(DataInput in, SimpleConfigOrigin ba
}
v = list;
break;
- case ORIGIN_NULL_DESCRIPTION: // FALL THRU
case ORIGIN_NULL_URL: // FALL THRU
case ORIGIN_NULL_COMMENTS:
// nothing to read besides code and length
View
51 config/src/main/java/com/typesafe/config/impl/SimpleConfigOrigin.java
@@ -32,6 +32,8 @@
protected SimpleConfigOrigin(String description, int lineNumber, int endLineNumber,
OriginType originType, String urlOrNull, List<String> commentsOrNull) {
+ if (description == null)
+ throw new ConfigException.BugOrBroken("description may not be null");
this.description = description;
this.lineNumber = lineNumber;
this.endLineNumber = endLineNumber;
@@ -354,8 +356,7 @@ static ConfigOrigin mergeOrigins(Collection<? extends ConfigOrigin> stack) {
Map<SerializedField, Object> toFields() {
Map<SerializedField, Object> m = new EnumMap<SerializedField, Object>(SerializedField.class);
- if (description != null)
- m.put(SerializedField.ORIGIN_DESCRIPTION, description);
+ m.put(SerializedField.ORIGIN_DESCRIPTION, description);
if (lineNumber >= 0)
m.put(SerializedField.ORIGIN_LINE_NUMBER, lineNumber);
@@ -399,8 +400,8 @@ static ConfigOrigin mergeOrigins(Collection<? extends ConfigOrigin> stack) {
// if field has been removed, we have to add a deletion entry
switch (f) {
case ORIGIN_DESCRIPTION:
- m.put(SerializedField.ORIGIN_NULL_DESCRIPTION, "");
- break;
+ throw new ConfigException.BugOrBroken("origin missing description field? "
+ + child);
case ORIGIN_LINE_NUMBER:
m.put(SerializedField.ORIGIN_LINE_NUMBER, -1);
break;
@@ -415,11 +416,11 @@ static ConfigOrigin mergeOrigins(Collection<? extends ConfigOrigin> stack) {
case ORIGIN_COMMENTS:
m.put(SerializedField.ORIGIN_NULL_COMMENTS, "");
break;
- case ORIGIN_NULL_DESCRIPTION: // FALL THRU
case ORIGIN_NULL_URL: // FALL THRU
case ORIGIN_NULL_COMMENTS:
- // inherit the deletion, nothing to do
- break;
+ throw new ConfigException.BugOrBroken(
+ "computing delta, base object should not contain " + f
+ + " " + base);
case END_MARKER:
case ROOT_VALUE:
case ROOT_WAS_CONFIG:
@@ -428,6 +429,8 @@ static ConfigOrigin mergeOrigins(Collection<? extends ConfigOrigin> stack) {
case VALUE_ORIGIN:
throw new ConfigException.BugOrBroken("should not appear here: " + f);
}
+ } else {
+ // field is in base and child, but differs, so leave it
}
}
@@ -459,35 +462,33 @@ static SimpleConfigOrigin fromFields(Map<SerializedField, Object> m) throws IOEx
if (delta.containsKey(f)) {
// delta overrides when keys are in both
// "m" should already contain the right thing
- } else if (!delta.containsKey(f)) {
+ } else {
// base has the key and delta does not.
// we inherit from base unless a "NULL" key blocks.
switch (f) {
case ORIGIN_DESCRIPTION:
- // add to assembled unless delta nulls
- if (!delta.containsKey(SerializedField.ORIGIN_NULL_DESCRIPTION))
- m.put(f, base.get(f));
+ m.put(f, base.get(f));
break;
case ORIGIN_URL:
- if (!delta.containsKey(SerializedField.ORIGIN_NULL_URL))
+ if (delta.containsKey(SerializedField.ORIGIN_NULL_URL)) {
+ m.remove(SerializedField.ORIGIN_NULL_URL);
+ } else {
m.put(f, base.get(f));
+ }
break;
case ORIGIN_COMMENTS:
- if (!delta.containsKey(SerializedField.ORIGIN_NULL_COMMENTS))
- m.put(f, base.get(f));
- break;
- case ORIGIN_NULL_DESCRIPTION:
- if (!delta.containsKey(SerializedField.ORIGIN_DESCRIPTION))
- m.put(f, base.get(f));
- break;
- case ORIGIN_NULL_URL:
- if (!delta.containsKey(SerializedField.ORIGIN_URL))
- m.put(f, base.get(f));
- break;
- case ORIGIN_NULL_COMMENTS:
- if (!delta.containsKey(SerializedField.ORIGIN_COMMENTS))
+ if (delta.containsKey(SerializedField.ORIGIN_NULL_COMMENTS)) {
+ m.remove(SerializedField.ORIGIN_NULL_COMMENTS);
+ } else {
m.put(f, base.get(f));
+ }
break;
+ case ORIGIN_NULL_URL: // FALL THRU
+ case ORIGIN_NULL_COMMENTS: // FALL THRU
+ // base objects shouldn't contain these, should just
+ // lack the field. these are only in deltas.
+ throw new ConfigException.BugOrBroken(
+ "applying fields, base object should not contain " + f + " " + base);
case ORIGIN_END_LINE_NUMBER: // FALL THRU
case ORIGIN_LINE_NUMBER: // FALL THRU
case ORIGIN_TYPE:
View
50 config/src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala
@@ -750,4 +750,54 @@ class ConfigValueTest extends TestUtils {
assertEquals(ResolveStatus.UNRESOLVED, obj.withoutKey("a").resolveStatus())
assertEquals(ResolveStatus.RESOLVED, obj.withoutKey("a").withoutKey("b").resolveStatus())
}
+
+ @Test
+ def configOriginsInSerialization() {
+ import scala.collection.JavaConverters._
+ val bases = Seq(
+ SimpleConfigOrigin.newSimple("foo"),
+ SimpleConfigOrigin.newFile("/tmp/blahblah"),
+ SimpleConfigOrigin.newURL(new URL("http://example.com")),
+ SimpleConfigOrigin.newResource("myresource"))
+ val combos = bases.flatMap({
+ base =>
+ Seq(
+ (base, base.setComments(Seq("this is a comment", "another one").asJava)),
+ (base, base.setComments(null)),
+ (base, base.setLineNumber(41)),
+ (base, SimpleConfigOrigin.mergeOrigins(base.setLineNumber(10), base.setLineNumber(20))))
+ }) ++
+ bases.sliding(2).map({ seq => (seq.head, seq.tail.head) }) ++
+ bases.sliding(3).map({ seq => (seq.head, seq.tail.tail.head) }) ++
+ bases.sliding(4).map({ seq => (seq.head, seq.tail.tail.tail.head) })
+ val withFlipped = combos ++ combos.map(_.swap)
+ val withDuplicate = withFlipped ++ withFlipped.map(p => (p._1, p._1))
+ val values = withDuplicate.flatMap({
+ combo =>
+ Seq(
+ // second inside first
+ new SimpleConfigList(combo._1,
+ Seq[AbstractConfigValue](new ConfigInt(combo._2, 42, "42")).asJava),
+ // triple-nested means we have to null then un-null then null, which is a tricky case
+ // in the origin-serialization code.
+ new SimpleConfigList(combo._1,
+ Seq[AbstractConfigValue](new SimpleConfigList(combo._2,
+ Seq[AbstractConfigValue](new ConfigInt(combo._1, 42, "42")).asJava)).asJava))
+ })
+ def top(v: SimpleConfigList) = v.origin
+ def middle(v: SimpleConfigList) = v.get(0).origin
+ def bottom(v: SimpleConfigList) = if (v.get(0).isInstanceOf[ConfigList])
+ Some(v.get(0).asInstanceOf[ConfigList].get(0).origin)
+ else
+ None
+
+ //System.err.println("values=\n " + values.map(v => top(v).description + ", " + middle(v).description + ", " + bottom(v).map(_.description)).mkString("\n "))
+ for (v <- values) {
+ val deserialized = checkSerializable(v)
+ // double-check that checkSerializable verified the origins
+ assertEquals(top(v), top(deserialized))
+ assertEquals(middle(v), middle(deserialized))
+ assertEquals(bottom(v), bottom(deserialized))
+ }
+ }
}

0 comments on commit 9859585

Please sign in to comment.