diff options
author | HÃ¥kon Hallingstad <hakon.hallingstad@gmail.com> | 2020-11-03 14:54:03 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-11-03 14:54:03 +0100 |
commit | 63eac0960cf9a7a6b4441459188586ad7813d820 (patch) | |
tree | 8189abd20d47a74d23b016a38c94639cce284af7 /config-application-package | |
parent | 0b902f8a83015de3f37c8adb449f360add305780 (diff) |
Revert "Revert "Add required="true" for override without node children""
Diffstat (limited to 'config-application-package')
2 files changed, 63 insertions, 4 deletions
diff --git a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java index 6a946e1ce75..35584bf9608 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java @@ -4,7 +4,6 @@ package com.yahoo.config.application; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; -import java.util.logging.Level; import com.yahoo.text.XML; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -19,6 +18,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -219,11 +219,20 @@ class OverrideProcessor implements PreProcessor { // if node capacity is specified explicitly for some combination we should require that capacity elements.forEach(element -> { if (element.getTagName().equals("nodes")) - if (element.getChildNodes().getLength() == 0) // specifies capacity, not a list of nodes + if (!hasChildWithTagName(element, "node")) // specifies capacity, not a list of nodes element.setAttribute("required", "true"); }); } - + + private static boolean hasChildWithTagName(Element element, String childName) { + for (var child : XML.getChildren(element)) { + if (child.getTagName().equals(childName)) + return true; + } + + return false; + } + /** * Retains all elements where at least one element is overridden. Removes non-overridden elements from map. */ diff --git a/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java index 05a5357a8ab..d8e35ced227 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java @@ -347,8 +347,58 @@ public class OverrideProcessorTest { new OverrideProcessor(InstanceName.from("default"), Environment.defaultEnvironment(), RegionName.from("us-west")).process(inputDoc); } + @Test + public void testImpliedRequired() throws TransformerException { + String input = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"?\" version=\"1.0\">" + + " <content id=\"foo\" version=\"1.0\">" + + " <nodes deploy:environment=\"dev\">" + + " <!-- comment -->" + + " <resources vcpu=\"2\" memory=\"8Gb\" disk=\"50Gb\" disk-speed=\"any\"/>" + + " </nodes>" + + " </content>" + + "</services>"; + String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"?\" version=\"1.0\">" + + " <content id=\"foo\" version=\"1.0\">" + + " <nodes required=\"true\">" + + " <!-- comment -->" + + " <resources vcpu=\"2\" memory=\"8Gb\" disk=\"50Gb\" disk-speed=\"any\"/>" + + " </nodes>" + + " </content>" + + "</services>"; + + assertOverride(input, Environment.dev, RegionName.defaultName(), expected); + } + + @Test + public void testNodeElementCancelsImpliedRequired() throws TransformerException { + String input = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"?\" version=\"1.0\">" + + " <content id=\"foo\" version=\"1.0\">" + + " <nodes deploy:environment=\"dev\">" + + " <node distribution-key=\"0\" hostalias=\"node0\"/>" + + " </nodes>" + + " </content>" + + "</services>"; + String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"?\" version=\"1.0\">" + + " <content id=\"foo\" version=\"1.0\">" + + " <nodes>" + + " <node distribution-key=\"0\" hostalias=\"node0\"/>" + + " </nodes>" + + " </content>" + + "</services>"; + + assertOverride(input, Environment.dev, RegionName.defaultName(), expected); + } + private void assertOverride(Environment environment, RegionName region, String expected) throws TransformerException { - Document inputDoc = Xml.getDocument(new StringReader(OverrideProcessorTest.input)); + assertOverride(input, environment, region, expected); + } + + private void assertOverride(String input, Environment environment, RegionName region, String expected) throws TransformerException { + Document inputDoc = Xml.getDocument(new StringReader(input)); Document newDoc = new OverrideProcessor(InstanceName.from("default"), environment, region).process(inputDoc); TestBase.assertDocument(expected, newDoc); } |