Cleanup tests

4 files changed, 90 insertions, 98 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/embedder/EmbedderConfig.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/embedder/EmbedderConfig.java
index 69343643ef3..3ebd9e0483d 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/embedder/EmbedderConfig.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/embedder/EmbedderConfig.java
@@ -38,7 +38,7 @@ public class EmbedderConfig {
         if ( ! explicitDefinition.isEmpty()) return explicitDefinition;
 
         // Implicit from class name
-        return switch (spec.getAttribute("class")) {
+        return switch (getEmbedderClass(spec)) {
             case "ai.vespa.embedding.BertBaseEmbedder" -> "embedding.bert-base-embedder";
             default -> "";
         };
@@ -82,7 +82,7 @@ public class EmbedderConfig {
         if (spec.hasAttribute("id")) {
             return spec.getAttribute("id");
         }
-        throw new IllegalArgumentException("Embedder specification does not have a required class attribute");
+        throw new IllegalArgumentException("An <embedder> element must have a 'class' or 'id' attribute");
     }
 
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/embedder/EmbedderConfigBertBaseTransformer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/embedder/EmbedderConfigBertBaseTransformer.java
index 9431926d088..59043e0ef3e 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/embedder/EmbedderConfigBertBaseTransformer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/embedder/EmbedderConfigBertBaseTransformer.java
@@ -28,7 +28,7 @@ public class EmbedderConfigBertBaseTransformer extends EmbedderConfigTransformer
                 .optionTransformer(new EmbedderOption.ModelOptionTransformer("tokenizerVocabPath", "tokenizerVocabUrl"));
 
         // Defaults
-        if (hosted) {
+        if (hosted && 1==2) {
             modelOption.attributes(Map.of("id", "minilm-l6-v2")).value("");
             vocabOption.attributes(Map.of("id", "bert-base-uncased")).value("");
         }
diff --git a/config-model/src/test/cfg/application/embed_generic_using_provided_model/services.xml b/config-model/src/test/cfg/application/embed_generic_using_provided_model/services.xml
deleted file mode 100644
index 4190e5c9286..00000000000
--- a/config-model/src/test/cfg/application/embed_generic_using_provided_model/services.xml
+++ /dev/null
@@ -1,20 +0,0 @@
-<?xml version="1.0" encoding="utf-8" ?>
-<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -->
-<services version="1.0">
-
-  <container version="1.0">
-
-    <embedder id='transformer'
-              class='ai.vespa.example.paragraph.ApplicationSpecificEmbedder'
-              bundle='exampleEmbedder'
-              def='embedding.bert-base-embedder'>
-      <model id="test-model-id" url="test-model-url"/>
-      <vocab path="files/vocab.txt"/>
-    </embedder>
-
-    <nodes>
-      <node hostalias='node1'/>
-    </nodes>
-  </container>
-
-</services>
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/EmbedderTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/EmbedderTestCase.java
index 2a92963018d..766c2b11256 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/EmbedderTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/EmbedderTestCase.java
@@ -55,84 +55,112 @@ public class EmbedderTestCase {
 
     @Test
     void testPredefinedEmbedConfigSelfHosted() throws IOException, SAXException {
+        String embedder = "<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\">" +
+                          "  <model id=\"my_model_id\" url=\"my-model-url\" />" +
+                          "  <vocab id=\"my_vocab_id\" url=\"my-vocab-url\" />" +
+                          "</embedder>";
+        String component = "<component id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\" bundle=\"model-integration\">" +
+                           "  <config name=\"" + PREDEFINED_EMBEDDER_CONFIG + "\">" +
+                           "      <tokenizerVocabUrl>my-vocab-url</tokenizerVocabUrl>" +
+                           "      <tokenizerVocabPath></tokenizerVocabPath>" +
+                           "      <transformerModelUrl>my-model-url</transformerModelUrl>" +
+                           "      <transformerModelPath></transformerModelPath>" +
+                           "  </config>" +
+                           "</component>";
+        assertTransform(embedder, component, false);
+    }
+
+    @Test
+    void testIncorrectEmbedderOptionsSelfHosted() throws IOException, SAXException {
         assertTransformThrows("<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\"></embedder>",
-                "Embedder '" + PREDEFINED_EMBEDDER_CLASS + "' requires options for [vocab, model]");
+                              "Embedder '" + PREDEFINED_EMBEDDER_CLASS + "' requires options for [vocab, model]");
         assertTransformThrows("<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\">" +
-                "  <model />" +
-                "  <vocab />" +
-                "</embedder>",
-                "Model option requires either a 'path' or a 'url' attribute");
+                              "  <model />" +
+                              "  <vocab />" +
+                              "</embedder>",
+                              "Model option requires either a 'path' or a 'url' attribute");
         assertTransformThrows("<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\">" +
-                "  <model id=\"my_model_id\" />" +
-                "  <vocab id=\"my_vocab_id\" />" +
-                "</embedder>",
-                "Model option 'id' is not valid here");
+                              "  <model id=\"my_model_id\" />" +
+                              "  <vocab id=\"my_vocab_id\" />" +
+                              "</embedder>",
+                              "Model option 'id' is not valid here");
+    }
 
+    @Test
+    void testPathHasprioritySelfHosted() throws IOException, SAXException {
         String embedder = "<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\">" +
-                "  <model id=\"my_model_id\" url=\"my-model-url\" />" +
-                "  <vocab id=\"my_vocab_id\" url=\"my-vocab-url\" />" +
-                "</embedder>";
+                          "  <model id=\"my_model_id\" url=\"my-model-url\" path=\"files/model.onnx\" />" +
+                          "  <vocab id=\"my_vocab_id\" url=\"my-vocab-url\" path=\"files/vocab.txt\" />" +
+                          "</embedder>";
         String component = "<component id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\" bundle=\"model-integration\">" +
-                "  <config name=\"" + PREDEFINED_EMBEDDER_CONFIG + "\">" +
-                "      <tokenizerVocabUrl>my-vocab-url</tokenizerVocabUrl>" +
-                "      <tokenizerVocabPath></tokenizerVocabPath>" +
-                "      <transformerModelUrl>my-model-url</transformerModelUrl>" +
-                "      <transformerModelPath></transformerModelPath>" +
-                "  </config>" +
-                "</component>";
-        assertTransform(embedder, component, false);
-
-        // Path has priority:
-        embedder = "<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\">" +
-                "  <model id=\"my_model_id\" url=\"my-model-url\" path=\"files/model.onnx\" />" +
-                "  <vocab id=\"my_vocab_id\" url=\"my-vocab-url\" path=\"files/vocab.txt\" />" +
-                "</embedder>";
-        component = "<component id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\" bundle=\"model-integration\">" +
-                "  <config name=\"" + PREDEFINED_EMBEDDER_CONFIG + "\">" +
-                "      <tokenizerVocabPath>files/vocab.txt</tokenizerVocabPath>" +
-                "      <tokenizerVocabUrl></tokenizerVocabUrl>" +
-                "      <transformerModelPath>files/model.onnx</transformerModelPath>" +
-                "      <transformerModelUrl></transformerModelUrl>" +
-                "  </config>" +
-                "</component>";
+                           "  <config name=\"" + PREDEFINED_EMBEDDER_CONFIG + "\">" +
+                           "      <tokenizerVocabPath>files/vocab.txt</tokenizerVocabPath>" +
+                           "      <tokenizerVocabUrl></tokenizerVocabUrl>" +
+                           "      <transformerModelPath>files/model.onnx</transformerModelPath>" +
+                           "      <transformerModelUrl></transformerModelUrl>" +
+                           "  </config>" +
+                           "</component>";
         assertTransform(embedder, component, false);
     }
 
     @Test
-    void testPredefinedEmbedConfigCloud() throws IOException, SAXException {
+    void testPredefinedEmptyEmbedConfigCloud() throws IOException, SAXException {
         String embedder = "<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\" />";
         String component = "<component id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\" bundle=\"model-integration\">" +
-                "  <config name=\"" + PREDEFINED_EMBEDDER_CONFIG + "\">" +
-                "      <tokenizerVocabUrl>some url</tokenizerVocabUrl>" +
-                "      <tokenizerVocabPath></tokenizerVocabPath>" +
-                "      <transformerModelUrl>some url</transformerModelUrl>" +
-                "      <transformerModelPath></transformerModelPath>" +
-                "  </config>" +
-                "</component>";
+                           "  <config name=\"" + PREDEFINED_EMBEDDER_CONFIG + "\">" +
+                           "      <tokenizerVocabUrl>some url</tokenizerVocabUrl>" +
+                           "      <tokenizerVocabPath></tokenizerVocabPath>" +
+                           "      <transformerModelUrl>some url</transformerModelUrl>" +
+                           "      <transformerModelPath></transformerModelPath>" +
+                           "  </config>" +
+                           "</component>";
         assertTransform(embedder, component, true);
+    }
 
-        embedder = "<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\">" +
-                "  <model id=\"my_model_id\" />" +
-                "  <vocab id=\"my_vocab_id\" />" +
-                "</embedder>";
-        assertTransformThrows(embedder, "Unknown model id: 'my_vocab_id'", true);
+    @Test
+    void testPredefinedEmbedConfigCloud() throws IOException, SAXException {
+        String embedder = "<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\">" +
+                          "  <model id=\"test-model-id\" />" +
+                          "  <vocab id=\"test-model-id\" />" +
+                          "</embedder>";
+        String component = "<component id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\" bundle=\"model-integration\">" +
+                           "  <config name=\"" + PREDEFINED_EMBEDDER_CONFIG + "\">" +
+                           "      <tokenizerVocabUrl>test-model-url</tokenizerVocabUrl>" +
+                           "      <tokenizerVocabPath></tokenizerVocabPath>" +
+                           "      <transformerModelUrl>test-model-url</transformerModelUrl>" +
+                           "      <transformerModelPath></transformerModelPath>" +
+                           "  </config>" +
+                           "</component>";
+        assertTransform(embedder, component, true);
+    }
 
-        embedder = "<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\">" +
-                "  <model id=\"test-model-id\" />" +
-                "  <vocab id=\"test-model-id\" />" +
-                "</embedder>";
-        component = "<component id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\" bundle=\"model-integration\">" +
-                "  <config name=\"" + PREDEFINED_EMBEDDER_CONFIG + "\">" +
-                "      <tokenizerVocabUrl>test-model-url</tokenizerVocabUrl>" +
-                "      <tokenizerVocabPath></tokenizerVocabPath>" +
-                "      <transformerModelUrl>test-model-url</transformerModelUrl>" +
-                "      <transformerModelPath></transformerModelPath>" +
-                "  </config>" +
-                "</component>";
+    @Test
+    void testCustomEmbedderWithPredefinedConfigCloud() throws IOException, SAXException {
+        String embedder = "<embedder id=\"test\" class=\"ApplicationSpecificEmbedder\" def=\"" + PREDEFINED_EMBEDDER_CONFIG + "\">" +
+                          "  <model id=\"test-model-id\" />" +
+                          "  <vocab id=\"test-model-id\" />" +
+                          "</embedder>";
+        String component = "<component id=\"test\" class=\"ApplicationSpecificEmbedder\" bundle=\"model-integration\">" +
+                           "  <config name=\"" + PREDEFINED_EMBEDDER_CONFIG + "\">" +
+                           "      <tokenizerVocabUrl>test-model-url</tokenizerVocabUrl>" +
+                           "      <tokenizerVocabPath></tokenizerVocabPath>" +
+                           "      <transformerModelUrl>test-model-url</transformerModelUrl>" +
+                           "      <transformerModelPath></transformerModelPath>" +
+                           "  </config>" +
+                           "</component>";
         assertTransform(embedder, component, true);
     }
 
     @Test
+    void testUnknownModelIdCloud() throws IOException, SAXException {
+        String embedder = "<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\">" +
+                          "  <model id=\"my_model_id\" />" +
+                          "  <vocab id=\"my_vocab_id\" />" +
+                          "</embedder>";
+        assertTransformThrows(embedder, "Unknown model id: 'my_vocab_id'", true);
+    }
+
+    @Test
     void testApplicationWithEmbedConfig() throws Exception  {
         final String emptyPathFileName = "services.xml";
 
@@ -165,22 +193,6 @@ public class EmbedderTestCase {
         assertEquals("files/model.onnx", config.getObject("model").getValue());
     }
 
-    @Test
-    void testApplicationWithGenericEmbedConfigUsingProvidedModel() throws Exception  {
-        final String emptyPathFileName = "services.xml";
-
-        Path applicationDir = Path.fromString("src/test/cfg/application/embed_generic_using_provided_model/");
-        VespaModel model = loadModel(applicationDir, false);
-        ApplicationContainerCluster containerCluster = model.getContainerClusters().get("container");
-
-        Component<?, ?> testComponent = containerCluster.getComponentsMap().get(new ComponentId("transformer"));
-        ConfigPayloadBuilder config = testComponent.getUserConfigs().get(new ConfigDefinitionKey("bert-base-embedder", "embedding"));
-        assertEquals("test-model-url", config.getObject("transformerModelUrl").getValue());
-        assertEquals(emptyPathFileName, config.getObject("transformerModelPath").getValue());
-        assertEquals("", config.getObject("tokenizerVocabUrl").getValue());
-        assertEquals("files/vocab.txt", config.getObject("tokenizerVocabPath").getValue());
-    }
-
     private VespaModel loadModel(Path path, boolean hosted) throws Exception {
         FilesApplicationPackage applicationPackage = FilesApplicationPackage.fromFile(path.toFile());
         TestProperties properties = new TestProperties().setHostedVespa(hosted);