Simplify and generalize

Let embedder rewrites simply always change <x path="y" url="z">
to <xPath>y</xPath> <xUrl>z</xUrl>, as well as translating
the id attirebute to the corresponding path if on hosted.

This means that the current built-in embedder accepting
"vocab" and "model" is broken as these names are different
from the names in the config model, but this isn't documented yet
so should be okay.

The effect of this is that our built-in models can be
used in any embedder and config, and the embedder syntax
can be used in application specific embedders.

1 files changed, 46 insertions, 88 deletions

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 766c2b11256..fef461a4b7a 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
@@ -34,101 +34,63 @@ public class EmbedderTestCase {
 
     @Test
     void testGenericEmbedConfig() throws IOException, SAXException {
-        String embedder = "<embedder id=\"test\" class=\"ai.vespa.test\" bundle=\"bundle\" def=\"def.name\">" +
-                "  <val>123</val>" +
-                "</embedder>";
-        String component = "<component id=\"test\" class=\"ai.vespa.test\" bundle=\"bundle\">" +
-                "  <config name=\"def.name\">" +
-                "    <val>123</val>" +
-                "  </config>" +
-                "</component>";
+        String embedder = "<embedder id='test' class='ai.vespa.test' bundle='bundle' def='def.name'>" +
+                          "  <val>123</val>" +
+                          "</embedder>";
+        String component = "<component id='test' class='ai.vespa.test' bundle='bundle'>" +
+                           "  <config name='def.name'>" +
+                           "    <val>123</val>" +
+                           "  </config>" +
+                           "</component>";
         assertTransform(embedder, component);
     }
 
     @Test
-    void testGenericEmbedConfigRequiresBundleAndDef() throws IOException, SAXException {
-        assertTransformThrows("<embedder id=\"test\" class=\"ai.vespa.test\"></embedder>",
-                "Embedder configuration requires a bundle name");
-        assertTransformThrows("<embedder id=\"test\" class=\"ai.vespa.test\" bundle=\"bundle\"></embedder>",
-                "Embedder configuration requires a config definition name");
-    }
-
-    @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\" />" +
+        String embedder = "<embedder id='test' class='" + PREDEFINED_EMBEDDER_CLASS + "'>" +
+                          "  <transformerModel id='my_model_id' url='my-model-url' />" +
+                          "  <tokenizerVocab 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>" +
+        String component = "<component id='test' class='" + PREDEFINED_EMBEDDER_CLASS + "' bundle='model-integration'>" +
+                           "  <config name='" + PREDEFINED_EMBEDDER_CONFIG + "'>" +
                            "      <transformerModelUrl>my-model-url</transformerModelUrl>" +
                            "      <transformerModelPath></transformerModelPath>" +
+                           "      <tokenizerVocabUrl>my-vocab-url</tokenizerVocabUrl>" +
+                           "      <tokenizerVocabPath></tokenizerVocabPath>" +
                            "  </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]");
