Supporting clearing config array when ovverriding config

3 files changed, 118 insertions, 34 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java
index e9abcfd87a9..d0abffcbac4 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java
@@ -169,9 +169,23 @@ public class DomConfigPayloadBuilder {
             // Check for legacy (pre Vespa 6) usage
             throw new IllegalArgumentException("The 'index' attribute on config elements is not supported - use <item>");
         } else if (element.hasAttribute("operation")) {
-            // inner array, currently the only supported operation is 'append'
-            verifyLegalOperation(element);
-            ConfigPayloadBuilder childPayloadBuilder = payloadBuilder.getArray(name).append();
+            // inner array, the supported operations are 'append' and 'clear'
+            String operation = verifyLegalOperation(element);
+            ConfigPayloadBuilder childPayloadBuilder;
+            switch (operation) {
+                case "append":
+                    childPayloadBuilder = payloadBuilder.getArray(name).append();
+                    break;
+                case "clear":
+                    // Clear array if it exists, use the existing builder
+                    // Creating the array happens when handling the children ('item's)
+                    if (payloadBuilder.arrayExists(name))
+                        payloadBuilder.clearArray(name);
+                    childPayloadBuilder = payloadBuilder;
+                    break;
+                default:
+                    throw new RuntimeException("Unknown operation '" + operation + "'");
+            }
             //Cursor array = node.setArray(name);
             for (Element child : children) {
                 //Cursor struct = array.addObject();
@@ -233,11 +247,12 @@ public class DomConfigPayloadBuilder {
         }
     }
 
-    private void verifyLegalOperation(Element currElem) {
+    private String verifyLegalOperation(Element currElem) {
         String operation = currElem.getAttribute("operation");
-        if (! operation.equalsIgnoreCase("append"))
-            throw new ConfigurationRuntimeException("The only supported array operation is 'append', got '"
+        if (! Arrays.asList("append", "clear").contains(operation))
+            throw new ConfigurationRuntimeException("The supported array operations are 'append' and 'clear', got '"
                     + operation + "' at XML node '" + XML.getNodePath(currElem, " > ") + "'.");
+        return operation;
     }
 
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java
index d1ef1010b73..6414f12c2fb 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java
@@ -8,7 +8,6 @@ import com.yahoo.config.model.deploy.ConfigDefinitionStore;
 import com.yahoo.test.SimpletypesConfig;
 import com.yahoo.config.model.producer.UserConfigRepo;
 import com.yahoo.config.model.builder.xml.XmlHelper;
-import com.yahoo.vespa.config.ConfigDefinition;
 import com.yahoo.vespa.config.ConfigDefinitionKey;
 import com.yahoo.vespa.config.ConfigPayload;
 import com.yahoo.vespa.config.ConfigPayloadBuilder;
@@ -17,30 +16,28 @@ import org.junit.Test;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.xml.sax.InputSource;
-import org.xml.sax.SAXException;
 
-import javax.xml.parsers.ParserConfigurationException;
-import java.io.IOException;
 import java.io.Reader;
 import java.io.StringReader;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.Optional;
 
 import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
 
 /**
  * @author Ulf Lilleengen
- * @since 5.1
  */
 public class UserConfigBuilderTest {
 
-    private final ConfigDefinitionStore configDefinitionStore = new ConfigDefinitionStore() {
-        @Override
-        public Optional<ConfigDefinition> getConfigDefinition(ConfigDefinitionKey defKey) { return Optional.empty(); }
-    };
+    private final ConfigDefinitionStore configDefinitionStore = defKey -> Optional.empty();
 
     @Test
-    public void require_that_simple_config_is_resolved() throws ParserConfigurationException, IOException, SAXException {
+    public void require_that_simple_config_is_resolved() {
         Element configRoot = getDocument("<config name=\"simpletypes\">" +
                                          "    <intval>13</intval>" +
                                          "</config>" +
@@ -56,31 +53,67 @@ public class UserConfigBuilderTest {
         assertThat(config.stringval(), is("foolio"));
     }
 
-    public static <ConfigType extends ConfigInstance> ConfigType createConfig(Class<ConfigType> clazz, ConfigPayloadBuilder builder) {
-        return ConfigPayload.fromBuilder(builder).toInstance(clazz, "");
-    }
-
-
     @Test
-    public void require_that_arrays_config_is_resolved() throws ParserConfigurationException, IOException, SAXException {
+    public void require_that_arrays_config_is_resolved() {
         Element configRoot = getDocument("<config name=\"arraytypes\">" +
-                "    <intarr operation=\"append\">13</intarr>" +
-                "    <intarr operation=\"append\">10</intarr>" +
-                "    <intarr operation=\"append\">1337</intarr>" +
-                "</config>");
+                                                 "    <intarr operation=\"append\">13</intarr>" +
+                                                 "    <intarr operation=\"append\">10</intarr>" +
+                                                 "    <intarr operation=\"append\">1337</intarr>" +
+                                                 "</config>");
         UserConfigRepo map = UserConfigBuilder.build(configRoot, configDefinitionStore, new BaseDeployLogger());
         assertFalse(map.isEmpty());
         ConfigDefinitionKey key = new ConfigDefinitionKey("arraytypes", "config");
         assertNotNull(map.get(key));
         ArraytypesConfig config = createConfig(ArraytypesConfig.class, map.get(key));
-        assertThat(config.intarr().size(), is(3));
-        assertThat(config.intarr(0), is(13));
-        assertThat(config.intarr(1), is(10));
-        assertThat(config.intarr(2), is(1337));
+        assertEquals(Arrays.asList(13,10,1337), config.intarr());
+
+        Element configRoot2 = getDocument("<config name=\"arraytypes\">" +
+                                                  "    <intarr operation=\"clear\">456</intarr>" +
+                                                  "</config>");
+        UserConfigRepo map2 = UserConfigBuilder.build(configRoot2, configDefinitionStore, new BaseDeployLogger());
+        map.merge(map2);
+        ArraytypesConfig config2 = createConfig(ArraytypesConfig.class, map2.get(key));
+        assertEquals(Collections.singletonList(456), config2.intarr());
+    }
+
+    @Test
+    public void require_that_array_with_items_is_resolved() {
+        ConfigDefinitionKey key = new ConfigDefinitionKey("arraytypes", "config");
+
+        Element configRoot = getDocument("<config name='arraytypes'>" +
+                                                 "    <intarr operation='clear'>" +
+                                                 "      <item>0</item>" +
+                                                 "    </intarr>" +
+                                                 "</config>");
+        UserConfigRepo map = UserConfigBuilder.build(configRoot, configDefinitionStore, new BaseDeployLogger());
+        assertFalse(map.isEmpty());
+        assertNotNull(map.get(key));
+        ArraytypesConfig config = createConfig(ArraytypesConfig.class, map.get(key));
+        assertEquals(Collections.singletonList(0), config.intarr());
+
+        Element configRoot2 = getDocument("<config name='arraytypes'>" +
+                                                 "    <intarr>" +
+                                                 "      <item>1</item>" +
+                                                 "      <item>2</item>" +
+                                                 "    </intarr>" +
+                                                 "</config>");
+        UserConfigRepo map2 = mergeAndCreateConfig(map, configRoot2);
+        ArraytypesConfig config2 = createConfig(ArraytypesConfig.class, map2.get(key));
+        assertEquals(Arrays.asList(1, 2), config2.intarr());
+
+
+        Element configRoot3 = getDocument("<config name='arraytypes'>" +
+                                                  "    <intarr operation='clear'>" +
+                                                  "      <item>3</item>" +
+                                                  "    </intarr>" +
+                                                  "</config>");
+        UserConfigRepo map3 = mergeAndCreateConfig(map2, configRoot3);
+        ArraytypesConfig config3 = createConfig(ArraytypesConfig.class, map3.get(key));
+        assertEquals(Collections.singletonList(3), config3.intarr());
     }
 
     @Test
-    public void require_that_arrays_of_structs_are_resolved() throws ParserConfigurationException, IOException, SAXException {
+    public void require_that_arrays_of_structs_are_resolved() {
         Element configRoot = getDocument(
                 "  <config name='vespa.configdefinition.specialtokens'>" +
                         "    <tokenlist operation='append'>" +
@@ -107,7 +140,7 @@ public class UserConfigBuilderTest {
     }
 
     @Test
-    public void no_exception_when_config_class_does_not_exist() throws ParserConfigurationException, IOException, SAXException {
+    public void no_exception_when_config_class_does_not_exist() {
         Element configRoot = getDocument("<config name=\"unknown\">" +
                 "    <foo>1</foo>" +
                 "</config>");
@@ -116,7 +149,7 @@ public class UserConfigBuilderTest {
         assertNotNull(builder);
     }
 
-    private Element getDocument(String xml) throws ParserConfigurationException {
+    private Element getDocument(String xml) {
         Reader xmlReader = new StringReader("<model>" + xml + "</model>");
         Document doc;
         try {
@@ -126,4 +159,15 @@ public class UserConfigBuilderTest {
         }
         return doc.getDocumentElement();
     }
+
+    private static <ConfigType extends ConfigInstance> ConfigType createConfig(Class<ConfigType> clazz, ConfigPayloadBuilder builder) {
+        return ConfigPayload.fromBuilder(builder).toInstance(clazz, "");
+    }
+
+    private UserConfigRepo mergeAndCreateConfig(UserConfigRepo original, Element newElement) {
+        UserConfigRepo map = UserConfigBuilder.build(newElement, configDefinitionStore, new BaseDeployLogger());
+        original.merge(map);
+        return map;
+    }
+
 }
diff --git a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java
index bb974ddae42..88eed8cbf36 100644
--- a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java
+++ b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java
@@ -109,6 +109,27 @@ public class ConfigPayloadBuilder {
         return a;
     }
 
+    /**
+     * Check if array with this name exists.
+     *
+     * @param name Name of array.
+     * @return true if array exists, false otherwise
+     */
+    public boolean arrayExists(String name) {
+        return arrayMap.containsKey(name);
+    }
+
+    /**
+     * Clears contents of an array
+     *
+     * @param name Name of array.
+     */
+    public void clearArray(String name) {
+        Array a = arrayMap.get(name);
+        if (a != null)
+            a.clear();
+    }
+
     private void validateArray(String name) {
         if (configDefinition != null) {
             configDefinition.verify(name);
@@ -417,6 +438,10 @@ public class ConfigPayloadBuilder {
             }
             return this;
         }
+
+        public void clear() {
+            elements.clear();
+        }
     }
 
     private ConfigPayloadBuilder(ConfigPayloadBuilder other) {