Include value of 'idref' attribute when deciding which elements to keep when preprocessing

We already include value of 'id' to key which is used to decide which elements are equal,
but also need to include value of 'idref' so that we know which elements to keep e.g.
in a list of searchers referred to by 'idref' in a search chain.

5 files changed, 79 insertions, 18 deletions

diff --git a/config-application-package/src/main/java/com/yahoo/config/application/ConfigDefinitionDir.java b/config-application-package/src/main/java/com/yahoo/config/application/ConfigDefinitionDir.java
index d4b257f0ba9..1329befbc9d 100644
--- a/config-application-package/src/main/java/com/yahoo/config/application/ConfigDefinitionDir.java
+++ b/config-application-package/src/main/java/com/yahoo/config/application/ConfigDefinitionDir.java
@@ -11,7 +11,6 @@ import java.util.List;
  * but they cannot conflict with the existing ones.
  *
  * @author Ulf Lilleengen
- * @since 5.1
  */
 public class ConfigDefinitionDir {
     private final File defDir;
diff --git a/config-application-package/src/main/java/com/yahoo/config/application/IncludeProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/IncludeProcessor.java
index bc48e7dd814..ac365fa5a3e 100644
--- a/config-application-package/src/main/java/com/yahoo/config/application/IncludeProcessor.java
+++ b/config-application-package/src/main/java/com/yahoo/config/application/IncludeProcessor.java
@@ -22,7 +22,6 @@ import static java.nio.charset.StandardCharsets.UTF_8;
  * Handles preprocess:include statements and returns a Document which has all the include statements resolved
  *
  * @author hmusum
- * @since 5.22
  */
 class IncludeProcessor implements PreProcessor {
 
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 bb456d95326..5f2046b1450 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
@@ -46,6 +46,7 @@ class OverrideProcessor implements PreProcessor {
     private final Tags tags;
 
     private static final String ID_ATTRIBUTE = "id";
+    private static final String IDREF_ATTRIBUTE = "idref";
     private static final String INSTANCE_ATTRIBUTE = "instance";
     private static final String ENVIRONMENT_ATTRIBUTE = "environment";
     private static final String REGION_ATTRIBUTE = "region";
@@ -200,7 +201,7 @@ class OverrideProcessor implements PreProcessor {
 
     /** Find the most specific element and remove all others. */
     private void retainMostSpecific(Element parent, List<Element> children, Context context) {
-        // Keep track of elements with highest number of matches (might be more than one element with same tag, need a list)
+        // Keep track of elements with the 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 (Element child : children) {
@@ -307,42 +308,43 @@ class OverrideProcessor implements PreProcessor {
 
     private Set<InstanceName> getInstances(Element element) {
         String instance = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, INSTANCE_ATTRIBUTE);
-        if (instance == null || instance.isEmpty()) return  Set.of();
+        if (instance.isEmpty()) return Set.of();
         return Arrays.stream(instance.split(" ")).map(InstanceName::from).collect(Collectors.toSet());
     }
 
     private Set<Environment> getEnvironments(Element element) {
         String env = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, ENVIRONMENT_ATTRIBUTE);
-        if (env == null || env.isEmpty()) return Set.of();
+        if (env.isEmpty()) return Set.of();
         return Arrays.stream(env.split(" ")).map(Environment::from).collect(Collectors.toSet());
     }
 
     private Set<RegionName> getRegions(Element element) {
         String reg = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, REGION_ATTRIBUTE);
-        if (reg == null || reg.isEmpty()) return Set.of();
+        if (reg.isEmpty()) return Set.of();
         return Arrays.stream(reg.split(" ")).map(RegionName::from).collect(Collectors.toSet());
     }
 
     private Set<CloudName> getClouds(Element element) {
         String reg = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, CLOUD_ATTRIBUTE);
-        if (reg == null || reg.isEmpty()) return Set.of();
+        if (reg.isEmpty()) return Set.of();
         return Arrays.stream(reg.split(" ")).map(CloudName::from).collect(Collectors.toSet());
     }
 
     private Tags getTags(Element element) {
         String env = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, TAGS_ATTRIBUTE);
-        if (env == null || env.isEmpty()) return Tags.empty();
+        if (env.isEmpty()) return Tags.empty();
         return Tags.fromString(env);
     }
 
     private Map<String, List<Element>> elementsByTagNameAndId(List<Element> children) {
         Map<String, List<Element>> elementsByTagName = new LinkedHashMap<>();
-        // Index by tag name
+        // Index by tag name and optionally add "id" or "idref" to key if they are set
         for (Element child : children) {
             String key = child.getTagName();
-            if (child.hasAttribute(ID_ATTRIBUTE)) {
+            if (child.hasAttribute(ID_ATTRIBUTE))
                 key += child.getAttribute(ID_ATTRIBUTE);
-            }
+            if (child.hasAttribute(IDREF_ATTRIBUTE))
+                key += child.getAttribute(IDREF_ATTRIBUTE);
             if ( ! elementsByTagName.containsKey(key)) {
                 elementsByTagName.put(key, new ArrayList<>());
             }
@@ -382,7 +384,7 @@ class OverrideProcessor implements PreProcessor {
     }
 
     /**
-     * Represents environment and region in a given context.
+     * Represents environments, regions, instances, clouds and tags in a given context.
      */
     private static final class Context {
 
diff --git a/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java
index b02ccc711c0..4dc40df61f4 100644
--- a/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java
+++ b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java
@@ -11,8 +11,8 @@ public class ValidationProcessor implements PreProcessor {
 
     @Override
     public Document process(Document input) throws IOException, TransformerException {
-        NodeList includeitems = input.getElementsByTagNameNS("http://www.w3.org/2001/XInclude", "*");
-        if (includeitems.getLength() > 0)
+        NodeList includeItems = input.getElementsByTagNameNS("http://www.w3.org/2001/XInclude", "*");
+        if (includeItems.getLength() > 0)
             throw new UnsupportedOperationException("XInclude not supported, use preprocess:include instead");
         return input;
     }
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 e5e36615b09..c2b0770ab06 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
@@ -2,6 +2,7 @@
 package com.yahoo.config.application;
 
 import com.yahoo.config.provision.Cloud;
+import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.RegionName;
@@ -12,6 +13,8 @@ import org.w3c.dom.Document;
 import javax.xml.transform.TransformerException;
 import java.io.StringReader;
 
+import static com.yahoo.config.provision.Tags.empty;
+
 /**
  * @author Ulf Lilleengen
  */
@@ -366,14 +369,72 @@ public class OverrideProcessorTest {
         assertOverride(input, Environment.dev, RegionName.defaultName(), expected);
     }
 
+    /**
+     * Tests that searchers referred to with idref are overridden per cloud
+     * and that searchers not referred to with idref are not overridden.
+     */
+    @Test
+    public void testSearchersReferredWithIdRefPerCloud() throws TransformerException {
+        String input =
+                """
+                        <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+                        <services xmlns:deploy="vespa" xmlns:preprocess="?" version="1.0">
+                        <container id="stateless" version="1.0">
+                          <search>
+                            <searcher id="AwsSearcher" class="ai.vespa.AwsSearcher" bundle="foo"/>
+                            <searcher id="GcpSearcher" class="ai.vespa.GcpSearcher" bundle="foo"/>
+                            <searcher id="OtherSearcher" class="ai.vespa.OtherSearcher" bundle="foo"/>
+                            <chain id="default" inherits="vespa">
+                               <searcher idref="AwsSearcher" deploy:cloud="aws"/>
+                               <searcher idref="GcpSearcher" deploy:cloud="gcp"/>
+                               <searcher idref="OtherSearcher"/>
+                            </chain>
+                          </search>
+                        </container>
+                        "</services>""";
+
+        String expected =
+                """
+                        <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+                        <services xmlns:deploy="vespa" xmlns:preprocess="?" version="1.0">
+                        <container id="stateless" version="1.0">
+                          <search>
+                            <searcher id="AwsSearcher" class="ai.vespa.AwsSearcher" bundle="foo"/>
+                            <searcher id="GcpSearcher" class="ai.vespa.GcpSearcher" bundle="foo"/>
+                            <searcher id="OtherSearcher" class="ai.vespa.OtherSearcher" bundle="foo"/>
+                            <chain id="default" inherits="vespa">
+                              <searcher idref="%s"/>
+                              <searcher idref="OtherSearcher"/>
+                            </chain>
+                          </search>
+                        </container>
+                        "</services>""";
+
+        assertOverride(input, "aws", expected.formatted("AwsSearcher"));
+        assertOverride(input, "gcp", expected.formatted("GcpSearcher"));
+    }
+
     private void assertOverride(Environment environment, RegionName region, String expected) throws TransformerException {
         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, Cloud.defaultCloud().name(), Tags.empty()).process(inputDoc);
-        TestBase.assertDocument(expected, newDoc);
+    private void assertOverride(String input, Environment environment, RegionName region, String expected) {
+        assertOverride(input, environment, region, Cloud.defaultCloud().name(), expected);
+    }
+
+    private void assertOverride(String input, String cloudName, String expected) {
+        assertOverride(input, Environment.defaultEnvironment(), RegionName.defaultName(), CloudName.from(cloudName), expected);
+    }
+
+    private void assertOverride(String input, Environment environment, RegionName region, CloudName cloudName, String expected) {
+        var inputDoc = Xml.getDocument(new StringReader(input));
+        try {
+            var newDoc = new OverrideProcessor(InstanceName.from("default"), environment, region, cloudName, Tags.empty())
+                    .process(inputDoc);
+            TestBase.assertDocument(expected, newDoc);
+        } catch (TransformerException e) {
+            throw new RuntimeException(e);
+        }
     }
 
 }