From 70146335d3550f40415def439238afad8f60fce0 Mon Sep 17 00:00:00 2001 From: john Date: Thu, 11 Aug 2016 01:26:47 +0100 Subject: [PATCH] implementation of "required" includes as discussed in https://github.com/typesafehub/config/issues/121 --- HOCON.md | 16 ++++- .../typesafe/config/ConfigIncludeContext.java | 8 +++ .../typesafe/config/ConfigParseOptions.java | 3 +- .../config/impl/ConfigDocumentParser.java | 66 ++++++++++++++++--- .../config/impl/ConfigNodeInclude.java | 8 ++- .../typesafe/config/impl/ConfigParser.java | 11 ++-- .../config/impl/SimpleIncludeContext.java | 14 +++- .../com/typesafe/config/impl/TokenType.java | 2 + .../com/typesafe/config/impl/Tokenizer.java | 10 ++- .../java/com/typesafe/config/impl/Tokens.java | 2 + .../typesafe/config/impl/ConfParserTest.scala | 46 ++++++++++++- .../com/typesafe/config/impl/TestUtils.scala | 6 +- .../typesafe/config/impl/TokenizerTest.scala | 45 ++++++++++++- 13 files changed, 211 insertions(+), 26 deletions(-) diff --git a/HOCON.md b/HOCON.md index 551fc045..229bb61c 100644 --- a/HOCON.md +++ b/HOCON.md @@ -1031,12 +1031,24 @@ get a value from a system property or from the reference configuration. So it's not enough to only look up the "fixed up" path, it's necessary to look up the original path as well. -#### Include semantics: missing files +#### Include semantics: missing files and required files -If an included file does not exist, the include statement should +By default, if an included file does not exist then the include statement should be silently ignored (as if the included file contained only an empty object). +If however an included resource is mandatory then the name of the +included resource may be wrapped with `required()`, in which case +file parsing will fail with an error if the resource cannot be resolved. + +The syntax for this is + + include required("foo.conf") + include required(file("foo.conf")) + include required(classpath("foo.conf")) + include required(url("http://localhost/foo.conf")) + + Other IO errors probably should not be ignored but implementations will have to make a judgment which IO errors reflect an ignorable missing file, and which reflect a problem to bring to the user's diff --git a/config/src/main/java/com/typesafe/config/ConfigIncludeContext.java b/config/src/main/java/com/typesafe/config/ConfigIncludeContext.java index 725d8f20..7560470e 100644 --- a/config/src/main/java/com/typesafe/config/ConfigIncludeContext.java +++ b/config/src/main/java/com/typesafe/config/ConfigIncludeContext.java @@ -43,4 +43,12 @@ public interface ConfigIncludeContext { * @return the parse options */ ConfigParseOptions parseOptions(); + + + /** + * Copy this {@link ConfigIncludeContext} giving it a new value for its parseOptions + * + * @return the updated copy of this context + */ + ConfigIncludeContext setParseOptions(ConfigParseOptions options); } diff --git a/config/src/main/java/com/typesafe/config/ConfigParseOptions.java b/config/src/main/java/com/typesafe/config/ConfigParseOptions.java index e1c1a770..fcfb4d69 100644 --- a/config/src/main/java/com/typesafe/config/ConfigParseOptions.java +++ b/config/src/main/java/com/typesafe/config/ConfigParseOptions.java @@ -111,7 +111,8 @@ public final class ConfigParseOptions { /** * Set to false to throw an exception if the item being parsed (for example * a file) is missing. Set to true to just return an empty document in that - * case. + * case. Note that this setting applies on only to fetching the root document, + * it has no effect on any nested includes. * * @param allowMissing true to silently ignore missing item * @return options with the "allow missing" flag set diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigDocumentParser.java b/config/src/main/java/com/typesafe/config/impl/ConfigDocumentParser.java index 25ce3c4b..2772654d 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigDocumentParser.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigDocumentParser.java @@ -312,6 +312,44 @@ final class ConfigDocumentParser { } private ConfigNodeInclude parseInclude(ArrayList children) { + + Token t = nextTokenCollectingWhitespace(children); + + // we either have a 'required(' or one of quoted string or the "file()" syntax + if (Tokens.isUnquotedText(t)) { + String kindText = Tokens.getUnquotedText(t); + + if (kindText.equals("required")) { + Token tOpen = nextToken(); + if (tOpen != Tokens.OPEN_ROUND) { + throw parseError("expecting include parameter to be quoted filename, file(), classpath(), url() or required(). No spaces are allowed before the open paren. Not expecting: " + + tOpen); + } + + children.add(new ConfigNodeSingleToken(t)); + children.add(new ConfigNodeSingleToken(tOpen)); + + ConfigNodeInclude res = parseIncludeResource(children, true); + + Token tClose = nextTokenCollectingWhitespace(children); + if (tClose != Tokens.CLOSE_ROUND) { + throw parseError("expecting the closing parentheses ')' of required() here, not: " + tClose); + } + children.add(new ConfigNodeSingleToken(tClose)); + + return res; + } else { + putBack(t); + return parseIncludeResource(children, false); + } + } + else { + putBack(t); + return parseIncludeResource(children, false); + } + } + + private ConfigNodeInclude parseIncludeResource(ArrayList children, boolean isRequired) { Token t = nextTokenCollectingWhitespace(children); // we either have a quoted string or the "file()" syntax @@ -320,11 +358,11 @@ final class ConfigDocumentParser { String kindText = Tokens.getUnquotedText(t); ConfigIncludeKind kind; - if (kindText.equals("url(")) { + if (kindText.equals("url")) { kind = ConfigIncludeKind.URL; - } else if (kindText.equals("file(")) { + } else if (kindText.equals("file")) { kind = ConfigIncludeKind.FILE; - } else if (kindText.equals("classpath(")) { + } else if (kindText.equals("classpath")) { kind = ConfigIncludeKind.CLASSPATH; } else { throw parseError("expecting include parameter to be quoted filename, file(), classpath(), or url(). No spaces are allowed before the open paren. Not expecting: " @@ -333,6 +371,14 @@ final class ConfigDocumentParser { children.add(new ConfigNodeSingleToken(t)); + // skip space inside parens + t = nextToken(); + + if (t != Tokens.OPEN_ROUND) { + throw parseError("expecting include parameter to be quoted filename, file(), classpath(), or url(). No spaces are allowed before the open paren. Not expecting: " + kindText + " followed by " + + t); + } + // skip space inside parens t = nextTokenCollectingWhitespace(children); @@ -342,19 +388,19 @@ final class ConfigDocumentParser { + t); } children.add(new ConfigNodeSimpleValue(t)); + // skip space after string, inside parens t = nextTokenCollectingWhitespace(children); - if (Tokens.isUnquotedText(t) && Tokens.getUnquotedText(t).equals(")")) { - // OK, close paren - } else { - throw parseError("expecting a close parentheses ')' here, not: " + t); + if (t != Tokens.CLOSE_ROUND) { + throw parseError("expecting the closing parentheses ')' of " + kindText + "() here, not: " + t); } - return new ConfigNodeInclude(children, kind); + + return new ConfigNodeInclude(children, kind, isRequired); } else if (Tokens.isValueWithType(t, ConfigValueType.STRING)) { children.add(new ConfigNodeSimpleValue(t)); - return new ConfigNodeInclude(children, ConfigIncludeKind.HEURISTIC); + return new ConfigNodeInclude(children, ConfigIncludeKind.HEURISTIC, isRequired); } else { throw parseError("include keyword is not followed by a quoted string, but by: " + t); } @@ -463,6 +509,8 @@ final class ConfigDocumentParser { // continue looping afterComma = true; } else { + // FIXME JL: Do ) balance check perhaps? + t = nextTokenCollectingWhitespace(objectNodes); if (t == Tokens.CLOSE_CURLY) { if (!hadOpenCurly) { diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigNodeInclude.java b/config/src/main/java/com/typesafe/config/impl/ConfigNodeInclude.java index 467c2b08..72f64109 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigNodeInclude.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigNodeInclude.java @@ -6,10 +6,12 @@ import java.util.Collection; final class ConfigNodeInclude extends AbstractConfigNode { final private ArrayList children; final private ConfigIncludeKind kind; + final private boolean isRequired; - ConfigNodeInclude(Collection children, ConfigIncludeKind kind) { + ConfigNodeInclude(Collection children, ConfigIncludeKind kind, boolean isRequired) { this.children = new ArrayList(children); this.kind = kind; + this.isRequired = isRequired; } final public Collection children() { @@ -29,6 +31,10 @@ final class ConfigNodeInclude extends AbstractConfigNode { return kind; } + protected boolean isRequired() { + return isRequired; + } + protected String name() { for (AbstractConfigNode n : children) { if (n instanceof ConfigNodeSimpleValue) { diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigParser.java b/config/src/main/java/com/typesafe/config/impl/ConfigParser.java index 8376a605..b2ce0455 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigParser.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigParser.java @@ -157,6 +157,9 @@ final class ConfigParser { } private void parseInclude(Map values, ConfigNodeInclude n) { + boolean isRequired = n.isRequired(); + ConfigIncludeContext cic = includeContext.setParseOptions(includeContext.parseOptions().setAllowMissing(!isRequired)); + AbstractConfigObject obj; switch (n.kind()) { case URL: @@ -166,21 +169,21 @@ final class ConfigParser { } catch (MalformedURLException e) { throw parseError("include url() specifies an invalid URL: " + n.name(), e); } - obj = (AbstractConfigObject) includer.includeURL(includeContext, url); + obj = (AbstractConfigObject) includer.includeURL(cic, url); break; case FILE: - obj = (AbstractConfigObject) includer.includeFile(includeContext, + obj = (AbstractConfigObject) includer.includeFile(cic, new File(n.name())); break; case CLASSPATH: - obj = (AbstractConfigObject) includer.includeResources(includeContext, n.name()); + obj = (AbstractConfigObject) includer.includeResources(cic, n.name()); break; case HEURISTIC: obj = (AbstractConfigObject) includer - .include(includeContext, n.name()); + .include(cic, n.name()); break; default: diff --git a/config/src/main/java/com/typesafe/config/impl/SimpleIncludeContext.java b/config/src/main/java/com/typesafe/config/impl/SimpleIncludeContext.java index 73ef3004..6111d6ac 100644 --- a/config/src/main/java/com/typesafe/config/impl/SimpleIncludeContext.java +++ b/config/src/main/java/com/typesafe/config/impl/SimpleIncludeContext.java @@ -10,9 +10,16 @@ import com.typesafe.config.ConfigParseable; class SimpleIncludeContext implements ConfigIncludeContext { private final Parseable parseable; + private final ConfigParseOptions options; SimpleIncludeContext(Parseable parseable) { this.parseable = parseable; + this.options = SimpleIncluder.clearForInclude(parseable.options()); + } + + private SimpleIncludeContext(Parseable parseable, ConfigParseOptions options) { + this.parseable = parseable; + this.options = options; } SimpleIncludeContext withParseable(Parseable parseable) { @@ -34,6 +41,11 @@ class SimpleIncludeContext implements ConfigIncludeContext { @Override public ConfigParseOptions parseOptions() { - return SimpleIncluder.clearForInclude(parseable.options()); + return options; + } + + @Override + public ConfigIncludeContext setParseOptions(ConfigParseOptions options) { + return new SimpleIncludeContext(parseable, options.setSyntax(null).setOriginDescription(null)); } } diff --git a/config/src/main/java/com/typesafe/config/impl/TokenType.java b/config/src/main/java/com/typesafe/config/impl/TokenType.java index c095c65d..77538aab 100644 --- a/config/src/main/java/com/typesafe/config/impl/TokenType.java +++ b/config/src/main/java/com/typesafe/config/impl/TokenType.java @@ -9,6 +9,8 @@ enum TokenType { COMMA, EQUALS, COLON, + OPEN_ROUND, + CLOSE_ROUND, OPEN_CURLY, CLOSE_CURLY, OPEN_SQUARE, diff --git a/config/src/main/java/com/typesafe/config/impl/Tokenizer.java b/config/src/main/java/com/typesafe/config/impl/Tokenizer.java index 0295ea58..5f54d351 100644 --- a/config/src/main/java/com/typesafe/config/impl/Tokenizer.java +++ b/config/src/main/java/com/typesafe/config/impl/Tokenizer.java @@ -299,7 +299,7 @@ final class Tokenizer { // chars JSON allows to be part of a number static final String numberChars = "0123456789eE+-."; // chars that stop an unquoted string - static final String notInUnquotedText = "$\"{}[]:=,+#`^?!@*&\\"; + static final String notInUnquotedText = "$\"{}()[]:=,+#`^?!@*&\\"; // The rules here are intended to maximize convenience while // avoiding confusion with real valid JSON. Basically anything @@ -481,7 +481,7 @@ final class Tokenizer { // the open quote has already been consumed StringBuilder sb = new StringBuilder(); - // We need a second string builder to keep track of escape characters. + // We need a sec ond string builder to keep track of escape characters. // We want to return them exactly as they appeared in the original text, // which means we will need a new StringBuilder to escape escape characters // so we can also keep the actual value of the string. This is gross. @@ -606,6 +606,12 @@ final class Tokenizer { case '=': t = Tokens.EQUALS; break; + case '(': + t = Tokens.OPEN_ROUND; + break; + case ')': + t = Tokens.CLOSE_ROUND; + break; case '{': t = Tokens.OPEN_CURLY; break; diff --git a/config/src/main/java/com/typesafe/config/impl/Tokens.java b/config/src/main/java/com/typesafe/config/impl/Tokens.java index ef2061c6..d521399f 100644 --- a/config/src/main/java/com/typesafe/config/impl/Tokens.java +++ b/config/src/main/java/com/typesafe/config/impl/Tokens.java @@ -449,6 +449,8 @@ final class Tokens { final static Token COMMA = Token.newWithoutOrigin(TokenType.COMMA, "','", ","); final static Token EQUALS = Token.newWithoutOrigin(TokenType.EQUALS, "'='", "="); final static Token COLON = Token.newWithoutOrigin(TokenType.COLON, "':'", ":"); + final static Token OPEN_ROUND = Token.newWithoutOrigin(TokenType.OPEN_ROUND, "'('", "("); + final static Token CLOSE_ROUND = Token.newWithoutOrigin(TokenType.CLOSE_ROUND, "')'", ")"); final static Token OPEN_CURLY = Token.newWithoutOrigin(TokenType.OPEN_CURLY, "'{'", "{"); final static Token CLOSE_CURLY = Token.newWithoutOrigin(TokenType.CLOSE_CURLY, "'}'", "}"); final static Token OPEN_SQUARE = Token.newWithoutOrigin(TokenType.OPEN_SQUARE, "'['", "["); diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala index 171778bb..b4c4ce68 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala @@ -5,10 +5,11 @@ package com.typesafe.config.impl import org.junit.Assert._ import org.junit._ -import java.io.StringReader +import java.io.{ File, IOException, StringReader } + import com.typesafe.config._ + import scala.collection.JavaConverters._ -import java.io.File import java.net.URL import java.util.Properties @@ -739,7 +740,7 @@ class ConfParserTest extends TestUtils { val e = intercept[ConfigException.Parse] { ConfigFactory.parseString("include file(" + jsonQuotedResourceFile("test01") + " something") } - assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close paren")) + assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting the closing parentheses")) } @Test @@ -779,6 +780,45 @@ class ConfParserTest extends TestUtils { assertEquals("abc", conf.getString("fromProps.abc")) } + @Test + def includeRequiredMissing() { + // set this to allowMissing=true to demonstrate that the missing inclusion causes failure despite this setting + val missing = ConfigParseOptions.defaults().setAllowMissing(true) + + val ex = intercept[Exception] { + ConfigFactory.parseString("include required(classpath( \"nonexistant\") )", missing) + } + + val actual = ex.getMessage + val expected = ".*resource not found on classpath.*" + assertTrue(s"expected match for <$expected> but got <$actual>", actual.matches(expected)) + } + + @Test + def includeRequiredFound() { + val confs = Seq( + "include required(\"test01\")", + "include required( \"test01\" )", + "include required( classpath(\"test01\") )", + "include required(classpath(\"test01\"))", + "include required( classpath(\"test01\"))", + "include required(classpath(\"test01\") )") + + // should have loaded conf, json, properties + confs.foreach { c => + try { + val conf = ConfigFactory.parseString(c) + assertEquals(42, conf.getInt("ints.fortyTwo")) + assertEquals(1, conf.getInt("fromJson1")) + assertEquals("abc", conf.getString("fromProps.abc")) + } catch { + case ex: Exception => + System.err.println(s"failed parsing: $c") + throw ex + } + } + } + @Test def includeURLHeuristically() { val url = resourceFile("test01.conf").toURI().toURL().toExternalForm() diff --git a/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala b/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala index a038b3ea..1bc39066 100644 --- a/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala +++ b/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala @@ -721,8 +721,10 @@ abstract trait TestUtils { val resourceDir = { val f = new File("config/src/test/resources") - if (!f.exists()) - throw new Exception("Tests must be run from the root project directory containing " + f.getPath()) + if (!f.exists()) { + val here = new File(".").getAbsolutePath + throw new Exception(s"Tests must be run from the root project directory containing ${f.getPath()}, however the current director is $here") + } f } diff --git a/config/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala b/config/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala index 7ce2b3b5..490866ef 100644 --- a/config/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala @@ -196,7 +196,7 @@ class TokenizerTest extends TestUtils { for (t <- invalidTests) { val tokenized = tokenizeAsList(t) val maybeProblem = tokenized.find(Tokens.isProblem(_)) - assertTrue(maybeProblem.isDefined) + assertTrue(s"expected failure for <$t> but got ${t}", maybeProblem.isDefined) } } @@ -305,4 +305,47 @@ class TokenizerTest extends TestUtils { assertEquals("" + invalid, Tokens.getProblemWhat(problem)) } } + + @Test + def tokenizeFunctionLikeUnquotedText() { + val source = """fn(TheURL)""" + val expected = List(tokenUnquoted("fn"), Tokens.OPEN_ROUND, tokenUnquoted("TheURL"), Tokens.CLOSE_ROUND) + tokenizerTest(expected, source) + } + + @Test + def tokenizeNestedFunctionLikeUnquotedText() { + val source = """fn1(fn2(TheURL))""" + val expected = List(tokenUnquoted("fn1"), Tokens.OPEN_ROUND, tokenUnquoted("fn2"), Tokens.OPEN_ROUND, tokenUnquoted("TheURL"), Tokens.CLOSE_ROUND, Tokens.CLOSE_ROUND) + tokenizerTest(expected, source) + } + + @Test + def tokenizeNestedFunctionLikeUnquotedTextWithWhitespace() { + val source = """fn1 ( fn2 ( TheURL ) ) """ + val expected = List( + tokenUnquoted("fn1"), + tokenWhitespace(" "), + Tokens.OPEN_ROUND, + tokenWhitespace(" "), + tokenUnquoted("fn2"), + tokenWhitespace(" "), + Tokens.OPEN_ROUND, + tokenWhitespace(" "), + tokenUnquoted("TheURL"), + tokenWhitespace(" "), + Tokens.CLOSE_ROUND, + tokenWhitespace(" "), + Tokens.CLOSE_ROUND, + tokenWhitespace(" ")) + + tokenizerTest(expected, source) + } + + @Test + def tokenizeFunctionLikeUnquotedTextWithNestedString() { + val source = """fn("TheURL")""" + val expected = List(tokenUnquoted("fn"), Tokens.OPEN_ROUND, tokenString("TheURL"), Tokens.CLOSE_ROUND) + tokenizerTest(expected, source) + } }