Handle overrides where multiple elements with same tag have overrides

VESPA-6219

2 files changed, 57 insertions, 33 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 32e9aec56cb..88e74e20875 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
@@ -3,7 +3,6 @@ package com.yahoo.config.application;
 
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
-import com.yahoo.data.access.simple.Value;
 import com.yahoo.log.LogLevel;
 import com.yahoo.text.XML;
 import org.w3c.dom.Document;
@@ -124,40 +123,53 @@ class OverrideProcessor implements PreProcessor {
      * Find the most specific element and remove all others.
      */
     private void retainMostSpecificEnvironmentAndRegion(Element parent, List<Element> children, Context context) {
-        Element bestMatchElement = null;
+        // 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 : children) {
-            int currentMatch = 1;
-            Optional<Environment> elementEnvironment = hasEnvironment(child) ? getEnvironment(child) : context.environment;
-            RegionName elementRegion = hasRegion(child) ? getRegion(child) : context.region;
-            if (elementEnvironment.isPresent() && elementEnvironment.get().equals(environment))
-                currentMatch++;
-            if ( ! elementRegion.isDefault() && elementRegion.equals(region))
-                currentMatch++;
-
-            if (currentMatch > bestMatch) {
-                bestMatchElement = child;
-                bestMatch = currentMatch;
+            int overrideCount = getNumberOfOverrides(child, context);
+            if (overrideCount >= bestMatch) {
+                updateBestMatchElements(bestMatchElements, child, overrideCount, bestMatch);
+                bestMatch = overrideCount;
             }
         }
 
-        if (bestMatch > 1) // there was a region/environment specific overriode
-            doElementSpecificProcessingOnOverride(bestMatchElement);
-
-        // Remove elements not specific
-        for (Element child : children) {
-            if (child != bestMatchElement) {
-                parent.removeChild(child);
+        if (bestMatch > 1) { // there was a region/environment specific override
+            doElementSpecificProcessingOnOverride(bestMatchElements);
+            for (Element child : children) {
+                // Remove elements not specific
+                if ( ! bestMatchElements.contains(child))
+                    parent.removeChild(child);
             }
         }
     }
 
+    private void updateBestMatchElements(List<Element> bestMatchElements, Element child, int currentMatch, int bestMatch) {
+        if (bestMatch != currentMatch)
+            bestMatchElements.clear();
+
+        bestMatchElements.add(child);
+    }
+
+    private int getNumberOfOverrides(Element child, Context context) {
+        int currentMatch = 1;
+        Optional<Environment> elementEnvironment = hasEnvironment(child) ? getEnvironment(child) : context.environment;
+        RegionName elementRegion = hasRegion(child) ? getRegion(child) : context.region;
+        if (elementEnvironment.isPresent() && elementEnvironment.get().equals(environment))
+            currentMatch++;
+        if ( ! elementRegion.isDefault() && elementRegion.equals(region))
+            currentMatch++;
+        return currentMatch;
+    }
+
     /** Called on each element which is selected by matching some override condition */
-    private void doElementSpecificProcessingOnOverride(Element element) {
-        // if node capacity is specified explicitly for some evn/region we should require that capacity
-        if ( element.getTagName().equals("nodes"))
-            if (element.getChildNodes().getLength() == 0) // specifies capacity, not a list of nodes
-                element.setAttribute("required", "true");
+    private void doElementSpecificProcessingOnOverride(List<Element> bestMatchElements) {
+        // if node capacity is specified explicitly for some env/region we should require that capacity
+        bestMatchElements.forEach(element -> {
+            if (element.getTagName().equals("nodes"))
+                if (element.getChildNodes().getLength() == 0) // specifies capacity, not a list of nodes
+                    element.setAttribute("required", "true");
+        });
     }
     
     /**
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 54d0a3cc797..c5fefc0f002 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
@@ -36,7 +36,12 @@ public class OverrideProcessorTest {
                 "  <content id=\"foo\" version=\"1.0\">" +
                 "    <redundancy>1</redundancy>" +
                 "    <documents>" +
-                "      <document mode=\"index\" type=\"music.sd\"/>" +
+                "      <document mode='index' type='music'/>\n" +
+                "      <document mode='index' type='music2'/>\n" +
+                "      <document deploy:environment='prod' deploy:region='us-east-3' mode='index' type='music'/>\n" +
+                "      <document deploy:environment='prod' deploy:region='us-east-3' mode='index' type='music2'/>\n" +
+                "      <document deploy:environment='prod' deploy:region='us-west' mode='index' type='music3'/>\n" +
+                "      <document deploy:environment='prod' deploy:region='us-west' mode='index' type='music4'/>\n" +
                 "    </documents>" +
                 "    <nodes>" +
                 "      <node distribution-key=\"0\" hostalias=\"node0\"/>" +
@@ -78,7 +83,8 @@ public class OverrideProcessorTest {
                 "  <content id=\"foo\" version=\"1.0\">" +
                 "    <redundancy>1</redundancy>" +
                 "    <documents>" +
-                "      <document mode=\"index\" type=\"music.sd\"/>" +
+                "      <document mode=\"index\" type=\"music\"/>" +
+                "      <document mode=\"index\" type=\"music2\"/>" +
                 "    </documents>" +
                 "    <nodes>" +
                 "      <node distribution-key=\"0\" hostalias=\"node0\"/>" +
@@ -106,7 +112,8 @@ public class OverrideProcessorTest {
                 "  <content id=\"foo\" version=\"1.0\">" +
                 "    <redundancy>1</redundancy>" +
                 "    <documents>" +
-                "      <document mode=\"index\" type=\"music.sd\"/>" +
+                "      <document mode=\"index\" type=\"music3\"/>" +
+                "      <document mode=\"index\" type=\"music4\"/>" +
                 "    </documents>" +
                 "    <nodes>" +
                 "      <node distribution-key=\"0\" hostalias=\"node0\"/>" +
@@ -138,7 +145,8 @@ public class OverrideProcessorTest {
                         "  <content id=\"foo\" version=\"1.0\">" +
                         "    <redundancy>1</redundancy>" +
                         "    <documents>" +
-                        "      <document mode=\"index\" type=\"music.sd\"/>" +
+                        "      <document mode=\"index\" type=\"music\"/>" +
+                        "      <document mode=\"index\" type=\"music2\"/>" +
                         "    </documents>" +
                         "    <nodes>" +
                         "      <node distribution-key=\"0\" hostalias=\"node0\"/>" +
@@ -169,7 +177,8 @@ public class OverrideProcessorTest {
                         "  <content id=\"foo\" version=\"1.0\">" +
                         "    <redundancy>1</redundancy>" +
                         "    <documents>" +
-                        "      <document mode=\"index\" type=\"music.sd\"/>" +
+                        "      <document mode=\"index\" type=\"music\"/>" +
+                        "      <document mode=\"index\" type=\"music2\"/>" +
                         "    </documents>" +
                         "    <nodes>" +
                         "      <node distribution-key=\"0\" hostalias=\"node0\"/>" +
@@ -200,7 +209,8 @@ public class OverrideProcessorTest {
                         "  <content id=\"foo\" version=\"1.0\">" +
                         "    <redundancy>1</redundancy>" +
                         "    <documents>" +
-                        "      <document mode=\"index\" type=\"music.sd\"/>" +
+                        "      <document mode=\"index\" type=\"music\"/>" +
+                        "      <document mode=\"index\" type=\"music2\"/>" +
                         "    </documents>" +
                         "    <nodes>" +
                         "      <node distribution-key=\"0\" hostalias=\"node0\"/>" +
@@ -228,7 +238,8 @@ public class OverrideProcessorTest {
                         "  <content id=\"foo\" version=\"1.0\">" +
                         "    <redundancy>1</redundancy>" +
                         "    <documents>" +
-                        "      <document mode=\"index\" type=\"music.sd\"/>" +
+                        "      <document mode=\"index\" type=\"music\"/>" +
+                        "      <document mode=\"index\" type=\"music2\"/>" +
                         "    </documents>" +
                         "    <nodes>" +
                         "      <node distribution-key=\"0\" hostalias=\"node0\"/>" +
@@ -256,7 +267,8 @@ public class OverrideProcessorTest {
                         "  <content id=\"foo\" version=\"1.0\">" +
                         "    <redundancy>1</redundancy>" +
                         "    <documents>" +
-                        "      <document mode=\"index\" type=\"music.sd\"/>" +
+                        "      <document mode=\"index\" type=\"music\"/>" +
+                        "      <document mode=\"index\" type=\"music2\"/>" +
                         "    </documents>" +
                         "    <nodes>" +
                         "      <node distribution-key=\"0\" hostalias=\"node1\"/>" +