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.

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();
+    }
+
+}