-        assertTransformThrows("<embedder id=\"test\" class=\"" + PREDEFINED_EMBEDDER_CLASS + "\">" +
-                              "  <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");
-    }
-
-    @Test
-    void testPathHasprioritySelfHosted() throws IOException, SAXException {
-        String 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\" />" +
+    void testPathHasPrioritySelfHosted() throws IOException, SAXException {
+        String embedder = "<embedder id='test' class='" + PREDEFINED_EMBEDDER_CLASS + "'>" +
+                          "  <transformerModel id='my_model_id' url='my-model-url' path='files/model.onnx' />" +
+                          "  <tokenizerVocab 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 + "\">" +
-                           "      <tokenizerVocabPath>files/vocab.txt</tokenizerVocabPath>" +
-                           "      <tokenizerVocabUrl></tokenizerVocabUrl>" +
-                           "      <transformerModelPath>files/model.onnx</transformerModelPath>" +
+        String component = "<component id='test' class='" + PREDEFINED_EMBEDDER_CLASS + "' bundle='model-integration'>" +
+                           "  <config name='" + PREDEFINED_EMBEDDER_CONFIG + "'>" +
                            "      <transformerModelUrl></transformerModelUrl>" +
+                           "      <transformerModelPath>files/model.onnx</transformerModelPath>" +
+                           "      <tokenizerVocabUrl></tokenizerVocabUrl>" +
+                           "      <tokenizerVocabPath>files/vocab.txt</tokenizerVocabPath>" +
                            "  </config>" +
                            "</component>";
         assertTransform(embedder, component, false);
     }
 
     @Test
-    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>";
-        assertTransform(embedder, component, 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\" />" +
+        String embedder = "<embedder id='test' class='" + PREDEFINED_EMBEDDER_CLASS + "'>" +
+                          "  <transformerModel id='test-model-id' />" +
+                          "  <tokenizerVocab 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>" +
+        String component = "<component id='test' class='" + PREDEFINED_EMBEDDER_CLASS + "' bundle='model-integration'>" +
+                           "  <config name='" + PREDEFINED_EMBEDDER_CONFIG + "'>" +
                            "      <transformerModelUrl>test-model-url</transformerModelUrl>" +
                            "      <transformerModelPath></transformerModelPath>" +
+                           "      <tokenizerVocabUrl>test-model-url</tokenizerVocabUrl>" +
+                           "      <tokenizerVocabPath></tokenizerVocabPath>" +
                            "  </config>" +
                            "</component>";
         assertTransform(embedder, component, true);
@@ -136,16 +98,16 @@ public class EmbedderTestCase {
 
     @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\" />" +
+        String embedder = "<embedder id='test' class='ApplicationSpecificEmbedder' def='" + PREDEFINED_EMBEDDER_CONFIG + "'>" +
+                          "  <transformerModel id='test-model-id' />" +
+                          "  <tokenizerVocab 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>" +
+        String component = "<component id='test' class='ApplicationSpecificEmbedder' bundle='model-integration'>" +
+                           "  <config name='" + PREDEFINED_EMBEDDER_CONFIG + "'>" +
                            "      <transformerModelUrl>test-model-url</transformerModelUrl>" +
                            "      <transformerModelPath></transformerModelPath>" +
+                           "      <tokenizerVocabUrl>test-model-url</tokenizerVocabUrl>" +
+                           "      <tokenizerVocabPath></tokenizerVocabPath>" +
                            "  </config>" +
                            "</component>";
         assertTransform(embedder, component, true);
@@ -153,11 +115,11 @@ public class EmbedderTestCase {
 
     @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\" />" +
+        String embedder = "<embedder id='test' class='" + PREDEFINED_EMBEDDER_CLASS + "'>" +
+                          "  <transformerModel id='my_model_id' />" +
+                          "  <tokenizerVocab id='my_vocab_id' />" +
                           "</embedder>";
-        assertTransformThrows(embedder, "Unknown model id: 'my_vocab_id'", true);
+        assertTransformThrows(embedder, "Unknown model id 'my_model_id'", true);
     }
 
     @Test
@@ -190,7 +152,7 @@ public class EmbedderTestCase {
         Component<?, ?> testComponent = containerCluster.getComponentsMap().get(new ComponentId("transformer"));
         ConfigPayloadBuilder config = testComponent.getUserConfigs().get(new ConfigDefinitionKey("sentence-embedder", "ai.vespa.example.paragraph"));
         assertEquals("files/vocab.txt", config.getObject("vocab").getValue());
-        assertEquals("files/model.onnx", config.getObject("model").getValue());
+        assertEquals("files/model.onnx", config.getObject("modelPath").getValue());
     }
 
     private VespaModel loadModel(Path path, boolean hosted) throws Exception {
@@ -237,16 +199,12 @@ public class EmbedderTestCase {
         }
     }
 
-    private void assertTransformThrows(String embedder, String msg) throws IOException, SAXException {
-        assertTransformThrows(embedder, msg, false);
-    }
-
-    private void assertTransformThrows(String embedder, String msg, boolean hosted) throws IOException, SAXException {
+    private void assertTransformThrows(String embedder, String expectedMessage, boolean hosted) throws IOException, SAXException {
         try {
             EmbedderConfig.transform(createEmptyDeployState(hosted), createElement(embedder));
-            fail("Expected exception was not thrown: " + msg);
+            fail("Expected exception was not thrown: " + expectedMessage);
         } catch (IllegalArgumentException e) {
-            assertEquals(e.getMessage(), msg);
+            assertEquals(expectedMessage, e.getMessage());
         }
     }