From 0c6fd1551d5b0554f25ffcc74c64b75dfe48a2eb Mon Sep 17 00:00:00 2001 From: Ryan O'Neill Date: Wed, 22 Apr 2015 17:22:49 -0700 Subject: [PATCH] Cleanup and exception handling --- .../typesafe/config/impl/ResolveSource.java | 48 +++++++------------ .../config/impl/ConcatenationTest.scala | 11 +---- .../typesafe/config/impl/PublicApiTest.scala | 11 ----- 3 files changed, 20 insertions(+), 50 deletions(-) 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 beb64618..6b864dd9 100644 --- a/config/src/main/java/com/typesafe/config/impl/ResolveSource.java +++ b/config/src/main/java/com/typesafe/config/impl/ResolveSource.java @@ -59,52 +59,40 @@ final class ResolveSource { try { // we'll fail if anything along the path can't // be looked at without resolving. - if (obj instanceof AbstractConfigObject) { - return findInObject((AbstractConfigObject) obj, path, null); - } else { - return findInList((SimpleConfigList) obj, path, null); - } + return findInObject(obj, path, null); } catch (ConfigException.NotResolved e) { throw ConfigImpl.improveNotResolved(path, e); } } - static private ValueWithPath findInObject(AbstractConfigObject obj, Path path, Node parents) { + static private ValueWithPath findInObject(AbstractConfigValue obj, Path path, Node parents) { String key = path.first(); Path next = path.remainder(); if (ConfigImpl.traceSubstitutionsEnabled()) ConfigImpl.trace("*** looking up '" + key + "' in " + obj); - AbstractConfigValue v = obj.attemptPeekWithPartialResolve(key); - Node newParents = parents == null ? new Node(obj) : parents.prepend(obj); - if (next == null) { - return new ValueWithPath(v, newParents); - } else { - if (v instanceof AbstractConfigObject) { - return findInObject((AbstractConfigObject) v, next, newParents); - } else if (v instanceof SimpleConfigList) { - return findInList((SimpleConfigList) v, next, newParents); - } else { - return new ValueWithPath(null, newParents); + AbstractConfigValue v = null; + Node newParents = null; + + if (obj instanceof AbstractConfigObject) { + v = ((AbstractConfigObject) obj).attemptPeekWithPartialResolve(key); + newParents = parents == null ? new Node((AbstractConfigObject) obj) : parents.prepend((AbstractConfigObject) obj); + } else if (obj instanceof SimpleConfigList) { + int ix; + try { + ix = Integer.parseInt(key); + } catch(Exception e) { + throw new ConfigException.BadPath(key, "path for list must be numeric index"); } + v = ((SimpleConfigList) obj).get(ix); + newParents = parents == null ? new Node((SimpleConfigList) obj) : parents.prepend((SimpleConfigList) obj); } - } - - static private ValueWithPath findInList(SimpleConfigList list, Path path, Node parents) { - String key = path.first(); - Path next = path.remainder(); - if (ConfigImpl.traceSubstitutionsEnabled()) - ConfigImpl.trace("*** looking up '" + key + "' in " + list); - AbstractConfigValue v = list.get(Integer.parseInt(key)); - Node newParents = parents == null ? new Node(list) : parents.prepend(list); if (next == null) { return new ValueWithPath(v, newParents); } else { - if (v instanceof AbstractConfigObject) { - return findInObject((AbstractConfigObject) v, next, newParents); - } else if (v instanceof SimpleConfigList) { - return findInList((SimpleConfigList) v, next, newParents); + if (v instanceof AbstractConfigObject || v instanceof SimpleConfigList) { + return findInObject(v, next, newParents); } else { return new ValueWithPath(null, newParents); } diff --git a/config/src/test/scala/com/typesafe/config/impl/ConcatenationTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConcatenationTest.scala index e3ca5412..deb70e7f 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConcatenationTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConcatenationTest.scala @@ -345,19 +345,12 @@ class ConcatenationTest extends TestUtils { assertEquals(Seq(1, 2, 3), conf.getObjectList("x.a").asScala.toList.map(_.toConfig.getInt("b"))) } - // We would ideally make this case NOT throw an exception but we need to do some work - // to get there, see https://github.com/typesafehub/config/issues/160 @Test def plusEqualsMultipleTimesNestedInArray() { - val e = intercept[ConfigException.Parse] { - val conf = parseConfig("""x = [ { a += 1, a += 2, a += 3 } ] """).resolve() - assertEquals(Seq(1, 2, 3), conf.getObjectList("x").asScala.toVector(0).toConfig.getIntList("a").asScala.toList) - } - assertTrue(e.getMessage.contains("limitation")) + val conf = parseConfig("""x = [ { a += 1, a += 2, a += 3 } ] """).resolve() + assertEquals(Seq(1, 2, 3), conf.getObjectList("x").asScala.toVector(0).toConfig.getIntList("a").asScala.toList) } - // We would ideally make this case NOT throw an exception but we need to do some work - // to get there, see https://github.com/typesafehub/config/issues/160 @Test def plusEqualsMultipleTimesNestedInPlusEquals() { val e = intercept[ConfigException.Parse] { diff --git a/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala b/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala index 796c71de..e0bc395a 100644 --- a/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala @@ -875,17 +875,6 @@ class PublicApiTest extends TestUtils { assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("include statements nested")) } - // We would ideally make this case NOT throw an exception but we need to do some work - // to get there, see https://github.com/typesafehub/config/issues/160 - @Test - def detectIncludeFromList() { - val e = intercept[ConfigException.Parse] { - ConfigFactory.load("include-from-list.conf") - } - - assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("limitation")) - } - @Test def missingOverrideResourceFails() { assertEquals("config.file is not set", null, System.getProperty("config.file"))