From a11ad79a04d6dc6a12fd4b4a2de6305688e9e6db Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 8 Nov 2018 11:18:51 +0100 Subject: Revert "Supporting clearing config array when overriding config" --- .../builder/xml/dom/DomConfigPayloadBuilder.java | 24 +----- .../vespa/model/builder/UserConfigBuilderTest.java | 92 +++++++--------------- 2 files changed, 31 insertions(+), 85 deletions(-) (limited to 'config-model/src') 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 20415343755..e9abcfd87a9 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,7 +169,9 @@ public class DomConfigPayloadBuilder { // Check for legacy (pre Vespa 6) usage throw new IllegalArgumentException("The 'index' attribute on config elements is not supported - use "); } else if (element.hasAttribute("operation")) { - ConfigPayloadBuilder childPayloadBuilder = getBuilderForInnerArray(element, payloadBuilder, name); + // inner array, currently the only supported operation is 'append' + verifyLegalOperation(element); + ConfigPayloadBuilder childPayloadBuilder = payloadBuilder.getArray(name).append(); //Cursor array = node.setArray(name); for (Element child : children) { //Cursor struct = array.addObject(); @@ -238,24 +240,4 @@ public class DomConfigPayloadBuilder { + operation + "' at XML node '" + XML.getNodePath(currElem, " > ") + "'."); } - private ConfigPayloadBuilder getBuilderForInnerArray(Element element, ConfigPayloadBuilder payloadBuilder, String arrayName) { - // inner array, the supported operations are 'append' and 'clear' - String operation = element.getAttribute("operation").toLowerCase(); - ConfigPayloadBuilder arrayPayloadBuilder; - switch (operation) { - case "append": - arrayPayloadBuilder = payloadBuilder.getArray(arrayName).append(); - break; - case "clear": - // Clear array if it exists, use the existing builder - // Creating the array happens when handling the children ('item's) - payloadBuilder.removeArray(arrayName); - arrayPayloadBuilder = payloadBuilder; - break; - default: - throw new RuntimeException("Unknown operation '" + operation + "' at XML node '" + XML.getNodePath(element, " > ") + "'."); - } - return arrayPayloadBuilder; - } - } 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 747ce93d86b..d1ef1010b73 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,6 +8,7 @@ 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; @@ -16,28 +17,30 @@ 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.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.*; /** * @author Ulf Lilleengen + * @since 5.1 */ public class UserConfigBuilderTest { - private final ConfigDefinitionStore configDefinitionStore = defKey -> Optional.empty(); + private final ConfigDefinitionStore configDefinitionStore = new ConfigDefinitionStore() { + @Override + public Optional getConfigDefinition(ConfigDefinitionKey defKey) { return Optional.empty(); } + }; @Test - public void require_that_simple_config_is_resolved() { + public void require_that_simple_config_is_resolved() throws ParserConfigurationException, IOException, SAXException { Element configRoot = getDocument("" + " 13" + "" + @@ -53,59 +56,31 @@ public class UserConfigBuilderTest { assertThat(config.stringval(), is("foolio")); } - @Test - public void require_that_arrays_config_is_resolved() { - Element configRoot = getDocument("" + - " 13" + - " 10" + - " 1337" + - ""); - 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)); - assertEquals(Arrays.asList(13,10,1337), config.intarr()); + public static ConfigType createConfig(Class clazz, ConfigPayloadBuilder builder) { + return ConfigPayload.fromBuilder(builder).toInstance(clazz, ""); } - @Test - public void require_that_array_with_items_is_resolved() { - ConfigDefinitionKey key = new ConfigDefinitionKey("arraytypes", "config"); - Element configRoot = getDocument("" + - " " + - " 0" + - " " + - ""); + @Test + public void require_that_arrays_config_is_resolved() throws ParserConfigurationException, IOException, SAXException { + Element configRoot = getDocument("" + + " 13" + + " 10" + + " 1337" + + ""); 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)); - assertEquals(Collections.singletonList(0), config.intarr()); - - Element configRoot2 = getDocument("" + - " " + - " 1" + - " 2" + - " " + - ""); - UserConfigRepo map2 = mergeAndCreateConfig(map, configRoot2); - ArraytypesConfig config2 = createConfig(ArraytypesConfig.class, map2.get(key)); - assertEquals(Arrays.asList(1, 2), config2.intarr()); - - - Element configRoot3 = getDocument("" + - " " + - " 3" + - " " + - ""); - UserConfigRepo map3 = mergeAndCreateConfig(map2, configRoot3); - ArraytypesConfig config3 = createConfig(ArraytypesConfig.class, map3.get(key)); - assertEquals(Collections.singletonList(3), config3.intarr()); + assertThat(config.intarr().size(), is(3)); + assertThat(config.intarr(0), is(13)); + assertThat(config.intarr(1), is(10)); + assertThat(config.intarr(2), is(1337)); } @Test - public void require_that_arrays_of_structs_are_resolved() { + public void require_that_arrays_of_structs_are_resolved() throws ParserConfigurationException, IOException, SAXException { Element configRoot = getDocument( " " + " " + @@ -132,7 +107,7 @@ public class UserConfigBuilderTest { } @Test - public void no_exception_when_config_class_does_not_exist() { + public void no_exception_when_config_class_does_not_exist() throws ParserConfigurationException, IOException, SAXException { Element configRoot = getDocument("" + " 1" + ""); @@ -141,7 +116,7 @@ public class UserConfigBuilderTest { assertNotNull(builder); } - private Element getDocument(String xml) { + private Element getDocument(String xml) throws ParserConfigurationException { Reader xmlReader = new StringReader("" + xml + ""); Document doc; try { @@ -151,15 +126,4 @@ public class UserConfigBuilderTest { } return doc.getDocumentElement(); } - - private static ConfigType createConfig(Class 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; - } - } -- cgit v1.2.3