diff options
author | gjoranv <gv@oath.com> | 2017-08-16 17:38:03 +0200 |
---|---|---|
committer | gjoranv <gv@oath.com> | 2017-08-17 12:00:02 +0200 |
commit | 2b2d0ea6280b54ad37f85b091e6a46b1566d430b (patch) | |
tree | 743f3d1d8f2970a93ff94036088f81d9d0619450 /configgen | |
parent | 380098c56a4d6be4762cd060734896fbe450cca7 (diff) |
Allow 'package' in def files to specify java package.
* For generating cpp, namespace is still required and
package is ignored.
* Generating java requires one of them, but both are allowed.
Diffstat (limited to 'configgen')
6 files changed, 180 insertions, 18 deletions
diff --git a/configgen/src/main/java/com/yahoo/config/codegen/CNode.java b/configgen/src/main/java/com/yahoo/config/codegen/CNode.java index 260d70ad0e6..9dc7923b71b 100644 --- a/configgen/src/main/java/com/yahoo/config/codegen/CNode.java +++ b/configgen/src/main/java/com/yahoo/config/codegen/CNode.java @@ -3,10 +3,12 @@ package com.yahoo.config.codegen; import java.util.StringTokenizer; +import static com.yahoo.config.codegen.DefParser.DEFAULT_PACKAGE_PREFIX; + /** * Abstract superclass for all nodes representing a definition file. * - * @author <a href="gv@yahoo-inc.com">G. Voldengen</a> + * @author gjoranv */ public abstract class CNode { @@ -20,8 +22,10 @@ public abstract class CNode { // TODO: remove! Only set for the root node, and root.getName() returns the same thing! String defName = null; + String defVersion = ""; String defNamespace = null; + String defPackage = null; String defMd5 = "MISSING MD5"; String comment = ""; @@ -78,13 +82,21 @@ public abstract class CNode { } public String getNamespace() { - return defNamespace; + if (defNamespace != null) return defNamespace; + if (defPackage != null) return defPackage; + return null; } void setNamespace(String namespace) { defNamespace = namespace; } + public String getPackage() { + return defPackage; + } + + void setPackage(String defPackage) { this.defPackage = defPackage; } + public String getComment() { return comment; } @@ -155,7 +167,11 @@ public abstract class CNode { @Override public String toString() { - return getNamespace()+"."+getName()+","+getVersion(); + return "CNode{" + + "namespace='" + defNamespace + '\'' + + ", package='" + defPackage + '\'' + + ", name='" + name + '\'' + + ", version='" + defVersion + '\'' + + '}'; } - } diff --git a/configgen/src/main/java/com/yahoo/config/codegen/DefParser.java b/configgen/src/main/java/com/yahoo/config/codegen/DefParser.java index 3fecc7e108e..09a751bdde7 100644 --- a/configgen/src/main/java/com/yahoo/config/codegen/DefParser.java +++ b/configgen/src/main/java/com/yahoo/config/codegen/DefParser.java @@ -18,8 +18,13 @@ public class DefParser { static final Pattern commentPattern = Pattern.compile("^\\s*#+\\s*(.*?)\\s*$"); public static final Pattern versionPattern = Pattern.compile("^(version\\s*=\\s*)([0-9][0-9-]*)$"); - // Namespace must start with a letter, since Java (Java language Spec, section 3.8) and C++ identifiers cannot start with a digit - public static final Pattern namespacePattern = Pattern.compile("^(namespace\\s*=\\s*)(([a-z][a-z0-9_]*)+([.][a-z][a-z0-9_]*)*)$"); + // Namespace/package must start with a letter, since Java (Java language Spec, section 3.8) and C++ identifiers cannot start with a digit + public static final Pattern namespacePattern = getNamespacePattern("namespace"); + public static final Pattern packagePattern = getNamespacePattern("package"); + + private static Pattern getNamespacePattern(String directive) { + return Pattern.compile("^(" + directive + "\\s*=\\s*)(([a-z][a-z0-9_]*)+([.][a-z][a-z0-9_]*)*)$"); + } private final BufferedReader reader; private final String name; @@ -129,6 +134,12 @@ public class DefParser { nd.addNormalizedLine(line); return; } + Matcher packageMatcher = packagePattern.matcher(line); + if (packageMatcher.matches()) { + parsePackageLine(packageMatcher.group(2)); + nd.addNormalizedLine(line); + return; + } // Only add lines that are not version, namespace or comment lines nd.addNormalizedLine(line); DefLine defLine = new DefLine(line); @@ -157,6 +168,12 @@ public class DefParser { comment = ""; } + private void parsePackageLine(String defPackage) { + root.setPackage(defPackage); + root.setComment(comment); + comment = ""; + } + void parseLines(CNode root, List<String> defLines, NormalizedDefinition nd) throws DefParserException { DefParserException failure = null; int lineNumber = 1; diff --git a/configgen/src/main/java/com/yahoo/config/codegen/MakeConfig.java b/configgen/src/main/java/com/yahoo/config/codegen/MakeConfig.java index 91d401c91b7..ef9af1c2b11 100644 --- a/configgen/src/main/java/com/yahoo/config/codegen/MakeConfig.java +++ b/configgen/src/main/java/com/yahoo/config/codegen/MakeConfig.java @@ -18,11 +18,11 @@ public class MakeConfig { classBuilder = createClassBuilder(root, nd, properties); } - public static ClassBuilder createClassBuilder(InnerCNode root, NormalizedDefinition nd, MakeConfigProperties prop) { - if (prop.language.equals("cppng") || prop.language.equals("cpp")) - return new CppClassBuilder(root, nd, prop.destDir, prop.dirInRoot); + public static ClassBuilder createClassBuilder(InnerCNode root, NormalizedDefinition nd, MakeConfigProperties properties) { + if (isCpp(properties)) + return new CppClassBuilder(root, nd, properties.destDir, properties.dirInRoot); else - return new JavaClassBuilder(root, nd, prop.destDir, prop.javaPackagePrefix); + return new JavaClassBuilder(root, nd, properties.destDir, properties.javaPackagePrefix); } public static boolean makeConfig(MakeConfigProperties properties) throws FileNotFoundException { @@ -31,7 +31,7 @@ public class MakeConfig { if (name.endsWith(".def")) name = name.substring(0, name.length() - 4); DefParser parser = new DefParser(name, new FileReader(specFile)); InnerCNode configRoot = parser.getTree(); - checkNamespace(name, configRoot); + checkNamespaceAndPacakge(name, configRoot, isCpp(properties)); if (configRoot != null) { MakeConfig mc = new MakeConfig(configRoot, parser.getNormalizedDefinition(), properties); mc.buildClasses(); @@ -73,9 +73,15 @@ public class MakeConfig { } } - private static void checkNamespace(String name, InnerCNode configRoot) { - if (configRoot.defNamespace == null) + private static void checkNamespaceAndPacakge(String name, InnerCNode configRoot, boolean isCpp) { + if (isCpp && configRoot.defNamespace == null) throw new IllegalArgumentException("In config definition '" + name + "': A namespace is required"); + if (configRoot.defNamespace == null && configRoot.defPackage == null) + throw new IllegalArgumentException("In config definition '" + name + "': A package (or namespace) is required"); + } + + private static boolean isCpp(MakeConfigProperties properties) { + return (properties.language.equals("cppng") || properties.language.equals("cpp")); } // The Exceptions class below is copied from vespajlib/com.yahoo.protect.Exceptions diff --git a/configgen/src/main/scala/com/yahoo/config/codegen/JavaClassBuilder.scala b/configgen/src/main/scala/com/yahoo/config/codegen/JavaClassBuilder.scala index 51065eba03e..518815d5a10 100644 --- a/configgen/src/main/scala/com/yahoo/config/codegen/JavaClassBuilder.scala +++ b/configgen/src/main/scala/com/yahoo/config/codegen/JavaClassBuilder.scala @@ -24,12 +24,12 @@ class JavaClassBuilder( import JavaClassBuilder._ val packagePrefix = if (rawPackagePrefix != null) rawPackagePrefix else DEFAULT_PACKAGE_PREFIX - val javaPackage = packagePrefix + root.getNamespace + val javaPackage = if (root.getPackage != null) root.getPackage else packagePrefix + root.getNamespace val className = createClassName(root.getName) override def createConfigClasses() { try { - val outFile = new File(getDestPath(destDir, root.getNamespace), className + ".java") + val outFile = new File(getDestPath(destDir, javaPackage), className + ".java") var out: PrintStream = null try { out = new PrintStream(new FileOutputStream(outFile)) @@ -127,12 +127,12 @@ class JavaClassBuilder( /** * @param rootDir The root directory for the destination path. - * @param namespace The namespace from the def file + * @param javaPackage The java package * @return the destination path for the generated config file, including the given rootDir. */ - private def getDestPath(rootDir: File, namespace: String): File = { + private def getDestPath(rootDir: File, javaPackage: String): File = { var dir: File = rootDir - val subDirs: Array[String] = (packagePrefix + namespace).split("""\.""") + val subDirs: Array[String] = javaPackage.split("""\.""") for (subDir <- subDirs) { dir = new File(dir, subDir) this.synchronized { diff --git a/configgen/src/test/java/com/yahoo/config/codegen/DefParserNamespaceTest.java b/configgen/src/test/java/com/yahoo/config/codegen/DefParserNamespaceTest.java index 0fef22c0d22..bab09f36ad0 100644 --- a/configgen/src/test/java/com/yahoo/config/codegen/DefParserNamespaceTest.java +++ b/configgen/src/test/java/com/yahoo/config/codegen/DefParserNamespaceTest.java @@ -24,6 +24,14 @@ public class DefParserNamespaceTest { assertThat(root.getNamespace(), is("myproject.config")); } + @Test + public void package_is_used_as_namespace_when_no_namespace_is_given() { + String PACKAGE = "com.github.myproject"; + DefParser parser = createParser("package=" + PACKAGE + "\n"); + CNode root = parser.getTree(); + assertThat(root.getNamespace(), is(PACKAGE)); + } + @Test(expected = CodegenRuntimeException.class) public void uppercase_chars_are_not_allowed() { createParser("version=1\nnamespace=Foo\na string\n").getTree(); diff --git a/configgen/src/test/java/com/yahoo/config/codegen/DefParserPackageTest.java b/configgen/src/test/java/com/yahoo/config/codegen/DefParserPackageTest.java new file mode 100644 index 00000000000..97986a05bf3 --- /dev/null +++ b/configgen/src/test/java/com/yahoo/config/codegen/DefParserPackageTest.java @@ -0,0 +1,115 @@ +package com.yahoo.config.codegen; + +import org.junit.Test; + +import java.io.IOException; + +import static com.yahoo.config.codegen.DefParser.DEFAULT_PACKAGE_PREFIX; +import static com.yahoo.config.codegen.DefParserTest.assertLineFails; +import static com.yahoo.config.codegen.DefParserTest.createDefTemplate; +import static com.yahoo.config.codegen.DefParserTest.createParser; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.Assert.assertThat; + +/** + * Tests setting explicit java package in the def file. + * + * @author gjoranv + */ +public class DefParserPackageTest { + String PACKAGE = "com.github.myproject"; + + @Test + public void package_is_set_on_root_node() { + DefParser parser = createParser("package=" + PACKAGE + "\n"); + CNode root = parser.getTree(); + assertThat(root.getPackage(), is(PACKAGE)); + } + + @Test + public void package_and_namespace_can_coexist() { + String namespace = "test.namespace"; + DefParser parser = createParser("package=" + PACKAGE + + "\nnamespace=" + namespace +"\n"); + CNode root = parser.getTree(); + assertThat(root.getPackage(), is(PACKAGE)); + assertThat(root.getNamespace(), is(namespace)); + } + + // Required by JavaClassBuilder ctor. + @Test + public void package_is_null_when_not_explicitly_given() { + String namespace = "test.namespace"; + DefParser parser = createParser("namespace=" + namespace + "\n"); + CNode root = parser.getTree(); + assertThat(root.getPackage(), nullValue()); + } + + @Test(expected = CodegenRuntimeException.class) + public void uppercase_chars_are_not_allowed() { + createParser("package=Foo.bar\n").getTree(); + } + + @Test + public void spaces_are_allowed_around_equals_sign() { + DefParser parser = createParser("package = " + PACKAGE + "\n"); + CNode root = parser.getTree(); + assertThat(root.getPackage(), is(PACKAGE)); + } + + @Test + public void empty_package_is_not_allowed() { + assertLineFails("package"); + } + + @Test + public void consecutive_dots_are_not_allowed() { + assertLineFails("package=a..b"); + } + + @Test + public void package_alters_def_md5() { + DefParser parser = createParser("a string\n"); + CNode root = parser.getTree(); + + parser = createParser("package=" + PACKAGE + "\na string\n"); + CNode rootWithPackage = parser.getTree(); + + assertThat(root.defMd5, not(rootWithPackage.defMd5)); + } + + + @Test + public void number_is_allowed_as_non_leading_char() throws IOException, DefParser.DefParserException { + StringBuilder sb = createDefTemplate(); + String line = "package=a.b.c2\n"; + sb.append(line); + createParser(sb.toString()).parse(); + } + + @Test + public void number_is_not_allowed_as_package_start_char() throws IOException, DefParser.DefParserException { + assertLineFails("package=2.a.b"); + } + + @Test + public void number_is_not_allowed_as_leading_char_in_package_token() throws IOException, DefParser.DefParserException { + assertLineFails("package=a.b.2c"); + } + + @Test + public void underscore_in_package_is_allowed() throws IOException, DefParser.DefParserException { + StringBuilder sb = createDefTemplate(); + String line = "package=a_b.c\n"; + sb.append(line); + createParser(sb.toString()).parse(); + + sb = createDefTemplate(); + line = "package=a_b.c_d\n"; + sb.append(line); + createParser(sb.toString()).parse(); + } + +} |