Permalink
Browse files

implemented "required include" along with code review comments from p…

…revious attempt at pull request
  • Loading branch information...
1 parent 596d6c9 commit f687a8fea3f6b8832ad3775ee13a0be8c9f5ac7b @Johnlon Johnlon committed Aug 27, 2016
View
@@ -906,6 +906,7 @@ followed by whitespace and then either:
- `url()`, `file()`, or `classpath()` surrounding a quoted string
which is then interpreted as a URL, file, or classpath. The
string must be quoted, unlike in CSS.
+ - `required()` surrounding one of the above
An include statement can appear in place of an object field.
@@ -1031,12 +1032,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
@@ -1494,7 +1507,9 @@ environment variables generally are capitalized. This avoids
naming collisions between environment variables and configuration
properties. (While on Windows getenv() is generally not
case-sensitive, the lookup will be case sensitive all the way
-until the env variable fallback lookup is reached.)
+until the env variable fallback lookup is reached).
+
+See also the notes below on Windows and case sensitivity.
An application can explicitly block looking up a substitution in
the environment by setting a value in the configuration, with the
@@ -1543,3 +1558,47 @@ Differences include but are probably not limited to:
properties files only recognize comment characters if they
occur as the first character on the line
- HOCON interprets `${}` as a substitution
+
+## Note on Windows and case sensitivity of environment variables
+
+HOCON's lookup of environment variable values is always case sensitive, but
+Linux and Windows differ in their handling of case.
+
+Linux allows one to define multiple environment variables with the same
+name but with different case; so both "PATH" and "Path" may be defined
+simultaneously. HOCON's access to these environment variables on Linux
+is straightforward; ie just make sure you define all your vars with the required case.
+
+Windows is more confusing. Windows environment variables names may contain a
+mix of upper and lowercase characters, eg "Path", however Windows does not
+allow one to define multiple instances of the same name but differing in case.
+Whilst accessing env vars in Windows is case insensitive, accessing env vars in
+HOCON is case sensitive.
+So if you know that you HOCON needs "PATH" then you must ensure that
+the variable is defined as "PATH" rather than some other name such as
+"Path" or "path".
+However, Windows does not allow us to change the case of an existing env var; we can't
+simply redefine the var with an upper case name.
+The only way to ensure that your environment variables have the desired case
+is to first undefine all the env vars that you will depend on then redefine
+them with the required case.
+
+For example, the the ambient environment might have this definition ...
+
+```
+set Path=A;B;C
+```
+.. we just don't know. But if the HOCON needs "PATH", then the start script must
+take a precautionary approach and enforce the necessary case as follows ...
+
+```
+set OLDPATH=%PATH%
+set PATH=
+set PATH=%OLDPATH%
+
+%JAVA_HOME%/bin/java ....
+```
+
+You cannot know what ambient environment variables might exist in the ambient environment
+when your program is invoked, nor what case those definitions might have.
+Therefore the only safe thing to do is redefine all the vars you rely on as shown above.
View
@@ -860,4 +860,4 @@ format.
#### Linting tool
- * A web based linting tool http://www.hoconlint.com/
+ * A web based linting tool http://www.hoconlint.com/
@@ -43,4 +43,12 @@
* @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);
}
@@ -111,7 +111,8 @@ ConfigParseOptions withFallbackOriginDescription(String originDescription) {
/**
* 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
@@ -275,8 +275,7 @@ private ConfigNodePath parseKey(Token token) {
}
if (expression.isEmpty()) {
- throw parseError("expecting a close brace or a field name here, got "
- + t);
+ throw parseError(ExpectingClosingParenthesisError + t);
}
putBack(t); // put back the token we ended with
@@ -311,25 +310,74 @@ private boolean isKeyValueSeparatorToken(Token t) {
}
}
+ private final String ExpectingClosingParenthesisError = "expecting a close parentheses ')' here, not: ";
+
private ConfigNodeInclude parseInclude(ArrayList<AbstractConfigNode> children) {
+
+ Token t = nextTokenCollectingWhitespace(children);
+
+ // we either have a 'required()' or a quoted string or the "file()" syntax
+ if (Tokens.isUnquotedText(t)) {
+ String kindText = Tokens.getUnquotedText(t);
+
+ if (kindText.startsWith("required(")) {
+ String r = kindText.replaceFirst("required\\(","");
+ if (r.length()>0) {
+ putBack(Tokens.newUnquotedText(t.origin(),r));
+ }
+
+ children.add(new ConfigNodeSingleToken(t));
+ //children.add(new ConfigNodeSingleToken(tOpen));
+
+ ConfigNodeInclude res = parseIncludeResource(children, true);
+
+ t = nextTokenCollectingWhitespace(children);
+
+ if (Tokens.isUnquotedText(t) && Tokens.getUnquotedText(t).equals(")")) {
+ // OK, close paren
+ } else {
+ throw parseError(ExpectingClosingParenthesisError + t);
+ }
+
+ return res;
+ } else {
+ putBack(t);
+ return parseIncludeResource(children, false);
+ }
+ }
+ else {
+ putBack(t);
+ return parseIncludeResource(children, false);
+ }
+ }
+
+ private ConfigNodeInclude parseIncludeResource(ArrayList<AbstractConfigNode> children, boolean isRequired) {
Token t = nextTokenCollectingWhitespace(children);
// we either have a quoted string or the "file()" syntax
if (Tokens.isUnquotedText(t)) {
// get foo(
String kindText = Tokens.getUnquotedText(t);
ConfigIncludeKind kind;
+ String prefix;
- if (kindText.equals("url(")) {
+ if (kindText.startsWith("url(")) {
kind = ConfigIncludeKind.URL;
- } else if (kindText.equals("file(")) {
+ prefix = "url(";
+ } else if (kindText.startsWith("file(")) {
kind = ConfigIncludeKind.FILE;
- } else if (kindText.equals("classpath(")) {
+ prefix = "file(";
+ } else if (kindText.startsWith("classpath(")) {
kind = ConfigIncludeKind.CLASSPATH;
+ prefix = "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: "
+ t);
}
+ String r = kindText.replaceFirst("[^(]*\\(","");
+ if (r.length()>0) {
+ putBack(Tokens.newUnquotedText(t.origin(),r));
+ }
children.add(new ConfigNodeSingleToken(t));
@@ -338,23 +386,26 @@ private ConfigNodeInclude parseInclude(ArrayList<AbstractConfigNode> children) {
// quoted string
if (!Tokens.isValueWithType(t, ConfigValueType.STRING)) {
- throw parseError("expecting a quoted string inside file(), classpath(), or url(), rather than: "
- + t);
+ throw parseError("expecting include " + prefix + ") parameter to be a quoted string, rather than: " + t);
}
children.add(new ConfigNodeSimpleValue(t));
// skip space after string, inside parens
t = nextTokenCollectingWhitespace(children);
- if (Tokens.isUnquotedText(t) && Tokens.getUnquotedText(t).equals(")")) {
+ if (Tokens.isUnquotedText(t) && Tokens.getUnquotedText(t).startsWith(")")) {
+ String rest = Tokens.getUnquotedText(t).substring(1);
+ if (rest.length()>0) {
+ putBack(Tokens.newUnquotedText(t.origin(),rest));
+ }
// OK, close paren
} else {
- throw parseError("expecting a close parentheses ')' here, not: " + t);
+ throw parseError(ExpectingClosingParenthesisError + 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);
}
@@ -6,10 +6,12 @@
final class ConfigNodeInclude extends AbstractConfigNode {
final private ArrayList<AbstractConfigNode> children;
final private ConfigIncludeKind kind;
+ final private boolean isRequired;
- ConfigNodeInclude(Collection<AbstractConfigNode> children, ConfigIncludeKind kind) {
+ ConfigNodeInclude(Collection<AbstractConfigNode> children, ConfigIncludeKind kind, boolean isRequired) {
this.children = new ArrayList<AbstractConfigNode>(children);
this.kind = kind;
+ this.isRequired = isRequired;
}
final public Collection<AbstractConfigNode> children() {
@@ -29,6 +31,10 @@ protected ConfigIncludeKind kind() {
return kind;
}
+ protected boolean isRequired() {
+ return isRequired;
+ }
+
protected String name() {
for (AbstractConfigNode n : children) {
if (n instanceof ConfigNodeSimpleValue) {
@@ -157,6 +157,9 @@ private static AbstractConfigObject createValueUnderPath(Path path,
}
private void parseInclude(Map<String, AbstractConfigValue> 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 @@ private void parseInclude(Map<String, AbstractConfigValue> values, ConfigNodeInc
} 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:
@@ -10,9 +10,16 @@
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 @@ public ConfigParseable relativeTo(String filename) {
@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));
}
}
@@ -235,15 +235,15 @@ class ConcatenationTest extends TestUtils {
val e = intercept[ConfigException.Parse] {
parseConfig("""{ { a : 1 } : "value" }""")
}
- assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close") && e.getMessage.contains("'{'"))
+ assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close parentheses") && e.getMessage.contains("'{'"))
}
@Test
def arraysAreNotKeys() {
val e = intercept[ConfigException.Parse] {
parseConfig("""{ [ "a" ] : "value" }""")
}
- assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close") && e.getMessage.contains("'['"))
+ assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("expecting a close parentheses") && e.getMessage.contains("'['"))
}
@Test
Oops, something went wrong.

0 comments on commit f687a8f

Please sign in to comment.