Merge pull request #3171 from vespa-engine/gjoranv/support-def-package-in-bundle-model

Gjoranv/support def package in bundle model

5 files changed, 82 insertions, 44 deletions

diff --git a/config-model/src/test/cfg/application/app1/components/defs-only.jar b/config-model/src/test/cfg/application/app1/components/defs-only.jar
index c0cf0397c97..88a51ff82f7 100644
--- a/config-model/src/test/cfg/application/app1/components/defs-only.jar
+++ b/config-model/src/test/cfg/application/app1/components/defs-only.jar
diff --git a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java
index bc84e109d7e..ee2e6ffcc74 100644
--- a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java
@@ -93,15 +93,17 @@ public class ApplicationDeployTest {
 
         List<FilesApplicationPackage.Component> components = app.getComponents();
         assertEquals(1, components.size());
-        Map<String, Bundle.DefEntry> defEntries =
+        Map<String, Bundle.DefEntry> defEntriesByName =
                 defEntries2map(components.get(0).getDefEntries());
-        assertEquals(2, defEntries.size());
-        System.out.println(defEntries);
-        Bundle.DefEntry def1 = defEntries.get("test1");
-        Bundle.DefEntry def2 = defEntries.get("test2");
+        assertEquals(5, defEntriesByName.size());
+        System.out.println(defEntriesByName);
+
+        Bundle.DefEntry def1 = defEntriesByName.get("test-namespace");
         assertNotNull(def1);
-        assertNotNull(def2);
         assertEquals("namespace=config\nintVal int default=0", def1.contents);
+
+        Bundle.DefEntry def2 = defEntriesByName.get("namespace-in-filename");
+        assertNotNull(def2);
         assertEquals("namespace=a.b\n\ndoubleVal double default=0.0", def2.contents);
 
         // Check that getFilename works
@@ -280,7 +282,7 @@ public class ApplicationDeployTest {
         String appName = "src/test/cfg//application/app1";
         FilesApplicationPackage app = FilesApplicationPackage.fromFile(new File(appName), false);
         Map<ConfigDefinitionKey, UnparsedConfigDefinition> defs = app.getAllExistingConfigDefs();
-        assertThat(defs.size(), is(2));
+        assertThat(defs.size(), is(5));
     }
 
     @Test
diff --git a/config-model/src/test/java/com/yahoo/config/model/ConfigModelUtilsTest.java b/config-model/src/test/java/com/yahoo/config/model/ConfigModelUtilsTest.java
index 0b9efd06db0..883e63a9c7e 100644
--- a/config-model/src/test/java/com/yahoo/config/model/ConfigModelUtilsTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/ConfigModelUtilsTest.java
@@ -17,40 +17,60 @@ import static org.junit.Assert.assertThat;
  */
 public class ConfigModelUtilsTest {
 
-    /**
-     * Tests that a def file both with and without namespace in file name are handled, and that
-     * def files in other directories than 'configdefinitions/' within the jar file are ignored.
-     */
+    public static final String VALID_TEST_BUNDLE = "src/test/cfg/application/app1/components/";
+    public static final String INVALID_TEST_BUNDLE = "src/test/cfg/application/validation/invalidjar_app/components";
+
     @Test
-    public void testDefFilesInBundle() {
-        List<Bundle> bundles = Bundle.getBundles(new File("src/test/cfg/application/app1/components/"));
+    public void all_def_files_in_correct_directory_are_handled_and_files_outside_are_ignored() {
+        List<Bundle> bundles = Bundle.getBundles(new File(VALID_TEST_BUNDLE));
         assertThat(bundles.size(), is(1));
-        Bundle bundle = bundles.get(0);
-        assertThat(bundle.getDefEntries().size(), is(2));
-
-        Bundle.DefEntry defEntry1 = bundle.getDefEntries().get(0);
-        Bundle.DefEntry defEntry2;
-        List<Bundle.DefEntry> defEntries = bundle.getDefEntries();
-        if (defEntry1.defName.equals("test1")) {
-            defEntry2 = defEntries.get(1);
-        } else {
-            defEntry1 = defEntries.get(1);
-            defEntry2 = defEntries.get(0);
-        }
-        assertThat(defEntry1.defName, is("test1"));
-        assertThat(defEntry1.defNamespace, is("config"));
+        assertThat(bundles.get(0).getDefEntries().size(), is(5));
+    }
+
+    @Test
+    public void def_file_with_namespace_is_handled() {
+        Bundle.DefEntry defEntry = getDefEntry("test-namespace");
+        assertThat(defEntry.defNamespace, is("config"));
+    }
+
+    @Test
+    public void def_file_with_namespace_and_namespace_in_filename_is_handled() {
+        Bundle.DefEntry defEntry = getDefEntry("namespace-in-filename");
+        assertThat(defEntry.defNamespace, is("a.b"));
+    }
+
+    @Test
+    public void def_file_with_package_is_handled() {
+        Bundle.DefEntry defEntry = getDefEntry("test-package");
+        assertThat(defEntry.defNamespace, is("com.mydomain.mypackage"));
+    }
+
+    @Test
+    public void def_file_with_package_and_pacakage_in_filename_is_handled() {
+        Bundle.DefEntry defEntry = getDefEntry("package-in-filename");
+        assertThat(defEntry.defNamespace, is("com.mydomain.mypackage"));
+    }
 
-        assertThat(defEntry2.defName, is("test2"));
-        assertThat(defEntry2.defNamespace, is("a.b"));
+    @Test
+    public void def_file_with_both_package_and_namespace_gets_package_as_namespace() {
+        Bundle.DefEntry defEntry = getDefEntry("namespace-and-package");
+        assertThat(defEntry.defNamespace, is("com.mydomain.mypackage"));
+    }
+
+    private static Bundle.DefEntry getDefEntry(String defName) {
+        Bundle bundle = Bundle.getBundles(new File(VALID_TEST_BUNDLE)).get(0);
+
+        for (Bundle.DefEntry defEntry : bundle.getDefEntries()) {
+            if (defEntry.defName.equals(defName))
+                return defEntry;
+        }
+        throw new IllegalArgumentException("No def file with name '" + defName + "' found in the test bundle.");
     }
 
-    /**
-     * Tests that an invalid jar is identified as not being a jar file
-     */
     @Test
-    public void testInvalidJar() {
+    public void invalid_jar_file_fails_to_load() {
         try {
-            Bundle.getBundles(new File("src/test/cfg/application/validation/invalidjar_app/components"));
+            Bundle.getBundles(new File(INVALID_TEST_BUNDLE));
             fail();
         } catch (IllegalArgumentException e) {
             assertThat(e.getMessage(), is("Error opening jar file 'invalid.jar'. Please check that this is a valid jar file"));
diff --git a/config-model/src/test/java/com/yahoo/config/model/deploy/DeployStateTest.java b/config-model/src/test/java/com/yahoo/config/model/deploy/DeployStateTest.java
index e64f45831ff..5896cdfa495 100644
--- a/config-model/src/test/java/com/yahoo/config/model/deploy/DeployStateTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/deploy/DeployStateTest.java
@@ -81,12 +81,12 @@ public class DeployStateTest {
         Map<ConfigDefinitionKey, com.yahoo.vespa.config.buildergen.ConfigDefinition> defs = new LinkedHashMap<>();
         defs.put(new ConfigDefinitionKey("foo", "bar"), new com.yahoo.vespa.config.buildergen.ConfigDefinition("foo", new String[]{"namespace=bar", "foo int default=0"}));
         defs.put(new ConfigDefinitionKey("test2", "a.b"),
-                 new com.yahoo.vespa.config.buildergen.ConfigDefinition("test2", new String[]{"namespace=a.b", "doubleVal double default=1.0"}));
+                 new com.yahoo.vespa.config.buildergen.ConfigDefinition("namespace-in-filename", new String[]{"namespace=a.b", "doubleVal double default=1.0"}));
         ApplicationPackage app = FilesApplicationPackage.fromFile(new File("src/test/cfg//application/app1"));
         DeployState state = createDeployState(app, defs);
 
         assertNotNull(state.getConfigDefinition(new ConfigDefinitionKey("foo", "bar")));
-        ConfigDefinition overridden = state.getConfigDefinition(new ConfigDefinitionKey("test2", "a.b")).get();
+        ConfigDefinition overridden = state.getConfigDefinition(new ConfigDefinitionKey("namespace-in-filename", "a.b")).get();
         assertNotNull(overridden);
         Double defaultValue = overridden.getDoubleDefs().get("doubleVal").getDefVal();
         assertNotNull(defaultValue);
diff --git a/config/src/main/java/com/yahoo/vespa/config/util/ConfigUtils.java b/config/src/main/java/com/yahoo/vespa/config/util/ConfigUtils.java
index ccb8993439e..0944569a413 100644
--- a/config/src/main/java/com/yahoo/vespa/config/util/ConfigUtils.java
+++ b/config/src/main/java/com/yahoo/vespa/config/util/ConfigUtils.java
@@ -208,14 +208,18 @@ public class ConfigUtils {
     }
 
     /**
-     * Finds the def namespace from a reader for a def-file. Returns "" (empty string)
-     * if no namespace was found.
+     * Finds the def package or namespace from a reader for a def-file. Returns "" (empty string)
+     * if no package or namespace was found. If both package and namespace are declared in the def
+     * file, the package is returned.
      *
      * @param in A reader to a def-file
      * @return namespace of the def-file, or "" (empty string) if no namespace was found
      */
     public static String getDefNamespace(Reader in) {
-        return getDefKeyword(in, "namespace");
+        List<String> defLines = getDefLines(in);
+        String defPackage = getDefKeyword(defLines, "package");
+        if (! defPackage.isEmpty()) return defPackage;
+        return getDefKeyword(defLines, "namespace");
     }
 
     /**
@@ -226,9 +230,24 @@ public class ConfigUtils {
      * @return value of keyword, or "" (empty string) if no line matching keyword was found
      */
     public static String getDefKeyword(Reader in, String keyword) {
+        return getDefKeyword(getDefLines(in), keyword);
+    }
+
+    private static String getDefKeyword(List<String> defLines, String keyword) {
+        for (String line : defLines) {
+            if (line.startsWith(keyword)) {
+                String[] v = line.split("=");
+                return v[1].trim();
+            }
+        }
+        return "";
+    }
+
+    private static List<String> getDefLines(Reader in) {
         if (null == in) {
             throw new IllegalArgumentException("Null reader.");
         }
+        List<String> defLines = new ArrayList<>();
         LineNumberReader reader;
         try {
             if (in instanceof LineNumberReader) {
@@ -240,17 +259,14 @@ public class ConfigUtils {
             while ((line = reader.readLine()) != null) {
                 line = line.trim();
                 if (!line.startsWith("#") && !line.equals("")) {
-                    if (line.startsWith(keyword)) {
-                        String[] v = line.split("=");
-                        return v[1].trim();
-                    }
+                    defLines.add(line);
                 }
             }
             reader.close();
         } catch (IOException e) {
             throw new RuntimeException("IOException", e);
         }
-        return "";
+        return defLines;
     }
 
     /**