diff options
author | Harald Musum <musum@yahoo-inc.com> | 2017-01-26 12:44:22 +0100 |
---|---|---|
committer | Harald Musum <musum@yahoo-inc.com> | 2017-01-26 12:44:22 +0100 |
commit | 3412a4094942b9fe9237261762d3c2f4e2d919db (patch) | |
tree | 27486d14b20e3e6b9839690bec738748737cac35 /config-application-package | |
parent | 1b7b90e5e67d2e3e23173540e4faf9f437a19855 (diff) |
Handle overrides correctly
* Make sure to process all elements before deciding which to retain
Diffstat (limited to 'config-application-package')
2 files changed, 61 insertions, 35 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 6776119567b..bd913393b9d 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 @@ -138,40 +138,40 @@ class OverrideProcessor implements PreProcessor { // Put elements with same attributes in a map with a key that is the concatenation of attribute names // (except the override attribute names) and process values for each key Map<String, List<Element>> elementsByEqualAttributeSet = elementsByEqualAttributeSet(children); + + // Keep track of elements with highest number of matches (might be more than one element with same tag, need a list) + List<Element> bestMatches = new ArrayList<>(); + int bestMatch = 0; for (Map.Entry<String, List<Element>> entry : elementsByEqualAttributeSet.entrySet()) { - List<Element> elements = entry.getValue(); - - // Keep track of elements with highest number of matches (might be more than one element with same tag, need a list) - List<Element> bestMatchElements = new ArrayList<>(); - int bestMatch = 0; - for (Element child : elements) { - int overrideCount = getNumberOfOverrides(child, context); - if (overrideCount >= bestMatch) { - updateBestMatchElements(bestMatchElements, child, overrideCount, bestMatch); - bestMatch = overrideCount; - } + for (Element child : entry.getValue()) { + bestMatch = findBestMatches(bestMatches, child, bestMatch, context); } - - if (bestMatch > 1) { // there was a region/environment specific override - doElementSpecificProcessingOnOverride(bestMatchElements); - for (Element child : elements) { - // Remove elements not specific - if (!bestMatchElements.contains(child)) - parent.removeChild(child); + } + if (bestMatch > 0) { // there was a region/environment specific override + doElementSpecificProcessingOnOverride(bestMatches); + for (Element child : children) { + if ( ! bestMatches.contains(child)) { + parent.removeChild(child); } } } } - private void updateBestMatchElements(List<Element> bestMatchElements, Element child, int currentMatch, int bestMatch) { - if (bestMatch != currentMatch) - bestMatchElements.clear(); + private int findBestMatches(List<Element> bestMatchElements, Element child, int bestMatch, Context context) { + int overrideCount = getNumberOfOverrides(child, context); + if (overrideCount >= bestMatch) { + if (bestMatch != overrideCount) + bestMatchElements.clear(); - bestMatchElements.add(child); + bestMatchElements.add(child); + return overrideCount; + } else { + return bestMatch; + } } private int getNumberOfOverrides(Element child, Context context) { - int currentMatch = 1; + int currentMatch = 0; Optional<Environment> elementEnvironment = hasEnvironment(child) ? getEnvironment(child) : context.environment; RegionName elementRegion = hasRegion(child) ? getRegion(child) : context.region; if (elementEnvironment.isPresent() && elementEnvironment.get().equals(environment)) diff --git a/config-application-package/src/test/java/com/yahoo/config/application/HostedOverrideProcessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/HostedOverrideProcessorTest.java index 338302e9e57..50ec8062e27 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/HostedOverrideProcessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/HostedOverrideProcessorTest.java @@ -24,15 +24,17 @@ public class HostedOverrideProcessorTest { } private static final String input = - "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + - "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"?\" version=\"1.0\">" + - " <container id=\"foo\" version=\"1.0\">" + - " <nodes count='1'/>" + - " <nodes deploy:environment=\"staging\" count='2'/>" + - " <nodes deploy:environment=\"prod\" count='3'/>" + - " <nodes deploy:environment=\"prod\" deploy:region=\"us-west\" count='4'/>" + - " </container>" + - "</services>"; + "<?xml version='1.0' encoding='UTF-8' standalone='no'?>" + + "<services xmlns:deploy='vespa' xmlns:preprocess='?' version='1.0'>" + + " <container id='foo' version='1.0'>" + + " <nodes count='1'/>" + + " <nodes count='3' deploy:environment='perf'/>" + + " <nodes deploy:environment='staging' count='2' required='true'/>" + + " <nodes deploy:environment='prod' count='3' flavor='v-4-8-100'/>" + + " <nodes deploy:environment='prod' deploy:region='us-west' count='4'/>" + + " <nodes deploy:environment='prod' deploy:region='us-east-3' flavor='v-8-8-100' count='5'/>" + + " </container>" + + "</services>"; @Test @@ -60,15 +62,39 @@ public class HostedOverrideProcessorTest { } @Test + public void testParsingEnvironmentAndRegion2() throws ParserConfigurationException, IOException, SAXException, TransformerException { + String expected = + "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"?\" version=\"1.0\">" + + " <container id=\"foo\" version=\"1.0\">" + + " <nodes count='5' flavor='v-8-8-100' required='true'/>" + + " </container>" + + "</services>"; + assertOverride(Environment.from("prod"), RegionName.from("us-east-3"), expected); + } + + @Test + public void testParsingEnvironmentAndRegion3() throws ParserConfigurationException, IOException, SAXException, TransformerException { + String expected = + "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"?\" version=\"1.0\">" + + " <container id=\"foo\" version=\"1.0\">" + + " <nodes count='3' required='true'/>" + + " </container>" + + "</services>"; + assertOverride(Environment.from("perf"), RegionName.from("us-east-3"), expected); + } + + @Test public void testParsingEnvironmentUnknownRegion() throws ParserConfigurationException, IOException, SAXException, TransformerException { String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"?\" version=\"1.0\">" + " <container id=\"foo\" version=\"1.0\">" + - " <nodes count='3' required='true'/>" + + " <nodes count='3' flavor='v-4-8-100' required='true'/>" + " </container>" + "</services>"; - assertOverride(Environment.valueOf("prod"), RegionName.from("us-east"), expected); + assertOverride(Environment.valueOf("prod"), RegionName.from("unknown"), expected); } @Test @@ -77,7 +103,7 @@ public class HostedOverrideProcessorTest { "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"?\" version=\"1.0\">" + " <container id=\"foo\" version=\"1.0\">" + - " <nodes count='3' required='true'/>" + + " <nodes count='3' flavor='v-4-8-100' required='true'/>" + " </container>" + "</services>"; assertOverride(Environment.from("prod"), RegionName.defaultName(), expected); |